Merge pull request #16162 from edx/naa/course-grade-report

Have SubsectionGrade compute problem_scores lazily

lms/djangoapps/grades/subsection_grade.py

 """
 SubsectionGrade Class
 """
+from abc import ABCMeta
 from collections import OrderedDict
 from logging import getLogger
 
@@ -17,8 +18,10 @@ log = getLogger(__name__)
 
 class SubsectionGradeBase(object):
     """
-    Class for Subsection Grades.
+    Abstract base class for Subsection Grades.
     """
+    __metaclass__ = ABCMeta
+
     def __init__(self, subsection):
         self.location = subsection.location
         self.display_name = block_metadata_utils.display_name_with_default_escaped(subsection)
@@ -108,47 +111,53 @@ class ZeroSubsectionGrade(SubsectionGradeBase):
         return locations
 
 
-class SubsectionGrade(SubsectionGradeBase):
+class NonZeroSubsectionGrade(SubsectionGradeBase):
     """
-    Class for Subsection Grades.
+    Abstract base class for Subsection Grades with
+    possibly NonZero values.
     """
-    def __init__(self, subsection, problem_scores, all_total, graded_total, override=None):
-        super(SubsectionGrade, self).__init__(subsection)
-        self.problem_scores = problem_scores
+    __metaclass__ = ABCMeta
+
+    def __init__(self, subsection, all_total, graded_total, override=None):
+        super(NonZeroSubsectionGrade, self).__init__(subsection)
         self.all_total = all_total
         self.graded_total = graded_total
         self.override = override
 
-    @classmethod
-    def create(cls, subsection, course_structure, submissions_scores, csm_scores):
-        """
-        Compute and create the subsection grade.
-        """
-        problem_scores = OrderedDict()
-        for block_key in course_structure.post_order_traversal(
-                filter_func=possibly_scored,
-                start_node=subsection.location,
-        ):
-            problem_score = cls._compute_block_score(block_key, course_structure, submissions_scores, csm_scores)
-            if problem_score:
-                problem_scores[block_key] = problem_score
-        all_total, graded_total = graders.aggregate_scores(problem_scores.values())
+    @property
+    def attempted_graded(self):
+        return self.graded_total.first_attempted is not None
 
-        return cls(subsection, problem_scores, all_total, graded_total)
+    @staticmethod
+    def _compute_block_score(
+            block_key,
+            course_structure,
+            submissions_scores,
+            csm_scores,
+            persisted_block=None,
+    ):
+        try:
+            block = course_structure[block_key]
+        except KeyError:
+            # It's possible that the user's access to that
+            # block has changed since the subsection grade
+            # was last persisted.
+            pass
+        else:
+            if getattr(block, 'has_score', False):
+                return get_score(
+                    submissions_scores,
+                    csm_scores,
+                    persisted_block,
+                    block,
+                )
 
-    @classmethod
-    def read(cls, subsection, model, course_structure, submissions_scores, csm_scores):
-        """
-        Read the subsection grade from the persisted model.
-        """
-        problem_scores = OrderedDict()
-        for block in model.visible_blocks.blocks:
-            problem_score = cls._compute_block_score(
-                block.locator, course_structure, submissions_scores, csm_scores, block,
-            )
-            if problem_score:
-                problem_scores[block.locator] = problem_score
 
+class ReadSubsectionGrade(NonZeroSubsectionGrade):
+    """
+    Class for Subsection grades that are read from the database.
+    """
+    def __init__(self, subsection, model, course_structure, submissions_scores, csm_scores):
         all_total = AggregatedScore(
             tw_earned=model.earned_all,
             tw_possible=model.possible_all,
@@ -162,7 +171,51 @@ class SubsectionGrade(SubsectionGradeBase):
             first_attempted=model.first_attempted,
         )
         override = model.override if hasattr(model, 'override') else None
-        return cls(subsection, problem_scores, all_total, graded_total, override)
+
+        # save these for later since we compute problem_scores lazily
+        self.model = model
+        self.course_structure = course_structure
+        self.submissions_scores = submissions_scores
+        self.csm_scores = csm_scores
+
+        super(ReadSubsectionGrade, self).__init__(subsection, all_total, graded_total, override)
+
+    @lazy
+    def problem_scores(self):
+        problem_scores = OrderedDict()
+        for block in self.model.visible_blocks.blocks:
+            problem_score = self._compute_block_score(
+                block.locator, self.course_structure, self.submissions_scores, self.csm_scores, block,
+            )
+            if problem_score:
+                problem_scores[block.locator] = problem_score
+        return problem_scores
+
+
+class CreateSubsectionGrade(NonZeroSubsectionGrade):
+    """
+    Class for Subsection grades that are newly created or updated.
+    """
+    def __init__(self, subsection, course_structure, submissions_scores, csm_scores):
+        self.problem_scores = OrderedDict()
+        for block_key in course_structure.post_order_traversal(
+                filter_func=possibly_scored,
+                start_node=subsection.location,
+        ):
+            problem_score = self._compute_block_score(block_key, course_structure, submissions_scores, csm_scores)
+            if problem_score:
+                self.problem_scores[block_key] = problem_score
+
+        all_total, graded_total = graders.aggregate_scores(self.problem_scores.values())
+
+        super(CreateSubsectionGrade, self).__init__(subsection, all_total, graded_total)
+
+    def update_or_create_model(self, student, score_deleted=False):
+        """
+        Saves or updates the subsection grade in a persisted model.
+        """
+        if self._should_persist_per_attempted(score_deleted):
+            return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student))
 
     @classmethod
     def bulk_create_models(cls, student, subsection_grades, course_key):
@@ -177,18 +230,6 @@ class SubsectionGrade(SubsectionGradeBase):
         ]
         return PersistentSubsectionGrade.bulk_create_grades(params, student.id, course_key)
 
-    def update_or_create_model(self, student, score_deleted=False):
-        """
-        Saves or updates the subsection grade in a persisted model.
-        """
-        if self._should_persist_per_attempted(score_deleted):
-            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
@@ -202,34 +243,6 @@ class SubsectionGrade(SubsectionGradeBase):
             score_deleted
         )
 
-    @staticmethod
-    def _compute_block_score(
-            block_key,
-            course_structure,
-            submissions_scores,
-            csm_scores,
-            persisted_block=None,
-    ):
-        """
-        Compute score for the given block. If persisted_values
-        is provided, it is used for possible and weight.
-        """
-        try:
-            block = course_structure[block_key]
-        except KeyError:
-            # It's possible that the user's access to that
-            # block has changed since the subsection grade
-            # was last persisted.
-            pass
-        else:
-            if getattr(block, 'has_score', False):
-                return get_score(
-                    submissions_scores,
-                    csm_scores,
-                    persisted_block,
-                    block,
-                )
-
     def _persisted_model_params(self, student):
         """
         Returns the parameters for creating/updating the
@@ -258,25 +271,3 @@ class SubsectionGrade(SubsectionGradeBase):
             for location, score in
             self.problem_scores.iteritems()
         ]
-
-    def _log_event(self, log_func, log_statement, student):
-        """
-        Logs the given statement, for this instance.
-        """
-        log_func(
-            u"Grades: SG.{}, subsection: {}, course: {}, "
-            u"version: {}, edit: {}, user: {},"
-            u"total: {}/{}, graded: {}/{}, show_correctness: {}".format(
-                log_statement,
-                self.location,
-                self.location.course_key,
-                self.course_version,
-                self.subtree_edited_timestamp,
-                student.id,
-                self.all_total.earned,
-                self.all_total.possible,
-                self.graded_total.earned,
-                self.graded_total.possible,
-                self.show_correctness,
-            )
-        )

lms/djangoapps/grades/subsection_grade_factory.py

@@ -12,7 +12,7 @@ from student.models import anonymous_id_for_user
 from submissions import api as submissions_api
 
 from .course_data import CourseData
-from .subsection_grade import SubsectionGrade, ZeroSubsectionGrade
+from .subsection_grade import CreateSubsectionGrade, ReadSubsectionGrade, ZeroSubsectionGrade
 
 log = getLogger(__name__)
 
@@ -43,7 +43,7 @@ class SubsectionGradeFactory(object):
             if assume_zero_if_absent(self.course_data.course_key):
                 subsection_grade = ZeroSubsectionGrade(subsection, self.course_data)
             else:
-                subsection_grade = SubsectionGrade.create(
+                subsection_grade = CreateSubsectionGrade(
                     subsection, self.course_data.structure, self._submissions_scores, self._csm_scores,
                 )
                 if should_persist_grades(self.course_data.course_key):
@@ -58,7 +58,7 @@ class SubsectionGradeFactory(object):
         """
         Bulk creates all the unsaved subsection_grades to this point.
         """
-        SubsectionGrade.bulk_create_models(
+        CreateSubsectionGrade.bulk_create_models(
             self.student, self._unsaved_subsection_grades.values(), self.course_data.course_key
         )
         self._unsaved_subsection_grades.clear()
@@ -69,7 +69,7 @@ class SubsectionGradeFactory(object):
         """
         self._log_event(log.debug, u"update, subsection: {}".format(subsection.location), subsection)
 
-        calculated_grade = SubsectionGrade.create(
+        calculated_grade = CreateSubsectionGrade(
             subsection, self.course_data.structure, self._submissions_scores, self._csm_scores,
         )
 
@@ -80,7 +80,7 @@ class SubsectionGradeFactory(object):
                 except PersistentSubsectionGrade.DoesNotExist:
                     pass
                 else:
-                    orig_subsection_grade = SubsectionGrade.read(
+                    orig_subsection_grade = ReadSubsectionGrade(
                         subsection, grade_model, self.course_data.structure, self._submissions_scores, self._csm_scores,
                     )
                     if not is_score_higher_or_equal(
@@ -125,7 +125,7 @@ class SubsectionGradeFactory(object):
             saved_subsection_grades = self._get_bulk_cached_subsection_grades()
             grade = saved_subsection_grades.get(subsection.location)
             if grade:
-                return SubsectionGrade.read(
+                return ReadSubsectionGrade(
                     subsection, grade, self.course_data.structure, self._submissions_scores, self._csm_scores,
                 )
 

lms/djangoapps/grades/tests/test_course_grade_factory.py

@@ -14,7 +14,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
 from ..config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT, waffle
 from ..course_grade import CourseGrade, ZeroCourseGrade
 from ..course_grade_factory import CourseGradeFactory
-from ..subsection_grade import SubsectionGrade, ZeroSubsectionGrade
+from ..subsection_grade import ReadSubsectionGrade, ZeroSubsectionGrade
 from .base import GradeTestBase
 from .utils import mock_get_score
 
@@ -131,7 +131,7 @@ class TestCourseGradeFactory(GradeTestBase):
         course_grade = CourseGradeFactory().update(self.request.user, self.course)
         subsection1_grade = course_grade.subsection_grades[self.sequence.location]
         subsection2_grade = course_grade.subsection_grades[self.sequence2.location]
-        self.assertIsInstance(subsection1_grade, SubsectionGrade)
+        self.assertIsInstance(subsection1_grade, ReadSubsectionGrade)
         self.assertIsInstance(subsection2_grade, ZeroSubsectionGrade)
 
     @ddt.data(True, False)

lms/djangoapps/grades/tests/test_subsection_grade.py

 from ..models import PersistentSubsectionGrade
-from ..subsection_grade import SubsectionGrade
+from ..subsection_grade import CreateSubsectionGrade, ReadSubsectionGrade
 from .utils import mock_get_score
 from .base import GradeTestBase
 
@@ -8,7 +8,7 @@ class SubsectionGradeTest(GradeTestBase):
     def test_create_and_read(self):
         with mock_get_score(1, 2):
             # Create a grade that *isn't* saved to the database
-            created_grade = SubsectionGrade.create(
+            created_grade = CreateSubsectionGrade(
                 self.sequence,
                 self.course_structure,
                 self.subsection_grade_factory._submissions_scores,
@@ -25,7 +25,7 @@ class SubsectionGradeTest(GradeTestBase):
                 user_id=self.request.user.id,
                 usage_key=self.sequence.location,
             )
-            read_grade = SubsectionGrade.read(
+            read_grade = ReadSubsectionGrade(
                 self.sequence,
                 saved_model,
                 self.course_structure,

lms/djangoapps/instructor_task/tasks_helper/grades.py

@@ -222,7 +222,7 @@ class CourseGradeReport(object):
         Returns a list of all applicable column headers for this grade report.
         """
         return (
-            ["Student ID", "Email", "Username", "Grade"] +
+            ["Student ID", "Email", "Username"] +
             self._grades_header(context) +
             (['Cohort Name'] if context.cohorts_enabled else []) +
             [u'Experiment Group ({})'.format(partition.name) for partition in context.course_experiments] +
@@ -278,7 +278,7 @@ class CourseGradeReport(object):
         Returns the applicable grades-related headers for this report.
         """
         graded_assignments = context.graded_assignments
-        grades_header = []
+        grades_header = ["Grade"]
         for assignment_info in graded_assignments.itervalues():
             if assignment_info['separate_subsection_avg_headers']:
                 grades_header.extend(assignment_info['subsection_headers'].itervalues())