refactor: update to work with latest openedx_content container changes [FC-0117]

cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py

@@ -128,7 +128,7 @@ def _create_block_from_upstream(
             "library_content_key": upstream_key,
         }, expect_response=expect_response)
 
-    def _update_course_block_fields(self, usage_key: str, fields: dict[str, Any] = None):
+    def _update_course_block_fields(self, usage_key: str, fields: dict[str, Any] | None = None):
         """ Update fields of an XBlock """
         return self._api('patch', f"/xblock/{usage_key}", {
             "metadata": fields,
@@ -158,7 +158,7 @@ def _get_downstream_links(
             data["use_top_level_parents"] = str(use_top_level_parents)
         return self.client.get("/api/contentstore/v2/downstreams/", data=data)
 
-    def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> bool:
+    def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> None:
         """ Assert that the given XML strings are equal, ignoring attribute order and some whitespace variations. """
         self.assertEqual(
             ElementTree.canonicalize(xml_str_a, strip_text=True),

cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py

@@ -11,6 +11,7 @@
 from freezegun import freeze_time
 from opaque_keys.edx.keys import ContainerKey, UsageKey
 from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
+from openedx_content import models_api as content_models
 from organizations.models import Organization
 
 from cms.djangoapps.contentstore.helpers import StaticFileNotices
@@ -221,6 +222,11 @@ def setUp(self):
         self._publish_library_block(self.video_lib_id)
         self._publish_library_block(self.html_lib_id)
 
+    def tearDown(self):
+        # If we're working with Containers in test cases, we need this line:
+        content_models.Container.reset_cache()
+        return super().tearDown()
+
     def _api(self, method, url, data, expect_response):
         """
         Call a REST API

cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py

@@ -3,10 +3,9 @@
 """
 
 from collections import OrderedDict
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, timezone
 
 import ddt
-import pytz
 from django.conf import settings
 from django.urls import reverse
 from rest_framework import status
@@ -36,7 +35,7 @@ def setUp(self):
             display_name="Demo Course (Sample)",
             id=archived_course_key,
             org=archived_course_key.org,
-            end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC),
+            end=(datetime.now() - timedelta(days=365)).replace(tzinfo=timezone.utc),
         )
         self.non_staff_client, _ = self.create_non_staff_authed_user_client()
 
@@ -256,7 +255,7 @@ def test_filter_and_ordering_courses(
             display_name="Course (Demo)",
             id=archived_course_key,
             org=archived_course_key.org,
-            end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC),
+            end=(datetime.now() - timedelta(days=365)).replace(tzinfo=timezone.utc),
         )
         active_course_key = self.store.make_course_key("foo-org", "foo-number", "foo-run")
         CourseOverviewFactory.create(

cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py

@@ -9,7 +9,7 @@
 Along with it, we moved the business logic of the other views in that file, since that is related.
 """
 import logging
-from datetime import datetime
+from datetime import datetime, timezone
 from uuid import uuid4
 
 from attrs import asdict
@@ -20,7 +20,9 @@
 from django.http import HttpResponse, HttpResponseBadRequest
 from django.utils.translation import gettext as _
 from edx_django_utils.plugins import pluggable_override
-from openedx.core.djangoapps.content_libraries.api import ContainerMetadata, ContainerType, LibraryXBlockMetadata
+from openedx_content import api as content_api
+from openedx_content import models_api as content_models
+from openedx.core.djangoapps.content_libraries.api import ContainerMetadata, LibraryXBlockMetadata
 from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts
 from openedx.core import toggles as core_toggles
 from openedx_authz import api as authz_api
@@ -40,7 +42,6 @@
 from edx_proctoring.exceptions import ProctoredExamNotFoundException
 from help_tokens.core import HelpUrlExpert
 from opaque_keys.edx.locator import LibraryUsageLocator, LibraryUsageLocatorV2
-from pytz import UTC
 from xblock.core import XBlock
 from xblock.fields import Scope
 from .xblock_helpers import get_block_key_string
@@ -659,7 +660,7 @@ def sync_library_content(
         with store.bulk_operations(downstream.usage_key.context_key):
             upstream_children = sync_from_upstream_container(downstream=downstream, user=request.user)
             downstream_children = downstream.get_children()
-            downstream_children_keys = [child.upstream for child in downstream_children]
+            downstream_children_key_strings: list[str] = [child.upstream for child in downstream_children]
             # Sync the children:
             notices = []
             # Store final children keys to update order of items in containers
@@ -669,30 +670,27 @@ def sync_library_content(
 
             for i, upstream_child in enumerate(upstream_children):
                 if isinstance(upstream_child, LibraryXBlockMetadata):
-                    upstream_key = str(upstream_child.usage_key)
+                    upstream_child_key_string = str(upstream_child.usage_key)
                     block_type = upstream_child.usage_key.block_type
                 elif isinstance(upstream_child, ContainerMetadata):
-                    upstream_key = str(upstream_child.container_key)
-                    match upstream_child.container_type:
-                        case ContainerType.Unit:
-                            block_type = "vertical"
-                        case ContainerType.Subsection:
-                            block_type = "sequential"
-                        case _:
-                            # We don't support other container types for now.
-                            log.error(
-                                "Unexpected upstream child container type: %s",
-                                upstream_child.container_type,
-                            )
-                            continue
+                    upstream_child_key_string = str(upstream_child.container_key)
+                    if upstream_child.container_type_code not in (
+                        content_models.Unit.type_code,
+                        content_models.Subsection.type_code,
+                    ):
+                        # We don't support other container types for now.
+                        log.error("Unexpected upstream child container type: %s", upstream_child.container_type_code)
+                        continue
+                    # convert "unit" -> "vertical", "subsection" -> "sequential"
+                    block_type = content_api.get_container_subclass(upstream_child.container_type_code).olx_tag_name
                 else:
                     log.error(
                         "Unexpected type of upstream child: %s",
                         type(upstream_child),
                     )
                     continue
 
-                if upstream_key not in downstream_children_keys:
+                if upstream_child_key_string not in downstream_children_key_strings:
                     # This upstream_child is new, create it.
                     downstream_child = store.create_child(
                         parent_usage_key=downstream.usage_key,
@@ -702,14 +700,14 @@ def sync_library_content(
                         # TODO: Can we generate a unique but friendly block_id, perhaps using upstream block_id
                         block_id=f"{block_type}{uuid4().hex[:8]}",
                         fields={
-                            "upstream": upstream_key,
+                            "upstream": upstream_child_key_string,
                             "top_level_downstream_parent_key": get_block_key_string(
                                 top_level_downstream_parent.usage_key,
                             ),
                         },
                     )
                 else:
-                    downstream_child_old_index = downstream_children_keys.index(upstream_key)
+                    downstream_child_old_index = downstream_children_key_strings.index(upstream_child_key_string)
                     downstream_child = downstream_children[downstream_child_old_index]
 
                 children.append(downstream_child.usage_key)
@@ -1313,7 +1311,7 @@ def create_xblock_info(  # lint-amnesty, pylint: disable=too-many-statements
                 "studio_url": xblock_studio_url(xblock, parent_xblock),
                 "lms_url": xblock_lms_url(xblock),
                 "embed_lms_url": xblock_embed_lms_url(xblock),
-                "released_to_students": datetime.now(UTC) > xblock.start,
+                "released_to_students": datetime.now(timezone.utc) > xblock.start,
                 "release_date": release_date,
                 "visibility_state": visibility_state,
                 "has_explicit_staff_lock": xblock.fields[
@@ -1652,7 +1650,7 @@ def _compute_visibility_state(
         return VisibilityState.needs_attention
 
     is_unscheduled = xblock.start == DEFAULT_START_DATE
-    is_live = is_course_self_paced or datetime.now(UTC) > xblock.start
+    is_live = is_course_self_paced or datetime.now(timezone.utc) > xblock.start
     if child_info and child_info.get("children", []):  # pylint: disable=too-many-nested-blocks
         all_staff_only = True
         all_unscheduled = True

cms/djangoapps/modulestore_migrator/api/read_api.py

@@ -60,9 +60,7 @@ def get_forwarding_for_blocks(source_keys: t.Iterable[UsageKey]) -> dict[UsageKe
         # For building component key
         "forwarded__target__component__component_type",
         # For building container key
-        "forwarded__target__container__section",
-        "forwarded__target__container__subsection",
-        "forwarded__target__container__unit",
+        "forwarded__target__container__container_type",
         # For determining title and version
         "forwarded__change_log_record__new_version",
     )
@@ -166,11 +164,7 @@ def get_migration_blocks(migration_pk: int) -> dict[UsageKey, ModulestoreBlockMi
             # For building component key
             "target__component__component_type",
             # For building container key.
-            # (Hard-coding these exact 3 container types here is not a good pattern, but it's what is needed
-            #  here in order to avoid additional SELECTs while determining the container type).
-            "target__container__section",
-            "target__container__subsection",
-            "target__container__unit",
+            "target__container__container_type",
             # For determining title and version
             "change_log_record__new_version",
         )

cms/djangoapps/modulestore_migrator/data.py

@@ -18,8 +18,7 @@
     LibraryLocatorV2,
     LibraryUsageLocatorV2,
 )
-
-from openedx.core.djangoapps.content_libraries.api import ContainerType
+from openedx_content import models_api as content_models
 
 
 class CompositionLevel(Enum):
@@ -32,9 +31,9 @@ class CompositionLevel(Enum):
     Component = 'component'
 
     # Container types currently supported by Content Libraries
-    Unit = ContainerType.Unit.value
-    Subsection = ContainerType.Subsection.value
-    Section = ContainerType.Section.value
+    Unit = content_models.Unit.type_code  # "unit"
+    Subsection = content_models.Subsection.type_code  # "subsection"
+    Section = content_models.Section.type_code  # "section"
 
     @property
     def is_container(self) -> bool:

cms/djangoapps/modulestore_migrator/tasks.py

@@ -46,7 +46,7 @@
 from common.djangoapps.split_modulestore_django.models import SplitModulestoreCourseIndex
 from common.djangoapps.util.date_utils import DEFAULT_DATE_TIME_FORMAT, strftime_localized
 from openedx.core.djangoapps.content_libraries import api as libraries_api
-from openedx.core.djangoapps.content_libraries.api import ContainerType, get_library
+from openedx.core.djangoapps.content_libraries.api import get_library
 from openedx.core.djangoapps.content_staging import api as staging_api
 from xmodule.modulestore import exceptions as modulestore_exceptions
 from xmodule.modulestore.django import modulestore
@@ -766,21 +766,21 @@ def _migrate_node(
     #                                     do not support in libraries as of Ulmo.
     should_migrate_node: bool
     should_migrate_children: bool
-    container_type: ContainerType | None  # if None, it's a Component
+    container_cls: content_api.ContainerSubclass | None  # if None, it's a Component
     if source_node.tag == "wiki":
         return _MigratedNode(None, [])
     try:
-        container_type = ContainerType.from_source_olx_tag(source_node.tag)
+        container_cls = libraries_api.container_subclass_for_olx_tag(source_node.tag)
     except ValueError:
-        container_type = None
+        container_cls = None
         if source_node.tag in {"course", "library"}:
             should_migrate_node = False
             should_migrate_children = True
         else:
             should_migrate_node = True
             should_migrate_children = False
     else:
-        node_level = CompositionLevel(container_type.value)
+        node_level = CompositionLevel(container_cls.type_code)
         should_migrate_node = not node_level.is_higher_than(context.composition_level)
         should_migrate_children = True
     migrated_children: list[_MigratedNode] = []
@@ -802,23 +802,23 @@ def _migrate_node(
                 _migrate_container(
                     context=context,
                     source_key=source_key,
-                    container_type=container_type,
+                    container_cls=container_cls,
                     title=title,
                     children=[
                         migrated_child.source_to_target[1]
                         for migrated_child in migrated_children if
                         migrated_child.source_to_target and migrated_child.source_to_target[1]
                     ],
                 )
-                if container_type else
+                if container_cls else
                 _migrate_component(
                     context=context,
                     source_key=source_key,
                     olx=source_olx,
                     title=title,
                 )
             )
-            if container_type is None and target_entity_version is None and reason is not None:
+            if container_cls is None and target_entity_version is None and reason is not None:
                 # Currently, components with children are not supported
                 children_length = len(source_node.getchildren())
                 if children_length:
@@ -842,7 +842,7 @@ def _migrate_container(
     *,
     context: _MigrationContext,
     source_key: UsageKey,
-    container_type: ContainerType,
+    container_cls: content_api.ContainerSubclass,
     title: str,
     children: list[PublishableEntityVersion],
 ) -> tuple[PublishableEntityVersion, str | None]:
@@ -857,7 +857,7 @@ def _migrate_container(
     target_key = _get_distinct_target_container_key(
         context,
         source_key,
-        container_type,
+        container_cls,
         title,
     )
     try:
@@ -874,7 +874,7 @@ def _migrate_container(
         else:
             container = libraries_api.create_container(
                 library_key=context.target_library_key,
-                container_type=container_type,
+                container_cls=container_cls,
                 slug=target_key.container_id,
                 title=title,
                 created=context.created_at,
@@ -889,13 +889,9 @@ def _migrate_container(
     container_publishable_entity_version = content_api.create_next_container_version(
         container.container_pk,
         title=title,
-        entity_rows=[
-            content_api.ContainerEntityRow(entity_pk=child.entity_id, version_pk=None)
-            for child in children
-        ],
+        entities=[child.entity for child in children],
         created=context.created_at,
         created_by=context.created_by,
-        container_version_cls=container_type.container_model_classes[1],
     ).publishable_entity_version
 
     # Publish the container
@@ -990,7 +986,7 @@ def _migrate_component(
 def _get_distinct_target_container_key(
     context: _MigrationContext,
     source_key: UsageKey,
-    container_type: ContainerType,
+    container_cls: content_api.ContainerSubclass,
     title: str,
 ) -> LibraryContainerLocator:
     """
@@ -1012,14 +1008,14 @@ def _get_distinct_target_container_key(
     # Use base base slug if available
     if base_slug not in context.used_container_slugs:
         return LibraryContainerLocator(
-            context.target_library_key, container_type.value, base_slug
+            context.target_library_key, container_cls.type_code, base_slug
         )
     # Try numbered variations until we find one that doesn't exist
     for i in range(1, _MAX_UNIQUE_SLUG_ATTEMPTS + 1):
         candidate_slug = f"{base_slug}_{i}"
         if candidate_slug not in context.used_container_slugs:
             return LibraryContainerLocator(
-                context.target_library_key, container_type.value, candidate_slug
+                context.target_library_key, container_cls.type_code, candidate_slug
             )
     # It would be extremely unlikely for us to run out of attempts
     raise RuntimeError(

cms/djangoapps/modulestore_migrator/tests/test_api.py

@@ -6,6 +6,7 @@
 from unittest.mock import patch
 from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2, CourseLocator
 from openedx_content import api as content_api
+from openedx_content import models_api as content_models
 from organizations.tests.factories import OrganizationFactory
 
 from cms.djangoapps.modulestore_migrator import api
@@ -148,6 +149,11 @@ def setUp(self):
                 user_id=self.user.id, publish_item=False,
             )
 
+    def tearDown(self):
+        # If we're working with Containers in test cases, we need this line:
+        content_models.Container.reset_cache()
+        return super().tearDown()
+
     def test_start_migration_to_library(self):
         """
         Test that the API can start a migration to a library.