Skip to content
Snippets Groups Projects
Commit 2d71eae7 authored by ml bonhomme's avatar ml bonhomme :bee:
Browse files

Prevent dataset deletion if some sets are used in a process

parent 18e60ff5
No related branches found
No related tags found
No related merge requests found
This commit is part of merge request !2267. Comments created here will be created in the context of that merge request.
...@@ -22,6 +22,7 @@ from rest_framework.generics import ( ...@@ -22,6 +22,7 @@ from rest_framework.generics import (
from rest_framework.response import Response from rest_framework.response import Response
from arkindex.documents.models import Corpus, Element from arkindex.documents.models import Corpus, Element
from arkindex.process.models import ProcessDatasetSet
from arkindex.project.mixins import ACLMixin, CorpusACLMixin, TrainingModelMixin from arkindex.project.mixins import ACLMixin, CorpusACLMixin, TrainingModelMixin
from arkindex.project.pagination import CountCursorPagination from arkindex.project.pagination import CountCursorPagination
from arkindex.project.permissions import IsVerified, IsVerifiedOrReadOnly from arkindex.project.permissions import IsVerified, IsVerifiedOrReadOnly
...@@ -677,6 +678,8 @@ class CorpusDataset(CorpusACLMixin, ListCreateAPIView): ...@@ -677,6 +678,8 @@ class CorpusDataset(CorpusACLMixin, ListCreateAPIView):
""" """
Delete a dataset. Only datasets in an `open` state can be deleted. 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. Requires an **admin** access to the dataset's corpus.
""" """
) )
...@@ -696,13 +699,14 @@ class DatasetUpdate(ACLMixin, RetrieveUpdateDestroyAPIView): ...@@ -696,13 +699,14 @@ class DatasetUpdate(ACLMixin, RetrieveUpdateDestroyAPIView):
if request.method in permissions.SAFE_METHODS: if request.method in permissions.SAFE_METHODS:
return return
role = Role.Contributor role = Role.Admin if request.method == "DELETE" else 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.")
if not self.has_access(obj.corpus, role.value): 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}.") 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 # Prevent editing anything on a complete dataset
if obj.state == DatasetState.Complete: if obj.state == DatasetState.Complete:
......
...@@ -1062,9 +1062,21 @@ class TestDatasetsAPI(FixtureAPITestCase): ...@@ -1062,9 +1062,21 @@ class TestDatasetsAPI(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
self.assertDictEqual(response.json(), {"detail": "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): def test_delete(self):
self.client.force_login(self.user) 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( response = self.client.delete(
reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}), reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}),
) )
...@@ -1072,8 +1084,9 @@ class TestDatasetsAPI(FixtureAPITestCase): ...@@ -1072,8 +1084,9 @@ class TestDatasetsAPI(FixtureAPITestCase):
with self.assertRaises(Dataset.DoesNotExist): with self.assertRaises(Dataset.DoesNotExist):
self.dataset.refresh_from_db() self.dataset.refresh_from_db()
def test_delete_not_open(self): @patch("arkindex.project.mixins.has_access", return_value=True)
self.client.force_login(self.user) def test_delete_not_open(self, has_access_mock):
self.client.force_login(self.write_user)
cases = [DatasetState.Building, DatasetState.Complete, DatasetState.Error] cases = [DatasetState.Building, DatasetState.Complete, DatasetState.Error]
for state in cases: for state in cases:
with self.subTest(state=state): with self.subTest(state=state):
...@@ -1083,8 +1096,11 @@ class TestDatasetsAPI(FixtureAPITestCase): ...@@ -1083,8 +1096,11 @@ class TestDatasetsAPI(FixtureAPITestCase):
response = self.client.delete( response = self.client.delete(
reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}), reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}),
) )
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {"detail": "Only datasets in an open state can be deleted."}) 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): def test_delete_elements(self):
""" """
...@@ -1096,9 +1112,11 @@ class TestDatasetsAPI(FixtureAPITestCase): ...@@ -1096,9 +1112,11 @@ class TestDatasetsAPI(FixtureAPITestCase):
train_set.set_elements.create(element_id=self.page1.id, set="training") 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.page2.id, set="validation")
validation_set.set_elements.create(element_id=self.page3.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) self.client.force_login(self.user)
with self.assertNumQueries(8): with self.assertNumQueries(7):
response = self.client.delete( response = self.client.delete(
reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}), reverse("api:dataset-update", kwargs={"pk": self.dataset.pk}),
) )
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment