Miscellaneous grades code cleanup - pending from previous PRs

lms/djangoapps/grades/exceptions.py

+"""
+Custom exceptions raised by grades.
+"""
+
+
+class DatabaseNotReadyError(IOError):
+    """
+    Subclass of IOError to indicate the database has not yet committed
+    the data we're trying to find.
+    """
+    pass

lms/djangoapps/grades/management/commands/tests/test_reset_grades.py

@@ -88,7 +88,7 @@ class TestResetGrades(TestCase):
             for user_id in self.user_ids:
                 course_grade_params['user_id'] = user_id
                 course_grade_params['course_id'] = course_key
-                PersistentCourseGrade.update_or_create_course_grade(**course_grade_params)
+                PersistentCourseGrade.update_or_create(**course_grade_params)
                 for subsection_key in self.subsection_keys_by_course[course_key]:
                     subsection_grade_params['user_id'] = user_id
                     subsection_grade_params['usage_key'] = subsection_key
@@ -100,7 +100,7 @@ class TestResetGrades(TestCase):
         """
         for course_key in course_keys:
             if db_table == "course" or db_table is None:
-                self.assertIsNotNone(PersistentCourseGrade.read_course_grade(self.user_ids[0], course_key))
+                self.assertIsNotNone(PersistentCourseGrade.read(self.user_ids[0], course_key))
             if db_table == "subsection" or db_table is None:
                 for subsection_key in self.subsection_keys_by_course[course_key]:
                     self.assertIsNotNone(PersistentSubsectionGrade.read_grade(self.user_ids[0], subsection_key))
@@ -112,7 +112,7 @@ class TestResetGrades(TestCase):
         for course_key in course_keys:
             if db_table == "course" or db_table is None:
                 with self.assertRaises(PersistentCourseGrade.DoesNotExist):
-                    PersistentCourseGrade.read_course_grade(self.user_ids[0], course_key)
+                    PersistentCourseGrade.read(self.user_ids[0], course_key)
 
             if db_table == "subsection" or db_table is None:
                 for subsection_key in self.subsection_keys_by_course[course_key]:

lms/djangoapps/grades/models.py

@@ -533,7 +533,7 @@ class PersistentCourseGrade(DeleteGradesMixin, TimeStampedModel):
         ])
 
     @classmethod
-    def read_course_grade(cls, user_id, course_id):
+    def read(cls, user_id, course_id):
         """
         Reads a grade from database
 
@@ -546,7 +546,7 @@ class PersistentCourseGrade(DeleteGradesMixin, TimeStampedModel):
         return cls.objects.get(user_id=user_id, course_id=course_id)
 
     @classmethod
-    def update_or_create_course_grade(cls, user_id, course_id, **kwargs):
+    def update_or_create(cls, user_id, course_id, **kwargs):
         """
         Creates a course grade in the database.
         Returns a PersistedCourseGrade object.

lms/djangoapps/grades/new/course_grade_factory.py

@@ -105,8 +105,7 @@ class CourseGradeFactory(object):
                     # Keep marching on even if this student couldn't be graded for
                     # some reason, but log it for future reference.
                     log.exception(
-                        'Cannot grade student %s (%s) in course %s because of exception: %s',
-                        student.username,
+                        'Cannot grade student %s in course %s because of exception: %s',
                         student.id,
                         course.id,
                         exc.message
@@ -130,7 +129,7 @@ class CourseGradeFactory(object):
         if not should_persist_grades(course_data.course_key):
             raise PersistentCourseGrade.DoesNotExist
 
-        persistent_grade = PersistentCourseGrade.read_course_grade(user.id, course_data.course_key)
+        persistent_grade = PersistentCourseGrade.read(user.id, course_data.course_key)
         course_grade = CourseGrade(
             user,
             course_data,
@@ -159,7 +158,7 @@ class CourseGradeFactory(object):
         )
         if should_persist:
             course_grade._subsection_grade_factory.bulk_create_unsaved()
-            PersistentCourseGrade.update_or_create_course_grade(
+            PersistentCourseGrade.update_or_create(
                 user_id=user.id,
                 course_id=course_data.course_key,
                 course_version=course_data.version,

lms/djangoapps/grades/signals/signals.py

@@ -78,3 +78,8 @@ SUBSECTION_SCORE_CHANGED = Signal(
         'subsection_grade',  # SubsectionGrade object
     ]
 )
+
+
+# Signal that indicates that a user's grade for a course has been updated.
+# This is a downstream signal of SUBSECTION_SCORE_CHANGED.
+from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED

lms/djangoapps/grades/tasks.py

@@ -31,24 +31,17 @@ from util.date_utils import from_timestamp
 from xmodule.modulestore.django import modulestore
 
 from .constants import ScoreDatabaseTableEnum
+from .exceptions import DatabaseNotReadyError
 from .new.subsection_grade_factory import SubsectionGradeFactory
 from .new.course_grade_factory import CourseGradeFactory
 from .signals.signals import SUBSECTION_SCORE_CHANGED
 from .transformer import GradesTransformer
 
 
-class DatabaseNotReadyError(IOError):
-    """
-    Subclass of IOError to indicate the database has not yet committed
-    the data we're trying to find.
-    """
-    pass
-
-
 KNOWN_RETRY_ERRORS = (  # Errors we expect occasionally, should be resolved on retry
     DatabaseError,
     ValidationError,
-    DatabaseNotReadyError
+    DatabaseNotReadyError,
 )
 RECALCULATE_GRADE_DELAY = 2  # in seconds, to prevent excessive _has_db_updated failures. See TNL-6424.
 
@@ -189,12 +182,7 @@ def _has_db_updated_with_new_score(self, scored_block_usage_key, **kwargs):
     return db_is_updated
 
 
-def _update_subsection_grades(
-        course_key,
-        scored_block_usage_key,
-        only_if_higher,
-        user_id,
-):
+def _update_subsection_grades(course_key, scored_block_usage_key, only_if_higher, user_id):
     """
     A helper function to update subsection grades in the database
     for each subsection containing the given block, and to signal

lms/djangoapps/grades/tests/test_models.py

@@ -349,10 +349,10 @@ class PersistentCourseGradesTest(GradesModelTestCase):
         }
 
     def test_update(self):
-        created_grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+        created_grade = PersistentCourseGrade.update_or_create(**self.params)
         self.params["percent_grade"] = 88.8
         self.params["letter_grade"] = "Better job"
-        updated_grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+        updated_grade = PersistentCourseGrade.update_or_create(**self.params)
         self.assertEqual(updated_grade.percent_grade, 88.8)
         self.assertEqual(updated_grade.letter_grade, "Better job")
         self.assertEqual(created_grade.id, updated_grade.id)
@@ -364,7 +364,7 @@ class PersistentCourseGradesTest(GradesModelTestCase):
             u'letter_grade': u'',
             u'passed': False,
         })
-        grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+        grade = PersistentCourseGrade.update_or_create(**self.params)
         self.assertIsNone(grade.passed_timestamp)
 
         # After the user earns a passing grade, the passed_timestamp is set
@@ -373,7 +373,7 @@ class PersistentCourseGradesTest(GradesModelTestCase):
             u'letter_grade': u'C',
             u'passed': True,
         })
-        grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+        grade = PersistentCourseGrade.update_or_create(**self.params)
         passed_timestamp = grade.passed_timestamp
         self.assertEqual(grade.letter_grade, u'C')
         self.assertIsInstance(passed_timestamp, datetime)
@@ -385,7 +385,7 @@ class PersistentCourseGradesTest(GradesModelTestCase):
             u'letter_grade': u'A',
             u'passed': True,
         })
-        grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+        grade = PersistentCourseGrade.update_or_create(**self.params)
         self.assertEqual(grade.letter_grade, u'A')
         self.assertEqual(grade.passed_timestamp, passed_timestamp)
 
@@ -395,18 +395,18 @@ class PersistentCourseGradesTest(GradesModelTestCase):
             u'letter_grade': u'',
             u'passed': False,
         })
-        grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+        grade = PersistentCourseGrade.update_or_create(**self.params)
         self.assertEqual(grade.letter_grade, u'')
         self.assertEqual(grade.passed_timestamp, passed_timestamp)
 
     @freeze_time(now())
     def test_passed_timestamp_is_now(self):
-        grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+        grade = PersistentCourseGrade.update_or_create(**self.params)
         self.assertEqual(now(), grade.passed_timestamp)
 
     def test_create_and_read_grade(self):
-        created_grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
-        read_grade = PersistentCourseGrade.read_course_grade(self.params["user_id"], self.params["course_id"])
+        created_grade = PersistentCourseGrade.update_or_create(**self.params)
+        read_grade = PersistentCourseGrade.read(self.params["user_id"], self.params["course_id"])
         for param in self.params:
             if param == u'passed':
                 continue  # passed/passed_timestamp takes special handling, and is tested separately
@@ -417,7 +417,7 @@ class PersistentCourseGradesTest(GradesModelTestCase):
     @ddt.data('course_version', 'course_edited_timestamp')
     def test_optional_fields(self, field):
         del self.params[field]
-        grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+        grade = PersistentCourseGrade.update_or_create(**self.params)
         self.assertFalse(getattr(grade, field))
 
     @ddt.data(
@@ -432,15 +432,15 @@ class PersistentCourseGradesTest(GradesModelTestCase):
     def test_update_or_create_with_bad_params(self, param, val, error):
         self.params[param] = val
         with self.assertRaises(error):
-            PersistentCourseGrade.update_or_create_course_grade(**self.params)
+            PersistentCourseGrade.update_or_create(**self.params)
 
     def test_grade_does_not_exist(self):
         with self.assertRaises(PersistentCourseGrade.DoesNotExist):
-            PersistentCourseGrade.read_course_grade(self.params["user_id"], self.params["course_id"])
+            PersistentCourseGrade.read(self.params["user_id"], self.params["course_id"])
 
     def test_update_or_create_event(self):
         with patch('lms.djangoapps.grades.models.tracker') as tracker_mock:
-            grade = PersistentCourseGrade.update_or_create_course_grade(**self.params)
+            grade = PersistentCourseGrade.update_or_create(**self.params)
         self._assert_tracker_emitted_event(tracker_mock, grade)
 
     def _assert_tracker_emitted_event(self, tracker_mock, grade):

lms/djangoapps/grades/tests/test_new.py

@@ -163,7 +163,7 @@ class TestCourseGradeFactory(GradeTestBase):
             course_id=self.course.id,
             enabled_for_course=course_setting
         ):
-            with patch('lms.djangoapps.grades.models.PersistentCourseGrade.read_course_grade') as mock_read_grade:
+            with patch('lms.djangoapps.grades.models.PersistentCourseGrade.read') as mock_read_grade:
                 grade_factory.create(self.request.user, self.course)
         self.assertEqual(mock_read_grade.called, feature_flag and course_setting)
 

lms/djangoapps/grades/tests/test_tasks.py

@@ -226,7 +226,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
                 with self.assertNumQueries(num_sql_queries):
                     self._apply_recalculate_subsection_grade()
             with self.assertRaises(PersistentCourseGrade.DoesNotExist):
-                PersistentCourseGrade.read_course_grade(self.user.id, self.course.id)
+                PersistentCourseGrade.read(self.user.id, self.course.id)
             self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
 
     @ddt.data(
@@ -240,7 +240,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             with check_mongo_calls(num_mongo_queries):
                 with self.assertNumQueries(num_sql_queries):
                     self._apply_recalculate_subsection_grade()
-            self.assertIsNotNone(PersistentCourseGrade.read_course_grade(self.user.id, self.course.id))
+            self.assertIsNotNone(PersistentCourseGrade.read(self.user.id, self.course.id))
             self.assertGreater(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
 
     @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send')