Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #14946 from edx/neem/refactor_grades_misc

Browse Source 
Grades API misc cleanup
This commit is contained in:
Nimisha Asthagiri
2017-04-25 10:14:25 -04:00
committed by GitHub
parent 1b6778f694 16114f585b
commit 6cdfeb98cb
9 changed files with 43 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
lms/djangoapps/grades/exceptions.py Normal file
View File

@@ -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

6
lms/djangoapps/grades/management/commands/tests/test_reset_grades.py
View File

@@ -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]:

4
lms/djangoapps/grades/models.py
View File

@@ -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.

7
lms/djangoapps/grades/new/course_grade_factory.py
View File

@@ -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,

5
lms/djangoapps/grades/signals/signals.py
View File

@@ -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

18
lms/djangoapps/grades/tasks.py
View File

@@ -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

26
lms/djangoapps/grades/tests/test_models.py
View File

@@ -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):

2
lms/djangoapps/grades/tests/test_new.py
View File

@@ -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)

4
lms/djangoapps/grades/tests/test_tasks.py
View File

@@ -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')