diff --git a/lms/djangoapps/grades/new/subsection_grade.py b/lms/djangoapps/grades/new/subsection_grade.py index 3900f3b0d7..31a004b292 100644 --- a/lms/djangoapps/grades/new/subsection_grade.py +++ b/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, diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 45dcf594c3..cef3d93b5b 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/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: " diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index ce5c0c5a81..dcb5422dbe 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/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}) diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index a24e8a9314..a329085e8c 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/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' diff --git a/openedx/core/lib/grade_utils.py b/openedx/core/lib/grade_utils.py index e7a68a2f06..5cf4dbf340 100644 --- a/openedx/core/lib/grade_utils.py +++ b/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