feat: api for adding, removing and updating components in container (#36434)

* feat: add components to container api

* feat: remove and replace components in container api

* refactor: container childern api

* chore: fix lint issues

* temp: install openedx-learning dev branch

* feat: update publish_status and children count in index

* chore: fix mypy issues

* test: fix reindex test

* refactor: rebase and fix conflicts

* test: update test to check signals

* docs: document can_stand_alone flag

* chore: bump openedx-learning version

Author: navinkarkera

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

@@ -6,11 +6,12 @@
 import logging
 from hashlib import blake2b
 
-from django.utils.text import slugify
 from django.core.exceptions import ObjectDoesNotExist
+from django.utils.text import slugify
 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
-from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator
+from openedx_learning.api.authoring_models import Collection
 from rest_framework.exceptions import NotFound
 
 from openedx.core.djangoapps.content.search.models import SearchAccess
@@ -19,7 +20,6 @@
 from openedx.core.djangoapps.content_tagging import api as tagging_api
 from openedx.core.djangoapps.xblock import api as xblock_api
 from openedx.core.djangoapps.xblock.data import LatestVersion
-from openedx_learning.api.authoring_models import Collection
 
 log = logging.getLogger(__name__)
 
@@ -554,7 +554,7 @@ def searchable_doc_for_container(
 ) -> dict:
     """
     Generate a dictionary document suitable for ingestion into a search engine
-    like Meilisearch or Elasticsearch, so that the given collection can be
+    like Meilisearch or Elasticsearch, so that the given container can be
     found using faceted search.
 
     If no container is found for the given container key, the returned document
@@ -576,29 +576,33 @@ def searchable_doc_for_container(
 
     try:
         container = lib_api.get_container(container_key)
-    except lib_api.ContentLibraryCollectionNotFound:
+    except lib_api.ContentLibraryContainerNotFound:
         # Container not found, so we can only return the base doc
-        pass
+        return doc
 
-    if container:
-        # TODO: check if there's a more efficient way to load these num_children counts?
-        draft_num_children = len(lib_api.get_container_children(container_key, published=False))
+    draft_num_children = lib_api.get_container_children_count(container_key, published=False)
+    publish_status = PublishStatus.published
+    if container.last_published is None:
+        publish_status = PublishStatus.never
+    elif container.has_unpublished_changes:
+        publish_status = PublishStatus.modified
 
-        doc.update({
-            Fields.display_name: container.display_name,
-            Fields.created: container.created.timestamp(),
-            Fields.modified: container.modified.timestamp(),
-            Fields.num_children: draft_num_children,
-        })
-        library = lib_api.get_library(container_key.library_key)
-        if library:
-            doc[Fields.breadcrumbs] = [{"display_name": library.title}]
-
-        if container.published_version_num is not None:
-            published_num_children = len(lib_api.get_container_children(container_key, published=True))
-            doc[Fields.published] = {
-                # Fields.published_display_name: container_published.title, TODO: set the published title
-                Fields.published_num_children: published_num_children,
-            }
+    doc.update({
+        Fields.display_name: container.display_name,
+        Fields.created: container.created.timestamp(),
+        Fields.modified: container.modified.timestamp(),
+        Fields.num_children: draft_num_children,
+        Fields.publish_status: publish_status,
+    })
+    library = lib_api.get_library(container_key.library_key)
+    if library:
+        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)
+        doc[Fields.published] = {
+            # Fields.published_display_name: container_published.title, TODO: set the published title
+            Fields.published_num_children: published_num_children,
+        }
 
     return doc

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

@@ -227,6 +227,7 @@ def setUp(self):
             "display_name": "Unit 1",
             # description is not set for containers
             "num_children": 0,
+            "publish_status": "never",
             "context_key": "lib:org1:lib",
             "org": "org1",
             "created": created_date.timestamp(),

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

@@ -3,12 +3,13 @@
 """
 from dataclasses import replace
 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 organizations.models import Organization
 
-from openedx.core.djangoapps.content_tagging import api as tagging_api
 from openedx.core.djangoapps.content_libraries import api as library_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
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
@@ -17,13 +18,13 @@
 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,
-        searchable_doc_tags_for_collection,
         searchable_doc_collections,
         searchable_doc_for_collection,
         searchable_doc_for_container,
+        searchable_doc_for_course_block,
         searchable_doc_for_library_block,
+        searchable_doc_tags,
+        searchable_doc_tags_for_collection,
     )
     from ..models import SearchAccess
 except RuntimeError:
@@ -522,11 +523,112 @@ def test_draft_container(self):
             "display_name": "A Unit in the Search Index",
             # description is not set for containers
             "num_children": 0,
+            "publish_status": "never",
+            "context_key": "lib:edX:2012_Fall",
+            "access_id": self.library_access_id,
+            "breadcrumbs": [{"display_name": "some content_library"}],
+            "created": 1680674828.0,
+            "modified": 1680674828.0,
+            # "tags" should be here but we haven't implemented them yet
+            # "published" is not set since we haven't published it yet
+        }
+
+    def test_published_container(self):
+        """
+        Test creating a search document for a published container
+        """
+        created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc)
+        with freeze_time(created_date):
+            container_meta = library_api.create_container(
+                self.library.key,
+                container_type=library_api.ContainerType.Unit,
+                slug="unit1",
+                title="A Unit in the Search Index",
+                user_id=None,
+            )
+            library_api.update_container_children(
+                container_meta.container_key,
+                [self.library_block.usage_key],
+                user_id=None,
+            )
+        library_api.publish_changes(self.library.key)
+
+        doc = searchable_doc_for_container(container_meta.container_key)
+
+        assert doc == {
+            "id": "lctedx2012_fallunitunit1-edd13a0c",
+            "block_id": "unit1",
+            "block_type": "unit",
+            "usage_key": "lct:edX:2012_Fall:unit:unit1",
+            "type": "library_container",
+            "org": "edX",
+            "display_name": "A Unit in the Search Index",
+            # description is not set for containers
+            "num_children": 1,
+            "publish_status": "published",
+            "context_key": "lib:edX:2012_Fall",
+            "access_id": self.library_access_id,
+            "breadcrumbs": [{"display_name": "some content_library"}],
+            "created": 1680674828.0,
+            "modified": 1680674828.0,
+            "published": {"num_children": 1},
+            # "tags" should be here but we haven't implemented them yet
+            # "published" is not set since we haven't published it yet
+        }
+
+    def test_published_container_with_changes(self):
+        """
+        Test creating a search document for a published container
+        """
+        created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc)
+        with freeze_time(created_date):
+            container_meta = library_api.create_container(
+                self.library.key,
+                container_type=library_api.ContainerType.Unit,
+                slug="unit1",
+                title="A Unit in the Search Index",
+                user_id=None,
+            )
+            library_api.update_container_children(
+                container_meta.container_key,
+                [self.library_block.usage_key],
+                user_id=None,
+            )
+        library_api.publish_changes(self.library.key)
+        block_2 = library_api.create_library_block(
+            self.library.key,
+            "html",
+            "text3",
+        )
+
+        # Add another component after publish
+        with freeze_time(created_date):
+            library_api.update_container_children(
+                container_meta.container_key,
+                [block_2.usage_key],
+                user_id=None,
+                entities_action=authoring_api.ChildrenEntitiesAction.APPEND,
+            )
+
+        doc = searchable_doc_for_container(container_meta.container_key)
+
+        assert doc == {
+            "id": "lctedx2012_fallunitunit1-edd13a0c",
+            "block_id": "unit1",
+            "block_type": "unit",
+            "usage_key": "lct:edX:2012_Fall:unit:unit1",
+            "type": "library_container",
+            "org": "edX",
+            "display_name": "A Unit in the Search Index",
+            # description is not set for containers
+            "num_children": 2,
+            "publish_status": "modified",
             "context_key": "lib:edX:2012_Fall",
             "access_id": self.library_access_id,
             "breadcrumbs": [{"display_name": "some content_library"}],
             "created": 1680674828.0,
             "modified": 1680674828.0,
+            "published": {"num_children": 1},
             # "tags" should be here but we haven't implemented them yet
             # "published" is not set since we haven't published it yet
         }

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

@@ -2,17 +2,18 @@
 API for containers (Sections, Subsections, Units) in Content Libraries
 """
 from __future__ import annotations
+
 from dataclasses import dataclass
 from datetime import datetime
 from enum import Enum
 from uuid import uuid4
 
 from django.utils.text import slugify
 from opaque_keys.edx.locator import (
-    LibraryLocatorV2,
     LibraryContainerLocator,
+    LibraryLocatorV2,
+    UsageKeyV2,
 )
-
 from openedx_events.content_authoring.data import LibraryContainerData
 from openedx_events.content_authoring.signals import (
     LIBRARY_CONTAINER_CREATED,
@@ -22,8 +23,10 @@
 from openedx_learning.api import authoring as authoring_api
 from openedx_learning.api.authoring_models import Container
 
+from openedx.core.djangoapps.xblock.api import get_component_from_usage_key
+
 from ..models import ContentLibrary
-from .libraries import PublishableItem
+from .libraries import LibraryXBlockMetadata, PublishableItem
 
 
 # The public API is only the following symbols:
@@ -34,9 +37,11 @@
     "get_container",
     "create_container",
     "get_container_children",
+    "get_container_children_count",
     "library_container_locator",
     "update_container",
     "delete_container",
+    "update_container_children",
 ]
 
 
@@ -252,14 +257,62 @@ def get_container_children(
     """
     Get the entities contained in the given container (e.g. the components/xblocks in a unit)
     """
-    assert isinstance(container_key, LibraryContainerLocator)
-    content_library = ContentLibrary.objects.get_by_key(container_key.library_key)
-    learning_package = content_library.learning_package
-    assert learning_package is not None
-    container = authoring_api.get_container_by_key(
-        learning_package.id,
-        key=container_key.container_id,
+    container = _get_container(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(
+            container_key.library_key,
+            entry.component
+        ) for entry in child_components]
+    else:
+        child_entities = authoring_api.get_entities_in_container(container, published=published)
+        return [ContainerMetadata.from_container(
+            container_key.library_key,
+            entry.entity
+        ) for entry in child_entities]
+
+
+def get_container_children_count(
+    container_key: LibraryContainerLocator,
+    published=False,
+) -> int:
+    """
+    Get the count of entities contained in the given container (e.g. the components/xblocks in a unit)
+    """
+    container = _get_container(container_key)
+    return authoring_api.get_container_children_count(container, published=published)
+
+
+def update_container_children(
+    container_key: LibraryContainerLocator,
+    children_ids: list[UsageKeyV2] | list[LibraryContainerLocator],
+    user_id: int | None,
+    entities_action: authoring_api.ChildrenEntitiesAction = authoring_api.ChildrenEntitiesAction.REPLACE,
+):
+    """
+    Adds children components or containers to given container.
+    """
+    library_key = container_key.library_key
+    container_type = container_key.container_type
+    container = _get_container(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]
+            new_version = authoring_api.create_next_unit_version(
+                container.unit,
+                components=components,  # type: ignore[arg-type]
+                created=datetime.now(),
+                created_by=user_id,
+                entities_action=entities_action,
+            )
+        case _:
+            raise ValueError(f"Invalid container type: {container_type}")
+
+    LIBRARY_CONTAINER_UPDATED.send_event(
+        library_container=LibraryContainerData(
+            library_key=library_key,
+            container_key=str(container_key),
+        )
     )
-    child_entities = authoring_api.get_entities_in_container(container, published=published)
-    # TODO: convert the return type to list[ContainerMetadata | LibraryXBlockMetadata] ?
-    return child_entities
+
+    return ContainerMetadata.from_container(library_key, new_version.container)