diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index b63c70c5f0b8..f015770e5db8 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/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) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index f64fba5c3358..e386ee234226 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/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), diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 1bc80b73727a..b693f7ee0f56 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/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): """ diff --git a/openedx/core/djangoapps/content_tagging/types.py b/openedx/core/djangoapps/content_tagging/types.py index 64fa0d58f000..9ffb090d61e3 100644 --- a/openedx/core/djangoapps/content_tagging/types.py +++ b/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]] diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py index 8cc9c9e7f7a9..39dd925c1acd 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/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 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 9c85f60fc3a8..8127bc618058 100644 --- a/requirements/edx/base.txt +++ b/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 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 15e078ae0dd3..d49e358ea3e8 100644 --- a/requirements/edx/development.txt +++ b/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 diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index defc09a23deb..8e06e72e60cc 100644 --- a/requirements/edx/doc.txt +++ b/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 diff --git a/requirements/edx/paver.txt b/requirements/edx/paver.txt index faa0085f1631..d86acae05f4f 100644 --- a/requirements/edx/paver.txt +++ b/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 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 49be07ecec71..548c0c636a6e 100644 --- a/requirements/edx/testing.txt +++ b/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