studio/docs
Archived
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

✨(backend) add API endpoint to move a document in the document tree

Browse Source 
Only administrators or owners of a document can move it to a target
document for which they are also administrator or owner.

We allow different moving modes:
- first-child: move the document as the first child of the target
- last-child: move the document as the last child of the target
- first-sibling: move the document as the first sibling of the target
- last-sibling: move the document as the last sibling of the target
- left: move the document as sibling ordered just before the target
- right: move the document as sibling ordered just after the target

The whole subtree below the document that is being moved, moves as
well and remains below the document after it is moved.
This commit is contained in:
Samuel Paccoud - DINUM
2025-01-02 23:15:03 +01:00
committed by Anthony LC
parent 2e8a399668
commit 4de03d292a
8 changed files with 466 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
src/backend/core/api/serializers.py
View File

@@ -568,3 +568,37 @@ class AITranslateSerializer(serializers.Serializer):
if len(value.strip()) == 0:
raise serializers.ValidationError("Text field cannot be empty.")
return value
class MoveDocumentSerializer(serializers.Serializer):
"""
Serializer for validating input data to move a document within the tree structure.
Fields:
- target_document_id (UUIDField): The ID of the target parent document where the
document should be moved. This field is required and must be a valid UUID.
- position (ChoiceField): Specifies the position of the document in relation to
the target parent's children.
Choices:
- "first-child": Place the document as the first child of the target parent.
- "last-child": Place the document as the last child of the target parent (default).
- "left": Place the document as the left sibling of the target parent.
- "right": Place the document as the right sibling of the target parent.
Example:
Input payload for moving a document:
{
"target_document_id": "123e4567-e89b-12d3-a456-426614174000",
"position": "first-child"
}
Notes:
- The `target_document_id` is mandatory.
- The `position` defaults to "last-child" if not provided.
"""
target_document_id = serializers.UUIDField(required=True)
position = serializers.ChoiceField(
choices=enums.MoveNodePositionChoices.choices,
default=enums.MoveNodePositionChoices.LAST_CHILD,
)

60
src/backend/core/api/viewsets.py
View File

@@ -311,6 +311,7 @@ class DocumentMetadata(drf.metadata.SimpleMetadata):
return simple_metadata
# pylint: disable=too-many-public-methods
class DocumentViewSet(
drf.mixins.CreateModelMixin,
drf.mixins.DestroyModelMixin,
@@ -490,6 +491,65 @@ class DocumentViewSet(
{"id": str(document.id)}, status=status.HTTP_201_CREATED
)
@drf.decorators.action(detail=True, methods=["post"])
@transaction.atomic
def move(self, request, *args, **kwargs):
"""
Move a document to another location within the document tree.
The user must be an administrator or owner of both the document being moved
and the target parent document.
"""
user = request.user
document = self.get_object() # including permission checks
# Validate the input payload
serializer = serializers.MoveDocumentSerializer(data=request.data)
serializer.is_valid(raise_exception=True)
validated_data = serializer.validated_data
target_document_id = validated_data["target_document_id"]
try:
target_document = models.Document.objects.get(
id=target_document_id, ancestors_deleted_at__isnull=True
)
except models.Document.DoesNotExist:
return drf.response.Response(
{"target_document_id": "Target parent document does not exist."},
status=status.HTTP_400_BAD_REQUEST,
)
position = validated_data["position"]
message = None
if position in [
enums.MoveNodePositionChoices.FIRST_CHILD,
enums.MoveNodePositionChoices.LAST_CHILD,
]:
if not target_document.get_abilities(user).get("move"):
message = (
"You do not have permission to move documents "
"as a child to this target document."
)
elif not target_document.is_root():
if not target_document.get_parent().get_abilities(user).get("move"):
message = (
"You do not have permission to move documents "
"as a sibling of this target document."
)
if message:
return drf.response.Response(
{"target_document_id": message},
status=status.HTTP_400_BAD_REQUEST,
)
document.move(target_document, pos=position)
return drf.response.Response(
{"message": "Document moved successfully."}, status=status.HTTP_200_OK
)
@drf.decorators.action(
detail=True,
methods=["get", "post"],

12
src/backend/core/enums.py
View File

@@ -3,6 +3,7 @@ Core application enums declaration
"""
from django.conf import global_settings
from django.db import models
from django.utils.translation import gettext_lazy as _
# In Django's code base, `LANGUAGES` is set by default with all supported languages.
@@ -10,3 +11,14 @@ from django.utils.translation import gettext_lazy as _
# active in the app.
# pylint: disable=no-member
ALL_LANGUAGES = {language: _(name) for language, name in global_settings.LANGUAGES}
class MoveNodePositionChoices(models.TextChoices):
"""Defines the possible positions when moving a django-treebeard node."""
FIRST_CHILD = "first-child", _("First child")
LAST_CHILD = "last-child", _("Last child")
FIRST_SIBLING = "first-sibling", _("First sibling")
LAST_SIBLING = "last-sibling", _("Last sibling")
LEFT = "left", _("Left")
RIGHT = "right", _("Right")

1
src/backend/core/models.py
View File

@@ -592,6 +592,7 @@ class Document(MP_Node, BaseModel):
"favorite": can_get and user.is_authenticated,
"link_configuration": is_owner_or_admin,
"invite_owner": RoleChoices.OWNER in roles,
"move": is_owner_or_admin,
"partial_update": can_update,
"retrieve": can_get,
"media_auth": can_get,

339
src/backend/core/tests/documents/test_api_documents_move.py Normal file
View File

@@ -0,0 +1,339 @@
"""
Test moving documents within the document tree via an detail action API endpoint.
"""
import random
from uuid import uuid4
from django.utils import timezone
import pytest
from rest_framework.test import APIClient
from core import enums, factories, models
pytestmark = pytest.mark.django_db
def test_api_documents_move_anonymous_user():
"""Anonymous users should not be able to move documents."""
document = factories.DocumentFactory()
target = factories.DocumentFactory()
response = APIClient().post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id)},
)
assert response.status_code == 401
assert response.json() == {
"detail": "Authentication credentials were not provided."
}
@pytest.mark.parametrize("role", [None, "reader", "editor"])
def test_api_documents_move_authenticated_document_no_permission(role):
"""
Authenticated users should not be able to move documents with insufficient
permissions on the origin document.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
document = factories.DocumentFactory()
target = factories.UserDocumentAccessFactory(user=user, role="owner").document
if role:
factories.UserDocumentAccessFactory(document=document, user=user, role=role)
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id)},
)
assert response.status_code == 403
assert response.json() == {
"detail": "You do not have permission to perform this action."
}
def test_api_documents_move_invalid_target_string():
"""Test for moving a document to an invalid target as a random string."""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
document = factories.UserDocumentAccessFactory(user=user, role="owner").document
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": "non-existent-id"},
)
assert response.status_code == 400
assert response.json() == {"target_document_id": ["Must be a valid UUID."]}
def test_api_documents_move_invalid_target_uuid():
"""Test for moving a document to an invalid target that looks like a UUID."""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
document = factories.UserDocumentAccessFactory(user=user, role="owner").document
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(uuid4())},
)
assert response.status_code == 400
assert response.json() == {
"target_document_id": "Target parent document does not exist."
}
def test_api_documents_move_invalid_position():
"""Test moving a document to an invalid position."""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
document = factories.UserDocumentAccessFactory(user=user, role="owner").document
target = factories.UserDocumentAccessFactory(user=user, role="owner").document
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={
"target_document_id": str(target.id),
"position": "invalid-position",
},
)
assert response.status_code == 400
assert response.json() == {
"position": ['"invalid-position" is not a valid choice.']
}
@pytest.mark.parametrize("position", enums.MoveNodePositionChoices.values)
@pytest.mark.parametrize("target_parent_role", models.RoleChoices.values)
@pytest.mark.parametrize("target_role", models.RoleChoices.values)
def test_api_documents_move_authenticated_target_roles_mocked(
target_role, target_parent_role, position
):
"""
Authenticated users with insufficient permissions on the target document (or its
parent depending on the position chosen), should not be allowed to move documents.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
power_roles = ["administrator", "owner"]
document = factories.DocumentFactory(users=[(user, random.choice(power_roles))])
children = factories.DocumentFactory.create_batch(3, parent=document)
target_parent = factories.DocumentFactory(users=[(user, target_parent_role)])
sibling1, target, sibling2 = factories.DocumentFactory.create_batch(
3, parent=target_parent
)
models.DocumentAccess.objects.create(document=target, user=user, role=target_role)
target_children = factories.DocumentFactory.create_batch(2, parent=target)
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": position},
)
document.refresh_from_db()
if (
position in ["first-child", "last-child"]
and (target_role in power_roles or target_parent_role in power_roles)
) or (
position in ["first-sibling", "last-sibling", "left", "right"]
and target_parent_role in power_roles
):
assert response.status_code == 200
assert response.json() == {"message": "Document moved successfully."}
match position:
case "first-child":
assert list(target.get_children()) == [document, *target_children]
case "last-child":
assert list(target.get_children()) == [*target_children, document]
case "first-sibling":
assert list(target.get_siblings()) == [
document,
sibling1,
target,
sibling2,
]
case "last-sibling":
assert list(target.get_siblings()) == [
sibling1,
target,
sibling2,
document,
]
case "left":
assert list(target.get_siblings()) == [
sibling1,
document,
target,
sibling2,
]
case "right":
assert list(target.get_siblings()) == [
sibling1,
target,
document,
sibling2,
]
case _:
raise ValueError(f"Invalid position: {position}")
# Verify that the document's children have also been moved
assert list(document.get_children()) == children
else:
assert response.status_code == 400
assert (
"You do not have permission to move documents"
in response.json()["target_document_id"]
)
assert document.is_root() is True
def test_api_documents_move_authenticated_deleted_document():
"""
It should not be possible to move a deleted document or its descendants, even
for an owner.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
document = factories.DocumentFactory(
users=[(user, "owner")], deleted_at=timezone.now()
)
child = factories.DocumentFactory(parent=document, users=[(user, "owner")])
target = factories.DocumentFactory(users=[(user, "owner")])
# Try moving the deleted document
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id)},
)
assert response.status_code == 403
assert response.json() == {
"detail": "You do not have permission to perform this action."
}
# Verify that the document has not moved
document.refresh_from_db()
assert document.is_root() is True
# Try moving the child of the deleted document
response = client.post(
f"/api/v1.0/documents/{child.id!s}/move/",
data={"target_document_id": str(target.id)},
)
assert response.status_code == 403
assert response.json() == {
"detail": "You do not have permission to perform this action."
}
# Verify that the child has not moved
child.refresh_from_db()
assert child.is_child_of(document) is True
@pytest.mark.parametrize(
"position",
enums.MoveNodePositionChoices.values,
)
def test_api_documents_move_authenticated_deleted_target_as_child(position):
"""
It should not be possible to move a document as a child of a deleted target
even for a owner.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
document = factories.DocumentFactory(users=[(user, "owner")])
target = factories.DocumentFactory(
users=[(user, "owner")], deleted_at=timezone.now()
)
child = factories.DocumentFactory(parent=target, users=[(user, "owner")])
# Try moving the document to the deleted target
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": position},
)
assert response.status_code == 400
assert response.json() == {
"target_document_id": "Target parent document does not exist."
}
# Verify that the document has not moved
document.refresh_from_db()
assert document.is_root() is True
# Try moving the document to the child of the deleted target
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(child.id), "position": position},
)
assert response.status_code == 400
assert response.json() == {
"target_document_id": "Target parent document does not exist."
}
# Verify that the document has not moved
document.refresh_from_db()
assert document.is_root() is True
@pytest.mark.parametrize(
"position",
["first-sibling", "last-sibling", "left", "right"],
)
def test_api_documents_move_authenticated_deleted_target_as_sibling(position):
"""
It should not be possible to move a document as a sibling of a deleted target document
if the user has no rigths on its parent.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)
document = factories.DocumentFactory(users=[(user, "owner")])
target_parent = factories.DocumentFactory(
users=[(user, "owner")], deleted_at=timezone.now()
)
target = factories.DocumentFactory(users=[(user, "owner")], parent=target_parent)
# Try moving the document as a sibling of the target
response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": position},
)
assert response.status_code == 400
assert response.json() == {
"target_document_id": "Target parent document does not exist."
}
# Verify that the document has not moved
document.refresh_from_db()
assert document.is_root() is True

8
src/backend/core/tests/documents/test_api_documents_retrieve.py
View File

@@ -36,6 +36,7 @@ def test_api_documents_retrieve_anonymous_public_standalone():
"invite_owner": False,
"link_configuration": False,
"media_auth": True,
"move": False,
"partial_update": document.link_role == "editor",
"retrieve": True,
"update": document.link_role == "editor",
@@ -90,6 +91,7 @@ def test_api_documents_retrieve_anonymous_public_parent():
"invite_owner": False,
"link_configuration": False,
"media_auth": True,
"move": False,
"partial_update": grand_parent.link_role == "editor",
"retrieve": True,
"update": grand_parent.link_role == "editor",
@@ -175,8 +177,9 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
"destroy": False,
"favorite": True,
"invite_owner": False,
"media_auth": True,
"link_configuration": False,
"media_auth": True,
"move": False,
"partial_update": document.link_role == "editor",
"retrieve": True,
"update": document.link_role == "editor",
@@ -237,6 +240,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea
"favorite": True,
"invite_owner": False,
"link_configuration": False,
"move": False,
"media_auth": True,
"partial_update": grand_parent.link_role == "editor",
"retrieve": True,
@@ -408,6 +412,7 @@ def test_api_documents_retrieve_authenticated_related_parent():
"invite_owner": access.role == "owner",
"link_configuration": access.role in ["administrator", "owner"],
"media_auth": True,
"move": access.role in ["administrator", "owner"],
"partial_update": access.role != "reader",
"retrieve": True,
"update": access.role != "reader",
@@ -753,4 +758,3 @@ def test_api_documents_retrieve_numqueries_with_link_trace(django_assert_num_que
assert response.status_code == 200
assert response.json()["id"] == str(document.id)

8
src/backend/core/tests/test_models_documents.py
View File

@@ -116,6 +116,7 @@ def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role)
"favorite": False,
"invite_owner": False,
"media_auth": False,
"move": False,
"link_configuration": False,
"partial_update": False,
"retrieve": False,
@@ -156,6 +157,7 @@ def test_models_documents_get_abilities_reader(is_authenticated, reach):
"invite_owner": False,
"link_configuration": False,
"media_auth": True,
"move": False,
"partial_update": False,
"retrieve": True,
"update": False,
@@ -195,6 +197,7 @@ def test_models_documents_get_abilities_editor(is_authenticated, reach):
"invite_owner": False,
"link_configuration": False,
"media_auth": True,
"move": False,
"partial_update": True,
"retrieve": True,
"update": True,
@@ -223,6 +226,7 @@ def test_models_documents_get_abilities_owner():
"invite_owner": True,
"link_configuration": True,
"media_auth": True,
"move": True,
"partial_update": True,
"retrieve": True,
"update": True,
@@ -250,6 +254,7 @@ def test_models_documents_get_abilities_administrator():
"invite_owner": False,
"link_configuration": True,
"media_auth": True,
"move": True,
"partial_update": True,
"retrieve": True,
"update": True,
@@ -280,6 +285,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries):
"invite_owner": False,
"link_configuration": False,
"media_auth": True,
"move": False,
"partial_update": True,
"retrieve": True,
"update": True,
@@ -312,6 +318,7 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries):
"invite_owner": False,
"link_configuration": False,
"media_auth": True,
"move": False,
"partial_update": False,
"retrieve": True,
"update": False,
@@ -345,6 +352,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries):
"invite_owner": False,
"link_configuration": False,
"media_auth": True,
"move": False,
"partial_update": False,
"retrieve": True,
"update": False,

8
src/backend/demo/tests/test_commands_create_demo.py
View File

@@ -1,4 +1,5 @@
"""Test the `create_demo` management command"""
from unittest import mock
from django.core.management import call_command
@@ -11,11 +12,14 @@ from core import models
pytestmark = pytest.mark.django_db
@mock.patch("demo.defaults.NB_OBJECTS", {
@mock.patch(
"demo.defaults.NB_OBJECTS",
{
"users": 10,
"docs": 10,
"max_users_per_document": 5,
} )
},
)
@override_settings(DEBUG=True)
def test_commands_create_demo():
"""The create_demo management command should create objects as expected."""