diff --git a/arkindex/documents/api/elements.py b/arkindex/documents/api/elements.py index e135afb0f1793196c98c16f242d1f1abeb6025ad..754c7800075c762b370e2576db078074c442edb1 100644 --- a/arkindex/documents/api/elements.py +++ b/arkindex/documents/api/elements.py @@ -61,6 +61,7 @@ from arkindex.documents.models import ( from arkindex.documents.serializers.elements import ( CorpusSerializer, ElementBulkSerializer, + ElementChildrenCreateSerializer, ElementCreateSerializer, ElementDestinationSerializer, ElementListSerializer, @@ -1210,6 +1211,41 @@ class ElementChildren(ElementsListBase): return Element.objects.all().distinct() +class ElementChildrenCreate(CreateAPIView): + """ + Add an element as a parent to multiple elements at once. + + Requires contributor access to the corpus. + + The child elements must be in the same corpus as the parent to add. + Elements that have child elements are not supported. + """ + serializer_class = ElementChildrenCreateSerializer + permission_classes = (IsVerified, ) + + def get_queryset(self): + return Element.objects.filter(corpus__in=Corpus.objects.writable(self.request.user)).prefetch_related("paths") + + @cached_property + def parent(self): + return self.get_object() + + def get_serializer_context(self): + context = super().get_serializer_context() + # The OpenAPI schema generator creates a fake request without a parent ID set + if self.lookup_field not in self.kwargs: + return context + context["parent"] = self.parent + return context + + @extend_schema( + operation_id="CreateElementChildren", + tags=["elements"], + ) + @transaction.atomic + def post(self, *args, **kwargs): + return super().post(*args, **kwargs) + @extend_schema(tags=["elements"]) @extend_schema_view( get=extend_schema(description="Retrieve a single element's information and metadata"), diff --git a/arkindex/documents/serializers/elements.py b/arkindex/documents/serializers/elements.py index 1f443526a73cf6e81fa0f0d21cab666d00eb9033..578b57de9ae1de58838332c567fa51401511e886 100644 --- a/arkindex/documents/serializers/elements.py +++ b/arkindex/documents/serializers/elements.py @@ -2,6 +2,7 @@ import math import uuid from collections import defaultdict from functools import cached_property +from itertools import chain from textwrap import dedent from django.conf import settings @@ -9,6 +10,7 @@ from django.contrib.gis.geos import LinearRing from django.core.exceptions import ValidationError as DjangoValidationError from django.core.validators import URLValidator from django.db import transaction +from django.db.models import Exists, Max, OuterRef, Q from drf_spectacular.utils import extend_schema_field from rest_framework import serializers from rest_framework.exceptions import ValidationError @@ -943,6 +945,97 @@ class ElementBulkSerializer(serializers.Serializer): return data +class ElementChildrenCreateSerializer(serializers.Serializer): + children = serializers.ListField(child=serializers.UUIDField()) + + def validate_children(self, children): + # Ignore duplicates, but preserve order. + # The order isn't necessary for this validation, but it is necessary to create paths afterwards, + # and duplicates would also cause duplicate paths. + # Sets don't preserve order, but dicts do! + unique_children = list({child_id: None for child_id in children}) + + # Check that no child ID is in another corpus and that they do not have children themselves + # If an ID does not exist, it is not returned by this query + checks_qs = ( + Element.objects + .using("default") + .filter(id__in=unique_children) + .annotate( + different_corpus=~Q(corpus_id=self.context["parent"].corpus_id), + has_children=Exists(ElementPath.objects.filter(path__overlap=[OuterRef("pk")])), + ) + .values_list("id", "different_corpus", "has_children") + ) + checks = {id: results for id, *results in checks_qs} + + # If any ID is an ancestor of the new parent, we know we would create a cycle + # Since the parent's paths are prefetched, we can do this check in memory. + parent_ancestors = set(chain.from_iterable(p.path for p in self.context["parent"].paths.all())) + + errors = defaultdict(list) + for i, child_id in enumerate(children): + if child_id == self.context["parent"].id: + errors[i].append("An element cannot be a child of itself.") + + if child_id in parent_ancestors: + errors[i].append("This element is an ancestor of the parent; adding the parent would create a cycle.") + + if child_id not in checks: + errors[i].append("This element does not exist.") + continue + + different_corpus, has_children = checks[child_id] + if different_corpus: + errors[i].append("This element does not belong to the same corpus as the parent.") + if has_children: + errors[i].append("This element has child elements and adding a parent in bulk is not supported.") + + if errors: + raise ValidationError(errors) + + return unique_children + + def create(self, validated_data): + new_parent_paths = [ + tuple(p.path + [self.context["parent"].id]) + for p in self.context["parent"].paths.all() + ] + + max_orderings = { + tuple(path): max_ordering + for path, max_ordering in ( + ElementPath.objects + .using("default") + .filter(path__in=new_parent_paths) + .annotate(max_ordering=Max("ordering") + 1) + .values_list("path", "max_ordering") + ) + } + + # Remove any top-level paths first + ElementPath.objects.filter(element_id__in=validated_data["children"], path=[]).delete() + + ElementPath.objects.bulk_create( + [ + ElementPath( + element_id=child_id, + path=path, + ordering=max_orderings.get(path, 0) + offset, + ) + for offset, child_id in enumerate(validated_data["children"]) + for path in new_parent_paths + ], + # When a path already exist for a child, update its ordering to move the child, + # as if we removed the path and added the parent again. + update_conflicts=True, + unique_fields=("element_id", "path"), + update_fields=("ordering", ), + ) + + return validated_data + + class WorkerStatisticsSerializer(serializers.Serializer): """ Serialize activity statistics of a worker version diff --git a/arkindex/documents/tests/test_bulk_children.py b/arkindex/documents/tests/test_bulk_children.py new file mode 100644 index 0000000000000000000000000000000000000000..803d90202a44031ca49713abef45865721bbdc5f --- /dev/null +++ b/arkindex/documents/tests/test_bulk_children.py @@ -0,0 +1,209 @@ +from unittest.mock import call, patch +from uuid import uuid4 + +from django.urls import reverse +from rest_framework import status + +from arkindex.documents.models import Corpus +from arkindex.project.tests import FixtureAPITestCase, force_constraints_immediate +from arkindex.users.models import Role + + +class TestElementChildrenCreate(FixtureAPITestCase): + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cls.volume1, cls.volume2 = cls.corpus.elements.filter(type__slug="volume").order_by("name") + cls.page1, cls.page2, cls.page3 = cls.corpus.elements.filter(type__slug="page", name__contains="Volume 2, page").order_by("name") + + def test_requires_login(self): + with self.assertNumQueries(0): + response = self.client.post( + reverse("api:element-children-create", kwargs={"pk": self.volume2.id}), + {"children": [str(self.page1.id)]}, + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + self.assertEqual(response.json(), { + "detail": "Authentication credentials were not provided.", + }) + + def test_requires_verified(self): + self.user.verified_email = False + self.user.save() + self.client.force_login(self.user) + + with self.assertNumQueries(2): + response = self.client.post( + reverse("api:element-children-create", kwargs={"pk": self.volume2.id}), + {"children": [str(self.page1.id)]}, + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + self.assertEqual(response.json(), { + "detail": "You do not have permission to perform this action.", + }) + + @patch("arkindex.users.utils.filter_rights", return_value=Corpus.objects.none()) + def test_requires_contributor(self, filter_rights_mock): + self.client.force_login(self.user) + + with self.assertNumQueries(5): + response = self.client.post( + reverse("api:element-children-create", kwargs={"pk": self.volume2.id}), + {"children": [str(self.page1.id)]}, + ) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + self.assertEqual(filter_rights_mock.call_count, 1) + self.assertEqual(filter_rights_mock.call_args, call(self.user, Corpus, Role.Contributor.value)) + + def test_not_empty(self): + self.client.force_login(self.user) + + with self.assertNumQueries(7): + response = self.client.post( + reverse("api:element-children-create", kwargs={"pk": self.volume2.id}), + {"children": []}, + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json(), { + "children": ["This field is required."], + }) + + def test_parent_not_found(self): + self.client.force_login(self.user) + + with self.assertNumQueries(6): + response = self.client.post( + reverse("api:element-children-create", kwargs={"pk": uuid4()}), + {"children": [str(self.page1.id)]}, + ) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_checks(self): + other_corpus = Corpus.objects.create(name="Another corpus") + other_element = other_corpus.elements.create( + type=other_corpus.types.create(slug="other", display_name="other"), + name="Element from other corpus", + ) + + grandparent = self.corpus.elements.create(type=self.volume2.type, name="Parent of the volume") + self.volume2.add_parent(grandparent) + + self.client.force_login(self.user) + + with self.assertNumQueries(8): + response = self.client.post( + reverse("api:element-children-create", kwargs={"pk": self.volume2.id}), + {"children": [ + # This element has child elements + self.volume1.id, + # This element has child elements, and it cannot be a parent of itself + self.volume2.id, + # This doesn't exist + uuid4(), + # This element should not cause any errors + self.page1.id, + # This is from a different corpus + other_element.id, + # This is a duplicate, but should be shown with the proper index in the errors + self.volume1.id, + # This would create a cycle + grandparent.id, + ]}, + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json(), { + "children": { + "0": [ + "This element has child elements and adding a parent in bulk is not supported.", + ], + "1": [ + "An element cannot be a child of itself.", + "This element has child elements and adding a parent in bulk is not supported.", + ], + "2": [ + "This element does not exist.", + ], + "4": [ + "This element does not belong to the same corpus as the parent.", + ], + "5": [ + "This element has child elements and adding a parent in bulk is not supported.", + ], + "6": [ + "This element is an ancestor of the parent; adding the parent would create a cycle.", + "This element has child elements and adding a parent in bulk is not supported.", + ], + } + }) + + def test_create(self): + self.client.force_login(self.user) + + self.volume2.add_parent(self.volume1) + self.volume2.remove_child(self.page1) + self.volume2.remove_child(self.page2) + other_parent = self.corpus.elements.create(type=self.volume2.type, name="Other parent") + self.page2.add_parent(other_parent) + self.page3.add_parent(other_parent) + + self.assertQuerySetEqual( + self.volume2.paths.values_list("path", flat=True), + [[self.volume1.id]], + ) + self.assertQuerySetEqual( + self.page1.paths.values_list("path", "ordering"), + [([], 0)], + ) + self.assertQuerySetEqual( + self.page2.paths.values_list("path", "ordering"), + [([other_parent.id], 0)], + ) + # This page is still in the volume, but it will be reordered + self.assertQuerySetEqual( + self.page3.paths.values_list("path", "ordering").order_by("ordering"), + [ + ([other_parent.id], 1), + ([self.volume1.id, self.volume2.id], 2), + ], + ) + + with force_constraints_immediate(), self.assertNumQueries(10): + response = self.client.post( + reverse("api:element-children-create", kwargs={"pk": self.volume2.id}), + {"children": [ + str(self.page3.id), + str(self.page2.id), + str(self.page1.id), + # This duplicate should be ignored + str(self.page3.id), + ]}, + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + self.assertQuerySetEqual( + self.page1.paths.values_list("path", "ordering"), + # The top-level path is removed, and the page is back in the volume, in last position + [([self.volume1.id, self.volume2.id], 5)], + ) + self.assertQuerySetEqual( + self.page2.paths.values_list("path", "ordering").order_by("ordering"), + [ + # The path to the other parent is left untouched + ([other_parent.id], 0), + # The page is back in the volume, in second position + ([self.volume1.id, self.volume2.id], 4), + ], + ) + self.assertQuerySetEqual( + self.page3.paths.values_list("path", "ordering").order_by("ordering"), + [ + # The path to the other parent is left untouched + ([other_parent.id], 1), + # The last page got reordered + ([self.volume1.id, self.volume2.id], 3), + ], + ) diff --git a/arkindex/project/api_v1.py b/arkindex/project/api_v1.py index 938af5179658db05049d0d79fb11e85298471961..c65d036fc6982901daab14edeed8e3b7acdc7905 100644 --- a/arkindex/project/api_v1.py +++ b/arkindex/project/api_v1.py @@ -10,6 +10,7 @@ from arkindex.documents.api.elements import ( CorpusSelectionDestroy, ElementBulkCreate, ElementChildren, + ElementChildrenCreate, ElementMetadata, ElementMetadataBulk, ElementMove, @@ -153,6 +154,7 @@ api = [ name="element-transcriptions-bulk" ), path("element/<uuid:child>/parent/<uuid:parent>/", ElementParent.as_view(), name="element-parent"), + path("element/parent/<uuid:pk>/", ElementChildrenCreate.as_view(), name="element-children-create"), path("element/move/", ElementMove.as_view(), name="move-element"), # Corpora