diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index c39e04c299dc..d80517c2a842 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -17,7 +17,7 @@ from opaque_keys.edx.locator import LibraryContainerLocator from openedx_content.api import get_published_version from openedx_content.models_api import Component, Container -from openedx_django_lib.fields import immutable_uuid_field, key_field, manual_date_time_field +from openedx_django_lib.fields import immutable_uuid_field, manual_date_time_field, ref_field logger = logging.getLogger(__name__) @@ -87,7 +87,7 @@ class EntityLinkBase(models.Model): """ uuid = immutable_uuid_field() # Search by library/upstream context key - upstream_context_key = key_field( + upstream_context_key = ref_field( help_text=_("Upstream context key i.e., learning_package/library key"), db_index=True, ) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index bebcdfd53dc5..7ac9e8e479ee 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -274,7 +274,7 @@ def setUp(self): collection_key = "test-collection" content_api.create_collection( learning_package_id=learning_package.id, - key=collection_key, + collection_code=collection_key, title="Test Collection", created_by=self.user.id, ) diff --git a/cms/djangoapps/modulestore_migrator/api/read_api.py b/cms/djangoapps/modulestore_migrator/api/read_api.py index 064223dc9633..cd9962c4aa19 100644 --- a/cms/djangoapps/modulestore_migrator/api/read_api.py +++ b/cms/djangoapps/modulestore_migrator/api/read_api.py @@ -135,9 +135,9 @@ def get_migrations( if source_key: migrations = migrations.filter(source__key=source_key) if target_key: - migrations = migrations.filter(target__key=str(target_key)) + migrations = migrations.filter(target__package_ref=str(target_key)) if target_collection_slug: - migrations = migrations.filter(target_collection__key=target_collection_slug) + migrations = migrations.filter(target_collection__collection_code=target_collection_slug) if task_uuid: migrations = migrations.filter(task_status__uuid=task_uuid) if is_failed is not None: @@ -176,9 +176,9 @@ def _migration(m: models.ModulestoreMigration) -> ModulestoreMigration: return ModulestoreMigration( pk=m.id, source_key=m.source.key, - target_key=LibraryLocatorV2.from_string(m.target.key), + target_key=LibraryLocatorV2.from_string(m.target.package_ref), target_title=m.target.title, - target_collection_slug=(m.target_collection.key if m.target_collection else None), + target_collection_slug=(m.target_collection.collection_code if m.target_collection else None), target_collection_title=(m.target_collection.title if m.target_collection else None), is_failed=m.is_failed, task_uuid=m.task_status.uuid, @@ -209,7 +209,7 @@ def _block_migration_success( """ Build an instance of the migration success dataclass """ - target_library_key = LibraryLocatorV2.from_string(target.learning_package.key) + target_library_key = LibraryLocatorV2.from_string(target.learning_package.package_ref) target_key: LibraryUsageLocatorV2 | LibraryContainerLocator if hasattr(target, "component"): target_key = library_component_usage_key(target_library_key, target.component) diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py index 797c42d9a5b3..9dc0c5dda5d3 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py @@ -18,6 +18,10 @@ class LibraryMigrationCollectionSerializer(serializers.ModelSerializer): """ Serializer for the target collection of a library migration. """ + # Expose Collection.collection_code as "key" to preserve the REST API field name. + # This is temporary: https://github.com/openedx/openedx-platform/issues/38406 + key = serializers.CharField(source='collection_code') + class Meta: model = Collection fields = ["key", "title"] diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py index 0b76c01cdaaa..594c9518a2ae 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py @@ -540,7 +540,7 @@ def get_queryset(self): self.request.user, lib_api.permissions.CAN_VIEW_THIS_CONTENT_LIBRARY ) - queryset = queryset.filter(target__key=library_key, source__key__startswith='course-v1') + queryset = queryset.filter(target__package_ref=str(library_key), source__key__startswith='course-v1') return queryset diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index b54b9191e6ba..1c4736a3d76a 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -346,13 +346,13 @@ def _import_structure( LibraryUsageLocatorV2(target_library.key, block_type, block_id) # type: ignore[abstract] for block_type, block_id in content_api.get_components(migration.target.id).values_list( - "component_type__name", "local_key" + "component_type__name", "component_code" ) ), used_container_slugs=set( content_api.get_containers( migration.target.id - ).values_list("publishable_entity__key", flat=True) + ).values_list("publishable_entity__entity_ref", flat=True) ), previous_block_migrations=( get_migration_blocks(source_data.previous_migration.pk) @@ -409,7 +409,7 @@ def _populate_collection(user_id: int, migration: models.ModulestoreMigration) - if block_target_pks: content_api.add_to_collection( learning_package_id=migration.target.pk, - key=migration.target_collection.key, + collection_code=migration.target_collection.collection_code, entities_qset=PublishableEntity.objects.filter(id__in=block_target_pks), created_by=user_id, ) @@ -867,7 +867,7 @@ def _migrate_container( container_exists = False if PublishableEntity.objects.filter( learning_package_id=context.target_package_id, - key=target_key.container_id, + entity_ref=target_key.container_id, ).exists(): libraries_api.restore_container(container_key=target_key) container = libraries_api.get_container(target_key) @@ -895,13 +895,9 @@ def _migrate_container( ).publishable_entity_version # Publish the container - # Call post publish events synchronously to avoid - # an error when calling `wait_for_post_publish_events` - # inside a celery task. libraries_api.publish_container_changes( container.container_key, context.created_by, - call_post_publish_events_sync=True, ) context.used_container_slugs.add(container.container_key.container_id) return container_publishable_entity_version, None @@ -932,7 +928,7 @@ def _migrate_component( try: component = content_api.get_components(context.target_package_id).get( component_type=component_type, - local_key=target_key.block_id, + component_code=target_key.block_id, ) component_existed = True # Do we have a specific method for this? @@ -953,7 +949,7 @@ def _migrate_component( component = content_api.create_component( context.target_package_id, component_type=component_type, - local_key=target_key.block_id, + component_code=target_key.block_id, created=context.created_at, created_by=context.created_by, ) @@ -971,7 +967,7 @@ def _migrate_component( continue new_path = f"static/{filename}" content_api.create_component_version_media( - component_version.pk, media_pk, key=new_path + component_version.pk, media_pk, path=new_path ) # Publish the component diff --git a/cms/djangoapps/modulestore_migrator/tests/test_api.py b/cms/djangoapps/modulestore_migrator/tests/test_api.py index 311d2b5b69ea..7e88e031a1ad 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_api.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_api.py @@ -232,7 +232,7 @@ def test_start_migration_to_library_with_collection(self): collection_key = "test-collection" content_api.create_collection( learning_package_id=self.learning_package.id, - key=collection_key, + collection_code=collection_key, title="Test Collection", created_by=user.id, ) @@ -249,7 +249,7 @@ def test_start_migration_to_library_with_collection(self): ) modulestoremigration = ModulestoreMigration.objects.get() - assert modulestoremigration.target_collection.key == collection_key + assert modulestoremigration.target_collection.collection_code == collection_key def test_start_migration_to_library_with_strategy_skip(self): """ @@ -487,19 +487,19 @@ def test_migration_api_for_various_scenarios(self): # Lib 2 has Collection C content_api.create_collection( learning_package_id=self.learning_package.id, - key="test-collection-1a", + collection_code="test-collection-1a", title="Test Collection A in Lib 1", created_by=user.id, ) content_api.create_collection( learning_package_id=self.learning_package.id, - key="test-collection-1b", + collection_code="test-collection-1b", title="Test Collection B in Lib 1", created_by=user.id, ) content_api.create_collection( learning_package_id=self.learning_package_2.id, - key="test-collection-2c", + collection_code="test-collection-2c", title="Test Collection C in Lib 2", created_by=user.id, ) diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py index 2285dd7d77e8..ae4ad1548937 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -89,12 +89,12 @@ def setUp(self): ) self.collection = Collection.objects.create( learning_package=self.learning_package, - key="test_collection", + collection_code="test_collection", title="Test Collection", ) self.collection2 = Collection.objects.create( learning_package=self.learning_package, - key="test_collection2", + collection_code="test_collection2", title="Test Collection 2", ) @@ -426,7 +426,7 @@ def test_migrate_component_with_static_content(self): self.assertIsNone(reason) # noqa: PT009 component_media = result.componentversion.componentversionmedia_set.filter( - key="static/test_image.png" + path="static/test_image.png" ).first() self.assertIsNotNone(component_media) # noqa: PT009 self.assertEqual(component_media.media.id, test_media.id) # noqa: PT009 @@ -673,12 +673,12 @@ def test_migrate_component_content_filename_not_in_olx(self): referenced_content_exists = ( result.componentversion.componentversionmedia_set.filter( - key="static/referenced.png" + path="static/referenced.png" ).exists() ) unreferenced_content_exists = ( result.componentversion.componentversionmedia_set.filter( - key="static/unreferenced.png" + path="static/unreferenced.png" ).exists() ) @@ -718,7 +718,7 @@ def test_migrate_container_creates_new_container(self): component_type=content_api.get_or_create_component_type( "xblock.v1", "problem" ), - local_key="child_problem_1", + component_code="child_problem_1", created=timezone.now(), created_by=self.user.id, ) @@ -734,7 +734,7 @@ def test_migrate_container_creates_new_container(self): component_type=content_api.get_or_create_component_type( "xblock.v1", "html" ), - local_key="child_html_1", + component_code="child_html_1", created=timezone.now(), created_by=self.user.id, ) @@ -906,7 +906,7 @@ def test_migrate_container_preserves_child_order(self): component_type=content_api.get_or_create_component_type( "xblock.v1", "problem" ), - local_key=f"child_problem_{i}", + component_code=f"child_problem_{i}", created=timezone.now(), created_by=self.user.id, ) @@ -946,7 +946,7 @@ def test_migrate_container_with_mixed_child_types(self): component_type=content_api.get_or_create_component_type( "xblock.v1", "problem" ), - local_key="mixed_problem", + component_code="mixed_problem", created=timezone.now(), created_by=self.user.id, ) @@ -962,7 +962,7 @@ def test_migrate_container_with_mixed_child_types(self): component_type=content_api.get_or_create_component_type( "xblock.v1", "html" ), - local_key="mixed_html", + component_code="mixed_html", created=timezone.now(), created_by=self.user.id, ) @@ -978,7 +978,7 @@ def test_migrate_container_with_mixed_child_types(self): component_type=content_api.get_or_create_component_type( "xblock.v1", "video" ), - local_key="mixed_video", + component_code="mixed_video", created=timezone.now(), created_by=self.user.id, ) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index f6bfdf13be77..2d044cd4210d 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -641,7 +641,7 @@ def index_collection_batch(batch, num_done, library_key) -> int: docs = [] for collection in batch: try: - collection_key = lib_api.library_collection_locator(library_key, collection.key) + collection_key = lib_api.library_collection_locator(library_key, collection.collection_code) doc = searchable_doc_for_collection(collection_key, collection=collection) doc.update(searchable_doc_tags(collection_key)) docs.append(doc) @@ -677,7 +677,7 @@ def index_container_batch(batch, num_done, library_key) -> int: doc.update(searchable_doc_containers(container_key, "sections")) docs.append(doc) except Exception as err: # pylint: disable=broad-except - status_cb(f"Error indexing container {container.key}: {err}") + status_cb(f"Error indexing container {container.entity_ref}: {err}") num_done += 1 if docs: @@ -1032,7 +1032,7 @@ def upsert_content_library_index_docs(library_key: LibraryLocatorV2, full_index: docs.append(doc) for collection in lib_api.get_library_collections(library_key): - collection_key = lib_api.library_collection_locator(library_key, collection.key) + collection_key = lib_api.library_collection_locator(library_key, collection.collection_code) doc = searchable_doc_for_collection(collection_key, collection=collection) docs.append(doc) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index b986966ec42c..678281191eeb 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -7,6 +7,7 @@ from hashlib import blake2b from django.core.exceptions import ObjectDoesNotExist +from django.db.models import F from django.utils.text import slugify from opaque_keys.edx.keys import ContainerKey, LearningContextKey, OpaqueKey, UsageKey from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator @@ -33,7 +34,7 @@ class Fields: usage_key = "usage_key" type = "type" # DocType.course_block or DocType.library_block (see below) # The block_id part of the usage key for course or library blocks. - # If it's a collection, the collection.key is stored here. + # If it's a collection, the collection.collection_code is stored here. # Sometimes human-readable, sometimes a random hex ID # Is only unique within the given context_key. block_id = "block_id" @@ -64,7 +65,8 @@ class Fields: tags_level2 = "level2" tags_level3 = "level3" # Collections (dictionary) that this object belongs to. - # Similarly to tags above, we collect the collection.titles and collection.keys into hierarchical facets. + # Similarly to tags above, we collect the collection.titles and collection.collection_codes + # into hierarchical facets. collections = "collections" collections_display_name = "display_name" collections_key = "key" @@ -448,10 +450,13 @@ def searchable_doc_collections(object_id: OpaqueKey) -> dict: try: if isinstance(object_id, UsageKey): component = lib_api.get_component_from_usage_key(object_id) + # Temporarily alias collection_code to "key" so downstream consumers + # (search indexer, REST API) keep the same field name. We will update + # downstream consumers later: https://github.com/openedx/openedx-platform/issues/38406 collections = content_api.get_entity_collections( component.learning_package_id, - component.key, - ).values('key', 'title') + component.entity_ref, + ).values("title", key=F('collection_code')) elif isinstance(object_id, LibraryContainerLocator): container = lib_api.get_container(object_id, include_collections=True) collections = container.collections @@ -543,7 +548,7 @@ def searchable_doc_for_collection( pass if collection: - assert collection.key == collection_key.collection_id + assert collection.collection_code == collection_key.collection_id draft_num_children = content_api.filter_publishable_entities( collection.entities, @@ -558,7 +563,7 @@ def searchable_doc_for_collection( 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, + Fields.block_id: collection.collection_code, Fields.type: DocType.collection, Fields.display_name: collection.title, Fields.description: collection.description, diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 8a29bb450326..afa847748e89 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -188,11 +188,11 @@ def setUp(self) -> None: tagging_api.add_tag_to_taxonomy(self.taxonomyB, "four") # Create a collection: - self.learning_package = content_api.get_learning_package_by_key(self.library.key) + self.learning_package = content_api.get_learning_package_by_ref(str(self.library.key)) with freeze_time(self.created_date): self.collection = content_api.create_collection( learning_package_id=self.learning_package.id, - key="MYCOL", + collection_code="MYCOL", title="my_collection", created_by=None, description="my collection description" @@ -202,7 +202,7 @@ def setUp(self) -> None: ) self.collection_dict = { "id": "lib-collectionorg1libmycol-5b647617", - "block_id": self.collection.key, + "block_id": self.collection.collection_code, "usage_key": str(self.collection_key), "type": "collection", "display_name": "my_collection", @@ -536,7 +536,7 @@ def test_reindex_meilisearch_library_block_error(self, mock_meilisearch) -> None def mocked_from_component(lib_key, component): # Simulate an error when processing problem 1 - if component.key == 'xblock.v1:problem:p1': + if component.entity_ref == 'xblock.v1:problem:p1': raise Exception('Error') return orig_from_component(lib_key, component) @@ -722,7 +722,7 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None: for collection in (collection2, collection1): library_api.update_library_collection_items( self.library.key, - collection_key=collection.key, + collection_key=collection.collection_code, opaque_keys=[ self.problem1.usage_key, ], @@ -732,8 +732,8 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None: lib_access, _ = SearchAccess.objects.get_or_create(context_key=self.library.key) doc_collection1_created = { "id": "lib-collectionorg1libcol1-283a79c9", - "block_id": collection1.key, - "usage_key": f"lib-collection:org1:lib:{collection1.key}", + "block_id": collection1.collection_code, + "usage_key": f"lib-collection:org1:lib:{collection1.collection_code}", "type": "collection", "display_name": "Collection 1", "description": "First Collection", @@ -750,8 +750,8 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None: } doc_collection2_created = { "id": "lib-collectionorg1libcol2-46823d4d", - "block_id": collection2.key, - "usage_key": f"lib-collection:org1:lib:{collection2.key}", + "block_id": collection2.collection_code, + "usage_key": f"lib-collection:org1:lib:{collection2.collection_code}", "type": "collection", "display_name": "Collection 2", "description": "Second Collection", @@ -768,8 +768,8 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None: } doc_collection2_updated = { "id": "lib-collectionorg1libcol2-46823d4d", - "block_id": collection2.key, - "usage_key": f"lib-collection:org1:lib:{collection2.key}", + "block_id": collection2.collection_code, + "usage_key": f"lib-collection:org1:lib:{collection2.collection_code}", "type": "collection", "display_name": "Collection 2", "description": "Second Collection", @@ -786,8 +786,8 @@ def test_index_library_block_and_collections(self, mock_meilisearch) -> None: } doc_collection1_updated = { "id": "lib-collectionorg1libcol1-283a79c9", - "block_id": collection1.key, - "usage_key": f"lib-collection:org1:lib:{collection1.key}", + "block_id": collection1.collection_code, + "usage_key": f"lib-collection:org1:lib:{collection1.collection_code}", "type": "collection", "display_name": "Collection 1", "description": "First Collection", @@ -904,7 +904,7 @@ def test_delete_collection(self, mock_meilisearch) -> None: with freeze_time(updated_date): library_api.update_library_collection_items( self.library.key, - collection_key=self.collection.key, + collection_key=self.collection.collection_code, opaque_keys=[ self.problem1.usage_key, self.unit.container_key @@ -918,14 +918,14 @@ def test_delete_collection(self, mock_meilisearch) -> None: "id": self.doc_problem1["id"], "collections": { "display_name": [self.collection.title], - "key": [self.collection.key], + "key": [self.collection.collection_code], }, } doc_unit_with_collection = { "id": self.unit_dict["id"], "collections": { "display_name": [self.collection.title], - "key": [self.collection.key], + "key": [self.collection.collection_code], }, } @@ -944,7 +944,7 @@ def test_delete_collection(self, mock_meilisearch) -> None: # Soft-delete the collection content_api.delete_collection( self.collection.learning_package_id, - self.collection.key, + self.collection.collection_code, ) doc_problem_without_collection = { @@ -979,7 +979,7 @@ def test_delete_collection(self, mock_meilisearch) -> None: with freeze_time(restored_date): content_api.restore_collection( self.collection.learning_package_id, - self.collection.key, + self.collection.collection_code, ) doc_collection = copy.deepcopy(self.collection_dict) @@ -1001,7 +1001,7 @@ def test_delete_collection(self, mock_meilisearch) -> None: # Hard-delete the collection content_api.delete_collection( self.collection.learning_package_id, - self.collection.key, + self.collection.collection_code, hard_delete=True, ) diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index a9aea3ab3cfb..ee4a1d613f61 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -492,7 +492,7 @@ def test_collection_with_library(self): assert doc == { "id": "lib-collectionedx2012_falltoy_collection-d1d907a4", - "block_id": self.collection.key, + "block_id": self.collection.collection_code, "usage_key": str(self.collection_key), "type": "collection", "org": "edX", @@ -521,7 +521,7 @@ def test_collection_with_published_library(self): assert doc == { "id": "lib-collectionedx2012_falltoy_collection-d1d907a4", - "block_id": self.collection.key, + "block_id": self.collection.collection_code, "usage_key": str(self.collection_key), "type": "collection", "org": "edX", diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index 2ddc06fde254..7aada0ecdc90 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -15,7 +15,7 @@ from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import validate_unicode_slug from django.db import transaction -from django.db.models import QuerySet +from django.db.models import F, QuerySet from django.urls import reverse from django.utils.text import slugify from django.utils.translation import gettext as _ @@ -25,20 +25,8 @@ from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_content import api as content_api from openedx_content.models_api import Collection, Component, ComponentVersion, Container, LearningPackage, MediaType -from openedx_events.content_authoring.data import ( - ContentObjectChangedData, - LibraryBlockData, - LibraryCollectionData, - LibraryContainerData, -) -from openedx_events.content_authoring.signals import ( - CONTENT_OBJECT_ASSOCIATIONS_CHANGED, - LIBRARY_BLOCK_CREATED, - LIBRARY_BLOCK_DELETED, - LIBRARY_BLOCK_UPDATED, - LIBRARY_COLLECTION_UPDATED, - LIBRARY_CONTAINER_UPDATED, -) +from openedx_events.content_authoring.data import LibraryBlockData +from openedx_events.content_authoring.signals import LIBRARY_BLOCK_DELETED from xblock.core import XBlock from openedx.core.djangoapps.content_staging.data import StagedContentID @@ -49,16 +37,13 @@ ) from openedx.core.types import User as UserType -from .. import tasks from ..models import ContentLibrary from .block_metadata import LibraryXBlockMetadata, LibraryXBlockStaticFile -from .collections import library_collection_locator from .container_metadata import container_subclass_for_olx_tag from .containers import ( ContainerMetadata, create_container, get_container, - get_containers_contains_item, update_container_children, ) from .exceptions import ( @@ -176,10 +161,13 @@ def get_library_block(usage_key: LibraryUsageLocatorV2, include_collections=Fals raise ContentLibraryBlockNotFound(usage_key) if include_collections: + # Temporarily alias collection_code to "key" so downstream consumers + # (search indexer, REST API) keep the same field name. We will update + # downstream consumers later: https://github.com/openedx/openedx-platform/issues/38406 associated_collections = content_api.get_entity_collections( component.learning_package_id, - component.key, - ).values('key', 'title') + component.entity_ref, + ).values("title", key=F('collection_code')) else: associated_collections = None xblock_metadata = LibraryXBlockMetadata.from_component( @@ -252,29 +240,6 @@ def set_library_block_olx(usage_key: LibraryUsageLocatorV2, new_olx_str: str) -> created=now, ) - # .. event_implemented_name: LIBRARY_BLOCK_UPDATED - # .. event_type: org.openedx.content_authoring.library_block.updated.v1 - transaction.on_commit(lambda: LIBRARY_BLOCK_UPDATED.send_event( - library_block=LibraryBlockData( - library_key=usage_key.context_key, - usage_key=usage_key - ) - )) - - # For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger - # container indexing asynchronously. - affected_containers = get_containers_contains_item(usage_key) - for container in affected_containers: - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - container_key = container.container_key - transaction.on_commit(lambda ck=container_key: LIBRARY_CONTAINER_UPDATED.send_event( # type: ignore[misc] - library_container=LibraryContainerData( - container_key=ck, - background=True, - ) - )) - return new_component_version @@ -351,16 +316,6 @@ def create_library_block( _create_component_for_block(content_library, usage_key, user_id, can_stand_alone) # Now return the metadata about the new block: - - # .. event_implemented_name: LIBRARY_BLOCK_CREATED - # .. event_type: org.openedx.content_authoring.library_block.created.v1 - LIBRARY_BLOCK_CREATED.send_event( - library_block=LibraryBlockData( - library_key=content_library.library_key, - usage_key=usage_key - ) - ) - return get_library_block(usage_key) @@ -425,7 +380,7 @@ def _import_staged_block( component = content_api.create_component( # noqa: F841 learning_package.id, component_type=component_type, - local_key=usage_key.block_id, + component_code=usage_key.block_id, created=now, created_by=user.id, ) @@ -490,19 +445,9 @@ def _import_staged_block( content_api.create_component_version_media( component_version.pk, content.id, - key=filename, + path=filename, ) - # Emit library block created event - # .. event_implemented_name: LIBRARY_BLOCK_CREATED - # .. event_type: org.openedx.content_authoring.library_block.created.v1 - transaction.on_commit(lambda: LIBRARY_BLOCK_CREATED.send_event( - library_block=LibraryBlockData( - library_key=content_library.library_key, - usage_key=usage_key - ) - )) - # Now return the metadata about the new block return get_library_block(usage_key) @@ -704,16 +649,6 @@ def delete_library_block( """ library_key = usage_key.context_key - def send_block_deleted_signal(): - # .. event_implemented_name: LIBRARY_BLOCK_DELETED - # .. event_type: org.openedx.content_authoring.library_block.deleted.v1 - LIBRARY_BLOCK_DELETED.send_event( - library_block=LibraryBlockData( - library_key=library_key, - usage_key=usage_key - ) - ) - try: component = get_component_from_usage_key(usage_key) except Component.DoesNotExist: @@ -722,46 +657,15 @@ def send_block_deleted_signal(): # (an intermediate error occurred). # In that case, we keep the index updated by removing the entry, # but still raise the error so the caller knows the component did not exist. - send_block_deleted_signal() - raise - affected_collections = content_api.get_entity_collections(component.learning_package_id, component.key) - affected_containers = get_containers_contains_item(usage_key) - - content_api.soft_delete_draft(component.id, deleted_by=user_id) - - send_block_deleted_signal() - - # For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger - # collection indexing asynchronously. - # - # To delete the component on collections - for collection in affected_collections: - # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 - LIBRARY_COLLECTION_UPDATED.send_event( - library_collection=LibraryCollectionData( - collection_key=library_collection_locator( - library_key=library_key, - collection_key=collection.key, - ), - background=True, - ) + # .. event_implemented_name: LIBRARY_BLOCK_DELETED + # .. event_type: org.openedx.content_authoring.library_block.deleted.v1 + LIBRARY_BLOCK_DELETED.send_event( + library_block=LibraryBlockData(library_key=library_key, usage_key=usage_key) ) + raise - # For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger - # container indexing asynchronously. - # - # To update the components count in containers - for container in affected_containers: - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=container.container_key, - background=True, - ) - ) + content_api.soft_delete_draft(component.id, deleted_by=user_id) def restore_library_block(usage_key: LibraryUsageLocatorV2, user_id: int | None = None) -> None: @@ -769,9 +673,6 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2, user_id: int | None Restore the specified library block. """ component = get_component_from_usage_key(usage_key) - library_key = usage_key.context_key - affected_collections = content_api.get_entity_collections(component.learning_package_id, component.key) - # Set draft version back to the latest available component version id. content_api.set_draft_version( component.id, @@ -779,57 +680,6 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2, user_id: int | None set_by=user_id, ) - # .. event_implemented_name: LIBRARY_BLOCK_CREATED - # .. event_type: org.openedx.content_authoring.library_block.created.v1 - LIBRARY_BLOCK_CREATED.send_event( - library_block=LibraryBlockData( - library_key=library_key, - usage_key=usage_key - ) - ) - - # Add tags and collections back to index - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(usage_key), - changes=["collections", "tags", "units"], - ), - ) - - # For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger - # collection indexing asynchronously. - # - # To restore the component in the collections - for collection in affected_collections: - # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 - LIBRARY_COLLECTION_UPDATED.send_event( - library_collection=LibraryCollectionData( - collection_key=library_collection_locator( - library_key=library_key, - collection_key=collection.key, - ), - background=True, - ) - ) - - # For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger - # container indexing asynchronously. - # - # To update the components count in containers - affected_containers = get_containers_contains_item(usage_key) - for container in affected_containers: - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=container.container_key, - background=True, - ) - ) - def get_library_block_static_asset_files(usage_key: LibraryUsageLocatorV2) -> list[LibraryXBlockStaticFile]: """ @@ -852,7 +702,7 @@ def get_library_block_static_asset_files(usage_key: LibraryUsageLocatorV2) -> li component_version .componentversionmedia_set .filter(media__has_file=True) - .order_by('key') + .order_by('path') .select_related('media') ) @@ -860,13 +710,13 @@ def get_library_block_static_asset_files(usage_key: LibraryUsageLocatorV2) -> li return [ LibraryXBlockStaticFile( - path=cvm.key, + path=cvm.path, size=cvm.media.size, url=site_root_url + reverse( 'content_libraries:library-assets', kwargs={ 'component_version_uuid': component_version.uuid, - 'asset_path': cvm.key, + 'asset_path': cvm.path, } ), ) @@ -915,16 +765,6 @@ def add_library_block_static_asset_file( created=datetime.now(tz=timezone.utc), # noqa: UP017 created_by=user.id if user else None, ) - transaction.on_commit( - # .. event_implemented_name: LIBRARY_BLOCK_UPDATED - # .. event_type: org.openedx.content_authoring.library_block.updated.v1 - lambda: LIBRARY_BLOCK_UPDATED.send_event( - library_block=LibraryBlockData( - library_key=usage_key.context_key, - usage_key=usage_key, - ) - ) - ) # Now figure out the URL for the newly created asset... site_root_url = get_xblock_app_config().get_site_root_url() @@ -963,16 +803,6 @@ def delete_library_block_static_asset_file(usage_key, file_path, user=None): created=now, created_by=user.id if user else None, ) - transaction.on_commit( - # .. event_implemented_name: LIBRARY_BLOCK_UPDATED - # .. event_type: org.openedx.content_authoring.library_block.updated.v1 - lambda: LIBRARY_BLOCK_UPDATED.send_event( - library_block=LibraryBlockData( - library_key=usage_key.context_key, - usage_key=usage_key, - ) - ) - ) def publish_component_changes(usage_key: LibraryUsageLocatorV2, user_id: int): @@ -985,15 +815,9 @@ def publish_component_changes(usage_key: LibraryUsageLocatorV2, user_id: int): learning_package = content_library.learning_package assert learning_package # The core publishing API is based on draft objects, so find the draft that corresponds to this component: - drafts_to_publish = content_api.get_all_drafts(learning_package.id).filter(entity__key=component.key) + drafts_to_publish = content_api.get_all_drafts(learning_package.id).filter(entity__entity_ref=component.entity_ref) # Publish the component and update anything that needs to be updated (e.g. search index): - publish_log = content_api.publish_from_drafts( - learning_package.id, draft_qset=drafts_to_publish, published_by=user_id, - ) - # Since this is a single component, it should be safe to process synchronously and in-process: - tasks.send_events_after_publish(publish_log.pk, str(library_key)) - # IF this is found to be a performance issue, we could instead make it async where necessary: - # tasks.wait_for_post_publish_events(publish_log, library_key=library_key) + content_api.publish_from_drafts(learning_package.id, draft_qset=drafts_to_publish, published_by=user_id) def _component_exists(usage_key: UsageKeyV2) -> bool: @@ -1046,7 +870,7 @@ def _create_component_for_block( component, component_version = content_api.create_component_and_version( learning_package.id, component_type=component_type, - local_key=usage_key.block_id, + component_code=usage_key.block_id, title=display_name, created=now, created_by=user_id, @@ -1061,7 +885,7 @@ def _create_component_for_block( content_api.create_component_version_media( component_version.pk, content.id, - key="block.xml", + path="block.xml", ) return component_version diff --git a/openedx/core/djangoapps/content_libraries/api/collections.py b/openedx/core/djangoapps/content_libraries/api/collections.py index 9d011bdae363..a8b715750d64 100644 --- a/openedx/core/djangoapps/content_libraries/api/collections.py +++ b/openedx/core/djangoapps/content_libraries/api/collections.py @@ -2,14 +2,11 @@ Python API for library collections ================================== """ -from django.db import IntegrityError from opaque_keys import OpaqueKey from opaque_keys.edx.keys import BlockTypeKey, UsageKeyV2 from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator, LibraryLocatorV2 from openedx_content import api as content_api from openedx_content.models_api import Collection, Component, PublishableEntity -from openedx_events.content_authoring.data import LibraryCollectionData -from openedx_events.content_authoring.signals import LIBRARY_COLLECTION_UPDATED from ..models import ContentLibrary from .exceptions import ( @@ -51,18 +48,18 @@ def create_library_collection( assert content_library.learning_package_id assert content_library.library_key == library_key - try: - collection = content_api.create_collection( - learning_package_id=content_library.learning_package_id, - key=collection_key, - title=title, - description=description, - created_by=created_by, - ) - except IntegrityError as err: - raise LibraryCollectionAlreadyExists from err - - return collection + if Collection.objects.filter( + learning_package_id=content_library.learning_package_id, + collection_code=collection_key, + ).exists(): + raise LibraryCollectionAlreadyExists(f"Collection {collection_key} already exists in {library_key}") + return content_api.create_collection( + learning_package_id=content_library.learning_package_id, + collection_code=collection_key, + title=title, + description=description, + created_by=created_by, + ) def update_library_collection( @@ -86,7 +83,7 @@ def update_library_collection( try: collection = content_api.update_collection( learning_package_id=content_library.learning_package_id, - key=collection_key, + collection_code=collection_key, title=title, description=description, ) @@ -127,39 +124,39 @@ def update_library_collection_items( assert content_library.learning_package_id assert content_library.library_key == library_key - # Fetch the Component.key values for the provided UsageKeys. - item_keys = [] + # Fetch the Component.entity_ref values for the provided UsageKeys. + item_refs = [] for opaque_key in opaque_keys: if isinstance(opaque_key, LibraryContainerLocator): try: - container = content_api.get_container_by_key( + container = content_api.get_container_by_code( content_library.learning_package_id, - key=opaque_key.container_id, + container_code=opaque_key.container_id, ) except Collection.DoesNotExist as exc: raise ContentLibraryContainerNotFound(opaque_key) from exc - item_keys.append(container.key) + item_refs.append(container.entity_ref) elif isinstance(opaque_key, UsageKeyV2): # Parse the block_family from the key to use as namespace. block_type = BlockTypeKey.from_string(str(opaque_key)) try: - component = content_api.get_component_by_key( + component = content_api.get_component_by_code( content_library.learning_package_id, namespace=block_type.block_family, type_name=opaque_key.block_type, - local_key=opaque_key.block_id, + component_code=opaque_key.block_id, ) except Component.DoesNotExist as exc: raise ContentLibraryBlockNotFound(opaque_key) from exc - item_keys.append(component.key) + item_refs.append(component.entity_ref) else: # This should never happen, but just in case. raise ValueError(f"Invalid opaque_key: {opaque_key}") entities_qset = PublishableEntity.objects.filter( - key__in=item_keys, + entity_ref__in=item_refs, ) if remove: @@ -181,7 +178,7 @@ def update_library_collection_items( def set_library_item_collections( library_key: LibraryLocatorV2, - entity_key: str, + entity_ref: str, *, collection_keys: list[str], created_by: int | None = None, @@ -207,37 +204,22 @@ def set_library_item_collections( assert content_library.learning_package_id assert content_library.library_key == library_key - publishable_entity = content_api.get_publishable_entity_by_key( + publishable_entity = content_api.get_publishable_entity_by_ref( content_library.learning_package_id, - key=entity_key, + entity_ref=entity_ref, ) - # Note: Component.key matches its PublishableEntity.key + # Note: Component.entity_ref matches its PublishableEntity.entity_ref collection_qs = content_api.get_collections(content_library.learning_package_id).filter( - key__in=collection_keys + collection_code__in=collection_keys ) - affected_collections = content_api.set_collections( + content_api.set_collections( publishable_entity, collection_qs, created_by=created_by, ) - # For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger - # collection indexing asynchronously. - for collection in affected_collections: - # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 - LIBRARY_COLLECTION_UPDATED.send_event( - library_collection=LibraryCollectionData( - collection_key=library_collection_locator( - library_key=library_key, - collection_key=collection.key, - ), - background=True, - ) - ) - return publishable_entity diff --git a/openedx/core/djangoapps/content_libraries/api/container_metadata.py b/openedx/core/djangoapps/content_libraries/api/container_metadata.py index 5bb8bcd50ae4..98a5024ac674 100644 --- a/openedx/core/djangoapps/content_libraries/api/container_metadata.py +++ b/openedx/core/djangoapps/content_libraries/api/container_metadata.py @@ -365,8 +365,9 @@ def library_container_locator( container_type_code = content_api.get_container_type_code_of(container) if container_type_code not in LIBRARY_ALLOWED_CONTAINER_TYPES: raise ValueError(f"Unsupported container type for content libraries: {container!r}") - - return LibraryContainerLocator(library_key, container_type=container_type_code, container_id=container.key) + return LibraryContainerLocator( + library_key, container_type=container_type_code, container_id=container.container_code, + ) def get_container_from_key(container_key: LibraryContainerLocator, include_deleted=False) -> Container: @@ -379,7 +380,7 @@ def get_container_from_key(container_key: LibraryContainerLocator, include_delet content_library = ContentLibrary.objects.get_by_key(container_key.lib_key) learning_package = content_library.learning_package assert learning_package is not None - container = content_api.get_container_by_key(learning_package.id, key=container_key.container_id) + container = content_api.get_container_by_code(learning_package.id, container_code=container_key.container_id) assert content_api.get_container_type_code_of(container) in LIBRARY_ALLOWED_CONTAINER_TYPES # We only return the container if it exists and either: # 1. the container has a draft version (which means it is not soft-deleted) OR diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 5db34dc753da..92707bb860b5 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -9,23 +9,14 @@ from datetime import datetime, timezone from uuid import uuid4 -from django.db import transaction +from django.db.models import F from django.utils.text import slugify from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_content import api as content_api -from openedx_content.models_api import Container, Unit -from openedx_events.content_authoring.data import ContentObjectChangedData, LibraryCollectionData, LibraryContainerData -from openedx_events.content_authoring.signals import ( - CONTENT_OBJECT_ASSOCIATIONS_CHANGED, - LIBRARY_COLLECTION_UPDATED, - LIBRARY_CONTAINER_CREATED, - LIBRARY_CONTAINER_DELETED, - LIBRARY_CONTAINER_UPDATED, -) - -from openedx.core.djangoapps.content_libraries.api.collections import library_collection_locator +from openedx_content.models_api import Container +from openedx_events.content_authoring.data import LibraryContainerData +from openedx_events.content_authoring.signals import LIBRARY_CONTAINER_DELETED -from .. import tasks from ..models import ContentLibrary from .block_metadata import LibraryXBlockMetadata from .container_metadata import ( @@ -73,10 +64,13 @@ def get_container( """ container = get_container_from_key(container_key) if include_collections: + # Temporarily alias collection_code to "key" so downstream consumers + # (search indexer, REST API) keep the same field name. We will update + # downstream consumers later: https://github.com/openedx/openedx-platform/issues/38406 associated_collections = content_api.get_entity_collections( container.publishable_entity.learning_package_id, container_key.container_id, - ).values("key", "title") + ).values("title", key=F("collection_code")) else: associated_collections = None container_meta = ContainerMetadata.from_container( @@ -109,7 +103,7 @@ def create_container( # Automatically generate a slug. Append a random suffix so it should be unique. slug = slugify(title, allow_unicode=True) + "-" + uuid4().hex[-6:] # Make sure the slug is valid by first creating a key for the new container: - container_key = LibraryContainerLocator( + _container_key = LibraryContainerLocator( library_key, container_type=container_cls.type_code, container_id=slug, @@ -121,7 +115,7 @@ def create_container( # Then try creating the actual container: container, _initial_version = content_api.create_container_and_version( content_library.learning_package_id, - key=slug, + container_code=slug, title=title, container_cls=container_cls, entities=[], @@ -129,14 +123,6 @@ def create_container( created_by=user_id, ) - # .. event_implemented_name: LIBRARY_CONTAINER_CREATED - # .. event_type: org.openedx.content_authoring.content_library.container.created.v1 - transaction.on_commit(lambda: LIBRARY_CONTAINER_CREATED.send_event( - library_container=LibraryContainerData( - container_key=container_key, - ) - )) - return ContainerMetadata.from_container(library_key, container) @@ -152,9 +138,6 @@ def update_container( library_key = container_key.lib_key created = datetime.now(tz=timezone.utc) # noqa: UP017 - # Get children containers or components to update their index data - children = get_container_children(container_key, published=False) - version = content_api.create_next_container_version( container, title=display_name, @@ -162,34 +145,6 @@ def update_container( created_by=user_id, ) - # Send event related to the updated container - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event(library_container=LibraryContainerData(container_key=container_key)) - - # Send events related to the containers that contains the updated container. - # This is to update the children display names used in the section/subsection previews. - affected_containers = get_containers_contains_item(container_key) - for affected_container in affected_containers: - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData(container_key=affected_container.container_key) - ) - # Update children components and containers index data, for example, - # All subsections under a section have section key in index that needs to be updated. - # So if parent section name has been changed, it needs to be reflected in sections key of children - is_unit = container_key.container_type == Unit.type_code - for child in children: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(child.usage_key if is_unit else child.container_key), # type: ignore[union-attr] - changes=[container_key.container_type + "s"], # e.g. "units" - ), - ) - return ContainerMetadata.from_container(library_key, version.container) @@ -201,15 +156,6 @@ def delete_container( No-op if container doesn't exist or has already been soft-deleted. """ - def send_container_deleted_signal(): - # .. event_implemented_name: LIBRARY_CONTAINER_DELETED - # .. event_type: org.openedx.content_authoring.content_library.container.deleted.v1 - LIBRARY_CONTAINER_DELETED.send_event( - library_container=LibraryContainerData( - container_key=container_key, - ) - ) - try: container = get_container_from_key(container_key) except Container.DoesNotExist: @@ -218,149 +164,20 @@ def send_container_deleted_signal(): # (an intermediate error occurred). # In that case, we keep the index updated by removing the entry, # but still raise the error so the caller knows the container did not exist. - send_container_deleted_signal() + # .. event_implemented_name: LIBRARY_CONTAINER_DELETED + # .. event_type: org.openedx.content_authoring.content_library.container.deleted.v1 + LIBRARY_CONTAINER_DELETED.send_event(library_container=LibraryContainerData(container_key=container_key)) raise - library_key = container_key.lib_key - - # Fetch related collections and containers before soft-delete - affected_collections = content_api.get_entity_collections( - container.publishable_entity.learning_package_id, - container.key, - ) - affected_containers = get_containers_contains_item(container_key) - # Get children containers or components to update their index data - children = get_container_children( - container_key, - published=False, - ) content_api.soft_delete_draft(container.id) - send_container_deleted_signal() - - # For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger - # collection indexing asynchronously. - # - # To delete the container on collections - for collection in affected_collections: - # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 - LIBRARY_COLLECTION_UPDATED.send_event( - library_collection=LibraryCollectionData( - collection_key=library_collection_locator( - library_key=library_key, - collection_key=collection.key, - ), - background=True, - ) - ) - # Send events related to the containers that contains the updated container. - # This is to update the children display names used in the section/subsection previews. - for affected_container in affected_containers: - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=affected_container.container_key, - ) - ) - key_name = "container_key" - if isinstance(container, Unit): - # Components have usage_key instead of container_key - key_name = "usage_key" - # Update children components and containers index data, for example, - # All subsections under a section have section key in index that needs to be updated. - # So if parent section is deleted, it needs to be removed from sections key of children - for child in children: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(getattr(child, key_name)), - changes=[container_key.container_type + "s"], - ), - ) - def restore_container(container_key: LibraryContainerLocator) -> None: """ [ 🛑 UNSTABLE ] Restore the specified library container. """ - library_key = container_key.lib_key container = get_container_from_key(container_key, include_deleted=True) - - affected_collections = content_api.get_entity_collections( - container.publishable_entity.learning_package_id, - container.key, - ) - content_api.set_draft_version(container.id, container.versioning.latest.pk) - # Fetch related containers after restore - affected_containers = get_containers_contains_item(container_key) - # Get children containers or components to update their index data - children = get_container_children(container_key, published=False) - - # .. event_implemented_name: LIBRARY_CONTAINER_CREATED - # .. event_type: org.openedx.content_authoring.content_library.container.created.v1 - LIBRARY_CONTAINER_CREATED.send_event( - library_container=LibraryContainerData( - container_key=container_key, - ) - ) - - content_changes = ["collections", "tags"] - if affected_containers and len(affected_containers) > 0: - # Update parent key data in index. Eg. `sections` key in index for subsection - content_changes.append(str(affected_containers[0].container_type_code) + "s") - # Add tags, collections and parent data back to index - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(container_key), - changes=content_changes, - ), - ) - - # For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger - # collection indexing asynchronously. - # - # To restore the container on collections - for collection in affected_collections: - # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 - LIBRARY_COLLECTION_UPDATED.send_event( - library_collection=LibraryCollectionData( - collection_key=library_collection_locator( - library_key=library_key, - collection_key=collection.key, - ), - ) - ) - # Send events related to the containers that contains the updated container. - # This is to update the children display names used in the section/subsection previews. - for affected_container in affected_containers: - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=affected_container.container_key, - ) - ) - - is_unit = container_key.container_type == Unit.type_code - # Update children components and containers index data, for example, - # All subsections under a section have section key in index that needs to be updated. - # Should restore removed parent section in sections key of children subsections - for child in children: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(child.usage_key if is_unit else child.container_key), # type: ignore[union-attr] - changes=[container_key.container_type + "s"], - ), - ) def get_container_children( @@ -415,23 +232,6 @@ def update_container_children( entities=[get_entity_from_key(key) for key in children_keys], entities_action=entities_action, ) - for key in children_keys: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(key), - changes=[f"{container_key.container_type}s"], # "units", "subsections", "sections" - ), - ) - - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=container_key, - ) - ) return ContainerMetadata.from_container(container_key.lib_key, new_version.container) @@ -448,7 +248,6 @@ def get_containers_contains_item(key: LibraryUsageLocatorV2 | LibraryContainerLo def publish_container_changes( container_key: LibraryContainerLocator, user_id: int | None, - call_post_publish_events_sync=False, ) -> None: """ [ 🛑 UNSTABLE ] Publish all unpublished changes in a container and all its child @@ -462,17 +261,7 @@ def publish_container_changes( # The core publishing API is based on draft objects, so find the draft that corresponds to this container: drafts_to_publish = content_api.get_all_drafts(learning_package.id).filter(entity__pk=container.id) # Publish the container, which will also auto-publish any unpublished child components: - publish_log = content_api.publish_from_drafts( - learning_package.id, - draft_qset=drafts_to_publish, - published_by=user_id, - ) - # Update the search index (and anything else) for the affected container + blocks - # This is mostly synchronous but may complete some work asynchronously if there are a lot of changes. - if call_post_publish_events_sync: - tasks.send_events_after_publish(publish_log.pk, str(library_key)) - else: - tasks.wait_for_post_publish_events(publish_log, library_key) + content_api.publish_from_drafts(learning_package.id, draft_qset=drafts_to_publish, published_by=user_id) def copy_container(container_key: LibraryContainerLocator, user_id: int) -> UserClipboardData: diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index bf91039b686b..bfce8c2d5959 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -72,7 +72,7 @@ from openedx.core.types import User as UserType -from .. import permissions, tasks +from .. import permissions from ..constants import ALL_RIGHTS_RESERVED from ..models import ContentLibrary, ContentLibraryPermission from .exceptions import LibraryAlreadyExists, LibraryPermissionIntegrityError @@ -461,14 +461,14 @@ def create_library( # and also update its title/description in case they differ. content_api.update_learning_package( learning_package.id, - key=str(ref.library_key), + package_ref=str(ref.library_key), title=title, description=description, ) else: # We have to generate a new LearningPackage for this library. learning_package = content_api.create_learning_package( - key=str(ref.library_key), + package_ref=str(ref.library_key), title=title, description=description, ) @@ -718,7 +718,7 @@ def library_component_usage_key( return LibraryUsageLocatorV2( # type: ignore[abstract] library_key, block_type=component.component_type.name, - usage_id=component.local_key, + usage_id=component.component_code, ) @@ -758,15 +758,7 @@ def publish_changes(library_key: LibraryLocatorV2, user_id: int | None = None): """ learning_package = ContentLibrary.objects.get_by_key(library_key).learning_package assert learning_package is not None # shouldn't happen but it's technically possible. - publish_log = content_api.publish_all_drafts(learning_package.id, published_by=user_id) - - # Update the search index (and anything else) for the affected blocks - # This is mostly synchronous but may complete some work asynchronously if there are a lot of changes. - tasks.wait_for_post_publish_events(publish_log, library_key) - - # Unlike revert_changes below, we do not have to re-index collections, - # because publishing changes does not affect the component counts, and - # collections themselves don't have draft/published/unpublished status. + content_api.publish_all_drafts(learning_package.id, published_by=user_id) def revert_changes(library_key: LibraryLocatorV2, user_id: int | None = None) -> None: @@ -776,12 +768,9 @@ def revert_changes(library_key: LibraryLocatorV2, user_id: int | None = None) -> """ learning_package = ContentLibrary.objects.get_by_key(library_key).learning_package assert learning_package is not None # shouldn't happen but it's technically possible. - with content_api.bulk_draft_changes_for(learning_package.id) as draft_change_log: + with content_api.bulk_draft_changes_for(learning_package.id): content_api.reset_drafts_to_published(learning_package.id, reset_by=user_id) - # Call the event handlers as needed. - tasks.wait_for_post_revert_events(draft_change_log, library_key) - def get_backup_task_status( user_id: int, diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py index bd91a3f89250..7cba96e0da94 100644 --- a/openedx/core/djangoapps/content_libraries/library_context.py +++ b/openedx/core/djangoapps/content_libraries/library_context.py @@ -7,8 +7,6 @@ from opaque_keys.edx.keys import UsageKeyV2 from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_content import api as content_api -from openedx_events.content_authoring.data import LibraryBlockData, LibraryContainerData -from openedx_events.content_authoring.signals import LIBRARY_BLOCK_UPDATED, LIBRARY_CONTAINER_UPDATED from rest_framework.exceptions import NotFound from openedx.core.djangoapps.content_libraries import api, permissions @@ -95,40 +93,9 @@ def block_exists(self, usage_key: LibraryUsageLocatorV2): if learning_package is None: return False - return content_api.component_exists_by_key( + return content_api.component_exists_by_code( learning_package.id, namespace='xblock.v1', type_name=usage_key.block_type, - local_key=usage_key.block_id, + component_code=usage_key.block_id, ) - - def send_block_updated_event(self, usage_key: UsageKeyV2): - """ - Send a "block updated" event for the library block with the given usage_key. - """ - assert isinstance(usage_key, LibraryUsageLocatorV2) - # .. event_implemented_name: LIBRARY_BLOCK_UPDATED - # .. event_type: org.openedx.content_authoring.library_block.updated.v1 - LIBRARY_BLOCK_UPDATED.send_event( - library_block=LibraryBlockData( - library_key=usage_key.lib_key, - usage_key=usage_key, - ) - ) - - def send_container_updated_events(self, usage_key: UsageKeyV2): - """ - Send "container updated" events for containers that contains the library block - with the given usage_key. - """ - assert isinstance(usage_key, LibraryUsageLocatorV2) - affected_containers = api.get_containers_contains_item(usage_key) - for container in affected_containers: - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=container.container_key, - background=True, - ) - ) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py index 518ac5b31fe3..8dd70a069edb 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py @@ -272,7 +272,7 @@ def patch(self, request: RestRequest, usage_key_str) -> Response: collection_keys = serializer.validated_data['collection_keys'] api.set_library_item_collections( library_key=key.lib_key, - entity_key=component.publishable_entity.key, + entity_ref=component.publishable_entity.entity_ref, collection_keys=collection_keys, created_by=request.user.id, content_library=content_library, @@ -379,7 +379,7 @@ def get_component_version_asset(request, component_version_uuid, asset_path): # Permissions check... learning_package = component_version.component.learning_package - library_key = LibraryLocatorV2.from_string(learning_package.key) + library_key = LibraryLocatorV2.from_string(learning_package.package_ref) api.require_permission_for_library_key( library_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) @@ -402,7 +402,7 @@ def get_component_version_asset(request, component_version_uuid, asset_path): return redirect_response # If we got here, we know that the asset exists and it's okay to download. - cv_media = component_version.componentversionmedia_set.get(key=asset_path) + cv_media = component_version.componentversionmedia_set.get(path=asset_path) media = cv_media.media # Delete the re-direct part of the response headers. We'll copy the rest. diff --git a/openedx/core/djangoapps/content_libraries/rest_api/collections.py b/openedx/core/djangoapps/content_libraries/rest_api/collections.py index 3f67f5e777a8..9875f31d79d5 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/collections.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/collections.py @@ -33,7 +33,10 @@ class LibraryCollectionsView(ModelViewSet): """ serializer_class = ContentLibraryCollectionSerializer - lookup_field = 'key' + # URL kwarg is `key` for backwards compatibility. + # https://github.com/openedx/openedx-platform/issues/38406 + lookup_field = 'collection_code' + lookup_url_kwarg = 'key' def __init__(self, *args, **kwargs) -> None: """ @@ -184,7 +187,7 @@ def destroy(self, request: RestRequest, *args, **kwargs) -> Response: assert collection.learning_package_id content_api.delete_collection( collection.learning_package_id, - collection.key, + collection.collection_code, hard_delete=False, ) return Response(None, status=HTTP_204_NO_CONTENT) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 04dde384361e..12a4132920c3 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -346,7 +346,7 @@ def patch(self, request: RestRequest, container_key: LibraryContainerLocator) -> collection_keys = serializer.validated_data['collection_keys'] api.set_library_item_collections( library_key=container_key.lib_key, - entity_key=container_key.container_id, + entity_ref=container_key.container_id, collection_keys=collection_keys, created_by=request.user.id, content_library=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 72c59f695833..f8dd8b18a839 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -284,10 +284,13 @@ class ContentLibraryCollectionSerializer(serializers.ModelSerializer): """ Serializer for a Content Library Collection """ + # Expose Collection.collection_code as "key" to preserve the REST API field name. + # https://github.com/openedx/openedx-platform/issues/38406 + key = serializers.CharField(source='collection_code') class Meta: model = Collection - fields = '__all__' + exclude = ['collection_code'] class ContentLibraryCollectionUpdateSerializer(serializers.Serializer): @@ -447,14 +450,16 @@ class RestoreSuccessDataSerializer(serializers.Serializer): """ learning_package_id = serializers.IntegerField(source="lp_restored_data.id") title = serializers.CharField(source="lp_restored_data.title") - org = serializers.CharField(source="lp_restored_data.archive_org_key") - slug = serializers.CharField(source="lp_restored_data.archive_slug") + org = serializers.SerializerMethodField() + slug = serializers.SerializerMethodField() - # The `key` is a unique temporary key assigned to the learning package during the restore process, - # whereas the `archive_key` is the original key of the learning package from the backup. - # The temporary learning package key is replaced with a standard key once it is added to a content library. - key = serializers.CharField(source="lp_restored_data.key") - archive_key = serializers.CharField(source="lp_restored_data.archive_lp_key") + # The `package_ref` is a unique temporary key assigned to the learning + # package during the restore process, whereas the `archive_package_ref` is + # the original key of the learning package from the backup. The temporary + # learning package_ref is replaced with a standard key once it is added to a + # content library. + key = serializers.CharField(source="lp_restored_data.package_ref") + archive_key = serializers.CharField(source="lp_restored_data.archive_package_ref") containers = serializers.IntegerField(source="lp_restored_data.num_containers") components = serializers.IntegerField(source="lp_restored_data.num_components") @@ -467,6 +472,18 @@ class RestoreSuccessDataSerializer(serializers.Serializer): created_at = serializers.DateTimeField(source="backup_metadata.created_at", format=DATETIME_FORMAT) created_by = serializers.SerializerMethodField() + def get_org(self, obj) -> str: + """ + The org code/slug, as parsed from archive_package_ref, or "unknown" if unparseable. + """ + return obj["lp_restored_data"]["archive_org_code"] or "unknown" + + def get_slug(self, obj) -> str: + """ + The library code/slug, as parsed from archive_package_ref, or "unknown" if unparseable. + """ + return obj["lp_restored_data"]["archive_package_code"] or "unknown" + def get_created_by(self, obj): """ Get the user information of the archive creator, if available. diff --git a/openedx/core/djangoapps/content_libraries/signal_handlers.py b/openedx/core/djangoapps/content_libraries/signal_handlers.py index 041a49b473e0..99647e389f39 100644 --- a/openedx/core/djangoapps/content_libraries/signal_handlers.py +++ b/openedx/core/djangoapps/content_libraries/signal_handlers.py @@ -3,180 +3,156 @@ """ import logging +from functools import partial -from django.db.models.signals import m2m_changed, post_delete, post_save +from attrs import asdict from django.dispatch import receiver -from opaque_keys import OpaqueKey -from opaque_keys.edx.locator import LibraryLocatorV2 -from openedx_content.api import get_components, get_containers -from openedx_content.models_api import Collection, CollectionPublishableEntity, PublishableEntity -from openedx_events.content_authoring.data import ContentObjectChangedData, LibraryCollectionData +from openedx_content.api import signals as content_signals +from openedx_events.content_authoring.data import LibraryCollectionData from openedx_events.content_authoring.signals import ( - CONTENT_OBJECT_ASSOCIATIONS_CHANGED, LIBRARY_COLLECTION_CREATED, LIBRARY_COLLECTION_DELETED, LIBRARY_COLLECTION_UPDATED, ) -from .api import library_collection_locator, library_component_usage_key, library_container_locator +from . import tasks +from .api import library_collection_locator from .models import ContentLibrary log = logging.getLogger(__name__) -@receiver(post_save, sender=Collection, dispatch_uid="library_collection_saved") -def library_collection_saved(sender, instance, created, **kwargs): - """ - Raises LIBRARY_COLLECTION_CREATED if the Collection is new, - or LIBRARY_COLLECTION_UPDATED if updated an existing Collection. +@receiver(content_signals.LEARNING_PACKAGE_ENTITIES_CHANGED) +def entities_updated( + learning_package: content_signals.LearningPackageEventData, + change_log: content_signals.DraftChangeLogEventData, + **kwargs, +) -> None: """ - try: - library = ContentLibrary.objects.get(learning_package_id=instance.learning_package_id) - except ContentLibrary.DoesNotExist: - log.error("{instance} is not associated with a content library.") - return - - if created: - # .. event_implemented_name: LIBRARY_COLLECTION_CREATED - # .. event_type: org.openedx.content_authoring.content_library.collection.created.v1 - LIBRARY_COLLECTION_CREATED.send_event( - library_collection=LibraryCollectionData( - collection_key=library_collection_locator( - library_key=library.library_key, - collection_key=instance.key, - ), - ) - ) - else: - # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 - LIBRARY_COLLECTION_UPDATED.send_event( - library_collection=LibraryCollectionData( - collection_key=library_collection_locator( - library_key=library.library_key, - collection_key=instance.key, - ), - ) - ) + Entities (containers/components) have been changed - handle that as needed. + We receive this low-level event from `openedx_content`, and check if it + happened in a library. If so, we emit more detailed library-specific events. -@receiver(post_delete, sender=Collection, dispatch_uid="library_collection_deleted") -def library_collection_deleted(sender, instance, **kwargs): - """ - Raises LIBRARY_COLLECTION_DELETED for the deleted Collection. + 💾 This event is only received after the transaction has committed. + ⏳ This event is emitted synchronously and this handler is called + synchronously. If a lot of entities were changed, we need to dispatch an + asynchronous handler to deal with them to avoid slowdowns. If only one + entity is changed, we want to deal with that synchronously so that we + can show the user correct data when the current requests completes. """ try: - library = ContentLibrary.objects.get(learning_package_id=instance.learning_package_id) + ContentLibrary.objects.get(learning_package_id=learning_package.id) except ContentLibrary.DoesNotExist: - log.error("{instance} is not associated with a content library.") - return - - # .. event_implemented_name: LIBRARY_COLLECTION_DELETED - # .. event_type: org.openedx.content_authoring.content_library.collection.deleted.v1 - LIBRARY_COLLECTION_DELETED.send_event( - library_collection=LibraryCollectionData( - collection_key=library_collection_locator( - library_key=library.library_key, - collection_key=instance.key, - ), - ) - ) + return # We don't care about non-library events. + + # Which entities were _directly_ changed here? + direct_changes = [asdict(change) for change in change_log.changes if change.new_version != change.old_version] + # And which entities were indirectly affected (e.g. parent containers)? + indirect_changes = [asdict(change) for change in change_log.changes if change.new_version == change.old_version] + + update_task_fn = tasks.send_change_events_for_modified_entities + update_sync = partial(update_task_fn, learning_package_id=learning_package.id) + update_async = partial(update_task_fn.delay, learning_package_id=learning_package.id) + + if len(direct_changes) == 1: + # We directly changed only one entity. Update it synchronously so that the UI will reflect changes right away. + if len(indirect_changes) <= 1: + # And update any other affected entity synchronously too; there's at most one. (More efficient, better UX.) + update_sync(change_list=[*direct_changes, *indirect_changes]) + else: + update_sync(change_list=direct_changes) # Update this one entity synchronously, and + update_async(change_list=indirect_changes) # update the many other affects entities async. + else: + # More than one entity was changed at once. Handle asynchronously: + update_async(change_list=[*direct_changes, *indirect_changes]) -def _library_collection_entity_changed( - publishable_entity: PublishableEntity, - library_key: LibraryLocatorV2 | None = None, +@receiver(content_signals.LEARNING_PACKAGE_ENTITIES_PUBLISHED) +def entities_published( + learning_package: content_signals.LearningPackageEventData, + change_log: content_signals.PublishLogEventData, + **kwargs, ) -> None: """ - Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for the entity. - """ - if not library_key: - try: - library = ContentLibrary.objects.get( - learning_package_id=publishable_entity.learning_package_id, - ) - except ContentLibrary.DoesNotExist: - log.error("{publishable_entity} is not associated with a content library.") - return + Entities (containers/components) have been published - handle that as needed. - library_key = library.library_key + We receive this low-level event from `openedx_content`, and check if it + happened in a library. If so, we emit more detailed library-specific events. - assert library_key - - opaque_key: OpaqueKey + 💾 This event is only received after the transaction has committed. + ⏳ This event is emitted synchronously and this handler is called + synchronously. If a lot of entities were published, we need to dispatch + an asynchronous handler to deal with them to avoid slowdowns. If only one + entity was published, we want to deal with that synchronously so that we + can show the user correct data when the current requests completes. + """ + try: + library = ContentLibrary.objects.get(learning_package_id=learning_package.id) + except ContentLibrary.DoesNotExist: + return # We don't care about non-library events. - if hasattr(publishable_entity, 'component'): - opaque_key = library_component_usage_key( - library_key, - publishable_entity.component, - ) - elif hasattr(publishable_entity, 'container'): - opaque_key = library_container_locator( - library_key, - publishable_entity.container, - ) + if len(change_log.changes) == 1: + fn = tasks.send_events_after_publish else: - log.error("Unknown publishable entity type: %s", publishable_entity) - return + # More than one entity was published at once. Handle asynchronously: + fn = tasks.send_events_after_publish.delay - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(opaque_key), - changes=["collections"], - ), - ) + fn(publish_log_id=change_log.publish_log_id, library_key_str=str(library.library_key)) -@receiver(post_save, sender=CollectionPublishableEntity, dispatch_uid="library_collection_entity_saved") -def library_collection_entity_saved(sender, instance, created, **kwargs): - """ - Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for components added to a collection. +@receiver(content_signals.LEARNING_PACKAGE_COLLECTION_CHANGED) +def collection_updated( + learning_package: content_signals.LearningPackageEventData, + change: content_signals.CollectionChangeData, + **kwargs, +) -> None: """ - if created: - _library_collection_entity_changed(instance.entity) + A Collection has been updated - handle that as needed. + We receive this low-level event from `openedx_content`, and check if it + happened in a library. If so, we emit more detailed library-specific events. -@receiver(post_delete, sender=CollectionPublishableEntity, dispatch_uid="library_collection_entity_deleted") -def library_collection_entity_deleted(sender, instance, **kwargs): - """ - Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for components removed from a collection. + ⏳ This event is emitted synchronously and this handler is called + synchronously. If a lot of entities were changed, we need to dispatch an + asynchronous handler to deal with them to avoid slowdowns. """ - # Only trigger component updates if CollectionPublishableEntity was cascade deleted due to deletion of a collection. - if isinstance(kwargs.get('origin'), Collection): - _library_collection_entity_changed(instance.entity) - + try: + library = ContentLibrary.objects.get(learning_package_id=learning_package.id) + except ContentLibrary.DoesNotExist: + return # We don't care about non-library events. -@receiver(m2m_changed, sender=CollectionPublishableEntity, dispatch_uid="library_collection_entities_changed") -def library_collection_entities_changed(sender, instance, action, pk_set, **kwargs): - """ - Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for components added/removed/cleared from a collection. - """ - if action not in ["post_add", "post_remove", "post_clear"]: - return + collection_key = library_collection_locator(library_key=library.library_key, collection_key=change.collection_code) + entities_changed = change.entities_added + change.entities_removed - try: - library = ContentLibrary.objects.get( - learning_package_id=instance.learning_package_id, + if change.created: # This is a newly-created collection, or was "un-deleted": + # .. event_implemented_name: LIBRARY_COLLECTION_CREATED + # .. event_type: org.openedx.content_authoring.content_library.collection.created.v1 + LIBRARY_COLLECTION_CREATED.send_event(library_collection=LibraryCollectionData(collection_key=collection_key)) + # As an example of what this event triggers, Collections are listed in the Meilisearch index as items in the + # library. So the handler will add this Collection as an entry in the Meilisearch index. + elif change.metadata_modified or entities_changed: + # The collection was renamed or its items were changed. + # This event is ambiguous but because the search index of the collection itself may have something like + # "contains 15 items", we _do_ need to emit it even when only the items have changed and not the metadata. + # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED + # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 + LIBRARY_COLLECTION_UPDATED.send_event(library_collection=LibraryCollectionData(collection_key=collection_key)) + elif change.deleted: + # .. event_implemented_name: LIBRARY_COLLECTION_DELETED + # .. event_type: org.openedx.content_authoring.content_library.collection.deleted.v1 + LIBRARY_COLLECTION_DELETED.send_event(library_collection=LibraryCollectionData(collection_key=collection_key)) + + # Now, what about the actual entities (containers/components) in the collection? + if entities_changed: + if len(entities_changed) == 1: + # If there's only one changed entity, emit the event synchronously: + fn = tasks.send_collections_changed_events + else: + # If there are more than one changed entities, emit the events asynchronously: + fn = tasks.send_collections_changed_events.delay + fn( + publishable_entity_ids=sorted(entities_changed), # sorted() is mostly for test purposes + learning_package_id=learning_package.id, + library_key_str=str(library.library_key), ) - except ContentLibrary.DoesNotExist: - log.error("{instance} is not associated with a content library.") - return - - if isinstance(instance, PublishableEntity): - _library_collection_entity_changed(instance, library.library_key) - return - - # When action=="post_clear", pk_set==None - # Since the collection instance now has an empty entities set, - # we don't know which ones were removed, so we need to update associations for all library - # components and containers. - components = get_components(instance.learning_package_id) - containers = get_containers(instance.learning_package_id) - if pk_set: - components = components.filter(pk__in=pk_set) - containers = containers.filter(pk__in=pk_set) - - for entity in list(components.all()) + list(containers.all()): - _library_collection_entity_changed(entity.publishable_entity, library.library_key) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index bb5db0a397c1..b3d38e9549f1 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -14,12 +14,14 @@ A longer-term solution to this issue would be to move the content_libraries app to cms: https://github.com/openedx/edx-platform/issues/33428 """ + from __future__ import annotations import json import logging import os import shutil +from collections.abc import Iterable from datetime import datetime from io import StringIO from tempfile import NamedTemporaryFile, mkdtemp @@ -38,22 +40,27 @@ set_code_owner_attribute_from_module, set_custom_attribute, ) +from opaque_keys import OpaqueKey from opaque_keys.edx.locator import ( BlockUsageLocator, - LibraryCollectionLocator, LibraryContainerLocator, LibraryLocatorV2, + LibraryUsageLocatorV2, ) from openedx_content import api as content_api from openedx_content.api import create_zip_file as create_lib_zip_file -from openedx_content.models_api import DraftChangeLog, PublishLog -from openedx_events.content_authoring.data import LibraryBlockData, LibraryCollectionData, LibraryContainerData +from openedx_content.models_api import LearningPackage, PublishableEntity, PublishLog +from openedx_events.content_authoring.data import ( + ContentObjectChangedData, + LibraryBlockData, + LibraryContainerData, +) from openedx_events.content_authoring.signals import ( + CONTENT_OBJECT_ASSOCIATIONS_CHANGED, LIBRARY_BLOCK_CREATED, LIBRARY_BLOCK_DELETED, LIBRARY_BLOCK_PUBLISHED, LIBRARY_BLOCK_UPDATED, - LIBRARY_COLLECTION_UPDATED, LIBRARY_CONTAINER_CREATED, LIBRARY_CONTAINER_DELETED, LIBRARY_CONTAINER_PUBLISHED, @@ -74,6 +81,7 @@ from xmodule.modulestore.mixed import MixedModuleStore from . import api +from .models import ContentLibrary log = logging.getLogger(__name__) TASK_LOGGER = get_task_logger(__name__) @@ -85,7 +93,220 @@ @shared_task(base=LoggedTask) @set_code_owner_attribute -def send_events_after_publish(publish_log_pk: int, library_key_str: str) -> None: +def send_change_events_for_modified_entities( + learning_package_id: LearningPackage.ID, + change_list: list[dict], # we want list[ChangeLogRecordData], but that's not JSON serializable, so use dicts +): + """ + Sends a various library-specific events for each modified library entity in + the given change log, after any kind of edit was made in the library. This + could be in response to an entity (component or container) being created, + modified, deleted, un-deleted, or one of its dependencies doing those + things. + + ⏳ This task is designed to be run asynchronously so it can handle many + entities, but you can also call it synchronously if you are only + processing a single entity. Handlers of the events that we emit here + should be synchronous and fast, to support the "update one item + synchronously" use case, but can be async if needed. + """ + changes = [content_api.signals.ChangeLogRecordData(**r) for r in change_list] + library = ContentLibrary.objects.get(learning_package_id=learning_package_id) + changes_by_entity_id = {change.entity_id: change for change in changes} + entities = ( + content_api.get_publishable_entities(learning_package_id) + .filter(id__in=changes_by_entity_id.keys()) + .select_related("component", "container") + ) + + for entity in entities: + change = changes_by_entity_id[entity.id] + if hasattr(entity, "component"): + # This is a library XBlock (component) + block_key = api.library_component_usage_key(library.library_key, entity.component) + event_data = LibraryBlockData(library_key=library.library_key, usage_key=block_key) + if change.old_version is None and change.new_version: + # .. event_implemented_name: LIBRARY_BLOCK_CREATED + # .. event_type: org.openedx.content_authoring.library_block.created.v1 + LIBRARY_BLOCK_CREATED.send_event(library_block=event_data) + elif change.old_version and change.new_version is None: + # .. event_implemented_name: LIBRARY_BLOCK_DELETED + # .. event_type: org.openedx.content_authoring.library_block.deleted.v1 + LIBRARY_BLOCK_DELETED.send_event(library_block=event_data) + else: + # If the version numbers are different, this block was modified. + # If not, it was included as a side effect of some other change, like renaming a parent container. + # .. event_implemented_name: LIBRARY_BLOCK_UPDATED + # .. event_type: org.openedx.content_authoring.library_block.updated.v1 + LIBRARY_BLOCK_UPDATED.send_event(library_block=event_data) + + elif hasattr(entity, "container"): + container_key = api.library_container_locator(library.library_key, entity.container) + event_data = LibraryContainerData(container_key=container_key) + if change.old_version is None and change.new_version: + # .. event_implemented_name: LIBRARY_CONTAINER_CREATED + # .. event_type: org.openedx.content_authoring.content_library.container.created.v1 + LIBRARY_CONTAINER_CREATED.send_event(library_container=event_data) + elif change.old_version and change.new_version is None: + # .. event_implemented_name: LIBRARY_CONTAINER_DELETED + # .. event_type: org.openedx.content_authoring.content_library.container.deleted.v1 + LIBRARY_CONTAINER_DELETED.send_event(library_container=event_data) + else: + # If the version numbers are different, this container was modified. + # If not, it was included as a side effect of some other change, like its child being modified. + container_itself_changed = change.old_version != change.new_version + + # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED + # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 + LIBRARY_CONTAINER_UPDATED.send_event(library_container=event_data) + # TODO: to optimze this, once we have https://github.com/openedx/openedx-events/pull/570 merged, + # change the above event to use `send_async=not container_itself_changed`, so that direct changes are + # processed immediately but side effects can happen async. + + if container_itself_changed: + # If entities were added/removed from this container, we need to notify things like the search index + # that the list of parent containers for each entity has changed. + check_container_content_changes.delay( + container_key_str=str(container_key), + old_version_id=change.old_version_id, + new_version_id=change.new_version_id, + ) + else: + log.error("Unknown publishable entity type: %s", entity) + continue + + +@shared_task(base=LoggedTask) +@set_code_owner_attribute +def check_container_content_changes( + container_key_str: str, + old_version_id: int | None, + new_version_id: int | None, +): + """ + Whenever a container is edited, we need to check if child entities were + added or removed, and if so send out a CONTENT_OBJECT_ASSOCIATIONS_CHANGED + event for each added/removed child. + + For example, removing an entity from a unit should result in:: + + CONTENT_OBJECT_ASSOCIATIONS_CHANGED( + object_id=..., + changes=["units"], + ) + + ⏳ This task is always run asynchronously. + """ + if old_version_id == new_version_id: + return # Same versions + + if old_version_id: + old_version = content_api.get_container_version(old_version_id) + old_entity_list_id = old_version.entity_list_id + else: + old_entity_list_id = None + if new_version_id: + new_version = content_api.get_container_version(new_version_id) + new_entity_list_id = new_version.entity_list_id + else: + new_entity_list_id = None + + old_child_ids: Iterable[PublishableEntity.ID] + new_child_ids: Iterable[PublishableEntity.ID] + # If the title has changed, we notify ALL children that their parent container(s) have changed, e.g. to update the + # list of "units this component is used in", "sections this subsection is used in", etc. in the search index + title_changed: bool = bool(old_version and new_version) and (old_version.title != new_version.title) + if title_changed: + # TODO: there is no "get entity list for container version" API in openedx_content + new_child_ids = new_version.entity_list.entitylistrow_set.values_list("entity_id", flat=True) + old_child_ids = old_version.entity_list.entitylistrow_set.values_list("entity_id", flat=True) + # notify ALL current children, plus any deleted children, that their parent container(s) changed + changed_child_ids = list(set(new_child_ids) | (set(old_child_ids) - set(new_child_ids))) + elif old_entity_list_id == new_entity_list_id: + # Different container versions but same list of child entities. For now we don't need to do anything, but in the + # future if we have some other kind of per-container settings relevant to child entities we might need to handle + # this the same way as title_changed. + return + else: + # TODO: there is no "get entity list for container version" API in openedx_content + old_child_ids = ( + old_version.entity_list.entitylistrow_set.values_list("entity_id", flat=True) if old_version else [] + ) + new_child_ids = ( + new_version.entity_list.entitylistrow_set.values_list("entity_id", flat=True) if new_version else [] + ) + # We only need to notify any added or removed children that their parent container(s) changed: + changed_child_ids = list(set(old_child_ids) ^ set(new_child_ids)) + + container_key = LibraryContainerLocator.from_string(container_key_str) + library = ContentLibrary.objects.get_by_key(container_key.lib_key) + entities = ( + content_api.get_publishable_entities(library.learning_package_id) + .filter(id__in=changed_child_ids) + .select_related("component", "container") + ) + for entity in entities: + child_key: LibraryUsageLocatorV2 | LibraryContainerLocator + if hasattr(entity, "component"): + child_key = api.library_component_usage_key(library.library_key, entity.component) + elif hasattr(entity, "container"): + child_key = api.library_container_locator(library.library_key, entity.container) + else: + log.error("Unknown publishable entity type: %s", entity) + continue + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + content_object=ContentObjectChangedData( + object_id=str(child_key), + changes=[container_key.container_type + "s"], # e.g. "units" + ), + ) + + +@shared_task(base=LoggedTask) +@set_code_owner_attribute +def send_collections_changed_events( + publishable_entity_ids: list[PublishableEntity.ID], + learning_package_id: LearningPackage.ID, + library_key_str: str, +): + """ + Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for each modified library + entity in the given list, because their associated collections have changed. + + ⏳ This task is designed to be run asynchronously so it can handle many + entities, but you can also call it synchronously if you are only + processing a single entity. Handlers should be synchronous and fast, to + support the "update one item synchronously" use case, but can be async if + needed. + """ + library_key = LibraryLocatorV2.from_string(library_key_str) + entities = ( + content_api.get_publishable_entities(learning_package_id) + .filter(id__in=publishable_entity_ids) + .select_related("component", "container") + ) + + for entity in entities: + opaque_key: OpaqueKey + + if hasattr(entity, "component"): + opaque_key = api.library_component_usage_key(library_key, entity.component) + elif hasattr(entity, "container"): + opaque_key = api.library_container_locator(library_key, entity.container) + else: + log.error("Unknown publishable entity type: %s", entity) + continue + + # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED + # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + content_object=ContentObjectChangedData(object_id=str(opaque_key), changes=["collections"]), + ) + + +@shared_task(base=LoggedTask) +@set_code_owner_attribute +def send_events_after_publish(publish_log_id: int, library_key_str: str) -> None: """ Send events to trigger actions like updating the search index, after we've published some items in a library. @@ -99,12 +320,11 @@ def send_events_after_publish(publish_log_pk: int, library_key_str: str) -> None event handlers like updating the search index may a while to complete in that case. """ - publish_log = PublishLog.objects.get(pk=publish_log_pk) + publish_log = PublishLog.objects.get(id=publish_log_id) library_key = LibraryLocatorV2.from_string(library_key_str) affected_entities = publish_log.records.select_related( "entity", "entity__container", "entity__container__container_type", "entity__component", ).all() - affected_containers: set[LibraryContainerLocator] = set() # Update anything that needs to be updated (e.g. search index): for record in affected_entities: @@ -118,198 +338,20 @@ def send_events_after_publish(publish_log_pk: int, library_key_str: str) -> None LIBRARY_BLOCK_PUBLISHED.send_event( library_block=LibraryBlockData(library_key=library_key, usage_key=usage_key) ) - # Publishing a container will auto-publish its children, but publishing a single component or all changes - # in the library will NOT usually include any parent containers. But we do need to notify listeners that the - # parent container(s) have changed, e.g. so the search index can update the "has_unpublished_changes" - try: - for parent_container in api.get_containers_contains_item(usage_key): - affected_containers.add(parent_container.container_key) - # TODO: should this be a CONTAINER_CHILD_PUBLISHED event instead of CONTAINER_PUBLISHED ? - except api.ContentLibraryBlockNotFound: - # The component has been deleted. - pass elif hasattr(record.entity, "container"): container_key = api.library_container_locator(library_key, record.entity.container) - affected_containers.add(container_key) - - try: - # We do need to notify listeners that the parent container(s) have changed, - # e.g. so the search index can update the "has_unpublished_changes" - for parent_container in api.get_containers_contains_item(container_key): - affected_containers.add(parent_container.container_key) - except api.ContentLibraryContainerNotFound: - # The deleted children remains in the entity, so, in this case, the container may not be found. - pass - else: - log.warning( - f"PublishableEntity {record.entity.pk} / {record.entity.key} was modified during publish operation " - "but is of unknown type." + # Note: this container may have been directly published, or perhaps one of its children was published and + # it hasn't technically changed. Such ancestors of published entities are still included in the publish log. + # .. event_implemented_name: LIBRARY_CONTAINER_PUBLISHED + # .. event_type: org.openedx.content_authoring.content_library.container.published.v1 + LIBRARY_CONTAINER_PUBLISHED.send_event( + library_container=LibraryContainerData(container_key=container_key) ) - - for container_key in affected_containers: - # .. event_implemented_name: LIBRARY_CONTAINER_PUBLISHED - # .. event_type: org.openedx.content_authoring.content_library.container.published.v1 - LIBRARY_CONTAINER_PUBLISHED.send_event( - library_container=LibraryContainerData(container_key=container_key) - ) - - -def wait_for_post_publish_events(publish_log: PublishLog, library_key: LibraryLocatorV2): - """ - After publishing some changes, trigger the required event handlers (e.g. - update the search index). Try to wait for that to complete before returning, - up to some reasonable timeout, and then finish anything remaining - asynchonrously. - """ - # Update the search index (and anything else) for the affected blocks - result = send_events_after_publish.apply_async(args=(publish_log.pk, str(library_key))) - # Try waiting a bit for those post-publish events to be handled: - try: - result.get(timeout=15) - except TimeoutError: - pass - # This is fine! The search index is still being updated, and/or other - # event handlers are still following up on the results, but the publish - # already *did* succeed, and the events will continue to be processed in - # the background by the celery worker until everything is updated. - - -@shared_task(base=LoggedTask) -@set_code_owner_attribute -def send_events_after_revert(draft_change_log_id: int, library_key_str: str) -> None: - """ - Send events to trigger actions like updating the search index, after we've - reverted some unpublished changes in a library. - - See notes on the analogous function above, send_events_after_publish. - """ - try: - draft_change_log = DraftChangeLog.objects.get(id=draft_change_log_id) - except DraftChangeLog.DoesNotExist: - # When a revert operation is a no-op, openedx_content deletes the empty - # DraftChangeLog, so we'll assume that's what happened here. - log.info(f"Library revert in {library_key_str} did not result in any changes.") - return - - library_key = LibraryLocatorV2.from_string(library_key_str) - affected_entities = draft_change_log.records.select_related( - "entity", "entity__container", "entity__component", - ).all() - - created_container_keys: set[LibraryContainerLocator] = set() - updated_container_keys: set[LibraryContainerLocator] = set() - deleted_container_keys: set[LibraryContainerLocator] = set() - affected_collection_keys: set[LibraryCollectionLocator] = set() - - # Update anything that needs to be updated (e.g. search index): - for record in affected_entities: - # This will be true if the entity was [soft] deleted, but we're now reverting that deletion: - is_undeleted = (record.old_version is None and record.new_version is not None) - # This will be true if the entity was created and we're now deleting it by reverting that creation: - is_deleted = (record.old_version is not None and record.new_version is None) - if hasattr(record.entity, "component"): - usage_key = api.library_component_usage_key(library_key, record.entity.component) - event = LIBRARY_BLOCK_UPDATED - if is_deleted: - event = LIBRARY_BLOCK_DELETED - elif is_undeleted: - event = LIBRARY_BLOCK_CREATED - - # .. event_implemented_name: LIBRARY_BLOCK_UPDATED - # .. event_type: org.openedx.content_authoring.library_block.updated.v1 - - # .. event_implemented_name: LIBRARY_BLOCK_DELETED - # .. event_type: org.openedx.content_authoring.library_block.deleted.v1 - - # .. event_implemented_name: LIBRARY_BLOCK_CREATED - # .. event_type: org.openedx.content_authoring.library_block.created.v1 - event.send_event(library_block=LibraryBlockData(library_key=library_key, usage_key=usage_key)) - # If any containers contain this component, their child list / component count may need to be updated - # e.g. if this was a newly created component in the container and is now deleted, or this was deleted and - # is now restored. - # TODO: we should be able to rewrite this to use the "side effects" functionality of the publishing API. - try: - for parent_container in api.get_containers_contains_item(usage_key): - updated_container_keys.add(parent_container.container_key) - except api.ContentLibraryBlockNotFound: - pass # The item 'usage_key' has been deleted. But shouldn't we still handle that? - - # TODO: do we also need to send CONTENT_OBJECT_ASSOCIATIONS_CHANGED for this component, or is - # LIBRARY_BLOCK_UPDATED sufficient? - elif hasattr(record.entity, "container"): - container_key = api.library_container_locator(library_key, record.entity.container) - if is_deleted: - deleted_container_keys.add(container_key) - elif is_undeleted: - created_container_keys.add(container_key) - else: - updated_container_keys.add(container_key) else: log.warning( - f"PublishableEntity {record.entity.pk} / {record.entity.key} was modified during publish operation " - "but is of unknown type." - ) - # If any collections contain this entity, their item count may need to be updated, e.g. if this was a - # newly created component in the collection and is now deleted, or this was deleted and is now re-added. - for parent_collection in content_api.get_entity_collections( - record.entity.learning_package_id, record.entity.key, - ): - collection_key = api.library_collection_locator( - library_key=library_key, - collection_key=parent_collection.key, + f"PublishableEntity {record.entity.pk} / {record.entity.entity_ref} " + "was modified during publish operation but is of unknown type." ) - affected_collection_keys.add(collection_key) - - for container_key in deleted_container_keys: - # .. event_implemented_name: LIBRARY_CONTAINER_DELETED - # .. event_type: org.openedx.content_authoring.content_library.container.deleted.v1 - LIBRARY_CONTAINER_DELETED.send_event( - library_container=LibraryContainerData(container_key=container_key) - ) - # Don't bother sending UPDATED events for these containers that are now deleted - created_container_keys.discard(container_key) - - for container_key in created_container_keys: - # .. event_implemented_name: LIBRARY_CONTAINER_CREATED - # .. event_type: org.openedx.content_authoring.content_library.container.created.v1 - LIBRARY_CONTAINER_CREATED.send_event( - library_container=LibraryContainerData(container_key=container_key) - ) - - for container_key in updated_container_keys: - # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData(container_key=container_key) - ) - - for collection_key in affected_collection_keys: - # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED - # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 - LIBRARY_COLLECTION_UPDATED.send_event( - library_collection=LibraryCollectionData(collection_key=collection_key) - ) - - -def wait_for_post_revert_events(draft_change_log: DraftChangeLog, library_key: LibraryLocatorV2): - """ - After discard all changes in a library, trigger the required event handlers - (e.g. update the search index). Try to wait for that to complete before - returning, up to some reasonable timeout, and then finish anything remaining - asynchonrously. - """ - # Update the search index (and anything else) for the affected blocks - result = send_events_after_revert.apply_async(args=(draft_change_log.pk, str(library_key))) - # Try waiting a bit for those post-publish events to be handled: - try: - result.get(timeout=15) - except TimeoutError: - pass - # This is fine! The search index is still being updated, and/or other - # event handlers are still following up on the results, but the revert - # already *did* succeed, and the events will continue to be processed in - # the background by the celery worker until everything is updated. - def _filter_child(store, usage_key, capa_type): @@ -541,7 +583,7 @@ def backup_library(self, user_id: int, library_key_str: str) -> None: file_path = os.path.join(root_dir, filename) user = User.objects.get(id=user_id) origin_server = getattr(settings, 'CMS_BASE', None) - create_lib_zip_file(lp_key=str(library_key), path=file_path, user=user, origin_server=origin_server) + create_lib_zip_file(package_ref=str(library_key), path=file_path, user=user, origin_server=origin_server) set_custom_attribute("exporting_completed", str(library_key)) with open(file_path, 'rb') as zipfile: @@ -651,7 +693,7 @@ def restore_library(self, user_id, storage_path): TASK_LOGGER.info( 'Restored learning package (id: %s) with key %s', learning_package_data.get('id'), - learning_package_data.get('key') + learning_package_data.get('package_ref') ) # Save the restore details as an artifact in JSON format diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 408d16618569..99f2568d6114 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -28,6 +28,8 @@ LIBRARY_CONTAINER_DELETED, LIBRARY_CONTAINER_UPDATED, ) +from openedx_events.testing import EventsIsolationMixin +from openedx_events.tooling import OpenEdxPublicSignal from user_tasks.models import UserTaskStatus from common.djangoapps.student.tests.factories import UserFactory @@ -37,13 +39,21 @@ from .base import ContentLibrariesRestApiTest -class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest): +class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, EventsIsolationMixin): """ Tests for Content Library API collections methods. Same guidelines as ContentLibrariesTestCase. """ + @classmethod + def setUpClass(cls): + """Test setup""" + super().setUpClass() + # By default, errors thrown in signal handlers get suppressed. We want to see them though! + # https://github.com/openedx/openedx-events/issues/569 + cls.allow_send_events_failure(*(s.event_type for s in OpenEdxPublicSignal.all_events())) + def setUp(self) -> None: super().setUp() @@ -115,7 +125,7 @@ def test_create_library_collection(self) -> None: description="Description for Collection 4", created_by=self.user.id, ) - assert collection.key == "COL4" + assert collection.collection_code == "COL4" assert collection.title == "Collection 4" assert collection.description == "Description for Collection 4" assert collection.created_by == self.user @@ -150,10 +160,10 @@ def test_update_library_collection(self) -> None: self.col1 = api.update_library_collection( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, title="New title for Collection 1", ) - assert self.col1.key == "COL1" + assert self.col1.collection_code == "COL1" assert self.col1.title == "New title for Collection 1" assert self.col1.description == "Description for Collection 1" assert self.col1.created_by == self.user @@ -177,7 +187,7 @@ def test_update_library_collection_wrong_library(self) -> None: with self.assertRaises(api.ContentLibraryCollectionNotFound) as exc: # noqa: F841, PT027 api.update_library_collection( self.lib1.library_key, - self.col2.key, + self.col2.collection_code, ) def test_delete_library_collection(self) -> None: @@ -187,7 +197,7 @@ def test_delete_library_collection(self) -> None: assert self.lib1.learning_package_id is not None content_api.delete_collection( self.lib1.learning_package_id, - self.col1.key, + self.col1.collection_code, hard_delete=True, ) @@ -211,7 +221,7 @@ def test_update_library_collection_items(self) -> None: self.col1 = api.update_library_collection_items( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, opaque_keys=[ LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), @@ -222,7 +232,7 @@ def test_update_library_collection_items(self) -> None: self.col1 = api.update_library_collection_items( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, opaque_keys=[ LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), ], @@ -240,7 +250,7 @@ def test_update_library_collection_components_event(self) -> None: api.update_library_collection_items( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, opaque_keys=[ LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), @@ -252,11 +262,12 @@ def test_update_library_collection_components_event(self) -> None: self.assertDictContainsEntries( event_receiver.call_args_list[0].kwargs, { - "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, - "sender": None, - "content_object": ContentObjectChangedData( - object_id=self.lib1_problem_block["id"], - changes=["collections"], + "signal": LIBRARY_COLLECTION_UPDATED, + "library_collection": LibraryCollectionData( + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), ), }, ) @@ -264,9 +275,8 @@ def test_update_library_collection_components_event(self) -> None: event_receiver.call_args_list[1].kwargs, { "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, - "sender": None, "content_object": ContentObjectChangedData( - object_id=self.lib1_html_block["id"], + object_id=self.lib1_problem_block["id"], changes=["collections"], ), }, @@ -275,9 +285,8 @@ def test_update_library_collection_components_event(self) -> None: event_receiver.call_args_list[2].kwargs, { "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, - "sender": None, "content_object": ContentObjectChangedData( - object_id=self.unit1["id"], + object_id=self.lib1_html_block["id"], changes=["collections"], ), }, @@ -285,13 +294,10 @@ def test_update_library_collection_components_event(self) -> None: self.assertDictContainsEntries( event_receiver.call_args_list[3].kwargs, { - "signal": LIBRARY_COLLECTION_UPDATED, - "sender": None, - "library_collection": LibraryCollectionData( - collection_key=api.library_collection_locator( - self.lib1.library_key, - collection_key="COL1", - ), + "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, + "content_object": ContentObjectChangedData( + object_id=self.unit1["id"], + changes=["collections"], ), }, ) @@ -300,7 +306,7 @@ def test_update_collection_components_from_wrong_library(self) -> None: with self.assertRaises(api.ContentLibraryBlockNotFound) as exc: # noqa: PT027 api.update_library_collection_items( self.lib2.library_key, - self.col2.key, + self.col2.collection_code, opaque_keys=[ LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), @@ -318,13 +324,15 @@ def test_set_library_component_collections(self) -> None: component = api.get_component_from_usage_key(UsageKeyV2.from_string(self.lib2_problem_block["id"])) api.set_library_item_collections( library_key=self.lib2.library_key, - entity_key=component.publishable_entity.key, - collection_keys=[self.col2.key, self.col3.key], + entity_ref=component.publishable_entity.entity_ref, + collection_keys=[self.col2.collection_code, self.col3.collection_code], ) assert self.lib2.learning_package_id is not None - assert len(content_api.get_collection(self.lib2.learning_package_id, self.col2.key).entities.all()) == 1 - assert len(content_api.get_collection(self.lib2.learning_package_id, self.col3.key).entities.all()) == 1 + col2 = content_api.get_collection(self.lib2.learning_package_id, self.col2.collection_code) + col3 = content_api.get_collection(self.lib2.learning_package_id, self.col3.collection_code) + assert len(col2.entities.all()) == 1 + assert len(col3.entities.all()) == 1 self.assertDictContainsEntries( event_receiver.call_args_list[0].kwargs, @@ -343,19 +351,21 @@ def test_set_library_component_collections(self) -> None: assert all(event["signal"] == LIBRARY_COLLECTION_UPDATED for event in collection_update_events) assert {event["library_collection"] for event in collection_update_events} == { LibraryCollectionData( - collection_key=api.library_collection_locator(self.lib2.library_key, collection_key=self.col2.key), - background=True, + collection_key=api.library_collection_locator( + self.lib2.library_key, collection_key=self.col2.collection_code + ) ), LibraryCollectionData( - collection_key=api.library_collection_locator(self.lib2.library_key, collection_key=self.col3.key), - background=True, - ) + collection_key=api.library_collection_locator( + self.lib2.library_key, collection_key=self.col3.collection_code + ) + ), } def test_delete_library_block(self) -> None: api.update_library_collection_items( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, opaque_keys=[ LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), @@ -375,10 +385,8 @@ def test_delete_library_block(self) -> None: "sender": None, "library_collection": LibraryCollectionData( collection_key=api.library_collection_locator( - self.lib1.library_key, - collection_key=self.col1.key, + self.lib1.library_key, collection_key=self.col1.collection_code ), - background=True, ), }, ) @@ -386,7 +394,7 @@ def test_delete_library_block(self) -> None: def test_delete_library_container(self) -> None: api.update_library_collection_items( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, opaque_keys=[ LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), @@ -415,9 +423,8 @@ def test_delete_library_container(self) -> None: "library_collection": LibraryCollectionData( collection_key=api.library_collection_locator( self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), - background=True, ), }, ) @@ -429,7 +436,7 @@ def test_delete_library_container(self) -> None: "library_container": LibraryContainerData( container_key=self.subsection1.container_key, background=False, - ) + ), }, ) @@ -497,19 +504,22 @@ def test_delete_library_block_when_component_does_not_exist(self) -> None: ) def test_restore_library_block(self) -> None: + usage_key = LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]) api.update_library_collection_items( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, opaque_keys=[ - LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + usage_key, LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), ], ) + api.delete_library_block(usage_key) + event_receiver = mock.Mock() LIBRARY_COLLECTION_UPDATED.connect(event_receiver) - api.restore_library_block(LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"])) + api.restore_library_block(usage_key) assert event_receiver.call_count == 1 self.assertDictContainsEntries( @@ -520,9 +530,8 @@ def test_restore_library_block(self) -> None: "library_collection": LibraryCollectionData( collection_key=api.library_collection_locator( self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), - background=True, ), }, ) @@ -539,7 +548,7 @@ def test_add_component_and_revert(self) -> None: # Add component. Note: collections are not part of the draft/publish cycle so this is not a draft change. api.update_library_collection_items( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, opaque_keys=[ LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), LibraryUsageLocatorV2.from_string(new_problem_block["id"]), @@ -549,6 +558,8 @@ def test_add_component_and_revert(self) -> None: collection_update_event_receiver = mock.Mock() LIBRARY_COLLECTION_UPDATED.connect(collection_update_event_receiver) + # Reverting the change essentially deletes the item, so we should get an event that the collection's contents + # have been updated. api.revert_changes(self.lib1.library_key) assert collection_update_event_receiver.call_count == 1 @@ -560,7 +571,7 @@ def test_add_component_and_revert(self) -> None: "library_collection": LibraryCollectionData( collection_key=api.library_collection_locator( self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), ), }, @@ -574,7 +585,7 @@ def test_delete_component_and_revert(self) -> None: # Add components and publish api.update_library_collection_items( self.lib1.library_key, - self.col1.key, + self.col1.collection_code, opaque_keys=[ LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]) @@ -599,7 +610,7 @@ def test_delete_component_and_revert(self) -> None: "library_collection": LibraryCollectionData( collection_key=api.library_collection_locator( self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), ), }, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 37ad621d26e6..a95b238b78a4 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -591,7 +591,7 @@ def test_unit_collections(self) -> None: result = self._patch_container_collections( self.unit["id"], - collection_keys=[col1.key], + collection_keys=[col1.collection_code], ) assert result['count'] == 1 @@ -600,7 +600,7 @@ def test_unit_collections(self) -> None: unit_as_read = self._get_container(self.unit["id"]) # Verify the collections - assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.key}] + assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.collection_code}] def test_section_hierarchy(self): with self.assertNumQueries(126): diff --git a/openedx/core/djangoapps/content_libraries/tests/test_events.py b/openedx/core/djangoapps/content_libraries/tests/test_events.py index bf4857a3cae7..c89905d7dd37 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_events.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_events.py @@ -1,7 +1,6 @@ """ Tests for openedx_content-based Content Libraries """ - from unittest import mock from django.db import transaction @@ -115,11 +114,23 @@ def expect_new_events(self, *expected_events: dict) -> None: found = True break if not found: - raise AssertionError(f"Event {expected} not found among actual events: {self.new_events}") + raise AssertionError(f"Event {expected} not found among actual events:\n{self.new_events_str}") if len(self.new_events) > 0: - raise AssertionError(f"Events were emitted but not expected: {self.new_events}") + raise AssertionError(f"Events were emitted but not expected:\n{self.new_events_str}") self.clear_events() + @property + def new_events_str(self) -> str: + """Friendly-ish string representation of self.new_events""" + simplified_events = [e.copy() for e in self.new_events] + for e in simplified_events: + if e["sender"] is None: + del e["sender"] + if e["from_event_bus"] is False: + del e["from_event_bus"] + del e["metadata"] + return "\n".join([str(e) for e in simplified_events]) + @skip_unless_cms class ContentLibrariesEventsTestCase(BaseEventsTestCase): @@ -451,21 +462,87 @@ def test_publish_container(self) -> None: self.lib1_key, LibraryUsageLocatorV2.from_string(html_block["id"]), ), }, - { # Not 100% sure we want this, but a PUBLISHED event is emitted for container 2 - # because one of its children's published versions has changed, so whether or - # not it contains unpublished changes may have changed and the search index - # may need to be updated. It is not actually published though. - # TODO: should this be a CONTAINER_CHILD_PUBLISHED event? + # No PUBLISHED event is emitted for container 2, because it doesn't have a published version yet. + # Publishing 'html_block' would have potentially affected it if container 2's published version had a + # reference to 'html_block', but it doesn't yet until we publish it. + ) + + # note that container 2 is still unpublished + c2_after = self._get_container(container2["id"]) + assert c2_after["has_unpublished_changes"] + + # publish container2 now: + self._publish_container(container2["id"]) + self.expect_new_events( + { # An event for container 1 being published: "signal": LIBRARY_CONTAINER_PUBLISHED, "library_container": LibraryContainerData( container_key=LibraryContainerLocator.from_string(container2["id"]), ), }, + { # An event for the html block in container 2 only: + "signal": LIBRARY_BLOCK_PUBLISHED, + "library_block": LibraryBlockData( + self.lib1_key, LibraryUsageLocatorV2.from_string(html_block2["id"]), + ), + }, ) - # note that container 2 is still unpublished - c2_after = self._get_container(container2["id"]) - assert c2_after["has_unpublished_changes"] + def test_publish_container_propagation(self) -> None: + """ + Test the events that get emitted when we publish the changes to an entity + that is used in multiple published containers + """ + # Create two containers and add the same component to both: + container1 = self._create_container(self.lib1_key, "unit", display_name="Alpha Unit", slug=None) + container2 = self._create_container(self.lib1_key, "unit", display_name="Bravo Unit", slug=None) + problem_block = self._add_block_to_library(self.lib1_key, "problem", "Problem1", can_stand_alone=False) + self._add_container_children(container1["id"], children_ids=[problem_block["id"]]) + self._add_container_children(container2["id"], children_ids=[problem_block["id"]]) + # Publish everything: + self._commit_library_changes(self.lib1_key) + + # clear event log after the initial mock data setup is complete: + self.clear_events() + + # Now modify the problem that's shared by both containers and publish the new version + self._set_library_block_olx(problem_block["id"], "UPDATED") + self.clear_events() # Clears the LIBRARY_BLOCK_UPDATED event + 2x LIBRARY_CONTAINER_UPDATED events + + # Now both containers have unpublished changes: + assert self._get_container(container1["id"])["has_unpublished_changes"] + assert self._get_container(container2["id"])["has_unpublished_changes"] + # Publish container1, which also published the shared problem component: + self._publish_container(container1["id"]) + # Now neither container has unpublished changes (even though we never touched container2): + assert self._get_container(container1["id"])["has_unpublished_changes"] is False + assert self._get_container(container2["id"])["has_unpublished_changes"] is False + + # And publish events were emitted: + self.expect_new_events( + # An event for the problem block in container 1 being indirectly published: + { + "signal": LIBRARY_BLOCK_PUBLISHED, + "library_block": LibraryBlockData( + self.lib1_key, LibraryUsageLocatorV2.from_string(problem_block["id"]), + ), + }, + # An event for container 1 being published *directly*: + { + "signal": LIBRARY_CONTAINER_PUBLISHED, + "library_container": LibraryContainerData( + container_key=LibraryContainerLocator.from_string(container1["id"]), + ), + }, + # And this time a PUBLISHED event should also be emitted for container2. + # It's published version hasn't changed, but its "contains unpublished changes" status has. + { + "signal": LIBRARY_CONTAINER_PUBLISHED, + "library_container": LibraryContainerData( + container_key=LibraryContainerLocator.from_string(container2["id"]), + ), + }, + ) def test_publish_child_container(self): """ @@ -502,7 +579,17 @@ def test_publish_child_container(self): container_key=LibraryContainerLocator.from_string(unit["id"]), ), }, - { # An event for parent (subsection): + # No PUBLISHED event is emitted for the subsection, because it doesn't have a published version yet. + ) + + # note that subsection is still unpublished + c2_after = self._get_container(subsection["id"]) + assert c2_after["has_unpublished_changes"] + + # Now publish the subsection + self._publish_container(subsection["id"]) + self.expect_new_events( + { # An event for the subsection being published: "signal": LIBRARY_CONTAINER_PUBLISHED, "library_container": LibraryContainerData( container_key=LibraryContainerLocator.from_string(subsection["id"]), @@ -510,9 +597,27 @@ def test_publish_child_container(self): }, ) - # note that subsection is still unpublished - c2_after = self._get_container(subsection["id"]) - assert c2_after["has_unpublished_changes"] + # Now rename the unit: + self._update_container(unit["id"], 'New Unit Display Name') + self.clear_events() + # Publish changes to the unit: + self._publish_container(unit["id"]) + self.expect_new_events( + { # An event for the unit being published: + "signal": LIBRARY_CONTAINER_PUBLISHED, + "library_container": LibraryContainerData( + container_key=LibraryContainerLocator.from_string(unit["id"]), + ), + }, + # And this time we DO get notified that the parent container is affected, because the unit is in its + # published version, and this publish affects the parent's "contains_unpublished_changes" status. + { + "signal": LIBRARY_CONTAINER_PUBLISHED, + "library_container": LibraryContainerData( + container_key=LibraryContainerLocator.from_string(subsection["id"]), + ), + }, + ) def test_restore_unit(self) -> None: """ @@ -543,13 +648,10 @@ def test_restore_unit(self) -> None: "signal": LIBRARY_CONTAINER_CREATED, "library_container": LibraryContainerData(container_key), }, - { - "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, - "content_object": ContentObjectChangedData( - object_id=str(container_key), - changes=["collections", "tags"], - ), - }, + # We used to emit CONTENT_OBJECT_ASSOCIATIONS_CHANGED here for the restored container, specifically noting + # that changes=["collections", "tags"], because deleted things may have collections+tags that are once + # again relevant when it is restored. However, the CREATED event should be sufficient for notifying of that. + # (Or should we emit CREATED+UPDATED to be extra sure?) ) def test_restore_unit_via_revert(self) -> None: @@ -611,11 +713,11 @@ def test_collection_crud(self) -> None: "library_collection": LibraryCollectionData(collection_key), }) - # Soft delete the collection. NOTE: at the moment, it's only possible to "soft delete" collections via - # the REST API, which sends an UPDATED event because the collection is now "disabled" but not deleted. + # Soft delete the collection. Whether we "soft" or "hard" delete, it sends a "DELETED" event. + # If we later restore it, it would send a "CREATED" event. self._soft_delete_collection(collection_key) self.expect_new_events({ - "signal": LIBRARY_COLLECTION_UPDATED, # UPDATED not DELETED. If we do a hard delete, it should be DELETED. + "signal": LIBRARY_COLLECTION_DELETED, "library_collection": LibraryCollectionData(collection_key), }) @@ -624,6 +726,22 @@ class ContentLibraryContainerEventsTest(BaseEventsTestCase): """ Event tests for container operations: signals emitted when components and containers are created, updated, deleted, and associated with one another. + + setUp() builds the following structure in lib1 (note that some entities + are shared across multiple parents, so this is a DAG, not a strict tree):: + + Section 1 Section 2 + ├── Subsection 1 ◄───────── (shared) ────────┴── Subsection 1 + │ ├── Unit 1 ◄────────────────┐ (shared) + │ │ ├── problem1 │ + │ │ └── html1 ◄──┐ │ + │ └── Unit 2 │(shared) │ + │ └── html1 ◄──┘ │ + └── Subsection 2 │ + └── Unit 1 ◄────────────────┘ + + Orphans (created but not attached to any parent): + Unit 3, problem2 """ def setUp(self) -> None: @@ -689,21 +807,38 @@ def setUp(self) -> None: def test_container_updated_when_component_deleted(self) -> None: api.delete_library_block(self.html_block_usage_key) self.expect_new_events( + # The block itself was deleted: { "signal": LIBRARY_BLOCK_DELETED, "library_block": LibraryBlockData(self.lib1_key, self.html_block_usage_key), }, + # That block was a child of two units, so both parent units are flagged as updated + # e.g. to update their "child_display_names" in the search index. { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit1.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.unit1.container_key), }, { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit2.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.unit2.container_key), + }, + # openedx_content also lists ancestor containers of the affected units as changed. + # We don't strictly need this at the moment, at least as far as keeping our search index updated. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection2.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section2.container_key), }, ) @@ -713,49 +848,79 @@ def test_container_updated_when_component_restored(self) -> None: api.restore_library_block(self.html_block_usage_key) self.expect_new_events( + # Restoring the block re-creates it: { "signal": LIBRARY_BLOCK_CREATED, "library_block": LibraryBlockData(self.lib1_key, self.html_block_usage_key), }, + # We used to emit CONTENT_OBJECT_ASSOCIATIONS_CHANGED here for the restored block, specifically noting + # that changes=["collections", "tags", "units"], because deleted things may have collections+tags+containers + # that are once again relevant when it is restored. However, the CREATED event should be sufficient for + # notifying of that. (Or should we emit CREATED+UPDATED to be extra sure?) + # The restored block is a child of two units, so both parent units are flagged as updated: { - "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, - "content_object": ContentObjectChangedData( - object_id=str(self.html_block_usage_key), - changes=["collections", "tags", "units"], - ), + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.unit1.container_key), }, { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit1.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.unit2.container_key), }, + # openedx_content also lists ancestor containers of the affected units as changed. + # We don't strictly need this at the moment, at least as far as keeping our search index updated. { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit2.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.subsection1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection2.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section2.container_key), }, ) def test_container_updated_when_component_olx_updated(self) -> None: self._set_library_block_olx(self.html_block_usage_key, "Hello world!") self.expect_new_events( + # The block's OLX changed: { "signal": LIBRARY_BLOCK_UPDATED, "library_block": LibraryBlockData(self.lib1_key, self.html_block_usage_key), }, + # That block is used in two units, so both parent units are flagged as updated + # e.g. to update their "child_display_names" in the search index, if the child's name has changed. { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit1.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.unit1.container_key), }, { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit2.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.unit2.container_key), + }, + # openedx_content also lists ancestor containers of the affected units as changed. + # We don't strictly need this at the moment, at least as far as keeping our search index updated. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection2.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section2.container_key), }, ) @@ -767,17 +932,33 @@ def test_container_updated_when_component_fields_updated(self) -> None: "signal": LIBRARY_BLOCK_UPDATED, "library_block": LibraryBlockData(self.lib1_key, self.html_block_usage_key), }, + # That block is used in two containers, so we expect events for them too: + # This is used e.g. to update "child_display_names" in the search index, if the child's name has changed. { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit1.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.unit1.container_key), }, { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit2.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.unit2.container_key), + }, + # openedx_content also lists and parent containers of affected containers as changed, and so on... + # We don't strictly need this at the moment, at least as far as keeping our search index updated. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection2.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section2.container_key), }, ) @@ -786,10 +967,13 @@ def test_container_updated_when_component_fields_updated(self) -> None: def test_container_updated_when_unit_updated(self) -> None: self._update_container(self.unit1.container_key, 'New Unit Display Name') self.expect_new_events( + # We renamed this unit, so we get an UPDATED event for it: { "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.unit1.container_key), }, + # We also get events for its parent containers + # e.g. to update their "child_display_names" in the search index. { "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.subsection1.container_key), @@ -798,6 +982,18 @@ def test_container_updated_when_unit_updated(self) -> None: "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.subsection2.container_key), }, + # openedx_content also lists ancestor containers of the affected unit as changed. + # We don't strictly need this at the moment, at least as far as keeping our search index updated. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section2.container_key), + }, + # Finally, any child components receive a "units changed" notification + # e.g. to update the "units this component is used in" in the search index. { "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "content_object": ContentObjectChangedData( @@ -815,10 +1011,13 @@ def test_container_updated_when_unit_updated(self) -> None: def test_container_updated_when_subsection_updated(self) -> None: self._update_container(self.subsection1.container_key, 'New Subsection Display Name') self.expect_new_events( + # We renamed this container, so we get an UPDATED event for it: { "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.subsection1.container_key), }, + # We also get events for its parent containers + # e.g. to update their "child_display_names" in the search index { "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.section1.container_key), @@ -827,6 +1026,8 @@ def test_container_updated_when_subsection_updated(self) -> None: "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.section2.container_key), }, + # Finally, any child containers receive a "subsections changed" notification + # e.g. to update the "subsections this unit is used in" in the search index. { "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "content_object": ContentObjectChangedData( @@ -865,10 +1066,10 @@ def test_container_updated_when_section_updated(self) -> None: ############################## Association change signals ################################## def test_associations_changed_when_component_removed(self) -> None: - html_block_1 = self._add_block_to_library(self.lib1_key, "html", "html3") + html_block_3 = self._add_block_to_library(self.lib1_key, "html", "html3") api.update_container_children( self.unit2.container_key, - [LibraryUsageLocatorV2.from_string(html_block_1["id"])], + [LibraryUsageLocatorV2.from_string(html_block_3["id"])], None, entities_action=content_api.ChildrenEntitiesAction.APPEND, ) @@ -876,7 +1077,7 @@ def test_associations_changed_when_component_removed(self) -> None: api.update_container_children( self.unit2.container_key, - [LibraryUsageLocatorV2.from_string(html_block_1["id"])], + [LibraryUsageLocatorV2.from_string(html_block_3["id"])], None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) @@ -884,13 +1085,27 @@ def test_associations_changed_when_component_removed(self) -> None: { "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "content_object": ContentObjectChangedData( - object_id=html_block_1["id"], changes=["units"], + object_id=html_block_3["id"], changes=["units"], ), }, { "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.unit2.container_key), }, + # Because we removed html3 from unit2, the ancestor containers of unit2 are also emitted as changed. + # We don't strictly need this at the moment, at least as far as keeping our search index updated. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section2.container_key), + }, ) def test_associations_changed_when_unit_removed(self) -> None: @@ -910,16 +1125,24 @@ def test_associations_changed_when_unit_removed(self) -> None: entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) self.expect_new_events( + # unit4 was removed from subsection2, so we get a notification that "parent subsection(s) have changed": { "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "content_object": ContentObjectChangedData( object_id=str(unit4.container_key), changes=["subsections"], ), }, + # We modified subsection2 by changing its list of children, so we get a CONTAINER_UPDATED event for it: { "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.subsection2.container_key), }, + # Because subsection2 itself was changed, we get change notifications for its ancestors. + # We don't strictly need this at the moment, at least as far as keeping our search index updated. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, ) def test_associations_changed_when_subsection_removed(self) -> None: @@ -968,6 +1191,7 @@ def test_associations_changed_when_components_added(self) -> None: entities_action=content_api.ChildrenEntitiesAction.APPEND, ) self.expect_new_events( + # We added html4 and html4 to a new unit, so they get "parent unit(s) changed" events: { "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "content_object": ContentObjectChangedData( @@ -980,13 +1204,29 @@ def test_associations_changed_when_components_added(self) -> None: object_id=html_block_2["id"], changes=["units"], ), }, + # We modified unit2 by changing its list of children, so we get a CONTAINER_UPDATED event for it: { "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.unit2.container_key), }, + # Because the unit itself was changed, we get change notifications for its ancestors. + # We don't strictly need this at the moment, at least as far as keeping our search index updated. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section2.container_key), + }, ) def test_associations_changed_when_units_added(self) -> None: + # Create "unit4" and "unit5" and add them to subsection2: unit4 = api.create_container(self.lib1_key, content_models.Unit, 'unit-4', 'Unit 4', None) unit5 = api.create_container(self.lib1_key, content_models.Unit, 'unit-5', 'Unit 5', None) self.clear_events() @@ -998,6 +1238,7 @@ def test_associations_changed_when_units_added(self) -> None: entities_action=content_api.ChildrenEntitiesAction.APPEND, ) self.expect_new_events( + # Each unit was added to a new subsection, so we get a "subsections changed" event for each: { "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "content_object": ContentObjectChangedData( @@ -1010,10 +1251,16 @@ def test_associations_changed_when_units_added(self) -> None: object_id=str(unit5.container_key), changes=["subsections"], ), }, + # The subsection itself was updated (its list of children changed): { "signal": LIBRARY_CONTAINER_UPDATED, "library_container": LibraryContainerData(container_key=self.subsection2.container_key), }, + # And because the subsection itself was changed, we get change notifications for its ancestors. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, ) def test_associations_changed_when_subsections_added(self) -> None: @@ -1111,11 +1358,29 @@ def test_signal_emitted_when_set_block_olx_succeeds(self) -> None: usage_key=self.problem_block_usage_key, ), }, + # Since the problem is part of a unit, we also get LIBRARY_CONTAINER_UPDATED on the parent unit. + # This is used e.g. to update "child_display_names" in the search index, if the child's name has changed. { "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData( - container_key=self.unit1.container_key, background=True, - ), + "library_container": LibraryContainerData(container_key=self.unit1.container_key), + }, + # openedx_content also lists and parent containers of affected containers as changed, and so on... + # We don't strictly need this at the moment, at least as far as keeping our search index updated. + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.subsection2.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section1.container_key), + }, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData(container_key=self.section2.container_key), }, ) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py b/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py index 270580cb2a61..9f8e5dc1cde3 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py @@ -88,7 +88,7 @@ def test_get_library_collection(self): Test retrieving a Content Library Collection """ resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.key) + URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.collection_code) ) # Check that correct Content Library Collection data retrieved @@ -103,7 +103,7 @@ def test_get_library_collection(self): random_user = UserFactory.create(username="Random", email="random@example.com") with self.as_user(random_user): resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.key) + URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.collection_code) ) assert resp.status_code == 403 @@ -113,7 +113,7 @@ def test_get_invalid_library_collection(self): """ # Fetch collection that belongs to a different library, it should fail resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib1.library_key, collection_key=self.col3.key) + URL_LIB_COLLECTION.format(lib_key=self.lib1.library_key, collection_key=self.col3.collection_code) ) assert resp.status_code == 404 @@ -247,9 +247,9 @@ def test_create_invalid_library_collection(self): assert resp.status_code == 400 - # Create collection with an existing collection.key; it should fail + # Create collection with an existing collection.collection_code; it should fail post_data_existing_key = { - "key": self.col1.key, + "key": self.col1.collection_code, "title": "Collection 4", } resp = self.client.post( @@ -275,7 +275,7 @@ def test_update_library_collection(self): "title": "Collection 3 Updated", } resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.key), + URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.collection_code), patch_data, format="json" ) @@ -297,7 +297,7 @@ def test_update_library_collection(self): "title": "Collection 3 should not update", } resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.key), + URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.collection_code), patch_data, format="json" ) @@ -313,7 +313,7 @@ def test_update_invalid_library_collection(self): } # Update collection that belongs to a different library, it should fail resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=self.lib1.library_key, collection_key=self.col3.key), + URL_LIB_COLLECTION.format(lib_key=self.lib1.library_key, collection_key=self.col3.collection_code), patch_data, format="json" ) @@ -331,7 +331,7 @@ def test_update_invalid_library_collection(self): # Update collection with invalid library_key provided, it should fail resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=123, collection_key=self.col3.key), + URL_LIB_COLLECTION.format(lib_key=123, collection_key=self.col3.collection_code), patch_data, format="json" ) @@ -342,22 +342,22 @@ def test_delete_library_collection(self): Test soft-deleting and restoring a Content Library Collection """ resp = self.client.delete( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.key) + URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.collection_code) ) assert resp.status_code == 204 resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.key) + URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.collection_code) ) assert resp.status_code == 404 resp = self.client.post( - URL_LIB_COLLECTION_RESTORE.format(lib_key=self.lib2.library_key, collection_key=self.col3.key) + URL_LIB_COLLECTION_RESTORE.format(lib_key=self.lib2.library_key, collection_key=self.col3.collection_code) ) assert resp.status_code == 204 resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.key) + URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_key=self.col3.collection_code) ) # Check that correct Content Library Collection data retrieved expected_collection = { @@ -375,7 +375,7 @@ def test_get_components(self): resp = self.client.get( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), ) assert resp.status_code == 405 @@ -388,7 +388,7 @@ def test_update_components(self): resp = self.client.patch( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), data={ "usage_keys": [ @@ -404,7 +404,7 @@ def test_update_components(self): resp = self.client.delete( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), data={ "usage_keys": [ @@ -423,7 +423,7 @@ def test_update_containers(self): resp = self.client.patch( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), data={ "usage_keys": [ @@ -440,7 +440,7 @@ def test_update_containers(self): resp = self.client.delete( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), data={ "usage_keys": [ @@ -460,7 +460,7 @@ def test_update_components_wrong_collection(self, method): resp = getattr(self.client, method)( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib2.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), data={ "usage_keys": [ @@ -478,7 +478,7 @@ def test_update_components_missing_data(self, method): resp = getattr(self.client, method)( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib2.library_key, - collection_key=self.col3.key, + collection_key=self.col3.collection_code, ), ) assert resp.status_code == 400 @@ -494,7 +494,7 @@ def test_update_components_from_another_library(self, method): resp = getattr(self.client, method)( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib2.library_key, - collection_key=self.col3.key, + collection_key=self.col3.collection_code, ), data={ "usage_keys": [ @@ -515,7 +515,7 @@ def test_update_components_permissions(self, method): resp = getattr(self.client, method)( URL_LIB_COLLECTION_COMPONENTS.format( lib_key=self.lib1.library_key, - collection_key=self.col1.key, + collection_key=self.col1.collection_code, ), ) assert resp.status_code == 403 diff --git a/openedx/core/djangoapps/video_config/transcripts_utils.py b/openedx/core/djangoapps/video_config/transcripts_utils.py index c76a2b8b0377..6e6f0fde7b8c 100644 --- a/openedx/core/djangoapps/video_config/transcripts_utils.py +++ b/openedx/core/djangoapps/video_config/transcripts_utils.py @@ -1016,7 +1016,7 @@ def get_transcript_from_openedx_content(video_block, language, output_format, tr .componentversionmedia_set .filter(media__has_file=True) .select_related('media') - .get(key=file_path) + .get(path=file_path) .media ) data = media.read_file().read() diff --git a/openedx/core/djangoapps/xblock/api.py b/openedx/core/djangoapps/xblock/api.py index 0ab620db9ab3..a4661f67b164 100644 --- a/openedx/core/djangoapps/xblock/api.py +++ b/openedx/core/djangoapps/xblock/api.py @@ -199,14 +199,14 @@ def get_component_from_usage_key(usage_key: UsageKeyV2) -> Component: This is a lower-level function that will return a Component even if there is no current draft version of that Component (because it's been soft-deleted). """ - learning_package = content_api.get_learning_package_by_key( + learning_package = content_api.get_learning_package_by_ref( str(usage_key.context_key) ) - return content_api.get_component_by_key( + return content_api.get_component_by_code( learning_package.id, namespace='xblock.v1', type_name=usage_key.block_type, - local_key=usage_key.block_id, + component_code=usage_key.block_id, ) @@ -232,9 +232,9 @@ def get_block_olx( raise NoSuchUsage(usage_key) # TODO: we should probably make a method on ComponentVersion that returns - # a content based on the name. Accessing by componentversionmedia__key is + # a content based on the name. Accessing by componentversionmedia__path is # awkward. - content = component_version.media.get(componentversionmedia__key="block.xml") + content = component_version.media.get(componentversionmedia__path="block.xml") return content.text diff --git a/openedx/core/djangoapps/xblock/learning_context/learning_context.py b/openedx/core/djangoapps/xblock/learning_context/learning_context.py index d3b4c8643a2f..22bd92701350 100644 --- a/openedx/core/djangoapps/xblock/learning_context/learning_context.py +++ b/openedx/core/djangoapps/xblock/learning_context/learning_context.py @@ -70,18 +70,3 @@ def definition_for_usage(self, usage_key, **kwargs): Retuns None if the usage key doesn't exist in this context. """ raise NotImplementedError - - def send_block_updated_event(self, usage_key): - """ - Send a "block updated" event for the block with the given usage_key in this context. - - usage_key: the UsageKeyV2 subclass used for this learning context - """ - - def send_container_updated_events(self, usage_key): - """ - Send "container updated" events for containers that contains the block with - the given usage_key in this context. - - usage_key: the UsageKeyV2 subclass used for this learning context - """ diff --git a/openedx/core/djangoapps/xblock/runtime/openedx_content_runtime.py b/openedx/core/djangoapps/xblock/runtime/openedx_content_runtime.py index 3a90fb27a5f7..8c4996869324 100644 --- a/openedx/core/djangoapps/xblock/runtime/openedx_content_runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/openedx_content_runtime.py @@ -191,7 +191,7 @@ def get_block(self, usage_key, for_parent=None, *, version: int | LatestVersion raise NoSuchUsage(usage_key) content = component_version.media.get( - componentversionmedia__key="block.xml" + componentversionmedia__path="block.xml" ) xml_node = etree.fromstring(content.text) block_type = usage_key.block_type @@ -251,13 +251,13 @@ def get_block_assets(self, block, fetch_asset_data): .componentversionmedia_set .filter(media__has_file=True) .select_related('media') - .order_by('key') + .order_by('path') ) return [ StaticFile( - name=cvm.key, - url=self._absolute_url_for_asset(component_version, cvm.key), + name=cvm.path, + url=self._absolute_url_for_asset(component_version, cvm.path), data=cvm.media.read_file().read() if fetch_asset_data else None, ) for cvm in cvm_list @@ -312,11 +312,6 @@ def save_block(self, block): ) self.authored_data_store.mark_unchanged(block) - # Signal that we've modified this block - learning_context = get_learning_context_impl(usage_key) - learning_context.send_block_updated_event(usage_key) - learning_context.send_container_updated_events(usage_key) - def _get_component_from_usage_key(self, usage_key): """ Note that Components aren't ever really truly deleted, so this will @@ -326,13 +321,13 @@ def _get_component_from_usage_key(self, usage_key): TODO: This is the third place where we're implementing this. Figure out where the definitive place should be and have everything else call that. """ - learning_package = content_api.get_learning_package_by_key(str(usage_key.lib_key)) + learning_package = content_api.get_learning_package_by_ref(str(usage_key.lib_key)) try: - component = content_api.get_component_by_key( + component = content_api.get_component_by_code( learning_package.id, namespace='xblock.v1', type_name=usage_key.block_type, - local_key=usage_key.block_id, + component_code=usage_key.block_id, ) except ObjectDoesNotExist as exc: raise NoSuchUsage(usage_key) from exc @@ -447,7 +442,7 @@ def _lookup_asset_url(self, block: XBlock, asset_path: str) -> str | None: component_version .componentversionmedia_set .filter(media__has_file=True) - .get(key=f"static/{asset_path}") + .get(path=f"static/{asset_path}") ) except ObjectDoesNotExist: try: @@ -458,7 +453,7 @@ def _lookup_asset_url(self, block: XBlock, asset_path: str) -> str | None: component_version .componentversionmedia_set .filter(media__has_file=True) - .get(key=f"static/{asset_path}") + .get(path=f"static/{asset_path}") ) except ObjectDoesNotExist: # This means we see a path that _looks_ like it should be a static diff --git a/requirements/constraints.txt b/requirements/constraints.txt index f051cc8f6114..73e9f6effa10 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -65,7 +65,7 @@ numpy<2.0.0 # breaking changes which openedx-core devs want to roll out manually. New patch versions # are OK to accept automatically. # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269 -openedx-core<0.40 +openedx-core<0.45 # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 298f05e12aab..6dcdb1f96219 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -840,7 +840,7 @@ openedx-calc==5.0.0 # via # -r requirements/edx/kernel.in # xblocks-contrib -openedx-core==0.39.2 +openedx-core @ git+https://github.com/openedx/openedx-core.git@braden/emit-events # via # -c requirements/constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index ea0fd8e68eae..e7e5e9c37765 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1394,7 +1394,7 @@ openedx-calc==5.0.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # xblocks-contrib -openedx-core==0.39.2 +openedx-core @ git+https://github.com/openedx/openedx-core.git@braden/emit-events # via # -c requirements/constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 194b5438ce1f..473b3bf81fa2 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1018,7 +1018,7 @@ openedx-calc==5.0.0 # via # -r requirements/edx/base.txt # xblocks-contrib -openedx-core==0.39.2 +openedx-core @ git+https://github.com/openedx/openedx-core.git@braden/emit-events # via # -c requirements/constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index eb9732129f73..39da746771df 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1065,7 +1065,7 @@ openedx-calc==5.0.0 # via # -r requirements/edx/base.txt # xblocks-contrib -openedx-core==0.39.2 +openedx-core @ git+https://github.com/openedx/openedx-core.git@braden/emit-events # via # -c requirements/constraints.txt # -r requirements/edx/base.txt