Skip to content

Commit e74d05f

Browse files
sampaccoudPanchoutNathan
authored andcommitted
✨(backend) add document path and depth to accesses endpoint
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".
1 parent 5f8af4e commit e74d05f

File tree

5 files changed

+132
-93
lines changed

5 files changed

+132
-93
lines changed

src/backend/core/api/serializers.py

Lines changed: 85 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -38,83 +38,6 @@ class Meta:
3838
read_only_fields = ["full_name", "short_name"]
3939

4040

41-
class DocumentAccessSerializer(serializers.ModelSerializer):
42-
"""Serialize document accesses."""
43-
44-
document_id = serializers.PrimaryKeyRelatedField(
45-
read_only=True,
46-
source="document",
47-
)
48-
user_id = serializers.PrimaryKeyRelatedField(
49-
queryset=models.User.objects.all(),
50-
write_only=True,
51-
source="user",
52-
required=False,
53-
allow_null=True,
54-
)
55-
user = UserSerializer(read_only=True)
56-
abilities = serializers.SerializerMethodField(read_only=True)
57-
max_ancestors_role = serializers.SerializerMethodField(read_only=True)
58-
59-
class Meta:
60-
model = models.DocumentAccess
61-
resource_field_name = "document"
62-
fields = [
63-
"id",
64-
"document_id",
65-
"user",
66-
"user_id",
67-
"team",
68-
"role",
69-
"abilities",
70-
"max_ancestors_role",
71-
]
72-
read_only_fields = ["id", "document_id", "abilities", "max_ancestors_role"]
73-
74-
def get_abilities(self, instance) -> dict:
75-
"""Return abilities of the logged-in user on the instance."""
76-
request = self.context.get("request")
77-
if request:
78-
return instance.get_abilities(request.user)
79-
return {}
80-
81-
def get_max_ancestors_role(self, instance):
82-
"""Return max_ancestors_role if annotated; else None."""
83-
return getattr(instance, "max_ancestors_role", None)
84-
85-
def update(self, instance, validated_data):
86-
"""Make "user" field is readonly but only on update."""
87-
validated_data.pop("user", None)
88-
return super().update(instance, validated_data)
89-
90-
91-
class DocumentAccessLightSerializer(DocumentAccessSerializer):
92-
"""Serialize document accesses with limited fields."""
93-
94-
user = UserLightSerializer(read_only=True)
95-
96-
class Meta:
97-
model = models.DocumentAccess
98-
resource_field_name = "document"
99-
fields = [
100-
"id",
101-
"document_id",
102-
"user",
103-
"team",
104-
"role",
105-
"abilities",
106-
"max_ancestors_role",
107-
]
108-
read_only_fields = [
109-
"id",
110-
"document_id",
111-
"team",
112-
"role",
113-
"abilities",
114-
"max_ancestors_role",
115-
]
116-
117-
11841
class TemplateAccessSerializer(serializers.ModelSerializer):
11942
"""Serialize template accesses."""
12043

@@ -139,6 +62,15 @@ def update(self, instance, validated_data):
13962
return super().update(instance, validated_data)
14063

14164

65+
class DocumentMinimalSerializer(serializers.ModelSerializer):
66+
"""Minial document serializer for nesting in document accesses."""
67+
68+
class Meta:
69+
model = models.Document
70+
fields = ["id", "path", "depth"]
71+
read_only_fields = ["id", "path", "depth"]
72+
73+
14274
class ListDocumentSerializer(serializers.ModelSerializer):
14375
"""Serialize documents with limited fields for display in lists."""
14476

@@ -357,6 +289,82 @@ def save(self, **kwargs):
357289
return super().save(**kwargs)
358290

359291

292+
class DocumentAccessSerializer(serializers.ModelSerializer):
293+
"""Serialize document accesses."""
294+
295+
document = DocumentMinimalSerializer(read_only=True)
296+
user_id = serializers.PrimaryKeyRelatedField(
297+
queryset=models.User.objects.all(),
298+
write_only=True,
299+
source="user",
300+
required=False,
301+
allow_null=True,
302+
)
303+
user = UserSerializer(read_only=True)
304+
team = serializers.CharField(required=False, allow_blank=True)
305+
abilities = serializers.SerializerMethodField(read_only=True)
306+
max_ancestors_role = serializers.SerializerMethodField(read_only=True)
307+
308+
class Meta:
309+
model = models.DocumentAccess
310+
resource_field_name = "document"
311+
fields = [
312+
"id",
313+
"document",
314+
"user",
315+
"user_id",
316+
"team",
317+
"role",
318+
"abilities",
319+
"max_ancestors_role",
320+
]
321+
read_only_fields = ["id", "document", "abilities", "max_ancestors_role"]
322+
323+
def get_abilities(self, instance) -> dict:
324+
"""Return abilities of the logged-in user on the instance."""
325+
request = self.context.get("request")
326+
if request:
327+
return instance.get_abilities(request.user)
328+
return {}
329+
330+
def get_max_ancestors_role(self, instance):
331+
"""Return max_ancestors_role if annotated; else None."""
332+
return getattr(instance, "max_ancestors_role", None)
333+
334+
def update(self, instance, validated_data):
335+
"""Make "user" field readonly but only on update."""
336+
validated_data.pop("team", None)
337+
validated_data.pop("user", None)
338+
return super().update(instance, validated_data)
339+
340+
341+
class DocumentAccessLightSerializer(DocumentAccessSerializer):
342+
"""Serialize document accesses with limited fields."""
343+
344+
user = UserLightSerializer(read_only=True)
345+
346+
class Meta:
347+
model = models.DocumentAccess
348+
resource_field_name = "document"
349+
fields = [
350+
"id",
351+
"document",
352+
"user",
353+
"team",
354+
"role",
355+
"abilities",
356+
"max_ancestors_role",
357+
]
358+
read_only_fields = [
359+
"id",
360+
"document",
361+
"team",
362+
"role",
363+
"abilities",
364+
"max_ancestors_role",
365+
]
366+
367+
360368
class ServerCreateDocumentSerializer(serializers.Serializer):
361369
"""
362370
Serializer for creating a document from a server-to-server request.

src/backend/core/api/viewsets.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,7 +1340,20 @@ class DocumentAccessViewSet(
13401340

13411341
lookup_field = "pk"
13421342
permission_classes = [permissions.DocumentAccessPermission]
1343-
queryset = models.DocumentAccess.objects.select_related("user").all()
1343+
queryset = models.DocumentAccess.objects.select_related("user", "document").only(
1344+
"id",
1345+
"created_at",
1346+
"role",
1347+
"team",
1348+
"user__id",
1349+
"user__short_name",
1350+
"user__full_name",
1351+
"user__email",
1352+
"user__language",
1353+
"document__id",
1354+
"document__path",
1355+
"document__depth",
1356+
)
13441357
resource_field_name = "document"
13451358

13461359
@cached_property
@@ -1379,11 +1392,7 @@ def list(self, request, *args, **kwargs):
13791392
if role not in choices.PRIVILEGED_ROLES:
13801393
queryset = queryset.filter(role__in=choices.PRIVILEGED_ROLES)
13811394

1382-
accesses = list(
1383-
queryset.annotate(document_path=db.F("document__path")).order_by(
1384-
"document_path"
1385-
)
1386-
)
1395+
accesses = list(queryset.order_by("document__path"))
13871396

13881397
# Annotate more information on roles
13891398
path_to_key_to_max_ancestors_role = defaultdict(

src/backend/core/models.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,9 +1095,7 @@ def get_abilities(self, user):
10951095
if self.role == RoleChoices.OWNER:
10961096
can_delete = role == RoleChoices.OWNER and (
10971097
# check if document is not root trying to avoid an extra query
1098-
# "document_path" is annotated by the viewset's list method
1099-
len(getattr(self, "document_path", "")) > Document.steplen
1100-
or not self.document.is_root()
1098+
self.document.depth > 1
11011099
or DocumentAccess.objects.filter(
11021100
document_id=self.document_id, role=RoleChoices.OWNER
11031101
).count()

src/backend/core/tests/documents/test_api_document_accesses.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,11 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
139139
[
140140
{
141141
"id": str(access.id),
142-
"document_id": str(access.document_id),
142+
"document": {
143+
"id": str(access.document_id),
144+
"path": access.document.path,
145+
"depth": access.document.depth,
146+
},
143147
"user": {
144148
"full_name": access.user.full_name,
145149
"short_name": access.user.short_name,
@@ -234,7 +238,11 @@ def test_api_document_accesses_list_authenticated_related_privileged(
234238
[
235239
{
236240
"id": str(access.id),
237-
"document_id": str(access.document_id),
241+
"document": {
242+
"id": str(access.document_id),
243+
"path": access.document.path,
244+
"depth": access.document.depth,
245+
},
238246
"user": {
239247
"id": str(access.user.id),
240248
"email": access.user.email,
@@ -600,7 +608,11 @@ def test_api_document_accesses_retrieve_authenticated_related(
600608
assert response.status_code == 200
601609
assert response.json() == {
602610
"id": str(access.id),
603-
"document_id": str(access.document_id),
611+
"document": {
612+
"id": str(access.document_id),
613+
"path": access.document.path,
614+
"depth": access.document.depth,
615+
},
604616
"user": access_user,
605617
"team": "",
606618
"role": access.role,

src/backend/core/tests/documents/test_api_document_accesses_create.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,12 +170,16 @@ def test_api_document_accesses_create_authenticated_administrator(
170170
other_user = serializers.UserSerializer(instance=other_user).data
171171
assert response.json() == {
172172
"abilities": new_document_access.get_abilities(user),
173-
"document_id": str(new_document_access.document_id),
173+
"document": {
174+
"id": str(new_document_access.document_id),
175+
"depth": new_document_access.document.depth,
176+
"path": new_document_access.document.path,
177+
},
174178
"id": str(new_document_access.id),
179+
"user": other_user,
175180
"team": "",
176181
"role": role,
177182
"max_ancestors_role": None,
178-
"user": other_user,
179183
}
180184
assert len(mail.outbox) == 1
181185
email = mail.outbox[0]
@@ -236,7 +240,11 @@ def test_api_document_accesses_create_authenticated_owner(via, depth, mock_user_
236240
new_document_access = models.DocumentAccess.objects.filter(user=other_user).get()
237241
other_user = serializers.UserSerializer(instance=other_user).data
238242
assert response.json() == {
239-
"document_id": str(new_document_access.document_id),
243+
"document": {
244+
"id": str(new_document_access.document_id),
245+
"path": new_document_access.document.path,
246+
"depth": new_document_access.document.depth,
247+
},
240248
"id": str(new_document_access.id),
241249
"user": other_user,
242250
"team": "",
@@ -302,7 +310,11 @@ def test_api_document_accesses_create_email_in_receivers_language(via, mock_user
302310
).get()
303311
other_user_data = serializers.UserSerializer(instance=other_user).data
304312
assert response.json() == {
305-
"document_id": str(new_document_access.document_id),
313+
"document": {
314+
"id": str(new_document_access.document_id),
315+
"path": new_document_access.document.path,
316+
"depth": new_document_access.document.depth,
317+
},
306318
"id": str(new_document_access.id),
307319
"user": other_user_data,
308320
"team": "",

0 commit comments

Comments
 (0)