diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index 21822945f8..7f48999633 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -20,6 +20,7 @@ from pytz import UTC from track.views import task_track from util.file import course_filename_prefix_generator, UniversalNewlineIterator from xmodule.modulestore.django import modulestore +from xmodule.split_test_module import get_split_user_partitions from courseware.courses import get_course_by_id from courseware.grades import iterate_grades_for @@ -553,9 +554,8 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, course = get_course_by_id(course_id) cohorts_header = ['Cohort Name'] if course.is_cohorted else [] - partition_service = LmsPartitionService(user=None, course_id=course_id) - partitions = partition_service.course_partitions - group_configs_header = ['Experiment Group ({})'.format(partition.name) for partition in partitions] + experiment_partitions = get_split_user_partitions(course.user_partitions) + group_configs_header = [u'Experiment Group ({})'.format(partition.name) for partition in experiment_partitions] # Loop over all our students and build our CSV lists in memory header = None @@ -589,7 +589,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, cohorts_group_name.append(group.name if group else '') group_configs_group_names = [] - for partition in partitions: + for partition in experiment_partitions: group = LmsPartitionService(student, course_id).get_group(partition, assign=False) group_configs_group_names.append(group.name if group else '') diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index ccbf190bfa..d2224e756b 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -16,7 +16,10 @@ from student.tests.factories import UserFactory from student.models import CourseEnrollment from xmodule.partitions.partitions import Group, UserPartition +from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory +import openedx.core.djangoapps.user_api.api.course_tag as course_tag_api +from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme from instructor_task.models import ReportStore from instructor_task.tasks_helper import cohort_students_and_upload, upload_grades_csv, upload_students_csv from instructor_task.tests.test_base import InstructorTaskCourseTestCase, TestReportMixin @@ -64,11 +67,10 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): report_store = ReportStore.from_config() self.assertTrue(any('grade_report_err' in item[0] for item in report_store.links_for(self.course.id))) - def _verify_cohort_data(self, course_id, expected_cohort_groups): + def _verify_cell_data_for_user(self, username, course_id, column_header, expected_cell_content): """ - Verify cohort data. + Verify cell data in the grades CSV for a particular user. """ - cohort_groups_in_csv = [] with patch('instructor_task.tasks_helper._get_current_task'): result = upload_grades_csv(None, None, course_id, None, 'graded') self.assertDictContainsSubset({'attempted': 2, 'succeeded': 2, 'failed': 0}, result) @@ -76,9 +78,8 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): report_csv_filename = report_store.links_for(course_id)[0][0] with open(report_store.path_to(course_id, report_csv_filename)) as csv_file: for row in unicodecsv.DictReader(csv_file): - cohort_groups_in_csv.append(row['Cohort Name']) - - self.assertEqual(cohort_groups_in_csv, expected_cohort_groups) + if row.get('username') == username: + self.assertEqual(row[column_header], expected_cell_content) def test_cohort_data_in_grading(self): """ @@ -87,20 +88,22 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): cohort_groups = ['cohort 1', 'cohort 2'] course = CourseFactory.create(cohort_config={'cohorted': True, 'auto_cohort': True, 'auto_cohort_groups': cohort_groups}) - for _ in range(2): - CourseEnrollment.enroll(UserFactory.create(), course.id) + + user_1 = 'user_1' + user_2 = 'user_2' + CourseEnrollment.enroll(UserFactory.create(username=user_1), course.id) + CourseEnrollment.enroll(UserFactory.create(username=user_2), course.id) # In auto cohorting a group will be assigned to a user only when user visits a problem # In grading calculation we only add a group in csv if group is already assigned to # user rather than creating a group automatically at runtime - expected_groups = ['', ''] - self._verify_cohort_data(course.id, expected_groups) + self._verify_cell_data_for_user(user_1, course.id, 'Cohort Name', '') + self._verify_cell_data_for_user(user_2, course.id, 'Cohort Name', '') def test_unicode_cohort_data_in_grading(self): """ Test that cohorts can contain unicode characters. """ - cohort_groups = [u'ÞrÖfessÖr X', u'MàgnëtÖ'] course = CourseFactory.create(cohort_config={'cohorted': True}) # Create users and manually assign cohorts @@ -108,12 +111,15 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): user2 = UserFactory.create(username='user2') CourseEnrollment.enroll(user1, course.id) CourseEnrollment.enroll(user2, course.id) - cohort1 = CohortFactory(course_id=course.id, name=u'ÞrÖfessÖr X') - cohort2 = CohortFactory(course_id=course.id, name=u'MàgnëtÖ') + professor_x = u'ÞrÖfessÖr X' + magneto = u'MàgnëtÖ' + cohort1 = CohortFactory(course_id=course.id, name=professor_x) + cohort2 = CohortFactory(course_id=course.id, name=magneto) cohort1.users.add(user1) cohort2.users.add(user2) - self._verify_cohort_data(course.id, cohort_groups) + self._verify_cell_data_for_user(user1.username, course.id, 'Cohort Name', professor_x) + self._verify_cell_data_for_user(user2.username, course.id, 'Cohort Name', magneto) def test_unicode_user_partitions(self): """ @@ -140,6 +146,100 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): _groups = [group.name for group in self.course.user_partitions[0].groups] self.assertEqual(_groups, user_groups) + def test_cohort_scheme_partition(self): + """ + Test that cohort-schemed user partitions are ignored in the + grades export. + """ + # Set up a course with 'cohort' and 'random' user partitions. + cohort_scheme_partition = UserPartition( + 0, + 'Cohort-schemed Group Configuration', + 'Group Configuration based on Cohorts', + [Group(0, 'Group A'), Group(1, 'Group B')], + scheme_id='cohort' + ) + experiment_group_a = Group(2, u'Expériment Group A') + experiment_group_b = Group(3, u'Expériment Group B') + experiment_partition = UserPartition( + 1, + u'Content Expériment Configuration', + u'Group Configuration for Content Expériments', + [experiment_group_a, experiment_group_b], + scheme_id='random' + ) + course = CourseFactory.create( + cohort_config={'cohorted': True}, + user_partitions=[cohort_scheme_partition, experiment_partition] + ) + + # Create user_a and user_b which are enrolled in the course + # and assigned to experiment_group_a and experiment_group_b, + # respectively. + user_a = UserFactory.create(username='user_a') + user_b = UserFactory.create(username='user_b') + CourseEnrollment.enroll(user_a, course.id) + CourseEnrollment.enroll(user_b, course.id) + course_tag_api.set_course_tag( + user_a, + course.id, + RandomUserPartitionScheme.key_for_partition(experiment_partition), + experiment_group_a.id + ) + course_tag_api.set_course_tag( + user_b, + course.id, + RandomUserPartitionScheme.key_for_partition(experiment_partition), + experiment_group_b.id + ) + + # Assign user_a to a group in the 'cohort'-schemed user + # partition (by way of a cohort) to verify that the user + # partition group does not show up in the "Experiment Group" + # cell. + cohort_a = CohortFactory.create(course_id=course.id, name=u'Cohørt A', users=[user_a]) + CourseUserGroupPartitionGroup( + course_user_group=cohort_a, + partition_id=cohort_scheme_partition.id, + group_id=cohort_scheme_partition.groups[0].id + ).save() + + # Verify that we see user_a and user_b in their respective + # content experiment groups, and that we do not see any + # content groups. + experiment_group_message = u'Experiment Group ({content_experiment})' + self._verify_cell_data_for_user( + user_a.username, + course.id, + experiment_group_message.format( + content_experiment=experiment_partition.name + ), + experiment_group_a.name + ) + self._verify_cell_data_for_user( + user_b.username, + course.id, + experiment_group_message.format( + content_experiment=experiment_partition.name + ), + experiment_group_b.name + ) + + # Make sure cohort info is correct. + cohort_name_header = 'Cohort Name' + self._verify_cell_data_for_user( + user_a.username, + course.id, + cohort_name_header, + cohort_a.name + ) + self._verify_cell_data_for_user( + user_b.username, + course.id, + cohort_name_header, + '' + ) + @patch('instructor_task.tasks_helper._get_current_task') @patch('instructor_task.tasks_helper.iterate_grades_for') def test_unicode_in_csv_header(self, mock_iterate_grades_for, _mock_current_task): diff --git a/openedx/core/djangoapps/user_api/partition_schemes.py b/openedx/core/djangoapps/user_api/partition_schemes.py index 756adc4049..53c9a8046b 100644 --- a/openedx/core/djangoapps/user_api/partition_schemes.py +++ b/openedx/core/djangoapps/user_api/partition_schemes.py @@ -22,7 +22,7 @@ class RandomUserPartitionScheme(object): 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) + partition_key = cls.key_for_partition(user_partition) group_id = course_tag_api.get_course_tag(user, course_key, partition_key) group = None @@ -72,7 +72,7 @@ class RandomUserPartitionScheme(object): return group @classmethod - def _key_for_partition(cls, user_partition): + def key_for_partition(cls, user_partition): """ Returns the key to use to look up and save the user's group for a given user partition. """