Skip to content

Commit 3057043

Browse files
committed
✨(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 1202b81 commit 3057043

File tree

5 files changed

+141
-102
lines changed

5 files changed

+141
-102
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: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,7 +1359,20 @@ class DocumentAccessViewSet(
13591359

13601360
lookup_field = "pk"
13611361
permission_classes = [permissions.DocumentAccessPermission]
1362-
queryset = models.DocumentAccess.objects.select_related("user").all()
1362+
queryset = models.DocumentAccess.objects.select_related("user", "document").only(
1363+
"id",
1364+
"created_at",
1365+
"role",
1366+
"team",
1367+
"user__id",
1368+
"user__short_name",
1369+
"user__full_name",
1370+
"user__email",
1371+
"user__language",
1372+
"document__id",
1373+
"document__path",
1374+
"document__depth",
1375+
)
13631376
resource_field_name = "document"
13641377

13651378
@cached_property
@@ -1398,37 +1411,33 @@ def list(self, request, *args, **kwargs):
13981411
if role not in choices.PRIVILEGED_ROLES:
13991412
queryset = queryset.filter(role__in=choices.PRIVILEGED_ROLES)
14001413

1401-
accesses = list(
1402-
queryset.annotate(document_path=db.F("document__path")).order_by(
1403-
"document_path"
1404-
)
1405-
)
1414+
accesses = list(queryset.order_by("document__path"))
14061415

14071416
# Annotate more information on roles
14081417
key_to_max_ancestors_role = defaultdict(lambda: None)
14091418
path_to_ancestors_roles = defaultdict(list)
14101419
path_to_role = defaultdict(lambda: None)
14111420
for access in accesses:
14121421
key = access.target_key
1413-
if access.document_path != self.document.path:
1422+
if access.document.path != self.document.path:
14141423
key_to_max_ancestors_role[key] = choices.RoleChoices.max(
14151424
key_to_max_ancestors_role[key], access.role
14161425
)
14171426

14181427
if access.user_id == user.id or access.team in user.teams:
1419-
parent_path = access.document_path[: -models.Document.steplen]
1428+
parent_path = access.document.path[: -models.Document.steplen]
14201429
if parent_path:
1421-
path_to_ancestors_roles[access.document_path].extend(
1430+
path_to_ancestors_roles[access.document.path].extend(
14221431
path_to_ancestors_roles[parent_path]
14231432
)
1424-
path_to_ancestors_roles[access.document_path].append(
1433+
path_to_ancestors_roles[access.document.path].append(
14251434
path_to_role[parent_path]
14261435
)
14271436
else:
1428-
path_to_ancestors_roles[access.document_path] = []
1437+
path_to_ancestors_roles[access.document.path] = []
14291438

1430-
path_to_role[access.document_path] = choices.RoleChoices.max(
1431-
path_to_role[access.document_path], access.role
1439+
path_to_role[access.document.path] = choices.RoleChoices.max(
1440+
path_to_role[access.document.path], access.role
14321441
)
14331442

14341443
# serialize and return the response
@@ -1438,8 +1447,8 @@ def list(self, request, *args, **kwargs):
14381447
for access in accesses:
14391448
access.max_ancestors_role = key_to_max_ancestors_role[access.target_key]
14401449
access.user_roles_tuple = (
1441-
choices.RoleChoices.max(*path_to_ancestors_roles[access.document_path]),
1442-
path_to_role.get(access.document_path),
1450+
choices.RoleChoices.max(*path_to_ancestors_roles[access.document.path]),
1451+
path_to_role.get(access.document.path),
14431452
)
14441453
serializer = serializer_class(access, context=context)
14451454
serialized_data.append(serializer.data)

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,
@@ -236,7 +240,11 @@ def test_api_document_accesses_list_authenticated_related_privileged(
236240
[
237241
{
238242
"id": str(access.id),
239-
"document_id": str(access.document_id),
243+
"document": {
244+
"id": str(access.document_id),
245+
"path": access.document.path,
246+
"depth": access.document.depth,
247+
},
240248
"user": {
241249
"id": str(access.user.id),
242250
"email": access.user.email,
@@ -533,7 +541,11 @@ def test_api_document_accesses_retrieve_authenticated_related(
533541
assert response.status_code == 200
534542
assert response.json() == {
535543
"id": str(access.id),
536-
"document_id": str(access.document_id),
544+
"document": {
545+
"id": str(access.document_id),
546+
"path": access.document.path,
547+
"depth": access.document.depth,
548+
},
537549
"user": access_user,
538550
"team": "",
539551
"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)