From bdf7d3e8c77b60f4b5ffad2458cdb4c77bb63d09 Mon Sep 17 00:00:00 2001 From: Davide Silvestri <davide@baserow.io> Date: Mon, 10 Feb 2025 16:49:10 +0000 Subject: [PATCH] Resolve "The notification mode settings is not shown correctly if the row is not in the view" --- .../contrib/database/api/rows/serializers.py | 63 ++++++++++----- .../contrib/database/api/rows/views.py | 41 ++++++++-- .../contrib/database/api/tokens/errors.py | 12 ++- .../database/api/views/gallery/views.py | 4 +- .../contrib/database/api/views/grid/views.py | 4 +- .../contrib/database/api/views/utils.py | 16 ++++ .../contrib/database/tokens/exceptions.py | 6 ++ .../contrib/database/tokens/handler.py | 76 +++++++++++-------- .../database/api/rows/test_row_serializers.py | 2 +- .../database/api/tokens/test_token_views.py | 28 +++++++ ...ettings_is_not_shown_correctly_if_the.json | 7 ++ .../api/views/calendar/serializers.py | 4 +- .../api/views/kanban/serializers.py | 4 +- .../api/views/timeline/views.py | 26 +++++-- .../baserow_premium/row_comments/models.py | 5 +- .../row_comments/row_metadata_types.py | 6 +- .../test_row_comments_notification_types.py | 41 ++++++++++ .../baserow_premium/store/row_comments.js | 15 +++- .../baserow_premium/store/view/calendar.js | 17 +---- .../baserow_premium/store/view/kanban.js | 17 +---- .../baserow_premium/store/view/timeline.js | 4 +- web-frontend/modules/database/services/row.js | 9 ++- .../modules/database/store/rowModal.js | 17 +++++ .../database/store/view/bufferedRows.js | 25 +++--- .../modules/database/store/view/gallery.js | 3 +- .../modules/database/store/view/grid.js | 18 ++--- web-frontend/modules/database/utils/row.js | 29 +++++++ 27 files changed, 361 insertions(+), 138 deletions(-) create mode 100644 changelog/entries/unreleased/bug/3411_the_notification_mode_settings_is_not_shown_correctly_if_the.json diff --git a/backend/src/baserow/contrib/database/api/rows/serializers.py b/backend/src/baserow/contrib/database/api/rows/serializers.py index 178bd6bed..2c63a5db7 100644 --- a/backend/src/baserow/contrib/database/api/rows/serializers.py +++ b/backend/src/baserow/contrib/database/api/rows/serializers.py @@ -13,7 +13,10 @@ from baserow.api.utils import get_serializer_class from baserow.contrib.database.api.rows.fields import UserFieldNamesField from baserow.contrib.database.fields.registries import field_type_registry from baserow.contrib.database.rows.models import RowHistory -from baserow.contrib.database.rows.registries import row_metadata_registry +from baserow.contrib.database.rows.registries import ( + RowMetadataType, + row_metadata_registry, +) class RowSerializer(serializers.ModelSerializer): @@ -173,18 +176,18 @@ class BatchDeleteRowsSerializer(serializers.Serializer): ) -def get_example_row_serializer_class(example_type="get", user_field_names=False): +def get_example_row_serializer_class( + example_type: str = "get", + user_field_names: bool = False, +) -> serializers.Serializer: """ Generates a serializer containing a field for each field type. It is only used for example purposes in the openapi documentation. :param example_type: Sets various parameters. Can be get, post, patch. - :type example_type: str :param user_field_names: Whether this example serializer help text should indicate the fields names can be switched using the `user_field_names` GET parameter. - :type user_field_names: bool :return: Generated serializer containing a field for each field type. - :rtype: Serializer """ config = { @@ -192,24 +195,28 @@ def get_example_row_serializer_class(example_type="get", user_field_names=False) "class_name": "ExampleRowResponseSerializer", "add_id": True, "add_order": True, + "add_metadata": True, "read_only_fields": True, }, "post": { "class_name": "ExampleRowRequestSerializer", "add_id": False, "add_order": False, + "add_metadata": False, "read_only_fields": False, }, "patch": { "class_name": "ExampleUpdateRowRequestSerializer", "add_id": False, "add_order": False, + "add_metadata": False, "read_only_fields": False, }, "patch_batch": { "class_name": "ExampleBatchUpdateRowRequestSerializer", "add_id": True, "add_order": False, + "add_metadata": False, "read_only_fields": False, }, } @@ -217,6 +224,7 @@ def get_example_row_serializer_class(example_type="get", user_field_names=False) class_name = config[example_type]["class_name"] add_id = config[example_type]["add_id"] add_order = config[example_type]["add_order"] + add_metadata = config[example_type]["add_metadata"] add_readonly_fields = config[example_type]["read_only_fields"] is_response_example = add_readonly_fields @@ -245,6 +253,16 @@ def get_example_row_serializer_class(example_type="get", user_field_names=False) "last.", ) + if add_metadata: + metadata_serializer = get_example_row_metadata_serializer() + fields["metadata"] = metadata_serializer( + required=False, + help_text=( + "Additional metadata for the row, if `include=metadata' " + "is provided as query parameter." + ), + ) + field_types = field_type_registry.registry.values() if len(field_types) == 0: @@ -285,30 +303,37 @@ def get_example_row_serializer_class(example_type="get", user_field_names=False) return class_object -def get_example_row_metadata_field_serializer(): +def get_example_row_metadata_serializer() -> serializers.Serializer: """ - Generates a serializer containing a field for each row metadata type which + Generates a serializer containing a field for each row metadata type which represents the metadata for a single row. - It is only used for example purposes in the openapi documentation. - - :return: Generated serializer for a single rows metadata - :rtype: Serializer + :return: A serializer containing a field for each row metadata type. """ - metadata_types = row_metadata_registry.get_all() - - if len(metadata_types) == 0: - return None + metadata_types: List[RowMetadataType] = row_metadata_registry.get_all() fields = {} for metadata_type in metadata_types: fields[metadata_type.type] = metadata_type.get_example_serializer_field() - per_row_serializer = type( - "RowMetadataSerializer", (serializers.Serializer,), fields - )() + return type("RowMetadataSerializer", (serializers.Serializer,), fields) + + +def get_example_multiple_rows_metadata_serializer() -> serializers.Serializer: + """ + Generates a serializer containing a field for each row metadata type which + represents the metadata for a single row. The single row serializer is then + nested in a dictionary where the key is the row id and the value is the single row + metadata serializer. + It is only used for example purposes in the openapi documentation. + + :return: A serializer containing a dictionary of row id to row metadata. + """ + + per_row_serializer = get_example_row_metadata_serializer() + return serializers.DictField( - child=per_row_serializer, + child=per_row_serializer(), required=False, help_text="An object keyed by row id with a value being an object containing " "additional metadata about that row. A row might not have metadata and will " diff --git a/backend/src/baserow/contrib/database/api/rows/views.py b/backend/src/baserow/contrib/database/api/rows/views.py index dd5e237f7..e198d4cf9 100644 --- a/backend/src/baserow/contrib/database/api/rows/views.py +++ b/backend/src/baserow/contrib/database/api/rows/views.py @@ -14,6 +14,7 @@ from rest_framework.status import HTTP_204_NO_CONTENT from rest_framework.views import APIView from baserow.api.decorators import ( + allowed_includes, map_exceptions, require_request_data_type, validate_body, @@ -51,7 +52,10 @@ from baserow.contrib.database.api.rows.exceptions import InvalidJoinParameterExc from baserow.contrib.database.api.rows.serializers import GetRowAdjacentSerializer from baserow.contrib.database.api.tables.errors import ERROR_TABLE_DOES_NOT_EXIST from baserow.contrib.database.api.tokens.authentications import TokenAuthentication -from baserow.contrib.database.api.tokens.errors import ERROR_NO_PERMISSION_TO_TABLE +from baserow.contrib.database.api.tokens.errors import ( + ERROR_CANNOT_INCLUDE_ROW_METADATA, + ERROR_NO_PERMISSION_TO_TABLE, +) from baserow.contrib.database.api.utils import ( extract_link_row_joins_from_request, extract_send_webhook_events_from_params, @@ -63,6 +67,7 @@ from baserow.contrib.database.api.views.errors import ( ERROR_VIEW_FILTER_TYPE_DOES_NOT_EXIST, ERROR_VIEW_FILTER_TYPE_UNSUPPORTED_FIELD, ) +from baserow.contrib.database.api.views.utils import serialize_single_row_metadata from baserow.contrib.database.fields.exceptions import ( FieldDoesNotExist, FilterFieldNotFound, @@ -100,7 +105,10 @@ from baserow.contrib.database.table.operations import ( ListRowNamesDatabaseTableOperationType, ListRowsDatabaseTableOperationType, ) -from baserow.contrib.database.tokens.exceptions import NoPermissionToTable +from baserow.contrib.database.tokens.exceptions import ( + NoPermissionToTable, + TokenCannotIncludeRowMetadata, +) from baserow.contrib.database.tokens.handler import TokenHandler from baserow.contrib.database.views.exceptions import ( ViewDoesNotExist, @@ -702,6 +710,16 @@ class RowView(APIView): "field names (e.g., field_123)." ), ), + OpenApiParameter( + name="include", + location=OpenApiParameter.QUERY, + type=OpenApiTypes.STR, + description=( + "Optionally include row's `metadata` in the response. " + "The `metadata` object includes extra row specific data like the " + "'row_comments_notification_mode' settings, if available." + ), + ), ], tags=["Database table rows"], operation_id="get_database_table_row", @@ -735,9 +753,11 @@ class RowView(APIView): TableDoesNotExist: ERROR_TABLE_DOES_NOT_EXIST, RowDoesNotExist: ERROR_ROW_DOES_NOT_EXIST, NoPermissionToTable: ERROR_NO_PERMISSION_TO_TABLE, + TokenCannotIncludeRowMetadata: ERROR_CANNOT_INCLUDE_ROW_METADATA, } ) - def get(self, request, table_id, row_id): + @allowed_includes("metadata") + def get(self, request, table_id, row_id, metadata): """ Responds with a serializer version of the row related to the provided row_id and table_id. @@ -745,7 +765,13 @@ class RowView(APIView): table = TableHandler().get_table(table_id) - TokenHandler().check_table_permissions(request, "read", table, False) + token_handler = TokenHandler() + db_token = token_handler.get_token_from_request(request) + if db_token is not None: + if metadata: + raise TokenCannotIncludeRowMetadata() + token_handler.check_table_permissions(db_token, "read", table) + user_field_names = extract_user_field_names_from_params(request.GET) model = table.get_model() row = RowHandler().get_row(request.user, table, row_id, model) @@ -753,10 +779,15 @@ class RowView(APIView): model, RowSerializer, is_response=True, user_field_names=user_field_names ) serializer = serializer_class(row) + response_data = serializer.data + + if metadata: + row_metadata = serialize_single_row_metadata(request.user, row) + response_data["metadata"] = row_metadata rows_loaded.send(sender=self, table=table) - return Response(serializer.data) + return Response(response_data) @extend_schema( parameters=[ diff --git a/backend/src/baserow/contrib/database/api/tokens/errors.py b/backend/src/baserow/contrib/database/api/tokens/errors.py index ba4166559..6cfe92b9f 100644 --- a/backend/src/baserow/contrib/database/api/tokens/errors.py +++ b/backend/src/baserow/contrib/database/api/tokens/errors.py @@ -1,4 +1,8 @@ -from rest_framework.status import HTTP_401_UNAUTHORIZED, HTTP_404_NOT_FOUND +from rest_framework.status import ( + HTTP_400_BAD_REQUEST, + HTTP_401_UNAUTHORIZED, + HTTP_404_NOT_FOUND, +) ERROR_TOKEN_DOES_NOT_EXIST = ( "ERROR_TOKEN_DOES_NOT_EXIST", @@ -10,3 +14,9 @@ ERROR_NO_PERMISSION_TO_TABLE = ( HTTP_401_UNAUTHORIZED, "The token does not have permissions to the table.", ) + +ERROR_CANNOT_INCLUDE_ROW_METADATA = ( + "ERROR_CANNOT_INCLUDE_ROW_METADATA", + HTTP_400_BAD_REQUEST, + "The token cannot include row metadata.", +) diff --git a/backend/src/baserow/contrib/database/api/views/gallery/views.py b/backend/src/baserow/contrib/database/api/views/gallery/views.py index 3d59afece..091816676 100644 --- a/backend/src/baserow/contrib/database/api/views/gallery/views.py +++ b/backend/src/baserow/contrib/database/api/views/gallery/views.py @@ -29,7 +29,7 @@ from baserow.contrib.database.api.fields.errors import ( ) from baserow.contrib.database.api.rows.serializers import ( RowSerializer, - get_example_row_metadata_field_serializer, + get_example_multiple_rows_metadata_serializer, get_example_row_serializer_class, get_row_serializer_class, ) @@ -150,7 +150,7 @@ class GalleryViewView(APIView): serializer_class=GalleryViewFieldOptionsSerializer, required=False, ), - "row_metadata": get_example_row_metadata_field_serializer(), + "row_metadata": get_example_multiple_rows_metadata_serializer(), }, serializer_name="PaginationSerializerWithGalleryViewFieldOptions", ), diff --git a/backend/src/baserow/contrib/database/api/views/grid/views.py b/backend/src/baserow/contrib/database/api/views/grid/views.py index a786ce997..c373c9c21 100644 --- a/backend/src/baserow/contrib/database/api/views/grid/views.py +++ b/backend/src/baserow/contrib/database/api/views/grid/views.py @@ -39,7 +39,7 @@ from baserow.contrib.database.api.fields.errors import ( ) from baserow.contrib.database.api.rows.serializers import ( RowSerializer, - get_example_row_metadata_field_serializer, + get_example_multiple_rows_metadata_serializer, get_example_row_serializer_class, get_row_serializer_class, ) @@ -172,7 +172,7 @@ class GridViewView(APIView): "field_options": FieldOptionsField( serializer_class=GridViewFieldOptionsSerializer, required=False ), - "row_metadata": get_example_row_metadata_field_serializer(), + "row_metadata": get_example_multiple_rows_metadata_serializer(), }, serializer_name="PaginationSerializerWithGridViewFieldOptions", ), diff --git a/backend/src/baserow/contrib/database/api/views/utils.py b/backend/src/baserow/contrib/database/api/views/utils.py index 5a0eb9db4..7aad04897 100644 --- a/backend/src/baserow/contrib/database/api/views/utils.py +++ b/backend/src/baserow/contrib/database/api/views/utils.py @@ -221,6 +221,22 @@ def serialize_rows_metadata( ) +def serialize_single_row_metadata( + user: AbstractUser, row: GeneratedTableModel +) -> Dict[str, Any]: + """ + Serializes the metadata for the provided row. + + :param user: The user to serialize the metadata for. + :param row: The row to serialize the metadata for. + :return: The serialized metadata for the provided rows. + """ + + return row_metadata_registry.generate_and_merge_metadata_for_row( + user, row.baserow_table, row.id + ) + + def serialize_group_by_fields_metadata( queryset: QuerySet[GeneratedTableModel], group_by_fields: List[Field], diff --git a/backend/src/baserow/contrib/database/tokens/exceptions.py b/backend/src/baserow/contrib/database/tokens/exceptions.py index f7426a3fc..31d439e6d 100644 --- a/backend/src/baserow/contrib/database/tokens/exceptions.py +++ b/backend/src/baserow/contrib/database/tokens/exceptions.py @@ -16,3 +16,9 @@ class NoPermissionToTable(Exception): """ Raised when a token does not have permissions to perform an operation for a table. """ + + +class TokenCannotIncludeRowMetadata(Exception): + """ + Raised if requested to include a row's metadata via token. + """ diff --git a/backend/src/baserow/contrib/database/tokens/handler.py b/backend/src/baserow/contrib/database/tokens/handler.py index 14cd4e6d2..62fb46b14 100644 --- a/backend/src/baserow/contrib/database/tokens/handler.py +++ b/backend/src/baserow/contrib/database/tokens/handler.py @@ -395,8 +395,41 @@ class TokenHandler: # At least one must be True return any([v is True for v in token_permission.values()]) + def get_token_from_request(self, request: Request) -> Token | None: + """ + Extracts the token from the request. If the token is not found then None is + returned. + + :param request: The request from which the token must be extracted. + :return: The extracted token or None if it could not be found. + """ + + return getattr(request, "user_token", None) + + def raise_table_permission_error(self, table: Table, type_name: str | list[str]): + """ + Raises an exception indicating that the provided token does not have permission + to the provided table. Used to raise a consistent exception when the token does + not have permission to the table. + + :param table: The table object to check the permissions for. + :param type_name: The CRUD operation, create, read, update or delete to check + the permissions for. Can be a list if you want to check at least one of the + listed operation. + :raises NoPermissionToTable: Raised when the token does not have permissions to + """ + + raise NoPermissionToTable( + f"The provided token does not have {type_name} " + f"permissions to table {table.id}." + ) + def check_table_permissions( - self, request_or_token, type_name, table, force_check=False + self, + request_or_token: Request | Token, + type_name: str | list[str], + table: Table, + force_check=False, ): """ Instead of returning True or False, this method will raise an exception if the @@ -404,52 +437,35 @@ class TokenHandler: :param request_or_token: If a request is provided then the token will be extracted from the request. Otherwise a token object is expected. - :type request_or_token: Request or Token :param type_name: The CRUD operation, create, read, update or delete to check the permissions for. Can be a list if you want to check at least one of the listed operation. - :type type_name: str | list :param table: The table object to check the permissions for. - :type table: Table :param force_check: Indicates if a NoPermissionToTable exception must be raised when the token could not be extracted from the request. This can be useful if a view accepts multiple types of authentication. - :type force_check: bool :raises ValueError: when neither a Token or HttpRequest is provided. :raises NoPermissionToTable: when the token does not have permissions to the table. """ token = None - - if not isinstance(request_or_token, Request) and not isinstance( - request_or_token, Token - ): - raise ValueError( - "The provided instance should be a HttpRequest or Token " "object." - ) - - if isinstance(request_or_token, Request) and hasattr( - request_or_token, "user_token" - ): - token = request_or_token.user_token - if isinstance(request_or_token, Token): token = request_or_token - - if not token and not force_check: - return - - if ( - not token - and force_check - or not TokenHandler().has_table_permission(token, type_name, table) - ): - raise NoPermissionToTable( - f"The provided token does not have {type_name} " - f"permissions to table {table.id}." + elif isinstance(request_or_token, Request): + token = self.get_token_from_request(request_or_token) + else: + raise ValueError( + "The provided instance should be a HttpRequest or Token object." ) + should_check_permissions = token is not None or force_check + has_table_permissions = token is not None and self.has_table_permission( + token, type_name, table + ) + if should_check_permissions and not has_table_permissions: + self.raise_table_permission_error(table, type_name) + def delete_token(self, user, token): """ Deletes an existing token diff --git a/backend/tests/baserow/contrib/database/api/rows/test_row_serializers.py b/backend/tests/baserow/contrib/database/api/rows/test_row_serializers.py index 271fb44b7..f31cf4365 100644 --- a/backend/tests/baserow/contrib/database/api/rows/test_row_serializers.py +++ b/backend/tests/baserow/contrib/database/api/rows/test_row_serializers.py @@ -233,7 +233,7 @@ def test_get_example_row_serializer_class(): num_readonly_fields = len( [ftype for ftype in field_type_registry.registry.values() if ftype.read_only] ) - num_extra_response_fields = 2 # id + order + num_extra_response_fields = 3 # id + order + metadata num_difference = num_readonly_fields + num_extra_response_fields assert num_request_fields == num_response_fields - num_difference diff --git a/backend/tests/baserow/contrib/database/api/tokens/test_token_views.py b/backend/tests/baserow/contrib/database/api/tokens/test_token_views.py index 5344192cb..82989c3e8 100644 --- a/backend/tests/baserow/contrib/database/api/tokens/test_token_views.py +++ b/backend/tests/baserow/contrib/database/api/tokens/test_token_views.py @@ -677,3 +677,31 @@ def test_check_token(api_client, data_fixture): ) assert response.status_code == HTTP_200_OK assert response.json() == {"token": "OK"} + + +@pytest.mark.django_db +def test_cannot_get_row_metadata_from_token(api_client, data_fixture): + user = data_fixture.create_user() + workspace = data_fixture.create_workspace(user=user) + token = data_fixture.create_token(user=user, workspace=workspace) + + database = data_fixture.create_database_application(workspace=workspace) + table = data_fixture.create_database_table(user=user, database=database) + row = table.get_model().objects.create() + + def get_row_from_api_with_metadata(): + return api_client.get( + reverse( + "api:database:rows:item", + kwargs={"table_id": table.id, "row_id": row.id}, + ) + + "?include=metadata", + HTTP_AUTHORIZATION=f"Token {token.key}", + ) + + response = get_row_from_api_with_metadata() + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json() == { + "error": "ERROR_CANNOT_INCLUDE_ROW_METADATA", + "detail": "The token cannot include row metadata.", + } diff --git a/changelog/entries/unreleased/bug/3411_the_notification_mode_settings_is_not_shown_correctly_if_the.json b/changelog/entries/unreleased/bug/3411_the_notification_mode_settings_is_not_shown_correctly_if_the.json new file mode 100644 index 000000000..24f3a1356 --- /dev/null +++ b/changelog/entries/unreleased/bug/3411_the_notification_mode_settings_is_not_shown_correctly_if_the.json @@ -0,0 +1,7 @@ +{ + "type": "bug", + "message": "Fixed an issue where the notification mode settings were not displayed correctly if the row was not in the view.", + "issue_number": 3411, + "bullet_points": [], + "created_at": "2025-02-05" +} \ No newline at end of file diff --git a/premium/backend/src/baserow_premium/api/views/calendar/serializers.py b/premium/backend/src/baserow_premium/api/views/calendar/serializers.py index ce81516b8..1c252ffb8 100644 --- a/premium/backend/src/baserow_premium/api/views/calendar/serializers.py +++ b/premium/backend/src/baserow_premium/api/views/calendar/serializers.py @@ -4,7 +4,7 @@ from baserow_premium.views.models import CalendarViewFieldOptions from rest_framework import serializers from baserow.contrib.database.api.rows.serializers import ( - get_example_row_metadata_field_serializer, + get_example_multiple_rows_metadata_serializer, get_example_row_serializer_class, ) from baserow.contrib.database.search.handler import ALL_SEARCH_MODES @@ -75,6 +75,6 @@ def get_calendar_view_example_response_serializer(): "field_options": serializers.ListSerializer( child=CalendarViewFieldOptionsSerializer() ), - "row_metadata": get_example_row_metadata_field_serializer(), + "row_metadata": get_example_multiple_rows_metadata_serializer(), }, ) diff --git a/premium/backend/src/baserow_premium/api/views/kanban/serializers.py b/premium/backend/src/baserow_premium/api/views/kanban/serializers.py index 11a79569d..d0592e649 100644 --- a/premium/backend/src/baserow_premium/api/views/kanban/serializers.py +++ b/premium/backend/src/baserow_premium/api/views/kanban/serializers.py @@ -2,7 +2,7 @@ from baserow_premium.views.models import KanbanViewFieldOptions from rest_framework import serializers from baserow.contrib.database.api.rows.serializers import ( - get_example_row_metadata_field_serializer, + get_example_multiple_rows_metadata_serializer, get_example_row_serializer_class, ) @@ -37,6 +37,6 @@ def get_kanban_view_example_response_serializer(): "field_options": serializers.ListSerializer( child=KanbanViewFieldOptionsSerializer() ), - "row_metadata": get_example_row_metadata_field_serializer(), + "row_metadata": get_example_multiple_rows_metadata_serializer(), }, ) diff --git a/premium/backend/src/baserow_premium/api/views/timeline/views.py b/premium/backend/src/baserow_premium/api/views/timeline/views.py index 0231421ea..5950f7c00 100644 --- a/premium/backend/src/baserow_premium/api/views/timeline/views.py +++ b/premium/backend/src/baserow_premium/api/views/timeline/views.py @@ -44,6 +44,7 @@ from baserow.contrib.database.api.fields.errors import ( ERROR_ORDER_BY_FIELD_NOT_POSSIBLE, ) from baserow.contrib.database.api.rows.serializers import ( + get_example_multiple_rows_metadata_serializer, get_example_row_serializer_class, ) from baserow.contrib.database.api.utils import get_include_exclude_field_ids @@ -56,6 +57,7 @@ from baserow.contrib.database.api.views.serializers import FieldOptionsField from baserow.contrib.database.api.views.utils import ( get_public_view_authorization_token, paginate_and_serialize_queryset, + serialize_rows_metadata, serialize_view_field_options, ) from baserow.contrib.database.fields.exceptions import ( @@ -105,12 +107,12 @@ class TimelineViewView(APIView): location=OpenApiParameter.QUERY, type=OpenApiTypes.STR, description=( - "A comma separated list allowing the values of " - "`field_options` which will add the object/objects with the " - "same " + "A comma separated list allowing the values of `field_options` and " + "`row_metadata` which will add the object/objects with the same " "name to the response if included. The `field_options` object " - "contains user defined view settings for each field. For " - "example the field's width is included in here." + "contains user defined view settings for each field. For example " + "the field's width is included in here. The `row_metadata` object" + " includes extra row specific data on a per row basis." ), ), ONLY_COUNT_API_PARAM, @@ -152,6 +154,7 @@ class TimelineViewView(APIView): serializer_class=TimelineViewFieldOptionsSerializer, required=False, ), + "row_metadata": get_example_multiple_rows_metadata_serializer(), }, serializer_name="PaginationSerializerWithTimelineViewFieldOptions", ), @@ -185,9 +188,9 @@ class TimelineViewView(APIView): IncompatibleField: ERROR_INCOMPATIBLE_FIELD, } ) - @allowed_includes("field_options") + @allowed_includes("field_options", "row_metadata") @validate_query_parameters(SearchQueryParamSerializer, return_validated=True) - def get(self, request, view_id, field_options, query_params): + def get(self, request, view_id, field_options, row_metadata, query_params): """ Lists all the rows of a timeline view, paginated either by a page or offset/limit. If the limit get parameter is provided the limit/offset pagination @@ -238,11 +241,18 @@ class TimelineViewView(APIView): if "count" in request.GET: return Response({"count": queryset.count()}) - response, _, _ = paginate_and_serialize_queryset(queryset, request, field_ids) + response, page, _ = paginate_and_serialize_queryset( + queryset, request, field_ids + ) if field_options: response.data.update(**serialize_view_field_options(view, model)) + if row_metadata: + response.data.update( + row_metadata=serialize_rows_metadata(request.user, view, page) + ) + view_loaded.send( sender=self, table=view.table, diff --git a/premium/backend/src/baserow_premium/row_comments/models.py b/premium/backend/src/baserow_premium/row_comments/models.py index a832fe806..0b765a181 100644 --- a/premium/backend/src/baserow_premium/row_comments/models.py +++ b/premium/backend/src/baserow_premium/row_comments/models.py @@ -63,6 +63,9 @@ ALL_ROW_COMMENT_NOTIFICATION_MODES = [ ] +ROW_COMMENT_NOTIFICATION_DEFAULT_MODE = RowCommentsNotificationModes.MODE_ONLY_MENTIONS + + class RowCommentsNotificationMode(CreatedAndUpdatedOnMixin, models.Model): """ A many to many relationship between users and table rows to keep track of @@ -91,7 +94,7 @@ class RowCommentsNotificationMode(CreatedAndUpdatedOnMixin, models.Model): (RowCommentsNotificationModes.MODE_ALL_COMMENTS, "All comments"), (RowCommentsNotificationModes.MODE_ONLY_MENTIONS, "Only mentions"), ), - default=RowCommentsNotificationModes.MODE_ONLY_MENTIONS, + default=ROW_COMMENT_NOTIFICATION_DEFAULT_MODE, help_text="The notification mode for this user and row.", ) diff --git a/premium/backend/src/baserow_premium/row_comments/row_metadata_types.py b/premium/backend/src/baserow_premium/row_comments/row_metadata_types.py index 6320fb182..2b18ab90f 100644 --- a/premium/backend/src/baserow_premium/row_comments/row_metadata_types.py +++ b/premium/backend/src/baserow_premium/row_comments/row_metadata_types.py @@ -4,9 +4,9 @@ from django.db.models import Count from baserow_premium.row_comments.models import ( ALL_ROW_COMMENT_NOTIFICATION_MODES, + ROW_COMMENT_NOTIFICATION_DEFAULT_MODE, RowComment, RowCommentsNotificationMode, - RowCommentsNotificationModes, ) from rest_framework import serializers from rest_framework.fields import Field @@ -38,7 +38,7 @@ class RowCommentCountMetadataType(RowMetadataType): ) -class RowCommentsNotificationModeMetadataType(RowCommentCountMetadataType): +class RowCommentsNotificationModeMetadataType(RowMetadataType): type = "row_comments_notification_mode" def generate_metadata_for_rows( @@ -61,7 +61,7 @@ class RowCommentsNotificationModeMetadataType(RowCommentCountMetadataType): table=table, row_id__in=row_ids, ) - .exclude(mode=RowCommentsNotificationModes.MODE_ONLY_MENTIONS) + .exclude(mode=ROW_COMMENT_NOTIFICATION_DEFAULT_MODE) .values("row_id", "mode") ) } diff --git a/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_notification_types.py b/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_notification_types.py index b92cc942d..ad5ceff0c 100644 --- a/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_notification_types.py +++ b/premium/backend/tests/baserow_premium_tests/row_comments/test_row_comments_notification_types.py @@ -648,3 +648,44 @@ def test_row_comment_notification_type_can_be_rendered_as_email( ) == "Test comment" ) + + +@override_settings(DEBUG=True) +@pytest.mark.django_db +def test_can_get_row_comment_notification_mode_from_row_metadata( + api_client, premium_data_fixture +): + user, token = premium_data_fixture.create_user_and_token( + has_active_premium_license=True + ) + table = premium_data_fixture.create_database_table(user=user) + row = table.get_model().objects.create() + + def get_row_from_api_with_metadata(): + return api_client.get( + reverse( + "api:database:rows:item", + kwargs={"table_id": table.id, "row_id": row.id}, + ) + + "?include=metadata", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + response = get_row_from_api_with_metadata() + assert response.status_code == HTTP_200_OK + assert response.json()["metadata"] == {} + + response = api_client.put( + reverse( + "api:premium:row_comments:notification_mode", + kwargs={"row_id": row.id, "table_id": table.id}, + ), + {"mode": "all"}, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + assert response.status_code == HTTP_204_NO_CONTENT + + response = get_row_from_api_with_metadata() + assert response.status_code == HTTP_200_OK + assert response.json()["metadata"]["row_comments_notification_mode"] == "all" diff --git a/premium/web-frontend/modules/baserow_premium/store/row_comments.js b/premium/web-frontend/modules/baserow_premium/store/row_comments.js index be6f53b1b..54aba1483 100644 --- a/premium/web-frontend/modules/baserow_premium/store/row_comments.js +++ b/premium/web-frontend/modules/baserow_premium/store/row_comments.js @@ -326,13 +326,22 @@ export const actions = { /** * Forcefully update the notification mode for a comments on a row. */ - async forceUpdateNotificationMode({ commit }, { tableId, rowId, mode }) { + async forceUpdateNotificationMode({ dispatch }, { tableId, rowId, mode }) { + const updateFunction = () => mode.toString() + const rowMetadataType = 'row_comments_notification_mode' await updateRowMetadataInViews( this, tableId, rowId, - 'row_comments_notification_mode', - () => mode.toString() + rowMetadataType, + updateFunction + ) + // Let's also make sure the local copy of the row edit modal is updated in case the + // row is not in any view buffer. + dispatch( + 'rowModal/updateRowMetadata', + { rowId, rowMetadataType, updateFunction }, + { root: true } ) }, } diff --git a/premium/web-frontend/modules/baserow_premium/store/view/calendar.js b/premium/web-frontend/modules/baserow_premium/store/view/calendar.js index 5166604eb..ab5491567 100644 --- a/premium/web-frontend/modules/baserow_premium/store/view/calendar.js +++ b/premium/web-frontend/modules/baserow_premium/store/view/calendar.js @@ -21,12 +21,14 @@ import { extractRowReadOnlyValues, prepareNewOldAndUpdateRequestValues, prepareRowForRequest, + updateRowMetadataType, + getRowMetadata, } from '@baserow/modules/database/utils/row' import { getDefaultSearchModeFromEnv } from '@baserow/modules/database/utils/search' export function populateRow(row, metadata = {}) { row._ = { - metadata, + metadata: getRowMetadata(row, metadata), // Whether the row should be displayed based on the current activeSearchTerm term. matchSearch: true, // Contains the specific field ids which match the activeSearchTerm term. @@ -212,18 +214,7 @@ export const mutations = { Object.assign(row, values) }, UPDATE_ROW_METADATA(state, { row, rowMetadataType, updateFunction }) { - const currentValue = row._.metadata[rowMetadataType] - const newValue = updateFunction(currentValue) - - if ( - !Object.prototype.hasOwnProperty.call(row._.metadata, rowMetadataType) - ) { - const metaDataCopy = clone(row._.metadata) - metaDataCopy[rowMetadataType] = newValue - Vue.set(row._, 'metadata', metaDataCopy) - } else { - Vue.set(row._.metadata, rowMetadataType, newValue) - } + updateRowMetadataType(row, rowMetadataType, updateFunction) }, SET_ADHOC_FILTERING(state, adhocFiltering) { state.adhocFiltering = adhocFiltering diff --git a/premium/web-frontend/modules/baserow_premium/store/view/kanban.js b/premium/web-frontend/modules/baserow_premium/store/view/kanban.js index 40a68f3d8..28e33a55c 100644 --- a/premium/web-frontend/modules/baserow_premium/store/view/kanban.js +++ b/premium/web-frontend/modules/baserow_premium/store/view/kanban.js @@ -17,11 +17,13 @@ import { extractRowReadOnlyValues, prepareNewOldAndUpdateRequestValues, prepareRowForRequest, + updateRowMetadataType, + getRowMetadata, } from '@baserow/modules/database/utils/row' export function populateRow(row, metadata = {}) { row._ = { - metadata, + metadata: getRowMetadata(row, metadata), dragging: false, } return row @@ -180,18 +182,7 @@ export const mutations = { Object.assign(row, values) }, UPDATE_ROW_METADATA(state, { row, rowMetadataType, updateFunction }) { - const currentValue = row._.metadata[rowMetadataType] - const newValue = updateFunction(currentValue) - - if ( - !Object.prototype.hasOwnProperty.call(row._.metadata, rowMetadataType) - ) { - const metaDataCopy = clone(row._.metadata) - metaDataCopy[rowMetadataType] = newValue - Vue.set(row._, 'metadata', metaDataCopy) - } else { - Vue.set(row._.metadata, rowMetadataType, newValue) - } + updateRowMetadataType(row, rowMetadataType, updateFunction) }, SET_ADHOC_FILTERING(state, adhocFiltering) { state.adhocFiltering = adhocFiltering diff --git a/premium/web-frontend/modules/baserow_premium/store/view/timeline.js b/premium/web-frontend/modules/baserow_premium/store/view/timeline.js index d5edfa4e5..9dd759278 100644 --- a/premium/web-frontend/modules/baserow_premium/store/view/timeline.js +++ b/premium/web-frontend/modules/baserow_premium/store/view/timeline.js @@ -1,9 +1,10 @@ import bufferedRows from '@baserow/modules/database/store/view/bufferedRows' import TimelineService from '@baserow_premium/services/views/timeline' +import { getRowMetadata } from '@baserow/modules/database/utils/row' export function populateRow(row, metadata = {}) { row._ = { - metadata, + metadata: getRowMetadata(row, metadata), } return row } @@ -32,7 +33,6 @@ export const actions = { fields, initialRowArguments: { includeFieldOptions: true, - includeRowMetadata: false, }, adhocFiltering, adhocSorting, diff --git a/web-frontend/modules/database/services/row.js b/web-frontend/modules/database/services/row.js index c6babbad1..ffc8c3bd9 100644 --- a/web-frontend/modules/database/services/row.js +++ b/web-frontend/modules/database/services/row.js @@ -7,8 +7,13 @@ const groupGetNameCalls = callGrouper(GRACE_DELAY) export default (client) => { return { - get(tableId, rowId) { - return client.get(`/database/rows/table/${tableId}/${rowId}/`) + get(tableId, rowId, includeMetadata = true) { + const searchParams = {} + if (includeMetadata) { + searchParams.include = 'metadata' + } + const params = new URLSearchParams(searchParams).toString() + return client.get(`/database/rows/table/${tableId}/${rowId}/?${params}`) }, fetchAll({ tableId, diff --git a/web-frontend/modules/database/store/rowModal.js b/web-frontend/modules/database/store/rowModal.js index 071f630ec..d625b9d72 100644 --- a/web-frontend/modules/database/store/rowModal.js +++ b/web-frontend/modules/database/store/rowModal.js @@ -1,3 +1,5 @@ +import { updateRowMetadataType } from '@baserow/modules/database/utils/row' + /** * This store exists to always keep a copy of the row that's being edited via the * row edit modal. It sometimes happen that row from the original source, where it was @@ -53,6 +55,13 @@ export const mutations = { UPDATE_ROW(state, { componentId, row }) { Object.assign(state.rows[componentId].row, row) }, + UPDATE_ROW_METADATA(state, { rowId, rowMetadataType, updateFunction }) { + Object.values(state.rows) + .filter((data) => data.row.id === rowId) + .forEach((data) => + updateRowMetadataType(data.row, rowMetadataType, updateFunction) + ) + }, } export const actions = { @@ -102,6 +111,14 @@ export const actions = { } }) }, + /** + * If a row is open in the modal but it's not present in the buffer, we need to + * manually update the metadata of the row. This is used for example to update the + * notification_mode setting of a row. + */ + updateRowMetadata({ commit }, { rowId, rowMetadataType, updateFunction }) { + commit('UPDATE_ROW_METADATA', { rowId, rowMetadataType, updateFunction }) + }, } export const getters = { diff --git a/web-frontend/modules/database/store/view/bufferedRows.js b/web-frontend/modules/database/store/view/bufferedRows.js index 10ee9a414..aacd051a3 100644 --- a/web-frontend/modules/database/store/view/bufferedRows.js +++ b/web-frontend/modules/database/store/view/bufferedRows.js @@ -16,6 +16,8 @@ import { extractRowReadOnlyValues, prepareNewOldAndUpdateRequestValues, prepareRowForRequest, + updateRowMetadataType, + getRowMetadata, } from '@baserow/modules/database/utils/row' import { getDefaultSearchModeFromEnv } from '@baserow/modules/database/utils/search' import fieldOptionsStoreFactory from '@baserow/modules/database/store/view/fieldOptions' @@ -59,13 +61,16 @@ export default ({ service, customPopulateRow, fieldOptions }) => { const populateRow = (row, metadata = {}) => { if (customPopulateRow) { - customPopulateRow(row) + customPopulateRow(row, metadata) } + + // Add the metadata to the row so that it can be used in the front-end. if (row._ == null) { row._ = { - metadata, + metadata: getRowMetadata(row, metadata), } } + // Matching rows for front-end only search is not yet properly // supported and tested in this store mixin. Only server-side search // implementation is finished. @@ -270,18 +275,7 @@ export default ({ service, customPopulateRow, fieldOptions }) => { row._.matchSearch = matchSearch }, UPDATE_ROW_METADATA(state, { row, rowMetadataType, updateFunction }) { - const currentValue = row._.metadata[rowMetadataType] - const newValue = updateFunction(currentValue) - - if ( - !Object.prototype.hasOwnProperty.call(row._.metadata, rowMetadataType) - ) { - const metaDataCopy = clone(row._.metadata) - metaDataCopy[rowMetadataType] = newValue - Vue.set(row._, 'metadata', metaDataCopy) - } else { - Vue.set(row._.metadata, rowMetadataType, newValue) - } + updateRowMetadataType(row, rowMetadataType, updateFunction) }, SET_ADHOC_FILTERING(state, adhocFiltering) { state.adhocFiltering = adhocFiltering @@ -507,7 +501,8 @@ export default ({ service, customPopulateRow, fieldOptions }) => { }) data.results.forEach((row, index) => { - rows[rangeToFetch.offset + index] = populateRow(row) + const metadata = extractRowMetadata(data, row.id) + rows[rangeToFetch.offset + index] = populateRow(row, metadata) }) if (includeFieldOptions) { diff --git a/web-frontend/modules/database/store/view/gallery.js b/web-frontend/modules/database/store/view/gallery.js index a68acbc83..47beccea6 100644 --- a/web-frontend/modules/database/store/view/gallery.js +++ b/web-frontend/modules/database/store/view/gallery.js @@ -1,9 +1,10 @@ import bufferedRows from '@baserow/modules/database/store/view/bufferedRows' import GalleryService from '@baserow/modules/database/services/view/gallery' +import { getRowMetadata } from '@baserow/modules/database/utils/row' export function populateRow(row, metadata = {}) { row._ = { - metadata, + metadata: getRowMetadata(row, metadata), dragging: false, } return row diff --git a/web-frontend/modules/database/store/view/grid.js b/web-frontend/modules/database/store/view/grid.js index 01c02328f..d31367b05 100644 --- a/web-frontend/modules/database/store/view/grid.js +++ b/web-frontend/modules/database/store/view/grid.js @@ -23,6 +23,8 @@ import { prepareRowForRequest, prepareNewOldAndUpdateRequestValues, extractRowReadOnlyValues, + updateRowMetadataType, + getRowMetadata, } from '@baserow/modules/database/utils/row' import { getDefaultSearchModeFromEnv } from '@baserow/modules/database/utils/search' import { fieldValuesAreEqualInObjects } from '@baserow/modules/database/utils/groupBy' @@ -32,7 +34,7 @@ const ORDER_STEP_BEFORE = '0.00000000000000000001' export function populateRow(row, metadata = {}) { row._ = { - metadata, + metadata: getRowMetadata(row, metadata), persistentId: uuid(), loading: false, hover: false, @@ -50,6 +52,7 @@ export function populateRow(row, metadata = {}) { selected: false, selectedFieldId: -1, } + return row } @@ -439,18 +442,7 @@ export const mutations = { row[`field_${field.id}`] = value }, UPDATE_ROW_METADATA(state, { row, rowMetadataType, updateFunction }) { - const currentValue = row._.metadata[rowMetadataType] - const newValue = updateFunction(currentValue) - - if ( - !Object.prototype.hasOwnProperty.call(row._.metadata, rowMetadataType) - ) { - const metaDataCopy = clone(row._.metadata) - metaDataCopy[rowMetadataType] = newValue - Vue.set(row._, 'metadata', metaDataCopy) - } else { - Vue.set(row._.metadata, rowMetadataType, newValue) - } + updateRowMetadataType(row, rowMetadataType, updateFunction) }, FINALIZE_ROWS_IN_BUFFER(state, { oldRows, newRows, fields }) { const stateRowsCopy = { ...state.rows } diff --git a/web-frontend/modules/database/utils/row.js b/web-frontend/modules/database/utils/row.js index 4da8b1ab1..c0cdb0540 100644 --- a/web-frontend/modules/database/utils/row.js +++ b/web-frontend/modules/database/utils/row.js @@ -1,3 +1,6 @@ +import Vue from 'vue' +import { clone } from '@baserow/modules/core/utils/object' + /** * Serializes a row to make sure that the values are according to what the API expects. * @@ -103,3 +106,29 @@ export function extractRowReadOnlyValues(row, allFields, registry) { }) return readOnlyValues } + +/** + * Call the given updateFunction with the current value of the row metadata type and + * set the new value. If the row metadata type does not exist yet, it will be + * created. + */ +export function updateRowMetadataType(row, rowMetadataType, updateFunction) { + const currentValue = row._.metadata[rowMetadataType] + const newValue = updateFunction(currentValue) + + if (!Object.prototype.hasOwnProperty.call(row._.metadata, rowMetadataType)) { + const metaDataCopy = clone(row._.metadata) + metaDataCopy[rowMetadataType] = newValue + Vue.set(row._, 'metadata', metaDataCopy) + } else { + Vue.set(row._.metadata, rowMetadataType, newValue) + } +} + +/** + * Return the metadata of a row. If the metadata does not exist yet, it will be created + * as an empty object. + */ +export function getRowMetadata(row, metadata = {}) { + return { ...metadata, ...(row.metadata || {}) } +}