diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index dda4bea63ac9..4a775a710da6 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -557,6 +557,22 @@ def upsert_library_block_index_doc(usage_key: UsageKey) -> None: _update_index_docs(docs) +def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collection_key: str) -> None: + """ + Creates or updates the document for the given Library Collection in the search index + """ + content_library = lib_api.ContentLibrary.objects.get_by_key(library_key) + collection = authoring_api.get_collection( + learning_package_id=content_library.learning_package_id, + collection_key=collection_key, + ) + docs = [ + searchable_doc_for_collection(collection) + ] + + _update_index_docs(docs) + + def upsert_content_library_index_docs(library_key: LibraryLocatorV2) -> None: """ Creates or updates the documents for the given Content Library in the search index diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index b5cd8dabddd3..6f19b610fe86 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -28,7 +28,11 @@ class Fields: id = "id" usage_key = "usage_key" type = "type" # DocType.course_block or DocType.library_block (see below) - block_id = "block_id" # The block_id part of the usage key. Sometimes human-readable, sometimes a random hex ID + # The block_id part of the usage key for course or library blocks. + # If it's a collection, the collection.key is stored here. + # Sometimes human-readable, sometimes a random hex ID + # Is only unique within the given context_key. + block_id = "block_id" display_name = "display_name" description = "description" modified = "modified" @@ -65,6 +69,10 @@ class Fields: # Text (html) blocks have an "html_content" key in here, capa has "capa_content" and "problem_types", and so on. content = "content" + # Collections use this field to communicate how many entities/components they contain. + # Structural XBlocks may use this one day to indicate how many child blocks they ocntain. + num_children = "num_children" + # Note: new fields or values can be added at any time, but if they need to be indexed for filtering or keyword # search, the index configuration will need to be changed, which is only done as part of the 'reindex_studio' # command (changing those settings on an large active index is not recommended). @@ -355,6 +363,7 @@ def searchable_doc_for_collection(collection) -> dict: """ doc = { Fields.id: collection.id, + Fields.block_id: collection.key, Fields.type: DocType.collection, Fields.display_name: collection.title, Fields.description: collection.description, @@ -364,6 +373,7 @@ def searchable_doc_for_collection(collection) -> dict: # If related contentlibrary is found, it will override this value below. # Mostly contentlibrary.library_key == learning_package.key Fields.context_key: collection.learning_package.key, + Fields.num_children: collection.entities.count(), } # Just in case learning_package is not related to a library try: diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 35f3ad68ee21..6a341c92ed2b 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -6,20 +6,23 @@ from django.db.models.signals import post_delete from django.dispatch import receiver +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey from openedx_events.content_authoring.data import ( ContentLibraryData, ContentObjectChangedData, LibraryBlockData, + LibraryCollectionData, XBlockData, ) -from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import UsageKey from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_DELETED, CONTENT_LIBRARY_UPDATED, LIBRARY_BLOCK_CREATED, LIBRARY_BLOCK_DELETED, LIBRARY_BLOCK_UPDATED, + LIBRARY_COLLECTION_CREATED, + LIBRARY_COLLECTION_UPDATED, XBLOCK_CREATED, XBLOCK_DELETED, XBLOCK_UPDATED, @@ -34,6 +37,7 @@ delete_library_block_index_doc, delete_xblock_index_doc, update_content_library_index_docs, + update_library_collection_index_doc, upsert_library_block_index_doc, upsert_xblock_index_doc, ) @@ -151,6 +155,27 @@ def content_library_updated_handler(**kwargs) -> None: update_content_library_index_docs.apply(args=[str(content_library_data.library_key)]) +@receiver(LIBRARY_COLLECTION_CREATED) +@receiver(LIBRARY_COLLECTION_UPDATED) +@only_if_meilisearch_enabled +def library_collection_updated_handler(**kwargs) -> None: + """ + Create or update the index for the content library collection + """ + library_collection = kwargs.get("library_collection", None) + if not library_collection or not isinstance(library_collection, LibraryCollectionData): # pragma: no cover + log.error("Received null or incorrect data for event") + return + + # Update collection index synchronously to make sure that search index is updated before + # the frontend invalidates/refetches index. + # See content_library_updated_handler for more details. + update_library_collection_index_doc.apply(args=[ + str(library_collection.library_key), + library_collection.collection_key, + ]) + + @receiver(CONTENT_OBJECT_ASSOCIATIONS_CHANGED) @only_if_meilisearch_enabled def content_object_associations_changed_handler(**kwargs) -> None: diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index dfd603776981..d9dad834db29 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -84,3 +84,16 @@ def update_content_library_index_docs(library_key_str: str) -> None: # Delete all documents in this library that were not published by above function # as this task is also triggered on discard event. api.delete_all_draft_docs_for_library(library_key) + + +@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) +@set_code_owner_attribute +def update_library_collection_index_doc(library_key_str: str, collection_key: str) -> None: + """ + Celery task to update the content index documents for a library collection + """ + library_key = LibraryLocatorV2.from_string(library_key_str) + + log.info("Updating content index documents for collection %s in library%s", collection_key, library_key) + + api.upsert_library_collection_index_doc(library_key, collection_key) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 0e5de4da91fe..023265f4d0f5 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -186,16 +186,18 @@ def setUp(self): description="my collection description" ) self.collection_dict = { - 'id': self.collection.id, - 'type': 'collection', - 'display_name': 'my_collection', - 'description': 'my collection description', - 'context_key': 'lib:org1:lib', - 'org': 'org1', - 'created': created_date.timestamp(), - 'modified': created_date.timestamp(), + "id": self.collection.id, + "block_id": self.collection.key, + "type": "collection", + "display_name": "my_collection", + "description": "my collection description", + "num_children": 0, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": created_date.timestamp(), "access_id": lib_access.id, - 'breadcrumbs': [{'display_name': 'Library'}] + "breadcrumbs": [{"display_name": "Library"}], } @override_settings(MEILISEARCH_ENABLED=False) @@ -415,38 +417,101 @@ def test_index_library_block_tags(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_index_library_block_collections(self, mock_meilisearch): + def test_index_library_block_and_collections(self, mock_meilisearch): """ - Test indexing an Library Block that is in two collections. + Test indexing an Library Block and the Collections it's in. """ - collection1 = authoring_api.create_collection( - learning_package_id=self.library.learning_package.id, - key="COL1", - title="Collection 1", - created_by=None, - description="First Collection", - ) - - collection2 = authoring_api.create_collection( - learning_package_id=self.library.learning_package.id, - key="COL2", - title="Collection 2", - created_by=None, - description="Second Collection", - ) + # Create collections (these internally call `upsert_library_collection_index_doc`) + created_date = datetime(2023, 5, 6, 7, 8, 9, tzinfo=timezone.utc) + with freeze_time(created_date): + collection1 = library_api.create_library_collection( + self.library.key, + collection_key="COL1", + title="Collection 1", + created_by=None, + description="First Collection", + ) - # Add Problem1 to both Collections (these internally call `upsert_block_collections_index_docs`) - # (adding in reverse order to test sorting of collection tag)) - for collection in (collection2, collection1): - library_api.update_library_collection_components( + collection2 = library_api.create_library_collection( self.library.key, - collection_key=collection.key, - usage_keys=[ - self.problem1.usage_key, - ], + collection_key="COL2", + title="Collection 2", + created_by=None, + description="Second Collection", ) - # Build expected docs with collections at each stage + # Add Problem1 to both Collections (these internally call `upsert_block_collections_index_docs` and + # `upsert_library_collection_index_doc`) + # (adding in reverse order to test sorting of collection tag) + updated_date = datetime(2023, 6, 7, 8, 9, 10, tzinfo=timezone.utc) + with freeze_time(updated_date): + for collection in (collection2, collection1): + library_api.update_library_collection_components( + self.library.key, + collection_key=collection.key, + usage_keys=[ + self.problem1.usage_key, + ], + ) + + # Build expected docs at each stage + lib_access, _ = SearchAccess.objects.get_or_create(context_key=self.library.key) + doc_collection1_created = { + "id": collection1.id, + "block_id": collection1.key, + "type": "collection", + "display_name": "Collection 1", + "description": "First Collection", + "num_children": 0, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": created_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + } + doc_collection2_created = { + "id": collection2.id, + "block_id": collection2.key, + "type": "collection", + "display_name": "Collection 2", + "description": "Second Collection", + "num_children": 0, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": created_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + } + doc_collection2_updated = { + "id": collection2.id, + "block_id": collection2.key, + "type": "collection", + "display_name": "Collection 2", + "description": "Second Collection", + "num_children": 1, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": updated_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + } + doc_collection1_updated = { + "id": collection1.id, + "block_id": collection1.key, + "type": "collection", + "display_name": "Collection 1", + "description": "First Collection", + "num_children": 1, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": updated_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + } doc_problem_with_collection1 = { "id": self.doc_problem1["id"], "collections": { @@ -462,9 +527,13 @@ def test_index_library_block_collections(self, mock_meilisearch): }, } - assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2 + assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 6 mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( [ + call([doc_collection1_created]), + call([doc_collection2_created]), + call([doc_collection2_updated]), + call([doc_collection1_updated]), call([doc_problem_with_collection1]), call([doc_problem_with_collection2]), ], diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index 97dcb5e79066..7ff330c0b491 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -298,10 +298,12 @@ def test_collection_with_library(self): doc = searchable_doc_for_collection(self.collection) assert doc == { "id": self.collection.id, + "block_id": self.collection.key, "type": "collection", "org": "edX", "display_name": "Toy Collection", "description": "my toy collection description", + "num_children": 1, "context_key": "lib:edX:2012_Fall", "access_id": self.library_access_id, "breadcrumbs": [{"display_name": "some content_library"}], @@ -327,9 +329,11 @@ def test_collection_with_no_library(self): doc = searchable_doc_for_collection(collection) assert doc == { "id": collection.id, + "block_id": collection.key, "type": "collection", "display_name": "my_collection", "description": "my collection description", + "num_children": 0, "context_key": learning_package.key, "access_id": self.toy_course_access_id, "breadcrumbs": [{"display_name": "some learning_package"}],