diff --git a/common/test/acceptance/pages/lms/staff_view.py b/common/test/acceptance/pages/lms/staff_view.py index b4eeb5d505..b083fe76ca 100644 --- a/common/test/acceptance/pages/lms/staff_view.py +++ b/common/test/acceptance/pages/lms/staff_view.py @@ -33,6 +33,19 @@ class StaffPage(CoursewarePage): self.q(css=self.VIEW_MODE_OPTIONS_CSS).filter(lambda el: el.text == view_mode).first.click() self.wait_for_ajax() + def set_staff_view_mode_specific_student(self, username_or_email): + """ + Set the current preview mode to "Specific Student" with the given username or email + """ + required_mode = "Specific student" + if self.staff_view_mode != required_mode: + self.q(css=self.VIEW_MODE_OPTIONS_CSS).filter(lambda el: el.text == required_mode).first.click() + # Use a script here because .clear() + .send_keys() triggers unwanted behavior if a username is already set + self.browser.execute_script( + '$(".action-preview-username").val("{}").blur().change();'.format(username_or_email) + ) + self.wait_for_ajax() + def open_staff_debug_info(self): """ Open the staff debug window diff --git a/common/test/acceptance/tests/lms/test_lms_user_preview.py b/common/test/acceptance/tests/lms/test_lms_user_preview.py index 292bbf8dcf..06b42f11a3 100644 --- a/common/test/acceptance/tests/lms/test_lms_user_preview.py +++ b/common/test/acceptance/tests/lms/test_lms_user_preview.py @@ -6,8 +6,10 @@ Tests the "preview" selector in the LMS that allows changing between Staff, Stud from ..helpers import UniqueCourseTest, create_user_partition_json from ...pages.studio.auto_auth import AutoAuthPage from ...pages.lms.courseware import CoursewarePage +from ...pages.lms.instructor_dashboard import InstructorDashboardPage from ...pages.lms.staff_view import StaffPage from ...fixtures.course import CourseFixture, XBlockFixtureDesc +from bok_choy.promise import EmptyPromise from xmodule.partitions.partitions import Group from textwrap import dedent @@ -335,6 +337,44 @@ class CourseWithContentGroupsTest(StaffViewTest): course_page.set_staff_view_mode('Student in beta') verify_expected_problem_visibility(self, course_page, [self.beta_text, self.everyone_text]) + def create_cohorts_and_assign_students(self, student_a_username, student_b_username): + """ + Adds 2 manual cohorts, linked to content groups, to the course. + Each cohort is assigned one student. + """ + instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id) + instructor_dashboard_page.visit() + cohort_management_page = instructor_dashboard_page.select_cohort_management() + cohort_management_page.is_cohorted = True + + def add_cohort_with_student(cohort_name, content_group, student): + """ Create cohort and assign student to it. """ + cohort_management_page.add_cohort(cohort_name, content_group=content_group) + # After adding the cohort, it should automatically be selected + EmptyPromise( + lambda: cohort_name == cohort_management_page.get_selected_cohort(), "Waiting for new cohort" + ).fulfill() + cohort_management_page.add_students_to_selected_cohort([student]) + add_cohort_with_student("Cohort Alpha", "alpha", student_a_username) + add_cohort_with_student("Cohort Beta", "beta", student_b_username) + cohort_management_page.wait_for_ajax() + + def test_as_specific_student(self): + student_a_username = 'tass_student_a' + student_b_username = 'tass_student_b' + AutoAuthPage(self.browser, username=student_a_username, course_id=self.course_id, no_login=True).visit() + AutoAuthPage(self.browser, username=student_b_username, course_id=self.course_id, no_login=True).visit() + self.create_cohorts_and_assign_students(student_a_username, student_b_username) + + # Masquerade as student in alpha cohort: + course_page = self._goto_staff_page() + course_page.set_staff_view_mode_specific_student(student_a_username) + verify_expected_problem_visibility(self, course_page, [self.alpha_text, self.everyone_text]) + + # Masquerade as student in beta cohort: + course_page.set_staff_view_mode_specific_student(student_b_username) + verify_expected_problem_visibility(self, course_page, [self.beta_text, self.everyone_text]) + def verify_expected_problem_visibility(test, courseware_page, expected_problems): """ diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index b0bbba9ebe..5c244f18f7 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -54,8 +54,8 @@ def handle_ajax(request, 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) + user_partition_id = request_json.get('user_partition_id', None) if group_id is not None else None user_name = request_json.get('user_name', None) if user_name: users_in_course = CourseEnrollment.objects.users_enrolled_in(course_key) diff --git a/openedx/core/djangoapps/course_groups/partition_scheme.py b/openedx/core/djangoapps/course_groups/partition_scheme.py index cb352ba993..346528e825 100644 --- a/openedx/core/djangoapps/course_groups/partition_scheme.py +++ b/openedx/core/djangoapps/course_groups/partition_scheme.py @@ -4,7 +4,11 @@ Provides a UserPartition driver for cohorts. import logging from courseware import courses -from courseware.masquerade import get_masquerading_group_info +from courseware.masquerade import ( # pylint: disable=import-error + get_course_masquerade, + get_masquerading_group_info, + is_masquerading_as_specific_student, +) from xmodule.partitions.partitions import NoSuchUserPartitionGroupError from .cohorts import get_cohort, get_group_info_for_cohort @@ -36,16 +40,19 @@ class CohortPartitionScheme(object): If the user has no cohort mapping, or there is no (valid) cohort -> partition group mapping found, the function returns None. """ - # If the current user is masquerading as being in a group - # belonging to the specified user partition, return the - # masquerading group or None if the group can't be found. - group_id, user_partition_id = get_masquerading_group_info(user, course_key) - if user_partition_id == user_partition.id: - if group_id is not None: + # First, check if we have to deal with masquerading. + # If the current user is masquerading as a specific student, use the + # same logic as normal to return that student's group. If the current + # user is masquerading as a generic student in a specific group, then + # return that group. + if get_course_masquerade(user, course_key) and not is_masquerading_as_specific_student(user, course_key): + group_id, user_partition_id = get_masquerading_group_info(user, course_key) + if user_partition_id == user_partition.id and group_id is not None: try: return user_partition.get_group(group_id) except NoSuchUserPartitionGroupError: return None + # The user is masquerading as a generic student. We can't show any particular group. return None cohort = get_cohort(user, course_key, use_cached=use_cached)