From 50faf766c83d22e3dcb2478dc678364f4874a5d8 Mon Sep 17 00:00:00 2001 From: Samuel Paccoud - DINUM Date: Wed, 7 May 2025 18:48:08 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20add=20document=20path=20an?= =?UTF-8?q?d=20depth=20to=20accesses=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontend requires this information about the ancestor document to which each access is related. We make sure it does not generate more db queries and does not fetch useless and heavy fields from the document like "excerpt". --- src/backend/core/api/serializers.py | 162 +++++++++--------- src/backend/core/api/viewsets.py | 6 +- src/backend/core/models.py | 4 +- .../documents/test_api_document_accesses.py | 18 +- .../test_api_document_accesses_create.py | 20 ++- 5 files changed, 118 insertions(+), 92 deletions(-) diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 6bd9da30..561c48ff 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -38,83 +38,6 @@ class UserLightSerializer(UserSerializer): read_only_fields = ["full_name", "short_name"] -class DocumentAccessSerializer(serializers.ModelSerializer): - """Serialize document accesses.""" - - document_id = serializers.PrimaryKeyRelatedField( - read_only=True, - source="document", - ) - user_id = serializers.PrimaryKeyRelatedField( - queryset=models.User.objects.all(), - write_only=True, - source="user", - required=False, - allow_null=True, - ) - user = UserSerializer(read_only=True) - abilities = serializers.SerializerMethodField(read_only=True) - max_ancestors_role = serializers.SerializerMethodField(read_only=True) - - class Meta: - model = models.DocumentAccess - resource_field_name = "document" - fields = [ - "id", - "document_id", - "user", - "user_id", - "team", - "role", - "abilities", - "max_ancestors_role", - ] - read_only_fields = ["id", "document_id", "abilities", "max_ancestors_role"] - - def get_abilities(self, instance) -> dict: - """Return abilities of the logged-in user on the instance.""" - request = self.context.get("request") - if request: - return instance.get_abilities(request.user) - return {} - - def get_max_ancestors_role(self, instance): - """Return max_ancestors_role if annotated; else None.""" - return getattr(instance, "max_ancestors_role", None) - - def update(self, instance, validated_data): - """Make "user" field is readonly but only on update.""" - validated_data.pop("user", None) - return super().update(instance, validated_data) - - -class DocumentAccessLightSerializer(DocumentAccessSerializer): - """Serialize document accesses with limited fields.""" - - user = UserLightSerializer(read_only=True) - - class Meta: - model = models.DocumentAccess - resource_field_name = "document" - fields = [ - "id", - "document_id", - "user", - "team", - "role", - "abilities", - "max_ancestors_role", - ] - read_only_fields = [ - "id", - "document_id", - "team", - "role", - "abilities", - "max_ancestors_role", - ] - - class TemplateAccessSerializer(serializers.ModelSerializer): """Serialize template accesses.""" @@ -223,6 +146,15 @@ class ListDocumentSerializer(serializers.ModelSerializer): return instance.get_role(request.user) if request else None +class DocumentLightSerializer(serializers.ModelSerializer): + """Minial document serializer for nesting in document accesses.""" + + class Meta: + model = models.Document + fields = ["id", "path", "depth"] + read_only_fields = ["id", "path", "depth"] + + class DocumentSerializer(ListDocumentSerializer): """Serialize documents with all fields for display in detail views.""" @@ -359,6 +291,82 @@ class DocumentSerializer(ListDocumentSerializer): return super().save(**kwargs) +class DocumentAccessSerializer(serializers.ModelSerializer): + """Serialize document accesses.""" + + document = DocumentLightSerializer(read_only=True) + user_id = serializers.PrimaryKeyRelatedField( + queryset=models.User.objects.all(), + write_only=True, + source="user", + required=False, + allow_null=True, + ) + user = UserSerializer(read_only=True) + team = serializers.CharField(required=False, allow_blank=True) + abilities = serializers.SerializerMethodField(read_only=True) + max_ancestors_role = serializers.SerializerMethodField(read_only=True) + + class Meta: + model = models.DocumentAccess + resource_field_name = "document" + fields = [ + "id", + "document", + "user", + "user_id", + "team", + "role", + "abilities", + "max_ancestors_role", + ] + read_only_fields = ["id", "document", "abilities", "max_ancestors_role"] + + def get_abilities(self, instance) -> dict: + """Return abilities of the logged-in user on the instance.""" + request = self.context.get("request") + if request: + return instance.get_abilities(request.user) + return {} + + def get_max_ancestors_role(self, instance): + """Return max_ancestors_role if annotated; else None.""" + return getattr(instance, "max_ancestors_role", None) + + def update(self, instance, validated_data): + """Make "user" field readonly but only on update.""" + validated_data.pop("team", None) + validated_data.pop("user", None) + return super().update(instance, validated_data) + + +class DocumentAccessLightSerializer(DocumentAccessSerializer): + """Serialize document accesses with limited fields.""" + + user = UserLightSerializer(read_only=True) + + class Meta: + model = models.DocumentAccess + resource_field_name = "document" + fields = [ + "id", + "document", + "user", + "team", + "role", + "abilities", + "max_ancestors_role", + ] + read_only_fields = [ + "id", + "document", + "team", + "role", + "abilities", + "max_ancestors_role", + ] + + class ServerCreateDocumentSerializer(serializers.Serializer): """ Serializer for creating a document from a server-to-server request. diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 5f766293..4f50d0cf 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1530,11 +1530,7 @@ class DocumentAccessViewSet( if role not in choices.PRIVILEGED_ROLES: queryset = queryset.filter(role__in=choices.PRIVILEGED_ROLES) - accesses = list( - queryset.annotate(document_path=db.F("document__path")).order_by( - "document_path" - ) - ) + accesses = list(queryset.order_by("document__path")) # Annotate more information on roles path_to_key_to_max_ancestors_role = defaultdict( diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 5d16dacf..c48af63e 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1123,9 +1123,7 @@ class DocumentAccess(BaseAccess): if self.role == RoleChoices.OWNER: can_delete = role == RoleChoices.OWNER and ( # check if document is not root trying to avoid an extra query - # "document_path" is annotated by the viewset's list method - len(getattr(self, "document_path", "")) > Document.steplen - or not self.document.is_root() + self.document.depth > 1 or DocumentAccess.objects.filter( document_id=self.document_id, role=RoleChoices.OWNER ).count() diff --git a/src/backend/core/tests/documents/test_api_document_accesses.py b/src/backend/core/tests/documents/test_api_document_accesses.py index beae157f..4f12b305 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses.py +++ b/src/backend/core/tests/documents/test_api_document_accesses.py @@ -139,7 +139,11 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( [ { "id": str(access.id), - "document_id": str(access.document_id), + "document": { + "id": str(access.document_id), + "path": access.document.path, + "depth": access.document.depth, + }, "user": { "full_name": access.user.full_name, "short_name": access.user.short_name, @@ -234,7 +238,11 @@ def test_api_document_accesses_list_authenticated_related_privileged( [ { "id": str(access.id), - "document_id": str(access.document_id), + "document": { + "id": str(access.document_id), + "path": access.document.path, + "depth": access.document.depth, + }, "user": { "id": str(access.user.id), "email": access.user.email, @@ -600,7 +608,11 @@ def test_api_document_accesses_retrieve_authenticated_related( assert response.status_code == 200 assert response.json() == { "id": str(access.id), - "document_id": str(access.document_id), + "document": { + "id": str(access.document_id), + "path": access.document.path, + "depth": access.document.depth, + }, "user": access_user, "team": "", "role": access.role, diff --git a/src/backend/core/tests/documents/test_api_document_accesses_create.py b/src/backend/core/tests/documents/test_api_document_accesses_create.py index 5af0da91..eaaf63c7 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses_create.py +++ b/src/backend/core/tests/documents/test_api_document_accesses_create.py @@ -170,12 +170,16 @@ def test_api_document_accesses_create_authenticated_administrator( other_user = serializers.UserSerializer(instance=other_user).data assert response.json() == { "abilities": new_document_access.get_abilities(user), - "document_id": str(new_document_access.document_id), + "document": { + "id": str(new_document_access.document_id), + "depth": new_document_access.document.depth, + "path": new_document_access.document.path, + }, "id": str(new_document_access.id), + "user": other_user, "team": "", "role": role, "max_ancestors_role": None, - "user": other_user, } assert len(mail.outbox) == 1 email = mail.outbox[0] @@ -236,7 +240,11 @@ def test_api_document_accesses_create_authenticated_owner(via, depth, mock_user_ new_document_access = models.DocumentAccess.objects.filter(user=other_user).get() other_user = serializers.UserSerializer(instance=other_user).data assert response.json() == { - "document_id": str(new_document_access.document_id), + "document": { + "id": str(new_document_access.document_id), + "path": new_document_access.document.path, + "depth": new_document_access.document.depth, + }, "id": str(new_document_access.id), "user": other_user, "team": "", @@ -302,7 +310,11 @@ def test_api_document_accesses_create_email_in_receivers_language(via, mock_user ).get() other_user_data = serializers.UserSerializer(instance=other_user).data assert response.json() == { - "document_id": str(new_document_access.document_id), + "document": { + "id": str(new_document_access.document_id), + "path": new_document_access.document.path, + "depth": new_document_access.document.depth, + }, "id": str(new_document_access.id), "user": other_user_data, "team": "",