diff --git a/lms/djangoapps/grades/exceptions.py b/lms/djangoapps/grades/exceptions.py new file mode 100644 index 0000000000..f8394d8db7 --- /dev/null +++ b/lms/djangoapps/grades/exceptions.py @@ -0,0 +1,11 @@ +""" +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 diff --git a/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py b/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py index b07a7ddbdc..3cfbbaf5e8 100644 --- a/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py +++ b/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]: diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index f9036851bf..16c9c4fbf0 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -536,7 +536,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 @@ -549,7 +549,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. diff --git a/lms/djangoapps/grades/new/course_grade_factory.py b/lms/djangoapps/grades/new/course_grade_factory.py index 410a9f81bc..af3ae8f8b1 100644 --- a/lms/djangoapps/grades/new/course_grade_factory.py +++ b/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, diff --git a/lms/djangoapps/grades/signals/signals.py b/lms/djangoapps/grades/signals/signals.py index 4dc274f6ae..0cf2976db4 100644 --- a/lms/djangoapps/grades/signals/signals.py +++ b/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 diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index d95a4edfa9..28fc798c71 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -32,24 +32,17 @@ from xmodule.modulestore.django import modulestore from .config.waffle import waffle, ESTIMATE_FIRST_ATTEMPTED 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. @@ -219,12 +212,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 diff --git a/lms/djangoapps/grades/tests/test_models.py b/lms/djangoapps/grades/tests/test_models.py index 0d1bee91aa..dfcc39354b 100644 --- a/lms/djangoapps/grades/tests/test_models.py +++ b/lms/djangoapps/grades/tests/test_models.py @@ -360,10 +360,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) @@ -375,7 +375,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 @@ -384,7 +384,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) @@ -396,7 +396,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) @@ -406,18 +406,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 @@ -428,7 +428,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( @@ -443,15 +443,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): diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index 69cbaf22bf..56d0caad6e 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/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) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 55db5655ef..2a011481d1 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/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')