feat: Copy/Paste associated static assets along with components (#32346)

* refactor: improve typing of StaticFile named tuple
* feat: copy static asset files into the clipboard
* feat: paste static assets
* feat: show notification in studio about pasted assets
* fix: HTML XBlocks would lose the editor="raw" setting when copy-pasted.
* feat: copy python_lib.zip to the clipboard when it seems to be in use
This commit is contained in:
Braden MacDonald
2023-06-27 12:06:43 -07:00
committed by GitHub
parent 32a3e2a94f
commit 12a8d99824
18 changed files with 735 additions and 45 deletions

View File

@@ -1,31 +1,33 @@
"""
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,13 +212,24 @@ class ImportIdGenerator(IdGenerator):
return DefinitionLocator(block_type, LocalId(block_type))
def import_staged_content_from_user_clipboard(parent_key: UsageKey, request):
@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) -> tuple[XBlock | None, StaticFileNotices]:
"""
Import a block (and any children it has) from "staged" OLX.
Import a block (along with its children and any required static assets) from
the "staged" OLX in the user's clipboard.
Does not deal with permissions or REST stuff - do that before calling this.
Returns the newly created block on success or None if the clipboard is
empty.
Returns (1) the newly created block on success or None if the clipboard is
empty, and (2) a summary of changes made to static files in the destination
course.
"""
from cms.djangoapps.contentstore.views.preview import _load_preview_block
@@ -226,9 +239,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 +261,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 +276,93 @@ 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)
# If it's an image file, also generate the thumbnail:
thumbnail_content, thumbnail_location = contentstore().generate_thumbnail(content)
if thumbnail_content is not None:
content.thumbnail_location = thumbnail_location
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):

View File

@@ -5,9 +5,9 @@ APIs.
"""
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 @@ class ClipboardPasteTestCase(ModuleStoreTestCase):
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.

View File

@@ -13,6 +13,7 @@ import logging
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):

View File

@@ -2728,3 +2728,11 @@ BRAZE_COURSE_ENROLLMENT_CANVAS_ID = ''
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",
]
}
}

View File

@@ -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);
}
});
},