diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0c6a1bbcac..186439355d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,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
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(