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..bc1b6b0be931 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,11 +25,9 @@ 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 -from cms.djangoapps.contentstore.views.component import ADVANCED_COMPONENT_TYPES from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError from common.djangoapps.course_action_state.models import CourseRerunState, CourseRerunUIStateManager from common.djangoapps.student import auth @@ -568,44 +561,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 +655,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 +1467,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..1ac5889908bb 100644 --- a/cms/djangoapps/contentstore/toggles.py +++ b/cms/djangoapps/contentstore/toggles.py @@ -194,24 +194,6 @@ 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 - """ - return not LEGACY_STUDIO_UNIT_EDITOR.is_enabled(course_key) - # .. toggle_name: contentstore.mock_video_uploads # .. toggle_implementation: WaffleFlag diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 143f01cde32a..ad03361a049f 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/component.py b/cms/djangoapps/contentstore/views/component.py index 05a9fc12d291..cbab7b844f6d 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_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_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index fbb14634820c..64abff538897 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -4,18 +4,12 @@ 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 @@ -62,82 +56,6 @@ def setUp(self): self.store.publish(self.released_public_vertical.location, self.user.id) self.store.publish(self.vertical.location, self.user.id) - def test_container_html(self): - assets_url = reverse( - 'assets_handler', kwargs={'course_key_string': str(self.child_container.location.course_key)} - ) - self._test_html_content( - self.child_container, - expected_section_tag=( - '