From 8c6bf045cc5735214643cc13e5498c062b66a0fa 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 8686ecf4..70a24392 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.""" @@ -357,6 +289,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 af28b809..f4d0cc61 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1451,11 +1451,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 83138113..ed892e83 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1122,9 +1122,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": "",