diff --git a/CHANGELOG.md b/CHANGELOG.md index e8f217056..5ac2e0719 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ and this project adheres to - 🐛(minio) fix user permission error with Minio and Windows #1264 - 🐛(frontend) fix export when quote block and inline code #1319 - 🐛(frontend) fix base64 font #1324 +- 🐛(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 a9c8ee249..f98cbfc6c 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,19 +845,27 @@ 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 ancestors = ( - (current_document.get_ancestors() | self.queryset.filter(pk=pk)) + ( + current_document.get_ancestors() + | self.queryset.select_related(None).filter(pk=pk) + ) .filter(ancestors_deleted_at__isnull=True) .order_by("path") ) # Get the highest readable ancestor highest_readable = ( - ancestors.readable_per_se(request.user).only("depth", "path").first() + ancestors.select_related(None) + .readable_per_se(request.user) + .only("depth", "path") + .first() ) if highest_readable is None: raise ( @@ -881,7 +893,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 +1300,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 8822c8131..6ccca08d9 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -753,6 +753,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( @@ -775,11 +781,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 63fa8f090..5b8b1403e 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 in ["administrator", "owner"], "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 6874009c9..58687092e 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",