From 27229a1f9bf91bbe838be41d698013fe3fc7366b Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 3 Oct 2017 11:22:49 -0400 Subject: [PATCH] Optimize grade reports for ZeroCourseGrades. EDUCATOR-558 --- lms/djangoapps/grades/course_grade.py | 20 +++++++++++++++++++ lms/djangoapps/grades/subsection_grade.py | 15 ++++++++++++++ .../grades/tests/test_course_grade_factory.py | 2 +- .../instructor_task/tasks_helper/grades.py | 8 +++----- .../instructor_task/tests/test_integration.py | 2 +- .../tests/test_tasks_helper.py | 20 ++++++++++++++++++- 6 files changed, 59 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/grades/course_grade.py b/lms/djangoapps/grades/course_grade.py index 333118ce63..58e47963a1 100644 --- a/lms/djangoapps/grades/course_grade.py +++ b/lms/djangoapps/grades/course_grade.py @@ -44,6 +44,20 @@ class CourseGradeBase(object): """ return False + def subsection_grade(self, subsection_key): + """ + Returns the subsection grade for given subsection usage key. + Raises KeyError if the user doesn't have access to that subsection. + """ + return self._get_subsection_grade(self.course_data.structure[subsection_key]) + + @abstractmethod + def assignment_average(self, assignment_type): + """ + Returns the average of all assignments of the given assignment type. + """ + raise NotImplementedError + @lazy def graded_subsections_by_format(self): """ @@ -207,6 +221,9 @@ class ZeroCourseGrade(CourseGradeBase): Course Grade class for Zero-value grades when no problems were attempted in the course. """ + def assignment_average(self, assignment_type): + return 0.0 + def _get_subsection_grade(self, subsection): return ZeroSubsectionGrade(subsection, self.course_data) @@ -248,6 +265,9 @@ class CourseGrade(CourseGradeBase): return True return False + def assignment_average(self, assignment_type): + return self.grader_result['grade_breakdown'].get(assignment_type, {}).get('percent') + def _get_subsection_grade(self, subsection): if self.force_update_subsections: return self._subsection_grade_factory.update(subsection) diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index 1e97fe6d59..6f90a36236 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -53,6 +53,13 @@ class SubsectionGradeBase(object): """ return ShowCorrectness.correctness_available(self.show_correctness, self.due, has_staff_access) + @property + def attempted_graded(self): + """ + Returns whether the user had attempted a graded problem in this subsection. + """ + raise NotImplementedError + class ZeroSubsectionGrade(SubsectionGradeBase): """ @@ -63,6 +70,10 @@ class ZeroSubsectionGrade(SubsectionGradeBase): super(ZeroSubsectionGrade, self).__init__(subsection) self.course_data = course_data + @property + def attempted_graded(self): + return False + @property def all_total(self): return self._aggregate_scores[0] @@ -174,6 +185,10 @@ class SubsectionGrade(SubsectionGradeBase): self._log_event(log.debug, u"update_or_create_model", student) return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student)) + @property + def attempted_graded(self): + return self.graded_total.first_attempted is not None + def _should_persist_per_attempted(self, score_deleted=False): """ Returns whether the SubsectionGrade's model should be diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 5f97c3997d..c0337a56df 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -36,7 +36,7 @@ class TestCourseGradeFactory(GradeTestBase): def test_course_grade_no_access(self): """ - Test to ensure a grade can ba calculated for a student in a course, even if they themselves do not have access. + Test to ensure a grade can be calculated for a student in a course, even if they themselves do not have access. """ invisible_course = CourseFactory.create(visible_to_staff_only=True) access = has_access(self.request.user, 'load', invisible_course) diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index da013c2830..2876f4c06a 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -306,19 +306,17 @@ class CourseGradeReport(object): for assignment_type, assignment_info in context.graded_assignments.iteritems(): for subsection_location in assignment_info['subsection_headers']: try: - subsection_grade = course_grade.graded_subsections_by_format[assignment_type][subsection_location] + subsection_grade = course_grade.subsection_grade(subsection_location) except KeyError: grade_result = u'Not Available' else: - if subsection_grade.graded_total.first_attempted is not None: + if subsection_grade.attempted_graded: grade_result = subsection_grade.graded_total.earned / subsection_grade.graded_total.possible else: grade_result = u'Not Attempted' grade_results.append([grade_result]) if assignment_info['separate_subsection_avg_headers']: - assignment_average = course_grade.grader_result['grade_breakdown'].get(assignment_type, {}).get( - 'percent' - ) + assignment_average = course_grade.assignment_average(assignment_type) grade_results.append([assignment_average]) return [course_grade.percent] + _flatten(grade_results) diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index 38f93a03da..783c4064f1 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -679,7 +679,7 @@ class TestGradeReportConditionalContent(TestReportMixin, TestConditionalContent, { self.student_b: { u'Grade': '0.0', - u'Homework': u'Not Available', + u'Homework': u'Not Attempted', } }, ], diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index a0d1021b43..32409b11bf 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1769,6 +1769,7 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded') + self.assertDictContainsSubset( {'action_name': 'graded', 'attempted': 1, 'succeeded': 1, 'failed': 0}, result, @@ -1783,13 +1784,30 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): u'Homework 1: Subsection': '0.5', u'Homework 2: Hidden': u'Not Available', u'Homework 3: Unattempted': u'Not Attempted', - u'Homework 4: Empty': u'Not Available', + u'Homework 4: Empty': u'Not Attempted', u'Homework (Avg)': '0.125', }, ], ignore_other_columns=True, ) + def test_fast_generation_zero_grade(self): + with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): + with patch('lms.djangoapps.grades.course_grade.CourseGradeBase._prep_course_for_grading') as mock_grader: + with patch('lms.djangoapps.grades.subsection_grade.get_score') as mock_get_score: + CourseGradeReport.generate(None, None, self.course.id, None, 'graded') + self.assertFalse(mock_grader.called) + self.assertFalse(mock_get_score.called) + + def test_slow_generation_nonzero_grade(self): + self.submit_student_answer(self.student.username, u'Problem1', ['Option 1']) + with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): + with patch('lms.djangoapps.grades.course_grade.CourseGradeBase._prep_course_for_grading') as mock_grader: + with patch('lms.djangoapps.grades.subsection_grade.get_score') as mock_get_score: + CourseGradeReport.generate(None, None, self.course.id, None, 'graded') + self.assertTrue(mock_grader.called) + self.assertTrue(mock_get_score.called) + @ddt.ddt @patch('lms.djangoapps.instructor_task.tasks_helper.misc.DefaultStorage', new=MockDefaultStorage)