[FC-0009] Copy/Paste associated static assets

cms/djangoapps/contentstore/helpers.py

@@ -1,31 +1,34 @@
 """
 Helper methods for Studio views.
 """
-
+from __future__ import annotations
+import logging
 import urllib
 from lxml import etree
+from mimetypes import guess_type
 
+from attrs import frozen, Factory
 from django.utils.translation import gettext as _
-from opaque_keys.edx.keys import UsageKey
+from opaque_keys.edx.keys import AssetKey, CourseKey, UsageKey
 from opaque_keys.edx.locator import DefinitionLocator, LocalId
 from xblock.core import XBlock
 from xblock.fields import ScopeIds
 from xblock.runtime import IdGenerator
+from xmodule.contentstore.content import StaticContent
+from xmodule.contentstore.django import contentstore
+from xmodule.exceptions import NotFoundError
 from xmodule.modulestore.django import modulestore
 
 # from cms.djangoapps.contentstore.views.preview import _load_preview_block
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from common.djangoapps.student import auth
 from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole
-
-try:
-    # Technically this is a django app plugin, so we should not error if it's not installed:
-    import openedx.core.djangoapps.content_staging.api as content_staging_api
-except ImportError:
-    content_staging_api = None
+import openedx.core.djangoapps.content_staging.api as content_staging_api
 
 from .utils import reverse_course_url, reverse_library_url, reverse_usage_url
 
+log = logging.getLogger(__name__)
+
 # Note: Grader types are used throughout the platform but most usages are simply in-line
 # strings.  In addition, new grader types can be defined on the fly anytime one is needed
 # (because they're just strings). This dict is an attempt to constrain the sprawl in Studio.
@@ -210,6 +213,14 @@ def create_definition(self, block_type, slug=None) -> DefinitionLocator:
         return DefinitionLocator(block_type, LocalId(block_type))
 
 
+@frozen
+class StaticFileNotices:
+    """ Information about what static files were updated (or not) when pasting content into another course """
+    new_files: list[str] = Factory(list)
+    conflicting_files: list[str] = Factory(list)
+    error_files: list[str] = Factory(list)
+
+
 def import_staged_content_from_user_clipboard(parent_key: UsageKey, request):
     """
     Import a block (and any children it has) from "staged" OLX.
@@ -226,9 +237,10 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request):
     user_clipboard = content_staging_api.get_user_clipboard(request.user.id)
     if not user_clipboard:
         # Clipboard is empty or expired/error/loading
-        return None
+        return None, StaticFileNotices()
     block_type = user_clipboard.content.block_type
     olx_str = content_staging_api.get_staged_content_olx(user_clipboard.content.id)
+    static_files = content_staging_api.get_staged_content_static_files(user_clipboard.content.id)
     node = etree.fromstring(olx_str)
     store = modulestore()
     with store.bulk_operations(parent_key.course_key):
@@ -247,6 +259,11 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request):
         # 'keys' and uses 'id_generator', but the default XBlock parse_xml ignores 'id_generator' and uses 'keys'.
         # For children of this block, obviously only id_generator is used.
         xblock_class = runtime.load_block_type(block_type)
+        # Note: if we find a case where any XBlock needs access to the block-specific static files that were saved to
+        # export_fs during copying, we could make them available here via runtime.resources_fs before calling parse_xml.
+        # However, currently the only known case for that is video block's transcript files, and those will
+        # automatically be "carried over" to the new XBlock even in a different course because the video ID is the same,
+        # and VAL will thus make the transcript available.
         temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator)
         if xblock_class.has_children and temp_xblock.children:
             raise NotImplementedError("We don't yet support pasting XBlocks with children")
@@ -257,7 +274,89 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request):
         new_xblock = store.update_item(temp_xblock, request.user.id, allow_not_found=True)
         parent_xblock.children.append(new_xblock.location)
         store.update_item(parent_xblock, request.user.id)
-        return new_xblock
+
+    # Now handle static files that need to go into Files & Uploads:
+    notices = _import_files_into_course(
+        course_key=parent_key.context_key,
+        staged_content_id=user_clipboard.content.id,
+        static_files=static_files,
+    )
+
+    return new_xblock, notices
+
+
+def _import_files_into_course(
+    course_key: CourseKey,
+    staged_content_id: int,
+    static_files: list[content_staging_api.StagedContentFileData],
+) -> StaticFileNotices:
+    """
+    For the given staged static asset files (which are in "Staged Content" such as the user's clipbaord, but which
+    need to end up in the course's Files & Uploads page), import them into the destination course, unless they already
+    exist.
+    """
+    # List of files that were newly added to the destination course
+    new_files = []
+    # List of files that conflicted with identically named files already in the destination course
+    conflicting_files = []
+    # List of files that had an error (shouldn't happen unless we have some kind of bug)
+    error_files = []
+    for file_data_obj in static_files:
+        if not isinstance(file_data_obj.source_key, AssetKey):
+            # This static asset was managed by the XBlock and instead of being added to "Files & Uploads", it is stored
+            # using some other system. We could make it available via runtime.resources_fs during XML parsing, but it's
+            # not needed here.
+            continue
+        # At this point, we know this is a "Files & Uploads" asset that we may need to copy into the course:
+        try:
+            result = _import_file_into_course(course_key, staged_content_id, file_data_obj)
+            if result is True:
+                new_files.append(file_data_obj.filename)
+            elif result is None:
+                pass  # This file already exists; no action needed.
+            else:
+                conflicting_files.append(file_data_obj.filename)
+        except Exception:  # lint-amnesty, pylint: disable=broad-except
+            error_files.append(file_data_obj.filename)
+            log.exception(f"Failed to import Files & Uploads file {file_data_obj.filename}")
+    return StaticFileNotices(
+        new_files=new_files,
+        conflicting_files=conflicting_files,
+        error_files=error_files,
+    )
+
+
+def _import_file_into_course(
+    course_key: CourseKey,
+    staged_content_id: int,
+    file_data_obj: content_staging_api.StagedContentFileData,
+) -> bool | None:
+    """
+    Import a single staged static asset file into the course, unless it already exists.
+    Returns True if it was imported, False if there's a conflict, or None if
+    the file already existed (no action needed).
+    """
+    filename = file_data_obj.filename
+    new_key = course_key.make_asset_key("asset", filename)
+    try:
+        current_file = contentstore().find(new_key)
+    except NotFoundError:
+        current_file = None
+    if not current_file:
+        # This static asset should be imported into the new course:
+        content_type = guess_type(filename)[0]
+        data = content_staging_api.get_staged_content_static_file_data(staged_content_id, filename)
+        if data is None:
+            raise NotFoundError(file_data_obj.source_key)
+        content = StaticContent(new_key, name=filename, content_type=content_type, data=data)
+        contentstore().save(content)
+        return True
+    elif current_file.content_digest == file_data_obj.md5_hash:
+        # The file already exists and matches exactly, so no action is needed
+        return None
+    else:
+        # There is a conflict with some other file that has the same name.
+        return False
 
 
 def is_item_in_course_tree(item):

cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py

@@ -5,9 +5,9 @@
 """
 from opaque_keys.edx.keys import UsageKey
 from rest_framework.test import APIClient
-from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from xmodule.modulestore.tests.factories import ToyCourseFactory
+from xmodule.modulestore.django import contentstore, modulestore
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course
+from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory
 
 CLIPBOARD_ENDPOINT = "/api/content-staging/v1/clipboard/"
 XBLOCK_ENDPOINT = "/xblock/"
@@ -60,3 +60,72 @@ def test_copy_and_paste_video(self):
         assert new_video.youtube_id_1_0 == orig_video.youtube_id_1_0
         # The new block should store a reference to where it was copied from
         assert new_video.copied_from_block == str(video_key)
+
+    def test_paste_with_assets(self):
+        """
+        When pasting into a different course, any required static assets should
+        be pasted too, unless they already exist in the destination course.
+        """
+        dest_course_key, client = self._setup_course()
+        # Make sure some files exist in the source course to be copied:
+        source_course = CourseFactory.create()
+        upload_file_to_course(
+            course_key=source_course.id,
+            contentstore=contentstore(),
+            source_file='./common/test/data/static/picture1.jpg',
+            target_filename="picture1.jpg",
+        )
+        upload_file_to_course(
+            course_key=source_course.id,
+            contentstore=contentstore(),
+            source_file='./common/test/data/static/picture2.jpg',
+            target_filename="picture2.jpg",
+        )
+        source_html = BlockFactory.create(
+            parent_location=source_course.location,
+            category="html",
+            display_name="Some HTML",
+            data="""
+            <p>
+                <a href="/static/picture1.jpg">Picture 1</a>
+                <a href="/static/picture2.jpg">Picture 2</a>
+            </p>
+            """,
+        )
+
+        # Now, to test conflict handling, we also upload a CONFLICTING image to
+        # the destination course under the same filename.
+        upload_file_to_course(
+            course_key=dest_course_key,
+            contentstore=contentstore(),
+            # Note this is picture 3, not picture 2, but we save it as picture 2:
+            source_file='./common/test/data/static/picture3.jpg',
+            target_filename="picture2.jpg",
+        )
+
+        # Now copy the HTML block from the source cost and paste it into the destination:
+        copy_response = client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(source_html.location)}, format="json")
+        assert copy_response.status_code == 200
+
+        # Paste the video
+        dest_parent_key = dest_course_key.make_usage_key("vertical", "vertical_test")
+        paste_response = client.post(XBLOCK_ENDPOINT, {
+            "parent_locator": str(dest_parent_key),
+            "staged_content": "clipboard",
+        }, format="json")
+        assert paste_response.status_code == 200
+        static_file_notices = paste_response.json()["static_file_notices"]
+        assert static_file_notices == {
+            "error_files": [],
+            "new_files": ["picture1.jpg"],
+            # The new course already had a file named "picture2.jpg" with different md5 hash, so it's a conflict:
+            "conflicting_files": ["picture2.jpg"],
+        }
+
+        # Check that the files are as we expect:
+        source_pic1_hash = contentstore().find(source_course.id.make_asset_key("asset", "picture1.jpg")).content_digest
+        dest_pic1_hash = contentstore().find(dest_course_key.make_asset_key("asset", "picture1.jpg")).content_digest
+        assert source_pic1_hash == dest_pic1_hash
+        source_pic2_hash = contentstore().find(source_course.id.make_asset_key("asset", "picture2.jpg")).content_digest
+        dest_pic2_hash = contentstore().find(dest_course_key.make_asset_key("asset", "picture2.jpg")).content_digest
+        assert source_pic2_hash != dest_pic2_hash  # Because there was a conflict, this file was unchanged.

cms/djangoapps/contentstore/xblock_services/xblock_service.py

@@ -13,6 +13,7 @@
 from datetime import datetime
 from uuid import uuid4
 
+from attrs import asdict
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import (User)  # lint-amnesty, pylint: disable=imported-auth-user
@@ -562,7 +563,7 @@ def _create_block(request):
     if request.json.get("staged_content") == "clipboard":
         # Paste from the user's clipboard (content_staging app clipboard, not browser clipboard) into 'usage_key':
         try:
-            created_xblock = import_staged_content_from_user_clipboard(
+            created_xblock, notices = import_staged_content_from_user_clipboard(
                 parent_key=usage_key, request=request
             )
         except Exception:  # pylint: disable=broad-except
@@ -576,12 +577,11 @@ def _create_block(request):
             return JsonResponse(
                 {"error": _("Your clipboard is empty or invalid.")}, status=400
             )
-        return JsonResponse(
-            {
-                "locator": str(created_xblock.location),
-                "courseKey": str(created_xblock.location.course_key),
-            }
-        )
+        return JsonResponse({
+            "locator": str(created_xblock.location),
+            "courseKey": str(created_xblock.location.course_key),
+            "static_file_notices": asdict(notices),
+        })
 
     category = request.json["category"]
     if isinstance(usage_key, LibraryUsageLocator):

cms/envs/common.py

@@ -2728,3 +2728,11 @@
 
 DISCUSSIONS_INCONTEXT_FEEDBACK_URL = ''
 DISCUSSIONS_INCONTEXT_LEARNMORE_URL = ''
+
+OPEN_EDX_FILTERS_CONFIG = {
+    "org.openedx.content_authoring.staged_content.static_filter_source.v1": {
+        "pipeline": [
+            "openedx.core.djangoapps.content_staging.filters.IgnoreLargeFiles",
+        ]
+    }
+}

cms/static/js/views/pages/container.js

@@ -6,10 +6,12 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page
     'common/js/components/utils/view_utils', 'js/views/container', 'js/views/xblock',
     'js/views/components/add_xblock', 'js/views/modals/edit_xblock', 'js/views/modals/move_xblock_modal',
     'js/models/xblock_info', 'js/views/xblock_string_field_editor', 'js/views/xblock_access_editor',
-    'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils'],
+    'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils',
+    'common/js/components/views/feedback_notification', 'common/js/components/views/feedback_prompt',
+],
 function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent,
     EditXBlockModal, MoveXBlockModal, XBlockInfo, XBlockStringFieldEditor, XBlockAccessEditor,
-    ContainerSubviews, UnitOutlineView, XBlockUtils) {
+    ContainerSubviews, UnitOutlineView, XBlockUtils, NotificationView, PromptView) {
     'use strict';
 
     var XBlockContainerPage = BasePage.extend({
@@ -255,10 +257,60 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView
                     staged_content: "clipboard",
                 }).then((data) => {
                     this.onNewXBlock(placeholderElement, scrollOffset, false, data);
+                    return data;
                 }).fail(() => {
                     // Remove the placeholder if the paste failed
                     placeholderElement.remove();
                 });
+            }).done((data) => {
+                const {
+                    conflicting_files: conflictingFiles,
+                    error_files: errorFiles,
+                    new_files: newFiles,
+                } = data.static_file_notices;
+
+                const notices = [];
+                if (errorFiles.length) {
+                    notices.push((next) => new PromptView.Error({
+                        title: gettext("Some errors occurred"),
+                        message: (
+                            gettext("The following required files could not be added to the course:") +
+                            " " + errorFiles.join(", ")
+                        ),
+                        actions: {primary: {text: gettext("OK"), click: (x) => { x.hide(); next(); }}},
+                    }));
+                }
+                if (conflictingFiles.length) {
+                    notices.push((next) => new PromptView.Warning({
+                        title: gettext("You may need to update a file(s) manually"),
+                        message: (
+                            gettext(
+                                "The following files already exist in this course but don't match the " +
+                                "version used by the component you pasted:"
+                            ) + " " + conflictingFiles.join(", ")
+                        ),
+                        actions: {primary: {text: gettext("OK"), click: (x) => { x.hide(); next(); }}},
+                    }));
+                }
+                if (newFiles.length) {
+                    notices.push(() => new NotificationView.Confirmation({
+                        title: gettext("New files were added to this course's Files & Uploads"),
+                        message: (
+                            gettext("The following required files were imported to this course:") +
+                            " "  + newFiles.join(", ")
+                        ),
+                        closeIcon: true,
+                    }));
+                }
+                if (notices.length) {
+                    // Show the notices, one at a time:
+                    const showNext = () => {
+                        const view = notices.shift()(showNext);
+                        view.show();
+                    }
+                    // Delay to avoid conflict with the "Pasting..." notification.
+                    setTimeout(showNext, 1250);
+                }
             });
         },