<%- gettext("Staff and Learners") %>
- - <%- gettext("Access to some content in this unit is restricted to specific groups of learners.") %> -
- <% if (xblockInfo.isVertical()) { %> - <%- gettext('If the unit was previously published and released to learners, any changes you made to the unit when it was hidden will now be visible to learners.') %> - <% } else { %> - <% var message = gettext('If you make this %(xblockType)s visible to learners, learners will be able to see its content after the release date has passed and you have published the unit. Only units that are explicitly hidden from learners will remain hidden after you clear this option for the %(xblockType)s.') %> - <%- interpolate(message, { xblockType: xblockType }, true) %> - <% } %> -
+ <% if (xblockInfo.isVertical()) { %> + <%- gettext('If the unit was previously published and released to learners, any changes you made to the unit when it was hidden will now be visible to learners.') %> + <% } else { %> + <% var message = gettext('If you make this %(xblockType)s visible to learners, learners will be able to see its content after the release date has passed and you have published the unit. Only units that are explicitly hidden from learners will remain hidden after you clear this option for the %(xblockType)s.') %> + <%- interpolate(message, { xblockType: xblockType }, true) %> + <% } %> +
tags to have been removed @@ -315,14 +315,15 @@ class EditContainerTest(NestedVerticalTest): class BaseGroupConfigurationsTest(ContainerBase): - ALL_LEARNERS_AND_STAFF = ComponentVisibilityEditorView.ALL_LEARNERS_AND_STAFF + ALL_LEARNERS_AND_STAFF = XBlockVisibilityEditorView.ALL_LEARNERS_AND_STAFF CHOOSE_ONE = "Select a group type" - CONTENT_GROUP_PARTITION = ComponentVisibilityEditorView.CONTENT_GROUP_PARTITION - ENROLLMENT_TRACK_PARTITION = ComponentVisibilityEditorView.ENROLLMENT_TRACK_PARTITION + CONTENT_GROUP_PARTITION = XBlockVisibilityEditorView.CONTENT_GROUP_PARTITION + ENROLLMENT_TRACK_PARTITION = XBlockVisibilityEditorView.ENROLLMENT_TRACK_PARTITION MISSING_GROUP_LABEL = 'Deleted Group\nThis group no longer exists. Choose another group or do not restrict access to this component.' VALIDATION_ERROR_LABEL = 'This component has validation issues.' VALIDATION_ERROR_MESSAGE = "Error:\nThis component's access settings refer to deleted or invalid groups." GROUP_VISIBILITY_MESSAGE = 'Access to some content in this unit is restricted to specific groups of learners.' + MODAL_NOT_RESTRICTED_MESSAGE = "Access is not restricted" def setUp(self): super(BaseGroupConfigurationsTest, self).setUp() @@ -365,10 +366,17 @@ class BaseGroupConfigurationsTest(ContainerBase): def edit_component_visibility(self, component): """ - Edit the visibility of an xblock on the container page. + Edit the visibility of an xblock on the container page and returns an XBlockVisibilityEditorView. """ component.edit_visibility() - return ComponentVisibilityEditorView(self.browser, component.locator) + return XBlockVisibilityEditorView(self.browser, component.locator) + + def edit_unit_visibility(self, unit): + """ + Edit the visibility of a unit on the container page and returns an XBlockVisibilityEditorView. + """ + unit.edit_visibility() + return XBlockVisibilityEditorView(self.browser, unit.locator) def verify_current_groups_message(self, visibility_editor, expected_current_groups): """ @@ -402,6 +410,7 @@ class BaseGroupConfigurationsTest(ContainerBase): """ # Make initial edit(s) and save visibility_editor = self.edit_component_visibility(component) + visibility_editor.select_groups_in_partition_scheme(partition_label, groups) # Re-open the modal and inspect its selected inputs. If no groups were selected, @@ -413,6 +422,22 @@ class BaseGroupConfigurationsTest(ContainerBase): self.verify_selected_groups(visibility_editor, groups) visibility_editor.save() + def select_and_verify_unit_group_access(self, unit, partition_label, groups=[]): + """ + Edit the visibility of an xblock on the unit page and + verify that the edit persists. Note that `groups` + are labels which should be clicked, but are not necessarily checked. + """ + unit_access_editor = self.edit_unit_visibility(unit) + unit_access_editor.select_groups_in_partition_scheme(partition_label, groups) + + if not groups: + partition_label = self.CHOOSE_ONE + unit_access_editor = self.edit_unit_visibility(unit) + self.verify_selected_partition_scheme(unit_access_editor, partition_label) + self.verify_selected_groups(unit_access_editor, groups) + unit_access_editor.save() + def verify_component_validation_error(self, component): """ Verify that we see validation errors for the given component. @@ -434,6 +459,19 @@ class BaseGroupConfigurationsTest(ContainerBase): self.assertNotIn(self.GROUP_VISIBILITY_MESSAGE, self.container_page.sidebar_visibility_message) self.assertFalse(component.has_group_visibility_set) + def verify_unit_visibility_set(self, unit, set_groups=[]): + """ + Verify that the container visibility modal shows that unit visibility + settings have been edited if there are `set_groups`. Otherwise verify + that the modal shows no such information. + """ + unit_access_editor = self.edit_unit_visibility(unit) + if set_groups: + self.assertIn(", ".join(set_groups), unit_access_editor.current_groups_message) + else: + self.assertEqual(self.MODAL_NOT_RESTRICTED_MESSAGE, unit_access_editor.current_groups_message) + unit_access_editor.cancel() + def update_component(self, component, metadata): """ Update a component's metadata and refresh the page. @@ -458,6 +496,59 @@ class BaseGroupConfigurationsTest(ContainerBase): self.assertFalse(component.has_validation_error) +class UnitAccessContainerTest(BaseGroupConfigurationsTest): + GROUP_RESTRICTED_MESSAGE = 'Access to this unit is restricted to: Dogs' + + def _toggle_container_unit_access(self, group_ids, unit): + """ + Toggle the unit level access on the course outline page + """ + unit.toggle_unit_access('Content Groups', group_ids) + + def _verify_container_unit_access_message(self, group_ids, expected_message): + """ + Check that the container page displays the correct unit + access message. + """ + self.outline.visit() + self.outline.expand_all_subsections() + unit = self.outline.section_at(0).subsection_at(0).unit_at(0) + self._toggle_container_unit_access(group_ids, unit) + + container_page = self.go_to_unit_page() + self.assertEqual(str(container_page.get_xblock_access_message()), expected_message) + + def test_default_selection(self): + """ + Tests that no message is displayed when there are no + restrictions on the unit or components. + """ + self._verify_container_unit_access_message([], '') + + def test_restricted_components_message(self): + """ + Test that the proper message is displayed when access to + some components is restricted. + """ + container_page = self.go_to_unit_page() + html_component = container_page.xblocks[1] + + # Initially set visibility to Dog group. + self.update_component( + html_component, + {'group_access': {self.id_base: [self.id_base + 1]}} + ) + + self._verify_container_unit_access_message([], self.GROUP_VISIBILITY_MESSAGE) + + def test_restricted_access_message(self): + """ + Test that the proper message is displayed when access to the + unit is restricted to a particular group. + """ + self._verify_container_unit_access_message([self.id_base + 1], self.GROUP_RESTRICTED_MESSAGE) + + @attr(shard=3) class ContentGroupVisibilityModalTest(BaseGroupConfigurationsTest): """ @@ -484,21 +575,36 @@ class ContentGroupVisibilityModalTest(BaseGroupConfigurationsTest): Scenario: The component visibility modal can be set to be visible to all students and staff. Given I have a unit with one component When I go to the container page for that unit - And I open the visibility editor modal for that unit's component - And I select 'Dogs' - And I save the modal - Then the container page should display the content visibility warning - And I re-open the visibility editor modal for that unit's component + Then the container page should not display the content visibility warning by default. + If I then restrict access and save, and then I open the visibility editor modal for that unit's component And I select 'All Students and Staff' And I save the modal Then the visibility selection should be 'All Students and Staff' - And the container page should not display the content visibility warning + And the container page should still not display the content visibility warning """ self.select_and_verify_saved(self.html_component, self.CONTENT_GROUP_PARTITION, ['Dogs']) - self.verify_visibility_set(self.html_component, True) self.select_and_verify_saved(self.html_component, self.ALL_LEARNERS_AND_STAFF) self.verify_visibility_set(self.html_component, False) + def test_reset_unit_access_to_all_students_and_staff(self): + """ + Scenario: The unit visibility modal can be set to be visible to all students and staff. + Given I have a unit + When I go to the container page for that unit + And I open the visibility editor modal for that unit + And I select 'Dogs' + And I save the modal + Then I re-open the modal, the unit access modal should display the content visibility settings + Then after re-opening the modal again + And I select 'All Learners and Staff' + And I save the modal + And I re-open the modal, the unit access modal should display that no content is restricted + """ + self.select_and_verify_unit_group_access(self.container_page, self.CONTENT_GROUP_PARTITION, ['Dogs']) + self.verify_unit_visibility_set(self.container_page, set_groups=["Dogs"]) + self.select_and_verify_unit_group_access(self.container_page, self.ALL_LEARNERS_AND_STAFF) + self.verify_unit_visibility_set(self.container_page) + def test_select_single_content_group(self): """ Scenario: The component visibility modal can be set to be visible to one content group. @@ -508,10 +614,8 @@ class ContentGroupVisibilityModalTest(BaseGroupConfigurationsTest): And I select 'Dogs' And I save the modal Then the visibility selection should be 'Dogs' and 'Specific Content Groups' - And the container page should display the content visibility warning """ self.select_and_verify_saved(self.html_component, self.CONTENT_GROUP_PARTITION, ['Dogs']) - self.verify_visibility_set(self.html_component, True) def test_select_multiple_content_groups(self): """ @@ -522,10 +626,8 @@ class ContentGroupVisibilityModalTest(BaseGroupConfigurationsTest): And I select 'Dogs' and 'Cats' And I save the modal Then the visibility selection should be 'Dogs', 'Cats', and 'Specific Content Groups' - And the container page should display the content visibility warning """ self.select_and_verify_saved(self.html_component, self.CONTENT_GROUP_PARTITION, ['Dogs', 'Cats']) - self.verify_visibility_set(self.html_component, True) def test_select_zero_content_groups(self): """ @@ -581,12 +683,10 @@ class ContentGroupVisibilityModalTest(BaseGroupConfigurationsTest): Then I should see a validation error message on that unit's component And I open the visibility editor modal for that unit's component Then I should see that I have selected multiple deleted groups - And the container page should display the content visibility warning - And I de-select the missing groups + And then if I de-select the missing groups And I save the modal Then the visibility selection should be the names of the valid groups. And I should not see any validation errors on the component - And the container page should display the content visibility warning """ self.update_component( self.html_component, @@ -603,7 +703,6 @@ class ContentGroupVisibilityModalTest(BaseGroupConfigurationsTest): expected_groups = ['Dogs', 'Cats'] self.verify_current_groups_message(visibility_editor, ", ".join(expected_groups)) self.verify_selected_groups(visibility_editor, expected_groups) - self.verify_visibility_set(self.html_component, True) def _verify_and_remove_missing_content_groups(self, current_groups_message, all_group_labels): self.verify_component_validation_error(self.html_component) @@ -1000,7 +1099,7 @@ class UnitPublishingTest(ContainerBase): unit = self.go_to_unit_page() component = unit.xblocks[1] component.edit() - HtmlComponentEditorView(self.browser, component.locator).set_content_and_save(modified_content) + HtmlXBlockEditorView(self.browser, component.locator).set_content_and_save(modified_content) self.assertEqual(component.student_content, modified_content) unit.verify_publish_title(self.DRAFT_STATUS) unit.publish_action.click() @@ -1023,7 +1122,7 @@ class UnitPublishingTest(ContainerBase): unit = self.go_to_unit_page() component = unit.xblocks[1] component.edit() - HtmlComponentEditorView(self.browser, component.locator).set_content_and_cancel("modified content") + HtmlXBlockEditorView(self.browser, component.locator).set_content_and_cancel("modified content") self.assertEqual(component.student_content, "Body of HTML Unit.") unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self.browser.refresh() diff --git a/common/test/acceptance/tests/studio/test_studio_outline.py b/common/test/acceptance/tests/studio/test_studio_outline.py index bf477071b0..207f2878dd 100644 --- a/common/test/acceptance/tests/studio/test_studio_outline.py +++ b/common/test/acceptance/tests/studio/test_studio_outline.py @@ -12,11 +12,14 @@ from pytz import UTC from base_studio_test import StudioCourseTest from common.test.acceptance.fixtures.config import ConfigModelFixture from common.test.acceptance.fixtures.course import XBlockFixtureDesc +from common.test.acceptance.pages.common.utils import add_enrollment_course_modes from common.test.acceptance.pages.lms.course_home import CourseHomePage from common.test.acceptance.pages.lms.courseware import CoursewarePage from common.test.acceptance.pages.lms.progress import ProgressPage +from common.test.acceptance.pages.lms.staff_view import StaffCoursewarePage from common.test.acceptance.pages.studio.overview import ContainerPage, CourseOutlinePage, ExpandCollapseLinkState from common.test.acceptance.pages.studio.settings_advanced import AdvancedSettingsPage +from common.test.acceptance.pages.studio.settings_group_configurations import GroupConfigurationsPage from common.test.acceptance.pages.studio.utils import add_discussion, drag, verify_ordering from common.test.acceptance.tests.helpers import disable_animations, load_data_str @@ -491,6 +494,152 @@ class EditingSectionsTest(CourseOutlineTest): self.assertIn(release_text, self.course_outline_page.section_at(0).subsection_at(0).release_date) +@attr(shard=3) +class UnitAccessTest(CourseOutlineTest): + """ + Feature: Units can be restricted and unrestricted to certain groups from the course outline. + """ + + __test__ = True + + def setUp(self): + super(UnitAccessTest, self).setUp() + self.group_configurations_page = GroupConfigurationsPage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + self.content_group_a = "Test Group A" + self.content_group_b = "Test Group B" + + self.group_configurations_page.visit() + self.group_configurations_page.create_first_content_group() + config_a = self.group_configurations_page.content_groups[0] + config_a.name = self.content_group_a + config_a.save() + self.content_group_a_id = config_a.id + + self.group_configurations_page.add_content_group() + config_b = self.group_configurations_page.content_groups[1] + config_b.name = self.content_group_b + config_b.save() + self.content_group_b_id = config_b.id + + def populate_course_fixture(self, course_fixture): + """ + Create a course with one section, one subsection, and two units + """ + # with collapsed outline + self.chap_1_handle = 0 + self.chap_1_seq_1_handle = 1 + + # with first sequential expanded + self.seq_1_vert_1_handle = 2 + self.seq_1_vert_2_handle = 3 + self.chap_1_seq_2_handle = 4 + + course_fixture.add_children( + XBlockFixtureDesc('chapter', "1").add_children( + XBlockFixtureDesc('sequential', '1.1').add_children( + XBlockFixtureDesc('vertical', '1.1.1'), + XBlockFixtureDesc('vertical', '1.1.2') + ) + ) + ) + + def _set_restriction_on_unrestricted_unit(self, unit): + """ + Restrict unit access to a certain group and confirm that a + warning is displayed. Then, remove the access restriction + and verify that the warning no longer appears. + """ + self.assertFalse(unit.has_restricted_warning) + unit.toggle_unit_access('Content Groups', [self.content_group_a_id]) + self.assertTrue(unit.has_restricted_warning) + unit.toggle_unit_access('Content Groups', [self.content_group_a_id]) + self.assertFalse(unit.has_restricted_warning) + + def test_units_can_be_restricted(self): + """ + Visit the course outline page, restrict access to a unit. + Verify that there is a restricted group warning. + Remove the group access restriction and verify that there + is no longer a warning. + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0) + self._set_restriction_on_unrestricted_unit(unit) + + def test_restricted_sections_for_content_group_users_in_lms(self): + """ + Verify that those who are in an content track with access to a restricted unit are able + to see that unit in lms, and those who are in an enrollment track without access to a restricted + unit are not able to see that unit in lms + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0) + unit.toggle_unit_access('Content Groups', [self.content_group_a_id]) + self.course_outline_page.view_live() + + course_home_page = CourseHomePage(self.browser, self.course_id) + course_home_page.visit() + course_home_page.resume_course_from_header() + self.assertEqual(course_home_page.outline.num_units, 2) + + # Test for a user without additional content available + staff_page = StaffCoursewarePage(self.browser, self.course_id) + staff_page.set_staff_view_mode('Learner in Test Group B') + staff_page.wait_for_page() + self.assertEqual(course_home_page.outline.num_units, 1) + + # Test for a user with additional content available + staff_page.set_staff_view_mode('Learner in Test Group A') + staff_page.wait_for_page() + self.assertEqual(course_home_page.outline.num_units, 2) + + def test_restricted_sections_for_enrollment_track_users_in_lms(self): + """ + Verify that those who are in an enrollment track with access to a restricted unit are able + to see that unit in lms, and those who are in an enrollment track without access to a restricted + unit are not able to see that unit in lms + """ + # Add just 1 enrollment track to verify the enrollment option isn't available in the modal + add_enrollment_course_modes(self.browser, self.course_id, ["audit"]) + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0) + enrollment_select_options = unit.get_enrollment_select_options() + self.assertFalse('Enrollment Track Groups' in enrollment_select_options) + + # Add the additional enrollment track so the unit access toggles should now be available + add_enrollment_course_modes(self.browser, self.course_id, ["verified"]) + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0) + unit.toggle_unit_access('Enrollment Track Groups', [1]) # Hard coded 1 for audit ID + self.course_outline_page.view_live() + + course_home_page = CourseHomePage(self.browser, self.course_id) + course_home_page.visit() + course_home_page.resume_course_from_header() + self.assertEqual(course_home_page.outline.num_units, 2) + + # Test for a user without additional content available + staff_page = StaffCoursewarePage(self.browser, self.course_id) + staff_page.set_staff_view_mode('Learner in Verified') + staff_page.wait_for_page() + self.assertEqual(course_home_page.outline.num_units, 1) + + # Test for a user with additional content available + staff_page = StaffCoursewarePage(self.browser, self.course_id) + staff_page.set_staff_view_mode('Learner in Audit') + staff_page.wait_for_page() + self.assertEqual(course_home_page.outline.num_units, 2) + + @attr(shard=3) class StaffLockTest(CourseOutlineTest): """ diff --git a/common/test/acceptance/tests/studio/test_studio_split_test.py b/common/test/acceptance/tests/studio/test_studio_split_test.py index 47eaef2886..b5427f1e43 100644 --- a/common/test/acceptance/tests/studio/test_studio_split_test.py +++ b/common/test/acceptance/tests/studio/test_studio_split_test.py @@ -12,11 +12,11 @@ from selenium.webdriver.support.ui import Select from base_studio_test import StudioCourseTest from common.test.acceptance.fixtures.course import XBlockFixtureDesc from common.test.acceptance.pages.lms.courseware import CoursewarePage -from common.test.acceptance.pages.studio.component_editor import ComponentEditorView from common.test.acceptance.pages.studio.container import ContainerPage from common.test.acceptance.pages.studio.overview import CourseOutlinePage, CourseOutlineUnit from common.test.acceptance.pages.studio.settings_group_configurations import GroupConfigurationsPage from common.test.acceptance.pages.studio.utils import add_advanced_component +from common.test.acceptance.pages.studio.xblock_editor import XBlockEditorView from common.test.acceptance.pages.xblock.utils import wait_for_xblock_initialization from common.test.acceptance.tests.helpers import create_user_partition_json from test_studio_container import ContainerBase @@ -126,7 +126,7 @@ class SplitTest(ContainerBase, SplitTestMixin): add_advanced_component(unit, 0, 'split_test') container = self.go_to_nested_container_page() container.edit() - component_editor = ComponentEditorView(self.browser, container.locator) + component_editor = XBlockEditorView(self.browser, container.locator) component_editor.set_select_value_and_save('Group Configuration', 'Configuration alpha,beta') self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { @@ -151,7 +151,7 @@ class SplitTest(ContainerBase, SplitTestMixin): add_advanced_component(unit, 0, 'split_test') container = self.go_to_nested_container_page() container.edit() - component_editor = ComponentEditorView(self.browser, container.locator) + component_editor = XBlockEditorView(self.browser, container.locator) component_editor.set_select_value_and_save('Group Configuration', 'Configuration alpha,beta') self.verify_groups(container, ['alpha', 'beta'], []) @@ -159,7 +159,7 @@ class SplitTest(ContainerBase, SplitTestMixin): # that there is only a single "editor" on the page. container = self.go_to_nested_container_page() container.edit() - component_editor = ComponentEditorView(self.browser, container.locator) + component_editor = XBlockEditorView(self.browser, container.locator) component_editor.set_select_value_and_save('Group Configuration', 'Configuration 0,1,2') self.verify_groups(container, ['Group 0', 'Group 1', 'Group 2'], ['Group ID 0', 'Group ID 1']) @@ -537,7 +537,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): container = ContainerPage(self.browser, split_test.locator) container.visit() container.edit() - component_editor = ComponentEditorView(self.browser, container.locator) + component_editor = XBlockEditorView(self.browser, container.locator) component_editor.set_select_value_and_save('Group Configuration', 'New Group Configuration Name') self.verify_groups(container, ['Group A', 'Group B', 'New group'], []) @@ -589,7 +589,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): container = ContainerPage(self.browser, split_test.locator) container.visit() container.edit() - component_editor = ComponentEditorView(self.browser, container.locator) + component_editor = XBlockEditorView(self.browser, container.locator) self.assertEqual( "Second Group Configuration Name", component_editor.get_selected_option_text('Group Configuration') diff --git a/common/test/acceptance/tests/test_cohorted_courseware.py b/common/test/acceptance/tests/test_cohorted_courseware.py index 721dfeadba..1eba04a0c8 100644 --- a/common/test/acceptance/tests/test_cohorted_courseware.py +++ b/common/test/acceptance/tests/test_cohorted_courseware.py @@ -10,8 +10,8 @@ from common.test.acceptance.pages.common.auto_auth import AutoAuthPage from common.test.acceptance.pages.common.utils import add_enrollment_course_modes, enroll_user_track from common.test.acceptance.pages.lms.courseware import CoursewarePage from common.test.acceptance.pages.lms.instructor_dashboard import InstructorDashboardPage -from common.test.acceptance.pages.studio.component_editor import ComponentVisibilityEditorView from common.test.acceptance.pages.studio.settings_group_configurations import GroupConfigurationsPage +from common.test.acceptance.pages.studio.xblock_editor import XBlockVisibilityEditorView from common.test.acceptance.tests.discussion.helpers import CohortTestMixin from common.test.acceptance.tests.lms.test_lms_user_preview import verify_expected_problem_visibility from studio.base_studio_test import ContainerBase @@ -144,7 +144,7 @@ class EndToEndCohortedCoursewareTest(ContainerBase, CohortTestMixin): def set_visibility(problem_index, groups, group_partition='content_group'): problem = container_page.xblocks[problem_index] problem.edit_visibility() - visibility_dialog = ComponentVisibilityEditorView(self.browser, problem.locator) + visibility_dialog = XBlockVisibilityEditorView(self.browser, problem.locator) partition_name = (visibility_dialog.ENROLLMENT_TRACK_PARTITION if group_partition == enrollment_group else visibility_dialog.CONTENT_GROUP_PARTITION) diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index 7374cc2fb3..04dbb1ffbb 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/lms/djangoapps/edxnotes/helpers.py @@ -23,6 +23,7 @@ from courseware.access import has_access from courseware.courses import get_current_child from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable from edxnotes.plugins import EdxNotesTab +from lms.lib.utils import get_parent_unit from openedx.core.lib.token_utils import JwtBuilder from student.models import anonymous_id_for_user from util.date_utils import get_default_time_display @@ -120,21 +121,6 @@ def send_request(user, course_id, page, page_size, path="", text=None): return response -def get_parent_unit(xblock): - """ - Find vertical that is a unit, not just some container. - """ - while xblock: - xblock = xblock.get_parent() - if xblock is None: - return None - parent = xblock.get_parent() - if parent is None: - return None - if parent.category == 'sequential': - return xblock - - def preprocess_collection(user, course, collection): """ Prepare `collection(notes_list)` provided by edx-notes-api diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index fe713c0421..dfc8e28139 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -691,21 +691,6 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): helpers.preprocess_collection(self.user, self.course, initial_collection) ) - def test_get_parent_unit(self): - """ - Tests `get_parent_unit` method for the successful result. - """ - parent = helpers.get_parent_unit(self.html_module_1) - self.assertEqual(parent.location, self.vertical.location) - - parent = helpers.get_parent_unit(self.child_html_module) - self.assertEqual(parent.location, self.vertical_with_container.location) - - self.assertIsNone(helpers.get_parent_unit(None)) - self.assertIsNone(helpers.get_parent_unit(self.course)) - self.assertIsNone(helpers.get_parent_unit(self.chapter)) - self.assertIsNone(helpers.get_parent_unit(self.sequential)) - def test_get_module_context_sequential(self): """ Tests `get_module_context` method for the sequential. diff --git a/lms/djangoapps/lms_xblock/mixin.py b/lms/djangoapps/lms_xblock/mixin.py index 2e850088a6..cc1082c56b 100644 --- a/lms/djangoapps/lms_xblock/mixin.py +++ b/lms/djangoapps/lms_xblock/mixin.py @@ -17,6 +17,7 @@ _ = lambda text: text INVALID_USER_PARTITION_VALIDATION = _(u"This component's access settings refer to deleted or invalid group configurations.") INVALID_USER_PARTITION_GROUP_VALIDATION = _(u"This component's access settings refer to deleted or invalid groups.") +NONSENSICAL_ACCESS_RESTRICTION = _(u"This component's access settings contradict its parent's access settings.") class GroupAccessDict(Dict): @@ -142,6 +143,40 @@ class LmsBlockMixin(XBlockMixin): raise NoSuchUserPartitionError("could not find a UserPartition with ID [{}]".format(user_partition_id)) + def _has_nonsensical_access_settings(self): + """ + Checks if a block's group access settings do not make sense. + + By nonsensical access settings, we mean a component's access + settings which contradict its parent's access in that they + restrict access to the component to a group that already + will not be able to see that content. + Note: This contradiction can occur when a component + restricts access to the same partition but a different group + than its parent, or when there is a parent access + restriction but the component attempts to allow access to + all learners. + + Returns: + bool: True if the block's access settings contradict its + parent's access settings. + """ + parent = self.get_parent() + if not parent: + return False + + parent_group_access = parent.group_access + component_group_access = self.group_access + + for user_partition_id, parent_group_ids in parent_group_access.iteritems(): + component_group_ids = component_group_access.get(user_partition_id) + if component_group_ids: + return parent_group_ids and not set(component_group_ids).issubset(set(parent_group_ids)) + else: + return not component_group_access + else: + return False + def validate(self): """ Validates the state of this xblock instance. @@ -150,6 +185,7 @@ class LmsBlockMixin(XBlockMixin): validation = super(LmsBlockMixin, self).validate() has_invalid_user_partitions = False has_invalid_groups = False + for user_partition_id, group_ids in self.group_access.iteritems(): try: user_partition = self._get_user_partition(user_partition_id) @@ -171,6 +207,7 @@ class LmsBlockMixin(XBlockMixin): INVALID_USER_PARTITION_VALIDATION ) ) + if has_invalid_groups: validation.add( ValidationMessage( @@ -178,4 +215,13 @@ class LmsBlockMixin(XBlockMixin): INVALID_USER_PARTITION_GROUP_VALIDATION ) ) + + if self._has_nonsensical_access_settings(): + validation.add( + ValidationMessage( + ValidationMessage.ERROR, + NONSENSICAL_ACCESS_RESTRICTION + ) + ) + return validation diff --git a/lms/lib/tests/test_utils.py b/lms/lib/tests/test_utils.py new file mode 100644 index 0000000000..4054ee979e --- /dev/null +++ b/lms/lib/tests/test_utils.py @@ -0,0 +1,59 @@ +""" +Tests for the LMS/lib utils +""" +from lms.lib import utils +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +class LmsUtilsTest(ModuleStoreTestCase): + """ + Tests for the LMS utility functions + """ + def setUp(self): + """ + Setup a dummy course content. + """ + super(LmsUtilsTest, self).setUp() + + with self.store.default_store(ModuleStoreEnum.Type.mongo): + self.course = CourseFactory.create() + self.chapter = ItemFactory.create(category="chapter", parent_location=self.course.location) + self.sequential = ItemFactory.create(category="sequential", parent_location=self.chapter.location) + self.vertical = ItemFactory.create(category="vertical", parent_location=self.sequential.location) + self.html_module_1 = ItemFactory.create(category="html", parent_location=self.vertical.location) + self.vertical_with_container = ItemFactory.create( + category="vertical", parent_location=self.sequential.location + ) + self.child_container = ItemFactory.create( + category="split_test", parent_location=self.vertical_with_container.location) + self.child_vertical = ItemFactory.create(category="vertical", parent_location=self.child_container.location) + self.child_html_module = ItemFactory.create(category="html", parent_location=self.child_vertical.location) + + # Read again so that children lists are accurate + self.course = self.store.get_item(self.course.location) + self.chapter = self.store.get_item(self.chapter.location) + self.sequential = self.store.get_item(self.sequential.location) + self.vertical = self.store.get_item(self.vertical.location) + + self.vertical_with_container = self.store.get_item(self.vertical_with_container.location) + self.child_container = self.store.get_item(self.child_container.location) + self.child_vertical = self.store.get_item(self.child_vertical.location) + self.child_html_module = self.store.get_item(self.child_html_module.location) + + def test_get_parent_unit(self): + """ + Tests `get_parent_unit` method for the successful result. + """ + parent = utils.get_parent_unit(self.html_module_1) + self.assertEqual(parent.location, self.vertical.location) + + parent = utils.get_parent_unit(self.child_html_module) + self.assertEqual(parent.location, self.vertical_with_container.location) + + self.assertIsNone(utils.get_parent_unit(None)) + self.assertIsNone(utils.get_parent_unit(self.vertical)) + self.assertIsNone(utils.get_parent_unit(self.course)) + self.assertIsNone(utils.get_parent_unit(self.chapter)) + self.assertIsNone(utils.get_parent_unit(self.sequential)) diff --git a/lms/lib/utils.py b/lms/lib/utils.py new file mode 100644 index 0000000000..3617767561 --- /dev/null +++ b/lms/lib/utils.py @@ -0,0 +1,30 @@ +""" +Helper methods for the LMS. +""" + + +def get_parent_unit(xblock): + """ + Finds xblock's parent unit if it exists. + + To find an xblock's parent unit, we traverse up the xblock's + family tree until we find an xblock whose parent is a + sequential xblock, which guarantees that the xblock is a unit. + The `get_parent()` call on both the xblock and the parent block + ensure that we don't accidentally return that a unit is its own + parent unit. + + Returns: + xblock: Returns the parent unit xblock if it exists. + If no parent unit exists, returns None + """ + while xblock: + parent = xblock.get_parent() + if parent is None: + return None + grandparent = parent.get_parent() + if grandparent is None: + return None + if parent.category == "vertical" and grandparent.category == "sequential": + return parent + xblock = parent diff --git a/lms/lib/xblock/test/test_mixin.py b/lms/lib/xblock/test/test_mixin.py index f6cd9d68e9..a4d8c4a114 100644 --- a/lms/lib/xblock/test/test_mixin.py +++ b/lms/lib/xblock/test/test_mixin.py @@ -4,7 +4,9 @@ Tests of the LMS XBlock Mixin import ddt from nose.plugins.attrib import attr -from lms_xblock.mixin import INVALID_USER_PARTITION_VALIDATION, INVALID_USER_PARTITION_GROUP_VALIDATION +from lms_xblock.mixin import ( + INVALID_USER_PARTITION_VALIDATION, INVALID_USER_PARTITION_GROUP_VALIDATION, NONSENSICAL_ACCESS_RESTRICTION +) from xblock.validation import ValidationMessage from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory, ToyCourseFactory, ItemFactory @@ -38,10 +40,16 @@ class LmsXBlockMixinTestCase(ModuleStoreTestCase): subsection = ItemFactory.create(parent=section, category='sequential', display_name='Test Subsection') vertical = ItemFactory.create(parent=subsection, category='vertical', display_name='Test Unit') video = ItemFactory.create(parent=vertical, category='video', display_name='Test Video 1') + split_test = ItemFactory.create(parent=vertical, category='split_test', display_name='Test Content Experiment') + child_vertical = ItemFactory.create(parent=split_test, category='vertical') + child_html_module = ItemFactory.create(parent=child_vertical, category='html') self.section_location = section.location self.subsection_location = subsection.location self.vertical_location = vertical.location self.video_location = video.location + self.split_test_location = split_test.location + self.child_vertical_location = child_vertical.location + self.child_html_module_location = child_html_module.location def set_group_access(self, block_location, access_dict): """ @@ -130,6 +138,98 @@ class XBlockValidationTest(LmsXBlockMixinTestCase): ValidationMessage.ERROR, ) + def test_validate_nonsensical_access_for_split_test_children(self): + """ + Test the validation messages produced for components within + a content group experiment (also known as a split_test). + Ensures that children of split_test xblocks only validate + their access settings off the parent, rather than any + grandparent. + """ + # Test that no validation message is displayed on split_test child when child agrees with parent + self.set_group_access(self.vertical_location, {self.user_partition.id: [self.group1.id]}) + self.set_group_access(self.split_test_location, {self.user_partition.id: [self.group2.id]}) + self.set_group_access(self.child_vertical_location, {self.user_partition.id: [self.group2.id]}) + self.set_group_access(self.child_html_module_location, {self.user_partition.id: [self.group2.id]}) + validation = self.store.get_item(self.child_html_module_location).validate() + self.assertEqual(len(validation.messages), 0) + + # Test that a validation message is displayed on split_test child when the child contradicts the parent, + # even though the child agrees with the grandparent unit. + self.set_group_access(self.child_html_module_location, {self.user_partition.id: [self.group1.id]}) + validation = self.store.get_item(self.child_html_module_location).validate() + self.assertEqual(len(validation.messages), 1) + self.verify_validation_message( + validation.messages[0], + NONSENSICAL_ACCESS_RESTRICTION, + ValidationMessage.ERROR, + ) + + def test_validate_nonsensical_access_restriction(self): + """ + Test the validation messages produced for a component whose + access settings contradict the unit level access. + """ + # Test that there is no validation message for non-contradicting access restrictions + self.set_group_access(self.vertical_location, {self.user_partition.id: [self.group1.id]}) + self.set_group_access(self.video_location, {self.user_partition.id: [self.group1.id]}) + validation = self.store.get_item(self.video_location).validate() + self.assertEqual(len(validation.messages), 0) + + # Now try again with opposing access restrictions + self.set_group_access(self.vertical_location, {self.user_partition.id: [self.group1.id]}) + self.set_group_access(self.video_location, {self.user_partition.id: [self.group2.id]}) + validation = self.store.get_item(self.video_location).validate() + self.assertEqual(len(validation.messages), 1) + self.verify_validation_message( + validation.messages[0], + NONSENSICAL_ACCESS_RESTRICTION, + ValidationMessage.ERROR, + ) + + # Now try again when the component restricts access to additional groups that the unit does not + self.set_group_access(self.vertical_location, {self.user_partition.id: [self.group1.id]}) + self.set_group_access(self.video_location, {self.user_partition.id: [self.group1.id, self.group2.id]}) + validation = self.store.get_item(self.video_location).validate() + self.assertEqual(len(validation.messages), 1) + self.verify_validation_message( + validation.messages[0], + NONSENSICAL_ACCESS_RESTRICTION, + ValidationMessage.ERROR, + ) + + # Now try again when the component tries to allow access to all learners and staff + self.set_group_access(self.vertical_location, {self.user_partition.id: [self.group1.id]}) + self.set_group_access(self.video_location, {}) + validation = self.store.get_item(self.video_location).validate() + self.assertEqual(len(validation.messages), 1) + self.verify_validation_message( + validation.messages[0], + NONSENSICAL_ACCESS_RESTRICTION, + ValidationMessage.ERROR, + ) + + def test_nonsensical_access_restriction_does_not_override(self): + """ + Test that the validation message produced for a component + whose access settings contradict the unit level access don't + override other messages but add on to them. + """ + self.set_group_access(self.vertical_location, {self.user_partition.id: [self.group1.id]}) + self.set_group_access(self.video_location, {self.user_partition.id: [self.group2.id, 999]}) + validation = self.store.get_item(self.video_location).validate() + self.assertEqual(len(validation.messages), 2) + self.verify_validation_message( + validation.messages[0], + INVALID_USER_PARTITION_GROUP_VALIDATION, + ValidationMessage.ERROR, + ) + self.verify_validation_message( + validation.messages[1], + NONSENSICAL_ACCESS_RESTRICTION, + ValidationMessage.ERROR, + ) + class OpenAssessmentBlockMixinTestCase(ModuleStoreTestCase): """