diff --git a/lms/djangoapps/instructor_task/models.py b/lms/djangoapps/instructor_task/models.py index 884c25f16c..e23f2ad1da 100644 --- a/lms/djangoapps/instructor_task/models.py +++ b/lms/djangoapps/instructor_task/models.py @@ -209,6 +209,15 @@ class ReportStore(object): elif storage_type.lower() == "localfs": return LocalFSReportStore.from_config() + def _get_utf8_encoded_rows(self, rows): + """ + Given a list of `rows` containing unicode strings, return a + new list of rows with those strings encoded as utf-8 for CSV + compatibility. + """ + for row in rows: + yield [unicode(item).encode('utf-8') for item in row] + class S3ReportStore(ReportStore): """ @@ -306,7 +315,8 @@ class S3ReportStore(ReportStore): """ output_buffer = StringIO() gzip_file = GzipFile(fileobj=output_buffer, mode="wb") - csv.writer(gzip_file).writerows(rows) + csvwriter = csv.writer(gzip_file) + csvwriter.writerows(self._get_utf8_encoded_rows(rows)) gzip_file.close() self.store(course_id, filename, output_buffer) @@ -384,7 +394,9 @@ class LocalFSReportStore(ReportStore): write this data out. """ output_buffer = StringIO() - csv.writer(output_buffer).writerows(rows) + csvwriter = csv.writer(output_buffer) + csvwriter.writerows(self._get_utf8_encoded_rows(rows)) + self.store(course_id, filename, output_buffer) def links_for(self, course_id): diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index 44beec3bb3..d1a8fbebd5 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -575,7 +575,7 @@ def push_grades_to_s3(_xmodule_instance_args, _entry_id, course_id, _task_input, # possible for a student to have a 0.0 show up in their row but # still have 100% for the course. row_percents = [percents.get(label, 0.0) for label in header] - rows.append([student.id, student.email.encode('utf-8'), student.username, gradeset['percent']] + row_percents) + rows.append([student.id, student.email, student.username, gradeset['percent']] + row_percents) else: # An empty gradeset means we failed to grade a student. num_failed += 1 diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 976022e93c..77bb9825cc 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -33,16 +33,16 @@ class TestReport(ModuleStoreTestCase): if os.path.exists(settings.GRADES_DOWNLOAD['ROOT_PATH']): shutil.rmtree(settings.GRADES_DOWNLOAD['ROOT_PATH']) + def create_student(self, username, email): + student = UserFactory.create(username=username, email=email) + CourseEnrollmentFactory.create(user=student, course_id=self.course.id) + @ddt.ddt class TestInstructorGradeReport(TestReport): """ Tests that CSV grade report generation works. """ - def create_student(self, username, email): - student = UserFactory.create(username=username, email=email) - CourseEnrollmentFactory.create(user=student, course_id=self.course.id) - @ddt.data([u'student@example.com', u'ni\xf1o@example.com']) def test_unicode_emails(self, emails): """ @@ -60,6 +60,7 @@ class TestInstructorGradeReport(TestReport): self.assertEquals(result['succeeded'], result['attempted']) +@ddt.ddt class TestStudentReport(TestReport): """ Tests that CSV student profile report generation works. @@ -73,3 +74,27 @@ class TestStudentReport(TestReport): self.assertEquals(len(links), 1) self.assertEquals(result, UPDATE_STATUS_SUCCEEDED) + + @ddt.data([u'student', u'student\xec']) + def test_unicode_usernames(self, students): + """ + Test that students with unicode characters in their usernames + are handled. + """ + for i, student in enumerate(students): + self.create_student(username=student, email='student{0}@example.com'.format(i)) + + self.current_task = Mock() + self.current_task.update_state = Mock() + task_input = { + 'features': [ + 'id', 'username', 'name', 'email', 'language', 'location', + 'year_of_birth', 'gender', 'level_of_education', 'mailing_address', + 'goals' + ] + } + with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task: + mock_current_task.return_value = self.current_task + result = push_students_csv_to_s3(None, None, self.course.id, task_input, 'calculated') + #This assertion simply confirms that the generation completed with no errors + self.assertEquals(result, UPDATE_STATUS_SUCCEEDED)