Skip to content
Snippets Groups Projects
Commit de12b60a authored by ml bonhomme's avatar ml bonhomme :bee: Committed by Bastien Abadie
Browse files

Apply review + require only Contributor access for non-delete endpoints +... 

Apply review + require only Contributor access for non-delete endpoints + update Dataset endpoints documentation
parent 598da8ca
No related branches found
No related tags found
1 merge request!2267Dataset sets management API endpoints
This commit is part of merge request !2267. Comments created here will be created in the context of that merge request.
Hide whitespace changes
Inline Side-by-side
arkindex/training/api.py
+ 6
− 14
View file @ de12b60a
......@@ -657,11 +657,6 @@ class CorpusDataset(CorpusACLMixin, ListCreateAPIView):
"""
Update a dataset. The dataset must not be in `complete` state.
The sets array can only be updated following those cases:
* Adding or removing sets, nothing specific is done.
* Updating a single set within the array,
all elements linked to the previous set will be moved to the new one.
Requires a **contributor** access to the dataset's corpus.
"""
),
......@@ -671,11 +666,6 @@ class CorpusDataset(CorpusACLMixin, ListCreateAPIView):
"""
Partially update a dataset. The dataset must not be in `complete` state.
The sets array can only be updated following those cases:
* Adding or removing sets, nothing specific is done.
* Updating a single set within the array,
all elements linked to the previous set will be moved to the new one.
Requires a **contributor** access to the dataset's corpus.
"""
)
......@@ -761,8 +751,10 @@ class DatasetSetBase():
.select_related("corpus"),
pk=dataset_id
)
if dataset.corpus not in Corpus.objects.admin(self.request.user):
if self.request.method == "DELETE" and not Corpus.objects.admin(self.request.user).filter(pk=dataset.corpus_id).exists():
raise PermissionDenied(detail="You do not have admin access to this dataset.")
elif self.request.method != "DELETE" and not Corpus.objects.writable(self.request.user).filter(pk=dataset.corpus_id).exists():
raise PermissionDenied(detail="You do not have contributor access to this dataset.")
if dataset.state != DatasetState.Open:
raise ValidationError(detail="You can only add or update sets from a dataset in an open state.")
return dataset
......@@ -784,7 +776,7 @@ class DatasetSetBase():
"""
Sets can only be created in **open** datasets.
Requires **admin** access to the dataset's corpus.
Requires **contributor** access to the dataset's corpus.
"""
)
),
......@@ -814,7 +806,7 @@ class DatasetSetCreate(DatasetSetBase, CreateAPIView):
"""
Sets can only be updated if the dataset is in the **open** state.
Requires **admin** access to the dataset's corpus.
Requires **contributor** access to the dataset's corpus.
"""
)
),
......@@ -824,7 +816,7 @@ class DatasetSetCreate(DatasetSetBase, CreateAPIView):
"""
Sets can only be updated if the dataset is in the **open** state.
Requires **admin** access to the dataset's corpus.
Requires **contributor** access to the dataset's corpus.
"""
)
)
......
arkindex/training/tests/test_dataset_sets_api.py
+ 58
− 155
View file @ de12b60a
......@@ -85,32 +85,27 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertEqual(filter_rights_mock.call_args, call(self.user, Corpus, Role.Guest.value))
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_create_requires_admin_access(self, filter_rights_mock):
for role in [Role.Guest, Role.Contributor]:
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.subTest(role=role):
self.private_corpus.memberships.update_or_create(user=self.user, defaults={"level": role.value})
self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.post(
reverse("api:dataset-sets", kwargs={"pk": self.private_dataset.pk}),
data={"name": "Unit-00"},
format="json"
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
def test_create_requires_contributor_access(self, filter_rights_mock):
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
self.private_corpus.memberships.update_or_create(user=self.user, defaults={"level": Role.Guest.value})
self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.post(
reverse("api:dataset-sets", kwargs={"pk": self.private_dataset.pk}),
data={"name": "Unit-00"},
format="json"
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertDictEqual(response.json(), {"detail": "You do not have admin access to this dataset."})
self.assertDictEqual(response.json(), {"detail": "You do not have contributor access to this dataset."})
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Contributor.value)])
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_create_requires_open_dataset(self, filter_rights_mock):
def test_create_requires_open_dataset(self):
self.client.force_login(self.user)
for state in set(DatasetState) - {DatasetState.Open}:
filter_rights_mock.side_effect = [Corpus.objects.filter(pk=self.corpus.pk), Corpus.objects.filter(pk=self.corpus.pk)]
with self.subTest(state=state):
self.dataset.state = state
self.dataset.save()
......@@ -125,16 +120,9 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertEqual(response.json(), ["You can only add or update sets from a dataset in an open state."])
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_create_duplicate_set_name(self, filter_rights_mock):
def test_create_duplicate_set_name(self):
self.client.force_login(self.user)
filter_rights_mock.side_effect = [Corpus.objects.filter(pk=self.corpus.pk), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(5):
response = self.client.post(
reverse("api:dataset-sets", kwargs={"pk": self.dataset.pk}),
......@@ -145,16 +133,9 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertDictEqual(response.json(), {"name": ["A set with this name already exists in this dataset."]})
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_create(self, filter_rights_mock):
def test_create(self):
self.client.force_login(self.user)
filter_rights_mock.side_effect = [Corpus.objects.filter(pk=self.corpus.pk), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(6):
response = self.client.post(
reverse("api:dataset-sets", kwargs={"pk": self.dataset.pk}),
......@@ -169,10 +150,6 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
"name": "Unit-00"
})
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
# UpdateDatasetSet
def test_update_requires_login(self):
......@@ -236,32 +213,27 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertEqual(filter_rights_mock.call_args, call(self.user, Corpus, Role.Guest.value))
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_update_requires_admin_access(self, filter_rights_mock):
for role in [Role.Guest, Role.Contributor]:
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.private_corpus.pk)]
with self.subTest(role=role):
self.private_corpus.memberships.update_or_create(user=self.user, defaults={"level": role.value})
self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.put(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
data={"name": "Unit-00"},
format="json"
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
def test_update_requires_contributor_access(self, filter_rights_mock):
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.private_corpus.pk)]
self.private_corpus.memberships.update_or_create(user=self.user, defaults={"level": Role.Guest.value})
self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.put(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
data={"name": "Unit-00"},
format="json"
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertDictEqual(response.json(), {"detail": "You do not have admin access to this dataset."})
self.assertDictEqual(response.json(), {"detail": "You do not have contributor access to this dataset."})
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Contributor.value)])
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_update_requires_open_dataset(self, filter_rights_mock):
def test_update_requires_open_dataset(self):
self.client.force_login(self.user)
for state in set(DatasetState) - {DatasetState.Open}:
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.subTest(state=state):
self.dataset.state = state
self.dataset.save()
......@@ -276,16 +248,9 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertEqual(response.json(), ["You can only add or update sets from a dataset in an open state."])
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_update_duplicate_set_name(self, filter_rights_mock):
def test_update_duplicate_set_name(self):
self.client.force_login(self.user)
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(6):
response = self.client.put(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
......@@ -296,16 +261,9 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertDictEqual(response.json(), {"name": ["A set with this name already exists in this dataset."]})
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_update(self, filter_rights_mock):
def test_update(self):
self.client.force_login(self.user)
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(7):
response = self.client.put(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
......@@ -321,10 +279,6 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.train_set.refresh_from_db()
self.assertEqual(self.train_set.name, "Unit-00")
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
# PartialUpdateDatasetSet
def test_partial_update_requires_login(self):
......@@ -388,32 +342,27 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertEqual(filter_rights_mock.call_args, call(self.user, Corpus, Role.Guest.value))
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_partial_update_requires_admin_access(self, filter_rights_mock):
for role in [Role.Guest, Role.Contributor]:
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.private_corpus.pk)]
with self.subTest(role=role):
self.private_corpus.memberships.update_or_create(user=self.user, defaults={"level": role.value})
self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.patch(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
data={"name": "Unit-00"},
format="json"
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
def test_partial_update_requires_contributor_access(self, filter_rights_mock):
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.private_corpus.pk)]
self.private_corpus.memberships.update_or_create(user=self.user, defaults={"level": Role.Guest.value})
self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.patch(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
data={"name": "Unit-00"},
format="json"
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertDictEqual(response.json(), {"detail": "You do not have admin access to this dataset."})
self.assertDictEqual(response.json(), {"detail": "You do not have contributor access to this dataset."})
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Contributor.value)])
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_partial_update_requires_open_dataset(self, filter_rights_mock):
def test_partial_update_requires_open_dataset(self):
self.client.force_login(self.user)
for state in set(DatasetState) - {DatasetState.Open}:
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.subTest(state=state):
self.dataset.state = state
self.dataset.save()
......@@ -428,16 +377,9 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertEqual(response.json(), ["You can only add or update sets from a dataset in an open state."])
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_partial_update_duplicate_set_name(self, filter_rights_mock):
def test_partial_update_duplicate_set_name(self):
self.client.force_login(self.user)
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(6):
response = self.client.patch(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
......@@ -448,16 +390,9 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertDictEqual(response.json(), {"name": ["A set with this name already exists in this dataset."]})
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_partial_update(self, filter_rights_mock):
def test_partial_update(self):
self.client.force_login(self.user)
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(7):
response = self.client.patch(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
......@@ -473,10 +408,6 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.train_set.refresh_from_db()
self.assertEqual(self.train_set.name, "Unit-00")
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
# DestroyDatasetSet
def test_destroy_requires_login(self):
......@@ -539,6 +470,7 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
for role in [Role.Guest, Role.Contributor]:
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.private_corpus.pk)]
with self.subTest(role=role):
filter_rights_mock.reset_mock()
self.private_corpus.memberships.update_or_create(user=self.user, defaults={"level": role.value})
self.client.force_login(self.user)
with self.assertNumQueries(5):
......@@ -550,16 +482,13 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertDictEqual(response.json(), {"detail": "You do not have admin access to this dataset."})
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_destroy_requires_open_dataset(self, filter_rights_mock):
def test_destroy_requires_open_dataset(self):
self.client.force_login(self.user)
for state in set(DatasetState) - {DatasetState.Open}:
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.subTest(state=state):
self.dataset.state = state
self.dataset.save()
......@@ -573,16 +502,9 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
self.assertEqual(response.json(), ["You can only add or update sets from a dataset in an open state."])
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_destroy_set_in_process_forbidden(self, filter_rights_mock):
def test_destroy_set_in_process_forbidden(self):
self.client.force_login(self.user)
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(7):
response = self.client.delete(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
......@@ -594,18 +516,11 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
# Check that self.train_set still exists
self.train_set.refresh_from_db()
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_destroy_only_set_forbidden(self, filter_rights_mock):
def test_destroy_only_set_forbidden(self):
self.client.force_login(self.user)
test_dataset = self.corpus.datasets.create(name="Tokyo 3", description="第３新東京市", creator=self.user)
test_set = test_dataset.sets.create(name="NERV HQ")
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(7):
response = self.client.delete(
reverse("api:dataset-set", kwargs={"dataset": test_dataset.pk, "set": test_set.pk}),
......@@ -617,18 +532,10 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
# Check that self.train_set still exists
test_set.refresh_from_db()
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
@patch("arkindex.users.managers.BaseACLManager.filter_rights")
def test_destroy(self, filter_rights_mock):
def test_destroy(self):
self.client.force_login(self.user)
# Remove train_set from dataset_process
self.dataset_process.process_sets.all().delete()
filter_rights_mock.side_effect = [Corpus.objects.all(), Corpus.objects.filter(pk=self.corpus.pk)]
with self.assertNumQueries(9):
response = self.client.delete(
reverse("api:dataset-set", kwargs={"dataset": self.dataset.pk, "set": self.train_set.pk}),
......@@ -638,7 +545,3 @@ class TestDatasetSetsAPI(FixtureAPITestCase):
with self.assertRaises(DatasetSet.DoesNotExist):
self.train_set.refresh_from_db()
self.assertEqual(filter_rights_mock.call_count, 2)
self.assertEqual(filter_rights_mock.call_args_list, [call(self.user, Corpus, Role.Guest.value), call(self.user, Corpus, Role.Admin.value)])
filter_rights_mock.reset_mock()
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment