feat: container collections support

Author: rpenido

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

@@ -17,35 +17,37 @@
 from meilisearch import Client as MeilisearchClient
 from meilisearch.errors import MeilisearchApiError, MeilisearchError
 from meilisearch.models.task import TaskInfo
+from opaque_keys import OpaqueKey
 from opaque_keys.edx.keys import UsageKey
-from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryCollectionLocator
+from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator, 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
+from common.djangoapps.student.roles import GlobalStaff
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
-from openedx.core.djangoapps.content.search.models import get_access_ids_for_request, IncrementalIndexCompleted
 from openedx.core.djangoapps.content.search.index_config import (
     INDEX_DISTINCT_ATTRIBUTE,
     INDEX_FILTERABLE_ATTRIBUTES,
-    INDEX_SEARCHABLE_ATTRIBUTES,
-    INDEX_SORTABLE_ATTRIBUTES,
     INDEX_RANKING_RULES,
+    INDEX_SEARCHABLE_ATTRIBUTES,
+    INDEX_SORTABLE_ATTRIBUTES
 )
+from openedx.core.djangoapps.content.search.models import IncrementalIndexCompleted, get_access_ids_for_request
 from openedx.core.djangoapps.content_libraries import api as lib_api
 from xmodule.modulestore.django import modulestore
 
 from .documents import (
     Fields,
     meili_id_from_opaque_key,
-    searchable_doc_for_course_block,
+    searchable_doc_collections,
     searchable_doc_for_collection,
     searchable_doc_for_container,
+    searchable_doc_for_course_block,
     searchable_doc_for_library_block,
     searchable_doc_for_usage_key,
-    searchable_doc_collections,
     searchable_doc_tags,
-    searchable_doc_tags_for_collection,
+    searchable_doc_tags_for_collection
 )
 
 log = logging.getLogger(__name__)
@@ -488,6 +490,7 @@ def index_container_batch(batch, num_done, library_key) -> int:
                     doc = searchable_doc_for_container(container_key)
                     # TODO: when we add container tags
                     # doc.update(searchable_doc_tags_for_container(container_key))
+                    doc.update(searchable_doc_collections(container_key))
                     docs.append(doc)
                 except Exception as err:  # pylint: disable=broad-except
                     status_cb(f"Error indexing container {container.key}: {err}")
@@ -710,15 +713,15 @@ def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collectio
     If the Collection is not found or disabled (i.e. soft-deleted), then delete it from the search index.
     """
     doc = searchable_doc_for_collection(library_key, collection_key)
-    update_components = False
+    update_items = False
 
     # Soft-deleted/disabled collections are removed from the index
     # and their components updated.
     if doc.get('_disabled'):
 
         _delete_index_doc(doc[Fields.id])
 
-        update_components = True
+        update_items = True
 
     # Hard-deleted collections are also deleted from the index,
     # but their components are automatically updated as part of the deletion process, so we don't have to.
@@ -736,10 +739,12 @@ def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collectio
         _update_index_docs([doc])
 
     # Asynchronously update the collection's components "collections" field
-    if update_components:
-        from .tasks import update_library_components_collections as update_task
+    if update_items:
+        from .tasks import update_library_components_collections as update_components_task
+        from .tasks import update_library_components_collections as update_containers_task
 
-        update_task.delay(str(library_key), collection_key)
+        update_components_task.delay(str(library_key), collection_key)
+        update_containers_task.delay(str(library_key), collection_key)
 
 
 def update_library_components_collections(
@@ -774,6 +779,38 @@ def update_library_components_collections(
         _update_index_docs(docs)
 
 
+def update_library_containers_collections(
+    library_key: LibraryLocatorV2,
+    collection_key: str,
+    batch_size: int = 1000,
+) -> None:
+    """
+    Updates the "collections" field for all containers associated with a given Library Collection.
+
+    Because there may be a lot of containers, we send these updates to Meilisearch in batches.
+    """
+    library = lib_api.get_library(library_key)
+    containers = authoring_api.get_collection_containers(library.learning_package_id, collection_key)
+
+    paginator = Paginator(containers, batch_size)
+    for page in paginator.page_range:
+        docs = []
+
+        for container in paginator.page(page).object_list:
+            container_key = lib_api.library_container_locator(
+                library_key,
+                container,
+            )
+            doc = searchable_doc_collections(container_key)
+            docs.append(doc)
+
+        log.info(
+            f"Updating document.collections for library {library_key} containers"
+            f" page {page} / {paginator.num_pages}"
+        )
+        _update_index_docs(docs)
+
+
 def upsert_library_container_index_doc(container_key: LibraryContainerLocator) -> None:
     """
     Creates, updates, or deletes the document for the given Library Container in the search index.
@@ -820,12 +857,12 @@ def upsert_block_tags_index_docs(usage_key: UsageKey):
     _update_index_docs([doc])
 
 
-def upsert_block_collections_index_docs(usage_key: UsageKey):
+def upsert_item_collections_index_docs(opaque_key: OpaqueKey):
     """
-    Updates the collections data in documents for the given Course/Library block
+    Updates the collections data in documents for the given Course/Library block, or Container
     """
-    doc = {Fields.id: meili_id_from_opaque_key(usage_key)}
-    doc.update(searchable_doc_collections(usage_key))
+    doc = {Fields.id: meili_id_from_opaque_key(opaque_key)}
+    doc.update(searchable_doc_collections(opaque_key))
     _update_index_docs([doc])
 
 

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

@@ -8,6 +8,7 @@
 
 from django.core.exceptions import ObjectDoesNotExist
 from django.utils.text import slugify
+from opaque_keys import OpaqueKey
 from opaque_keys.edx.keys import LearningContextKey, UsageKey
 from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2
 from openedx_learning.api import authoring as authoring_api
@@ -113,7 +114,7 @@ class PublishStatus:
     modified = "modified"
 
 
-def meili_id_from_opaque_key(usage_key: UsageKey) -> str:
+def meili_id_from_opaque_key(opaque_key: OpaqueKey) -> str:
     """
     Meilisearch requires each document to have a primary key that's either an
     integer or a string composed of alphanumeric characters (a-z A-Z 0-9),
@@ -124,7 +125,7 @@ def meili_id_from_opaque_key(usage_key: UsageKey) -> str:
     we could use PublishableEntity's primary key / UUID instead.
     """
     # The slugified key _may_ not be unique so we append a hashed string to make it unique:
-    key_str = str(usage_key)
+    key_str = str(opaque_key)
     key_bin = key_str.encode()
 
     suffix = blake2b(key_bin, digest_size=4, usedforsecurity=False).hexdigest()
@@ -309,7 +310,7 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict:
     return {Fields.tags: result}
 
 
-def _collections_for_content_object(object_id: UsageKey | LearningContextKey) -> dict:
+def _collections_for_content_object(object_id: OpaqueKey) -> dict:
     """
     Given an XBlock, course, library, etc., get the collections for its index doc.
 
@@ -340,11 +341,21 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) ->
     # Gather the collections associated with this object
     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,
-        )
+        if isinstance(object_id, UsageKey):
+            component = lib_api.get_component_from_usage_key(object_id)
+            collections = authoring_api.get_entity_collections(
+                component.learning_package_id,
+                component.key,
+            )
+        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,
+            )
+        else:
+            return result
+
     except ObjectDoesNotExist:
         log.warning(f"No component found for {object_id}")
 
@@ -438,13 +449,13 @@ def searchable_doc_tags(usage_key: UsageKey) -> dict:
     return doc
 
 
-def searchable_doc_collections(usage_key: UsageKey) -> dict:
+def searchable_doc_collections(opaque_key: OpaqueKey) -> dict:
     """
     Generate a dictionary document suitable for ingestion into a search engine
     like Meilisearch or Elasticsearch, with the collections data for the given content object.
     """
-    doc = searchable_doc_for_usage_key(usage_key)
-    doc.update(_collections_for_content_object(usage_key))
+    doc = searchable_doc_for_usage_key(opaque_key)
+    doc.update(_collections_for_content_object(opaque_key))
 
     return doc
 

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

@@ -8,7 +8,7 @@
 from django.dispatch import receiver
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import UsageKey
-from opaque_keys.edx.locator import LibraryCollectionLocator
+from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator
 from openedx_events.content_authoring.data import (
     ContentLibraryData,
     ContentObjectChangedData,
@@ -40,7 +40,7 @@
 
 from .api import (
     only_if_meilisearch_enabled,
-    upsert_block_collections_index_docs,
+    upsert_item_collections_index_docs,
     upsert_block_tags_index_docs,
     upsert_collection_tags_index_docs,
 )
@@ -211,25 +211,30 @@ def content_object_associations_changed_handler(**kwargs) -> None:
         return
 
     try:
-        # Check if valid if course or library block
-        usage_key = UsageKey.from_string(str(content_object.object_id))
+        # Check if valid course or library block
+        opaque_key = UsageKey.from_string(str(content_object.object_id))
     except InvalidKeyError:
         try:
-            # Check if valid if library collection
-            usage_key = LibraryCollectionLocator.from_string(str(content_object.object_id))
+            # Check if valid library collection
+            opaque_key = LibraryCollectionLocator.from_string(str(content_object.object_id))
         except InvalidKeyError:
-            log.error("Received invalid content object id")
-            return
+            try:
+                # Check if valid library container
+                opaque_key = LibraryContainerLocator.from_string(str(content_object.object_id))
+            except InvalidKeyError:
+                log.error("Received invalid content object id")
+                log.error(str(content_object.object_id))
+                return
 
     # 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 not content_object.changes or "tags" in content_object.changes:
-        if isinstance(usage_key, LibraryCollectionLocator):
-            upsert_collection_tags_index_docs(usage_key)
+        if isinstance(opaque_key, LibraryCollectionLocator):
+            upsert_collection_tags_index_docs(opaque_key)
         else:
-            upsert_block_tags_index_docs(usage_key)
+            upsert_block_tags_index_docs(opaque_key)
     if not content_object.changes or "collections" in content_object.changes:
-        upsert_block_collections_index_docs(usage_key)
+        upsert_item_collections_index_docs(opaque_key)
 
 
 @receiver(LIBRARY_CONTAINER_CREATED)

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

@@ -112,6 +112,19 @@ def update_library_components_collections(library_key_str: str, collection_key:
     api.update_library_components_collections(library_key, collection_key)
 
 
+@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))
+@set_code_owner_attribute
+def update_library_containers_collections(library_key_str: str, collection_key: str) -> None:
+    """
+    Celery task to update the "collections" field for containers in the given content library collection.
+    """
+    library_key = LibraryLocatorV2.from_string(library_key_str)
+
+    log.info("Updating document.collections for library %s collection %s containers", library_key, collection_key)
+
+    api.update_library_containers_collections(library_key, collection_key)
+
+
 @shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))
 @set_code_owner_attribute
 def update_library_container_index_doc(library_key_str: str, container_key_str: str) -> None:

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

@@ -581,7 +581,7 @@ def test_index_library_block_and_collections(self, mock_meilisearch):
                 description="Second Collection",
             )
 
-        # Add Problem1 to both Collections (these internally call `upsert_block_collections_index_docs` and
+        # Add Problem1 to both Collections (these internally call `upsert_item_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)

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

@@ -3,66 +3,51 @@
 
 These methods don't enforce permissions (only the REST APIs do).
 """
-from dataclasses import dataclass
-from datetime import datetime, timezone
 import logging
 import mimetypes
+from dataclasses import dataclass
+from datetime import datetime, timezone
 
 from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist
 from django.core.validators import validate_unicode_slug
-from django.db.models import QuerySet
 from django.db import transaction
-from django.utils.translation import gettext as _
+from django.db.models import QuerySet
 from django.urls import reverse
+from django.utils.translation import gettext as _
 from lxml import etree
-from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
 from opaque_keys.edx.keys import UsageKeyV2
+from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
 from openedx_events.content_authoring.data import (
+    ContentObjectChangedData,
     LibraryBlockData,
     LibraryCollectionData,
-    LibraryContainerData,
-    ContentObjectChangedData,
+    LibraryContainerData
 )
 from openedx_events.content_authoring.signals import (
+    CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
     LIBRARY_BLOCK_CREATED,
     LIBRARY_BLOCK_DELETED,
     LIBRARY_BLOCK_UPDATED,
     LIBRARY_COLLECTION_UPDATED,
-    LIBRARY_CONTAINER_UPDATED,
-    CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
+    LIBRARY_CONTAINER_UPDATED
 )
-from xblock.core import XBlock
-
 from openedx_learning.api import authoring as authoring_api
-from openedx_learning.api.authoring_models import (
-    Component,
-    ComponentVersion,
-    LearningPackage,
-    MediaType,
-)
+from openedx_learning.api.authoring_models import Component, ComponentVersion, LearningPackage, MediaType
+from xblock.core import XBlock
 
+from openedx.core.djangoapps.content_libraries import api as lib_api
 from openedx.core.djangoapps.xblock.api import (
     get_component_from_usage_key,
     get_xblock_app_config,
-    xblock_type_display_name,
+    xblock_type_display_name
 )
 from openedx.core.types import User as UserType
-from openedx.core.djangoapps.content_libraries import api as lib_api
 
 from ..models import ContentLibrary
 from ..permissions import CAN_EDIT_THIS_CONTENT_LIBRARY
-from .exceptions import (
-    BlockLimitReachedError,
-    ContentLibraryBlockNotFound,
-    InvalidNameError,
-    LibraryBlockAlreadyExists,
-)
-from .libraries import (
-    library_component_usage_key,
-    require_permission_for_library_key,
-    PublishableItem,
-)
+from .exceptions import BlockLimitReachedError, ContentLibraryBlockNotFound, InvalidNameError, LibraryBlockAlreadyExists
+from .libraries import PublishableItem, library_component_usage_key, require_permission_for_library_key
 
 log = logging.getLogger(__name__)