🛂(backend) make document access list visible to all collaborators

Return all accesses on the document and its ancestors to any
user with access, keeping the limited user details serializer
and adding the user id to it, so that collaborators allowed
to comment can identify and mention each other.

Signed-off-by: Mohamed El Amine BOUKERFA <boukerfa.ma@gmail.com>

CHANGELOG.md

@@ -263,6 +263,7 @@ and this project adheres to
 
 ### Changed
 
+- 🛂(backend) make document access list visible to all collaborators
 - 👷(CI) remove test-e2e-other-browser job #2404
 - ♿️(frontend) use heading element for pinned documents section title #2380
 - ♿️(frontend) use anchor links for table of contents entries #2390

src/backend/core/api/serializers.py

@@ -73,8 +73,8 @@ class UserLightSerializer(UserSerializer):
 
     class Meta:
         model = models.User
-        fields = ["full_name", "short_name"]
-        read_only_fields = ["full_name", "short_name"]
+        fields = ["id", "full_name", "short_name"]
+        read_only_fields = ["id", "full_name", "short_name"]
 
 
 class ListDocumentSerializer(serializers.ModelSerializer):

src/backend/core/api/viewsets.py

@@ -2599,11 +2599,11 @@ class DocumentAccessViewSet(
             | models.Document.objects.filter(pk=self.document.pk)
         ).filter(ancestors_deleted_at__isnull=True)
 
+        # All users with access see the full list of accesses (with limited
+        # user details for unprivileged roles) so that any collaborator
+        # allowed to comment can mention the others.
         queryset = self.get_queryset().filter(document__in=ancestors)
 
-        if role not in choices.PRIVILEGED_ROLES:
-            queryset = queryset.filter(role__in=choices.PRIVILEGED_ROLES)
-
         accesses = list(queryset.order_by("document__path"))
 
         # Annotate more information on roles

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

@@ -97,8 +97,9 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
     via, role, mock_user_teams, django_assert_num_queries
 ):
     """
-    Authenticated users with no privileged role should only be able to list document
-    accesses associated with privileged roles for a document, including from ancestors.
+    Authenticated users with no privileged role should be able to list all document
+    accesses, including from ancestors, but with limited user information, so that
+    any collaborator allowed to comment can mention the others.
     """
     user = factories.UserFactory()
     client = APIClient()
@@ -125,18 +126,20 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
     factories.UserDocumentAccessFactory(document=child)
 
     if via == USER:
-        models.DocumentAccess.objects.create(
+        user_access = models.DocumentAccess.objects.create(
             document=document,
             user=user,
             role=role,
         )
     elif via == TEAM:
         mock_user_teams.return_value = ["lasuite", "unknown"]
-        models.DocumentAccess.objects.create(
+        user_access = models.DocumentAccess.objects.create(
             document=document,
             team="lasuite",
             role=role,
         )
+    else:
+        raise RuntimeError()
 
     # Accesses for other documents to which the user is related should not be listed either
     other_access = factories.UserDocumentAccessFactory(user=user)
@@ -148,11 +151,9 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
     assert response.status_code == 200
     content = response.json()
 
-    # Make sure only privileged roles are returned
-    privileged_accesses = [
-        acc for acc in accesses if acc.role in choices.PRIVILEGED_ROLES
-    ]
-    assert len(content) == len(privileged_accesses)
+    # All accesses on the document and its ancestors are returned
+    all_accesses = [*accesses, user_access]
+    assert len(content) == len(all_accesses)
 
     assert sorted(content, key=lambda x: x["id"]) == sorted(
         [
@@ -164,6 +165,7 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
                     "depth": access.document.depth,
                 },
                 "user": {
+                    "id": str(access.user.id),
                     "full_name": access.user.full_name,
                     "short_name": access.user.short_name,
                 }
@@ -176,14 +178,14 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
                 "abilities": {
                     "destroy": False,
                     "partial_update": False,
-                    "retrieve": False,
+                    "retrieve": access.user is not None and access.user.id == user.id,
                     "set_role_to": [],
                     "update": False,
                 },
                 "updated_at": access.updated_at.isoformat().replace("+00:00", "Z"),
                 "created_at": access.created_at.isoformat().replace("+00:00", "Z"),
             }
-            for access in privileged_accesses
+            for access in all_accesses
         ],
         key=lambda x: x["id"],
     )

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

@@ -48,7 +48,11 @@ def expected_access(access, user, via):
         },
         "user": None
         if via == TEAM
-        else {"full_name": user.full_name, "short_name": user.short_name},
+        else {
+            "full_name": user.full_name,
+            "short_name": user.short_name,
+            "id": str(user.id),
+        },
         "team": "lasuite" if via == TEAM else "",
         "role": access.role,
         "max_ancestors_role": None,

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

@@ -1,5 +1,6 @@
 """Test API for comments on documents."""
 
+# pylint: disable=too-many-lines
 import random
 from unittest import mock
 
@@ -41,6 +42,7 @@ def test_list_comments_anonymous_user_public_document():
                 "created_at": comment1.created_at.isoformat().replace("+00:00", "Z"),
                 "updated_at": comment1.updated_at.isoformat().replace("+00:00", "Z"),
                 "user": {
+                    "id": str(comment1.user.id),
                     "full_name": comment1.user.full_name,
                     "short_name": comment1.user.short_name,
                 },
@@ -53,6 +55,7 @@ def test_list_comments_anonymous_user_public_document():
                 "created_at": comment2.created_at.isoformat().replace("+00:00", "Z"),
                 "updated_at": comment2.updated_at.isoformat().replace("+00:00", "Z"),
                 "user": {
+                    "id": str(comment2.user.id),
                     "full_name": comment2.user.full_name,
                     "short_name": comment2.user.short_name,
                 },
@@ -110,6 +113,7 @@ def test_list_comments_authenticated_user_accessible_document():
                 "created_at": comment1.created_at.isoformat().replace("+00:00", "Z"),
                 "updated_at": comment1.updated_at.isoformat().replace("+00:00", "Z"),
                 "user": {
+                    "id": str(comment1.user.id),
                     "full_name": comment1.user.full_name,
                     "short_name": comment1.user.short_name,
                 },
@@ -122,6 +126,7 @@ def test_list_comments_authenticated_user_accessible_document():
                 "created_at": comment2.created_at.isoformat().replace("+00:00", "Z"),
                 "updated_at": comment2.updated_at.isoformat().replace("+00:00", "Z"),
                 "user": {
+                    "id": str(comment2.user.id),
                     "full_name": comment2.user.full_name,
                     "short_name": comment2.user.short_name,
                 },
@@ -263,6 +268,7 @@ def test_create_comment_authenticated_user_accessible_document():
         "created_at": response.json()["created_at"],
         "updated_at": response.json()["updated_at"],
         "user": {
+            "id": str(user.id),
             "full_name": user.full_name,
             "short_name": user.short_name,
         },
@@ -317,6 +323,7 @@ def test_retrieve_comment_anonymous_user_public_document():
         "created_at": comment.created_at.isoformat().replace("+00:00", "Z"),
         "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"),
         "user": {
+            "id": str(comment.user.id),
             "full_name": comment.user.full_name,
             "short_name": comment.user.short_name,
         },

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

@@ -175,6 +175,7 @@ def test_api_documents_threads_restricted_document_editor(role):
         "created_at": thread.created_at.isoformat().replace("+00:00", "Z"),
         "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"),
         "creator": {
+            "id": str(user.id),
             "full_name": user.full_name,
             "short_name": user.short_name,
         },
@@ -185,6 +186,7 @@ def test_api_documents_threads_restricted_document_editor(role):
                 "created_at": comment.created_at.isoformat().replace("+00:00", "Z"),
                 "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"),
                 "user": {
+                    "id": str(user.id),
                     "full_name": user.full_name,
                     "short_name": user.short_name,
                 },
@@ -296,6 +298,7 @@ def test_api_documents_threads_authenticated_document(link_role):
         "created_at": thread.created_at.isoformat().replace("+00:00", "Z"),
         "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"),
         "creator": {
+            "id": str(user.id),
             "full_name": user.full_name,
             "short_name": user.short_name,
         },
@@ -306,6 +309,7 @@ def test_api_documents_threads_authenticated_document(link_role):
                 "created_at": comment.created_at.isoformat().replace("+00:00", "Z"),
                 "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"),
                 "user": {
+                    "id": str(user.id),
                     "full_name": user.full_name,
                     "short_name": user.short_name,
                 },

src/backend/core/tests/serializers/test_user_light_serializer.py

@@ -16,8 +16,10 @@ def test_user_light_serializer():
         short_name="John",
     )
     serializer = UserLightSerializer(user)
+    assert serializer.data["id"] == str(user.id)
     assert serializer.data["full_name"] == "John Doe"
     assert serializer.data["short_name"] == "John"
+    assert "email" not in serializer.data
 
 
 def test_user_light_serializer_no_full_name():