fix: error seen when deleting a component used in a container (requires API change) [FC-0117]

cms/djangoapps/modulestore_migrator/tasks.py

@@ -881,6 +881,7 @@ def _migrate_container(
                 user_id=context.created_by,
             )
     if container_exists and context.should_skip_strategy:
+        assert container.draft_version_num is not None  # We know it exists, this is just for mypy
         return PublishableEntityVersion.objects.get(
             entity_id=container.container_id,
             version_num=container.draft_version_num,

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

@@ -12,7 +12,7 @@
 from opaque_keys.edx.keys import ContainerKey, LearningContextKey, OpaqueKey, UsageKey
 from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator
 from openedx_content import api as content_api
-from openedx_content.models_api import Collection, Section, Subsection, Unit
+from openedx_content.models_api import Collection
 from rest_framework.exceptions import NotFound
 
 from openedx.core.djangoapps.content.search.models import SearchAccess
@@ -627,42 +627,21 @@ def searchable_doc_for_container(
         log.error(f"Container {container_key} not found")
         return doc
 
-    draft_children = lib_api.get_container_children(
-        container_key,
-        published=False,
-    )
+    draft_children = lib_api.get_container_children_list(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
 
-    container_type_code = container_key.container_type
-
-    def get_child_keys(children) -> list[str]:
-        match container_type_code:
-            case Unit.type_code:
-                return [
-                    str(child.usage_key)
-                    for child in children
-                ]
-            case Subsection.type_code | Section.type_code:
-                return [
-                    str(child.container_key)
-                    for child in children
-                ]
-
-    def get_child_names(children) -> list[str]:
-        return [child.display_name for child in children]
-
     doc.update({
         Fields.display_name: container.display_name,
         Fields.created: container.created.timestamp(),
         Fields.modified: container.modified.timestamp(),
         Fields.num_children: len(draft_children),
         Fields.content: {
-            Fields.child_usage_keys: get_child_keys(draft_children),
-            Fields.child_display_names: get_child_names(draft_children),
+            Fields.child_usage_keys: [str(child.key) for child in draft_children],
+            Fields.child_display_names: [child.display_name for child in draft_children],
         },
         Fields.publish_status: publish_status,
         Fields.last_published: container.last_published.timestamp() if container.last_published else None,
@@ -672,16 +651,13 @@ def get_child_names(children) -> list[str]:
         doc[Fields.breadcrumbs] = [{"display_name": library.title}]
 
     if container.published_version_num is not None:
-        published_children = lib_api.get_container_children(
-            container_key,
-            published=True,
-        )
+        published_children = lib_api.get_container_children_list(container_key, published=True)
         doc[Fields.published] = {
             Fields.published_display_name: container.published_display_name,
             Fields.published_num_children: len(published_children),
             Fields.published_content: {
-                Fields.child_usage_keys: get_child_keys(published_children),
-                Fields.child_display_names: get_child_names(published_children),
+                Fields.child_usage_keys: [str(child.key) for child in published_children],
+                Fields.child_display_names: [child.display_name for child in published_children],
             },
         }
 

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

@@ -46,9 +46,20 @@ class LibraryXBlockMetadata(PublishableItem):
     usage_key: LibraryUsageLocatorV2
 
     @classmethod
-    def from_component(cls, library_key, component, associated_collections=None):
+    def from_component(
+        cls,
+        library_key: LibraryLocatorV2,
+        component,
+        associated_collections=None,
+        use_published=False,
+    ) -> LibraryXBlockMetadata:
         """
         Construct a LibraryXBlockMetadata from a Component object.
+
+        Requires that the draft version of the component exists, unless you
+        specify use_published=True, in which case it requires that the published
+        version exists. The 'display_name' and 'modified' fields will depend on
+        which version you request.
         """
         # Import content_tagging.api here to avoid circular imports
         from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts
@@ -61,17 +72,17 @@ def from_component(cls, library_key, component, associated_collections=None):
         draft = component.versioning.draft
         published = component.versioning.published
         last_draft_created = draft.created if draft else None
-        last_draft_created_by = draft.publishable_entity_version.created_by if draft else None
+        last_draft_created_by = draft.publishable_entity_version.created_by if draft else ""
         usage_key = library_component_usage_key(library_key, component)
         tags = get_object_tag_counts(str(usage_key), count_implicit=True)
 
         return cls(
             usage_key=usage_key,
-            display_name=draft.title,
+            display_name=published.title if use_published else draft.title,
             created=component.created,
             created_by=component.created_by.username if component.created_by else None,
-            modified=draft.created,
-            draft_version_num=draft.version_num,
+            modified=published.created if use_published else draft.created,
+            draft_version_num=draft.version_num if draft else None,
             published_version_num=published.version_num if published else None,
             published_display_name=published.title if published else None,
             last_published=None if last_publish_log is None else last_publish_log.published_at,

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

@@ -417,6 +417,7 @@ def set_library_block_olx(
     usage_key: LibraryUsageLocatorV2,
     new_olx_str: str,
     paths_to_media: dict | None = None,
+    created_by: int | None = None,
 ) -> ComponentVersion:
     """
     Replace the OLX source of the given XBlock.
@@ -488,6 +489,7 @@ def set_library_block_olx(
                 'block.xml': new_olx_media.pk,
             },
             created=now,
+            created_by=created_by,
         )
 
     return new_component_version

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

@@ -74,9 +74,14 @@ class ContainerMetadata(PublishableItem):
     container_id: Container.ID
 
     @classmethod
-    def from_container(cls, library_key, container: Container, associated_collections=None):
+    def from_container(cls, library_key, container: Container, associated_collections=None, use_published=False):
         """
         Construct a ContainerMetadata object from a Container object.
+
+        Requires that the draft version of the container exists, unless you
+        specify use_published=True, in which case it requires that the published
+        version exists. The 'display_name' and 'modified' fields will depend on
+        which version you request.
         """
         last_publish_log = container.versioning.last_publish_log
         container_key = library_container_locator(
@@ -100,10 +105,10 @@ def from_container(cls, library_key, container: Container, associated_collection
             container_key=container_key,
             container_type_code=container_key.container_type,
             container_id=container.id,
-            display_name=draft.title,
+            display_name=published.title if use_published else draft.title,
             created=container.created,
-            modified=draft.created,
-            draft_version_num=draft.version_num,
+            modified=published.created if use_published else draft.created,
+            draft_version_num=draft.version_num if draft else None,
             published_version_num=published.version_num if published else None,
             published_display_name=published.title if published else None,
             last_published=None if last_publish_log is None else last_publish_log.published_at,

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

@@ -7,6 +7,7 @@
 import logging
 import operator
 import typing
+from dataclasses import dataclass
 from datetime import datetime, timezone
 from functools import cache
 from uuid import UUID, uuid4
@@ -28,6 +29,7 @@
     LibraryXBlockMetadata,
     direct_published_entity_from_record,
     get_entity_item_type,
+    library_component_usage_key,
     make_contributor,
     resolve_change_action,
 )
@@ -48,9 +50,11 @@
 # 🛑 UNSTABLE: All APIs related to containers are unstable until we've figured
 #              out our approach to dynamic content (randomized, A/B tests, etc.)
 __all__ = [
+    "ContainerChildMetadata",
     "get_container",
     "create_container",
     "get_container_children",
+    "get_container_children_list",
     "get_container_children_count",
     "update_container",
     "delete_container",
@@ -70,6 +74,15 @@
 log = logging.getLogger(__name__)
 
 
+@dataclass(frozen=True)
+class ContainerChildMetadata:
+    """
+    Class that represents limited metadata about a child entity of a container
+    """
+    display_name: str
+    key: LibraryUsageLocatorV2 | LibraryContainerLocator
+
+
 def get_container(
     container_key: LibraryContainerLocator,
     *,
@@ -202,19 +215,49 @@ def get_container_children(
     published=False,
 ) -> list[LibraryXBlockMetadata | ContainerMetadata]:
     """
-    [ 🛑 UNSTABLE ] Get the entities contained in the given container
-    (e.g. the components/xblocks in a unit, units in a subsection, subsections in a section)
+    [ 🛑 UNSTABLE ] Get the entities contained in the given container (e.g. the
+    components in a unit, units in a subsection, subsections in a section).
     """
     container = get_container_from_key(container_key)
 
     child_entities = content_api.get_entities_in_container(container, published=published)
     result: list[LibraryXBlockMetadata | ContainerMetadata] = []
     for entry in child_entities:
         if hasattr(entry.entity, "component"):  # the child is a Component
-            result.append(LibraryXBlockMetadata.from_component(container_key.lib_key, entry.entity.component))
+            result.append(LibraryXBlockMetadata.from_component(
+                container_key.lib_key, entry.entity.component, use_published=published,
+            ))
+        else:
+            assert isinstance(entry.entity.container, Container)
+            result.append(ContainerMetadata.from_container(
+                container_key.lib_key, entry.entity.container, use_published=published,
+            ))
+    return result
+
+
+def get_container_children_list(
+    container_key: LibraryContainerLocator, *,
+    published: bool,
+) -> list[ContainerChildMetadata]:
+    """
+    [ 🛑 UNSTABLE ] Get the entities contained in the given container (e.g. the
+    components/xblocks in a unit, units in a subsection, subsections in a section)
+
+    Returns a list of ``ContainerChildMetadata`` objects (which give only each
+    child's display name and opaque key, though the opaque key also includes
+    information on what "type" of component/container it is).
+    """
+    container = get_container_from_key(container_key)
+    result: list[ContainerChildMetadata] = []
+    for entry in content_api.get_entities_in_container(container, published=published):
+        key: LibraryUsageLocatorV2 | LibraryContainerLocator
+        if hasattr(entry.entity, "component"):  # the child is a Component
+            key = library_component_usage_key(container_key.lib_key, entry.entity.component)
         else:
             assert isinstance(entry.entity.container, Container)
-            result.append(ContainerMetadata.from_container(container_key.lib_key, entry.entity.container))
+            key = library_container_locator(container_key.lib_key, entry.entity.container)
+        display_name = entry.entity_version.title
+        result.append(ContainerChildMetadata(display_name=display_name, key=key))
     return result
 
 
@@ -254,7 +297,7 @@ def update_container_children(
 
 def get_containers_contains_item(key: LibraryUsageLocatorV2 | LibraryContainerLocator) -> list[ContainerMetadata]:
     """
-    [ 🛑 UNSTABLE ] Get containers that contains the item, that can be a component or another container.
+    [ 🛑 UNSTABLE ] Get list of draft containers that contain the item. Item can be a component or another container.
     """
     entity = get_entity_from_key(key)
     containers = content_api.get_containers_with_entity(entity.id).select_related("container_type")

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

@@ -200,7 +200,7 @@ class PublishableItem(LibraryItem):
     Common fields for anything that can be found in a content library that has
     draft/publish support.
     """
-    draft_version_num: int
+    draft_version_num: int | None
     published_version_num: int | None = None
     published_display_name: str | None
     last_published: datetime | None = None

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

@@ -433,7 +433,7 @@ def post(self, request, usage_key_str):
         serializer.is_valid(raise_exception=True)
         new_olx_str = serializer.validated_data["olx"]
         try:
-            version_num = api.set_library_block_olx(key, new_olx_str).version_num
+            version_num = api.set_library_block_olx(key, new_olx_str, created_by=request.user.id).version_num
         except ValueError as err:
             raise ValidationError(detail=str(err))  # pylint: disable=raise-missing-from  # noqa: B904
         return Response(self.serializer_class({"olx": new_olx_str, "version_num": version_num}).data)

openedx/core/djangoapps/content_libraries/tests/base.py

@@ -489,11 +489,11 @@ def _restore_container(self, container_key: ContainerKey | str, expect_response=
         """ Restore a deleted a container (unit etc.) """
         return self._api('post', URL_LIB_CONTAINER_RESTORE.format(container_key=container_key), None, expect_response)
 
-    def _get_container_children(self, container_key: ContainerKey | str, expect_response=200):
+    def _get_container_children(self, container_key: ContainerKey | str, published=False, expect_response=200):
         """ Get container children"""
         return self._api(
             'get',
-            URL_LIB_CONTAINER_CHILDREN.format(container_key=container_key),
+            URL_LIB_CONTAINER_CHILDREN.format(container_key=container_key) + ("?published=true" if published else ""),
             None,
             expect_response
         )