From 01f545718abd85ad750a7792da43b709fee71c61 Mon Sep 17 00:00:00 2001 From: Valentin Rigal <rigal@teklia.com> Date: Tue, 9 Jun 2020 15:20:56 +0000 Subject: [PATCH] Prevent updating element type if it has forbidden manual transcriptions --- arkindex/documents/api/elements.py | 6 ++ arkindex/documents/serializers/elements.py | 14 ++++ arkindex/documents/tests/test_elements_api.py | 78 +++++++++++++++++-- 3 files changed, 93 insertions(+), 5 deletions(-) diff --git a/arkindex/documents/api/elements.py b/arkindex/documents/api/elements.py index d71eeeace9..88d74b8bd7 100644 --- a/arkindex/documents/api/elements.py +++ b/arkindex/documents/api/elements.py @@ -401,6 +401,12 @@ class ElementRetrieve(RetrieveUpdateDestroyAPIView): 'tags': ['elements'], } + def get_object(self): + # Prevent duplicating database request + if not hasattr(self, 'element'): + self.element = super().get_object() + return self.element + def get_queryset(self): corpora = Corpus.objects.readable(self.request.user) return Element.objects \ diff --git a/arkindex/documents/serializers/elements.py b/arkindex/documents/serializers/elements.py index 2d27bbe05f..970d48e7d6 100644 --- a/arkindex/documents/serializers/elements.py +++ b/arkindex/documents/serializers/elements.py @@ -229,6 +229,7 @@ class ElementSerializer(ElementSlimSerializer): return instance def update(self, instance, validated_data): + image = validated_data.pop('image', None) polygon = validated_data.pop('polygon', None) if polygon or image: @@ -248,6 +249,19 @@ class ElementSerializer(ElementSlimSerializer): instance.zone, _ = image.zones.get_or_create(polygon=polygon) + new_type = validated_data.get('type') + if new_type: + allowed_transcription = instance.type.allowed_transcription + if (allowed_transcription + # Element type has been updated with different annotation permissions + and new_type.allowed_transcription != allowed_transcription + # And a manual transcription exists + and instance.transcriptions.filter(source__slug='manual', type=allowed_transcription).exists()): + raise ValidationError({'type': [ + 'The updated type does not allow the element to keep its ' + f'manual {allowed_transcription.value} transcriptions' + ]}) + instance = super().update(instance, validated_data) return instance diff --git a/arkindex/documents/tests/test_elements_api.py b/arkindex/documents/tests/test_elements_api.py index 1c18344448..376f009d5c 100644 --- a/arkindex/documents/tests/test_elements_api.py +++ b/arkindex/documents/tests/test_elements_api.py @@ -20,8 +20,10 @@ class TestElementsAPI(FixtureAPITestCase): cls.volume_type = cls.corpus.types.get(slug='volume') cls.page_type = cls.corpus.types.get(slug='page') cls.act_type = cls.corpus.types.get(slug='act') + cls.line_type = cls.corpus.types.get(slug='text_line') cls.vol = cls.corpus.elements.get(name='Volume 1') cls.element = Element.objects.get(name='Volume 1, page 2r') + cls.line = cls.corpus.elements.get(name='Text line') cls.image = ImageServer.objects.local.images.create( path="kingdom/far/away", status=S3FileStatus.Checked, @@ -30,6 +32,7 @@ class TestElementsAPI(FixtureAPITestCase): ) cls.private_corpus = Corpus.objects.create(name='private', public=False) cls.private_elt = cls.private_corpus.elements.create(type=cls.private_corpus.types.create(slug='type')) + cls.manual_source = DataSource.objects.create(type=MLToolType.Recognizer, slug='manual', internal=True) def setUp(self): self.page = self.corpus.elements.get(name='Volume 1, page 1r') @@ -150,11 +153,12 @@ class TestElementsAPI(FixtureAPITestCase): def test_patch_element(self): self.client.force_login(self.user) - response = self.client.patch( - reverse('api:element-retrieve', kwargs={'pk': str(self.vol.id)}), - data={'name': 'Untitled (2)'}, - format='json', - ) + with self.assertNumQueries(10): + response = self.client.patch( + reverse('api:element-retrieve', kwargs={'pk': str(self.vol.id)}), + data={'name': 'Untitled (2)'}, + format='json', + ) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json()['name'], 'Untitled (2)') self.vol.refresh_from_db() @@ -261,6 +265,70 @@ class TestElementsAPI(FixtureAPITestCase): self.assertEqual(self.vol.zone.polygon.width, 20) self.assertEqual(self.vol.zone.polygon.height, 10) + def test_element_patch_type_allowed_transcription(self): + """ + The base number of requests is increased by 1 because + of the check on the given type allowed transcriptions + """ + self.client.force_login(self.user) + with self.assertNumQueries(11): + response = self.client.patch( + reverse('api:element-retrieve', kwargs={'pk': str(self.vol.id)}), + data={'type': self.line_type.slug, 'name': 'Text line'}, + format='json' + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_element_patch_type_with_allowed_transcription(self): + """ + Update an element with a type previously allowing a manual transcription + The base number of requests is increased by 2 because of the checks on the + given type allowed transcriptions and the element manual transcriptions existence + """ + self.client.force_login(self.user) + with self.assertNumQueries(12): + response = self.client.patch( + reverse('api:element-retrieve', kwargs={'pk': str(self.line.id)}), + data={'type': self.volume_type.slug, 'name': 'Text line'}, + format='json' + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_element_patch_type_no_allowed_transcription(self): + """ + Patching the element type must be forbidden if it has manual transcriptions + """ + self.line.transcriptions.create(text='A line', type=TranscriptionType.Line, source=self.manual_source) + + self.client.force_login(self.user) + with self.assertNumQueries(9): + response = self.client.patch( + reverse('api:element-retrieve', kwargs={'pk': str(self.line.id)}), + data={'type': self.page_type.slug}, + format='json' + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual(response.json(), { + 'type': ['The updated type does not allow the element to keep its manual line transcriptions'] + }) + + def test_element_patch_type_same_allowed_transcription(self): + """ + Allows patching the element with a type allowing the same manual transcriptions + """ + self.line.transcriptions.create(text='A line', type=TranscriptionType.Line, source=self.manual_source) + self.element.type.allowed_transcription = TranscriptionType.Line + self.element.type.save() + + self.client.force_login(self.user) + response = self.client.patch( + reverse('api:element-retrieve', kwargs={'pk': str(self.line.id)}), + data={'type': self.page_type.slug}, + format='json' + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json().get('type'), 'page') + def make_create_request(self, name='default', corpus=None, elt_type='volume', **options): request = { 'path': reverse('api:elements-create'), -- GitLab