🐛(backend) prevent restoring a document when an ancestor is deleted

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
 

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):

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

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,
             }
         ],
     }

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():

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,
     }
 
 

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):

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')}
       </Box>
-      <Button
-        onClick={() =>
-          restoreDoc({
-            docId: doc.id,
-          })
-        }
-        color="error"
-        variant="tertiary"
-        size="nano"
-        icon={
-          <Icon
-            iconName="undo"
-            $withThemeInherited
-            $size="18px"
-            variant="symbols-outlined"
-          />
-        }
-      >
-        Restore
-      </Button>
+      {!doc.has_deleted_ancestor && (
+        <Button
+          onClick={() =>
+            restoreDoc({
+              docId: doc.id,
+            })
+          }
+          color="error"
+          variant="tertiary"
+          size="nano"
+          icon={
+            <Icon
+              iconName="undo"
+              $withThemeInherited
+              $size="18px"
+              variant="symbols-outlined"
+            />
+          }
+        >
+          Restore
+        </Button>
+      )}
     </Card>
   );
 };

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;

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(