diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 333cdbeb76..80628dc2f2 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -335,8 +335,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> """ from cms.djangoapps.contentstore.views.preview import _load_preview_block - if not content_staging_api: - raise RuntimeError("The required content_staging app is not installed") user_clipboard = content_staging_api.get_user_clipboard(request.user.id) if not user_clipboard: # Clipboard is empty or expired/error/loading @@ -384,8 +382,6 @@ def import_static_assets_for_library_sync(downstream_xblock: XBlock, lib_block: """ if not lib_block.runtime.get_block_assets(lib_block, fetch_asset_data=False): return StaticFileNotices() - if not content_staging_api: - raise RuntimeError("The required content_staging app is not installed") staged_content = content_staging_api.stage_xblock_temporarily(lib_block, request.user.id, LIBRARY_SYNC_PURPOSE) if not staged_content: # expired/error/loading diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index 5a328594cc..02e725a968 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -3,21 +3,25 @@ Content libraries API methods related to XBlocks/Components. These methods don't enforce permissions (only the REST APIs do). """ +from __future__ import annotations import logging import mimetypes from dataclasses import dataclass from datetime import datetime, timezone +from typing import TYPE_CHECKING +from uuid import uuid4 from django.conf import settings -from django.core.exceptions import ObjectDoesNotExist +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import validate_unicode_slug from django.db import transaction from django.db.models import QuerySet from django.urls import reverse +from django.utils.text import slugify from django.utils.translation import gettext as _ from lxml import etree -from opaque_keys.edx.keys import UsageKeyV2 from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from opaque_keys.edx.keys import LearningContextKey, UsageKeyV2 from openedx_events.content_authoring.data import ( ContentObjectChangedData, LibraryBlockData, @@ -36,7 +40,6 @@ from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Component, ComponentVersion, LearningPackage, MediaType from xblock.core import XBlock -from openedx.core.djangoapps.content_libraries import api as lib_api from openedx.core.djangoapps.xblock.api import ( get_component_from_usage_key, get_xblock_app_config, @@ -49,9 +52,18 @@ from ..permissions import CAN_EDIT_THIS_CONTENT_LIBRARY from .exceptions import ( BlockLimitReachedError, ContentLibraryBlockNotFound, + IncompatibleTypesError, InvalidNameError, LibraryBlockAlreadyExists, ) +from .containers import ( + create_container, + get_container, + get_containers_contains_component, + update_container_children, + ContainerMetadata, + ContainerType, +) from .libraries import ( library_collection_locator, library_component_usage_key, @@ -59,6 +71,12 @@ from .libraries import ( PublishableItem, ) +# This content_libraries API is sometimes imported in the LMS (should we prevent that?), but the content_staging app +# cannot be. For now we only need this one type import at module scope, so only import it during type checks. +# To use the content_staging API or other CMS-only code, we import it within the functions below. +if TYPE_CHECKING: + from openedx.core.djangoapps.content_staging.api import StagedContentFileData + log = logging.getLogger(__name__) # The public API is only the following symbols: @@ -275,7 +293,7 @@ def set_library_block_olx(usage_key: LibraryUsageLocatorV2, new_olx_str: str) -> # For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger # container indexing asynchronously. - affected_containers = lib_api.get_containers_contains_component(usage_key) + affected_containers = get_containers_contains_component(usage_key) for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( @@ -315,7 +333,11 @@ def validate_can_add_block_to_library( # Make sure the proposed ID will be valid: validate_unicode_slug(block_id) # Ensure the XBlock type is valid and installed: - XBlock.load_class(block_type) # Will raise an exception if invalid + block_class = XBlock.load_class(block_type) # Will raise an exception if invalid + if block_class.has_children: + raise IncompatibleTypesError( + 'The "{block_type}" XBlock (ID: "{block_id}") has children, so it not supported in content libraries', + ) # Make sure the new ID is not taken already: usage_key = LibraryUsageLocatorV2( # type: ignore[abstract] lib_key=library_key, @@ -361,29 +383,49 @@ def create_library_block( return get_library_block(usage_key) -def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, user, block_id) -> XBlock: +def _title_from_olx_node(olx_node) -> str: + """ + Given an OLX XML node (etree node), find an appropriate title for that + XBlock. + """ + title = olx_node.attrib.get("display_name") + if not title: + # Find a localized default title if none was set: + from cms.djangoapps.contentstore import helpers as studio_helpers + title = studio_helpers.xblock_type_display_name(olx_node.tag) + return title + + +def _import_staged_block( + block_type: str, + olx_str: str, + library_key: LibraryLocatorV2, + source_context_key: LearningContextKey, + user, + staged_content_id: int, + staged_content_files: list[StagedContentFileData], + now: datetime, +) -> LibraryXBlockMetadata: """ Create a new library block and populate it with staged content from clipboard Returns the newly created library block """ from openedx.core.djangoapps.content_staging import api as content_staging_api - if not content_staging_api: - raise RuntimeError("The required content_staging app is not installed") - user_clipboard = content_staging_api.get_user_clipboard(user) - if not user_clipboard: - return None - - staged_content_id = user_clipboard.content.id - olx_str = content_staging_api.get_staged_content_olx(staged_content_id) - if olx_str is None: - return None # Shouldn't happen since we checked that the clipboard exists - mostly here for type checker - staged_content_files = content_staging_api.get_staged_content_static_files(staged_content_id) + # Generate a block_id: + try: + olx_node = etree.fromstring(olx_str) + title = _title_from_olx_node(olx_node) + # Slugify the title and append some random numbers to make a unique slug + block_id = slugify(title, allow_unicode=True) + '-' + uuid4().hex[-6:] + except Exception: # pylint: disable=broad-except + # Just generate a random block_id if we can't make a nice slug. + block_id = uuid4().hex[-12:] content_library, usage_key = validate_can_add_block_to_library( library_key, - user_clipboard.content.block_type, + block_type, block_id ) @@ -392,10 +434,8 @@ def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, use # without one at the moment. TODO: fix this at the model level. learning_package: LearningPackage = content_library.learning_package # type: ignore - now = datetime.now(tz=timezone.utc) - # Create component for block then populate it with clipboard data - with transaction.atomic(): + with transaction.atomic(savepoint=False): # First create the Component, but do not initialize it to anything (i.e. # no ComponentVersion). component_type = authoring_api.get_or_create_component_type( @@ -448,7 +488,7 @@ def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, use # Learning Core components can store other kinds of files if they # wish (though none currently do). source_assumes_global_assets = not isinstance( - user_clipboard.source_context_key, LibraryLocatorV2 + source_context_key, LibraryLocatorV2 ) if source_assumes_global_assets: filename = f"static/{filename}" @@ -484,6 +524,109 @@ def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, use return get_library_block(usage_key) +def _import_staged_block_as_container( + olx_str: str, + library_key: LibraryLocatorV2, + source_context_key: LearningContextKey, + user, + staged_content_id: int, + staged_content_files: list[StagedContentFileData], + now: datetime, +) -> ContainerMetadata: + """ + Convert the given XBlock (e.g. "vertical") to a Container (e.g. Unit) and + import it into the library, along with all its child XBlocks. + """ + olx_node = etree.fromstring(olx_str) + if olx_node.tag != "vertical": + raise ValueError("This method is only designed to work with XBlocks (units).") + # The olx_str looks like this: + # ...[XML]......[XML]...... + # Ideally we could split it up and preserve the strings, but that is difficult to do correctly, so we'll split + # it up using the XML nodes. This will unfortunately remove any custom comments or formatting in the XML, but that's + # OK since Studio-edited blocks won't have that anyways (hand-edited and library blocks can and do). + + title = _title_from_olx_node(olx_node) + + # Start an atomic section so the whole paste succeeds or fails together: + with transaction.atomic(): + container = create_container( + library_key=library_key, + container_type=ContainerType.Unit, + slug=None, # auto-generate slug from title + title=title, + user_id=user.id, + ) + new_child_keys: list[UsageKeyV2] = [] + for child_node in olx_node: + try: + child_metadata = _import_staged_block( + block_type=child_node.tag, + olx_str=etree.tostring(child_node, encoding='unicode'), + library_key=library_key, + source_context_key=source_context_key, + user=user, + staged_content_id=staged_content_id, + staged_content_files=staged_content_files, + now=now, + ) + new_child_keys.append(child_metadata.usage_key) + except IncompatibleTypesError: + continue # Skip blocks that won't work in libraries + update_container_children(container.container_key, new_child_keys, user_id=user.id) + # Re-fetch the container because the 'last_draft_created' will have changed when we added children + container = get_container(container.container_key) + return container + + +def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, user) -> PublishableItem: + """ + Create a new library item from the staged content from clipboard. + Can create containers (e.g. units) or XBlocks. + + Returns the newly created item metadata + """ + from openedx.core.djangoapps.content_staging import api as content_staging_api + + user_clipboard = content_staging_api.get_user_clipboard(user) + if not user_clipboard: + raise ValidationError("The user's clipboard is empty") + + staged_content_id = user_clipboard.content.id + source_context_key: LearningContextKey = user_clipboard.source_context_key + + staged_content_files = content_staging_api.get_staged_content_static_files(staged_content_id) + + olx_str = content_staging_api.get_staged_content_olx(staged_content_id) + if olx_str is None: + raise RuntimeError("olx_str missing") # Shouldn't happen - mostly here for type checker + + now = datetime.now(tz=timezone.utc) + + if user_clipboard.content.block_type == "vertical": + # This is a Unit. To import it into a library, we have to create it as a container. + return _import_staged_block_as_container( + olx_str, + library_key, + source_context_key, + user, + staged_content_id, + staged_content_files, + now, + ) + else: + return _import_staged_block( + user_clipboard.content.block_type, + olx_str, + library_key, + source_context_key, + user, + staged_content_id, + staged_content_files, + now, + ) + + def get_or_create_olx_media_type(block_type: str) -> MediaType: """ Get or create a MediaType for the block type. @@ -503,7 +646,7 @@ def delete_library_block(usage_key: LibraryUsageLocatorV2, remove_from_parent=Tr component = get_component_from_usage_key(usage_key) library_key = usage_key.context_key affected_collections = authoring_api.get_entity_collections(component.learning_package_id, component.key) - affected_containers = lib_api.get_containers_contains_component(usage_key) + affected_containers = get_containers_contains_component(usage_key) authoring_api.soft_delete_draft(component.pk) @@ -587,7 +730,7 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2) -> None: # container indexing asynchronously. # # To update the components count in containers - affected_containers = lib_api.get_containers_contains_component(usage_key) + affected_containers = get_containers_contains_component(usage_key) for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 526a556a8e..71b582f15b 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -167,6 +167,7 @@ def create_container( slug: str | None, title: str, user_id: int | None, + created: datetime | None = None, ) -> ContainerMetadata: """ Create a container (e.g. a Unit) in the specified content library. @@ -192,7 +193,7 @@ def create_container( content_library.learning_package_id, key=slug, title=title, - created=datetime.now(), + created=created or datetime.now(), created_by=user_id, ) case _: diff --git a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py index 5c026fecee..72423e0f79 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py @@ -110,7 +110,7 @@ from openedx.core.djangoapps.content_libraries.rest_api.serializers import ( LibraryXBlockMetadataSerializer, LibraryXBlockTypeSerializer, ContentLibraryAddPermissionByEmailSerializer, - LibraryPasteClipboardSerializer, + PublishableItemSerializer, ) import openedx.core.djangoapps.site_configuration.helpers as configuration_helpers from openedx.core.lib.api.view_utils import view_auth_classes @@ -485,12 +485,11 @@ class LibraryPasteClipboardView(GenericAPIView): """ Paste content of clipboard into Library. """ - serializer_class = LibraryXBlockMetadataSerializer + serializer_class = PublishableItemSerializer @convert_exceptions @swagger_auto_schema( - request_body=LibraryPasteClipboardSerializer, - responses={200: LibraryXBlockMetadataSerializer} + responses={200: PublishableItemSerializer} ) def post(self, request, lib_key_str): """ @@ -498,19 +497,13 @@ class LibraryPasteClipboardView(GenericAPIView): """ 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 = LibraryPasteClipboardSerializer(data=request.data) - serializer.is_valid(raise_exception=True) try: - result = api.import_staged_content_from_user_clipboard( - library_key, request.user, **serializer.validated_data - ) + result = api.import_staged_content_from_user_clipboard(library_key, request.user) except api.IncompatibleTypesError as err: - raise ValidationError( # lint-amnesty, pylint: disable=raise-missing-from - detail={'block_type': str(err)}, - ) + raise ValidationError(detail={'block_type': str(err)}) from err - return Response(LibraryXBlockMetadataSerializer(result).data) + return Response(PublishableItemSerializer(result).data) @method_decorator(non_atomic_requests, name="dispatch") diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index d171afe6ca..707c45d8ce 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -12,7 +12,7 @@ from opaque_keys.edx.locator import LibraryContainerLocator from opaque_keys import InvalidKeyError from openedx_learning.api.authoring_models import Collection -from openedx.core.djangoapps.content_libraries.api.containers import ContainerMetadata, ContainerType +from openedx.core.djangoapps.content_libraries.api.containers import ContainerType from openedx.core.djangoapps.content_libraries.constants import ( ALL_RIGHTS_RESERVED, LICENSE_OPTIONS, @@ -133,20 +133,12 @@ class CollectionMetadataSerializer(serializers.Serializer): title = serializers.CharField() -class LibraryXBlockMetadataSerializer(serializers.Serializer): +class PublishableItemSerializer(serializers.Serializer): """ - Serializer for LibraryXBlockMetadata + Serializer for any PublishableItem in a library (XBlock, Container, etc.) """ - id = serializers.CharField(source="usage_key", read_only=True) - - # TODO: Remove this serializer field once the frontend no longer relies on - # it. Learning Core doesn't use definition IDs, but we're passing this dummy - # value back to preserve the REST API contract (just to reduce the number of - # things we're changing at one time). - def_key = serializers.ReadOnlyField(default=None) - - block_type = serializers.CharField(source="usage_key.block_type") - display_name = serializers.CharField(read_only=True) + id = serializers.SerializerMethodField() + display_name = serializers.CharField() last_published = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) published_by = serializers.CharField(read_only=True) last_draft_created = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) @@ -163,6 +155,25 @@ class LibraryXBlockMetadataSerializer(serializers.Serializer): collections = CollectionMetadataSerializer(many=True, required=False) can_stand_alone = serializers.BooleanField(read_only=True) + # Fields that are _sometimes_ set, depending on the subclass: + block_type = serializers.CharField(source="usage_key.block_type", required=False) + container_type = serializers.CharField(source="container_key.block_type", required=False) + + def get_id(self, obj) -> str: + """ Get a unique ID for this PublishableItem """ + if hasattr(obj, "usage_key"): + return str(obj.usage_key) + elif hasattr(obj, "container_key"): + return str(obj.container_key) + return "" + + +class LibraryXBlockMetadataSerializer(PublishableItemSerializer): + """ + Serializer for LibraryXBlockMetadata + """ + block_type = serializers.CharField(source="usage_key.block_type") + class LibraryXBlockTypeSerializer(serializers.Serializer): """ @@ -200,13 +211,6 @@ class LibraryXBlockCreationSerializer(serializers.Serializer): can_stand_alone = serializers.BooleanField(required=False, default=True) -class LibraryPasteClipboardSerializer(serializers.Serializer): - """ - Serializer for pasting clipboard data into library - """ - block_id = serializers.CharField(validators=(validate_unicode_slug, )) - - class LibraryXBlockOlxSerializer(serializers.Serializer): """ Serializer for representing an XBlock's OLX @@ -237,35 +241,19 @@ class LibraryXBlockStaticFilesSerializer(serializers.Serializer): files = LibraryXBlockStaticFileSerializer(many=True) -class LibraryContainerMetadataSerializer(serializers.Serializer): +class LibraryContainerMetadataSerializer(PublishableItemSerializer): """ Serializer for Containers like Sections, Subsections, Units Converts from ContainerMetadata to JSON-compatible data """ - container_key = serializers.CharField(read_only=True) - container_type = serializers.CharField() - display_name = serializers.CharField() - last_published = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) - published_by = serializers.CharField(read_only=True) - last_draft_created = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) - last_draft_created_by = serializers.CharField(read_only=True) - has_unpublished_changes = serializers.BooleanField(read_only=True) - created = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) - modified = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) - tags_count = serializers.IntegerField(read_only=True) - collections = CollectionMetadataSerializer(many=True, required=False, read_only=True) + # Use 'source' to get this as a string, not an enum value instance which the container_type field has. + container_type = serializers.CharField(source="container_key.container_type") # When creating a new container in a library, the slug becomes the ID part of # the definition key and usage key: slug = serializers.CharField(write_only=True, required=False) - def to_representation(self, instance: ContainerMetadata): - """ Convert to JSON-serializable data types """ - data = super().to_representation(instance) - data["container_type"] = instance.container_type.value # Force to a string, not an enum value instance - return data - def to_internal_value(self, data): """ Convert JSON-ish data back to native python types. diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index c015886af7..195805f94e 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -306,11 +306,10 @@ class ContentLibrariesRestApiTest(APITransactionTestCase): """ Publish changes from a specified XBlock """ return self._api('post', URL_LIB_BLOCK_PUBLISH.format(block_key=block_key), None, expect_response) - def _paste_clipboard_content_in_library(self, lib_key, block_id, expect_response=200): + def _paste_clipboard_content_in_library(self, lib_key, expect_response=200): """ Paste's the users clipboard content into Library """ url = URL_LIB_PASTE_CLIPBOARD.format(lib_key=lib_key) - data = {"block_id": block_id} - return self._api('post', url, data, expect_response) + return self._api('post', url, {}, expect_response) def _render_block_view(self, block_key, view_name, version=None, expect_response=200): """ diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 7a20f75f85..8f79ec7f63 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -446,7 +446,7 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe opaque_keys=[ UsageKey.from_string(self.lib1_problem_block["id"]), UsageKey.from_string(self.lib1_html_block["id"]), - LibraryContainerLocator.from_string(self.unit1["container_key"]), + LibraryContainerLocator.from_string(self.unit1["id"]), ], ) assert len(self.col1.entities.all()) == 3 @@ -475,7 +475,7 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe opaque_keys=[ UsageKey.from_string(self.lib1_problem_block["id"]), UsageKey.from_string(self.lib1_html_block["id"]), - LibraryContainerLocator.from_string(self.unit1["container_key"]), + LibraryContainerLocator.from_string(self.unit1["id"]), ], ) @@ -507,7 +507,7 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "sender": None, "content_object": ContentObjectChangedData( - object_id=self.unit1["container_key"], + object_id=self.unit1["id"], changes=["collections"], ), }, @@ -535,7 +535,7 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe opaque_keys=[ UsageKey.from_string(self.lib1_problem_block["id"]), UsageKey.from_string(self.lib1_html_block["id"]), - LibraryContainerLocator.from_string(self.unit1["container_key"]), + LibraryContainerLocator.from_string(self.unit1["id"]), ], ) assert self.lib1_problem_block["id"] in str(exc.exception) @@ -634,14 +634,14 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe opaque_keys=[ UsageKey.from_string(self.lib1_problem_block["id"]), UsageKey.from_string(self.lib1_html_block["id"]), - LibraryContainerLocator.from_string(self.unit1["container_key"]), + LibraryContainerLocator.from_string(self.unit1["id"]), ], ) event_receiver = mock.Mock() LIBRARY_COLLECTION_UPDATED.connect(event_receiver) - api.delete_container(LibraryContainerLocator.from_string(self.unit1["container_key"])) + api.delete_container(LibraryContainerLocator.from_string(self.unit1["id"])) assert event_receiver.call_count == 1 self.assertDictContainsSubset( diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 41752eb41e..10f7f269c6 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -69,7 +69,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): with freeze_time(create_date): container_data = self._create_container(lib["id"], "unit", slug="u1", display_name="Test Unit") expected_data = { - "container_key": "lct:CL-TEST:containers:unit:u1", + "id": "lct:CL-TEST:containers:unit:u1", "container_type": "unit", "display_name": "Test Unit", "last_published": None, @@ -99,7 +99,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): ) # Fetch the unit: - unit_as_read = self._get_container(container_data["container_key"]) + unit_as_read = self._get_container(container_data["id"]) # make sure it contains the same data when we read it back: self.assertDictContainsEntries(unit_as_read, expected_data) @@ -124,13 +124,13 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): ) # Re-fetch the unit - unit_as_re_read = self._get_container(container_data["container_key"]) + unit_as_re_read = self._get_container(container_data["id"]) # make sure it contains the same data when we read it back: self.assertDictContainsEntries(unit_as_re_read, expected_data) # Delete the unit - self._delete_container(container_data["container_key"]) - self._get_container(container_data["container_key"], expect_response=404) + self._delete_container(container_data["id"]) + self._get_container(container_data["id"], expect_response=404) assert delete_receiver.call_count == 1 self.assertDictContainsSubset( { @@ -153,17 +153,17 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): random_user = UserFactory.create(username="Random", email="random@example.com") with self.as_user(random_user): self._create_container(lib["id"], "unit", slug="u3", display_name="Test Unit", expect_response=403) - self._get_container(container_data["container_key"], expect_response=403) - self._update_container(container_data["container_key"], display_name="Unit ABC", expect_response=403) - self._delete_container(container_data["container_key"], expect_response=403) + self._get_container(container_data["id"], expect_response=403) + self._update_container(container_data["id"], display_name="Unit ABC", expect_response=403) + self._delete_container(container_data["id"], expect_response=403) # Granting read-only permissions on the library should only allow retrieval, nothing else. self._add_user_by_email(lib["id"], random_user.email, access_level="read") with self.as_user(random_user): self._create_container(lib["id"], "unit", slug="u2", display_name="Test Unit", expect_response=403) - self._get_container(container_data["container_key"], expect_response=200) - self._update_container(container_data["container_key"], display_name="Unit ABC", expect_response=403) - self._delete_container(container_data["container_key"], expect_response=403) + self._get_container(container_data["id"], expect_response=200) + self._update_container(container_data["id"], display_name="Unit ABC", expect_response=403) + self._delete_container(container_data["id"], expect_response=403) def test_unit_gets_auto_slugs(self): """ @@ -176,9 +176,9 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): container1_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) container2_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) # Notice the container IDs below are slugified from the title: "alpha-bravo-NNNNN" - 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"] + assert container1_data["id"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") + assert container2_data["id"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") + assert container1_data["id"] != container2_data["id"] def test_unit_add_children(self): """ @@ -194,10 +194,10 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): 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"], + container_data["id"], children_ids=[problem_block["id"], html_block["id"]] ) - data = self._get_container_components(container_data["container_key"]) + data = self._get_container_components(container_data["id"]) assert len(data) == 2 assert data[0]['id'] == problem_block['id'] assert not data[0]['can_stand_alone'] @@ -207,7 +207,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") # Add two more components self._add_container_components( - container_data["container_key"], + container_data["id"], children_ids=[problem_block_2["id"], html_block_2["id"]] ) self.assertDictContainsSubset( @@ -216,13 +216,13 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "sender": None, "library_container": LibraryContainerData( container_key=LibraryContainerLocator.from_string( - container_data["container_key"], + container_data["id"], ), ), }, update_receiver.call_args_list[0].kwargs, ) - data = self._get_container_components(container_data["container_key"]) + data = self._get_container_components(container_data["id"]) # Verify total number of components to be 2 + 2 = 4 assert len(data) == 4 assert data[2]['id'] == problem_block_2['id'] @@ -246,17 +246,17 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): 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"], + container_data["id"], 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"]) + data = self._get_container_components(container_data["id"]) assert len(data) == 4 # Remove both problem blocks. self._remove_container_components( - container_data["container_key"], + container_data["id"], children_ids=[problem_block_2["id"], problem_block["id"]] ) - data = self._get_container_components(container_data["container_key"]) + data = self._get_container_components(container_data["id"]) assert len(data) == 2 assert data[0]['id'] == html_block['id'] assert data[1]['id'] == html_block_2['id'] @@ -266,7 +266,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "sender": None, "library_container": LibraryContainerData( container_key=LibraryContainerLocator.from_string( - container_data["container_key"], + container_data["id"], ), ), }, @@ -289,10 +289,10 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): 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"], + container_data["id"], 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"]) + data = self._get_container_components(container_data["id"]) assert len(data) == 4 assert data[0]['id'] == problem_block['id'] assert data[1]['id'] == html_block['id'] @@ -301,10 +301,10 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): # Reorder the components self._patch_container_components( - container_data["container_key"], + container_data["id"], 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"]) + data = self._get_container_components(container_data["id"]) assert len(data) == 4 assert data[0]['id'] == problem_block['id'] assert data[1]['id'] == problem_block_2['id'] @@ -315,10 +315,10 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): 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"], + container_data["id"], children_ids=[new_problem_block["id"], new_html_block["id"]], ) - data = self._get_container_components(container_data["container_key"]) + data = self._get_container_components(container_data["id"]) assert len(data) == 2 assert data[0]['id'] == new_problem_block['id'] assert data[1]['id'] == new_html_block['id'] @@ -328,7 +328,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): "sender": None, "library_container": LibraryContainerData( container_key=LibraryContainerLocator.from_string( - container_data["container_key"], + container_data["id"], ), ), }, @@ -348,16 +348,16 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): container_data = self._create_container(lib["id"], "unit", slug="u1", display_name="Test Unit") # Delete the unit - self._delete_container(container_data["container_key"]) + self._delete_container(container_data["id"]) create_receiver = mock.Mock() LIBRARY_CONTAINER_CREATED.connect(create_receiver) # Restore container - self._restore_container(container_data["container_key"]) - new_container_data = self._get_container(container_data["container_key"]) + self._restore_container(container_data["id"]) + new_container_data = self._get_container(container_data["id"]) expected_data = { - "container_key": "lct:CL-TEST:containers:unit:u1", + "id": "lct:CL-TEST:containers:unit:u1", "container_type": "unit", "display_name": "Test Unit", "last_published": None, @@ -402,14 +402,14 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): ) result = self._patch_container_collections( - container_data["container_key"], + container_data["id"], collection_keys=[col1.key], ) assert result['count'] == 1 # Fetch the unit - unit_as_read = self._get_container(container_data["container_key"]) + unit_as_read = self._get_container(container_data["id"]) # Verify the collections assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.key}] diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index bcb1573a27..e1b34ebfcb 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -4,7 +4,6 @@ Tests for Learning-Core-based Content Libraries from datetime import datetime, timezone from unittest import skip from unittest.mock import Mock, patch -from uuid import uuid4 import ddt from django.contrib.auth.models import Group @@ -344,9 +343,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix "last_draft_created_by": "Bob", }) block_id = block_data["id"] - # Confirm that the result contains a definition key, but don't check its value, - # which for the purposes of these tests is an implementation detail. - assert 'def_key' in block_data # now the library should contain one block and have unpublished changes: assert self._get_library_blocks(lib_id)['results'] == [block_data] @@ -509,7 +505,7 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix """ lib = self._create_library(slug="list_blocks-slug", title="Library 1") block1 = self._add_block_to_library(lib["id"], "problem", "problem1") - self._add_block_to_library(lib["id"], "unit", "unit1") + self._add_block_to_library(lib["id"], "html", "html1") response = self._get_library_blocks(lib["id"]) result = response['results'] @@ -792,7 +788,7 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix description="Testing XBlocks limits in a library" ) lib_id = lib["id"] - self._add_block_to_library(lib_id, "unit", "unit1") + self._add_block_to_library(lib_id, "html", "html1") # Second block should throw error self._add_block_to_library(lib_id, "problem", "problem1", expect_response=400) @@ -981,14 +977,14 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix library_key = LibraryLocatorV2.from_string(lib_id) - block = self._add_block_to_library(lib_id, "unit", "u1") + block = self._add_block_to_library(lib_id, "html", "h1") block_id = block["id"] self._set_library_block_asset(block_id, "static/test.txt", b"data") usage_key = LibraryUsageLocatorV2( lib_key=library_key, - block_type="unit", - usage_id="u1" + block_type="html", + usage_id="h1" ) event_receiver.assert_called_once() @@ -1020,7 +1016,7 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix library_key = LibraryLocatorV2.from_string(lib_id) - block = self._add_block_to_library(lib_id, "unit", "u1") + block = self._add_block_to_library(lib_id, "html", "h321") block_id = block["id"] self._set_library_block_asset(block_id, "static/test.txt", b"data") @@ -1028,8 +1024,8 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix usage_key = LibraryUsageLocatorV2( lib_key=library_key, - block_type="unit", - usage_id="u1" + block_type="html", + usage_id="h321" ) event_receiver.assert_called() @@ -1084,7 +1080,7 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix event_receiver.call_args.kwargs ) - def test_library_paste_clipboard(self): + def test_library_paste_xblock(self): """ Check the a new block is created in the library after pasting from clipboard. The content of the new block should match the content of the block in the clipboard. @@ -1123,13 +1119,8 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix save_xblock_to_user_clipboard(block, author.id) # Paste the content of the clipboard into the library - pasted_block_id = str(uuid4()) - paste_data = self._paste_clipboard_content_in_library(lib_id, pasted_block_id) - pasted_usage_key = LibraryUsageLocatorV2( - lib_key=library_key, - block_type="problem", - usage_id=pasted_block_id - ) + paste_data = self._paste_clipboard_content_in_library(lib_id) + pasted_usage_key = LibraryUsageLocatorV2.from_string(paste_data["id"]) self._get_library_block_asset(pasted_usage_key, "static/hello.txt") # Compare the two text files @@ -1145,7 +1136,7 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix "last_draft_created": paste_data["last_draft_created"], "created": paste_data["created"], "modified": paste_data["modified"], - "id": f"lb:CL-TEST:test_lib_paste_clipboard:problem:{pasted_block_id}", + "id": f"lb:CL-TEST:test_lib_paste_clipboard:problem:{pasted_usage_key.block_id}", }) @override_settings(LIBRARY_ENABLED_BLOCKS=['problem', 'video', 'html']) 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 new file mode 100644 index 0000000000..e6b379d298 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py @@ -0,0 +1,87 @@ +""" +Tests for Imports from Courses to Learning-Core-based Content Libraries +""" +import ddt +from opaque_keys.edx.locator import LibraryContainerLocator +from openedx_events.content_authoring import signals +from openedx_events.tests.utils import OpenEdxEventsTestMixin + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import ToyCourseFactory +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest +from openedx.core.djangolib.testing.utils import skip_unless_cms + + +@skip_unless_cms +@ddt.ddt +class CourseToLibraryTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest, ModuleStoreTestCase): + """ + Tests that involve copying content from courses to libraries. + """ + ENABLED_OPENEDX_EVENTS = [ + signals.LIBRARY_CONTAINER_CREATED.event_type, + signals.LIBRARY_CONTAINER_DELETED.event_type, + signals.LIBRARY_CONTAINER_UPDATED.event_type, + ] + + def test_library_paste_unit_from_course(self): + """ + Test that we can paste a Unit from a course into a Library, and it gets + converted from an XBlock to a Container. + """ + # Create user to perform tests on + author = UserFactory.create(username="Author", email="author@example.com", is_staff=True) + with self.as_user(author): + lib = self._create_library( + slug="test_lib_paste_clipboard", + title="Paste Clipboard Test Library", + description="Testing pasting clipboard in library" + ) + lib_id = lib["id"] + + course_key = ToyCourseFactory.create().id # See xmodule/modulestore/tests/sample_courses.py + unit_key = course_key.make_usage_key("vertical", "vertical_test") + + # Copy the unit + self._api('post', "/api/content-staging/v1/clipboard/", {"usage_key": str(unit_key)}, expect_response=200) + + # Paste the content of the clipboard into the library + paste_data = self._paste_clipboard_content_in_library(lib_id) + pasted_container_key = LibraryContainerLocator.from_string(paste_data["id"]) + + self.assertDictContainsEntries(self._get_container(paste_data["id"]), { + "id": str(pasted_container_key), + "container_type": "unit", + "display_name": "Unit", + "last_draft_created_by": author.username, + "last_draft_created": paste_data["last_draft_created"], + "created": paste_data["created"], + "modified": paste_data["modified"], + "last_published": None, + "published_by": "", + "has_unpublished_changes": True, + "collections": [], + "can_stand_alone": True, + }) + + children = self._get_container_components(paste_data["id"]) + assert len(children) == 4 + + self.assertDictContainsEntries(children[0], {"display_name": "default", "block_type": "video"}) + assert children[0]["id"].startswith("lb:CL-TEST:test_lib_paste_clipboard:video:default-") + assert "container_type" not in children[0] + + self.assertDictContainsEntries(children[1], {"display_name": "default", "block_type": "video"}) + assert children[1]["id"].startswith("lb:CL-TEST:test_lib_paste_clipboard:video:default-") + assert children[0]["id"] != children[1]["id"] + + self.assertDictContainsEntries(children[2], {"display_name": "default", "block_type": "video"}) + assert children[2]["id"].startswith("lb:CL-TEST:test_lib_paste_clipboard:video:default-") + assert children[0]["id"] != children[2]["id"] + + self.assertDictContainsEntries(children[3], { + "display_name": "Change your answer", + "block_type": "poll_question", + }) + assert children[3]["id"].startswith("lb:CL-TEST:test_lib_paste_clipboard:poll_question:change-your-answer-") diff --git a/openedx/core/djangoapps/content_staging/api.py b/openedx/core/djangoapps/content_staging/api.py index acc920fd6c..7f8167d9aa 100644 --- a/openedx/core/djangoapps/content_staging/api.py +++ b/openedx/core/djangoapps/content_staging/api.py @@ -110,17 +110,17 @@ def _save_static_assets_to_staged_content( ) # Compute the MD5 hash and get the content: content: bytes | None = f.data - md5_hash = "" # Unknown if content: - md5_hash = hashlib.md5(content).hexdigest() # This asset came from the XBlock's filesystem, e.g. a video block's transcript file source_key = usage_key # Check if the asset file exists. It can be absent if an XBlock contains an invalid link. elif source_key and (sc := contentstore().find(source_key, throw_on_not_found=False)): - md5_hash = sc.content_digest content = sc.data - else: + # Note that sc.content_digest has an md5_hash but it's sometimes NULL so we just compute it ourselves. + if not content: continue # Skip this file - we don't need a reference to a non-existent file. + # Compute the md5 hash + md5_hash = hashlib.md5(content).hexdigest() # Because we store clipboard files on S3, uploading really large files will be too slow. And it's wasted if # the copy-paste is just happening within a single course. So for files > 10MB, users must copy the files