studio/docs
Archived
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

✨(backend) add user roles as field in the document API representation

Browse Source 
user roles were already computed as an annotation on the query for
performance as we must look at all the document's ancestors to determine
the roles that apply recursively. We can easily expose them as readonly
via the serializer.
This commit is contained in:
Samuel Paccoud - DINUM
2025-01-02 14:02:03 +01:00
committed by Anthony LC
parent 8117866ce7
commit 0003f9d0de
4 changed files with 114 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/backend/core/api/serializers.py
View File

@@ -147,6 +147,7 @@ class ListDocumentSerializer(BaseResourceSerializer):
is_favorite = serializers.BooleanField(read_only=True)
nb_accesses = serializers.IntegerField(read_only=True)
user_roles = serializers.SerializerMethodField(read_only=True)
class Meta:
model = models.Document
@@ -165,6 +166,7 @@ class ListDocumentSerializer(BaseResourceSerializer):
"path",
"title",
"updated_at",
"user_roles",
]
read_only_fields = [
"id",
@@ -180,8 +182,19 @@ class ListDocumentSerializer(BaseResourceSerializer):
"numchild",
"path",
"updated_at",
"user_roles",
]
def get_user_roles(self, document):
"""
Return roles of the logged-in user for the current document,
taking into account ancestors.
"""
request = self.context.get("request")
if request:
return document.get_roles(request.user)
return []
class DocumentSerializer(ListDocumentSerializer):
"""Serialize documents with all fields for display in detail views."""
@@ -206,6 +219,7 @@ class DocumentSerializer(ListDocumentSerializer):
"path",
"title",
"updated_at",
"user_roles",
]
read_only_fields = [
"id",
@@ -220,6 +234,7 @@ class DocumentSerializer(ListDocumentSerializer):
"numchild",
"path",
"updated_at",
"user_roles",
]
def get_fields(self):

20
src/backend/core/tests/documents/test_api_documents_children_list.py
View File

@@ -43,6 +43,7 @@ def test_api_documents_children_list_anonymous_public_standalone():
"path": child1.path,
"title": child1.title,
"updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
},
{
"abilities": child2.get_abilities(AnonymousUser()),
@@ -59,6 +60,7 @@ def test_api_documents_children_list_anonymous_public_standalone():
"path": child2.path,
"title": child2.title,
"updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
},
],
}
@@ -102,6 +104,7 @@ def test_api_documents_children_list_anonymous_public_parent():
"path": child1.path,
"title": child1.title,
"updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
},
{
"abilities": child2.get_abilities(AnonymousUser()),
@@ -118,6 +121,7 @@ def test_api_documents_children_list_anonymous_public_parent():
"path": child2.path,
"title": child2.title,
"updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
},
],
}
@@ -179,6 +183,7 @@ def test_api_documents_children_list_authenticated_unrelated_public_or_authentic
"path": child1.path,
"title": child1.title,
"updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
},
{
"abilities": child2.get_abilities(user),
@@ -195,6 +200,7 @@ def test_api_documents_children_list_authenticated_unrelated_public_or_authentic
"path": child2.path,
"title": child2.title,
"updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
},
],
}
@@ -242,6 +248,7 @@ def test_api_documents_children_list_authenticated_public_or_authenticated_paren
"path": child1.path,
"title": child1.title,
"updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
},
{
"abilities": child2.get_abilities(user),
@@ -258,6 +265,7 @@ def test_api_documents_children_list_authenticated_public_or_authenticated_paren
"path": child2.path,
"title": child2.title,
"updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
},
],
}
@@ -327,6 +335,7 @@ def test_api_documents_children_list_authenticated_related_direct():
"path": child1.path,
"title": child1.title,
"updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [access.role],
},
{
"abilities": child2.get_abilities(user),
@@ -343,6 +352,7 @@ def test_api_documents_children_list_authenticated_related_direct():
"path": child2.path,
"title": child2.title,
"updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [access.role],
},
],
}
@@ -365,8 +375,10 @@ def test_api_documents_children_list_authenticated_related_parent():
child1, child2 = factories.DocumentFactory.create_batch(2, parent=document)
factories.UserDocumentAccessFactory(document=child1)
grand_parent_access = factories.UserDocumentAccessFactory(
document=grand_parent, user=user
)
factories.UserDocumentAccessFactory(document=grand_parent, user=user)
factories.UserDocumentAccessFactory(document=grand_parent)
response = client.get(
f"/api/v1.0/documents/{document.id!s}/children/",
@@ -392,6 +404,7 @@ def test_api_documents_children_list_authenticated_related_parent():
"path": child1.path,
"title": child1.title,
"updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [grand_parent_access.role],
},
{
"abilities": child2.get_abilities(user),
@@ -408,6 +421,7 @@ def test_api_documents_children_list_authenticated_related_parent():
"path": child2.path,
"title": child2.title,
"updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [grand_parent_access.role],
},
],
}
@@ -479,7 +493,7 @@ def test_api_documents_children_list_authenticated_related_team_members(
document = factories.DocumentFactory(link_reach="restricted")
child1, child2 = factories.DocumentFactory.create_batch(2, parent=document)
factories.TeamDocumentAccessFactory(document=document, team="myteam")
access = factories.TeamDocumentAccessFactory(document=document, team="myteam")
response = client.get(f"/api/v1.0/documents/{document.id!s}/children/")
@@ -505,6 +519,7 @@ def test_api_documents_children_list_authenticated_related_team_members(
"path": child1.path,
"title": child1.title,
"updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [access.role],
},
{
"abilities": child2.get_abilities(user),
@@ -521,6 +536,7 @@ def test_api_documents_children_list_authenticated_related_team_members(
"path": child2.path,
"title": child2.title,
"updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [access.role],
},
],
}

6
src/backend/core/tests/documents/test_api_documents_list.py
View File

@@ -35,16 +35,16 @@ def test_api_documents_list_anonymous(reach, role):
def test_api_documents_list_format():
"""Validate the format of documents as returned by the list view."""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
other_users = factories.UserFactory.create_batch(3)
document = factories.DocumentFactory(
users=[user, *factories.UserFactory.create_batch(2)],
users=factories.UserFactory.create_batch(2),
favorited_by=[user, *other_users],
link_traces=other_users,
)
access = factories.UserDocumentAccessFactory(document=document, user=user)
response = client.get("/api/v1.0/documents/")
@@ -72,6 +72,7 @@ def test_api_documents_list_format():
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [access.role],
}
@@ -230,7 +231,6 @@ def test_api_documents_list_authenticated_link_reach_public_or_authenticated(
assert response.status_code == 200
results = response.json()["results"]
assert len(results) == 3
results_id = {result["id"] for result in results}
assert expected_ids == results_id

97
src/backend/core/tests/documents/test_api_documents_retrieve.py
View File

@@ -56,6 +56,7 @@ def test_api_documents_retrieve_anonymous_public_standalone():
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
}
@@ -109,6 +110,7 @@ def test_api_documents_retrieve_anonymous_public_parent():
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
}
@@ -195,6 +197,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
}
assert (
models.LinkTrace.objects.filter(document=document, user=user).exists() is True
@@ -255,6 +258,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [],
}
@@ -340,7 +344,7 @@ def test_api_documents_retrieve_authenticated_related_direct():
client.force_login(user)
document = factories.DocumentFactory()
factories.UserDocumentAccessFactory(document=document, user=user)
access = factories.UserDocumentAccessFactory(document=document, user=user)
factories.UserDocumentAccessFactory(document=document)
response = client.get(
@@ -363,6 +367,7 @@ def test_api_documents_retrieve_authenticated_related_direct():
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [access.role],
}
@@ -423,6 +428,7 @@ def test_api_documents_retrieve_authenticated_related_parent():
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": [access.role],
}
@@ -516,16 +522,16 @@ def test_api_documents_retrieve_authenticated_related_team_none(mock_user_teams)
@pytest.mark.parametrize(
"teams",
"teams,roles",
[
["readers"],
["unknown", "readers"],
["editors"],
["unknown", "editors"],
[["readers"], ["reader"]],
[["unknown", "readers"], ["reader"]],
[["editors"], ["editor"]],
[["unknown", "editors"], ["editor"]],
],
)
def test_api_documents_retrieve_authenticated_related_team_members(
teams, mock_user_teams
teams, roles, mock_user_teams
):
"""
Authenticated users should be allowed to retrieve a document to which they
@@ -573,19 +579,20 @@ def test_api_documents_retrieve_authenticated_related_team_members(
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": roles,
}
@pytest.mark.parametrize(
"teams",
"teams,roles",
[
["administrators"],
["editors", "administrators"],
["unknown", "administrators"],
[["administrators"], ["administrator"]],
[["editors", "administrators"], ["administrator", "editor"]],
[["unknown", "administrators"], ["administrator"]],
],
)
def test_api_documents_retrieve_authenticated_related_team_administrators(
teams, mock_user_teams
teams, roles, mock_user_teams
):
"""
Authenticated users should be allowed to retrieve a document to which they
@@ -633,20 +640,21 @@ def test_api_documents_retrieve_authenticated_related_team_administrators(
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": roles,
}
@pytest.mark.parametrize(
"teams",
"teams,roles",
[
["owners"],
["owners", "administrators"],
["members", "administrators", "owners"],
["unknown", "owners"],
[["owners"], ["owner"]],
[["owners", "administrators"], ["owner", "administrator"]],
[["members", "administrators", "owners"], ["owner", "administrator"]],
[["unknown", "owners"], ["owner"]],
],
)
def test_api_documents_retrieve_authenticated_related_team_owners(
teams, mock_user_teams
teams, roles, mock_user_teams
):
"""
Authenticated users should be allowed to retrieve a restricted document to which
@@ -655,7 +663,6 @@ def test_api_documents_retrieve_authenticated_related_team_owners(
mock_user_teams.return_value = teams
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
@@ -694,4 +701,56 @@ def test_api_documents_retrieve_authenticated_related_team_owners(
"path": document.path,
"title": document.title,
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
"user_roles": roles,
}
def test_api_documents_retrieve_user_roles(django_assert_num_queries):
"""
Roles should be annotated on querysets taking into account all documents ancestors.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
grand_parent = factories.DocumentFactory(
users=factories.UserFactory.create_batch(2)
)
parent = factories.DocumentFactory(
parent=grand_parent, users=factories.UserFactory.create_batch(2)
)
document = factories.DocumentFactory(
parent=parent, users=factories.UserFactory.create_batch(2)
)
accesses = (
factories.UserDocumentAccessFactory(document=grand_parent, user=user),
factories.UserDocumentAccessFactory(document=parent, user=user),
factories.UserDocumentAccessFactory(document=document, user=user),
)
expected_roles = {access.role for access in accesses}
with django_assert_num_queries(8):
response = client.get(f"/api/v1.0/documents/{document.id!s}/")
assert response.status_code == 200
user_roles = response.json()["user_roles"]
assert set(user_roles) == expected_roles
def test_api_documents_retrieve_numqueries_with_link_trace(django_assert_num_queries):
"""If the link traced already exists, the number of queries should be minimal."""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
document = factories.DocumentFactory(users=[user], link_traces=[user])
with django_assert_num_queries(2):
response = client.get(f"/api/v1.0/documents/{document.id!s}/")
assert response.status_code == 200
assert response.json()["id"] == str(document.id)