feat: add tag method

openedx_tagging/core/tagging/rest_api/v1/serializers.py

@@ -55,3 +55,24 @@ class Meta:
             "tag_ref",
             "is_valid",
         ]
+
+class ObjectTagUpdateItemSerializer(serializers.Serializer):
+    """
+    Serialize for a single ObjectTag item
+    """
+    id = serializers.IntegerField(required=False)
+    value = serializers.CharField(required=False)
+
+    def validate(self, attrs):
+        if attrs['id'] and attrs['value']:
+            raise serializers.ValidationError('A tag should be referenced by either id or value, not both')
+        if not (attrs['id'] or attrs['value']):
+            raise serializers.ValidationError('A tag should be referenced by either id or value')
+
+        return attrs
+
+class ObjectTagUpdateSerializer(serializers.ModelSerializer):
+    """
+    Serialize for a list of ObjectTag items
+    """
+    tags = ObjectTagUpdateItemSerializer(many=True)

openedx_tagging/core/tagging/rest_api/v1/views.py

@@ -2,23 +2,18 @@
 Tagging API Views
 """
 from django.http import Http404
-from rest_framework import status
-from rest_framework.decorators import action
-from rest_framework.viewsets import ModelViewSet, ReadOnlyModelViewSet
-from rest_framework.response import Response
-
-from ...api import (
-    create_taxonomy,
-    get_taxonomy,
-    get_taxonomies,
-    get_object_tags,
-)
-from .permissions import TaxonomyObjectPermissions, ObjectTagObjectPermissions
+from rest_framework import mixins
+from rest_framework.exceptions import MethodNotAllowed
+from rest_framework.viewsets import GenericViewSet, ModelViewSet
+
+from ...api import create_taxonomy, get_object_tags, get_taxonomies, get_taxonomy, tag_object
+from .permissions import ObjectTagObjectPermissions, TaxonomyObjectPermissions
 from .serializers import (
-    TaxonomyListQueryParamsSerializer,
-    TaxonomySerializer,
     ObjectTagListQueryParamsSerializer,
     ObjectTagSerializer,
+    ObjectTagUpdateSerializer,
+    TaxonomyListQueryParamsSerializer,
+    TaxonomySerializer,
 )
 
 
@@ -141,9 +136,7 @@ def get_queryset(self):
         If you want the disabled taxonomies, pass enabled=False.
         If you want the enabled taxonomies, pass enabled=True.
         """
-        query_params = TaxonomyListQueryParamsSerializer(
-            data=self.request.query_params.dict()
-        )
+        query_params = TaxonomyListQueryParamsSerializer(data=self.request.query_params.dict())
         query_params.is_valid(raise_exception=True)
         enabled = query_params.data.get("enabled", None)
 
@@ -156,7 +149,7 @@ def perform_create(self, serializer):
         serializer.instance = create_taxonomy(**serializer.validated_data)
 
 
-class ObjectTagView(ReadOnlyModelViewSet):
+class ObjectTagView(mixins.RetrieveModelMixin, mixins.UpdateModelMixin, mixins.ListModelMixin, GenericViewSet):
     """
     View to retrieve paginated ObjectTags for a provided Object ID (object_id).
 
@@ -184,6 +177,7 @@ class ObjectTagView(ReadOnlyModelViewSet):
         * 405 - Method not allowed
 
     **Update Query Returns**
+        ToDo: update docstring
         * 403 - Permission denied
         * 405 - Method not allowed
 
@@ -203,9 +197,7 @@ def get_queryset(self):
         If a taxonomy is passed in, object tags are limited to that taxonomy.
         """
         object_id = self.kwargs.get("object_id")
-        query_params = ObjectTagListQueryParamsSerializer(
-            data=self.request.query_params.dict()
-        )
+        query_params = ObjectTagListQueryParamsSerializer(data=self.request.query_params.dict())
         query_params.is_valid(raise_exception=True)
         taxonomy_id = query_params.data.get("taxonomy", None)
         return get_object_tags(object_id, taxonomy_id)
@@ -227,6 +219,19 @@ def retrieve(self, request, object_id=None):
         serializer = ObjectTagSerializer(paginated_object_tags, many=True)
         return self.get_paginated_response(serializer.data)
 
-    @action(detail=True, methods=["post"])
-    def tag_object(self, request, object_id=None):
-        return Response(status=status.HTTP_200_OK)
+    def update(self, request, object_id=None, partial=False):
+        if partial:
+            raise MethodNotAllowed("PATCH", detail="PATCH not allowed")
+
+        query_params = ObjectTagListQueryParamsSerializer(data=request.query_params.dict())
+        query_params.is_valid(raise_exception=True)
+        taxonomy = query_params.data.get("taxonomy", None)
+        request.user.has_perm("change_objecttag", taxonomy)
+
+        body = ObjectTagUpdateSerializer(data=request.data)
+        body.is_valid(raise_exception=True)
+
+        tags = body.data.get("tags", [])
+        tag_object(taxonomy, tags, object_id)
+
+        return self.retrieve(request, object_id)

openedx_tagging/core/tagging/rules.py

@@ -47,18 +47,19 @@ def can_change_tag(user: User, tag: Tag = None) -> bool:
 
 
 @rules.predicate
-def can_change_object_tag(user: User, object_tag: ObjectTag = None) -> bool:
+def can_change_object_tag(user: User, taxonomy: Taxonomy = None) -> bool:
     """
-    Taxonomy admins can create or modify object tags on enabled taxonomies.
+    Everyone can potentially create/edit object tags (taxonomy=None). The object permission must be checked
+    to determine if the user can create/edit a object_tag for a specific taxonomy.
+
+    Everyone can create or modify object tags on enabled taxonomies.
     """
-    taxonomy = (
-        object_tag.taxonomy.cast() if (object_tag and object_tag.taxonomy_id) else None
-    )
-    object_tag = taxonomy.object_tag_class.cast(object_tag) if taxonomy else object_tag
-    return is_taxonomy_admin(user) and (
-        not object_tag or not taxonomy or (taxonomy and taxonomy.enabled)
-    )
+    if not taxonomy:
+        return True
+
+    taxonomy = taxonomy.cast()
 
+    return taxonomy.enabled
 
 # Taxonomy
 rules.add_perm("oel_tagging.add_taxonomy", can_change_taxonomy)

tests/openedx_tagging/core/tagging/test_rules.py

@@ -165,13 +165,15 @@ def test_view_tag(self):
         "oel_tagging.change_objecttag",
     )
     def test_add_change_object_tag(self, perm):
-        """Taxonomy administrators can create/edit an ObjectTag with an enabled Taxonomy"""
+        """
+        Everyone can create/edit an ObjectTag with an enabled Taxonomy
+        """
         assert self.superuser.has_perm(perm)
         assert self.superuser.has_perm(perm, self.object_tag)
         assert self.staff.has_perm(perm)
         assert self.staff.has_perm(perm, self.object_tag)
-        assert not self.learner.has_perm(perm)
-        assert not self.learner.has_perm(perm, self.object_tag)
+        assert self.learner.has_perm(perm)
+        assert self.learner.has_perm(perm, self.object_tag)
 
     @ddt.data(
         "oel_tagging.add_objecttag",

tests/openedx_tagging/core/tagging/test_views.py

@@ -2,13 +2,14 @@
 Tests tagging rest api views
 """
 
+from urllib.parse import parse_qs, urlparse
+
 import ddt
 from django.contrib.auth import get_user_model
 from rest_framework import status
 from rest_framework.test import APITestCase
-from urllib.parse import urlparse, parse_qs
 
-from openedx_tagging.core.tagging.models import Taxonomy, ObjectTag, Tag
+from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy
 from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy
 
 User = get_user_model()
@@ -17,8 +18,8 @@
 TAXONOMY_DETAIL_URL = "/tagging/rest_api/v1/taxonomies/{pk}/"
 
 
-OBJECT_TAGS_RETRIEVE_URL = '/tagging/rest_api/v1/object_tags/{object_id}/'
-OBJECT_TAGS_TAG_URL = '/tagging/rest_api/v1/object_tags/{object_id}/tag_object/'
+OBJECT_TAGS_RETRIEVE_URL = "/tagging/rest_api/v1/object_tags/{object_id}/"
+OBJECT_TAGS_TAG_URL = "/tagging/rest_api/v1/object_tags/{object_id}/tag_object/"
 
 
 def check_taxonomy(
@@ -403,45 +404,24 @@ def setUp(self):
         )
 
         # System-defined language taxonomy with valid ObjectTag
-        self.system_taxonomy = SystemDefinedTaxonomy.objects.create(
-            name="System Taxonomy"
-        )
-        self.tag1 = Tag.objects.create(
-            taxonomy=self.system_taxonomy, value="Tag 1"
-        )
-        ObjectTag.objects.create(
-            object_id="abc",
-            taxonomy=self.system_taxonomy,
-            tag=self.tag1
-        )
+        self.system_taxonomy = SystemDefinedTaxonomy.objects.create(name="System Taxonomy")
+        self.tag1 = Tag.objects.create(taxonomy=self.system_taxonomy, value="Tag 1")
+        ObjectTag.objects.create(object_id="abc", taxonomy=self.system_taxonomy, tag=self.tag1)
 
         # Closed Taxonomies created by taxonomy admins, each with 20 ObjectTags
         self.enabled_taxonomy = Taxonomy.objects.create(name="Enabled Taxonomy")
         for i in range(20):
             # Valid ObjectTags
-            tag = Tag.objects.create(
-                taxonomy=self.enabled_taxonomy, value=f"Tag {i}"
-            )
-            ObjectTag.objects.create(
-                object_id="abc", taxonomy=self.enabled_taxonomy,
-                tag=tag, _value=tag.value
-            )
+            tag = Tag.objects.create(taxonomy=self.enabled_taxonomy, value=f"Tag {i}")
+            ObjectTag.objects.create(object_id="abc", taxonomy=self.enabled_taxonomy, tag=tag, _value=tag.value)
 
         # Free-Text Taxonomies created by taxonomy admins, each linked
         # to 200 ObjectTags
-        self.open_taxonomy_enabled = Taxonomy.objects.create(
-            name="Enabled Free-Text Taxonomy"
-        )
-        self.open_taxonomy_disabled = Taxonomy.objects.create(
-            name="Disabled Free-Text Taxonomy", enabled=False
-        )
+        self.open_taxonomy_enabled = Taxonomy.objects.create(name="Enabled Free-Text Taxonomy")
+        self.open_taxonomy_disabled = Taxonomy.objects.create(name="Disabled Free-Text Taxonomy", enabled=False)
         for i in range(200):
-            ObjectTag.objects.create(
-                object_id="abc", taxonomy=self.open_taxonomy_enabled, _value=f"Free Text {i}"
-            )
-            ObjectTag.objects.create(
-                object_id="abc", taxonomy=self.open_taxonomy_disabled, _value=f"Free Text {i}"
-            )
+            ObjectTag.objects.create(object_id="abc", taxonomy=self.open_taxonomy_enabled, _value=f"Free Text {i}")
+            ObjectTag.objects.create(object_id="abc", taxonomy=self.open_taxonomy_disabled, _value=f"Free Text {i}")
 
     @ddt.data(
         (None, "abc", status.HTTP_403_FORBIDDEN, None, None),
@@ -452,10 +432,7 @@ def setUp(self):
         ("staff", "non-existing-id", status.HTTP_200_OK, 0, 0),
     )
     @ddt.unpack
-    def test_retrieve_object_tags(
-        self, user_attr, object_id, expected_status, expected_count, expected_results
-
-    ):
+    def test_retrieve_object_tags(self, user_attr, object_id, expected_status, expected_count, expected_results):
         """
         Test retrieving object tags
         """
@@ -508,9 +485,7 @@ def test_retrieve_object_tags_taxonomy_queryparam(
         ("staff", "abc", status.HTTP_400_BAD_REQUEST),
     )
     @ddt.unpack
-    def test_retrieve_object_tags_invalid_taxonomy_queryparam(
-        self, user_attr, object_id, expected_status
-    ):
+    def test_retrieve_object_tags_invalid_taxonomy_queryparam(self, user_attr, object_id, expected_status):
         """
         Test retrieving object tags for invalid taxonomy
         """
@@ -555,10 +530,7 @@ def test_retrieve_object_tags_pagination(
             user = getattr(self, user_attr)
             self.client.force_authenticate(user=user)
 
-        query_params = {
-            "taxonomy": self.open_taxonomy_enabled.pk,
-            "page": page
-        }
+        query_params = {"taxonomy": self.open_taxonomy_enabled.pk, "page": page}
         if page_size:
             query_params["page_size"] = page_size
 
@@ -574,22 +546,21 @@ def test_retrieve_object_tags_pagination(
 
     @ddt.data(
         (None, "POST", status.HTTP_403_FORBIDDEN),
-        (None, "PUT", status.HTTP_403_FORBIDDEN),
         (None, "PATCH", status.HTTP_403_FORBIDDEN),
         (None, "DELETE", status.HTTP_403_FORBIDDEN),
-        ("user", "POST", status.HTTP_403_FORBIDDEN),
-        ("user", "PUT", status.HTTP_403_FORBIDDEN),
-        ("user", "PATCH", status.HTTP_403_FORBIDDEN),
+        ("user", "POST", status.HTTP_405_METHOD_NOT_ALLOWED),
+        ("user", "PATCH", status.HTTP_405_METHOD_NOT_ALLOWED),
         ("user", "DELETE", status.HTTP_403_FORBIDDEN),
         ("staff", "POST", status.HTTP_405_METHOD_NOT_ALLOWED),
-        ("staff", "PUT", status.HTTP_405_METHOD_NOT_ALLOWED),
         ("staff", "PATCH", status.HTTP_405_METHOD_NOT_ALLOWED),
         ("staff", "DELETE", status.HTTP_405_METHOD_NOT_ALLOWED),
     )
     @ddt.unpack
     def test_object_tags_remaining_http_methods(
-        self, user_attr, http_method, expected_status,
-
+        self,
+        user_attr,
+        http_method,
+        expected_status,
     ):
         """
         Test POST/PUT/PATCH/DELETE method for ObjectTagView
@@ -604,27 +575,26 @@ def test_object_tags_remaining_http_methods(
             self.client.force_authenticate(user=user)
 
         if http_method == "POST":
-            response = self.client.post(
-                url, {"test": "payload"}, format="json"
-            )
-        elif http_method == "PUT":
-            response = self.client.put(
-                url, {"test": "payload"}, format="json"
-            )
+            response = self.client.post(url, {"test": "payload"}, format="json")
         elif http_method == "PATCH":
-            response = self.client.patch(
-                url, {"test": "payload"}, format="json"
-            )
+            response = self.client.patch(url, {"test": "payload"}, format="json")
         elif http_method == "DELETE":
             response = self.client.delete(url)
 
         assert response.status_code == expected_status
 
+    @ddt.data(
+        (None, "system_taxonomy", ({"id": "pt"}), status.HTTP_403_FORBIDDEN),
+        ("user", "system_taxonomy", ({"id": "pt"}), status.HTTP_200_OK),
+        ("staff", "system_taxonomy", ({"id": "pt"}), status.HTTP_200_OK),
+    )
+    @ddt.unpack
+    def test_tag_object(self, user_attr, taxonomy_attr, tag_values, expected_status):
+        url = OBJECT_TAGS_RETRIEVE_URL.format(object_id="abc")
 
-    def test_tag_object(self):
-        url = OBJECT_TAGS_TAG_URL.format(object_id="abc")
-        self.client.force_authenticate(user=self.staff)
-        response = self.client.post(
-            url, {"test": "payload"}, format="json"
-        )
-        assert response.status_code == status.HTTP_200_OK
+        if user_attr:
+            user = getattr(self, user_attr)
+            self.client.force_authenticate(user=user)
+
+        response = self.client.put(url, {"tags": tag_values}, format="json")
+        assert response.status_code == expected_status