diff --git a/arkindex/training/api.py b/arkindex/training/api.py index 69dbd8bf7a6dd3f449f3e2702055891298b50b93..66b6d00a377014677c08e54e7a3e8dcb5df39336 100644 --- a/arkindex/training/api.py +++ b/arkindex/training/api.py @@ -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. """ ) ) diff --git a/arkindex/training/tests/test_dataset_sets_api.py b/arkindex/training/tests/test_dataset_sets_api.py index a02279793c6003cda5bdce604a5c1ae4b17ce036..17f43037e47afdbca6fa59290033fdda5ab7071a 100644 --- a/arkindex/training/tests/test_dataset_sets_api.py +++ b/arkindex/training/tests/test_dataset_sets_api.py @@ -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="第3新æ±äº¬å¸‚", 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()