From 7ad9550f54dd0b7e0187f87e7aa5afc67fe174d6 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 22 May 2026 15:30:39 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20add=20unresolve=20action?= =?UTF-8?q?=20to=20ThreadViewSet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have now the possibility to unresolve a thread from the frontend. We need to add the unresolve action to the ThreadViewSet. --- src/backend/core/api/viewsets.py | 11 ++ src/backend/core/models.py | 1 + .../documents/test_api_documents_threads.py | 116 +++++++++++++----- .../components/comments/DocsThreadStore.tsx | 5 - .../comments/DocsThreadStoreAuth.tsx | 11 +- .../doc-editor/components/comments/types.ts | 1 + 6 files changed, 105 insertions(+), 40 deletions(-) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index e04014d01..d7d4552f8 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -2972,6 +2972,17 @@ class ThreadViewSet( thread.save(update_fields=["resolved", "resolved_at", "resolved_by"]) return drf.response.Response(status=status.HTTP_204_NO_CONTENT) + @drf.decorators.action(detail=True, methods=["post"], url_path="unresolve") + def unresolve(self, request, *args, **kwargs): + """Unresolve a thread.""" + thread = self.get_object() + if thread.resolved: + thread.resolved = False + thread.resolved_at = None + thread.resolved_by = None + thread.save(update_fields=["resolved", "resolved_at", "resolved_by"]) + return drf.response.Response(status=status.HTTP_204_NO_CONTENT) + class CommentViewSet( CommentViewSetMixin, diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 1599accd0..626304e54 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1851,6 +1851,7 @@ class Thread(BaseModel): "update": write_access, "partial_update": write_access, "resolve": write_access, + "unresolve": write_access, "retrieve": read_access, } diff --git a/src/backend/core/tests/documents/test_api_documents_threads.py b/src/backend/core/tests/documents/test_api_documents_threads.py index a950bf76e..d3831de7c 100644 --- a/src/backend/core/tests/documents/test_api_documents_threads.py +++ b/src/backend/core/tests/documents/test_api_documents_threads.py @@ -85,6 +85,7 @@ def test_api_documents_threads_public_document(link_role): "update": False, "partial_update": False, "resolve": False, + "unresolve": False, "retrieve": True, }, "metadata": {}, @@ -179,6 +180,7 @@ def test_api_documents_threads_restricted_document_editor(role): "update": True, "partial_update": True, "resolve": True, + "unresolve": True, "retrieve": True, }, "metadata": {}, @@ -289,6 +291,7 @@ def test_api_documents_threads_authenticated_document(link_role): "update": True, "partial_update": True, "resolve": True, + "unresolve": True, "retrieve": True, }, "metadata": {}, @@ -549,6 +552,7 @@ def test_api_documents_threads_retrieve_public_document_link_role_higher_than_re "update": False, "partial_update": False, "resolve": False, + "unresolve": False, "retrieve": True, }, "metadata": {}, @@ -652,6 +656,7 @@ def test_api_documents_threads_retrieve_authenticated_document(link_role): "update": False, "partial_update": False, "resolve": False, + "unresolve": False, "retrieve": True, }, } @@ -749,6 +754,7 @@ def test_api_documents_threads_retrieve_restricted_document_editor(role): "update": False, "partial_update": False, "resolve": False, + "unresolve": False, "retrieve": True, }, "metadata": {}, @@ -808,6 +814,7 @@ def test_api_documents_threads_retrieve_restricted_document_privileged_roles(rol "update": True, "partial_update": True, "resolve": True, + "unresolve": True, "retrieve": True, }, "metadata": {}, @@ -1019,12 +1026,13 @@ def test_api_documents_threads_destroy_restricted_document_privileged_roles(role assert not models.Thread.objects.filter(id=thread.id).exists() -# Resolve +# Resolve / Unresolve -def test_api_documents_threads_resolve_public_document_anonymous_user(): +@pytest.mark.parametrize("action", ["resolve", "unresolve"]) +def test_api_documents_threads_resolve_unresolve_public_document_anonymous_user(action): """ - Anonymous users should not be allowed to resolve threads on public documents. + Anonymous users should not be allowed to resolve or unresolve threads on public documents. """ document = factories.DocumentFactory( link_reach="public", @@ -1036,14 +1044,17 @@ def test_api_documents_threads_resolve_public_document_anonymous_user(): client = APIClient() response = client.post( - f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/{action}/", ) assert response.status_code == 401 -def test_api_documents_threads_resolve_public_document_authenticated_user(): +@pytest.mark.parametrize("action", ["resolve", "unresolve"]) +def test_api_documents_threads_resolve_unresolve_public_document_authenticated_user( + action, +): """ - Authenticated users should not be allowed to resolve threads on public documents. + Authenticated users should not be allowed to resolve or unresolve threads on public documents. """ user = factories.UserFactory() document = factories.DocumentFactory( @@ -1057,14 +1068,18 @@ def test_api_documents_threads_resolve_public_document_authenticated_user(): client = APIClient() client.force_login(user) response = client.post( - f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/{action}/", ) assert response.status_code == 403 -def test_api_documents_threads_resolve_authenticated_document_anonymous_user(): +@pytest.mark.parametrize("action", ["resolve", "unresolve"]) +def test_api_documents_threads_resolve_unresolve_authenticated_document_anonymous_user( + action, +): """ - Anonymous users should not be allowed to resolve threads on authenticated documents. + Anonymous users should not be allowed to resolve or unresolve threads on authenticated + documents. """ document = factories.DocumentFactory( link_reach="authenticated", @@ -1076,14 +1091,17 @@ def test_api_documents_threads_resolve_authenticated_document_anonymous_user(): client = APIClient() response = client.post( - f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/{action}/", ) assert response.status_code == 401 -def test_api_documents_threads_resolve_authenticated_document_reader_role(): +@pytest.mark.parametrize("action", ["resolve", "unresolve"]) +def test_api_documents_threads_resolve_unresolve_authenticated_document_reader_role( + action, +): """ - Authenticated users should not be allowed to resolve threads on authenticated + Authenticated users should not be allowed to resolve or unresolve threads on authenticated documents with reader link_role. """ user = factories.UserFactory() @@ -1098,18 +1116,21 @@ def test_api_documents_threads_resolve_authenticated_document_reader_role(): client = APIClient() client.force_login(user) response = client.post( - f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/{action}/", ) assert response.status_code == 403 +@pytest.mark.parametrize("action", ["resolve", "unresolve"]) @pytest.mark.parametrize( "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] ) -def test_api_documents_threads_resolve_authenticated_document(link_role): +def test_api_documents_threads_resolve_unresolve_authenticated_document( + link_role, action +): """ - Authenticated users should not be allowed to resolve threads on authenticated documents with - commenter or editor link_role. + Authenticated users should not be allowed to resolve or unresolve threads on authenticated + documents with commenter or editor link_role. """ user = factories.UserFactory() document = factories.DocumentFactory( @@ -1123,14 +1144,17 @@ def test_api_documents_threads_resolve_authenticated_document(link_role): client = APIClient() client.force_login(user) response = client.post( - f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/{action}/", ) assert response.status_code == 403 -def test_api_documents_threads_resolve_restricted_document_anonymous_user(): +@pytest.mark.parametrize("action", ["resolve", "unresolve"]) +def test_api_documents_threads_resolve_unresolve_restricted_document_anonymous_user( + action, +): """ - Anonymous users should not be allowed to resolve threads on restricted documents. + Anonymous users should not be allowed to resolve or unresolve threads on restricted documents. """ document = factories.DocumentFactory( link_reach="restricted", @@ -1142,15 +1166,18 @@ def test_api_documents_threads_resolve_restricted_document_anonymous_user(): client = APIClient() response = client.post( - f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/{action}/", ) assert response.status_code == 401 -def test_api_documents_threads_resolve_restricted_document_reader_role(): +@pytest.mark.parametrize("action", ["resolve", "unresolve"]) +def test_api_documents_threads_resolve_unresolve_restricted_document_reader_role( + action, +): """ - Authenticated users should not be allowed to resolve threads on restricted documents with - reader roles. + Authenticated users should not be allowed to resolve or unresolve threads on restricted + documents with reader roles. """ user = factories.UserFactory() document = factories.DocumentFactory( @@ -1165,18 +1192,21 @@ def test_api_documents_threads_resolve_restricted_document_reader_role(): client = APIClient() client.force_login(user) response = client.post( - f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/{action}/", ) assert response.status_code == 403 +@pytest.mark.parametrize("action", ["resolve", "unresolve"]) @pytest.mark.parametrize( "role", [models.RoleChoices.COMMENTER, models.RoleChoices.EDITOR] ) -def test_api_documents_threads_resolve_restricted_document_editor(role): +def test_api_documents_threads_resolve_unresolve_restricted_document_editor( + role, action +): """ - Authenticated users should not be allowed to resolve threads on restricted documents with - editor roles. + Authenticated users should not be allowed to resolve or unresolve threads on restricted + documents with commenter or editor roles. """ user = factories.UserFactory() document = factories.DocumentFactory( @@ -1191,7 +1221,7 @@ def test_api_documents_threads_resolve_restricted_document_editor(role): client = APIClient() client.force_login(user) response = client.post( - f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/{action}/", ) assert response.status_code == 403 @@ -1219,8 +1249,36 @@ def test_api_documents_threads_resolve_restricted_document_privileged_roles(role ) assert response.status_code == 204 - # Verify thread is resolved thread.refresh_from_db() assert thread.resolved is True assert thread.resolved_at is not None assert thread.resolved_by == user + + +@pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) +def test_api_documents_threads_unresolve_restricted_document_privileged_roles(role): + """ + Authenticated users with privileged roles should be allowed to unresolve threads on + restricted documents. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + thread = factories.ThreadFactory(document=document, creator=None, resolved=True) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/unresolve/", + ) + assert response.status_code == 204 + + thread.refresh_from_db() + assert thread.resolved is False + assert thread.resolved_at is None + assert thread.resolved_by is None diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStore.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStore.tsx index 9a86bd86a..4fc1a2131 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStore.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStore.tsx @@ -534,11 +534,6 @@ export class DocsThreadStore extends ThreadStore { this.ping(threadId); }; - /** - * Todo: Not implemented backend side - * @returns - * @throws - */ public unresolveThread = async (_options: { threadId: string }) => { const response = await fetchAPI( `documents/${this.docId}/threads/${_options.threadId}/unresolve/`, diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStoreAuth.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStoreAuth.tsx index 57f614813..068a0172c 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStoreAuth.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStoreAuth.tsx @@ -53,12 +53,11 @@ export class DocsThreadStoreAuth extends ThreadStoreAuth { return false; } - /** - * Not implemented backend side - * @param _thread - * @returns - */ - canUnresolveThread(_thread: ClientThreadData): boolean { + canUnresolveThread(thread: ClientThreadData): boolean { + if (thread.metadata.abilities.unresolve) { + return true; + } + return false; } diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/types.ts b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/types.ts index be4781657..5cc37c810 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/types.ts +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/types.ts @@ -15,6 +15,7 @@ export interface ThreadAbilities { partial_update: boolean; retrieve: boolean; resolve: boolean; + unresolve: boolean; } export interface ServerReaction {