Fix versioned URLs for XBlock handlers in v2 runtime [FC-0062]

cms/static/js/views/modals/preview_v2_library_changes.js

@@ -17,7 +17,7 @@ function($, _, gettext, BaseModal, ViewUtils, XBlockViewUtils) {
 
         options: $.extend({}, BaseModal.prototype.options, {
             modalName: 'preview-lib-changes',
-            modalSize: 'med',
+            modalSize: 'lg',
             view: 'studio_view',
             viewSpecificClasses: 'modal-lib-preview confirm',
             // Translators: "title" is the name of the current component being edited.

cms/static/sass/views/_container.scss

@@ -570,7 +570,7 @@
 
     & > iframe {
       width: 100%;
-      min-height: 350px;
+      min-height: 450px;
       background: url('#{$static-path}/images/spinner.gif') center center no-repeat;
     }
   }

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

@@ -292,12 +292,14 @@ def _paste_clipboard_content_in_library(self, lib_key, block_id, expect_response
         data = {"block_id": block_id}
         return self._api('post', url, data, expect_response)
 
-    def _render_block_view(self, block_key, view_name, expect_response=200):
+    def _render_block_view(self, block_key, view_name, version=None, expect_response=200):
         """
         Render an XBlock's view in the active application's runtime.
         Note that this endpoint has different behavior in Studio (draft mode)
         vs. the LMS (published version only).
         """
+        if version is not None:
+            block_key += f"@{version}"
         url = URL_BLOCK_RENDER_VIEW.format(block_key=block_key, view_name=view_name)
         return self._api('get', url, None, expect_response)
 
@@ -328,8 +330,17 @@ def _get_block_handler_url(self, block_key, handler_name):
         url = URL_BLOCK_GET_HANDLER_URL.format(block_key=block_key, handler_name=handler_name)
         return self._api('get', url, None, expect_response=200)["handler_url"]
 
-    def _get_library_block_fields(self, block_key, expect_response=200):
+    def _get_basic_xblock_metadata(self, block_key, version=None, expect_response=200):
+        """ Get basic metadata about a specific block in the library. """
+        if version is not None:
+            block_key += f"@{version}"
+        result = self._api('get', URL_BLOCK_METADATA_URL.format(block_key=block_key), None, expect_response)
+        return result
+
+    def _get_library_block_fields(self, block_key, version=None, expect_response=200):
         """ Get the fields of a specific block in the library. This API is only used by the MFE editors. """
+        if version is not None:
+            block_key += f"@{version}"
         result = self._api('get', URL_BLOCK_FIELDS_URL.format(block_key=block_key), None, expect_response)
         return result
 

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

@@ -1101,10 +1101,6 @@ def test_invalid_key(self, endpoint, endpoint_parameters):
             endpoint.format(**endpoint_parameters),
         )
         self.assertEqual(response.status_code, 404)
-        msg = f"XBlock {endpoint_parameters.get('block_key')} does not exist, or you don't have permission to view it."
-        self.assertEqual(response.json(), {
-            'detail': msg,
-        })
 
     def test_xblock_handler_invalid_key(self):
         """This endpoint is tested separately from the previous ones as it's not a DRF endpoint."""

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

@@ -0,0 +1,104 @@
+"""
+Tests that several XBlock APIs support versioning
+"""
+from django.test.utils import override_settings
+from openedx_events.tests.utils import OpenEdxEventsTestMixin
+from xblock.core import XBlock
+
+from openedx.core.djangoapps.content_libraries.tests.base import (
+    ContentLibrariesRestApiTest
+)
+from openedx.core.djangolib.testing.utils import skip_unless_cms
+from .fields_test_block import FieldsTestBlock
+
+
+@skip_unless_cms
+@override_settings(CORS_ORIGIN_WHITELIST=[])  # For some reason, this setting isn't defined in our test environment?
+class VersionedXBlockApisTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMixin):
+    """
+    Tests for three APIs implemented by djangoapps.xblock, and used by content
+    libraries. These tests focus on versioning.
+
+    Note the metadata endpoint is different than the similar "metadata" endpoint
+    within the content libraries API, which returns a lot more information. This
+    endpoint pretty much only returns the display name of a block, but it does
+    allow retrieving past versions.
+    """
+
+    @XBlock.register_temp_plugin(FieldsTestBlock, FieldsTestBlock.BLOCK_TYPE)
+    def test_versioned_metadata(self):
+        """
+        Test that metadata endpoint can get different versions of the block's metadata
+        """
+        # Create a library:
+        lib = self._create_library(slug="test-eb-1", title="Test Library", description="")
+        lib_id = lib["id"]
+        # Create an XBlock. This will be the empty version 1:
+        create_response = self._add_block_to_library(lib_id, FieldsTestBlock.BLOCK_TYPE, "block1")
+        block_id = create_response["id"]
+        # Create version 2 of the block by setting its OLX:
+        olx_response = self._set_library_block_olx(block_id, """
+            <fields-test
+                display_name="Field Test Block (Old, v2)"
+                setting_field="Old setting value 2."
+                content_field="Old content value 2."
+            />
+        """)
+        assert olx_response["version_num"] == 2
+        # Create version 3 of the block by setting its OLX again:
+        olx_response = self._set_library_block_olx(block_id, """
+            <fields-test
+                display_name="Field Test Block (Published, v3)"
+                setting_field="Published setting value 3."
+                content_field="Published content value 3."
+            />
+        """)
+        assert olx_response["version_num"] == 3
+        # Publish the library:
+        self._commit_library_changes(lib_id)
+
+        # Create the draft (version 4) of the block:
+        olx_response = self._set_library_block_olx(block_id, """
+            <fields-test
+                display_name="Field Test Block (Draft, v4)"
+                setting_field="Draft setting value 4."
+                content_field="Draft content value 4."
+            />
+        """)
+
+        def check_results(version, display_name, settings_field, content_field):
+            meta = self._get_basic_xblock_metadata(block_id, version=version)
+            assert meta["block_id"] == block_id
+            assert meta["block_type"] == FieldsTestBlock.BLOCK_TYPE
+            assert meta["display_name"] == display_name
+            fields = self._get_library_block_fields(block_id, version=version)
+            assert fields["display_name"] == display_name
+            assert fields["metadata"]["setting_field"] == settings_field
+            rendered = self._render_block_view(block_id, "student_view", version=version)
+            assert rendered["block_id"] == block_id
+            assert f"SF: {settings_field}" in rendered["content"]
+            assert f"CF: {content_field}" in rendered["content"]
+
+        # Now get the metadata. If we don't specify a version, it should be the latest draft (in Studio):
+        check_results(
+            version=None,
+            display_name="Field Test Block (Draft, v4)",
+            settings_field="Draft setting value 4.",
+            content_field="Draft content value 4.",
+        )
+
+        # Get the published version:
+        check_results(
+            version="published",
+            display_name="Field Test Block (Published, v3)",
+            settings_field="Published setting value 3.",
+            content_field="Published content value 3.",
+        )
+
+        # Get a specific version:
+        check_results(
+            version="2",
+            display_name="Field Test Block (Old, v2)",
+            settings_field="Old setting value 2.",
+            content_field="Old content value 2.",
+        )

openedx/core/djangoapps/content_libraries/views.py

@@ -617,7 +617,13 @@ class LibraryBlockView(APIView):
     @convert_exceptions
     def get(self, request, usage_key_str):
         """
-        Get metadata about an existing XBlock in the content library
+        Get metadata about an existing XBlock in the content library.
+
+        This API doesn't support versioning; most of the information it returns
+        is related to the latest draft version, or to all versions of the block.
+        If you need to get the display name of a previous version, use the
+        similar "metadata" API from djangoapps.xblock, which does support
+        versioning.
         """
         key = LibraryUsageLocatorV2.from_string(usage_key_str)
         api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)

openedx/core/djangoapps/xblock/api.py

@@ -279,7 +279,6 @@ def get_handler_url(
 
     This view does not check/care if the XBlock actually exists.
     """
-    usage_key_str = str(usage_key)
     site_root_url = get_xblock_app_config().get_site_root_url()
     if not user:
         raise TypeError("Cannot get handler URLs without specifying a specific user ID.")
@@ -291,17 +290,17 @@ def get_handler_url(
         raise ValueError("Invalid user value")
     # Now generate a token-secured URL for this handler, specific to this user
     # and this XBlock:
-    secure_token = get_secure_token_for_xblock_handler(user_id, usage_key_str)
+    secure_token = get_secure_token_for_xblock_handler(user_id, str(usage_key))
     # Now generate the URL to that handler:
     kwargs = {
-        'usage_key_str': usage_key_str,
+        'usage_key': usage_key,
         'user_id': user_id,
         'secure_token': secure_token,
         'handler_name': handler_name,
     }
-    path = reverse('xblock_api:xblock_handler', kwargs=kwargs)
     if version != LatestVersion.AUTO:
-        path += "?version=" + (str(version) if isinstance(version, int) else version.value)
+        kwargs["version"] = version
+    path = reverse('xblock_api:xblock_handler', kwargs=kwargs)
 
     # We must return an absolute URL. We can't just use
     # rest_framework.reverse.reverse to get the absolute URL because this method

openedx/core/djangoapps/xblock/rest_api/url_converters.py

@@ -0,0 +1,57 @@
+"""
+URL pattern converters
+https://docs.djangoproject.com/en/5.1/topics/http/urls/#registering-custom-path-converters
+"""
+from opaque_keys import InvalidKeyError
+from opaque_keys.edx.keys import UsageKeyV2
+
+from ..api import LatestVersion
+
+
+class UsageKeyV2Converter:
+    """
+    Converter that matches V2 usage keys like:
+        lb:Org:LIB:drag-and-drop-v2:91c2b1d5
+    """
+    regex = r'[\w-]+(:[\w\-.]+)+'
+
+    def to_python(self, value: str) -> UsageKeyV2:
+        try:
+            return UsageKeyV2.from_string(value)
+        except InvalidKeyError as exc:
+            raise ValueError from exc
+
+    def to_url(self, value: UsageKeyV2) -> str:
+        return str(value)
+
+
+class VersionConverter:
+    """
+    Converter that matches a version string like "draft", "published", or a
+    number, and converts it to either an 'int' or a LatestVersion enum value.
+    """
+    regex = r'(draft|published|\d+)'
+
+    def to_python(self, value: str | None) -> LatestVersion | int:
+        """ Convert from string to LatestVersion or integer version spec """
+        if value is None:
+            return LatestVersion.AUTO  # AUTO = published if we're in the LMS, draft if we're in Studio.
+        if value == "draft":
+            return LatestVersion.DRAFT
+        if value == "published":
+            return LatestVersion.PUBLISHED
+        return int(value)  # May raise ValueError, which Django will convert to 404
+
+    def to_url(self, value: LatestVersion | int | None) -> str:
+        """
+        Convert from LatestVersion or integer version spec to URL path string.
+
+        Note that if you provide any value at all, django won't be able to
+        match the paths that don't have a version in the URL, so if you want
+        LatestVersion.AUTO, don't pass any value for 'version' to reverse(...).
+        """
+        if value is None or value == LatestVersion.AUTO:
+            raise ValueError  # This default value does not need to be in the URL
+        if isinstance(value, int):
+            return str(value)
+        return value.value  # LatestVersion.DRAFT or PUBLISHED

openedx/core/djangoapps/xblock/rest_api/urls.py

@@ -1,7 +1,8 @@
 """
 URL configuration for the new XBlock API
 """
-from django.urls import include, path, re_path
+from django.urls import include, path, re_path, register_converter
+from . import url_converters
 from . import views
 
 # Note that the exact same API URLs are used in Studio and the LMS, but the API
@@ -10,26 +11,34 @@
 # urls_studio and urls_lms, and/or the views could be likewise duplicated.
 app_name = 'openedx.core.djangoapps.xblock.rest_api'
 
+register_converter(url_converters.UsageKeyV2Converter, "usage_v2")
+register_converter(url_converters.VersionConverter, "block_version")
+
+block_endpoints = [
+    # get metadata about an XBlock:
+    path('', views.block_metadata),
+    # get/post full json fields of an XBlock:
+    path('fields/', views.BlockFieldsView.as_view()),
+    # render one of this XBlock's views (e.g. student_view)
+    path('view/<str:view_name>/', views.render_block_view),
+    # get the URL needed to call this XBlock's handlers
+    path('handler_url/<str:handler_name>/', views.get_handler_url),
+    # call one of this block's handlers
+    re_path(
+        r'^handler/(?P<user_id>\w+)-(?P<secure_token>\w+)/(?P<handler_name>[\w\-]+)/(?P<suffix>.+)?$',
+        views.xblock_handler,
+        name='xblock_handler',
+    ),
+    # API endpoints related to a specific version of this XBlock:
+]
+
 urlpatterns = [
     path('api/xblock/v2/', include([
-        path('xblocks/<str:usage_key_str>/', include([
-            # get metadata about an XBlock:
-            path('', views.block_metadata),
-            # get/post full json fields of an XBlock:
-            path('fields/', views.BlockFieldsView.as_view()),
-            # render one of this XBlock's views (e.g. student_view)
-            re_path(r'^view/(?P<view_name>[\w\-]+)/$', views.render_block_view),
-            # get the URL needed to call this XBlock's handlers
-            re_path(r'^handler_url/(?P<handler_name>[\w\-]+)/$', views.get_handler_url),
-            # call one of this block's handlers
-            re_path(
-                r'^handler/(?P<user_id>\w+)-(?P<secure_token>\w+)/(?P<handler_name>[\w\-]+)/(?P<suffix>.+)?$',
-                views.xblock_handler,
-                name='xblock_handler',
-            ),
-        ])),
+        path(r'xblocks/<usage_v2:usage_key>/', include(block_endpoints)),
+        path(r'xblocks/<usage_v2:usage_key>@<block_version:version>/', include(block_endpoints)),
     ])),
-    path('xblocks/v2/<str:usage_key_str>/', include([
+    # Non-API views (these return HTML, not JSON):
+    path('xblocks/v2/<usage_v2:usage_key>/', include([
         # render one of this XBlock's views (e.g. student_view) for embedding in an iframe
         # NOTE: this endpoint is **unstable** and subject to changes after Sumac
         path('embed/<str:view_name>/', views.embed_block_view),