diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 28cd4efa..556f209e 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -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): diff --git a/src/backend/core/tests/documents/test_api_documents_children_list.py b/src/backend/core/tests/documents/test_api_documents_children_list.py index 1f25cf7b..81c3f3c1 100644 --- a/src/backend/core/tests/documents/test_api_documents_children_list.py +++ b/src/backend/core/tests/documents/test_api_documents_children_list.py @@ -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], }, ], } diff --git a/src/backend/core/tests/documents/test_api_documents_list.py b/src/backend/core/tests/documents/test_api_documents_list.py index ca2de1b1..b9ab03c2 100644 --- a/src/backend/core/tests/documents/test_api_documents_list.py +++ b/src/backend/core/tests/documents/test_api_documents_list.py @@ -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 diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index 239c9acb..7c530e4f 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -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) +