Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix: static assets used in problem bank and library content block (#36173)

Browse Source 
Static assets were not being copied into the course when using library content via Problem Bank or "Add Library Content" workflows.
This commit is contained in:
Daniel Valenzuela
2025-02-07 15:30:05 -03:00
committed by GitHub
parent 02d2d34a25
commit 36c16d6952
7 changed files with 165 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

93
cms/djangoapps/contentstore/helpers.py
View File

@@ -29,6 +29,7 @@ from cms.lib.xblock.upstream_sync import UpstreamLink, UpstreamLinkException, fe
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, {}

10
cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py
View File

@@ -56,8 +56,10 @@ UpstreamLink response schema:
"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 @@ from cms.lib.xblock.upstream_sync import (
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 @@ class SyncFromUpstreamView(DeveloperErrorViewMixin, APIView):
"""
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 @@ class SyncFromUpstreamView(DeveloperErrorViewMixin, APIView):
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:
"""

5
cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py
View File

@@ -4,6 +4,7 @@ Unit tests for /api/contentstore/v2/downstreams/* JSON APIs.
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 @@ class PostDownstreamSyncViewTest(_DownstreamSyncViewTestMixin, SharedModuleStore
@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 @@ class PostDownstreamSyncViewTest(_DownstreamSyncViewTestMixin, SharedModuleStore
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):

7
cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
View File

@@ -80,6 +80,7 @@ from .xblock_helpers import usage_key_with_run
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)

3
cms/lib/xblock/upstream_sync.py
View File

@@ -186,7 +186,7 @@ class UpstreamLink:
)
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:

111
openedx/core/djangoapps/content_staging/api.py
View File

@@ -14,29 +14,36 @@ from opaque_keys.edx.keys import AssetKey, UsageKey
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(),
)

4
openedx/core/djangoapps/content_staging/data.py
View File

@@ -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.