From c33c3fa65e79ba7223b062b8f2a885883a1be4b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 11 Sep 2024 11:38:42 -0300 Subject: [PATCH] fix: update index with empty collection list --- .../djangoapps/content/search/documents.py | 6 +-- .../content/search/tests/test_api.py | 51 +++++++++++++++++-- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 51eca8816430..8bd64ed5ec95 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -236,7 +236,7 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) -> "collections": ["COL_A", "COL_B"], } - Returns an empty dict if the object is not in any collections. + Returns an empty list if the object is not in any collections. """ # Gather the collections associated with this object result = {} @@ -247,12 +247,10 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) -> component.learning_package_id, component.key, ).values_list("key", flat=True) + result[Fields.collections] = list(collections) except ObjectDoesNotExist: log.warning(f"No component found for {object_id}") - if collections: - result[Fields.collections] = list(collections) - return result diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index f8f13d8b9a6f..fa33b4b5fb32 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -215,17 +215,22 @@ def test_reindex_meilisearch(self, mock_meilisearch): doc_vertical["tags"] = {} doc_problem1 = copy.deepcopy(self.doc_problem1) doc_problem1["tags"] = {} + doc_problem1["collections"] = [] doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} + doc_problem2["collections"] = [] api.rebuild_index() + self.assertEqual( + mock_meilisearch.return_value.index.return_value.add_documents.call_count, + 3, + ) mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls( [ - call([doc_sequential, doc_vertical]), call([doc_problem1, doc_problem2]), call([self.collection_dict]), + call([doc_sequential, doc_vertical]), ], - any_order=True, ) @override_settings(MEILISEARCH_ENABLED=True) @@ -254,6 +259,7 @@ def test_reindex_meilisearch_library_block_error(self, mock_meilisearch): doc_vertical["tags"] = {} doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} + doc_problem2["collections"] = [] orig_from_component = library_api.LibraryXBlockMetadata.from_component @@ -271,13 +277,17 @@ def mocked_from_component(lib_key, component): ): api.rebuild_index() + self.assertEqual( + mock_meilisearch.return_value.index.return_value.add_documents.call_count, + 3, + ) mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls( [ - call([doc_sequential, doc_vertical]), # Problem 1 should not be indexed call([doc_problem2]), + call([self.collection_dict]), + call([doc_sequential, doc_vertical]), ], - any_order=True, ) # Check that the sorting-related settings were updated to support sorting on the expected fields @@ -450,12 +460,43 @@ def test_index_library_block_collections(self, mock_meilisearch): "collections": [collection1.key, collection2.key], } + self.assertEqual( + 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_problem_with_collection2]), call([doc_problem_with_collection1]), ], - any_order=True, + ) + + # Remove Problem1 from both Collections + mock_meilisearch.return_value.index.return_value.update_documents.reset_mock() + doc_problem_without_collections = { + "id": self.doc_problem1["id"], + "collections": [], + } + + for collection in (collection1, collection2): + library_api.update_library_collection_components( + self.library.key, + collection_key=collection.key, + usage_keys=[ + self.problem1.usage_key, + ], + remove=True, + ) + + self.assertEqual( + 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_problem_with_collection2]), + call([doc_problem_without_collections]), + ], ) @override_settings(MEILISEARCH_ENABLED=True)