Tag in Collections [FC-0062] (#35383)

* feat: Allow tag in collections

* chore: Bump version of opaque-keys

openedx/core/djangoapps/content_tagging/api.py

@@ -12,7 +12,7 @@
 import openedx_tagging.core.tagging.api as oel_tagging
 from django.db.models import Exists, OuterRef, Q, QuerySet
 from django.utils.timezone import now
-from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.keys import CourseKey, LibraryCollectionKey
 from opaque_keys.edx.locator import LibraryLocatorV2
 from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy
 from openedx_tagging.core.tagging.models.utils import TAGS_CSV_SEPARATOR
@@ -230,7 +230,7 @@ def generate_csv_rows(object_id, buffer) -> Iterator[str]:
     """
     content_key = get_content_key_from_string(object_id)
 
-    if isinstance(content_key, UsageKey):
+    if isinstance(content_key, (UsageKey, LibraryCollectionKey)):
         raise ValueError("The object_id must be a CourseKey or a LibraryLocatorV2.")
 
     all_object_tags, taxonomies = get_all_object_tags(content_key)

openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py

@@ -13,7 +13,7 @@
 import ddt
 from django.contrib.auth import get_user_model
 from django.core.files.uploadedfile import SimpleUploadedFile
-from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
+from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryCollectionLocator
 from openedx_tagging.core.tagging.models import Tag, Taxonomy
 from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy
 from openedx_tagging.core.tagging.rest_api.v1.serializers import TaxonomySerializer
@@ -111,6 +111,9 @@ def _setUp_library(self):
         )
         self.libraryA = str(self.content_libraryA.key)
 
+    def _setUp_collection(self):
+        self.collection_key = str(LibraryCollectionLocator(self.content_libraryA.key, 'test-collection'))
+
     def _setUp_users(self):
         """
         Create users for testing
@@ -284,6 +287,7 @@ def setUp(self):
         self._setUp_library()
         self._setUp_users()
         self._setUp_taxonomies()
+        self._setUp_collection()
 
         # Clear the rules cache in between test runs to keep query counts consistent.
         rules_cache.clear()
@@ -1653,6 +1657,87 @@ def test_tag_library_invalid(self, user_attr, taxonomy_attr):
         response = self._call_put_request(self.libraryA, taxonomy.pk, ["invalid"])
         assert response.status_code == status.HTTP_400_BAD_REQUEST
 
+    @ddt.data(
+        # staffA and staff are staff in collection and can tag using enabled taxonomies
+        ("user", "tA1", ["Tag 1"], status.HTTP_403_FORBIDDEN),
+        ("staffA", "tA1", ["Tag 1"], status.HTTP_200_OK),
+        ("staff", "tA1", ["Tag 1"], status.HTTP_200_OK),
+        ("user", "tA1", [], status.HTTP_403_FORBIDDEN),
+        ("staffA", "tA1", [], status.HTTP_200_OK),
+        ("staff", "tA1", [], status.HTTP_200_OK),
+        ("user", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN),
+        ("staffA", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK),
+        ("staff", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK),
+        ("user", "open_taxonomy", ["tag1"], status.HTTP_403_FORBIDDEN),
+        ("staffA", "open_taxonomy", ["tag1"], status.HTTP_200_OK),
+        ("staff", "open_taxonomy", ["tag1"], status.HTTP_200_OK),
+    )
+    @ddt.unpack
+    def test_tag_collection(self, user_attr, taxonomy_attr, tag_values, expected_status):
+        """
+        Tests that only staff and org level users can tag collections
+        """
+        user = getattr(self, user_attr)
+        self.client.force_authenticate(user=user)
+
+        taxonomy = getattr(self, taxonomy_attr)
+
+        response = self._call_put_request(self.collection_key, taxonomy.pk, tag_values)
+
+        assert response.status_code == expected_status
+        if status.is_success(expected_status):
+            tags_by_taxonomy = response.data[str(self.collection_key)]["taxonomies"]
+            if tag_values:
+                response_taxonomy = tags_by_taxonomy[0]
+                assert response_taxonomy["name"] == taxonomy.name
+                response_tags = response_taxonomy["tags"]
+                assert [t["value"] for t in response_tags] == tag_values
+            else:
+                assert tags_by_taxonomy == []  # No tags are set from any taxonomy
+
+            # Check that re-fetching the tags returns what we set
+            url = OBJECT_TAG_UPDATE_URL.format(object_id=self.collection_key)
+            new_response = self.client.get(url, format="json")
+            assert status.is_success(new_response.status_code)
+            assert new_response.data == response.data
+
+    @ddt.data(
+        "staffA",
+        "staff",
+    )
+    def test_tag_collection_disabled_taxonomy(self, user_attr):
+        """
+        Nobody can use disabled taxonomies to tag objects
+        """
+        user = getattr(self, user_attr)
+        self.client.force_authenticate(user=user)
+
+        disabled_taxonomy = self.tA2
+        assert disabled_taxonomy.enabled is False
+
+        response = self._call_put_request(self.collection_key, disabled_taxonomy.pk, ["Tag 1"])
+
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+
+    @ddt.data(
+        ("staffA", "tA1"),
+        ("staff", "tA1"),
+        ("staffA", "multiple_taxonomy"),
+        ("staff", "multiple_taxonomy"),
+    )
+    @ddt.unpack
+    def test_tag_collection_invalid(self, user_attr, taxonomy_attr):
+        """
+        Tests that nobody can add invalid tags to a collection using a closed taxonomy
+        """
+        user = getattr(self, user_attr)
+        self.client.force_authenticate(user=user)
+
+        taxonomy = getattr(self, taxonomy_attr)
+
+        response = self._call_put_request(self.collection_key, taxonomy.pk, ["invalid"])
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+
     @ddt.data(
         ("superuser", status.HTTP_200_OK),
         ("staff", status.HTTP_403_FORBIDDEN),
@@ -1768,10 +1853,14 @@ def test_get_tags(self):
     @ddt.data(
         ('staff', 'courseA', 8),
         ('staff', 'libraryA', 8),
+        ('staff', 'collection_key', 8),
         ("content_creatorA", 'courseA', 11, False),
         ("content_creatorA", 'libraryA', 11, False),
+        ("content_creatorA", 'collection_key', 11, False),
         ("library_staffA", 'libraryA', 11, False),  # Library users can only view objecttags, not change them?
+        ("library_staffA", 'collection_key', 11, False),
         ("library_userA", 'libraryA', 11, False),
+        ("library_userA", 'collection_key', 11, False),
         ("instructorA", 'courseA', 11),
         ("course_instructorA", 'courseA', 11),
         ("course_staffA", 'courseA', 11),

openedx/core/djangoapps/content_tagging/tests/test_api.py

@@ -5,7 +5,7 @@
 import ddt
 from django.test.testcases import TestCase
 from fs.osfs import OSFS
-from opaque_keys.edx.keys import CourseKey, UsageKey
+from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey
 from opaque_keys.edx.locator import LibraryLocatorV2
 from openedx_tagging.core.tagging.models import ObjectTag
 from organizations.models import Organization
@@ -380,6 +380,23 @@ def test_copy_cross_org_tags(self):
         with self.assertNumQueries(31):  # TODO why so high?
             self._test_copy_object_tags(src_key, dst_key, expected_tags)
 
+    def test_tag_collection(self):
+        collection_key = LibraryCollectionKey.from_string("lib-collection:orgA:libX:1")
+
+        api.tag_object(
+            object_id=str(collection_key),
+            taxonomy=self.taxonomy_3,
+            tags=["Tag 3.1"],
+        )
+
+        with self.assertNumQueries(1):
+            object_tags, taxonomies = api.get_all_object_tags(collection_key)
+
+        assert object_tags == {'lib-collection:orgA:libX:1': {3: ['Tag 3.1']}}
+        assert taxonomies == {
+            self.taxonomy_3.id: self.taxonomy_3,
+        }
+
 
 class TestExportImportTags(TaggedCourseMixin):
     """

openedx/core/djangoapps/content_tagging/types.py

@@ -5,11 +5,11 @@
 
 from typing import Dict, List, Union
 
-from opaque_keys.edx.keys import CourseKey, UsageKey
+from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey
 from opaque_keys.edx.locator import LibraryLocatorV2
 from openedx_tagging.core.tagging.models import Taxonomy
 
-ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey]
+ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey, LibraryCollectionKey]
 ContextKey = Union[LibraryLocatorV2, CourseKey]
 
 TagValuesByTaxonomyIdDict = Dict[int, List[str]]

openedx/core/djangoapps/content_tagging/utils.py

@@ -5,7 +5,7 @@
 
 from edx_django_utils.cache import RequestCache
 from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import CourseKey, UsageKey
+from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey
 from opaque_keys.edx.locator import LibraryLocatorV2
 from openedx_tagging.core.tagging.models import Taxonomy
 from organizations.models import Organization
@@ -26,8 +26,14 @@ def get_content_key_from_string(key_str: str) -> ContentKey:
         except InvalidKeyError:
             try:
                 return UsageKey.from_string(key_str)
-            except InvalidKeyError as usage_key_error:
-                raise ValueError("object_id must be a CourseKey, LibraryLocatorV2 or a UsageKey") from usage_key_error
+            except InvalidKeyError:
+                try:
+                    return LibraryCollectionKey.from_string(key_str)
+                except InvalidKeyError as usage_key_error:
+                    raise ValueError(
+                        "object_id must be one of the following "
+                        "keys: CourseKey, LibraryLocatorV2, UsageKey or LibCollectionKey"
+                    ) from usage_key_error
 
 
 def get_context_key_from_key(content_key: ContentKey) -> ContextKey:
@@ -38,6 +44,10 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey:
     if isinstance(content_key, (CourseKey, LibraryLocatorV2)):
         return content_key
 
+    # If the content key is a LibraryCollectionKey, return the LibraryLocatorV2
+    if isinstance(content_key, LibraryCollectionKey):
+        return content_key.library_key
+
     # If the content key is a UsageKey, return the context key
     context_key = content_key.context_key
 

requirements/edx/base.txt

@@ -484,7 +484,7 @@ edx-milestones==0.6.0
     # via -r requirements/edx/kernel.in
 edx-name-affirmation==2.4.0
     # via -r requirements/edx/kernel.in
-edx-opaque-keys[django]==2.10.0
+edx-opaque-keys[django]==2.11.0
     # via
     #   -r requirements/edx/kernel.in
     #   -r requirements/edx/paver.txt

requirements/edx/development.txt

@@ -770,7 +770,7 @@ edx-name-affirmation==2.4.0
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
-edx-opaque-keys[django]==2.10.0
+edx-opaque-keys[django]==2.11.0
     # via
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt

requirements/edx/doc.txt

@@ -564,7 +564,7 @@ edx-milestones==0.6.0
     # via -r requirements/edx/base.txt
 edx-name-affirmation==2.4.0
     # via -r requirements/edx/base.txt
-edx-opaque-keys[django]==2.10.0
+edx-opaque-keys[django]==2.11.0
     # via
     #   -r requirements/edx/base.txt
     #   edx-bulk-grades

requirements/edx/paver.txt

@@ -12,7 +12,7 @@ charset-normalizer==2.0.12
     #   requests
 dnspython==2.6.1
     # via pymongo
-edx-opaque-keys==2.10.0
+edx-opaque-keys==2.11.0
     # via -r requirements/edx/paver.in
 idna==3.7
     # via requests

requirements/edx/testing.txt

@@ -590,7 +590,7 @@ edx-milestones==0.6.0
     # via -r requirements/edx/base.txt
 edx-name-affirmation==2.4.0
     # via -r requirements/edx/base.txt
-edx-opaque-keys[django]==2.10.0
+edx-opaque-keys[django]==2.11.0
     # via
     #   -r requirements/edx/base.txt
     #   edx-bulk-grades