diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index bb6d42025e..5b910565e4 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -681,47 +681,6 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes assert bool(access._has_access_course(self.student, 'load_mobile', course)) == student_expected assert bool(access._has_access_course(self.staff, 'load_mobile', course)) == staff_expected - @patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) - def test_courseware_page_unfulfilled_prereqs(self): - """ - Test courseware access when a course has pre-requisite course yet to be completed - """ - pre_requisite_course = CourseFactory.create( - org='edX', - course='900', - run='test_run', - ) - - pre_requisite_courses = [str(pre_requisite_course.id)] - course = CourseFactory.create( - org='edX', - course='1000', - run='test_run', - pre_requisite_courses=pre_requisite_courses, - ) - set_prerequisite_courses(course.id, pre_requisite_courses) - - test_password = 't3stp4ss.!' - user = UserFactory.create() - user.set_password(test_password) - user.save() - self.login(user.email, test_password) - CourseEnrollmentFactory(user=user, course_id=course.id) - - url = reverse('courseware', args=[str(course.id)]) - response = self.client.get(url) - self.assertRedirects( - response, - reverse( - 'dashboard' - ) - ) - assert response.status_code == 302 - - fulfill_course_milestone(pre_requisite_course.id, user) - response = self.client.get(url) - assert response.status_code == 200 - class UserRoleTestCase(TestCase): """ diff --git a/lms/djangoapps/courseware/tests/test_course_survey.py b/lms/djangoapps/courseware/tests/test_course_survey.py index 98b5842cd2..25dc37724d 100644 --- a/lms/djangoapps/courseware/tests/test_course_survey.py +++ b/lms/djangoapps/courseware/tests/test_course_survey.py @@ -16,6 +16,7 @@ from common.test.utils import XssTestMixin from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase from lms.djangoapps.survey.models import SurveyAnswer, SurveyForm from openedx.features.course_experience import course_home_url +from openedx.features.course_experience.url_helpers import make_learning_mfe_courseware_url class SurveyViewsTests(LoginEnrollmentTestCase, SharedModuleStoreTestCase, XssTestMixin): @@ -89,26 +90,44 @@ class SurveyViewsTests(LoginEnrollmentTestCase, SharedModuleStoreTestCase, XssTe reverse('course_survey', kwargs={'course_id': str(course.id)}) ) - def _assert_no_redirect(self, course): + def _assert_no_survey_redirect(self, course): """ - Helper method to asswer that all known conditionally redirect points do - not redirect as expected + Helper method to assert that all known conditionally redirecting endpoints do + not redirect to the survey as expected """ - for view_name in ['courseware', 'progress']: - resp = self.client.get( - reverse( - view_name, - kwargs={'course_id': str(course.id)} - ) + + # Make sure we get to the progress page. + resp = self.client.get( + reverse( + 'progress', + kwargs={'course_id': str(course.id)} ) - assert resp.status_code == 200 + ) + assert resp.status_code == 200 + + # Make sure we are redirected to the MFE for courseware + resp = self.client.get( + reverse( + 'courseware', + kwargs={'course_id': str(course.id)} + ) + ) + assert resp.status_code == 302 + expected_redirect_url = make_learning_mfe_courseware_url( + course.id, + None, + None, + params=None, + preview=False + ) + assert resp.url == expected_redirect_url def test_visiting_course_without_survey(self): """ Verifies that going to the courseware which does not have a survey does not redirect to a survey """ - self._assert_no_redirect(self.course_without_survey) + self._assert_no_survey_redirect(self.course_without_survey) def test_visiting_course_with_survey_redirects(self): """ @@ -143,7 +162,7 @@ class SurveyViewsTests(LoginEnrollmentTestCase, SharedModuleStoreTestCase, XssTe ) assert resp.status_code == 200 - self._assert_no_redirect(self.course) + self._assert_no_survey_redirect(self.course) def test_course_id_field(self): """ @@ -180,7 +199,7 @@ class SurveyViewsTests(LoginEnrollmentTestCase, SharedModuleStoreTestCase, XssTe ) assert resp.status_code == 200 - self._assert_no_redirect(self.course) + self._assert_no_survey_redirect(self.course) # however we want to make sure we persist the course_id answer_objs = SurveyAnswer.objects.filter( @@ -195,7 +214,7 @@ class SurveyViewsTests(LoginEnrollmentTestCase, SharedModuleStoreTestCase, XssTe """ Verifies that going to the courseware with a required, but non-existing survey, does not redirect """ - self._assert_no_redirect(self.course_with_bogus_survey) + self._assert_no_survey_redirect(self.course_with_bogus_survey) def test_visiting_survey_with_bogus_survey_name(self): """ diff --git a/lms/djangoapps/courseware/tests/test_masquerade.py b/lms/djangoapps/courseware/tests/test_masquerade.py index 33d100c7a2..0f0eca4dc2 100644 --- a/lms/djangoapps/courseware/tests/test_masquerade.py +++ b/lms/djangoapps/courseware/tests/test_masquerade.py @@ -96,11 +96,11 @@ class MasqueradeTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase, Mas Returns the server response for the courseware page. """ url = reverse( - 'courseware_section', + 'courseware_subsection', kwargs={ 'course_id': str(self.course.id), - 'chapter': self.chapter.location.block_id, - 'section': self.sequential.location.block_id, + 'section': self.chapter.location.block_id, + 'subsection': self.sequential.location.block_id, } ) return self.client.get(url) @@ -244,51 +244,6 @@ class TestMasqueradeOptionsNoContentGroups(StaffMasqueradeTestCase): assert is_target_available == expected -# These tests are testing a capability of the old courseware page. We have to not -# force redirect to the new MFE in order to be able to load the old pages which are -# being tested by this page. -# -# This is a temporary change, until we can remove the old courseware pages -# all together. -@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) -class TestStaffMasqueradeAsStudent(StaffMasqueradeTestCase): - """ - Check for staff being able to masquerade as student. - """ - - @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) - def test_staff_debug_with_masquerade(self, mock_redirect): - """ - Tests that staff debug control is not visible when masquerading as a student. - """ - # Verify staff initially can see staff debug - self.verify_staff_debug_present(True) - - # Toggle masquerade to student - self.update_masquerade(role='student') - self.verify_staff_debug_present(False) - - # Toggle masquerade back to staff - self.update_masquerade(role='staff') - self.verify_staff_debug_present(True) - - @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) - def test_show_answer_for_staff(self, mock_redirect): - """ - Tests that "Show Answer" is not visible when masquerading as a student. - """ - # Verify that staff initially can see "Show Answer". - self.verify_show_answer_present(True) - - # Toggle masquerade to student - self.update_masquerade(role='student') - self.verify_show_answer_present(False) - - # Toggle masquerade back to staff - self.update_masquerade(role='staff') - self.verify_show_answer_present(True) - - @ddt.ddt class TestStaffMasqueradeAsSpecificStudent(StaffMasqueradeTestCase, ProblemSubmissionTestMixin): """ diff --git a/lms/djangoapps/courseware/tests/test_navigation.py b/lms/djangoapps/courseware/tests/test_navigation.py deleted file mode 100644 index 8c2144ee2a..0000000000 --- a/lms/djangoapps/courseware/tests/test_navigation.py +++ /dev/null @@ -1,285 +0,0 @@ -""" -This test file will run through some LMS test scenarios regarding access and navigation of the LMS -""" - - -import time - -from unittest.mock import patch -from django.conf import settings -from django.test.utils import override_settings -from django.urls import reverse -from edx_toggles.toggles.testutils import override_waffle_flag -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory - -from common.djangoapps.student.tests.factories import GlobalStaffFactory -from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase -from openedx.features.course_experience import DISABLE_COURSE_OUTLINE_PAGE_FLAG -from openedx.features.course_experience.url_helpers import make_learning_mfe_courseware_url - - -class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): - """ - Check that navigation state is saved properly. - """ - @classmethod - def setUpClass(cls): - # pylint: disable=super-method-not-called - with super().setUpClassAndTestData(): - cls.test_course = CourseFactory.create() - cls.test_course_proctored = CourseFactory.create() - cls.course = CourseFactory.create() - - @classmethod - def setUpTestData(cls): # lint-amnesty, pylint: disable=super-method-not-called - cls.chapter0 = BlockFactory.create(parent=cls.course, - display_name='Overview') - cls.chapter9 = BlockFactory.create(parent=cls.course, - display_name='factory_chapter') - cls.section0 = BlockFactory.create(parent=cls.chapter0, - display_name='Welcome') - cls.section9 = BlockFactory.create(parent=cls.chapter9, - display_name='factory_section') - cls.unit0 = BlockFactory.create(parent=cls.section0, - display_name='New Unit 0') - - cls.chapterchrome = BlockFactory.create(parent=cls.course, - display_name='Chrome') - cls.chromelesssection = BlockFactory.create(parent=cls.chapterchrome, - display_name='chromeless', - chrome='none') - cls.accordionsection = BlockFactory.create(parent=cls.chapterchrome, - display_name='accordion', - chrome='accordion') - cls.tabssection = BlockFactory.create(parent=cls.chapterchrome, - display_name='tabs', - chrome='tabs') - cls.defaultchromesection = BlockFactory.create( - parent=cls.chapterchrome, - display_name='defaultchrome', - ) - cls.fullchromesection = BlockFactory.create(parent=cls.chapterchrome, - display_name='fullchrome', - chrome='accordion,tabs') - cls.tabtest = BlockFactory.create(parent=cls.chapterchrome, - display_name='pdf_textbooks_tab', - default_tab='progress') - - cls.user = GlobalStaffFactory(password='test') - - def setUp(self): - super().setUp() - self.login(self.user.email, 'test') - self.patcher = patch( - 'lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - self.mock_redirect = self.patcher.start() - - def tearDown(self): - self.patcher.stop() - super().tearDown() - - def assertTabActive(self, tabname, response): - ''' Check if the progress tab is active in the tab set ''' - for line in response.content.decode('utf-8').split('\n'): - if tabname in line and 'active' in line: - return - raise AssertionError(f"assertTabActive failed: {tabname} not active") - - def assertTabInactive(self, tabname, response): # lint-amnesty, pylint: disable=useless-return - ''' Check if the progress tab is active in the tab set ''' - for line in response.content.decode('utf-8').split('\n'): - if tabname in line and 'active' in line: - raise AssertionError("assertTabInactive failed: " + tabname + " active") - return - - # TODO: LEARNER-71: Do we need to adjust or remove this test? - @override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True) - def test_chrome_settings(self): - ''' - Test settings for disabling and modifying navigation chrome in the courseware: - - Accordion enabled, or disabled - - Navigation tabs enabled, disabled, or redirected - ''' - test_data = ( - ('tabs', False, True), - ('none', False, False), - ('accordion', True, False), - ('fullchrome', True, True), - ) - for (displayname, accordion, tabs) in test_data: - response = self.client.get(reverse('courseware_section', kwargs={ - 'course_id': str(self.course.id), - 'chapter': 'Chrome', - 'section': displayname, - })) - assert ('course-tabs' in response.content.decode('utf-8')) == tabs - assert ('course-navigation' in response.content.decode('utf-8')) == accordion - - self.assertTabInactive('progress', response) - self.assertTabActive(make_learning_mfe_courseware_url(self.course.id), response) - - response = self.client.get(reverse('courseware_section', kwargs={ - 'course_id': str(self.course.id), - 'chapter': 'Chrome', - 'section': 'pdf_textbooks_tab', - })) - - self.assertTabActive('progress', response) - self.assertTabInactive(make_learning_mfe_courseware_url(self.course.id), response) - - @override_settings(SESSION_INACTIVITY_TIMEOUT_IN_SECONDS=1) - def test_inactive_session_timeout(self): - """ - Verify that an inactive session times out and redirects to the - login page - """ - # make sure we can access courseware immediately - resp = self.client.get(reverse('dashboard')) - assert resp.status_code == 200 - - # then wait a bit and see if we get timed out - time.sleep(2) - - resp = self.client.get(reverse('dashboard')) - - # re-request, and we should get a redirect to login page - self.assertRedirects(resp, settings.LOGIN_REDIRECT_URL + '?next=' + reverse('dashboard')) - - def test_redirects_first_time(self): - """ - Verify that the first time we click on the courseware tab we are - redirected to the 'Welcome' section. - """ - resp = self.client.get(reverse('courseware', - kwargs={'course_id': str(self.course.id)})) - self.assertRedirects(resp, reverse( - 'courseware_section', kwargs={'course_id': str(self.course.id), - 'chapter': 'Overview', - 'section': 'Welcome'})) - - def test_redirects_second_time(self): - """ - Verify the accordion remembers we've already visited the Welcome section - and redirects correspondingly. - """ - section_url = reverse( - 'courseware_section', - kwargs={ - 'course_id': str(self.course.id), - 'chapter': 'Overview', - 'section': 'Welcome', - }, - ) - self.client.get(section_url) - resp = self.client.get( - reverse('courseware', kwargs={'course_id': str(self.course.id)}), - ) - self.assertRedirects(resp, section_url) - - def test_accordion_state(self): - """ - Verify the accordion remembers which chapter you were last viewing. - """ - # Now we directly navigate to a section in a chapter other than 'Overview'. - section_url = reverse( - 'courseware_section', - kwargs={ - 'course_id': str(self.course.id), - 'chapter': 'factory_chapter', - 'section': 'factory_section', - } - ) - self.assert_request_status_code(200, section_url) - - # And now hitting the courseware tab should redirect to 'factory_chapter' - url = reverse( - 'courseware', - kwargs={'course_id': str(self.course.id)} - ) - resp = self.client.get(url) - self.assertRedirects(resp, section_url) - - # TODO: LEARNER-71: Do we need to adjust or remove this test? - @override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True) - def test_incomplete_course(self): - test_course_id = str(self.test_course.id) - - url = reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) - response = self.assert_request_status_code(200, url) - self.assertContains(response, "No content has been added to this course") - - section = BlockFactory.create( - parent_location=self.test_course.location, - display_name='New Section' - ) - url = reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) - response = self.assert_request_status_code(200, url) - self.assertNotContains(response, "No content has been added to this course") - self.assertContains(response, "New Section") - - subsection = BlockFactory.create( - parent_location=section.location, - display_name='New Subsection', - ) - url = reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) - response = self.assert_request_status_code(200, url) - self.assertContains(response, "New Subsection") - self.assertNotContains(response, "sequence-nav") - - BlockFactory.create( - parent_location=subsection.location, - display_name='New Unit', - ) - url = reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) - self.assert_request_status_code(302, url) - - def test_proctoring_js_includes(self): - """ - Make sure that proctoring JS does not get included on - courseware pages if either the FEATURE flag is turned off - or the course is not proctored enabled - """ - test_course_id = str(self.test_course_proctored.id) - - with patch.dict(settings.FEATURES, {'ENABLE_SPECIAL_EXAMS': False}): - url = reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) - resp = self.client.get(url) - - self.assertNotContains(resp, '/static/js/lms-proctoring.js') - - with patch.dict(settings.FEATURES, {'ENABLE_SPECIAL_EXAMS': True}): - url = reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) - resp = self.client.get(url) - - self.assertNotContains(resp, '/static/js/lms-proctoring.js') - - # now set up a course which is proctored enabled - - self.test_course_proctored.enable_proctored_exams = True - self.test_course_proctored.save() - - modulestore().update_item(self.test_course_proctored, self.user.id) - - resp = self.client.get(url) - - self.assertContains(resp, '/static/js/lms-proctoring.js') diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py index 67ada18e85..f9668853b2 100644 --- a/lms/djangoapps/courseware/tests/test_view_authentication.py +++ b/lms/djangoapps/courseware/tests/test_view_authentication.py @@ -63,7 +63,7 @@ class TestViewAuth(EnterpriseTestConsentRequired, ModuleStoreTestCase, LoginEnro Check that non-staff don't have access to dark urls. """ - names = ['courseware', 'progress'] + names = ['progress'] urls = self._reverse_urls(names, course) urls.extend([ reverse('book', kwargs={'course_id': str(course.id), @@ -106,10 +106,6 @@ class TestViewAuth(EnterpriseTestConsentRequired, ModuleStoreTestCase, LoginEnro ) self.assert_request_status_code(302, url) - # The courseware url should redirect, not 200 - url = self._reverse_urls(['courseware'], course)[0] - self.assert_request_status_code(302, url) - def login(self, user): # lint-amnesty, pylint: disable=arguments-differ return super().login(user.email, self.TEST_PASSWORD) @@ -164,60 +160,6 @@ class TestViewAuth(EnterpriseTestConsentRequired, ModuleStoreTestCase, LoginEnro self.org_staff_user = OrgStaffFactory(course_key=self.course.id) self.org_instructor_user = OrgInstructorFactory(course_key=self.course.id) - def test_redirection_unenrolled(self): - """ - Verify unenrolled student is redirected to the 'about' section of the chapter - instead of the 'Welcome' section after clicking on the courseware tab. - """ - self.login(self.unenrolled_user) - response = self.client.get(reverse('courseware', - kwargs={'course_id': str(self.course.id)})) - self.assertRedirects( - response, - reverse( - 'about_course', - args=[str(self.course.id)] - ) - ) - - def test_redirection_enrolled(self): - """ - Verify enrolled student is redirected to the 'Welcome' section of - the chapter after clicking on the courseware tab. - """ - self.login(self.enrolled_user) - - response = self.client.get( - reverse( - 'courseware', - kwargs={'course_id': str(self.course.id)} - ) - ) - - self.assertRedirects( - response, - reverse( - 'courseware_section', - kwargs={'course_id': str(self.course.id), - 'chapter': self.overview_chapter.url_name, - 'section': self.welcome_section.url_name} - ), - fetch_redirect_response=False, # just sends us on to MFE - ) - - def test_redirection_missing_enterprise_consent(self): - """ - Verify that enrolled students are redirected to the Enterprise consent - URL if a linked Enterprise Customer requires data sharing consent - and it has not yet been provided. - """ - self.login(self.enrolled_user) - url = reverse( - 'courseware', - kwargs={'course_id': str(self.course.id)} - ) - self.verify_consent_required(self.client, url, status_code=302) # lint-amnesty, pylint: disable=no-value-for-parameter - def test_instructor_page_access_nonstaff(self): """ Verify non-staff cannot load the instructor diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 2a27a3fc41..4b59bfac24 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -73,7 +73,6 @@ from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.courseware.access_utils import check_course_open_for_learner from lms.djangoapps.courseware.model_data import FieldDataCache, set_score from lms.djangoapps.courseware.block_render import get_block, handle_xblock_callback -from lms.djangoapps.courseware.tests.factories import StudentModuleFactory from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin, get_expiration_banner_text from lms.djangoapps.courseware.testutils import RenderXBlockTestMixin from lms.djangoapps.courseware.toggles import ( @@ -94,7 +93,6 @@ from lms.djangoapps.verify_student.services import IDVerificationService from openedx.core.djangoapps.catalog.tests.factories import CourseFactory as CatalogCourseFactory from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory, ProgramFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.crawlers.models import CrawlersConfig from openedx.core.djangoapps.credit.api import set_credit_requirements from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES @@ -103,12 +101,8 @@ from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE from openedx.core.lib.url_utils import quote_slashes from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_duration_limits.models import CourseDurationLimitConfig -from openedx.features.course_experience import ( - DISABLE_COURSE_OUTLINE_PAGE_FLAG, -) 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 +226,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,80 +242,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): - """ - Tests for query count. - """ - NUM_PROBLEMS = 20 - - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_index_query_counts(self, mock_redirect): - # TODO: decrease query count as part of REVO-28 - ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) - with self.store.default_store(ModuleStoreEnum.Type.split): - course = CourseFactory.create() - with self.store.bulk_operations(course.id): - chapter = BlockFactory.create(category='chapter', parent_location=course.location) - section = BlockFactory.create(category='sequential', parent_location=chapter.location) - vertical = BlockFactory.create(category='vertical', parent_location=section.location) - for _ in range(self.NUM_PROBLEMS): - BlockFactory.create(category='problem', parent_location=vertical.location) - - self.client.login(username=self.user.username, password=self.user_password) - CourseEnrollment.enroll(self.user, course.id) - - with self.assertNumQueries(152, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): - with check_mongo_calls(3): - url = reverse( - 'courseware_section', - kwargs={ - 'course_id': str(course.id), - 'chapter': str(chapter.location.block_id), - 'section': str(section.location.block_id), - } - ) - response = self.client.get(url) - assert response.status_code == 200 - class BaseViewsTestCase(ModuleStoreTestCase, MasqueradeMixin): """Base class for courseware tests""" @@ -430,109 +325,47 @@ class CoursewareIndexTestCase(BaseViewsTestCase): super().setUp() self._create_global_staff_user() # this view needs staff permission - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_index_success(self, mock_redirect): - response = self._verify_index_response() - self.assertContains(response, self.problem2.location.replace(branch=None, version_guid=None)) + def test_course_redirect(self): + lms_url = reverse( + 'courseware', + kwargs={ + 'course_id': str(self.course_key), + } + ) - # re-access to the main course page redirects to last accessed view. - url = reverse('courseware', kwargs={'course_id': str(self.course_key)}) - response = self.client.get(url) - assert response.status_code == 302 - response = self.client.get(response.url) - self.assertNotContains(response, self.problem.location.replace(branch=None, version_guid=None)) - self.assertContains(response, self.problem2.location.replace(branch=None, version_guid=None)) + mfe_url = make_learning_mfe_courseware_url(self.course.id) - def test_index_nonexistent_chapter(self): - self._verify_index_response(expected_response_code=404, chapter_name='non-existent') + response = self.client.get(lms_url) + assert response.url == mfe_url - def test_index_nonexistent_chapter_masquerade(self): - self.update_masquerade(username=self.user.username) - self._verify_index_response(expected_response_code=302, chapter_name='non-existent') - - def test_index_nonexistent_section(self): - self._verify_index_response(expected_response_code=404, section_name='non-existent') - - def test_index_nonexistent_section_masquerade(self): - self.update_masquerade(username=self.user.username) - self._verify_index_response(expected_response_code=302, section_name='non-existent') - - def _verify_index_response(self, expected_response_code=200, chapter_name=None, section_name=None): - """ - Verifies the response when the courseware index page is accessed with - the given chapter and section names. - """ - url = reverse( + def test_section_redirect(self): + lms_url = reverse( 'courseware_section', kwargs={ 'course_id': str(self.course_key), - 'chapter': str(self.chapter.location.block_id) if chapter_name is None else chapter_name, - 'section': str(self.section2.location.block_id) if section_name is None else section_name, + 'section': str(self.chapter.location.block_id), } ) - response = self.client.get(url) - 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) + mfe_url = make_learning_mfe_courseware_url(self.course.id) - def test_index_invalid_position(self): - request_url = '/'.join([ - '/courses', - str(self.course.id), - 'courseware', - self.chapter.location.block_id, - self.section.location.block_id, - 'f' - ]) - response = self.client.get(request_url) - assert response.status_code == 404 + response = self.client.get(lms_url) + assert response.url == mfe_url - def test_unicode_handling_in_url(self): - url_parts = [ - '/courses', - str(self.course.id), - 'courseware', - self.chapter.location.block_id, - self.section.location.block_id, - '1' - ] - for idx, val in enumerate(url_parts): - url_parts_copy = url_parts[:] - url_parts_copy[idx] = val + 'χ' - request_url = '/'.join(url_parts_copy) - response = self.client.get(request_url) - assert response.status_code == 404 - - # TODO: TNL-6387: Remove test - @override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True) - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_accordion(self, mock_redirect): - """ - This needs a response_context, which is not included in the render_accordion's main method - returning a render_to_string, so we will render via the courseware URL in order to include - the needed context - """ - response = self.client.get( - reverse('courseware', args=[str(self.course.id)]), - follow=True + def test_subsection_redirect(self): + lms_url = reverse( + 'courseware_subsection', + kwargs={ + 'course_id': str(self.course_key), + 'section': str(self.chapter.location.block_id), + 'subsection': str(self.section2.location.block_id), + } ) - test_responses = [ - '

Sequential 1 current section

', - '

Sequential 2

' - ] - for test in test_responses: - self.assertContains(response, test) + + mfe_url = make_learning_mfe_courseware_url(self.course.id, self.section2.location) + + response = self.client.get(lms_url) + assert response.url == mfe_url @ddt.ddt @@ -1122,56 +955,6 @@ class TestProgressDueDate(BaseDueDateTests): return self.client.get(reverse('progress', args=[str(course.id)])) -# TODO: LEARNER-71: Delete entire TestAccordionDueDate class -class TestAccordionDueDate(BaseDueDateTests): - """ - Test that the accordion page displays due dates correctly - """ - __test__ = True - - def setUp(self): - super().setUp() - self.patcher = patch( - 'lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - self.mock_redirect = self.patcher.start() - - def tearDown(self): - self.patcher.stop() - super().tearDown() - - def get_response(self, course): - """ Returns the HTML for the accordion """ - return self.client.get( - reverse('courseware', args=[str(course.id)]), - follow=True - ) - - # TODO: LEARNER-71: Delete entire TestAccordionDueDate class - @override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True) - def test_backwards_compatibility(self): - super().test_backwards_compatibility() - - # TODO: LEARNER-71: Delete entire TestAccordionDueDate class - @override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True) - def test_defaults(self): - super().test_defaults() - - # TODO: LEARNER-71: Delete entire TestAccordionDueDate class - @override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True) - def test_format_date(self): - super().test_format_date() - - # TODO: LEARNER-71: Delete entire TestAccordionDueDate class - @override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True) - def test_format_invalid(self): - super().test_format_invalid() - - # TODO: LEARNER-71: Delete entire TestAccordionDueDate class - @override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True) - def test_format_none(self): - super().test_format_none() - - class StartDateTests(ModuleStoreTestCase): """ Test that start dates are properly localized and displayed on the student @@ -2351,72 +2134,6 @@ class TestIndexView(ModuleStoreTestCase): """ Tests of the courseware.views.index view. """ - @XBlock.register_temp_plugin(ViewCheckerBlock, 'view_checker') - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_student_state(self, mock_redirect): - """ - Verify that saved student state is loaded for xblocks rendered in the index view. - """ - with modulestore().default_store(ModuleStoreEnum.Type.split): - course = CourseFactory.create() - chapter = BlockFactory.create(parent_location=course.location, category='chapter') - section = BlockFactory.create(parent_location=chapter.location, category='view_checker', - display_name="Sequence Checker") - vertical = BlockFactory.create(parent_location=section.location, category='view_checker', - display_name="Vertical Checker") - block = BlockFactory.create(parent_location=vertical.location, category='view_checker', - display_name="Block Checker") - - for item in (section, vertical, block): - StudentModuleFactory.create( - student=self.user, - course_id=course.id, - module_state_key=item.scope_ids.usage_id, - state=json.dumps({'state': str(item.scope_ids.usage_id)}) - ) - - CourseOverview.load_from_module_store(course.id) - CourseEnrollmentFactory(user=self.user, course_id=course.id) - - assert self.client.login(username=self.user.username, password=self.user_password) - response = self.client.get( - reverse( - 'courseware_section', - kwargs={ - 'course_id': str(course.id), - 'chapter': chapter.url_name, - 'section': section.url_name, - } - ) - ) - # Trigger the assertions embedded in the ViewCheckerBlocks - self.assertContains(response, "ViewCheckerPassed", count=3) - - @XBlock.register_temp_plugin(ActivateIDCheckerBlock, 'id_checker') - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_activate_block_id(self, mock_redirect): - course = CourseFactory.create() - with self.store.bulk_operations(course.id): - chapter = BlockFactory.create(parent=course, category='chapter') - section = BlockFactory.create(parent=chapter, category='sequential', display_name="Sequence") - vertical = BlockFactory.create(parent=section, category='vertical', display_name="Vertical") - BlockFactory.create(parent=vertical, category='id_checker', display_name="ID Checker") - - CourseOverview.load_from_module_store(course.id) - CourseEnrollmentFactory(user=self.user, course_id=course.id) - - assert self.client.login(username=self.user.username, password=self.user_password) - response = self.client.get( - reverse( - 'courseware_section', - kwargs={ - 'course_id': str(course.id), - 'chapter': chapter.url_name, - 'section': section.url_name, - } - ) + '?activate_block_id=test_block_id' - ) - self.assertContains(response, "Activate Block ID: test_block_id") @patch('lms.djangoapps.courseware.views.views.CourseTabView.course_open_for_learner_enrollment') @patch('openedx.core.djangoapps.util.user_messages.PageLevelMessages.register_warning_message') @@ -2501,207 +2218,6 @@ class TestIndexView(ModuleStoreTestCase): assert views.CourseTabView.course_open_for_learner_enrollment(course) == expected_should_show_enroll_button -@ddt.ddt -class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin): - """ - Tests CompleteOnView is set up correctly in CoursewareIndex. - """ - - def setup_course(self, default_store): - """ - Set up course content for modulestore. - """ - # pylint:disable=attribute-defined-outside-init - - self.request_factory = RequestFactoryNoCsrf() - - with modulestore().default_store(default_store): - self.course = CourseFactory.create() - - with self.store.bulk_operations(self.course.id): - - self.chapter = BlockFactory.create( - parent_location=self.course.location, category='chapter', display_name='Week 1' - ) - self.section_1 = BlockFactory.create( - parent_location=self.chapter.location, category='sequential', display_name='Lesson 1' - ) - self.vertical_1 = BlockFactory.create( - parent_location=self.section_1.location, category='vertical', display_name='Subsection 1' - ) - self.html_1_1 = BlockFactory.create( - parent_location=self.vertical_1.location, category='html', display_name="HTML 1_1" - ) - self.problem_1 = BlockFactory.create( - parent_location=self.vertical_1.location, category='problem', display_name="Problem 1" - ) - self.html_1_2 = BlockFactory.create( - parent_location=self.vertical_1.location, category='html', display_name="HTML 1_2" - ) - - self.section_2 = BlockFactory.create( - parent_location=self.chapter.location, category='sequential', display_name='Lesson 2' - ) - self.vertical_2 = BlockFactory.create( - parent_location=self.section_2.location, category='vertical', display_name='Subsection 2' - ) - self.video_2 = BlockFactory.create( - parent_location=self.vertical_2.location, category='video', display_name="Video 2" - ) - self.problem_2 = BlockFactory.create( - parent_location=self.vertical_2.location, category='problem', display_name="Problem 2" - ) - - self.section_1_url = reverse( - 'courseware_section', - kwargs={ - 'course_id': str(self.course.id), - 'chapter': self.chapter.url_name, - 'section': self.section_1.url_name, - } - ) - - self.section_2_url = reverse( - 'courseware_section', - kwargs={ - 'course_id': str(self.course.id), - 'chapter': self.chapter.url_name, - 'section': self.section_2.url_name, - } - ) - - CourseOverview.load_from_module_store(self.course.id) - CourseEnrollmentFactory(user=self.user, course_id=self.course.id) - assert self.client.login(username=self.user.username, password=self.user_password) - - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_completion_service_disabled(self, mock_redirect): - - self.setup_course(ModuleStoreEnum.Type.split) - - response = self.client.get(self.section_1_url) - self.assertNotContains(response, 'data-mark-completed-on-view-after-delay') - - response = self.client.get(self.section_2_url) - self.assertNotContains(response, 'data-mark-completed-on-view-after-delay') - - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_completion_service_enabled(self, mock_redirect): - - self.override_waffle_switch(True) - - self.setup_course(ModuleStoreEnum.Type.split) - - response = self.client.get(self.section_1_url) - self.assertContains(response, 'data-mark-completed-on-view-after-delay') - self.assertContains(response, 'data-mark-completed-on-view-after-delay', count=2) - - request = self.request_factory.post( - '/', - data=json.dumps({"completion": 1}), - content_type='application/json', - ) - request.user = self.user - request.session = {} - response = handle_xblock_callback( - request, - str(self.course.id), - quote_slashes(str(self.html_1_1.scope_ids.usage_id)), - 'publish_completion', - ) - assert json.loads(response.content.decode('utf-8')) == {'result': 'ok'} - - response = self.client.get(self.section_1_url) - self.assertContains(response, 'data-mark-completed-on-view-after-delay') - self.assertContains(response, 'data-mark-completed-on-view-after-delay', count=1) - - request = self.request_factory.post( - '/', - data=json.dumps({"completion": 1}), - content_type='application/json', - ) - request.user = self.user - request.session = {} - response = handle_xblock_callback( - request, - str(self.course.id), - quote_slashes(str(self.html_1_2.scope_ids.usage_id)), - 'publish_completion', - ) - assert json.loads(response.content.decode('utf-8')) == {'result': 'ok'} - - response = self.client.get(self.section_1_url) - self.assertNotContains(response, 'data-mark-completed-on-view-after-delay') - - response = self.client.get(self.section_2_url) - self.assertNotContains(response, 'data-mark-completed-on-view-after-delay') - - -@ddt.ddt -class TestIndexViewWithVerticalPositions(ModuleStoreTestCase): - """ - Test the index view to handle vertical positions. Confirms that first position is loaded - if input position is non-positive or greater than number of positions available. - """ - - def setUp(self): - """ - Set up initial test data - """ - super().setUp() - - # create course with 3 positions - self.course = CourseFactory.create() - with self.store.bulk_operations(self.course.id): - self.chapter = BlockFactory.create(parent_location=self.course.location, category='chapter') - self.section = BlockFactory.create(parent_location=self.chapter.location, category='sequential', - display_name="Sequence") - BlockFactory.create(parent_location=self.section.location, category='vertical', display_name="Vertical1") - BlockFactory.create(parent_location=self.section.location, category='vertical', display_name="Vertical2") - BlockFactory.create(parent_location=self.section.location, category='vertical', display_name="Vertical3") - - CourseOverview.load_from_module_store(self.course.id) - - self.client.login(username=self.user, password=self.user_password) - CourseEnrollmentFactory(user=self.user, course_id=self.course.id) - - def _get_course_vertical_by_position(self, input_position): - """ - Returns client response to input position. - """ - return self.client.get( - reverse( - 'courseware_position', - kwargs={ - 'course_id': str(self.course.id), - 'chapter': self.chapter.url_name, - 'section': self.section.url_name, - 'position': input_position, - } - ) - ) - - def _assert_correct_position(self, response, expected_position): - """ - Asserts that the expected position and the position in the response are the same - """ - self.assertContains(response, f'data-position="{expected_position}"') - - @ddt.data(("-1", 1), ("0", 1), ("-0", 1), ("2", 2), ("5", 1)) - @ddt.unpack - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_vertical_positions(self, input_position, expected_position, mock_redirect): - """ - Tests the following cases: - * Load first position when negative position inputted. - * Load first position when 0/-0 position inputted. - * Load given position when 0 < input_position <= num_positions_available. - * Load first position when positive position > num_positions_available. - """ - resp = self._get_course_vertical_by_position(input_position) - self._assert_correct_position(resp, expected_position) - - @ddt.ddt class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase, CompletionWaffleTestMixin): """ @@ -3083,89 +2599,6 @@ class TestRenderXBlockSelfPaced(TestRenderXBlock): # lint-amnesty, pylint: disa return options -class TestIndexViewCrawlerStudentStateWrites(SharedModuleStoreTestCase): - """ - Ensure that courseware index requests do not trigger student state writes. - This is to prevent locking issues that have caused latency spikes in the - courseware_studentmodule table when concurrent requests each try to update - the same rows for sequence, section, and course positions. - """ - @classmethod - def setUpClass(cls): - """Set up the simplest course possible.""" - # setUpClassAndTestData() already calls setUpClass on SharedModuleStoreTestCase - # pylint: disable=super-method-not-called - with super().setUpClassAndTestData(): - cls.course = CourseFactory.create() - with cls.store.bulk_operations(cls.course.id): - cls.chapter = BlockFactory.create(category='chapter', parent_location=cls.course.location) - cls.section = BlockFactory.create(category='sequential', parent_location=cls.chapter.location) - cls.vertical = BlockFactory.create(category='vertical', parent_location=cls.section.location) - - @classmethod - def setUpTestData(cls): # lint-amnesty, pylint: disable=super-method-not-called - """Set up and enroll our fake user in the course.""" - cls.user = UserFactory(is_staff=True) - CourseEnrollment.enroll(cls.user, cls.course.id) - - def setUp(self): - """Do the client login.""" - super().setUp() - self.client.login(username=self.user.username, password=TEST_PASSWORD) - - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_write_by_default(self, mock_redirect): - """By default, always write student state, regardless of user agent.""" - with patch('lms.djangoapps.courseware.model_data.UserStateCache.set_many') as patched_state_client_set_many: - # Simulate someone using Chrome - self._load_courseware('Mozilla/5.0 AppleWebKit/537.36') - assert patched_state_client_set_many.called - patched_state_client_set_many.reset_mock() - - # Common crawler user agent - self._load_courseware('edX-downloader/0.1') - assert patched_state_client_set_many.called - - @patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None) - def test_writes_with_config(self, mock_redirect): - """Test state writes (or lack thereof) based on config values.""" - CrawlersConfig.objects.create(known_user_agents='edX-downloader,crawler_foo', enabled=True) - with patch('lms.djangoapps.courseware.model_data.UserStateCache.set_many') as patched_state_client_set_many: - # Exact matching of crawler user agent - self._load_courseware('crawler_foo') - assert not patched_state_client_set_many.called - - # Partial matching of crawler user agent - self._load_courseware('edX-downloader/0.1') - assert not patched_state_client_set_many.called - - # Simulate an actual browser hitting it (we should write) - self._load_courseware('Mozilla/5.0 AppleWebKit/537.36') - assert patched_state_client_set_many.called - - # Disabling the crawlers config should revert us to default behavior - CrawlersConfig.objects.create(enabled=False) - - # Disabling the violation because pylint just can't see that we'll get the mock_redirect param passed in via the - # patch. - self.test_write_by_default() # pylint: disable=no-value-for-parameter - - def _load_courseware(self, user_agent): - """Helper to load the actual courseware page.""" - url = reverse( - 'courseware_section', - kwargs={ - 'course_id': str(self.course.id), - 'chapter': str(self.chapter.location.block_id), - 'section': str(self.section.location.block_id), - } - ) - response = self.client.get(url, HTTP_USER_AGENT=user_agent) - # Make sure we get back an actual 200, and aren't redirected because we - # messed up the setup somehow (e.g. didn't enroll properly) - assert response.status_code == 200 - - class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ModuleStoreTestCase): """ Ensure that the Enterprise Data Consent redirects are in place only when consent is required. @@ -3188,7 +2621,6 @@ class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ModuleStoreTestCa course_id = str(self.course.id) for url in ( - reverse("courseware", kwargs=dict(course_id=course_id)), reverse("progress", kwargs=dict(course_id=course_id)), reverse("student_progress", kwargs=dict(course_id=course_id, student_id=str(self.user.id))), ): @@ -3419,7 +2851,6 @@ class TestCourseWideResources(ModuleStoreTestCase): """ @ddt.data( - ('courseware', 'course_id', False, True), ('progress', 'course_id', False, False), ('instructor_dashboard', 'course_id', True, False), ('forum_form_discussion', 'course_id', False, False), diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index b0f444f288..5f47b258ae 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -12,12 +12,10 @@ 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 -from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order @@ -165,32 +163,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) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 02f0ef1f7f..1034674b7b 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -6,72 +6,32 @@ View for Courseware Index import logging -import urllib -from django.conf import settings from django.contrib.auth.views import redirect_to_login -from django.db import transaction -from django.http import Http404 -from django.template.context_processors import csrf -from django.urls import reverse from django.utils.decorators import method_decorator -from django.utils.functional import cached_property -from django.utils.translation import gettext as _ from django.views.decorators.cache import cache_control from django.views.decorators.csrf import ensure_csrf_cookie +from django.utils.functional import cached_property from django.views.generic import View -from edx_django_utils.monitoring import set_custom_attributes_for_course_key + from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey -from web_fragments.fragment import Fragment -from xmodule.course_block import COURSE_VISIBILITY_PUBLIC from xmodule.modulestore.django import modulestore -from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW -from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string -from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.util.views import ensure_valid_course_key -from lms.djangoapps.courseware.exceptions import CourseAccessRedirect, Redirect -from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context -from lms.djangoapps.gating.api import get_entrance_exam_score, get_entrance_exam_usage_key -from lms.djangoapps.grades.api import CourseGradeFactory -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.crawlers.models import CrawlersConfig -from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY -from openedx.core.djangoapps.user_api.preferences.api import get_user_preference -from openedx.core.djangoapps.util.user_messages import PageLevelMessages -from openedx.core.djangolib.markup import HTML, Text -from openedx.features.course_experience import ( - COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, - DISABLE_COURSE_OUTLINE_PAGE_FLAG, - default_course_url -) +from lms.djangoapps.courseware.exceptions import Redirect +from lms.djangoapps.courseware.masquerade import setup_masquerade from openedx.features.course_experience.url_helpers import make_learning_mfe_courseware_url +from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG from openedx.features.enterprise_support.api import data_sharing_consent_required -from ..access import has_access -from ..access_utils import check_public_access -from ..courses import get_course_with_access, get_current_child, get_studio_url -from ..entrance_exams import ( - course_has_entrance_exam, - get_entrance_exam_content, - user_can_skip_entrance_exam, - user_has_passed_entrance_exam -) -from ..masquerade import check_content_start_date_for_masquerade_user, setup_masquerade -from ..model_data import FieldDataCache -from ..block_render import get_block_for_descriptor, toc_for_course +from ..block_render import get_block_for_descriptor +from ..courses import get_course_with_access from ..permissions import MASQUERADE_AS_STUDENT -from ..toggles import ENABLE_OPTIMIZELY_IN_COURSEWARE -from .views import CourseTabView log = logging.getLogger("edx.courseware.views.index") -TEMPLATE_IMPORTS = {'urllib': urllib} -CONTENT_DEPTH = 2 - -@method_decorator(transaction.non_atomic_requests, name='dispatch') class CoursewareIndex(View): """ View class for the Courseware page. @@ -85,477 +45,74 @@ class CoursewareIndex(View): @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True)) @method_decorator(ensure_valid_course_key) @method_decorator(data_sharing_consent_required) - def get(self, request, course_id, chapter=None, section=None, position=None): + def get(self, request, course_id, section=None, subsection=None, position=None): """ - Displays courseware accordion and associated content. If course, chapter, - and section are all specified, renders the page, or returns an error if they - are invalid. - - If section is not specified, displays the accordion opened to the right - chapter. - - If neither chapter or section are specified, displays the user's most - recent chapter, or the first chapter if this is the user's first visit. + Instead of loading the legacy courseware sequences pages, load the equivalent URL + in the learning MFE. This view does not do any auth checks since they are done by + the MFE when attempting to load content. Arguments: request: HTTP request course_id (unicode): course id - chapter (unicode): chapter url_name section (unicode): section url_name + subsection (unicode): subsection url_name position (unicode): position in block, eg of block + """ + self.course_key = CourseKey.from_string(course_id) if not (request.user.is_authenticated or self.enable_unenrolled_access): return redirect_to_login(request.get_full_path()) - self.original_chapter_url_name = chapter - self.original_section_url_name = section - self.chapter_url_name = chapter - self.section_url_name = section - self.position = position - self.chapter, self.section = None, None - self.course = None - self.url = request.path + # Course load to resolve sections/subsections + with modulestore().bulk_operations(self.course_key): + course = get_course_with_access( + request.user, + "load", + self.course_key, + depth=2, + check_if_enrolled=True, + check_if_authenticated=True, + ) + + # Get the section, subsection and unit blocks so that we can redirect to the right content + # location in the MFE + subsection_location = None + if section and subsection: + section_block = course.get_child_by(lambda m: m.location.block_id == section) + if section_block: + subsection_block = section_block.get_child_by(lambda m: m.location.block_id == subsection) + if subsection_block: + subsection_location = subsection_block.location try: - set_custom_attributes_for_course_key(self.course_key) - self._clean_position() - with modulestore().bulk_operations(self.course_key): - - self.view = STUDENT_VIEW - - self.course = get_course_with_access( - request.user, 'load', self.course_key, - depth=CONTENT_DEPTH, - check_if_enrolled=True, - check_if_authenticated=True - ) - self.course_overview = CourseOverview.get_from_id(self.course.id) - self.is_staff = has_access(request.user, 'staff', self.course) - - # There's only one situation where we want to show the public view - if ( - not self.is_staff and - self.enable_unenrolled_access and - self.course.course_visibility == COURSE_VISIBILITY_PUBLIC and - not CourseEnrollment.is_enrolled(request.user, self.course_key) - ): - self.view = PUBLIC_VIEW - - self.can_masquerade = request.user.has_perm(MASQUERADE_AS_STUDENT, self.course) - self._setup_masquerade_for_effective_user() - - return self.render(request) - except Exception as exception: # pylint: disable=broad-except - return CourseTabView.handle_exceptions(request, self.course_key, self.course, exception) - - def _setup_masquerade_for_effective_user(self): - """ - Setup the masquerade information to allow the request to - be processed for the requested effective user. - """ - self.real_user = self.request.user - self.masquerade, self.effective_user = setup_masquerade( - self.request, - self.course_key, - self.can_masquerade, - reset_masquerade_data=True - ) - # Set the user in the request to the effective user. - self.request.user = self.effective_user - - def _redirect_to_learning_mfe(self): - """ - Can the user access this sequence in the courseware MFE? If so, redirect to MFE. - """ - # If the MFE is active, prefer that - raise Redirect(self.microfrontend_url) - - @property - def microfrontend_url(self): - """ - Return absolute URL to this section in the courseware micro-frontend. - """ - try: - unit_key = UsageKey.from_string(self.request.GET.get('activate_block_id', '')) - # `activate_block_id` is typically a Unit (a.k.a. Vertical), - # but it can technically be any block type. Do a check to - # make sure it's really a Unit before we use it for the MFE. + unit_key = UsageKey.from_string(request.GET.get('activate_block_id', '')) if unit_key.block_type != 'vertical': unit_key = None except InvalidKeyError: unit_key = None - is_preview = False - url = make_learning_mfe_courseware_url( + + # Setup masquerading if needed for this user. + # This in needed even though this view just does a redirect because + # the relevant cookies and session data is set for future requests + # when this function is called. + self.masquerade, self.effective_user = setup_masquerade( + self.request, self.course_key, - self.section.location if self.section else None, + request.user.has_perm(MASQUERADE_AS_STUDENT, course), + reset_masquerade_data=True + ) + # Set the user in the request to the effective user. + self.request.user = self.effective_user + mfe_url = make_learning_mfe_courseware_url( + self.course_key, + subsection_location, unit_key, - params=self.request.GET, - preview=is_preview, + params=request.GET, + preview=False ) - return url - - def render(self, request): - """ - Render the index page. - """ - self._prefetch_and_bind_course(request) - - if self.course.has_children_at_depth(CONTENT_DEPTH): - self._reset_section_to_exam_if_required() - self.chapter = self._find_chapter() - self.section = self._find_section() - - if self.chapter and self.section: - self._redirect_if_not_requested_section() - self._save_positions() - self._prefetch_and_bind_section() - self._redirect_to_learning_mfe() - - check_content_start_date_for_masquerade_user(self.course_key, self.effective_user, request, - self.course.start, self.chapter.start, self.section.start) - - if not request.user.is_authenticated: - qs = urllib.parse.urlencode({ - 'course_id': self.course_key, - 'enrollment_action': 'enroll', - 'email_opt_in': False, - }) - - allow_anonymous = check_public_access(self.course, [COURSE_VISIBILITY_PUBLIC]) - - if not allow_anonymous: - PageLevelMessages.register_warning_message( - request, - Text(_("You are not signed in. To see additional course content, {sign_in_link} or " - "{register_link}, and enroll in this course.")).format( - sign_in_link=HTML('{sign_in_label}').format( - sign_in_label=_('sign in'), - url='{}?{}'.format(reverse('signin_user'), qs), - ), - register_link=HTML('{register_label}').format( - register_label=_('register'), - url='{}?{}'.format(reverse('register_user'), qs), - ), - ) - ) - - return render_to_response('courseware/courseware.html', self._create_courseware_context(request)) - - def _redirect_if_not_requested_section(self): - """ - If the resulting section and chapter are different from what was initially - requested, redirect back to the index page, but with an updated URL that includes - the correct section and chapter values. We do this so that our analytics events - and error logs have the appropriate URLs. - """ - if ( - self.chapter.url_name != self.original_chapter_url_name or - (self.original_section_url_name and self.section.url_name != self.original_section_url_name) - ): - raise CourseAccessRedirect( - reverse( - 'courseware_section', - kwargs={ - 'course_id': str(self.course_key), - 'chapter': self.chapter.url_name, - 'section': self.section.url_name, - }, - ) - ) - - def _clean_position(self): - """ - Verify that the given position is an integer. If it is not positive, set it to 1. - """ - if self.position is not None: - try: - self.position = max(int(self.position), 1) - except ValueError: - raise Http404(f"Position {self.position} is not an integer!") # lint-amnesty, pylint: disable=raise-missing-from - - def _reset_section_to_exam_if_required(self): - """ - Check to see if an Entrance Exam is required for the user. - """ - if not user_can_skip_entrance_exam(self.effective_user, self.course): - exam_chapter = get_entrance_exam_content(self.effective_user, self.course) - if exam_chapter and exam_chapter.get_children(): - exam_section = exam_chapter.get_children()[0] - if exam_section: - self.chapter_url_name = exam_chapter.url_name - self.section_url_name = exam_section.url_name - - def _get_language_preference(self): - """ - Returns the preferred language for the actual user making the request. - """ - language_preference = settings.LANGUAGE_CODE - - if self.request.user.is_authenticated: - language_preference = get_user_preference(self.real_user, LANGUAGE_KEY) - - return language_preference - - def _is_masquerading_as_student(self): - """ - Returns whether the current request is masquerading as a student. - """ - return self.masquerade and self.masquerade.role == 'student' - - def _is_masquerading_as_specific_student(self): - """ - Returns whether the current request is masqueurading as a specific student. - """ - return self._is_masquerading_as_student() and self.masquerade.user_name - - def _find_block(self, parent, url_name, block_type, min_depth=None): - """ - Finds the block in the parent with the specified url_name. - If not found, calls get_current_child on the parent. - """ - child = None - if url_name: - child = parent.get_child_by(lambda m: m.location.block_id == url_name) - if not child: - # User may be trying to access a child that isn't live yet - if not self._is_masquerading_as_student(): - raise Http404('No {block_type} found with name {url_name}'.format( - block_type=block_type, - url_name=url_name, - )) - elif min_depth and not child.has_children_at_depth(min_depth - 1): - child = None - if not child: - child = get_current_child(parent, min_depth=min_depth, requested_child=self.request.GET.get("child")) - return child - - def _find_chapter(self): - """ - Finds the requested chapter. - """ - return self._find_block(self.course, self.chapter_url_name, 'chapter', CONTENT_DEPTH - 1) - - def _find_section(self): - """ - Finds the requested section. - """ - if self.chapter: - return self._find_block(self.chapter, self.section_url_name, 'section') - - def _prefetch_and_bind_course(self, request): - """ - Prefetches all descendant data for the requested section and - sets up the runtime, which binds the request user to the section. - """ - self.field_data_cache = FieldDataCache.cache_for_block_descendents( - self.course_key, - self.effective_user, - self.course, - depth=CONTENT_DEPTH, - read_only=CrawlersConfig.is_crawler(request), - ) - - self.course = get_block_for_descriptor( - self.effective_user, - self.request, - self.course, - self.field_data_cache, - self.course_key, - course=self.course, - will_recheck_access=True, - ) - - def _prefetch_and_bind_section(self): - """ - Prefetches all descendant data for the requested section and - sets up the runtime, which binds the request user to the section. - """ - # Pre-fetch all descendant data - self.section = modulestore().get_item(self.section.location, depth=None, lazy=False) - self.field_data_cache.add_block_descendents(self.section, depth=None) - - # Bind section to user - self.section = get_block_for_descriptor( - self.effective_user, - self.request, - self.section, - self.field_data_cache, - self.course_key, - self.position, - course=self.course, - will_recheck_access=True, - ) - - def _save_positions(self): - """ - Save where we are in the course and chapter. - """ - save_child_position(self.course, self.chapter_url_name) - save_child_position(self.chapter, self.section_url_name) - - def _create_courseware_context(self, request): - """ - Returns and creates the rendering context for the courseware. - Also returns the table of contents for the courseware. - """ - - course_url = default_course_url(self.course.id) - show_search = ( - settings.FEATURES.get('ENABLE_COURSEWARE_SEARCH') or - (settings.FEATURES.get('ENABLE_COURSEWARE_SEARCH_FOR_COURSE_STAFF') and self.is_staff) - ) - staff_access = self.is_staff - - courseware_context = { - 'csrf': csrf(self.request)['csrf_token'], - 'course': self.course, - 'course_url': course_url, - 'chapter': self.chapter, - 'section': self.section, - 'init': '', - 'fragment': Fragment(), - 'staff_access': staff_access, - 'can_masquerade': self.can_masquerade, - 'masquerade': self.masquerade, - 'supports_preview_menu': True, - 'studio_url': get_studio_url(self.course, 'course'), - 'xqa_server': settings.FEATURES.get('XQA_SERVER', "http://your_xqa_server.com"), - 'bookmarks_api_url': reverse('bookmarks'), - 'language_preference': self._get_language_preference(), - 'disable_optimizely': not ENABLE_OPTIMIZELY_IN_COURSEWARE.is_enabled(), - 'section_title': None, - 'sequence_title': None, - 'disable_accordion': not DISABLE_COURSE_OUTLINE_PAGE_FLAG.is_enabled(self.course.id), - 'show_search': show_search, - 'render_course_wide_assets': True, - } - courseware_context.update( - get_experiment_user_metadata_context( - self.course, - self.effective_user, - ) - ) - table_of_contents = toc_for_course( - self.effective_user, - self.request, - self.course, - self.chapter_url_name, - self.section_url_name, - self.field_data_cache, - ) - courseware_context['accordion'] = render_accordion( - self.request, - self.course, - table_of_contents['chapters'], - ) - - # entrance exam data - self._add_entrance_exam_to_context(courseware_context) - - if self.section: - # chromeless data - if self.section.chrome: - chrome = [s.strip() for s in self.section.chrome.lower().split(",")] - if 'accordion' not in chrome: - courseware_context['disable_accordion'] = True - if 'tabs' not in chrome: - courseware_context['disable_tabs'] = True - - # default tab - if self.section.default_tab: - courseware_context['default_tab'] = self.section.default_tab - - # section data - courseware_context['section_title'] = self.section.display_name_with_default - section_context = self._create_section_context( - table_of_contents['previous_of_active_section'], - table_of_contents['next_of_active_section'], - ) - courseware_context['fragment'] = self.section.render(self.view, section_context) - - if self.section.position and self.section.has_children: - self._add_sequence_title_to_context(courseware_context) - - return courseware_context - - def _add_sequence_title_to_context(self, courseware_context): - """ - Adds sequence title to the given context. - - If we're rendering a section with some display items, but position - exceeds the length of the displayable items, default the position - to the first element. - """ - display_items = self.section.get_children() - if not display_items: - return - if self.section.position > len(display_items): - self.section.position = 1 - courseware_context['sequence_title'] = display_items[self.section.position - 1].display_name_with_default - - def _add_entrance_exam_to_context(self, courseware_context): - """ - Adds entrance exam related information to the given context. - """ - if course_has_entrance_exam(self.course) and getattr(self.chapter, 'is_entrance_exam', False): - courseware_context['entrance_exam_passed'] = user_has_passed_entrance_exam(self.effective_user, self.course) - courseware_context['entrance_exam_current_score'] = get_entrance_exam_score( - CourseGradeFactory().read(self.effective_user, self.course), - get_entrance_exam_usage_key(self.course), - ) - - def _create_section_context(self, previous_of_active_section, next_of_active_section): - """ - Returns and creates the rendering context for the section. - """ - def _compute_section_url(section_info, requested_child): - """ - Returns the section URL for the given section_info with the given child parameter. - """ - return "{url}?child={requested_child}".format( - url=reverse( - 'courseware_section', - args=[str(self.course_key), section_info['chapter_url_name'], section_info['url_name']], - ), - requested_child=requested_child, - ) - - # NOTE (CCB): Pull the position from the URL for un-authenticated users. Otherwise, pull the saved - # state from the data store. - position = None if self.request.user.is_authenticated else self.position - section_context = { - 'activate_block_id': self.request.GET.get('activate_block_id'), - 'requested_child': self.request.GET.get("child"), - 'progress_url': reverse('progress', kwargs={'course_id': str(self.course_key)}), - 'user_authenticated': self.request.user.is_authenticated, - 'position': position, - } - if previous_of_active_section: - section_context['prev_url'] = _compute_section_url(previous_of_active_section, 'last') - if next_of_active_section: - section_context['next_url'] = _compute_section_url(next_of_active_section, 'first') - # sections can hide data that masquerading staff should see when debugging issues with specific students - section_context['specific_masquerade'] = self._is_masquerading_as_specific_student() - return section_context - - -def render_accordion(request, course, table_of_contents): - """ - Returns the HTML that renders the navigation for the given course. - Expects the table_of_contents to have data on each chapter and section, - including which ones are active. - """ - context = dict( - [ - ('toc', table_of_contents), - ('course_id', str(course.id)), - ('csrf', csrf(request)['csrf_token']), - ('due_date_display_format', course.due_date_display_format), - ] + list(TEMPLATE_IMPORTS.items()) - ) - return render_to_string('courseware/accordion.html', context) + raise Redirect(mfe_url) def save_child_position(seq_block, child_name): diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index f81947ce1e..5e68ae4b20 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/lms/djangoapps/edxnotes/helpers.py @@ -405,35 +405,35 @@ def get_course_position(course_block): """ Return the user's current place in the course. - If this is the user's first time, leads to COURSE/CHAPTER/SECTION. - If this isn't the users's first time, leads to COURSE/CHAPTER. + If this is the user's first time, leads to COURSE/SECTION/SUBSECTION. + If this isn't the users's first time, leads to COURSE/SECTION. - If there is no current position in the course or chapter, then selects + If there is no current position in the course or subsection, then selects the first child. """ urlargs = {'course_id': str(course_block.id)} - chapter = get_current_child(course_block, min_depth=1) - if chapter is None: - log.debug("No chapter found when loading current position in course") - return None - - urlargs['chapter'] = chapter.url_name - if course_block.position is not None: - return { - 'display_name': Text(chapter.display_name_with_default), - 'url': reverse('courseware_chapter', kwargs=urlargs), - } - - # Relying on default of returning first child - section = get_current_child(chapter, min_depth=1) + section = get_current_child(course_block, min_depth=1) if section is None: log.debug("No section found when loading current position in course") return None urlargs['section'] = section.url_name + if course_block.position is not None: + return { + 'display_name': Text(section.display_name_with_default), + 'url': reverse('courseware_section', kwargs=urlargs), + } + + # Relying on default of returning first child + subsection = get_current_child(section, min_depth=1) + if subsection is None: + log.debug("No subsection found when loading current position in course") + return None + + urlargs['subsection'] = subsection.url_name return { - 'display_name': Text(section.display_name_with_default), - 'url': reverse('courseware_section', kwargs=urlargs) + 'display_name': Text(subsection.display_name_with_default), + 'url': reverse('courseware_subsection', kwargs=urlargs) } diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 688cd54386..acc6140d1a 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -244,8 +244,8 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): """ return reverse('courseware_position', kwargs={ 'course_id': course.id, - 'chapter': chapter.url_name, - 'section': section.url_name, + 'section': chapter.url_name, + 'subsection': section.url_name, 'position': position, }) @@ -813,34 +813,34 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): timeout=(settings.EDXNOTES_CONNECT_TIMEOUT, settings.EDXNOTES_READ_TIMEOUT) ) - def test_get_course_position_no_chapter(self): + def test_get_course_position_no_section(self): """ - Returns `None` if no chapter found. + Returns `None` if no section found. """ mock_course_block = MagicMock() mock_course_block.position = 3 mock_course_block.get_children.return_value = [] assert helpers.get_course_position(mock_course_block) is None - def test_get_course_position_to_chapter(self): + def test_get_course_position_to_section(self): """ - Returns a position that leads to COURSE/CHAPTER if this isn't the users's + Returns a position that leads to COURSE/SECTION if this isn't the users's first time. """ mock_course_block = MagicMock(id=self.course.id, position=3) - mock_chapter = MagicMock() - mock_chapter.url_name = 'chapter_url_name' - mock_chapter.display_name_with_default = 'Test Chapter Display Name' + mock_section = MagicMock() + mock_section.url_name = 'section_url_name' + mock_section.display_name_with_default = 'Test Chapter Display Name' - mock_course_block.get_children.return_value = [mock_chapter] + mock_course_block.get_children.return_value = [mock_section] assert helpers.get_course_position(mock_course_block) == { 'display_name': 'Test Chapter Display Name', - 'url': f'/courses/{self.course.id}/courseware/chapter_url_name/', + 'url': f'/courses/{self.course.id}/courseware/section_url_name/', } - def test_get_course_position_no_section(self): + def test_get_course_position_no_subsection(self): """ Returns `None` if no section found. """ @@ -848,27 +848,27 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): mock_course_block.get_children.return_value = [MagicMock()] assert helpers.get_course_position(mock_course_block) is None - def test_get_course_position_to_section(self): + def test_get_course_position_to_subsection(self): """ - Returns a position that leads to COURSE/CHAPTER/SECTION if this is the + Returns a position that leads to COURSE/SECTION/SUBSECTION if this is the user's first time. """ mock_course_block = MagicMock(id=self.course.id, position=None) - mock_chapter = MagicMock() - mock_chapter.url_name = 'chapter_url_name' - mock_course_block.get_children.return_value = [mock_chapter] - mock_section = MagicMock() mock_section.url_name = 'section_url_name' - mock_section.display_name_with_default = 'Test Section Display Name' + mock_course_block.get_children.return_value = [mock_section] - mock_chapter.get_children.return_value = [mock_section] - mock_section.get_children.return_value = [MagicMock()] + mock_subsection = MagicMock() + mock_subsection.url_name = 'subsection_url_name' + mock_subsection.display_name_with_default = 'Test Section Display Name' + + mock_section.get_children.return_value = [mock_subsection] + mock_subsection.get_children.return_value = [MagicMock()] assert helpers.get_course_position(mock_course_block) == { 'display_name': 'Test Section Display Name', - 'url': f'/courses/{self.course.id}/courseware/chapter_url_name/section_url_name/', + 'url': f'/courses/{self.course.id}/courseware/section_url_name/subsection_url_name/', } def test_get_index(self): diff --git a/lms/templates/courseware/accordion.html b/lms/templates/courseware/accordion.html deleted file mode 100644 index bfbff20731..0000000000 --- a/lms/templates/courseware/accordion.html +++ /dev/null @@ -1,94 +0,0 @@ -<%page expression_filter="h"/> -<%namespace name='static' file='../static_content.html'/> -<%! - from django.urls import reverse - from django.utils.translation import gettext as _ - from django.conf import settings - from openedx.core.djangolib.markup import HTML, Text -%> - -<%def name="make_chapter(chapter)"> -<% -if chapter.get('active'): - aria_label = _('{chapter} current chapter').format(chapter=chapter['display_name']) - active_class = 'active' -else: - aria_label = chapter['display_name'] - active_class = '' -%> - -
- -
- - -% for chapter in toc: - ${make_chapter(chapter)} -% endfor - - -% if toc: - <%static:require_module_async module_name="js/courseware/accordion_events" class_name="AccordionEvents"> - AccordionEvents(); - - - <%static:require_module_async module_name="js/dateutil_factory" class_name="DateUtilFactory"> - DateUtilFactory.transform(iterationKey=".localized-datetime"); - -% endif diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html deleted file mode 100644 index ce9dd19b29..0000000000 --- a/lms/templates/courseware/courseware.html +++ /dev/null @@ -1,318 +0,0 @@ -<%page expression_filter="h"/> -<%inherit file="/main.html" /> -<%namespace name='static' file='/static_content.html'/> -<%def name="online_help_token()"><% return "courseware" %> -<%! -import waffle - -from django.conf import settings -from django.urls import reverse -from django.utils.translation import gettext as _ - -from lms.djangoapps.edxnotes.helpers import is_feature_enabled as is_edxnotes_enabled -from openedx.core.djangolib.js_utils import js_escaped_string -from openedx.core.djangolib.markup import HTML -from openedx.features.course_experience import course_home_page_title, DISABLE_COURSE_OUTLINE_PAGE_FLAG -%> -<% - include_special_exams = ( - request.user.is_authenticated and - settings.FEATURES.get('ENABLE_SPECIAL_EXAMS', False) and - (course.enable_proctored_exams or course.enable_timed_exams) - ) - - completion_aggregator_url = getattr(settings, "COMPLETION_AGGREGATOR_URL", "") -%> - -<%def name="course_name()"> - <% return _("{course_number} Courseware").format(course_number=course.display_number_with_default) %> - - -<%block name="bodyclass">view-in-course view-courseware courseware ${course.css_class or ''} - -<%block name="title"> - - ${static.get_page_title_breadcrumbs(sequence_title, section_title, course_name())} - - - -<%block name="header_extras"> - -% for template_name in ["image-modal"]: - -% endfor - -% if include_special_exams is not UNDEFINED and include_special_exams: - % for template_name in ["proctored-exam-status"]: - - % endfor -% endif - - - -<%block name="headextra"> -<%static:css group='style-course-vendor'/> -<%static:css group='style-course'/> -## Utility: Notes -% if is_edxnotes_enabled(course, request.user): -<%static:css group='style-student-notes'/> -% endif - - - - - - ${HTML(fragment.head_html())} - - -<%block name="js_extra"> - - - - <%static:js group='courseware'/> - <%include file="/mathjax_include.html" args="disable_fast_preview=True"/> - - % if show_search: - <%static:require_module module_name="course_search/js/course_search_factory" class_name="CourseSearchFactory"> - var courseId = $('.courseware-results').data('courseId'); - CourseSearchFactory({ - courseId: courseId, - searchHeader: $('.search-bar') - }); - - % endif - - <%static:require_module module_name="js/courseware/courseware_factory" class_name="CoursewareFactory"> - CoursewareFactory(); - - - % if staff_access: - <%include file="xqa_interface.html"/> - % endif - - - - % if not request.user.is_authenticated: - - % endif - - - -${HTML(fragment.foot_html())} - - - -
- -% if default_tab: - <%include file="/courseware/course_navigation.html" /> -% else: - <%include file="/courseware/course_navigation.html" args="active_page='courseware'" /> -% endif - -
- -
- -% if course.show_calculator or is_edxnotes_enabled(course, request.user): - -% endif diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index 26f1bb59cb..711fad8954 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -187,7 +187,7 @@ username = get_enterprise_learner_generic_name(request) or student.username %endif

%else: - + ${ section.display_name} %if (total > 0 or earned > 0) and section.show_grades(staff_access): diff --git a/lms/urls.py b/lms/urls.py index 6cd51e94cf..092dcb6be9 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -482,21 +482,21 @@ urlpatterns += [ name='courseware', ), re_path( - r'^courses/{}/courseware/(?P[^/]*)/$'.format( - settings.COURSE_ID_PATTERN, - ), - CoursewareIndex.as_view(), - name='courseware_chapter', - ), - re_path( - r'^courses/{}/courseware/(?P[^/]*)/(?P
[^/]*)/$'.format( + r'^courses/{}/courseware/(?P
[^/]*)/$'.format( settings.COURSE_ID_PATTERN, ), CoursewareIndex.as_view(), name='courseware_section', ), re_path( - r'^courses/{}/courseware/(?P[^/]*)/(?P
[^/]*)/(?P[^/]*)/?$'.format( + r'^courses/{}/courseware/(?P
[^/]*)/(?P[^/]*)/$'.format( + settings.COURSE_ID_PATTERN, + ), + CoursewareIndex.as_view(), + name='courseware_subsection', + ), + re_path( + r'^courses/{}/courseware/(?P
[^/]*)/(?P[^/]*)/(?P[^/]*)/?$'.format( settings.COURSE_ID_PATTERN, ), CoursewareIndex.as_view(), diff --git a/openedx/features/course_experience/url_helpers.py b/openedx/features/course_experience/url_helpers.py index be8c02d2bd..44cfdb4aa1 100644 --- a/openedx/features/course_experience/url_helpers.py +++ b/openedx/features/course_experience/url_helpers.py @@ -10,13 +10,12 @@ from django.conf import settings from django.contrib.auth import get_user_model from django.http import HttpRequest from django.http.request import QueryDict -from django.urls import reverse from opaque_keys.edx.keys import CourseKey, UsageKey -from six.moves.urllib.parse import urlencode, urlparse +from urllib.parse import urlparse from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.search import navigation_index, path_to_location # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.search import path_to_location # lint-amnesty, pylint: disable=wrong-import-order User = get_user_model() @@ -44,47 +43,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, diff --git a/openedx/tests/xblock_integration/test_crowdsource_hinter.py b/openedx/tests/xblock_integration/test_crowdsource_hinter.py index 61f1097481..ecd3c09dd6 100644 --- a/openedx/tests/xblock_integration/test_crowdsource_hinter.py +++ b/openedx/tests/xblock_integration/test_crowdsource_hinter.py @@ -55,11 +55,11 @@ class TestCrowdsourceHinter(SharedModuleStoreTestCase, LoginEnrollmentTestCase): ) cls.course_url = reverse( - 'courseware_section', + 'courseware_subsection', kwargs={ 'course_id': str(cls.course.id), - 'chapter': 'Overview', - 'section': 'Welcome', + 'section': 'Overview', + 'subsection': 'Welcome', } )