From 797618d4a4e453fe32dab6cc893a22d242276dc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Mon, 15 Apr 2024 12:38:56 -0500 Subject: [PATCH] feat: Update ObjectTagView (#180) * feat: Update ObjectTagView to allow tag_bject with multiple taxonomies in once * chore: Bump version --- openedx_learning/__init__.py | 2 +- .../core/tagging/rest_api/v1/serializers.py | 17 ++- .../core/tagging/rest_api/v1/views.py | 93 ++++++++------ .../core/tagging/test_models.py | 1 + .../core/tagging/test_views.py | 116 ++++++++++++++++-- 5 files changed, 170 insertions(+), 59 deletions(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index dfbcb853..59c26bc9 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.9.0" +__version__ = "0.9.1" diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 354bb434..6eb1f021 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -191,22 +191,21 @@ def to_representation(self, instance: list[ObjectTag]) -> dict: return by_object -class ObjectTagUpdateBodySerializer(serializers.Serializer): # pylint: disable=abstract-method +class ObjectTagUpdateByTaxonomySerializer(serializers.Serializer): # pylint: disable=abstract-method """ - Serializer of the body for the ObjectTag UPDATE view + Serializer of a taxonomy item of ObjectTag UPDATE view. """ - + taxonomy = serializers.PrimaryKeyRelatedField( + queryset=Taxonomy.objects.all(), required=True + ) tags = serializers.ListField(child=serializers.CharField(), required=True) -class ObjectTagUpdateQueryParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method +class ObjectTagUpdateBodySerializer(serializers.Serializer): # pylint: disable=abstract-method """ - Serializer of the query params for the ObjectTag UPDATE view + Serializer of the body for the ObjectTag UPDATE view """ - - taxonomy = serializers.PrimaryKeyRelatedField( - queryset=Taxonomy.objects.all(), required=True - ) + tagsData = serializers.ListField(child=ObjectTagUpdateByTaxonomySerializer(), required=True) class TagDataSerializer(UserPermissionsSerializerMixin, serializers.Serializer): # pylint: disable=abstract-method diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 4d398f52..1bd1735a 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -40,7 +40,6 @@ ObjectTagsByTaxonomySerializer, ObjectTagSerializer, ObjectTagUpdateBodySerializer, - ObjectTagUpdateQueryParamsSerializer, TagDataSerializer, TaxonomyExportQueryParamsSerializer, TaxonomyImportBodySerializer, @@ -501,8 +500,8 @@ def update(self, request, *args, **kwargs) -> Response: Update ObjectTags that belong to a given object_id Pass a list of Tag ids or Tag values to be applied to an object id in the - body `tag` parameter. Passing an empty list will remove all tags from - the object id. + body `tag` parameter, by each taxonomy. Passing an empty list will remove all tags from + the object id on an specific taxonomy. **Example Body Requests** @@ -511,54 +510,76 @@ def update(self, request, *args, **kwargs) -> Response: **Example Body Requests** ```json { - "tags": [1, 2, 3] + "tagsData": [ + { + "taxonomy": 1, + "tags": [1, 2, 3] + }, + { + "taxonomy": 1, + "tags": [1, 2, 3] + } + ], }, { - "tags": ["Tag 1", "Tag 2"] + "tagsData": [ + { + "taxonomy": 1, + "tags": ["Tag 1", "Tag 2"] + }, + ] }, { - "tags": [] + "tagsData": [ + { + "taxonomy": 1, + "tags": [] + }, + ] } """ - partial = kwargs.pop('partial', False) if partial: raise MethodNotAllowed("PATCH", detail="PATCH not allowed") - query_params = ObjectTagUpdateQueryParamsSerializer( - data=request.query_params.dict() - ) - query_params.is_valid(raise_exception=True) - taxonomy = query_params.validated_data.get("taxonomy", None) - taxonomy = taxonomy.cast() - + object_id = kwargs.pop('object_id') perm = "oel_tagging.can_tag_object" + body = ObjectTagUpdateBodySerializer(data=request.data) + body.is_valid(raise_exception=True) - object_id = kwargs.pop('object_id') - perm_obj = ObjectTagPermissionItem( - taxonomy=taxonomy, - object_id=object_id, - ) + data = body.validated_data.get("tagsData", []) - if not request.user.has_perm( - perm, - # The obj arg expects a model, but we are passing an object - perm_obj, # type: ignore[arg-type] - ): - raise PermissionDenied( - "You do not have permission to change object tags for this taxonomy or object_id." - ) + if not data: + return self.retrieve(request, object_id) - body = ObjectTagUpdateBodySerializer(data=request.data) - body.is_valid(raise_exception=True) + # Check permissions + for tagsData in data: + taxonomy = tagsData.get("taxonomy") - tags = body.data.get("tags", []) - try: - tag_object(object_id, taxonomy, tags) - except TagDoesNotExist as e: - raise ValidationError from e - except ValueError as e: - raise ValidationError from e + perm_obj = ObjectTagPermissionItem( + taxonomy=taxonomy, + object_id=object_id, + ) + if not request.user.has_perm( + perm, + # The obj arg expects a model, but we are passing an object + perm_obj, # type: ignore[arg-type] + ): + raise PermissionDenied(f""" + You do not have permission to change object tags + for Taxonomy: {str(taxonomy)} or Object: {object_id}. + """) + + # Tag object_id per taxonomy + for tagsData in data: + taxonomy = tagsData.get("taxonomy") + tags = tagsData.get("tags", []) + try: + tag_object(object_id, taxonomy, tags) + except TagDoesNotExist as e: + raise ValidationError from e + except ValueError as e: + raise ValidationError from e return self.retrieve(request, object_id) diff --git a/tests/openedx_tagging/core/tagging/test_models.py b/tests/openedx_tagging/core/tagging/test_models.py index f17f56ce..9dbcdbde 100644 --- a/tests/openedx_tagging/core/tagging/test_models.py +++ b/tests/openedx_tagging/core/tagging/test_models.py @@ -40,6 +40,7 @@ def setUp(self): self.language_taxonomy = LanguageTaxonomy.objects.get(name="Languages") self.user_taxonomy = Taxonomy.objects.get(name="User Authors").cast() self.free_text_taxonomy = api.create_taxonomy(name="Free Text", allow_free_text=True) + self.import_taxonomy = Taxonomy.objects.get(name="Import Taxonomy Test") # References to some tags: self.archaea = get_tag("Archaea") diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 682535b4..d470fd34 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -38,7 +38,7 @@ OBJECT_TAGS_RETRIEVE_URL = "/tagging/rest_api/v1/object_tags/{object_id}/" OBJECT_TAG_COUNTS_URL = "/tagging/rest_api/v1/object_tag_counts/{object_id_pattern}/" -OBJECT_TAGS_UPDATE_URL = "/tagging/rest_api/v1/object_tags/{object_id}/?taxonomy={taxonomy_id}" +OBJECT_TAGS_UPDATE_URL = "/tagging/rest_api/v1/object_tags/{object_id}/" LANGUAGE_TAXONOMY_ID = -1 @@ -1049,15 +1049,45 @@ def test_tag_object(self, user_attr, taxonomy_attr, taxonomy_flags, tag_values, object_id = "abc" - url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id, taxonomy_id=taxonomy.pk) + url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id) - response = self.client.put(url, {"tags": tag_values}, format="json") + data = [{ + "taxonomy": taxonomy.pk, + "tags": tag_values, + }] + + response = self.client.put(url, {"tagsData": data}, format="json") assert response.status_code == expected_status if status.is_success(expected_status): assert [t["value"] for t in response.data[object_id]["taxonomies"][0]["tags"]] == tag_values # And retrieving the object tags again should return an identical response: assert response.data == self.client.get(OBJECT_TAGS_RETRIEVE_URL.format(object_id=object_id)).data + def test_tag_object_multiple_taxonomy(self): + self.client.force_authenticate(user=self.staff) + + object_id = "abc" + url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id) + tag_value_1 = ["Tag 4"] + tag_value_2 = ["Mammalia", "Fungi"] + data = [ + { + "taxonomy": self.import_taxonomy.pk, + "tags": tag_value_1, + }, + { + "taxonomy": self.taxonomy.pk, + "tags": tag_value_2, + }, + ] + + response = self.client.put(url, {"tagsData": data}, format="json") + assert response.status_code == status.HTTP_200_OK + assert [t["value"] for t in response.data[object_id]["taxonomies"][0]["tags"]] == tag_value_1 + assert [t["value"] for t in response.data[object_id]["taxonomies"][1]["tags"]] == tag_value_2 + # And retrieving the object tags again should return an identical response: + assert response.data == self.client.get(OBJECT_TAGS_RETRIEVE_URL.format(object_id=object_id)).data + @ddt.data( # Users and staff can clear tags (None, {}, status.HTTP_401_UNAUTHORIZED), @@ -1089,9 +1119,14 @@ def test_tag_object_clear(self, user_attr, taxonomy_flags, expected_status): setattr(self.taxonomy, k, v) self.taxonomy.save() - url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id, taxonomy_id=self.taxonomy.pk) + url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id) - response = self.client.put(url, {"tags": []}, format="json") + data = [{ + "taxonomy": self.taxonomy.pk, + "tags": [], + }] + + response = self.client.put(url, {"tagsData": data}, format="json") assert response.status_code == expected_status if status.is_success(expected_status): # Now there are no tags applied: @@ -1103,6 +1138,47 @@ def test_tag_object_clear(self, user_attr, taxonomy_flags, expected_status): self.taxonomy.save() assert [ot.value for ot in api.get_object_tags(object_id=object_id)] == ["Fungi"] + def test_tag_object_clear_multiple_taxonomy(self): + object_id = "abc" + self.client.force_authenticate(user=self.staff) + api.tag_object(object_id=object_id, taxonomy=self.taxonomy, tags=["Mammalia", "Fungi"]) + api.tag_object(object_id=object_id, taxonomy=self.import_taxonomy, tags=["Tag 4"]) + + url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id) + data = [ + { + "taxonomy": self.import_taxonomy.pk, + "tags": [], + }, + { + "taxonomy": self.taxonomy.pk, + "tags": [], + }, + ] + + response = self.client.put(url, {"tagsData": data}, format="json") + assert response.status_code == status.HTTP_200_OK + assert response.data[object_id]["taxonomies"] == [] + + def test_tag_object_clear_simple_taxonomy(self): + object_id = "abc" + self.client.force_authenticate(user=self.staff) + tag_values = ["Mammalia", "Fungi"] + api.tag_object(object_id=object_id, taxonomy=self.taxonomy, tags=tag_values) + api.tag_object(object_id=object_id, taxonomy=self.import_taxonomy, tags=["Tag 4"]) + + url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id) + data = [ + { + "taxonomy": self.import_taxonomy.pk, + "tags": [], + }, + ] + + response = self.client.put(url, {"tagsData": data}, format="json") + assert response.status_code == status.HTTP_200_OK + assert [t["value"] for t in response.data[object_id]["taxonomies"][0]["tags"]] == tag_values + @ddt.data( (None, status.HTTP_401_UNAUTHORIZED), ("user_1", status.HTTP_403_FORBIDDEN), @@ -1114,9 +1190,14 @@ def test_tag_object_without_permission(self, user_attr, expected_status): user = getattr(self, user_attr) self.client.force_authenticate(user=user) - url = OBJECT_TAGS_UPDATE_URL.format(object_id="view_only", taxonomy_id=self.taxonomy.pk) + url = OBJECT_TAGS_UPDATE_URL.format(object_id="view_only") + + data = [{ + "taxonomy": self.taxonomy.pk, + "tags": ["Tag 1"], + }] - response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") + response = self.client.put(url, {"tagsData": data}, format="json") assert response.status_code == expected_status assert not status.is_success(expected_status) # No success cases here @@ -1127,22 +1208,31 @@ def test_tag_object_count_limit(self): object_id = "limit_tag_count" dummy_taxonomies = self.create_100_taxonomies() - url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id, taxonomy_id=self.taxonomy.pk) + url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id) self.client.force_authenticate(user=self.staff) - response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") + response = self.client.put(url, {"tagsData": [{ + "taxonomy": self.taxonomy.pk, + "tags": ["Tag 1"], + }]}, format="json") # Can't add another tag because the object already has 100 tags assert response.status_code == status.HTTP_400_BAD_REQUEST # The user can edit the tags that are already on the object for taxonomy in dummy_taxonomies: - url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id, taxonomy_id=taxonomy.pk) - response = self.client.put(url, {"tags": ["New Tag"]}, format="json") + url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id) + response = self.client.put(url, {"tagsData": [{ + "taxonomy": taxonomy.pk, + "tags": ["New Tag"], + }]}, format="json") assert response.status_code == status.HTTP_200_OK # Editing tags adding another one will fail for taxonomy in dummy_taxonomies: - url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id, taxonomy_id=taxonomy.pk) - response = self.client.put(url, {"tags": ["New Tag 1", "New Tag 2"]}, format="json") + url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id) + response = self.client.put(url, {"tagsData": [{ + "taxonomy": taxonomy.pk, + "tags": ["New Tag 1", "New Tag 2"], + }]}, format="json") assert response.status_code == status.HTTP_400_BAD_REQUEST