Merge pull request #14675 from edx/sstudent/TNL-6644

change rescore-if-higher to update if scores are equal

lms/djangoapps/grades/new/subsection_grade.py

@@ -8,7 +8,7 @@ from courseware.model_data import ScoresClient
 from lms.djangoapps.grades.scores import get_score, possibly_scored
 from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade
 from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag
-from openedx.core.lib.grade_utils import is_score_higher
+from openedx.core.lib.grade_utils import is_score_higher_or_equal
 from student.models import anonymous_id_for_user
 from submissions import api as submissions_api
 from xmodule import block_metadata_utils, graders
@@ -265,7 +265,7 @@ class SubsectionGradeFactory(object):
                 orig_subsection_grade = SubsectionGrade(subsection).init_from_model(
                     self.student, grade_model, self.course_structure, self._submissions_scores, self._csm_scores,
                 )
-                if not is_score_higher(
+                if not is_score_higher_or_equal(
                         orig_subsection_grade.graded_total.earned,
                         orig_subsection_grade.graded_total.possible,
                         calculated_grade.graded_total.earned,

lms/djangoapps/grades/signals/handlers.py

@@ -10,7 +10,7 @@ from xblock.scorable import ScorableXBlockMixin, Score
 
 from courseware.model_data import get_score, set_score
 from eventtracking import tracker
-from openedx.core.lib.grade_utils import is_score_higher
+from openedx.core.lib.grade_utils import is_score_higher_or_equal
 from student.models import user_by_anonymous_id
 from util.date_utils import to_timestamp
 from track.event_transaction_utils import (
@@ -122,7 +122,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_
         if previous_score is not None:
             prev_raw_earned, prev_raw_possible = (previous_score.grade, previous_score.max_grade)
 
-            if not is_score_higher(prev_raw_earned, prev_raw_possible, raw_earned, raw_possible):
+            if not is_score_higher_or_equal(prev_raw_earned, prev_raw_possible, raw_earned, raw_possible):
                 update_score = False
                 log.warning(
                     u"Grades: Rescore is not higher than previous: "

lms/djangoapps/grades/tests/test_new.py

@@ -251,8 +251,8 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase):
                 self.assert_grade(grade, *expected_grade)
 
         verify_update_if_higher((1, 2), (1, 2))  # previous value was non-existent
-        verify_update_if_higher((2, 4), (1, 2))  # previous value was equivalent
-        verify_update_if_higher((1, 4), (1, 2))  # previous value was greater
+        verify_update_if_higher((2, 4), (2, 4))  # previous value was equivalent
+        verify_update_if_higher((1, 4), (2, 4))  # previous value was greater
         verify_update_if_higher((3, 4), (3, 4))  # previous value was less
 
     @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False})

lms/djangoapps/instructor_task/tests/test_integration.py

@@ -224,7 +224,6 @@ class TestRescoringTask(TestIntegrationTask):
     @ddt.data(
         RescoreTestData(edit=dict(), new_expected_scores=(2, 1, 1, 0), new_expected_max=2),
         RescoreTestData(edit=dict(correct_answer=OPTION_2), new_expected_scores=(2, 1, 1, 2), new_expected_max=2),
-        RescoreTestData(edit=dict(num_inputs=2), new_expected_scores=(2, 1, 1, 0), new_expected_max=2),
     )
     @ddt.unpack
     def test_rescoring_if_higher(self, problem_edit, new_expected_scores, new_expected_max):
@@ -232,6 +231,43 @@ class TestRescoringTask(TestIntegrationTask):
             problem_edit, new_expected_scores, new_expected_max, rescore_if_higher=True,
         )
 
+    def test_rescoring_if_higher_scores_equal(self):
+        """
+        Specifically tests rescore when the previous and new raw scores are equal. In this case, the scores should
+        be updated.
+        """
+        problem_edit = dict(num_inputs=2)  # this change to the problem means the problem will now have a max score of 4
+        unchanged_max = 2
+        new_max = 4
+        problem_url_name = 'H1P1'
+        self.define_option_problem(problem_url_name)
+        location = InstructorTaskModuleTestCase.problem_location(problem_url_name)
+        descriptor = self.module_store.get_item(location)
+
+        # first store answers for each of the separate users:
+        self.submit_student_answer('u1', problem_url_name, [OPTION_1, OPTION_1])
+        self.submit_student_answer('u2', problem_url_name, [OPTION_2, OPTION_2])
+
+        # verify each user's grade
+        self.check_state(self.user1, descriptor, 2, 2)  # user 1 has a 2/2
+        self.check_state(self.user2, descriptor, 0, 2)  # user 2 has a 0/2
+
+        # update the data in the problem definition so the answer changes.
+        self.redefine_option_problem(problem_url_name, **problem_edit)
+
+        # confirm that simply rendering the problem again does not change the grade
+        self.render_problem('u1', problem_url_name)
+        self.check_state(self.user1, descriptor, 2, 2)
+        self.check_state(self.user2, descriptor, 0, 2)
+
+        # rescore the problem for all students
+        self.submit_rescore_all_student_answers('instructor', problem_url_name, True)
+
+        # user 1's score would go down, so it remains 2/2. user 2's score was 0/2, which is equivalent to the new score
+        # of 0/4, so user 2's score changes to 0/4.
+        self.check_state(self.user1, descriptor, 2, unchanged_max)
+        self.check_state(self.user2, descriptor, 0, new_max)
+
     def test_rescoring_failure(self):
         """Simulate a failure in rescoring a problem"""
         problem_url_name = 'H1P1'

openedx/core/lib/grade_utils.py

@@ -12,13 +12,13 @@ def compare_scores(earned1, possible1, earned2, possible2):
     """
     percentage1 = float(earned1) / float(possible1)
     percentage2 = float(earned2) / float(possible2)
-    is_higher = percentage2 > percentage1
+    is_higher = percentage2 >= percentage1
     return is_higher, percentage1, percentage2
 
 
-def is_score_higher(earned1, possible1, earned2, possible2):
+def is_score_higher_or_equal(earned1, possible1, earned2, possible2):
     """
     Returns whether the 2nd set of scores is higher than the first.
     """
-    is_higher, _, _ = compare_scores(earned1, possible1, earned2, possible2)
-    return is_higher
+    is_higher_or_equal, _, _ = compare_scores(earned1, possible1, earned2, possible2)
+    return is_higher_or_equal