feat: store content.child_usage_keys in Container search document [FC-0083] (#36528)

* feat: store content.child_usage_keys in Container search document
  Stores the draft children + published children (if applicable)

Related fixes:
* fix: lib_api.get_container does not take a "user" arg
* refactor: fetch_customizable_fields_from_container does not need a "user" arg
* refactor: moves tags_count into LibraryItem
   because anything that appears in a library may be tagged.
* refactor: remove get_container_from_key from public API
   API users must use get_container and ContainerMetadata.
* refactor: made set_library_item_collections take an entity_key string instead of
  a PublishableEntity instance, so we don't need to fetch a Container object to call it.
* refactor: changed ContainerLink.update_or_create to take the container PK
  instead of a Container object, since this is enough.
  Added container_pk to ContainerMetadata to support this.

Author: pomegranited

cms/djangoapps/contentstore/models.py

@@ -354,7 +354,7 @@ def summarize_by_downstream_context(cls, downstream_context_key: CourseKey) -> Q
     @classmethod
     def update_or_create(
         cls,
-        upstream_container: Container | None,
+        upstream_container_id: int | None,
         /,
         upstream_container_key: LibraryContainerLocator,
         upstream_context_key: str,
@@ -377,8 +377,8 @@ def update_or_create(
             'version_synced': version_synced,
             'version_declined': version_declined,
         }
-        if upstream_container:
-            new_values['upstream_container'] = upstream_container
+        if upstream_container_id:
+            new_values['upstream_container_id'] = upstream_container_id
         try:
             link = cls.objects.get(downstream_usage_key=downstream_usage_key)
             has_changes = False

cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py

@@ -267,7 +267,7 @@ def put(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response
                     fetch_customizable_fields_from_block(downstream=downstream, user=request.user)
                 else:
                     assert isinstance(link.upstream_key, LibraryContainerLocator)
-                    fetch_customizable_fields_from_container(downstream=downstream, user=request.user)
+                    fetch_customizable_fields_from_container(downstream=downstream)
         except BadDownstream as exc:
             logger.exception(
                 "'%s' is an invalid downstream; refusing to set its upstream to '%s'",

cms/djangoapps/contentstore/utils.py

@@ -87,7 +87,7 @@
 from common.djangoapps.xblock_django.api import deprecated_xblocks
 from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
 from openedx.core import toggles as core_toggles
-from openedx.core.djangoapps.content_libraries.api import get_container_from_key
+from openedx.core.djangoapps.content_libraries.api import get_container
 from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
 from openedx.core.djangoapps.credit.api import get_credit_requirements, is_credit_course
 from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND
@@ -2402,7 +2402,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime |
     """
     upstream_container_key = LibraryContainerLocator.from_string(xblock.upstream)
     try:
-        lib_component = get_container_from_key(upstream_container_key)
+        lib_component = get_container(upstream_container_key).container_pk
     except ObjectDoesNotExist:
         log.error(f"Library component not found for {upstream_container_key}")
         lib_component = None

cms/lib/xblock/upstream_sync_container.py

@@ -45,7 +45,7 @@ def sync_from_upstream_container(
         user,
         permission=lib_api.permissions.CAN_VIEW_THIS_CONTENT_LIBRARY,
     )
-    upstream_meta = lib_api.get_container(link.upstream_key, user)
+    upstream_meta = lib_api.get_container(link.upstream_key)
     upstream_children = lib_api.get_container_children(link.upstream_key, published=True)
     _update_customizable_fields(upstream=upstream_meta, downstream=downstream, only_fetch=False)
     _update_non_customizable_fields(upstream=upstream_meta, downstream=downstream)
@@ -54,15 +54,15 @@ def sync_from_upstream_container(
     return upstream_children
 
 
-def fetch_customizable_fields_from_container(*, downstream: XBlock, user: User) -> None:
+def fetch_customizable_fields_from_container(*, downstream: XBlock) -> None:
     """
     Fetch upstream-defined value of customizable fields and save them on the downstream.
 
     The container version only retrieves values from *published* containers.
 
     Basically, this sets the value of "upstream_display_name" on the downstream block.
     """
-    upstream = lib_api.get_container(LibraryContainerLocator.from_string(downstream.upstream), user)
+    upstream = lib_api.get_container(LibraryContainerLocator.from_string(downstream.upstream))
     _update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=True)
 
 

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

@@ -71,6 +71,7 @@ class Fields:
     # 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.
+    # Containers store their list of child usage keys here.
     content = "content"
 
     # Collections use this field to communicate how many entities/components they contain.
@@ -87,6 +88,7 @@ class Fields:
     published = "published"
     published_display_name = "display_name"
     published_description = "description"
+    published_content = "content"
     published_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
@@ -347,13 +349,10 @@ def _collections_for_content_object(object_id: OpaqueKey) -> dict:
             collections = authoring_api.get_entity_collections(
                 component.learning_package_id,
                 component.key,
-            )
+            ).values('key', 'title')
         elif isinstance(object_id, LibraryContainerLocator):
-            container = lib_api.get_container_from_key(object_id)
-            collections = authoring_api.get_entity_collections(
-                container.publishable_entity.learning_package_id,
-                container.key,
-            )
+            container = lib_api.get_container(object_id, include_collections=True)
+            collections = container.collections
         else:
             log.warning(f"Unexpected key type for {object_id}")
 
@@ -364,8 +363,8 @@ def _collections_for_content_object(object_id: OpaqueKey) -> dict:
         return result
 
     for collection in collections:
-        result[Fields.collections][Fields.collections_display_name].append(collection.title)
-        result[Fields.collections][Fields.collections_key].append(collection.key)
+        result[Fields.collections][Fields.collections_display_name].append(collection["title"])
+        result[Fields.collections][Fields.collections_key].append(collection["key"])
 
     return result
 
@@ -581,9 +580,13 @@ def searchable_doc_for_container(
         container = lib_api.get_container(container_key)
     except lib_api.ContentLibraryContainerNotFound:
         # Container not found, so we can only return the base doc
+        log.error(f"Container {container_key} not found")
         return doc
 
-    draft_num_children = lib_api.get_container_children_count(container_key, published=False)
+    draft_children = lib_api.get_container_children(
+        container_key,
+        published=False,
+    )
     publish_status = PublishStatus.published
     if container.last_published is None:
         publish_status = PublishStatus.never
@@ -594,7 +597,13 @@ def searchable_doc_for_container(
         Fields.display_name: container.display_name,
         Fields.created: container.created.timestamp(),
         Fields.modified: container.modified.timestamp(),
-        Fields.num_children: draft_num_children,
+        Fields.num_children: len(draft_children),
+        Fields.content: {
+            "child_usage_keys": [
+                str(child.usage_key)
+                for child in draft_children
+            ],
+        },
         Fields.publish_status: publish_status,
         Fields.last_published: container.last_published.timestamp() if container.last_published else None,
     })
@@ -603,10 +612,19 @@ def searchable_doc_for_container(
         doc[Fields.breadcrumbs] = [{"display_name": library.title}]
 
     if container.published_version_num is not None:
-        published_num_children = lib_api.get_container_children_count(container_key, published=True)
+        published_children = lib_api.get_container_children(
+            container_key,
+            published=True,
+        )
         doc[Fields.published] = {
-            Fields.published_num_children: published_num_children,
             Fields.published_display_name: container.published_display_name,
+            Fields.published_num_children: len(published_children),
+            Fields.published_content: {
+                "child_usage_keys": [
+                    str(child.usage_key)
+                    for child in published_children
+                ],
+            },
         }
 
     return doc

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

@@ -238,6 +238,7 @@ def setUp(self):
             "display_name": "Unit 1",
             # description is not set for containers
             "num_children": 0,
+            "content": {"child_usage_keys": []},
             "publish_status": "never",
             "context_key": "lib:org1:lib",
             "org": "org1",

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

@@ -531,6 +531,9 @@ def test_draft_container(self):
             "display_name": "A Unit in the Search Index",
             # description is not set for containers
             "num_children": 0,
+            "content": {
+                "child_usage_keys": [],
+            },
             "publish_status": "never",
             "context_key": "lib:edX:2012_Fall",
             "access_id": self.library_access_id,
@@ -571,6 +574,11 @@ def test_published_container(self):
             "display_name": "A Unit in the Search Index",
             # description is not set for containers
             "num_children": 1,
+            "content": {
+                "child_usage_keys": [
+                    "lb:edX:2012_Fall:html:text2",
+                ],
+            },
             "publish_status": "published",
             "context_key": "lib:edX:2012_Fall",
             "access_id": self.library_access_id,
@@ -585,6 +593,11 @@ def test_published_container(self):
             "published": {
                 "num_children": 1,
                 "display_name": "A Unit in the Search Index",
+                "content": {
+                    "child_usage_keys": [
+                        "lb:edX:2012_Fall:html:text2",
+                    ],
+                },
             },
         }
 
@@ -627,6 +640,12 @@ def test_published_container_with_changes(self):
             "display_name": "A Unit in the Search Index",
             # description is not set for containers
             "num_children": 2,
+            "content": {
+                "child_usage_keys": [
+                    "lb:edX:2012_Fall:html:text2",
+                    "lb:edX:2012_Fall:html:text3",
+                ],
+            },
             "publish_status": "modified",
             "context_key": "lib:edX:2012_Fall",
             "access_id": self.library_access_id,
@@ -641,6 +660,11 @@ def test_published_container_with_changes(self):
             "published": {
                 "num_children": 1,
                 "display_name": "A Unit in the Search Index",
+                "content": {
+                    "child_usage_keys": [
+                        "lb:edX:2012_Fall:html:text2",
+                    ],
+                },
             },
         }
 

openedx/core/djangoapps/content_libraries/api/block_metadata.py

@@ -26,8 +26,6 @@ class LibraryXBlockMetadata(PublishableItem):
     Class that represents the metadata about an XBlock in a content library.
     """
     usage_key: LibraryUsageLocatorV2
-    # TODO: move tags_count to LibraryItem as all objects under a library can be tagged.
-    tags_count: int = 0
 
     @classmethod
     def from_component(cls, library_key, component, associated_collections=None):

openedx/core/djangoapps/content_libraries/api/collections.py

@@ -181,7 +181,7 @@ def update_library_collection_items(
 
 def set_library_item_collections(
     library_key: LibraryLocatorV2,
-    publishable_entity: PublishableEntity,
+    entity_key: str,
     *,
     collection_keys: list[str],
     created_by: int | None = None,
@@ -207,6 +207,11 @@ def set_library_item_collections(
     assert content_library.learning_package_id
     assert content_library.library_key == library_key
 
+    publishable_entity = authoring_api.get_publishable_entity_by_key(
+        content_library.learning_package_id,
+        key=entity_key,
+    )
+
     # Note: Component.key matches its PublishableEntity.key
     collection_qs = authoring_api.get_collections(content_library.learning_package_id).filter(
         key__in=collection_keys

openedx/core/djangoapps/content_libraries/api/containers.py

@@ -43,7 +43,6 @@
     "ContainerMetadata",
     "ContainerType",
     # API methods
-    "get_container_from_key",
     "get_container",
     "create_container",
     "get_container_children",
@@ -111,6 +110,7 @@ class ContainerMetadata(PublishableItem):
     """
     container_key: LibraryContainerLocator
     container_type: ContainerType
+    container_pk: int
     published_display_name: str | None
 
     @classmethod
@@ -139,6 +139,7 @@ def from_container(cls, library_key, container: Container, associated_collection
         return cls(
             container_key=container_key,
             container_type=container_type,
+            container_pk=container.pk,
             display_name=draft.title,
             created=container.created,
             modified=draft.created,
@@ -173,7 +174,7 @@ def library_container_locator(
     )
 
 
-def get_container_from_key(container_key: LibraryContainerLocator, isDeleted=False) -> Container:
+def _get_container_from_key(container_key: LibraryContainerLocator, isDeleted=False) -> Container:
     """
     Internal method to fetch the Container object from its LibraryContainerLocator
 
@@ -192,11 +193,15 @@ def get_container_from_key(container_key: LibraryContainerLocator, isDeleted=Fal
     raise ContentLibraryContainerNotFound
 
 
-def get_container(container_key: LibraryContainerLocator, include_collections=False) -> ContainerMetadata:
+def get_container(
+    container_key: LibraryContainerLocator,
+    *,
+    include_collections=False,
+) -> ContainerMetadata:
     """
     Get a container (a Section, Subsection, or Unit).
     """
-    container = get_container_from_key(container_key)
+    container = _get_container_from_key(container_key)
     if include_collections:
         associated_collections = authoring_api.get_entity_collections(
             container.publishable_entity.learning_package_id,
@@ -268,7 +273,7 @@ def update_container(
     """
     Update a container (e.g. a Unit) title.
     """
-    container = get_container_from_key(container_key)
+    container = _get_container_from_key(container_key)
     library_key = container_key.lib_key
 
     assert container.unit
@@ -297,7 +302,7 @@ def delete_container(
     No-op if container doesn't exist or has already been soft-deleted.
     """
     library_key = container_key.lib_key
-    container = get_container_from_key(container_key)
+    container = _get_container_from_key(container_key)
 
     affected_collections = authoring_api.get_entity_collections(
         container.publishable_entity.learning_package_id,
@@ -332,7 +337,7 @@ def restore_container(container_key: LibraryContainerLocator) -> None:
     Restore the specified library container.
     """
     library_key = container_key.lib_key
-    container = get_container_from_key(container_key, isDeleted=True)
+    container = _get_container_from_key(container_key, isDeleted=True)
 
     affected_collections = authoring_api.get_entity_collections(
         container.publishable_entity.learning_package_id,
@@ -372,12 +377,13 @@ def restore_container(container_key: LibraryContainerLocator) -> None:
 
 def get_container_children(
     container_key: LibraryContainerLocator,
+    *,
     published=False,
 ) -> list[LibraryXBlockMetadata | ContainerMetadata]:
     """
     Get the entities contained in the given container (e.g. the components/xblocks in a unit)
     """
-    container = get_container_from_key(container_key)
+    container = _get_container_from_key(container_key)
     if container_key.container_type == ContainerType.Unit.value:
         child_components = authoring_api.get_components_in_unit(container.unit, published=published)
         return [LibraryXBlockMetadata.from_component(
@@ -399,7 +405,7 @@ def get_container_children_count(
     """
     Get the count of entities contained in the given container (e.g. the components/xblocks in a unit)
     """
-    container = get_container_from_key(container_key)
+    container = _get_container_from_key(container_key)
     return authoring_api.get_container_children_count(container, published=published)
 
 
@@ -414,7 +420,7 @@ def update_container_children(
     """
     library_key = container_key.lib_key
     container_type = container_key.container_type
-    container = get_container_from_key(container_key)
+    container = _get_container_from_key(container_key)
     match container_type:
         case ContainerType.Unit.value:
             components = [get_component_from_usage_key(key) for key in children_ids]  # type: ignore[arg-type]
@@ -459,7 +465,7 @@ def publish_container_changes(container_key: LibraryContainerLocator, user_id: i
     Publish all unpublished changes in a container and all its child
     containers/blocks.
     """
-    container = get_container_from_key(container_key)
+    container = _get_container_from_key(container_key)
     library_key = container_key.lib_key
     content_library = ContentLibrary.objects.get_by_key(library_key)  # type: ignore[attr-defined]
     learning_package = content_library.learning_package