Merge pull request #15627 from edx/sstudent/compute_grades_fix

force subsection grades to update when course grade updates

lms/djangoapps/grades/new/course_grade.py

@@ -21,7 +21,7 @@ class CourseGradeBase(object):
     """
     Base class for Course Grades.
     """
-    def __init__(self, user, course_data, percent=0, letter_grade=None, passed=False):
+    def __init__(self, user, course_data, percent=0, letter_grade=None, passed=False, force_update_subsections=False):
         self.user = user
         self.course_data = course_data
 
@@ -30,6 +30,7 @@ class CourseGradeBase(object):
 
         # Convert empty strings to None when reading from the table
         self.letter_grade = letter_grade or None
+        self.force_update_subsections = force_update_subsections
 
     def __unicode__(self):
         return u'Course Grade: percent: {}, letter_grade: {}, passed: {}'.format(
@@ -203,7 +204,9 @@ class CourseGrade(CourseGradeBase):
 
     def update(self):
         """
-        Updates the grade for the course.
+        Updates the grade for the course. Also updates subsection grades
+        if self.force_update_subsections is true, via the lazy call
+        to self.grader_result.
         """
         grade_cutoffs = self.course_data.course.grade_cutoffs
         self.percent = self._compute_percent(self.grader_result)
@@ -224,7 +227,10 @@ class CourseGrade(CourseGradeBase):
 
     def _get_subsection_grade(self, subsection):
         # Pass read_only here so the subsection grades can be persisted in bulk at the end.
-        return self._subsection_grade_factory.create(subsection, read_only=True)
+        if self.force_update_subsections:
+            return self._subsection_grade_factory.update(subsection)
+        else:
+            return self._subsection_grade_factory.create(subsection, read_only=True)
 
     @staticmethod
     def _compute_percent(grader_result):

lms/djangoapps/grades/new/course_grade_factory.py

@@ -66,7 +66,15 @@ class CourseGradeFactory(object):
             else:
                 return None
 
-    def update(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None):
+    def update(
+            self,
+            user,
+            course=None,
+            collected_block_structure=None,
+            course_structure=None,
+            course_key=None,
+            force_update_subsections=False,
+    ):
         """
         Computes, updates, and returns the CourseGrade for the given
         user in the course.
@@ -75,7 +83,7 @@ class CourseGradeFactory(object):
         or course_key should be provided.
         """
         course_data = CourseData(user, course, collected_block_structure, course_structure, course_key)
-        return self._update(user, course_data, read_only=False)
+        return self._update(user, course_data, read_only=False, force_update_subsections=force_update_subsections)
 
     @contextmanager
     def _course_transaction(self, course_key):
@@ -118,10 +126,17 @@ class CourseGradeFactory(object):
 
     def _iter_grade_result(self, user, course_data, force_update):
         try:
+            kwargs = {
+                'user': user,
+                'course': course_data.course,
+                'collected_block_structure': course_data.collected_structure,
+                'course_key': course_data.course_key
+            }
+            if force_update:
+                kwargs['force_update_subsections'] = True
+
             method = CourseGradeFactory().update if force_update else CourseGradeFactory().create
-            course_grade = method(
-                user, course_data.course, course_data.collected_structure, course_key=course_data.course_key,
-            )
+            course_grade = method(**kwargs)
             return 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
@@ -165,14 +180,14 @@ class CourseGradeFactory(object):
         return course_grade, persistent_grade.grading_policy_hash
 
     @staticmethod
-    def _update(user, course_data, read_only):
+    def _update(user, course_data, read_only, force_update_subsections=False):
         """
         Computes, saves, and returns a CourseGrade object for the
         given user and course.
         Sends a COURSE_GRADE_CHANGED signal to listeners and a
         COURSE_GRADE_NOW_PASSED if learner has passed course.
         """
-        course_grade = CourseGrade(user, course_data)
+        course_grade = CourseGrade(user, course_data, force_update_subsections=force_update_subsections)
         course_grade.update()
 
         should_persist = (

lms/djangoapps/grades/tasks.py

@@ -106,7 +106,7 @@ def compute_grades_for_course_v2(self, **kwargs):
 @task(base=_BaseTask)
 def compute_grades_for_course(course_key, offset, batch_size, **kwargs):  # pylint: disable=unused-argument
     """
-    Compute grades for a set of students in the specified course.
+    Compute and save grades for a set of students in the specified course.
 
     The set of students will be determined by the order of enrollment date, and
     limited to at most <batch_size> students, starting from the specified

lms/djangoapps/grades/tests/test_new.py

@@ -246,6 +246,20 @@ class TestCourseGradeFactory(GradeTestBase):
             else:
                 self.assertIsNone(course_grade)
 
+    @ddt.data(True, False)
+    def test_iter_force_update(self, force_update):
+        base_string = 'lms.djangoapps.grades.new.subsection_grade_factory.SubsectionGradeFactory.{}'
+        desired_method_name = base_string.format('update' if force_update else 'create')
+        undesired_method_name = base_string.format('create' if force_update else 'update')
+        with patch(desired_method_name) as desired_call:
+            with patch(undesired_method_name) as undesired_call:
+                set(CourseGradeFactory().iter(
+                    users=[self.request.user], course=self.course, force_update=force_update
+                ))
+
+        self.assertTrue(desired_call.called)
+        self.assertFalse(undesired_call.called)
+
 
 @ddt.ddt
 class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase):