diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 9b57a9c709..b3614e9cc6 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -760,7 +760,7 @@ def update_library_components_collections( Because there may be a lot of components, we send these updates to Meilisearch in batches. """ - library_key = collection_key.library_key + library_key = collection_key.lib_key library = lib_api.get_library(library_key) components = authoring_api.get_collection_components( library.learning_package_id, @@ -795,7 +795,7 @@ def update_library_containers_collections( Because there may be a lot of containers, we send these updates to Meilisearch in batches. """ - library_key = collection_key.library_key + library_key = collection_key.lib_key library = lib_api.get_library(library_key) containers = authoring_api.get_collection_containers( library.learning_package_id, diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 110c400219..6a40f049bf 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -8,7 +8,7 @@ from hashlib import blake2b from django.core.exceptions import ObjectDoesNotExist from django.utils.text import slugify -from opaque_keys.edx.keys import LearningContextKey, UsageKey, OpaqueKey +from opaque_keys.edx.keys import ContainerKey, LearningContextKey, UsageKey, OpaqueKey from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Collection @@ -523,7 +523,7 @@ def searchable_doc_for_collection( ).count() doc.update({ - Fields.context_key: str(collection_key.library_key), + Fields.context_key: str(collection_key.context_key), Fields.org: str(collection_key.org), Fields.usage_key: str(collection_key), Fields.block_id: collection.key, @@ -536,7 +536,7 @@ def searchable_doc_for_collection( Fields.published: { Fields.published_num_children: published_num_children, }, - Fields.access_id: _meili_access_id_from_context_key(collection_key.library_key), + Fields.access_id: _meili_access_id_from_context_key(collection_key.context_key), Fields.breadcrumbs: [{"display_name": collection.learning_package.title}], }) @@ -549,7 +549,7 @@ def searchable_doc_for_collection( def searchable_doc_for_container( - container_key: LibraryContainerLocator, + container_key: ContainerKey, ) -> dict: """ Generate a dictionary document suitable for ingestion into a search engine @@ -562,7 +562,7 @@ def searchable_doc_for_container( """ doc = { Fields.id: meili_id_from_opaque_key(container_key), - Fields.context_key: str(container_key.library_key), + Fields.context_key: str(container_key.context_key), Fields.org: str(container_key.org), # In the future, this may be either course_container or library_container Fields.type: DocType.library_container, @@ -570,7 +570,7 @@ def searchable_doc_for_container( Fields.block_type: container_key.container_type, Fields.usage_key: str(container_key), # Field name isn't exact but this is the closest match Fields.block_id: container_key.container_id, # Field name isn't exact but this is the closest match - Fields.access_id: _meili_access_id_from_context_key(container_key.library_key), + Fields.access_id: _meili_access_id_from_context_key(container_key.context_key), Fields.publish_status: PublishStatus.never, Fields.last_published: None, } @@ -596,7 +596,7 @@ def searchable_doc_for_container( Fields.publish_status: publish_status, Fields.last_published: container.last_published.timestamp() if container.last_published else None, }) - library = lib_api.get_library(container_key.library_key) + library = lib_api.get_library(container_key.context_key) if library: doc[Fields.breadcrumbs] = [{"display_name": library.title}] diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index a1d47fc6f9..1ab77aba38 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -98,7 +98,7 @@ def update_library_collection_index_doc(collection_key_str: str) -> None: Celery task to update the content index document for a library collection """ collection_key = LibraryCollectionLocator.from_string(collection_key_str) - library_key = collection_key.library_key + library_key = collection_key.lib_key log.info("Updating content index documents for collection %s in library%s", collection_key, library_key) @@ -112,7 +112,7 @@ def update_library_components_collections(collection_key_str: str) -> None: Celery task to update the "collections" field for components in the given content library collection. """ collection_key = LibraryCollectionLocator.from_string(collection_key_str) - library_key = collection_key.library_key + library_key = collection_key.lib_key log.info("Updating document.collections for library %s collection %s components", library_key, collection_key) @@ -126,7 +126,7 @@ def update_library_containers_collections(collection_key_str: str) -> None: Celery task to update the "collections" field for containers in the given content library collection. """ collection_key = LibraryCollectionLocator.from_string(collection_key_str) - library_key = collection_key.library_key + library_key = collection_key.lib_key log.info("Updating document.collections for library %s collection %s containers", library_key, collection_key) @@ -140,7 +140,7 @@ def update_library_container_index_doc(container_key_str: str) -> None: Celery task to update the content index document for a library container """ container_key = LibraryContainerLocator.from_string(container_key_str) - library_key = container_key.library_key + library_key = container_key.lib_key log.info("Updating content index documents for container %s in library%s", container_key, library_key) diff --git a/openedx/core/djangoapps/content_libraries/api/collections.py b/openedx/core/djangoapps/content_libraries/api/collections.py index 8e20710ab8..2b0ddc08d8 100644 --- a/openedx/core/djangoapps/content_libraries/api/collections.py +++ b/openedx/core/djangoapps/content_libraries/api/collections.py @@ -251,7 +251,7 @@ def get_library_collection_from_locator( """ Return a Collection using the LibraryCollectionLocator """ - library_key = collection_locator.library_key + library_key = collection_locator.lib_key collection_key = collection_locator.collection_id content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] assert content_library.learning_package_id is not None # shouldn't happen but it's technically possible. diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 393755ed1c..b04a43a970 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -179,7 +179,7 @@ def get_container_from_key(container_key: LibraryContainerLocator, isDeleted=Fal Raises ContentLibraryContainerNotFound if no container found, or if the container has been soft deleted. """ assert isinstance(container_key, LibraryContainerLocator) - content_library = ContentLibrary.objects.get_by_key(container_key.library_key) + content_library = ContentLibrary.objects.get_by_key(container_key.lib_key) learning_package = content_library.learning_package assert learning_package is not None container = authoring_api.get_container_by_key( @@ -204,7 +204,7 @@ def get_container(container_key: LibraryContainerLocator, include_collections=Fa else: associated_collections = None container_meta = ContainerMetadata.from_container( - container_key.library_key, + container_key.lib_key, container, associated_collections=associated_collections, ) @@ -268,7 +268,7 @@ def update_container( Update a container (e.g. a Unit) title. """ container = get_container_from_key(container_key) - library_key = container_key.library_key + library_key = container_key.lib_key assert container.unit unit_version = authoring_api.create_next_unit_version( @@ -295,7 +295,7 @@ def delete_container( No-op if container doesn't exist or has already been soft-deleted. """ - library_key = container_key.library_key + library_key = container_key.lib_key container = get_container_from_key(container_key) affected_collections = authoring_api.get_entity_collections( @@ -330,7 +330,7 @@ def restore_container(container_key: LibraryContainerLocator) -> None: """ Restore the specified library container. """ - library_key = container_key.library_key + library_key = container_key.lib_key container = get_container_from_key(container_key, isDeleted=True) affected_collections = authoring_api.get_entity_collections( @@ -380,13 +380,13 @@ def get_container_children( if container_key.container_type == ContainerType.Unit.value: child_components = authoring_api.get_components_in_unit(container.unit, published=published) return [LibraryXBlockMetadata.from_component( - container_key.library_key, + container_key.lib_key, entry.component ) for entry in child_components] else: child_entities = authoring_api.get_entities_in_container(container, published=published) return [ContainerMetadata.from_container( - container_key.library_key, + container_key.lib_key, entry.entity ) for entry in child_entities] @@ -411,7 +411,7 @@ def update_container_children( """ Adds children components or containers to given container. """ - library_key = container_key.library_key + library_key = container_key.lib_key container_type = container_key.container_type container = get_container_from_key(container_key) match container_type: @@ -459,7 +459,7 @@ def publish_container_changes(container_key: LibraryContainerLocator, user_id: i containers/blocks. """ container = get_container_from_key(container_key) - library_key = container_key.library_key + library_key = container_key.lib_key content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] learning_package = content_library.learning_package assert learning_package diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index cd1b7602fb..061f4a0693 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -77,7 +77,7 @@ class LibraryContainerView(GenericAPIView): Get information about a container """ api.require_permission_for_library_key( - container_key.library_key, + container_key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) @@ -94,7 +94,7 @@ class LibraryContainerView(GenericAPIView): Update a Container. """ api.require_permission_for_library_key( - container_key.library_key, + container_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, ) @@ -115,7 +115,7 @@ class LibraryContainerView(GenericAPIView): Delete a Container (soft delete). """ api.require_permission_for_library_key( - container_key.library_key, + container_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, ) @@ -180,7 +180,7 @@ class LibraryContainerChildrenView(GenericAPIView): """ published = request.GET.get('published', False) api.require_permission_for_library_key( - container_key.library_key, + container_key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) @@ -201,7 +201,7 @@ class LibraryContainerChildrenView(GenericAPIView): Helper function to update children in container. """ api.require_permission_for_library_key( - container_key.library_key, + container_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, ) @@ -288,7 +288,7 @@ class LibraryContainerRestore(GenericAPIView): Restores a soft-deleted library container """ api.require_permission_for_library_key( - container_key.library_key, + container_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, ) @@ -310,7 +310,7 @@ class LibraryContainerCollectionsView(GenericAPIView): Collection and Components must all be part of the given library/learning package. """ content_library = api.require_permission_for_library_key( - container_key.library_key, + container_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY ) @@ -320,7 +320,7 @@ class LibraryContainerCollectionsView(GenericAPIView): collection_keys = serializer.validated_data['collection_keys'] api.set_library_item_collections( - library_key=container_key.library_key, + library_key=container_key.lib_key, publishable_entity=container.publishable_entity, collection_keys=collection_keys, created_by=request.user.id, @@ -342,7 +342,7 @@ class LibraryContainerPublishView(GenericAPIView): Publish the container and its children """ api.require_permission_for_library_key( - container_key.library_key, + container_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, ) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index 707c45d8ce..be386e0e9d 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -7,8 +7,7 @@ from rest_framework import serializers from rest_framework.exceptions import ValidationError from opaque_keys import OpaqueKey -from opaque_keys.edx.keys import UsageKeyV2 -from opaque_keys.edx.locator import LibraryContainerLocator +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryUsageLocatorV2 from opaque_keys import InvalidKeyError from openedx_learning.api.authoring_models import Collection @@ -319,22 +318,22 @@ class ContentLibraryCollectionUpdateSerializer(serializers.Serializer): class UsageKeyV2Serializer(serializers.BaseSerializer): """ - Serializes a UsageKeyV2. + Serializes a library Component (XBlock) key. """ - def to_representation(self, value: UsageKeyV2) -> str: + def to_representation(self, value: LibraryUsageLocatorV2) -> str: """ - Returns the UsageKeyV2 value as a string. + Returns the LibraryUsageLocatorV2 value as a string. """ return str(value) - def to_internal_value(self, value: str) -> UsageKeyV2: + def to_internal_value(self, value: str) -> LibraryUsageLocatorV2: """ - Returns a UsageKeyV2 from the string value. + Returns a LibraryUsageLocatorV2 from the string value. - Raises ValidationError if invalid UsageKeyV2. + Raises ValidationError if invalid LibraryUsageLocatorV2. """ try: - return UsageKeyV2.from_string(value) + return LibraryUsageLocatorV2.from_string(value) except InvalidKeyError as err: raise ValidationError from err @@ -359,12 +358,12 @@ class OpaqueKeySerializer(serializers.BaseSerializer): def to_internal_value(self, value: str) -> OpaqueKey: """ - Returns a UsageKeyV2 or a LibraryContainerLocator from the string value. + Returns a LibraryUsageLocatorV2 or a LibraryContainerLocator from the string value. Raises ValidationError if invalid UsageKeyV2 or LibraryContainerLocator. """ try: - return UsageKeyV2.from_string(value) + return LibraryUsageLocatorV2.from_string(value) except InvalidKeyError: try: return LibraryContainerLocator.from_string(value) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 9163ebd58a..5f4130c842 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -7,12 +7,11 @@ import io from itertools import groupby import csv from typing import Iterator -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import CourseKey, CollectionKey, ContainerKey, UsageKey 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, LibraryItemKey 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,8 +229,8 @@ def generate_csv_rows(object_id, buffer) -> Iterator[str]: """ content_key = get_content_key_from_string(object_id) - if isinstance(content_key, (UsageKey, LibraryItemKey)): - raise ValueError("The object_id must be a CourseKey or a LibraryLocatorV2.") + if isinstance(content_key, (UsageKey, CollectionKey, ContainerKey)): + raise ValueError("The object_id must be a component, collection, or container.") all_object_tags, taxonomies = get_all_object_tags(content_key) tagged_content = build_object_tree_with_objecttags(content_key, all_object_tags) diff --git a/openedx/core/djangoapps/content_tagging/types.py b/openedx/core/djangoapps/content_tagging/types.py index 3684e57053..6e54caa051 100644 --- a/openedx/core/djangoapps/content_tagging/types.py +++ b/openedx/core/djangoapps/content_tagging/types.py @@ -5,11 +5,11 @@ from __future__ import annotations from typing import Dict, List, Union -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryItemKey +from opaque_keys.edx.keys import CourseKey, UsageKey, CollectionKey, ContainerKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import Taxonomy -ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey, LibraryItemKey] +ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey, CollectionKey, ContainerKey] 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 419b960927..f6e4883251 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -5,7 +5,7 @@ from __future__ import annotations from edx_django_utils.cache import RequestCache from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryItemKey +from opaque_keys.edx.keys import CourseKey, CollectionKey, ContainerKey, UsageKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import Taxonomy from organizations.models import Organization @@ -18,22 +18,16 @@ def get_content_key_from_string(key_str: str) -> ContentKey: """ Get content key from string """ - try: - return CourseKey.from_string(key_str) - except InvalidKeyError: + for key_type in (LibraryLocatorV2, CourseKey, UsageKey, ContainerKey, CollectionKey): try: - return LibraryLocatorV2.from_string(key_str) + return key_type.from_string(key_str) except InvalidKeyError: - try: - return UsageKey.from_string(key_str) - except InvalidKeyError: - try: - return LibraryItemKey.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 LibraryItemKey" - ) from usage_key_error + continue + raise ValueError( + "For tagging, object_id must be one of the following " + "key types: LibraryLocatorV2, CourseKey, UsageKey, ContainerKey, CollectionKey. " + f"got: {key_str}" + ) def get_context_key_from_key(content_key: ContentKey) -> ContextKey: @@ -41,20 +35,11 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey: Returns the context key from a given content key. """ # If the content key is a CourseKey or a LibraryLocatorV2, return it - if isinstance(content_key, (CourseKey, LibraryLocatorV2)): + if isinstance(content_key, (LibraryLocatorV2, CourseKey)): return content_key - - # If the content key is a LibraryItemKey, return the LibraryLocatorV2 - if isinstance(content_key, LibraryItemKey): - return content_key.library_key - - # If the content key is a UsageKey, return the context key - context_key = content_key.context_key - - if isinstance(context_key, (CourseKey, LibraryLocatorV2)): - return context_key - - raise ValueError("context must be a CourseKey or a LibraryLocatorV2") + else: + assert isinstance(content_key.context_key, (CourseKey, LibraryLocatorV2)) # for type checker + return content_key.context_key def get_context_key_from_key_string(key_str: str) -> ContextKey: diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index e19edf6e32..5d1b25a092 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -478,7 +478,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/kernel.in edx-name-affirmation==3.0.1 # via -r requirements/edx/kernel.in -edx-opaque-keys[django]==2.12.0 +edx-opaque-keys[django]==3.0.0 # via # -r requirements/edx/kernel.in # edx-bulk-grades diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 5840e5bdcd..dea264a70c 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -767,7 +767,7 @@ edx-name-affirmation==3.0.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-opaque-keys[django]==2.12.0 +edx-opaque-keys[django]==3.0.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 96c941c445..57d9b52ee1 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -562,7 +562,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/base.txt edx-name-affirmation==3.0.1 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==2.12.0 +edx-opaque-keys[django]==3.0.0 # via # -r requirements/edx/base.txt # edx-bulk-grades diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 101011f92c..d117630253 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -589,7 +589,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/base.txt edx-name-affirmation==3.0.1 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==2.12.0 +edx-opaque-keys[django]==3.0.0 # via # -r requirements/edx/base.txt # edx-bulk-grades