Skip to content

Commit e695ed5

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 da6dda7 commit e695ed5

File tree

5 files changed

+118
-92
lines changed

5 files changed

+118
-92
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

@@ -223,6 +146,15 @@ def get_user_role(self, instance):
223146
return instance.get_role(request.user) if request else None
224147

225148

149+
class DocumentLightSerializer(serializers.ModelSerializer):
150+
"""Minial document serializer for nesting in document accesses."""
151+
152+
class Meta:
153+
model = models.Document
154+
fields = ["id", "path", "depth"]
155+
read_only_fields = ["id", "path", "depth"]
156+
157+
226158
class DocumentSerializer(ListDocumentSerializer):
227159
"""Serialize documents with all fields for display in detail views."""
228160

@@ -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 = DocumentLightSerializer(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: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,11 +1392,7 @@ def list(self, request, *args, **kwargs):
13921392
if role not in choices.PRIVILEGED_ROLES:
13931393
queryset = queryset.filter(role__in=choices.PRIVILEGED_ROLES)
13941394

1395-
accesses = list(
1396-
queryset.annotate(document_path=db.F("document__path")).order_by(
1397-
"document_path"
1398-
)
1399-
)
1395+
accesses = list(queryset.order_by("document__path"))
14001396

14011397
# Annotate more information on roles
14021398
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
@@ -1121,9 +1121,7 @@ def get_abilities(self, user):
11211121
if self.role == RoleChoices.OWNER:
11221122
can_delete = role == RoleChoices.OWNER and (
11231123
# check if document is not root trying to avoid an extra query
1124-
# "document_path" is annotated by the viewset's list method
1125-
len(getattr(self, "document_path", "")) > Document.steplen
1126-
or not self.document.is_root()
1124+
self.document.depth > 1
11271125
or DocumentAccess.objects.filter(
11281126
document_id=self.document_id, role=RoleChoices.OWNER
11291127
).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)