Merge branch 'master' into kiram15/upgrade-edx-enterprise-08c8e9d

README.rst

@@ -70,6 +70,11 @@ complexity of Open edX configuration and deployment into their own hands.
 System Dependencies
 -------------------
 
+OS:
+* Ubuntu 20.04
+
+* Ubuntu 22.04
+
 Interperters/Tools:
 
 * Python 3.11

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
@@ -11,7 +12,7 @@
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.utils.translation import gettext as _
-from opaque_keys.edx.keys import AssetKey, CourseKey, UsageKey
+from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locator import DefinitionLocator, LocalId
 from xblock.core import XBlock
 from xblock.fields import ScopeIds
@@ -192,6 +193,8 @@ def xblock_type_display_name(xblock, default_display_name=None):
         return _('Problem')
     elif category == 'library_v2':
         return _('Library Content')
+    elif category == 'itembank':
+        return _('Problem Bank')
     component_class = XBlock.load_class(category)
     if hasattr(component_class, 'display_name') and component_class.display_name.default:
         return _(component_class.display_name.default)  # lint-amnesty, pylint: disable=translation-of-non-string
@@ -278,7 +281,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):
@@ -295,12 +297,29 @@ 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,
+        )
+
+        # 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)
+
     return new_xblock, notices
 
 
@@ -454,78 +473,122 @@ def _import_files_into_course(
     course_key: CourseKey,
     staged_content_id: int,
     static_files: list[content_staging_api.StagedContentFileData],
-) -> StaticFileNotices:
+    usage_key: UsageKey,
+) -> tuple[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
+    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
     exist.
+
+    This function returns a tuple of StaticFileNotices (assets added, errors,
+    conflicts), and static asset path substitutions that should be made in the
+    OLX in order to paste this content into this course. The latter is for the
+    case in which we're brining content in from a v2 library, which stores
+    static assets locally to a Component and needs to go into a subdirectory
+    when pasting into a course to avoid overwriting commonly named things, e.g.
+    "figure1.png".
     """
     # List of files that were newly added to the destination course
     new_files = []
     # List of files that conflicted with identically named files already in the destination 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):
-            # 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
         # 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,
+) -> tuple[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)
+    clipboard_file_path = file_data_obj.filename
+
+    # We need to generate an AssetKey to add an asset to a course. The mapping
+    # of directories '/' -> '_' is a long-existing contentstore convention that
+    # we're not going to attempt to change.
+    if clipboard_file_path.startswith('static/'):
+        # If it's in this form, it came from a library and assumes component-local assets
+        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:
+        # Otherwise it came from a course...
+        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("/", "_"))
+
     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):

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

@@ -2,6 +2,7 @@
 Unit tests for /api/contentstore/v2/downstreams/* JSON APIs.
 """
 from unittest.mock import patch
+from django.conf import settings
 
 from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream
 from common.djangoapps.student.tests.factories import UserFactory
@@ -12,7 +13,13 @@
 from .. import downstreams as downstreams_views
 
 
-MOCK_UPSTREAM_REF = "mock-upstream-ref"
+MOCK_LIB_KEY = "lib:OpenedX:CSPROB3"
+MOCK_UPSTREAM_REF = "lb:OpenedX:CSPROB3:html:843b4c73-1e2d-4ced-a0ff-24e503cdb3e4"
+MOCK_UPSTREAM_LINK = "{mfe_url}/library/{lib_key}/components?usageKey={usage_key}".format(
+    mfe_url=settings.COURSE_AUTHORING_MICROFRONTEND_URL,
+    lib_key=MOCK_LIB_KEY,
+    usage_key=MOCK_UPSTREAM_REF,
+)
 MOCK_UPSTREAM_ERROR = "your LibraryGPT subscription has expired"
 
 
@@ -92,6 +99,7 @@ def test_200_good_upstream(self):
         assert response.data['upstream_ref'] == MOCK_UPSTREAM_REF
         assert response.data['error_message'] is None
         assert response.data['ready_to_sync'] is True
+        assert response.data['upstream_link'] == MOCK_UPSTREAM_LINK
 
     @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_bad)
     def test_200_bad_upstream(self):
@@ -104,6 +112,7 @@ def test_200_bad_upstream(self):
         assert response.data['upstream_ref'] == MOCK_UPSTREAM_REF
         assert response.data['error_message'] == MOCK_UPSTREAM_ERROR
         assert response.data['ready_to_sync'] is False
+        assert response.data['upstream_link'] is None
 
     def test_200_no_upstream(self):
         """
@@ -115,6 +124,7 @@ def test_200_no_upstream(self):
         assert response.data['upstream_ref'] is None
         assert "is not linked" in response.data['error_message']
         assert response.data['ready_to_sync'] is False
+        assert response.data['upstream_link'] is None
 
 
 class PutDownstreamViewTest(_DownstreamViewTestMixin, SharedModuleStoreTestCase):

cms/djangoapps/contentstore/utils.py

@@ -436,7 +436,7 @@ def get_library_content_picker_url(course_locator) -> str:
     content_picker_url = None
     if libraries_v2_enabled():
         mfe_base_url = get_course_authoring_url(course_locator)
-        content_picker_url = f'{mfe_base_url}/component-picker'
+        content_picker_url = f'{mfe_base_url}/component-picker?variant=published'
 
     return content_picker_url
 

cms/djangoapps/contentstore/views/component.py

@@ -47,6 +47,7 @@
     'discussion',
     'library',
     'library_v2',  # Not an XBlock
+    'itembank',
     'html',
     'openassessment',
     'problem',
@@ -67,7 +68,7 @@
     "add-xblock-component-support-legend", "add-xblock-component-support-level", "add-xblock-component-menu-problem",
     "xblock-string-field-editor", "xblock-access-editor", "publish-xblock", "publish-history", "tag-list",
     "unit-outline", "container-message", "container-access", "license-selector", "copy-clipboard-button",
-    "edit-title-button",
+    "edit-title-button", "edit-upstream-alert",
 ]
 
 
@@ -262,6 +263,7 @@ def create_support_legend_dict():
         'openassessment': _("Open Response"),
         'library': _("Legacy Library"),
         'library_v2': _("Library Content"),
+        'itembank': _("Problem Bank"),
         'drag-and-drop-v2': _("Drag and Drop"),
     }
 
@@ -488,6 +490,7 @@ def _filter_disabled_blocks(all_blocks):
     disabled_block_names = [block.name for block in disabled_xblocks()]
     if not libraries_v2_enabled():
         disabled_block_names.append('library_v2')
+        disabled_block_names.append('itembank')
     return [block_name for block_name in all_blocks if block_name not in disabled_block_names]
 
 

cms/djangoapps/contentstore/views/preview.py

@@ -300,8 +300,9 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
             selected_groups_label = _('Access restricted to: {list_of_groups}').format(list_of_groups=selected_groups_label)  # lint-amnesty, pylint: disable=line-too-long
         course = modulestore().get_course(xblock.location.course_key)
         can_edit = context.get('can_edit', True)
+        can_add = context.get('can_add', True)
         # Is this a course or a library?
-        is_course = xblock.scope_ids.usage_id.context_key.is_course
+        is_course = xblock.context_key.is_course
         tags_count_map = context.get('tags_count_map')
         tags_count = 0
         if tags_count_map:
@@ -320,7 +321,10 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
             'is_selected': context.get('is_selected', False),
             'selectable': context.get('selectable', False),
             'selected_groups_label': selected_groups_label,
-            'can_add': context.get('can_add', True),
+            'can_add': can_add,
+            # Generally speaking, "if you can add, you can delete". One exception is itembank (Problem Bank)
+            # which has its own separate "add" workflow but uses the normal delete workflow for its child blocks.
+            'can_delete': can_add or (root_xblock and root_xblock.scope_ids.block_type == "itembank" and can_edit),
             'can_move': context.get('can_move', is_course),
             'language': getattr(course, 'language', None),
             'is_course': is_course,

cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py

@@ -498,6 +498,64 @@ def test_paste_from_library_read_only_tags(self):
             assert object_tag.value in self.lib_block_tags
             assert object_tag.is_copied
 
+    def test_paste_from_library_copies_asset(self):
+        """
+        Assets from a library component copied into a subdir of Files & Uploads.
+        """
+        # This is the binary for a real, 1px webp file – we need actual image
+        # data because contentstore will try to make a thumbnail and grab
+        # metadata.
+        webp_raw_data = b'RIFF\x16\x00\x00\x00WEBPVP8L\n\x00\x00\x00/\x00\x00\x00\x00E\xff#\xfa\x1f'
+
+        # First add the asset.
+        library_api.add_library_block_static_asset_file(
+            self.lib_block_key,
+            "static/1px.webp",
+            webp_raw_data,
+        )  # v==4
+
+        # Now add the reference to the asset
+        library_api.set_library_block_olx(self.lib_block_key, """
+        <problem display_name="MCQ-draft" max_attempts="5">
+            <p>Including this totally real image: <img src="/static/1px.webp" /></p>
+            <multiplechoiceresponse>
+                <label>Q</label>
+                <choicegroup type="MultipleChoice">
+                    <choice correct="false">Wrong</choice>
+                    <choice correct="true">Right</choice>
+                </choicegroup>
+            </multiplechoiceresponse>
+        </problem>
+        """)  # v==5
+
+        copy_response = self.client.post(
+            CLIPBOARD_ENDPOINT,
+            {"usage_key": str(self.lib_block_key)},
+            format="json"
+        )
+        assert copy_response.status_code == 200
+
+        paste_response = self.client.post(XBLOCK_ENDPOINT, {
+            "parent_locator": str(self.course.usage_key),
+            "staged_content": "clipboard",
+        }, format="json")
+        assert paste_response.status_code == 200
+
+        new_block_key = UsageKey.from_string(paste_response.json()["locator"])
+        new_block = modulestore().get_item(new_block_key)
+
+        # Check that the substitution worked.
+        expected_import_path = f"components/{new_block_key.block_type}/{new_block_key.block_id}/1px.webp"
+        assert f"/static/{expected_import_path}" in new_block.data
+
+        # Check that the asset was copied over properly
+        image_asset = contentstore().find(
+            self.course.id.make_asset_key("asset", expected_import_path.replace('/', '_'))
+        )
+        assert image_asset.import_path == expected_import_path
+        assert image_asset.name == "1px.webp"
+        assert image_asset.length == len(webp_raw_data)
+
 
 class ClipboardPasteFromV1LibraryTestCase(ModuleStoreTestCase):
     """