From ea64255949f70afe8ac8384c551437336c1e2573 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 21 Mar 2025 19:31:23 +0530 Subject: [PATCH 01/12] feat: add components to container api --- .../djangoapps/content/search/documents.py | 2 +- .../content_libraries/api/containers.py | 63 +++++++++--- .../content_libraries/api/libraries.py | 14 ++- .../content_libraries/rest_api/collections.py | 4 +- .../content_libraries/rest_api/containers.py | 98 ++++++++++++++++++- .../content_libraries/rest_api/serializers.py | 6 +- .../content_libraries/tests/base.py | 40 +++++++- .../tests/test_containers.py | 32 ++++++ .../core/djangoapps/content_libraries/urls.py | 2 + 9 files changed, 239 insertions(+), 22 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 14c7f712dcfd..69316a2a94e9 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -575,7 +575,7 @@ def searchable_doc_for_container( } try: - container = lib_api.get_container(container_key) + container = lib_api.get_container_metadata(container_key) except lib_api.ContentLibraryCollectionNotFound: # Container not found, so we can only return the base doc pass diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 5b24b6540b10..c325de65fdbb 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -2,6 +2,7 @@ API for containers (Sections, Subsections, Units) in Content Libraries """ from __future__ import annotations + from dataclasses import dataclass from datetime import datetime from enum import Enum @@ -9,10 +10,10 @@ from django.utils.text import slugify from opaque_keys.edx.locator import ( - LibraryLocatorV2, LibraryContainerLocator, + LibraryLocatorV2, + UsageKeyV2, ) - from openedx_events.content_authoring.data import LibraryContainerData from openedx_events.content_authoring.signals import ( LIBRARY_CONTAINER_CREATED, @@ -22,8 +23,10 @@ from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Container +from openedx.core.djangoapps.xblock.api import get_component_from_usage_key + from ..models import ContentLibrary -from .libraries import PublishableItem +from .libraries import LibraryXBlockMetadata, PublishableItem # The public API is only the following symbols: @@ -37,6 +40,7 @@ "library_container_locator", "update_container", "delete_container", + "add_container_children", ] @@ -252,14 +256,47 @@ def get_container_children( """ Get the entities contained in the given container (e.g. the components/xblocks in a unit) """ - assert isinstance(container_key, LibraryContainerLocator) - content_library = ContentLibrary.objects.get_by_key(container_key.library_key) - learning_package = content_library.learning_package - assert learning_package is not None - container = authoring_api.get_container_by_key( - learning_package.id, - key=container_key.container_id, + container = get_container(container_key) + if container_key.container_type == ContainerType.Unit.value: + child_entities = authoring_api.get_components_in_unit(container.unit, published=published) + return [LibraryXBlockMetadata.from_component( + container_key.library_key, + entry.component + ) for entry in child_entities] + else: + child_entities = authoring_api.get_entities_in_container(container, published=published) + return [ContainerMetadata.from_container(entry.entity) for entry in child_entities] + + +def add_container_children( + container_key: LibraryContainerLocator, + children_ids: list[UsageKeyV2] | list[LibraryContainerLocator], + user_id: int | None, +): + """ + Adds children components or containers to given container. + """ + library_key = container_key.library_key + container_type = container_key.container_type + container = get_container(container_key) + match container_type: + case ContainerType.Unit.value: + components = [get_component_from_usage_key(key) for key in children_ids] + new_version = authoring_api.create_next_unit_version( + container.unit, + components=components, + created=datetime.now(), + created_by=user_id, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + case _: + raise ValueError(f"Invalid container type: {container_type}") + + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + library_key=library_key, + container_key=str(container_key), + ) ) - child_entities = authoring_api.get_entities_in_container(container, published=published) - # TODO: convert the return type to list[ContainerMetadata | LibraryXBlockMetadata] ? - return child_entities + + return ContainerMetadata.from_container(library_key, new_version.container) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index 13d41921e860..4a89a358f87d 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -302,6 +302,7 @@ class PublishableItem(LibraryItem): last_draft_created_by: str = "" has_unpublished_changes: bool = False collections: list[CollectionMetadata] = field(default_factory=list) + can_stand_alone: bool = True @dataclass(frozen=True, kw_only=True) @@ -343,6 +344,7 @@ def from_component(cls, library_key, component, associated_collections=None): last_draft_created_by=last_draft_created_by, has_unpublished_changes=component.versioning.has_unpublished_changes, collections=associated_collections or [], + can_stand_alone=component.publishable_entity.can_stand_alone, ) @@ -958,7 +960,13 @@ def validate_can_add_block_to_library( return content_library, usage_key -def create_library_block(library_key, block_type, definition_id, user_id=None): +def create_library_block( + library_key: LibraryLocatorV2, + block_type: str, + definition_id: str, + user_id: int | None = None, + can_stand_alone: bool = True, +): """ Create a new XBlock in this library of the specified type (e.g. "html"). """ @@ -969,7 +977,7 @@ def create_library_block(library_key, block_type, definition_id, user_id=None): content_library, usage_key = validate_can_add_block_to_library(library_key, block_type, block_id) - _create_component_for_block(content_library, usage_key, user_id) + _create_component_for_block(content_library, usage_key, user_id, can_stand_alone) # Now return the metadata about the new block: LIBRARY_BLOCK_CREATED.send_event( @@ -1135,6 +1143,7 @@ def _create_component_for_block( content_lib: ContentLibrary, usage_key: LibraryUsageLocatorV2, user_id: int | None = None, + can_stand_alone: bool = True, ): """ Create a Component for an XBlock type, initialize it, and return the ComponentVersion. @@ -1168,6 +1177,7 @@ def _create_component_for_block( title=display_name, created=now, created_by=user_id, + can_stand_alone=can_stand_alone, ) content = authoring_api.get_or_create_text_content( learning_package.id, diff --git a/openedx/core/djangoapps/content_libraries/rest_api/collections.py b/openedx/core/djangoapps/content_libraries/rest_api/collections.py index f1b63b2c1845..c49822ae2f0f 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/collections.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/collections.py @@ -21,8 +21,8 @@ from .utils import convert_exceptions from .serializers import ( ContentLibraryCollectionSerializer, - ContentLibraryCollectionComponentsUpdateSerializer, ContentLibraryCollectionUpdateSerializer, + ContentLibraryComponentKeysSerializer, ) from openedx.core.types.http import RestRequest @@ -200,7 +200,7 @@ def update_components(self, request: RestRequest, *args, **kwargs) -> Response: content_library = self.get_content_library() collection_key = kwargs["key"] - serializer = ContentLibraryCollectionComponentsUpdateSerializer(data=request.data) + serializer = ContentLibraryComponentKeysSerializer(data=request.data) serializer.is_valid(raise_exception=True) usage_keys = serializer.validated_data["usage_keys"] diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index ad23a51d55b3..bec21c61daac 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -80,7 +80,103 @@ def get(self, request, container_key: LibraryContainerLocator): request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) - container = api.get_container(container_key) + container = api.get_container_metadata(container_key) + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainerChildrenView(GenericAPIView): + """ + View to get or update children of specific container (a section, subsection, or unit) + """ + serializer_class = serializers.LibraryXBlockMetadataSerializer + + @convert_exceptions + @swagger_auto_schema( + responses={200: list[serializers.LibraryXBlockMetadataSerializer]} + ) + def get(self, request, container_key: LibraryContainerLocator): + """ + Get children components of given container + Example: + GET /api/libraries/v2/containers//children/ + Result: + [ + { + 'block_type': 'problem', + 'can_stand_alone': True, + 'collections': [], + 'created': '2025-03-21T13:53:55Z', + 'def_key': None, + 'display_name': 'Blank Problem', + 'has_unpublished_changes': True, + 'id': 'lb:CL-TEST:containers:problem:Problem1', + 'last_draft_created': '2025-03-21T13:53:55Z', + 'last_draft_created_by': 'Bob', + 'last_published': None, + 'modified': '2025-03-21T13:53:55Z', + 'published_by': None, + }, + { + 'block_type': 'html', + 'can_stand_alone': False, + 'collections': [], + 'created': '2025-03-21T13:53:55Z', + 'def_key': None, + 'display_name': 'Text', + 'has_unpublished_changes': True, + 'id': 'lb:CL-TEST:containers:html:Html1', + 'last_draft_created': '2025-03-21T13:53:55Z', + 'last_draft_created_by': 'Bob', + 'last_published': None, + 'modified': '2025-03-21T13:53:55Z', + 'published_by': None, + } + ] + """ + published = request.GET.get('published', False) + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, + ) + child_entities = api.get_container_children(container_key, published) + if container_key.container_type == api.ContainerType.Unit.value: + data = serializers.LibraryXBlockMetadataSerializer(child_entities, many=True).data + else: + data = serializers.LibraryContainerMetadataSerializer(child_entities, many=True).data + return Response(data) + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.ContentLibraryComponentKeysSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def post(self, request, container_key: LibraryContainerLocator): + """ + Add components to unit + Example: + POST /api/libraries/v2/containers//children/ + Request body: + {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + ) + serializer = serializers.ContentLibraryComponentKeysSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + # Only components under units are supported for now. + assert container_key.container_type == api.ContainerType.Unit.value + + container = api.add_container_children( + container_key, + children_ids=serializer.validated_data["usage_keys"], + user_id=request.user.id, + ) return Response(serializers.LibraryContainerMetadataSerializer(container).data) @convert_exceptions diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index 67b8ec00e56f..46bec36e3c94 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -159,6 +159,7 @@ class LibraryXBlockMetadataSerializer(serializers.Serializer): tags_count = serializers.IntegerField(read_only=True) collections = CollectionMetadataSerializer(many=True, required=False) + can_stand_alone = serializers.BooleanField(read_only=True) class LibraryXBlockTypeSerializer(serializers.Serializer): @@ -193,6 +194,9 @@ class LibraryXBlockCreationSerializer(serializers.Serializer): # creating new block from scratch staged_content = serializers.CharField(required=False) + # Optional param defaults to True, set to False if block is being created under a container. + can_stand_alone = serializers.BooleanField(required=False, default=True) + class LibraryPasteClipboardSerializer(serializers.Serializer): """ @@ -345,7 +349,7 @@ def to_internal_value(self, value: str) -> UsageKeyV2: raise ValidationError from err -class ContentLibraryCollectionComponentsUpdateSerializer(serializers.Serializer): +class ContentLibraryComponentKeysSerializer(serializers.Serializer): """ Serializer for adding/removing Components to/from a Collection. """ diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index d8030afae7f0..9841bc689908 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -33,6 +33,7 @@ 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_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a container in this library +URL_LIB_CONTAINER_COMPONENTS = URL_LIB_CONTAINER + 'children/' # Get, add or delete a component in this container URL_LIB_LTI_PREFIX = URL_PREFIX + 'lti/1.3/' URL_LIB_LTI_JWKS = URL_LIB_LTI_PREFIX + 'pub/jwks/' @@ -229,9 +230,21 @@ def _get_library_blocks(self, lib_key, query_params_dict=None, expect_response=2 expect_response ) - def _add_block_to_library(self, lib_key, block_type, slug, parent_block=None, expect_response=200): + def _add_block_to_library( + self, + lib_key, + block_type, + slug, + parent_block=None, + can_stand_alone=True, + expect_response=200, + ): """ Add a new XBlock to the library """ - data = {"block_type": block_type, "definition_id": slug} + data = { + "block_type": block_type, + "definition_id": slug, + "can_stand_alone": can_stand_alone, + } if parent_block: data["parent_block"] = parent_block return self._api('post', URL_LIB_BLOCKS.format(lib_key=lib_key), data, expect_response) @@ -372,3 +385,26 @@ def _update_container(self, container_key: str, display_name: str, expect_respon def _delete_container(self, container_key: str, expect_response=204): """ Delete a container (unit etc.) """ return self._api('delete', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response) + + def _get_container_components(self, container_key: str, expect_response=200): + """ Get container components""" + return self._api( + 'get', + URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + None, + expect_response + ) + + def _add_container_components( + self, + container_key: str, + children_ids: list[str], + expect_response=200, + ): + """ Add container components""" + return self._api( + 'post', + URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + {'usage_keys': children_ids}, + expect_response + ) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 23a519899a85..dcbd6b6023ff 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -178,3 +178,35 @@ def test_unit_gets_auto_slugs(self): assert container1_data["container_key"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") assert container2_data["container_key"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") assert container1_data["container_key"] != container2_data["container_key"] + + def test_unit_add_children(self): + """ + Test that we can add and get unit children components + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + + container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) + html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) + self._add_container_components( + container_data["container_key"], + children_ids=[problem_block["id"], html_block["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 2 + assert data[0]['id'] == problem_block['id'] + assert not data[0]['can_stand_alone'] + assert data[1]['id'] == html_block['id'] + assert not data[1]['can_stand_alone'] + problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) + html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") + self._add_container_components( + container_data["container_key"], + children_ids=[problem_block_2["id"], html_block_2["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 4 + assert data[2]['id'] == problem_block_2['id'] + assert not data[2]['can_stand_alone'] + assert data[3]['id'] == html_block_2['id'] + assert data[3]['can_stand_alone'] diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index 1656a8589616..99e98a2cc424 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -80,6 +80,8 @@ path('containers//', include([ # Get metadata about a specific container in this library, update or delete the container: path('', containers.LibraryContainerView.as_view()), + # update components under container + path('children/', containers.LibraryContainerChildrenView.as_view()), # Update collections for a given container # path('collections/', views.LibraryContainerCollectionsView.as_view(), name='update-collections-ct'), # path('publish/', views.LibraryContainerPublishView.as_view()), From e49762e4f3d789430bdeba1b5d04e6a46a0a672a Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 24 Mar 2025 10:51:55 +0530 Subject: [PATCH 02/12] feat: remove and replace components in container api --- .../content_libraries/api/containers.py | 7 +- .../content_libraries/rest_api/containers.py | 81 +++++++++++++++++-- .../content_libraries/tests/base.py | 28 +++++++ .../tests/test_containers.py | 80 +++++++++++++++++- 4 files changed, 186 insertions(+), 10 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index c325de65fdbb..4f61bee62166 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -40,7 +40,7 @@ "library_container_locator", "update_container", "delete_container", - "add_container_children", + "update_container_children", ] @@ -268,10 +268,11 @@ def get_container_children( return [ContainerMetadata.from_container(entry.entity) for entry in child_entities] -def add_container_children( +def update_container_children( container_key: LibraryContainerLocator, children_ids: list[UsageKeyV2] | list[LibraryContainerLocator], user_id: int | None, + entities_action: authoring_api.ChildrenEntitiesAction = authoring_api.ChildrenEntitiesAction.REPLACE, ): """ Adds children components or containers to given container. @@ -287,7 +288,7 @@ def add_container_children( components=components, created=datetime.now(), created_by=user_id, - entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + entities_action=entities_action, ) case _: raise ValueError(f"Invalid container type: {container_type}") diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index bec21c61daac..f38fa3082aca 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -8,10 +8,10 @@ from django.contrib.auth import get_user_model from django.db.transaction import non_atomic_requests from django.utils.decorators import method_decorator -from django.utils.translation import gettext as _ from drf_yasg.utils import swagger_auto_schema from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator +from openedx_learning.api import authoring as authoring_api from rest_framework.generics import GenericAPIView from rest_framework.response import Response from rest_framework.status import HTTP_204_NO_CONTENT @@ -148,6 +148,19 @@ def get(self, request, container_key: LibraryContainerLocator): data = serializers.LibraryContainerMetadataSerializer(child_entities, many=True).data return Response(data) + def _check_perm_and_serialize(self, request, library_key, perm): + """ + Helper function to check permission and serialize data. + """ + api.require_permission_for_library_key( + library_key, + request.user, + perm, + ) + serializer = serializers.ContentLibraryComponentKeysSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + return serializer + @convert_exceptions @swagger_auto_schema( request_body=serializers.ContentLibraryComponentKeysSerializer, @@ -161,21 +174,77 @@ def post(self, request, container_key: LibraryContainerLocator): Request body: {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} """ - api.require_permission_for_library_key( + serializer = self._check_perm_and_serialize( + request, container_key.library_key, - request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY ) - serializer = serializers.ContentLibraryComponentKeysSerializer(data=request.data) - serializer.is_valid(raise_exception=True) + # Only components under units are supported for now. + assert container_key.container_type == api.ContainerType.Unit.value + + container = api.update_container_children( + container_key, + children_ids=serializer.validated_data["usage_keys"], + user_id=request.user.id, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.ContentLibraryComponentKeysSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def delete(self, request, container_key: LibraryContainerLocator): + """ + Remove components from unit + Example: + DELETE /api/libraries/v2/containers//children/ + Request body: + {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} + """ + serializer = self._check_perm_and_serialize( + request, + container_key.library_key, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + ) + # Only components under units are supported for now. + assert container_key.container_type == api.ContainerType.Unit.value + + container = api.update_container_children( + container_key, + children_ids=serializer.validated_data["usage_keys"], + user_id=request.user.id, + entities_action=authoring_api.ChildrenEntitiesAction.REMOVE, + ) + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.ContentLibraryComponentKeysSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def patch(self, request, container_key: LibraryContainerLocator): + """ + Replace components in unit, can be used to reorder components as well. + Example: + PATCH /api/libraries/v2/containers//children/ + Request body: + {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} + """ + serializer = self._check_perm_and_serialize( + request, + container_key.library_key, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + ) # Only components under units are supported for now. assert container_key.container_type == api.ContainerType.Unit.value - container = api.add_container_children( + container = api.update_container_children( container_key, children_ids=serializer.validated_data["usage_keys"], user_id=request.user.id, + entities_action=authoring_api.ChildrenEntitiesAction.REPLACE, ) return Response(serializers.LibraryContainerMetadataSerializer(container).data) diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 9841bc689908..6adb8184ea00 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -408,3 +408,31 @@ def _add_container_components( {'usage_keys': children_ids}, expect_response ) + + def _remove_container_components( + self, + container_key: str, + children_ids: list[str], + expect_response=200, + ): + """ Remove container components""" + return self._api( + 'delete', + URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + {'usage_keys': children_ids}, + expect_response + ) + + def _patch_container_components( + self, + container_key: str, + children_ids: list[str], + expect_response=200, + ): + """ Update container components""" + return self._api( + 'patch', + URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + {'usage_keys': children_ids}, + expect_response + ) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index dcbd6b6023ff..b4cc94caae2a 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -2,10 +2,10 @@ Tests for Learning-Core-based Content Libraries """ from datetime import datetime, timezone +from unittest import mock import ddt from freezegun import freeze_time -from unittest import mock from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_events.content_authoring.data import LibraryContainerData @@ -185,6 +185,7 @@ def test_unit_add_children(self): """ lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + # Create container and add some components container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) @@ -200,13 +201,90 @@ def test_unit_add_children(self): assert not data[1]['can_stand_alone'] problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") + # Add two more components self._add_container_components( container_data["container_key"], children_ids=[problem_block_2["id"], html_block_2["id"]] ) data = self._get_container_components(container_data["container_key"]) + # Verify total number of components to be 2 + 2 = 4 assert len(data) == 4 assert data[2]['id'] == problem_block_2['id'] assert not data[2]['can_stand_alone'] assert data[3]['id'] == html_block_2['id'] assert data[3]['can_stand_alone'] + + def test_unit_remove_children(self): + """ + Test that we can remove unit children components + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + + # Create container and add some components + container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) + html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) + problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) + html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") + self._add_container_components( + container_data["container_key"], + children_ids=[problem_block["id"], html_block["id"], problem_block_2["id"], html_block_2["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 4 + # Remove both problem blocks. + self._remove_container_components( + container_data["container_key"], + children_ids=[problem_block_2["id"], problem_block["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 2 + assert data[0]['id'] == html_block['id'] + assert data[1]['id'] == html_block_2['id'] + + def test_unit_replace_children(self): + """ + Test that we can completely replace/reorder unit children components. + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + + # Create container and add some components + container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) + html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) + problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) + html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") + self._add_container_components( + container_data["container_key"], + children_ids=[problem_block["id"], html_block["id"], problem_block_2["id"], html_block_2["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 4 + assert data[0]['id'] == problem_block['id'] + assert data[1]['id'] == html_block['id'] + assert data[2]['id'] == problem_block_2['id'] + assert data[3]['id'] == html_block_2['id'] + + # Reorder the components + self._patch_container_components( + container_data["container_key"], + children_ids=[problem_block["id"], problem_block_2["id"], html_block["id"], html_block_2["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 4 + assert data[0]['id'] == problem_block['id'] + assert data[1]['id'] == problem_block_2['id'] + assert data[2]['id'] == html_block['id'] + assert data[3]['id'] == html_block_2['id'] + + # Replace with new components + new_problem_block = self._add_block_to_library(lib["id"], "problem", "New_Problem", can_stand_alone=False) + new_html_block = self._add_block_to_library(lib["id"], "html", "New_Html", can_stand_alone=False) + self._patch_container_components( + container_data["container_key"], + children_ids=[new_problem_block["id"], new_html_block["id"]], + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 2 + assert data[0]['id'] == new_problem_block['id'] + assert data[1]['id'] == new_html_block['id'] From 704dd37eeac9ea6e4793c2378009dbc80c0064fa Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 24 Mar 2025 11:50:25 +0530 Subject: [PATCH 03/12] refactor: container childern api --- .../content_libraries/rest_api/containers.py | 150 ++++++++---------- 1 file changed, 67 insertions(+), 83 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index f38fa3082aca..671ea8c54f37 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -83,6 +83,48 @@ def get(self, request, container_key: LibraryContainerLocator): container = api.get_container_metadata(container_key) return Response(serializers.LibraryContainerMetadataSerializer(container).data) + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.LibraryContainerUpdateSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def patch(self, request, container_key: LibraryContainerLocator): + """ + Update a Container. + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + serializer = serializers.LibraryContainerUpdateSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + container = api.update_container( + container_key, + display_name=serializer.validated_data['display_name'], + user_id=request.user.id, + ) + + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + @convert_exceptions + def delete(self, request, container_key: LibraryContainerLocator): + """ + Delete a Container (soft delete). + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + + api.delete_container( + container_key, + ) + + return Response({}, status=HTTP_204_NO_CONTENT) + @method_decorator(non_atomic_requests, name="dispatch") @view_auth_classes() @@ -148,18 +190,32 @@ def get(self, request, container_key: LibraryContainerLocator): data = serializers.LibraryContainerMetadataSerializer(child_entities, many=True).data return Response(data) - def _check_perm_and_serialize(self, request, library_key, perm): + def _update_component_children( + self, + request, + container_key: LibraryContainerLocator, + action: authoring_api.ChildrenEntitiesAction, + ): """ - Helper function to check permission and serialize data. + Helper function to update children in container. """ api.require_permission_for_library_key( - library_key, + container_key.library_key, request.user, - perm, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, ) serializer = serializers.ContentLibraryComponentKeysSerializer(data=request.data) serializer.is_valid(raise_exception=True) - return serializer + # Only components under units are supported for now. + assert container_key.container_type == api.ContainerType.Unit.value + + container = api.update_container_children( + container_key, + children_ids=serializer.validated_data["usage_keys"], + user_id=request.user.id, + entities_action=action, + ) + return Response(serializers.LibraryContainerMetadataSerializer(container).data) @convert_exceptions @swagger_auto_schema( @@ -174,21 +230,11 @@ def post(self, request, container_key: LibraryContainerLocator): Request body: {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} """ - serializer = self._check_perm_and_serialize( + return self._update_component_children( request, - container_key.library_key, - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY - ) - # Only components under units are supported for now. - assert container_key.container_type == api.ContainerType.Unit.value - - container = api.update_container_children( container_key, - children_ids=serializer.validated_data["usage_keys"], - user_id=request.user.id, - entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + action=authoring_api.ChildrenEntitiesAction.APPEND, ) - return Response(serializers.LibraryContainerMetadataSerializer(container).data) @convert_exceptions @swagger_auto_schema( @@ -203,21 +249,11 @@ def delete(self, request, container_key: LibraryContainerLocator): Request body: {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} """ - serializer = self._check_perm_and_serialize( + return self._update_component_children( request, - container_key.library_key, - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY - ) - # Only components under units are supported for now. - assert container_key.container_type == api.ContainerType.Unit.value - - container = api.update_container_children( container_key, - children_ids=serializer.validated_data["usage_keys"], - user_id=request.user.id, - entities_action=authoring_api.ChildrenEntitiesAction.REMOVE, + action=authoring_api.ChildrenEntitiesAction.REMOVE, ) - return Response(serializers.LibraryContainerMetadataSerializer(container).data) @convert_exceptions @swagger_auto_schema( @@ -232,60 +268,8 @@ def patch(self, request, container_key: LibraryContainerLocator): Request body: {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} """ - serializer = self._check_perm_and_serialize( + return self._update_component_children( request, - container_key.library_key, - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY - ) - # Only components under units are supported for now. - assert container_key.container_type == api.ContainerType.Unit.value - - container = api.update_container_children( container_key, - children_ids=serializer.validated_data["usage_keys"], - user_id=request.user.id, - entities_action=authoring_api.ChildrenEntitiesAction.REPLACE, + action=authoring_api.ChildrenEntitiesAction.REPLACE, ) - return Response(serializers.LibraryContainerMetadataSerializer(container).data) - - @convert_exceptions - @swagger_auto_schema( - request_body=serializers.LibraryContainerUpdateSerializer, - responses={200: serializers.LibraryContainerMetadataSerializer} - ) - def patch(self, request, container_key: LibraryContainerLocator): - """ - Update a Container. - """ - api.require_permission_for_library_key( - container_key.library_key, - request.user, - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, - ) - serializer = serializers.LibraryContainerUpdateSerializer(data=request.data) - serializer.is_valid(raise_exception=True) - - container = api.update_container( - container_key, - display_name=serializer.validated_data['display_name'], - user_id=request.user.id, - ) - - return Response(serializers.LibraryContainerMetadataSerializer(container).data) - - @convert_exceptions - def delete(self, request, container_key: LibraryContainerLocator): - """ - Delete a Container (soft delete). - """ - api.require_permission_for_library_key( - container_key.library_key, - request.user, - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, - ) - - api.delete_container( - container_key, - ) - - return Response({}, status=HTTP_204_NO_CONTENT) From 985a884247b0f119d2007fe981637e084640953c Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 24 Mar 2025 16:17:15 +0530 Subject: [PATCH 04/12] chore: fix lint issues --- openedx/core/djangoapps/content_libraries/api/containers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 4f61bee62166..cb166b8b5085 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -265,7 +265,10 @@ def get_container_children( ) for entry in child_entities] else: child_entities = authoring_api.get_entities_in_container(container, published=published) - return [ContainerMetadata.from_container(entry.entity) for entry in child_entities] + return [ContainerMetadata.from_container( + container_key.library_key, + entry.entity + ) for entry in child_entities] def update_container_children( From 185c9caf5f14f4abef910d472af4bef624ea3824 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 24 Mar 2025 16:33:17 +0530 Subject: [PATCH 05/12] temp: install openedx-learning dev branch --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index fed7e47c5447..270e4eedbb5e 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -112,7 +112,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.19.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container # 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 24ede4f1f253..f6a2ad78351e 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -820,7 +820,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/kernel.in -openedx-learning==0.19.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index c9acc77e898b..25c6f950eefd 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1383,7 +1383,7 @@ openedx-forum==0.1.9 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-learning==0.19.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index afe2bedf5954..5ed792f95a24 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -992,7 +992,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/base.txt -openedx-learning==0.19.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 795e66c61adc..ea71d69fe4f1 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1050,7 +1050,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/base.txt -openedx-learning==0.19.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 95dbca4c78acbd4df5ca6677113987120d0fba0f Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 24 Mar 2025 20:47:35 +0530 Subject: [PATCH 06/12] feat: update publish_status and children count in index --- .../djangoapps/content/search/documents.py | 54 +++++---- .../content/search/tests/test_documents.py | 112 +++++++++++++++++- .../content_libraries/api/containers.py | 12 ++ 3 files changed, 148 insertions(+), 30 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 69316a2a94e9..fb3ef465af5b 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -6,11 +6,12 @@ import logging from hashlib import blake2b -from django.utils.text import slugify from django.core.exceptions import ObjectDoesNotExist +from django.utils.text import slugify from opaque_keys.edx.keys import LearningContextKey, UsageKey +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 from openedx_learning.api import authoring as authoring_api -from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator +from openedx_learning.api.authoring_models import Collection from rest_framework.exceptions import NotFound from openedx.core.djangoapps.content.search.models import SearchAccess @@ -19,7 +20,6 @@ from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.djangoapps.xblock.data import LatestVersion -from openedx_learning.api.authoring_models import Collection log = logging.getLogger(__name__) @@ -554,7 +554,7 @@ def searchable_doc_for_container( ) -> dict: """ Generate a dictionary document suitable for ingestion into a search engine - like Meilisearch or Elasticsearch, so that the given collection can be + like Meilisearch or Elasticsearch, so that the given container can be found using faceted search. If no container is found for the given container key, the returned document @@ -576,29 +576,33 @@ def searchable_doc_for_container( try: container = lib_api.get_container_metadata(container_key) - except lib_api.ContentLibraryCollectionNotFound: + except lib_api.ContentLibraryContainerNotFound: # Container not found, so we can only return the base doc - pass + return doc - if container: - # TODO: check if there's a more efficient way to load these num_children counts? - draft_num_children = len(lib_api.get_container_children(container_key, published=False)) + draft_num_children = lib_api.get_container_children_count(container_key, published=False) + publish_status = PublishStatus.published + if container.last_published is None: + publish_status = PublishStatus.never + elif container.has_unpublished_changes: + publish_status = PublishStatus.modified - doc.update({ - Fields.display_name: container.display_name, - Fields.created: container.created.timestamp(), - Fields.modified: container.modified.timestamp(), - Fields.num_children: draft_num_children, - }) - library = lib_api.get_library(container_key.library_key) - if library: - doc[Fields.breadcrumbs] = [{"display_name": library.title}] - - if container.published_version_num is not None: - published_num_children = len(lib_api.get_container_children(container_key, published=True)) - doc[Fields.published] = { - # Fields.published_display_name: container_published.title, TODO: set the published title - Fields.published_num_children: published_num_children, - } + doc.update({ + Fields.display_name: container.display_name, + Fields.created: container.created.timestamp(), + Fields.modified: container.modified.timestamp(), + Fields.num_children: draft_num_children, + Fields.publish_status: publish_status, + }) + library = lib_api.get_library(container_key.library_key) + if library: + doc[Fields.breadcrumbs] = [{"display_name": library.title}] + + if container.published_version_num is not None: + published_num_children = lib_api.get_container_children_count(container_key, published=True) + doc[Fields.published] = { + # Fields.published_display_name: container_published.title, TODO: set the published title + Fields.published_num_children: published_num_children, + } return doc diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index 38b1d607ab05..a2964436d039 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -3,12 +3,13 @@ """ from dataclasses import replace from datetime import datetime, timezone -from organizations.models import Organization from freezegun import freeze_time +from openedx_learning.api import authoring as authoring_api +from organizations.models import Organization -from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangoapps.content_libraries import api as library_api +from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -17,13 +18,13 @@ try: # This import errors in the lms because content.search is not an installed app there. from ..documents import ( - searchable_doc_for_course_block, - searchable_doc_tags, - searchable_doc_tags_for_collection, searchable_doc_collections, searchable_doc_for_collection, searchable_doc_for_container, + searchable_doc_for_course_block, searchable_doc_for_library_block, + searchable_doc_tags, + searchable_doc_tags_for_collection, ) from ..models import SearchAccess except RuntimeError: @@ -522,11 +523,112 @@ def test_draft_container(self): "display_name": "A Unit in the Search Index", # description is not set for containers "num_children": 0, + "publish_status": "never", + "context_key": "lib:edX:2012_Fall", + "access_id": self.library_access_id, + "breadcrumbs": [{"display_name": "some content_library"}], + "created": 1680674828.0, + "modified": 1680674828.0, + # "tags" should be here but we haven't implemented them yet + # "published" is not set since we haven't published it yet + } + + def test_published_container(self): + """ + Test creating a search document for a published container + """ + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + container_meta = library_api.create_container( + self.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit1", + title="A Unit in the Search Index", + user_id=None, + ) + library_api.update_container_children( + container_meta.container_key, + [self.library_block.usage_key], + user_id=None, + ) + library_api.publish_changes(self.library.key) + + doc = searchable_doc_for_container(container_meta.container_key) + + assert doc == { + "id": "lctedx2012_fallunitunit1-edd13a0c", + "block_id": "unit1", + "block_type": "unit", + "usage_key": "lct:edX:2012_Fall:unit:unit1", + "type": "library_container", + "org": "edX", + "display_name": "A Unit in the Search Index", + # description is not set for containers + "num_children": 1, + "publish_status": "published", + "context_key": "lib:edX:2012_Fall", + "access_id": self.library_access_id, + "breadcrumbs": [{"display_name": "some content_library"}], + "created": 1680674828.0, + "modified": 1680674828.0, + "published": {"num_children": 1}, + # "tags" should be here but we haven't implemented them yet + # "published" is not set since we haven't published it yet + } + + def test_published_container_with_changes(self): + """ + Test creating a search document for a published container + """ + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + container_meta = library_api.create_container( + self.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit1", + title="A Unit in the Search Index", + user_id=None, + ) + library_api.update_container_children( + container_meta.container_key, + [self.library_block.usage_key], + user_id=None, + ) + library_api.publish_changes(self.library.key) + block_2 = library_api.create_library_block( + self.library.key, + "html", + "text3", + ) + + # Add another component after publish + with freeze_time(created_date): + library_api.update_container_children( + container_meta.container_key, + [block_2.usage_key], + user_id=None, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + + doc = searchable_doc_for_container(container_meta.container_key) + + assert doc == { + "id": "lctedx2012_fallunitunit1-edd13a0c", + "block_id": "unit1", + "block_type": "unit", + "usage_key": "lct:edX:2012_Fall:unit:unit1", + "type": "library_container", + "org": "edX", + "display_name": "A Unit in the Search Index", + # description is not set for containers + "num_children": 2, + "publish_status": "modified", "context_key": "lib:edX:2012_Fall", "access_id": self.library_access_id, "breadcrumbs": [{"display_name": "some content_library"}], "created": 1680674828.0, "modified": 1680674828.0, + "published": {"num_children": 1}, # "tags" should be here but we haven't implemented them yet # "published" is not set since we haven't published it yet } diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index cb166b8b5085..e3fc087c8f6c 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -37,6 +37,7 @@ "get_container", "create_container", "get_container_children", + "get_container_children_count", "library_container_locator", "update_container", "delete_container", @@ -271,6 +272,17 @@ def get_container_children( ) for entry in child_entities] +def get_container_children_count( + container_key: LibraryContainerLocator, + published=False, +) -> int: + """ + Get the count of entities contained in the given container (e.g. the components/xblocks in a unit) + """ + container = get_container(container_key) + return authoring_api.get_container_children_count(container, published=published) + + def update_container_children( container_key: LibraryContainerLocator, children_ids: list[UsageKeyV2] | list[LibraryContainerLocator], From 48f54dc8b6432365338dd2f8d6a2862b87dcfa8f Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 24 Mar 2025 21:16:26 +0530 Subject: [PATCH 07/12] chore: fix mypy issues --- .../core/djangoapps/content_libraries/api/containers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index e3fc087c8f6c..baaa880b3b0d 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -259,11 +259,11 @@ def get_container_children( """ container = get_container(container_key) if container_key.container_type == ContainerType.Unit.value: - child_entities = authoring_api.get_components_in_unit(container.unit, published=published) + child_components = authoring_api.get_components_in_unit(container.unit, published=published) return [LibraryXBlockMetadata.from_component( container_key.library_key, entry.component - ) for entry in child_entities] + ) for entry in child_components] else: child_entities = authoring_api.get_entities_in_container(container, published=published) return [ContainerMetadata.from_container( @@ -297,10 +297,10 @@ def update_container_children( container = get_container(container_key) match container_type: case ContainerType.Unit.value: - components = [get_component_from_usage_key(key) for key in children_ids] + components = [get_component_from_usage_key(key) for key in children_ids] # type: ignore[arg-type] new_version = authoring_api.create_next_unit_version( container.unit, - components=components, + components=components, # type: ignore[arg-type] created=datetime.now(), created_by=user_id, entities_action=entities_action, From 2f6706957e522e5ea6dc408d22c8f749741b87da Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 25 Mar 2025 10:14:13 +0530 Subject: [PATCH 08/12] test: fix reindex test --- openedx/core/djangoapps/content/search/tests/test_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 2decf2374fac..813db0241db1 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -227,6 +227,7 @@ def setUp(self): "display_name": "Unit 1", # description is not set for containers "num_children": 0, + "publish_status": "never", "context_key": "lib:org1:lib", "org": "org1", "created": created_date.timestamp(), From f002ef409d90192ef290ba09d68fbcff1011fa0b Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 27 Mar 2025 16:01:16 +0530 Subject: [PATCH 09/12] refactor: rebase and fix conflicts --- openedx/core/djangoapps/content/search/documents.py | 2 +- openedx/core/djangoapps/content_libraries/api/containers.py | 6 +++--- .../djangoapps/content_libraries/rest_api/containers.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index fb3ef465af5b..c6ca52098abc 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -575,7 +575,7 @@ def searchable_doc_for_container( } try: - container = lib_api.get_container_metadata(container_key) + container = lib_api.get_container(container_key) except lib_api.ContentLibraryContainerNotFound: # Container not found, so we can only return the base doc return doc diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index baaa880b3b0d..6719c07065e4 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -257,7 +257,7 @@ def get_container_children( """ Get the entities contained in the given container (e.g. the components/xblocks in a unit) """ - container = get_container(container_key) + container = _get_container(container_key) if container_key.container_type == ContainerType.Unit.value: child_components = authoring_api.get_components_in_unit(container.unit, published=published) return [LibraryXBlockMetadata.from_component( @@ -279,7 +279,7 @@ def get_container_children_count( """ Get the count of entities contained in the given container (e.g. the components/xblocks in a unit) """ - container = get_container(container_key) + container = _get_container(container_key) return authoring_api.get_container_children_count(container, published=published) @@ -294,7 +294,7 @@ def update_container_children( """ library_key = container_key.library_key container_type = container_key.container_type - container = get_container(container_key) + container = _get_container(container_key) match container_type: case ContainerType.Unit.value: components = [get_component_from_usage_key(key) for key in children_ids] # type: ignore[arg-type] diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 671ea8c54f37..95e468b4a43a 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -80,7 +80,7 @@ def get(self, request, container_key: LibraryContainerLocator): request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) - container = api.get_container_metadata(container_key) + container = api.get_container(container_key) return Response(serializers.LibraryContainerMetadataSerializer(container).data) @convert_exceptions From 453b63228ed22c8423f936c690be102012802582 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 27 Mar 2025 16:42:49 +0530 Subject: [PATCH 10/12] test: update test to check signals --- .../tests/test_containers.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index b4cc94caae2a..52546396f2bb 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -183,7 +183,10 @@ def test_unit_add_children(self): """ Test that we can add and get unit children components """ + update_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(update_receiver) lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) # Create container and add some components container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) @@ -206,6 +209,17 @@ def test_unit_add_children(self): container_data["container_key"], children_ids=[problem_block_2["id"], html_block_2["id"]] ) + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key=container_data["container_key"], + ), + }, + update_receiver.call_args_list[0].kwargs, + ) data = self._get_container_components(container_data["container_key"]) # Verify total number of components to be 2 + 2 = 4 assert len(data) == 4 @@ -218,7 +232,10 @@ def test_unit_remove_children(self): """ Test that we can remove unit children components """ + update_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(update_receiver) lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) # Create container and add some components container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) @@ -241,12 +258,26 @@ def test_unit_remove_children(self): assert len(data) == 2 assert data[0]['id'] == html_block['id'] assert data[1]['id'] == html_block_2['id'] + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key=container_data["container_key"], + ), + }, + update_receiver.call_args_list[0].kwargs, + ) def test_unit_replace_children(self): """ Test that we can completely replace/reorder unit children components. """ + update_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(update_receiver) lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) # Create container and add some components container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) @@ -288,3 +319,14 @@ def test_unit_replace_children(self): assert len(data) == 2 assert data[0]['id'] == new_problem_block['id'] assert data[1]['id'] == new_html_block['id'] + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key=container_data["container_key"], + ), + }, + update_receiver.call_args_list[0].kwargs, + ) From 50fb8075b9fac5c429aad4432e75f371a43e22f5 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 28 Mar 2025 16:58:17 +0530 Subject: [PATCH 11/12] docs: document can_stand_alone flag --- openedx/core/djangoapps/content_libraries/api/libraries.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index 4a89a358f87d..a4b001cd9a47 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -969,6 +969,8 @@ def create_library_block( ): """ Create a new XBlock in this library of the specified type (e.g. "html"). + + Set can_stand_alone = False when a component is created under a container, like unit. """ # It's in the serializer as ``definition_id``, but for our purposes, it's # the block_id. See the comments in ``LibraryXBlockCreationSerializer`` for @@ -1153,6 +1155,8 @@ def _create_component_for_block( will be set as the current draft. This function does not publish the Component. + Set can_stand_alone = False when a component is created under a container, like unit. + TODO: We should probably shift this to openedx.core.djangoapps.xblock.api (along with its caller) since it gives runtime storage specifics. The Library-specific logic stays in this module, so "create a block for my lib" From d1d5b8e00f74af885f51c9aceb25ffc2125f6059 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 31 Mar 2025 17:18:59 +0530 Subject: [PATCH 12/12] chore: bump openedx-learning version --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 270e4eedbb5e..2870901ee6be 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -112,7 +112,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 @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container +openedx-learning==0.19.2 # 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 f6a2ad78351e..fb768f360ae0 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -820,7 +820,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/kernel.in -openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container +openedx-learning==0.19.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 25c6f950eefd..aaecf53c55c1 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1383,7 +1383,7 @@ openedx-forum==0.1.9 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container +openedx-learning==0.19.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 5ed792f95a24..6a22620520c8 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -992,7 +992,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/base.txt -openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container +openedx-learning==0.19.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index ea71d69fe4f1..a677d1ab17f5 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1050,7 +1050,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/base.txt -openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@navin/fal-4109/add-components-to-container +openedx-learning==0.19.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt