Skip to content

Commit

Permalink
✨(backend) add API endpoint to move a document in the document tree
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sampaccoud committed Jan 2, 2025
1 parent 66ad169 commit 3485d66
Show file tree
Hide file tree
Showing 6 changed files with 295 additions and 1 deletion.
41 changes: 41 additions & 0 deletions src/backend/core/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -558,3 +558,44 @@ def validate_text(self, value):
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=[
"first-child",
"last-child",
"first-sibling",
"last-sibling",
"left",
"right",
],
default="last-child",
)
40 changes: 40 additions & 0 deletions src/backend/core/api/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ def determine_metadata(self, request, view):
return simple_metadata


# pylint: disable=too-many-public-methods
class DocumentViewSet(
drf.mixins.CreateModelMixin,
drf.mixins.DestroyModelMixin,
Expand Down Expand Up @@ -609,6 +610,45 @@ def create_for_owner(self, request):
{"id": str(document.id)}, status=status.HTTP_201_CREATED
)

@drf.decorators.action(detail=True, methods=["post"])
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)
except models.Document.DoesNotExist:
return drf.response.Response(
{"target_document_id": "Target parent document does not exist."},
status=status.HTTP_400_BAD_REQUEST,
)

# Check permission for the target parent document
if not target_document.get_abilities(user).get("move"):
message = "You do not have permission to move documents to this target."
return drf.response.Response(
{"target_document_id": message},
status=status.HTTP_400_BAD_REQUEST,
)

document.move(target_document, pos=validated_data["position"])

return drf.response.Response(
{"message": "Document moved successfully."}, status=status.HTTP_200_OK
)

@drf.decorators.action(
detail=True,
methods=["post"],
Expand Down
1 change: 1 addition & 0 deletions src/backend/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,7 @@ def get_abilities(self, user):
"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,
"restore": is_owner,
"retrieve": can_get,
Expand Down
199 changes: 199 additions & 0 deletions src/backend/core/tests/documents/test_api_documents_move.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
"""
Test moving documents within the document tree via an detail action API endpoint.
"""

from uuid import uuid4

import pytest
from rest_framework.test import APIClient

from core import factories

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."
}


@pytest.mark.parametrize("role", [None, "reader", "editor"])
def test_api_documents_move_authenticated_target_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.UserDocumentAccessFactory(user=user, role="owner").document
target = factories.DocumentFactory()

if role:
factories.UserDocumentAccessFactory(document=target, 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 == 400
assert response.json() == {
"target_document_id": "You do not have permission to move documents to this target."
}


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",
["first-child", "last-child", "first-sibling", "last-sibling", "left", "right"],
)
@pytest.mark.parametrize("target_role", ["administrator", "owner"])
@pytest.mark.parametrize("role", ["administrator", "owner"])
def test_api_documents_move_authenticated_success(role, target_role, position):
"""
Authenticated users with sufficient permissions should be able to move documents
as a child of the target document.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)

document = factories.UserDocumentAccessFactory(user=user, role=role).document
children = factories.DocumentFactory.create_batch(3, parent=document)

target_parent = factories.UserDocumentAccessFactory(
user=user, role=target_role
).document
sibling1, target, sibling2 = factories.DocumentFactory.create_batch(
3, parent=target_parent
)
target_children = factories.DocumentFactory.create_batch(3, parent=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 == 200
assert response.json() == {"message": "Document moved successfully."}

# Verify that the document has moved as expected in the tree
document.refresh_from_db()
target.refresh_from_db()

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
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,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",
"restore": False,
"retrieve": True,
Expand Down Expand Up @@ -94,6 +95,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",
"restore": False,
"retrieve": True,
Expand Down Expand Up @@ -180,8 +182,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",
"restore": False,
"retrieve": True,
Expand Down Expand Up @@ -243,6 +246,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",
"restore": False,
Expand Down Expand Up @@ -415,6 +419,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",
"restore": access.role == "owner",
"retrieve": True,
Expand Down
Loading

0 comments on commit 3485d66

Please sign in to comment.