From c66053465922310acaf0255e1e6cba80997d87e2 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Fri, 22 Aug 2025 14:57:02 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(backend)=20allow=20creator=20to=20?= =?UTF-8?q?delete=20subpages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An editor who created a subpages should be allowed to delete it. We change the abilities to be coherent between the creation and the deletion. Fixes #1193 --- CHANGELOG.md | 1 + src/backend/core/api/viewsets.py | 22 +++-- src/backend/core/models.py | 10 ++- .../documents/test_api_documents_retrieve.py | 2 +- .../core/tests/test_models_documents.py | 80 +++++++++++++++++++ 5 files changed, 107 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69791c9ee0..1fe95d9a1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to - 🐛(makefile) Windows compatibility fix for Docker volume mounting #1264 - 🐛(minio) fix user permission error with Minio and Windows #1264 +- 🐛(backend) allow editor to delete subpages #1296 ## [3.5.0] - 2025-07-31 diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index a9c8ee2493..e06a7dba97 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -360,7 +360,7 @@ class DocumentViewSet( permission_classes = [ permissions.DocumentPermission, ] - queryset = models.Document.objects.all() + queryset = models.Document.objects.select_related("creator").all() serializer_class = serializers.DocumentSerializer ai_translate_serializer_class = serializers.AITranslateSerializer children_serializer_class = serializers.ListDocumentSerializer @@ -787,7 +787,11 @@ def children(self, request, *args, **kwargs): ) # GET: List children - queryset = document.get_children().filter(ancestors_deleted_at__isnull=True) + queryset = ( + document.get_children() + .select_related("creator") + .filter(ancestors_deleted_at__isnull=True) + ) queryset = self.filter_queryset(queryset) filterset = DocumentFilter(request.GET, queryset=queryset) @@ -841,7 +845,9 @@ def tree(self, request, pk, *args, **kwargs): user = self.request.user try: - current_document = self.queryset.only("depth", "path").get(pk=pk) + current_document = ( + self.queryset.select_related(None).only("depth", "path").get(pk=pk) + ) except models.Document.DoesNotExist as excpt: raise drf.exceptions.NotFound() from excpt @@ -881,7 +887,12 @@ def tree(self, request, pk, *args, **kwargs): children = self.queryset.filter(children_clause, deleted_at__isnull=True) - queryset = ancestors.filter(depth__gte=highest_readable.depth) | children + queryset = ( + ancestors.select_related("creator").filter( + depth__gte=highest_readable.depth + ) + | children + ) queryset = queryset.order_by("path") queryset = queryset.annotate_user_roles(user) queryset = queryset.annotate_is_favorite(user) @@ -1283,7 +1294,8 @@ def media_auth(self, request, *args, **kwargs): ) attachments_documents = ( - self.queryset.filter(attachments__contains=[key]) + self.queryset.select_related(None) + .filter(attachments__contains=[key]) .only("path") .order_by("path") ) diff --git a/src/backend/core/models.py b/src/backend/core/models.py index a1182964da..50fc9a55d2 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -762,6 +762,12 @@ def get_abilities(self, user): can_update = ( is_owner_or_admin or role == RoleChoices.EDITOR ) and not is_deleted + can_create_children = can_update and user.is_authenticated + can_destroy = ( + is_owner + if self.is_root() + else (is_owner_or_admin or (user.is_authenticated and self.creator == user)) + ) ai_allow_reach_from = settings.AI_ALLOW_REACH_FROM ai_access = any( @@ -784,11 +790,11 @@ def get_abilities(self, user): "media_check": can_get, "can_edit": can_update, "children_list": can_get, - "children_create": can_update and user.is_authenticated, + "children_create": can_create_children, "collaboration_auth": can_get, "cors_proxy": can_get, "descendants": can_get, - "destroy": is_owner, + "destroy": can_destroy, "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_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index 63fa8f0902..afaa43b7ce 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -494,7 +494,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): "collaboration_auth": True, "descendants": True, "cors_proxy": True, - "destroy": access.role == "owner", + "destroy": access.role != "reader", "duplicate": True, "favorite": True, "invite_owner": access.role == "owner", diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 6874009c97..58687092e8 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -593,6 +593,86 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): } +@pytest.mark.parametrize( + "is_authenticated, is_creator,role,link_reach,link_role,can_destroy", + [ + (True, False, "owner", "restricted", "editor", True), + (True, True, "owner", "restricted", "editor", True), + (True, False, "owner", "restricted", "reader", True), + (True, True, "owner", "restricted", "reader", True), + (True, False, "owner", "authenticated", "editor", True), + (True, True, "owner", "authenticated", "editor", True), + (True, False, "owner", "authenticated", "reader", True), + (True, True, "owner", "authenticated", "reader", True), + (True, False, "owner", "public", "editor", True), + (True, True, "owner", "public", "editor", True), + (True, False, "owner", "public", "reader", True), + (True, True, "owner", "public", "reader", True), + (True, False, "administrator", "restricted", "editor", True), + (True, True, "administrator", "restricted", "editor", True), + (True, False, "administrator", "restricted", "reader", True), + (True, True, "administrator", "restricted", "reader", True), + (True, False, "administrator", "authenticated", "editor", True), + (True, True, "administrator", "authenticated", "editor", True), + (True, False, "administrator", "authenticated", "reader", True), + (True, True, "administrator", "authenticated", "reader", True), + (True, False, "administrator", "public", "editor", True), + (True, True, "administrator", "public", "editor", True), + (True, False, "administrator", "public", "reader", True), + (True, True, "administrator", "public", "reader", True), + (True, False, "editor", "restricted", "editor", False), + (True, True, "editor", "restricted", "editor", True), + (True, False, "editor", "restricted", "reader", False), + (True, True, "editor", "restricted", "reader", True), + (True, False, "editor", "authenticated", "editor", False), + (True, True, "editor", "authenticated", "editor", True), + (True, False, "editor", "authenticated", "reader", False), + (True, True, "editor", "authenticated", "reader", True), + (True, False, "editor", "public", "editor", False), + (True, True, "editor", "public", "editor", True), + (True, False, "editor", "public", "reader", False), + (True, True, "editor", "public", "reader", True), + (True, False, "reader", "restricted", "editor", False), + (True, False, "reader", "restricted", "reader", False), + (True, False, "reader", "authenticated", "editor", False), + (True, True, "reader", "authenticated", "editor", True), + (True, False, "reader", "authenticated", "reader", False), + (True, False, "reader", "public", "editor", False), + (True, True, "reader", "public", "editor", True), + (True, False, "reader", "public", "reader", False), + (False, False, None, "restricted", "editor", False), + (False, False, None, "restricted", "reader", False), + (False, False, None, "authenticated", "editor", False), + (False, False, None, "authenticated", "reader", False), + (False, False, None, "public", "editor", False), + (False, False, None, "public", "reader", False), + ], +) +# pylint: disable=too-many-arguments, too-many-positional-arguments +def test_models_documents_get_abilities_children_destroy( # noqa: PLR0913 + is_authenticated, + is_creator, + role, + link_reach, + link_role, + can_destroy, +): + """For a sub document, if a user can create children, he can destroy it.""" + user = factories.UserFactory() if is_authenticated else AnonymousUser() + parent = factories.DocumentFactory(link_reach=link_reach, link_role=link_role) + document = factories.DocumentFactory( + link_reach=link_reach, + link_role=link_role, + parent=parent, + creator=user if is_creator else None, + ) + if is_authenticated: + factories.UserDocumentAccessFactory(document=parent, user=user, role=role) + + abilities = document.get_abilities(user) + assert abilities["destroy"] is can_destroy + + @override_settings(AI_ALLOW_REACH_FROM="public") @pytest.mark.parametrize( "is_authenticated,reach",