From 561c57dbe9c26f518e177f0934c876492641b3c7 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Mon, 1 Dec 2014 16:34:11 -0500 Subject: [PATCH] Extend preview to support cohorted courseware TNL-651 --- CHANGELOG.rst | 5 + .../test/acceptance/pages/lms/staff_view.py | 17 +- common/test/acceptance/tests/helpers.py | 13 +- ...staff_view.py => test_lms_user_preview.py} | 180 +++++++++-- .../tests/studio/test_studio_container.py | 4 +- .../tests/studio/test_studio_outline.py | 8 +- .../tests/studio/test_studio_split_test.py | 43 +-- lms/djangoapps/courseware/access.py | 11 +- .../courseware/features/lti.feature | 8 +- lms/djangoapps/courseware/features/lti.py | 6 +- lms/djangoapps/courseware/masquerade.py | 125 +++++--- lms/djangoapps/courseware/module_render.py | 9 +- .../courseware/tests/test_access.py | 13 +- .../courseware/tests/test_masquerade.py | 297 +++++++++++++----- lms/djangoapps/courseware/views.py | 16 +- lms/lib/xblock/test/test_mixin.py | 38 --- .../courseware/course_navigation.html | 72 ++--- lms/templates/courseware/info.html | 10 +- lms/urls.py | 3 +- .../core/djangoapps/course_groups/cohorts.py | 9 +- .../course_groups/partition_scheme.py | 36 ++- .../course_groups/tests/test_cohorts.py | 26 +- .../tests/test_partition_scheme.py | 115 ++++++- .../djangoapps/user_api/partition_schemes.py | 6 +- 24 files changed, 753 insertions(+), 317 deletions(-) rename common/test/acceptance/tests/lms/{test_lms_staff_view.py => test_lms_user_preview.py} (54%) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b12fc47216..a3862ff21a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Studio/LMS: Implement cohorted courseware. TNL-648 + LMS: Student Notes: Eventing for Student Notes. TNL-931 LMS: Student Notes: Add course structure view. TNL-762 @@ -26,6 +28,9 @@ LMS: Student Notes: Toggle single note visibility. TNL-660 LMS: Student Notes: Add Notes page. TNL-797 LMS: Student Notes: Add possibility to add/edit/remove notes. TNL-655 +======= +LMS: Extend preview to support cohorted courseware. TNL-651 +>>>>>>> Extend preview to support cohorted courseware Platform: Add group_access field to all xblocks. TNL-670 diff --git a/common/test/acceptance/pages/lms/staff_view.py b/common/test/acceptance/pages/lms/staff_view.py index a434b7cc69..b4eeb5d505 100644 --- a/common/test/acceptance/pages/lms/staff_view.py +++ b/common/test/acceptance/pages/lms/staff_view.py @@ -11,25 +11,26 @@ class StaffPage(CoursewarePage): """ url = None - STAFF_STATUS_CSS = '#staffstatus' + PREVIEW_MENU_CSS = '.preview-menu' + VIEW_MODE_OPTIONS_CSS = '.preview-menu .action-preview-select option' def is_browser_on_page(self): if not super(StaffPage, self).is_browser_on_page(): return False - return self.q(css=self.STAFF_STATUS_CSS).present + return self.q(css=self.PREVIEW_MENU_CSS).present @property - def staff_status(self): + def staff_view_mode(self): """ - Return the current status, either Staff view or Student view + Return the currently chosen view mode, e.g. "Staff", "Student" or a content group. """ - return self.q(css=self.STAFF_STATUS_CSS).text[0] + return self.q(css=self.VIEW_MODE_OPTIONS_CSS).filter(lambda el: el.is_selected()).first.text[0] - def toggle_staff_view(self): + def set_staff_view_mode(self, view_mode): """ - Toggle between staff view and student view. + Set the current view mode, e.g. "Staff", "Student" or a content group. """ - self.q(css=self.STAFF_STATUS_CSS).first.click() + self.q(css=self.VIEW_MODE_OPTIONS_CSS).filter(lambda el: el.text == view_mode).first.click() self.wait_for_ajax() def open_staff_debug_info(self): diff --git a/common/test/acceptance/tests/helpers.py b/common/test/acceptance/tests/helpers.py index 147d698299..e7a85b40ba 100644 --- a/common/test/acceptance/tests/helpers.py +++ b/common/test/acceptance/tests/helpers.py @@ -7,9 +7,11 @@ import functools import requests import os from path import path +from bok_choy.javascript import js_defined from bok_choy.web_app_test import WebAppTest from opaque_keys.edx.locator import CourseLocator -from bok_choy.javascript import js_defined +from xmodule.partitions.tests.test_partitions import MockUserPartitionScheme +from xmodule.partitions.partitions import UserPartition def skip_if_browser(browser): @@ -279,3 +281,12 @@ class YouTubeStubConfig(object): return json.loads(response.content) else: return {} + + +def create_user_partition_json(partition_id, name, description, groups, scheme="random"): + """ + Helper method to create user partition JSON. If scheme is not supplied, "random" is used. + """ + return UserPartition( + partition_id, name, description, groups, MockUserPartitionScheme(scheme) + ).to_json() diff --git a/common/test/acceptance/tests/lms/test_lms_staff_view.py b/common/test/acceptance/tests/lms/test_lms_user_preview.py similarity index 54% rename from common/test/acceptance/tests/lms/test_lms_staff_view.py rename to common/test/acceptance/tests/lms/test_lms_user_preview.py index b2d5b52e69..9eb6666773 100644 --- a/common/test/acceptance/tests/lms/test_lms_staff_view.py +++ b/common/test/acceptance/tests/lms/test_lms_user_preview.py @@ -1,13 +1,15 @@ # -*- coding: utf-8 -*- """ -End-to-end tests for the LMS. +Tests the "preview" selector in the LMS that allows changing between Staff, Student, and Content Groups. """ -from ..helpers import UniqueCourseTest +from ..helpers import UniqueCourseTest, create_user_partition_json from ...pages.studio.auto_auth import AutoAuthPage from ...pages.lms.courseware import CoursewarePage from ...pages.lms.staff_view import StaffPage +from ...pages.lms.course_nav import CourseNavPage from ...fixtures.course import CourseFixture, XBlockFixtureDesc +from xmodule.partitions.partitions import Group from textwrap import dedent @@ -24,30 +26,14 @@ class StaffViewTest(UniqueCourseTest): self.courseware_page = CoursewarePage(self.browser, self.course_id) # Install a course with sections/problems, tabs, updates, and handouts - course_fix = CourseFixture( + self.course_fixture = CourseFixture( self.course_info['org'], self.course_info['number'], self.course_info['run'], self.course_info['display_name'] ) - problem_data = dedent(""" - -

Choose Yes.

- - - Yes - - - - """) + self.populate_course_fixture(self.course_fixture) # pylint: disable=no-member - course_fix.add_children( - XBlockFixtureDesc('chapter', 'Test Section').add_children( - XBlockFixtureDesc('sequential', 'Test Subsection').add_children( - XBlockFixtureDesc('problem', 'Test Problem 1', data=problem_data), - XBlockFixtureDesc('problem', 'Test Problem 2', data=problem_data) - ) - ) - ).install() + self.course_fixture.install() # Auto-auth register for the course. # Do this as global staff so that you will see the Staff View @@ -60,11 +46,41 @@ class StaffViewTest(UniqueCourseTest): """ self.courseware_page.visit() staff_page = StaffPage(self.browser, self.course_id) - self.assertEqual(staff_page.staff_status, 'Staff view') + self.assertEqual(staff_page.staff_view_mode, 'Staff') return staff_page -class StaffViewToggleTest(StaffViewTest): +class CourseWithoutContentGroupsTest(StaffViewTest): + """ + Setup for tests that have no content restricted to specific content groups. + """ + + def populate_course_fixture(self, course_fixture): + """ + Populates test course with chapter, sequential, and 2 problems. + """ + problem_data = dedent(""" + +

Choose Yes.

+ + + Yes + + + + """) + + course_fixture.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children( + XBlockFixtureDesc('problem', 'Test Problem 1', data=problem_data), + XBlockFixtureDesc('problem', 'Test Problem 2', data=problem_data) + ) + ) + ) + + +class StaffViewToggleTest(CourseWithoutContentGroupsTest): """ Tests for the staff view toggle button. """ @@ -75,12 +91,12 @@ class StaffViewToggleTest(StaffViewTest): course_page = self._goto_staff_page() self.assertTrue(course_page.has_tab('Instructor')) - course_page.toggle_staff_view() - self.assertEqual(course_page.staff_status, 'Student view') + course_page.set_staff_view_mode('Student') + self.assertEqual(course_page.staff_view_mode, 'Student') self.assertFalse(course_page.has_tab('Instructor')) -class StaffDebugTest(StaffViewTest): +class StaffDebugTest(CourseWithoutContentGroupsTest): """ Tests that verify the staff debug info. """ @@ -209,3 +225,115 @@ class StaffDebugTest(StaffViewTest): msg = staff_debug_page.idash_msg[0] self.assertEqual(u'Successfully deleted student state ' 'for user {}'.format(self.USERNAME), msg) + + +class CourseWithContentGroupsTest(StaffViewTest): + """ + Verifies that changing the "previewing as" selector works properly for cohorted content. + """ + + def setUp(self): + super(CourseWithContentGroupsTest, self).setUp() + # pylint: disable=protected-access + self.course_fixture._update_xblock(self.course_fixture._course_location, { + "metadata": { + u"user_partitions": [ + create_user_partition_json( + 0, + 'Configuration alpha,beta', + 'Content Group Partition', + [Group("0", 'alpha'), Group("1", 'beta')], + scheme="cohort" + ) + ], + }, + }) + + def populate_course_fixture(self, course_fixture): + """ + Populates test course with chapter, sequential, and 3 problems. + One problem is visible to all, one problem is visible only to Group "alpha", and + one problem is visible only to Group "beta". + """ + problem_data = dedent(""" + +

Choose Yes.

+ + + Yes + + + + """) + + course_fixture.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children( + XBlockFixtureDesc( + 'problem', 'Visible to alpha', data=problem_data, metadata={"group_access": {0: [0]}} + ), + XBlockFixtureDesc( + 'problem', 'Visible to beta', data=problem_data, metadata={"group_access": {0: [1]}} + ), + XBlockFixtureDesc('problem', 'Visible to everyone', data=problem_data) + ) + ) + ) + + def _verify_visible_problems(self, expected_items): + """ + Verify that the expected problems are visible. + """ + course_nav = CourseNavPage(self.browser) + actual_items = course_nav.sequence_items + self.assertItemsEqual(expected_items, actual_items) + + def test_staff_sees_all_problems(self): + """ + Scenario: Staff see all problems + Given I have a course with a cohort user partition + And problems that are associated with specific groups in the user partition + When I view the courseware in the LMS with staff access + Then I see all the problems, regardless of their group_access property + """ + self._goto_staff_page() + self._verify_visible_problems(['Visible to alpha', 'Visible to beta', 'Visible to everyone']) + + def test_student_not_in_content_group(self): + """ + Scenario: When previewing as a student, only content visible to all is shown + Given I have a course with a cohort user partition + And problems that are associated with specific groups in the user partition + When I view the courseware in the LMS with staff access + And I change to previewing as a Student + Then I see only problems visible to all users + """ + course_page = self._goto_staff_page() + course_page.set_staff_view_mode('Student') + self._verify_visible_problems(['Visible to everyone']) + + def test_as_student_in_alpha(self): + """ + Scenario: When previewing as a student in group alpha, only content visible to alpha is shown + Given I have a course with a cohort user partition + And problems that are associated with specific groups in the user partition + When I view the courseware in the LMS with staff access + And I change to previewing as a Student in group alpha + Then I see only problems visible to group alpha + """ + course_page = self._goto_staff_page() + course_page.set_staff_view_mode('Student in alpha') + self._verify_visible_problems(['Visible to alpha', 'Visible to everyone']) + + def test_as_student_in_beta(self): + """ + Scenario: When previewing as a student in group beta, only content visible to beta is shown + Given I have a course with a cohort user partition + And problems that are associated with specific groups in the user partition + When I view the courseware in the LMS with staff access + And I change to previewing as a Student in group beta + Then I see only problems visible to group beta + """ + course_page = self._goto_staff_page() + course_page.set_staff_view_mode('Student in beta') + self._verify_visible_problems(['Visible to beta', 'Visible to everyone']) diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index 79a2fc8471..979132e704 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -698,14 +698,14 @@ class UnitPublishingTest(ContainerBase): """ Verifies no component is visible when viewing as a student. """ - self._verify_and_return_staff_page().toggle_staff_view() + self._verify_and_return_staff_page().set_staff_view_mode('Student') self.assertEqual(0, self.courseware.num_xblock_components) def _verify_student_view_visible(self, expected_components): """ Verifies expected components are visible when viewing as a student. """ - self._verify_and_return_staff_page().toggle_staff_view() + self._verify_and_return_staff_page().set_staff_view_mode('Student') self._verify_components_visible(expected_components) def _verify_components_visible(self, expected_components): diff --git a/common/test/acceptance/tests/studio/test_studio_outline.py b/common/test/acceptance/tests/studio/test_studio_outline.py index fc787d7ed7..a19dc1a5bc 100644 --- a/common/test/acceptance/tests/studio/test_studio_outline.py +++ b/common/test/acceptance/tests/studio/test_studio_outline.py @@ -708,7 +708,7 @@ class StaffLockTest(CourseOutlineTest): When I enable explicit staff lock on one section And I click the View Live button to switch to staff view Then I see two sections in the sidebar - And when I click to toggle to student view + And when I switch the view mode to student view Then I see one section in the sidebar """ self.course_outline_page.visit() @@ -718,7 +718,7 @@ class StaffLockTest(CourseOutlineTest): courseware = CoursewarePage(self.browser, self.course_id) courseware.wait_for_page() self.assertEqual(courseware.num_sections, 2) - StaffPage(self.browser, self.course_id).toggle_staff_view() + StaffPage(self.browser, self.course_id).set_staff_view_mode('Student') self.assertEqual(courseware.num_sections, 1) def test_locked_subsections_do_not_appear_in_lms(self): @@ -728,7 +728,7 @@ class StaffLockTest(CourseOutlineTest): When I enable explicit staff lock on one subsection And I click the View Live button to switch to staff view Then I see two subsections in the sidebar - And when I click to toggle to student view + And when I switch the view mode to student view Then I see one section in the sidebar """ self.course_outline_page.visit() @@ -737,7 +737,7 @@ class StaffLockTest(CourseOutlineTest): courseware = CoursewarePage(self.browser, self.course_id) courseware.wait_for_page() self.assertEqual(courseware.num_subsections, 2) - StaffPage(self.browser, self.course_id).toggle_staff_view() + StaffPage(self.browser, self.course_id).set_staff_view_mode('Student') self.assertEqual(courseware.num_subsections, 1) def test_toggling_staff_lock_on_section_does_not_publish_draft_units(self): 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 defd8d879d..48ebef6f2c 100644 --- a/common/test/acceptance/tests/studio/test_studio_split_test.py +++ b/common/test/acceptance/tests/studio/test_studio_split_test.py @@ -9,7 +9,6 @@ from nose.plugins.attrib import attr from selenium.webdriver.support.ui import Select from xmodule.partitions.partitions import Group, UserPartition -from xmodule.partitions.tests.test_partitions import MockUserPartitionScheme from bok_choy.promise import Promise, EmptyPromise from ...fixtures.course import XBlockFixtureDesc @@ -21,6 +20,7 @@ from ...pages.studio.settings_group_configurations import GroupConfigurationsPag from ...pages.studio.utils import add_advanced_component from ...pages.xblock.utils import wait_for_xblock_initialization from ...pages.lms.courseware import CoursewarePage +from ..helpers import create_user_partition_json from base_studio_test import StudioCourseTest @@ -31,15 +31,6 @@ class SplitTestMixin(object): """ Mixin that contains useful methods for split_test module testing. """ - @staticmethod - def create_user_partition_json(partition_id, name, description, groups): - """ - Helper method to create user partition JSON. - """ - return UserPartition( - partition_id, name, description, groups, MockUserPartitionScheme("random") - ).to_json() - def verify_groups(self, container, active_groups, inactive_groups, verify_missing_groups_not_present=True): """ Check that the groups appear and are correctly categorized as to active and inactive. @@ -90,13 +81,13 @@ class SplitTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, 'Configuration alpha,beta', 'first', [Group("0", 'alpha'), Group("1", 'beta')] ), - self.create_user_partition_json( + create_user_partition_json( 1, 'Configuration 0,1,2', 'second', @@ -144,7 +135,7 @@ class SplitTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, 'Configuration alpha,beta', 'first', @@ -348,7 +339,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json(0, "Name", "Description.", groups), + create_user_partition_json(0, "Name", "Description.", groups), ], }, }) @@ -420,13 +411,13 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, 'Name of the Group Configuration', 'Description of the group configuration.', [Group("0", 'Group 0'), Group("1", 'Group 1')] ), - self.create_user_partition_json( + create_user_partition_json( 1, 'Name of second Group Configuration', 'Second group configuration.', @@ -565,7 +556,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, 'Name of the Group Configuration', 'Description of the group configuration.', @@ -649,13 +640,13 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, 'Name of the Group Configuration', 'Description of the group configuration.', [Group("0", 'Group 0'), Group("1", 'Group 1')] ), - self.create_user_partition_json( + create_user_partition_json( 1, 'Name of second Group Configuration', 'Second group configuration.', @@ -745,7 +736,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, "Name", "Description.", @@ -782,7 +773,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, "Name", "Description.", @@ -830,13 +821,13 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, 'Configuration 1', 'Description of the group configuration.', [Group("0", 'Group 0'), Group("1", 'Group 1')] ), - self.create_user_partition_json( + create_user_partition_json( 1, 'Configuration 2', 'Second group configuration.', @@ -873,7 +864,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, "Name", "Description.", @@ -914,13 +905,13 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_fixture._update_xblock(self.course_fixture._course_location, { "metadata": { u"user_partitions": [ - self.create_user_partition_json( + create_user_partition_json( 0, "Name", "Description.", [Group("0", "Group A"), Group("1", "Group B")] ), - self.create_user_partition_json( + create_user_partition_json( 1, 'Name of second Group Configuration', 'Second group configuration.', diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 8ed87b2ce0..e0fac92929 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -18,7 +18,7 @@ from xblock.core import XBlock from xmodule.partitions.partitions import NoSuchUserPartitionError, NoSuchUserPartitionGroupError from external_auth.models import ExternalAuthMap -from courseware.masquerade import is_masquerading_as_student +from courseware.masquerade import get_masquerade_role, is_masquerading_as_student from django.utils.timezone import UTC from student import auth from student.roles import ( @@ -396,7 +396,7 @@ def _has_access_descriptor(user, action, descriptor, course_key=None): return _has_staff_access_to_descriptor(user, descriptor, course_key) # If start dates are off, can always load - if settings.FEATURES['DISABLE_START_DATES'] and not is_masquerading_as_student(user): + if settings.FEATURES['DISABLE_START_DATES'] and not is_masquerading_as_student(user, course_key): debug("Allow: DISABLE_START_DATES") return True @@ -579,7 +579,7 @@ def _has_access_to_course(user, access_level, course_key): debug("Deny: no user or anon user") return False - if is_masquerading_as_student(user): + if is_masquerading_as_student(user, course_key): return False if GlobalStaff().has_user(user): @@ -636,8 +636,9 @@ def get_user_role(user, course_key): Return corresponding string if user has staff, instructor or student course role in LMS. """ - if is_masquerading_as_student(user): - return 'student' + role = get_masquerade_role(user, course_key) + if role: + return role elif has_access(user, 'instructor', course_key): return 'instructor' elif has_access(user, 'staff', course_key): diff --git a/lms/djangoapps/courseware/features/lti.feature b/lms/djangoapps/courseware/features/lti.feature index ea34a7bb56..dc754fdfae 100644 --- a/lms/djangoapps/courseware/features/lti.feature +++ b/lms/djangoapps/courseware/features/lti.feature @@ -64,7 +64,7 @@ Feature: LMS.LTI component | False | True | And I view the LTI and it is rendered in iframe And I see in iframe that LTI role is Instructor - And I switch to Student view + And I switch to student Then I see in iframe that LTI role is Student #8 @@ -162,7 +162,7 @@ Feature: LMS.LTI component | True | Then I view the permission alert Then I reject the permission alert and do not view the LTI - + #16 Scenario: LTI component requests permission for username and displays LTI when accepted Given the course has correct LTI credentials with registered Instructor @@ -180,7 +180,7 @@ Feature: LMS.LTI component | True | Then I view the permission alert Then I reject the permission alert and do not view the LTI - + #18 Scenario: LTI component requests permission for email and displays LTI when accepted Given the course has correct LTI credentials with registered Instructor @@ -198,7 +198,7 @@ Feature: LMS.LTI component | True | True | Then I view the permission alert Then I reject the permission alert and do not view the LTI - + #20 Scenario: LTI component requests permission for email and username and displays LTI when accepted Given the course has correct LTI credentials with registered Instructor diff --git a/lms/djangoapps/courseware/features/lti.py b/lms/djangoapps/courseware/features/lti.py index 2df7ea3de1..8110067820 100644 --- a/lms/djangoapps/courseware/features/lti.py +++ b/lms/djangoapps/courseware/features/lti.py @@ -387,9 +387,9 @@ def check_role(_step, role): @step('I switch to (.*)$') def switch_view(_step, view): - staff_status = world.css_find('#staffstatus').first - if staff_status.text != view: - world.css_click('#staffstatus') + staff_status = world.css_find('#action-preview-select').first.value + if staff_status != view: + world.browser.select("select", view) world.wait_for_ajax_complete() diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index 41a868cc9b..0ea3d6b6e1 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -1,42 +1,69 @@ ''' ----------------------------------------- Masequerade ---------------------------------------- +---------------------------------------- Masquerade ---------------------------------------- Allow course staff to see a student or staff view of courseware. Which kind of view has been selected is stored in the session state. ''' -import json import logging -from django.http import HttpResponse from django.conf import settings +from django.contrib.auth.decorators import login_required +from django.views.decorators.http import require_POST +from util.json_request import expect_json, JsonResponse + +from opaque_keys.edx.keys import CourseKey log = logging.getLogger(__name__) -MASQ_KEY = 'masquerade_identity' +# The key used to store a user's course-level masquerade information in the Django session. +# The value is a dict from course keys to CourseMasquerade objects. +MASQUERADE_SETTINGS_KEY = 'masquerade_settings' -def handle_ajax(request, marg): - ''' - Handle ajax call from "staff view" / "student view" toggle button - ''' - if marg == 'toggle': - status = request.session.get(MASQ_KEY, '') - if status is None or status in ['', 'staff']: - status = 'student' - else: - status = 'staff' - request.session[MASQ_KEY] = status - return HttpResponse(json.dumps({'status': status})) +class CourseMasquerade(object): + """ + Masquerade settings for a particular course. + """ + def __init__(self, course_key, role='student', user_partition_id=None, group_id=None): + self.course_key = course_key + self.role = role + self.user_partition_id = user_partition_id + self.group_id = group_id -def setup_masquerade(request, staff_access=False): - ''' - Setup masquerade identity (allows staff to view courseware as either staff or student) +@require_POST +@login_required +@expect_json +def handle_ajax(request, course_key_string): + """ + Handle AJAX posts to update the current user's masquerade for the specified course. + The masquerade settings are stored in the Django session as a dict from course keys + to CourseMasquerade objects. + """ + course_key = CourseKey.from_string(course_key_string) + masquerade_settings = request.session.get(MASQUERADE_SETTINGS_KEY, {}) + request_json = request.json + role = request_json.get('role', 'student') + user_partition_id = request_json.get('user_partition_id', None) + group_id = request_json.get('group_id', None) + masquerade_settings[course_key] = CourseMasquerade( + course_key, + role=role, + user_partition_id=user_partition_id, + group_id=group_id + ) + request.session[MASQUERADE_SETTINGS_KEY] = masquerade_settings + return JsonResponse() - Uses request.session[MASQ_KEY] to store status of masquerading. - Adds masquerade status to request.user, if masquerading active. - Return string version of status of view (either 'staff' or 'student') - ''' + +def setup_masquerade(request, course_key, staff_access=False): + """ + Sets up masquerading for the current user within the current request. The + request's user is updated to have a 'masquerade_settings' attribute with + the dict of all masqueraded settings if called from within a request context. + The function then returns the CourseMasquerade object for the specified + course key, or None if there isn't one. + """ if request.user is None: return None @@ -46,20 +73,46 @@ def setup_masquerade(request, staff_access=False): if not staff_access: # can masquerade only if user has staff access to course return None - usertype = request.session.get(MASQ_KEY, '') - if usertype is None or not usertype: - request.session[MASQ_KEY] = 'staff' - usertype = 'staff' + masquerade_settings = request.session.get(MASQUERADE_SETTINGS_KEY, {}) - if usertype == 'student': - request.user.masquerade_as_student = True + # Store the masquerade settings on the user so it can be accessed without the request + request.user.masquerade_settings = masquerade_settings - return usertype + # Return the masquerade for the current course, or none if there isn't one + return masquerade_settings.get(course_key, None) -def is_masquerading_as_student(user): - ''' - Return True if user is masquerading as a student, False otherwise - ''' - masq = getattr(user, 'masquerade_as_student', False) - return masq is True +def get_course_masquerade(user, course_key): + """ + Returns the masquerade for the current user for the specified course. If no masquerade has + been installed, then a default no-op masquerade is returned. + """ + masquerade_settings = getattr(user, 'masquerade_settings', {}) + return masquerade_settings.get(course_key, None) + + +def get_masquerade_role(user, course_key): + """ + Returns the role that the user is masquerading as, or None if no masquerade is in effect. + """ + course_masquerade = get_course_masquerade(user, course_key) + return course_masquerade.role if course_masquerade else None + + +def is_masquerading_as_student(user, course_key): + """ + Returns true if the user is a staff member masquerading as a student. + """ + return get_masquerade_role(user, course_key) == 'student' + + +def get_masquerading_group_info(user, course_key): + """ + If the user is masquerading as belonging to a group, then this method returns + two values: the id of the group, and the id of the user partition that the group + belongs to. If the user is not masquerading as a group, then None is returned. + """ + course_masquerade = get_course_masquerade(user, course_key) + if not course_masquerade: + return None, None + return course_masquerade.group_id, course_masquerade.user_partition_id diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 2f84fcb155..3d2d6daf53 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -213,7 +213,7 @@ def get_xqueue_callback_url_prefix(request): return settings.XQUEUE_INTERFACE.get('callback_url', prefix) -def get_module_for_descriptor(user, request, descriptor, field_data_cache, course_id, +def get_module_for_descriptor(user, request, descriptor, field_data_cache, course_key, position=None, wrap_xmodule_display=True, grade_bucket_type=None, static_asset_path=''): """ @@ -221,10 +221,6 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours See get_module() docstring for further details. """ - # allow course staff to masquerade as student - if has_access(user, 'staff', descriptor, course_id): - setup_masquerade(request, True) - track_function = make_track_function(request) xqueue_callback_url_prefix = get_xqueue_callback_url_prefix(request) @@ -234,7 +230,7 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours user=user, descriptor=descriptor, field_data_cache=field_data_cache, - course_id=course_id, + course_id=course_key, track_function=track_function, xqueue_callback_url_prefix=xqueue_callback_url_prefix, position=position, @@ -782,6 +778,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user): user, descriptor ) + setup_masquerade(request, course_id, has_access(user, 'staff', descriptor, course_id)) instance = get_module(user, request, usage_key, field_data_cache, grade_bucket_type='ajax') if instance is None: # Either permissions just changed, or someone is trying to be clever diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 105171c71b..f7a5178685 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -6,6 +6,7 @@ from mock import Mock, patch from opaque_keys.edx.locations import SlashSeparatedCourseKey import courseware.access as access +from courseware.masquerade import CourseMasquerade from courseware.tests.factories import UserFactory, StaffFactory, InstructorFactory from student.tests.factories import AnonymousUserFactory, CourseEnrollmentAllowedFactory from xmodule.course_module import ( @@ -255,6 +256,14 @@ class UserRoleTestCase(TestCase): self.course_staff = StaffFactory(course_key=self.course_key) self.course_instructor = InstructorFactory(course_key=self.course_key) + def _install_masquerade(self, user, role='student'): + """ + Installs a masquerade for the specified user. + """ + user.masquerade_settings = { + self.course_key: CourseMasquerade(self.course_key, role=role) + } + def test_user_role_staff(self): """Ensure that user role is student for staff masqueraded as student.""" self.assertEqual( @@ -262,7 +271,7 @@ class UserRoleTestCase(TestCase): access.get_user_role(self.course_staff, self.course_key) ) # Masquerade staff - self.course_staff.masquerade_as_student = True + self._install_masquerade(self.course_staff) self.assertEqual( 'student', access.get_user_role(self.course_staff, self.course_key) @@ -275,7 +284,7 @@ class UserRoleTestCase(TestCase): access.get_user_role(self.course_instructor, self.course_key) ) # Masquerade instructor - self.course_instructor.masquerade_as_student = True + self._install_masquerade(self.course_instructor) self.assertEqual( 'student', access.get_user_role(self.course_instructor, self.course_key) diff --git a/lms/djangoapps/courseware/tests/test_masquerade.py b/lms/djangoapps/courseware/tests/test_masquerade.py index 6a567cb466..7b838f6cb1 100644 --- a/lms/djangoapps/courseware/tests/test_masquerade.py +++ b/lms/djangoapps/courseware/tests/test_masquerade.py @@ -1,113 +1,250 @@ """ -Unit tests for masquerade - -Based on (and depends on) unit tests for courseware. - -Notes for running by hand: - -./manage.py lms --settings test test lms/djangoapps/courseware +Unit tests for masquerade. """ import json +from mock import patch +from datetime import datetime from django.core.urlresolvers import reverse from django.test.utils import override_settings -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from django.utils.timezone import UTC +from capa.tests.response_xml_factory import OptionResponseXMLFactory +from courseware.masquerade import handle_ajax, setup_masquerade, get_masquerading_group_info from courseware.tests.factories import StaffFactory -from courseware.tests.helpers import LoginEnrollmentTestCase -from lms.djangoapps.lms_xblock.runtime import quote_slashes -from xmodule.modulestore.django import modulestore, clear_existing_modulestores -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_GRADED_MODULESTORE +from courseware.tests.helpers import LoginEnrollmentTestCase, get_request_for_user +from student.tests.factories import UserFactory +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MOCK_MODULESTORE +from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory +from xmodule.partitions.partitions import Group, UserPartition -# TODO: the "abtest" node in the sample course "graded" is currently preventing -# it from being successfully loaded in the mongo modulestore. -# Fix this testcase class to not depend on that course, and let it use -# the mocked modulestore instead of the XML. -@override_settings(MODULESTORE=TEST_DATA_MIXED_GRADED_MODULESTORE) -class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase): +@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) +class MasqueradeTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): """ - Check for staff being able to masquerade as student. + Base class for masquerade tests that sets up a test course and enrolls a user in the course. """ - def setUp(self): - # Clear out the modulestores, causing them to reload - clear_existing_modulestores() - self.graded_course = modulestore().get_course(SlashSeparatedCourseKey("edX", "graded", "2012_Fall")) + # By default, tests run with DISABLE_START_DATES=True. To test that masquerading as a student is + # working properly, we must use start dates and set a start date in the past (otherwise the access + # checks exist prematurely). + self.course = CourseFactory.create(number='masquerade-test', metadata={'start': datetime.now(UTC())}) + self.chapter = ItemFactory.create( + parent_location=self.course.location, + category="chapter", + display_name="Test Section", + ) + self.sequential_display_name = "Test Masquerade Subsection" + self.sequential = ItemFactory.create( + parent_location=self.chapter.location, + category="sequential", + display_name=self.sequential_display_name, + ) + self.vertical = ItemFactory.create( + parent_location=self.sequential.location, + category="vertical", + display_name="Test Unit", + ) + problem_xml = OptionResponseXMLFactory().build_xml( + question_text='The correct answer is Correct', + num_inputs=2, + weight=2, + options=['Correct', 'Incorrect'], + correct_option='Correct' + ) + self.problem_display_name = "Test Masquerade Problem" + self.problem = ItemFactory.create( + parent_location=self.vertical.location, + category='problem', + data=problem_xml, + display_name=self.problem_display_name + ) + self.test_user = self.create_user() + self.login(self.test_user.email, 'test') + self.enroll(self.course, True) - # Create staff account - self.staff = StaffFactory(course_key=self.graded_course.id) + def get_courseware_page(self): + """ + Returns the server response for the courseware page. + """ + url = reverse( + 'courseware_section', + kwargs={ + 'course_id': unicode(self.course.id), + 'chapter': self.chapter.location.name, + 'section': self.sequential.location.name, + } + ) + return self.client.get(url) - self.logout() - # self.staff.password is the sha hash but login takes the plain text - self.login(self.staff.email, 'test') - self.enroll(self.graded_course) + def _create_mock_json_request(self, user, body, method='POST', session=None): + """ + Returns a mock JSON request for the specified user + """ + request = get_request_for_user(user) + request.method = method + request.META = {'CONTENT_TYPE': ['application/json']} + request.body = body + request.session = session or {} + return request - def get_cw_section(self): - url = reverse('courseware_section', - kwargs={'course_id': self.graded_course.id.to_deprecated_string(), - 'chapter': 'GradedChapter', - 'section': 'Homework1'}) + def verify_staff_debug_present(self, staff_debug_expected): + """ + Verifies that the staff debug control visibility is as expected (for staff only). + """ + content = self.get_courseware_page().content + self.assertTrue(self.sequential_display_name in content, "Subsection should be visible") + self.assertEqual(staff_debug_expected, 'Staff Debug Info' in content) - resp = self.client.get(url) + def get_problem(self): + """ + Returns the JSON content for the problem in the course. + """ + problem_url = reverse( + 'xblock_handler', + kwargs={ + 'course_id': unicode(self.course.id), + 'usage_id': unicode(self.problem.location), + 'handler': 'xmodule_handler', + 'suffix': 'problem_get' + } + ) + return self.client.get(problem_url) - print "url ", url - return resp + def verify_show_answer_present(self, show_answer_expected): + """ + Verifies that "Show Answer" is only present when expected (for staff only). + """ + problem_html = json.loads(self.get_problem().content)['html'] + self.assertTrue(self.problem_display_name in problem_html) + self.assertEqual(show_answer_expected, "Show Answer" in problem_html) - def test_staff_debug_for_staff(self): - resp = self.get_cw_section() - sdebug = 'Staff Debug Info' - print resp.content - self.assertTrue(sdebug in resp.content) - def toggle_masquerade(self): +class NormalStudentVisibilityTest(MasqueradeTestCase): + """ + Verify the course displays as expected for a "normal" student (to ensure test setup is correct). + """ + def create_user(self): + """ + Creates a normal student user. + """ + return UserFactory() + + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + def test_staff_debug_not_visible(self): + """ + Tests that staff debug control is not present for a student. + """ + self.verify_staff_debug_present(False) + + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + def test_show_answer_not_visible(self): + """ + Tests that "Show Answer" is not visible for a student. + """ + self.verify_show_answer_present(False) + + +class StaffMasqueradeTestCase(MasqueradeTestCase): + """ + Base class for tests of the masquerade behavior for a staff member. + """ + def create_user(self): + """ + Creates a staff user. + """ + return StaffFactory(course_key=self.course.id) + + def update_masquerade(self, role, group_id=None): """ Toggle masquerade state. """ - masq_url = reverse('masquerade-switch', kwargs={'marg': 'toggle'}) - print "masq_url ", masq_url - resp = self.client.get(masq_url) - return resp + masquerade_url = reverse( + 'masquerade_update', + kwargs={ + 'course_key_string': unicode(self.course.id), + } + ) + response = self.client.post( + masquerade_url, + json.dumps({"role": role, "group_id": group_id}), + "application/json" + ) + self.assertEqual(response.status_code, 204) + return response - def test_no_staff_debug_for_student(self): - togresp = self.toggle_masquerade() - print "masq now ", togresp.content - self.assertEqual(togresp.content, '{"status": "student"}', '') - resp = self.get_cw_section() - sdebug = 'Staff Debug Info' +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): + """ + 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) - self.assertFalse(sdebug in resp.content) + # Toggle masquerade to student + self.update_masquerade(role='student') + self.verify_staff_debug_present(False) - def get_problem(self): - pun = 'H1P1' - problem_location = self.graded_course.id.make_usage_key("problem", pun) + # Toggle masquerade back to staff + self.update_masquerade(role='staff') + self.verify_staff_debug_present(True) - modx_url = reverse('xblock_handler', - kwargs={'course_id': self.graded_course.id.to_deprecated_string(), - 'usage_id': quote_slashes(problem_location.to_deprecated_string()), - 'handler': 'xmodule_handler', - 'suffix': 'problem_get'}) + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + def test_show_answer_for_staff(self): + """ + 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) - resp = self.client.get(modx_url) + # Toggle masquerade to student + self.update_masquerade(role='student') + self.verify_show_answer_present(False) - print "modx_url ", modx_url - return resp + # Toggle masquerade back to staff + self.update_masquerade(role='staff') + self.verify_show_answer_present(True) - def test_showanswer_for_staff(self): - resp = self.get_problem() - html = json.loads(resp.content)['html'] - print html - sabut = '' - self.assertTrue(sabut in html) - def test_no_showanswer_for_student(self): - togresp = self.toggle_masquerade() - print "masq now ", togresp.content - self.assertEqual(togresp.content, '{"status": "student"}', '') +class TestGetMasqueradingGroupId(StaffMasqueradeTestCase): + """ + Check for staff being able to masquerade as belonging to a group. + """ + def setUp(self): + super(TestGetMasqueradingGroupId, self).setUp() + self.user_partition = UserPartition( + 0, 'Test User Partition', '', + [Group(0, 'Group 1'), Group(1, 'Group 2')], + scheme_id='cohort' + ) + self.course.user_partitions.append(self.user_partition) + modulestore().update_item(self.course, self.test_user.id) - resp = self.get_problem() - html = json.loads(resp.content)['html'] - sabut = '' - self.assertFalse(sabut in html) + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + def test_group_masquerade(self): + """ + Tests that a staff member can masquerade as being in a particular group. + """ + # Verify that there is no masquerading group initially + group_id, user_partition_id = get_masquerading_group_info(self.test_user, self.course.id) + self.assertIsNone(group_id) + self.assertIsNone(user_partition_id) + + # Install a masquerading group + request = self._create_mock_json_request( + self.test_user, + body='{"role": "student", "user_partition_id": 0, "group_id": 1}' + ) + handle_ajax(request, unicode(self.course.id)) + setup_masquerade(request, self.test_user, True) + + # Verify that the masquerading group is returned + group_id, user_partition_id = get_masquerading_group_info(self.test_user, self.course.id) + self.assertEqual(group_id, 1) + self.assertEqual(user_partition_id, 0) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 30347abc23..e769e97082 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -354,7 +354,7 @@ def _index_bulk_op(request, course_key, chapter, section, position): if survey.utils.must_answer_survey(course, user): return redirect(reverse('course_survey', args=[unicode(course.id)])) - masq = setup_masquerade(request, staff_access) + masquerade = setup_masquerade(request, course_key, staff_access) try: field_data_cache = FieldDataCache.cache_for_descriptor_descendents( @@ -377,7 +377,7 @@ def _index_bulk_op(request, course_key, chapter, section, position): 'fragment': Fragment(), 'staff_access': staff_access, 'studio_url': studio_url, - 'masquerade': masq, + 'masquerade': masquerade, 'xqa_server': settings.FEATURES.get('USE_XQA_SERVER', 'http://xqa:server@content-qa.mitx.mit.edu/xqa'), 'reverifications': fetch_reverify_banner_info(request, course_key), } @@ -419,8 +419,8 @@ def _index_bulk_op(request, course_key, chapter, section, position): chapter_module = course_module.get_child_by(lambda m: m.location.name == chapter) if chapter_module is None: # User may be trying to access a chapter that isn't live yet - if masq == 'student': # if staff is masquerading as student be kinder, don't 404 - log.debug('staff masq as student: no chapter %s' % chapter) + if masquerade and masquerade.role == 'student': # if staff is masquerading as student be kinder, don't 404 + log.debug('staff masquerading as student: no chapter %s', chapter) return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) raise Http404 @@ -429,8 +429,8 @@ def _index_bulk_op(request, course_key, chapter, section, position): if section_descriptor is None: # Specifically asked-for section doesn't exist - if masq == 'student': # if staff is masquerading as student be kinder, don't 404 - log.debug('staff masq as student: no section %s' % section) + if masquerade and masquerade.role == 'student': # don't 404 if staff is masquerading as student + log.debug('staff masquerading as student: no section %s', section) return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) raise Http404 @@ -625,7 +625,7 @@ def course_info(request, course_id): return redirect(reverse('course_survey', args=[unicode(course.id)])) staff_access = has_access(request.user, 'staff', course) - masq = setup_masquerade(request, staff_access) # allow staff to toggle masquerade on info page + masquerade = setup_masquerade(request, course_key, staff_access) # allow staff to masquerade on the info page reverifications = fetch_reverify_banner_info(request, course_key) studio_url = get_studio_url(course, 'course_info') @@ -643,7 +643,7 @@ def course_info(request, course_id): 'cache': None, 'course': course, 'staff_access': staff_access, - 'masquerade': masq, + 'masquerade': masquerade, 'studio_url': studio_url, 'reverifications': reverifications, 'show_enroll_banner': show_enroll_banner, diff --git a/lms/lib/xblock/test/test_mixin.py b/lms/lib/xblock/test/test_mixin.py index 642aa3d3c8..4fda7a5401 100644 --- a/lms/lib/xblock/test/test_mixin.py +++ b/lms/lib/xblock/test/test_mixin.py @@ -97,44 +97,6 @@ class XBlockValidationTest(LmsXBlockMixinTestCase): ) -class XBlockGroupAccessTest(LmsXBlockMixinTestCase): - """ - Unit tests for XBlock group access. - """ - def setUp(self): - super(XBlockGroupAccessTest, self).setUp() - self.build_course() - - def test_is_visible_to_group(self): - """ - Test the behavior of is_visible_to_group. - """ - # All groups are visible for an unrestricted xblock - self.assertTrue(self.video.is_visible_to_group(self.user_partition, self.group1)) - self.assertTrue(self.video.is_visible_to_group(self.user_partition, self.group2)) - - # Verify that all groups are visible if the set of group ids is empty - self.video.group_access[self.user_partition.id] = [] # pylint: disable=no-member - self.assertTrue(self.video.is_visible_to_group(self.user_partition, self.group1)) - self.assertTrue(self.video.is_visible_to_group(self.user_partition, self.group2)) - - # Verify that only specified groups are visible - self.video.group_access[self.user_partition.id] = [self.group1.id] # pylint: disable=no-member - self.assertTrue(self.video.is_visible_to_group(self.user_partition, self.group1)) - self.assertFalse(self.video.is_visible_to_group(self.user_partition, self.group2)) - - # Verify that having an invalid user partition does not affect group visibility of other partitions - self.video.group_access[999] = [self.group1.id] - self.assertTrue(self.video.is_visible_to_group(self.user_partition, self.group1)) - self.assertFalse(self.video.is_visible_to_group(self.user_partition, self.group2)) - - # Verify that group access is still correct even with invalid group ids - self.video.group_access.clear() - self.video.group_access[self.user_partition.id] = [self.group2.id, 999] # pylint: disable=no-member - self.assertFalse(self.video.is_visible_to_group(self.user_partition, self.group1)) - self.assertTrue(self.video.is_visible_to_group(self.user_partition, self.group2)) - - class OpenAssessmentBlockMixinTestCase(ModuleStoreTestCase): """ Tests for OpenAssessmentBlock mixin. diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index c60e6673b5..32606658d8 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -14,13 +14,19 @@ def url_class(is_active): %> <%! from xmodule.tabs import CourseTabList %> <%! from courseware.access import has_access %> +<%! from courseware.masquerade import get_course_masquerade %> +<%! from courseware.views import notification_image_for_tab %> <%! from django.conf import settings %> <%! from django.core.urlresolvers import reverse %> <%! from django.utils.translation import ugettext as _ %> -<%! from courseware.views import notification_image_for_tab %> -<%! from student.models import CourseEnrollment%> +<%! from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition %> +<%! from student.models import CourseEnrollment %> <% user_is_enrolled = user.is_authenticated() and CourseEnrollment.is_enrolled(user, course.id) + cohorted_user_partition = get_cohorted_user_partition(course.id) + show_preview_menu = staff_access and active_page in ['courseware', 'info'] + is_student_masquerade = masquerade and masquerade.role == 'student' + masquerade_group_id = masquerade.group_id if masquerade else None %> % if show_preview_menu: @@ -72,51 +78,45 @@ def url_class(is_active): % endfor - <%block name="extratabs" /> - % if masquerade is not UNDEFINED: - % if staff_access and masquerade is not None: -
  • ${_("Staff view")}
    • - % endif - % endif %endif -% if masquerade is not UNDEFINED: - % if staff_access and masquerade is not None: +% if show_preview_menu: - % endif % endif diff --git a/lms/templates/courseware/info.html b/lms/templates/courseware/info.html index 9b8b5b5e28..730175cde0 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -47,12 +47,10 @@ $(document).ready(function(){
    % if user.is_authenticated():
    - % if staff_access and masquerade is not UNDEFINED and studio_url is not None: - % if masquerade == 'staff': -
    - ${_("View Updates in Studio")} -
    - % endif + % if studio_url is not None and masquerade and masquerade.role == 'staff': +
    + ${_("View Updates in Studio")} +
    % endif

    ${_("Course Updates & News")}

    diff --git a/lms/urls.py b/lms/urls.py index 4af2de7571..aeac3f6ed2 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -389,7 +389,8 @@ if settings.COURSEWARE_ENABLED: # allow course staff to change to student view of courseware if settings.FEATURES.get('ENABLE_MASQUERADE'): urlpatterns += ( - url(r'^masquerade/(?P.*)$', 'courseware.masquerade.handle_ajax', name="masquerade-switch"), + url(r'^courses/{}/masquerade$'.format(settings.COURSE_KEY_PATTERN), + 'courseware.masquerade.handle_ajax', name="masquerade_update"), ) # discussion forums live within courseware, so courseware must be enabled first diff --git a/openedx/core/djangoapps/course_groups/cohorts.py b/openedx/core/djangoapps/course_groups/cohorts.py index 7fba71cafa..8d0b096271 100644 --- a/openedx/core/djangoapps/course_groups/cohorts.py +++ b/openedx/core/djangoapps/course_groups/cohorts.py @@ -16,6 +16,7 @@ from eventtracking import tracker from student.models import get_user_by_username_or_email from .models import CourseUserGroup, CourseUserGroupPartitionGroup + log = logging.getLogger(__name__) @@ -381,15 +382,15 @@ def add_user_to_cohort(cohort, username_or_email): return (user, previous_cohort_name) -def get_partition_group_id_for_cohort(cohort): +def get_group_info_for_cohort(cohort): """ - Get the ids of the partition and group to which this cohort has been linked + Get the ids of the group and partition to which this cohort has been linked as a tuple of (int, int). - If the cohort has not been linked to any partition/group, both values in the + If the cohort has not been linked to any group/partition, both values in the tuple will be None. """ res = CourseUserGroupPartitionGroup.objects.filter(course_user_group=cohort) if len(res): - return res[0].partition_id, res[0].group_id + return res[0].group_id, res[0].partition_id return None, None diff --git a/openedx/core/djangoapps/course_groups/partition_scheme.py b/openedx/core/djangoapps/course_groups/partition_scheme.py index 5ab30fc5d1..bb1530ac38 100644 --- a/openedx/core/djangoapps/course_groups/partition_scheme.py +++ b/openedx/core/djangoapps/course_groups/partition_scheme.py @@ -3,9 +3,12 @@ Provides a UserPartition driver for cohorts. """ import logging +from courseware import courses +from courseware.masquerade import get_masquerading_group_info from xmodule.partitions.partitions import NoSuchUserPartitionGroupError -from .cohorts import get_cohort, get_partition_group_id_for_cohort +from .cohorts import get_cohort, get_group_info_for_cohort + log = logging.getLogger(__name__) @@ -17,8 +20,9 @@ class CohortPartitionScheme(object): Groups. """ + # pylint: disable=unused-argument @classmethod - def get_group_for_user(cls, course_id, user, user_partition, track_function=None): + def get_group_for_user(cls, course_key, user, user_partition, track_function=None): """ Returns the Group from the specified user partition to which the user is assigned, via their cohort membership and any mappings from cohorts @@ -32,12 +36,22 @@ class CohortPartitionScheme(object): If the user has no cohort mapping, or there is no (valid) cohort -> partition group mapping found, the function returns None. """ - cohort = get_cohort(user, course_id) + # If the current user is masquerading as being in a group belonging to the + # specified user partition then return the masquerading group. + group_id, user_partition_id = get_masquerading_group_info(user, course_key) + if group_id is not None and user_partition_id == user_partition.id: + try: + return user_partition.get_group(group_id) + except NoSuchUserPartitionGroupError: + # If the group no longer exists then the masquerade is not in effect + pass + + cohort = get_cohort(user, course_key) if cohort is None: # student doesn't have a cohort return None - partition_id, group_id = get_partition_group_id_for_cohort(cohort) + group_id, partition_id = get_group_info_for_cohort(cohort) if partition_id is None: # cohort isn't mapped to any partition group. return None @@ -75,3 +89,17 @@ class CohortPartitionScheme(object): ) # fail silently return None + + +def get_cohorted_user_partition(course_key): + """ + Returns the first user partition from the specified course which uses the CohortPartitionScheme, + or None if one is not found. Note that it is currently recommended that each course have only + one cohorted user partition. + """ + course = courses.get_course_by_id(course_key) + for user_partition in course.user_partitions: + if user_partition.scheme == CohortPartitionScheme: + return user_partition + + return None diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index 1707dd0530..aed51f7a23 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -623,13 +623,13 @@ class TestCohortsAndPartitionGroups(TestCase): link.save() return link - def test_get_partition_group_id_for_cohort(self): + def test_get_group_info_for_cohort(self): """ - Basic test of the partition_group_id accessor function + Basic test of the partition_group_info accessor function """ # api should return nothing for an unmapped cohort self.assertEqual( - cohorts.get_partition_group_id_for_cohort(self.first_cohort), + cohorts.get_group_info_for_cohort(self.first_cohort), (None, None), ) # create a link for the cohort in the db @@ -640,14 +640,14 @@ class TestCohortsAndPartitionGroups(TestCase): ) # api should return the specified partition and group self.assertEqual( - cohorts.get_partition_group_id_for_cohort(self.first_cohort), - (self.partition_id, self.group1_id) + cohorts.get_group_info_for_cohort(self.first_cohort), + (self.group1_id, self.partition_id) ) # delete the link in the db link.delete() # api should return nothing again self.assertEqual( - cohorts.get_partition_group_id_for_cohort(self.first_cohort), + cohorts.get_group_info_for_cohort(self.first_cohort), (None, None), ) @@ -666,12 +666,12 @@ class TestCohortsAndPartitionGroups(TestCase): self.group1_id, ) self.assertEqual( - cohorts.get_partition_group_id_for_cohort(self.first_cohort), - (self.partition_id, self.group1_id), + cohorts.get_group_info_for_cohort(self.first_cohort), + (self.group1_id, self.partition_id), ) self.assertEqual( - cohorts.get_partition_group_id_for_cohort(self.second_cohort), - cohorts.get_partition_group_id_for_cohort(self.first_cohort), + cohorts.get_group_info_for_cohort(self.second_cohort), + cohorts.get_group_info_for_cohort(self.first_cohort), ) def test_multiple_partition_groups(self): @@ -701,14 +701,14 @@ class TestCohortsAndPartitionGroups(TestCase): self.group1_id ) self.assertEqual( - cohorts.get_partition_group_id_for_cohort(self.first_cohort), - (self.partition_id, self.group1_id) + cohorts.get_group_info_for_cohort(self.first_cohort), + (self.group1_id, self.partition_id) ) # delete the link self.first_cohort.delete() # api should return nothing at that point self.assertEqual( - cohorts.get_partition_group_id_for_cohort(self.first_cohort), + cohorts.get_group_info_for_cohort(self.first_cohort), (None, None), ) # link should no longer exist because of delete cascade diff --git a/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py b/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py index 43817268d7..7891be1844 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py +++ b/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py @@ -3,18 +3,23 @@ Test the partitions and partitions service """ +import json from django.conf import settings import django.test from django.test.utils import override_settings from mock import patch +from unittest import skipUnless +from courseware.masquerade import handle_ajax, setup_masquerade +from courseware.tests.test_masquerade import StaffMasqueradeTestCase from student.tests.factories import UserFactory from xmodule.partitions.partitions import Group, UserPartition, UserPartitionError from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.tests.django_utils import mixed_store_config from opaque_keys.edx.locations import SlashSeparatedCourseKey -from ..partition_scheme import CohortPartitionScheme +from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme +from ..partition_scheme import CohortPartitionScheme, get_cohorted_user_partition from ..models import CourseUserGroupPartitionGroup from ..cohorts import add_user_to_cohort, get_course_cohorts from .helpers import CohortFactory, config_course_cohorts @@ -280,3 +285,111 @@ class TestExtension(django.test.TestCase): self.assertEqual(UserPartition.get_scheme('cohort'), CohortPartitionScheme) with self.assertRaisesRegexp(UserPartitionError, 'Unrecognized scheme'): UserPartition.get_scheme('other') + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class TestGetCohortedUserPartition(django.test.TestCase): + """ + Test that `get_cohorted_user_partition` returns the first user_partition with scheme `CohortPartitionScheme`. + """ + def setUp(self): + """ + Regenerate a course with cohort configuration, partition and groups, + and a student for each test. + """ + self.course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall") + self.course = modulestore().get_course(self.course_key) + self.student = UserFactory.create() + + self.random_user_partition = UserPartition( + 1, + 'Random Partition', + 'Should not be returned', + [Group(0, 'Group 0'), Group(1, 'Group 1')], + scheme=RandomUserPartitionScheme + ) + + self.cohort_user_partition = UserPartition( + 0, + 'Cohort Partition 1', + 'Should be returned', + [Group(10, 'Group 10'), Group(20, 'Group 20')], + scheme=CohortPartitionScheme + ) + + self.second_cohort_user_partition = UserPartition( + 2, + 'Cohort Partition 2', + 'Should not be returned', + [Group(10, 'Group 10'), Group(1, 'Group 1')], + scheme=CohortPartitionScheme + ) + + def test_returns_first_cohort_user_partition(self): + """ + Test get_cohorted_user_partition returns first user_partition with scheme `CohortPartitionScheme`. + """ + self.course.user_partitions.append(self.random_user_partition) + self.course.user_partitions.append(self.cohort_user_partition) + self.course.user_partitions.append(self.second_cohort_user_partition) + self.assertEqual(self.cohort_user_partition, get_cohorted_user_partition(self.course_key)) + + def test_no_cohort_user_partitions(self): + """ + Test get_cohorted_user_partition returns None when there are no cohorted user partitions. + """ + self.course.user_partitions.append(self.random_user_partition) + self.assertIsNone(get_cohorted_user_partition(self.course_key)) + + +class TestMasqueradedGroup(StaffMasqueradeTestCase): + """ + Check for staff being able to masquerade as belonging to a group. + """ + def setUp(self): + super(TestMasqueradedGroup, self).setUp() + self.user_partition = UserPartition( + 0, 'Test User Partition', '', + [Group(0, 'Group 1'), Group(1, 'Group 2')], + scheme_id='cohort' + ) + self.course.user_partitions.append(self.user_partition) + self.session = {} + modulestore().update_item(self.course, self.test_user.id) + + def _verify_masquerade_for_group(self, group): + """ + Verify that the masquerade works for the specified group id. + """ + # Send the request to set the masquerade + request_json = { + "role": "student", + } + if group and self.user_partition: + request_json['user_partition_id'] = self.user_partition.id + request_json['group_id'] = group.id + request = self._create_mock_json_request( + self.test_user, + body=json.dumps(request_json), + session=self.session + ) + handle_ajax(request, unicode(self.course.id)) + + # Now setup the masquerade for the test user + setup_masquerade(request, self.test_user, True) + scheme = self.user_partition.scheme # pylint: disable=no-member + self.assertEqual( + scheme.get_group_for_user(self.course.id, self.test_user, self.user_partition), + group + ) + + @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + def test_group_masquerade(self): + """ + Tests that a staff member can masquerade as being in a particular group. + """ + self._verify_masquerade_for_group(self.user_partition.groups[0]) + self._verify_masquerade_for_group(self.user_partition.groups[1]) + self._verify_masquerade_for_group(None) + diff --git a/openedx/core/djangoapps/user_api/partition_schemes.py b/openedx/core/djangoapps/user_api/partition_schemes.py index 0e30f195bd..756adc4049 100644 --- a/openedx/core/djangoapps/user_api/partition_schemes.py +++ b/openedx/core/djangoapps/user_api/partition_schemes.py @@ -17,13 +17,13 @@ class RandomUserPartitionScheme(object): RANDOM = random.Random() @classmethod - def get_group_for_user(cls, course_id, user, user_partition, assign=True, track_function=None): + def get_group_for_user(cls, course_key, user, user_partition, assign=True, track_function=None): """ Returns the group from the specified user position to which the user is assigned. If the user has not yet been assigned, a group will be randomly chosen for them if assign flag is True. """ partition_key = cls._key_for_partition(user_partition) - group_id = course_tag_api.get_course_tag(user, course_id, partition_key) + group_id = course_tag_api.get_course_tag(user, course_key, partition_key) group = None if group_id is not None: @@ -52,7 +52,7 @@ class RandomUserPartitionScheme(object): group = cls.RANDOM.choice(user_partition.groups) # persist the value as a course tag - course_tag_api.set_course_tag(user, course_id, partition_key, group.id) + course_tag_api.set_course_tag(user, course_key, partition_key, group.id) if track_function: # emit event for analytics