diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 32e52f710e..51d5e1db86 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -906,6 +906,65 @@ def create_for_owner(self, request): {"id": str(document.id)}, status=status.HTTP_201_CREATED ) + @drf.decorators.action(detail=True, methods=["post"]) + @transaction.atomic + def detach(self, request, *args, **kwargs): + """ + Detach a subdocument from it's parent + + The user must be an administrator or owner of root document or an + editor (on target document) and creator of the document creator + """ + user = request.user + document = self.get_object() + + if document.is_root(): + return drf.response.Response( + # mettre le message dans position est valid ? + {"message": "You cannot detach a root document"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + target_document = document.get_root() + if not target_document: + return drf.response.Response( + {"message": "Parent document does not exist."}, + status=status.HTTP_400_BAD_REQUEST, + ) + + if not document.get_abilities(user).get("detach"): + return drf.response.Response( + { + "message": ( + "You do not have permission to move documents " + "as a sibling of this target document." + ) + }, + status=status.HTTP_400_BAD_REQUEST, + ) + + owner_accesses = document.get_root().accesses.filter( + role=models.RoleChoices.OWNER + ) + + document.move(target_document, pos=enums.MoveNodePositionChoices.LAST_SIBLING) + + if ( + owner_accesses + and not document.accesses.filter(role=models.RoleChoices.OWNER).exists() + ): + for owner_access in owner_accesses: + models.DocumentAccess.objects.update_or_create( + document=document, + user=owner_access.user, + team=owner_access.team, + defaults={"role": models.RoleChoices.OWNER}, + ) + + return drf.response.Response( + {"message": "Document detached successfully."}, status=status.HTTP_200_OK + ) + @drf.decorators.action(detail=True, methods=["post"]) @transaction.atomic def move(self, request, *args, **kwargs): @@ -941,15 +1000,22 @@ def move(self, request, *args, **kwargs): enums.MoveNodePositionChoices.FIRST_CHILD, enums.MoveNodePositionChoices.LAST_CHILD, ]: - if not target_document.get_abilities(user).get("move"): + if not target_document.get_abilities(user).get("accesses_update"): message = ( "You do not have permission to move documents " "as a child to this target document." ) elif target_document.is_root(): - owner_accesses = document.get_root().accesses.filter( - role=models.RoleChoices.OWNER - ) + if not document.is_root(): + message = ( + "You cannot move a document relative to this target document " + "unless as its child." + ) + elif not target_document.get_abilities(user).get("move"): + message = ( + "You do not have permission to move documents " + "as a sibling of this target document." + ) elif not target_document.get_parent().get_abilities(user).get("move"): message = ( "You do not have permission to move documents " diff --git a/src/backend/core/models.py b/src/backend/core/models.py index c6ed9ae16e..c19cb178f5 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1242,8 +1242,11 @@ def get_abilities(self, user): # Characteristics that are based only on specific access is_owner = role == RoleChoices.OWNER + is_creator = self.creator == user + is_deleted = self.ancestors_deleted_at is_owner_or_admin = (is_owner or role == RoleChoices.ADMIN) and not is_deleted + is_editor_creator = (role == RoleChoices.EDITOR) and is_creator # Compute access roles before adding link roles because we don't # want anonymous users to access versions (we wouldn't know from @@ -1280,7 +1283,7 @@ def get_abilities(self, user): can_destroy = ( is_owner if self.is_root() - else (is_owner_or_admin or (user.is_authenticated and self.creator == user)) + else (is_owner_or_admin or (user.is_authenticated and is_creator)) ) and not is_deleted ai_allow_reach_from = settings.AI_ALLOW_REACH_FROM @@ -1297,6 +1300,7 @@ def get_abilities(self, user): return { "accesses_manage": is_owner_or_admin, + "accesses_update": can_update_from_access, "accesses_view": has_access_role, "ai_proxy": ai_access, "ai_transform": ai_access, @@ -1312,6 +1316,7 @@ def get_abilities(self, user): "cors_proxy": can_get, "descendants": can_get, "destroy": can_destroy, + "detach": (is_owner_or_admin or is_editor_creator) and not is_deleted, "duplicate": can_get and user.is_authenticated, "favorite": can_get and user.is_authenticated, "link_configuration": is_owner_or_admin, diff --git a/src/backend/core/tests/documents/test_api_documents_detach.py b/src/backend/core/tests/documents/test_api_documents_detach.py new file mode 100644 index 0000000000..8d1f41d61f --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_detach.py @@ -0,0 +1,207 @@ +""" +Test moving documents within the document tree via an detail action API endpoint. +""" + +import pytest +from rest_framework.test import APIClient + +from core import factories, models + +pytestmark = pytest.mark.django_db + + +def test_api_documents_detach_anonymous_user(): + """Anonymous users should not be able to detach documents.""" + target = factories.DocumentFactory() + document = factories.DocumentFactory(parent=target) + + response = APIClient().post(f"/api/v1.0/documents/{document.id!s}/detach/") + + assert response.status_code == 401 + assert response.json() == { + "detail": "Authentication credentials were not provided." + } + + +def test_api_documents_move_document_is_root(): + """ + Detaching a root document should not be allowed + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(users=[(user, "owner")]) + + response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/") + + assert response.status_code == 400 + assert response.json() == {"message": "You cannot detach a root document"} + + +@pytest.mark.parametrize("role", [None, "reader", "commenter", "editor"]) +def test_api_documents_detach_authenticated_document_no_permission(role): + """ + Authenticated users should not be able to detach documents with insufficient + permissions on the root document. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + target = factories.DocumentFactory() + if role: + factories.UserDocumentAccessFactory(document=target, user=user, role=role) + document = factories.DocumentFactory(parent=target) + + response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/") + + assert response.status_code == 403 + assert response.json() == { + "detail": "You do not have permission to perform this action." + } + + +@pytest.mark.parametrize("user_role", models.RoleChoices.values) +@pytest.mark.parametrize("is_creator", [True, False]) +def test_api_documents_move_authenticated_detach( + is_creator, + user_role, +): + """ + Authenticated users with permissions should be allowed to detach documents + """ + + power_roles = ["administrator", "owner"] + + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + target_owner = factories.UserFactory() + target = factories.DocumentFactory( + users=[(target_owner, "owner"), (user, user_role)] + ) + + document_args = {"parent": target} + if is_creator: + document_args["creator"] = user + + document = factories.DocumentFactory(**document_args) + child = factories.DocumentFactory(parent=document) + + response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/") + document.refresh_from_db() + + if user_role in power_roles or ( + ( + user_role == models.RoleChoices.EDITOR + or target.get_role(user) == models.RoleChoices.EDITOR + ) + and is_creator + ): + assert response.status_code == 200 + + assert target in list(target.get_siblings()) + assert document in list(target.get_siblings()) + assert list(document.get_children()) == [child] + else: + assert response.status_code == 403 + + +def test_api_documents_move_authenticated_no_owner_user_and_team(): + """ + Detaching a document with no owner to the root of the tree should automatically declare + the owner of the previous root of the document as owner of the document itself. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + parent_owner = factories.UserFactory() + parent = factories.DocumentFactory( + users=[(parent_owner, "owner")], teams=[("lasuite", "owner")] + ) + # A document with no owner + document = factories.DocumentFactory(parent=parent, users=[(user, "administrator")]) + child = factories.DocumentFactory(parent=document) + target = factories.DocumentFactory() + + response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/") + + assert response.status_code == 200 + assert response.json() == {"message": "Document detached successfully."} + assert document in target.get_siblings() + assert parent in target.get_siblings() + assert target in target.get_siblings() + + document.refresh_from_db() + assert list(document.get_children()) == [child] + assert document.accesses.count() == 3 + assert document.accesses.get(user__isnull=False, role="owner").user == parent_owner + assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite" + assert document.accesses.get(role="administrator").user == user + + +def test_api_documents_move_authenticated_no_owner_same_user(): + """ + Detaching a document should not fail if the user moving a document with no owner was + at the same time owner of the previous root and has a role on the document being moved. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory( + users=[(user, "owner")], teams=[("lasuite", "owner")] + ) + # A document with no owner + document = factories.DocumentFactory(parent=parent, users=[(user, "reader")]) + child = factories.DocumentFactory(parent=document) + target = factories.DocumentFactory() + + response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/") + + assert response.status_code == 200 + assert response.json() == {"message": "Document detached successfully."} + assert document in target.get_siblings() + assert parent in target.get_siblings() + assert target in target.get_siblings() + + document.refresh_from_db() + assert list(document.get_children()) == [child] + assert document.accesses.count() == 2 + assert document.accesses.get(user__isnull=False, role="owner").user == user + assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite" + + +def test_api_documents_move_authenticated_no_owner_same_team(): + """ + Detaching a document should not fail if the team that is owner of the document root was + already declared on the document with a different role. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory(teams=[("lasuite", "owner")]) + # A document with no owner but same team + document = factories.DocumentFactory( + parent=parent, users=[(user, "administrator")], teams=[("lasuite", "reader")] + ) + child = factories.DocumentFactory(parent=document) + target = factories.DocumentFactory() + + response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/") + + assert response.status_code == 200 + assert response.json() == {"message": "Document detached successfully."} + assert document in target.get_siblings() + assert parent in target.get_siblings() + assert target in target.get_siblings() + + document.refresh_from_db() + assert list(document.get_children()) == [child] + assert document.accesses.count() == 2 + assert document.accesses.get(user__isnull=False, role="administrator").user == user + assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite" diff --git a/src/backend/core/tests/documents/test_api_documents_move.py b/src/backend/core/tests/documents/test_api_documents_move.py index ad4f68d415..04f88ccc8e 100644 --- a/src/backend/core/tests/documents/test_api_documents_move.py +++ b/src/backend/core/tests/documents/test_api_documents_move.py @@ -133,6 +133,7 @@ def test_api_documents_move_authenticated_target_roles_mocked( client.force_login(user) power_roles = ["administrator", "owner"] + update_roles = [*power_roles, "editor"] document = factories.DocumentFactory(users=[(user, random.choice(power_roles))]) children = factories.DocumentFactory.create_batch(3, parent=document) @@ -153,7 +154,7 @@ def test_api_documents_move_authenticated_target_roles_mocked( if ( position in ["first-child", "last-child"] - and (target_role in power_roles or target_parent_role in power_roles) + and (target_role in update_roles or target_parent_role in update_roles) ) or ( position in ["first-sibling", "last-sibling", "left", "right"] and target_parent_role in power_roles @@ -208,105 +209,64 @@ def test_api_documents_move_authenticated_target_roles_mocked( assert document.is_root() is True -def test_api_documents_move_authenticated_no_owner_user_and_team(): - """ - Moving a document with no owner to the root of the tree should automatically declare - the owner of the previous root of the document as owner of the document itself. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - parent_owner = factories.UserFactory() - parent = factories.DocumentFactory( - users=[(parent_owner, "owner")], teams=[("lasuite", "owner")] - ) - # A document with no owner - document = factories.DocumentFactory(parent=parent, users=[(user, "administrator")]) - child = factories.DocumentFactory(parent=document) - target = factories.DocumentFactory() - - response = client.post( - f"/api/v1.0/documents/{document.id!s}/move/", - data={"target_document_id": str(target.id), "position": "first-sibling"}, - ) - - assert response.status_code == 200 - assert response.json() == {"message": "Document moved successfully."} - assert list(target.get_siblings()) == [document, parent, target] - - document.refresh_from_db() - assert list(document.get_children()) == [child] - assert document.accesses.count() == 3 - assert document.accesses.get(user__isnull=False, role="owner").user == parent_owner - assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite" - assert document.accesses.get(role="administrator").user == user - +@pytest.mark.parametrize("position", enums.MoveNodePositionChoices.values) +def test_api_documents_move_root_target_root(position): + """Moving a root document relatively to a root target document + should be allowed.""" -def test_api_documents_move_authenticated_no_owner_same_user(): - """ - Moving a document should not fail if the user moving a document with no owner was - at the same time owner of the previous root and has a role on the document being moved. - """ user = factories.UserFactory() client = APIClient() client.force_login(user) - parent = factories.DocumentFactory( - users=[(user, "owner")], teams=[("lasuite", "owner")] - ) - # A document with no owner - document = factories.DocumentFactory(parent=parent, users=[(user, "reader")]) - child = factories.DocumentFactory(parent=document) - target = factories.DocumentFactory() + target = factories.DocumentFactory(users=[(user, "owner")]) + document = factories.DocumentFactory(users=[(user, "owner")]) response = client.post( f"/api/v1.0/documents/{document.id!s}/move/", - data={"target_document_id": str(target.id), "position": "first-sibling"}, + data={ + "target_document_id": str(target.id), + "position": position, + }, ) assert response.status_code == 200 assert response.json() == {"message": "Document moved successfully."} - assert list(target.get_siblings()) == [document, parent, target] - document.refresh_from_db() - assert list(document.get_children()) == [child] - assert document.accesses.count() == 2 - assert document.accesses.get(user__isnull=False, role="owner").user == user - assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite" +@pytest.mark.parametrize("position", enums.MoveNodePositionChoices.values) +def test_api_documents_move_root_target_not_child(position): + """Moving a document relatively to a root document should be + allowed only with child positions.""" -def test_api_documents_move_authenticated_no_owner_same_team(): - """ - Moving a document should not fail if the team that is owner of the document root was - already declared on the document with a different role. - """ user = factories.UserFactory() client = APIClient() client.force_login(user) - parent = factories.DocumentFactory(teams=[("lasuite", "owner")]) - # A document with no owner but same team - document = factories.DocumentFactory( - parent=parent, users=[(user, "administrator")], teams=[("lasuite", "reader")] - ) - child = factories.DocumentFactory(parent=document) - target = factories.DocumentFactory() + root_target = factories.UserDocumentAccessFactory(user=user, role="owner").document + root_target_sibling = factories.UserDocumentAccessFactory( + user=user, role="owner" + ).document + document = factories.DocumentFactory(parent=root_target, users=[(user, "owner")]) response = client.post( f"/api/v1.0/documents/{document.id!s}/move/", - data={"target_document_id": str(target.id), "position": "first-sibling"}, + data={ + "target_document_id": str(root_target_sibling.id), + "position": position, + }, ) - assert response.status_code == 200 - assert response.json() == {"message": "Document moved successfully."} - assert list(target.get_siblings()) == [document, parent, target] - - document.refresh_from_db() - assert list(document.get_children()) == [child] - assert document.accesses.count() == 2 - assert document.accesses.get(user__isnull=False, role="administrator").user == user - assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite" + if position in [ + enums.MoveNodePositionChoices.FIRST_CHILD, + enums.MoveNodePositionChoices.LAST_CHILD, + ]: + assert response.status_code == 200 + else: + assert response.status_code == 400 + assert ( + "You cannot move a document relative to this target document" + in response.json()["target_document_id"] + ) def test_api_documents_move_authenticated_deleted_document(): 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 f4a4876c5b..2e51182d93 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -28,6 +28,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "id": str(document.id), "abilities": { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": False, "ai_transform": False, @@ -42,6 +43,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "content": True, "descendants": True, "destroy": False, + "detach": False, "duplicate": False, # Anonymous user can't favorite a document even with read access "favorite": False, @@ -108,6 +110,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "id": str(document.id), "abilities": { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": False, "ai_transform": False, @@ -122,6 +125,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "cors_proxy": True, "content": True, "destroy": False, + "detach": False, "duplicate": False, # Anonymous user can't favorite a document even with read access "favorite": False, @@ -218,6 +222,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "id": str(document.id), "abilities": { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": document.link_role == "editor", "ai_transform": document.link_role == "editor", @@ -228,10 +233,11 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "children_list": True, "collaboration_auth": True, "comment": document.link_role in ["commenter", "editor"], - "descendants": True, "cors_proxy": True, "content": True, + "descendants": True, "destroy": False, + "detach": False, "duplicate": True, "favorite": True, "invite_owner": False, @@ -305,6 +311,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "id": str(document.id), "abilities": { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": grand_parent.link_role == "editor", "ai_transform": grand_parent.link_role == "editor", @@ -320,6 +327,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "content": True, "destroy": False, "duplicate": True, + "detach": False, "favorite": True, "invite_owner": False, "link_configuration": False, @@ -505,6 +513,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): "id": str(document.id), "abilities": { "accesses_manage": access.role in ["administrator", "owner"], + "accesses_update": access.role in ["administrator", "owner", "editor"], "accesses_view": True, "ai_proxy": access.role not in ["reader", "commenter"], "ai_transform": access.role not in ["reader", "commenter"], @@ -519,6 +528,8 @@ def test_api_documents_retrieve_authenticated_related_parent(): "cors_proxy": True, "content": True, "destroy": access.role in ["administrator", "owner"], + "detach": access.role in ["administrator", "owner"] + or (access.role == "editor" and document.creator == user), "duplicate": True, "favorite": True, "invite_owner": access.role == "owner", 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 a8ee836890..c901d72c44 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -71,6 +71,7 @@ def test_api_documents_trashbin_format(): "id": str(document.id), "abilities": { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": False, "ai_transform": False, @@ -85,6 +86,7 @@ def test_api_documents_trashbin_format(): "comment": False, "content": False, "destroy": False, + "detach": False, "duplicate": False, "favorite": False, "invite_owner": False, diff --git a/src/backend/core/tests/external_api/test_external_api_documents.py b/src/backend/core/tests/external_api/test_external_api_documents.py index 036a87c811..2356adede1 100644 --- a/src/backend/core/tests/external_api/test_external_api_documents.py +++ b/src/backend/core/tests/external_api/test_external_api_documents.py @@ -721,18 +721,16 @@ def test_external_api_documents_move_can_be_allowed( client = APIClient() client.credentials(HTTP_AUTHORIZATION=f"Bearer {user_token}") - parent = factories.DocumentFactory( + document = factories.DocumentFactory( users=[(user_specific_sub, "owner")], teams=[("lasuite", "owner")] ) - # A document with no owner - document = factories.DocumentFactory( - parent=parent, users=[(user_specific_sub, "reader")] + target = factories.DocumentFactory( + users=[(user_specific_sub, "owner")], teams=[("lasuite", "owner")] ) - target = factories.DocumentFactory() response = client.post( f"/external_api/v1.0/documents/{document.id!s}/move/", - data={"target_document_id": str(target.id), "position": "first-sibling"}, + data={"target_document_id": str(target.id), "position": "first-child"}, ) assert response.status_code == 200 diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 82edf82d9a..668db57de5 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -154,6 +154,7 @@ def test_models_documents_get_abilities_forbidden( user = factories.UserFactory() if is_authenticated else AnonymousUser() expected_abilities = { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": False, "ai_transform": False, @@ -167,6 +168,7 @@ def test_models_documents_get_abilities_forbidden( "cors_proxy": False, "content": False, "destroy": False, + "detach": False, "duplicate": False, "favorite": False, "comment": False, @@ -221,6 +223,7 @@ def test_models_documents_get_abilities_reader( user = factories.UserFactory() if is_authenticated else AnonymousUser() expected_abilities = { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": False, "ai_transform": False, @@ -235,6 +238,7 @@ def test_models_documents_get_abilities_reader( "cors_proxy": True, "content": True, "destroy": False, + "detach": False, "duplicate": is_authenticated, "favorite": is_authenticated, "invite_owner": False, @@ -293,6 +297,7 @@ def test_models_documents_get_abilities_commenter( user = factories.UserFactory() if is_authenticated else AnonymousUser() expected_abilities = { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": False, "ai_transform": False, @@ -307,6 +312,7 @@ def test_models_documents_get_abilities_commenter( "descendants": True, "cors_proxy": True, "destroy": False, + "detach": False, "duplicate": is_authenticated, "favorite": is_authenticated, "invite_owner": False, @@ -362,6 +368,7 @@ def test_models_documents_get_abilities_editor( user = factories.UserFactory() if is_authenticated else AnonymousUser() expected_abilities = { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": is_authenticated, "ai_transform": is_authenticated, @@ -376,6 +383,7 @@ def test_models_documents_get_abilities_editor( "cors_proxy": True, "content": True, "destroy": False, + "detach": False, "duplicate": is_authenticated, "favorite": is_authenticated, "invite_owner": False, @@ -420,6 +428,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): document = factories.DocumentFactory(users=[(user, "owner")]) expected_abilities = { "accesses_manage": True, + "accesses_update": True, "accesses_view": True, "ai_proxy": True, "ai_transform": True, @@ -434,6 +443,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "cors_proxy": True, "content": True, "destroy": True, + "detach": True, "duplicate": True, "favorite": True, "invite_owner": True, @@ -464,6 +474,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): document.refresh_from_db() assert document.get_abilities(user) == { "accesses_manage": False, + "accesses_update": False, "accesses_view": False, "ai_proxy": False, "ai_transform": False, @@ -478,6 +489,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "cors_proxy": False, "content": False, "destroy": False, + "detach": False, "duplicate": False, "favorite": False, "invite_owner": False, @@ -512,6 +524,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) document = factories.DocumentFactory(users=[(user, "administrator")]) expected_abilities = { "accesses_manage": True, + "accesses_update": True, "accesses_view": True, "ai_proxy": True, "ai_transform": True, @@ -526,6 +539,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "cors_proxy": True, "content": True, "destroy": False, + "detach": True, "duplicate": True, "favorite": True, "invite_owner": False, @@ -570,6 +584,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): document = factories.DocumentFactory(users=[(user, "editor")]) expected_abilities = { "accesses_manage": False, + "accesses_update": True, "accesses_view": True, "ai_proxy": True, "ai_transform": True, @@ -584,6 +599,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "cors_proxy": True, "content": True, "destroy": False, + "detach": False, "duplicate": True, "favorite": True, "invite_owner": False, @@ -633,6 +649,7 @@ def test_models_documents_get_abilities_reader_user( expected_abilities = { "accesses_manage": False, + "accesses_update": 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 @@ -650,6 +667,7 @@ def test_models_documents_get_abilities_reader_user( "cors_proxy": True, "content": True, "destroy": False, + "detach": False, "duplicate": True, "favorite": True, "invite_owner": False, @@ -701,6 +719,7 @@ def test_models_documents_get_abilities_commenter_user( expected_abilities = { "accesses_manage": False, + "accesses_update": 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 @@ -717,6 +736,7 @@ def test_models_documents_get_abilities_commenter_user( "descendants": True, "cors_proxy": True, "destroy": False, + "detach": False, "duplicate": True, "favorite": True, "invite_owner": False, @@ -766,6 +786,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): assert abilities == { "accesses_manage": False, + "accesses_update": False, "accesses_view": True, "ai_proxy": False, "ai_transform": False, @@ -780,6 +801,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "cors_proxy": True, "content": True, "destroy": False, + "detach": False, "duplicate": True, "favorite": True, "invite_owner": False, diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-grid-move.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-grid-move.spec.ts index 136488a5d3..8962163338 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-grid-move.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-grid-move.spec.ts @@ -121,7 +121,7 @@ test.describe('Doc grid move', () => { await expect(dragOverlay).toBeVisible(); await expect(dragOverlay).toHaveText( - 'You must be at least the administrator of the target document', + 'You must be at least an editor of the target document', ); await page.mouse.up(); @@ -171,7 +171,7 @@ test.describe('Doc grid move', () => { await expect(dragOverlay).toBeVisible(); await expect(dragOverlay).toHaveText( - 'You must be the owner to move the document', + 'You must be at least an administrator of the document', ); await page.mouse.up(); @@ -434,6 +434,7 @@ const data = [ id: 'can-drop-and-drag', abilities: { accesses_manage: true, + accesses_update: true, accesses_view: true, ai_transform: true, ai_translate: true, @@ -483,6 +484,7 @@ const data = [ title: 'Can only drop', abilities: { accesses_manage: true, + accesses_update: true, accesses_view: true, ai_transform: true, ai_translate: true, @@ -531,6 +533,7 @@ const data = [ id: 'no-drop-and-no-drag', abilities: { accesses_manage: false, + accesses_update: false, accesses_view: true, ai_transform: false, ai_translate: false, diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-tree.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-tree.spec.ts index 3fe255c5b3..3e527906ea 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-tree.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-tree.spec.ts @@ -36,6 +36,7 @@ test.describe('Doc Tree', () => { id: `doc-child-${pageId}-${i}`, abilities: { accesses_manage: true, + accesses_update: true, accesses_view: true, ai_proxy: true, ai_transform: true, @@ -51,6 +52,7 @@ test.describe('Doc Tree', () => { cors_proxy: true, descendants: true, destroy: true, + detach: true, duplicate: true, favorite: true, link_configuration: true, @@ -267,7 +269,10 @@ test.describe('Doc Tree', () => { await expect(page.getByText(docChild)).toBeVisible(); }); - test('Only owner can detaches a document', async ({ page, browserName }) => { + test('At least administrator to detach a document', async ({ + page, + browserName, + }) => { const [docParent] = await createDoc( page, 'doc-tree-detach', @@ -290,7 +295,7 @@ test.describe('Doc Tree', () => { ); const currentUserRole = currentUser.getByTestId('doc-role-dropdown'); await currentUserRole.click(); - await page.getByRole('menuitemradio', { name: 'Administrator' }).click(); + await page.getByRole('menuitemradio', { name: 'Editor' }).click(); await list.click(); await page.getByRole('button', { name: 'Ok' }).click(); 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 fcf93edd4e..d374619a0b 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 @@ -73,6 +73,7 @@ export interface Doc { user_role: Role; abilities: { accesses_manage: boolean; + accesses_update: boolean; accesses_view: boolean; ai_proxy: boolean; ai_transform: boolean; @@ -84,6 +85,7 @@ export interface Doc { comment: boolean; destroy: boolean; duplicate: boolean; + detach: boolean; favorite: boolean; invite_owner: boolean; link_configuration: boolean; diff --git a/src/frontend/apps/impress/src/features/docs/doc-tree/api/useDetach.tsx b/src/frontend/apps/impress/src/features/docs/doc-tree/api/useDetach.tsx index 8e261c4926..b50c79d7d1 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-tree/api/useDetach.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-tree/api/useDetach.tsx @@ -22,12 +22,8 @@ export const detachDoc = async ({ documentId, rootId, }: DetachDocParam): Promise => { - const response = await fetchAPI(`documents/${documentId}/move/`, { + const response = await fetchAPI(`documents/${documentId}/detach/`, { method: 'POST', - body: JSON.stringify({ - target_document_id: rootId, - position: POSITION_MOVE.LAST_SIBLING, - }), }); if (!response.ok) { diff --git a/src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx b/src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx index 797c03761b..e03e2b88b8 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx @@ -122,7 +122,7 @@ export const DocTreeItemActions = ({ : []), { label: t('Move to my docs'), - isDisabled: doc.user_role !== Role.OWNER, + isDisabled: !doc.abilities.detach, icon: ( { if (!canDrag) { - return t('You must be the owner to move the document'); + return t('You must be at least an administrator of the document'); } if (!canDrop) { - return t('You must be at least the administrator of the target document'); + return t('You must be at least an editor of the target document'); } return selectedDoc?.title || untitledDocument; @@ -263,7 +263,7 @@ export const DraggableDocGridItem = ({ canDrag, updateCanDrop, }: DraggableDocGridItemProps) => { - const canDrop = doc.abilities.move; + const canDrop = doc.abilities.accesses_update; return ( void) { const [selectedDoc, setSelectedDoc] = useState(); const [canDrop, setCanDrop] = useState(); - const canDrag = selectedDoc?.user_role === Role.OWNER; + const canDrag = selectedDoc?.abilities.move; const mouseSensor = useSensor(MouseSensor, { activationConstraint }); const touchSensor = useSensor(TouchSensor, { activationConstraint }); 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 ecb64dd1f4..7589c28ab9 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 @@ -181,6 +181,7 @@ export class ApiPlugin implements WorkboxPlugin { updated_at: new Date().toISOString(), abilities: { accesses_manage: true, + accesses_update: true, accesses_view: true, ai_proxy: true, ai_transform: true, @@ -191,6 +192,7 @@ export class ApiPlugin implements WorkboxPlugin { collaboration_auth: true, comment: true, destroy: true, + detach: true, duplicate: true, favorite: true, invite_owner: true, diff --git a/src/frontend/apps/impress/src/i18n/translations.json b/src/frontend/apps/impress/src/i18n/translations.json index 9a36f7cb01..ff82925f45 100644 --- a/src/frontend/apps/impress/src/i18n/translations.json +++ b/src/frontend/apps/impress/src/i18n/translations.json @@ -547,6 +547,8 @@ "You have already requested access to this document.": "Sie haben bereits um Zugang zu diesem Dokument gebeten.", "You must be at least the administrator of the target document": "Sie müssen beim Zieldokument mindestens die Rolle \"Administrator\" haben", "You must be the owner to move the document": "Sie müssen Besitzer des Dokuments sein, um es zu verschieben", + "You must be at least an administrator of the document": "Sie müssen beim dokument mindestens die Rolle \"Administrator\" haben", + "You must be at least an editor of the target document": "Sie müssen beim Zieldokument mindestens die Rolle \"Editor\" haben", "You're currently viewing a sub-document. To gain access, please request permission from the main document.": "Sie sehen derzeit ein untergeordnetes Dokument. Um Zugriff zu erhalten, fordern Sie bitte Berechtigung am Wurzeldokument an.", "Your access request for this document is pending.": "Ihre Zugriffsanfrage für dieses Dokument steht noch aus.", "Your {{format}} was downloaded succesfully": "Ihr {{format}} wurde erfolgreich heruntergeladen", @@ -1519,6 +1521,8 @@ "You have already requested access to this document.": "Vous avez déjà demandé l'accès à ce document.", "You must be at least the administrator of the target document": "Vous devez être au moins l'administrateur du document cible", "You must be the owner to move the document": "Vous devez être le propriétaire pour déplacer le document", + "You must be at least an administrator of the document": "Vous devez être au moins un administrateur du document", + "You must be at least an editor of the target document": "Vous devez être au moins éditeur du document cible", "You're currently viewing a sub-document. To gain access, please request permission from the main document.": "Vous visualisez actuellement un sous-document. Pour obtenir un accès, veuillez demander la permission du document principal.", "Your access request for this document is pending.": "Votre demande d'accès à ce document est en attente.", "Your {{format}} was downloaded succesfully": "Votre {{format}} a été téléchargé avec succès", @@ -1984,6 +1988,8 @@ "You have already requested access to this document.": "U hebt al toegang gevraagd tot dit document.", "You must be at least the administrator of the target document": "U moet tenminste beheerder zijn van het doeldocument", "You must be the owner to move the document": "U moet de eigenaar zijn om het document te verplaatsen", + "You must be at least an administrator of the document": "U moet tenminste beheerder zijn van het document", + "You must be at least an editor of the target document": "U moet tenminste redacteur zijn van het doeldocument", "You're currently viewing a sub-document. To gain access, please request permission from the main document.": "U bekijkt momenteel een subdocument. Om toegang te krijgen, vraagt u toestemming voor het hoofddocument.", "Your access request for this document is pending.": "Uw toegangsverzoek voor dit document is in behandeling.", "Your {{format}} was downloaded succesfully": "Jouw {{format}} is succesvol gedownload",