From 27c4ea44f22959c3849cd2cc08908cbbf63cb6d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Mon, 19 May 2025 12:34:39 -0500 Subject: [PATCH] feat: Add units dict to index [FC-0083] (#36650) * Adds the units dict to the component search documents. * Send CONTENT_OBJECT_ASSOCIATIONS_CHANGED signal when add/remove components in units. --- openedx/core/djangoapps/content/search/api.py | 11 +++ .../djangoapps/content/search/documents.py | 63 +++++++++++++++ .../djangoapps/content/search/handlers.py | 3 + .../content/search/tests/test_api.py | 60 ++++++++++++--- .../content_libraries/api/blocks.py | 4 +- .../content_libraries/api/containers.py | 8 ++ .../content_libraries/tests/test_api.py | 77 +++++++++++++++++++ 7 files changed, 212 insertions(+), 14 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index b866f13dc4..d81128f659 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -52,6 +52,7 @@ from .documents import ( searchable_doc_for_key, searchable_doc_tags, searchable_doc_tags_for_collection, + searchable_doc_units, ) log = logging.getLogger(__name__) @@ -451,6 +452,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None, incremental=Fa doc.update(searchable_doc_for_library_block(metadata)) doc.update(searchable_doc_tags(metadata.usage_key)) doc.update(searchable_doc_collections(metadata.usage_key)) + doc.update(searchable_doc_units(metadata.usage_key)) docs.append(doc) except Exception as err: # pylint: disable=broad-except status_cb(f"Error indexing library component {component}: {err}") @@ -853,6 +855,15 @@ def upsert_item_collections_index_docs(opaque_key: OpaqueKey): _update_index_docs([doc]) +def upsert_item_units_index_docs(opaque_key: OpaqueKey): + """ + Updates the units data in documents for the given Course/Library block + """ + doc = {Fields.id: meili_id_from_opaque_key(opaque_key)} + doc.update(searchable_doc_units(opaque_key)) + _update_index_docs([doc]) + + def upsert_collection_tags_index_docs(collection_key: LibraryCollectionLocator): """ Updates the tags data in documents for the given library collection diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 28b5e74450..36698da6a0 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -67,6 +67,10 @@ class Fields: collections = "collections" collections_display_name = "display_name" collections_key = "key" + # Units (dictionary) that this object belongs to. + units = "units" + units_display_name = "display_name" + units_key = "key" # The "content" field is a dictionary of arbitrary data, depending on the block_type. # It comes from each XBlock's index_dictionary() method (if present) plus some processing. @@ -369,6 +373,54 @@ def _collections_for_content_object(object_id: OpaqueKey) -> dict: return result +def _units_for_content_object(object_id: OpaqueKey) -> dict: + """ + Given an XBlock, course, library, etc., get the units for its index doc. + + e.g. for something in Units "UNIT_A" and "UNIT_B", this would return: + { + "units": { + "display_name": ["Unit A", "Unit B"], + "key": ["UNIT_A", "UNIT_B"], + } + } + + If the object is in no collections, returns: + { + "collections": { + "display_name": [], + "key": [], + }, + } + """ + result = { + Fields.units: { + Fields.units_display_name: [], + Fields.units_key: [], + } + } + + # Gather the units associated with this object + units = None + try: + if isinstance(object_id, UsageKey): + units = lib_api.get_containers_contains_component(object_id) + else: + log.warning(f"Unexpected key type for {object_id}") + + except ObjectDoesNotExist: + log.warning(f"No library item found for {object_id}") + + if not units: + return result + + for unit in units: + result[Fields.units][Fields.units_display_name].append(unit.display_name) + result[Fields.units][Fields.units_key].append(str(unit.container_key)) + + return result + + def _published_data_from_block(block_published) -> dict: """ Given an library block get the published data. @@ -460,6 +512,17 @@ def searchable_doc_collections(opaque_key: OpaqueKey) -> dict: return doc +def searchable_doc_units(opaque_key: OpaqueKey) -> dict: + """ + Generate a dictionary document suitable for ingestion into a search engine + like Meilisearch or Elasticsearch, with the units data for the given content object. + """ + doc = searchable_doc_for_key(opaque_key) + doc.update(_units_for_content_object(opaque_key)) + + return doc + + def searchable_doc_tags_for_collection( collection_key: LibraryCollectionLocator ) -> dict: diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 315d3cde53..fdc52a968e 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -46,6 +46,7 @@ from .api import ( upsert_content_object_tags_index_doc, upsert_collection_tags_index_docs, upsert_item_collections_index_docs, + upsert_item_units_index_docs, ) from .tasks import ( delete_library_block_index_doc, @@ -263,6 +264,8 @@ def content_object_associations_changed_handler(**kwargs) -> None: upsert_content_object_tags_index_doc(opaque_key) if not content_object.changes or "collections" in content_object.changes: upsert_item_collections_index_docs(opaque_key) + if not content_object.changes or "units" in content_object.changes: + upsert_item_units_index_docs(opaque_key) @receiver(LIBRARY_CONTAINER_CREATED) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 90b6a407e7..2093b3dc85 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -8,7 +8,7 @@ import copy from datetime import datetime, timezone from unittest.mock import MagicMock, Mock, call, patch from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryCollectionLocator +from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator import ddt import pytest @@ -134,8 +134,8 @@ class TestSearchApi(ModuleStoreTestCase): lib_access, _ = SearchAccess.objects.get_or_create(context_key=self.library.key) # Populate it with 2 problems, freezing the date so we can verify created date serializes correctly. - created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) - with freeze_time(created_date): + self.created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(self.created_date): self.problem1 = library_api.create_library_block(self.library.key, "problem", "p1") self.problem2 = library_api.create_library_block(self.library.key, "problem", "p2") # Update problem1, freezing the date so we can verify modified date serializes correctly. @@ -155,7 +155,7 @@ class TestSearchApi(ModuleStoreTestCase): "type": "library_block", "access_id": lib_access.id, "last_published": None, - "created": created_date.timestamp(), + "created": self.created_date.timestamp(), "modified": modified_date.timestamp(), "publish_status": "never", } @@ -172,8 +172,8 @@ class TestSearchApi(ModuleStoreTestCase): "type": "library_block", "access_id": lib_access.id, "last_published": None, - "created": created_date.timestamp(), - "modified": created_date.timestamp(), + "created": self.created_date.timestamp(), + "modified": self.created_date.timestamp(), "publish_status": "never", } @@ -189,7 +189,7 @@ class TestSearchApi(ModuleStoreTestCase): # Create a collection: self.learning_package = authoring_api.get_learning_package_by_key(self.library.key) - with freeze_time(created_date): + with freeze_time(self.created_date): self.collection = authoring_api.create_collection( learning_package_id=self.learning_package.id, key="MYCOL", @@ -210,8 +210,8 @@ class TestSearchApi(ModuleStoreTestCase): "num_children": 0, "context_key": "lib:org1:lib", "org": "org1", - "created": created_date.timestamp(), - "modified": created_date.timestamp(), + "created": self.created_date.timestamp(), + "modified": self.created_date.timestamp(), "access_id": lib_access.id, "published": { "num_children": 0 @@ -220,7 +220,7 @@ class TestSearchApi(ModuleStoreTestCase): } # Create a unit: - with freeze_time(created_date): + with freeze_time(self.created_date): self.unit = library_api.create_container( library_key=self.library.key, container_type=library_api.ContainerType.Unit, @@ -242,8 +242,8 @@ class TestSearchApi(ModuleStoreTestCase): "publish_status": "never", "context_key": "lib:org1:lib", "org": "org1", - "created": created_date.timestamp(), - "modified": created_date.timestamp(), + "created": self.created_date.timestamp(), + "modified": self.created_date.timestamp(), "last_published": None, "access_id": lib_access.id, "breadcrumbs": [{"display_name": "Library"}], @@ -268,9 +268,11 @@ class TestSearchApi(ModuleStoreTestCase): doc_problem1 = copy.deepcopy(self.doc_problem1) doc_problem1["tags"] = {} doc_problem1["collections"] = {'display_name': [], 'key': []} + doc_problem1["units"] = {'display_name': [], 'key': []} doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} doc_problem2["collections"] = {'display_name': [], 'key': []} + doc_problem2["units"] = {'display_name': [], 'key': []} doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} doc_unit = copy.deepcopy(self.unit_dict) @@ -300,9 +302,11 @@ class TestSearchApi(ModuleStoreTestCase): doc_problem1 = copy.deepcopy(self.doc_problem1) doc_problem1["tags"] = {} doc_problem1["collections"] = {"display_name": [], "key": []} + doc_problem1["units"] = {'display_name': [], 'key': []} doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} doc_problem2["collections"] = {"display_name": [], "key": []} + doc_problem2["units"] = {'display_name': [], 'key': []} doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} doc_unit = copy.deepcopy(self.unit_dict) @@ -417,6 +421,7 @@ class TestSearchApi(ModuleStoreTestCase): doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} doc_problem2["collections"] = {'display_name': [], 'key': []} + doc_problem2["units"] = {'display_name': [], 'key': []} orig_from_component = library_api.LibraryXBlockMetadata.from_component @@ -939,3 +944,34 @@ class TestSearchApi(ModuleStoreTestCase): ], any_order=True, ) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_block_in_units(self, mock_meilisearch): + with freeze_time(self.created_date): + library_api.update_container_children( + LibraryContainerLocator.from_string(self.unit_key), + [self.problem1.usage_key], + None, + ) + + doc_block_with_units = { + "id": self.doc_problem1["id"], + "units": { + "display_name": [self.unit.display_name], + "key": [self.unit_key], + }, + } + new_unit_dict = { + **self.unit_dict, + "num_children": 1, + 'content': {'child_usage_keys': [self.doc_problem1["usage_key"]]} + } + + assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2 + mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( + [ + call([doc_block_with_units]), + call([new_unit_dict]), + ], + any_order=True, + ) diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index d693ff30d7..dd67095597 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -646,11 +646,11 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2, user_id: int | None ) ) - # Add tags and collections back to index + # Add tags, collections and units back to index CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( content_object=ContentObjectChangedData( object_id=str(usage_key), - changes=["collections", "tags"], + changes=["collections", "tags", "units"], ), ) diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index d97a6100a6..01974a441e 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -429,6 +429,14 @@ def update_container_children( created_by=user_id, entities_action=entities_action, ) + + for key in children_ids: + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + content_object=ContentObjectChangedData( + object_id=str(key), + changes=["units"], + ), + ) case _: raise ValueError(f"Invalid container type: {container_type}") diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 3a1121da38..085f69d0a3 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -839,6 +839,83 @@ class ContentLibraryContainersTest(ContentLibrariesRestApiTest): self._set_library_block_fields(self.html_block_usage_key, {"data": block_olx, "metadata": {}}) self._validate_calls_of_html_block(container_update_event_receiver) + def test_call_object_changed_signal_when_remove_component(self): + html_block_1 = self._add_block_to_library( + self.lib1.library_key, "html", "html3", + ) + api.update_container_children( + self.unit2.container_key, + [UsageKey.from_string(html_block_1["id"])], + None, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + + event_reciver = mock.Mock() + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_reciver) + api.update_container_children( + self.unit2.container_key, + [UsageKey.from_string(html_block_1["id"])], + None, + entities_action=authoring_api.ChildrenEntitiesAction.REMOVE, + ) + + assert event_reciver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, + "sender": None, + "content_object": ContentObjectChangedData( + object_id=html_block_1["id"], + changes=["units"], + ), + }, + event_reciver.call_args_list[0].kwargs, + ) + + def test_call_object_changed_signal_when_add_component(self): + event_reciver = mock.Mock() + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_reciver) + html_block_1 = self._add_block_to_library( + self.lib1.library_key, "html", "html4", + ) + html_block_2 = self._add_block_to_library( + self.lib1.library_key, "html", "html5", + ) + + api.update_container_children( + self.unit2.container_key, + [ + UsageKey.from_string(html_block_1["id"]), + UsageKey.from_string(html_block_2["id"]) + ], + None, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + + assert event_reciver.call_count == 2 + self.assertDictContainsSubset( + { + "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, + "sender": None, + "content_object": ContentObjectChangedData( + object_id=html_block_1["id"], + changes=["units"], + ), + }, + event_reciver.call_args_list[0].kwargs, + ) + self.assertDictContainsSubset( + { + "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, + "sender": None, + "content_object": ContentObjectChangedData( + object_id=html_block_2["id"], + changes=["units"], + ), + }, + event_reciver.call_args_list[1].kwargs, + ) + def test_delete_component_and_revert(self): """ When a component is deleted and then the delete is reverted, signals