diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index f567960f394f..64e48c6e5d20 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -49,6 +49,7 @@ URL_BLOCK_RENDER_VIEW = '/api/xblock/v2/xblocks/{block_key}/view/{view_name}/' 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/' URL_BLOCK_XBLOCK_HANDLER = '/api/xblock/v2/xblocks/{block_key}/handler/{user_id}-{secure_token}/{handler_name}/' diff --git a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py index e21fd1edd82f..fefe1536a248 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py @@ -21,6 +21,7 @@ URL_BLOCK_RENDER_VIEW, URL_BLOCK_GET_HANDLER_URL, URL_BLOCK_METADATA_URL, + URL_BLOCK_FIELDS_URL, ) from openedx.core.djangoapps.content_libraries.tests.user_state_block import UserStateTestBlock from openedx.core.djangoapps.content_libraries.constants import COMPLEX, ALL_RIGHTS_RESERVED, CC_4_BY @@ -42,6 +43,9 @@ def setUp(self): self.student_a = UserFactory.create(username="Alice", email="alice@example.com", password="edx") self.student_b = UserFactory.create(username="Bob", email="bob@example.com", password="edx") + # staff user + self.staff_user = UserFactory(password="edx", is_staff=True) + # Create a collection using Blockstore API directly only because there # is not yet any Studio REST API for doing so: self.collection = blockstore_api.create_collection("Content Library Test Collection") @@ -182,6 +186,43 @@ def test_xblock_metadata(self): assert metadata_view_result.data['student_view_data'] is None # Capa doesn't provide student_view_data + @skip_unless_cms # modifying blocks only works properly in Studio + def test_xblock_fields(self): + """ + Test the XBlock fields API + """ + # act as staff: + client = APIClient() + client.login(username=self.staff_user.username, password='edx') + + # create/save a block using the library APIs first + unit_block_key = library_api.create_library_block(self.library.key, "unit", "fields-u1").usage_key + block_key = library_api.create_library_block_child(unit_block_key, "html", "fields-p1").usage_key + new_olx = """ + +

This is some HTML.

+ + """.strip() + library_api.set_library_block_olx(block_key, new_olx) + library_api.publish_changes(self.library.key) + + # Check the GET API for the block: + fields_get_result = client.get(URL_BLOCK_FIELDS_URL.format(block_key=block_key)) + assert fields_get_result.data['display_name'] == 'New Text Block' + assert fields_get_result.data['data'].strip() == '

This is some HTML.

' + assert fields_get_result.data['metadata']['display_name'] == 'New Text Block' + + # Check the POST API for the block: + fields_post_result = client.post(URL_BLOCK_FIELDS_URL.format(block_key=block_key), data={ + 'data': '

test

', + 'metadata': { + 'display_name': 'New Display Name', + } + }, format='json') + block_saved = xblock_api.load_block(block_key, self.staff_user) + assert block_saved.data == '\n

test

\n' + assert xblock_api.get_block_display_name(block_saved) == 'New Display Name' + @requires_blockstore class ContentLibraryRuntimeBServiceTest(ContentLibraryRuntimeTestMixin, TestCase): diff --git a/openedx/core/djangoapps/xblock/rest_api/urls.py b/openedx/core/djangoapps/xblock/rest_api/urls.py index 03132f412f73..034d490aa3a8 100644 --- a/openedx/core/djangoapps/xblock/rest_api/urls.py +++ b/openedx/core/djangoapps/xblock/rest_api/urls.py @@ -15,6 +15,8 @@ path('xblocks//', 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[\w\-]+)/$', views.render_block_view), # get the URL needed to call this XBlock's handlers diff --git a/openedx/core/djangoapps/xblock/rest_api/views.py b/openedx/core/djangoapps/xblock/rest_api/views.py index dc64df6e5049..4af890bb4323 100644 --- a/openedx/core/djangoapps/xblock/rest_api/views.py +++ b/openedx/core/djangoapps/xblock/rest_api/views.py @@ -1,25 +1,28 @@ """ Views that implement a RESTful API for interacting with XBlocks. - -Note that these views are only for interacting with existing blocks. Other -Studio APIs cover use cases like adding/deleting/editing blocks. """ +from common.djangoapps.util.json_request import JsonResponse from corsheaders.signals import check_request_enabled from django.contrib.auth import get_user_model +from django.db.transaction import atomic from django.http import Http404 +from django.utils.translation import gettext as _ from django.views.decorators.clickjacking import xframe_options_exempt from django.views.decorators.csrf import csrf_exempt from rest_framework import permissions from rest_framework.decorators import api_view, permission_classes # lint-amnesty, pylint: disable=unused-import from rest_framework.exceptions import PermissionDenied, AuthenticationFailed, NotFound from rest_framework.response import Response +from rest_framework.views import APIView from xblock.django.request import DjangoWebobRequest, webob_to_django_response +from xblock.fields import Scope from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey from openedx.core.lib.api.view_utils import view_auth_classes from ..api import ( + get_block_display_name, get_block_metadata, get_handler_url as _get_handler_url, load_block, @@ -168,3 +171,86 @@ def cors_allow_xblock_handler(sender, request, **kwargs): # lint-amnesty, pylin check_request_enabled.connect(cors_allow_xblock_handler) + + +@view_auth_classes() +class BlockFieldsView(APIView): + """ + View to get/edit the field values of an XBlock as JSON (in the v2 runtime) + + This class mimics the functionality of xblock_handler in block.py (for v1 xblocks), but for v2 xblocks. + However, it only implements the exact subset of functionality needed to support the v2 editors (from + the frontend-lib-content-components project). As such, it only supports GET and POST, and only the + POSTing of data/metadata fields. + """ + + @atomic + def get(self, request, usage_key_str): + """ + retrieves the xblock, returning display_name, data, and metadata + """ + try: + usage_key = UsageKey.from_string(usage_key_str) + except InvalidKeyError as e: + raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e + + block = load_block(usage_key, request.user) + block_dict = { + "display_name": get_block_display_name(block), # potentially duplicated from metadata + "data": block.data, + "metadata": block.get_explicitly_set_fields_by_scope(Scope.settings), + } + return Response(block_dict) + + @atomic + def post(self, request, usage_key_str): + """ + edits the xblock, saving changes to data and metadata only (display_name included in metadata) + """ + try: + usage_key = UsageKey.from_string(usage_key_str) + except InvalidKeyError as e: + raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e + + user = request.user + block = load_block(usage_key, user) + data = request.data.get("data") + metadata = request.data.get("metadata") + + old_metadata = block.get_explicitly_set_fields_by_scope(Scope.settings) + old_content = block.get_explicitly_set_fields_by_scope(Scope.content) + + block.data = data + + # update existing metadata with submitted metadata (which can be partial) + # IMPORTANT NOTE: if the client passed 'null' (None) for a piece of metadata that means 'remove it'. + if metadata is not None: + for metadata_key, value in metadata.items(): + field = block.fields[metadata_key] + + if value is None: + field.delete_from(block) + else: + try: + value = field.from_json(value) + except ValueError as verr: + reason = _("Invalid data") + if str(verr): + reason = _("Invalid data ({details})").format( + details=str(verr) + ) + return JsonResponse({"error": reason}, 400) + + field.write_to(block, value) + + if callable(getattr(block, "editor_saved", None)): + block.editor_saved(user, old_metadata, old_content) + + # Save after the callback so any changes made in the callback will get persisted. + block.save() + + return Response({ + "id": str(block.location), + "data": data, + "metadata": block.get_explicitly_set_fields_by_scope(Scope.settings), + }) diff --git a/openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py b/openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py index 8c73b5b87556..33ea8514b632 100644 --- a/openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py @@ -14,13 +14,13 @@ from openedx.core.djangoapps.xblock.learning_context.manager import get_learning_context_impl from openedx.core.djangoapps.xblock.runtime.runtime import XBlockRuntime from openedx.core.djangoapps.xblock.runtime.olx_parsing import parse_xblock_include, BundleFormatException -from openedx.core.djangoapps.xblock.runtime.serializer import serialize_xblock from openedx.core.djangolib.blockstore_cache import ( BundleCache, get_bundle_file_data_with_cache, get_bundle_file_metadata_with_cache, ) from openedx.core.lib import blockstore_api +from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_blockstore log = logging.getLogger(__name__) @@ -133,7 +133,9 @@ def save_block(self, block): if not learning_context.can_edit_block(self.user, block.scope_ids.usage_id): log.warning("User %s does not have permission to edit %s", self.user.username, block.scope_ids.usage_id) raise RuntimeError("You do not have permission to edit this XBlock") - olx_str, static_files = serialize_xblock(block) + serialized = serialize_modulestore_block_for_blockstore(block) + olx_str = serialized.olx_str + static_files = serialized.static_files # Write the OLX file to the bundle: draft_uuid = blockstore_api.get_or_create_bundle_draft( definition_key.bundle_uuid, definition_key.draft_name diff --git a/openedx/core/djangoapps/xblock/runtime/serializer.py b/openedx/core/djangoapps/xblock/runtime/serializer.py deleted file mode 100644 index 693a0f1577e0..000000000000 --- a/openedx/core/djangoapps/xblock/runtime/serializer.py +++ /dev/null @@ -1,139 +0,0 @@ -""" -Code to serialize an XBlock to OLX -""" - -from collections import namedtuple -from contextlib import contextmanager -import logging -import os - -from fs.memoryfs import MemoryFS -from fs.wrapfs import WrapFS -from lxml.etree import Element -from lxml.etree import tostring as etree_tostring - -from xmodule.xml_block import XmlMixin - -log = logging.getLogger(__name__) - -# A static file required by an XBlock -StaticFile = namedtuple('StaticFile', ['name', 'data']) - - -def serialize_xblock(block): - """ - Given an XBlock instance, serialize it to OLX - - Returns - (olx_str, static_files) - where olx_str is the XML as a string, and static_files is a list of - StaticFile objects for any small data files that the XBlock may need for - complete serialization (e.g. video subtitle files or a .html data file for - an HTML block). - """ - static_files = [] - # Create an XML node to hold the exported data - olx_node = Element("root") # The node name doesn't matter: add_xml_to_node will change it - # ^ Note: We could pass nsmap=xblock.core.XML_NAMESPACES here, but the - # resulting XML namespace attributes don't seem that useful? - - with override_export_fs(block) as filesystem: # Needed for XBlocks that inherit XModuleDescriptor - # Tell the block to serialize itself as XML/OLX: - if not block.has_children: - block.add_xml_to_node(olx_node) - else: - # We don't want the children serialized at this time, because - # otherwise we can't tell which files in 'filesystem' belong to - # this block and which belong to its children. So, temporarily - # disable any children: - children = block.children - block.children = [] - block.add_xml_to_node(olx_node) - block.children = children - - # Now the block/module may have exported addtional data as files in - # 'filesystem'. If so, store them: - for item in filesystem.walk(): # pylint: disable=not-callable - for unit_file in item.files: - file_path = os.path.join(item.path, unit_file.name) - with filesystem.open(file_path, 'rb') as fh: - data = fh.read() - static_files.append(StaticFile(name=unit_file.name, data=data)) - - # Remove 'url_name' - we store the definition key in the folder name - # that holds the OLX and the usage key elsewhere, so specifying it - # within the OLX file is redundant and can lead to issues if the file is - # copied and pasted elsewhere in the bundle with a new definition key. - olx_node.attrib.pop('url_name', None) - - # Add tags for each child: - if block.has_children and block.children: - try: - child_includes = block.runtime.child_includes_of(block) - except AttributeError: - raise RuntimeError("Cannot get child includes of block. Make sure it's using BlockstoreXBlockRuntime") # lint-amnesty, pylint: disable=raise-missing-from - if len(child_includes) != len(block.children): - raise RuntimeError( - "Mistmatch between block.children and runtime.child_includes_of()." - "Make sure the block was loaded via runtime.get_block() and that " - "the block.children field was not modified directly (use " - "block.runtime.add_child_include() instead)." - ) - for include_data in child_includes: - definition_str = include_data.block_type + "/" + include_data.definition_id - attrs = {"definition": definition_str} - if include_data.usage_hint: - attrs["usage"] = include_data.usage_hint - if include_data.link_id: - attrs["source"] = include_data.link_id - olx_node.append(olx_node.makeelement("xblock-include", attrs)) - - # Serialize the resulting XML to a string: - olx_str = etree_tostring(olx_node, encoding="utf-8", pretty_print=True) - return (olx_str, static_files) - - -@contextmanager -def override_export_fs(block): - """ - Hack that makes some legacy XBlocks which inherit `XmlMixin.add_xml_to_node` - instead of the usual `XmlSerialization.add_xml_to_node` serializable to a string. - This is needed for the OLX export API. - - Originally, `add_xml_to_node` was `XModuleDescriptor`'s method and was migrated to `XmlMixin` - as part of the content core platform refactoring. It differs from `XmlSerialization.add_xml_to_node` - in that it relies on `XmlMixin.export_to_file` (or `CustomTagBlock.export_to_file`) method to control - whether a block has to be exported as two files (one .olx pointing to one .xml) file, or a single XML node. - - For the legacy blocks (`AnnotatableBlock` for instance) `export_to_file` returns `True` by default. - The only exception is `CustomTagBlock`, for which this method was originally developed, as customtags don't - have to be exported as separate files. - - This method temporarily replaces a block's runtime's `export_fs` system with an in-memory filesystem. - Also, it abuses the `XmlMixin.export_to_file` API to prevent the XBlock export code from exporting - each block as two files (one .olx pointing to one .xml file). - - Although `XModuleDescriptor` has been removed a long time ago, we have to keep this hack untill the legacy - `add_xml_to_node` implementation is removed in favor of `XmlSerialization.add_xml_to_node`, which itself - is a hard task involving refactoring of `CourseExportManager`. - """ - fs = WrapFS(MemoryFS()) - fs.makedir('course') - fs.makedir('course/static') # Video XBlock requires this directory to exists, to put srt files etc. - - old_export_fs = block.runtime.export_fs - block.runtime.export_fs = fs - if hasattr(block, 'export_to_file'): - old_export_to_file = block.export_to_file - block.export_to_file = lambda: False - old_global_export_to_file = XmlMixin.export_to_file - XmlMixin.export_to_file = lambda _: False # So this applies to child blocks that get loaded during export - try: - yield fs - except: # lint-amnesty, pylint: disable=try-except-raise - raise - finally: - block.runtime.export_fs = old_export_fs - if hasattr(block, 'export_to_file'): - block.export_to_file = old_export_to_file - XmlMixin.export_to_file = old_global_export_to_file diff --git a/openedx/core/lib/xblock_serializer/utils.py b/openedx/core/lib/xblock_serializer/utils.py index 389571e4cb3b..75dea641fa8a 100644 --- a/openedx/core/lib/xblock_serializer/utils.py +++ b/openedx/core/lib/xblock_serializer/utils.py @@ -45,7 +45,8 @@ def rewrite_absolute_static_urls(text, course_id): /static/SCI_1.2_Image_.png format for consistency and portability. """ - assert isinstance(course_id, CourseKey) + if not course_id.is_course: + return text # We can't rewrite URLs for libraries, which don't have "Files & Uploads". asset_full_url_re = r'https?://[^/]+/(?P[^\s\'"&]+)' def check_asset_key(match_obj):