✨(backend) add API endpoint action to restore a soft deleted document
Only owners can see and restore deleted documents. They can only do it during the grace period before the document is considered hard deleted and hidden from everybody on the API.
This commit is contained in:
committed by
Anthony LC
parent
8ccfdb3c6a
commit
239342fbbd
@@ -11,7 +11,7 @@ and this project adheres to
|
||||
|
||||
## Added
|
||||
|
||||
- ✨(backend) add soft delete API endpoint to documents #516
|
||||
- ✨(backend) add soft delete and restore API endpoints to documents #516
|
||||
- ✨(backend) allow organizing documents in a tree structure #516
|
||||
- ✨(backend) add "excerpt" field to document list serializer #516
|
||||
- ✨(backend) add github actions to manage Crowdin workflow #559 & #563
|
||||
|
||||
@@ -714,6 +714,22 @@ class DocumentViewSet(
|
||||
{"message": "Document moved successfully."}, status=status.HTTP_200_OK
|
||||
)
|
||||
|
||||
@drf.decorators.action(
|
||||
detail=True,
|
||||
methods=["post"],
|
||||
)
|
||||
def restore(self, request, *args, **kwargs):
|
||||
"""
|
||||
Restore a soft-deleted document if it was deleted less than x days ago.
|
||||
"""
|
||||
document = self.get_object()
|
||||
document.restore()
|
||||
|
||||
return drf_response.Response(
|
||||
{"detail": "Document has been successfully restored."},
|
||||
status=status.HTTP_200_OK,
|
||||
)
|
||||
|
||||
@drf.decorators.action(
|
||||
detail=True,
|
||||
methods=["get", "post"],
|
||||
|
||||
@@ -662,6 +662,7 @@ class Document(MP_Node, BaseModel):
|
||||
"invite_owner": is_owner,
|
||||
"move": is_owner_or_admin and not self.ancestors_deleted_at,
|
||||
"partial_update": can_update,
|
||||
"restore": is_owner,
|
||||
"retrieve": can_get,
|
||||
"media_auth": can_get,
|
||||
"update": can_update,
|
||||
@@ -753,6 +754,52 @@ class Document(MP_Node, BaseModel):
|
||||
ancestors_deleted_at=self.ancestors_deleted_at
|
||||
)
|
||||
|
||||
@transaction.atomic
|
||||
def restore(self):
|
||||
"""Cancelling a soft delete with checks."""
|
||||
# This should not happen
|
||||
if self.deleted_at is None:
|
||||
raise ValidationError({"deleted_at": [_("This document is not deleted.")]})
|
||||
|
||||
if self.deleted_at < get_trashbin_cutoff():
|
||||
raise ValidationError(
|
||||
{
|
||||
"deleted_at": [
|
||||
_(
|
||||
"This document was permanently deleted and cannot be restored."
|
||||
)
|
||||
]
|
||||
}
|
||||
)
|
||||
|
||||
# Restore the current document
|
||||
self.deleted_at = None
|
||||
|
||||
# Calculate the minimum `deleted_at` among all ancestors
|
||||
ancestors_deleted_at = (
|
||||
self.get_ancestors()
|
||||
.filter(deleted_at__isnull=False)
|
||||
.values_list("deleted_at", flat=True)
|
||||
)
|
||||
self.ancestors_deleted_at = min(ancestors_deleted_at, default=None)
|
||||
self.save()
|
||||
|
||||
# Update descendants excluding those who were deleted prior to the deletion of the
|
||||
# current document (the ancestor_deleted_at date for those should already by good)
|
||||
# The number of deleted descendants should not be too big so we can handcraft a union
|
||||
# clause for them:
|
||||
deleted_descendants_paths = (
|
||||
self.get_descendants()
|
||||
.filter(deleted_at__isnull=False)
|
||||
.values_list("path", flat=True)
|
||||
)
|
||||
exclude_condition = models.Q(
|
||||
*(models.Q(path__startswith=path) for path in deleted_descendants_paths)
|
||||
)
|
||||
self.get_descendants().exclude(exclude_condition).update(
|
||||
ancestors_deleted_at=self.ancestors_deleted_at
|
||||
)
|
||||
|
||||
|
||||
class LinkTrace(BaseModel):
|
||||
"""
|
||||
|
||||
126
src/backend/core/tests/documents/test_api_documents_restore.py
Normal file
126
src/backend/core/tests/documents/test_api_documents_restore.py
Normal file
@@ -0,0 +1,126 @@
|
||||
"""
|
||||
Test restoring documents after a soft delete via the detail action API endpoint.
|
||||
"""
|
||||
|
||||
from datetime import timedelta
|
||||
|
||||
from django.utils import timezone
|
||||
|
||||
import pytest
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
from core import factories
|
||||
|
||||
pytestmark = pytest.mark.django_db
|
||||
|
||||
|
||||
def test_api_documents_restore_anonymous_user():
|
||||
"""Anonymous users should not be able to restore deleted documents."""
|
||||
now = timezone.now() - timedelta(days=15)
|
||||
document = factories.DocumentFactory(deleted_at=now)
|
||||
|
||||
response = APIClient().post(f"/api/v1.0/documents/{document.id!s}/restore/")
|
||||
|
||||
assert response.status_code == 404
|
||||
assert response.json() == {"detail": "Not found."}
|
||||
|
||||
document.refresh_from_db()
|
||||
assert document.deleted_at == now
|
||||
assert document.ancestors_deleted_at == now
|
||||
|
||||
|
||||
@pytest.mark.parametrize("role", [None, "reader", "editor", "administrator"])
|
||||
def test_api_documents_restore_authenticated_no_permission(role):
|
||||
"""
|
||||
Authenticated users who are not owners of a deleted document should
|
||||
not be allowed to restore it.
|
||||
"""
|
||||
user = factories.UserFactory()
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
|
||||
now = timezone.now() - timedelta(days=15)
|
||||
document = factories.DocumentFactory(
|
||||
deleted_at=now, link_reach="public", link_role="editor"
|
||||
)
|
||||
if role:
|
||||
factories.UserDocumentAccessFactory(document=document, user=user, role=role)
|
||||
|
||||
response = client.post(f"/api/v1.0/documents/{document.id!s}/restore/")
|
||||
|
||||
assert response.status_code == 404
|
||||
assert response.json() == {"detail": "Not found."}
|
||||
|
||||
document.refresh_from_db()
|
||||
assert document.deleted_at == now
|
||||
assert document.ancestors_deleted_at == now
|
||||
|
||||
|
||||
def test_api_documents_restore_authenticated_owner_success():
|
||||
"""The owner of a deleted document should be able to restore it."""
|
||||
user = factories.UserFactory()
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
|
||||
now = timezone.now() - timedelta(days=15)
|
||||
document = factories.DocumentFactory(deleted_at=now)
|
||||
factories.UserDocumentAccessFactory(document=document, user=user, role="owner")
|
||||
|
||||
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."}
|
||||
|
||||
document.refresh_from_db()
|
||||
assert document.deleted_at is None
|
||||
assert document.ancestors_deleted_at is None
|
||||
|
||||
|
||||
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.
|
||||
"""
|
||||
user = factories.UserFactory()
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
|
||||
grand_parent = factories.DocumentFactory()
|
||||
parent = factories.DocumentFactory(parent=grand_parent)
|
||||
document = factories.DocumentFactory(parent=parent)
|
||||
factories.UserDocumentAccessFactory(document=document, user=user, role="owner")
|
||||
|
||||
document.soft_delete()
|
||||
document_deleted_at = document.deleted_at
|
||||
assert document_deleted_at is not None
|
||||
|
||||
grand_parent.soft_delete()
|
||||
grand_parent_deleted_at = grand_parent.deleted_at
|
||||
assert grand_parent_deleted_at is not None
|
||||
|
||||
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."}
|
||||
|
||||
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
|
||||
|
||||
|
||||
def test_api_documents_restore_authenticated_owner_expired():
|
||||
"""It should not be possible to restore a document beyond the allowed time limit."""
|
||||
user = factories.UserFactory()
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
|
||||
now = timezone.now() - timedelta(days=40)
|
||||
document = factories.DocumentFactory(deleted_at=now)
|
||||
factories.UserDocumentAccessFactory(document=document, user=user, role="owner")
|
||||
|
||||
response = client.post(f"/api/v1.0/documents/{document.id!s}/restore/")
|
||||
|
||||
assert response.status_code == 404
|
||||
assert response.json() == {"detail": "Not found."}
|
||||
@@ -42,6 +42,7 @@ def test_api_documents_retrieve_anonymous_public_standalone():
|
||||
"media_auth": True,
|
||||
"move": False,
|
||||
"partial_update": document.link_role == "editor",
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": document.link_role == "editor",
|
||||
"versions_destroy": False,
|
||||
@@ -97,6 +98,7 @@ def test_api_documents_retrieve_anonymous_public_parent():
|
||||
"media_auth": True,
|
||||
"move": False,
|
||||
"partial_update": grand_parent.link_role == "editor",
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": grand_parent.link_role == "editor",
|
||||
"versions_destroy": False,
|
||||
@@ -185,6 +187,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
|
||||
"media_auth": True,
|
||||
"move": False,
|
||||
"partial_update": document.link_role == "editor",
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": document.link_role == "editor",
|
||||
"versions_destroy": False,
|
||||
@@ -247,6 +250,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea
|
||||
"move": False,
|
||||
"media_auth": True,
|
||||
"partial_update": grand_parent.link_role == "editor",
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": grand_parent.link_role == "editor",
|
||||
"versions_destroy": False,
|
||||
@@ -418,6 +422,7 @@ def test_api_documents_retrieve_authenticated_related_parent():
|
||||
"media_auth": True,
|
||||
"move": access.role in ["administrator", "owner"],
|
||||
"partial_update": access.role != "reader",
|
||||
"restore": access.role == "owner",
|
||||
"retrieve": True,
|
||||
"update": access.role != "reader",
|
||||
"versions_destroy": access.role in ["administrator", "owner"],
|
||||
|
||||
@@ -160,6 +160,7 @@ def test_models_documents_get_abilities_forbidden(
|
||||
"move": False,
|
||||
"link_configuration": False,
|
||||
"partial_update": False,
|
||||
"restore": False,
|
||||
"retrieve": False,
|
||||
"update": False,
|
||||
"versions_destroy": False,
|
||||
@@ -207,6 +208,7 @@ def test_models_documents_get_abilities_reader(
|
||||
"media_auth": True,
|
||||
"move": False,
|
||||
"partial_update": False,
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": False,
|
||||
"versions_destroy": False,
|
||||
@@ -254,6 +256,7 @@ def test_models_documents_get_abilities_editor(
|
||||
"media_auth": True,
|
||||
"move": False,
|
||||
"partial_update": True,
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": True,
|
||||
"versions_destroy": False,
|
||||
@@ -288,6 +291,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries):
|
||||
"media_auth": True,
|
||||
"move": True,
|
||||
"partial_update": True,
|
||||
"restore": True,
|
||||
"retrieve": True,
|
||||
"update": True,
|
||||
"versions_destroy": True,
|
||||
@@ -322,6 +326,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries)
|
||||
"media_auth": True,
|
||||
"move": True,
|
||||
"partial_update": True,
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": True,
|
||||
"versions_destroy": True,
|
||||
@@ -355,6 +360,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries):
|
||||
"media_auth": True,
|
||||
"move": False,
|
||||
"partial_update": True,
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": True,
|
||||
"versions_destroy": False,
|
||||
@@ -391,6 +397,7 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries):
|
||||
"media_auth": True,
|
||||
"move": False,
|
||||
"partial_update": access_from_link,
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": access_from_link,
|
||||
"versions_destroy": False,
|
||||
@@ -430,6 +437,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries):
|
||||
"media_auth": True,
|
||||
"move": False,
|
||||
"partial_update": False,
|
||||
"restore": False,
|
||||
"retrieve": True,
|
||||
"update": False,
|
||||
"versions_destroy": False,
|
||||
|
||||
Reference in New Issue
Block a user