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

* fix: render library v2 assets with whitespace (openedx#35974)

Assets that contain whitespace fail to be rendered when uploaded to library v2

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

Static assets were not being copied into the course when using library content via Problem Bank or "Add Library Content" workflows.

Author: DanielVZ96

cms/djangoapps/contentstore/helpers.py

@@ -28,6 +28,7 @@
 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
 
@@ -261,6 +262,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
@@ -298,31 +330,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,
@@ -543,6 +587,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:
@@ -613,8 +660,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, {}

cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py

@@ -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
@@ -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_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 @@ 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_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 @@ 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:
         """

cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py

@@ -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
@@ -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_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?
         """
         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):

cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py

@@ -80,6 +80,7 @@
 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,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_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)
 

cms/lib/xblock/upstream_sync.py

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

openedx/core/djangoapps/content_libraries/tests/test_static_assets.py

@@ -63,6 +63,13 @@ def test_asset_filenames(self):
         file_name = "a////////b"
         self._set_library_block_asset(block_id, file_name, SVG_DATA, expect_response=400)
 
+        # Names with spaces are allowed but replaced with underscores
+        file_name_with_space = "o w o.svg"
+        self._set_library_block_asset(block_id, file_name_with_space, SVG_DATA)
+        file_name = "o_w_o.svg"
+        assert self._get_library_block_asset(block_id, file_name)['path'] == file_name
+        assert self._get_library_block_asset(block_id, file_name)['size'] == file_size
+
     def test_video_transcripts(self):
         """
         Test that video blocks can read transcript files out of learning core.

openedx/core/djangoapps/content_libraries/views.py

@@ -788,6 +788,7 @@ def put(self, request, usage_key_str, file_path):
         """
         Replace a static asset file belonging to this block.
         """
+        file_path = file_path.replace(" ", "_")  # Messes up url/name correspondence due to URL encoding.
         usage_key = LibraryUsageLocatorV2.from_string(usage_key_str)
         api.require_permission_for_library_key(
             usage_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,