PROD-287 | Add an optional treat_undefined_as_zero flag to the helper...

PROD-287 | Add an optional treat_undefined_as_zero flag to the helper functions is_score_higher_or_equal() and compare_scores(), which defaults to False, that we call with value True from SubsectionGradeFactory.update(), which occassionally deals with invalid subsection scores containing zero as the denominator.

lms/djangoapps/grades/subsection_grade_factory.py

@@ -88,10 +88,11 @@ class SubsectionGradeFactory(object):
                 else:
                     orig_subsection_grade = ReadSubsectionGrade(subsection, grade_model, self)
                     if not is_score_higher_or_equal(
-                            orig_subsection_grade.graded_total.earned,
-                            orig_subsection_grade.graded_total.possible,
-                            calculated_grade.graded_total.earned,
-                            calculated_grade.graded_total.possible,
+                        orig_subsection_grade.graded_total.earned,
+                        orig_subsection_grade.graded_total.possible,
+                        calculated_grade.graded_total.earned,
+                        calculated_grade.graded_total.possible,
+                        treat_undefined_as_zero=True,
                     ):
                         return orig_subsection_grade
 

lms/djangoapps/grades/tests/test_subsection_grade_factory.py

@@ -74,6 +74,22 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase):
         # ensure a grade has been persisted
         self.assertEqual(1, len(PersistentSubsectionGrade.objects.all()))
 
+    def test_update_if_higher_zero_denominator(self):
+        """
+        Test that we get an updated score of 0, and not a ZeroDivisionError,
+        when dealing with an invalid score like 0/0.
+        """
+        # This will create a PersistentSubsectionGrade with a score of 0/0.
+        with mock_get_score(0, 0):
+            grade = self.subsection_grade_factory.update(self.sequence)
+        self.assert_grade(grade, 0, 0)
+
+        # Ensure that previously storing a possible score of 0
+        # does not raise a ZeroDivisionError when updating the grade.
+        with mock_get_score(2, 2):
+            grade = self.subsection_grade_factory.update(self.sequence, only_if_higher=True)
+        self.assert_grade(grade, 2, 2)
+
     def test_update_if_higher(self):
         def verify_update_if_higher(mock_score, expected_grade):
             """

openedx/core/lib/grade_utils.py

@@ -3,22 +3,42 @@ Helpers functions for grades and scores.
 """
 
 
-def compare_scores(earned1, possible1, earned2, possible2):
+def compare_scores(earned1, possible1, earned2, possible2, treat_undefined_as_zero=False):
     """
     Returns a tuple of:
         1. Whether the 2nd set of scores is higher than the first.
         2. Grade percentage of 1st set of scores.
         3. Grade percentage of 2nd set of scores.
+    If ``treat_undefined_as_zero`` is True, this function will treat
+    cases where ``possible1`` or ``possible2`` is 0 as if
+    the (earned / possible) score is 0.  If this flag is false,
+    a ZeroDivisionError is raised.
     """
-    percentage1 = float(earned1) / float(possible1)
-    percentage2 = float(earned2) / float(possible2)
+    try:
+        percentage1 = float(earned1) / float(possible1)
+    except ZeroDivisionError:
+        if not treat_undefined_as_zero:
+            raise
+        percentage1 = 0.0
+
+    try:
+        percentage2 = float(earned2) / float(possible2)
+    except ZeroDivisionError:
+        if not treat_undefined_as_zero:
+            raise
+        percentage2 = 0.0
+
     is_higher = percentage2 >= percentage1
     return is_higher, percentage1, percentage2
 
 
-def is_score_higher_or_equal(earned1, possible1, earned2, possible2):
+def is_score_higher_or_equal(earned1, possible1, earned2, possible2, treat_undefined_as_zero=False):
     """
     Returns whether the 2nd set of scores is higher than the first.
+    If ``treat_undefined_as_zero`` is True, this function will treat
+    cases where ``possible1`` or ``possible2`` is 0 as if
+    the (earned / possible) score is 0.  If this flag is false,
+    a ZeroDivisionError is raised.
     """
-    is_higher_or_equal, _, _ = compare_scores(earned1, possible1, earned2, possible2)
+    is_higher_or_equal, _, _ = compare_scores(earned1, possible1, earned2, possible2, treat_undefined_as_zero)
     return is_higher_or_equal

openedx/core/lib/tests/test_grade_utils.py

+"""
+Tests for graph traversal generator functions.
+"""
+
+from __future__ import absolute_import
+from unittest import TestCase
+
+import ddt
+
+from ..grade_utils import compare_scores
+
+
+@ddt.ddt
+class TestGradeUtils(TestCase):
+    """ Tests for the grade_utils module. """
+    @ddt.data(
+        (1, 2, 3, 4, False, True, 0.5, 0.75),
+        (3, 4, 1, 2, False, False, 0.75, 0.5),
+        (1, 2, 1, 2, False, True, 0.5, 0.5),
+        (1, 1, 0, 1, False, False, 1, 0),
+    )
+    @ddt.unpack
+    def test_compare_scores_happy_path(
+        self, earned_1, possible_1, earned_2, possible_2, treat_undefined_as_zero,
+        expected_is_higher, expected_percentage_1, expected_percentage_2
+    ):
+        is_higher, percentage_1, percentage_2 = compare_scores(
+            earned_1, possible_1, earned_2, possible_2, treat_undefined_as_zero
+        )
+        assert expected_is_higher == is_higher
+        assert expected_percentage_1 == percentage_1
+        assert expected_percentage_2 == percentage_2
+
+    def test_compare_scores_raise_zero_division(self):
+        with self.assertRaises(ZeroDivisionError):
+            compare_scores(1, 0, 1, 2)
+
+        with self.assertRaises(ZeroDivisionError):
+            compare_scores(1, 2, 0, 0)
+
+    def test_compare_scores_treat_undefined_as_zero(self):
+        is_higher, percentage_1, percentage_2 = compare_scores(
+            0, 0, 0, 0, treat_undefined_as_zero=True
+        )
+        assert is_higher is True
+        assert 0 == percentage_1
+        assert 0 == percentage_2