Merge branch 'jill/collection-components-search' into jill/reindex-co…

…llection

openedx/core/djangoapps/content/search/api.py

@@ -324,6 +324,8 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
             Fields.tags + "." + Fields.tags_level2,
             Fields.tags + "." + Fields.tags_level3,
             Fields.collections,
+            Fields.collections + "." + Fields.collections_display_name,
+            Fields.collections + "." + Fields.collections_key,
             Fields.type,
             Fields.access_id,
             Fields.last_published,
@@ -335,8 +337,8 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
             Fields.display_name,
             Fields.block_id,
             Fields.content,
-            Fields.tags,
             Fields.description,
+            Fields.tags,
             Fields.collections,
             # 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,
@@ -347,6 +349,8 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
             Fields.tags + "." + Fields.tags_level1,
             Fields.tags + "." + Fields.tags_level2,
             Fields.tags + "." + Fields.tags_level3,
+            Fields.collections + "." + Fields.collections_display_name,
+            Fields.collections + "." + Fields.collections_key,
         ])
         # Mark which attributes can be used for sorting search results:
         client.index(temp_index_name).update_sortable_attributes([

openedx/core/djangoapps/content/search/documents.py

@@ -58,8 +58,12 @@ class Fields:
     tags_level1 = "level1"
     tags_level2 = "level2"
     tags_level3 = "level3"
-    # List of collection.block_id strings this object belongs to.
+    # Collections (dictionary) that this object belongs to.
+    # Similarly to tags above, we collect the collection.titles and collection.keys into hierarchical facets.
     collections = "collections"
+    collections_display_name = "display_name"
+    collections_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.
     # Text (html) blocks have an "html_content" key in here, capa has "capa_content" and "problem_types", and so on.
@@ -241,25 +245,41 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) ->
 
     e.g. for something in Collections "COL_A" and "COL_B", this would return:
         {
-            "collections": ["COL_A", "COL_B"],
+            "collections":  {
+                "display_name": ["Collection A", "Collection B"],
+                "key": ["COL_A", "COL_B"],
+            }
+        }
+
+    If the object is in no collections, returns:
+        {
+            "collections":  {},
         }
 
-    Returns an empty dict if the object is not in any collections.
     """
     # Gather the collections associated with this object
-    result = {}
-    collections = []
+    collections = None
     try:
         component = lib_api.get_component_from_usage_key(object_id)
         collections = authoring_api.get_entity_collections(
             component.learning_package_id,
             component.key,
-        ).values_list("key", flat=True)
+        )
     except ObjectDoesNotExist:
         log.warning(f"No component found for {object_id}")
 
-    if collections:
-        result[Fields.collections] = list(collections)
+    if not collections:
+        return {Fields.collections: {}}
+
+    result = {
+        Fields.collections: {
+            Fields.collections_display_name: [],
+            Fields.collections_key: [],
+        }
+    }
+    for collection in collections:
+        result[Fields.collections][Fields.collections_display_name].append(collection.title)
+        result[Fields.collections][Fields.collections_key].append(collection.key)
 
     return result
 

openedx/core/djangoapps/content/search/handlers.py

@@ -193,7 +193,7 @@ def content_object_associations_changed_handler(**kwargs) -> None:
 
     # This event's changes may contain both "tags" and "collections", but this will happen rarely, if ever.
     # So we allow a potential double "upsert" here.
-    if "tags" in content_object.changes:
+    if not content_object.changes or "tags" in content_object.changes:
         upsert_block_tags_index_docs(usage_key)
-    elif "collections" in content_object.changes:
+    if not content_object.changes or "collections" in content_object.changes:
         upsert_block_collections_index_docs(usage_key)

openedx/core/djangoapps/content/search/tests/test_api.py

@@ -217,10 +217,13 @@ 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()
+        assert 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]),
@@ -256,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
 
@@ -348,6 +352,7 @@ def test_index_xblock_tags(self, mock_meilisearch):
             }
         }
 
+        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_sequential_with_tags1]),
@@ -402,6 +407,7 @@ def test_index_library_block_tags(self, mock_meilisearch):
             }
         }
 
+        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_problem_with_tags1]),
@@ -506,23 +512,30 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
             "access_id": lib_access.id,
             "breadcrumbs": [{"display_name": "Library"}],
         }
-        doc_problem_with_collection2 = {
+        doc_problem_with_collection1 = {
             "id": self.doc_problem1["id"],
-            "collections": [collection2.key],
+            "collections": {
+                "display_name": ["Collection 2"],
+                "key": ["COL2"],
+            },
         }
-        doc_problem_with_collection1 = {
+        doc_problem_with_collection2 = {
             "id": self.doc_problem1["id"],
-            "collections": [collection1.key, collection2.key],
+            "collections": {
+                "display_name": ["Collection 1", "Collection 2"],
+                "key": ["COL1", "COL2"],
+            },
         }
 
+        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_collection2]),
                 call([doc_problem_with_collection1]),
+                call([doc_problem_with_collection2]),
             ],
             any_order=True,
         )

openedx/core/djangoapps/content/search/tests/test_documents.py

@@ -284,7 +284,10 @@ def test_html_library_block(self):
             "content": {
                 "html_content": "",
             },
-            "collections": ["TOY_COLLECTION"],
+            "collections": {
+                "key": ["TOY_COLLECTION"],
+                "display_name": ["Toy Collection"],
+            },
             "tags": {
                 "taxonomy": ["Difficulty"],
                 "level0": ["Difficulty > Normal"],