From 11bab7d2eda70d385aa46efbae74a7a4fd2f5801 Mon Sep 17 00:00:00 2001 From: Jillian Date: Sat, 12 Apr 2025 00:51:58 +0930 Subject: [PATCH] refactor!: use LibraryCollectionLocator and LibraryContainerLocator [FC-0083] (#36476) efactors the content_libraries.api to use LibraryCollectionLocator and LibraryContainerLocator keys, instead of passing separate LibraryUsageKeyV2 keys along with the collection/container locators. Renames misleading uses of collection_usage_key to collection_locator, including in the content_libraries.api and content.search.api method names and parameters. This change impacts Developers, but should not affect end users. This refactoring seems reasonable to do without going through deprecation, given the minimal use of these APIs. --- openedx/core/djangoapps/content/search/api.py | 30 +++++---- .../djangoapps/content/search/documents.py | 35 ++++------ .../djangoapps/content/search/handlers.py | 12 ++-- .../core/djangoapps/content/search/tasks.py | 19 +++--- .../content/search/tests/test_api.py | 11 ++-- .../content/search/tests/test_documents.py | 25 ++++--- .../content_libraries/api/blocks.py | 22 ++++--- .../content_libraries/api/collections.py | 21 +++--- .../content_libraries/api/containers.py | 13 ++-- .../content_libraries/api/libraries.py | 7 +- .../content_libraries/library_context.py | 3 +- .../content_libraries/signal_handlers.py | 20 ++++-- .../content_libraries/tests/test_api.py | 66 ++++++++++++------- .../tests/test_containers.py | 30 +++++---- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 18 files changed, 176 insertions(+), 146 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 31db56bbf2..49bf4be768 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -18,7 +18,11 @@ from meilisearch import Client as MeilisearchClient from meilisearch.errors import MeilisearchApiError, MeilisearchError from meilisearch.models.task import TaskInfo from opaque_keys.edx.keys import UsageKey, OpaqueKey -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryCollectionLocator +from opaque_keys.edx.locator import ( + LibraryCollectionLocator, + LibraryContainerLocator, + LibraryLocatorV2, +) from openedx_learning.api import authoring as authoring_api from common.djangoapps.student.roles import GlobalStaff from rest_framework.request import Request @@ -461,8 +465,9 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None, incremental=Fa docs = [] for collection in batch: try: - doc = searchable_doc_for_collection(library_key, collection.key, collection=collection) - doc.update(searchable_doc_tags_for_collection(library_key, collection.key)) + collection_key = lib_api.library_collection_locator(library_key, collection.key) + doc = searchable_doc_for_collection(collection_key, collection=collection) + doc.update(searchable_doc_tags_for_collection(collection_key)) docs.append(doc) except Exception as err: # pylint: disable=broad-except status_cb(f"Error indexing collection {collection}: {err}") @@ -702,13 +707,13 @@ def _get_document_from_index(document_id: str) -> dict: return document -def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collection_key: str) -> None: +def upsert_library_collection_index_doc(collection_key: LibraryCollectionLocator) -> None: """ Creates, updates, or deletes the document for the given Library Collection in the search index. If the Collection is not found or disabled (i.e. soft-deleted), then delete it from the search index. """ - doc = searchable_doc_for_collection(library_key, collection_key) + doc = searchable_doc_for_collection(collection_key) update_components = False # Soft-deleted/disabled collections are removed from the index @@ -738,12 +743,11 @@ def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collectio if update_components: from .tasks import update_library_components_collections as update_task - update_task.delay(str(library_key), collection_key) + update_task.delay(str(collection_key)) def update_library_components_collections( - library_key: LibraryLocatorV2, - collection_key: str, + collection_key: LibraryCollectionLocator, batch_size: int = 1000, ) -> None: """ @@ -751,8 +755,12 @@ 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 = lib_api.get_library(library_key) - components = authoring_api.get_collection_components(library.learning_package_id, collection_key) + components = authoring_api.get_collection_components( + library.learning_package_id, + collection_key.collection_id, + ) paginator = Paginator(components, batch_size) for page in paginator.page_range: @@ -828,12 +836,12 @@ def upsert_block_collections_index_docs(usage_key: UsageKey): _update_index_docs([doc]) -def upsert_collection_tags_index_docs(collection_usage_key: LibraryCollectionLocator): +def upsert_collection_tags_index_docs(collection_key: LibraryCollectionLocator): """ Updates the tags data in documents for the given library collection """ - doc = searchable_doc_tags_for_collection(collection_usage_key.library_key, collection_usage_key.collection_id) + doc = searchable_doc_tags_for_collection(collection_key) _update_index_docs([doc]) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index dbb55cb755..1c09303427 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -9,7 +9,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.locator import LibraryContainerLocator, LibraryLocatorV2 +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 from rest_framework.exceptions import NotFound @@ -450,19 +450,14 @@ def searchable_doc_collections(usage_key: UsageKey) -> dict: def searchable_doc_tags_for_collection( - library_key: LibraryLocatorV2, - collection_key: str, + collection_key: LibraryCollectionLocator ) -> dict: """ Generate a dictionary document suitable for ingestion into a search engine like Meilisearch or Elasticsearch, with the tags data for the given library collection. """ - collection_usage_key = lib_api.get_library_collection_usage_key( - library_key, - collection_key, - ) - doc = searchable_doc_for_key(collection_usage_key) - doc.update(_tags_for_content_object(collection_usage_key)) + doc = searchable_doc_for_key(collection_key) + doc.update(_tags_for_content_object(collection_key)) return doc @@ -484,8 +479,7 @@ def searchable_doc_for_course_block(block) -> dict: def searchable_doc_for_collection( - library_key: LibraryLocatorV2, - collection_key: str, + collection_key: LibraryCollectionLocator, *, # Optionally provide the collection if we've already fetched one collection: Collection | None = None, @@ -498,21 +492,16 @@ def searchable_doc_for_collection( If no collection is found for the given library_key + collection_key, the returned document will contain only basic information derived from the collection usage key, and no Fields.type value will be included in the returned dict. """ - collection_usage_key = lib_api.get_library_collection_usage_key( - library_key, - collection_key, - ) - - doc = searchable_doc_for_key(collection_usage_key) + doc = searchable_doc_for_key(collection_key) try: - collection = collection or lib_api.get_library_collection_from_usage_key(collection_usage_key) + collection = collection or lib_api.get_library_collection_from_locator(collection_key) except lib_api.ContentLibraryCollectionNotFound: # Collection not found, so we can only return the base doc pass if collection: - assert collection.key == collection_key + assert collection.key == collection_key.collection_id draft_num_children = authoring_api.filter_publishable_entities( collection.entities, @@ -524,9 +513,9 @@ def searchable_doc_for_collection( ).count() doc.update({ - Fields.context_key: str(library_key), - Fields.org: str(library_key.org), - Fields.usage_key: str(collection_usage_key), + Fields.context_key: str(collection_key.library_key), + Fields.org: str(collection_key.org), + Fields.usage_key: str(collection_key), Fields.block_id: collection.key, Fields.type: DocType.collection, Fields.display_name: collection.title, @@ -537,7 +526,7 @@ def searchable_doc_for_collection( Fields.published: { Fields.published_num_children: published_num_children, }, - Fields.access_id: _meili_access_id_from_context_key(library_key), + Fields.access_id: _meili_access_id_from_context_key(collection_key.library_key), Fields.breadcrumbs: [{"display_name": collection.learning_package.title}], }) diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index ba7e58eb80..d817de1519 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -187,16 +187,14 @@ def library_collection_updated_handler(**kwargs) -> None: if library_collection.background: update_library_collection_index_doc.delay( - str(library_collection.library_key), - library_collection.collection_key, + str(library_collection.collection_key), ) else: # Update collection index synchronously to make sure that search index is updated before # the frontend invalidates/refetches index. # See content_library_updated_handler for more details. update_library_collection_index_doc.apply(args=[ - str(library_collection.library_key), - library_collection.collection_key, + str(library_collection.collection_key), ]) @@ -252,16 +250,14 @@ def library_container_updated_handler(**kwargs) -> None: if library_container.background: update_library_container_index_doc.delay( - str(library_container.library_key), - library_container.container_key, + str(library_container.container_key), ) else: # Update container index synchronously to make sure that search index is updated before # the frontend invalidates/refetches index. # See content_library_updated_handler for more details. update_library_container_index_doc.apply(args=[ - str(library_container.library_key), - library_container.container_key, + str(library_container.container_key), ]) diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index 1c66050415..48fc6b327c 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -12,6 +12,7 @@ from edx_django_utils.monitoring import set_code_owner_attribute from meilisearch.errors import MeilisearchError from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import ( + LibraryCollectionLocator, LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2, @@ -92,38 +93,40 @@ def update_content_library_index_docs(library_key_str: str) -> None: @shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) @set_code_owner_attribute -def update_library_collection_index_doc(library_key_str: str, collection_key: str) -> None: +def update_library_collection_index_doc(collection_key_str: str) -> None: """ Celery task to update the content index document for a library collection """ - library_key = LibraryLocatorV2.from_string(library_key_str) + collection_key = LibraryCollectionLocator.from_string(collection_key_str) + library_key = collection_key.library_key log.info("Updating content index documents for collection %s in library%s", collection_key, library_key) - api.upsert_library_collection_index_doc(library_key, collection_key) + api.upsert_library_collection_index_doc(collection_key) @shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) @set_code_owner_attribute -def update_library_components_collections(library_key_str: str, collection_key: str) -> None: +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. """ - library_key = LibraryLocatorV2.from_string(library_key_str) + collection_key = LibraryCollectionLocator.from_string(collection_key_str) + library_key = collection_key.library_key log.info("Updating document.collections for library %s collection %s components", library_key, collection_key) - api.update_library_components_collections(library_key, collection_key) + api.update_library_components_collections(collection_key) @shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) @set_code_owner_attribute -def update_library_container_index_doc(library_key_str: str, container_key_str: str) -> None: +def update_library_container_index_doc(container_key_str: str) -> None: """ Celery task to update the content index document for a library container """ - library_key = LibraryLocatorV2.from_string(library_key_str) container_key = LibraryContainerLocator.from_string(container_key_str) + library_key = container_key.library_key log.info("Updating content index documents for container %s in library%s", container_key, library_key) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 63317d020f..bd9fb803c1 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -8,6 +8,7 @@ import copy from datetime import datetime, timezone from unittest.mock import MagicMock, Mock, call, patch from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.locator import LibraryCollectionLocator import ddt import pytest @@ -188,11 +189,13 @@ class TestSearchApi(ModuleStoreTestCase): created_by=None, description="my collection description" ) - self.collection_usage_key = "lib-collection:org1:lib:MYCOL" + self.collection_key = LibraryCollectionLocator.from_string( + "lib-collection:org1:lib:MYCOL", + ) self.collection_dict = { "id": "lib-collectionorg1libmycol-5b647617", "block_id": self.collection.key, - "usage_key": self.collection_usage_key, + "usage_key": str(self.collection_key), "type": "collection", "display_name": "my_collection", "description": "my collection description", @@ -737,8 +740,8 @@ class TestSearchApi(ModuleStoreTestCase): @override_settings(MEILISEARCH_ENABLED=True) def test_index_tags_in_collections(self, mock_meilisearch): # Tag collection - tagging_api.tag_object(self.collection_usage_key, self.taxonomyA, ["one", "two"]) - tagging_api.tag_object(self.collection_usage_key, self.taxonomyB, ["three", "four"]) + tagging_api.tag_object(str(self.collection_key), self.taxonomyA, ["one", "two"]) + tagging_api.tag_object(str(self.collection_key), self.taxonomyB, ["three", "four"]) # Build expected docs with tags at each stage doc_collection_with_tags1 = { diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index fb41e8efc2..1f6c56f7d6 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -5,6 +5,7 @@ from dataclasses import replace from datetime import datetime, timezone from freezegun import freeze_time +from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator from openedx_learning.api import authoring as authoring_api from organizations.models import Organization @@ -81,7 +82,9 @@ class StudioDocumentsTest(SharedModuleStoreTestCase): created_by=None, description="my toy collection description" ) - cls.collection_usage_key = "lib-collection:edX:2012_Fall:TOY_COLLECTION" + cls.collection_key = LibraryCollectionLocator.from_string( + "lib-collection:edX:2012_Fall:TOY_COLLECTION", + ) cls.library_block = library_api.create_library_block( cls.library.key, "html", @@ -94,7 +97,9 @@ class StudioDocumentsTest(SharedModuleStoreTestCase): title="A Unit in the Search Index", user_id=None, ) - cls.container_usage_key = "lct:edX:2012_Fall:unit:unit1" + cls.container_key = LibraryContainerLocator.from_string( + "lct:edX:2012_Fall:unit:unit1", + ) # Add the problem block to the collection library_api.update_library_collection_components( @@ -123,8 +128,8 @@ class StudioDocumentsTest(SharedModuleStoreTestCase): tagging_api.tag_object(str(cls.html_block_key), cls.subject_tags, tags=["Chinese", "Jump Links"]) tagging_api.tag_object(str(cls.html_block_key), cls.difficulty_tags, tags=["Normal"]) tagging_api.tag_object(str(cls.library_block.usage_key), cls.difficulty_tags, tags=["Normal"]) - tagging_api.tag_object(cls.collection_usage_key, cls.difficulty_tags, tags=["Normal"]) - tagging_api.tag_object(cls.container_usage_key, cls.difficulty_tags, tags=["Normal"]) + tagging_api.tag_object(str(cls.collection_key), cls.difficulty_tags, tags=["Normal"]) + tagging_api.tag_object(str(cls.container_key), cls.difficulty_tags, tags=["Normal"]) @property def toy_course_access_id(self): @@ -451,13 +456,13 @@ class StudioDocumentsTest(SharedModuleStoreTestCase): assert doc["publish_status"] == "modified" def test_collection_with_library(self): - doc = searchable_doc_for_collection(self.library.key, self.collection.key) - doc.update(searchable_doc_tags_for_collection(self.library.key, self.collection.key)) + doc = searchable_doc_for_collection(self.collection_key) + doc.update(searchable_doc_tags_for_collection(self.collection_key)) assert doc == { "id": "lib-collectionedx2012_falltoy_collection-d1d907a4", "block_id": self.collection.key, - "usage_key": self.collection_usage_key, + "usage_key": str(self.collection_key), "type": "collection", "org": "edX", "display_name": "Toy Collection", @@ -480,13 +485,13 @@ class StudioDocumentsTest(SharedModuleStoreTestCase): def test_collection_with_published_library(self): library_api.publish_changes(self.library.key) - doc = searchable_doc_for_collection(self.library.key, self.collection.key) - doc.update(searchable_doc_tags_for_collection(self.library.key, self.collection.key)) + doc = searchable_doc_for_collection(self.collection_key) + doc.update(searchable_doc_tags_for_collection(self.collection_key)) assert doc == { "id": "lib-collectionedx2012_falltoy_collection-d1d907a4", "block_id": self.collection.key, - "usage_key": self.collection_usage_key, + "usage_key": str(self.collection_key), "type": "collection", "org": "edX", "display_name": "Toy Collection", diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index 2451076226..04649a8350 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -59,6 +59,7 @@ from .exceptions import ( LibraryBlockAlreadyExists, ) from .libraries import ( + library_collection_locator, library_component_usage_key, require_permission_for_library_key, PublishableItem, @@ -284,8 +285,7 @@ def set_library_block_olx(usage_key: LibraryUsageLocatorV2, new_olx_str: str) -> for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=usage_key.lib_key, - container_key=str(container.container_key), + container_key=container.container_key, background=True, ) ) @@ -527,8 +527,10 @@ def delete_library_block(usage_key: LibraryUsageLocatorV2, remove_from_parent=Tr for collection in affected_collections: LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library_key, - collection_key=collection.key, + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), background=True, ) ) @@ -540,8 +542,7 @@ def delete_library_block(usage_key: LibraryUsageLocatorV2, remove_from_parent=Tr for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container.container_key), + container_key=container.container_key, background=True, ) ) @@ -580,8 +581,10 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2) -> None: for collection in affected_collections: LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library_key, - collection_key=collection.key, + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), background=True, ) ) @@ -594,8 +597,7 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2) -> None: for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container.container_key), + container_key=container.container_key, background=True, ) ) diff --git a/openedx/core/djangoapps/content_libraries/api/collections.py b/openedx/core/djangoapps/content_libraries/api/collections.py index 435804c414..f94c9e5401 100644 --- a/openedx/core/djangoapps/content_libraries/api/collections.py +++ b/openedx/core/djangoapps/content_libraries/api/collections.py @@ -32,8 +32,8 @@ __all__ = [ "update_library_collection", "update_library_collection_components", "set_library_component_collections", - "get_library_collection_usage_key", - "get_library_collection_from_usage_key", + "library_collection_locator", + "get_library_collection_from_locator", ] @@ -218,8 +218,10 @@ def set_library_component_collections( for collection in affected_collections: LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library_key, - collection_key=collection.key, + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), background=True, ) ) @@ -227,7 +229,7 @@ def set_library_component_collections( return component -def get_library_collection_usage_key( +def library_collection_locator( library_key: LibraryLocatorV2, collection_key: str, ) -> LibraryCollectionLocator: @@ -238,15 +240,14 @@ def get_library_collection_usage_key( return LibraryCollectionLocator(library_key, collection_key) -def get_library_collection_from_usage_key( - collection_usage_key: LibraryCollectionLocator, +def get_library_collection_from_locator( + collection_locator: LibraryCollectionLocator, ) -> Collection: """ Return a Collection using the LibraryCollectionLocator """ - - library_key = collection_usage_key.library_key - collection_key = collection_usage_key.collection_id + library_key = collection_locator.library_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. try: diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 8a80cec352..31eae20e7f 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -190,8 +190,7 @@ def create_container( LIBRARY_CONTAINER_CREATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container_key), + container_key=container_key, ) ) @@ -219,8 +218,7 @@ def update_container( LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container_key), + container_key=container_key, ) ) @@ -241,8 +239,7 @@ def delete_container( LIBRARY_CONTAINER_DELETED.send_event( library_container=LibraryContainerData( - library_key=container_key.library_key, - container_key=str(container_key), + container_key=container_key, ) ) @@ -259,7 +256,6 @@ def restore_container(container_key: LibraryContainerLocator) -> None: LIBRARY_CONTAINER_CREATED.send_event( library_container=LibraryContainerData( - library_key=container_key.library_key, container_key=str(container_key), ) ) @@ -325,8 +321,7 @@ def update_container_children( LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=library_key, - container_key=str(container_key), + container_key=container_key, ) ) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index bf9ca6f60d..de7be56f7f 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -75,6 +75,7 @@ from openedx.core.types import User as UserType from .. import permissions from ..constants import ALL_RIGHTS_RESERVED from ..models import ContentLibrary, ContentLibraryPermission +from .collections import library_collection_locator from .exceptions import ( LibraryAlreadyExists, LibraryPermissionIntegrityError, @@ -739,8 +740,10 @@ def revert_changes(library_key: LibraryLocatorV2) -> None: for collection in authoring_api.get_collections(learning_package.id): LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library_key, - collection_key=collection.key, + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), background=True, ) ) diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py index d5f121d839..34a5a90269 100644 --- a/openedx/core/djangoapps/content_libraries/library_context.py +++ b/openedx/core/djangoapps/content_libraries/library_context.py @@ -125,8 +125,7 @@ class LibraryContextImpl(LearningContext): for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( - library_key=usage_key.lib_key, - container_key=str(container.container_key), + container_key=container.container_key, background=True, ) ) diff --git a/openedx/core/djangoapps/content_libraries/signal_handlers.py b/openedx/core/djangoapps/content_libraries/signal_handlers.py index 68cc7512e5..08b30b10bb 100644 --- a/openedx/core/djangoapps/content_libraries/signal_handlers.py +++ b/openedx/core/djangoapps/content_libraries/signal_handlers.py @@ -25,7 +25,7 @@ from openedx_learning.api.authoring_models import Collection, CollectionPublisha from lms.djangoapps.grades.api import signals as grades_signals -from .api import library_component_usage_key +from .api import library_collection_locator, library_component_usage_key from .models import ContentLibrary, LtiGradedResource log = logging.getLogger(__name__) @@ -86,15 +86,19 @@ def library_collection_saved(sender, instance, created, **kwargs): if created: LIBRARY_COLLECTION_CREATED.send_event( library_collection=LibraryCollectionData( - library_key=library.library_key, - collection_key=instance.key, + collection_key=library_collection_locator( + library_key=library.library_key, + collection_key=instance.key, + ), ) ) else: LIBRARY_COLLECTION_UPDATED.send_event( library_collection=LibraryCollectionData( - library_key=library.library_key, - collection_key=instance.key, + collection_key=library_collection_locator( + library_key=library.library_key, + collection_key=instance.key, + ), ) ) @@ -112,8 +116,10 @@ def library_collection_deleted(sender, instance, **kwargs): LIBRARY_COLLECTION_DELETED.send_event( library_collection=LibraryCollectionData( - library_key=library.library_key, - collection_key=instance.key, + collection_key=library_collection_locator( + library_key=library.library_key, + collection_key=instance.key, + ), ) ) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 71360cde3e..b0934768b7 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -352,8 +352,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_CREATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib2.library_key, - collection_key="COL4", + collection_key=api.library_collection_locator( + self.lib2.library_key, + collection_key="COL4", + ), ), }, event_receiver.call_args_list[0].kwargs, @@ -388,8 +390,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key="COL1", + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), ), }, event_receiver.call_args_list[0].kwargs, @@ -418,8 +422,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_DELETED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key="COL1", + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), ), }, event_receiver.call_args_list[0].kwargs, @@ -493,8 +499,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key="COL1", + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), ), }, event_receiver.call_args_list[2].kwargs, @@ -544,8 +552,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib2.library_key, - collection_key=self.col2.key, + collection_key=api.library_collection_locator( + self.lib2.library_key, + collection_key=self.col2.key, + ), background=True, ), }, @@ -556,8 +566,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib2.library_key, - collection_key=self.col3.key, + collection_key=api.library_collection_locator( + self.lib2.library_key, + collection_key=self.col3.key, + ), background=True, ), }, @@ -585,8 +597,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key=self.col1.key, + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key=self.col1.key, + ), background=True, ), }, @@ -614,8 +628,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key=self.col1.key, + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key=self.col1.key, + ), background=True, ), }, @@ -656,8 +672,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key=self.col1.key, + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key=self.col1.key, + ), background=True, ), }, @@ -715,8 +733,10 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": LIBRARY_COLLECTION_UPDATED, "sender": None, "library_collection": LibraryCollectionData( - self.lib1.library_key, - collection_key=self.col1.key, + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key=self.col1.key, + ), background=True, ), }, @@ -824,8 +844,7 @@ class ContentLibraryContainersTest(ContentLibrariesRestApiTest, OpenEdxEventsTes "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - library_key=self.lib1.library_key, - container_key=str(self.unit1.container_key), + container_key=self.unit1.container_key, background=True, ) }, @@ -836,8 +855,7 @@ class ContentLibraryContainersTest(ContentLibrariesRestApiTest, OpenEdxEventsTes "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - library_key=self.lib1.library_key, - container_key=str(self.unit2.container_key), + container_key=self.unit2.container_key, background=True, ) }, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 46d206af50..85e04260cf 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -7,7 +7,7 @@ from unittest import mock import ddt from freezegun import freeze_time -from opaque_keys.edx.locator import LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 from openedx_events.content_authoring.data import LibraryContainerData from openedx_events.content_authoring.signals import ( LIBRARY_CONTAINER_CREATED, @@ -83,13 +83,15 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): self.assertDictContainsEntries(container_data, expected_data) assert create_receiver.call_count == 1 + container_key = LibraryContainerLocator.from_string( + "lct:CL-TEST:containers:unit:u1", + ) self.assertDictContainsSubset( { "signal": LIBRARY_CONTAINER_CREATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key="lct:CL-TEST:containers:unit:u1", + container_key, ), }, create_receiver.call_args_list[0].kwargs, @@ -114,8 +116,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key="lct:CL-TEST:containers:unit:u1", + container_key, ), }, update_receiver.call_args_list[0].kwargs, @@ -135,8 +136,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "signal": LIBRARY_CONTAINER_DELETED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key="lct:CL-TEST:containers:unit:u1", + container_key, ), }, delete_receiver.call_args_list[0].kwargs, @@ -214,8 +214,9 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key=container_data["container_key"], + container_key=LibraryContainerLocator.from_string( + container_data["container_key"], + ), ), }, update_receiver.call_args_list[0].kwargs, @@ -263,8 +264,9 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key=container_data["container_key"], + container_key=LibraryContainerLocator.from_string( + container_data["container_key"], + ), ), }, update_receiver.call_args_list[0].kwargs, @@ -324,8 +326,9 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, "library_container": LibraryContainerData( - lib_key, - container_key=container_data["container_key"], + container_key=LibraryContainerLocator.from_string( + container_data["container_key"], + ), ), }, update_receiver.call_args_list[0].kwargs, @@ -374,7 +377,6 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "signal": LIBRARY_CONTAINER_CREATED, "sender": None, "library_container": LibraryContainerData( - lib_key, container_key="lct:CL-TEST:containers:unit:u1", ), }, diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 29c056d671..f1ef8845df 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -804,7 +804,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/kernel.in openedx-django-wiki==2.1.0 # via -r requirements/edx/kernel.in -openedx-events==9.20.0 +openedx-events==10.0.0 # via # -r requirements/edx/kernel.in # edx-enterprise diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index d7f7674bd6..5bfbca431f 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1363,7 +1363,7 @@ openedx-django-wiki==2.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-events==9.20.0 +openedx-events==10.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 d3372e8dc6..7b4fb5682a 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -976,7 +976,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events==9.20.0 +openedx-events==10.0.0 # via # -r requirements/edx/base.txt # edx-enterprise diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 174a06ed72..404104e975 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1034,7 +1034,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events==9.20.0 +openedx-events==10.0.0 # via # -r requirements/edx/base.txt # edx-enterprise