-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat!: Remove Legacy Preview Functionality #36460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6ac0eb5
6b70b85
f128ee9
ab136ba
168f1b1
beb9f23
4865cab
ab4a81e
3f80924
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,8 +49,10 @@ | |
| CATALOG_VISIBILITY_CATALOG_AND_ABOUT, | ||
| CATALOG_VISIBILITY_NONE | ||
| ) | ||
|
|
||
| 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.exceptions import ItemNotFoundError | ||
| from xmodule.modulestore.tests.django_utils import ( # lint-amnesty, pylint: disable=wrong-import-order | ||
| ModuleStoreTestCase, | ||
| SharedModuleStoreTestCase | ||
|
|
@@ -245,46 +247,64 @@ def test_student_has_access(self): | |
| course_key = self.course.id | ||
| chapter = BlockFactory.create(category="chapter", parent_location=self.course.location) | ||
| overview = CourseOverview.get_from_id(course_key) | ||
| subsection = BlockFactory.create(category="sequential", parent_location=chapter.location) | ||
| unit = BlockFactory.create(category="vertical", parent_location=subsection.location) | ||
|
|
||
| with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): | ||
| html_block = BlockFactory.create( | ||
| category="html", | ||
| parent_location=unit.location, | ||
| display_name="Unpublished Block", | ||
| data='<p>This block should not be published.</p>', | ||
| publish_item=False, | ||
| ) | ||
|
|
||
| # Enroll student to the course | ||
| CourseEnrollmentFactory(user=self.student, course_id=self.course.id) | ||
|
|
||
| modules = [ | ||
| self.course, | ||
| overview, | ||
| chapter, | ||
| overview, | ||
| subsection, | ||
| unit, | ||
|
|
||
| ] | ||
| with patch('lms.djangoapps.courseware.access.in_preview_mode') as mock_preview: | ||
| mock_preview.return_value = False | ||
| for obj in modules: | ||
| assert bool(access.has_access(self.student, 'load', obj, course_key=self.course.id)) | ||
| # Student should have access to modules they're enrolled in | ||
| for obj in modules: | ||
| assert bool(access.has_access( | ||
| self.student, | ||
| 'load', | ||
| self.store.get_item(obj.location), | ||
| course_key=self.course.id) | ||
| ) | ||
|
|
||
| with patch('lms.djangoapps.courseware.access.in_preview_mode') as mock_preview: | ||
| mock_preview.return_value = True | ||
| for obj in modules: | ||
| assert not bool(access.has_access(self.student, 'load', obj, course_key=self.course.id)) | ||
| # If the document is not published yet, it should return an error when we try to fetch | ||
| # it from the store. This check confirms that the student would not be able to access it. | ||
| with pytest.raises(ItemNotFoundError): | ||
| self.store.get_item(html_block.location) | ||
|
|
||
| @patch('lms.djangoapps.courseware.access.in_preview_mode', Mock(return_value=True)) | ||
| def test_has_access_with_preview_mode(self): | ||
| def test_has_access_based_on_roles(self): | ||
| """ | ||
| Tests particular user's can access content via has_access in preview mode. | ||
| Tests user access to content based on their role. | ||
| """ | ||
| assert bool(access.has_access(self.global_staff, 'staff', self.course, course_key=self.course.id)) | ||
| assert bool(access.has_access(self.course_staff, 'staff', self.course, course_key=self.course.id)) | ||
| assert bool(access.has_access(self.course_instructor, 'staff', self.course, course_key=self.course.id)) | ||
| assert not bool(access.has_access(self.student, 'staff', self.course, course_key=self.course.id)) | ||
| assert not bool(access.has_access(self.student, 'load', self.course, course_key=self.course.id)) | ||
|
|
||
| # Student should be able to load the course even if they don't have staff access. | ||
| assert bool(access.has_access(self.student, 'load', self.course, course_key=self.course.id)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did this behavior change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously this test was being run with the preview domain where the student would indeed not have access. Now that the test is being run on the default domain, the student does have access again. |
||
|
|
||
| # When masquerading is true, user should not be able to access staff content | ||
| with patch('lms.djangoapps.courseware.access.is_masquerading_as_student') as mock_masquerade: | ||
| mock_masquerade.return_value = True | ||
| assert not bool(access.has_access(self.global_staff, 'staff', self.course, course_key=self.course.id)) | ||
| assert not bool(access.has_access(self.student, 'staff', self.course, course_key=self.course.id)) | ||
|
|
||
| @patch('lms.djangoapps.courseware.access_utils.in_preview_mode', Mock(return_value=True)) | ||
| def test_has_access_in_preview_mode_with_group(self): | ||
| def test_has_access_with_content_groups(self): | ||
| """ | ||
| Test that a user masquerading as a member of a group sees appropriate content in preview mode. | ||
| Test that a user masquerading as a member of a group sees appropriate content. | ||
| """ | ||
| # Note about UserPartition and UserPartition Group IDs: these must not conflict with IDs used | ||
| # by dynamic user partitions. | ||
|
|
@@ -423,35 +443,18 @@ def test__has_access_to_block_beta_user(self): | |
|
|
||
| assert bool(access._has_access_to_block(self.beta_user, 'load', mock_unit, course_key=self.course.id)) | ||
|
|
||
| @ddt.data(None, YESTERDAY, TOMORROW) | ||
| @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) | ||
| @patch( | ||
| 'lms.djangoapps.courseware.access_utils.get_current_request_hostname', | ||
| Mock(return_value='preview.localhost') | ||
| ) | ||
| def test__has_access_to_block_in_preview_mode(self, start): | ||
| """ | ||
| Tests that block has access in preview mode. | ||
| """ | ||
| mock_unit = Mock(location=self.course.location, user_partitions=[]) | ||
| mock_unit._class_tags = {} # Needed for detached check in _has_access_to_block | ||
| mock_unit.visible_to_staff_only = False | ||
| mock_unit.start = self.DATES[start] | ||
| mock_unit.merged_group_access = {} | ||
|
|
||
| self.verify_access(mock_unit, True) | ||
|
|
||
| @ddt.data( | ||
| (TOMORROW, access_response.StartDateError), | ||
| (None, None), | ||
| (YESTERDAY, None) | ||
| ) # ddt throws an error if I don't put the None argument there | ||
| @ddt.unpack | ||
| @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) | ||
| @patch('lms.djangoapps.courseware.access_utils.get_current_request_hostname', Mock(return_value='localhost')) | ||
| def test__has_access_to_block_when_not_in_preview_mode(self, start, expected_error_type): | ||
| def test__has_access_to_block_with_start_date(self, start, expected_error_type): | ||
| """ | ||
| Tests that block has no access when start date in future & without preview. | ||
| Tests that block access follows start date rules. | ||
| Access should be denied when start date is in the future and granted when | ||
| start date is in the past or not set. | ||
| """ | ||
| expected_access = expected_error_type is None | ||
| mock_unit = Mock(location=self.course.location, user_partitions=[]) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,7 @@ | |
| ) | ||
|
|
||
| from lms.djangoapps.courseware.tests.helpers import ( | ||
| LoginEnrollmentTestCase, MasqueradeMixin, masquerade_as_group_member, set_preview_mode, | ||
| LoginEnrollmentTestCase, MasqueradeMixin, masquerade_as_group_member | ||
| ) | ||
| from lms.djangoapps.courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin | ||
| from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY | ||
|
|
@@ -244,14 +244,20 @@ def testMasqueradeCohortAvailable(self, target, expected): | |
| assert is_target_available == expected | ||
|
|
||
|
|
||
| @set_preview_mode(True) | ||
| # 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a comment explaining why this patch exists here.
feanil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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): | ||
| def test_staff_debug_with_masquerade(self, mock_redirect): | ||
| """ | ||
| Tests that staff debug control is not visible when masquerading as a student. | ||
| """ | ||
|
|
@@ -267,7 +273,7 @@ def test_staff_debug_with_masquerade(self): | |
| self.verify_staff_debug_present(True) | ||
|
|
||
| @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) | ||
| def test_show_answer_for_staff(self): | ||
| def test_show_answer_for_staff(self, mock_redirect): | ||
| """ | ||
| Tests that "Show Answer" is not visible when masquerading as a student. | ||
| """ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an equivalent test to make sure that students cannot load the preview/draft state of unreleased content if they know the correct URL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to add a check that a student will not have access to the unpublished content when this is not published yet.