From d48ae03ec2490fa8e3ad259b9bf28b5ffdc8e140 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 19 Aug 2024 15:54:21 +0530 Subject: [PATCH 01/12] feat: index library collections in meillisearch --- openedx/core/djangoapps/content/search/api.py | 42 +++++++++++++++++-- .../djangoapps/content/search/documents.py | 23 ++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 4262b8a7b13f..14a250205aff 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -19,6 +19,7 @@ from meilisearch.models.task import TaskInfo from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import LibraryLocatorV2 +from openedx_learning.api import authoring as authoring_api from common.djangoapps.student.roles import GlobalStaff from rest_framework.request import Request from common.djangoapps.student.role_helpers import get_course_roles @@ -31,8 +32,9 @@ Fields, meili_id_from_opaque_key, searchable_doc_for_course_block, + searchable_doc_for_collection, searchable_doc_for_library_block, - searchable_doc_tags + searchable_doc_tags, ) log = logging.getLogger(__name__) @@ -294,12 +296,16 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: status_cb("Counting courses...") num_courses = CourseOverview.objects.count() + # Get the list of collections + status_cb("Counting collections...") + num_collections = authoring_api.get_collections().count() + # Some counters so we can track our progress as indexing progresses: - num_contexts = num_courses + num_libraries + num_contexts = num_courses + num_libraries + num_collections num_contexts_done = 0 # How many courses/libraries we've indexed num_blocks_done = 0 # How many individual components/XBlocks we've indexed - status_cb(f"Found {num_courses} courses and {num_libraries} libraries.") + status_cb(f"Found {num_courses} courses, {num_libraries} libraries and {num_collections} collections.") with _using_temp_index(status_cb) as temp_index_name: ############## Configure the index ############## @@ -415,7 +421,35 @@ def add_with_children(block): num_contexts_done += 1 num_blocks_done += len(docs) - status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses and libraries.") + ############## Collections ############## + status_cb("Indexing collections...") + # To reduce memory usage on large instances, split up the Collections into pages of 1,00 collections: + paginator = Paginator(authoring_api.get_collections(), 100) + for p in paginator.page_range: + docs = [] + for collection in paginator.page(p).object_list: + status_cb( + f"{num_contexts_done + 1}/{num_contexts}. Now indexing collection {collection.name} ({collection.id})" + ) + try: + doc = searchable_doc_for_collection(collection) + # Uncomment below line once collections are tagged. + # doc.update(searchable_doc_tags(metadata.usage_key)) + docs.append(doc) + except Exception as err: # pylint: disable=broad-except + status_cb(f"Error indexing collection {collection}: {err}") + + if docs: + try: + # Add docs in batch of 100 at once (usually faster than adding one at a time): + _wait_for_meili_task(client.index(temp_index_name).add_documents(docs)) + except (TypeError, KeyError, MeilisearchError) as err: + status_cb(f"Error indexing collection {collection}: {err}") + + num_contexts_done += len(docs) + + + status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses, collections and libraries.") def upsert_xblock_index_doc(usage_key: UsageKey, recursive: bool = True) -> None: diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 032023f97c60..33fac9b5da6e 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -65,6 +65,7 @@ class DocType: """ course_block = "course_block" library_block = "library_block" + collection = "collection" def meili_id_from_opaque_key(usage_key: UsageKey) -> str: @@ -275,3 +276,25 @@ def searchable_doc_for_course_block(block) -> dict: doc.update(_fields_from_block(block)) return doc + + +def searchable_doc_for_collection(collection) -> dict: + """ + Generate a dictionary document suitable for ingestion into a search engine + like Meilisearch or Elasticsearch, so that the given collection can be + found using faceted search. + """ + # TODO: Add collection key once new collectionKey type is added to opaque_keys + doc = { + Fields.id: collection.id, + Fields.type: DocType.collection, + Fields.display_name: collection.name, + Fields.created: collection.created.timestamp(), + Fields.modified: collection.modified.timestamp(), + # Using learning_package.key as context key. + Fields.context_key: str(collection.learning_package.key), + # TODO: Get org value from collection_key.context_key.org + # Fields.org: str(collection.collection_key.context_key.org), + } + + return doc From 9e6dc436057a7b06b407aa55af75dc8e170755a6 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 19 Aug 2024 19:17:12 +0530 Subject: [PATCH 02/12] test: add collection indexing test --- .../content/search/tests/test_api.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index e8616cee60a8..cc25fa052883 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -12,6 +12,7 @@ import ddt from django.test import override_settings from freezegun import freeze_time +from openedx_learning.api import authoring as authoring_api from organizations.tests.factories import OrganizationFactory from common.djangoapps.student.tests.factories import UserFactory @@ -174,6 +175,23 @@ def setUp(self): tagging_api.add_tag_to_taxonomy(self.taxonomyB, "three") tagging_api.add_tag_to_taxonomy(self.taxonomyB, "four") + # Create a collection: + self.learning_package = authoring_api.get_learning_package_by_key(self.library.key) + self.collection_dict = { + 'id': 1, + 'type': 'collection', + 'display_name': 'my_collection', + 'context_key': 'lib:org1:lib', + 'created': created_date.timestamp(), + 'modified': created_date.timestamp(), + } + with freeze_time(created_date): + self.collection = authoring_api.create_collection( + learning_package_id=self.learning_package.id, + name="my_collection", + description="my collection description" + ) + @override_settings(MEILISEARCH_ENABLED=False) def test_reindex_meilisearch_disabled(self, mock_meilisearch): with self.assertRaises(RuntimeError): @@ -199,6 +217,7 @@ def test_reindex_meilisearch(self, mock_meilisearch): [ call([doc_sequential, doc_vertical]), call([doc_problem1, doc_problem2]), + call([self.collection_dict]), ], any_order=True, ) From 7769685c9b84a5ebc5f71fdac4beb91431a5c38f Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 19 Aug 2024 19:19:30 +0530 Subject: [PATCH 03/12] fix: lint issues --- openedx/core/djangoapps/content/search/api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 14a250205aff..85378ef50d75 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -423,13 +423,14 @@ def add_with_children(block): ############## Collections ############## status_cb("Indexing collections...") - # To reduce memory usage on large instances, split up the Collections into pages of 1,00 collections: + # To reduce memory usage on large instances, split up the Collections into pages of 100 collections: paginator = Paginator(authoring_api.get_collections(), 100) for p in paginator.page_range: docs = [] for collection in paginator.page(p).object_list: status_cb( - f"{num_contexts_done + 1}/{num_contexts}. Now indexing collection {collection.name} ({collection.id})" + f"{num_contexts_done + 1}/{num_contexts}. " + f"Now indexing collection {collection.name} ({collection.id})" ) try: doc = searchable_doc_for_collection(collection) @@ -444,11 +445,10 @@ def add_with_children(block): # Add docs in batch of 100 at once (usually faster than adding one at a time): _wait_for_meili_task(client.index(temp_index_name).add_documents(docs)) except (TypeError, KeyError, MeilisearchError) as err: - status_cb(f"Error indexing collection {collection}: {err}") + status_cb(f"Error indexing collection batch {p}: {err}") num_contexts_done += len(docs) - status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses, collections and libraries.") From 9ee0bd8057dfffef5b0e1ef92e0479018e753478 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 20 Aug 2024 16:19:52 +0530 Subject: [PATCH 04/12] refactor: include library details with collection --- openedx/core/djangoapps/content/search/api.py | 5 +++-- .../djangoapps/content/search/documents.py | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 85378ef50d75..5b02a7f3c402 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -435,10 +435,12 @@ def add_with_children(block): try: doc = searchable_doc_for_collection(collection) # Uncomment below line once collections are tagged. - # doc.update(searchable_doc_tags(metadata.usage_key)) + # doc.update(searchable_doc_tags(collection.id)) docs.append(doc) except Exception as err: # pylint: disable=broad-except status_cb(f"Error indexing collection {collection}: {err}") + finally: + num_contexts_done += 1 if docs: try: @@ -447,7 +449,6 @@ def add_with_children(block): except (TypeError, KeyError, MeilisearchError) as err: status_cb(f"Error indexing collection batch {p}: {err}") - num_contexts_done += len(docs) status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses, collections and libraries.") diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 33fac9b5da6e..0dc6c924e9b4 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -27,6 +27,7 @@ class Fields: 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 display_name = "display_name" + description = "description" modified = "modified" created = "created" last_published = "last_published" @@ -284,17 +285,24 @@ def searchable_doc_for_collection(collection) -> dict: like Meilisearch or Elasticsearch, so that the given collection can be found using faceted search. """ - # TODO: Add collection key once new collectionKey type is added to opaque_keys doc = { Fields.id: collection.id, Fields.type: DocType.collection, Fields.display_name: collection.name, + Fields.description: collection.description, Fields.created: collection.created.timestamp(), Fields.modified: collection.modified.timestamp(), - # Using learning_package.key as context key. - Fields.context_key: str(collection.learning_package.key), - # TODO: Get org value from collection_key.context_key.org - # Fields.org: str(collection.collection_key.context_key.org), } + # Just in case learning_package is not related to a library + if hasattr(collection.learning_package, 'contentlibrary'): + context_key = collection.learning_package.contentlibrary.library_key + org = str(context_key.org) + doc.update({ + Fields.context_key: str(context_key), + Fields.org: org, + Fields.access_id: _meili_access_id_from_context_key(context_key), + }) + # Add the breadcrumbs. + doc[Fields.breadcrumbs] = [{"display_name": collection.learning_package.title}] return doc From 88783ba698d8298de63b4c131c2ac0e01dbf3ccf Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 20 Aug 2024 16:58:13 +0530 Subject: [PATCH 05/12] test: update collection indexing tests --- .../content/search/tests/test_api.py | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index cc25fa052883..f13891e32d54 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -6,7 +6,7 @@ import copy from datetime import datetime, timezone -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, Mock, call, patch from opaque_keys.edx.keys import UsageKey import ddt @@ -181,9 +181,13 @@ def setUp(self): 'id': 1, '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(), + "access_id": lib_access.id, + 'breadcrumbs': [{'display_name': 'Library'}] } with freeze_time(created_date): self.collection = authoring_api.create_collection( @@ -222,6 +226,22 @@ def test_reindex_meilisearch(self, mock_meilisearch): any_order=True, ) + @override_settings(MEILISEARCH_ENABLED=True) + @patch( + "openedx.core.djangoapps.content.search.api.searchable_doc_for_collection", + Mock(side_effect=Exception("Failed to generate document")), + ) + def test_reindex_meilisearch_collection_error(self, mock_meilisearch): + + mock_logger = Mock() + api.rebuild_index(mock_logger) + assert call( + [self.collection_dict] + ) not in mock_meilisearch.return_value.index.return_value.add_documents.mock_calls + mock_logger.assert_any_call( + f"Error indexing collection {self.collection}: Failed to generate document" + ) + @override_settings(MEILISEARCH_ENABLED=True) def test_reindex_meilisearch_library_block_error(self, mock_meilisearch): From 18c7d07e5b441a7a1e4d73ddeb96cbafb6634ed9 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 20 Aug 2024 17:05:34 +0530 Subject: [PATCH 06/12] fix: lint issues --- openedx/core/djangoapps/content/search/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 5b02a7f3c402..2c7e1fe33514 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -449,7 +449,6 @@ def add_with_children(block): except (TypeError, KeyError, MeilisearchError) as err: status_cb(f"Error indexing collection batch {p}: {err}") - status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses, collections and libraries.") From ca0c8ce8b897fbf6e690f8d7ffb1a573c0e3fdfe Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 20 Aug 2024 19:21:25 +0530 Subject: [PATCH 07/12] build: point to temporary openedx-learning branch [REMOVE before merge] --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 3 ++- requirements/edx/testing.txt | 2 +- 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 74263b5f7141..5a1e4dc78c46 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -93,7 +93,7 @@ libsass==0.10.0 click==8.1.6 # pinning this version to avoid updates while the library is being developed -openedx-learning==0.11.1 +openedx-learning==0.11.2 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. openai<=0.28.1 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 27de7847f284..7e2ce3c3b060 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -823,7 +823,7 @@ openedx-filters==1.9.0 # -r requirements/edx/kernel.in # lti-consumer-xblock # ora2 -openedx-learning==0.11.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 0bdc8144ee71..8a97792c514a 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1372,7 +1372,7 @@ openedx-filters==1.9.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # ora2 -openedx-learning==0.11.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 91b30d81df6d..1973d6a84699 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -982,7 +982,7 @@ openedx-filters==1.9.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-learning==0.11.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index a5b510742ac7..d2e00e4b73b5 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -119,7 +119,8 @@ openedx-calc # Library supporting mathematical calculatio openedx-django-require openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) -openedx-learning # Open edX Learning core (experimental) +# FIXME Remove this line after release of openedx-learning +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api # Open edX Learning core (experimental) openedx-mongodbproxy openedx-django-wiki path diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 2092c9354834..a1a44f8dd595 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1033,7 +1033,7 @@ openedx-filters==1.9.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-learning==0.11.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 61c8715ac4ed58b37da82265ed645b3299671059 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 21 Aug 2024 16:52:56 +0530 Subject: [PATCH 08/12] refactor: rebase with latest changes --- openedx/core/djangoapps/content/search/api.py | 4 ++-- openedx/core/djangoapps/content/search/documents.py | 8 ++++++-- openedx/core/djangoapps/content/search/tests/test_api.py | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 2c7e1fe33514..c4d2b273bcd6 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -424,13 +424,13 @@ def add_with_children(block): ############## Collections ############## status_cb("Indexing collections...") # To reduce memory usage on large instances, split up the Collections into pages of 100 collections: - paginator = Paginator(authoring_api.get_collections(), 100) + paginator = Paginator(authoring_api.get_collections(enabled=True), 100) for p in paginator.page_range: docs = [] for collection in paginator.page(p).object_list: status_cb( f"{num_contexts_done + 1}/{num_contexts}. " - f"Now indexing collection {collection.name} ({collection.id})" + f"Now indexing collection {collection.title} ({collection.id})" ) try: doc = searchable_doc_for_collection(collection) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 0dc6c924e9b4..6766166c5858 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -288,10 +288,14 @@ def searchable_doc_for_collection(collection) -> dict: doc = { Fields.id: collection.id, Fields.type: DocType.collection, - Fields.display_name: collection.name, + Fields.display_name: collection.title, Fields.description: collection.description, Fields.created: collection.created.timestamp(), Fields.modified: collection.modified.timestamp(), + # Add related learning_package.key as context_key by default. + # 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, } # Just in case learning_package is not related to a library if hasattr(collection.learning_package, 'contentlibrary'): @@ -300,8 +304,8 @@ def searchable_doc_for_collection(collection) -> dict: doc.update({ Fields.context_key: str(context_key), Fields.org: org, - Fields.access_id: _meili_access_id_from_context_key(context_key), }) + doc[Fields.access_id] = _meili_access_id_from_context_key(doc[Fields.context_key]) # Add the breadcrumbs. doc[Fields.breadcrumbs] = [{"display_name": collection.learning_package.title}] diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index f13891e32d54..549817700370 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -192,7 +192,8 @@ def setUp(self): with freeze_time(created_date): self.collection = authoring_api.create_collection( learning_package_id=self.learning_package.id, - name="my_collection", + title="my_collection", + created_by=None, description="my collection description" ) From 59e26aa9b149b2c3c96f7d9b42f227394bcbe9cb Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 22 Aug 2024 14:21:59 +0530 Subject: [PATCH 09/12] refactor: rebuild_index function --- openedx/core/djangoapps/content/search/api.py | 67 +++++++++++-------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index c4d2b273bcd6..6aeb7354066c 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -337,6 +337,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: Fields.block_id, Fields.content, Fields.tags, + Fields.description, # If we don't list the following sub-fields _explicitly_, they're only sometimes searchable - that is, they # are searchable only if at least one document in the index has a value. If we didn't list them here and, # say, there were no tags.level3 tags in the index, the client would get an error if trying to search for @@ -368,8 +369,8 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: ############## Libraries ############## status_cb("Indexing libraries...") - for lib_key in lib_keys: - status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing library {lib_key}") + + def index_library(lib_key: str) -> list: docs = [] for component in lib_api.get_library_components(lib_key): try: @@ -380,54 +381,60 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: docs.append(doc) except Exception as err: # pylint: disable=broad-except status_cb(f"Error indexing library component {component}: {err}") - finally: - num_blocks_done += 1 if docs: try: # Add all the docs in this library at once (usually faster than adding one at a time): _wait_for_meili_task(client.index(temp_index_name).add_documents(docs)) except (TypeError, KeyError, MeilisearchError) as err: status_cb(f"Error indexing library {lib_key}: {err}") + return docs + for lib_key in lib_keys: + status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing library {lib_key}") + lib_docs = index_library(lib_key) + num_blocks_done += len(lib_docs) num_contexts_done += 1 ############## Courses ############## status_cb("Indexing courses...") # To reduce memory usage on large instances, split up the CourseOverviews into pages of 1,000 courses: + + def index_course(course: CourseOverview) -> list: + docs = [] + # Pre-fetch the course with all of its children: + course = store.get_course(course.id, depth=None) + + def add_with_children(block): + """ Recursively index the given XBlock/component """ + doc = searchable_doc_for_course_block(block) + doc.update(searchable_doc_tags(block.usage_key)) + docs.append(doc) # pylint: disable=cell-var-from-loop + _recurse_children(block, add_with_children) # pylint: disable=cell-var-from-loop + + # Index course children + _recurse_children(course, add_with_children) + + if docs: + # Add all the docs in this course at once (usually faster than adding one at a time): + _wait_for_meili_task(client.index(temp_index_name).add_documents(docs)) + return docs + paginator = Paginator(CourseOverview.objects.only('id', 'display_name'), 1000) for p in paginator.page_range: for course in paginator.page(p).object_list: status_cb( f"{num_contexts_done + 1}/{num_contexts}. Now indexing course {course.display_name} ({course.id})" ) - docs = [] - - # Pre-fetch the course with all of its children: - course = store.get_course(course.id, depth=None) - - def add_with_children(block): - """ Recursively index the given XBlock/component """ - doc = searchable_doc_for_course_block(block) - doc.update(searchable_doc_tags(block.usage_key)) - docs.append(doc) # pylint: disable=cell-var-from-loop - _recurse_children(block, add_with_children) # pylint: disable=cell-var-from-loop - - # Index course children - _recurse_children(course, add_with_children) - - if docs: - # Add all the docs in this course at once (usually faster than adding one at a time): - _wait_for_meili_task(client.index(temp_index_name).add_documents(docs)) + course_docs = index_course(course) num_contexts_done += 1 - num_blocks_done += len(docs) + num_blocks_done += len(course_docs) ############## Collections ############## status_cb("Indexing collections...") - # To reduce memory usage on large instances, split up the Collections into pages of 100 collections: - paginator = Paginator(authoring_api.get_collections(enabled=True), 100) - for p in paginator.page_range: + + def index_collection_batch(batch, num_contexts_done) -> int: docs = [] - for collection in paginator.page(p).object_list: + for collection in batch: status_cb( f"{num_contexts_done + 1}/{num_contexts}. " f"Now indexing collection {collection.title} ({collection.id})" @@ -448,6 +455,12 @@ def add_with_children(block): _wait_for_meili_task(client.index(temp_index_name).add_documents(docs)) except (TypeError, KeyError, MeilisearchError) as err: status_cb(f"Error indexing collection batch {p}: {err}") + return num_contexts_done + + # To reduce memory usage on large instances, split up the Collections into pages of 100 collections: + paginator = Paginator(authoring_api.get_collections(enabled=True), 100) + for p in paginator.page_range: + num_contexts_done = index_collection_batch(paginator.page(p).object_list, num_contexts_done) status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses, collections and libraries.") From 7664e0270de86470d89f8eeaf881fe570cf10c72 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 23 Aug 2024 11:24:57 +0530 Subject: [PATCH 10/12] refactor: collection with unrelated library --- .../djangoapps/content/search/documents.py | 5 ++- .../content/search/tests/test_documents.py | 35 ++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 6766166c5858..6fab50d847f8 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -13,6 +13,7 @@ from openedx.core.djangoapps.content_libraries import api as lib_api from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangoapps.xblock import api as xblock_api +from openedx_learning.api.authoring_models import LearningPackage log = logging.getLogger(__name__) @@ -298,13 +299,15 @@ def searchable_doc_for_collection(collection) -> dict: Fields.context_key: collection.learning_package.key, } # Just in case learning_package is not related to a library - if hasattr(collection.learning_package, 'contentlibrary'): + try: context_key = collection.learning_package.contentlibrary.library_key org = str(context_key.org) doc.update({ Fields.context_key: str(context_key), Fields.org: org, }) + except LearningPackage.contentlibrary.RelatedObjectDoesNotExist: + log.warning(f"Related library not found for collection: {collection.title} <{collection.id}>") doc[Fields.access_id] = _meili_access_id_from_context_key(doc[Fields.context_key]) # Add the breadcrumbs. doc[Fields.breadcrumbs] = [{"display_name": collection.learning_package.title}] diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index e853fd425273..6140411705bb 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -1,8 +1,12 @@ """ Tests for the Studio content search documents (what gets stored in the index) """ +from datetime import datetime, timezone from organizations.models import Organization +from freezegun import freeze_time +from openedx_learning.api import authoring as authoring_api + from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.django import modulestore @@ -11,10 +15,12 @@ try: # This import errors in the lms because content.search is not an installed app there. - from ..documents import searchable_doc_for_course_block, searchable_doc_tags + from ..documents import searchable_doc_for_course_block, searchable_doc_tags, searchable_doc_for_collection from ..models import SearchAccess except RuntimeError: searchable_doc_for_course_block = lambda x: x + searchable_doc_tags = lambda x: x + searchable_doc_for_collection = lambda x: x SearchAccess = {} @@ -198,3 +204,30 @@ def test_video_block_untagged(self): "content": {}, # This video has no tags. } + + def test_collection_with_no_library(self): + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + learning_package = authoring_api.create_learning_package( + key="course-v1:edX+toy+2012_Fall", + title="some learning_package", + description="some description", + ) + collection = authoring_api.create_collection( + learning_package_id=learning_package.id, + title="my_collection", + created_by=None, + description="my collection description" + ) + doc = searchable_doc_for_collection(collection) + assert doc == { + "id": collection.id, + "type": "collection", + "display_name": collection.title, + "description": collection.description, + "context_key": learning_package.key, + "access_id": self.toy_course_access_id, + "breadcrumbs": [{"display_name": learning_package.title}], + "created": created_date.timestamp(), + "modified": created_date.timestamp(), + } From af577d6acd3bf2a6c2097b95db6cc9c8097712a8 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 23 Aug 2024 20:16:41 +0530 Subject: [PATCH 11/12] refactor: warning text Co-authored-by: Jillian --- openedx/core/djangoapps/content/search/documents.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 6fab50d847f8..e058591bcbbc 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -307,7 +307,7 @@ def searchable_doc_for_collection(collection) -> dict: Fields.org: org, }) except LearningPackage.contentlibrary.RelatedObjectDoesNotExist: - log.warning(f"Related library not found for collection: {collection.title} <{collection.id}>") + log.warning(f"Related library not found for {collection}") doc[Fields.access_id] = _meili_access_id_from_context_key(doc[Fields.context_key]) # Add the breadcrumbs. doc[Fields.breadcrumbs] = [{"display_name": collection.learning_package.title}] From 2ac22c35d54ce4c6a1387e34d53f6e8786fb95f1 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Tue, 27 Aug 2024 14:10:46 +0930 Subject: [PATCH 12/12] chore: use openedx-learning==0.11.2 --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 3 +-- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 7e2ce3c3b060..e567648d6dfc 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -823,7 +823,7 @@ openedx-filters==1.9.0 # -r requirements/edx/kernel.in # lti-consumer-xblock # ora2 -openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api +openedx-learning==0.11.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 8a97792c514a..22a0b038ac38 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1372,7 +1372,7 @@ openedx-filters==1.9.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # ora2 -openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api +openedx-learning==0.11.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 1973d6a84699..51e51c7d8ac6 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -982,7 +982,7 @@ openedx-filters==1.9.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api +openedx-learning==0.11.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index d2e00e4b73b5..a5b510742ac7 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -119,8 +119,7 @@ openedx-calc # Library supporting mathematical calculatio openedx-django-require openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) -# FIXME Remove this line after release of openedx-learning -openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api # Open edX Learning core (experimental) +openedx-learning # Open edX Learning core (experimental) openedx-mongodbproxy openedx-django-wiki path diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index a1a44f8dd595..8356529fe231 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1033,7 +1033,7 @@ openedx-filters==1.9.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/update-collections-api +openedx-learning==0.11.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt