diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py index e89520ab6e551f28d254d4c22d9f0c651a1cdd26..5c5bd97bea9be67fcb5693779c69d00cb6bf193c 100644 --- a/common/lib/xmodule/xmodule/modulestore/search.py +++ b/common/lib/xmodule/xmodule/modulestore/search.py @@ -5,12 +5,14 @@ from logging import getLogger from six.moves import range +from lms.djangoapps.courseware.masquerade import MASQUERADE_SETTINGS_KEY +from student.roles import GlobalStaff from .exceptions import ItemNotFoundError, NoPathToItem LOGGER = getLogger(__name__) -def path_to_location(modulestore, usage_key, full_path=False): +def path_to_location(modulestore, usage_key, request=None, full_path=False): ''' Try to find a course_id/chapter/section[/position] path to location in modulestore. The courseware insists that the first level in the course is @@ -19,6 +21,7 @@ def path_to_location(modulestore, usage_key, full_path=False): Args: modulestore: which store holds the relevant objects usage_key: :class:`UsageKey` the id of the location to which to generate the path + request: Request object containing information about user and masquerade settings, Default is None full_path: :class:`Bool` if True, return the full path to location. Default is False. Raises @@ -104,6 +107,7 @@ def path_to_location(modulestore, usage_key, full_path=False): # (e.g. sequential and videosequence) currently deal with this form of # representing nested positions. This needs to happen before jumping to a # module nested in more than one positional module will work. + if n > 3: position_list = [] for path_index in range(2, n - 1): @@ -112,18 +116,54 @@ def path_to_location(modulestore, usage_key, full_path=False): section_desc = modulestore.get_item(path[path_index]) # this calls get_children rather than just children b/c old mongo includes private children # in children but not in get_children - child_locs = [c.location for c in section_desc.get_children()] + child_locs = get_child_locations(section_desc, request, course_id) # positions are 1-indexed, and should be strings to be consistent with # url parsing. - position_list.append(str(child_locs.index(path[path_index + 1]) + 1)) + if path[path_index + 1] in child_locs: + position_list.append(str(child_locs.index(path[path_index + 1]) + 1)) position = "_".join(position_list) return (course_id, chapter, section, vertical, position, path[-1]) +def get_child_locations(section_desc, request, course_id): + """ + Returns all child locations for a section. If user is learner or masquerading as learner, + staff only blocks are excluded. + """ + is_staff_user = GlobalStaff().has_user(request.user) if request else False + + def is_masquerading_as_student(): + """ + Return True if user is masquerading as learner. + """ + masquerade_settings = request.session.get(MASQUERADE_SETTINGS_KEY, {}) + course_info = masquerade_settings.get(course_id) + return masquerade_settings and course_info and getattr(course_info, 'role', '') == 'student' + + def is_user_staff_and_not_masquerading_learner(): + """ + Return True if user is staff and not masquerading as learner. + """ + return is_staff_user and not is_masquerading_as_student() + + def is_child_appendable(child_instance): + """ + Return True if child is appendable based on request and request's user type. + """ + return (request and is_user_staff_and_not_masquerading_learner()) or not child_instance.visible_to_staff_only + + child_locs = [] + for child in section_desc.get_children(): + if not is_child_appendable(child): + continue + child_locs.append(child.location) + return child_locs + + def navigation_index(position): """ - Get the navigation index from the position argument (where the position argument was recieved from a call to + Get the navigation index from the position argument (where the position argument was received from a call to path_to_location) Argument: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 3dbc6a9addfcaab07034040029e8f63916d7d663..8eeb67826250a08b503a4b5aef3ea10539994ca4 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1651,7 +1651,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # 8-9. get vertical, compute inheritance # 10-11. get other vertical_x1b (why?) and compute inheritance # Split: active_versions & structure - @ddt.data((ModuleStoreEnum.Type.mongo, [12, 3], 0), (ModuleStoreEnum.Type.split, [2, 2], 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, [12, 3], 0), (ModuleStoreEnum.Type.split, [3, 2], 0)) @ddt.unpack def test_path_to_location(self, default_ms, num_finds, num_sends): """ diff --git a/common/test/acceptance/tests/lms/test_lms_dashboard.py b/common/test/acceptance/tests/lms/test_lms_dashboard.py index 96209c432c772bf82b3ce86b9136b103f02c7522..fec1e8491338c65014c05464078863d2be5beebd 100644 --- a/common/test/acceptance/tests/lms/test_lms_dashboard.py +++ b/common/test/acceptance/tests/lms/test_lms_dashboard.py @@ -7,11 +7,13 @@ from __future__ import absolute_import import datetime import re import six - from six.moves.urllib.parse import unquote # pylint: disable=import-error -from common.test.acceptance.fixtures.course import CourseFixture +from common.test.acceptance.fixtures.course import CourseFixture, XBlockFixtureDesc from common.test.acceptance.pages.common.auto_auth import AutoAuthPage +from common.test.acceptance.pages.lms.course_home import CourseHomePage from common.test.acceptance.pages.lms.dashboard import DashboardPage +from common.test.acceptance.pages.lms.problem import ProblemPage +from common.test.acceptance.pages.lms.staff_view import StaffPreviewPage from common.test.acceptance.tests.helpers import UniqueCourseTest, generate_course_key DEFAULT_SHORT_DATE_FORMAT = u'{dt:%b} {dt.day}, {dt.year}' @@ -125,8 +127,24 @@ class BaseLmsDashboardTestMultiple(UniqueCourseTest): u"social_sharing_url": {u"value": "http://custom/course/url"}, u"cert_name_long": {u"value": value['cert_name_long']} }) - - course_fixture.install() + course_fixture.add_children( + XBlockFixtureDesc('chapter', 'Test Section 1').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection 1,1').add_children( + XBlockFixtureDesc('problem', 'Test Problem 1', data='<problem>problem 1 dummy body</problem>'), + XBlockFixtureDesc('html', 'html 1', data="<html>html 1 dummy body</html>"), + XBlockFixtureDesc('problem', 'Test Problem 2', data="<problem>problem 2 dummy body</problem>"), + XBlockFixtureDesc('html', 'html 2', data="<html>html 2 dummy body</html>"), + ), + XBlockFixtureDesc('sequential', 'Test Subsection 1,2').add_children( + XBlockFixtureDesc('problem', 'Test Problem 3', data='<problem>problem 3 dummy body</problem>'), + ), + XBlockFixtureDesc( + 'sequential', 'Test HIDDEN Subsection', metadata={'visible_to_staff_only': True} + ).add_children( + XBlockFixtureDesc('problem', 'Test HIDDEN Problem', data='<problem>hidden problem</problem>'), + ), + ) + ).install() self.course_keys[key] = course_key self.course_fixtures[key] = course_fixture @@ -201,7 +219,6 @@ class LmsDashboardPageTest(BaseLmsDashboardTest): Scenario: Course Date should have the format 'Ended - Sep 23, 2015' if the course on student dashboard has ended. - As a Student, Given that I have enrolled to a course And the course has ended in the past @@ -233,7 +250,6 @@ class LmsDashboardPageTest(BaseLmsDashboardTest): Scenario: Course Date should have the format 'Started - Sep 23, 2015' if the course on student dashboard is running. - As a Student, Given that I have enrolled to a course And the course has started @@ -266,7 +282,6 @@ class LmsDashboardPageTest(BaseLmsDashboardTest): Scenario: Course Date should have the format 'Starts - Sep 23, 2015' if the course on student dashboard starts in future. - As a Student, Given that I have enrolled to a course And the course starts in future @@ -300,7 +315,6 @@ class LmsDashboardPageTest(BaseLmsDashboardTest): Scenario: Course Date should have the format 'Starts - Wednesday at 5am UTC' if the course on student dashboard starts within 5 days. - As a Student, Given that I have enrolled to a course And the course starts within 5 days @@ -436,3 +450,49 @@ class LmsDashboardA11yTest(BaseLmsDashboardTestMultiple): course_listings = self.dashboard_page.get_courses() self.assertEqual(len(course_listings), 3) self.dashboard_page.a11y_audit.check_for_accessibility_errors() + + +class TestMasqueradeAndSwitchCourse(BaseLmsDashboardTestMultiple): + """ + Class to test lms dashboard accessibility of courses when masquerading as learner. + """ + + def test_masquerade_and_switch_course(self): + """ + Scenario: + Staff user should be able to access other courses after + masquerading as student in one course + + As Staff user, Select a course + When I click to change view from Staff to Learner + Then the first subsection from course outline should be visible as Learner + When I click to select a different course + Then the first subsection from new course outline should be visible as Staff + """ + AutoAuthPage( + self.browser, + username=self.username, + email=self.email, + staff=True + ).visit() + self.dashboard_page.visit() + + section_title = 'Test Section 1' + subsection_title = 'Test Subsection 1,1' + course_page = CourseHomePage(self.browser, str(self.course_keys['A'])) + course_page.visit() + + problem_name = u'Test Problem 1' + + staff_page = StaffPreviewPage(self.browser) + staff_page.set_staff_view_mode('Learner') + + course_page.outline.go_to_section(section_title, subsection_title) + self.assertEqual(staff_page.staff_view_mode, 'Learner') + self.assertEqual(ProblemPage(self.browser).problem_name, problem_name) + + course_page.course_id = str(self.course_keys['B']) + course_page.visit() + course_page.outline.go_to_section(section_title, subsection_title) + self.assertNotEqual(staff_page.staff_view_mode, 'Learner') + self.assertEqual(ProblemPage(self.browser).problem_name, problem_name) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index aa0021bfbbf1be68a1fac107b91a192f9eaefea2..d79ed2f7e70393fa656ce72ec4bdd8e3426fe5f5 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -25,7 +25,6 @@ from freezegun import freeze_time from markupsafe import escape from milestones.tests.utils import MilestonesTestCaseMixin from mock import MagicMock, PropertyMock, create_autospec, patch -from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from pytz import UTC from six import text_type @@ -62,6 +61,7 @@ from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.grades.config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT from lms.djangoapps.grades.config.waffle import waffle as grades_waffle from lms.djangoapps.verify_student.services import IDVerificationService +from opaque_keys.edx.keys import CourseKey, UsageKey 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 @@ -104,6 +104,7 @@ FEATURES_WITH_DISABLE_HONOR_CERTIFICATE = settings.FEATURES.copy() FEATURES_WITH_DISABLE_HONOR_CERTIFICATE['DISABLE_HONOR_CERTIFICATES'] = True +@ddt.ddt class TestJumpTo(ModuleStoreTestCase): """ Check the jumpto link for a course. @@ -191,7 +192,6 @@ class TestJumpTo(ModuleStoreTestCase): module2 = ItemFactory.create(category='html', parent_location=nested_vertical2.location) # internal position of module2 will be 1_2 (2nd item withing 1st item) - expected = '/courses/{course_id}/courseware/{chapter_id}/{section_id}/1?{activate_block_id}'.format( course_id=six.text_type(course.id), chapter_id=chapter.url_name, @@ -215,6 +215,41 @@ class TestJumpTo(ModuleStoreTestCase): response = self.client.get(jumpto_url) self.assertEqual(response.status_code, 404) + @ddt.data( + (False, '1'), + (True, '2') + ) + @ddt.unpack + def test_jump_to_for_learner_with_staff_only_content(self, is_staff_user, position): + """ + Test for checking correct position in redirect_url for learner when a course has staff-only units. + """ + course = CourseFactory.create() + request = RequestFactory().get('/') + request.user = UserFactory(is_staff=is_staff_user, username="staff") + request.session = {} + course_key = CourseKey.from_string(six.text_type(course.id)) + chapter = ItemFactory.create(category='chapter', parent_location=course.location) + section = ItemFactory.create(category='sequential', parent_location=chapter.location) + __ = ItemFactory.create(category='vertical', parent_location=section.location) + staff_only_vertical = ItemFactory.create(category='vertical', parent_location=section.location, + metadata=dict(visible_to_staff_only=True)) + __ = ItemFactory.create(category='vertical', parent_location=section.location) + + usage_key = UsageKey.from_string(six.text_type(staff_only_vertical.location)).replace(course_key=course_key) + expected_url = reverse( + 'courseware_position', + kwargs={ + 'course_id': six.text_type(course.id), + 'chapter': chapter.url_name, + 'section': section.url_name, + 'position': position, + } + ) + expected_url += "?{}".format(urlencode({'activate_block_id': six.text_type(staff_only_vertical.location)})) + + self.assertEqual(expected_url, get_redirect_url(course_key, usage_key, request)) + @ddt.ddt class IndexQueryTestCase(ModuleStoreTestCase): diff --git a/lms/djangoapps/courseware/url_helpers.py b/lms/djangoapps/courseware/url_helpers.py index 0cf00a194bd2730845a177746063f3dd10e831a1..2f753407b6e6b83e9d945c58cd042b26638a92c2 100644 --- a/lms/djangoapps/courseware/url_helpers.py +++ b/lms/djangoapps/courseware/url_helpers.py @@ -11,7 +11,7 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.search import navigation_index, path_to_location -def get_redirect_url(course_key, usage_key): +def get_redirect_url(course_key, usage_key, request=None): """ Returns the redirect url back to courseware Args: @@ -28,7 +28,7 @@ def get_redirect_url(course_key, usage_key): ( course_key, chapter, section, vertical_unused, position, final_target_id - ) = path_to_location(modulestore(), usage_key) + ) = path_to_location(modulestore(), usage_key, request) # choose the appropriate view (and provide the necessary args) based on the # args provided by the redirect. diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 33dff1d60cb8a85502a720845a87e08f6509fb91..da08896d562e40887ca8968d17c7ba0851e071aa 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -355,7 +355,7 @@ def jump_to(_request, course_id, location): except InvalidKeyError: raise Http404(u"Invalid course_key or usage_key") try: - redirect_url = get_redirect_url(course_key, usage_key) + redirect_url = get_redirect_url(course_key, usage_key, _request) except ItemNotFoundError: raise Http404(u"No data at this location: {0}".format(usage_key)) except NoPathToItem: