From cebdbc18e7a4bc11c4e481010bb6dcf27fa7169f Mon Sep 17 00:00:00 2001 From: "J. Cliff Dyer" Date: Mon, 8 May 2017 12:16:52 -0400 Subject: [PATCH] Update CourseGradeFactory().iter() to return exception objects --- lms/djangoapps/grades/new/course_grade_factory.py | 6 +++--- lms/djangoapps/grades/tests/test_grades.py | 8 ++++---- lms/djangoapps/instructor_task/tasks_helper/grades.py | 7 ++++--- lms/djangoapps/instructor_task/tests/test_tasks_helper.py | 6 +++--- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/grades/new/course_grade_factory.py b/lms/djangoapps/grades/new/course_grade_factory.py index b939fcae9a..c731f29ef7 100644 --- a/lms/djangoapps/grades/new/course_grade_factory.py +++ b/lms/djangoapps/grades/new/course_grade_factory.py @@ -18,7 +18,7 @@ class CourseGradeFactory(object): """ Factory class to create Course Grade objects. """ - GradeResult = namedtuple('GradeResult', ['student', 'course_grade', 'err_msg']) + GradeResult = namedtuple('GradeResult', ['student', 'course_grade', 'error']) def create(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None): """ @@ -108,7 +108,7 @@ class CourseGradeFactory(object): try: method = CourseGradeFactory().update if force_update else CourseGradeFactory().create course_grade = method(user, course, course_data.collected_structure, course_structure, course_key) - yield self.GradeResult(user, course_grade, "") + yield self.GradeResult(user, course_grade, None) except Exception as exc: # pylint: disable=broad-except # Keep marching on even if this student couldn't be graded for @@ -119,7 +119,7 @@ class CourseGradeFactory(object): course_data.course_key, exc.message ) - yield self.GradeResult(user, None, exc.message) + yield self.GradeResult(user, None, exc) @staticmethod def _create_zero(user, course_data): diff --git a/lms/djangoapps/grades/tests/test_grades.py b/lms/djangoapps/grades/tests/test_grades.py index 7ee4e593e7..f7d6684ce0 100644 --- a/lms/djangoapps/grades/tests/test_grades.py +++ b/lms/djangoapps/grades/tests/test_grades.py @@ -101,7 +101,7 @@ class TestGradeIteration(SharedModuleStoreTestCase): with self.assertNumQueries(4): all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) self.assertEqual( - all_errors, + {student: all_errors[student].message for student in all_errors}, { student3: "Error for student3.", student4: "Error for student4.", @@ -130,10 +130,10 @@ class TestGradeIteration(SharedModuleStoreTestCase): students_to_course_grades = {} students_to_errors = {} - for student, course_grade, err_msg in CourseGradeFactory().iter(students, course): + for student, course_grade, error in CourseGradeFactory().iter(students, course): students_to_course_grades[student] = course_grade - if err_msg: - students_to_errors[student] = err_msg + if error: + students_to_errors[student] = error return students_to_course_grades, students_to_errors diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index 810b8f51eb..96030fdb28 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -322,10 +322,10 @@ class CourseGradeReport(object): certificate_whitelist = CertificateWhitelist.objects.filter(course_id=context.course_id, whitelist=True) whitelisted_user_ids = [entry.user_id for entry in certificate_whitelist] success_rows, error_rows = [], [] - for user, course_grade, err_msg in CourseGradeFactory().iter(users, course_key=context.course_id): + for user, course_grade, error in CourseGradeFactory().iter(users, course_key=context.course_id): if not course_grade: # An empty gradeset means we failed to grade a student. - error_rows.append([user.id, user.username, err_msg]) + error_rows.append([user.id, user.username, error.message]) else: success_rows.append( [user.id, user.email, user.username] + @@ -365,11 +365,12 @@ class ProblemGradeReport(object): current_step = {'step': 'Calculating Grades'} course = get_course_by_id(course_id) - for student, course_grade, err_msg in CourseGradeFactory().iter(enrolled_students, course): + for student, course_grade, error in CourseGradeFactory().iter(enrolled_students, course): student_fields = [getattr(student, field_name) for field_name in header_row] task_progress.attempted += 1 if not course_grade: + err_msg = error.message # There was an error grading this student. if not err_msg: err_msg = u'Unknown error' diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index abe4c30e43..dd230dd320 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -132,7 +132,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): progress dict and uploaded to the report store. """ mock_grades_iter.return_value = [ - (self.create_student('username', 'student@example.com'), None, 'Cannot grade student') + (self.create_student('username', 'student@example.com'), None, TypeError('Cannot grade student')) ] result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded') self.assertDictContainsSubset({'attempted': 1, 'succeeded': 0, 'failed': 1}, result) @@ -315,7 +315,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): ( self.create_student('username', 'student@example.com'), mock_course_grade, - '', + None, ) ] result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded') @@ -667,7 +667,7 @@ class TestProblemGradeReport(TestReportMixin, InstructorTaskModuleTestCase): """ student = self.create_student(u'username', u'student@example.com') mock_grades_iter.return_value = [ - (student, None, error_message) + (student, None, Exception(error_message)) ] result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded') self.assertDictContainsSubset({'attempted': 1, 'succeeded': 0, 'failed': 1}, result)