From 66fdd2fbd379eb25468d2d197504b3c3983774c6 Mon Sep 17 00:00:00 2001 From: Sanford Student Date: Thu, 20 Jul 2017 15:47:52 -0400 Subject: [PATCH] EDUCATOR-915: force subsection grades to update when course grade updates --- lms/djangoapps/grades/new/course_grade.py | 12 ++++++-- .../grades/new/course_grade_factory.py | 29 ++++++++++++++----- lms/djangoapps/grades/tasks.py | 2 +- lms/djangoapps/grades/tests/test_new.py | 14 +++++++++ 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/lms/djangoapps/grades/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index 745c6434b4..b438001d07 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/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): diff --git a/lms/djangoapps/grades/new/course_grade_factory.py b/lms/djangoapps/grades/new/course_grade_factory.py index 9085436068..4ffe091b79 100644 --- a/lms/djangoapps/grades/new/course_grade_factory.py +++ b/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 = ( diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 3bb8d9e3f9..3e4df7605e 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/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 students, starting from the specified diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index d5a942df0c..fb7feccbf0 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/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):