From 017ffaa3c95f76b5d253eb3efb4a5df963205438 Mon Sep 17 00:00:00 2001 From: dakshesh14 <65905942+dakshesh14@users.noreply.github.com> Date: Sat, 28 Mar 2026 23:50:09 +0530 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B(backend)=20prevent=20restoring?= =?UTF-8?q?=20a=20document=20when=20an=20ancestor=20is=20deleted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a validation in restore() to block restoration if any ancestor is soft-deleted. This avoids a 500 error when attempting to restore a child whose parent is deleted. Signed-off-by: dakshesh14 <65905942+dakshesh14@users.noreply.github.com> --- src/backend/core/api/serializers.py | 2 + src/backend/core/models.py | 10 +++++ .../test_api_documents_favorite_list.py | 1 + .../documents/test_api_documents_restore.py | 17 ++++---- .../documents/test_api_documents_retrieve.py | 9 ++++ .../core/tests/test_models_documents.py | 20 +++++---- .../doc-header/components/AlertRestore.tsx | 42 ++++++++++--------- .../features/docs/doc-management/types.tsx | 1 + .../service-worker/plugins/ApiPlugin.ts | 1 + 9 files changed, 66 insertions(+), 37 deletions(-) diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index bdb05ce16a..ebdc671d40 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -210,6 +210,7 @@ class Meta: "updated_at", "user_role", "websocket", + "has_deleted_ancestor", ] read_only_fields = [ "id", @@ -231,6 +232,7 @@ class Meta: "path", "updated_at", "user_role", + "has_deleted_ancestor", ] def get_fields(self): diff --git a/src/backend/core/models.py b/src/backend/core/models.py index c6ed9ae16e..447cd16c58 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1233,6 +1233,11 @@ def computed_link_role(self): """Actual link role on the document.""" return self.computed_link_definition["link_role"] + @property + def has_deleted_ancestor(self): + """Check whether any ancestor document is deleted.""" + return self.get_ancestors().filter(deleted_at__isnull=False).exists() + def get_abilities(self, user): """ Compute and return abilities for a given user on the document. @@ -1432,6 +1437,11 @@ def soft_delete(self): @transaction.atomic def restore(self): """Cancelling a soft delete with checks.""" + if self.has_deleted_ancestor: + raise ValidationError( + "Cannot restore this document because one of its ancestors is deleted." + ) + # This should not happen if self._meta.model.objects.filter( pk=self.pk, deleted_at__isnull=True diff --git a/src/backend/core/tests/documents/test_api_documents_favorite_list.py b/src/backend/core/tests/documents/test_api_documents_favorite_list.py index df10f3109d..3f0c2decff 100644 --- a/src/backend/core/tests/documents/test_api_documents_favorite_list.py +++ b/src/backend/core/tests/documents/test_api_documents_favorite_list.py @@ -80,6 +80,7 @@ def test_api_document_favorite_list_authenticated_with_favorite(): "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": "reader", + "has_deleted_ancestor": False, } ], } diff --git a/src/backend/core/tests/documents/test_api_documents_restore.py b/src/backend/core/tests/documents/test_api_documents_restore.py index 5ae64aec12..f4c663a610 100644 --- a/src/backend/core/tests/documents/test_api_documents_restore.py +++ b/src/backend/core/tests/documents/test_api_documents_restore.py @@ -78,8 +78,7 @@ def test_api_documents_restore_authenticated_owner_success(): def test_api_documents_restore_authenticated_owner_ancestor_deleted(): """ - The restored document should still be marked as deleted if one of its - ancestors is soft deleted as well. + Restore should fail if one of the ancestors is soft deleted. """ user = factories.UserFactory() client = APIClient() @@ -100,14 +99,16 @@ def test_api_documents_restore_authenticated_owner_ancestor_deleted(): response = client.post(f"/api/v1.0/documents/{document.id!s}/restore/") - assert response.status_code == 200 - assert response.json() == {"detail": "Document has been successfully restored."} + # Restore should be rejected + assert response.status_code == 400 + assert response.json()[0] == ( + "Cannot restore this document because one of its ancestors is deleted." + ) + # Ensure state is unchanged document.refresh_from_db() - assert document.deleted_at is None - # document is still marked as deleted - assert document.ancestors_deleted_at == grand_parent_deleted_at - assert grand_parent_deleted_at > document_deleted_at + assert document.deleted_at == document_deleted_at + assert document.ancestors_deleted_at == document_deleted_at def test_api_documents_restore_authenticated_owner_expired(): 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..d090d323cb 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -86,6 +86,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": None, + "has_deleted_ancestor": False, } @@ -164,6 +165,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": None, + "has_deleted_ancestor": False, } @@ -275,6 +277,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": None, + "has_deleted_ancestor": False, } assert ( models.LinkTrace.objects.filter(document=document, user=user).exists() is True @@ -360,6 +363,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": None, + "has_deleted_ancestor": False, } @@ -475,6 +479,7 @@ def test_api_documents_retrieve_authenticated_related_direct(): "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": access.role, + "has_deleted_ancestor": False, } @@ -560,6 +565,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": access.role, + "has_deleted_ancestor": False, } @@ -717,6 +723,7 @@ def test_api_documents_retrieve_authenticated_related_team_members( "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": role, + "has_deleted_ancestor": False, } @@ -784,6 +791,7 @@ def test_api_documents_retrieve_authenticated_related_team_administrators( "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": role, + "has_deleted_ancestor": False, } @@ -851,6 +859,7 @@ def test_api_documents_retrieve_authenticated_related_team_owners( "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "user_role": role, + "has_deleted_ancestor": False, } diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 82edf82d9a..10d6d7e399 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -17,6 +17,7 @@ from django.utils import timezone import pytest +from rest_framework.exceptions import ValidationError as DRFValidationError from core import factories, models @@ -1431,8 +1432,8 @@ def test_models_documents_restore(django_assert_num_queries): assert document.ancestors_deleted_at == document.deleted_at -def test_models_documents_restore_complex(django_assert_num_queries): - """The restore method should restore a soft-deleted document and its ancestors.""" +def test_models_documents_restore_complex(): + """Restore should fail if a soft-deleted document has a deleted ancestor.""" grand_parent = factories.DocumentFactory() parent = factories.DocumentFactory(parent=grand_parent) document = factories.DocumentFactory(parent=parent) @@ -1466,18 +1467,19 @@ def test_models_documents_restore_complex(django_assert_num_queries): assert child1.ancestors_deleted_at == document.deleted_at assert child2.ancestors_deleted_at == document.deleted_at - # Restore the item - with django_assert_num_queries(14): + # Restore should fail because ancestor is deleted + with pytest.raises(DRFValidationError): document.restore() document.refresh_from_db() child1.refresh_from_db() child2.refresh_from_db() grand_parent.refresh_from_db() - assert document.deleted_at is None - assert document.ancestors_deleted_at == grand_parent.deleted_at - # child 1 and child 2 should now have the same ancestors_deleted_at as the grand parent - assert child1.ancestors_deleted_at == grand_parent.deleted_at - assert child2.ancestors_deleted_at == grand_parent.deleted_at + + # State remains unchanged + assert document.deleted_at is not None + assert document.ancestors_deleted_at == document.deleted_at + assert child1.ancestors_deleted_at == document.deleted_at + assert child2.ancestors_deleted_at == document.deleted_at def test_models_documents_restore_complex_bis(django_assert_num_queries): diff --git a/src/frontend/apps/impress/src/features/docs/doc-header/components/AlertRestore.tsx b/src/frontend/apps/impress/src/features/docs/doc-header/components/AlertRestore.tsx index 4cfcf010a8..3e7a93fa1a 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-header/components/AlertRestore.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-header/components/AlertRestore.tsx @@ -73,26 +73,28 @@ export const AlertRestore = ({ doc }: { doc: Doc }) => { /> {t('Document deleted')} - + {!doc.has_deleted_ancestor && ( + + )} ); }; 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..83cee75185 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 @@ -71,6 +71,7 @@ export interface Doc { numchild: number; updated_at: string; user_role: Role; + has_deleted_ancestor: boolean; abilities: { accesses_manage: boolean; accesses_view: 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 ecb64dd1f4..cb16ca5929 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 @@ -218,6 +218,7 @@ export class ApiPlugin implements WorkboxPlugin { computed_link_role: LinkRole.READER, ancestors_link_reach: LinkReach.RESTRICTED, ancestors_link_role: undefined, + has_deleted_ancestor: false, }; await DocsDB.cacheResponse( From 2f6b0d490608458b7f1103113e62e2d9bd6460cc Mon Sep 17 00:00:00 2001 From: dakshesh14 <65905942+dakshesh14@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:05:55 +0530 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B(changelog)=20added=20pr=20in?= =?UTF-8?q?=20changelog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added PR in changelog Signed-off-by: dakshesh14 <65905942+dakshesh14@users.noreply.github.com> --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b579a907c..d251304339 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to - ♿️(frontend) add aria-hidden to decorative icons in dropdown menu #2093 - 🐛(backend) move lock table closer to the insert operation targeted - ♿️(frontend) replace ARIA grid pattern with list in docs grid #2131 +- 🐛(backend): prevent restoring a document when an ancestor is deleted #2148 ### Fixed