From 87c14041d6bfd5d60edb71348630862adbbdf71e Mon Sep 17 00:00:00 2001 From: Eva Bardou <ebardou@teklia.com> Date: Thu, 16 Sep 2021 17:09:14 +0200 Subject: [PATCH] Add a new endpoint UpdateMLClass --- arkindex/documents/api/ml.py | 30 ++++++----- arkindex/documents/tests/test_classes.py | 66 ++++++++++++++++++++++-- arkindex/project/api_v1.py | 4 +- 3 files changed, 79 insertions(+), 21 deletions(-) diff --git a/arkindex/documents/api/ml.py b/arkindex/documents/api/ml.py index c0169b8e50..b5b2c378a8 100644 --- a/arkindex/documents/api/ml.py +++ b/arkindex/documents/api/ml.py @@ -5,13 +5,7 @@ from django.db import transaction from drf_spectacular.utils import PolymorphicProxySerializer, extend_schema, extend_schema_view from rest_framework import permissions, status from rest_framework.exceptions import PermissionDenied, ValidationError -from rest_framework.generics import ( - CreateAPIView, - DestroyAPIView, - GenericAPIView, - ListCreateAPIView, - RetrieveUpdateDestroyAPIView, -) +from rest_framework.generics import CreateAPIView, GenericAPIView, ListCreateAPIView, RetrieveUpdateDestroyAPIView from rest_framework.response import Response from arkindex.documents.models import ( @@ -320,20 +314,28 @@ class CorpusMLClassList(CorpusACLMixin, ListCreateAPIView): return context -@extend_schema(tags=['classifications'], responses=None) -class MLClassDestroy(CorpusACLMixin, DestroyAPIView): - """ - Delete an ML class if it is not used by any classification. - """ +@extend_schema(tags=['classifications']) +@extend_schema_view( + get=extend_schema(description='Retrieve a ML class.'), + patch=extend_schema(description='Rename a ML class.'), + put=extend_schema(description='Rename a ML class.'), + delete=extend_schema(description='Delete a ML class if it is not used by any classification.'), +) +class MLClassRetrieve(CorpusACLMixin, RetrieveUpdateDestroyAPIView): + serializer_class = MLClassSerializer queryset = MLClass.objects.none() permission_classes = (IsVerified, ) lookup_url_kwarg = 'mlclass' def get_queryset(self): - return MLClass.objects.filter(corpus=self.get_corpus(self.kwargs['corpus'], role=Role.Contributor)) + role = Role.Guest + if self.request and self.request.method != 'GET': + role = Role.Contributor + + return MLClass.objects.filter(corpus=self.get_corpus(self.kwargs['corpus'], role=role)) def check_object_permissions(self, request, obj): - if obj.classifications.count() != 0: + if self.request and self.request.method == 'DELETE' and obj.classifications.count() != 0: raise ValidationError(['This ML class is used by some classifications on this corpus.']) diff --git a/arkindex/documents/tests/test_classes.py b/arkindex/documents/tests/test_classes.py index 0c4a7f507a..59d3196bab 100644 --- a/arkindex/documents/tests/test_classes.py +++ b/arkindex/documents/tests/test_classes.py @@ -1,3 +1,5 @@ +from unittest.case import expectedFailure + from django.test import override_settings from django.urls import reverse from rest_framework import status @@ -180,10 +182,64 @@ class TestClasses(FixtureAPITestCase): 'non_field_errors': ['The fields name, corpus must make a unique set.'] }) + def test_retrieve(self): + self.client.force_login(self.superuser) + response = self.client.get(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(response.json(), { + 'id': str(self.text.id), + 'name': 'text' + }) + + def test_retrieve_requires_login(self): + response = self.client.get(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + @expectedFailure + def test_retrieve_requires_verified(self): + """ + This test fails due to a bug in the IsVerified permission class. + https://gitlab.com/arkindex/backend/-/issues/554 + """ + self.user.verified_email = False + self.user.save() + self.client.force_login(self.user) + response = self.client.get(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_update(self): + self.client.force_login(self.superuser) + response = self.client.patch(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id}), {'name': 'new name'}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(response.json(), { + 'id': str(self.text.id), + 'name': 'new name' + }) + self.text.refresh_from_db() + self.assertEqual(self.text.name, "new name") + + def test_update_requires_login(self): + response = self.client.patch(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id}), {'name': 'new name'}) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_update_requires_verified(self): + self.user.verified_email = False + self.user.save() + self.client.force_login(self.user) + response = self.client.patch(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id}), {'name': 'new name'}) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_update_requires_contributor(self): + self.user.rights.update(level=Role.Guest.value) + self.client.force_login(self.user) + response = self.client.patch(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id}), {'name': 'new name'}) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual(response.json(), {'detail': 'You do not have contributor access to this corpus.'}) + def test_destroy(self): self.client.force_login(self.superuser) self.text.classifications.all().delete() - response = self.client.delete(reverse('api:ml-class-destroy', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) + response = self.client.delete(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) with self.assertRaises(MLClass.DoesNotExist): self.text.refresh_from_db() @@ -191,25 +247,25 @@ class TestClasses(FixtureAPITestCase): def test_destroy_non_empty(self): self.client.force_login(self.superuser) self.assertTrue(self.text.classifications.exists()) - response = self.client.delete(reverse('api:ml-class-destroy', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) + response = self.client.delete(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertListEqual(response.json(), ['This ML class is used by some classifications on this corpus.']) def test_destroy_requires_login(self): - response = self.client.delete(reverse('api:ml-class-destroy', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) + response = self.client.delete(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_destroy_requires_verified(self): self.user.verified_email = False self.user.save() self.client.force_login(self.user) - response = self.client.delete(reverse('api:ml-class-destroy', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) + response = self.client.delete(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_destroy_requires_contributor(self): self.user.rights.update(level=Role.Guest.value) self.client.force_login(self.user) - response = self.client.delete(reverse('api:ml-class-destroy', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) + response = self.client.delete(reverse('api:ml-class-retrieve', kwargs={'corpus': self.corpus.id, 'mlclass': self.text.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertDictEqual(response.json(), {'detail': 'You do not have contributor access to this corpus.'}) diff --git a/arkindex/project/api_v1.py b/arkindex/project/api_v1.py index 53994d3aaa..6d5a87d636 100644 --- a/arkindex/project/api_v1.py +++ b/arkindex/project/api_v1.py @@ -74,7 +74,7 @@ from arkindex.documents.api.ml import ( CorpusMLClassList, ElementTranscriptionsBulk, ManageClassificationsSelection, - MLClassDestroy, + MLClassRetrieve, TranscriptionBulk, TranscriptionCreate, TranscriptionEdit, @@ -138,7 +138,7 @@ api = [ # `corpus` and not `pk` so that OpenAPI uses a `corpus` parameter path('corpus/<uuid:corpus>/elements/', CorpusElements.as_view(), name='corpus-elements'), path('corpus/<uuid:pk>/classes/', CorpusMLClassList.as_view(), name='corpus-classes'), - path('corpus/<uuid:corpus>/classes/<uuid:mlclass>/', MLClassDestroy.as_view(), name='ml-class-destroy'), + path('corpus/<uuid:corpus>/classes/<uuid:mlclass>/', MLClassRetrieve.as_view(), name='ml-class-retrieve'), path('corpus/<uuid:pk>/entities/', CorpusEntities.as_view(), name='corpus-entities'), path('corpus/<uuid:pk>/roles/', CorpusRoles.as_view(), name='corpus-roles'), path('corpus/<uuid:pk>/allowed-metadata/', CorpusAllowedMetaData.as_view(), name='corpus-allowed-metadata'), -- GitLab