From 27651325bebd86905d3a97e2e9f7cbdd6f05538b Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 27 Aug 2024 17:21:40 -0500 Subject: [PATCH 01/10] chore: Use opencraft branch of opaque-keys --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 27de7847f284..e4b5d936dcd8 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.3.7 # via -r requirements/edx/kernel.in -edx-opaque-keys[django]==2.10.0 +edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key # via # -r requirements/edx/kernel.in # -r requirements/edx/paver.txt diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 0bdc8144ee71..ba960085ed54 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -770,7 +770,7 @@ edx-name-affirmation==2.3.7 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-opaque-keys[django]==2.10.0 +edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 91b30d81df6d..fbf1e86ea86d 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.3.7 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==2.10.0 +edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key # via # -r requirements/edx/base.txt # edx-bulk-grades diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index a5b510742ac7..3e22fbf68957 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -76,7 +76,7 @@ edx-event-bus-kafka>=5.6.0 # Kafka implementation of event bus edx-event-bus-redis edx-milestones edx-name-affirmation -edx-opaque-keys +git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key#egg=edx-opaque-keys edx-organizations edx-proctoring>=2.0.1 # using hash to support django42 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 2092c9354834..96d6cd92592b 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.3.7 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==2.10.0 +edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key # via # -r requirements/edx/base.txt # edx-bulk-grades From b2e9ca7122767c677879b1dce2b01890b1a02e6c Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 27 Aug 2024 19:07:34 -0500 Subject: [PATCH 02/10] feat: Allow tag collections --- .../content_tagging/tests/test_api.py | 19 ++++++++++++++++++- .../core/djangoapps/content_tagging/types.py | 4 ++-- .../core/djangoapps/content_tagging/utils.py | 11 +++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 1bc80b73727a..1aec825aa51d 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, LibCollectionKey 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 = LibCollectionKey.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..3fdb261abfd6 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, LibCollectionKey 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, LibCollectionKey] 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..bc076d38458c 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, LibCollectionKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import Taxonomy from organizations.models import Organization @@ -26,8 +26,11 @@ 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 LibCollectionKey.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 def get_context_key_from_key(content_key: ContentKey) -> ContextKey: @@ -41,7 +44,7 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey: # If the content key is a UsageKey, return the context key context_key = content_key.context_key - if isinstance(context_key, (CourseKey, LibraryLocatorV2)): + if isinstance(context_key, (CourseKey, LibraryLocatorV2, LibCollectionKey)): return context_key raise ValueError("context must be a CourseKey or a LibraryLocatorV2") From 98ffb2354ccd99d652eaf4b9c067e92219f33a54 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 27 Aug 2024 19:35:48 -0500 Subject: [PATCH 03/10] style: Fix lint --- .../content_tagging/helpers/objecttag_export_helpers.py | 4 ++-- openedx/core/djangoapps/content_tagging/tests/test_api.py | 2 +- openedx/core/djangoapps/content_tagging/utils.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py index cb7865e136c6..05cc2b4617be 100644 --- a/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py @@ -7,7 +7,7 @@ from typing import Any, Callable, Iterator, Union from attrs import define -from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.keys import CourseKey, UsageKey, LibCollectionKey from opaque_keys.edx.locator import LibraryLocatorV2 from xblock.core import XBlock @@ -135,7 +135,7 @@ def _get_library_block_tagged_object( def build_object_tree_with_objecttags( - content_key: LibraryLocatorV2 | CourseKey, + content_key: LibraryLocatorV2 | CourseKey | LibCollectionKey, object_tag_cache: TagValuesByObjectIdDict, ) -> TaggedContent: """ diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 1aec825aa51d..83bedf289cf1 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -396,7 +396,7 @@ def test_tag_collection(self): assert taxonomies == { self.taxonomy_3.id: self.taxonomy_3, } - + class TestExportImportTags(TaggedCourseMixin): """ diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py index bc076d38458c..ff94126dc41d 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -44,7 +44,7 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey: # If the content key is a UsageKey, return the context key context_key = content_key.context_key - if isinstance(context_key, (CourseKey, LibraryLocatorV2, LibCollectionKey)): + if isinstance(context_key, (CourseKey, LibraryLocatorV2)): return context_key raise ValueError("context must be a CourseKey or a LibraryLocatorV2") From 2849d4bdd4fe071fd1e8339e580d1bb4ecdbc738 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 29 Aug 2024 22:00:28 -0500 Subject: [PATCH 04/10] refactor: Update LibCollectionKey to LibraryCollectionKey --- .../helpers/objecttag_export_helpers.py | 4 ++-- .../djangoapps/content_tagging/tests/test_api.py | 4 ++-- openedx/core/djangoapps/content_tagging/types.py | 4 ++-- openedx/core/djangoapps/content_tagging/utils.py | 13 ++++++++++--- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py index 05cc2b4617be..8cc5d2684cb0 100644 --- a/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py @@ -7,7 +7,7 @@ from typing import Any, Callable, Iterator, Union from attrs import define -from opaque_keys.edx.keys import CourseKey, UsageKey, LibCollectionKey +from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey from opaque_keys.edx.locator import LibraryLocatorV2 from xblock.core import XBlock @@ -135,7 +135,7 @@ def _get_library_block_tagged_object( def build_object_tree_with_objecttags( - content_key: LibraryLocatorV2 | CourseKey | LibCollectionKey, + content_key: LibraryLocatorV2 | CourseKey | LibraryCollectionKey, object_tag_cache: TagValuesByObjectIdDict, ) -> TaggedContent: """ diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 83bedf289cf1..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, LibCollectionKey +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 @@ -381,7 +381,7 @@ def test_copy_cross_org_tags(self): self._test_copy_object_tags(src_key, dst_key, expected_tags) def test_tag_collection(self): - collection_key = LibCollectionKey.from_string("lib-collection:orgA:libX:1") + collection_key = LibraryCollectionKey.from_string("lib-collection:orgA:libX:1") api.tag_object( object_id=str(collection_key), diff --git a/openedx/core/djangoapps/content_tagging/types.py b/openedx/core/djangoapps/content_tagging/types.py index 3fdb261abfd6..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, LibCollectionKey +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, LibCollectionKey] +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 ff94126dc41d..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, LibCollectionKey +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 @@ -28,9 +28,12 @@ def get_content_key_from_string(key_str: str) -> ContentKey: return UsageKey.from_string(key_str) except InvalidKeyError: try: - return LibCollectionKey.from_string(key_str) + return LibraryCollectionKey.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 + 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: @@ -41,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 From af1665b19419089e103094aea5baa410c1eda2b3 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 29 Aug 2024 22:17:25 -0500 Subject: [PATCH 05/10] fix: Type check fails --- openedx/core/djangoapps/content_tagging/utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py index 39dd925c1acd..1c90f1c142da 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -46,6 +46,8 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey: # If the content key is a LibraryCollectionKey, return the LibraryLocatorV2 if isinstance(content_key, LibraryCollectionKey): + if not isinstance(content_key.library_key, LibraryLocatorV2): + raise TypeError("Expected a LibraryLocatorV2") return content_key.library_key # If the content key is a UsageKey, return the context key From 5eac5132bc8e21d65aed85fbb83c79a823b64725 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Sat, 31 Aug 2024 00:06:57 -0500 Subject: [PATCH 06/10] style: Clean code in get_context_key_from_key --- openedx/core/djangoapps/content_tagging/utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py index 1c90f1c142da..39dd925c1acd 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -46,8 +46,6 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey: # If the content key is a LibraryCollectionKey, return the LibraryLocatorV2 if isinstance(content_key, LibraryCollectionKey): - if not isinstance(content_key.library_key, LibraryLocatorV2): - raise TypeError("Expected a LibraryLocatorV2") return content_key.library_key # If the content key is a UsageKey, return the context key From 278ab4566ba4a9073f4bccf8bf49ab52beed8945 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Sat, 31 Aug 2024 00:29:13 -0500 Subject: [PATCH 07/10] test: Check permissions for tag collections --- .../rest_api/v1/tests/test_views.py | 91 ++++++++++++++++++- 1 file changed, 90 insertions(+), 1 deletion(-) 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), From 4bc07a0570210dc607c62d91d0bbcd996b8db358 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Sat, 31 Aug 2024 00:37:09 -0500 Subject: [PATCH 08/10] refactor: Verify LibraryCollectionKey in generate_csv_rows --- openedx/core/djangoapps/content_tagging/api.py | 4 ++-- .../content_tagging/helpers/objecttag_export_helpers.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 47b157c1a34e..94ffaa569944 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 @@ -227,7 +227,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/helpers/objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py index 8cc5d2684cb0..cb7865e136c6 100644 --- a/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py @@ -7,7 +7,7 @@ from typing import Any, Callable, Iterator, Union from attrs import define -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey +from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryLocatorV2 from xblock.core import XBlock @@ -135,7 +135,7 @@ def _get_library_block_tagged_object( def build_object_tree_with_objecttags( - content_key: LibraryLocatorV2 | CourseKey | LibraryCollectionKey, + content_key: LibraryLocatorV2 | CourseKey, object_tag_cache: TagValuesByObjectIdDict, ) -> TaggedContent: """ From c01c51f3f9ddb5c7590e4f940d8b95edb04c2c7b Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 11 Sep 2024 19:43:54 -0500 Subject: [PATCH 09/10] chore: Bump version of opaque-keys --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 2 +- requirements/edx/paver.in | 2 +- requirements/edx/paver.txt | 2 +- requirements/edx/testing.txt | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 88b9b29d23d3..473e5402f563 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.3.7 # via -r requirements/edx/kernel.in -edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key +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 a4514ebef01d..ce15567157b4 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -770,7 +770,7 @@ edx-name-affirmation==2.3.7 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key +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 c4b54c5f752c..b2cf90498da7 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.3.7 # via -r requirements/edx/base.txt -edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key +edx-opaque-keys[django]==2.11.0 # via # -r requirements/edx/base.txt # edx-bulk-grades diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 3e22fbf68957..1970686d327d 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -76,7 +76,7 @@ edx-event-bus-kafka>=5.6.0 # Kafka implementation of event bus edx-event-bus-redis edx-milestones edx-name-affirmation -git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key#egg=edx-opaque-keys +edx-opaque-keys>=2.11.0 edx-organizations edx-proctoring>=2.0.1 # using hash to support django42 diff --git a/requirements/edx/paver.in b/requirements/edx/paver.in index 6987ede82275..22746d1e04ee 100644 --- a/requirements/edx/paver.in +++ b/requirements/edx/paver.in @@ -10,7 +10,7 @@ -c ../constraints.txt -edx-opaque-keys # Create and introspect course and xblock identities +edx-opaque-keys>=2.11.0 # Create and introspect course and xblock identities lazy # Lazily-evaluated attributes for Python objects libsass # Python bindings for the LibSass CSS compiler markupsafe # XML/HTML/XHTML Markup safe strings 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 f25d1f8ca162..0610eb337323 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.3.7 # via -r requirements/edx/base.txt -edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key +edx-opaque-keys[django]==2.11.0 # via # -r requirements/edx/base.txt # edx-bulk-grades From 120900d8ddf23daf01565dff45472afd732563a7 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 11 Sep 2024 20:22:14 -0500 Subject: [PATCH 10/10] chore: Bumpb version of opaque-keys --- requirements/edx/kernel.in | 2 +- requirements/edx/paver.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 1970686d327d..a5b510742ac7 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -76,7 +76,7 @@ edx-event-bus-kafka>=5.6.0 # Kafka implementation of event bus edx-event-bus-redis edx-milestones edx-name-affirmation -edx-opaque-keys>=2.11.0 +edx-opaque-keys edx-organizations edx-proctoring>=2.0.1 # using hash to support django42 diff --git a/requirements/edx/paver.in b/requirements/edx/paver.in index 22746d1e04ee..6987ede82275 100644 --- a/requirements/edx/paver.in +++ b/requirements/edx/paver.in @@ -10,7 +10,7 @@ -c ../constraints.txt -edx-opaque-keys>=2.11.0 # Create and introspect course and xblock identities +edx-opaque-keys # Create and introspect course and xblock identities lazy # Lazily-evaluated attributes for Python objects libsass # Python bindings for the LibSass CSS compiler markupsafe # XML/HTML/XHTML Markup safe strings