From 15f903c39afad8426f941865bb8521522b6c67f9 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Fri, 20 Oct 2023 09:52:58 +0300 Subject: [PATCH] refactor: Move validation serializer -> model method In addition to other PR requested changes: * Remove `PUT` method from docs string to avoid confusion with future changes * Replace `pk` with `id` in doc string to make it more RESTful --- openedx_tagging/core/tagging/models/base.py | 21 ++++++++-- .../core/tagging/rest_api/v1/serializers.py | 42 ++----------------- .../core/tagging/rest_api/v1/views.py | 23 ++++------ 3 files changed, 31 insertions(+), 55 deletions(-) diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py index ebbe5b68..a101435a 100644 --- a/openedx_tagging/core/tagging/models/base.py +++ b/openedx_tagging/core/tagging/models/base.py @@ -370,6 +370,12 @@ def add_tag( parent = None if parent_tag_value: + # Check if parent tag is valid + if not Tag.objects.filter(value__iexact=parent_tag_value).exists(): + raise ValueError("Invalid `parent_tag_value` provided") + + # Get parent tag from taxonomy, raises Tag.DoesNotExist if doesn't + # belong to taxonomy parent = self.tag_set.get(value__iexact=parent_tag_value) tag = Tag.objects.create( @@ -395,7 +401,12 @@ def update_tag(self, tag: str, new_value: str) -> Tag: "update_tag() doesn't work for system defined taxonomies. They cannot be modified." ) - # Update Tag instance with new value + # Check if tag is valid + if not Tag.objects.filter(value__iexact=tag).exists(): + raise ValueError("Invalid `tag` provided") + + # Update Tag instance with new value, raises Tag.DoesNotExist if + # tag doesn't belong to taxonomy tag_to_update = self.tag_set.get(value__iexact=tag) tag_to_update.value = new_value tag_to_update.save() @@ -419,11 +430,15 @@ def delete_tags(self, tags: List[str], with_subtags: bool = False): "delete_tags() doesn't work for system defined taxonomies. They cannot be modified." ) + # Check if tags provided are valid + if not Tag.objects.filter(value__in=tags).count() == len(tags): + raise ValueError("One or more tag in `tags` is invalid") + tags_to_delete = self.tag_set.filter(value__in=tags) if tags_to_delete.count() != len(tags): - # If they do not match that means there is a Tag ID in the provided - # list that is either invalid or does not belong to this Taxonomy + # If they do not match that means there is one or more Tag ID(s) + # provided that do not belong to this Taxonomy raise ValueError("Invalid tag id provided or tag id does not belong to taxonomy") # Check if any Tag contains subtags (children) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index bceae7b7..e15e65b8 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -197,63 +197,29 @@ def get_children_count(self, obj): class TaxonomyTagCreateBodySerializer(serializers.Serializer): # pylint: disable=abstract-method """ - Serializer of the body for the Taxonomy Tags CREATE view + Serializer of the body for the Taxonomy Tags CREATE request """ tag = serializers.CharField(required=True) - parent_tag_value = serializers.CharField( - source='parent.value', required=False - ) + parent_tag_value = serializers.CharField(required=False) external_id = serializers.CharField(required=False) - def validate_parent_tag_value(self, value): - """ - Check that the provided parent Tag exists based on the value - """ - valid = Tag.objects.filter(value__iexact=value).exists() - if not valid: - raise serializers.ValidationError("Invalid `parent_tag_value` provided") - - return value - class TaxonomyTagUpdateBodySerializer(serializers.Serializer): # pylint: disable=abstract-method """ - Serializer of the body for the Taxonomy Tags UPDATE view + Serializer of the body for the Taxonomy Tags UPDATE request """ tag = serializers.CharField(source="value", required=True) updated_tag_value = serializers.CharField(required=True) - def validate_tag(self, value): - """ - Check that the provided Tag exists based on the value - """ - - valid = Tag.objects.filter(value__iexact=value).exists() - if not valid: - raise serializers.ValidationError("Invalid `tag` provided") - - return value - class TaxonomyTagDeleteBodySerializer(serializers.Serializer): # pylint: disable=abstract-method """ - Serializer of the body for the Taxonomy Tags DELETE view + Serializer of the body for the Taxonomy Tags DELETE request """ tags = serializers.ListField( child=serializers.CharField(), required=True ) with_subtags = serializers.BooleanField(required=False) - - def validate_tags(self, value): - """ - Check that the provided Tags exists based on the values - """ - - valid = Tag.objects.filter(value__in=value).count() == len(value) - if not valid: - raise serializers.ValidationError("One or more tag in `tags` is invalid") - - return value diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index abeb4152..6428043c 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -416,14 +416,14 @@ class TaxonomyTagsView(ListAPIView, RetrieveUpdateDestroyAPIView): View to list/create/update/delete tags of a taxonomy. **List Query Parameters** - * pk (required) - The pk of the taxonomy to retrieve tags. + * id (required) - The ID of the taxonomy to retrieve tags. * parent_tag_id (optional) - Id of the tag to retrieve children tags. * page (optional) - Page number (default: 1) * page_size (optional) - Number of items per page (default: 10) **List Example Requests** - GET api/tagging/v1/taxonomy/:pk/tags - Get tags of taxonomy - GET api/tagging/v1/taxonomy/:pk/tags?parent_tag_id=30 - Get children tags of tag + GET api/tagging/v1/taxonomy/:id/tags - Get tags of taxonomy + GET api/tagging/v1/taxonomy/:id/tags?parent_tag_id=30 - Get children tags of tag **List Query Returns** * 200 - Success @@ -432,7 +432,7 @@ class TaxonomyTagsView(ListAPIView, RetrieveUpdateDestroyAPIView): * 404 - Taxonomy not found **Create Query Parameters** - * pk (required) - The pk of the taxonomy to create a Tag for + * id (required) - The ID of the taxonomy to create a Tag for **Create Request Body** * tag (required): The value of the Tag that should be added to @@ -442,7 +442,7 @@ class TaxonomyTagsView(ListAPIView, RetrieveUpdateDestroyAPIView): * extenal_id (optional): The external id for the new Tag **Create Example Requests** - POST api/tagging/v1/taxonomy/:pk/tags - Create a Tag in taxonomy + POST api/tagging/v1/taxonomy/:id/tags - Create a Tag in taxonomy { "value": "New Tag", "parent_tag_value": "Parent Tag" @@ -456,19 +456,14 @@ class TaxonomyTagsView(ListAPIView, RetrieveUpdateDestroyAPIView): * 404 - Taxonomy not found **Update Query Parameters** - * pk (required) - The pk of the taxonomy to update a Tag in + * id (required) - The ID of the taxonomy to update a Tag in **Update Request Body** * tag (required): The value (identifier) of the Tag to be updated * updated_tag_value (required): The updated value of the Tag **Update Example Requests** - PUT api/tagging/v1/taxonomy/:pk/tags - Update a Tag in Taxonomy - { - "tag": "Tag 1", - "updated_tag_value": "Updated Tag Value" - } - PATCH api/tagging/v1/taxonomy/:pk/tags - Update a Tag in Taxonomy + PATCH api/tagging/v1/taxonomy/:id/tags - Update a Tag in Taxonomy { "tag": "Tag 1", "updated_tag_value": "Updated Tag Value" @@ -481,7 +476,7 @@ class TaxonomyTagsView(ListAPIView, RetrieveUpdateDestroyAPIView): * 404 - Taxonomy, Tag or Parent Tag not found **Delete Query Parameters** - * pk (required) - The pk of the taxonomy to Delete Tag(s) in + * id (required) - The ID of the taxonomy to Delete Tag(s) in **Delete Request Body** * tags (required): The values (identifiers) of Tags that should be @@ -491,7 +486,7 @@ class TaxonomyTagsView(ListAPIView, RetrieveUpdateDestroyAPIView): set to `True`. Defaults to `False`. **Delete Example Requests** - DELETE api/tagging/v1/taxonomy/:pk/tags - Delete Tag(s) in Taxonomy + DELETE api/tagging/v1/taxonomy/:id/tags - Delete Tag(s) in Taxonomy { "tags": ["Tag 1", "Tag 2", "Tag 3"], "with_subtags": True