Skip to content

Commit

Permalink
Merge branch 'master' into farhan/sass-to-css-video-block
Browse files Browse the repository at this point in the history
  • Loading branch information
farhan authored Oct 23, 2024
2 parents 7764e47 + d25e651 commit 6259520
Show file tree
Hide file tree
Showing 26 changed files with 579 additions and 190 deletions.
115 changes: 89 additions & 26 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""
from __future__ import annotations
import logging
import pathlib
import urllib
from lxml import etree
from mimetypes import guess_type
Expand All @@ -11,7 +12,7 @@
from django.conf import settings
from django.contrib.auth import get_user_model
from django.utils.translation import gettext as _
from opaque_keys.edx.keys import AssetKey, CourseKey, UsageKey
from opaque_keys.edx.keys import CourseKey, UsageKey
from opaque_keys.edx.locator import DefinitionLocator, LocalId
from xblock.core import XBlock
from xblock.fields import ScopeIds
Expand Down Expand Up @@ -192,6 +193,8 @@ def xblock_type_display_name(xblock, default_display_name=None):
return _('Problem')
elif category == 'library_v2':
return _('Library Content')
elif category == 'itembank':
return _('Problem Bank')
component_class = XBlock.load_class(category)
if hasattr(component_class, 'display_name') and component_class.display_name.default:
return _(component_class.display_name.default) # lint-amnesty, pylint: disable=translation-of-non-string
Expand Down Expand Up @@ -278,7 +281,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) ->
# Clipboard is empty or expired/error/loading
return None, StaticFileNotices()
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):
Expand All @@ -295,12 +297,29 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) ->
copied_from_version_num=user_clipboard.content.version_num,
tags=user_clipboard.content.tags,
)
# 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,
)

# 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)

return new_xblock, notices


Expand Down Expand Up @@ -454,78 +473,122 @@ def _import_files_into_course(
course_key: CourseKey,
staged_content_id: int,
static_files: list[content_staging_api.StagedContentFileData],
) -> StaticFileNotices:
usage_key: UsageKey,
) -> tuple[StaticFileNotices, dict[str, str]]:
"""
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
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.
This function returns a tuple of StaticFileNotices (assets added, errors,
conflicts), and static asset path substitutions that should be made in the
OLX in order to paste this content into this course. The latter is for the
case in which we're brining content in from a v2 library, which stores
static assets locally to a Component and needs to go into a subdirectory
when pasting into a course to avoid overwriting commonly named things, e.g.
"figure1.png".
"""
# 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 = []

# Store a mapping of asset URLs that need to be modified for the destination
# assets. This is necessary when you take something from a library and paste
# it into a course, because we need to translate Component-local static
# assets and shove them into the Course's global Files & Uploads space in a
# nested directory structure.
substitutions = {}
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)
result, substitution_for_file = _import_file_into_course(
course_key,
staged_content_id,
file_data_obj,
usage_key,
)
if result is True:
new_files.append(file_data_obj.filename)
substitutions.update(substitution_for_file)
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(

notices = StaticFileNotices(
new_files=new_files,
conflicting_files=conflicting_files,
error_files=error_files,
)

return notices, substitutions


def _import_file_into_course(
course_key: CourseKey,
staged_content_id: int,
file_data_obj: content_staging_api.StagedContentFileData,
) -> bool | None:
usage_key: UsageKey,
) -> tuple[bool | None, dict]:
"""
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)
clipboard_file_path = file_data_obj.filename

# We need to generate an AssetKey to add an asset to a course. The mapping
# of directories '/' -> '_' is a long-existing contentstore convention that
# we're not going to attempt to change.
if clipboard_file_path.startswith('static/'):
# If it's in this form, it came from a library and assumes component-local assets
file_path = clipboard_file_path.lstrip('static/')
import_path = f"components/{usage_key.block_type}/{usage_key.block_id}/{file_path}"
filename = pathlib.Path(file_path).name
new_key = course_key.make_asset_key("asset", import_path.replace("/", "_"))
else:
# Otherwise it came from a course...
file_path = clipboard_file_path
import_path = None
filename = pathlib.Path(file_path).name
new_key = course_key.make_asset_key("asset", file_path.replace("/", "_"))

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)
data = content_staging_api.get_staged_content_static_file_data(staged_content_id, clipboard_file_path)
if data is None:
raise NotFoundError(file_data_obj.source_key)
content = StaticContent(new_key, name=filename, content_type=content_type, data=data)
content = StaticContent(
new_key,
name=filename,
content_type=content_type,
data=data,
import_path=import_path
)
# 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
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
return None, {}
else:
# There is a conflict with some other file that has the same name.
return False
return False, {}


def is_item_in_course_tree(item):
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ def get_library_content_picker_url(course_locator) -> str:
content_picker_url = None
if libraries_v2_enabled():
mfe_base_url = get_course_authoring_url(course_locator)
content_picker_url = f'{mfe_base_url}/component-picker'
content_picker_url = f'{mfe_base_url}/component-picker?variant=published'

return content_picker_url

Expand Down
3 changes: 3 additions & 0 deletions cms/djangoapps/contentstore/views/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
'discussion',
'library',
'library_v2', # Not an XBlock
'itembank',
'html',
'openassessment',
'problem',
Expand Down Expand Up @@ -262,6 +263,7 @@ def create_support_legend_dict():
'openassessment': _("Open Response"),
'library': _("Legacy Library"),
'library_v2': _("Library Content"),
'itembank': _("Problem Bank"),
'drag-and-drop-v2': _("Drag and Drop"),
}

Expand Down Expand Up @@ -488,6 +490,7 @@ def _filter_disabled_blocks(all_blocks):
disabled_block_names = [block.name for block in disabled_xblocks()]
if not libraries_v2_enabled():
disabled_block_names.append('library_v2')
disabled_block_names.append('itembank')
return [block_name for block_name in all_blocks if block_name not in disabled_block_names]


Expand Down
8 changes: 6 additions & 2 deletions cms/djangoapps/contentstore/views/preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,9 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
selected_groups_label = _('Access restricted to: {list_of_groups}').format(list_of_groups=selected_groups_label) # lint-amnesty, pylint: disable=line-too-long
course = modulestore().get_course(xblock.location.course_key)
can_edit = context.get('can_edit', True)
can_add = context.get('can_add', True)
# Is this a course or a library?
is_course = xblock.scope_ids.usage_id.context_key.is_course
is_course = xblock.context_key.is_course
tags_count_map = context.get('tags_count_map')
tags_count = 0
if tags_count_map:
Expand All @@ -320,7 +321,10 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
'is_selected': context.get('is_selected', False),
'selectable': context.get('selectable', False),
'selected_groups_label': selected_groups_label,
'can_add': context.get('can_add', True),
'can_add': can_add,
# Generally speaking, "if you can add, you can delete". One exception is itembank (Problem Bank)
# which has its own separate "add" workflow but uses the normal delete workflow for its child blocks.
'can_delete': can_add or (root_xblock and root_xblock.scope_ids.block_type == "itembank" and can_edit),
'can_move': context.get('can_move', is_course),
'language': getattr(course, 'language', None),
'is_course': is_course,
Expand Down
58 changes: 58 additions & 0 deletions cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,64 @@ def test_paste_from_library_read_only_tags(self):
assert object_tag.value in self.lib_block_tags
assert object_tag.is_copied

def test_paste_from_library_copies_asset(self):
"""
Assets from a library component copied into a subdir of Files & Uploads.
"""
# This is the binary for a real, 1px webp file – we need actual image
# data because contentstore will try to make a thumbnail and grab
# metadata.
webp_raw_data = b'RIFF\x16\x00\x00\x00WEBPVP8L\n\x00\x00\x00/\x00\x00\x00\x00E\xff#\xfa\x1f'

# First add the asset.
library_api.add_library_block_static_asset_file(
self.lib_block_key,
"static/1px.webp",
webp_raw_data,
) # v==4

# Now add the reference to the asset
library_api.set_library_block_olx(self.lib_block_key, """
<problem display_name="MCQ-draft" max_attempts="5">
<p>Including this totally real image: <img src="/static/1px.webp" /></p>
<multiplechoiceresponse>
<label>Q</label>
<choicegroup type="MultipleChoice">
<choice correct="false">Wrong</choice>
<choice correct="true">Right</choice>
</choicegroup>
</multiplechoiceresponse>
</problem>
""") # v==5

copy_response = self.client.post(
CLIPBOARD_ENDPOINT,
{"usage_key": str(self.lib_block_key)},
format="json"
)
assert copy_response.status_code == 200

paste_response = self.client.post(XBLOCK_ENDPOINT, {
"parent_locator": str(self.course.usage_key),
"staged_content": "clipboard",
}, format="json")
assert paste_response.status_code == 200

new_block_key = UsageKey.from_string(paste_response.json()["locator"])
new_block = modulestore().get_item(new_block_key)

# Check that the substitution worked.
expected_import_path = f"components/{new_block_key.block_type}/{new_block_key.block_id}/1px.webp"
assert f"/static/{expected_import_path}" in new_block.data

# Check that the asset was copied over properly
image_asset = contentstore().find(
self.course.id.make_asset_key("asset", expected_import_path.replace('/', '_'))
)
assert image_asset.import_path == expected_import_path
assert image_asset.name == "1px.webp"
assert image_asset.length == len(webp_raw_data)


class ClipboardPasteFromV1LibraryTestCase(ModuleStoreTestCase):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,9 +594,13 @@ def _create_block(request):
# 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)
except BadUpstream:
except BadUpstream as exc:
_delete_item(created_block.location, request.user)
return JsonResponse({"error": _("Invalid library xblock reference.")}, status=400)
log.exception(
f"Could not sync to new block at '{created_block.usage_key}' "
f"using provided library_content_key='{upstream_ref}'"
)
return JsonResponse({"error": str(exc)}, status=400)
modulestore().update_item(created_block, request.user.id)

return JsonResponse(
Expand Down
3 changes: 3 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,9 @@
'corsheaders',
'openedx.core.djangoapps.cors_csrf',

# Provides the 'django_markup' template library so we can use 'interpolate_html' in django templates
'xss_utils',

# History tables
'simple_history',

Expand Down
Binary file added cms/static/images/large-itembank-icon.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions cms/static/js/views/components/add_library_content.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
/**
* Provides utilities to open and close the library content picker.
* This is for adding a single, selected, non-randomized component (XBlock)
* from the library into the course. It achieves the same effect as copy-pasting
* the block from a library into the course. The block will remain synced with
* the "upstream" library version.
*
* Compare cms/static/js/views/modals/select_v2_library_content.js which uses
* a multi-select modal to add component(s) to a Problem Bank (for
* randomization).
*/
define(['jquery', 'underscore', 'gettext', 'js/views/modals/base_modal'],
function($, _, gettext, BaseModal) {
Expand Down
Loading

0 comments on commit 6259520

Please sign in to comment.