diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 706a202737..c7a39be8f7 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -18,6 +18,7 @@ from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import modulestore +from xmodule.xml_block import XmlMixin from cms.djangoapps.models.settings.course_grading import CourseGradingModel import openedx.core.djangoapps.content_staging.api as content_staging_api @@ -238,7 +239,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> if not user_clipboard: # Clipboard is empty or expired/error/loading return None, StaticFileNotices() - block_type = user_clipboard.content.block_type 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) @@ -246,35 +246,15 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> with store.bulk_operations(parent_key.course_key): parent_descriptor = store.get_item(parent_key) # Some blocks like drag-and-drop only work here with the full XBlock runtime loaded: - parent_xblock = _load_preview_block(request, parent_descriptor) - runtime = parent_xblock.runtime - # Generate the new ID: - id_generator = ImportIdGenerator(parent_key.context_key) - def_id = id_generator.create_definition(block_type, user_clipboard.source_usage_key.block_id) - usage_id = id_generator.create_usage(def_id) - keys = ScopeIds(None, block_type, def_id, usage_id) - # parse_xml is a really messy API. We pass both 'keys' and 'id_generator' and, depending on the XBlock, either - # one may be used to determine the new XBlock's usage key, and the other will be ignored. e.g. video ignores - # 'keys' and uses 'id_generator', but the default XBlock parse_xml ignores 'id_generator' and uses 'keys'. - # For children of this block, obviously only id_generator is used. - xblock_class = runtime.load_block_type(block_type) - # Note: if we find a case where any XBlock needs access to the block-specific static files that were saved to - # export_fs during copying, we could make them available here via runtime.resources_fs before calling parse_xml. - # However, currently the only known case for that is video block's transcript files, and those will - # automatically be "carried over" to the new XBlock even in a different course because the video ID is the same, - # and VAL will thus make the transcript available. - temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator) - if xblock_class.has_children and temp_xblock.children: - raise NotImplementedError("We don't yet support pasting XBlocks with children") - temp_xblock.parent = parent_key - # Store a reference to where this block was copied from, in the 'copied_from_block' field (AuthoringMixin) - temp_xblock.copied_from_block = str(user_clipboard.source_usage_key) - # Save the XBlock into modulestore. We need to save the block and its parent for this to work: - new_xblock = store.update_item(temp_xblock, request.user.id, allow_not_found=True) - parent_xblock.children.append(new_xblock.location) - store.update_item(parent_xblock, request.user.id) - + new_xblock = _import_xml_node_to_parent( + node, + parent_xblock, + store, + user_id=request.user.id, + slug_hint=user_clipboard.source_usage_key.block_id, + copied_from_block=str(user_clipboard.source_usage_key), + ) # Now handle static files that need to go into Files & Uploads: notices = _import_files_into_course( course_key=parent_key.context_key, @@ -285,6 +265,80 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> return new_xblock, notices +def _import_xml_node_to_parent( + node, + parent_xblock: XBlock, + # The modulestore we're using + store, + # The ID of the user who is performing this operation + user_id: int, + # Hint to use as usage ID (block_id) for the new XBlock + slug_hint: str | None = None, + # UsageKey of the XBlock that this one is a copy of + copied_from_block: str | None = None, +) -> XBlock: + """ + Given an XML node representing a serialized XBlock (OLX), import it into modulestore 'store' as a child of the + specified parent block. Recursively copy children as needed. + """ + runtime = parent_xblock.runtime + parent_key = parent_xblock.scope_ids.usage_id + block_type = node.tag + + # Generate the new ID: + id_generator = ImportIdGenerator(parent_key.context_key) + def_id = id_generator.create_definition(block_type, slug_hint) + usage_id = id_generator.create_usage(def_id) + keys = ScopeIds(None, block_type, def_id, usage_id) + # parse_xml is a really messy API. We pass both 'keys' and 'id_generator' and, depending on the XBlock, either + # one may be used to determine the new XBlock's usage key, and the other will be ignored. e.g. video ignores + # 'keys' and uses 'id_generator', but the default XBlock parse_xml ignores 'id_generator' and uses 'keys'. + # For children of this block, obviously only id_generator is used. + xblock_class = runtime.load_block_type(block_type) + # Note: if we find a case where any XBlock needs access to the block-specific static files that were saved to + # export_fs during copying, we could make them available here via runtime.resources_fs before calling parse_xml. + # However, currently the only known case for that is video block's transcript files, and those will + # automatically be "carried over" to the new XBlock even in a different course because the video ID is the same, + # and VAL will thus make the transcript available. + + child_nodes = [] + if not xblock_class.has_children: + # No children to worry about. The XML may contain child nodes, but they're not XBlocks. + temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator) + else: + # We have to handle the children ourselves, because there are lots of complex interactions between + # * the vanilla XBlock parse_xml() method, and its lack of API for "create and save a new XBlock" + # * the XmlMixin version of parse_xml() which only works with ImportSystem, not modulestore or the v2 runtime + # * the modulestore APIs for creating and saving a new XBlock, which work but don't support XML parsing. + # We can safely assume that if the XBLock class supports children, every child node will be the XML + # serialization of a child block, in order. For blocks that don't support children, their XML content/nodes + # could be anything (e.g. HTML, capa) + node_without_children = etree.Element(node.tag, **node.attrib) + if issubclass(xblock_class, XmlMixin): + # Hack: XBlocks that use "XmlMixin" have their own XML parsing behavior, and in particular if they encounter + # an XML node that has no children and has only a "url_name" attribute, they'll try to load the XML data + # from an XML file in runtime.resources_fs. But that file doesn't exist here. So we set at least one + # additional attribute here to make sure that url_name is not the only attribute; otherwise in some cases, + # XmlMixin.parse_xml will try to load an XML file that doesn't exist, giving an error. The name and value + # of this attribute don't matter and should be ignored. + node_without_children.attrib["x-is-pointer-node"] = "no" + temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys, id_generator) + child_nodes = list(node) + if xblock_class.has_children and temp_xblock.children: + raise NotImplementedError("We don't yet support pasting XBlocks with children") + temp_xblock.parent = parent_key + if copied_from_block: + # Store a reference to where this block was copied from, in the 'copied_from_block' field (AuthoringMixin) + temp_xblock.copied_from_block = copied_from_block + # Save the XBlock into modulestore. We need to save the block and its parent for this to work: + new_xblock = store.update_item(temp_xblock, user_id, allow_not_found=True) + parent_xblock.children.append(new_xblock.location) + store.update_item(parent_xblock, user_id) + for child_node in child_nodes: + _import_xml_node_to_parent(child_node, new_xblock, store, user_id=user_id) + return new_xblock + + def _import_files_into_course( course_key: CourseKey, staged_content_id: int, diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py b/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py index 7c3de4fa79..7516bba372 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py @@ -3,7 +3,6 @@ from django import forms import edx_api_doc_tools as apidocs from opaque_keys.edx.keys import CourseKey -from rest_framework import status from rest_framework.exceptions import ValidationError from rest_framework.request import Request from rest_framework.response import Response @@ -12,7 +11,6 @@ from xmodule.modulestore.django import modulestore from cms.djangoapps.models.settings.course_metadata import CourseMetadata from cms.djangoapps.contentstore.api.views.utils import get_bool_param -from cms.djangoapps.contentstore.toggles import use_new_advanced_settings_page from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes from ..serializers import CourseAdvancedSettingsSerializer @@ -117,8 +115,6 @@ class AdvancedCourseSettingsView(DeveloperErrorViewMixin, APIView): if not filter_query_data.is_valid(): raise ValidationError(filter_query_data.errors) course_key = CourseKey.from_string(course_id) - if not use_new_advanced_settings_page(course_key): - return Response(status=status.HTTP_403_FORBIDDEN) if not has_studio_read_access(request.user, course_key): self.permission_denied(request) course_block = modulestore().get_course(course_key) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py index 0b65389596..feec260620 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py @@ -34,8 +34,10 @@ class CourseSettingsSerializer(serializers.Serializer): is_prerequisite_courses_enabled = serializers.BooleanField() language_options = serializers.ListField(child=serializers.ListField(child=serializers.CharField())) lms_link_for_about_page = serializers.URLField() + licensing_enabled = serializers.BooleanField() marketing_enabled = serializers.BooleanField() mfe_proctored_exam_settings_url = serializers.CharField(required=False, allow_null=True, allow_blank=True) + platform_name = serializers.CharField() possible_pre_requisite_courses = PossiblePreRequisiteCourseSerializer(required=False, many=True) short_description_editable = serializers.BooleanField() show_min_grade_warning = serializers.BooleanField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/settings.py index e42b7d85da..e921ac6039 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/settings.py @@ -74,9 +74,11 @@ class CourseSettingsView(DeveloperErrorViewMixin, APIView): ], ... ], + "licensing_enabled": false, "lms_link_for_about_page": "http://localhost:18000/courses/course-v1:edX+E2E-101+course/about", "marketing_enabled": true, "mfe_proctored_exam_settings_url": "", + "platform_name": "edX", "possible_pre_requisite_courses": [ { "course_key": "course-v1:edX+M12+2T2023", @@ -108,6 +110,8 @@ class CourseSettingsView(DeveloperErrorViewMixin, APIView): 'can_show_certificate_available_date_field': can_show_certificate_available_date_field(course_block), 'course_display_name': course_block.display_name, 'course_display_name_with_default': course_block.display_name_with_default, + 'platform_name': settings.PLATFORM_NAME, + 'licensing_enabled': settings.FEATURES.get("LICENSING", False), 'use_v2_cert_display_settings': settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS", False), }) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py index 4831ad4899..2c867ad094 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py @@ -48,10 +48,12 @@ class CourseSettingsViewTest(CourseTestCase, PermissionAccessMixin): "mfe_proctored_exam_settings_url": get_proctored_exam_settings_url( self.course.id ), + "platform_name": settings.PLATFORM_NAME, "short_description_editable": True, "sidebar_html_enabled": False, "show_min_grade_warning": False, "upgrade_deadline": None, + "licensing_enabled": False, "use_v2_cert_display_settings": False, } diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index d17745654a..429630ac8d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -5,7 +5,7 @@ APIs. """ from opaque_keys.edx.keys import UsageKey from rest_framework.test import APIClient -from xmodule.modulestore.django import contentstore, modulestore +from xmodule.modulestore.django import contentstore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory @@ -34,7 +34,7 @@ class ClipboardPasteTestCase(ModuleStoreTestCase): # Check how many blocks are in the vertical currently parent_key = course_key.make_usage_key("vertical", "vertical_test") # This is the vertical that holds the video - orig_vertical = modulestore().get_item(parent_key) + orig_vertical = self.store.get_item(parent_key) assert len(orig_vertical.children) == 4 # Copy the video @@ -51,16 +51,54 @@ class ClipboardPasteTestCase(ModuleStoreTestCase): new_block_key = UsageKey.from_string(paste_response.json()["locator"]) # Now there should be an extra block in the vertical: - updated_vertical = modulestore().get_item(parent_key) + updated_vertical = self.store.get_item(parent_key) assert len(updated_vertical.children) == 5 assert updated_vertical.children[-1] == new_block_key # And it should match the original: - orig_video = modulestore().get_item(video_key) - new_video = modulestore().get_item(new_block_key) + orig_video = self.store.get_item(video_key) + new_video = self.store.get_item(new_block_key) assert new_video.youtube_id_1_0 == orig_video.youtube_id_1_0 # The new block should store a reference to where it was copied from assert new_video.copied_from_block == str(video_key) + def test_copy_and_paste_unit(self): + """ + Test copying a unit (vertical) from one course into another + """ + course_key, client = self._setup_course() + dest_course = CourseFactory.create(display_name='Destination Course') + with self.store.bulk_operations(dest_course.id): + dest_chapter = BlockFactory.create(parent=dest_course, category='chapter', display_name='Section') + dest_sequential = BlockFactory.create(parent=dest_chapter, category='sequential', display_name='Subsection') + + # Copy the unit + unit_key = course_key.make_usage_key("vertical", "vertical_test") + copy_response = client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(unit_key)}, format="json") + assert copy_response.status_code == 200 + + # Paste the unit + paste_response = client.post(XBLOCK_ENDPOINT, { + "parent_locator": str(dest_sequential.location), + "staged_content": "clipboard", + }, format="json") + assert paste_response.status_code == 200 + dest_unit_key = UsageKey.from_string(paste_response.json()["locator"]) + + # Now there should be a one unit/vertical as a child of the destination sequential/subsection: + updated_sequential = self.store.get_item(dest_sequential.location) + assert updated_sequential.children == [dest_unit_key] + # And it should match the original: + orig_unit = self.store.get_item(unit_key) + dest_unit = self.store.get_item(dest_unit_key) + assert len(orig_unit.children) == len(dest_unit.children) + # Check details of the fourth child (a poll) + orig_poll = self.store.get_item(orig_unit.children[3]) + dest_poll = self.store.get_item(dest_unit.children[3]) + assert dest_poll.display_name == orig_poll.display_name + assert dest_poll.question == orig_poll.question + # The new block should store a reference to where it was copied from + assert dest_unit.copied_from_block == str(unit_key) + def test_paste_with_assets(self): """ When pasting into a different course, any required static assets should diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index b2b96e2427..e26242f81f 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -882,7 +882,6 @@ def _duplicate_block( @login_required -@expect_json def delete_item(request, usage_key): """ Exposes internal helper method without breaking existing bindings/dependencies diff --git a/cms/static/cms/js/spec/main.js b/cms/static/cms/js/spec/main.js index 46a20ac517..4e093ee573 100644 --- a/cms/static/cms/js/spec/main.js +++ b/cms/static/cms/js/spec/main.js @@ -243,7 +243,6 @@ 'js/spec/video/transcripts/message_manager_spec', 'js/spec/video/transcripts/utils_spec', 'js/spec/video/transcripts/editor_spec', - 'js/spec/video/transcripts/videolist_spec', 'js/spec/video/transcripts/file_uploader_spec', 'js/spec/models/component_template_spec', 'js/spec/models/explicit_url_spec', diff --git a/cms/static/js/spec/video/transcripts/videolist_spec.js b/cms/static/js/spec/video/transcripts/videolist_spec.js deleted file mode 100644 index 908d519d89..0000000000 --- a/cms/static/js/spec/video/transcripts/videolist_spec.js +++ /dev/null @@ -1,788 +0,0 @@ -define( - [ - 'jquery', 'underscore', 'backbone', - 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', - 'js/views/video/transcripts/utils', - 'js/views/video/transcripts/editor', - 'js/views/video/transcripts/metadata_videolist', 'js/models/metadata', - 'js/views/abstract_editor', - 'js/views/video/transcripts/message_manager', - 'xmodule' - ], - function($, _, Backbone, AjaxHelpers, Utils, Editor, VideoList, MetadataModel, AbstractEditor, MessageManager) { - 'use strict'; - - describe('CMS.Views.Metadata.VideoList', function() { - var videoListEntryTemplate = readFixtures( - 'video/transcripts/metadata-videolist-entry.underscore' - ), - abstractEditor = AbstractEditor.prototype, - component_locator = 'component_locator', - videoList = [ - { - mode: 'youtube', - type: 'youtube', - video: '12345678901' - }, - { - mode: 'html5', - type: 'mp4', - video: 'video' - }, - { - mode: 'html5', - type: 'webm', - video: 'video' - } - ], - modelStub = { - default_value: ['a thing', 'another thing'], - display_name: 'Video URL', - explicitly_set: true, - field_name: 'video_url', - help: 'A list of things.', - options: [], - type: MetadataModel.VIDEO_LIST_TYPE, - value: [ - 'http://youtu.be/12345678901', - 'video.mp4', - 'video.webm' - ] - }, - videoIDStub = { - default_value: 'test default value', - display_name: 'Video ID', - explicitly_set: true, - field_name: 'edx_video_id', - help: 'Specifies the video ID.', - options: [], - type: 'VideoID', - value: 'advanced tab video id' - }, - response = JSON.stringify({ - command: 'found', - status: 'Success', - subs: 'video_id' - }), - waitForEvent, - createVideoListView; - - var createMockAjaxServer = function() { - var mockServer = AjaxHelpers.server( - [ - 200, - {'Content-Type': 'application/json'}, - response - ] - ); - mockServer.autoRespond = true; - return mockServer; - }; - - beforeEach(function() { - var tpl = sandbox({ // eslint-disable-line no-undef - class: 'component', - 'data-locator': component_locator - }); - - setFixtures(tpl); - - appendSetFixtures( - $('