diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index e40eddb6c99e..1ceb2bc3c01d 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -29,6 +29,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers import openedx.core.djangoapps.content_staging.api as content_staging_api import openedx.core.djangoapps.content_tagging.api as content_tagging_api +from openedx.core.djangoapps.content_staging.data import LIBRARY_SYNC_PURPOSE from .utils import reverse_course_url, reverse_library_url, reverse_usage_url @@ -262,6 +263,37 @@ class StaticFileNotices: error_files: list[str] = Factory(list) +def _insert_static_files_into_downstream_xblock( + downstream_xblock: XBlock, staged_content_id: int, request +) -> StaticFileNotices: + """ + Gets static files from staged content, and inserts them into the downstream XBlock. + """ + static_files = content_staging_api.get_staged_content_static_files(staged_content_id) + notices, substitutions = _import_files_into_course( + course_key=downstream_xblock.context_key, + staged_content_id=staged_content_id, + static_files=static_files, + usage_key=downstream_xblock.scope_ids.usage_id, + ) + + # Rewrite the OLX's static asset references to point to the new + # locations for those assets. See _import_files_into_course for more + # info on why this is necessary. + store = modulestore() + if hasattr(downstream_xblock, "data") and substitutions: + data_with_substitutions = downstream_xblock.data + for old_static_ref, new_static_ref in substitutions.items(): + data_with_substitutions = data_with_substitutions.replace( + old_static_ref, + new_static_ref, + ) + downstream_xblock.data = data_with_substitutions + if store is not None: + store.update_item(downstream_xblock, request.user.id) + return notices + + def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> tuple[XBlock | None, StaticFileNotices]: """ Import a block (along with its children and any required static assets) from @@ -299,31 +331,43 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> tags=user_clipboard.content.tags, ) - # Now handle static files that need to go into Files & Uploads. - static_files = content_staging_api.get_staged_content_static_files(user_clipboard.content.id) - notices, substitutions = _import_files_into_course( - course_key=parent_key.context_key, - staged_content_id=user_clipboard.content.id, - static_files=static_files, - usage_key=new_xblock.scope_ids.usage_id, - ) - - # Rewrite the OLX's static asset references to point to the new - # locations for those assets. See _import_files_into_course for more - # info on why this is necessary. - if hasattr(new_xblock, 'data') and substitutions: - data_with_substitutions = new_xblock.data - for old_static_ref, new_static_ref in substitutions.items(): - data_with_substitutions = data_with_substitutions.replace( - old_static_ref, - new_static_ref, - ) - new_xblock.data = data_with_substitutions - store.update_item(new_xblock, request.user.id) + notices = _insert_static_files_into_downstream_xblock(new_xblock, user_clipboard.content.id, request) return new_xblock, notices +def import_static_assets_for_library_sync(downstream_xblock: XBlock, lib_block: XBlock, request) -> StaticFileNotices: + """ + Import the static assets from the library xblock to the downstream xblock + through staged content. Also updates the OLX references to point to the new + locations of those assets in the downstream course. + + Does not deal with permissions or REST stuff - do that before calling this. + + Returns a summary of changes made to static files in the destination + course. + """ + 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 + return StaticFileNotices() + + store = modulestore() + try: + with store.bulk_operations(downstream_xblock.context_key): + # Now handle static files that need to go into Files & Uploads. + # If the required files already exist, nothing will happen besides updating the olx. + notices = _insert_static_files_into_downstream_xblock(downstream_xblock, staged_content.id, request) + finally: + staged_content.delete() + + return notices + + def _fetch_and_set_upstream_link( copied_from_block: str, copied_from_version_num: int, @@ -548,6 +592,9 @@ def _import_files_into_course( if result is True: new_files.append(file_data_obj.filename) substitutions.update(substitution_for_file) + elif substitution_for_file: + # substitutions need to be made because OLX references to these files need to be updated + substitutions.update(substitution_for_file) elif result is None: pass # This file already exists; no action needed. else: @@ -618,8 +665,8 @@ def _import_file_into_course( contentstore().save(content) return True, {clipboard_file_path: f"static/{import_path}"} elif current_file.content_digest == file_data_obj.md5_hash: - # The file already exists and matches exactly, so no action is needed - return None, {} + # The file already exists and matches exactly, so no action is needed except substitutions + return None, {clipboard_file_path: f"static/{import_path}"} else: # There is a conflict with some other file that has the same name. return False, {} diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index 8c0d651910a6..46e67e87ea0c 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -56,8 +56,10 @@ "ready_to_sync": Boolean } """ + import logging +from attrs import asdict as attrs_asdict from django.contrib.auth.models import User # pylint: disable=imported-auth-user from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey @@ -71,6 +73,7 @@ UpstreamLink, UpstreamLinkException, NoUpstream, BadUpstream, BadDownstream, fetch_customizable_fields, sync_from_upstream, decline_sync, sever_upstream_link ) +from cms.djangoapps.contentstore.helpers import import_static_assets_for_library_sync from common.djangoapps.student.auth import has_studio_write_access, has_studio_read_access from openedx.core.lib.api.view_utils import ( DeveloperErrorViewMixin, @@ -195,7 +198,8 @@ def post(self, request: _AuthenticatedRequest, usage_key_string: str) -> Respons """ downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) try: - sync_from_upstream(downstream, request.user) + upstream = sync_from_upstream(downstream, request.user) + static_file_notices = import_static_assets_for_library_sync(downstream, upstream, request) except UpstreamLinkException as exc: logger.exception( "Could not sync from upstream '%s' to downstream '%s'", @@ -206,7 +210,9 @@ def post(self, request: _AuthenticatedRequest, usage_key_string: str) -> Respons modulestore().update_item(downstream, request.user.id) # Note: We call `get_for_block` (rather than `try_get_for_block`) because if anything is wrong with the # upstream at this point, then that is completely unexpected, so it's appropriate to let the 500 happen. - return Response(UpstreamLink.get_for_block(downstream).to_json()) + response = UpstreamLink.get_for_block(downstream).to_json() + response["static_file_notices"] = attrs_asdict(static_file_notices) + return Response(response) def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: """ diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index 92da28bde989..31877b9153d5 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -4,6 +4,7 @@ from unittest.mock import patch from django.conf import settings +from cms.djangoapps.contentstore.helpers import StaticFileNotices from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.django import modulestore @@ -247,7 +248,8 @@ def call_api(self, usage_key_string): @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) @patch.object(downstreams_views, "sync_from_upstream") - def test_200(self, mock_sync_from_upstream): + @patch.object(downstreams_views, "import_static_assets_for_library_sync", return_value=StaticFileNotices()) + def test_200(self, mock_sync_from_upstream, mock_import_staged_content): """ Does the happy path work? """ @@ -255,6 +257,7 @@ def test_200(self, mock_sync_from_upstream): response = self.call_api(self.downstream_video_key) assert response.status_code == 200 assert mock_sync_from_upstream.call_count == 1 + assert mock_import_staged_content.call_count == 1 class DeleteDownstreamSyncViewtest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 3b9ba5798a01..775f08e5fa2c 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -80,6 +80,7 @@ from ..helpers import ( get_parent_xblock, import_staged_content_from_user_clipboard, + import_static_assets_for_library_sync, is_unit, xblock_embed_lms_url, xblock_lms_url, @@ -598,7 +599,7 @@ def _create_block(request): try: # Set `created_block.upstream` and then sync this with the upstream (library) version. created_block.upstream = upstream_ref - sync_from_upstream(downstream=created_block, user=request.user) + lib_block = sync_from_upstream(downstream=created_block, user=request.user) except BadUpstream as exc: _delete_item(created_block.location, request.user) log.exception( @@ -606,8 +607,10 @@ def _create_block(request): f"using provided library_content_key='{upstream_ref}'" ) return JsonResponse({"error": str(exc)}, status=400) + static_file_notices = import_static_assets_for_library_sync(created_block, lib_block, request) modulestore().update_item(created_block, request.user.id) - response['upstreamRef'] = upstream_ref + response["upstreamRef"] = upstream_ref + response["static_file_notices"] = asdict(static_file_notices) return JsonResponse(response) diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index 0d95931ce29d..418b95a39fe3 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -186,7 +186,7 @@ def get_for_block(cls, downstream: XBlock) -> t.Self: ) -def sync_from_upstream(downstream: XBlock, user: User) -> None: +def sync_from_upstream(downstream: XBlock, user: User) -> XBlock: """ Update `downstream` with content+settings from the latest available version of its linked upstream content. @@ -200,6 +200,7 @@ def sync_from_upstream(downstream: XBlock, user: User) -> None: _update_non_customizable_fields(upstream=upstream, downstream=downstream) _update_tags(upstream=upstream, downstream=downstream) downstream.upstream_version = link.version_available + return upstream def fetch_customizable_fields(*, downstream: XBlock, user: User, upstream: XBlock | None = None) -> None: diff --git a/openedx/core/djangoapps/content_staging/api.py b/openedx/core/djangoapps/content_staging/api.py index 5f85d701faa5..acc920fd6cb4 100644 --- a/openedx/core/djangoapps/content_staging/api.py +++ b/openedx/core/djangoapps/content_staging/api.py @@ -14,29 +14,36 @@ from xblock.core import XBlock from openedx.core.lib.xblock_serializer.api import StaticFile, XBlockSerializer -from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none from xmodule import block_metadata_utils from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore from .data import ( CLIPBOARD_PURPOSE, - StagedContentData, StagedContentFileData, StagedContentStatus, UserClipboardData + StagedContentData, + StagedContentFileData, + StagedContentStatus, + UserClipboardData, ) from .models import ( UserClipboard as _UserClipboard, StagedContent as _StagedContent, StagedContentFile as _StagedContentFile, ) -from .serializers import UserClipboardSerializer as _UserClipboardSerializer +from .serializers import ( + UserClipboardSerializer as _UserClipboardSerializer, +) from .tasks import delete_expired_clipboards log = logging.getLogger(__name__) -def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int | None = None) -> UserClipboardData: +def _save_xblock_to_staged_content( + block: XBlock, user_id: int, purpose: str, version_num: int | None = None +) -> _StagedContent: """ - Copy an XBlock's OLX to the user's clipboard. + Generic function to save an XBlock's OLX to staged content. + Used by both clipboard and library sync functionality. """ block_data = XBlockSerializer( block, @@ -49,7 +56,7 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int # Mark all of the user's existing StagedContent rows as EXPIRED to_expire = _StagedContent.objects.filter( user_id=user_id, - purpose=CLIPBOARD_PURPOSE, + purpose=purpose, ).exclude( status=StagedContentStatus.EXPIRED, ) @@ -60,7 +67,7 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int # Insert a new StagedContent row for this staged_content = _StagedContent.objects.create( user_id=user_id, - purpose=CLIPBOARD_PURPOSE, + purpose=purpose, status=StagedContentStatus.READY, block_type=usage_key.block_type, olx=block_data.olx_str, @@ -69,23 +76,16 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int tags=block_data.tags or {}, version_num=(version_num or 0), ) - (clipboard, _created) = _UserClipboard.objects.update_or_create(user_id=user_id, defaults={ - "content": staged_content, - "source_usage_key": usage_key, - }) # Log an event so we can analyze how this feature is used: - log.info(f"Copied {usage_key.block_type} component \"{usage_key}\" to their clipboard.") + log.info(f'Saved {usage_key.block_type} component "{usage_key}" to staged content for {purpose}.') - # Try to copy the static files. If this fails, we still consider the overall copy attempt to have succeeded, - # because intra-course pasting will still work fine, and in any case users can manually resolve the file issue. + # Try to copy the static files. If this fails, we still consider the overall save attempt to have succeeded, + # because intra-course operations will still work fine, and users can manually resolve file issues. try: - _save_static_assets_to_user_clipboard(block_data.static_files, usage_key, staged_content) + _save_static_assets_to_staged_content(block_data.static_files, usage_key, staged_content) except Exception: # pylint: disable=broad-except - # Regardless of what happened, with get_asset_key_from_path or contentstore or run_filter, we don't want the - # whole "copy to clipboard" operation to fail, which would be a bad user experience. For copying and pasting - # within a single course, static assets don't even matter. So any such errors become warnings here. - log.exception(f"Unable to copy static files to clipboard for component {usage_key}") + log.exception(f"Unable to copy static files to staged content for component {usage_key}") # Enqueue a (potentially slow) task to delete the old staged content try: @@ -93,14 +93,15 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int except Exception: # pylint: disable=broad-except log.exception(f"Unable to enqueue cleanup task for StagedContents: {','.join(str(x) for x in expired_ids)}") - return _user_clipboard_model_to_data(clipboard) + return staged_content -def _save_static_assets_to_user_clipboard( +def _save_static_assets_to_staged_content( static_files: list[StaticFile], usage_key: UsageKey, staged_content: _StagedContent ): """ - Helper method for save_xblock_to_user_clipboard endpoint. This deals with copying static files into the clipboard. + Helper method for saving static files into staged content. + Used by both clipboard and library sync functionality. """ for f in static_files: source_key = ( @@ -144,6 +145,37 @@ def _save_static_assets_to_user_clipboard( log.exception(f"Unable to copy static file {f.name} to clipboard for component {usage_key}") +def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int | None = None) -> UserClipboardData: + """ + Copy an XBlock's OLX to the user's clipboard. + """ + staged_content = _save_xblock_to_staged_content(block, user_id, CLIPBOARD_PURPOSE, version_num) + usage_key = block.usage_key + + # Create/update the clipboard entry + (clipboard, _created) = _UserClipboard.objects.update_or_create( + user_id=user_id, + defaults={ + "content": staged_content, + "source_usage_key": usage_key, + }, + ) + + return _user_clipboard_model_to_data(clipboard) + + +def stage_xblock_temporarily( + block: XBlock, user_id: int, purpose: str, version_num: int | None = None, +) -> _StagedContent: + """ + "Stage" an XBlock by copying it (and its associated children + static assets) + into the content staging area. This XBlock can then be accessed and manipulated + using any of the staged content APIs, before being deleted. + """ + staged_content = _save_xblock_to_staged_content(block, user_id, purpose, version_num) + return staged_content + + def get_user_clipboard(user_id: int, only_ready: bool = True) -> UserClipboardData | None: """ Get the details of the user's clipboard. @@ -190,28 +222,29 @@ def get_user_clipboard_json(user_id: int, request: HttpRequest | None = None): return serializer.data +def _staged_content_to_data(content: _StagedContent) -> StagedContentData: + """ + Convert a StagedContent model instance to an immutable data object. + """ + return StagedContentData( + id=content.id, + user_id=content.user_id, + created=content.created, + purpose=content.purpose, + status=content.status, + block_type=content.block_type, + display_name=content.display_name, + tags=content.tags or {}, + version_num=content.version_num, + ) + + def _user_clipboard_model_to_data(clipboard: _UserClipboard) -> UserClipboardData: """ Convert a UserClipboard model instance to an immutable data object. """ - content = clipboard.content - source_context_key = clipboard.source_usage_key.context_key - if source_context_key.is_course and (course_overview := get_course_overview_or_none(source_context_key)): - source_context_title = course_overview.display_name_with_default - else: - source_context_title = str(source_context_key) # Fall back to stringified context key as a title return UserClipboardData( - content=StagedContentData( - id=content.id, - user_id=content.user_id, - created=content.created, - purpose=content.purpose, - status=content.status, - block_type=content.block_type, - display_name=content.display_name, - tags=content.tags or {}, - version_num=content.version_num, - ), + content=_staged_content_to_data(clipboard.content), source_usage_key=clipboard.source_usage_key, source_context_title=clipboard.get_source_context_title(), ) diff --git a/openedx/core/djangoapps/content_staging/data.py b/openedx/core/djangoapps/content_staging/data.py index d077d05a0aa4..d095f2506b17 100644 --- a/openedx/core/djangoapps/content_staging/data.py +++ b/openedx/core/djangoapps/content_staging/data.py @@ -25,6 +25,10 @@ class StagedContentStatus(TextChoices): # Value of the "purpose" field on StagedContent objects used for clipboards. CLIPBOARD_PURPOSE = "clipboard" + +# Value of the "purpose" field on StagedContent objects used for library to course sync. +LIBRARY_SYNC_PURPOSE = "library_sync" + # There may be other valid values of "purpose" which aren't defined within this app.