Merge pull request #16149 from edx/naa/grade-report

Optimize grade reports for ZeroCourseGrades

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)

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

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)

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)
 

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',
                         }
                     },
                 ],

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)