feat: Allow specifying a version when loading a v2 XBlock (#35626)

openedx/core/djangoapps/content_libraries/api.py

@@ -743,13 +743,15 @@ def get_library_block(usage_key, include_collections=False) -> LibraryXBlockMeta
     return xblock_metadata
 
 
-def set_library_block_olx(usage_key, new_olx_str):
+def set_library_block_olx(usage_key, new_olx_str) -> int:
     """
     Replace the OLX source of the given XBlock.
 
     This is only meant for use by developers or API client applications, as
     very little validation is done and this can easily result in a broken XBlock
     that won't load.
+
+    Returns the version number of the newly created ComponentVersion.
     """
     # because this old pylint can't understand attr.ib() objects, pylint: disable=no-member
     assert isinstance(usage_key, LibraryUsageLocatorV2)
@@ -786,7 +788,7 @@ def set_library_block_olx(usage_key, new_olx_str):
             text=new_olx_str,
             created=now,
         )
-        authoring_api.create_next_version(
+        new_component_version = authoring_api.create_next_component_version(
             component.pk,
             title=new_title,
             content_to_replace={
@@ -802,6 +804,8 @@ def set_library_block_olx(usage_key, new_olx_str):
         )
     )
 
+    return new_component_version.version_num
+
 
 def library_component_usage_key(
     library_key: LibraryLocatorV2,

openedx/core/djangoapps/content_libraries/serializers.py

@@ -217,6 +217,7 @@ class LibraryXBlockOlxSerializer(serializers.Serializer):
     Serializer for representing an XBlock's OLX
     """
     olx = serializers.CharField()
+    version_num = serializers.IntegerField(read_only=True, required=False)
 
 
 class LibraryXBlockStaticFileSerializer(serializers.Serializer):

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

@@ -36,6 +36,7 @@
 URL_LIB_LTI_LAUNCH = URL_LIB_LTI_PREFIX + 'launch/'
 
 URL_BLOCK_RENDER_VIEW = '/api/xblock/v2/xblocks/{block_key}/view/{view_name}/'
+URL_BLOCK_EMBED_VIEW = '/xblocks/v2/{block_key}/embed/{view_name}/'  # Returns HTML not JSON so its URL is different
 URL_BLOCK_GET_HANDLER_URL = '/api/xblock/v2/xblocks/{block_key}/handler_url/{handler_name}/'
 URL_BLOCK_METADATA_URL = '/api/xblock/v2/xblocks/{block_key}/'
 URL_BLOCK_FIELDS_URL = '/api/xblock/v2/xblocks/{block_key}/fields/'
@@ -300,6 +301,24 @@ def _render_block_view(self, block_key, view_name, expect_response=200):
         url = URL_BLOCK_RENDER_VIEW.format(block_key=block_key, view_name=view_name)
         return self._api('get', url, None, expect_response)
 
+    def _embed_block(
+        self,
+        block_key,
+        *,
+        view_name="student_view",
+        version: str | int | None = None,
+        expect_response=200,
+    ) -> str:
+        """
+        Get an HTML response that displays the given XBlock. Returns HTML.
+        """
+        url = URL_BLOCK_EMBED_VIEW.format(block_key=block_key, view_name=view_name)
+        if version is not None:
+            url += f"?version={version}"
+        response = self.client.get(url)
+        assert response.status_code == expect_response, 'Unexpected response code {}:'.format(response.status_code)
+        return response.content.decode()
+
     def _get_block_handler_url(self, block_key, handler_name):
         """
         Get the URL to call a specific XBlock's handler.

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

@@ -0,0 +1,60 @@
+"""
+Block for testing variously scoped XBlock fields.
+"""
+import json
+
+from webob import Response
+from web_fragments.fragment import Fragment
+from xblock.core import XBlock, Scope
+from xblock import fields
+
+
+class FieldsTestBlock(XBlock):
+    """
+    Block for testing variously scoped XBlock fields and XBlock handlers.
+
+    This has only authored fields. See also UserStateTestBlock which has user fields.
+    """
+    BLOCK_TYPE = "fields-test"
+    has_score = False
+
+    display_name = fields.String(scope=Scope.settings, name='User State Test Block')
+    setting_field = fields.String(scope=Scope.settings, name='A setting')
+    content_field = fields.String(scope=Scope.content, name='A setting')
+
+    @XBlock.json_handler
+    def update_fields(self, data, suffix=None):  # pylint: disable=unused-argument
+        """
+        Update the authored fields of this block
+        """
+        self.display_name = data["display_name"]
+        self.setting_field = data["setting_field"]
+        self.content_field = data["content_field"]
+        return {}
+
+    @XBlock.handler
+    def get_fields(self, request, suffix=None):  # pylint: disable=unused-argument
+        """
+        Get the various fields of this XBlock.
+        """
+        return Response(
+            json.dumps({
+                "display_name": self.display_name,
+                "setting_field": self.setting_field,
+                "content_field": self.content_field,
+            }),
+            content_type='application/json',
+            charset='UTF-8',
+        )
+
+    def student_view(self, _context):
+        """
+        Return the student view.
+        """
+        fragment = Fragment()
+        fragment.add_content(f'<h1>{self.display_name}</h1>\n')
+        fragment.add_content(f'<p>SF: {self.setting_field}</p>\n')
+        fragment.add_content(f'<p>CF: {self.content_field}</p>\n')
+        handler_url = self.runtime.handler_url(self, 'get_fields')
+        fragment.add_content(f'<p>handler URL: {handler_url}</p>\n')
+        return fragment

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

@@ -0,0 +1,184 @@
+"""
+Tests for the XBlock v2 runtime's "embed" view, using Content Libraries
+
+This view is used in the MFE to preview XBlocks that are in the library.
+"""
+import re
+
+import ddt
+from django.core.exceptions import ValidationError
+from django.test.utils import override_settings
+from openedx_events.tests.utils import OpenEdxEventsTestMixin
+import pytest
+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
+@ddt.ddt
+@override_settings(CORS_ORIGIN_WHITELIST=[])  # For some reason, this setting isn't defined in our test environment?
+class LibrariesEmbedViewTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMixin):
+    """
+    Tests for embed_view and interacting with draft/published/past versions of
+    Learning-Core-based XBlocks (in Content Libraries).
+
+    These tests use the REST API, which in turn relies on the Python API.
+    Some tests may use the python API directly if necessary to provide
+    coverage of any code paths not accessible via the REST API.
+
+    In general, these tests should
+    (1) Use public APIs only - don't directly create data using other methods,
+        which results in a less realistic test and ties the test suite too
+        closely to specific implementation details.
+        (Exception: users can be provisioned using a user factory)
+    (2) Assert that fields are present in responses, but don't assert that the
+        entire response has some specific shape. That way, things like adding
+        new fields to an API response, which are backwards compatible, won't
+        break any tests, but backwards-incompatible API changes will.
+
+    WARNING: every test should have a unique library slug, because even though
+    the django/mysql database gets reset for each test case, the lookup between
+    library slug and bundle UUID does not because it's assumed to be immutable
+    and cached forever.
+    """
+
+    @XBlock.register_temp_plugin(FieldsTestBlock, FieldsTestBlock.BLOCK_TYPE)
+    def test_embed_vew_versions(self):
+        """
+        Test that the embed_view renders a block and can render different versions of it.
+        """
+        # 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."
+            />
+        """)
+
+        # Now render the "embed block" view. This test only runs in CMS so it should default to the draft:
+        html = self._embed_block(block_id)
+
+        def check_fields(display_name, setting_value, content_value):
+            assert f'<h1>{display_name}</h1>' in html
+            assert f'<p>SF: {setting_value}</p>' in html
+            assert f'<p>CF: {content_value}</p>' in html
+            handler_url = re.search(r'<p>handler URL: ([^<]+)</p>', html).group(1)
+            assert handler_url.startswith('http')
+            handler_result = self.client.get(handler_url).json()
+            assert handler_result == {
+                "display_name": display_name,
+                "setting_field": setting_value,
+                "content_field": content_value,
+            }
+        check_fields('Field Test Block (Draft, v4)', 'Draft setting value 4.', 'Draft content value 4.')
+
+        # But if we request the published version, we get that:
+        html = self._embed_block(block_id, version="published")
+        check_fields('Field Test Block (Published, v3)', 'Published setting value 3.', 'Published content value 3.')
+
+        # And if we request a specific version, we get that:
+        html = self._embed_block(block_id, version=3)
+        check_fields('Field Test Block (Published, v3)', 'Published setting value 3.', 'Published content value 3.')
+
+        # And if we request a specific version, we get that:
+        html = self._embed_block(block_id, version=2)
+        check_fields('Field Test Block (Old, v2)', 'Old setting value 2.', 'Old content value 2.')
+
+        html = self._embed_block(block_id, version=4)
+        check_fields('Field Test Block (Draft, v4)', 'Draft setting value 4.', 'Draft content value 4.')
+
+    @XBlock.register_temp_plugin(FieldsTestBlock, FieldsTestBlock.BLOCK_TYPE)
+    def test_handlers_modifying_published_data(self):
+        """
+        Test that if we requested any version other than "draft", the handlers should not allow _writing_ to authored
+        field data (because you'd be overwriting the latest draft version with changes based on an old version).
+
+        We may decide to relax this restriction in the future. Not sure how important it is.
+
+        Writing to student state is OK.
+        """
+        # Create a library:
+        lib = self._create_library(slug="test-eb-2", 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"]
+
+        # Now render the "embed block" view. This test only runs in CMS so it should default to the draft:
+        html = self._embed_block(block_id)
+
+        def call_update_handler(**kwargs):
+            handler_url = re.search(r'<p>handler URL: ([^<]+)</p>', html).group(1)
+            assert handler_url.startswith('http')
+            handler_url = handler_url.replace('get_fields', 'update_fields')
+            response = self.client.post(handler_url, kwargs, format='json')
+            assert response.status_code == 200
+
+        def check_fields(display_name, setting_field, content_field):
+            assert f'<h1>{display_name}</h1>' in html
+            assert f'<p>SF: {setting_field}</p>' in html
+            assert f'<p>CF: {content_field}</p>' in html
+
+        # Call the update handler to change the fields on the draft:
+        call_update_handler(display_name="DN-01", setting_field="SV-01", content_field="CV-01")
+
+        # Render the block again and check that the handler was able to update the fields:
+        html = self._embed_block(block_id)
+        check_fields(display_name="DN-01", setting_field="SV-01", content_field="CV-01")
+
+        # Publish the library:
+        self._commit_library_changes(lib_id)
+
+        # Now try changing the authored fields of the published version using a handler:
+        html = self._embed_block(block_id, version="published")
+        expected_msg = "Do not make changes to a component starting from the published or past versions."
+        with pytest.raises(ValidationError, match=expected_msg) as err:
+            call_update_handler(display_name="DN-X", setting_field="SV-X", content_field="CV-X")
+
+        # Now try changing the authored fields of a specific past version using a handler:
+        html = self._embed_block(block_id, version=2)
+        with pytest.raises(ValidationError, match=expected_msg) as err:
+            call_update_handler(display_name="DN-X", setting_field="SV-X", content_field="CV-X")
+
+        # Make sure the fields were not updated:
+        html = self._embed_block(block_id)
+        check_fields(display_name="DN-01", setting_field="SV-01", content_field="CV-01")
+
+    # TODO: test that any static assets referenced in the student_view html are loaded as the correct version, and not
+    # always loaded as "latest draft".
+
+    # TODO: if we are ever able to run these tests in the LMS, test that the LMS only allows accessing the published
+    # version.

openedx/core/djangoapps/content_libraries/views.py

@@ -731,10 +731,10 @@ def post(self, request, usage_key_str):
         serializer.is_valid(raise_exception=True)
         new_olx_str = serializer.validated_data["olx"]
         try:
-            api.set_library_block_olx(key, new_olx_str)
+            version_num = api.set_library_block_olx(key, new_olx_str)
         except ValueError as err:
             raise ValidationError(detail=str(err))  # lint-amnesty, pylint: disable=raise-missing-from
-        return Response(LibraryXBlockOlxSerializer({"olx": new_olx_str}).data)
+        return Response(LibraryXBlockOlxSerializer({"olx": new_olx_str, "version_num": version_num}).data)
 
 
 @method_decorator(non_atomic_requests, name="dispatch")

openedx/core/djangoapps/waffle_utils/__init__.py

@@ -5,7 +5,7 @@
 import logging
 
 from edx_toggles.toggles import WaffleFlag
-from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.keys import CourseKey, LearningContextKey
 
 log = logging.getLogger(__name__)
 
@@ -99,20 +99,24 @@ def _get_course_override_value(self, course_key):
 
     def is_enabled(self, course_key=None):  # pylint: disable=arguments-differ
         """
-        Returns whether or not the flag is enabled within the context of a given course.
+        Returns whether or not the flag is enabled within the context of a given
+        course.
+
+        Can also be given the key of any other learning context (like a content
+        library), but it will act like a regular waffle flag in that case.
 
         Arguments:
             course_key (Optional[CourseKey]): The course to check for override before
                 checking waffle. If omitted, check whether the flag is enabled
                 outside the context of any course.
         """
         if course_key:
-            assert isinstance(
-                course_key, CourseKey
-            ), "Provided course_key '{}' is not instance of CourseKey.".format(
-                course_key
-            )
-        is_enabled_for_course = self._get_course_override_value(course_key)
-        if is_enabled_for_course is not None:
-            return is_enabled_for_course
+            if isinstance(course_key, CourseKey):
+                is_enabled_for_course = self._get_course_override_value(course_key)
+                if is_enabled_for_course is not None:
+                    return is_enabled_for_course
+            else:
+                # In case this gets called with a content library key, that's fine - just ignore it and
+                # act like a normal waffle flag. We currently don't support library-specific overrides.
+                assert isinstance(course_key, LearningContextKey), "expected a course key or other learning context key"
         return super().is_enabled()