Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix: Drop _get_legacy_courseware_url and related usage.

Browse Source 
We were running some tests using this function but it is not actually
used in the running application anymore so drop those tests and remove
the function in preparation for removing the legacy courseware itself.
This commit is contained in:
Feanil Patel
2025-05-22 10:58:52 -04:00
parent 31a5ca4d8a
commit 3334325ff9
3 changed files with 0 additions and 144 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -108,7 +108,6 @@ from openedx.features.course_experience import (
)
from openedx.features.course_experience.tests.views.helpers import add_course_mode
from openedx.features.course_experience.url_helpers import (
_get_legacy_courseware_url,
get_learning_mfe_home_url,
make_learning_mfe_courseware_url
)
@@ -232,31 +231,6 @@ class TestJumpTo(ModuleStoreTestCase):
assert response.status_code == 302
assert response.url == expected_redirect_url
# The new courseware experience does not support this sort of course structure;
# it assumes a simple course->chapter->sequence->unit->component tree.
@patch.object(views, 'get_courseware_url', new=_get_legacy_courseware_url)
def test_jump_to_legacy_from_nested_block(self):
with self.store.default_store(ModuleStoreEnum.Type.split):
course = CourseFactory.create()
chapter = BlockFactory.create(category='chapter', parent_location=course.location)
sequence = BlockFactory.create(category='sequential', parent_location=chapter.location)
vertical = BlockFactory.create(category='vertical', parent_location=sequence.location)
nested_sequence = BlockFactory.create(category='sequential', parent_location=vertical.location)
nested_vertical1 = BlockFactory.create(category='vertical', parent_location=nested_sequence.location)
# put a block into nested_vertical1 for completeness
BlockFactory.create(category='html', parent_location=nested_vertical1.location)
nested_vertical2 = BlockFactory.create(category='vertical', parent_location=nested_sequence.location)
block2 = BlockFactory.create(category='html', parent_location=nested_vertical2.location)
# internal position of block2 will be 1_2 (2nd item withing 1st item)
activate_block_id = urlencode({'activate_block_id': str(block2.location)})
expected_redirect_url = (
f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/1?{activate_block_id}'
)
jumpto_url = f'/courses/{course.id}/jump_to/{block2.location}'
response = self.client.get(jumpto_url)
self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302)
@ddt.data(
(False, ModuleStoreEnum.Type.split),
(True, ModuleStoreEnum.Type.split),
@@ -273,44 +247,6 @@ class TestJumpTo(ModuleStoreTestCase):
response = self.client.get(jumpto_url)
assert response.status_code == 404
@ddt.data(
(ModuleStoreEnum.Type.split, False, '1'),
(ModuleStoreEnum.Type.split, True, '2'),
)
@ddt.unpack
def test_jump_to_legacy_for_learner_with_staff_only_content(self, store_type, is_staff_user, position):
"""
Test for checking correct position in redirect_url for learner when a course has staff-only units.
(When the MFE is active, it handles this logic itself with the help of the
courseware blocks/metadata/outline APIs, so we don't test for it here.)
"""
with self.store.default_store(store_type):
course = CourseFactory.create()
request = RequestFactory().get('/')
request.user = UserFactory(is_staff=is_staff_user, username="staff")
request.session = {}
course_key = CourseKey.from_string(str(course.id))
chapter = BlockFactory.create(category='chapter', parent_location=course.location)
sequence = BlockFactory.create(category='sequential', parent_location=chapter.location)
__ = BlockFactory.create(category='vertical', parent_location=sequence.location)
staff_only_vertical = BlockFactory.create(category='vertical', parent_location=sequence.location,
metadata=dict(visible_to_staff_only=True))
__ = BlockFactory.create(category='vertical', parent_location=sequence.location)
usage_key = UsageKey.from_string(str(staff_only_vertical.location)).replace(course_key=course_key)
expected_url = reverse(
'courseware_position',
kwargs={
'course_id': str(course.id),
'chapter': chapter.url_name,
'section': sequence.url_name,
'position': position,
}
)
expected_url += "?{}".format(urlencode({'activate_block_id': str(staff_only_vertical.location)}))
assert expected_url == _get_legacy_courseware_url(usage_key, request)
class IndexQueryTestCase(ModuleStoreTestCase):
"""
@@ -474,18 +410,6 @@ class CoursewareIndexTestCase(BaseViewsTestCase):
assert response.status_code == expected_response_code
return response
def test_get_redirect_url(self):
# test the course location
assert '/courses/{course_key}/courseware?{activate_block_id}'.format(
course_key=str(self.course_key),
activate_block_id=urlencode({'activate_block_id': str(self.course.location)})
) == _get_legacy_courseware_url(self.course.location)
# test a section location
assert '/courses/{course_key}/courseware/Chapter_1/Sequential_1/?{activate_block_id}'.format(
course_key=str(self.course_key),
activate_block_id=urlencode({'activate_block_id': str(self.section.location)})
) == _get_legacy_courseware_url(self.section.location)
def test_index_invalid_position(self):
request_url = '/'.join([
'/courses',

27
lms/djangoapps/courseware/testutils.py
View File

@@ -12,7 +12,6 @@ import ddt
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from lms.djangoapps.courseware.utils import is_mode_upsellable
from openedx.features.course_experience.url_helpers import _get_legacy_courseware_url
from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
@@ -165,32 +164,6 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
self.assertNotContains(response, self.html_block.data, status_code=expected_response_code)
return response
@ddt.data(
('vertical_block', 4),
('html_block', 4),
)
@ddt.unpack
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_courseware_html(self, block_name, mongo_calls, mock_redirect):
"""
To verify that the removal of courseware chrome elements is working,
we include this test here to make sure the chrome elements that should
be removed actually exist in the full courseware page.
If this test fails, it's probably because the HTML template for courseware
has changed and COURSEWARE_CHROME_HTML_ELEMENTS needs to be updated.
"""
with self.store.default_store(ModuleStoreEnum.Type.split):
self.block_name_to_be_tested = block_name
self.setup_course(ModuleStoreEnum.Type.split)
self.setup_user(admin=True, enroll=True, login=True)
with check_mongo_calls(mongo_calls):
url = _get_legacy_courseware_url(self.block_to_be_tested.location)
response = self.client.get(url)
expected_elements = self.block_specific_chrome_html_elements + self.COURSEWARE_CHROME_HTML_ELEMENTS
for chrome_element in expected_elements:
self.assertContains(response, chrome_element)
def test_success_enrolled_staff(self):
self.setup_course()
self.setup_user(admin=True, enroll=True, login=True)

41
openedx/features/course_experience/url_helpers.py
View File

@@ -44,47 +44,6 @@ def get_courseware_url(
return _get_new_courseware_url(usage_key=usage_key, request=request, is_staff=is_staff)
def _get_legacy_courseware_url(
usage_key: UsageKey,
request: Optional[HttpRequest] = None,
is_staff: bool = None
) -> str:
"""
Return the URL to Legacy (LMS-rendered) courseware content.
Raises:
* ItemNotFoundError if no data at the usage_key.
* NoPathToItem if location not in any class.
"""
(
course_key, chapter, section, vertical_unused,
position, final_target_id
) = path_to_location(modulestore(), usage_key, request)
# choose the appropriate view (and provide the necessary args) based on the
# args provided by the redirect.
# Rely on index to do all error handling and access control.
if chapter is None:
redirect_url = reverse('courseware', args=(str(course_key), ))
elif section is None:
redirect_url = reverse('courseware_chapter', args=(str(course_key), chapter))
elif position is None:
redirect_url = reverse(
'courseware_section',
args=(str(course_key), chapter, section)
)
else:
# Here we use the navigation_index from the position returned from
# path_to_location - we can only navigate to the topmost vertical at the
# moment
redirect_url = reverse(
'courseware_position',
args=(str(course_key), chapter, section, navigation_index(position))
)
redirect_url += "?{}".format(urlencode({'activate_block_id': str(final_target_id)}))
return redirect_url
def _get_new_courseware_url(
usage_key: UsageKey,
request: Optional[HttpRequest] = None,