From 16df53d104cf44c94219f685f93f1a634299c9fa Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Tue, 26 Aug 2025 17:55:53 +0200 Subject: [PATCH 01/10] =?UTF-8?q?=E2=9C=A8(backend)=20add=20commentator=20?= =?UTF-8?q?role?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To allow a user to comment a document we added a new role: commentator. Commentator is higher than reader but lower than editor. --- src/backend/core/choices.py | 2 + src/backend/core/models.py | 9 +- .../documents/test_api_document_accesses.py | 39 ++-- .../documents/test_api_documents_retrieve.py | 15 +- .../documents/test_api_documents_trashbin.py | 7 +- .../tests/test_models_document_accesses.py | 18 +- .../core/tests/test_models_documents.py | 221 +++++++++++++++--- 7 files changed, 249 insertions(+), 62 deletions(-) diff --git a/src/backend/core/choices.py b/src/backend/core/choices.py index e6b975111a..8505ebab87 100644 --- a/src/backend/core/choices.py +++ b/src/backend/core/choices.py @@ -33,6 +33,7 @@ class LinkRoleChoices(PriorityTextChoices): """Defines the possible roles a link can offer on a document.""" READER = "reader", _("Reader") # Can read + COMMENTATOR = "commentator", _("Commentator") # Can read and comment EDITOR = "editor", _("Editor") # Can read and edit @@ -40,6 +41,7 @@ class RoleChoices(PriorityTextChoices): """Defines the possible roles a user can have in a resource.""" READER = "reader", _("Reader") # Can read + COMMENTATOR = "commentator", _("Commentator") # Can read and comment EDITOR = "editor", _("Editor") # Can read and edit ADMIN = "administrator", _("Administrator") # Can read, edit, delete and share OWNER = "owner", _("Owner") diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 9b8e13fec1..4243d6c43c 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -753,6 +753,7 @@ def get_abilities(self, user): can_update = ( is_owner_or_admin or role == RoleChoices.EDITOR ) and not is_deleted + can_comment = (can_update or role == RoleChoices.COMMENTATOR) and not is_deleted can_create_children = can_update and user.is_authenticated can_destroy = ( is_owner @@ -783,6 +784,7 @@ def get_abilities(self, user): "children_list": can_get, "children_create": can_create_children, "collaboration_auth": can_get, + "comment": can_comment, "content": can_get, "cors_proxy": can_get, "descendants": can_get, @@ -1143,7 +1145,12 @@ def get_abilities(self, user): set_role_to = [] if is_owner_or_admin: set_role_to.extend( - [RoleChoices.READER, RoleChoices.EDITOR, RoleChoices.ADMIN] + [ + RoleChoices.READER, + RoleChoices.COMMENTATOR, + RoleChoices.EDITOR, + RoleChoices.ADMIN, + ] ) if role == RoleChoices.OWNER: set_role_to.append(RoleChoices.OWNER) diff --git a/src/backend/core/tests/documents/test_api_document_accesses.py b/src/backend/core/tests/documents/test_api_document_accesses.py index 280b2bc921..cdb3d4d63a 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses.py +++ b/src/backend/core/tests/documents/test_api_document_accesses.py @@ -293,6 +293,7 @@ def test_api_document_accesses_retrieve_set_role_to_child(): } assert result_dict[str(document_access_other_user.id)] == [ "reader", + "commentator", "editor", "administrator", "owner", @@ -301,7 +302,7 @@ def test_api_document_accesses_retrieve_set_role_to_child(): # Add an access for the other user on the parent parent_access_other_user = factories.UserDocumentAccessFactory( - document=parent, user=other_user, role="editor" + document=parent, user=other_user, role="commentator" ) response = client.get(f"/api/v1.0/documents/{document.id!s}/accesses/") @@ -314,6 +315,7 @@ def test_api_document_accesses_retrieve_set_role_to_child(): result["id"]: result["abilities"]["set_role_to"] for result in content } assert result_dict[str(document_access_other_user.id)] == [ + "commentator", "editor", "administrator", "owner", @@ -321,6 +323,7 @@ def test_api_document_accesses_retrieve_set_role_to_child(): assert result_dict[str(parent_access.id)] == [] assert result_dict[str(parent_access_other_user.id)] == [ "reader", + "commentator", "editor", "administrator", "owner", @@ -333,28 +336,28 @@ def test_api_document_accesses_retrieve_set_role_to_child(): [ ["administrator", "reader", "reader", "reader"], [ - ["reader", "editor", "administrator"], + ["reader", "commentator", "editor", "administrator"], [], [], - ["reader", "editor", "administrator"], + ["reader", "commentator", "editor", "administrator"], ], ], [ ["owner", "reader", "reader", "reader"], [ - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], [], [], - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], ], ], [ ["owner", "reader", "reader", "owner"], [ - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], [], [], - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], ], ], ], @@ -415,44 +418,44 @@ def test_api_document_accesses_list_authenticated_related_same_user(roles, resul [ ["administrator", "reader", "reader", "reader"], [ - ["reader", "editor", "administrator"], + ["reader", "commentator", "editor", "administrator"], [], [], - ["reader", "editor", "administrator"], + ["reader", "commentator", "editor", "administrator"], ], ], [ ["owner", "reader", "reader", "reader"], [ - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], [], [], - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], ], ], [ ["owner", "reader", "reader", "owner"], [ - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], [], [], - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], ], ], [ ["reader", "reader", "reader", "owner"], [ - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], [], [], - ["reader", "editor", "administrator", "owner"], + ["reader", "commentator", "editor", "administrator", "owner"], ], ], [ ["reader", "administrator", "reader", "editor"], [ - ["reader", "editor", "administrator"], - ["reader", "editor", "administrator"], + ["reader", "commentator", "editor", "administrator"], + ["reader", "commentator", "editor", "administrator"], [], [], ], @@ -460,7 +463,7 @@ def test_api_document_accesses_list_authenticated_related_same_user(roles, resul [ ["editor", "editor", "administrator", "editor"], [ - ["reader", "editor", "administrator"], + ["reader", "commentator", "editor", "administrator"], [], ["editor", "administrator"], [], 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 d1f8e1f031..6046338d60 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -36,6 +36,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "children_create": False, "children_list": True, "collaboration_auth": True, + "comment": document.link_role in ["commentator", "editor"], "cors_proxy": True, "content": True, "descendants": True, @@ -46,8 +47,8 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": False, @@ -112,6 +113,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "children_create": False, "children_list": True, "collaboration_auth": True, + "comment": grand_parent.link_role in ["commentator", "editor"], "descendants": True, "cors_proxy": True, "content": True, @@ -218,6 +220,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "children_create": document.link_role == "editor", "children_list": True, "collaboration_auth": True, + "comment": document.link_role in ["commentator", "editor"], "descendants": True, "cors_proxy": True, "content": True, @@ -227,8 +230,8 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": True, @@ -301,6 +304,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "children_create": grand_parent.link_role == "editor", "children_list": True, "collaboration_auth": True, + "comment": grand_parent.link_role in ["commentator", "editor"], "descendants": True, "cors_proxy": True, "content": True, @@ -492,10 +496,11 @@ def test_api_documents_retrieve_authenticated_related_parent(): "ai_transform": access.role != "reader", "ai_translate": access.role != "reader", "attachment_upload": access.role != "reader", - "can_edit": access.role != "reader", + "can_edit": access.role not in ["reader", "commentator"], "children_create": access.role != "reader", "children_list": True, "collaboration_auth": True, + "comment": access.role != "reader", "descendants": True, "cors_proxy": True, "content": True, diff --git a/src/backend/core/tests/documents/test_api_documents_trashbin.py b/src/backend/core/tests/documents/test_api_documents_trashbin.py index 28ea6a01af..998d283a69 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -79,8 +79,9 @@ def test_api_documents_trashbin_format(): "children_create": True, "children_list": True, "collaboration_auth": True, - "descendants": True, + "comment": True, "cors_proxy": True, + "descendants": True, "content": True, "destroy": True, "duplicate": True, @@ -88,8 +89,8 @@ def test_api_documents_trashbin_format(): "invite_owner": True, "link_configuration": True, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": True, diff --git a/src/backend/core/tests/test_models_document_accesses.py b/src/backend/core/tests/test_models_document_accesses.py index 2fa88cf1fb..eb7675c00c 100644 --- a/src/backend/core/tests/test_models_document_accesses.py +++ b/src/backend/core/tests/test_models_document_accesses.py @@ -123,7 +123,7 @@ def test_models_document_access_get_abilities_for_owner_of_self_allowed(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], } @@ -166,7 +166,7 @@ def test_models_document_access_get_abilities_for_owner_of_self_last_on_child( "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], } @@ -183,7 +183,7 @@ def test_models_document_access_get_abilities_for_owner_of_owner(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], } @@ -200,7 +200,7 @@ def test_models_document_access_get_abilities_for_owner_of_administrator(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], } @@ -217,7 +217,7 @@ def test_models_document_access_get_abilities_for_owner_of_editor(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], } @@ -234,7 +234,7 @@ def test_models_document_access_get_abilities_for_owner_of_reader(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], } @@ -271,7 +271,7 @@ def test_models_document_access_get_abilities_for_administrator_of_administrator "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator"], + "set_role_to": ["reader", "commentator", "editor", "administrator"], } @@ -288,7 +288,7 @@ def test_models_document_access_get_abilities_for_administrator_of_editor(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator"], + "set_role_to": ["reader", "commentator", "editor", "administrator"], } @@ -305,7 +305,7 @@ def test_models_document_access_get_abilities_for_administrator_of_reader(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "editor", "administrator"], + "set_role_to": ["reader", "commentator", "editor", "administrator"], } diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index cc760aff33..84f65a02ee 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -134,10 +134,13 @@ def test_models_documents_soft_delete(depth): [ (True, "restricted", "reader"), (True, "restricted", "editor"), + (True, "restricted", "commentator"), (False, "restricted", "reader"), (False, "restricted", "editor"), + (False, "restricted", "commentator"), (False, "authenticated", "reader"), (False, "authenticated", "editor"), + (False, "authenticated", "commentator"), ], ) def test_models_documents_get_abilities_forbidden( @@ -165,6 +168,7 @@ def test_models_documents_get_abilities_forbidden( "destroy": False, "duplicate": False, "favorite": False, + "comment": False, "invite_owner": False, "mask": False, "media_auth": False, @@ -172,8 +176,8 @@ def test_models_documents_get_abilities_forbidden( "move": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "partial_update": False, @@ -223,6 +227,7 @@ def test_models_documents_get_abilities_reader( "children_create": False, "children_list": True, "collaboration_auth": True, + "comment": False, "descendants": True, "cors_proxy": True, "content": True, @@ -232,8 +237,77 @@ def test_models_documents_get_abilities_reader( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], + "restricted": None, + }, + "mask": is_authenticated, + "media_auth": True, + "media_check": True, + "move": False, + "partial_update": False, + "restore": False, + "retrieve": True, + "tree": True, + "update": False, + "versions_destroy": False, + "versions_list": False, + "versions_retrieve": False, + } + nb_queries = 1 if is_authenticated else 0 + with django_assert_num_queries(nb_queries): + assert document.get_abilities(user) == expected_abilities + + document.soft_delete() + document.refresh_from_db() + assert all( + value is False + for key, value in document.get_abilities(user).items() + if key not in ["link_select_options", "ancestors_links_definition"] + ) + + +@override_settings( + AI_ALLOW_REACH_FROM=random.choice(["public", "authenticated", "restricted"]) +) +@pytest.mark.parametrize( + "is_authenticated,reach", + [ + (True, "public"), + (False, "public"), + (True, "authenticated"), + ], +) +def test_models_documents_get_abilities_commentator( + is_authenticated, reach, django_assert_num_queries +): + """ + Check abilities returned for a document giving commentator role to link holders + i.e anonymous users or authenticated users who have no specific role on the document. + """ + document = factories.DocumentFactory(link_reach=reach, link_role="commentator") + user = factories.UserFactory() if is_authenticated else AnonymousUser() + expected_abilities = { + "accesses_manage": False, + "accesses_view": False, + "ai_transform": False, + "ai_translate": False, + "attachment_upload": False, + "can_edit": False, + "children_create": False, + "children_list": True, + "collaboration_auth": True, + "comment": True, + "descendants": True, + "cors_proxy": True, + "destroy": False, + "duplicate": is_authenticated, + "favorite": is_authenticated, + "invite_owner": False, + "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": is_authenticated, @@ -289,6 +363,7 @@ def test_models_documents_get_abilities_editor( "children_create": is_authenticated, "children_list": True, "collaboration_auth": True, + "comment": True, "descendants": True, "cors_proxy": True, "content": True, @@ -298,8 +373,8 @@ def test_models_documents_get_abilities_editor( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": is_authenticated, @@ -344,6 +419,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "children_create": True, "children_list": True, "collaboration_auth": True, + "comment": True, "descendants": True, "cors_proxy": True, "content": True, @@ -353,8 +429,8 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "invite_owner": True, "link_configuration": True, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": True, @@ -396,6 +472,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "children_create": True, "children_list": True, "collaboration_auth": True, + "comment": True, "descendants": True, "cors_proxy": True, "content": True, @@ -405,8 +482,8 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "invite_owner": False, "link_configuration": True, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": True, @@ -451,6 +528,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "children_create": True, "children_list": True, "collaboration_auth": True, + "comment": True, "descendants": True, "cors_proxy": True, "content": True, @@ -460,8 +538,8 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": True, @@ -513,6 +591,8 @@ def test_models_documents_get_abilities_reader_user( "children_create": access_from_link, "children_list": True, "collaboration_auth": True, + "comment": document.link_reach != "restricted" + and document.link_role in ["commentator", "editor"], "descendants": True, "cors_proxy": True, "content": True, @@ -522,8 +602,72 @@ def test_models_documents_get_abilities_reader_user( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], + "restricted": None, + }, + "mask": True, + "media_auth": True, + "media_check": True, + "move": False, + "partial_update": access_from_link, + "restore": False, + "retrieve": True, + "tree": True, + "update": access_from_link, + "versions_destroy": False, + "versions_list": True, + "versions_retrieve": True, + } + + with override_settings(AI_ALLOW_REACH_FROM=ai_access_setting): + with django_assert_num_queries(1): + assert document.get_abilities(user) == expected_abilities + + document.soft_delete() + document.refresh_from_db() + assert all( + value is False + for key, value in document.get_abilities(user).items() + if key not in ["link_select_options", "ancestors_links_definition"] + ) + + +@pytest.mark.parametrize("ai_access_setting", ["public", "authenticated", "restricted"]) +def test_models_documents_get_abilities_commentator_user( + ai_access_setting, django_assert_num_queries +): + """Check abilities returned for the commentator of a document.""" + user = factories.UserFactory() + document = factories.DocumentFactory(users=[(user, "commentator")]) + + access_from_link = ( + document.link_reach != "restricted" and document.link_role == "editor" + ) + + expected_abilities = { + "accesses_manage": False, + "accesses_view": True, + # If you get your editor rights from the link role and not your access role + # You should not access AI if it's restricted to users with specific access + "ai_transform": access_from_link and ai_access_setting != "restricted", + "ai_translate": access_from_link and ai_access_setting != "restricted", + "attachment_upload": access_from_link, + "can_edit": access_from_link, + "children_create": access_from_link, + "children_list": True, + "collaboration_auth": True, + "comment": True, + "descendants": True, + "cors_proxy": True, + "destroy": False, + "duplicate": True, + "favorite": True, + "invite_owner": False, + "link_configuration": False, + "link_select_options": { + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": True, @@ -573,6 +717,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "children_create": False, "children_list": True, "collaboration_auth": True, + "comment": False, "descendants": True, "cors_proxy": True, "content": True, @@ -582,8 +727,8 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], "restricted": None, }, "mask": True, @@ -1286,7 +1431,14 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): "public", "reader", { - "public": ["reader", "editor"], + "public": ["reader", "commentator", "editor"], + }, + ), + ( + "public", + "commentator", + { + "public": ["commentator", "editor"], }, ), ("public", "editor", {"public": ["editor"]}), @@ -1294,8 +1446,16 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): "authenticated", "reader", { - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], + }, + ), + ( + "authenticated", + "commentator", + { + "authenticated": ["commentator", "editor"], + "public": ["commentator", "editor"], }, ), ( @@ -1308,8 +1468,17 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): "reader", { "restricted": None, - "authenticated": ["reader", "editor"], - "public": ["reader", "editor"], + "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commentator", "editor"], + }, + ), + ( + "restricted", + "commentator", + { + "restricted": None, + "authenticated": ["commentator", "editor"], + "public": ["commentator", "editor"], }, ), ( @@ -1326,15 +1495,15 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): "public", None, { - "public": ["reader", "editor"], + "public": ["reader", "commentator", "editor"], }, ), ( None, "reader", { - "public": ["reader", "editor"], - "authenticated": ["reader", "editor"], + "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commentator", "editor"], "restricted": None, }, ), @@ -1342,8 +1511,8 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): None, None, { - "public": ["reader", "editor"], - "authenticated": ["reader", "editor"], + "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commentator", "editor"], "restricted": None, }, ), From 908e929cec96e5d3141ea7844c0f1a88319cde78 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Wed, 27 Aug 2025 16:38:42 +0200 Subject: [PATCH 02/10] =?UTF-8?q?=E2=9C=A8(backend)=20add=20Comment=20mode?= =?UTF-8?q?l?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to store the comments on a document, we created a new model Comment. User is nullable because anonymous users can comment a Document is this one is public with a link_role commentator. --- src/backend/core/factories.py | 11 + ...role_alter_documentaccess_role_and_more.py | 146 ++++++++++ src/backend/core/models.py | 42 +++ src/backend/core/tests/test_models_comment.py | 273 ++++++++++++++++++ 4 files changed, 472 insertions(+) create mode 100644 src/backend/core/migrations/0025_alter_document_link_role_alter_documentaccess_role_and_more.py create mode 100644 src/backend/core/tests/test_models_comment.py diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index 1b3715e749..24bdd317e9 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -256,3 +256,14 @@ class Meta: document = factory.SubFactory(DocumentFactory) role = factory.fuzzy.FuzzyChoice([role[0] for role in models.RoleChoices.choices]) issuer = factory.SubFactory(UserFactory) + + +class CommentFactory(factory.django.DjangoModelFactory): + """A factory to create comments for a document""" + + class Meta: + model = models.Comment + + document = factory.SubFactory(DocumentFactory) + user = factory.SubFactory(UserFactory) + content = factory.Faker("text") diff --git a/src/backend/core/migrations/0025_alter_document_link_role_alter_documentaccess_role_and_more.py b/src/backend/core/migrations/0025_alter_document_link_role_alter_documentaccess_role_and_more.py new file mode 100644 index 0000000000..a34ad05b88 --- /dev/null +++ b/src/backend/core/migrations/0025_alter_document_link_role_alter_documentaccess_role_and_more.py @@ -0,0 +1,146 @@ +# Generated by Django 5.2.4 on 2025-08-26 08:11 + +import uuid + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0024_add_is_masked_field_to_link_trace"), + ] + + operations = [ + migrations.AlterField( + model_name="document", + name="link_role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commentator", "Commentator"), + ("editor", "Editor"), + ], + default="reader", + max_length=20, + ), + ), + migrations.AlterField( + model_name="documentaccess", + name="role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commentator", "Commentator"), + ("editor", "Editor"), + ("administrator", "Administrator"), + ("owner", "Owner"), + ], + default="reader", + max_length=20, + ), + ), + migrations.AlterField( + model_name="documentaskforaccess", + name="role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commentator", "Commentator"), + ("editor", "Editor"), + ("administrator", "Administrator"), + ("owner", "Owner"), + ], + default="reader", + max_length=20, + ), + ), + migrations.AlterField( + model_name="invitation", + name="role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commentator", "Commentator"), + ("editor", "Editor"), + ("administrator", "Administrator"), + ("owner", "Owner"), + ], + default="reader", + max_length=20, + ), + ), + migrations.AlterField( + model_name="templateaccess", + name="role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commentator", "Commentator"), + ("editor", "Editor"), + ("administrator", "Administrator"), + ("owner", "Owner"), + ], + default="reader", + max_length=20, + ), + ), + migrations.CreateModel( + name="Comment", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, + editable=False, + help_text="primary key for the record as UUID", + primary_key=True, + serialize=False, + verbose_name="id", + ), + ), + ( + "created_at", + models.DateTimeField( + auto_now_add=True, + help_text="date and time at which a record was created", + verbose_name="created on", + ), + ), + ( + "updated_at", + models.DateTimeField( + auto_now=True, + help_text="date and time at which a record was last updated", + verbose_name="updated on", + ), + ), + ("content", models.TextField()), + ( + "document", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="comments", + to="core.document", + ), + ), + ( + "user", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="comments", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "verbose_name": "Comment", + "verbose_name_plural": "Comments", + "db_table": "impress_comment", + "ordering": ("-created_at",), + }, + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 4243d6c43c..af08814a11 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1282,6 +1282,48 @@ def send_ask_for_access_email(self, email, language=None): self.document.send_email(subject, [email], context, language) +class Comment(BaseModel): + """User comment on a document.""" + + document = models.ForeignKey( + Document, + on_delete=models.CASCADE, + related_name="comments", + ) + user = models.ForeignKey( + User, + on_delete=models.SET_NULL, + related_name="comments", + null=True, + blank=True, + ) + content = models.TextField() + + class Meta: + db_table = "impress_comment" + ordering = ("-created_at",) + verbose_name = _("Comment") + verbose_name_plural = _("Comments") + + def __str__(self): + author = self.user or _("Anonymous") + return f"{author!s} on {self.document!s}" + + def get_abilities(self, user): + """Compute and return abilities for a given user.""" + role = self.document.get_role(user) + can_comment = self.document.get_abilities(user)["comment"] + return { + "destroy": self.user == user + or role in [RoleChoices.OWNER, RoleChoices.ADMIN], + "update": self.user == user + or role in [RoleChoices.OWNER, RoleChoices.ADMIN], + "partial_update": self.user == user + or role in [RoleChoices.OWNER, RoleChoices.ADMIN], + "retrieve": can_comment, + } + + class Template(BaseModel): """HTML and CSS code used for formatting the print around the MarkDown body.""" diff --git a/src/backend/core/tests/test_models_comment.py b/src/backend/core/tests/test_models_comment.py new file mode 100644 index 0000000000..dac0b36c22 --- /dev/null +++ b/src/backend/core/tests/test_models_comment.py @@ -0,0 +1,273 @@ +"""Test the comment model.""" + +import random + +from django.contrib.auth.models import AnonymousUser + +import pytest + +from core import factories +from core.models import LinkReachChoices, LinkRoleChoices, RoleChoices + +pytestmark = pytest.mark.django_db + + +@pytest.mark.parametrize( + "role,can_comment", + [ + (LinkRoleChoices.READER, False), + (LinkRoleChoices.COMMENTATOR, True), + (LinkRoleChoices.EDITOR, True), + ], +) +def test_comment_get_abilities_anonymous_user_public_document(role, can_comment): + """Anonymous users cannot comment on a document.""" + document = factories.DocumentFactory( + link_role=role, link_reach=LinkReachChoices.PUBLIC + ) + comment = factories.CommentFactory(document=document) + user = AnonymousUser() + + assert comment.get_abilities(user) == { + "destroy": False, + "update": False, + "partial_update": False, + "retrieve": can_comment, + } + + +@pytest.mark.parametrize( + "link_reach", [LinkReachChoices.RESTRICTED, LinkReachChoices.AUTHENTICATED] +) +def test_comment_get_abilities_anonymous_user_restricted_document(link_reach): + """Anonymous users cannot comment on a restricted document.""" + document = factories.DocumentFactory(link_reach=link_reach) + comment = factories.CommentFactory(document=document) + user = AnonymousUser() + + assert comment.get_abilities(user) == { + "destroy": False, + "update": False, + "partial_update": False, + "retrieve": False, + } + + +@pytest.mark.parametrize( + "link_role,link_reach,can_comment", + [ + (LinkRoleChoices.READER, LinkReachChoices.PUBLIC, False), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC, True), + (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC, True), + (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED, False), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED, False), + (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED, False), + (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED, False), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED, True), + (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED, True), + ], +) +def test_comment_get_abilities_user_reader(link_role, link_reach, can_comment): + """Readers cannot comment on a document.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_role=link_role, link_reach=link_reach, users=[(user, RoleChoices.READER)] + ) + comment = factories.CommentFactory(document=document) + + assert comment.get_abilities(user) == { + "destroy": False, + "update": False, + "partial_update": False, + "retrieve": can_comment, + } + + +@pytest.mark.parametrize( + "link_role,link_reach,can_comment", + [ + (LinkRoleChoices.READER, LinkReachChoices.PUBLIC, False), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC, True), + (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC, True), + (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED, False), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED, False), + (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED, False), + (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED, False), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED, True), + (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED, True), + ], +) +def test_comment_get_abilities_user_reader_own_comment( + link_role, link_reach, can_comment +): + """User with reader role on a document has all accesses to its own comment.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_role=link_role, link_reach=link_reach, users=[(user, RoleChoices.READER)] + ) + comment = factories.CommentFactory( + document=document, user=user if can_comment else None + ) + + assert comment.get_abilities(user) == { + "destroy": can_comment, + "update": can_comment, + "partial_update": can_comment, + "retrieve": can_comment, + } + + +@pytest.mark.parametrize( + "link_role,link_reach", + [ + (LinkRoleChoices.READER, LinkReachChoices.PUBLIC), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED), + ], +) +def test_comment_get_abilities_user_commentator(link_role, link_reach): + """Commentators can comment on a document.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_role=link_role, + link_reach=link_reach, + users=[(user, RoleChoices.COMMENTATOR)], + ) + comment = factories.CommentFactory(document=document) + + assert comment.get_abilities(user) == { + "destroy": False, + "update": False, + "partial_update": False, + "retrieve": True, + } + + +@pytest.mark.parametrize( + "link_role,link_reach", + [ + (LinkRoleChoices.READER, LinkReachChoices.PUBLIC), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED), + ], +) +def test_comment_get_abilities_user_commentator_own_comment(link_role, link_reach): + """Commentators have all accesses to its own comment.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_role=link_role, + link_reach=link_reach, + users=[(user, RoleChoices.COMMENTATOR)], + ) + comment = factories.CommentFactory(document=document, user=user) + + assert comment.get_abilities(user) == { + "destroy": True, + "update": True, + "partial_update": True, + "retrieve": True, + } + + +@pytest.mark.parametrize( + "link_role,link_reach", + [ + (LinkRoleChoices.READER, LinkReachChoices.PUBLIC), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED), + ], +) +def test_comment_get_abilities_user_editor(link_role, link_reach): + """Editors can comment on a document.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_role=link_role, link_reach=link_reach, users=[(user, RoleChoices.EDITOR)] + ) + comment = factories.CommentFactory(document=document) + + assert comment.get_abilities(user) == { + "destroy": False, + "update": False, + "partial_update": False, + "retrieve": True, + } + + +@pytest.mark.parametrize( + "link_role,link_reach", + [ + (LinkRoleChoices.READER, LinkReachChoices.PUBLIC), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED), + ], +) +def test_comment_get_abilities_user_editor_own_comment(link_role, link_reach): + """Editors have all accesses to its own comment.""" + user = factories.UserFactory() + document = factories.DocumentFactory( + link_role=link_role, link_reach=link_reach, users=[(user, RoleChoices.EDITOR)] + ) + comment = factories.CommentFactory(document=document, user=user) + + assert comment.get_abilities(user) == { + "destroy": True, + "update": True, + "partial_update": True, + "retrieve": True, + } + + +def test_comment_get_abilities_user_admin(): + """Admins have all accesses to a comment.""" + user = factories.UserFactory() + document = factories.DocumentFactory(users=[(user, RoleChoices.ADMIN)]) + comment = factories.CommentFactory( + document=document, user=random.choice([user, None]) + ) + + assert comment.get_abilities(user) == { + "destroy": True, + "update": True, + "partial_update": True, + "retrieve": True, + } + + +def test_comment_get_abilities_user_owner(): + """Owners have all accesses to a comment.""" + user = factories.UserFactory() + document = factories.DocumentFactory(users=[(user, RoleChoices.OWNER)]) + comment = factories.CommentFactory( + document=document, user=random.choice([user, None]) + ) + + assert comment.get_abilities(user) == { + "destroy": True, + "update": True, + "partial_update": True, + "retrieve": True, + } From 89f6135169da44819f03964c51d564957de2b643 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Thu, 28 Aug 2025 08:21:35 +0200 Subject: [PATCH 03/10] =?UTF-8?q?=E2=9C=A8(backend)=20add=20comment=20view?= =?UTF-8?q?set?= 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 6dc38d58ef..e67ebfddfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,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 09007847bf..29df311c82 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -171,3 +171,19 @@ def has_object_permission(self, request, view, obj): 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 a4e5d9d8e3..1296c2ae59 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -877,3 +877,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 1fb95c4eb6..b61d000acb 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -2201,3 +2201,36 @@ def _load_theme_customization(self): ) 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 0000000000..2a0cb7ced7 --- /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 2ad8b00395..2df79fcc4a 100644 --- a/src/backend/core/urls.py +++ b/src/backend/core/urls.py @@ -26,7 +26,11 @@ viewsets.InvitationViewset, basename="invitations", ) - +document_related_router.register( + "comments", + viewsets.CommentViewSet, + basename="comments", +) document_related_router.register( "ask-for-access", viewsets.DocumentAskForAccessViewSet, From 47f97df3b7540d6ab2376ad4c98b8cba7b2ccaa4 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 12 Sep 2025 15:28:25 +0200 Subject: [PATCH 04/10] =?UTF-8?q?=E2=9C=A8(backend)=20implement=20thread?= =?UTF-8?q?=20and=20reactions=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to use comment we also have to implement a thread and reactions API. A thread has multiple comments and comments can have multiple reactions. --- src/backend/core/api/serializers.py | 101 +- src/backend/core/api/viewsets.py | 121 +- src/backend/core/choices.py | 4 +- src/backend/core/factories.py | 41 +- ...role_alter_documentaccess_role_and_more.py | 146 -- src/backend/core/migrations/0025_comments.py | 275 ++++ src/backend/core/models.py | 141 +- .../documents/test_api_document_accesses.py | 42 +- .../documents/test_api_documents_comments.py | 516 +++++-- .../documents/test_api_documents_retrieve.py | 18 +- .../documents/test_api_documents_threads.py | 1226 +++++++++++++++++ .../documents/test_api_documents_trashbin.py | 4 +- src/backend/core/tests/test_models_comment.py | 80 +- .../tests/test_models_document_accesses.py | 18 +- .../core/tests/test_models_documents.py | 98 +- src/backend/core/urls.py | 17 +- 16 files changed, 2415 insertions(+), 433 deletions(-) delete mode 100644 src/backend/core/migrations/0025_alter_document_link_role_alter_documentaccess_role_and_more.py create mode 100644 src/backend/core/migrations/0025_comments.py create mode 100644 src/backend/core/tests/documents/test_api_documents_threads.py diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 1296c2ae59..2b1c4dd930 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -879,45 +879,122 @@ class MoveDocumentSerializer(serializers.Serializer): ) +class ReactionSerializer(serializers.ModelSerializer): + """Serialize reactions.""" + + users = UserLightSerializer(many=True, read_only=True) + + class Meta: + model = models.Reaction + fields = [ + "id", + "emoji", + "created_at", + "users", + ] + read_only_fields = ["id", "created_at", "users"] + + class CommentSerializer(serializers.ModelSerializer): - """Serialize comments.""" + """Serialize comments (nested under a thread) with reactions and abilities.""" user = UserLightSerializer(read_only=True) - abilities = serializers.SerializerMethodField(read_only=True) + abilities = serializers.SerializerMethodField() + reactions = ReactionSerializer(many=True, read_only=True) class Meta: model = models.Comment fields = [ "id", - "content", + "user", + "body", "created_at", "updated_at", - "user", - "document", + "reactions", "abilities", ] read_only_fields = [ "id", + "user", "created_at", "updated_at", - "user", - "document", + "reactions", "abilities", ] - def get_abilities(self, comment) -> dict: - """Return abilities of the logged-in user on the instance.""" + def validate(self, attrs): + """Validate comment data.""" + + request = self.context.get("request") + user = getattr(request, "user", None) + + attrs["thread_id"] = self.context["thread_id"] + attrs["user_id"] = user.id if user else None + return attrs + + def get_abilities(self, obj): + """Return comment's abilities.""" request = self.context.get("request") if request: - return comment.get_abilities(request.user) + return obj.get_abilities(request.user) return {} + +class ThreadSerializer(serializers.ModelSerializer): + """Serialize threads in a backward compatible shape for current frontend. + + We expose a flatten representation where ``content`` maps to the first + comment's body. Creating a thread requires a ``content`` field which is + stored as the first comment. + """ + + creator = UserLightSerializer(read_only=True) + abilities = serializers.SerializerMethodField(read_only=True) + body = serializers.JSONField(write_only=True, required=True) + comments = serializers.SerializerMethodField(read_only=True) + comments = CommentSerializer(many=True, read_only=True) + + class Meta: + model = models.Thread + fields = [ + "id", + "body", + "created_at", + "updated_at", + "creator", + "abilities", + "comments", + "resolved", + "resolved_at", + "resolved_by", + "metadata", + ] + read_only_fields = [ + "id", + "created_at", + "updated_at", + "creator", + "abilities", + "comments", + "resolved", + "resolved_at", + "resolved_by", + "metadata", + ] + def validate(self, attrs): - """Validate invitation data.""" + """Validate thread 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 + attrs["creator_id"] = user.id if user else None return attrs + + def get_abilities(self, thread): + """Return thread's abilities.""" + request = self.context.get("request") + if request: + return thread.get_abilities(request.user) + return {} diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index b61d000acb..1e9606b1c3 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -21,6 +21,7 @@ from django.db.models.functions import Left, Length from django.http import Http404, StreamingHttpResponse from django.urls import reverse +from django.utils import timezone from django.utils.functional import cached_property from django.utils.text import capfirst, slugify from django.utils.translation import gettext_lazy as _ @@ -2203,15 +2204,9 @@ def _load_theme_customization(self): return theme_customization -class CommentViewSet( - viewsets.ModelViewSet, -): - """API ViewSet for comments.""" +class CommentViewSetMixin: + """Comment ViewSet Mixin.""" - 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): @@ -2225,12 +2220,114 @@ def get_document_or_404(self): raise drf.exceptions.NotFound("Document not found.") from e return self._document + +class ThreadViewSet( + ResourceAccessViewsetMixin, + CommentViewSetMixin, + drf.mixins.CreateModelMixin, + drf.mixins.ListModelMixin, + drf.mixins.RetrieveModelMixin, + drf.mixins.DestroyModelMixin, + viewsets.GenericViewSet, +): + """Thread API: list/create threads and nested comment operations.""" + + permission_classes = [permissions.CommentPermission] + pagination_class = Pagination + serializer_class = serializers.ThreadSerializer + queryset = models.Thread.objects.select_related("creator", "document").filter( + resolved=False + ) + resource_field_name = "document" + + def perform_create(self, serializer): + """Create the first comment of the thread.""" + body = serializer.validated_data["body"] + del serializer.validated_data["body"] + thread = serializer.save() + + models.Comment.objects.create( + thread=thread, + user=self.request.user if self.request.user.is_authenticated else None, + body=body, + ) + + @drf.decorators.action(detail=True, methods=["post"], url_path="resolve") + def resolve(self, request, *args, **kwargs): + """Resolve a thread.""" + thread = self.get_object() + if not thread.resolved: + thread.resolved = True + thread.resolved_at = timezone.now() + thread.resolved_by = request.user + thread.save(update_fields=["resolved", "resolved_at", "resolved_by"]) + return drf.response.Response(status=status.HTTP_204_NO_CONTENT) + + +class CommentViewSet( + CommentViewSetMixin, + viewsets.ModelViewSet, +): + """Comment API: list/create comments and nested reaction operations.""" + + permission_classes = [permissions.CommentPermission] + pagination_class = Pagination + serializer_class = serializers.CommentSerializer + queryset = models.Comment.objects.select_related("user").all() + + def get_queryset(self): + """Override to filter on related resource.""" + return ( + super() + .get_queryset() + .filter( + thread=self.kwargs["thread_id"], + thread__document=self.kwargs["resource_id"], + ) + ) + def get_serializer_context(self): """Extra context provided to the serializer class.""" context = super().get_serializer_context() - context["resource_id"] = self.kwargs["resource_id"] + context["document_id"] = self.kwargs["resource_id"] + context["thread_id"] = self.kwargs["thread_id"] return context - def get_queryset(self): - """Return the queryset according to the action.""" - return super().get_queryset().filter(document=self.kwargs["resource_id"]) + @drf.decorators.action( + detail=True, + methods=["post", "delete"], + ) + def reactions(self, request, *args, **kwargs): + """POST: add reaction; DELETE: remove reaction. + + Emoji is expected in request.data['emoji'] for both operations. + """ + comment = self.get_object() + serializer = serializers.ReactionSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + if request.method == "POST": + reaction, created = models.Reaction.objects.get_or_create( + comment=comment, + emoji=serializer.validated_data["emoji"], + ) + if not created and reaction.users.filter(id=request.user.id).exists(): + return drf.response.Response( + {"user_already_reacted": True}, status=status.HTTP_400_BAD_REQUEST + ) + reaction.users.add(request.user) + return drf.response.Response(status=status.HTTP_201_CREATED) + + # DELETE + try: + reaction = models.Reaction.objects.get( + comment=comment, + emoji=serializer.validated_data["emoji"], + users__in=[request.user], + ) + except models.Reaction.DoesNotExist as e: + raise drf.exceptions.NotFound("Reaction not found.") from e + reaction.users.remove(request.user) + if not reaction.users.exists(): + reaction.delete() + return drf.response.Response(status=status.HTTP_204_NO_CONTENT) diff --git a/src/backend/core/choices.py b/src/backend/core/choices.py index 8505ebab87..e185153103 100644 --- a/src/backend/core/choices.py +++ b/src/backend/core/choices.py @@ -33,7 +33,7 @@ class LinkRoleChoices(PriorityTextChoices): """Defines the possible roles a link can offer on a document.""" READER = "reader", _("Reader") # Can read - COMMENTATOR = "commentator", _("Commentator") # Can read and comment + COMMENTER = "commenter", _("Commenter") # Can read and comment EDITOR = "editor", _("Editor") # Can read and edit @@ -41,7 +41,7 @@ class RoleChoices(PriorityTextChoices): """Defines the possible roles a user can have in a resource.""" READER = "reader", _("Reader") # Can read - COMMENTATOR = "commentator", _("Commentator") # Can read and comment + COMMENTER = "commenter", _("Commenter") # Can read and comment EDITOR = "editor", _("Editor") # Can read and edit ADMIN = "administrator", _("Administrator") # Can read, edit, delete and share OWNER = "owner", _("Owner") diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index 24bdd317e9..c0737cdce9 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -258,12 +258,47 @@ class Meta: issuer = factory.SubFactory(UserFactory) +class ThreadFactory(factory.django.DjangoModelFactory): + """A factory to create threads for a document""" + + class Meta: + model = models.Thread + + document = factory.SubFactory(DocumentFactory) + creator = factory.SubFactory(UserFactory) + + class CommentFactory(factory.django.DjangoModelFactory): - """A factory to create comments for a document""" + """A factory to create comments for a thread""" class Meta: model = models.Comment - document = factory.SubFactory(DocumentFactory) + thread = factory.SubFactory(ThreadFactory) user = factory.SubFactory(UserFactory) - content = factory.Faker("text") + body = factory.Faker("text") + + +class ReactionFactory(factory.django.DjangoModelFactory): + """A factory to create reactions for a comment""" + + class Meta: + model = models.Reaction + + comment = factory.SubFactory(CommentFactory) + emoji = "test" + + @factory.post_generation + def users(self, create, extracted, **kwargs): + """Add users to reaction from a given list of users or create one if not provided.""" + if not create: + return + + if not extracted: + # the factory is being created, but no users were provided + user = UserFactory() + self.users.add(user) + return + + # Add the iterable of groups using bulk addition + self.users.add(*extracted) diff --git a/src/backend/core/migrations/0025_alter_document_link_role_alter_documentaccess_role_and_more.py b/src/backend/core/migrations/0025_alter_document_link_role_alter_documentaccess_role_and_more.py deleted file mode 100644 index a34ad05b88..0000000000 --- a/src/backend/core/migrations/0025_alter_document_link_role_alter_documentaccess_role_and_more.py +++ /dev/null @@ -1,146 +0,0 @@ -# Generated by Django 5.2.4 on 2025-08-26 08:11 - -import uuid - -import django.db.models.deletion -from django.conf import settings -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("core", "0024_add_is_masked_field_to_link_trace"), - ] - - operations = [ - migrations.AlterField( - model_name="document", - name="link_role", - field=models.CharField( - choices=[ - ("reader", "Reader"), - ("commentator", "Commentator"), - ("editor", "Editor"), - ], - default="reader", - max_length=20, - ), - ), - migrations.AlterField( - model_name="documentaccess", - name="role", - field=models.CharField( - choices=[ - ("reader", "Reader"), - ("commentator", "Commentator"), - ("editor", "Editor"), - ("administrator", "Administrator"), - ("owner", "Owner"), - ], - default="reader", - max_length=20, - ), - ), - migrations.AlterField( - model_name="documentaskforaccess", - name="role", - field=models.CharField( - choices=[ - ("reader", "Reader"), - ("commentator", "Commentator"), - ("editor", "Editor"), - ("administrator", "Administrator"), - ("owner", "Owner"), - ], - default="reader", - max_length=20, - ), - ), - migrations.AlterField( - model_name="invitation", - name="role", - field=models.CharField( - choices=[ - ("reader", "Reader"), - ("commentator", "Commentator"), - ("editor", "Editor"), - ("administrator", "Administrator"), - ("owner", "Owner"), - ], - default="reader", - max_length=20, - ), - ), - migrations.AlterField( - model_name="templateaccess", - name="role", - field=models.CharField( - choices=[ - ("reader", "Reader"), - ("commentator", "Commentator"), - ("editor", "Editor"), - ("administrator", "Administrator"), - ("owner", "Owner"), - ], - default="reader", - max_length=20, - ), - ), - migrations.CreateModel( - name="Comment", - fields=[ - ( - "id", - models.UUIDField( - default=uuid.uuid4, - editable=False, - help_text="primary key for the record as UUID", - primary_key=True, - serialize=False, - verbose_name="id", - ), - ), - ( - "created_at", - models.DateTimeField( - auto_now_add=True, - help_text="date and time at which a record was created", - verbose_name="created on", - ), - ), - ( - "updated_at", - models.DateTimeField( - auto_now=True, - help_text="date and time at which a record was last updated", - verbose_name="updated on", - ), - ), - ("content", models.TextField()), - ( - "document", - models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - related_name="comments", - to="core.document", - ), - ), - ( - "user", - models.ForeignKey( - blank=True, - null=True, - on_delete=django.db.models.deletion.SET_NULL, - related_name="comments", - to=settings.AUTH_USER_MODEL, - ), - ), - ], - options={ - "verbose_name": "Comment", - "verbose_name_plural": "Comments", - "db_table": "impress_comment", - "ordering": ("-created_at",), - }, - ), - ] diff --git a/src/backend/core/migrations/0025_comments.py b/src/backend/core/migrations/0025_comments.py new file mode 100644 index 0000000000..35d8941092 --- /dev/null +++ b/src/backend/core/migrations/0025_comments.py @@ -0,0 +1,275 @@ +# Generated by Django 5.2.6 on 2025-09-16 08:59 + +import uuid + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0024_add_is_masked_field_to_link_trace"), + ] + + operations = [ + migrations.AlterField( + model_name="document", + name="link_role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commenter", "Commenter"), + ("editor", "Editor"), + ], + default="reader", + max_length=20, + ), + ), + migrations.AlterField( + model_name="documentaccess", + name="role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commenter", "Commenter"), + ("editor", "Editor"), + ("administrator", "Administrator"), + ("owner", "Owner"), + ], + default="reader", + max_length=20, + ), + ), + migrations.AlterField( + model_name="documentaskforaccess", + name="role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commenter", "Commenter"), + ("editor", "Editor"), + ("administrator", "Administrator"), + ("owner", "Owner"), + ], + default="reader", + max_length=20, + ), + ), + migrations.AlterField( + model_name="invitation", + name="role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commenter", "Commenter"), + ("editor", "Editor"), + ("administrator", "Administrator"), + ("owner", "Owner"), + ], + default="reader", + max_length=20, + ), + ), + migrations.AlterField( + model_name="templateaccess", + name="role", + field=models.CharField( + choices=[ + ("reader", "Reader"), + ("commenter", "Commenter"), + ("editor", "Editor"), + ("administrator", "Administrator"), + ("owner", "Owner"), + ], + default="reader", + max_length=20, + ), + ), + migrations.CreateModel( + name="Thread", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, + editable=False, + help_text="primary key for the record as UUID", + primary_key=True, + serialize=False, + verbose_name="id", + ), + ), + ( + "created_at", + models.DateTimeField( + auto_now_add=True, + help_text="date and time at which a record was created", + verbose_name="created on", + ), + ), + ( + "updated_at", + models.DateTimeField( + auto_now=True, + help_text="date and time at which a record was last updated", + verbose_name="updated on", + ), + ), + ("resolved", models.BooleanField(default=False)), + ("resolved_at", models.DateTimeField(blank=True, null=True)), + ("metadata", models.JSONField(blank=True, default=dict)), + ( + "creator", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="threads", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "document", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="threads", + to="core.document", + ), + ), + ( + "resolved_by", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="resolved_threads", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "verbose_name": "Thread", + "verbose_name_plural": "Threads", + "db_table": "impress_thread", + "ordering": ("-created_at",), + }, + ), + migrations.CreateModel( + name="Comment", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, + editable=False, + help_text="primary key for the record as UUID", + primary_key=True, + serialize=False, + verbose_name="id", + ), + ), + ( + "created_at", + models.DateTimeField( + auto_now_add=True, + help_text="date and time at which a record was created", + verbose_name="created on", + ), + ), + ( + "updated_at", + models.DateTimeField( + auto_now=True, + help_text="date and time at which a record was last updated", + verbose_name="updated on", + ), + ), + ("body", models.JSONField()), + ("metadata", models.JSONField(blank=True, default=dict)), + ( + "user", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="thread_comment", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "thread", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="comments", + to="core.thread", + ), + ), + ], + options={ + "verbose_name": "Comment", + "verbose_name_plural": "Comments", + "db_table": "impress_comment", + "ordering": ("created_at",), + }, + ), + migrations.CreateModel( + name="Reaction", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, + editable=False, + help_text="primary key for the record as UUID", + primary_key=True, + serialize=False, + verbose_name="id", + ), + ), + ( + "created_at", + models.DateTimeField( + auto_now_add=True, + help_text="date and time at which a record was created", + verbose_name="created on", + ), + ), + ( + "updated_at", + models.DateTimeField( + auto_now=True, + help_text="date and time at which a record was last updated", + verbose_name="updated on", + ), + ), + ("emoji", models.CharField(max_length=32)), + ( + "comment", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="reactions", + to="core.comment", + ), + ), + ( + "users", + models.ManyToManyField( + related_name="reactions", to=settings.AUTH_USER_MODEL + ), + ), + ], + options={ + "verbose_name": "Reaction", + "verbose_name_plural": "Reactions", + "db_table": "impress_comment_reaction", + "constraints": [ + models.UniqueConstraint( + fields=("comment", "emoji"), + name="unique_comment_emoji", + violation_error_message="This emoji has already been reacted to this comment.", + ) + ], + }, + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index af08814a11..ba812fcf27 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -753,7 +753,7 @@ def get_abilities(self, user): can_update = ( is_owner_or_admin or role == RoleChoices.EDITOR ) and not is_deleted - can_comment = (can_update or role == RoleChoices.COMMENTATOR) and not is_deleted + can_comment = (can_update or role == RoleChoices.COMMENTER) and not is_deleted can_create_children = can_update and user.is_authenticated can_destroy = ( is_owner @@ -1147,7 +1147,7 @@ def get_abilities(self, user): set_role_to.extend( [ RoleChoices.READER, - RoleChoices.COMMENTATOR, + RoleChoices.COMMENTER, RoleChoices.EDITOR, RoleChoices.ADMIN, ] @@ -1282,48 +1282,153 @@ def send_ask_for_access_email(self, email, language=None): self.document.send_email(subject, [email], context, language) -class Comment(BaseModel): - """User comment on a document.""" +class Thread(BaseModel): + """Discussion thread attached to a document. + + A thread groups one or many comments. For backward compatibility with the + existing frontend (useComments hook) we still expose a flattened serializer + that returns a "content" field representing the first comment's body. + """ document = models.ForeignKey( Document, on_delete=models.CASCADE, + related_name="threads", + ) + creator = models.ForeignKey( + User, + on_delete=models.SET_NULL, + related_name="threads", + null=True, + blank=True, + ) + resolved = models.BooleanField(default=False) + resolved_at = models.DateTimeField(null=True, blank=True) + resolved_by = models.ForeignKey( + User, + on_delete=models.SET_NULL, + related_name="resolved_threads", + null=True, + blank=True, + ) + metadata = models.JSONField(default=dict, blank=True) + + class Meta: + db_table = "impress_thread" + ordering = ("-created_at",) + verbose_name = _("Thread") + verbose_name_plural = _("Threads") + + def __str__(self): + author = self.creator or _("Anonymous") + return f"Thread by {author!s} on {self.document!s}" + + def get_abilities(self, user): + """Compute and return abilities for a given user (mirrors comment logic).""" + role = self.document.get_role(user) + doc_abilities = self.document.get_abilities(user) + read_access = doc_abilities.get("comment", False) + write_access = self.creator == user or role in [ + RoleChoices.OWNER, + RoleChoices.ADMIN, + ] + return { + "destroy": write_access, + "update": write_access, + "partial_update": write_access, + "resolve": write_access, + "retrieve": read_access, + } + + @property + def first_comment(self): + """Return the first createdcomment of the thread.""" + return self.comments.order_by("created_at").first() + + +class Comment(BaseModel): + """A comment belonging to a thread.""" + + thread = models.ForeignKey( + Thread, + on_delete=models.CASCADE, related_name="comments", ) user = models.ForeignKey( User, on_delete=models.SET_NULL, - related_name="comments", + related_name="thread_comment", null=True, blank=True, ) - content = models.TextField() + body = models.JSONField() + metadata = models.JSONField(default=dict, blank=True) class Meta: db_table = "impress_comment" - ordering = ("-created_at",) + ordering = ("created_at",) verbose_name = _("Comment") verbose_name_plural = _("Comments") def __str__(self): + """Return the string representation of the comment.""" author = self.user or _("Anonymous") - return f"{author!s} on {self.document!s}" + return f"Comment by {author!s} on thread {self.thread_id}" def get_abilities(self, user): - """Compute and return abilities for a given user.""" - role = self.document.get_role(user) - can_comment = self.document.get_abilities(user)["comment"] + """Return the abilities of the comment.""" + role = self.thread.document.get_role(user) + doc_abilities = self.thread.document.get_abilities(user) + read_access = doc_abilities.get("comment", False) + can_react = read_access and user.is_authenticated + write_access = self.user == user or role in [ + RoleChoices.OWNER, + RoleChoices.ADMIN, + ] return { - "destroy": self.user == user - or role in [RoleChoices.OWNER, RoleChoices.ADMIN], - "update": self.user == user - or role in [RoleChoices.OWNER, RoleChoices.ADMIN], - "partial_update": self.user == user - or role in [RoleChoices.OWNER, RoleChoices.ADMIN], - "retrieve": can_comment, + "destroy": write_access, + "update": write_access, + "partial_update": write_access, + "reactions": can_react, + "retrieve": read_access, } +class Reaction(BaseModel): + """Aggregated reactions for a given emoji on a comment. + + We store one row per (comment, emoji) and maintain the list of user IDs who + reacted with that emoji. This matches the frontend interface where a + reaction exposes: emoji, createdAt (first reaction date) and userIds. + """ + + comment = models.ForeignKey( + Comment, + on_delete=models.CASCADE, + related_name="reactions", + ) + emoji = models.CharField(max_length=32) + users = models.ManyToManyField(User, related_name="reactions") + + class Meta: + db_table = "impress_comment_reaction" + constraints = [ + models.UniqueConstraint( + fields=["comment", "emoji"], + name="unique_comment_emoji", + violation_error_message=_( + "This emoji has already been reacted to this comment." + ), + ), + ] + verbose_name = _("Reaction") + verbose_name_plural = _("Reactions") + + def __str__(self): + """Return the string representation of the reaction.""" + return f"Reaction {self.emoji} on comment {self.comment.id}" + + class Template(BaseModel): """HTML and CSS code used for formatting the print around the MarkDown body.""" diff --git a/src/backend/core/tests/documents/test_api_document_accesses.py b/src/backend/core/tests/documents/test_api_document_accesses.py index cdb3d4d63a..aa21544cae 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses.py +++ b/src/backend/core/tests/documents/test_api_document_accesses.py @@ -293,7 +293,7 @@ def test_api_document_accesses_retrieve_set_role_to_child(): } assert result_dict[str(document_access_other_user.id)] == [ "reader", - "commentator", + "commenter", "editor", "administrator", "owner", @@ -302,7 +302,7 @@ def test_api_document_accesses_retrieve_set_role_to_child(): # Add an access for the other user on the parent parent_access_other_user = factories.UserDocumentAccessFactory( - document=parent, user=other_user, role="commentator" + document=parent, user=other_user, role="commenter" ) response = client.get(f"/api/v1.0/documents/{document.id!s}/accesses/") @@ -315,7 +315,7 @@ def test_api_document_accesses_retrieve_set_role_to_child(): result["id"]: result["abilities"]["set_role_to"] for result in content } assert result_dict[str(document_access_other_user.id)] == [ - "commentator", + "commenter", "editor", "administrator", "owner", @@ -323,7 +323,7 @@ def test_api_document_accesses_retrieve_set_role_to_child(): assert result_dict[str(parent_access.id)] == [] assert result_dict[str(parent_access_other_user.id)] == [ "reader", - "commentator", + "commenter", "editor", "administrator", "owner", @@ -336,28 +336,28 @@ def test_api_document_accesses_retrieve_set_role_to_child(): [ ["administrator", "reader", "reader", "reader"], [ - ["reader", "commentator", "editor", "administrator"], + ["reader", "commenter", "editor", "administrator"], [], [], - ["reader", "commentator", "editor", "administrator"], + ["reader", "commenter", "editor", "administrator"], ], ], [ ["owner", "reader", "reader", "reader"], [ - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], [], [], - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], ], ], [ ["owner", "reader", "reader", "owner"], [ - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], [], [], - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], ], ], ], @@ -418,44 +418,44 @@ def test_api_document_accesses_list_authenticated_related_same_user(roles, resul [ ["administrator", "reader", "reader", "reader"], [ - ["reader", "commentator", "editor", "administrator"], + ["reader", "commenter", "editor", "administrator"], [], [], - ["reader", "commentator", "editor", "administrator"], + ["reader", "commenter", "editor", "administrator"], ], ], [ ["owner", "reader", "reader", "reader"], [ - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], [], [], - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], ], ], [ ["owner", "reader", "reader", "owner"], [ - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], [], [], - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], ], ], [ ["reader", "reader", "reader", "owner"], [ - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], [], [], - ["reader", "commentator", "editor", "administrator", "owner"], + ["reader", "commenter", "editor", "administrator", "owner"], ], ], [ ["reader", "administrator", "reader", "editor"], [ - ["reader", "commentator", "editor", "administrator"], - ["reader", "commentator", "editor", "administrator"], + ["reader", "commenter", "editor", "administrator"], + ["reader", "commenter", "editor", "administrator"], [], [], ], @@ -463,7 +463,7 @@ def test_api_document_accesses_list_authenticated_related_same_user(roles, resul [ ["editor", "editor", "administrator", "editor"], [ - ["reader", "commentator", "editor", "administrator"], + ["reader", "commenter", "editor", "administrator"], [], ["editor", "administrator"], [], diff --git a/src/backend/core/tests/documents/test_api_documents_comments.py b/src/backend/core/tests/documents/test_api_documents_comments.py index 2a0cb7ced7..98cbc0ef98 100644 --- a/src/backend/core/tests/documents/test_api_documents_comments.py +++ b/src/backend/core/tests/documents/test_api_documents_comments.py @@ -17,42 +17,45 @@ 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 + link_reach="public", link_role=models.LinkRoleChoices.COMMENTER ) - comment1, comment2 = factories.CommentFactory.create_batch(2, document=document) + thread = factories.ThreadFactory(document=document) + comment1, comment2 = factories.CommentFactory.create_batch(2, thread=thread) # other comments not linked to the document factories.CommentFactory.create_batch(2) - response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/comments/") + response = APIClient().get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.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, + "body": comment1.body, "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()), + "reactions": [], + }, + { + "id": str(comment2.id), + "body": comment2.body, + "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, + }, + "abilities": comment2.get_abilities(AnonymousUser()), + "reactions": [], }, ], } @@ -62,13 +65,16 @@ def test_list_comments_anonymous_user_public_document(): 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 + link_reach=link_reach, link_role=models.LinkRoleChoices.COMMENTER ) - factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + factories.CommentFactory(thread=thread) # other comments not linked to the document factories.CommentFactory.create_batch(2) - response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/comments/") + response = APIClient().get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/" + ) assert response.status_code == 401 @@ -76,46 +82,49 @@ 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)] + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTER)] ) - comment1 = factories.CommentFactory(document=document) - comment2 = factories.CommentFactory(document=document, user=user) + thread = factories.ThreadFactory(document=document) + comment1 = factories.CommentFactory(thread=thread) + comment2 = factories.CommentFactory(thread=thread, 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/") + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.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, + "body": comment1.body, "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), + "reactions": [], + }, + { + "id": str(comment2.id), + "body": comment2.body, + "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, + }, + "abilities": comment2.get_abilities(user), + "reactions": [], }, ], } @@ -125,14 +134,17 @@ 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) + thread = factories.ThreadFactory(document=document) + factories.CommentFactory(thread=thread) # 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/") + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/" + ) assert response.status_code == 403 @@ -145,14 +157,17 @@ def test_list_comments_authenticated_user_not_enough_access(): document = factories.DocumentFactory( link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] ) - factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + factories.CommentFactory(thread=thread) # 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/") + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/" + ) assert response.status_code == 403 @@ -160,30 +175,35 @@ def test_list_comments_authenticated_user_not_enough_access(): def test_create_comment_anonymous_user_public_document(): - """Anonymous users should not be allowed to create comments on a public document.""" + """ + Anonymous users should be allowed to create comments on a public document + with commenter link_role. + """ document = factories.DocumentFactory( - link_reach="public", link_role=models.LinkRoleChoices.COMMENTATOR + link_reach="public", link_role=models.LinkRoleChoices.COMMENTER ) + thread = factories.ThreadFactory(document=document) client = APIClient() response = client.post( - f"/api/v1.0/documents/{document.id!s}/comments/", {"content": "test"} + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/", + {"body": "test"}, ) - assert response.status_code == 201 assert response.json() == { "id": str(response.json()["id"]), - "content": "test", + "body": "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, + "reactions": False, "retrieve": True, }, + "reactions": [], } @@ -192,9 +212,11 @@ def test_create_comment_anonymous_user_non_accessible_document(): document = factories.DocumentFactory( link_reach="public", link_role=models.LinkRoleChoices.READER ) + thread = factories.ThreadFactory(document=document) client = APIClient() response = client.post( - f"/api/v1.0/documents/{document.id!s}/comments/", {"content": "test"} + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/", + {"body": "test"}, ) assert response.status_code == 401 @@ -204,31 +226,34 @@ 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)] + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTER)] ) + thread = factories.ThreadFactory(document=document) client = APIClient() client.force_login(user) response = client.post( - f"/api/v1.0/documents/{document.id!s}/comments/", {"content": "test"} + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/", + {"body": "test"}, ) assert response.status_code == 201 assert response.json() == { "id": str(response.json()["id"]), - "content": "test", + "body": "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, + "reactions": True, "retrieve": True, }, + "reactions": [], } @@ -241,10 +266,12 @@ def test_create_comment_authenticated_user_not_enough_access(): document = factories.DocumentFactory( link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] ) + thread = factories.ThreadFactory(document=document) client = APIClient() client.force_login(user) response = client.post( - f"/api/v1.0/documents/{document.id!s}/comments/", {"content": "test"} + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/", + {"body": "test"}, ) assert response.status_code == 403 @@ -255,24 +282,25 @@ def test_create_comment_authenticated_user_not_enough_access(): 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 + link_reach="public", link_role=models.LinkRoleChoices.COMMENTER ) - comment = factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() response = client.get( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 200 assert response.json() == { "id": str(comment.id), - "content": comment.content, + "body": comment.body, "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), + "reactions": [], "abilities": comment.get_abilities(AnonymousUser()), } @@ -282,10 +310,11 @@ def test_retrieve_comment_anonymous_user_non_accessible_document(): document = factories.DocumentFactory( link_reach="public", link_role=models.LinkRoleChoices.READER ) - comment = factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() response = client.get( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 401 @@ -294,13 +323,14 @@ 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)] + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTER)] ) - comment = factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() client.force_login(user) response = client.get( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 200 @@ -314,11 +344,12 @@ def test_retrieve_comment_authenticated_user_not_enough_access(): document = factories.DocumentFactory( link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] ) - comment = factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() client.force_login(user) response = client.get( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 403 @@ -329,13 +360,14 @@ def test_retrieve_comment_authenticated_user_not_enough_access(): 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 + link_reach="public", link_role=models.LinkRoleChoices.COMMENTER ) - comment = factories.CommentFactory(document=document, content="test") + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, body="test") client = APIClient() response = client.put( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", - {"content": "other content"}, + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/", + {"body": "other content"}, ) assert response.status_code == 401 @@ -345,11 +377,12 @@ def test_update_comment_anonymous_user_non_accessible_document(): document = factories.DocumentFactory( link_reach="public", link_role=models.LinkRoleChoices.READER ) - comment = factories.CommentFactory(document=document, content="test") + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, body="test") client = APIClient() response = client.put( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/", - {"content": "other content"}, + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/", + {"body": "other content"}, ) assert response.status_code == 401 @@ -363,17 +396,18 @@ def test_update_comment_authenticated_user_accessible_document(): ( user, random.choice( - [models.LinkRoleChoices.COMMENTATOR, models.LinkRoleChoices.EDITOR] + [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] ), ) ], ) - comment = factories.CommentFactory(document=document, content="test") + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, body="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"}, + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/", + {"body": "other content"}, ) assert response.status_code == 403 @@ -387,22 +421,23 @@ def test_update_comment_authenticated_user_own_comment(): ( user, random.choice( - [models.LinkRoleChoices.COMMENTATOR, models.LinkRoleChoices.EDITOR] + [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] ), ) ], ) - comment = factories.CommentFactory(document=document, content="test", user=user) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, body="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"}, + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/", + {"body": "other content"}, ) assert response.status_code == 200 comment.refresh_from_db() - assert comment.content == "other content" + assert comment.body == "other content" def test_update_comment_authenticated_user_not_enough_access(): @@ -414,12 +449,13 @@ def test_update_comment_authenticated_user_not_enough_access(): document = factories.DocumentFactory( link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] ) - comment = factories.CommentFactory(document=document, content="test") + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, body="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"}, + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/", + {"body": "other content"}, ) assert response.status_code == 403 @@ -431,12 +467,13 @@ def test_update_comment_authenticated_no_access(): """ user = factories.UserFactory() document = factories.DocumentFactory(link_reach="restricted") - comment = factories.CommentFactory(document=document, content="test") + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, body="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"}, + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/", + {"body": "other content"}, ) assert response.status_code == 403 @@ -448,18 +485,19 @@ def test_update_comment_authenticated_admin_or_owner_can_update_any_comment(role """ user = factories.UserFactory() document = factories.DocumentFactory(users=[(user, role)]) - comment = factories.CommentFactory(document=document, content="test") + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, body="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"}, + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/", + {"body": "other content"}, ) assert response.status_code == 200 comment.refresh_from_db() - assert comment.content == "other content" + assert comment.body == "other content" @pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) @@ -469,18 +507,19 @@ def test_update_comment_authenticated_admin_or_owner_can_update_own_comment(role """ user = factories.UserFactory() document = factories.DocumentFactory(users=[(user, role)]) - comment = factories.CommentFactory(document=document, content="test", user=user) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, body="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"}, + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/", + {"body": "other content"}, ) assert response.status_code == 200 comment.refresh_from_db() - assert comment.content == "other content" + assert comment.body == "other content" # Delete comment @@ -489,12 +528,13 @@ def test_update_comment_authenticated_admin_or_owner_can_update_own_comment(role 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 + link_reach="public", link_role=models.LinkRoleChoices.COMMENTER ) - comment = factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() response = client.delete( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 401 @@ -504,10 +544,11 @@ def test_delete_comment_anonymous_user_non_accessible_document(): document = factories.DocumentFactory( link_reach="public", link_role=models.LinkRoleChoices.READER ) - comment = factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() response = client.delete( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 401 @@ -516,13 +557,14 @@ 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)] + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTER)] ) - comment = factories.CommentFactory(document=document, user=user) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, user=user) client = APIClient() client.force_login(user) response = client.delete( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 204 @@ -531,13 +573,14 @@ 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)] + link_reach="restricted", users=[(user, models.LinkRoleChoices.COMMENTER)] ) - comment = factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() client.force_login(user) response = client.delete( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 403 @@ -547,11 +590,12 @@ def test_delete_comment_authenticated_user_admin_or_owner_can_delete_any_comment """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) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() client.force_login(user) response = client.delete( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 204 @@ -561,11 +605,12 @@ def test_delete_comment_authenticated_user_admin_or_owner_can_delete_own_comment """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) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread, user=user) client = APIClient() client.force_login(user) response = client.delete( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" ) assert response.status_code == 204 @@ -579,10 +624,255 @@ def test_delete_comment_authenticated_user_not_enough_access(): document = factories.DocumentFactory( link_reach="restricted", users=[(user, models.LinkRoleChoices.READER)] ) - comment = factories.CommentFactory(document=document) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) client = APIClient() client.force_login(user) response = client.delete( - f"/api/v1.0/documents/{document.id!s}/comments/{comment.id!s}/" + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/comments/{comment.id!s}/" + ) + assert response.status_code == 403 + + +# Create reaction + + +@pytest.mark.parametrize("link_role", models.LinkRoleChoices.values) +def test_create_reaction_anonymous_user_public_document(link_role): + """No matter the link_role, an anonymous user can not react to a comment.""" + + document = factories.DocumentFactory(link_reach="public", link_role=link_role) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, + ) + assert response.status_code == 401 + + +def test_create_reaction_authenticated_user_public_document(): + """ + Authenticated users should not be able to reaction to a comment on a public document with + link_role reader. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.READER + ) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, ) assert response.status_code == 403 + + +def test_create_reaction_authenticated_user_accessible_public_document(): + """ + Authenticated users should be able to react to a comment on a public document. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="public", link_role=models.LinkRoleChoices.COMMENTER + ) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, + ) + assert response.status_code == 201 + + assert models.Reaction.objects.filter( + comment=comment, emoji="test", users__in=[user] + ).exists() + + +def test_create_reaction_authenticated_user_connected_document_link_role_reader(): + """ + Authenticated users should not be able to react to a comment on a connected document + with link_role reader. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", link_role=models.LinkRoleChoices.READER + ) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "link_role", + [ + role + for role in models.LinkRoleChoices.values + if role != models.LinkRoleChoices.READER + ], +) +def test_create_reaction_authenticated_user_connected_document(link_role): + """ + Authenticated users should be able to react to a comment on a connected document. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", link_role=link_role + ) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, + ) + assert response.status_code == 201 + + assert models.Reaction.objects.filter( + comment=comment, emoji="test", users__in=[user] + ).exists() + + +def test_create_reaction_authenticated_user_restricted_accessible_document(): + """ + Authenticated users should not be able to react to a comment on a restricted accessible document + they don't have access to. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, + ) + assert response.status_code == 403 + + +def test_create_reaction_authenticated_user_restricted_accessible_document_role_reader(): + """ + Authenticated users should not be able to react to a comment on a restricted accessible + document with role reader. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", link_role=models.LinkRoleChoices.READER + ) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "role", + [role for role in models.RoleChoices.values if role != models.RoleChoices.READER], +) +def test_create_reaction_authenticated_user_restricted_accessible_document_role_commenter( + role, +): + """ + Authenticated users should be able to react to a comment on a restricted accessible document + with role commenter. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted", users=[(user, role)]) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, + ) + assert response.status_code == 201 + + assert models.Reaction.objects.filter( + comment=comment, emoji="test", users__in=[user] + ).exists() + + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": "test"}, + ) + assert response.status_code == 400 + assert response.json() == {"user_already_reacted": True} + + +# Delete reaction + + +def test_delete_reaction_not_owned_by_the_current_user(): + """ + Users should not be able to delete reactions not owned by the current user. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.RoleChoices.ADMIN)] + ) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + reaction = factories.ReactionFactory(comment=comment) + + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": reaction.emoji}, + ) + assert response.status_code == 404 + + +def test_delete_reaction_owned_by_the_current_user(): + """ + Users should not be able to delete reactions not owned by the current user. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", users=[(user, models.RoleChoices.ADMIN)] + ) + thread = factories.ThreadFactory(document=document) + comment = factories.CommentFactory(thread=thread) + reaction = factories.ReactionFactory(comment=comment) + + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/" + f"comments/{comment.id!s}/reactions/", + {"emoji": reaction.emoji}, + ) + assert response.status_code == 404 + + reaction.refresh_from_db() + assert reaction.users.exists() 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 6046338d60..4c3fef6a0f 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -36,7 +36,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "children_create": False, "children_list": True, "collaboration_auth": True, - "comment": document.link_role in ["commentator", "editor"], + "comment": document.link_role in ["commenter", "editor"], "cors_proxy": True, "content": True, "descendants": True, @@ -47,8 +47,8 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": False, @@ -113,7 +113,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "children_create": False, "children_list": True, "collaboration_auth": True, - "comment": grand_parent.link_role in ["commentator", "editor"], + "comment": grand_parent.link_role in ["commenter", "editor"], "descendants": True, "cors_proxy": True, "content": True, @@ -220,7 +220,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "children_create": document.link_role == "editor", "children_list": True, "collaboration_auth": True, - "comment": document.link_role in ["commentator", "editor"], + "comment": document.link_role in ["commenter", "editor"], "descendants": True, "cors_proxy": True, "content": True, @@ -230,8 +230,8 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": True, @@ -304,7 +304,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "children_create": grand_parent.link_role == "editor", "children_list": True, "collaboration_auth": True, - "comment": grand_parent.link_role in ["commentator", "editor"], + "comment": grand_parent.link_role in ["commenter", "editor"], "descendants": True, "cors_proxy": True, "content": True, @@ -496,7 +496,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): "ai_transform": access.role != "reader", "ai_translate": access.role != "reader", "attachment_upload": access.role != "reader", - "can_edit": access.role not in ["reader", "commentator"], + "can_edit": access.role not in ["reader", "commenter"], "children_create": access.role != "reader", "children_list": True, "collaboration_auth": True, diff --git a/src/backend/core/tests/documents/test_api_documents_threads.py b/src/backend/core/tests/documents/test_api_documents_threads.py new file mode 100644 index 0000000000..cea0ae966f --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_threads.py @@ -0,0 +1,1226 @@ +"""Test Thread viewset.""" + +import pytest +from rest_framework.test import APIClient + +from core import factories, models + +pytestmark = pytest.mark.django_db + +# pylint: disable=too-many-lines + + +# Create + + +def test_api_documents_threads_public_document_link_role_reader(): + """ + Anonymous users should not be allowed to create threads on public documents with reader + link_role. + """ + document = factories.DocumentFactory( + link_reach="public", + link_role=models.LinkRoleChoices.READER, + ) + + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/", + { + "body": "test", + }, + ) + assert response.status_code == 401 + + +@pytest.mark.parametrize( + "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] +) +def test_api_documents_threads_public_document(link_role): + """ + Anonymous users should be allowed to create threads on public documents with commenter + link_role. + """ + document = factories.DocumentFactory( + link_reach="public", + link_role=link_role, + ) + + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/", + { + "body": "test", + }, + ) + + assert response.status_code == 201 + thread = models.Thread.objects.first() + comment = thread.comments.first() + content = response.json() + assert content == { + "id": str(thread.id), + "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), + "creator": None, + "comments": [ + { + "id": str(comment.id), + "body": "test", + "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), + "user": None, + "reactions": [], + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "reactions": False, + "retrieve": True, + }, + } + ], + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "resolve": False, + "retrieve": True, + }, + "metadata": {}, + "resolved": False, + "resolved_at": None, + "resolved_by": None, + } + + +def test_api_documents_threads_restricted_document(): + """ + Authenticated users should not be allowed to create threads on restricted + documents with reader roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.READER, + users=[(user, models.LinkRoleChoices.READER)], + ) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/", + { + "body": "test", + }, + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "role", + [role for role in models.RoleChoices.values if role != models.RoleChoices.READER], +) +def test_api_documents_threads_restricted_document_editor(role): + """ + Authenticated users should be allowed to create threads on restricted + documents with editor roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/", + { + "body": "test", + }, + ) + + assert response.status_code == 201 + thread = models.Thread.objects.first() + comment = thread.comments.first() + content = response.json() + assert content == { + "id": str(thread.id), + "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), + "creator": { + "full_name": user.full_name, + "short_name": user.short_name, + }, + "comments": [ + { + "id": str(comment.id), + "body": "test", + "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), + "user": { + "full_name": user.full_name, + "short_name": user.short_name, + }, + "reactions": [], + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "reactions": True, + "retrieve": True, + }, + } + ], + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "resolve": True, + "retrieve": True, + }, + "metadata": {}, + "resolved": False, + "resolved_at": None, + "resolved_by": None, + } + + +def test_api_documents_threads_authenticated_document_anonymous_user(): + """ + Anonymous users should not be allowed to create threads on authenticated documents. + """ + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/", + { + "body": "test", + }, + ) + assert response.status_code == 401 + + +def test_api_documents_threads_authenticated_document_reader_role(): + """ + Authenticated users should not be allowed to create threads on authenticated + documents with reader link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.READER, + ) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/", + { + "body": "test", + }, + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] +) +def test_api_documents_threads_authenticated_document(link_role): + """ + Authenticated users should be allowed to create threads on authenticated + documents with commenter or editor link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=link_role, + ) + + client = APIClient() + client.force_login(user) + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/", + { + "body": "test", + }, + ) + + assert response.status_code == 201 + thread = models.Thread.objects.first() + comment = thread.comments.first() + content = response.json() + assert content == { + "id": str(thread.id), + "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), + "creator": { + "full_name": user.full_name, + "short_name": user.short_name, + }, + "comments": [ + { + "id": str(comment.id), + "body": "test", + "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), + "user": { + "full_name": user.full_name, + "short_name": user.short_name, + }, + "reactions": [], + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "reactions": True, + "retrieve": True, + }, + } + ], + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "resolve": True, + "retrieve": True, + }, + "metadata": {}, + "resolved": False, + "resolved_at": None, + "resolved_by": None, + } + + +# List + + +def test_api_documents_threads_list_public_document_link_role_reader(): + """ + Anonymous users should not be allowed to retrieve threads on public documents with reader + link_role. + """ + document = factories.DocumentFactory( + link_reach="public", + link_role=models.LinkRoleChoices.READER, + ) + + factories.ThreadFactory.create_batch(3, document=document) + + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/", + ) + assert response.status_code == 401 + + +@pytest.mark.parametrize( + "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] +) +def test_api_documents_threads_list_public_document_link_role_higher_than_reader( + link_role, +): + """ + Anonymous users should be allowed to retrieve threads on public documents with commenter or + editor link_role. + """ + document = factories.DocumentFactory( + link_reach="public", + link_role=link_role, + ) + + factories.ThreadFactory.create_batch(3, document=document) + + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/", + ) + assert response.status_code == 200 + assert response.json()["count"] == 3 + + +def test_api_documents_threads_list_authenticated_document_anonymous_user(): + """ + Anonymous users should not be allowed to retrieve threads on authenticated documents. + """ + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + factories.ThreadFactory.create_batch(3, document=document) + + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_list_authenticated_document_reader_role(): + """ + Authenticated users should not be allowed to retrieve threads on authenticated + documents with reader link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.READER, + ) + + factories.ThreadFactory.create_batch(3, document=document) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] +) +def test_api_documents_threads_list_authenticated_document(link_role): + """ + Authenticated users should be allowed to retrieve threads on authenticated + documents with commenter or editor link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=link_role, + ) + + factories.ThreadFactory.create_batch(3, document=document) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/", + ) + assert response.status_code == 200 + assert response.json()["count"] == 3 + + +def test_api_documents_threads_list_restricted_document_anonymous_user(): + """ + Anonymous users should not be allowed to retrieve threads on restricted documents. + """ + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + factories.ThreadFactory.create_batch(3, document=document) + + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_list_restricted_document_reader_role(): + """ + Authenticated users should not be allowed to retrieve threads on restricted + documents with reader roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.READER, + users=[(user, models.LinkRoleChoices.READER)], + ) + + factories.ThreadFactory.create_batch(3, document=document) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "role", + [role for role in models.RoleChoices.values if role != models.RoleChoices.READER], +) +def test_api_documents_threads_list_restricted_document_editor(role): + """ + Authenticated users should be allowed to retrieve threads on restricted + documents with editor roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + factories.ThreadFactory.create_batch(3, document=document) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/", + ) + assert response.status_code == 200 + assert response.json()["count"] == 3 + + +# Retrieve + + +def test_api_documents_threads_retrieve_public_document_link_role_reader(): + """ + Anonymous users should not be allowed to retrieve threads on public documents with reader + link_role. + """ + document = factories.DocumentFactory( + link_reach="public", + link_role=models.LinkRoleChoices.READER, + ) + + thread = factories.ThreadFactory(document=document) + + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 401 + + +@pytest.mark.parametrize( + "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] +) +def test_api_documents_threads_retrieve_public_document_link_role_higher_than_reader( + link_role, +): + """ + Anonymous users should be allowed to retrieve threads on public documents with commenter or + editor link_role. + """ + document = factories.DocumentFactory( + link_reach="public", + link_role=link_role, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + comment = factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 200 + content = response.json() + assert content == { + "id": str(thread.id), + "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), + "creator": None, + "comments": [ + { + "id": str(comment.id), + "body": comment.body, + "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), + "user": None, + "reactions": [], + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "reactions": False, + "retrieve": True, + }, + } + ], + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "resolve": False, + "retrieve": True, + }, + "metadata": {}, + "resolved": False, + "resolved_at": None, + "resolved_by": None, + } + + +def test_api_documents_threads_retrieve_authenticated_document_anonymous_user(): + """ + Anonymous users should not be allowed to retrieve threads on authenticated documents. + """ + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document) + + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_retrieve_authenticated_document_reader_role(): + """ + Authenticated users should not be allowed to retrieve threads on authenticated + documents with reader link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.READER, + ) + + thread = factories.ThreadFactory(document=document) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] +) +def test_api_documents_threads_retrieve_authenticated_document(link_role): + """ + Authenticated users should be allowed to retrieve threads on authenticated + documents with commenter or editor link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=link_role, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + comment = factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 200 + content = response.json() + assert content == { + "id": str(thread.id), + "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), + "creator": None, + "metadata": {}, + "resolved": False, + "resolved_at": None, + "resolved_by": None, + "comments": [ + { + "id": str(comment.id), + "body": comment.body, + "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), + "user": None, + "reactions": [], + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "reactions": True, + "retrieve": True, + }, + } + ], + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "resolve": False, + "retrieve": True, + }, + } + + +def test_api_documents_threads_retrieve_restricted_document_anonymous_user(): + """ + Anonymous users should not be allowed to retrieve threads on restricted documents. + """ + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document) + + client = APIClient() + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_retrieve_restricted_document_reader_role(): + """ + Authenticated users should not be allowed to retrieve threads on restricted + documents with reader roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.READER, + users=[(user, models.LinkRoleChoices.READER)], + ) + + thread = factories.ThreadFactory(document=document) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "role", [models.RoleChoices.COMMENTER, models.RoleChoices.EDITOR] +) +def test_api_documents_threads_retrieve_restricted_document_editor(role): + """ + Authenticated users should be allowed to retrieve threads on restricted + documents with editor roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + thread = factories.ThreadFactory(document=document, creator=None) + comment = factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 200 + content = response.json() + assert content == { + "id": str(thread.id), + "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), + "creator": None, + "comments": [ + { + "id": str(comment.id), + "body": comment.body, + "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), + "user": None, + "reactions": [], + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "reactions": True, + "retrieve": True, + }, + } + ], + "abilities": { + "destroy": False, + "update": False, + "partial_update": False, + "resolve": False, + "retrieve": True, + }, + "metadata": {}, + "resolved": False, + "resolved_at": None, + "resolved_by": None, + } + + +@pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) +def test_api_documents_threads_retrieve_restricted_document_privileged_roles(role): + """ + Authenticated users with privileged roles should be allowed to retrieve + threads on restricted documents with editor roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + thread = factories.ThreadFactory(document=document, creator=None) + comment = factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 200 + content = response.json() + assert content == { + "id": str(thread.id), + "created_at": thread.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": thread.updated_at.isoformat().replace("+00:00", "Z"), + "creator": None, + "comments": [ + { + "id": str(comment.id), + "body": comment.body, + "created_at": comment.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": comment.updated_at.isoformat().replace("+00:00", "Z"), + "user": None, + "reactions": [], + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "reactions": True, + "retrieve": True, + }, + } + ], + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "resolve": True, + "retrieve": True, + }, + "metadata": {}, + "resolved": False, + "resolved_at": None, + "resolved_by": None, + } + + +# Destroy + + +def test_api_documents_threads_destroy_public_document_anonymous_user(): + """ + Anonymous users should not be allowed to destroy threads on public documents. + """ + document = factories.DocumentFactory( + link_reach="public", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_destroy_public_document_authenticated_user(): + """ + Authenticated users should not be allowed to destroy threads on public documents. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="public", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 403 + + +def test_api_documents_threads_destroy_authenticated_document_anonymous_user(): + """ + Anonymous users should not be allowed to destroy threads on authenticated documents. + """ + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_destroy_authenticated_document_reader_role(): + """ + Authenticated users should not be allowed to destroy threads on authenticated + documents with reader link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.READER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] +) +def test_api_documents_threads_destroy_authenticated_document(link_role): + """ + Authenticated users should not be allowed to destroy threads on authenticated + documents with commenter or editor link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=link_role, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 403 + + +def test_api_documents_threads_destroy_restricted_document_anonymous_user(): + """ + Anonymous users should not be allowed to destroy threads on restricted documents. + """ + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_destroy_restricted_document_reader_role(): + """ + Authenticated users should not be allowed to destroy threads on restricted + documents with reader roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.READER, + users=[(user, models.LinkRoleChoices.READER)], + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "role", [models.RoleChoices.COMMENTER, models.RoleChoices.EDITOR] +) +def test_api_documents_threads_destroy_restricted_document_editor(role): + """ + Authenticated users should not be allowed to destroy threads on restricted + documents with editor roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) +def test_api_documents_threads_destroy_restricted_document_privileged_roles(role): + """ + Authenticated users with privileged roles should be allowed to destroy + threads on restricted documents with editor roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + client.force_login(user) + response = client.delete( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/", + ) + assert response.status_code == 204 + assert not models.Thread.objects.filter(id=thread.id).exists() + + +# Resolve + + +def test_api_documents_threads_resolve_public_document_anonymous_user(): + """ + Anonymous users should not be allowed to resolve threads on public documents. + """ + document = factories.DocumentFactory( + link_reach="public", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_resolve_public_document_authenticated_user(): + """ + Authenticated users should not be allowed to resolve threads on public documents. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="public", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + 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}/resolve/", + ) + assert response.status_code == 403 + + +def test_api_documents_threads_resolve_authenticated_document_anonymous_user(): + """ + Anonymous users should not be allowed to resolve threads on authenticated documents. + """ + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_resolve_authenticated_document_reader_role(): + """ + Authenticated users should not be allowed to resolve threads on authenticated + documents with reader link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=models.LinkRoleChoices.READER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + 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}/resolve/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "link_role", [models.LinkRoleChoices.COMMENTER, models.LinkRoleChoices.EDITOR] +) +def test_api_documents_threads_resolve_authenticated_document(link_role): + """ + Authenticated users should not be allowed to resolve threads on authenticated documents with + commenter or editor link_role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="authenticated", + link_role=link_role, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + 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}/resolve/", + ) + assert response.status_code == 403 + + +def test_api_documents_threads_resolve_restricted_document_anonymous_user(): + """ + Anonymous users should not be allowed to resolve threads on restricted documents. + """ + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.COMMENTER, + ) + + thread = factories.ThreadFactory(document=document, creator=None) + factories.CommentFactory(thread=thread, user=None) + + client = APIClient() + response = client.post( + f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/resolve/", + ) + assert response.status_code == 401 + + +def test_api_documents_threads_resolve_restricted_document_reader_role(): + """ + Authenticated users should not be allowed to resolve threads on restricted documents with + reader roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.READER, + users=[(user, models.LinkRoleChoices.READER)], + ) + + thread = factories.ThreadFactory(document=document, creator=None) + 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}/resolve/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "role", [models.RoleChoices.COMMENTER, models.RoleChoices.EDITOR] +) +def test_api_documents_threads_resolve_restricted_document_editor(role): + """ + Authenticated users should not be allowed to resolve threads on restricted documents with + editor roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + thread = factories.ThreadFactory(document=document, creator=None) + 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}/resolve/", + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize("role", [models.RoleChoices.ADMIN, models.RoleChoices.OWNER]) +def test_api_documents_threads_resolve_restricted_document_privileged_roles(role): + """ + Authenticated users with privileged roles should be allowed to resolve threads on + restricted documents with editor roles. + """ + user = factories.UserFactory() + document = factories.DocumentFactory( + link_reach="restricted", + link_role=models.LinkRoleChoices.EDITOR, + users=[(user, role)], + ) + + thread = factories.ThreadFactory(document=document, creator=None) + 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}/resolve/", + ) + 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 diff --git a/src/backend/core/tests/documents/test_api_documents_trashbin.py b/src/backend/core/tests/documents/test_api_documents_trashbin.py index 998d283a69..ec4923ad0f 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -89,8 +89,8 @@ def test_api_documents_trashbin_format(): "invite_owner": True, "link_configuration": True, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": True, diff --git a/src/backend/core/tests/test_models_comment.py b/src/backend/core/tests/test_models_comment.py index dac0b36c22..7ff8cc8760 100644 --- a/src/backend/core/tests/test_models_comment.py +++ b/src/backend/core/tests/test_models_comment.py @@ -16,7 +16,7 @@ "role,can_comment", [ (LinkRoleChoices.READER, False), - (LinkRoleChoices.COMMENTATOR, True), + (LinkRoleChoices.COMMENTER, True), (LinkRoleChoices.EDITOR, True), ], ) @@ -25,13 +25,14 @@ def test_comment_get_abilities_anonymous_user_public_document(role, can_comment) document = factories.DocumentFactory( link_role=role, link_reach=LinkReachChoices.PUBLIC ) - comment = factories.CommentFactory(document=document) + comment = factories.CommentFactory(thread__document=document) user = AnonymousUser() assert comment.get_abilities(user) == { "destroy": False, "update": False, "partial_update": False, + "reactions": False, "retrieve": can_comment, } @@ -42,13 +43,14 @@ def test_comment_get_abilities_anonymous_user_public_document(role, can_comment) def test_comment_get_abilities_anonymous_user_restricted_document(link_reach): """Anonymous users cannot comment on a restricted document.""" document = factories.DocumentFactory(link_reach=link_reach) - comment = factories.CommentFactory(document=document) + comment = factories.CommentFactory(thread__document=document) user = AnonymousUser() assert comment.get_abilities(user) == { "destroy": False, "update": False, "partial_update": False, + "reactions": False, "retrieve": False, } @@ -57,13 +59,13 @@ def test_comment_get_abilities_anonymous_user_restricted_document(link_reach): "link_role,link_reach,can_comment", [ (LinkRoleChoices.READER, LinkReachChoices.PUBLIC, False), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC, True), + (LinkRoleChoices.COMMENTER, LinkReachChoices.PUBLIC, True), (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC, True), (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED, False), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED, False), + (LinkRoleChoices.COMMENTER, LinkReachChoices.RESTRICTED, False), (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED, False), (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED, False), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED, True), + (LinkRoleChoices.COMMENTER, LinkReachChoices.AUTHENTICATED, True), (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED, True), ], ) @@ -73,12 +75,13 @@ def test_comment_get_abilities_user_reader(link_role, link_reach, can_comment): document = factories.DocumentFactory( link_role=link_role, link_reach=link_reach, users=[(user, RoleChoices.READER)] ) - comment = factories.CommentFactory(document=document) + comment = factories.CommentFactory(thread__document=document) assert comment.get_abilities(user) == { "destroy": False, "update": False, "partial_update": False, + "reactions": can_comment, "retrieve": can_comment, } @@ -87,13 +90,13 @@ def test_comment_get_abilities_user_reader(link_role, link_reach, can_comment): "link_role,link_reach,can_comment", [ (LinkRoleChoices.READER, LinkReachChoices.PUBLIC, False), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC, True), + (LinkRoleChoices.COMMENTER, LinkReachChoices.PUBLIC, True), (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC, True), (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED, False), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED, False), + (LinkRoleChoices.COMMENTER, LinkReachChoices.RESTRICTED, False), (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED, False), (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED, False), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED, True), + (LinkRoleChoices.COMMENTER, LinkReachChoices.AUTHENTICATED, True), (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED, True), ], ) @@ -106,13 +109,14 @@ def test_comment_get_abilities_user_reader_own_comment( link_role=link_role, link_reach=link_reach, users=[(user, RoleChoices.READER)] ) comment = factories.CommentFactory( - document=document, user=user if can_comment else None + thread__document=document, user=user if can_comment else None ) assert comment.get_abilities(user) == { "destroy": can_comment, "update": can_comment, "partial_update": can_comment, + "reactions": can_comment, "retrieve": can_comment, } @@ -121,30 +125,31 @@ def test_comment_get_abilities_user_reader_own_comment( "link_role,link_reach", [ (LinkRoleChoices.READER, LinkReachChoices.PUBLIC), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.COMMENTER, LinkReachChoices.PUBLIC), (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC), (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.COMMENTER, LinkReachChoices.RESTRICTED), (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED), (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.COMMENTER, LinkReachChoices.AUTHENTICATED), (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED), ], ) -def test_comment_get_abilities_user_commentator(link_role, link_reach): - """Commentators can comment on a document.""" +def test_comment_get_abilities_user_commenter(link_role, link_reach): + """Commenters can comment on a document.""" user = factories.UserFactory() document = factories.DocumentFactory( link_role=link_role, link_reach=link_reach, - users=[(user, RoleChoices.COMMENTATOR)], + users=[(user, RoleChoices.COMMENTER)], ) - comment = factories.CommentFactory(document=document) + comment = factories.CommentFactory(thread__document=document) assert comment.get_abilities(user) == { "destroy": False, "update": False, "partial_update": False, + "reactions": True, "retrieve": True, } @@ -153,30 +158,31 @@ def test_comment_get_abilities_user_commentator(link_role, link_reach): "link_role,link_reach", [ (LinkRoleChoices.READER, LinkReachChoices.PUBLIC), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.COMMENTER, LinkReachChoices.PUBLIC), (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC), (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.COMMENTER, LinkReachChoices.RESTRICTED), (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED), (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.COMMENTER, LinkReachChoices.AUTHENTICATED), (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED), ], ) -def test_comment_get_abilities_user_commentator_own_comment(link_role, link_reach): - """Commentators have all accesses to its own comment.""" +def test_comment_get_abilities_user_commenter_own_comment(link_role, link_reach): + """Commenters have all accesses to its own comment.""" user = factories.UserFactory() document = factories.DocumentFactory( link_role=link_role, link_reach=link_reach, - users=[(user, RoleChoices.COMMENTATOR)], + users=[(user, RoleChoices.COMMENTER)], ) - comment = factories.CommentFactory(document=document, user=user) + comment = factories.CommentFactory(thread__document=document, user=user) assert comment.get_abilities(user) == { "destroy": True, "update": True, "partial_update": True, + "reactions": True, "retrieve": True, } @@ -185,13 +191,13 @@ def test_comment_get_abilities_user_commentator_own_comment(link_role, link_reac "link_role,link_reach", [ (LinkRoleChoices.READER, LinkReachChoices.PUBLIC), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.COMMENTER, LinkReachChoices.PUBLIC), (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC), (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.COMMENTER, LinkReachChoices.RESTRICTED), (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED), (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.COMMENTER, LinkReachChoices.AUTHENTICATED), (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED), ], ) @@ -201,12 +207,13 @@ def test_comment_get_abilities_user_editor(link_role, link_reach): document = factories.DocumentFactory( link_role=link_role, link_reach=link_reach, users=[(user, RoleChoices.EDITOR)] ) - comment = factories.CommentFactory(document=document) + comment = factories.CommentFactory(thread__document=document) assert comment.get_abilities(user) == { "destroy": False, "update": False, "partial_update": False, + "reactions": True, "retrieve": True, } @@ -215,13 +222,13 @@ def test_comment_get_abilities_user_editor(link_role, link_reach): "link_role,link_reach", [ (LinkRoleChoices.READER, LinkReachChoices.PUBLIC), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.PUBLIC), + (LinkRoleChoices.COMMENTER, LinkReachChoices.PUBLIC), (LinkRoleChoices.EDITOR, LinkReachChoices.PUBLIC), (LinkRoleChoices.READER, LinkReachChoices.RESTRICTED), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.RESTRICTED), + (LinkRoleChoices.COMMENTER, LinkReachChoices.RESTRICTED), (LinkRoleChoices.EDITOR, LinkReachChoices.RESTRICTED), (LinkRoleChoices.READER, LinkReachChoices.AUTHENTICATED), - (LinkRoleChoices.COMMENTATOR, LinkReachChoices.AUTHENTICATED), + (LinkRoleChoices.COMMENTER, LinkReachChoices.AUTHENTICATED), (LinkRoleChoices.EDITOR, LinkReachChoices.AUTHENTICATED), ], ) @@ -231,12 +238,13 @@ def test_comment_get_abilities_user_editor_own_comment(link_role, link_reach): document = factories.DocumentFactory( link_role=link_role, link_reach=link_reach, users=[(user, RoleChoices.EDITOR)] ) - comment = factories.CommentFactory(document=document, user=user) + comment = factories.CommentFactory(thread__document=document, user=user) assert comment.get_abilities(user) == { "destroy": True, "update": True, "partial_update": True, + "reactions": True, "retrieve": True, } @@ -246,13 +254,14 @@ def test_comment_get_abilities_user_admin(): user = factories.UserFactory() document = factories.DocumentFactory(users=[(user, RoleChoices.ADMIN)]) comment = factories.CommentFactory( - document=document, user=random.choice([user, None]) + thread__document=document, user=random.choice([user, None]) ) assert comment.get_abilities(user) == { "destroy": True, "update": True, "partial_update": True, + "reactions": True, "retrieve": True, } @@ -262,12 +271,13 @@ def test_comment_get_abilities_user_owner(): user = factories.UserFactory() document = factories.DocumentFactory(users=[(user, RoleChoices.OWNER)]) comment = factories.CommentFactory( - document=document, user=random.choice([user, None]) + thread__document=document, user=random.choice([user, None]) ) assert comment.get_abilities(user) == { "destroy": True, "update": True, "partial_update": True, + "reactions": True, "retrieve": True, } diff --git a/src/backend/core/tests/test_models_document_accesses.py b/src/backend/core/tests/test_models_document_accesses.py index eb7675c00c..b8c3e93dd6 100644 --- a/src/backend/core/tests/test_models_document_accesses.py +++ b/src/backend/core/tests/test_models_document_accesses.py @@ -123,7 +123,7 @@ def test_models_document_access_get_abilities_for_owner_of_self_allowed(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commenter", "editor", "administrator", "owner"], } @@ -166,7 +166,7 @@ def test_models_document_access_get_abilities_for_owner_of_self_last_on_child( "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commenter", "editor", "administrator", "owner"], } @@ -183,7 +183,7 @@ def test_models_document_access_get_abilities_for_owner_of_owner(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commenter", "editor", "administrator", "owner"], } @@ -200,7 +200,7 @@ def test_models_document_access_get_abilities_for_owner_of_administrator(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commenter", "editor", "administrator", "owner"], } @@ -217,7 +217,7 @@ def test_models_document_access_get_abilities_for_owner_of_editor(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commenter", "editor", "administrator", "owner"], } @@ -234,7 +234,7 @@ def test_models_document_access_get_abilities_for_owner_of_reader(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator", "owner"], + "set_role_to": ["reader", "commenter", "editor", "administrator", "owner"], } @@ -271,7 +271,7 @@ def test_models_document_access_get_abilities_for_administrator_of_administrator "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator"], + "set_role_to": ["reader", "commenter", "editor", "administrator"], } @@ -288,7 +288,7 @@ def test_models_document_access_get_abilities_for_administrator_of_editor(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator"], + "set_role_to": ["reader", "commenter", "editor", "administrator"], } @@ -305,7 +305,7 @@ def test_models_document_access_get_abilities_for_administrator_of_reader(): "retrieve": True, "update": True, "partial_update": True, - "set_role_to": ["reader", "commentator", "editor", "administrator"], + "set_role_to": ["reader", "commenter", "editor", "administrator"], } diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 84f65a02ee..ba4f9ce678 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -134,13 +134,13 @@ def test_models_documents_soft_delete(depth): [ (True, "restricted", "reader"), (True, "restricted", "editor"), - (True, "restricted", "commentator"), + (True, "restricted", "commenter"), (False, "restricted", "reader"), (False, "restricted", "editor"), - (False, "restricted", "commentator"), + (False, "restricted", "commenter"), (False, "authenticated", "reader"), (False, "authenticated", "editor"), - (False, "authenticated", "commentator"), + (False, "authenticated", "commenter"), ], ) def test_models_documents_get_abilities_forbidden( @@ -176,8 +176,8 @@ def test_models_documents_get_abilities_forbidden( "move": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "partial_update": False, @@ -237,8 +237,8 @@ def test_models_documents_get_abilities_reader( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": is_authenticated, @@ -278,14 +278,14 @@ def test_models_documents_get_abilities_reader( (True, "authenticated"), ], ) -def test_models_documents_get_abilities_commentator( +def test_models_documents_get_abilities_commenter( is_authenticated, reach, django_assert_num_queries ): """ - Check abilities returned for a document giving commentator role to link holders + Check abilities returned for a document giving commenter role to link holders i.e anonymous users or authenticated users who have no specific role on the document. """ - document = factories.DocumentFactory(link_reach=reach, link_role="commentator") + document = factories.DocumentFactory(link_reach=reach, link_role="commenter") user = factories.UserFactory() if is_authenticated else AnonymousUser() expected_abilities = { "accesses_manage": False, @@ -298,6 +298,7 @@ def test_models_documents_get_abilities_commentator( "children_list": True, "collaboration_auth": True, "comment": True, + "content": True, "descendants": True, "cors_proxy": True, "destroy": False, @@ -306,8 +307,8 @@ def test_models_documents_get_abilities_commentator( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": is_authenticated, @@ -373,8 +374,8 @@ def test_models_documents_get_abilities_editor( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": is_authenticated, @@ -429,8 +430,8 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "invite_owner": True, "link_configuration": True, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": True, @@ -482,8 +483,8 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "invite_owner": False, "link_configuration": True, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": True, @@ -538,8 +539,8 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": True, @@ -592,7 +593,7 @@ def test_models_documents_get_abilities_reader_user( "children_list": True, "collaboration_auth": True, "comment": document.link_reach != "restricted" - and document.link_role in ["commentator", "editor"], + and document.link_role in ["commenter", "editor"], "descendants": True, "cors_proxy": True, "content": True, @@ -602,8 +603,8 @@ def test_models_documents_get_abilities_reader_user( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": True, @@ -634,12 +635,12 @@ def test_models_documents_get_abilities_reader_user( @pytest.mark.parametrize("ai_access_setting", ["public", "authenticated", "restricted"]) -def test_models_documents_get_abilities_commentator_user( +def test_models_documents_get_abilities_commenter_user( ai_access_setting, django_assert_num_queries ): - """Check abilities returned for the commentator of a document.""" + """Check abilities returned for the commenter of a document.""" user = factories.UserFactory() - document = factories.DocumentFactory(users=[(user, "commentator")]) + document = factories.DocumentFactory(users=[(user, "commenter")]) access_from_link = ( document.link_reach != "restricted" and document.link_role == "editor" @@ -658,6 +659,7 @@ def test_models_documents_get_abilities_commentator_user( "children_list": True, "collaboration_auth": True, "comment": True, + "content": True, "descendants": True, "cors_proxy": True, "destroy": False, @@ -666,8 +668,8 @@ def test_models_documents_get_abilities_commentator_user( "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": True, @@ -727,8 +729,8 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "invite_owner": False, "link_configuration": False, "link_select_options": { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], "restricted": None, }, "mask": True, @@ -1431,14 +1433,14 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): "public", "reader", { - "public": ["reader", "commentator", "editor"], + "public": ["reader", "commenter", "editor"], }, ), ( "public", - "commentator", + "commenter", { - "public": ["commentator", "editor"], + "public": ["commenter", "editor"], }, ), ("public", "editor", {"public": ["editor"]}), @@ -1446,16 +1448,16 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): "authenticated", "reader", { - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], }, ), ( "authenticated", - "commentator", + "commenter", { - "authenticated": ["commentator", "editor"], - "public": ["commentator", "editor"], + "authenticated": ["commenter", "editor"], + "public": ["commenter", "editor"], }, ), ( @@ -1468,17 +1470,17 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): "reader", { "restricted": None, - "authenticated": ["reader", "commentator", "editor"], - "public": ["reader", "commentator", "editor"], + "authenticated": ["reader", "commenter", "editor"], + "public": ["reader", "commenter", "editor"], }, ), ( "restricted", - "commentator", + "commenter", { "restricted": None, - "authenticated": ["commentator", "editor"], - "public": ["commentator", "editor"], + "authenticated": ["commenter", "editor"], + "public": ["commenter", "editor"], }, ), ( @@ -1495,15 +1497,15 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): "public", None, { - "public": ["reader", "commentator", "editor"], + "public": ["reader", "commenter", "editor"], }, ), ( None, "reader", { - "public": ["reader", "commentator", "editor"], - "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commenter", "editor"], + "authenticated": ["reader", "commenter", "editor"], "restricted": None, }, ), @@ -1511,8 +1513,8 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries): None, None, { - "public": ["reader", "commentator", "editor"], - "authenticated": ["reader", "commentator", "editor"], + "public": ["reader", "commenter", "editor"], + "authenticated": ["reader", "commenter", "editor"], "restricted": None, }, ), diff --git a/src/backend/core/urls.py b/src/backend/core/urls.py index 2df79fcc4a..b843c89171 100644 --- a/src/backend/core/urls.py +++ b/src/backend/core/urls.py @@ -27,9 +27,9 @@ basename="invitations", ) document_related_router.register( - "comments", - viewsets.CommentViewSet, - basename="comments", + "threads", + viewsets.ThreadViewSet, + basename="threads", ) document_related_router.register( "ask-for-access", @@ -37,6 +37,13 @@ basename="ask_for_access", ) +thread_related_router = DefaultRouter() +thread_related_router.register( + "comments", + viewsets.CommentViewSet, + basename="comments", +) + # - Routes nested under a template template_related_router = DefaultRouter() @@ -58,6 +65,10 @@ r"^documents/(?P[0-9a-z-]*)/", include(document_related_router.urls), ), + re_path( + r"^documents/(?P[0-9a-z-]*)/threads/(?P[0-9a-z-]*)/", + include(thread_related_router.urls), + ), re_path( r"^templates/(?P[0-9a-z-]*)/", include(template_related_router.urls), From 012d5c03f3886be6c8f59df0cc03991b9b515f32 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 12 Sep 2025 15:44:27 +0200 Subject: [PATCH 05/10] =?UTF-8?q?=E2=9C=A8(frontend)=20add=20comments=20fe?= =?UTF-8?q?ature?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implemented the comments feature for the document editor. We are now able to add, view, and manage comments within the document editor interface. --- CHANGELOG.md | 8 +- .../app-impress/doc-comments.spec.ts | 289 +++++++++ .../app-impress/doc-member-create.spec.ts | 4 +- .../app-impress/doc-visibility.spec.ts | 8 +- .../e2e/__tests__/app-impress/utils-common.ts | 12 +- .../e2e/__tests__/app-impress/utils-share.ts | 37 +- .../impress/src/features/auth/api/types.ts | 2 + .../doc-editor/components/BlockNoteEditor.tsx | 37 +- .../components/comments/DocsThreadStore.tsx | 569 ++++++++++++++++++ .../comments/DocsThreadStoreAuth.tsx | 94 +++ .../doc-editor/components/comments/index.ts | 2 + .../doc-editor/components/comments/styles.tsx | 202 +++++++ .../doc-editor/components/comments/types.ts | 55 ++ .../components/comments/useComments.ts | 30 + .../src/features/docs/doc-editor/styles.tsx | 9 +- .../features/docs/doc-management/types.tsx | 1 + .../service-worker/plugins/ApiPlugin.ts | 1 + 17 files changed, 1330 insertions(+), 30 deletions(-) create mode 100644 src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStore.tsx create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStoreAuth.tsx create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.ts create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsx create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/types.ts create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e67ebfddfd..b45de3fdcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to ### Added - โœจ(frontend) add pdf block to the editor #1293 +- โœจ Add comments feature to the editor #1330 ### Changed @@ -25,8 +26,10 @@ and this project adheres to - โ™ฟ remove redundant aria-label on hidden icons and update tests #1432 - โ™ฟ improve semantic structure and aria roles of leftpanel #1431 - โ™ฟ add default background to left panel for better accessibility #1423 + - โ™ฟimprove NVDA navigation in DocShareModal #1396 - โ™ฟ restyle checked checkboxes: removing strikethrough #1439 - โ™ฟ add h1 for SR on 40X pages and remove alt texts #1438 + ### Fixed - ๐Ÿ›(backend) duplicate sub docs as root for reader users @@ -35,11 +38,6 @@ and this project adheres to - ๐Ÿ›(frontend) fix legacy role computation #1376 - ๐Ÿ›(frontend) scroll back to top when navigate to a document #1406 -### Changed - -- โ™ฟ(frontend) improve accessibility: - - โ™ฟimprove NVDA navigation in DocShareModal #1396 - ## [3.7.0] - 2025-09-12 ### Added diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts new file mode 100644 index 0000000000..a53c561ebf --- /dev/null +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts @@ -0,0 +1,289 @@ +import { expect, test } from '@playwright/test'; + +import { createDoc, getOtherBrowserName, verifyDocName } from './utils-common'; +import { writeInEditor } from './utils-editor'; +import { + addNewMember, + connectOtherUserToDoc, + updateRoleUser, + updateShareLink, +} from './utils-share'; + +test.beforeEach(async ({ page }) => { + await page.goto('/'); +}); + +test.describe('Doc Comments', () => { + test('it checks comments with 2 users in real time', async ({ + page, + browserName, + }) => { + const [docTitle] = await createDoc(page, 'comment-doc', browserName, 1); + + // We share the doc with another user + const otherBrowserName = getOtherBrowserName(browserName); + await page.getByRole('button', { name: 'Share' }).click(); + await addNewMember(page, 0, 'Administrator', otherBrowserName); + + await expect( + page + .getByRole('listbox', { name: 'Suggestions' }) + .getByText(new RegExp(otherBrowserName)), + ).toBeVisible(); + + await page.getByRole('button', { name: 'close' }).click(); + + // We add a comment with the first user + const editor = await writeInEditor({ page, text: 'Hello World' }); + await editor.getByText('Hello').selectText(); + await page.getByRole('button', { name: 'Add comment' }).click(); + + const thread = page.locator('.bn-thread'); + await thread.getByRole('paragraph').first().fill('This is a comment'); + await thread.locator('[data-test="save"]').click(); + await expect(thread.getByText('This is a comment').first()).toBeHidden(); + + await editor.getByText('Hello').click(); + + await thread.getByText('This is a comment').first().hover(); + + // We add a reaction with the first user + await thread.locator('[data-test="addreaction"]').first().click(); + await thread.getByRole('button', { name: '๐Ÿ‘' }).click(); + + await expect(thread.getByText('This is a comment').first()).toBeVisible(); + await expect(thread.getByText(`E2E ${browserName}`).first()).toBeVisible(); + await expect(thread.locator('.bn-comment-reaction')).toHaveText('๐Ÿ‘1'); + + const urlCommentDoc = page.url(); + + const { otherPage, cleanup } = await connectOtherUserToDoc({ + otherBrowserName, + docUrl: urlCommentDoc, + docTitle, + }); + + const otherEditor = otherPage.locator('.ProseMirror'); + await otherEditor.getByText('Hello').click(); + const otherThread = otherPage.locator('.bn-thread'); + + await otherThread.getByText('This is a comment').first().hover(); + await otherThread.locator('[data-test="addreaction"]').first().click(); + await otherThread.getByRole('button', { name: '๐Ÿ‘' }).click(); + + // We check that the comment made by the first user is visible for the second user + await expect( + otherThread.getByText('This is a comment').first(), + ).toBeVisible(); + await expect( + otherThread.getByText(`E2E ${browserName}`).first(), + ).toBeVisible(); + await expect(otherThread.locator('.bn-comment-reaction')).toHaveText('๐Ÿ‘2'); + + // We add a comment with the second user + await otherThread + .getByRole('paragraph') + .last() + .fill('This is a comment from the other user'); + await otherThread.locator('[data-test="save"]').click(); + + // We check that the second user can see the comment he just made + await expect( + otherThread.getByText('This is a comment from the other user').first(), + ).toBeVisible(); + await expect( + otherThread.getByText(`E2E ${otherBrowserName}`).first(), + ).toBeVisible(); + + // We check that the first user can see the comment made by the second user in real time + await expect( + thread.getByText('This is a comment from the other user').first(), + ).toBeVisible(); + await expect( + thread.getByText(`E2E ${otherBrowserName}`).first(), + ).toBeVisible(); + + await cleanup(); + }); + + test('it checks the comments interactions', async ({ page, browserName }) => { + await createDoc(page, 'comment-interaction', browserName, 1); + + // Checks add react reaction + const editor = page.locator('.ProseMirror'); + await editor.locator('.bn-block-outer').last().fill('Hello World'); + await editor.getByText('Hello').selectText(); + await page.getByRole('button', { name: 'Add comment' }).click(); + + const thread = page.locator('.bn-thread'); + await thread.getByRole('paragraph').first().fill('This is a comment'); + await thread.locator('[data-test="save"]').click(); + await expect(thread.getByText('This is a comment').first()).toBeHidden(); + + // Check background color changed + await expect(editor.getByText('Hello')).toHaveCSS( + 'background-color', + 'rgba(237, 180, 0, 0.4)', + ); + await editor.getByText('Hello').click(); + + await thread.getByText('This is a comment').first().hover(); + + // We add a reaction with the first user + await thread.locator('[data-test="addreaction"]').first().click(); + await thread.getByRole('button', { name: '๐Ÿ‘' }).click(); + + await expect(thread.locator('.bn-comment-reaction')).toHaveText('๐Ÿ‘1'); + + // Edit Comment + await thread.getByText('This is a comment').first().hover(); + await thread.locator('[data-test="moreactions"]').first().click(); + await thread.getByRole('menuitem', { name: 'Edit comment' }).click(); + const commentEditor = thread.getByText('This is a comment').first(); + await commentEditor.fill('This is an edited comment'); + const saveBtn = thread.getByRole('button', { name: 'Save' }); + await saveBtn.click(); + await expect(saveBtn).toBeHidden(); + await expect( + thread.getByText('This is an edited comment').first(), + ).toBeVisible(); + await expect(thread.getByText('This is a comment').first()).toBeHidden(); + + // Add second comment + await thread.getByRole('paragraph').last().fill('This is a second comment'); + await thread.getByRole('button', { name: 'Save' }).click(); + await expect( + thread.getByText('This is an edited comment').first(), + ).toBeVisible(); + await expect( + thread.getByText('This is a second comment').first(), + ).toBeVisible(); + + // Delete second comment + await thread.getByText('This is a second comment').first().hover(); + await thread.locator('[data-test="moreactions"]').first().click(); + await thread.getByRole('menuitem', { name: 'Delete comment' }).click(); + await expect( + thread.getByText('This is a second comment').first(), + ).toBeHidden(); + + // Resolve thread + await thread.getByText('This is an edited comment').first().hover(); + await thread.locator('[data-test="resolve"]').click(); + await expect(thread).toBeHidden(); + await expect(editor.getByText('Hello')).toHaveCSS( + 'background-color', + 'rgba(0, 0, 0, 0)', + ); + }); + + test('it checks the comments abilities', async ({ page, browserName }) => { + test.slow(); + + const [docTitle] = await createDoc(page, 'comment-doc', browserName, 1); + + // We share the doc with another user + const otherBrowserName = getOtherBrowserName(browserName); + + // Add a new member with editor role + await page.getByRole('button', { name: 'Share' }).click(); + await addNewMember(page, 0, 'Editor', otherBrowserName); + + await expect( + page + .getByRole('listbox', { name: 'Suggestions' }) + .getByText(new RegExp(otherBrowserName)), + ).toBeVisible(); + + const urlCommentDoc = page.url(); + + const { otherPage, cleanup } = await connectOtherUserToDoc({ + otherBrowserName, + docUrl: urlCommentDoc, + docTitle, + }); + + const otherEditor = await writeInEditor({ + page: otherPage, + text: 'Hello, I can edit the document', + }); + await expect( + otherEditor.getByText('Hello, I can edit the document'), + ).toBeVisible(); + await otherEditor.getByText('Hello').selectText(); + await otherPage.getByRole('button', { name: 'Comment' }).click(); + const otherThread = otherPage.locator('.bn-thread'); + await otherThread + .getByRole('paragraph') + .first() + .fill('I can add a comment'); + await otherThread.locator('[data-test="save"]').click(); + await expect( + otherThread.getByText('I can add a comment').first(), + ).toBeHidden(); + + await expect(otherEditor.getByText('Hello')).toHaveCSS( + 'background-color', + 'rgba(237, 180, 0, 0.4)', + ); + + // We change the role of the second user to reader + updateRoleUser(page, 'Reader', `user.test@${otherBrowserName}.test`); + + // With the reader role, the second user cannot see comments + await otherPage.reload(); + await verifyDocName(otherPage, docTitle); + + await expect(otherEditor.getByText('Hello')).toHaveCSS( + 'background-color', + 'rgba(0, 0, 0, 0)', + ); + await otherEditor.getByText('Hello').click(); + await expect(otherThread).toBeHidden(); + await otherEditor.getByText('Hello').selectText(); + await expect( + otherPage.getByRole('button', { name: 'Comment' }), + ).toBeHidden(); + + await otherPage.reload(); + + // Change the link role of the doc to set it in commenting mode + updateShareLink(page, 'Public', 'Editing'); + + // Anonymous user can see and add comments + await otherPage.getByRole('button', { name: 'Logout' }).click(); + + await otherPage.goto(urlCommentDoc); + + await verifyDocName(otherPage, docTitle); + + await expect(otherEditor.getByText('Hello')).toHaveCSS( + 'background-color', + 'rgba(237, 180, 0, 0.4)', + ); + await otherEditor.getByText('Hello').click(); + await expect( + otherThread.getByText('I can add a comment').first(), + ).toBeVisible(); + + await otherThread + .locator('.ProseMirror.bn-editor[contenteditable="true"]') + .getByRole('paragraph') + .first() + .fill('Comment by anonymous user'); + await otherThread.locator('[data-test="save"]').click(); + + await expect( + otherThread.getByText('Comment by anonymous user').first(), + ).toBeVisible(); + + await expect( + otherThread.getByRole('img', { name: `Anonymous` }).first(), + ).toBeVisible(); + + await otherThread.getByText('Comment by anonymous user').first().hover(); + await expect(otherThread.locator('[data-test="moreactions"]')).toBeHidden(); + + await cleanup(); + }); +}); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts index 724d04ed40..d426ac50d9 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts @@ -229,11 +229,11 @@ test.describe('Document create member', () => { .last() .fill('Hello World'); - const urlDoc = page.url(); + const docUrl = page.url(); // Other user will request access const { otherPage, otherBrowserName, cleanup } = - await connectOtherUserToDoc(browserName, urlDoc); + await connectOtherUserToDoc({ browserName, docUrl }); await expect( otherPage.getByText('Insufficient access rights to view the document.'), diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts index 7027ccc114..0f761c3fd7 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts @@ -157,12 +157,12 @@ test.describe('Doc Visibility: Restricted', () => { .last() .fill('Hello World'); - const urlDoc = page.url(); + const docUrl = page.url(); - const { otherBrowserName, otherPage } = await connectOtherUserToDoc( + const { otherBrowserName, otherPage } = await connectOtherUserToDoc({ browserName, - urlDoc, - ); + docUrl, + }); await expect( otherPage.getByText('Insufficient access rights to view the document.'), diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts index d7387378a5..0c744a3bdf 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts @@ -69,6 +69,14 @@ export const keyCloakSignIn = async ( await page.click('button[type="submit"]', { force: true }); }; +export const getOtherBrowserName = (browserName: BrowserName) => { + const otherBrowserName = BROWSERS.find((b) => b !== browserName); + if (!otherBrowserName) { + throw new Error('No alternative browser found'); + } + return otherBrowserName; +}; + export const randomName = (name: string, browserName: string, length: number) => Array.from({ length }, (_el, index) => { return `${browserName}-${Math.floor(Math.random() * 10000)}-${index}-${name}`; @@ -124,7 +132,9 @@ export const verifyDocName = async (page: Page, docName: string) => { try { await expect( page.getByRole('textbox', { name: 'Document title' }), - ).toContainText(docName); + ).toContainText(docName, { + timeout: 1000, + }); } catch { await expect(page.getByRole('heading', { name: docName })).toBeVisible(); } diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts index 700ad8056b..091536a819 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts @@ -1,20 +1,20 @@ import { Page, chromium, expect } from '@playwright/test'; import { - BROWSERS, BrowserName, + getOtherBrowserName, keyCloakSignIn, verifyDocName, } from './utils-common'; export type Role = 'Administrator' | 'Owner' | 'Member' | 'Editor' | 'Reader'; export type LinkReach = 'Private' | 'Connected' | 'Public'; -export type LinkRole = 'Reading' | 'Edition'; +export type LinkRole = 'Reading' | 'Editing'; export const addNewMember = async ( page: Page, index: number, - role: 'Administrator' | 'Owner' | 'Editor' | 'Reader', + role: Role, fillText: string = 'user.test', ) => { const responsePromiseSearchUser = page.waitForResponse( @@ -88,15 +88,28 @@ export const updateRoleUser = async ( * @param docTitle The title of the document (optional). * @returns An object containing the other browser, context, and page. */ -export const connectOtherUserToDoc = async ( - browserName: BrowserName, - docUrl: string, - docTitle?: string, -) => { - const otherBrowserName = BROWSERS.find((b) => b !== browserName); - if (!otherBrowserName) { - throw new Error('No alternative browser found'); - } +type ConnectOtherUserToDocParams = { + docUrl: string; + docTitle?: string; +} & ( + | { + otherBrowserName: BrowserName; + browserName?: never; + } + | { + browserName: BrowserName; + otherBrowserName?: never; + } +); + +export const connectOtherUserToDoc = async ({ + browserName, + docUrl, + docTitle, + otherBrowserName: _otherBrowserName, +}: ConnectOtherUserToDocParams) => { + const otherBrowserName = + _otherBrowserName || getOtherBrowserName(browserName); const otherBrowser = await chromium.launch({ headless: true }); const otherContext = await otherBrowser.newContext({ diff --git a/src/frontend/apps/impress/src/features/auth/api/types.ts b/src/frontend/apps/impress/src/features/auth/api/types.ts index 680329d1cb..75a46581cf 100644 --- a/src/frontend/apps/impress/src/features/auth/api/types.ts +++ b/src/frontend/apps/impress/src/features/auth/api/types.ts @@ -13,3 +13,5 @@ export interface User { short_name: string; language?: string; } + +export type UserLight = Pick; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx index 68d7c269c5..4f813ac801 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx @@ -13,11 +13,13 @@ import { useCreateBlockNote } from '@blocknote/react'; import { HocuspocusProvider } from '@hocuspocus/provider'; import { useEffect } from 'react'; import { useTranslation } from 'react-i18next'; +import { css } from 'styled-components'; import * as Y from 'yjs'; import { Box, TextErrors } from '@/components'; import { Doc, useIsCollaborativeEditable } from '@/docs/doc-management'; import { useAuth } from '@/features/auth'; +import { useResponsiveStore } from '@/stores'; import { useHeadings, @@ -33,6 +35,7 @@ import { randomColor } from '../utils'; import { BlockNoteSuggestionMenu } from './BlockNoteSuggestionMenu'; import { BlockNoteToolbar } from './BlockNoteToolBar/BlockNoteToolbar'; +import { cssComments, useComments } from './comments/'; import { AccessibleImageBlock, CalloutBlock, @@ -79,10 +82,12 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { const { user } = useAuth(); const { setEditor } = useEditorStore(); const { t } = useTranslation(); + const { isDesktop } = useResponsiveStore(); const { isEditable, isLoading } = useIsCollaborativeEditable(doc); const isConnectedToCollabServer = provider.isSynced; const readOnly = !doc.abilities.partial_update || !isEditable || isLoading; + const canSeeComment = doc.abilities.comment && isDesktop; useSaveDoc(doc.id, provider.document, !readOnly, isConnectedToCollabServer); const { i18n } = useTranslation(); @@ -95,6 +100,8 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { : user?.full_name || user?.email || t('Anonymous'); const showCursorLabels: 'always' | 'activity' | (string & {}) = 'activity'; + const threadStore = useComments(doc, user); + const editor: DocsBlockNoteEditor = useCreateBlockNote( { codeBlock, @@ -147,11 +154,25 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { }, showCursorLabels: showCursorLabels as 'always' | 'activity', }, + comments: { threadStore }, dictionary: { ...locales[lang as keyof typeof locales], multi_column: multiColumnLocales?.[lang as keyof typeof multiColumnLocales], }, + resolveUsers: async (userIds) => { + return Promise.resolve( + userIds.map((encodedURIUserId) => { + const fullName = decodeURIComponent(encodedURIUserId); + + return { + id: encodedURIUserId, + username: fullName || t('Anonymous'), + avatarUrl: 'https://i.pravatar.cc/300', + }; + }), + ); + }, tables: { splitCells: true, cellBackgroundColor: true, @@ -161,7 +182,7 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { uploadFile, schema: blockNoteSchema, }, - [collabName, lang, provider, uploadFile], + [collabName, lang, provider, uploadFile, threadStore], ); useHeadings(editor); @@ -180,7 +201,10 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { {errorAttachment && ( @@ -194,11 +218,13 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { )} @@ -232,7 +258,12 @@ export const BlockNoteEditorVersion = ({ return ( - + ); }; 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 new file mode 100644 index 0000000000..f09c20b255 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStore.tsx @@ -0,0 +1,569 @@ +import { CommentBody, ThreadStore } from '@blocknote/core/comments'; +import type { Awareness } from 'y-protocols/awareness'; + +import { APIError, APIList, errorCauses, fetchAPI } from '@/api'; +import { Doc } from '@/features/docs/doc-management'; + +import { useEditorStore } from '../../stores'; + +import { DocsThreadStoreAuth } from './DocsThreadStoreAuth'; +import { + ClientCommentData, + ClientThreadData, + ServerComment, + ServerReaction, + ServerThread, +} from './types'; + +type ServerThreadListResponse = APIList; + +export class DocsThreadStore extends ThreadStore { + protected static COMMENTS_PING = 'commentsPing'; + protected threads: Map = new Map(); + private subscribers = new Set< + (threads: Map) => void + >(); + private awareness?: Awareness; + private lastPingAt = 0; + private pingTimer?: ReturnType; + + constructor( + protected docId: Doc['id'], + awareness: Awareness | undefined, + protected docAuth: DocsThreadStoreAuth, + ) { + super(docAuth); + + if (docAuth.canSee) { + this.awareness = awareness; + this.awareness?.on('update', this.onAwarenessUpdate); + void this.refreshThreads(); + } + } + + public destroy() { + this.awareness?.off('update', this.onAwarenessUpdate); + if (this.pingTimer) { + clearTimeout(this.pingTimer); + } + } + + private onAwarenessUpdate = async ({ + added, + updated, + }: { + added: number[]; + updated: number[]; + }) => { + if (!this.awareness) { + return; + } + const states = this.awareness.getStates(); + const listClientIds = [...added, ...updated]; + for (const clientId of listClientIds) { + // Skip our own client ID + if (clientId === this.awareness.clientID) { + continue; + } + + const state = states.get(clientId) as + | { + [DocsThreadStore.COMMENTS_PING]?: { + at: number; + docId: string; + isResolving: boolean; + threadId: string; + }; + } + | undefined; + + const ping = state?.commentsPing; + + // Skip if no ping information is available + if (!ping) { + continue; + } + + // Skip if the document ID doesn't match + if (ping.docId !== this.docId) { + continue; + } + + // Skip if the ping timestamp is past + if (ping.at <= this.lastPingAt) { + continue; + } + + this.lastPingAt = ping.at; + + // If we know the threadId, schedule a targeted refresh. Otherwise, fall back to full refresh. + if (ping.threadId) { + await this.refreshThread(ping.threadId); + } else { + await this.refreshThreads(); + } + } + }; + + /** + * To ping the other clients for updates on a specific thread + * @param threadId + */ + private ping(threadId?: string) { + this.awareness?.setLocalStateField(DocsThreadStore.COMMENTS_PING, { + at: Date.now(), + docId: this.docId, + threadId, + }); + } + + /** + * Notifies all subscribers about the current thread state + */ + private notifySubscribers() { + // Always emit a new Map reference to help consumers detect changes + const threads = new Map(this.threads); + this.subscribers.forEach((cb) => { + try { + cb(threads); + } catch (e) { + console.warn('DocsThreadStore subscriber threw', e); + } + }); + } + + private upsertClientThreadData(thread: ClientThreadData) { + const next = new Map(this.threads); + next.set(thread.id, thread); + this.threads = next; + } + + private removeThread(threadId: string) { + const next = new Map(this.threads); + next.delete(threadId); + this.threads = next; + } + + /** + * To subscribe to thread updates + * @param cb + * @returns + */ + public subscribe(cb: (threads: Map) => void) { + if (!this.docAuth.canSee) { + return () => {}; + } + + this.subscribers.add(cb); + + // Emit initial state asynchronously to avoid running during editor init + setTimeout(() => { + if (this.subscribers.has(cb)) { + cb(this.getThreads()); + } + }, 0); + + return () => { + this.subscribers.delete(cb); + }; + } + + public addThreadToDocument = (options: { + threadId: string; + selection: { + prosemirror: { + head: number; + anchor: number; + }; + yjs: { + head: unknown; + anchor: unknown; + }; + }; + }) => { + const { threadId } = options; + const { editor } = useEditorStore.getState(); + + // Should not happen + if (!editor) { + console.warn('Editor to add thread not ready'); + return Promise.resolve(); + } + + editor._tiptapEditor + .chain() + .focus?.() + .setMark?.('comment', { orphan: false, threadId }) + .run?.(); + + return Promise.resolve(); + }; + + public createThread = async (options: { + initialComment: { + body: CommentBody; + metadata?: unknown; + }; + metadata?: unknown; + }) => { + const response = await fetchAPI(`documents/${this.docId}/threads/`, { + method: 'POST', + body: JSON.stringify({ + body: options.initialComment.body, + }), + }); + + if (!response.ok) { + throw new APIError( + 'Failed to create thread in document', + await errorCauses(response), + ); + } + + const thread = (await response.json()) as ServerThread; + const threadData: ClientThreadData = serverThreadToClientThread(thread); + this.upsertClientThreadData(threadData); + this.notifySubscribers(); + this.ping(threadData.id); + return threadData; + }; + + public getThread(threadId: string) { + const thread = this.threads.get(threadId); + if (!thread) { + throw new Error('Thread not found'); + } + + return thread; + } + + public getThreads(): Map { + if (!this.docAuth.canSee) { + return new Map(); + } + + return this.threads; + } + + public async refreshThread(threadId: string) { + const response = await fetchAPI( + `documents/${this.docId}/threads/${threadId}/`, + { method: 'GET' }, + ); + + // If not OK and 404, the thread might have been deleted but the + // thread modal is still open, so we close it to avoid side effects + if (response.status === 404) { + // use escape key event to close the thread modal + document.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Escape', + code: 'Escape', + keyCode: 27, + bubbles: true, + cancelable: true, + }), + ); + + await this.refreshThreads(); + return; + } + + if (!response.ok) { + throw new APIError( + `Failed to fetch thread ${threadId}`, + await errorCauses(response), + ); + } + + const serverThread = (await response.json()) as ServerThread; + + const clientThread = serverThreadToClientThread(serverThread); + this.upsertClientThreadData(clientThread); + this.notifySubscribers(); + } + + public async refreshThreads(): Promise { + const response = await fetchAPI(`documents/${this.docId}/threads/`, { + method: 'GET', + }); + + if (!response.ok) { + throw new APIError( + 'Failed to get threads in document', + await errorCauses(response), + ); + } + + const threads = (await response.json()) as ServerThreadListResponse; + const next = new Map(); + threads.results.forEach((thread) => { + const threadData: ClientThreadData = serverThreadToClientThread(thread); + next.set(thread.id, threadData); + }); + this.threads = next; + this.notifySubscribers(); + } + + public addComment = async (options: { + comment: { + body: CommentBody; + metadata?: unknown; + }; + threadId: string; + }) => { + const { threadId } = options; + + const response = await fetchAPI( + `documents/${this.docId}/threads/${threadId}/comments/`, + { + method: 'POST', + body: JSON.stringify({ + body: options.comment.body, + }), + }, + ); + + if (!response.ok) { + throw new APIError('Failed to add comment ', await errorCauses(response)); + } + + const comment = (await response.json()) as ServerComment; + + // Optimistically update local thread with new comment + const existing = this.threads.get(threadId); + if (existing) { + const updated: ClientThreadData = { + ...existing, + updatedAt: new Date(comment.updated_at || comment.created_at), + comments: [...existing.comments, serverCommentToClientComment(comment)], + }; + this.upsertClientThreadData(updated); + this.notifySubscribers(); + } else { + // Fallback to fetching the thread if we don't have it locally + await this.refreshThread(threadId); + } + this.ping(threadId); + return serverCommentToClientComment(comment); + }; + + public updateComment = async (options: { + comment: { + body: CommentBody; + metadata?: unknown; + }; + threadId: string; + commentId: string; + }) => { + const { threadId, commentId, comment } = options; + + const response = await fetchAPI( + `documents/${this.docId}/threads/${threadId}/comments/${commentId}/`, + { + method: 'PUT', + body: JSON.stringify({ + body: comment.body, + }), + }, + ); + + if (!response.ok) { + throw new APIError( + 'Failed to add thread to document', + await errorCauses(response), + ); + } + + await this.refreshThread(threadId); + this.ping(threadId); + + return; + }; + + public deleteComment = async (options: { + threadId: string; + commentId: string; + softDelete?: boolean; + }) => { + const { threadId, commentId } = options; + + const response = await fetchAPI( + `documents/${this.docId}/threads/${threadId}/comments/${commentId}/`, + { + method: 'DELETE', + }, + ); + + if (!response.ok) { + throw new APIError( + 'Failed to delete comment', + await errorCauses(response), + ); + } + + // Optimistically remove the comment locally if we have the thread + const existing = this.threads.get(threadId); + if (existing) { + const updated: ClientThreadData = { + ...existing, + updatedAt: new Date(), + comments: existing.comments.filter((c) => c.id !== commentId), + }; + this.upsertClientThreadData(updated); + this.notifySubscribers(); + } else { + // Fallback to fetching the thread + await this.refreshThread(threadId); + } + this.ping(threadId); + }; + + /** + * UI not implemented + * @param _options + */ + public deleteThread = async (_options: { threadId: string }) => { + const response = await fetchAPI( + `documents/${this.docId}/threads/${_options.threadId}/`, + { + method: 'DELETE', + }, + ); + + if (!response.ok) { + throw new APIError( + 'Failed to delete thread', + await errorCauses(response), + ); + } + + // Remove locally and notify; no need to refetch everything + this.removeThread(_options.threadId); + this.notifySubscribers(); + this.ping(_options.threadId); + }; + + public resolveThread = async (_options: { threadId: string }) => { + const { threadId } = _options; + + const response = await fetchAPI( + `documents/${this.docId}/threads/${threadId}/resolve/`, + { method: 'POST' }, + ); + + if (!response.ok) { + throw new APIError( + 'Failed to resolve thread', + await errorCauses(response), + ); + } + + await this.refreshThreads(); + 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/`, + { method: 'POST' }, + ); + + if (!response.ok) { + throw new APIError( + 'Failed to unresolve thread', + await errorCauses(response), + ); + } + + await this.refreshThread(_options.threadId); + this.ping(_options.threadId); + }; + + public addReaction = async (options: { + threadId: string; + commentId: string; + emoji: string; + }) => { + const response = await fetchAPI( + `documents/${this.docId}/threads/${options.threadId}/comments/${options.commentId}/reactions/`, + { + method: 'POST', + body: JSON.stringify({ emoji: options.emoji }), + }, + ); + + if (!response.ok) { + throw new APIError( + 'Failed to add reaction to comment', + await errorCauses(response), + ); + } + + await this.refreshThread(options.threadId); + this.notifySubscribers(); + this.ping(options.threadId); + }; + + public deleteReaction = async (options: { + threadId: string; + commentId: string; + emoji: string; + }) => { + const response = await fetchAPI( + `documents/${this.docId}/threads/${options.threadId}/comments/${options.commentId}/reactions/`, + { method: 'DELETE', body: JSON.stringify({ emoji: options.emoji }) }, + ); + + if (!response.ok) { + throw new APIError( + 'Failed to delete reaction from comment', + await errorCauses(response), + ); + } + + await this.refreshThread(options.threadId); + this.notifySubscribers(); + this.ping(options.threadId); + }; +} + +const serverReactionToReactionData = (r: ServerReaction) => { + return { + emoji: r.emoji, + createdAt: new Date(r.created_at), + userIds: r.users?.map((user) => + encodeURIComponent(user.full_name || ''), + ) || [''], + }; +}; + +const serverCommentToClientComment = (c: ServerComment): ClientCommentData => ({ + type: 'comment', + id: c.id, + userId: encodeURIComponent(c.user?.full_name || ''), + body: c.body, + createdAt: new Date(c.created_at), + updatedAt: new Date(c.updated_at), + reactions: (c.reactions ?? []).map(serverReactionToReactionData), + metadata: { abilities: c.abilities }, +}); + +const serverThreadToClientThread = (t: ServerThread): ClientThreadData => ({ + type: 'thread', + id: t.id, + createdAt: new Date(t.created_at), + updatedAt: new Date(t.updated_at), + comments: (t.comments ?? []).map(serverCommentToClientComment), + resolved: t.resolved, + resolvedUpdatedAt: t.resolved_updated_at + ? new Date(t.resolved_updated_at) + : undefined, + resolvedBy: t.resolved_by || undefined, + metadata: { abilities: t.abilities, metadata: t.metadata }, +}); 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 new file mode 100644 index 0000000000..57f614813f --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStoreAuth.tsx @@ -0,0 +1,94 @@ +import { ThreadStoreAuth } from '@blocknote/core/comments'; + +import { ClientCommentData, ClientThreadData } from './types'; + +export class DocsThreadStoreAuth extends ThreadStoreAuth { + constructor( + private readonly userId: string, + public canSee: boolean, + ) { + super(); + } + + canCreateThread(): boolean { + return true; + } + + canAddComment(_thread: ClientThreadData): boolean { + return true; + } + + canUpdateComment(comment: ClientCommentData): boolean { + if ( + comment.metadata.abilities.partial_update && + comment.userId === this.userId + ) { + return true; + } + + return false; + } + + canDeleteComment(comment: ClientCommentData): boolean { + if (comment.metadata.abilities.destroy) { + return true; + } + + return false; + } + + canDeleteThread(thread: ClientThreadData): boolean { + if (thread.metadata.abilities.destroy) { + return true; + } + + return false; + } + + canResolveThread(thread: ClientThreadData): boolean { + if (thread.metadata.abilities.resolve) { + return true; + } + + return false; + } + + /** + * Not implemented backend side + * @param _thread + * @returns + */ + canUnresolveThread(_thread: ClientThreadData): boolean { + return false; + } + + canAddReaction(comment: ClientCommentData, emoji?: string): boolean { + if (!comment.metadata.abilities.reactions) { + return false; + } + + if (!emoji) { + return true; + } + + return !comment.reactions.some( + (reaction) => + reaction.emoji === emoji && reaction.userIds.includes(this.userId), + ); + } + + canDeleteReaction(comment: ClientCommentData, emoji?: string): boolean { + if (!comment.metadata.abilities.reactions) { + return false; + } + + if (!emoji) { + return true; + } + + return comment.reactions.some( + (reaction) => + reaction.emoji === emoji && reaction.userIds.includes(this.userId), + ); + } +} diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.ts b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.ts new file mode 100644 index 0000000000..28c0870b8e --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.ts @@ -0,0 +1,2 @@ +export * from './styles'; +export * from './useComments'; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsx new file mode 100644 index 0000000000..a3051a5a05 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsx @@ -0,0 +1,202 @@ +import { css } from 'styled-components'; + +export const cssComments = (canSeeComment: boolean) => css` + & .--docs--main-editor, + & .--docs--main-editor .ProseMirror { + // Comments marks in the editor + .bn-editor { + .bn-thread-mark:not([data-orphan='true']), + .bn-thread-mark-selected:not([data-orphan='true']) { + background: ${canSeeComment ? '#EDB40066' : 'transparent'}; + color: var(--c--theme--colors--greyscale-700); + } + } + + // Thread modal + .bn-thread { + width: 400px; + padding: 8px; + box-shadow: 0px 6px 18px 0px #00001229; + margin-left: 20px; + gap: 0; + + .bn-default-styles { + font-family: var(--c--theme--font--families--base); + } + + .bn-block { + font-size: 14px; + } + + .bn-inline-content:has(> .ProseMirror-trailingBreak:only-child):before { + font-style: normal; + font-size: 14px; + } + + // Remove tooltip + *[role='tooltip'] { + display: none; + } + + .bn-thread-comments { + overflow: auto; + max-height: 500px; + + // to allow popovers to escape the thread container + &:has(em-emoji-picker) { + max-height: none; + overflow: visible; + } + + em-emoji-picker { + box-shadow: 0px 6px 18px 0px #00001229; + } + } + + .bn-thread-comment { + padding: 8px; + + & .bn-editor { + padding-left: 32px; + .bn-inline-content { + color: var(--c--theme--colors--greyscale-700); + } + } + + // Emoji + & .bn-badge-group { + padding-left: 32px; + .bn-badge label { + padding: 0 4px; + background: none; + border: 1px solid var(--c--theme--colors--greyscale-300); + border-radius: 4px; + height: 24px; + } + } + + // Top bar (Name / Date / Actions) when actions displayed + &:has(.bn-comment-actions) { + & > .mantine-Group-root { + max-width: 70%; + right: 0.3rem !important; + top: 0.3rem !important; + } + } + + // Top bar (Name / Date / Actions) + & > .mantine-Group-root { + flex-wrap: nowrap; + max-width: 100%; + gap: 0.5rem; + + // Date + span.mantine-focus-auto { + display: inline-block; + } + + .bn-comment-actions { + background: transparent; + border: none; + + .mantine-Button-root { + background-color: transparent; + + &:hover { + background-color: var(--c--theme--colors--greyscale-100); + } + } + + button[role='menuitem'] svg { + color: var(--c--theme--colors--greyscale-600); + } + } + + & svg { + color: var(--c--theme--colors--info-600); + } + } + + // Actions button edit comment + .bn-container + .bn-comment-actions-wrapper { + .bn-comment-actions { + flex-direction: row-reverse; + background: none; + border: none; + gap: 0.4rem !important; + + & > button { + height: 24px; + padding-inline: 4px; + + &[data-test='save'] { + border: 1px solid var(--c--theme--colors--info-600); + background: var(--c--theme--colors--info-600); + color: white; + } + + &[data-test='cancel'] { + background: white; + border: 1px solid var(--c--theme--colors--greyscale-300); + color: var(--c--theme--colors--info-600); + } + } + } + } + } + + // Input to add a new comment + .bn-thread-composer, + &:has(> .bn-comment-editor + .bn-comment-actions-wrapper) { + padding: 0.5rem 8px; + flex-direction: row; + gap: 10px; + + .bn-container.bn-comment-editor { + min-width: 0; + } + } + + // Actions button send comment + .bn-thread-composer .bn-comment-actions-wrapper, + &:not(.selected) .bn-comment-actions-wrapper { + flex-basis: fit-content; + + .bn-action-toolbar.bn-comment-actions { + border: none; + + button { + font-size: 0; + background: var(--c--theme--colors--info-600); + width: 24px; + height: 24px; + padding: 0; + + &:disabled { + background: var(--c--theme--colors--greyscale-300); + } + + & .mantine-Button-label::before { + content: '๐Ÿกก'; + font-size: 13px; + color: var(--c--theme--colors--greyscale-100); + } + } + } + } + + // Input first comment + &:not(.selected) { + gap: 0.5rem; + + .bn-container.bn-comment-editor { + min-width: 0; + + .ProseMirror.bn-editor { + cursor: text; + } + } + } + } + } +`; 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 new file mode 100644 index 0000000000..be47816572 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/types.ts @@ -0,0 +1,55 @@ +import { CommentData, ThreadData } from '@blocknote/core/comments'; + +import { UserLight } from '@/features/auth'; + +export interface CommentAbilities { + destroy: boolean; + update: boolean; + partial_update: boolean; + retrieve: boolean; + reactions: boolean; +} +export interface ThreadAbilities { + destroy: boolean; + update: boolean; + partial_update: boolean; + retrieve: boolean; + resolve: boolean; +} + +export interface ServerReaction { + emoji: string; + created_at: string; + users: UserLight[] | null; +} + +export interface ServerComment { + id: string; + user: UserLight | null; + body: unknown; + created_at: string; + updated_at: string; + reactions: ServerReaction[]; + abilities: CommentAbilities; +} + +export interface ServerThread { + id: string; + created_at: string; + updated_at: string; + user: UserLight | null; + resolved: boolean; + resolved_updated_at: string | null; + resolved_by: string | null; + metadata: unknown; + comments: ServerComment[]; + abilities: ThreadAbilities; +} + +export type ClientCommentData = Omit & { + metadata: { abilities: CommentAbilities }; +}; + +export type ClientThreadData = Omit & { + metadata: { abilities: ThreadAbilities; metadata: unknown }; +}; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts new file mode 100644 index 0000000000..3e53bba1c1 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts @@ -0,0 +1,30 @@ +import { useEffect, useMemo } from 'react'; +import type { Awareness } from 'y-protocols/awareness'; + +import { User } from '@/features/auth'; +import { Doc, useProviderStore } from '@/features/docs/doc-management'; + +import { DocsThreadStore } from './DocsThreadStore'; +import { DocsThreadStoreAuth } from './DocsThreadStoreAuth'; + +export function useComments(doc: Doc, user: User | null | undefined) { + const { provider } = useProviderStore(); + const threadStore = useMemo(() => { + return new DocsThreadStore( + doc.id, + (provider?.awareness ?? undefined) as Awareness | undefined, + new DocsThreadStoreAuth( + encodeURIComponent(user?.full_name || ''), + doc.abilities.comment, + ), + ); + }, [doc.id, doc.abilities.comment, provider?.awareness, user?.full_name]); + + useEffect(() => { + return () => { + threadStore?.destroy(); + }; + }, [threadStore]); + + return threadStore; +} diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/styles.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/styles.tsx index 786e9a0d36..4f57e9661f 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/styles.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/styles.tsx @@ -1,11 +1,14 @@ import { css } from 'styled-components'; export const cssEditor = (readonly: boolean) => css` - &, - & > .bn-container, - & .ProseMirror { + & { height: 100%; padding-bottom: 2rem; + } + + & .--docs--main-editor, + & .--docs--main-editor .ProseMirror { + height: 100%; /** * WCAG Accessibility contrast fixes for BlockNote editor diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/types.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/types.tsx index c6fd9f2c0d..1c5f65540c 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/types.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/types.tsx @@ -79,6 +79,7 @@ export interface Doc { children_create: boolean; children_list: boolean; collaboration_auth: boolean; + comment: boolean; destroy: boolean; duplicate: boolean; favorite: boolean; diff --git a/src/frontend/apps/impress/src/features/service-worker/plugins/ApiPlugin.ts b/src/frontend/apps/impress/src/features/service-worker/plugins/ApiPlugin.ts index f050e98ee1..d7981e79f6 100644 --- a/src/frontend/apps/impress/src/features/service-worker/plugins/ApiPlugin.ts +++ b/src/frontend/apps/impress/src/features/service-worker/plugins/ApiPlugin.ts @@ -187,6 +187,7 @@ export class ApiPlugin implements WorkboxPlugin { children_create: true, children_list: true, collaboration_auth: true, + comment: true, destroy: true, duplicate: true, favorite: true, From 5546da8ea6a31286c0db54e1f460dfed1b6cae0f Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 12 Sep 2025 15:45:39 +0200 Subject: [PATCH 06/10] =?UTF-8?q?=E2=99=BB=EF=B8=8F(frontend)=20add=20user?= =?UTF-8?q?=20avatar=20to=20thread=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We extracted the UserAvatar component from the doc-share feature and integrated it into the users feature. It will be used in the thread comments feature as well. --- .../app-impress/doc-comments.spec.ts | 6 ++ src/frontend/apps/impress/cunningham.ts | 4 +- .../src/cunningham/cunningham-tokens.css | 6 +- .../src/cunningham/cunningham-tokens.ts | 7 +- .../features/auth/components/AvatarSvg.tsx | 49 +++++++++++++ .../features/auth/components/UserAvatar.tsx | 70 +++++++++++++++++++ .../src/features/auth/components/index.ts | 1 + .../doc-editor/components/BlockNoteEditor.tsx | 24 ++++--- .../doc-editor/components/comments/styles.tsx | 18 ++++- .../doc-share/components/SearchUserRow.tsx | 6 +- .../docs/doc-share/components/UserAvatar.tsx | 62 ---------------- 11 files changed, 173 insertions(+), 80 deletions(-) create mode 100644 src/frontend/apps/impress/src/features/auth/components/AvatarSvg.tsx create mode 100644 src/frontend/apps/impress/src/features/auth/components/UserAvatar.tsx delete mode 100644 src/frontend/apps/impress/src/features/docs/doc-share/components/UserAvatar.tsx diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts index a53c561ebf..3db5cce332 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts @@ -51,6 +51,9 @@ test.describe('Doc Comments', () => { await thread.locator('[data-test="addreaction"]').first().click(); await thread.getByRole('button', { name: '๐Ÿ‘' }).click(); + await expect( + thread.getByRole('img', { name: 'E2E Chromium' }).first(), + ).toBeVisible(); await expect(thread.getByText('This is a comment').first()).toBeVisible(); await expect(thread.getByText(`E2E ${browserName}`).first()).toBeVisible(); await expect(thread.locator('.bn-comment-reaction')).toHaveText('๐Ÿ‘1'); @@ -88,6 +91,9 @@ test.describe('Doc Comments', () => { await otherThread.locator('[data-test="save"]').click(); // We check that the second user can see the comment he just made + await expect( + otherThread.getByRole('img', { name: `E2E ${otherBrowserName}` }).first(), + ).toBeVisible(); await expect( otherThread.getByText('This is a comment from the other user').first(), ).toBeVisible(); diff --git a/src/frontend/apps/impress/cunningham.ts b/src/frontend/apps/impress/cunningham.ts index 10b51205b2..5accadd33c 100644 --- a/src/frontend/apps/impress/cunningham.ts +++ b/src/frontend/apps/impress/cunningham.ts @@ -98,8 +98,8 @@ const dsfrTheme = { }, font: { families: { - base: 'Marianne', - accent: 'Marianne', + base: 'Marianne, Inter, Roboto Flex Variable, sans-serif', + accent: 'Marianne, Inter, Roboto Flex Variable, sans-serif', }, }, }, diff --git a/src/frontend/apps/impress/src/cunningham/cunningham-tokens.css b/src/frontend/apps/impress/src/cunningham/cunningham-tokens.css index 3b1c544def..8bfbc0977b 100644 --- a/src/frontend/apps/impress/src/cunningham/cunningham-tokens.css +++ b/src/frontend/apps/impress/src/cunningham/cunningham-tokens.css @@ -556,8 +556,10 @@ --c--theme--logo--widthHeader: 110px; --c--theme--logo--widthFooter: 220px; --c--theme--logo--alt: gouvernement logo; - --c--theme--font--families--base: marianne; - --c--theme--font--families--accent: marianne; + --c--theme--font--families--base: + marianne, inter, roboto flex variable, sans-serif; + --c--theme--font--families--accent: + marianne, inter, roboto flex variable, sans-serif; --c--components--la-gaufre: true; --c--components--home-proconnect: true; --c--components--favicon--ico: /assets/favicon-dsfr.ico; diff --git a/src/frontend/apps/impress/src/cunningham/cunningham-tokens.ts b/src/frontend/apps/impress/src/cunningham/cunningham-tokens.ts index f0ebb07ea5..6261deb30a 100644 --- a/src/frontend/apps/impress/src/cunningham/cunningham-tokens.ts +++ b/src/frontend/apps/impress/src/cunningham/cunningham-tokens.ts @@ -436,7 +436,12 @@ export const tokens = { widthFooter: '220px', alt: 'Gouvernement Logo', }, - font: { families: { base: 'Marianne', accent: 'Marianne' } }, + font: { + families: { + base: 'Marianne, Inter, Roboto Flex Variable, sans-serif', + accent: 'Marianne, Inter, Roboto Flex Variable, sans-serif', + }, + }, }, components: { 'la-gaufre': true, diff --git a/src/frontend/apps/impress/src/features/auth/components/AvatarSvg.tsx b/src/frontend/apps/impress/src/features/auth/components/AvatarSvg.tsx new file mode 100644 index 0000000000..44c183f0e8 --- /dev/null +++ b/src/frontend/apps/impress/src/features/auth/components/AvatarSvg.tsx @@ -0,0 +1,49 @@ +import React from 'react'; + +import { Box, BoxType } from '@/components'; + +type AvatarSvgProps = { + initials: string; + background: string; + fontFamily?: string; +} & BoxType; + +export const AvatarSvg: React.FC = ({ + initials, + background, + fontFamily, + ...props +}) => ( + + + + {initials} + + +); diff --git a/src/frontend/apps/impress/src/features/auth/components/UserAvatar.tsx b/src/frontend/apps/impress/src/features/auth/components/UserAvatar.tsx new file mode 100644 index 0000000000..9bf836c5de --- /dev/null +++ b/src/frontend/apps/impress/src/features/auth/components/UserAvatar.tsx @@ -0,0 +1,70 @@ +import { renderToStaticMarkup } from 'react-dom/server'; + +import { tokens } from '@/cunningham'; + +import { AvatarSvg } from './AvatarSvg'; + +const colors = tokens.themes.default.theme.colors; + +const avatarsColors = [ + colors['blue-500'], + colors['brown-500'], + colors['cyan-500'], + colors['gold-500'], + colors['green-500'], + colors['olive-500'], + colors['orange-500'], + colors['pink-500'], + colors['purple-500'], + colors['yellow-500'], +]; + +const getColorFromName = (name: string) => { + let hash = 0; + for (let i = 0; i < name.length; i++) { + hash = name.charCodeAt(i) + ((hash << 5) - hash); + } + return avatarsColors[Math.abs(hash) % avatarsColors.length]; +}; + +const getInitialFromName = (name: string) => { + const splitName = name?.split(' '); + return (splitName[0]?.charAt(0) || '?') + (splitName?.[1]?.charAt(0) || ''); +}; + +type UserAvatarProps = { + fullName?: string; + background?: string; +}; + +export const UserAvatar = ({ fullName, background }: UserAvatarProps) => { + const name = fullName?.trim() || '?'; + + return ( + + ); +}; + +export const avatarUrlFromName = ( + fullName?: string, + fontFamily?: string, +): string => { + const name = fullName?.trim() || '?'; + const initials = getInitialFromName(name).toUpperCase(); + const background = getColorFromName(name); + + const svgMarkup = renderToStaticMarkup( + , + ); + + return `data:image/svg+xml;charset=UTF-8,${encodeURIComponent(svgMarkup)}`; +}; diff --git a/src/frontend/apps/impress/src/features/auth/components/index.ts b/src/frontend/apps/impress/src/features/auth/components/index.ts index 17f3a9058a..26ebaf2e8b 100644 --- a/src/frontend/apps/impress/src/features/auth/components/index.ts +++ b/src/frontend/apps/impress/src/features/auth/components/index.ts @@ -1,2 +1,3 @@ export * from './Auth'; export * from './ButtonLogin'; +export * from './UserAvatar'; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx index 4f813ac801..d0cabcdca1 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx @@ -17,8 +17,9 @@ import { css } from 'styled-components'; import * as Y from 'yjs'; import { Box, TextErrors } from '@/components'; +import { useCunninghamTheme } from '@/cunningham'; import { Doc, useIsCollaborativeEditable } from '@/docs/doc-management'; -import { useAuth } from '@/features/auth'; +import { avatarUrlFromName, useAuth } from '@/features/auth'; import { useResponsiveStore } from '@/stores'; import { @@ -82,6 +83,7 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { const { user } = useAuth(); const { setEditor } = useEditorStore(); const { t } = useTranslation(); + const { themeTokens } = useCunninghamTheme(); const { isDesktop } = useResponsiveStore(); const { isEditable, isLoading } = useIsCollaborativeEditable(doc); @@ -95,13 +97,16 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { const { uploadFile, errorAttachment } = useUploadFile(doc.id); - const collabName = readOnly - ? 'Reader' - : user?.full_name || user?.email || t('Anonymous'); + const collabName = user?.full_name || user?.email; + const cursorName = readOnly ? 'Reader' : collabName || t('Anonymous'); const showCursorLabels: 'always' | 'activity' | (string & {}) = 'activity'; const threadStore = useComments(doc, user); + const currentUserAvatarUrl = canSeeComment + ? avatarUrlFromName(collabName, themeTokens?.font?.families?.base) + : undefined; + const editor: DocsBlockNoteEditor = useCreateBlockNote( { codeBlock, @@ -109,7 +114,7 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { provider, fragment: provider.document.getXmlFragment('document-store'), user: { - name: collabName, + name: cursorName, color: randomColor(), }, /** @@ -168,7 +173,10 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { return { id: encodedURIUserId, username: fullName || t('Anonymous'), - avatarUrl: 'https://i.pravatar.cc/300', + avatarUrl: avatarUrlFromName( + fullName, + themeTokens?.font?.families?.base, + ), }; }), ); @@ -182,7 +190,7 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { uploadFile, schema: blockNoteSchema, }, - [collabName, lang, provider, uploadFile, threadStore], + [cursorName, lang, provider, uploadFile, threadStore], ); useHeadings(editor); @@ -203,7 +211,7 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { $background="white" $css={css` ${cssEditor(readOnly)}; - ${cssComments(canSeeComment)} + ${cssComments(canSeeComment, currentUserAvatarUrl)} `} className="--docs--editor-container" > diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsx index a3051a5a05..446e0ceec7 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsx @@ -1,6 +1,9 @@ import { css } from 'styled-components'; -export const cssComments = (canSeeComment: boolean) => css` +export const cssComments = ( + canSeeComment: boolean, + currentUserAvatarUrl?: string, +) => css` & .--docs--main-editor, & .--docs--main-editor .ProseMirror { // Comments marks in the editor @@ -155,6 +158,19 @@ export const cssComments = (canSeeComment: boolean) => css` .bn-container.bn-comment-editor { min-width: 0; } + + &::before { + content: ''; + width: 26px; + height: 26px; + flex: 0 0 26px; + background-image: ${currentUserAvatarUrl + ? `url("${currentUserAvatarUrl}")` + : 'none'}; + background-position: center; + background-repeat: no-repeat; + background-size: cover; + } } // Actions button send comment diff --git a/src/frontend/apps/impress/src/features/docs/doc-share/components/SearchUserRow.tsx b/src/frontend/apps/impress/src/features/docs/doc-share/components/SearchUserRow.tsx index 109a60763f..cec9bf675b 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-share/components/SearchUserRow.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-share/components/SearchUserRow.tsx @@ -4,9 +4,7 @@ import { QuickSearchItemContentProps, } from '@/components/quick-search'; import { useCunninghamTheme } from '@/cunningham'; -import { User } from '@/features/auth'; - -import { UserAvatar } from './UserAvatar'; +import { User, UserAvatar } from '@/features/auth'; type Props = { user: User; @@ -36,7 +34,7 @@ export const SearchUserRow = ({ className="--docs--search-user-row" > { - let hash = 0; - for (let i = 0; i < name.length; i++) { - hash = name.charCodeAt(i) + ((hash << 5) - hash); - } - return avatarsColors[Math.abs(hash) % avatarsColors.length]; -}; - -type Props = { - user: User; - background?: string; -}; - -export const UserAvatar = ({ user, background }: Props) => { - const name = user.full_name || user.email || '?'; - const splitName = name?.split(' '); - - return ( - - {splitName[0]?.charAt(0)} - {splitName?.[1]?.charAt(0)} - - ); -}; From ad5061a404f9c062ba9268e909abf6afd8acf6f3 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 12 Sep 2025 15:54:10 +0200 Subject: [PATCH 07/10] =?UTF-8?q?=F0=9F=90=9B(frontend)=20fix=20circular?= =?UTF-8?q?=20dependency=20problems?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A circular dependency was introduced in the previous commit. This commit resolves the circular dependency by refactoring the code to remove the circular reference. --- .../src/features/docs/doc-share/api/useCreateDocAccess.tsx | 2 +- .../src/features/docs/doc-share/api/useCreateDocInvitation.tsx | 3 ++- .../src/features/docs/doc-share/api/useDeleteDocAccess.ts | 2 +- .../src/features/docs/doc-share/api/useDocInvitations.tsx | 3 ++- .../src/features/docs/doc-share/api/useUpdateDocInvitation.ts | 3 ++- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/frontend/apps/impress/src/features/docs/doc-share/api/useCreateDocAccess.tsx b/src/frontend/apps/impress/src/features/docs/doc-share/api/useCreateDocAccess.tsx index c391dbfa42..db6de8c193 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-share/api/useCreateDocAccess.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-share/api/useCreateDocAccess.tsx @@ -8,12 +8,12 @@ import { KEY_LIST_DOC, Role, } from '@/docs/doc-management'; -import { KEY_LIST_DOC_ACCESSES } from '@/docs/doc-share'; import { User } from '@/features/auth'; import { useBroadcastStore } from '@/stores'; import { OptionType } from '../types'; +import { KEY_LIST_DOC_ACCESSES } from './useDocAccesses'; import { KEY_LIST_USER } from './useUsers'; interface CreateDocAccessParams { diff --git a/src/frontend/apps/impress/src/features/docs/doc-share/api/useCreateDocInvitation.tsx b/src/frontend/apps/impress/src/features/docs/doc-share/api/useCreateDocInvitation.tsx index 536a5e2487..a13e423576 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-share/api/useCreateDocInvitation.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-share/api/useCreateDocInvitation.tsx @@ -2,9 +2,10 @@ import { useMutation, useQueryClient } from '@tanstack/react-query'; import { APIError, errorCauses, fetchAPI } from '@/api'; import { Doc, Role } from '@/docs/doc-management'; -import { Invitation, OptionType } from '@/docs/doc-share/types'; import { User } from '@/features/auth'; +import { Invitation, OptionType } from '../types'; + import { KEY_LIST_DOC_INVITATIONS } from './useDocInvitations'; interface CreateDocInvitationParams { diff --git a/src/frontend/apps/impress/src/features/docs/doc-share/api/useDeleteDocAccess.ts b/src/frontend/apps/impress/src/features/docs/doc-share/api/useDeleteDocAccess.ts index a563ea7e81..6b99e79e9f 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-share/api/useDeleteDocAccess.ts +++ b/src/frontend/apps/impress/src/features/docs/doc-share/api/useDeleteDocAccess.ts @@ -6,10 +6,10 @@ import { import { APIError, errorCauses, fetchAPI } from '@/api'; import { KEY_DOC, KEY_LIST_DOC } from '@/docs/doc-management'; -import { KEY_LIST_USER } from '@/docs/doc-share'; import { useBroadcastStore } from '@/stores'; import { KEY_LIST_DOC_ACCESSES } from './useDocAccesses'; +import { KEY_LIST_USER } from './useUsers'; interface DeleteDocAccessProps { docId: string; diff --git a/src/frontend/apps/impress/src/features/docs/doc-share/api/useDocInvitations.tsx b/src/frontend/apps/impress/src/features/docs/doc-share/api/useDocInvitations.tsx index 150f99adb0..dbea614b9e 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-share/api/useDocInvitations.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-share/api/useDocInvitations.tsx @@ -7,7 +7,8 @@ import { fetchAPI, useAPIInfiniteQuery, } from '@/api'; -import { Invitation } from '@/docs/doc-share/types'; + +import { Invitation } from '../types'; export type DocInvitationsParams = { docId: string; diff --git a/src/frontend/apps/impress/src/features/docs/doc-share/api/useUpdateDocInvitation.ts b/src/frontend/apps/impress/src/features/docs/doc-share/api/useUpdateDocInvitation.ts index 51f22950f7..f7befde186 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-share/api/useUpdateDocInvitation.ts +++ b/src/frontend/apps/impress/src/features/docs/doc-share/api/useUpdateDocInvitation.ts @@ -6,7 +6,8 @@ import { import { APIError, errorCauses, fetchAPI } from '@/api'; import { Role } from '@/docs/doc-management'; -import { Invitation } from '@/docs/doc-share/types'; + +import { Invitation } from '../types'; import { KEY_LIST_DOC_INVITATIONS } from './useDocInvitations'; From 3400ce148a1a3e4a6a19f78a73e231e8b503a3fd Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 12 Sep 2025 15:55:31 +0200 Subject: [PATCH 08/10] =?UTF-8?q?=E2=99=BB=EF=B8=8F(frontend)=20replace=20?= =?UTF-8?q?default=20comment=20toolbar=20button?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the default comment toolbar button with a custom one to follow the design system. --- .../app-impress/doc-comments.spec.ts | 4 +- .../BlockNoteToolBar/BlockNoteToolbar.tsx | 17 +++- .../comments/CommentToolbarButton.tsx | 81 +++++++++++++++++++ .../doc-editor/components/comments/index.ts | 1 + 4 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/CommentToolbarButton.tsx diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts index 3db5cce332..e1f504f853 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.ts @@ -36,7 +36,7 @@ test.describe('Doc Comments', () => { // We add a comment with the first user const editor = await writeInEditor({ page, text: 'Hello World' }); await editor.getByText('Hello').selectText(); - await page.getByRole('button', { name: 'Add comment' }).click(); + await page.getByRole('button', { name: 'Comment' }).click(); const thread = page.locator('.bn-thread'); await thread.getByRole('paragraph').first().fill('This is a comment'); @@ -119,7 +119,7 @@ test.describe('Doc Comments', () => { const editor = page.locator('.ProseMirror'); await editor.locator('.bn-block-outer').last().fill('Hello World'); await editor.getByText('Hello').selectText(); - await page.getByRole('button', { name: 'Add comment' }).click(); + await page.getByRole('button', { name: 'Comment' }).click(); const thread = page.locator('.bn-thread'); await thread.getByRole('paragraph').first().fill('This is a comment'); diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/BlockNoteToolbar.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/BlockNoteToolbar.tsx index 6b7a8181b5..b88e09c7a4 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/BlockNoteToolbar.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/BlockNoteToolbar.tsx @@ -10,6 +10,7 @@ import { useTranslation } from 'react-i18next'; import { useConfig } from '@/core/config/api'; +import { CommentToolbarButton } from '../comments/CommentToolbarButton'; import { getCalloutFormattingToolbarItems } from '../custom-blocks'; import { AIGroupButton } from './AIButton'; @@ -25,10 +26,12 @@ export const BlockNoteToolbar = () => { const { data: conf } = useConfig(); const toolbarItems = useMemo(() => { - const toolbarItems = getFormattingToolbarItems([ + let toolbarItems = getFormattingToolbarItems([ ...blockTypeSelectItems(dict), getCalloutFormattingToolbarItems(t), ]); + + // Find the index of the file download button const fileDownloadButtonIndex = toolbarItems.findIndex( (item) => typeof item === 'object' && @@ -36,6 +39,8 @@ export const BlockNoteToolbar = () => { 'key' in item && (item as { key: string }).key === 'fileDownloadButton', ); + + // Replace the default file download button with our custom FileDownloadButton if (fileDownloadButtonIndex !== -1) { toolbarItems.splice( fileDownloadButtonIndex, @@ -50,12 +55,22 @@ export const BlockNoteToolbar = () => { ); } + // Remove default Comment button + toolbarItems = toolbarItems.filter((item) => { + if (typeof item === 'object' && item !== null && 'key' in item) { + return item.key !== 'addCommentButton'; + } + return true; + }); + return toolbarItems; }, [dict, t]); const formattingToolbar = useCallback(() => { return ( + + {toolbarItems} {/* Extra button to do some AI powered actions */} diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/CommentToolbarButton.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/CommentToolbarButton.tsx new file mode 100644 index 0000000000..3e81a1fff5 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/CommentToolbarButton.tsx @@ -0,0 +1,81 @@ +import { useBlockNoteEditor, useComponentsContext } from '@blocknote/react'; +import { useTranslation } from 'react-i18next'; +import { css } from 'styled-components'; + +import { Box, Icon } from '@/components'; +import { useCunninghamTheme } from '@/cunningham'; +import { useDocStore } from '@/features/docs/doc-management'; + +import { + DocsBlockSchema, + DocsInlineContentSchema, + DocsStyleSchema, +} from '../../types'; + +export const CommentToolbarButton = () => { + const Components = useComponentsContext(); + const { currentDoc } = useDocStore(); + const { t } = useTranslation(); + const { spacingsTokens, colorsTokens } = useCunninghamTheme(); + const editor = useBlockNoteEditor< + DocsBlockSchema, + DocsInlineContentSchema, + DocsStyleSchema + >(); + + const hasActiveUnresolvedThread = editor._tiptapEditor.isActive('comment', { + orphan: false, + }); + + if ( + !editor.isEditable || + !Components || + !currentDoc?.abilities.comment || + hasActiveUnresolvedThread + ) { + return null; + } + + return ( + + { + editor.comments?.startPendingComment(); + }} + isDisabled={hasActiveUnresolvedThread} + > + + + {t('Comment')} + + + + + ); +}; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.ts b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.ts index 28c0870b8e..99acd58df1 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.ts +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.ts @@ -1,2 +1,3 @@ +export * from './CommentToolbarButton'; export * from './styles'; export * from './useComments'; From c246367f944fb8b5b6924e641299b15ebfbfe213 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Wed, 1 Oct 2025 18:45:18 +0200 Subject: [PATCH 09/10] =?UTF-8?q?=F0=9F=90=9B(frontend)=20fix=20button=20m?= =?UTF-8?q?arkdown=20not=20visible?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On smaller screens, the markdown button in the toolbar was not every time visible. We fix this issue. --- .../BlockNoteToolBar/MarkdownButton.tsx | 19 ++++++++++++++++--- .../src/features/docs/doc-editor/styles.tsx | 4 ++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/MarkdownButton.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/MarkdownButton.tsx index 3589562121..f128a76471 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/MarkdownButton.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/MarkdownButton.tsx @@ -6,6 +6,9 @@ import { import { forEach, isArray } from 'lodash'; import React, { useMemo } from 'react'; import { useTranslation } from 'react-i18next'; +import { css } from 'styled-components'; + +import { Text } from '@/components'; type Block = { type: string; @@ -83,8 +86,18 @@ export function MarkdownButton() { mainTooltip={t('Convert Markdown')} onClick={handleConvertMarkdown} className="--docs--editor-markdown-button" - > - M - + label="M" + icon={ + + M + + } + /> ); } diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/styles.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/styles.tsx index 4f57e9661f..2fe0e927ce 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/styles.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/styles.tsx @@ -117,6 +117,10 @@ export const cssEditor = (readonly: boolean) => css` .bn-block-outer:not([data-prev-depth-changed]):before { border-left: none; } + + .bn-toolbar { + max-width: 95vw; + } } & .bn-editor { From 68594f5af01fd85c91f7c9ef1986e0f71053adfa Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Mon, 29 Sep 2025 14:44:53 +0200 Subject: [PATCH 10/10] test-on-staging --- .github/workflows/docker-hub.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/docker-hub.yml b/.github/workflows/docker-hub.yml index 1a99c7ce36..b6db001e01 100644 --- a/.github/workflows/docker-hub.yml +++ b/.github/workflows/docker-hub.yml @@ -6,6 +6,7 @@ on: push: branches: - 'main' + - 'feat/comments-frontend' tags: - 'v*' pull_request: