diff --git a/arkindex/training/api.py b/arkindex/training/api.py index a2b21048eb3ff3dc8be2aba8b6ce2c03a9868ece..d8e952f15758132a581024beaa56d4c724855cba 100644 --- a/arkindex/training/api.py +++ b/arkindex/training/api.py @@ -22,6 +22,7 @@ from rest_framework.generics import ( from rest_framework.response import Response from arkindex.documents.models import Corpus, Element +from arkindex.process.models import ProcessDatasetSet from arkindex.project.mixins import ACLMixin, CorpusACLMixin, TrainingModelMixin from arkindex.project.pagination import CountCursorPagination from arkindex.project.permissions import IsVerified, IsVerifiedOrReadOnly @@ -683,6 +684,8 @@ class CorpusDataset(CorpusACLMixin, ListCreateAPIView): """ Delete a dataset. Only datasets in an `open` state can be deleted. + A dataset cannot be deleted if one or more of its sets is selected in a process. + Requires an **admin** access to the dataset's corpus. """ ) @@ -708,13 +711,14 @@ class DatasetUpdate(ACLMixin, RetrieveUpdateDestroyAPIView): if request.method in permissions.SAFE_METHODS: return - role = Role.Contributor - if request.method == "DELETE": - role = Role.Admin - if obj.state != DatasetState.Open: - raise PermissionDenied(detail="Only datasets in an open state can be deleted.") + role = Role.Admin if request.method == "DELETE" else Role.Contributor if not self.has_access(obj.corpus, role.value): raise PermissionDenied(detail=f"You do not have {str(role).lower()} access to corpus {obj.corpus.name}.") + if self.request.method == "DELETE": + if obj.state != DatasetState.Open: + raise ValidationError(detail="Only datasets in an open state can be deleted.") + if ProcessDatasetSet.objects.filter(set__in=obj.sets.all()).exists(): + raise ValidationError(detail="This dataset cannot be deleted because at least one of its sets is used in a process.") # Prevent editing anything on a complete dataset if obj.state == DatasetState.Complete: diff --git a/arkindex/training/tests/test_datasets_api.py b/arkindex/training/tests/test_datasets_api.py index b928b865134741cb1605e42220521f933d4ad893..16f3cfa09915f57a068ecac22060b566ef5ceb1c 100644 --- a/arkindex/training/tests/test_datasets_api.py +++ b/arkindex/training/tests/test_datasets_api.py @@ -1062,9 +1062,21 @@ class TestDatasetsAPI(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.assertDictEqual(response.json(), {"detail": "Not found."}) + def test_delete_dataset_in_process_forbidden(self): + self.client.force_login(self.user) + with self.assertNumQueries(4): + response = self.client.delete( + reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}), + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json(), ["This dataset cannot be deleted because at least one of its sets is used in a process."]) + self.dataset.refresh_from_db() + def test_delete(self): self.client.force_login(self.user) - with self.assertNumQueries(8): + # Remove dataset sets from process + ProcessDatasetSet.objects.filter(process_id=self.process.id, set__dataset_id=self.dataset.id).delete() + with self.assertNumQueries(7): response = self.client.delete( reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}), ) @@ -1072,8 +1084,9 @@ class TestDatasetsAPI(FixtureAPITestCase): with self.assertRaises(Dataset.DoesNotExist): self.dataset.refresh_from_db() - def test_delete_not_open(self): - self.client.force_login(self.user) + @patch("arkindex.project.mixins.has_access", return_value=True) + def test_delete_not_open(self, has_access_mock): + self.client.force_login(self.write_user) cases = [DatasetState.Building, DatasetState.Complete, DatasetState.Error] for state in cases: with self.subTest(state=state): @@ -1083,8 +1096,11 @@ class TestDatasetsAPI(FixtureAPITestCase): response = self.client.delete( reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}), ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertDictEqual(response.json(), {"detail": "Only datasets in an open state can be deleted."}) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json(), ["Only datasets in an open state can be deleted."]) + self.assertEqual(has_access_mock.call_count, 1) + self.assertEqual(has_access_mock.call_args, call(self.write_user, self.corpus, Role.Admin.value, skip_public=False)) + has_access_mock.reset_mock() def test_delete_elements(self): """ @@ -1096,9 +1112,11 @@ class TestDatasetsAPI(FixtureAPITestCase): train_set.set_elements.create(element_id=self.page1.id, set="training") validation_set.set_elements.create(element_id=self.page2.id, set="validation") validation_set.set_elements.create(element_id=self.page3.id, set="validation") + # Remove dataset sets from process + ProcessDatasetSet.objects.filter(process_id=self.process.id, set__dataset_id=self.dataset.id).delete() self.client.force_login(self.user) - with self.assertNumQueries(8): + with self.assertNumQueries(7): response = self.client.delete( reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}), )