Commit edd833eb by Daniel Friedman

Fix grades download in courses with cohorted content

TNL-1351
parent 0f92b092
...@@ -20,6 +20,7 @@ from pytz import UTC ...@@ -20,6 +20,7 @@ from pytz import UTC
from track.views import task_track from track.views import task_track
from util.file import course_filename_prefix_generator, UniversalNewlineIterator from util.file import course_filename_prefix_generator, UniversalNewlineIterator
from xmodule.modulestore.django import modulestore 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.courses import get_course_by_id
from courseware.grades import iterate_grades_for from courseware.grades import iterate_grades_for
...@@ -553,9 +554,8 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, ...@@ -553,9 +554,8 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input,
course = get_course_by_id(course_id) course = get_course_by_id(course_id)
cohorts_header = ['Cohort Name'] if course.is_cohorted else [] cohorts_header = ['Cohort Name'] if course.is_cohorted else []
partition_service = LmsPartitionService(user=None, course_id=course_id) experiment_partitions = get_split_user_partitions(course.user_partitions)
partitions = partition_service.course_partitions group_configs_header = [u'Experiment Group ({})'.format(partition.name) for partition in experiment_partitions]
group_configs_header = ['Experiment Group ({})'.format(partition.name) for partition in partitions]
# Loop over all our students and build our CSV lists in memory # Loop over all our students and build our CSV lists in memory
header = None header = None
...@@ -589,7 +589,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, ...@@ -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 '') cohorts_group_name.append(group.name if group else '')
group_configs_group_names = [] group_configs_group_names = []
for partition in partitions: for partition in experiment_partitions:
group = LmsPartitionService(student, course_id).get_group(partition, assign=False) group = LmsPartitionService(student, course_id).get_group(partition, assign=False)
group_configs_group_names.append(group.name if group else '') group_configs_group_names.append(group.name if group else '')
......
...@@ -16,7 +16,10 @@ from student.tests.factories import UserFactory ...@@ -16,7 +16,10 @@ from student.tests.factories import UserFactory
from student.models import CourseEnrollment from student.models import CourseEnrollment
from xmodule.partitions.partitions import Group, UserPartition 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 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.models import ReportStore
from instructor_task.tasks_helper import cohort_students_and_upload, upload_grades_csv, upload_students_csv 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 from instructor_task.tests.test_base import InstructorTaskCourseTestCase, TestReportMixin
...@@ -64,11 +67,10 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): ...@@ -64,11 +67,10 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase):
report_store = ReportStore.from_config() report_store = ReportStore.from_config()
self.assertTrue(any('grade_report_err' in item[0] for item in report_store.links_for(self.course.id))) 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'): with patch('instructor_task.tasks_helper._get_current_task'):
result = upload_grades_csv(None, None, course_id, None, 'graded') result = upload_grades_csv(None, None, course_id, None, 'graded')
self.assertDictContainsSubset({'attempted': 2, 'succeeded': 2, 'failed': 0}, result) self.assertDictContainsSubset({'attempted': 2, 'succeeded': 2, 'failed': 0}, result)
...@@ -76,9 +78,8 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): ...@@ -76,9 +78,8 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase):
report_csv_filename = report_store.links_for(course_id)[0][0] 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: with open(report_store.path_to(course_id, report_csv_filename)) as csv_file:
for row in unicodecsv.DictReader(csv_file): for row in unicodecsv.DictReader(csv_file):
cohort_groups_in_csv.append(row['Cohort Name']) if row.get('username') == username:
self.assertEqual(row[column_header], expected_cell_content)
self.assertEqual(cohort_groups_in_csv, expected_cohort_groups)
def test_cohort_data_in_grading(self): def test_cohort_data_in_grading(self):
""" """
...@@ -87,20 +88,22 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): ...@@ -87,20 +88,22 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase):
cohort_groups = ['cohort 1', 'cohort 2'] cohort_groups = ['cohort 1', 'cohort 2']
course = CourseFactory.create(cohort_config={'cohorted': True, 'auto_cohort': True, course = CourseFactory.create(cohort_config={'cohorted': True, 'auto_cohort': True,
'auto_cohort_groups': cohort_groups}) '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 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 # 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 # user rather than creating a group automatically at runtime
expected_groups = ['', ''] self._verify_cell_data_for_user(user_1, course.id, 'Cohort Name', '')
self._verify_cohort_data(course.id, expected_groups) self._verify_cell_data_for_user(user_2, course.id, 'Cohort Name', '')
def test_unicode_cohort_data_in_grading(self): def test_unicode_cohort_data_in_grading(self):
""" """
Test that cohorts can contain unicode characters. 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}) course = CourseFactory.create(cohort_config={'cohorted': True})
# Create users and manually assign cohorts # Create users and manually assign cohorts
...@@ -108,12 +111,15 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): ...@@ -108,12 +111,15 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase):
user2 = UserFactory.create(username='user2') user2 = UserFactory.create(username='user2')
CourseEnrollment.enroll(user1, course.id) CourseEnrollment.enroll(user1, course.id)
CourseEnrollment.enroll(user2, course.id) CourseEnrollment.enroll(user2, course.id)
cohort1 = CohortFactory(course_id=course.id, name=u'ÞrÖfessÖr X') professor_x = u'ÞrÖfessÖr X'
cohort2 = CohortFactory(course_id=course.id, name=u'MàgnëtÖ') 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) cohort1.users.add(user1)
cohort2.users.add(user2) 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): def test_unicode_user_partitions(self):
""" """
...@@ -140,6 +146,100 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): ...@@ -140,6 +146,100 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase):
_groups = [group.name for group in self.course.user_partitions[0].groups] _groups = [group.name for group in self.course.user_partitions[0].groups]
self.assertEqual(_groups, user_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._get_current_task')
@patch('instructor_task.tasks_helper.iterate_grades_for') @patch('instructor_task.tasks_helper.iterate_grades_for')
def test_unicode_in_csv_header(self, mock_iterate_grades_for, _mock_current_task): def test_unicode_in_csv_header(self, mock_iterate_grades_for, _mock_current_task):
......
...@@ -22,7 +22,7 @@ class RandomUserPartitionScheme(object): ...@@ -22,7 +22,7 @@ class RandomUserPartitionScheme(object):
Returns the group from the specified user position to which the user is assigned. 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. 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_id = course_tag_api.get_course_tag(user, course_key, partition_key)
group = None group = None
...@@ -72,7 +72,7 @@ class RandomUserPartitionScheme(object): ...@@ -72,7 +72,7 @@ class RandomUserPartitionScheme(object):
return group return group
@classmethod @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. Returns the key to use to look up and save the user's group for a given user partition.
""" """
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment