feat: add params to XBlockSerializer for clipboard use case

cms/djangoapps/contentstore/helpers.py

@@ -3,6 +3,7 @@
 """
 from __future__ import annotations
 import logging
+import pathlib
 import urllib
 from lxml import etree
 from mimetypes import guess_type
@@ -277,7 +278,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):
@@ -294,12 +294,32 @@ 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,
+        )
+
+        if hasattr(new_xblock, 'data') and substitutions:
+            data_with_substitutions = new_xblock.data
+
+            print(f"Substitutions: {substitutions}")
+
+            for old_static_ref, new_static_ref in substitutions.items():
+                data_with_substitutions = data_with_substitutions.replace(
+                    old_static_ref,
+                    new_static_ref,
+                )
+
+            print(f"data_with_substitutions: {data_with_substitutions}")
+
+            new_xblock.data = data_with_substitutions
+            store.update_item(new_xblock, request.user.id)
+
     return new_xblock, notices
 
 
@@ -444,7 +464,8 @@ def _import_files_into_course(
     course_key: CourseKey,
     staged_content_id: int,
     static_files: list[content_staging_api.StagedContentFileData],
-) -> StaticFileNotices:
+    usage_key: UsageKey,
+) -> (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
@@ -456,66 +477,103 @@ def _import_files_into_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):
+#        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
+#            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,
+) -> (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)
+    # If this came from a library (need to adjust this condition later)
+    clipboard_file_path = file_data_obj.filename
+    if clipboard_file_path.startswith('static/'):
+        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:
+        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("/", "_"))
+
+    # Yeah, I'd prefer a different delimiter, but this is what we already do
+    # during file import.
     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):

openedx/core/djangoapps/content_staging/api.py

@@ -13,7 +13,7 @@
 from opaque_keys.edx.keys import AssetKey, UsageKey
 from xblock.core import XBlock
 
-from openedx.core.lib.xblock_serializer.api import serialize_xblock_to_olx, StaticFile
+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
@@ -38,7 +38,12 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int
     """
     Copy an XBlock's OLX to the user's clipboard.
     """
-    block_data = serialize_xblock_to_olx(block)
+    block_data = XBlockSerializer(
+        block,
+        write_url_name=False,
+        fetch_asset_data=True,
+        normalize_asset_refs=True,
+    )
     usage_key = block.usage_key
 
     expired_ids = []

openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py

@@ -233,10 +233,17 @@ def get_block(self, usage_key, for_parent=None, *, version: int | LatestVersion
 
         return block
 
-    def get_block_assets(self, block):
+    def get_block_assets(self, block, fetch_asset_data):
         """
         Return a list of StaticFile entries.
 
+        If ``fetch_data`` is True, we will read the actual asset file data from
+        storage and return it as part of the ``StaticFiles``. This is expensive,
+        and not necessary for something like writing a new version of the OLX in
+        response to a "Save" in the editor. But it is necessary for something
+        like serializing to the clipboard, where we make full copies of the
+        assets.
+
         TODO: When we want to copy a whole Section at a time, doing these
         lookups one by one is going to get slow. At some point we're going to
         want something to look up a bunch of blocks at once.
@@ -252,17 +259,11 @@ def get_block_assets(self, block):
             .order_by('key')
         )
 
-        # TODO: We're returning both the URL and the data here because this is
-        # invoked from the XBlockSerializer for both saving a new version of a
-        # block, as well as for saving into the clipboard. The clipboard needs
-        # the actual static asset data to be dumped in here, but saving a new
-        # block version doesn't. We should clean this up later so we're not
-        # doing the unnecessary read() calls for saving new versions.
         return [
             StaticFile(
                 name=cvc.key,
                 url=self._absolute_url_for_asset(component_version, cvc.key),
-                data=cvc.content.read_file().read(),
+                data=cvc.content.read_file().read() if fetch_asset_data else None,
             )
             for cvc in cvc_list
         ]

openedx/core/lib/xblock_serializer/api.py

@@ -10,6 +10,9 @@ def serialize_xblock_to_olx(block):
     This class will serialize an XBlock, producing:
         (1) an XML string defining the XBlock and all of its children (inline)
         (2) a list of any static files required by the XBlock and their URL
+
+    This calls XBlockSerializer with all default options. To actually tweak the
+    output, instantiate XBlockSerializer directly.
     """
     return XBlockSerializer(block)
 

openedx/core/lib/xblock_serializer/block_serializer.py

@@ -22,15 +22,19 @@ class XBlockSerializer:
     static_files: list[StaticFile]
     tags: TagValuesByObjectIdDict
 
-    def __init__(self, block):
+    def __init__(self, block, write_url_name=True, fetch_asset_data=False, normalize_asset_refs=False):
         """
         Serialize an XBlock to an OLX string + supporting files, and store the
         resulting data in this object.
         """
+        self.normalize_asset_refs = normalize_asset_refs
+        self.write_url_name = write_url_name
+
         self.orig_block_key = block.scope_ids.usage_id
         self.static_files = []
         self.tags = {}
         olx_node = self._serialize_block(block)
+
         self.olx_str = etree.tostring(olx_node, encoding="unicode", pretty_print=True)
 
         course_key = self.orig_block_key.course_key
@@ -44,7 +48,7 @@ def __init__(self, block):
             # Learning Core backed content supports this, which currently means
             # v2 Content Libraries.
             self.static_files.extend(
-                block.runtime.get_block_assets(block)
+                block.runtime.get_block_assets(block, fetch_asset_data)
             )
         else:
             # Otherwise, we have to scan the content to extract associated asset
@@ -71,6 +75,12 @@ def _serialize_block(self, block) -> etree.Element:
         else:
             olx = self._serialize_normal_block(block)
 
+        # The url_name attribute can come either because it was already in the
+        # block's field data, or because this class adds it in the calls above.
+        # However it gets set though, we can remove it here:
+        if not self.write_url_name:
+            olx.attrib.pop("url_name", None)
+
         # Store the block's tags
         block_key = block.scope_ids.usage_id
         block_id = str(block_key)
@@ -161,12 +171,12 @@ class XBlockSerializerForLearningCore(XBlockSerializer):
         (3) a list of any static files required by the XBlock and their URL
     """
 
-    def __init__(self, block):
+    def __init__(self, block, *args, **kwargs):
         """
         Serialize an XBlock to an OLX string + supporting files, and store the
         resulting data in this object.
         """
-        super().__init__(block)
+        super().__init__(block, *args, **kwargs)
         self.def_id = utils.learning_core_def_key_from_modulestore_usage_key(self.orig_block_key)
 
     def _serialize_block(self, block) -> etree.Element: