From a53903c46abab80c902738ffee1885c64e08bd59 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 14 Sep 2015 09:49:18 -0400 Subject: [PATCH] Add team column to grade reports. --- lms/djangoapps/instructor/tests/test_api.py | 20 +++ lms/djangoapps/instructor/views/api.py | 4 + lms/djangoapps/instructor_analytics/basic.py | 12 ++ .../instructor_task/tasks_helper.py | 17 ++- .../tests/test_tasks_helper.py | 133 ++++++++++++++++-- lms/djangoapps/teams/tests/factories.py | 2 +- 6 files changed, 170 insertions(+), 18 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 230b0ffb4a..b2a9be1919 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -2424,6 +2424,26 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment self.assertEqual('cohort' in res_json['feature_names'], is_cohorted) + @ddt.data(True, False) + def test_get_students_features_teams(self, has_teams): + """ + Test that get_students_features includes team info when the course is + has teams enabled, and does not when the course does not have teams enabled + """ + if has_teams: + self.course = CourseFactory.create(teams_configuration={ + 'max_size': 2, 'topics': [{'topic-id': 'topic', 'name': 'Topic', 'description': 'A Topic'}] + }) + course_instructor = InstructorFactory(course_key=self.course.id) + self.client.login(username=course_instructor.username, password='test') + + url = reverse('get_students_features', kwargs={'course_id': unicode(self.course.id)}) + + response = self.client.get(url, {}) + res_json = json.loads(response.content) + + self.assertEqual('team' in res_json['feature_names'], has_teams) + def test_get_students_who_may_enroll(self): """ Test whether get_students_who_may_enroll returns an appropriate diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index c559aadccb..529dacc26a 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1138,6 +1138,10 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red query_features.append('cohort') query_features_names['cohort'] = _('Cohort') + if course.teams_enabled: + query_features.append('team') + query_features_names['team'] = _('Team') + if not csv: student_data = instructor_analytics.basic.enrolled_students_features(course_key, query_features) response_payload = { diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py index 8624d27b26..a91fdc9cde 100644 --- a/lms/djangoapps/instructor_analytics/basic.py +++ b/lms/djangoapps/instructor_analytics/basic.py @@ -39,6 +39,8 @@ AVAILABLE_FEATURES = STUDENT_FEATURES + PROFILE_FEATURES COURSE_REGISTRATION_FEATURES = ('code', 'course_id', 'created_by', 'created_at', 'is_valid') COUPON_FEATURES = ('code', 'course_id', 'percentage_discount', 'description', 'expiration_date', 'is_active') +UNAVAILABLE = "[unavailable]" + def sale_order_record_features(course_id, features): """ @@ -172,6 +174,7 @@ def enrolled_students_features(course_key, features): ] """ include_cohort_column = 'cohort' in features + include_team_column = 'team' in features students = User.objects.filter( courseenrollment__course_id=course_key, @@ -181,6 +184,9 @@ def enrolled_students_features(course_key, features): if include_cohort_column: students = students.prefetch_related('course_groups') + if include_team_column: + students = students.prefetch_related('teams') + def extract_student(student, features): """ convert student to dictionary """ student_features = [x for x in STUDENT_FEATURES if x in features] @@ -216,6 +222,12 @@ def enrolled_students_features(course_key, features): (cohort.name for cohort in student.course_groups.all() if cohort.course_id == course_key), "[unassigned]" ) + + if include_team_column: + student_dict['team'] = next( + (team.name for team in student.teams.all() if team.course_id == course_key), + UNAVAILABLE + ) return student_dict return [extract_student(student, features) for student in students] diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index 59c4463614..e494c9a2b6 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -62,6 +62,7 @@ from openedx.core.djangoapps.content.course_structures.models import CourseStruc from opaque_keys.edx.keys import UsageKey from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted from student.models import CourseEnrollment, CourseAccessRole +from teams.models import CourseTeamMembership from verify_student.models import SoftwareSecurePhotoVerification # define different loggers for use within tasks and on client side @@ -681,7 +682,9 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, course = get_course_by_id(course_id) course_is_cohorted = is_course_cohorted(course.id) + teams_enabled = course.teams_enabled cohorts_header = ['Cohort Name'] if course_is_cohorted else [] + teams_header = ['Team Name'] if teams_enabled else [] experiment_partitions = get_split_user_partitions(course.user_partitions) group_configs_header = [u'Experiment Group ({})'.format(partition.name) for partition in experiment_partitions] @@ -703,6 +706,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, task_info_string, action_name, current_step, + total_enrolled_students ) for student, gradeset, err_msg in iterate_grades_for(course_id, enrolled_students): @@ -730,7 +734,8 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, header = [section['label'] for section in gradeset[u'section_breakdown']] rows.append( ["id", "email", "username", "grade"] + header + cohorts_header + - group_configs_header + ['Enrollment Track', 'Verification Status'] + certificate_info_header + group_configs_header + teams_header + + ['Enrollment Track', 'Verification Status'] + certificate_info_header ) percents = { @@ -749,6 +754,14 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, group = LmsPartitionService(student, course_id).get_group(partition, assign=False) group_configs_group_names.append(group.name if group else '') + team_name = [] + if teams_enabled: + try: + membership = CourseTeamMembership.objects.get(user=student, team__course_id=course_id) + team_name.append(membership.team.name) + except CourseTeamMembership.DoesNotExist: + team_name.append('') + enrollment_mode = CourseEnrollment.enrollment_mode_for_user(student, course_id)[0] verification_status = SoftwareSecurePhotoVerification.verification_status_for_user( student, @@ -771,7 +784,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, row_percents = [percents.get(label, 0.0) for label in header] rows.append( [student.id, student.email, student.username, gradeset['percent']] + - row_percents + cohorts_group_name + group_configs_group_names + + row_percents + cohorts_group_name + group_configs_group_names + team_name + [enrollment_mode] + [verification_status] + certificate_info ) else: diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index c6701ebc55..7e3d7d3867 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -42,11 +42,31 @@ from instructor_task.tasks_helper import ( upload_exec_summary_report, generate_students_certificates, ) +from instructor_analytics.basic import UNAVAILABLE from openedx.core.djangoapps.util.testing import ContentGroupTestCase, TestConditionalContent +from teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory + + +class InstructorGradeReportTestCase(TestReportMixin, InstructorTaskCourseTestCase): + """ Base class for grade report tests. """ + + def _verify_cell_data_for_user(self, username, course_id, column_header, expected_cell_content): + """ + Verify cell data in the grades CSV for a particular user. + """ + 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) + report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD') + 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): + if row.get('username') == username: + self.assertEqual(row[column_header], expected_cell_content) @ddt.ddt -class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): +class TestInstructorGradeReport(InstructorGradeReportTestCase): """ Tests that CSV grade report generation works. """ @@ -87,20 +107,6 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD') self.assertTrue(any('grade_report_err' in item[0] for item in report_store.links_for(self.course.id))) - def _verify_cell_data_for_user(self, username, course_id, column_header, expected_cell_content): - """ - Verify cell data in the grades CSV for a particular user. - """ - 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) - report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD') - 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): - if row.get('username') == username: - self.assertEqual(row[column_header], expected_cell_content) - def test_cohort_data_in_grading(self): """ Test that cohort data is included in grades csv if cohort configuration is enabled for course. @@ -278,6 +284,43 @@ class TestInstructorGradeReport(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'attempted': 1, 'succeeded': 1, 'failed': 0}, result) +class TestTeamGradeReport(InstructorGradeReportTestCase): + """ Test that teams appear correctly in the grade report when it is enabled for the course. """ + + def setUp(self): + super(TestTeamGradeReport, self).setUp() + self.course = CourseFactory.create(teams_configuration={ + 'max_size': 2, 'topics': [{'topic-id': 'topic', 'name': 'Topic', 'description': 'A Topic'}] + }) + self.student1 = UserFactory.create() + CourseEnrollment.enroll(self.student1, self.course.id) + self.student2 = UserFactory.create() + CourseEnrollment.enroll(self.student2, self.course.id) + + def test_team_in_grade_report(self): + self._verify_cell_data_for_user(self.student1.username, self.course.id, 'Team Name', '') + + def test_correct_team_name_in_grade_report(self): + team1 = CourseTeamFactory.create(course_id=self.course.id) + CourseTeamMembershipFactory.create(team=team1, user=self.student1) + team2 = CourseTeamFactory.create(course_id=self.course.id) + CourseTeamMembershipFactory.create(team=team2, user=self.student2) + self._verify_cell_data_for_user(self.student1.username, self.course.id, 'Team Name', team1.name) + self._verify_cell_data_for_user(self.student2.username, self.course.id, 'Team Name', team2.name) + + def test_team_deleted(self): + team1 = CourseTeamFactory.create(course_id=self.course.id) + membership1 = CourseTeamMembershipFactory.create(team=team1, user=self.student1) + team2 = CourseTeamFactory.create(course_id=self.course.id) + CourseTeamMembershipFactory.create(team=team2, user=self.student2) + + team1.delete() + membership1.delete() + + self._verify_cell_data_for_user(self.student1.username, self.course.id, 'Team Name', '') + self._verify_cell_data_for_user(self.student2.username, self.course.id, 'Team Name', team2.name) + + class TestProblemResponsesReport(TestReportMixin, InstructorTaskCourseTestCase): """ Tests that generation of CSV files listing student answers to a @@ -912,6 +955,66 @@ class TestStudentReport(TestReportMixin, InstructorTaskCourseTestCase): self.assertDictContainsSubset({'attempted': num_students, 'succeeded': num_students, 'failed': 0}, result) +class TestTeamStudentReport(TestReportMixin, InstructorTaskCourseTestCase): + "Test the student report when including teams information. " + + def setUp(self): + super(TestTeamStudentReport, self).setUp() + self.course = CourseFactory.create(teams_configuration={ + 'max_size': 2, 'topics': [{'topic-id': 'topic', 'name': 'Topic', 'description': 'A Topic'}] + }) + self.student1 = UserFactory.create() + CourseEnrollment.enroll(self.student1, self.course.id) + self.student2 = UserFactory.create() + CourseEnrollment.enroll(self.student2, self.course.id) + + def _generate_and_verify_teams_column(self, username, expected_team): + """ Run the upload_students_csv task and verify that the correct team was added to the CSV. """ + current_task = Mock() + current_task.update_state = Mock() + task_input = { + 'features': [ + 'id', 'username', 'name', 'email', 'language', 'location', + 'year_of_birth', 'gender', 'level_of_education', 'mailing_address', + 'goals', 'team' + ] + } + with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task: + mock_current_task.return_value = current_task + result = upload_students_csv(None, None, self.course.id, task_input, 'calculated') + self.assertDictContainsSubset({'attempted': 2, 'succeeded': 2, 'failed': 0}, result) + report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD') + report_csv_filename = report_store.links_for(self.course.id)[0][0] + with open(report_store.path_to(self.course.id, report_csv_filename)) as csv_file: + for row in unicodecsv.DictReader(csv_file): + if row.get('username') == username: + self.assertEqual(row['team'], expected_team) + + def test_team_column_no_teams(self): + self._generate_and_verify_teams_column(self.student1.username, UNAVAILABLE) + self._generate_and_verify_teams_column(self.student2.username, UNAVAILABLE) + + def test_team_column_with_teams(self): + team1 = CourseTeamFactory.create(course_id=self.course.id) + CourseTeamMembershipFactory.create(team=team1, user=self.student1) + team2 = CourseTeamFactory.create(course_id=self.course.id) + CourseTeamMembershipFactory.create(team=team2, user=self.student2) + self._generate_and_verify_teams_column(self.student1.username, team1.name) + self._generate_and_verify_teams_column(self.student2.username, team2.name) + + def test_team_column_with_deleted_team(self): + team1 = CourseTeamFactory.create(course_id=self.course.id) + membership1 = CourseTeamMembershipFactory.create(team=team1, user=self.student1) + team2 = CourseTeamFactory.create(course_id=self.course.id) + CourseTeamMembershipFactory.create(team=team2, user=self.student2) + + team1.delete() + membership1.delete() + + self._generate_and_verify_teams_column(self.student1.username, UNAVAILABLE) + self._generate_and_verify_teams_column(self.student2.username, team2.name) + + @ddt.ddt class TestListMayEnroll(TestReportMixin, InstructorTaskCourseTestCase): """ diff --git a/lms/djangoapps/teams/tests/factories.py b/lms/djangoapps/teams/tests/factories.py index ee58e1ad69..e608055290 100644 --- a/lms/djangoapps/teams/tests/factories.py +++ b/lms/djangoapps/teams/tests/factories.py @@ -24,7 +24,7 @@ class CourseTeamFactory(DjangoModelFactory): team_id = factory.Sequence('team-{0}'.format) discussion_topic_id = factory.LazyAttribute(lambda a: uuid4().hex) - name = "Awesome Team" + name = factory.Sequence("Awesome Team {0}".format) description = "A simple description" last_activity_at = LAST_ACTIVITY_AT