Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,14 @@ def get_use_new_course_outline_page(self, obj):

def get_use_new_unit_page(self, obj):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use_new_unit_page is kept in the serializer response for now as a field returning True. Once the MFE confirms it no longer reads this key from the waffle flags API, the field and its method can be removed in a follow-up PR.

"""
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):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
62 changes: 4 additions & 58 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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("<script>alert(1)</script>", "alert('hi')", "</script><script>alert(1)</script>")
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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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. """
Expand Down
18 changes: 0 additions & 18 deletions cms/djangoapps/contentstore/toggles.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 4 additions & 8 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
59 changes: 26 additions & 33 deletions cms/djangoapps/contentstore/views/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/<usage_key>.
"""
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
Expand All @@ -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
Expand Down
Loading
Loading