From 6f8383199fa0c695b4e7309edfed136be5e86aca Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Fri, 27 Jan 2017 09:26:50 -0500 Subject: [PATCH] Allow null edited_timestamp Some old mongo courses do not have this field, and the team has opted to allow null values rather than inserting a default. This change affects both course and subsection grades. TNL-6408 --- .../migrations/0011_null_edited_time.py | 29 +++++++++++++++++++ lms/djangoapps/grades/models.py | 6 ++-- lms/djangoapps/grades/new/subsection_grade.py | 4 +-- lms/djangoapps/grades/signals/handlers.py | 4 +-- lms/djangoapps/grades/tasks.py | 1 + lms/djangoapps/grades/tests/test_models.py | 13 +++++---- lms/djangoapps/grades/tests/test_tasks.py | 4 +-- 7 files changed, 46 insertions(+), 15 deletions(-) create mode 100644 lms/djangoapps/grades/migrations/0011_null_edited_time.py diff --git a/lms/djangoapps/grades/migrations/0011_null_edited_time.py b/lms/djangoapps/grades/migrations/0011_null_edited_time.py new file mode 100644 index 0000000000..e8e0da92f8 --- /dev/null +++ b/lms/djangoapps/grades/migrations/0011_null_edited_time.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('grades', '0010_auto_20170112_1156'), + ] + + operations = [ + migrations.AlterField( + model_name='persistentcoursegrade', + name='course_edited_timestamp', + field=models.DateTimeField(null=True, verbose_name='Last content edit timestamp', blank=True), + ), + migrations.AlterField( + model_name='persistentsubsectiongrade', + name='course_version', + field=models.CharField(max_length=255, verbose_name='Guid of latest course version', blank=True), + ), + migrations.AlterField( + model_name='persistentsubsectiongrade', + name='subtree_edited_timestamp', + field=models.DateTimeField(null=True, verbose_name='Last content edit timestamp', blank=True), + ), + ] diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index cb21a7539a..ed6c66426b 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -277,8 +277,8 @@ class PersistentSubsectionGrade(DeleteGradesMixin, TimeStampedModel): usage_key = UsageKeyField(blank=False, max_length=255) # Information relating to the state of content when grade was calculated - subtree_edited_timestamp = models.DateTimeField('last content edit timestamp', blank=False) - course_version = models.CharField('guid of latest course version', blank=True, max_length=255) + subtree_edited_timestamp = models.DateTimeField(u'Last content edit timestamp', blank=True, null=True) + course_version = models.CharField(u'Guid of latest course version', blank=True, max_length=255) # earned/possible refers to the number of points achieved and available to achieve. # graded refers to the subset of all problems that are marked as being graded. @@ -529,7 +529,7 @@ class PersistentCourseGrade(DeleteGradesMixin, TimeStampedModel): course_id = CourseKeyField(blank=False, max_length=255) # Information relating to the state of content when grade was calculated - course_edited_timestamp = models.DateTimeField(u'Last content edit timestamp', blank=False) + course_edited_timestamp = models.DateTimeField(u'Last content edit timestamp', blank=True, null=True) course_version = models.CharField(u'Course content version identifier', blank=True, max_length=255) grading_policy_hash = models.CharField(u'Hash of grading policy', blank=False, max_length=255) diff --git a/lms/djangoapps/grades/new/subsection_grade.py b/lms/djangoapps/grades/new/subsection_grade.py index ef5e4597fe..3900f3b0d7 100644 --- a/lms/djangoapps/grades/new/subsection_grade.py +++ b/lms/djangoapps/grades/new/subsection_grade.py @@ -32,7 +32,7 @@ class SubsectionGrade(object): self.graded = getattr(subsection, 'graded', False) self.course_version = getattr(subsection, 'course_version', None) - self.subtree_edited_timestamp = subsection.subtree_edited_on + self.subtree_edited_timestamp = getattr(subsection, 'subtree_edited_on', None) self.graded_total = None # aggregated grade for all graded problems self.all_total = None # aggregated grade for all problems, regardless of whether they are graded @@ -341,6 +341,6 @@ class SubsectionGradeFactory(object): log_statement, self.course.id, getattr(subsection, 'course_version', None), - subsection.subtree_edited_on, + getattr(subsection, 'subtree_edited_on', None), self.student.id, )) diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index cdbe957f4e..8b64ddf2c3 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -27,7 +27,7 @@ from .signals import ( from ..constants import ScoreDatabaseTableEnum from ..new.course_grade import CourseGradeFactory from ..scores import weighted_score -from ..tasks import recalculate_subsection_grade_v3 +from ..tasks import recalculate_subsection_grade_v3, RECALCULATE_GRADE_DELAY log = getLogger(__name__) @@ -192,7 +192,7 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum event_transaction_type=unicode(get_event_transaction_type()), score_db_table=kwargs['score_db_table'], ), - countdown=2, + countdown=RECALCULATE_GRADE_DELAY, ) log.info( u'Grades: Request async calculation of subsection grades with args: {}. Task [{}]'.format( diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 4782ba6a6f..c838471825 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -33,6 +33,7 @@ from .transformer import GradesTransformer log = getLogger(__name__) KNOWN_RETRY_ERRORS = (DatabaseError, ValidationError) # Errors we expect occasionally, should be resolved on retry +RECALCULATE_GRADE_DELAY = 2 # in seconds, to prevent excessive _has_db_updated failures. See TNL-6424. @task(bind=True, base=PersistOnFailureTask, default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) diff --git a/lms/djangoapps/grades/tests/test_models.py b/lms/djangoapps/grades/tests/test_models.py index 9b22fa6cab..d8805a06f2 100644 --- a/lms/djangoapps/grades/tests/test_models.py +++ b/lms/djangoapps/grades/tests/test_models.py @@ -231,14 +231,14 @@ class PersistentSubsectionGradeTest(GradesModelTestCase): with self.assertRaises(ValidationError): PersistentSubsectionGrade.create_grade(**self.params) - def test_optional_fields(self): - del self.params["course_version"] + @ddt.data('course_version', 'subtree_edited_timestamp') + def test_optional_fields(self, field): + del self.params[field] PersistentSubsectionGrade.create_grade(**self.params) @ddt.data( ("user_id", ValidationError), ("usage_key", KeyError), - ("subtree_edited_timestamp", ValidationError), ("earned_all", ValidationError), ("possible_all", ValidationError), ("earned_graded", ValidationError), @@ -427,10 +427,11 @@ class PersistentCourseGradesTest(GradesModelTestCase): self.assertIsInstance(created_grade.passed_timestamp, datetime) self.assertEqual(created_grade, read_grade) - def test_course_version_optional(self): - del self.params["course_version"] + @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) - self.assertEqual("", grade.course_version) + self.assertFalse(getattr(grade, field)) @ddt.data( ("percent_grade", "Not a float at all", ValueError), diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index c70fb88aeb..bc669d4b45 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -27,7 +27,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, chec from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED -from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3 +from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3, RECALCULATE_GRADE_DELAY @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @@ -114,7 +114,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): return_value=None ) as mock_task_apply: PROBLEM_WEIGHTED_SCORE_CHANGED.send(sender=None, **send_args) - mock_task_apply.assert_called_once_with(kwargs=local_task_args) + mock_task_apply.assert_called_once_with(countdown=RECALCULATE_GRADE_DELAY, kwargs=local_task_args) @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') def test_triggers_subsection_score_signal(self, mock_subsection_signal):