diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py index 64d8b4ab5cdd..2879325cb522 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py @@ -560,7 +560,7 @@ def test_unit_sync(self): 'single select...' ) self._add_container_children(self.upstream_unit["id"], [upstream_problem3["id"]]) - self._remove_container_components(self.upstream_unit["id"], [self.upstream_problem2["id"]]) + self._remove_container_children(self.upstream_unit["id"], [self.upstream_problem2["id"]]) self._commit_library_changes(self.library["id"]) # publish everything status = self._get_sync_status(downstream_unit["locator"]) @@ -691,7 +691,7 @@ def test_unit_sync(self): self.assertListEqual(data["results"], expected_downstreams) # 4️⃣ Now, reorder components - self._patch_container_components(self.upstream_unit["id"], [ + self._patch_container_children(self.upstream_unit["id"], [ upstream_problem3["id"], self.upstream_problem1["id"], self.upstream_html1["id"], diff --git a/openedx/core/djangoapps/content_libraries/api/container_metadata.py b/openedx/core/djangoapps/content_libraries/api/container_metadata.py index 841a51b625ff..65779fafb42b 100644 --- a/openedx/core/djangoapps/content_libraries/api/container_metadata.py +++ b/openedx/core/djangoapps/content_libraries/api/container_metadata.py @@ -3,16 +3,20 @@ """ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, field as dataclass_field from enum import Enum +from django.db.models import QuerySet -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_learning.api import authoring as authoring_api -from openedx_learning.api.authoring_models import Container +from openedx_learning.api.authoring_models import Container, Component, PublishableEntity from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts +from openedx.core.djangoapps.xblock.api import get_component_from_usage_key -from .libraries import PublishableItem +from ..models import ContentLibrary +from .exceptions import ContentLibraryContainerNotFound +from .libraries import PublishableItem, library_component_usage_key # The public API is only the following symbols: __all__ = [ @@ -88,7 +92,7 @@ def from_container(cls, library_key, container: Container, associated_collection container=container, ) container_type = ContainerType(container_key.container_type) - published_by = "" + published_by = None if last_publish_log and last_publish_log.published_by: published_by = last_publish_log.published_by.username @@ -121,6 +125,240 @@ def from_container(cls, library_key, container: Container, associated_collection ) +@dataclass(frozen=True, kw_only=True) +class ContainerHierarchy: + """ + Describes the full ancestry and descendents of a given library object. + + TODO: We intend to replace this implementation with a more efficient one that makes fewer + 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) + components: list[ContainerHierarchyMember] = dataclass_field(default_factory=list) + object_key: LibraryUsageLocatorV2 | LibraryContainerLocator + + class Level(Enum): + """ + Enumeratable levels contained by the ContainerHierarchy. + """ + none = 0 + components = 1 + units = 2 + subsections = 3 + sections = 4 + + def __bool__(self) -> bool: + """ + Level.none is False + All others are True. + """ + return self != ContainerHierarchy.Level.none + + @property + def parent(self) -> ContainerHierarchy.Level: + """ + Returns the parent level above the given level, + or Level.none if this is already the top level. + """ + if not self: + return self + try: + return ContainerHierarchy.Level(self.value + 1) + except ValueError: + return ContainerHierarchy.Level.none + + @property + def child(self) -> ContainerHierarchy.Level: + """ + Returns the name of the child field below the given level, + or None if level is already the lowest level. + """ + if not self: + return self + try: + return ContainerHierarchy.Level(self.value - 1) + except ValueError: + return ContainerHierarchy.Level.none + + def append( + self, + level: Level, + *items: Component | Container, + ) -> list[ContainerHierarchyMember]: + """ + Appends the metadata for the given items to the given level of the hierarchy. + Returns the resulting list. + + Arguments: + * level: a valid Level (not Level.none) + * ...list of Components or Containers to add to this level. + """ + assert level + for item in items: + getattr(self, level.name).append( + ContainerHierarchyMember.create( + self.object_key.context_key, + item, + ) + ) + + return getattr(self, level.name) + + @classmethod + def create_from_library_object_key( + cls, + object_key: LibraryUsageLocatorV2 | LibraryContainerLocator, + ): + """ + Returns a ContainerHierarchy populated from the library object represented by the given object_key. + """ + root_items: list[Component] | list[Container] + root_level: ContainerHierarchy.Level + + if isinstance(object_key, LibraryUsageLocatorV2): + root_items = [get_component_from_usage_key(object_key)] + root_level = ContainerHierarchy.Level.components + + elif isinstance(object_key, LibraryContainerLocator): + root_items = [get_container_from_key(object_key)] + root_level = ContainerHierarchy.Level[f"{object_key.container_type}s"] + + if not root_level: + raise TypeError(f"Unexpected '{object_key}': must be LibraryUsageLocatorv2 or LibraryContainerLocator") + + # Fill in root level of hierarchy + hierarchy = cls(object_key=object_key) + root_members = hierarchy.append(root_level, *root_items) + + # Fill in hierarchy up through parents + level = root_level + members = root_members + while level := level.parent: + items = list(_get_containers_with_entities(members).all()) + members = hierarchy.append(level, *items) + + # Fill in hierarchy down from root_level. + if root_level != cls.Level.components: # Components have no children + level = root_level + members = root_members + while level := level.child: + children = _get_containers_children(level, members) + members = hierarchy.append(level, *children) + + return hierarchy + + +def _get_containers_with_entities( + members: list[ContainerHierarchyMember], + *, + ignore_pinned=False, +) -> QuerySet[Container]: + """ + Find all draft containers that directly contain the given entities. + + Args: + entities: iterable list or queryset of PublishableEntities. + ignore_pinned: if true, ignore any pinned references to the entity. + """ + qs = Container.objects.none() + for member in members: + qs = qs.union(authoring_api.get_containers_with_entity( + member.entity.pk, + ignore_pinned=ignore_pinned, + )) + return qs + + +def _get_containers_children( + level: ContainerHierarchy.Level, + members: list[ContainerHierarchyMember], + *, + published=False, +) -> list[Component | Container]: + """ + Find all components or containers directly contained by the given hierarchy members. + + Args: + containers: iterable list or queryset of Containers of the same type. + published: `True` if we want the published version of the children, or + `False` for the draft version. + """ + children: list[Component | Container] = [] + for member in members: + container = member.container + assert container + for entry in authoring_api.get_entities_in_container( + container, + published=published, + ): + match level: + case ContainerHierarchy.Level.components: + children.append(entry.entity_version.componentversion.component) + case _: + children.append(entry.entity_version.containerversion.container) + + return children + + +@dataclass(frozen=True, kw_only=True) +class ContainerHierarchyMember: + """ + Represents an individual member of ContainerHierarchy which is ready to be serialized. + """ + id: LibraryContainerLocator | LibraryUsageLocatorV2 + display_name: str + has_unpublished_changes: bool + component: Component | None + container: Container | None + + @classmethod + def create( + cls, + library_key: LibraryLocatorV2, + entity: Container | Component, + ) -> ContainerHierarchyMember: + """ + Creates a ContainerHierarchyMember. + + Arguments: + * library_key: required for generating a usage/locator key for the given entitity. + * entity: the Container or Component + """ + if isinstance(entity, Component): + return ContainerHierarchyMember( + id=library_component_usage_key(library_key, entity), + display_name=entity.versioning.draft.title, + has_unpublished_changes=entity.versioning.has_unpublished_changes, + component=entity, + container=None, + ) + assert isinstance(entity, Container) + return ContainerHierarchyMember( + id=library_container_locator( + library_key, + container=entity, + ), + display_name=entity.versioning.draft.title, + has_unpublished_changes=authoring_api.contains_unpublished_changes(entity.pk), + container=entity, + component=None, + ) + + @property + def entity(self) -> PublishableEntity: + """ + Returns the PublishableEntity associated with this member. + + Raises AssertError if there isn't a Component or Container set. + """ + entity = self.component or self.container + assert entity + return entity.publishable_entity + + def library_container_locator( library_key: LibraryLocatorV2, container: Container, @@ -145,3 +383,25 @@ def library_container_locator( container_type=container_type.value, container_id=container.publishable_entity.key, ) + + +def get_container_from_key(container_key: LibraryContainerLocator, include_deleted=False) -> Container: + """ + Internal method to fetch the Container object from its LibraryContainerLocator + + Raises ContentLibraryContainerNotFound if no container found, or if the container has been soft deleted. + """ + assert isinstance(container_key, LibraryContainerLocator) + content_library = ContentLibrary.objects.get_by_key(container_key.lib_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, + ) + # 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 diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 788e9116b7e1..28f05a05723a 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -28,19 +28,25 @@ 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 ContainerMetadata, ContainerType, library_container_locator -from .exceptions import ContentLibraryContainerNotFound +from .container_metadata import ( + ContainerHierarchy, + ContainerMetadata, + ContainerType, + library_container_locator, + get_container_from_key, +) from .serializers import ContainerSerializer -from .. import tasks if typing.TYPE_CHECKING: from openedx.core.djangoapps.content_staging.api import UserClipboardData -# The public API is only the following symbols: +# 🛑 UNSTABLE: All APIs related to containers are unstable until we've figured +# out our approach to dynamic content (randomized, A/B tests, etc.) __all__ = [ "get_container", "create_container", @@ -52,6 +58,7 @@ "update_container_children", "get_containers_contains_item", "publish_container_changes", + "get_library_object_hierarchy", "copy_container", "library_container_locator", ] @@ -59,34 +66,15 @@ log = logging.getLogger(__name__) -def _get_container_from_key(container_key: LibraryContainerLocator, isDeleted=False) -> Container: - """ - Internal method to fetch the Container object from its LibraryContainerLocator - - Raises ContentLibraryContainerNotFound if no container found, or if the container has been soft deleted. - """ - assert isinstance(container_key, LibraryContainerLocator) - content_library = ContentLibrary.objects.get_by_key(container_key.lib_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, - ) - if container and (isDeleted or container.versioning.draft): - return container - raise ContentLibraryContainerNotFound - - def get_container( container_key: LibraryContainerLocator, *, include_collections=False, ) -> ContainerMetadata: """ - Get a container (a Section, Subsection, or Unit). + [ 🛑 UNSTABLE ] Get a container (a Section, Subsection, or Unit). """ - container = _get_container_from_key(container_key) + container = get_container_from_key(container_key) if include_collections: associated_collections = authoring_api.get_entity_collections( container.publishable_entity.learning_package_id, @@ -112,7 +100,7 @@ def create_container( created: datetime | None = None, ) -> ContainerMetadata: """ - Create a container (a Section, Subsection, or Unit) in the specified content library. + [ 🛑 UNSTABLE ] Create a container (a Section, Subsection, or Unit) in the specified content library. It will initially be empty. """ @@ -181,9 +169,9 @@ def update_container( user_id: int | None, ) -> ContainerMetadata: """ - Update a container (a Section, Subsection, or Unit) title. + [ 🛑 UNSTABLE ] Update a container (a Section, Subsection, or Unit) title. """ - container = _get_container_from_key(container_key) + container = get_container_from_key(container_key) library_key = container_key.lib_key created = datetime.now(tz=timezone.utc) @@ -269,12 +257,12 @@ def delete_container( container_key: LibraryContainerLocator, ) -> None: """ - Delete a container (a Section, Subsection, or Unit) (soft delete). + [ 🛑 UNSTABLE ] Delete a container (a Section, Subsection, or Unit) (soft delete). No-op if container doesn't exist or has already been soft-deleted. """ library_key = container_key.lib_key - container = _get_container_from_key(container_key) + container = get_container_from_key(container_key) # Fetch related collections and containers before soft-delete affected_collections = authoring_api.get_entity_collections( @@ -344,10 +332,10 @@ def delete_container( def restore_container(container_key: LibraryContainerLocator) -> None: """ - Restore the specified library container. + [ 🛑 UNSTABLE ] Restore the specified library container. """ library_key = container_key.lib_key - container = _get_container_from_key(container_key, isDeleted=True) + container = get_container_from_key(container_key, include_deleted=True) affected_collections = authoring_api.get_entity_collections( container.publishable_entity.learning_package_id, @@ -434,10 +422,10 @@ def get_container_children( published=False, ) -> list[LibraryXBlockMetadata | ContainerMetadata]: """ - Get the entities contained in the given container + [ 🛑 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) """ - container = _get_container_from_key(container_key) + container = get_container_from_key(container_key) container_type = ContainerType(container_key.container_type) match container_type: @@ -472,9 +460,9 @@ def get_container_children_count( published=False, ) -> int: """ - Get the count of entities contained in the given container (e.g. the components/xblocks in a unit) + [ 🛑 UNSTABLE ] Get the count of entities contained in the given container (e.g. the components/xblocks in a unit) """ - container = _get_container_from_key(container_key) + container = get_container_from_key(container_key) return authoring_api.get_container_children_count(container, published=published) @@ -485,11 +473,11 @@ def update_container_children( entities_action: authoring_api.ChildrenEntitiesAction = authoring_api.ChildrenEntitiesAction.REPLACE, ): """ - Adds children components or containers to given container. + [ 🛑 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) + container = get_container_from_key(container_key) created = datetime.now(tz=timezone.utc) new_version: ContainerVersion match container_type: @@ -513,7 +501,7 @@ def update_container_children( ), ) case ContainerType.Subsection: - units = [_get_container_from_key(key).unit for key in children_ids] # type: ignore[arg-type] + units = [get_container_from_key(key).unit for key in children_ids] # type: ignore[arg-type] new_version = authoring_api.create_next_subsection_version( container.subsection, units=units, # type: ignore[arg-type] @@ -532,7 +520,7 @@ def update_container_children( ), ) case ContainerType.Section: - subsections = [_get_container_from_key(key).subsection for key in children_ids] # type: ignore[arg-type] + subsections = [get_container_from_key(key).subsection for key in children_ids] # type: ignore[arg-type] new_version = authoring_api.create_next_section_version( container.section, subsections=subsections, # type: ignore[arg-type] @@ -568,8 +556,7 @@ def get_containers_contains_item( key: LibraryUsageLocatorV2 | LibraryContainerLocator ) -> list[ContainerMetadata]: """ - Get containers that contains the item, - that can be a component or another container. + [ 🛑 UNSTABLE ] Get containers that contains the item, that can be a component or another container. """ item: Component | Container @@ -577,7 +564,7 @@ def get_containers_contains_item( item = get_component_from_usage_key(key) elif isinstance(key, LibraryContainerLocator): - item = _get_container_from_key(key) + item = get_container_from_key(key) containers = authoring_api.get_containers_with_entity( item.publishable_entity.pk, @@ -590,10 +577,10 @@ def get_containers_contains_item( def publish_container_changes(container_key: LibraryContainerLocator, user_id: int | None) -> None: """ - Publish all unpublished changes in a container and all its child + [ 🛑 UNSTABLE ] Publish all unpublished changes in a container and all its child containers/blocks. """ - container = _get_container_from_key(container_key) + container = get_container_from_key(container_key) library_key = container_key.lib_key content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] learning_package = content_library.learning_package @@ -613,7 +600,7 @@ def publish_container_changes(container_key: LibraryContainerLocator, user_id: i def copy_container(container_key: LibraryContainerLocator, user_id: int) -> UserClipboardData: """ - Copy a container (a Section, Subsection, or Unit) to the content staging. + [ 🛑 UNSTABLE ] Copy a container (a Section, Subsection, or Unit) to the content staging. """ container_metadata = get_container(container_key) container_serializer = ContainerSerializer(container_metadata) @@ -632,3 +619,16 @@ def copy_container(container_key: LibraryContainerLocator, user_id: int) -> User version_num=container_metadata.published_version_num, static_files=container_serializer.static_files, ) + + +def get_library_object_hierarchy( + object_key: LibraryUsageLocatorV2 | LibraryContainerLocator, +) -> ContainerHierarchy: + """ + [ 🛑 UNSTABLE ] Returns the full ancestry and descendents of the library object with the given object_key. + + TODO: We intend to replace this implementation with a more efficient one that makes fewer + database queries in the future. More details being discussed in + https://github.com/openedx/edx-platform/pull/36813#issuecomment-3136631767 + """ + return ContainerHierarchy.create_from_library_object_key(object_key) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index 293f9761b809..ff90c6972523 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -194,7 +194,7 @@ class PublishableItem(LibraryItem): published_display_name: str | None last_published: datetime | None = None # The username of the user who last published this. - published_by: str = "" + published_by: str | None = "" last_draft_created: datetime | None = None # The username of the user who created the last draft. last_draft_created_by: str = "" diff --git a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py index bc314099893c..b93b48e5ad86 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py @@ -18,14 +18,7 @@ from rest_framework.views import APIView from openedx.core.djangoapps.content_libraries import api, permissions -from openedx.core.djangoapps.content_libraries.rest_api.serializers import ( - ContentLibraryItemCollectionsUpdateSerializer, - LibraryXBlockCreationSerializer, - LibraryXBlockMetadataSerializer, - LibraryXBlockOlxSerializer, - LibraryXBlockStaticFileSerializer, - LibraryXBlockStaticFilesSerializer, -) +from openedx.core.djangoapps.content_libraries.rest_api import serializers from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.lib.api.view_utils import view_auth_classes from openedx.core.types.http import RestRequest @@ -40,7 +33,7 @@ class LibraryBlocksView(GenericAPIView): """ Views to work with XBlocks in a specific content library. """ - serializer_class = LibraryXBlockMetadataSerializer + serializer_class = serializers.LibraryXBlockMetadataSerializer @apidocs.schema( parameters=[ @@ -74,13 +67,13 @@ def get(self, request, lib_key_str): api.LibraryXBlockMetadata.from_component(key, component) for component in self.paginate_queryset(components) ] - serializer = LibraryXBlockMetadataSerializer(paginated_xblock_metadata, many=True) + serializer = self.serializer_class(paginated_xblock_metadata, many=True) return self.get_paginated_response(serializer.data) @convert_exceptions @swagger_auto_schema( - request_body=LibraryXBlockCreationSerializer, - responses={200: LibraryXBlockMetadataSerializer} + request_body=serializers.LibraryXBlockCreationSerializer, + responses={200: serializers.LibraryXBlockMetadataSerializer} ) def post(self, request, lib_key_str): """ @@ -88,7 +81,7 @@ def post(self, request, lib_key_str): """ library_key = LibraryLocatorV2.from_string(lib_key_str) api.require_permission_for_library_key(library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY) - serializer = LibraryXBlockCreationSerializer(data=request.data) + serializer = serializers.LibraryXBlockCreationSerializer(data=request.data) serializer.is_valid(raise_exception=True) # Create a new regular top-level block: @@ -99,7 +92,7 @@ def post(self, request, lib_key_str): detail={'block_type': str(err)}, ) - return Response(LibraryXBlockMetadataSerializer(result).data) + return Response(self.serializer_class(result).data) @method_decorator(non_atomic_requests, name="dispatch") @@ -108,6 +101,8 @@ class LibraryBlockView(APIView): """ Views to work with an existing XBlock in a content library. """ + serializer_class = serializers.LibraryXBlockMetadataSerializer + @convert_exceptions def get(self, request, usage_key_str): """ @@ -123,7 +118,7 @@ def get(self, request, usage_key_str): api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY) result = api.get_library_block(key, include_collections=True) - return Response(LibraryXBlockMetadataSerializer(result).data) + return Response(self.serializer_class(result).data) @convert_exceptions def delete(self, request, usage_key_str): # pylint: disable=unused-argument @@ -150,6 +145,8 @@ class LibraryBlockAssetListView(APIView): """ Views to list an existing XBlock's static asset files """ + serializer_class = serializers.LibraryXBlockStaticFilesSerializer + @convert_exceptions def get(self, request, usage_key_str): """ @@ -158,7 +155,7 @@ def get(self, request, usage_key_str): key = LibraryUsageLocatorV2.from_string(usage_key_str) api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY) files = api.get_library_block_static_asset_files(key) - return Response(LibraryXBlockStaticFilesSerializer({"files": files}).data) + return Response(self.serializer_class({"files": files}).data) @method_decorator(non_atomic_requests, name="dispatch") @@ -168,6 +165,7 @@ class LibraryBlockAssetView(APIView): Views to work with an existing XBlock's static asset files """ parser_classes = (MultiPartParser, ) + serializer_class = serializers.LibraryXBlockStaticFileSerializer @convert_exceptions def get(self, request, usage_key_str, file_path): @@ -179,7 +177,7 @@ def get(self, request, usage_key_str, file_path): files = api.get_library_block_static_asset_files(key) for f in files: if f.path == file_path: - return Response(LibraryXBlockStaticFileSerializer(f).data) + return Response(self.serializer_class(f).data) raise NotFound @convert_exceptions @@ -206,7 +204,7 @@ def put(self, request, usage_key_str, file_path): result = api.add_library_block_static_asset_file(usage_key, file_path, file_content, request.user) except ValueError: raise ValidationError("Invalid file path") # lint-amnesty, pylint: disable=raise-missing-from - return Response(LibraryXBlockStaticFileSerializer(result).data) + return Response(self.serializer_class(result).data) @convert_exceptions def delete(self, request, usage_key_str, file_path): @@ -265,7 +263,7 @@ def patch(self, request: RestRequest, usage_key_str) -> Response: request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY ) - serializer = ContentLibraryItemCollectionsUpdateSerializer(data=request.data) + serializer = serializers.ContentLibraryItemCollectionsUpdateSerializer(data=request.data) serializer.is_valid(raise_exception=True) component = api.get_component_from_usage_key(key) @@ -309,6 +307,8 @@ class LibraryBlockOlxView(APIView): """ Views to work with an existing XBlock's OLX """ + serializer_class = serializers.LibraryXBlockOlxSerializer + @convert_exceptions def get(self, request, usage_key_str): """ @@ -320,7 +320,7 @@ def get(self, request, usage_key_str): key = LibraryUsageLocatorV2.from_string(usage_key_str) api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY) xml_str = xblock_api.get_block_draft_olx(key) - return Response(LibraryXBlockOlxSerializer({"olx": xml_str}).data) + return Response(self.serializer_class({"olx": xml_str}).data) @convert_exceptions def post(self, request, usage_key_str): @@ -332,14 +332,14 @@ def post(self, request, usage_key_str): """ key = LibraryUsageLocatorV2.from_string(usage_key_str) api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY) - serializer = LibraryXBlockOlxSerializer(data=request.data) + serializer = self.serializer_class(data=request.data) 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 except ValueError as err: raise ValidationError(detail=str(err)) # lint-amnesty, pylint: disable=raise-missing-from - return Response(LibraryXBlockOlxSerializer({"olx": new_olx_str, "version_num": version_num}).data) + return Response(self.serializer_class({"olx": new_olx_str, "version_num": version_num}).data) @view_auth_classes() @@ -358,6 +358,25 @@ def post(self, request, usage_key_str) -> Response: return Response(None, status=status.HTTP_204_NO_CONTENT) +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryBlockHierarchy(GenericAPIView): + """ + View to return the full hierarchy of containers that contain a library block. + """ + serializer_class = serializers.ContainerHierarchySerializer + + @convert_exceptions + def get(self, request, usage_key_str) -> Response: + """ + Fetches and returns the full container hierarchy for the given library block. + """ + key = LibraryUsageLocatorV2.from_string(usage_key_str) + api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY) + hierarchy = api.get_library_object_hierarchy(key) + return Response(self.serializer_class(hierarchy).data) + + def get_component_version_asset(request, component_version_uuid, asset_path): """ Serves static assets associated with particular Component versions. diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 4289fbdf4fef..67070a0a82f9 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -410,3 +410,28 @@ def post(self, request: RestRequest, container_key: LibraryContainerLocator) -> ) return Response({}) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainerHierarchy(GenericAPIView): + """ + View to return the full hierarchy of containers that contain and are contained by a library container. + """ + serializer_class = serializers.ContainerHierarchySerializer + + @convert_exceptions + @swagger_auto_schema( + responses={200: serializers.ContainerHierarchySerializer} + ) + def get(self, request: RestRequest, container_key: LibraryContainerLocator) -> Response: + """ + Fetches and returns the full container hierarchy for the given library block. + """ + api.require_permission_for_library_key( + container_key.lib_key, + request.user, + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, + ) + hierarchy = api.get_library_object_hierarchy(container_key) + return Response(self.serializer_class(hierarchy).data) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index 86573d827fe2..9cdbe4390194 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -395,3 +395,24 @@ class UnionLibraryMetadataSerializer(serializers.Serializer): type_a = LibraryXBlockMetadataSerializer(many=True, required=False) type_b = LibraryContainerMetadataSerializer(many=True, required=False) + + +class ContainerHierarchyMemberSerializer(serializers.Serializer): + """ + Serializer for the members of a hierarchy, which can be either Components or Containers. + """ + id = OpaqueKeySerializer() + display_name = serializers.CharField() + has_unpublished_changes = serializers.BooleanField() + + +class ContainerHierarchySerializer(serializers.Serializer): + """ + Serializer which represents the full hierarchy of containers and components that contain and are contained by a + given library container or library block. + """ + sections = serializers.ListField(child=ContainerHierarchyMemberSerializer(), allow_empty=True) + subsections = serializers.ListField(child=ContainerHierarchyMemberSerializer(), allow_empty=True) + units = serializers.ListField(child=ContainerHierarchyMemberSerializer(), allow_empty=True) + components = serializers.ListField(child=ContainerHierarchyMemberSerializer(), allow_empty=True) + object_key = OpaqueKeySerializer() diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index f3ae4749e523..1c1bf1b1373b 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -37,8 +37,10 @@ URL_LIB_BLOCK_OLX = URL_LIB_BLOCK + 'olx/' # Get or set the OLX of the specified XBlock URL_LIB_BLOCK_ASSETS = URL_LIB_BLOCK + 'assets/' # List the static asset files of the specified XBlock URL_LIB_BLOCK_ASSET_FILE = URL_LIB_BLOCK + 'assets/{file_name}' # Get, delete, or upload a specific static asset file +URL_LIB_BLOCK_HIERARCHY = URL_LIB_BLOCK + 'hierarchy/' # Get a library block's full hierarchy URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a container in this library URL_LIB_CONTAINER_CHILDREN = URL_LIB_CONTAINER + 'children/' # Get, add or delete a component in this container +URL_LIB_CONTAINER_HIERARCHY = URL_LIB_CONTAINER + 'hierarchy/' # Get a container's full hierarchy URL_LIB_CONTAINER_RESTORE = URL_LIB_CONTAINER + 'restore/' # Restore a deleted container URL_LIB_CONTAINER_COLLECTIONS = URL_LIB_CONTAINER + 'collections/' # Handle associated collections URL_LIB_CONTAINER_PUBLISH = URL_LIB_CONTAINER + 'publish/' # Publish changes to the specified container + children @@ -420,13 +422,13 @@ def _add_container_children( expect_response ) - def _remove_container_components( + def _remove_container_children( self, container_key: ContainerKey | str, children_ids: list[str], expect_response=200, ): - """ Remove container components""" + """ Remove container children""" return self._api( 'delete', URL_LIB_CONTAINER_CHILDREN.format(container_key=container_key), @@ -434,13 +436,13 @@ def _remove_container_components( expect_response ) - def _patch_container_components( + def _patch_container_children( self, container_key: ContainerKey | str, children_ids: list[str], expect_response=200, ): - """ Update container components""" + """ Update container children""" return self._api( 'patch', URL_LIB_CONTAINER_CHILDREN.format(container_key=container_key), @@ -470,6 +472,27 @@ def _copy_container(self, container_key: ContainerKey | str, expect_response=200 """ Copy the specified container to the clipboard """ return self._api('post', URL_LIB_CONTAINER_COPY.format(container_key=container_key), None, expect_response) + @staticmethod + def _hierarchy_member(obj) -> dict: + """ + Returns the subset of metadata fields used by the container hierarchy. + """ + return { + "id": obj["id"], + "display_name": obj["display_name"], + "has_unpublished_changes": obj["has_unpublished_changes"], + } + + def _get_block_hierarchy(self, block_key, expect_response=200): + """ Returns the hierarchy of containers that contain the given block """ + url = URL_LIB_BLOCK_HIERARCHY.format(block_key=block_key) + return self._api('get', url, None, expect_response) + + def _get_container_hierarchy(self, container_key, expect_response=200): + """ Returns the hierarchy of containers that contain and are contained by the given container """ + url = URL_LIB_CONTAINER_HIERARCHY.format(container_key=container_key) + return self._api('get', url, None, expect_response) + def _create_collection( self, lib_key: LibraryLocatorV2 | str, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 807054883667..8b7b0c527381 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -40,6 +40,7 @@ class ContainersTestCase(ContentLibrariesRestApiTest): def setUp(self) -> None: super().setUp() self.create_date = datetime(2024, 9, 8, 7, 6, 5, tzinfo=timezone.utc) + self.modified_date = datetime(2024, 10, 9, 8, 7, 6, tzinfo=timezone.utc) self.lib = self._create_library( slug="containers", title="Container Test Library", @@ -108,36 +109,45 @@ def setUp(self) -> None: self.problem_block_2 = self._add_block_to_library(self.lib["id"], "problem", "Problem2", can_stand_alone=False) self.html_block_2 = self._add_block_to_library(self.lib["id"], "html", "Html2") - # Add components to `unit_with_components` - self._add_container_children( - self.unit_with_components["id"], - children_ids=[ - self.problem_block["id"], - self.html_block["id"], - self.problem_block_2["id"], - self.html_block_2["id"], - ], - ) - # Add units to `subsection_with_units` - self._add_container_children( - self.subsection_with_units["id"], - children_ids=[ - self.unit["id"], + with freeze_time(self.modified_date): + # Add components to `unit_with_components` + self._add_container_children( self.unit_with_components["id"], - self.unit_2["id"], - self.unit_3["id"], - ], - ) - # Add subsections to `section_with_subsections` - self._add_container_children( - self.section_with_subsections["id"], - children_ids=[ - self.subsection["id"], + children_ids=[ + self.problem_block["id"], + self.html_block["id"], + self.problem_block_2["id"], + self.html_block_2["id"], + ], + ) + # Refetch to update modified dates + self.unit_with_components = self._get_container(self.unit_with_components["id"]) + + # Add units to `subsection_with_units` + self._add_container_children( self.subsection_with_units["id"], - self.subsection_2["id"], - self.subsection_3["id"], - ], - ) + children_ids=[ + self.unit["id"], + self.unit_with_components["id"], + self.unit_2["id"], + self.unit_3["id"], + ], + ) + # Refetch to update modified dates + self.subsection_with_units = self._get_container(self.subsection_with_units["id"]) + + # Add subsections to `section_with_subsections` + self._add_container_children( + self.section_with_subsections["id"], + children_ids=[ + self.subsection["id"], + self.subsection_with_units["id"], + self.subsection_2["id"], + self.subsection_3["id"], + ], + ) + # Refetch to update modified dates + self.section_with_subsections = self._get_container(self.section_with_subsections["id"]) @ddt.data( ("unit", "u1", "Test Unit"), @@ -164,7 +174,7 @@ def test_container_crud(self, container_type, slug, display_name) -> None: "container_type": container_type, "display_name": display_name, "last_published": None, - "published_by": "", + "published_by": None, "last_draft_created": "2024-09-08T07:06:05Z", "last_draft_created_by": 'Bob', 'has_unpublished_changes': True, @@ -380,7 +390,7 @@ def test_container_remove_children(self, container_name, items_to_remove, expect data = self._get_container_children(container["id"]) assert len(data) == 4 # Remove items. - self._remove_container_components( + self._remove_container_children( container["id"], children_ids=[item_to_remove_1["id"], item_to_remove_2["id"]] ) @@ -401,7 +411,7 @@ def test_unit_replace_children(self) -> None: assert data[3]['id'] == self.html_block_2['id'] # Reorder the components - self._patch_container_components( + self._patch_container_children( self.unit_with_components["id"], children_ids=[ self.problem_block["id"], @@ -420,7 +430,7 @@ def test_unit_replace_children(self) -> None: # Replace with new components new_problem_block = self._add_block_to_library(self.lib["id"], "problem", "New_Problem", can_stand_alone=False) new_html_block = self._add_block_to_library(self.lib["id"], "html", "New_Html", can_stand_alone=False) - self._patch_container_components( + self._patch_container_children( self.unit_with_components["id"], children_ids=[new_problem_block["id"], new_html_block["id"]], ) @@ -441,7 +451,7 @@ def test_subsection_replace_children(self) -> None: assert data[3]['id'] == self.unit_3['id'] # Reorder the units - self._patch_container_components( + self._patch_container_children( self.subsection_with_units["id"], children_ids=[ self.unit_2["id"], @@ -460,7 +470,7 @@ def test_subsection_replace_children(self) -> None: # Replace with new units new_unit_1 = self._create_container(self.lib["id"], "unit", display_name="New Unit 1", slug=None) new_unit_2 = self._create_container(self.lib["id"], "unit", display_name="New Unit 2", slug=None) - self._patch_container_components( + self._patch_container_children( self.subsection_with_units["id"], children_ids=[new_unit_1["id"], new_unit_2["id"]], ) @@ -481,7 +491,7 @@ def test_section_replace_children(self) -> None: assert data[3]['id'] == self.subsection_3['id'] # Reorder the subsections - self._patch_container_components( + self._patch_container_children( self.section_with_subsections["id"], children_ids=[ self.subsection_2["id"], @@ -510,7 +520,7 @@ def test_section_replace_children(self) -> None: display_name="New Subsection 2", slug=None, ) - self._patch_container_components( + self._patch_container_children( self.section_with_subsections["id"], children_ids=[new_subsection_1["id"], new_subsection_2["id"]], ) @@ -541,7 +551,7 @@ def test_restore_containers(self, container_type) -> None: "container_type": container_type, "display_name": container["display_name"], "last_published": None, - "published_by": "", + "published_by": None, "last_draft_created": "2024-09-08T07:06:05Z", "last_draft_created_by": 'Bob', 'has_unpublished_changes': True, @@ -570,7 +580,7 @@ def test_tag_containers(self, container_type) -> None: new_container_data = self._get_container(container["id"]) assert new_container_data["tags_count"] == 3 - def test_container_collections(self) -> None: + def test_unit_collections(self) -> None: # Create a collection col1 = api.create_library_collection( self.lib_key, @@ -593,9 +603,128 @@ def test_container_collections(self) -> None: # Verify the collections assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.key}] - def test_publish_container(self) -> None: # pylint: disable=too-many-statements + def test_section_hierarchy(self): + with self.assertNumQueries(133): + hierarchy = self._get_container_hierarchy(self.section_with_subsections["id"]) + assert hierarchy["object_key"] == self.section_with_subsections["id"] + assert hierarchy["components"] == [ + self._hierarchy_member(self.problem_block), + self._hierarchy_member(self.html_block), + self._hierarchy_member(self.problem_block_2), + self._hierarchy_member(self.html_block_2), + ] + assert hierarchy["units"] == [ + self._hierarchy_member(self.unit), + self._hierarchy_member(self.unit_with_components), + self._hierarchy_member(self.unit_2), + self._hierarchy_member(self.unit_3), + ] + assert hierarchy["subsections"] == [ + self._hierarchy_member(self.subsection), + self._hierarchy_member(self.subsection_with_units), + self._hierarchy_member(self.subsection_2), + self._hierarchy_member(self.subsection_3), + ] + assert hierarchy["sections"] == [ + self._hierarchy_member(self.section_with_subsections), + ] + + def test_subsection_hierarchy(self): + with self.assertNumQueries(93): + hierarchy = self._get_container_hierarchy(self.subsection_with_units["id"]) + assert hierarchy["object_key"] == self.subsection_with_units["id"] + assert hierarchy["components"] == [ + self._hierarchy_member(self.problem_block), + self._hierarchy_member(self.html_block), + self._hierarchy_member(self.problem_block_2), + self._hierarchy_member(self.html_block_2), + ] + assert hierarchy["units"] == [ + self._hierarchy_member(self.unit), + self._hierarchy_member(self.unit_with_components), + self._hierarchy_member(self.unit_2), + self._hierarchy_member(self.unit_3), + ] + assert hierarchy["subsections"] == [ + self._hierarchy_member(self.subsection_with_units), + ] + assert hierarchy["sections"] == [ + self._hierarchy_member(self.section_with_subsections), + ] + + def test_units_hierarchy(self): + with self.assertNumQueries(56): + hierarchy = self._get_container_hierarchy(self.unit_with_components["id"]) + assert hierarchy["object_key"] == self.unit_with_components["id"] + assert hierarchy["components"] == [ + self._hierarchy_member(self.problem_block), + self._hierarchy_member(self.html_block), + self._hierarchy_member(self.problem_block_2), + self._hierarchy_member(self.html_block_2), + ] + assert hierarchy["units"] == [ + self._hierarchy_member(self.unit_with_components), + ] + assert hierarchy["subsections"] == [ + self._hierarchy_member(self.subsection_with_units), + ] + assert hierarchy["sections"] == [ + self._hierarchy_member(self.section_with_subsections), + ] + + def test_container_hierarchy_not_found(self): + self._get_container_hierarchy( + "lct:CL-TEST:containers:section:does-not-exist", + expect_response=404, + ) + + def test_block_hierarchy(self): + with self.assertNumQueries(21): + hierarchy = self._get_block_hierarchy(self.problem_block["id"]) + assert hierarchy["object_key"] == self.problem_block["id"] + assert hierarchy["components"] == [ + self._hierarchy_member(self.problem_block), + ] + assert hierarchy["units"] == [ + self._hierarchy_member(self.unit_with_components), + ] + assert hierarchy["subsections"] == [ + self._hierarchy_member(self.subsection_with_units), + ] + assert hierarchy["sections"] == [ + self._hierarchy_member(self.section_with_subsections), + ] + + def test_block_hierarchy_not_found(self): + self._get_block_hierarchy( + "lb:CL-TEST:problem:does-not-exist", + expect_response=404, + ) + + def _verify_publish_state( + self, + container_id, + expected_childen_len, + expected_has_unpublished_changes, + expected_published_by, + expected_children_ids, + ) -> None: + """ + Verify the publish state of a container + """ + container = self._get_container(container_id) + assert container["has_unpublished_changes"] == expected_has_unpublished_changes + container_children = self._get_container_children(container_id) + assert len(container_children) == expected_childen_len + + for index in range(expected_childen_len): + assert container_children[index]["id"] == expected_children_ids[index] + assert container_children[index]["has_unpublished_changes"] == expected_has_unpublished_changes + assert container_children[index]["published_by"] == expected_published_by + + def test_publish_unit(self) -> None: """ - Test that we can publish the changes to a specific container + Test that we can publish the changes to a specific unit """ html_block_3 = self._add_block_to_library(self.lib["id"], "html", "Html3") self._add_container_children( @@ -607,53 +736,53 @@ def test_publish_container(self) -> None: # pylint: disable=too-many-statements ) # At first everything is unpublished: - c1_before = self._get_container(self.unit_with_components["id"]) - assert c1_before["has_unpublished_changes"] - c1_components_before = self._get_container_children(self.unit_with_components["id"]) - assert len(c1_components_before) == 4 - assert c1_components_before[0]["id"] == self.problem_block["id"] - assert c1_components_before[0]["has_unpublished_changes"] - assert c1_components_before[0]["published_by"] is None - assert c1_components_before[1]["id"] == self.html_block["id"] - assert c1_components_before[1]["has_unpublished_changes"] - assert c1_components_before[1]["published_by"] is None - assert c1_components_before[2]["id"] == self.problem_block_2["id"] - assert c1_components_before[2]["has_unpublished_changes"] - assert c1_components_before[2]["published_by"] is None - assert c1_components_before[3]["id"] == self.html_block_2["id"] - assert c1_components_before[3]["has_unpublished_changes"] - assert c1_components_before[3]["published_by"] is None - c2_before = self._get_container(self.unit["id"]) - assert c2_before["has_unpublished_changes"] - - # Now publish only Container 1 + self._verify_publish_state( + container_id=self.unit_with_components["id"], + expected_childen_len=4, + expected_has_unpublished_changes=True, + expected_published_by=None, + expected_children_ids=[ + self.problem_block["id"], + self.html_block["id"], + self.problem_block_2["id"], + self.html_block_2["id"], + ], + ) + self._verify_publish_state( + container_id=self.unit["id"], + expected_childen_len=2, + expected_has_unpublished_changes=True, + expected_published_by=None, + expected_children_ids=[ + self.html_block["id"], + html_block_3["id"], + ], + ) + + # Now publish only unit 1 self._publish_container(self.unit_with_components["id"]) # Now it is published: - c1_after = self._get_container(self.unit_with_components["id"]) - assert c1_after["has_unpublished_changes"] is False - c1_components_after = self._get_container_children(self.unit_with_components["id"]) - assert len(c1_components_after) == 4 - assert c1_components_after[0]["id"] == self.problem_block["id"] - assert c1_components_after[0]["has_unpublished_changes"] is False - assert c1_components_after[0]["published_by"] == self.user.username - assert c1_components_after[1]["id"] == self.html_block["id"] - assert c1_components_after[1]["has_unpublished_changes"] is False - assert c1_components_after[1]["published_by"] == self.user.username - assert c1_components_after[2]["id"] == self.problem_block_2["id"] - assert c1_components_after[2]["has_unpublished_changes"] is False - assert c1_components_after[2]["published_by"] == self.user.username - assert c1_components_after[3]["id"] == self.html_block_2["id"] - assert c1_components_after[3]["has_unpublished_changes"] is False - assert c1_components_after[3]["published_by"] == self.user.username - - # and container 2 is still unpublished, except for the shared HTML block that is also in container 1: + self._verify_publish_state( + container_id=self.unit_with_components["id"], + expected_childen_len=4, + expected_has_unpublished_changes=False, + expected_published_by=self.user.username, + expected_children_ids=[ + self.problem_block["id"], + self.html_block["id"], + self.problem_block_2["id"], + self.html_block_2["id"], + ], + ) + + # and unit 2 is still unpublished, except for the shared HTML block that is also in unit 1: c2_after = self._get_container(self.unit["id"]) assert c2_after["has_unpublished_changes"] c2_components_after = self._get_container_children(self.unit["id"]) assert len(c2_components_after) == 2 assert c2_components_after[0]["id"] == self.html_block["id"] - assert c2_components_after[0]["has_unpublished_changes"] is False # published since it's also in container 1 + assert c2_components_after[0]["has_unpublished_changes"] is False # published since it's also in unit 1 assert c2_components_after[0]["published_by"] == self.user.username assert c2_components_after[1]["id"] == html_block_3["id"] assert c2_components_after[1]["has_unpublished_changes"] # unaffected @@ -729,3 +858,157 @@ def test_copy_container(self) -> None: """) + + def test_publish_subsection(self) -> None: + """ + Test that we can publish the changes to a specific subsection + """ + unit_4 = self._create_container(self.lib["id"], "unit", display_name="Test Unit 4", slug=None) + + # Add units on subsection + self._add_container_children( + self.subsection["id"], + children_ids=[ + self.unit["id"], + unit_4["id"], + ] + ) + + # TODO -- remove this when containers publish their children: + # https://github.com/openedx/openedx-learning/pull/307 + # Removing the unit with components because the components (children of children) are not published. + # If the unit is kept, the subsection continues to have changes even after it is published. + self._remove_container_children( + self.subsection_with_units["id"], + children_ids=[ + self.unit_with_components["id"], + ] + ) + # /TODO + + # At first everything is unpublished: + self._verify_publish_state( + container_id=self.subsection_with_units["id"], + expected_childen_len=3, + expected_has_unpublished_changes=True, + expected_published_by=None, + expected_children_ids=[ + self.unit["id"], + self.unit_2["id"], + self.unit_3["id"], + ], + ) + self._verify_publish_state( + container_id=self.subsection["id"], + expected_childen_len=2, + expected_has_unpublished_changes=True, + expected_published_by=None, + expected_children_ids=[ + self.unit["id"], + unit_4["id"], + ], + ) + + # Now publish only subsection 1 + self._publish_container(self.subsection_with_units["id"]) + + # Now it is published: + self._verify_publish_state( + container_id=self.subsection_with_units["id"], + expected_childen_len=3, + expected_has_unpublished_changes=False, + expected_published_by=self.user.username, + expected_children_ids=[ + self.unit["id"], + self.unit_2["id"], + self.unit_3["id"], + ], + ) + + # and subsection 2 is still unpublished, except for the shared unit that is also in subsection 1: + c2_after = self._get_container(self.subsection["id"]) + assert c2_after["has_unpublished_changes"] + c2_units_after = self._get_container_children(self.subsection["id"]) + assert len(c2_units_after) == 2 + assert c2_units_after[0]["id"] == self.unit["id"] + assert c2_units_after[0]["has_unpublished_changes"] is False # published since it's also in subsection 1 + assert c2_units_after[0]["published_by"] == self.user.username + assert c2_units_after[1]["id"] == unit_4["id"] + assert c2_units_after[1]["has_unpublished_changes"] # unaffected + assert c2_units_after[1]["published_by"] is None + + def test_publish_section(self) -> None: + """ + Test that we can publish the changes to a specific section + """ + subsection_4 = self._create_container(self.lib["id"], "subsection", display_name="Test Subsection 4", slug=None) + self._add_container_children( + self.section["id"], + children_ids=[ + self.subsection["id"], + subsection_4["id"], + ] + ) + + # TODO -- remove this when containers publish their children: + # https://github.com/openedx/openedx-learning/pull/307 + # Removing the subsection with units because the units (children of children) are not published. + # If the subsection is kept, the section continues to have changes even after it is published. + self._remove_container_children( + self.section_with_subsections["id"], + children_ids=[ + self.subsection_with_units["id"], + ] + ) + # /TODO + + # At first everything is unpublished: + self._verify_publish_state( + container_id=self.section_with_subsections["id"], + expected_childen_len=3, + expected_has_unpublished_changes=True, + expected_published_by=None, + expected_children_ids=[ + self.subsection["id"], + self.subsection_2["id"], + self.subsection_3["id"], + ], + ) + self._verify_publish_state( + container_id=self.section["id"], + expected_childen_len=2, + expected_has_unpublished_changes=True, + expected_published_by=None, + expected_children_ids=[ + self.subsection["id"], + subsection_4["id"], + ], + ) + + # Now publish only section 1 + self._publish_container(self.section_with_subsections["id"]) + + # Now it is published: + self._verify_publish_state( + container_id=self.section_with_subsections["id"], + expected_childen_len=3, + expected_has_unpublished_changes=False, + expected_published_by=self.user.username, + expected_children_ids=[ + self.subsection["id"], + self.subsection_2["id"], + self.subsection_3["id"], + ], + ) + + # and section 2 is still unpublished, except for the shared subsection that is also in section 1: + c2_after = self._get_container(self.section["id"]) + assert c2_after["has_unpublished_changes"] + c2_units_after = self._get_container_children(self.section["id"]) + assert len(c2_units_after) == 2 + assert c2_units_after[0]["id"] == self.subsection["id"] + assert c2_units_after[0]["has_unpublished_changes"] is False # published since it's also in section 1 + assert c2_units_after[0]["published_by"] == self.user.username + assert c2_units_after[1]["id"] == subsection_4["id"] + assert c2_units_after[1]["has_unpublished_changes"] # unaffected + assert c2_units_after[1]["published_by"] is None diff --git a/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py b/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py index 5c1bd5774403..5b77791e9377 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py @@ -52,7 +52,7 @@ def test_library_paste_unit_from_course(self): "created": paste_data["created"], "modified": paste_data["modified"], "last_published": None, - "published_by": "", + "published_by": None, "has_unpublished_changes": True, "collections": [], "can_stand_alone": True, diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index aa3dfc469b8e..d0e30a4200d2 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -64,6 +64,8 @@ path('restore/', blocks.LibraryBlockRestore.as_view()), # Update collections for a given component path('collections/', blocks.LibraryBlockCollectionsView.as_view(), name='update-collections'), + # Get the full hierarchy that the block belongs to + path('hierarchy/', blocks.LibraryBlockHierarchy.as_view()), # Get the LTI URL of a specific XBlock path('lti/', blocks.LibraryBlockLtiUrlView.as_view(), name='lti-url'), # Get the OLX source code of the specified block: @@ -80,6 +82,8 @@ path('', containers.LibraryContainerView.as_view()), # update components under container path('children/', containers.LibraryContainerChildrenView.as_view()), + # Get the full hierarchy that the container belongs to + path('hierarchy/', containers.LibraryContainerHierarchy.as_view()), # Restore a soft-deleted container path('restore/', containers.LibraryContainerRestore.as_view()), # Update collections for a given container diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 44fe374e89bd..91cabdb0921d 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -66,7 +66,7 @@ numpy<2.0.0 # Date: 2023-09-18 # pinning this version to avoid updates while the library is being developed # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269 -openedx-learning==0.27.0 +openedx-learning==0.27.1 # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f83db850fa28..65d8530fdc4d 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -849,7 +849,7 @@ openedx-filters==2.1.0 # ora2 openedx-forum==0.3.2 # via -r requirements/edx/kernel.in -openedx-learning==0.27.0 +openedx-learning==0.27.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 6be7caad7e1f..ce6061213449 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1407,7 +1407,7 @@ openedx-forum==0.3.2 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-learning==0.27.0 +openedx-learning==0.27.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index ff777e5da9bd..b362e383993a 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1025,7 +1025,7 @@ openedx-filters==2.1.0 # ora2 openedx-forum==0.3.2 # via -r requirements/edx/base.txt -openedx-learning==0.27.0 +openedx-learning==0.27.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 3b816bebffda..f065c3bf2e37 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1071,7 +1071,7 @@ openedx-filters==2.1.0 # ora2 openedx-forum==0.3.2 # via -r requirements/edx/base.txt -openedx-learning==0.27.0 +openedx-learning==0.27.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt