diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 907f8551..f14dcd1f 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1433,14 +1433,21 @@ class Item(TreeModel, BaseModel): "Cannot delete this item because one or more ancestors are already deleted." ) - # No shortcut may keep pointing into the trash - if self.is_restricted: - self._meta.model.objects.filter(target=self).delete() - self.ancestors_deleted_at = self.deleted_at = timezone.now() self.save(update_fields=["deleted_at", "ancestors_deleted_at"]) + # The shortcut of a restricted folder lives in another subtree: trash + # it along so that restoring the folder can bring it back + if self.is_restricted: + self._meta.model.objects.filter( + target=self, + ancestors_deleted_at__isnull=True, + ).update( + deleted_at=self.deleted_at, + ancestors_deleted_at=self.deleted_at, + ) + # Mark all descendants as soft deleted if self.type == ItemTypeChoices.FOLDER: self.descendants().filter(ancestors_deleted_at__isnull=True).update( @@ -1488,6 +1495,12 @@ class Item(TreeModel, BaseModel): # Mark all descendants as hard deleted self.descendants().update(hard_deleted_at=self.hard_deleted_at) + # The shortcut of a restricted folder must not survive its target + if self.is_restricted: + self._meta.model.objects.filter(target=self).update( + hard_deleted_at=self.hard_deleted_at + ) + transaction.on_commit(lambda: invalidate_storage_used_cache(creator_ids)) @transaction.atomic @@ -1543,6 +1556,17 @@ class Item(TreeModel, BaseModel): | models.Q(ancestors_deleted_at__lt=current_deleted_at) ).update(ancestors_deleted_at=None) + # Bring back the shortcut trashed along the restricted folder, unless + # its own subtree went to the trash meanwhile + if self.is_restricted: + shortcut = self._meta.model.objects.filter( + target=self, deleted_at=current_deleted_at + ).first() + if shortcut and not shortcut.ancestors().filter(deleted_at__isnull=False).exists(): + self._meta.model.objects.filter(pk=shortcut.pk).update( + deleted_at=None, ancestors_deleted_at=None + ) + @transaction.atomic def move(self, target): """ diff --git a/src/backend/core/tests/test_models_items_shortcuts.py b/src/backend/core/tests/test_models_items_shortcuts.py index d781c1e3..8302669b 100644 --- a/src/backend/core/tests/test_models_items_shortcuts.py +++ b/src/backend/core/tests/test_models_items_shortcuts.py @@ -128,8 +128,8 @@ def test_models_items_shortcuts_ancestor_restore_brings_them_back(): assert str(folder.path) == str(folder.id) -def test_models_items_shortcuts_target_soft_delete_detaches(): - """Trashing a restricted folder detaches its shortcut.""" +def test_models_items_shortcuts_target_soft_delete_trashes_its_shortcut(): + """Trashing a restricted folder trashes its shortcut along.""" user = factories.UserFactory() parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) folder = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) @@ -138,28 +138,67 @@ def test_models_items_shortcuts_target_soft_delete_detaches(): folder.soft_delete() - assert not models.Item.objects.filter(pk=shortcut.pk).exists() folder.refresh_from_db() + shortcut.refresh_from_db() + assert shortcut.deleted_at == folder.deleted_at + assert shortcut.ancestors_deleted_at == folder.deleted_at assert folder.deleted_at is not None assert folder.is_restricted is True assert str(folder.path) == str(folder.id) -def test_models_items_shortcuts_target_restore_stays_detached(): - """A restored restricted folder comes back as a detached root.""" +def test_models_items_shortcuts_target_restore_restores_its_shortcut(): + """Restoring a restricted folder restores its shortcut.""" user = factories.UserFactory() parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) folder = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) folder = folder.restrict(user) + shortcut = folder.shortcut folder.soft_delete() folder.restore() folder.refresh_from_db() + shortcut.refresh_from_db() assert folder.deleted_at is None assert folder.is_restricted is True assert str(folder.path) == str(folder.id) - assert not models.Item.objects.filter(target=folder).exists() + assert shortcut.deleted_at is None + assert shortcut.ancestors_deleted_at is None + + +def test_models_items_shortcuts_target_restore_leaves_shortcut_in_trashed_subtree(): + """The shortcut stays in the trash when its own subtree was trashed meanwhile.""" + user = factories.UserFactory() + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + folder = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + folder = folder.restrict(user) + shortcut = folder.shortcut + folder.soft_delete() + parent.soft_delete() + + folder.restore() + + folder.refresh_from_db() + shortcut.refresh_from_db() + assert folder.deleted_at is None + assert str(folder.path) == str(folder.id) + assert shortcut.deleted_at is not None + + +def test_models_items_shortcuts_target_hard_delete_marks_the_shortcut(): + """Hard deleting a restricted folder hard deletes its shortcut.""" + user = factories.UserFactory() + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + folder = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + folder = folder.restrict(user) + shortcut = folder.shortcut + folder.soft_delete() + + folder.hard_delete() + + shortcut.refresh_from_db() + assert shortcut.hard_deleted_at is not None def test_models_items_shortcuts_item_factory_never_generates_shortcuts():