diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py index b00f769b5754..d2b2791af710 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py @@ -158,10 +158,14 @@ def get_use_new_course_outline_page(self, obj): def get_use_new_unit_page(self, obj): """ - Method to get the use_new_unit_page switch + Method to get the use_new_unit_page switch. + + Always returns True. The legacy unit editor has been removed. + This method will be removed in a follow-up cleanup. + + See https://github.com/openedx/edx-platform/issues/36275 """ - course_key = self.get_course_key() - return toggles.use_new_unit_page(course_key) + return True def get_use_new_course_team_page(self, obj): """ diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/views/course_waffle_flags.py index 69b2898912aa..ffab81ef8a66 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/course_waffle_flags.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/course_waffle_flags.py @@ -57,7 +57,7 @@ def get(self, request, course_id=None): "use_new_files_uploads_page": true, "use_new_video_uploads_page": false, "use_new_course_outline_page": true, - "use_new_unit_page": false, + "use_new_unit_page": true, "use_new_course_team_page": true, "use_new_certificates_page": true, "use_new_textbooks_page": true, diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index d09a40ba71ef..8894596511fe 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1,10 +1,5 @@ # pylint: disable=missing-module-docstring -# TODO: Rewrite several of these assertions so that they check the output of the REST or Python -# APIs rather than parsing HTML from the deprecated legacy frontend pages. In particular, any -# test case using override_waffle_flag(toggles.LEGACY_STUDIO_*, True) will need to be fixed. -# Part of https://github.com/openedx/edx-platform/issues/36275. - import copy import re import shutil @@ -21,7 +16,7 @@ from django.test import TestCase from django.test.utils import override_settings from django.urls import reverse -from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch +from edx_toggles.toggles.testutils import override_waffle_switch from edxval.api import create_video, get_videos_for_course from fs.osfs import OSFS from lxml import etree @@ -30,7 +25,6 @@ from opaque_keys.edx.locations import CourseLocator from path import Path as path -from cms.djangoapps.contentstore import toggles from cms.djangoapps.contentstore.config import waffle from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient, CourseTestCase, get_url, parse_json from cms.djangoapps.contentstore.utils import delete_course, reverse_course_url, reverse_url @@ -568,44 +562,6 @@ def setUp(self): ) self.course = self.store.publish(self.course.location, self.user.id) - def check_components_on_page(self, component_types, expected_types): - """ - Ensure that the right types end up on the page. - - component_types is the list of advanced components. - - expected_types is the list of elements that should appear on the page. - - expected_types and component_types should be similar, but not - exactly the same -- for example, 'video' in - component_types should cause 'Video' to be present. - """ - self.course.advanced_modules = component_types - self.store.update_item(self.course, self.user.id) - - # just pick one vertical - resp = self.client.get_html(get_url('container_handler', self.vert_loc)) - for expected in expected_types: - self.assertContains(resp, expected) - - @override_waffle_flag(toggles.LEGACY_STUDIO_UNIT_EDITOR, True) - @ddt.data("", "alert('hi')", "") - def test_container_handler_xss_prevent(self, malicious_code): - """ - Test that XSS attack is prevented - """ - resp = self.client.get_html(get_url('container_handler', self.vert_loc) + '?action=' + malicious_code) - # Test that malicious code does not appear in html - self.assertNotContains(resp, malicious_code) - - @override_waffle_flag(toggles.LEGACY_STUDIO_UNIT_EDITOR, True) - def test_advanced_components_in_edit_unit(self): - # This could be made better, but for now let's just assert that we see the advanced modules mentioned in the - # page response HTML - self.check_components_on_page( - ADVANCED_COMPONENT_TYPES, - ['Word cloud', 'Annotation', 'split_test'], - ) @ddt.data('/Fake/asset/displayname', '\\Fake\\asset\\displayname') def test_export_on_invalid_displayname(self, invalid_displayname): @@ -700,14 +656,6 @@ def test_assets_overwrite(self): # Remove tempdir shutil.rmtree(root_dir) - @override_waffle_flag(toggles.LEGACY_STUDIO_UNIT_EDITOR, True) - def test_advanced_components_require_two_clicks(self): - self.check_components_on_page(['word_cloud'], ['Word cloud']) - - @override_waffle_flag(toggles.LEGACY_STUDIO_UNIT_EDITOR, True) - def test_edit_unit(self): - """Verifies rendering the editor in all the verticals in the given test course""" - self._check_verticals([self.vert_loc]) def _get_draft_counts(self, item): # pylint: disable=missing-function-docstring cnt = 1 if not self.store.has_published_version(item) else 0 @@ -1520,11 +1468,10 @@ def test_get_json(handler): ) self.assertEqual(resp.status_code, 200) # noqa: PT009 - # go look at the Edit page + # go look at the Edit page — now redirects to MFE unit editor unit_key = course_key.make_usage_key('vertical', 'test_vertical') - with override_waffle_flag(toggles.LEGACY_STUDIO_UNIT_EDITOR, True): - resp = self.client.get_html(get_url('container_handler', unit_key)) - self.assertEqual(resp.status_code, 200) # noqa: PT009 + resp = self.client.get_html(get_url('container_handler', unit_key)) + self.assertEqual(resp.status_code, 302) # noqa: PT009 def delete_item(category, name): """ Helper method for testing the deletion of an xblock item. """ diff --git a/cms/djangoapps/contentstore/toggles.py b/cms/djangoapps/contentstore/toggles.py index 94960ba66ec0..f7f67adc9057 100644 --- a/cms/djangoapps/contentstore/toggles.py +++ b/cms/djangoapps/contentstore/toggles.py @@ -194,23 +194,14 @@ def use_new_video_uploads_page(course_key): LEGACY_STUDIO_COURSE_OUTLINE = CourseWaffleFlag('legacy_studio.course_outline', __name__) -# .. toggle_name: legacy_studio.unit_editor -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: Temporarily fall back to the old Studio unit editing page. -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2025-03-14 -# .. toggle_target_removal_date: 2025-09-14 -# .. toggle_tickets: https://github.com/openedx/edx-platform/issues/36275 -# .. toggle_warning: In Ulmo, this toggle will be removed. Only the new (React-based) experience will be available. -LEGACY_STUDIO_UNIT_EDITOR = CourseWaffleFlag('legacy_studio.unit_editor', __name__) - - def use_new_unit_page(course_key): """ - Returns a boolean if new studio course outline mfe is enabled + Returns a boolean if new studio unit page is enabled. + + Always returns True. The legacy unit editor has been removed. + This function will be removed in a follow-up cleanup once all callers are updated. """ - return not LEGACY_STUDIO_UNIT_EDITOR.is_enabled(course_key) + return True # .. toggle_name: contentstore.mock_video_uploads diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 2f43a00610e8..cdd0bdf8c671 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -42,7 +42,6 @@ libraries_v1_enabled, libraries_v2_enabled, split_library_view_on_dashboard, - use_new_unit_page, ) from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.djangoapps.models.settings.course_metadata import CourseMetadata @@ -425,13 +424,10 @@ def get_unit_url(course_locator, unit_locator) -> str: """ Gets course authoring microfrontend URL for unit page view. """ - unit_url = None - if use_new_unit_page(course_locator): - mfe_base_url = get_course_authoring_url(course_locator) - course_mfe_url = f'{mfe_base_url}/course/{course_locator}/container/{unit_locator}' - if mfe_base_url: - unit_url = course_mfe_url - return unit_url + mfe_base_url = get_course_authoring_url(course_locator) + if mfe_base_url: + return f'{mfe_base_url}/course/{course_locator}/container/{unit_locator}' + return None def get_certificates_url(course_locator) -> str: diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index b2c0e39df77c..09f8c34a89aa 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -41,7 +41,6 @@ ) from ..helpers import is_unit -from ..utils import get_container_handler_context from .component import _get_item_in_course from .preview import get_preview_fragment @@ -289,38 +288,6 @@ def xblock_view_handler(request, usage_key_string, view_name): return HttpResponse(status=406) -@xframe_options_exempt -@require_http_methods(["GET"]) -@login_required -def xblock_edit_view(request, usage_key_string): - """ - Return rendered xblock edit view. - - Allows editing of an XBlock specified by the usage key. - """ - usage_key = usage_key_with_run(usage_key_string) - if not has_studio_read_access(request.user, usage_key.course_key): - raise PermissionDenied() - - store = modulestore() - - with store.bulk_operations(usage_key.course_key): - course, xblock, _, __ = _get_item_in_course(request, usage_key) - container_handler_context = get_container_handler_context(request, usage_key, course, xblock) - - fragment = get_preview_fragment(request, xblock, {}) - - hashed_resources = { - hash_resource(resource): resource._asdict() for resource in fragment.resources - } - - container_handler_context.update({ - "action_name": "edit", - "resources": list(hashed_resources.items()), - }) - - return render_to_response('container_editor.html', container_handler_context) - @require_http_methods("GET") @login_required diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 05a9fc12d291..0b8176fbb5bd 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -22,7 +22,7 @@ from xblock.runtime import Mixologist from cms.djangoapps.contentstore.helpers import get_parent_if_split_test, is_library_content, is_unit -from cms.djangoapps.contentstore.toggles import libraries_v2_enabled, use_new_unit_page +from cms.djangoapps.contentstore.toggles import libraries_v1_enabled, libraries_v2_enabled from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import load_services_for_studio from common.djangoapps.edxmako.shortcuts import render_to_response from common.djangoapps.student.auth import has_course_author_access @@ -130,44 +130,36 @@ def _load_mixed_class(category): @require_GET @login_required -def container_handler(request, usage_key_string): # pylint: disable=too-many-statements +def container_handler(request, usage_key_string): """ - The restful handler for container xblock requests. + Redirects to the MFE unit editor. - GET - html: returns the HTML page for editing a container - json: not currently supported + The legacy Django-template-based unit editor has been removed. + This view exists for backward compatibility with any existing links to /container/. """ + from ..utils import get_unit_url - from ..utils import get_container_handler_context, get_unit_url + if 'text/html' not in request.META.get('HTTP_ACCEPT', 'text/html'): + return HttpResponseBadRequest("Only supports HTML requests") - if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): + try: + usage_key = UsageKey.from_string(usage_key_string) + except InvalidKeyError: + raise Http404 # pylint: disable=raise-missing-from # noqa: B904 + with modulestore().bulk_operations(usage_key.course_key): try: - usage_key = UsageKey.from_string(usage_key_string) - except InvalidKeyError: # Raise Http404 on invalid 'usage_key_string' - raise Http404 # pylint: disable=raise-missing-from # noqa: B904 - with modulestore().bulk_operations(usage_key.course_key): - try: - course, xblock, lms_link, preview_lms_link = _get_item_in_course(request, usage_key) - except ItemNotFoundError: - return HttpResponseBadRequest() - - if use_new_unit_page(course.id): - if is_unit(xblock) or is_library_content(xblock): - return redirect(get_unit_url(course.id, xblock.location)) - - if split_xblock := get_parent_if_split_test(xblock): - return redirect(get_unit_url(course.id, split_xblock.location)) - - container_handler_context = get_container_handler_context(request, usage_key, course, xblock) - container_handler_context.update({ - 'draft_preview_link': preview_lms_link, - 'published_preview_link': lms_link, - }) - return render_to_response('container.html', container_handler_context) - else: - return HttpResponseBadRequest("Only supports HTML requests") + course, xblock, lms_link, preview_lms_link = _get_item_in_course(request, usage_key) + except ItemNotFoundError: + return HttpResponseBadRequest() + + if is_unit(xblock) or is_library_content(xblock): + return redirect(get_unit_url(course.id, xblock.location)) + + if split_xblock := get_parent_if_split_test(xblock): + return redirect(get_unit_url(course.id, split_xblock.location)) + + raise Http404 @require_GET @@ -176,7 +168,8 @@ def container_handler(request, usage_key_string): # pylint: disable=too-many-st def container_embed_handler(request, usage_key_string): # pylint: disable=too-many-statements """ Returns an HttpResponse with HTML content for the container XBlock. - The returned HTML is a chromeless rendering of the XBlock. + The returned HTML is a chromeless rendering of the XBlock, used by the + Authoring MFE to display xblock content in iframes. GET html: returns the HTML page for editing a container diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 9e53a3aa0195..44b2cbdbca8d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -4700,76 +4700,3 @@ def test_update_clobbers(self): self.check_updated(source_block, destination_block.location) -class TestXblockEditView(CourseTestCase): - """ - Test xblock_edit_view. - """ - - def setUp(self): - super().setUp() - self.chapter = self._create_block(self.course, "chapter", "Week 1") - self.sequential = self._create_block(self.chapter, "sequential", "Lesson 1") - self.vertical = self._create_block(self.sequential, "vertical", "Unit") - self.html = self._create_block(self.vertical, "html", "HTML") - self.child_container = self._create_block( - self.vertical, "split_test", "Split Test" - ) - self.child_vertical = self._create_block( - self.child_container, "vertical", "Child Vertical" - ) - self.video = self._create_block(self.child_vertical, "video", "My Video") - self.store = modulestore() - - self.store.publish(self.vertical.location, self.user.id) - - def _create_block(self, parent, category, display_name, **kwargs): - """ - creates a block in the module store, without publishing it. - """ - return BlockFactory.create( - parent=parent, - category=category, - display_name=display_name, - publish_item=False, - user_id=self.user.id, - **kwargs, - ) - - def test_xblock_edit_view(self): - url = reverse_usage_url("xblock_edit_handler", self.video.location) - resp = self.client.get_html(url) - self.assertEqual(resp.status_code, 200) # noqa: PT009 - - html_content = resp.content.decode(resp.charset) - self.assertIn("var decodedActionName = 'edit';", html_content) # noqa: PT009 - - def test_xblock_edit_view_contains_resources(self): - url = reverse_usage_url("xblock_edit_handler", self.video.location) - resp = self.client.get(url) - self.assertEqual(resp.status_code, 200) # noqa: PT009 - - html_content = resp.content.decode(resp.charset) - soup = BeautifulSoup(html_content, "html.parser") - - resource_links = [link["href"] for link in soup.find_all("link", {"rel": "stylesheet"})] - script_sources = [script["src"] for script in soup.find_all("script") if script.get("src")] - - self.assertGreater(len(resource_links), 0, f"No CSS resources found in HTML. Found: {resource_links}") # noqa: PT009 # pylint: disable=line-too-long - self.assertGreater(len(script_sources), 0, f"No JS resources found in HTML. Found: {script_sources}") # noqa: PT009 # pylint: disable=line-too-long - - def test_xblock_edit_view_contains_page_notification(self): - """ - The page-notification element is required for XBlock runtime error - notifications (e.g. ORA validation errors) to be visible to the user. - """ - url = reverse_usage_url("xblock_edit_handler", self.video.location) - resp = self.client.get(url) - self.assertEqual(resp.status_code, 200) # noqa: PT009 - - html_content = resp.content.decode(resp.charset) - soup = BeautifulSoup(html_content, "html.parser") - self.assertIsNotNone( # noqa: PT009 - soup.find(id="page-notification"), - "container_editor.html must include a #page-notification element " - "so that XBlock runtime error notifications are rendered.", - ) diff --git a/cms/djangoapps/contentstore/views/tests/test_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index fbb14634820c..f0df927d49f8 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -5,17 +5,14 @@ import datetime import re -from unittest.mock import Mock, patch from urllib.parse import quote from django.http import Http404 from django.test.client import RequestFactory from django.urls import reverse -from edx_toggles.toggles.testutils import override_waffle_flag from pytz import UTC import cms.djangoapps.contentstore.views.component as views -from cms.djangoapps.contentstore import toggles from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase from xmodule.modulestore import ModuleStoreEnum # pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # pylint: disable=wrong-import-order @@ -88,46 +85,6 @@ def test_container_html(self): ), ) - @override_waffle_flag(toggles.LEGACY_STUDIO_UNIT_EDITOR, True) - def test_container_on_container_html(self): - """ - Create the scenario of an xblock with children (non-vertical) on the container page. - This should create a container page that is a child of another container page. - """ - draft_container = self._create_block(self.child_container, "wrapper", "Wrapper") - self._create_block(draft_container, "html", "Child HTML") - - def test_container_html(xblock): - assets_url = reverse( - 'assets_handler', kwargs={'course_key_string': str(draft_container.location.course_key)} - ) - self._test_html_content( - xblock, - expected_section_tag=( - '