Trigger recalculate subsection, undo override

lms/djangoapps/grades/constants.py

@@ -9,3 +9,4 @@ class ScoreDatabaseTableEnum(object):
     """
     courseware_student_module = 'csm'
     submissions = 'submissions'
+    overrides = 'overrides'

lms/djangoapps/grades/models.py

@@ -695,6 +695,10 @@ class PersistentSubsectionGradeOverride(models.Model):
 
     grade = models.OneToOneField(PersistentSubsectionGrade, related_name='override')
 
+    # Created/modified timestamps prevent race-conditions when using with async rescoring tasks
+    created = models.DateTimeField(auto_now_add=True, db_index=True)
+    modified = models.DateTimeField(auto_now=True, db_index=True)
+
     # 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.
     earned_all_override = models.FloatField(null=True, blank=True)

lms/djangoapps/grades/services.py

+from datetime import datetime
+
+import pytz
+
 from opaque_keys.edx.keys import CourseKey, UsageKey
-from lms.djangoapps.grades.models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride
+
+from .constants import ScoreDatabaseTableEnum
+from .models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride
 
 
 def _get_key(key_or_id, key_cls):
@@ -24,48 +30,93 @@ class GradesService(object):
     def get_subsection_grade(self, user_id, course_key_or_id, usage_key_or_id):
         """
         Finds and returns the earned subsection grade for user
-
-        Result is a dict of two key value pairs with keys: earned_all and earned_graded.
         """
         course_key = _get_key(course_key_or_id, CourseKey)
         usage_key = _get_key(usage_key_or_id, UsageKey)
 
-        grade = PersistentSubsectionGrade.objects.get(
+        return PersistentSubsectionGrade.objects.get(
             user_id=user_id,
             course_id=course_key,
             usage_key=usage_key
         )
-        return {
-            'earned_all': grade.earned_all,
-            'earned_graded': grade.earned_graded
-        }
+
+    def get_subsection_grade_override(self, user_id, course_key_or_id, usage_key_or_id):
+        """
+        Finds the subsection grade for user and returns the override for that grade if it exists
+
+        If override does not exist, returns None. If subsection grade does not exist, will raise an exception.
+        """
+        course_key = _get_key(course_key_or_id, CourseKey)
+        usage_key = _get_key(usage_key_or_id, UsageKey)
+
+        grade = self.get_subsection_grade(user_id, course_key, usage_key)
+
+        try:
+            return PersistentSubsectionGradeOverride.objects.get(
+                grade=grade
+            )
+        except PersistentSubsectionGradeOverride.DoesNotExist:
+            return None
 
     def override_subsection_grade(self, user_id, course_key_or_id, usage_key_or_id, earned_all=None,
                                   earned_graded=None):
         """
         Override subsection grade (the PersistentSubsectionGrade model must already exist)
 
-        Will not override earned_all or earned_graded value if they are None. Both default to None.
+        Fires off a recalculate_subsection_grade async task to update the PersistentSubsectionGrade table. Will not
+        override earned_all or earned_graded value if they are None. Both default to None.
         """
+        from .tasks import recalculate_subsection_grade_v3  # prevent circular import
+
         course_key = _get_key(course_key_or_id, CourseKey)
-        subsection_key = _get_key(usage_key_or_id, UsageKey)
+        usage_key = _get_key(usage_key_or_id, UsageKey)
 
         grade = PersistentSubsectionGrade.objects.get(
             user_id=user_id,
             course_id=course_key,
-            usage_key=subsection_key
+            usage_key=usage_key
         )
 
         # Create override that will prevent any future updates to grade
-        PersistentSubsectionGradeOverride.objects.create(
+        override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(
             grade=grade,
             earned_all_override=earned_all,
             earned_graded_override=earned_graded
         )
 
-        # Change the grade as it is now
-        if earned_all is not None:
-            grade.earned_all = earned_all
-        if earned_graded is not None:
-            grade.earned_graded = earned_graded
-        grade.save()
+        # Recalculation will call PersistentSubsectionGrade.update_or_create_grade which will use the above override
+        # to update the grade before writing to the table.
+        recalculate_subsection_grade_v3.apply_async(
+            sender=None,
+            user_id=user_id,
+            course_id=unicode(course_key),
+            usage_id=unicode(usage_key),
+            only_if_higher=False,
+            expeected_modified=override.modified,
+            score_db_table=ScoreDatabaseTableEnum.overrides
+        )
+
+    def undo_override_subsection_grade(self, user_id, course_key_or_id, usage_key_or_id):
+        """
+        Delete the override subsection grade row (the PersistentSubsectionGrade model must already exist)
+
+        Fires off a recalculate_subsection_grade async task to update the PersistentSubsectionGrade table.
+        """
+        from .tasks import recalculate_subsection_grade_v3  # prevent circular import
+
+        course_key = _get_key(course_key_or_id, CourseKey)
+        usage_key = _get_key(usage_key_or_id, UsageKey)
+
+        override = self.get_subsection_grade_override(user_id, course_key, usage_key)
+        override.delete()
+
+        recalculate_subsection_grade_v3.apply_async(
+            sender=None,
+            user_id=user_id,
+            course_id=unicode(course_key),
+            usage_id=unicode(usage_key),
+            only_if_higher=False,
+            expected_modified=datetime.now().replace(tzinfo=pytz.UTC),  # Not used when score_deleted=True
+            score_deleted=True,
+            score_db_table=ScoreDatabaseTableEnum.overrides
+        )

lms/djangoapps/grades/tasks.py

@@ -31,6 +31,7 @@ from .constants import ScoreDatabaseTableEnum
 from .exceptions import DatabaseNotReadyError
 from .new.course_grade_factory import CourseGradeFactory
 from .new.subsection_grade_factory import SubsectionGradeFactory
+from .services import GradesService
 from .signals.signals import SUBSECTION_SCORE_CHANGED
 from .transformer import GradesTransformer
 
@@ -201,8 +202,7 @@ def _has_db_updated_with_new_score(self, scored_block_usage_key, **kwargs):
         score = get_score(kwargs['user_id'], scored_block_usage_key)
         found_modified_time = score.modified if score is not None else None
 
-    else:
-        assert kwargs['score_db_table'] == ScoreDatabaseTableEnum.submissions
+    elif kwargs['score_db_table'] == ScoreDatabaseTableEnum.submissions:
         score = sub_api.get_score(
             {
                 "student_id": kwargs['anonymous_user_id'],
@@ -212,6 +212,14 @@ def _has_db_updated_with_new_score(self, scored_block_usage_key, **kwargs):
             }
         )
         found_modified_time = score['created_at'] if score is not None else None
+    else:
+        assert kwargs['score_db_table'] == ScoreDatabaseTableEnum.overrides
+        score = GradesService().get_subsection_grade_override(
+            user_id=kwargs['user_id'],
+            course_key_or_id=kwargs['course_id'],
+            usage_key_or_id=kwargs['usage_id']
+        )
+        found_modified_time = score.modified if score is not None else None
 
     if score is None:
         # score should be None only if it was deleted.

lms/djangoapps/grades/tests/test_services.py

 import ddt
+import pytz
+from datetime import datetime
+from freezegun import freeze_time
 from lms.djangoapps.grades.models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride
 from lms.djangoapps.grades.services import GradesService, _get_key
+from mock import patch
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 
+from ..constants import ScoreDatabaseTableEnum
+
 
 @ddt.ddt
 class GradesServiceTests(ModuleStoreTestCase):
@@ -29,27 +35,73 @@ class GradesServiceTests(ModuleStoreTestCase):
             earned_graded=5.0,
             possible_graded=5.0
         )
+        self.patcher = patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.apply_async')
+        self.mock_recalculate = self.patcher.start()
+
+    def tearDown(self):
+        self.patcher.stop()
+
+    def subsection_grade_to_dict(self, grade):
+        return {
+            'earned_all': grade.earned_all,
+            'earned_graded': grade.earned_graded
+        }
+
+    def subsection_grade_override_to_dict(self, grade):
+        return {
+            'earned_all_override': grade.earned_all_override,
+            'earned_graded_override': grade.earned_graded_override
+        }
 
     def test_get_subsection_grade(self):
-        self.assertDictEqual(self.service.get_subsection_grade(
+        self.assertDictEqual(self.subsection_grade_to_dict(self.service.get_subsection_grade(
             user_id=self.user.id,
             course_key_or_id=self.course.id,
             usage_key_or_id=self.subsection.location
-        ), {
+        )), {
             'earned_all': 6.0,
             'earned_graded': 5.0
         })
 
         # test with id strings as parameters instead
-        self.assertDictEqual(self.service.get_subsection_grade(
+        self.assertDictEqual(self.subsection_grade_to_dict(self.service.get_subsection_grade(
             user_id=self.user.id,
-            course_key_or_id=str(self.course.id),
-            usage_key_or_id=str(self.subsection.location)
-        ), {
+            course_key_or_id=unicode(self.course.id),
+            usage_key_or_id=unicode(self.subsection.location)
+        )), {
             'earned_all': 6.0,
             'earned_graded': 5.0
         })
 
+    def test_get_subsection_grade_override(self):
+        override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(grade=self.grade)
+
+        self.assertDictEqual(self.subsection_grade_override_to_dict(self.service.get_subsection_grade_override(
+            user_id=self.user.id,
+            course_key_or_id=self.course.id,
+            usage_key_or_id=self.subsection.location
+        )), {
+            'earned_all_override': override.earned_all_override,
+            'earned_graded_override': override.earned_graded_override
+        })
+
+        override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(
+            grade=self.grade,
+            defaults={
+                'earned_all_override': 9.0
+            }
+        )
+
+        # test with id strings as parameters instead
+        self.assertDictEqual(self.subsection_grade_override_to_dict(self.service.get_subsection_grade_override(
+            user_id=self.user.id,
+            course_key_or_id=unicode(self.course.id),
+            usage_key_or_id=unicode(self.subsection.location)
+        )), {
+            'earned_all_override': override.earned_all_override,
+            'earned_graded_override': override.earned_graded_override
+        })
+
     @ddt.data(
         [{
             'earned_all': 0.0,
@@ -92,14 +144,48 @@ class GradesServiceTests(ModuleStoreTestCase):
             earned_graded=override['earned_graded']
         )
 
-        grade = PersistentSubsectionGrade.objects.get(
+        override_obj = self.service.get_subsection_grade_override(
+            self.user.id,
+            self.course.id,
+            self.subsection.location
+        )
+        self.assertIsNotNone(override_obj)
+        self.assertEqual(override_obj.earned_all_override, override['earned_all'])
+        self.assertEqual(override_obj.earned_graded_override, override['earned_graded'])
+
+        self.mock_recalculate.called_with(
+            sender=None,
             user_id=self.user.id,
-            course_id=self.course.id,
-            usage_key=self.subsection.location
+            course_id=unicode(self.course.id),
+            usage_id=unicode(self.subsection.location),
+            only_if_higher=False,
+            expected_modified=override_obj.modified,
+            score_db_table=ScoreDatabaseTableEnum.overrides
+        )
+
+    @freeze_time('2017-01-01')
+    def test_undo_override_subsection_grade(self):
+        override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(grade=self.grade)
+
+        self.service.undo_override_subsection_grade(
+            user_id=self.user.id,
+            course_key_or_id=self.course.id,
+            usage_key_or_id=self.subsection.location,
         )
 
-        self.assertEqual(grade.earned_all, expected['earned_all'])
-        self.assertEqual(grade.earned_graded, expected['earned_graded'])
+        override = self.service.get_subsection_grade_override(self.user.id, self.course.id, self.subsection.location)
+        self.assertIsNone(override)
+
+        self.mock_recalculate.called_with(
+            sender=None,
+            user_id=self.user.id,
+            course_id=unicode(self.course.id),
+            usage_id=unicode(self.subsection.location),
+            only_if_higher=False,
+            expected_modified=datetime.now().replace(tzinfo=pytz.UTC),
+            score_deleted=True,
+            score_db_table=ScoreDatabaseTableEnum.overrides
+        )
 
     @ddt.data(
         ['edX/DemoX/Demo_Course', CourseKey.from_string('edX/DemoX/Demo_Course'), CourseKey],

lms/djangoapps/grades/tests/test_tasks.py

@@ -17,6 +17,7 @@ from mock import MagicMock, patch
 from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag
 from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum
 from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGrade
+from lms.djangoapps.grades.services import GradesService
 from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED
 from lms.djangoapps.grades.tasks import (
     RECALCULATE_GRADE_DELAY,
@@ -36,6 +37,15 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
 
 
+class MockGradesService(GradesService):
+    def __init__(self, mocked_return_value=None):
+        super(MockGradesService, self).__init__()
+        self.mocked_return_value = mocked_return_value
+
+    def get_subsection_grade_override(self, user_id, course_key_or_id, usage_key_or_id):
+        return self.mocked_return_value
+
+
 class HasCourseWithProblemsMixin(object):
     """
     Mixin to provide tests with a sample course with graded subsections
@@ -153,10 +163,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEquals(mock_block_structure_create.call_count, 1)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 28, True),
-        (ModuleStoreEnum.Type.mongo, 1, 24, False),
-        (ModuleStoreEnum.Type.split, 3, 28, True),
-        (ModuleStoreEnum.Type.split, 3, 24, False),
+        (ModuleStoreEnum.Type.mongo, 1, 29, True),
+        (ModuleStoreEnum.Type.mongo, 1, 25, False),
+        (ModuleStoreEnum.Type.split, 3, 29, True),
+        (ModuleStoreEnum.Type.split, 3, 25, False),
     )
     @ddt.unpack
     def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
@@ -168,8 +178,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
                     self._apply_recalculate_subsection_grade()
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 28),
-        (ModuleStoreEnum.Type.split, 3, 28),
+        (ModuleStoreEnum.Type.mongo, 1, 29),
+        (ModuleStoreEnum.Type.split, 3, 29),
     )
     @ddt.unpack
     def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
@@ -229,8 +239,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 25),
-        (ModuleStoreEnum.Type.split, 3, 25),
+        (ModuleStoreEnum.Type.mongo, 1, 26),
+        (ModuleStoreEnum.Type.split, 3, 26),
     )
     @ddt.unpack
     def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):
@@ -264,7 +274,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
         self._apply_recalculate_subsection_grade()
         self._assert_retry_called(mock_retry)
 
-    @ddt.data(ScoreDatabaseTableEnum.courseware_student_module, ScoreDatabaseTableEnum.submissions)
+    @ddt.data(ScoreDatabaseTableEnum.courseware_student_module, ScoreDatabaseTableEnum.submissions,
+              ScoreDatabaseTableEnum.overrides)
     @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry')
     @patch('lms.djangoapps.grades.tasks.log')
     def test_retry_when_db_not_updated(self, score_db_table, mock_log, mock_retry):
@@ -279,10 +290,16 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
                 self._apply_recalculate_subsection_grade(
                     mock_score=MagicMock(module_type='any_block_type')
                 )
-        else:
+        elif score_db_table == ScoreDatabaseTableEnum.courseware_student_module:
             self._apply_recalculate_subsection_grade(
                 mock_score=MagicMock(modified=modified_datetime)
             )
+        else:
+            with patch(
+                'lms.djangoapps.grades.tasks.GradesService',
+                return_value=MockGradesService(mocked_return_value=MagicMock(modified=modified_datetime))
+            ):
+                recalculate_subsection_grade_v3.apply(kwargs=self.recalculate_subsection_grade_kwargs)
 
         self._assert_retry_called(mock_retry)
         self.assertIn(
@@ -293,7 +310,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
     @ddt.data(
         *itertools.product(
             (True, False),
-            (ScoreDatabaseTableEnum.courseware_student_module, ScoreDatabaseTableEnum.submissions),
+            (ScoreDatabaseTableEnum.courseware_student_module, ScoreDatabaseTableEnum.submissions,
+             ScoreDatabaseTableEnum.overrides),
         )
     )
     @ddt.unpack
@@ -310,6 +328,11 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
                 self._apply_recalculate_subsection_grade(
                     mock_score=MagicMock(module_type='any_block_type')
                 )
+        elif score_db_table == ScoreDatabaseTableEnum.overrides:
+            with patch('lms.djangoapps.grades.tasks.GradesService',
+                       return_value=MockGradesService(mocked_return_value=None)) as mock_service:
+                mock_service.get_subsection_grade_override.return_value = None
+                recalculate_subsection_grade_v3.apply(kwargs=self.recalculate_subsection_grade_kwargs)
         else:
             self._apply_recalculate_subsection_grade(mock_score=None)