Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,49 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) ->

return new_xblock, notices

def import_staged_content_for_library_sync(new_xblock: XBlock, lib_block: XBlock, request) -> StaticFileNotices:
"""
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 (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.
"""
if not content_staging_api:
raise RuntimeError("The required content_staging app is not installed")
library_sync = content_staging_api.save_xblock_to_user_library_sync(lib_block, request.user.id)
if not library_sync:
# expired/error/loading
return StaticFileNotices()

store = modulestore()
with store.bulk_operations(new_xblock.scope_ids.usage_id.context_key):
# Now handle static files that need to go into Files & Uploads.
static_files = content_staging_api.get_staged_content_static_files(library_sync.content.id)
notices, substitutions = _import_files_into_course(
course_key=new_xblock.scope_ids.usage_id.context_key,
staged_content_id=library_sync.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

return notices


def _fetch_and_set_upstream_link(
copied_from_block: str,
Expand Down
10 changes: 8 additions & 2 deletions cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@
"ready_to_sync": Boolean
}
"""
from dataclasses import asdict
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
Expand All @@ -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_staged_content_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,
Expand Down Expand Up @@ -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_staged_content_for_library_sync(downstream, upstream, request)
except UpstreamLinkException as exc:
logger.exception(
"Could not sync from upstream '%s' to downstream '%s'",
Expand All @@ -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:
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
from ..helpers import (
get_parent_xblock,
import_staged_content_from_user_clipboard,
import_staged_content_for_library_sync,
is_unit,
xblock_embed_lms_url,
xblock_lms_url,
Expand Down Expand Up @@ -598,16 +599,18 @@ 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(
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)
static_file_notices = import_staged_content_for_library_sync(created_block, lib_block, request)
modulestore().update_item(created_block, request.user.id)
response['upstreamRef'] = upstream_ref
response['static_file_notices'] = asdict(static_file_notices)

return JsonResponse(response)

Expand Down
3 changes: 2 additions & 1 deletion cms/lib/xblock/upstream_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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:
Expand Down
161 changes: 123 additions & 38 deletions openedx/core/djangoapps/content_staging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,33 @@

from .data import (
CLIPBOARD_PURPOSE,
StagedContentData, StagedContentFileData, StagedContentStatus, UserClipboardData
LIBRARY_SYNC_PURPOSE,
StagedContentData, StagedContentFileData, StagedContentStatus, UserClipboardData, UserLibrarySyncData,
)
from .models import (
UserClipboard as _UserClipboard,
StagedContent as _StagedContent,
StagedContentFile as _StagedContentFile,
UserLibrarySync as _UserLibrarySync,
)
from .serializers import (
UserClipboardSerializer as _UserClipboardSerializer,
UserLibrarySyncSerializer as _UserLibrarySyncSerializer,
)
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,
Expand All @@ -49,7 +60,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,
)
Expand All @@ -60,7 +71,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,
Expand All @@ -69,38 +80,32 @@ 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:
delete_expired_clipboards.delay(expired_ids)
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 = (
Expand Down Expand Up @@ -144,6 +149,46 @@ 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 save_xblock_to_user_library_sync(
block: XBlock,
user_id: int,
version_num: int | None = None
) -> UserLibrarySyncData:
"""
Save an XBlock's OLX for library sync.
"""
staged_content = _save_xblock_to_staged_content(block, user_id, LIBRARY_SYNC_PURPOSE, version_num)
usage_key = block.usage_key

# Create/update the library sync entry
(sync, _created) = _UserLibrarySync.objects.update_or_create(
user_id=user_id,
defaults={
"content": staged_content,
"source_usage_key": usage_key,
}
)

return _user_library_sync_model_to_data(sync)

def get_user_clipboard(user_id: int, only_ready: bool = True) -> UserClipboardData | None:
"""
Get the details of the user's clipboard.
Expand Down Expand Up @@ -190,32 +235,72 @@ def get_user_clipboard_json(user_id: int, request: HttpRequest | None = None):
return serializer.data


def get_user_library_sync_json(user_id: int, request: HttpRequest | None = None):
"""
Get the detailed status of the user's library sync, in exactly the same format
as returned from the
/api/content-staging/v1/library-sync/
REST API endpoint. This version of the API is meant for "preloading" that
REST API endpoint so it can be embedded in a larger response sent to the
user's browser.

(request is optional; including it will make the "olx_url" absolute instead
of relative.)
"""
try:
sync = _UserLibrarySync.objects.get(user_id=user_id)
except _UserLibrarySync.DoesNotExist:
# This user does not have any library sync content.
return {
"content": None,
"source_usage_key": "",
"source_context_title": "",
"source_edit_url": ""
}

serializer = _UserLibrarySyncSerializer(
_user_library_sync_model_to_data(sync),
context={'request': request},
)
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(),
)

def _user_library_sync_model_to_data(sync: _UserLibrarySync) -> UserLibrarySyncData:
"""
Convert a UserLibrarySync model instance to an immutable data object.
"""
return UserLibrarySyncData(
content=_staged_content_to_data(sync.content),
source_usage_key=sync.source_usage_key,
source_context_title=sync.get_source_context_title(),
)


def get_staged_content_olx(staged_content_id: int) -> str | None:
"""
Expand Down
Loading
Loading