Skip to content

Commit

Permalink
refactor: Move validation serializer -> model method
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@yusuf-musleh
yusuf-musleh committed Oct 20, 2023
1 parent 7126e46 commit b0e4b26
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 56 deletions.
21 changes: 18 additions & 3 deletions openedx_tagging/core/tagging/models/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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()
Expand All @@ -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)
Expand Down
44 changes: 5 additions & 39 deletions openedx_tagging/core/tagging/rest_api/v1/serializers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
tag = serializers.CharField(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
23 changes: 9 additions & 14 deletions openedx_tagging/core/tagging/rest_api/v1/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit b0e4b26

Please sign in to comment.