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

Merged
merged 7 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from 2 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
44 changes: 44 additions & 0 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,50 @@ 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.
bradenmacdonald marked this conversation as resolved.
Show resolved Hide resolved
"""
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):
bradenmacdonald marked this conversation as resolved.
Show resolved Hide resolved
# Now handle static files that need to go into Files & Uploads.
bradenmacdonald marked this conversation as resolved.
Show resolved Hide resolved
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
bradenmacdonald marked this conversation as resolved.
Show resolved Hide resolved

return notices


def _fetch_and_set_upstream_link(
copied_from_block: str,
copied_from_version_num: int,
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
}
"""

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)
bradenmacdonald marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -4,6 +4,7 @@
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
Expand Down Expand Up @@ -247,14 +248,16 @@ def call_api(self, usage_key_string):

@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_staged_content_for_library_sync", return_value=StaticFileNotices())
def test_200(self, mock_sync_from_upstream, mock_import_staged_content):
"""
Does the happy path work?
"""
self.client.login(username="superuser", password="password")
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):
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["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
Loading
Loading