From a2a63cd13e540327216933772281711f53ae3eb6 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Thu, 28 Aug 2025 08:21:35 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20add=20comment=20viewset?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit add the CRUD part to manage comment lifeycle. Permissions are relying on the Document and Comment abilities. Comment viewset depends on the Document route and is added to the document_related_router. Dedicated serializer and permission are created. --- CHANGELOG.md | 1 + src/backend/core/api/permissions.py | 16 + src/backend/core/api/serializers.py | 44 ++ src/backend/core/api/viewsets.py | 33 + .../documents/test_api_documents_comments.py | 588 ++++++++++++++++++ src/backend/core/urls.py | 6 +- 6 files changed, 687 insertions(+), 1 deletion(-) create mode 100644 src/backend/core/tests/documents/test_api_documents_comments.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bea43202..199ae96a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -182,6 +182,7 @@ and this project adheres to ### Added +- ✨(backend) Comments on text editor #1309 - 👷(CI) add bundle size check job #1268 - ✨(frontend) use title first emoji as doc icon in tree #1289 diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index 09007847..29df311c 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -171,3 +171,19 @@ class ResourceAccessPermission(IsAuthenticated): action = view.action return abilities.get(action, False) + + +class CommentPermission(permissions.BasePermission): + """Permission class for comments.""" + + def has_permission(self, request, view): + """Check permission for a given object.""" + if view.action in ["create", "list"]: + document_abilities = view.get_document_or_404().get_abilities(request.user) + return document_abilities["comment"] + + return True + + def has_object_permission(self, request, view, obj): + """Check permission for a given object.""" + return obj.get_abilities(request.user).get(view.action, False) diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index c175745f..dbd010f3 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -891,3 +891,47 @@ class MoveDocumentSerializer(serializers.Serializer): choices=enums.MoveNodePositionChoices.choices, default=enums.MoveNodePositionChoices.LAST_CHILD, ) + + +class CommentSerializer(serializers.ModelSerializer): + """Serialize comments.""" + + user = UserLightSerializer(read_only=True) + abilities = serializers.SerializerMethodField(read_only=True) + + class Meta: + model = models.Comment + fields = [ + "id", + "content", + "created_at", + "updated_at", + "user", + "document", + "abilities", + ] + read_only_fields = [ + "id", + "created_at", + "updated_at", + "user", + "document", + "abilities", + ] + + def get_abilities(self, comment) -> dict: + """Return abilities of the logged-in user on the instance.""" + request = self.context.get("request") + if request: + return comment.get_abilities(request.user) + return {} + + def validate(self, attrs): + """Validate invitation data.""" + request = self.context.get("request") + user = getattr(request, "user", None) + + attrs["document_id"] = self.context["resource_id"] + attrs["user_id"] = user.id if user else None + + return attrs diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index c4bdb462..1d79b53d 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -2150,3 +2150,36 @@ class ConfigView(drf.views.APIView): ) return theme_customization + + +class CommentViewSet( + viewsets.ModelViewSet, +): + """API ViewSet for comments.""" + + permission_classes = [permissions.CommentPermission] + queryset = models.Comment.objects.select_related("user", "document").all() + serializer_class = serializers.CommentSerializer + pagination_class = Pagination + _document = None + + def get_document_or_404(self): + """Get the document related to the viewset or raise a 404 error.""" + if self._document is None: + try: + self._document = models.Document.objects.get( + pk=self.kwargs["resource_id"], + ) + except models.Document.DoesNotExist as e: + raise drf.exceptions.NotFound("Document not found.") from e + return self._document + + def get_serializer_context(self): + """Extra context provided to the serializer class.""" + context = super().get_serializer_context() + context["resource_id"] = self.kwargs["resource_id"] + return context + + def get_queryset(self): + """Return the queryset according to the action.""" + return super().get_queryset().filter(document=self.kwargs["resource_id"]) diff --git a/src/backend/core/tests/documents/test_api_documents_comments.py b/src/backend/core/tests/documents/test_api_documents_comments.py new file mode 100644 index 00000000..2a0cb7ce --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_comments.py @@ -0,0 +1,588 @@ +"""Test API for comments on documents.""" + +import random + +from django.contrib.auth.models import AnonymousUser + +import pytest +from rest_framework.test import APIClient + +from core import factories, models + +pytestmark = pytest.mark.django_db + +# List comments + + +def test_list_comments_anonymous_user_public_document(): + """Anonymous users should be allowed to list comments on a public document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.COMMENTATOR + ) + comment1, comment2 = factories.CommentFactory.create_batch(2, document=document) + # other comments not linked to the document + factories.CommentFactory.create_batch(2) + + response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/comments/") + assert response.status_code == 200 + assert response.json() == { + "count": 2, + "next": None, + "previous": None, + "results": [ + { + "id": str(comment2.id), + "content": comment2.content, + "created_at": comment2.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment2.updated_at.isoformat().replace("+00:00", "Z"), + "user": { + "full_name": comment2.user.full_name, + "short_name": comment2.user.short_name, + }, + "document": str(comment2.document.id), + "abilities": comment2.get_abilities(AnonymousUser()), + }, + { + "id": str(comment1.id), + "content": comment1.content, + "created_at": comment1.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment1.updated_at.isoformat().replace("+00:00", "Z"), + "user": { + "full_name": comment1.user.full_name, + "short_name": comment1.user.short_name, + }, + "document": str(comment1.document.id), + "abilities": comment1.get_abilities(AnonymousUser()), + }, + ], + } + + +@pytest.mark.parametrize("link_reach", ["restricted", "authenticated"]) +def test_list_comments_anonymous_user_non_public_document(link_reach): + """Anonymous users should not be allowed to list comments on a non-public document.""" + document = factories.DocumentFactory( + link_reach=link_reach, link_role=models.LinkRoleChoices.COMMENTATOR + ) + factories.CommentFactory(document=document) + # other comments not linked to the document + factories.CommentFactory.create_batch(2) + + response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/comments/") + assert response.status_code == 401 + + +def test_list_comments_authenticated_user_accessible_document(): + """Authenticated users should be allowed to list comments on an accessible document.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTATOR)] + ) + comment1 = factories.CommentFactory(document=document) + comment2 = factories.CommentFactory(document=document, user=user) + # other comments not linked to the document + factories.CommentFactory.create_batch(2) + + client = APIClient() + client.force_login(user) + + response = client.get(f"/api/v1.0/documents/{document.id!s}/comments/") + assert response.status_code == 200 + assert response.json() == { + "count": 2, + "next": None, + "previous": None, + "results": [ + { + "id": str(comment2.id), + "content": comment2.content, + "created_at": comment2.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment2.updated_at.isoformat().replace("+00:00", "Z"), + "user": { + "full_name": comment2.user.full_name, + "short_name": comment2.user.short_name, + }, + "document": str(comment2.document.id), + "abilities": comment2.get_abilities(user), + }, + { + "id": str(comment1.id), + "content": comment1.content, + "created_at": comment1.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment1.updated_at.isoformat().replace("+00:00", "Z"), + "user": { + "full_name": comment1.user.full_name, + "short_name": comment1.user.short_name, + }, + "document": str(comment1.document.id), + "abilities": comment1.get_abilities(user), + }, + ], + } + + +def test_list_comments_authenticated_user_non_accessible_document(): + """Authenticated users should not be allowed to list comments on a non-accessible document.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.CommentFactory(document=document) + # other comments not linked to the document + factories.CommentFactory.create_batch(2) + + client = APIClient() + client.force_login(user) + + response = client.get(f"/api/v1.0/documents/{document.id!s}/comments/") + assert response.status_code == 403 + + +def test_list_comments_authenticated_user_not_enough_access(): + """ + Authenticated users should not be allowed to list comments on a document they don't have + comment access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] + ) + factories.CommentFactory(document=document) + # other comments not linked to the document + factories.CommentFactory.create_batch(2) + + client = APIClient() + client.force_login(user) + + response = client.get(f"/api/v1.0/documents/{document.id!s}/comments/") + assert response.status_code == 403 + + +# Create comment + + +def test_create_comment_anonymous_user_public_document(): + """Anonymous users should not be allowed to create comments on a public document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.COMMENTATOR + ) + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/comments/", {"content": "test"} + ) + + assert response.status_code == 201 + + assert response.json() == { + "id": str(response.json()["id"]), + "content": "test", + "created_at": response.json()["created_at"], + "updated_at": response.json()["updated_at"], + "user": None, + "document": str(document.id), + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "retrieve": True, + }, + } + + +def test_create_comment_anonymous_user_non_accessible_document(): + """Anonymous users should not be allowed to create comments on a non-accessible document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.READER + ) + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/comments/", {"content": "test"} + ) + + assert response.status_code == 401 + + +def test_create_comment_authenticated_user_accessible_document(): + """Authenticated users should be allowed to create comments on an accessible document.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTATOR)] + ) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/comments/", {"content": "test"} + ) + assert response.status_code == 201 + + assert response.json() == { + "id": str(response.json()["id"]), + "content": "test", + "created_at": response.json()["created_at"], + "updated_at": response.json()["updated_at"], + "user": { + "full_name": user.full_name, + "short_name": user.short_name, + }, + "document": str(document.id), + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "retrieve": True, + }, + } + + +def test_create_comment_authenticated_user_not_enough_access(): + """ + Authenticated users should not be allowed to create comments on a document they don't have + comment access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] + ) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/comments/", {"content": "test"} + ) + assert response.status_code == 403 + + +# Retrieve comment + + +def test_retrieve_comment_anonymous_user_public_document(): + """Anonymous users should be allowed to retrieve comments on a public document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.COMMENTATOR + ) + comment = factories.CommentFactory(document=document) + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 200 + assert response.json() == { + "id": str(comment.id), + "content": comment.content, + "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), + "user": { + "full_name": comment.user.full_name, + "short_name": comment.user.short_name, + }, + "document": str(comment.document.id), + "abilities": comment.get_abilities(AnonymousUser()), + } + + +def test_retrieve_comment_anonymous_user_non_accessible_document(): + """Anonymous users should not be allowed to retrieve comments on a non-accessible document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.READER + ) + comment = factories.CommentFactory(document=document) + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 401 + + +def test_retrieve_comment_authenticated_user_accessible_document(): + """Authenticated users should be allowed to retrieve comments on an accessible document.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTATOR)] + ) + comment = factories.CommentFactory(document=document) + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 200 + + +def test_retrieve_comment_authenticated_user_not_enough_access(): + """ + Authenticated users should not be allowed to retrieve comments on a document they don't have + comment access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] + ) + comment = factories.CommentFactory(document=document) + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 403 + + +# Update comment + + +def test_update_comment_anonymous_user_public_document(): + """Anonymous users should not be allowed to update comments on a public document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.COMMENTATOR + ) + comment = factories.CommentFactory(document=document, content="test") + client = APIClient() + response = client.put( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", + {"content": "other content"}, + ) + assert response.status_code == 401 + + +def test_update_comment_anonymous_user_non_accessible_document(): + """Anonymous users should not be allowed to update comments on a non-accessible document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.READER + ) + comment = factories.CommentFactory(document=document, content="test") + client = APIClient() + response = client.put( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", + {"content": "other content"}, + ) + assert response.status_code == 401 + + +def test_update_comment_authenticated_user_accessible_document(): + """Authenticated users should not be able to update comments not their own.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + users=[ + ( + user, + random.choice( + [models.LinkRoleChoices.COMMENTATOR, models.LinkRoleChoices.EDITOR] + ), + ) + ], + ) + comment = factories.CommentFactory(document=document, content="test") + client = APIClient() + client.force_login(user) + response = client.put( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", + {"content": "other content"}, + ) + assert response.status_code == 403 + + +def test_update_comment_authenticated_user_own_comment(): + """Authenticated users should be able to update comments not their own.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + users=[ + ( + user, + random.choice( + [models.LinkRoleChoices.COMMENTATOR, models.LinkRoleChoices.EDITOR] + ), + ) + ], + ) + comment = factories.CommentFactory(document=document, content="test", user=user) + client = APIClient() + client.force_login(user) + response = client.put( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", + {"content": "other content"}, + ) + assert response.status_code == 200 + + comment.refresh_from_db() + assert comment.content == "other content" + + +def test_update_comment_authenticated_user_not_enough_access(): + """ + Authenticated users should not be allowed to update comments on a document they don't + have comment access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] + ) + comment = factories.CommentFactory(document=document, content="test") + client = APIClient() + client.force_login(user) + response = client.put( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", + {"content": "other content"}, + ) + assert response.status_code == 403 + + +def test_update_comment_authenticated_no_access(): + """ + Authenticated users should not be allowed to update comments on a document they don't + have access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + comment = factories.CommentFactory(document=document, content="test") + client = APIClient() + client.force_login(user) + response = client.put( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", + {"content": "other content"}, + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) +def test_update_comment_authenticated_admin_or_owner_can_update_any_comment(role): + """ + Authenticated users should be able to update comments on a document they don't have access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(users=[(user, role)]) + comment = factories.CommentFactory(document=document, content="test") + client = APIClient() + client.force_login(user) + + response = client.put( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", + {"content": "other content"}, + ) + assert response.status_code == 200 + + comment.refresh_from_db() + assert comment.content == "other content" + + +@pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) +def test_update_comment_authenticated_admin_or_owner_can_update_own_comment(role): + """ + Authenticated users should be able to update comments on a document they don't have access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(users=[(user, role)]) + comment = factories.CommentFactory(document=document, content="test", user=user) + client = APIClient() + client.force_login(user) + + response = client.put( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", + {"content": "other content"}, + ) + assert response.status_code == 200 + + comment.refresh_from_db() + assert comment.content == "other content" + + +# Delete comment + + +def test_delete_comment_anonymous_user_public_document(): + """Anonymous users should not be allowed to delete comments on a public document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.COMMENTATOR + ) + comment = factories.CommentFactory(document=document) + client = APIClient() + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 401 + + +def test_delete_comment_anonymous_user_non_accessible_document(): + """Anonymous users should not be allowed to delete comments on a non-accessible document.""" + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.READER + ) + comment = factories.CommentFactory(document=document) + client = APIClient() + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 401 + + +def test_delete_comment_authenticated_user_accessible_document_own_comment(): + """Authenticated users should be able to delete comments on an accessible document.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTATOR)] + ) + comment = factories.CommentFactory(document=document, user=user) + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 204 + + +def test_delete_comment_authenticated_user_accessible_document_not_own_comment(): + """Authenticated users should not be able to delete comments on an accessible document.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTATOR)] + ) + comment = factories.CommentFactory(document=document) + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) +def test_delete_comment_authenticated_user_admin_or_owner_can_delete_any_comment(role): + """Authenticated users should be able to delete comments on a document they have access to.""" + user = factories.UserFactory() + document = factories.DocumentFactory(users=[(user, role)]) + comment = factories.CommentFactory(document=document) + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 204 + + +@pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) +def test_delete_comment_authenticated_user_admin_or_owner_can_delete_own_comment(role): + """Authenticated users should be able to delete comments on a document they have access to.""" + user = factories.UserFactory() + document = factories.DocumentFactory(users=[(user, role)]) + comment = factories.CommentFactory(document=document, user=user) + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 204 + + +def test_delete_comment_authenticated_user_not_enough_access(): + """ + Authenticated users should not be able to delete comments on a document they don't + have access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] + ) + comment = factories.CommentFactory(document=document) + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 403 diff --git a/src/backend/core/urls.py b/src/backend/core/urls.py index a55795ae..84e13789 100644 --- a/src/backend/core/urls.py +++ b/src/backend/core/urls.py @@ -26,7 +26,11 @@ document_related_router.register( viewsets.InvitationViewset, basename="invitations", ) - +document_related_router.register( + "comments", + viewsets.CommentViewSet, + basename="comments", +) document_related_router.register( "ask-for-access", viewsets.DocumentAskForAccessViewSet,