diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index f7c96d588a2e..f0650f9fab79 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -20,7 +20,8 @@ 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.core.djangoapps.content_libraries.api import ContainerMetadata, LibraryXBlockMetadata from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts from edx_proctoring.api import ( does_backend_support_onboarding, @@ -572,18 +573,15 @@ def sync_library_content( 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 + # convert "unit" -> "vertical", "subsection" -> "sequential" + block_type = content_api.get_container_type(upstream_child.container_type_code).olx_tag_name + if not block_type or block_type not in ("vertical", "sequential"): + # We don't support other container types for now. + log.error( + "Unexpected upstream child container type: %s", + upstream_child.container_type, + ) + continue else: log.error( "Unexpected type of upstream child: %s", diff --git a/cms/djangoapps/modulestore_migrator/data.py b/cms/djangoapps/modulestore_migrator/data.py index f2a9d725656c..a410ea01133f 100644 --- a/cms/djangoapps/modulestore_migrator/data.py +++ b/cms/djangoapps/modulestore_migrator/data.py @@ -18,8 +18,7 @@ LibraryLocatorV2, LibraryUsageLocatorV2, ) - -from openedx.core.djangoapps.content_libraries.api import ContainerType +from openedx_content.models_api import Unit, Subsection, Section 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 = Unit.type_code # "unit" + Subsection = Subsection.type_code # "subsection" + Section = Section.type_code # "section" @property def is_container(self) -> bool: diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index cbcbc6db52c8..47729069602b 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/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,11 +766,11 @@ 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_type: content_api.ContainerType | 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_type = libraries_api.container_type_for_olx_tag(source_node.tag) except ValueError: container_type = None if source_node.tag in {"course", "library"}: @@ -780,7 +780,7 @@ def _migrate_node( should_migrate_node = True should_migrate_children = False else: - node_level = CompositionLevel(container_type.value) + node_level = CompositionLevel(container_type.type_code) should_migrate_node = not node_level.is_higher_than(context.composition_level) should_migrate_children = True migrated_children: list[_MigratedNode] = [] @@ -842,7 +842,7 @@ def _migrate_container( *, context: _MigrationContext, source_key: UsageKey, - container_type: ContainerType, + container_type: content_api.ContainerType, title: str, children: list[PublishableEntityVersion], ) -> tuple[PublishableEntityVersion, str | None]: @@ -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_type: content_api.ContainerType, 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_type.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_type.type_code, candidate_slug ) # It would be extremely unlikely for us to run out of attempts raise RuntimeError( diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index b36beeba4710..5bc7780723f6 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -827,19 +827,19 @@ def update_library_containers_collections( """ library_key = collection_key.lib_key library = lib_api.get_library(library_key) - containers = content_api.get_collection_containers( + container_entities = content_api.get_collection_entities( library.learning_package_id, collection_key.collection_id, - ) + ).exclude(container=None).select_related("container") - paginator = Paginator(containers, batch_size) + paginator = Paginator(container_entities, batch_size) for page in paginator.page_range: docs = [] - for container in paginator.page(page).object_list: + for container_entity in paginator.page(page).object_list: container_key = lib_api.library_container_locator( library_key, - container, + container_entity.container, ) doc = searchable_doc_for_key(container_key) doc.update(searchable_doc_collections(container_key)) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 4ac9abe3008d..00c0e28fcb4c 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -9,6 +9,7 @@ from unittest.mock import MagicMock, Mock, call, patch from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator +from openedx_content import models_api as content_models import ddt import pytest @@ -224,7 +225,7 @@ def setUp(self) -> None: with freeze_time(self.created_date): self.unit = library_api.create_container( library_key=self.library.key, - container_type=library_api.ContainerType.Unit, + container_type=content_models.Unit, slug="unit-1", title="Unit 1", user_id=None, @@ -232,7 +233,7 @@ def setUp(self) -> None: self.unit_key = "lct:org1:lib:unit:unit-1" self.subsection = library_api.create_container( self.library.key, - container_type=library_api.ContainerType.Subsection, + container_type=content_models.Subsection, slug="subsection-1", title="Subsection 1", user_id=None, @@ -245,7 +246,7 @@ def setUp(self) -> None: self.subsection_key = "lct:org1:lib:subsection:subsection-1" self.section = library_api.create_container( self.library.key, - container_type=library_api.ContainerType.Section, + container_type=content_models.Section, slug="section-1", title="Section 1", user_id=None, diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index a2bc09052b50..b074730d3c45 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -8,6 +8,7 @@ from freezegun import freeze_time from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator from openedx_content import api as content_api +from openedx_content import models_api as content_models from organizations.models import Organization from openedx.core.djangoapps.content_libraries import api as library_api @@ -92,7 +93,7 @@ def setUpClass(cls): ) cls.container = library_api.create_container( cls.library.key, - container_type=library_api.ContainerType.Unit, + container_type=content_models.Unit, slug="unit1", title="A Unit in the Search Index", user_id=None, @@ -102,7 +103,7 @@ def setUpClass(cls): ) cls.subsection = library_api.create_container( cls.library.key, - container_type=library_api.ContainerType.Subsection, + container_type=content_models.Subsection, slug="subsection1", title="A Subsection in the Search Index", user_id=None, @@ -112,7 +113,7 @@ def setUpClass(cls): ) cls.section = library_api.create_container( cls.library.key, - container_type=library_api.ContainerType.Section, + container_type=content_models.Section, slug="section1", title="A Section in the Search Index", user_id=None, diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index dc0913d0fdc7..46ec16eb515b 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -65,8 +65,8 @@ get_containers_contains_item, update_container_children, ContainerMetadata, - ContainerType, ) +from .container_metadata import container_type_for_olx_tag from .collections import library_collection_locator from .libraries import PublishableItem from .. import tasks @@ -547,7 +547,7 @@ def _import_staged_block_as_container( container = create_container( library_key=library_key, - container_type=ContainerType.from_source_olx_tag(olx_node.tag), + container_type=container_type_for_olx_tag(olx_node.tag), slug=None, # auto-generate slug from title title=title, user_id=user.id, diff --git a/openedx/core/djangoapps/content_libraries/api/container_metadata.py b/openedx/core/djangoapps/content_libraries/api/container_metadata.py index 0e6ed1ea6173..9caf3d405dbf 100644 --- a/openedx/core/djangoapps/content_libraries/api/container_metadata.py +++ b/openedx/core/djangoapps/content_libraries/api/container_metadata.py @@ -1,8 +1,11 @@ """ Content libraries data classes related to Containers. """ + from __future__ import annotations +from typing import Self + from dataclasses import dataclass, field as dataclass_field from enum import Enum from django.db.models import QuerySet @@ -33,73 +36,31 @@ __all__ = [ # Models "ContainerMetadata", - "ContainerType", # Methods + "container_type_for_olx_tag", "library_container_locator", ] +# For now, we only allow the following types of containers in content libraries, and their hierarchy is hard-coded. +LIBRARY_ALLOWED_CONTAINER_TYPES = ["unit", "subsection", "section"] -class ContainerType(Enum): - """ - The container types supported by content_libraries, and logic to map them to OLX. - """ - Unit = "unit" - Subsection = "subsection" - Section = "section" - - @property - def container_model_classes(self) -> tuple[type[Container], type[ContainerVersion]]: - """ - Get the container, containerversion subclasses associated with this type. - @@TODO Is this what we want, a hard mapping between container_types and Container classes? - * If so, then expand on this pattern, so that all ContainerType logic is contained within - this class, and get rid of the match-case statements that are all over the content_libraries - app. - * If not, then figure out what to do instead. - """ - match self: - case self.Unit: - return (Unit, UnitVersion) - case self.Subsection: - return (Subsection, SubsectionVersion) - case self.Section: - return (Section, SectionVersion) - raise TypeError(f"unexpected ContainerType: {self!r}") - @property - def olx_tag(self) -> str: - """ - Canonical XML tag to use when representing this container as OLX. - - For example, Units are encoded as .... - - These tag names are historical. We keep them around for the backwards compatibility of OLX - and for easier interaction with legacy modulestore-powered structural XBlocks - (e.g., copy-paste of Units between courses and V2 libraries). - """ - match self: - case self.Unit: - return "vertical" - case self.Subsection: - return "sequential" - case self.Section: - return "chapter" - raise TypeError(f"unexpected ContainerType: {self!r}") +def container_type_for_olx_tag(olx_tag: str) -> content_api.ContainerType: + """ + Given an OLX tag code (e.g. `"vertical"` for ``), get the + corresponding `ContainerType`, e.g. `Unit`. - @classmethod - def from_source_olx_tag(cls, olx_tag: str) -> 'ContainerType': - """ - Get the ContainerType that this OLX tag maps to. - """ - if olx_tag == "unit": - # There is an alternative implementation to VerticalBlock called UnitBlock whose - # OLX tag is . When converting from OLX, we want to handle both - # and as Unit containers, although the canonical serialization is still . - return cls.Unit - try: - return next(ct for ct in cls if olx_tag == ct.olx_tag) - except StopIteration: - raise ValueError(f"no container_type for XML tag: <{olx_tag}>") from None + This method is specific to content libraries. + """ + try: + ct = next(ct for ct in content_api.get_all_container_types() if olx_tag == ct.olx_tag_name) + except StopIteration: + raise ValueError(f"Content libraries does not support containers with XML tag: <{olx_tag}>") from None + if ct.type_code not in LIBRARY_ALLOWED_CONTAINER_TYPES: + raise ValueError( + f'Content libraries does not support "{ct.type_code}" containers (with XML tag <{olx_tag}>)' + ) from None + return ct @dataclass(frozen=True, kw_only=True) @@ -107,8 +68,9 @@ class ContainerMetadata(PublishableItem): """ Class that represents the metadata about a Container (e.g. Unit) in a content library. """ + container_key: LibraryContainerLocator - container_type: ContainerType + container_type_code: str container_pk: int @classmethod @@ -121,7 +83,7 @@ def from_container(cls, library_key, container: Container, associated_collection library_key, container=container, ) - container_type = ContainerType(container_key.container_type) + type_code = content_api.get_container_type_code_of(container) published_by = None if last_publish_log and last_publish_log.published_by: published_by = last_publish_log.published_by.username @@ -137,7 +99,7 @@ def from_container(cls, library_key, container: Container, associated_collection return cls( container_key=container_key, - container_type=container_type, + container_type_code=type_code, container_pk=container.pk, display_name=draft.title, created=container.created, @@ -164,6 +126,7 @@ class ContainerHierarchy: database queries in the future. More details being discussed in https://github.com/openedx/edx-platform/pull/36813#issuecomment-3136631767 """ + sections: list[ContainerHierarchyMember] = dataclass_field(default_factory=list) subsections: list[ContainerHierarchyMember] = dataclass_field(default_factory=list) units: list[ContainerHierarchyMember] = dataclass_field(default_factory=list) @@ -174,6 +137,7 @@ class Level(Enum): """ Enumeratable levels contained by the ContainerHierarchy. """ + none = 0 components = 1 units = 2 @@ -295,10 +259,12 @@ def _get_containers_with_entities( """ qs = Container.objects.none() for member in members: - qs = qs.union(content_api.get_containers_with_entity( - member.entity.pk, - ignore_pinned=ignore_pinned, - )) + qs = qs.union( + content_api.get_containers_with_entity( + member.entity.pk, + ignore_pinned=ignore_pinned, + ) + ) return qs @@ -338,6 +304,7 @@ class ContainerHierarchyMember: """ Represents an individual member of ContainerHierarchy which is ready to be serialized. """ + id: LibraryContainerLocator | LibraryUsageLocatorV2 display_name: str has_unpublished_changes: bool @@ -396,23 +363,10 @@ def library_container_locator( """ Returns a LibraryContainerLocator for the given library + container. """ - container_type = None - if hasattr(container, 'unit'): - container_type = ContainerType.Unit - elif hasattr(container, 'subsection'): - container_type = ContainerType.Subsection - elif hasattr(container, 'section'): - container_type = ContainerType.Section - else: - # This should never happen, but we assert to ensure that we handle all cases. - # If this fails, it means that a new Container type was added without updating this code. - raise ValueError(f"Unexpected container type: {container!r}") + if container.type_code not in LIBRARY_ALLOWED_CONTAINER_TYPES: + raise ValueError(f"Unsupported container type for content libraries: {container!r}") - return LibraryContainerLocator( - library_key, - container_type=container_type.value, - container_id=container.publishable_entity.key, - ) + return LibraryContainerLocator(library_key, container_type=container.type_code, container_id=container.key) def get_container_from_key(container_key: LibraryContainerLocator, include_deleted=False) -> Container: @@ -425,13 +379,27 @@ def get_container_from_key(container_key: LibraryContainerLocator, include_delet content_library = ContentLibrary.objects.get_by_key(container_key.lib_key) learning_package = content_library.learning_package assert learning_package is not None - container = content_api.get_container_by_key( - learning_package.id, - key=container_key.container_id, - ) + container = content_api.get_container_by_key(learning_package.id, key=container_key.container_id) + assert container.type_code in LIBRARY_ALLOWED_CONTAINER_TYPES # We only return the container if it exists and either: # 1. the container has a draft version (which means it is not soft-deleted) OR # 2. the container was soft-deleted but the `include_deleted` flag is set to True if container and (include_deleted or container.versioning.draft): return container raise ContentLibraryContainerNotFound + + +def get_entity_from_key( + key: LibraryContainerLocator | LibraryUsageLocatorV2, /, *, include_deleted=False +) -> PublishableEntity: + """ + Given a key for an item in a library, load it as a `Component` or a `Container` subclass. + """ + if isinstance(key, LibraryContainerLocator): + return get_container_from_key(key, include_deleted=False) + else: + assert isinstance(key, LibraryUsageLocatorV2) + component = get_component_from_usage_key(key) + if not include_deleted and not component.versioning.draft: + raise ContentLibraryContainerNotFound("Component has been deleted.") + return component diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 54740d130321..14ea88a648f8 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -1,6 +1,7 @@ """ API for containers (Sections, Subsections, Units) in Content Libraries """ + from __future__ import annotations from datetime import datetime, timezone @@ -23,20 +24,19 @@ LIBRARY_CONTAINER_UPDATED, ) from openedx_content import api as content_api -from openedx_content.models_api import Container, ContainerVersion, Component +from openedx_content.models_api import Container, Component, Unit from openedx.core.djangoapps.content_libraries.api.collections import library_collection_locator -from openedx.core.djangoapps.xblock.api import get_component_from_usage_key - from .. import tasks from ..models import ContentLibrary from .block_metadata import LibraryXBlockMetadata from .container_metadata import ( ContainerHierarchy, ContainerMetadata, - ContainerType, library_container_locator, get_container_from_key, + get_entity_from_key, + LIBRARY_ALLOWED_CONTAINER_TYPES, ) from .serializers import ContainerSerializer @@ -79,7 +79,7 @@ def get_container( associated_collections = content_api.get_entity_collections( container.publishable_entity.learning_package_id, container_key.container_id, - ).values('key', 'title') + ).values("key", "title") else: associated_collections = None container_meta = ContainerMetadata.from_container( @@ -93,7 +93,7 @@ def get_container( def create_container( library_key: LibraryLocatorV2, - container_type: ContainerType, + container_type: content_api.ContainerType, slug: str | None, title: str, user_id: int | None, @@ -104,61 +104,37 @@ def create_container( It will initially be empty. """ + assert container_type.type_code in LIBRARY_ALLOWED_CONTAINER_TYPES assert isinstance(library_key, LibraryLocatorV2) content_library = ContentLibrary.objects.get_by_key(library_key) assert content_library.learning_package_id # Should never happen but we made this a nullable field so need to check if slug is None: # Automatically generate a slug. Append a random suffix so it should be unique. - slug = slugify(title, allow_unicode=True) + '-' + uuid4().hex[-6:] + slug = slugify(title, allow_unicode=True) + "-" + uuid4().hex[-6:] # Make sure the slug is valid by first creating a key for the new container: container_key = LibraryContainerLocator( library_key, - container_type=container_type.value, + container_type=container_type.type_code, container_id=slug, ) if not created: created = datetime.now(tz=timezone.utc) - container: Container - _initial_version: ContainerVersion - # Then try creating the actual container: - match container_type: - case ContainerType.Unit: - container, _initial_version = content_api.create_unit_and_version( - content_library.learning_package_id, - key=slug, - title=title, - created=created, - created_by=user_id, - ) - case ContainerType.Subsection: - container, _initial_version = content_api.create_subsection_and_version( - content_library.learning_package_id, - key=slug, - title=title, - created=created, - created_by=user_id, - ) - case ContainerType.Section: - container, _initial_version = content_api.create_section_and_version( - content_library.learning_package_id, - key=slug, - title=title, - created=created, - created_by=user_id, - ) - case _: - raise NotImplementedError(f"Library does not support {container_type} yet") + container, _initial_version = content_api.create_container_and_version( + content_library.learning_package_id, + key=slug, + title=title, + container_type=container_type, + entities=[], + created=created, + created_by=user_id, + ) # .. event_implemented_name: LIBRARY_CONTAINER_CREATED # .. event_type: org.openedx.content_authoring.content_library.container.created.v1 - LIBRARY_CONTAINER_CREATED.send_event( - library_container=LibraryContainerData( - container_key=container_key, - ) - ) + LIBRARY_CONTAINER_CREATED.send_event(library_container=LibraryContainerData(container_key=container_key)) return ContainerMetadata.from_container(library_key, container) @@ -175,57 +151,22 @@ def update_container( library_key = container_key.lib_key created = datetime.now(tz=timezone.utc) - container_type = ContainerType(container_key.container_type) - - version: ContainerVersion affected_containers: list[ContainerMetadata] = [] # Get children containers or components to update their index data - children = get_container_children( - container_key, - published=False, - ) - child_key_name = 'container_key' - - match container_type: - case ContainerType.Unit: - version = content_api.create_next_unit_version( - container.unit, - title=display_name, - created=created, - created_by=user_id, - ) - affected_containers = get_containers_contains_item(container_key) - # Components have usage_key instead of container_key - child_key_name = 'usage_key' - case ContainerType.Subsection: - version = content_api.create_next_subsection_version( - container.subsection, - title=display_name, - created=created, - created_by=user_id, - ) - affected_containers = get_containers_contains_item(container_key) - case ContainerType.Section: - version = content_api.create_next_section_version( - container.section, - title=display_name, - created=created, - created_by=user_id, - ) + children = get_container_children(container_key, published=False) + child_key_name = "usage_key" if isinstance(container, Unit) else "container_key" - # The `affected_containers` are not obtained, because the sections are - # not contained in any container. - case _: - raise NotImplementedError(f"Library does not support {container_type} yet") + version = content_api.create_next_container_version( + container, + title=display_name, + created=created, + created_by=user_id, + ) # Send event related to the updated container # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=container_key, - ) - ) + LIBRARY_CONTAINER_UPDATED.send_event(library_container=LibraryContainerData(container_key=container_key)) # Send events related to the containers that contains the updated container. # This is to update the children display names used in the section/subsection previews. @@ -233,9 +174,7 @@ def update_container( # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=affected_container.container_key, - ) + library_container=LibraryContainerData(container_key=affected_container.container_key) ) # Update children components and containers index data, for example, # All subsections under a section have section key in index that needs to be updated. @@ -311,11 +250,10 @@ def delete_container( container_key=affected_container.container_key, ) ) - container_type = ContainerType(container_key.container_type) - key_name = 'container_key' - if container_type == ContainerType.Unit: + key_name = "container_key" + if isinstance(container, Unit): # Components have usage_key instead of container_key - key_name = 'usage_key' + key_name = "usage_key" # Update children components and containers index data, for example, # All subsections under a section have section key in index that needs to be updated. # So if parent section is deleted, it needs to be removed from sections key of children @@ -346,10 +284,7 @@ def restore_container(container_key: LibraryContainerLocator) -> None: # Fetch related containers after restore affected_containers = get_containers_contains_item(container_key) # Get children containers or components to update their index data - children = get_container_children( - container_key, - published=False, - ) + children = get_container_children(container_key, published=False) # .. event_implemented_name: LIBRARY_CONTAINER_CREATED # .. event_type: org.openedx.content_authoring.content_library.container.created.v1 @@ -398,10 +333,9 @@ def restore_container(container_key: LibraryContainerLocator) -> None: container_key=affected_container.container_key, ) ) - container_type = ContainerType(container_key.container_type) - key_name = 'container_key' - if container_type == ContainerType.Unit: - key_name = 'usage_key' + key_name = "container_key" + if content_api.get_container_type(container_key.container_type) is Unit: + key_name = "usage_key" # Update children components and containers index data, for example, # All subsections under a section have section key in index that needs to be updated. # Should restore removed parent section in sections key of children subsections @@ -426,33 +360,16 @@ def get_container_children( (e.g. the components/xblocks in a unit, units in a subsection, subsections in a section) """ container = get_container_from_key(container_key) - container_type = ContainerType(container_key.container_type) - - match container_type: - case ContainerType.Unit: - child_components = content_api.get_components_in_unit(container.unit, published=published) - return [LibraryXBlockMetadata.from_component( - container_key.lib_key, - entry.component - ) for entry in child_components] - case ContainerType.Subsection: - child_units = content_api.get_units_in_subsection(container.subsection, published=published) - return [ContainerMetadata.from_container( - container_key.lib_key, - entry.unit - ) for entry in child_units] - case ContainerType.Section: - child_subsections = content_api.get_subsections_in_section(container.section, published=published) - return [ContainerMetadata.from_container( - container_key.lib_key, - entry.subsection, - ) for entry in child_subsections] - case _: - child_entities = content_api.get_entities_in_container(container, published=published) - return [ContainerMetadata.from_container( - container_key.lib_key, - entry.entity - ) for entry in child_entities] + + child_entities = content_api.get_entities_in_container(container, published=published) + result: list[LibraryXBlockMetadata | ContainerMetadata] = [] + for entry in child_entities: + if isinstance(entry.entity, Component): + result.append(LibraryXBlockMetadata.from_component(container_key.lib_key, entry.component)) + else: + assert isinstance(entry.entity.container, Container) + result.append(ContainerMetadata.from_container(container_key.lib_key, entry.entity.container)) + return result def get_container_children_count( @@ -468,78 +385,32 @@ def get_container_children_count( def update_container_children( container_key: LibraryContainerLocator, - children_ids: list[LibraryUsageLocatorV2] | list[LibraryContainerLocator], + children_keys: list[LibraryUsageLocatorV2] | list[LibraryContainerLocator], user_id: int | None, entities_action: content_api.ChildrenEntitiesAction = content_api.ChildrenEntitiesAction.REPLACE, ): """ [ 🛑 UNSTABLE ] Adds children components or containers to given container. """ - library_key = container_key.lib_key - container_type = ContainerType(container_key.container_type) container = get_container_from_key(container_key) created = datetime.now(tz=timezone.utc) - new_version: ContainerVersion - match container_type: - case ContainerType.Unit: - components = [get_component_from_usage_key(key) for key in children_ids] # type: ignore[arg-type] - new_version = content_api.create_next_unit_version( - container.unit, - components=components, # type: ignore[arg-type] - created=created, - created_by=user_id, - entities_action=entities_action, - ) - - for key in children_ids: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(key), - changes=["units"], - ), - ) - case ContainerType.Subsection: - units = [get_container_from_key(key).unit for key in children_ids] # type: ignore[arg-type] - new_version = content_api.create_next_subsection_version( - container.subsection, - units=units, # type: ignore[arg-type] - created=created, - created_by=user_id, - entities_action=entities_action, - ) - - for key in children_ids: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(key), - changes=["subsections"], - ), - ) - case ContainerType.Section: - subsections = [get_container_from_key(key).subsection for key in children_ids] # type: ignore[arg-type] - new_version = content_api.create_next_section_version( - container.section, - subsections=subsections, # type: ignore[arg-type] - created=created, - created_by=user_id, - entities_action=entities_action, - ) - for key in children_ids: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(key), - changes=["sections"], - ), - ) - case _: - raise ValueError(f"Invalid container type: {container_type}") + new_version = content_api.create_next_container_version( + container, + created=created, + created_by=user_id, + entities=[get_entity_from_key(key) for key in children_keys], + entities_action=entities_action, + ) + for key in children_keys: + # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED + # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + content_object=ContentObjectChangedData( + object_id=str(key), + changes=[f"{container.type_code}s"], # "units", "subsections", "sections" + ), + ) # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 @@ -549,30 +420,16 @@ def update_container_children( ) ) - return ContainerMetadata.from_container(library_key, new_version.container) + return ContainerMetadata.from_container(container_key.lib_key, new_version.container) -def get_containers_contains_item( - key: LibraryUsageLocatorV2 | LibraryContainerLocator -) -> list[ContainerMetadata]: +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. """ - item: Component | Container - - if isinstance(key, LibraryUsageLocatorV2): - item = get_component_from_usage_key(key) - - elif isinstance(key, LibraryContainerLocator): - item = get_container_from_key(key) - - containers = content_api.get_containers_with_entity( - item.publishable_entity.pk, - ) - return [ - ContainerMetadata.from_container(key.lib_key, container) - for container in containers - ] + entity = get_entity_from_key(key) + containers = content_api.get_containers_with_entity(entity.pk) + return [ContainerMetadata.from_container(key.lib_key, container) for container in containers] def publish_container_changes( @@ -611,7 +468,7 @@ def copy_container(container_key: LibraryContainerLocator, user_id: int) -> User """ container_metadata = get_container(container_key) container_serializer = ContainerSerializer(container_metadata) - block_type = ContainerType(container_key.container_type).olx_tag + block_type = content_api.get_container_type(container_key.container_type).olx_tag_name from openedx.core.djangoapps.content_staging import api as content_staging_api diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index 87a5dd3e3b6f..4e326b90c49a 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -15,7 +15,6 @@ from openedx.core.djangoapps.content_libraries.tasks import LibraryRestoreTask from openedx.core.djangoapps.content_libraries import api -from openedx.core.djangoapps.content_libraries.api.containers import ContainerType from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED, LICENSE_OPTIONS from openedx.core.djangoapps.content_libraries.models import ( ContentLibrary, @@ -252,7 +251,8 @@ class LibraryContainerMetadataSerializer(PublishableItemSerializer): Converts from ContainerMetadata to JSON-compatible data """ - # Use 'source' to get this as a string, not an enum value instance which the container_type field has. + container_type_code = serializers.CharField(source="container_key.container_type") + # Deprecated - ambiguous type container_type = serializers.CharField(source="container_key.container_type") # When creating a new container in a library, the slug becomes the ID part of @@ -265,7 +265,7 @@ def to_internal_value(self, data): Returns a dictionary, not a ContainerMetadata instance. """ result = super().to_internal_value(data) - result["container_type"] = ContainerType(data["container_type"]) + del result["container_type"] # Use container_type_code instead return result