From dd93e67b5d776f4178b5243d78c74a88e05a39ce Mon Sep 17 00:00:00 2001 From: Sofiya Semenova Date: Mon, 18 Dec 2017 16:30:27 -0500 Subject: [PATCH] Force regrade on enrollment track change. --- lms/djangoapps/grades/course_grade.py | 10 +++++----- lms/djangoapps/grades/course_grade_factory.py | 12 ++++++++++-- lms/djangoapps/grades/signals/handlers.py | 6 +++++- lms/djangoapps/grades/subsection_grade.py | 14 ++++++++++---- lms/djangoapps/grades/subsection_grade_factory.py | 8 ++++++-- .../grades/tests/test_course_grade_factory.py | 4 ++-- 6 files changed, 38 insertions(+), 16 deletions(-) diff --git a/lms/djangoapps/grades/course_grade.py b/lms/djangoapps/grades/course_grade.py index c50505f2b9..7cb742a341 100644 --- a/lms/djangoapps/grades/course_grade.py +++ b/lms/djangoapps/grades/course_grade.py @@ -214,12 +214,12 @@ class CourseGradeBase(object): Returns a list of subsection grades for the given chapter. """ return [ - self._get_subsection_grade(course_structure[subsection_key]) + self._get_subsection_grade(course_structure[subsection_key], self.force_update_subsections) for subsection_key in _uniqueify_and_keep_order(course_structure.get_children(chapter_key)) ] @abstractmethod - def _get_subsection_grade(self, subsection): + def _get_subsection_grade(self, subsection, force_update_subsections=False): """ Abstract method to be implemented by subclasses for returning the grade of the given subsection. @@ -232,7 +232,7 @@ class ZeroCourseGrade(CourseGradeBase): Course Grade class for Zero-value grades when no problems were attempted in the course. """ - def _get_subsection_grade(self, subsection): + def _get_subsection_grade(self, subsection, force_update_subsections=False): return ZeroSubsectionGrade(subsection, self.course_data) @@ -276,9 +276,9 @@ class CourseGrade(CourseGradeBase): return True return False - def _get_subsection_grade(self, subsection): + def _get_subsection_grade(self, subsection, force_update_subsections=False): if self.force_update_subsections: - return self._subsection_grade_factory.update(subsection) + return self._subsection_grade_factory.update(subsection, force_update_subsections) else: # 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) diff --git a/lms/djangoapps/grades/course_grade_factory.py b/lms/djangoapps/grades/course_grade_factory.py index d818e3b2be..c51a4f0b3a 100644 --- a/lms/djangoapps/grades/course_grade_factory.py +++ b/lms/djangoapps/grades/course_grade_factory.py @@ -66,7 +66,11 @@ 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, force_update_subsections=force_update_subsections) + return self._update( + user, + course_data, + force_update_subsections=force_update_subsections + ) def iter( self, @@ -164,7 +168,11 @@ class CourseGradeFactory(object): if should_persist and force_update_subsections: prefetch(user, course_data.course_key) - course_grade = CourseGrade(user, course_data, force_update_subsections=force_update_subsections) + course_grade = CourseGrade( + user, + course_data, + force_update_subsections=force_update_subsections + ) course_grade = course_grade.update() should_persist = should_persist and course_grade.attempted diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 2b5521d0f2..d7fff719f7 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -248,4 +248,8 @@ def recalculate_course_and_subsection_grades(sender, user, course_key, **kwargs) """ previous_course_grade = CourseGradeFactory().read(user, course_key=course_key) if previous_course_grade and previous_course_grade.attempted: - CourseGradeFactory().update(user=user, course_key=course_key, force_update_subsections=True) + CourseGradeFactory().update( + user=user, + course_key=course_key, + force_update_subsections=True + ) diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index 09ddf3e3c7..a6196b7fcd 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -230,11 +230,11 @@ class CreateSubsectionGrade(NonZeroSubsectionGrade): super(CreateSubsectionGrade, self).__init__(subsection, all_total, graded_total) - def update_or_create_model(self, student, score_deleted=False): + def update_or_create_model(self, student, score_deleted=False, force_update_subsections=False): """ Saves or updates the subsection grade in a persisted model. """ - if self._should_persist_per_attempted(score_deleted): + if self._should_persist_per_attempted(score_deleted, force_update_subsections): return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student)) @classmethod @@ -250,17 +250,23 @@ class CreateSubsectionGrade(NonZeroSubsectionGrade): ] return PersistentSubsectionGrade.bulk_create_grades(params, student.id, course_key) - def _should_persist_per_attempted(self, score_deleted=False): + def _should_persist_per_attempted(self, score_deleted=False, force_update_subsections=False): """ Returns whether the SubsectionGrade's model should be persisted based on settings and attempted status. If the learner's score was just deleted, they will have no attempts but the grade should still be persisted. + + If the learner's enrollment track has changed, and the + subsection *only* contains track-specific problems that the + user has attempted, a re-grade will not occur. Should force + a re-grade in this case. See EDUCATOR-1280. """ return ( self.all_total.first_attempted is not None or - score_deleted + score_deleted or + force_update_subsections ) def _persisted_model_params(self, student): diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index dfbd21ec11..fbf57fbc5c 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -63,7 +63,7 @@ class SubsectionGradeFactory(object): ) self._unsaved_subsection_grades.clear() - def update(self, subsection, only_if_higher=None, score_deleted=False): + def update(self, subsection, only_if_higher=None, score_deleted=False, force_update_subsections=False): """ Updates the SubsectionGrade object for the student and subsection. """ @@ -89,7 +89,11 @@ class SubsectionGradeFactory(object): ): return orig_subsection_grade - grade_model = calculated_grade.update_or_create_model(self.student, score_deleted) + grade_model = calculated_grade.update_or_create_model( + self.student, + score_deleted, + force_update_subsections + ) self._update_saved_subsection_grade(subsection.location, grade_model) return calculated_grade diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 577837ab11..8f1db48d1e 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -94,7 +94,7 @@ class TestCourseGradeFactory(GradeTestBase): with self.assertNumQueries(2), mock_get_score(1, 2): _assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0 - with self.assertNumQueries(29), mock_get_score(1, 2): + with self.assertNumQueries(32), mock_get_score(1, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) with self.assertNumQueries(2): @@ -106,7 +106,7 @@ class TestCourseGradeFactory(GradeTestBase): with self.assertNumQueries(2): _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 - with self.assertNumQueries(12), mock_get_score(2, 2): + with self.assertNumQueries(15), mock_get_score(2, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) with self.assertNumQueries(2):