Use signal instead of calling recalculate directly

lms/djangoapps/grades/services.py

@@ -9,6 +9,7 @@ from util.date_utils import to_timestamp
 from .config.waffle import waffle_flags, REJECTED_EXAM_OVERRIDES_GRADE
 from .constants import ScoreDatabaseTableEnum
 from .models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride
+from .signals.signals import SUBSECTION_OVERRIDE_CHANGED
 
 
 def _get_key(key_or_id, key_cls):
@@ -70,8 +71,7 @@ class GradesService(object):
         override earned_all or earned_graded value if they are None. Both default to None.
         """
         # prevent circular imports:
-        from .signals.handlers import SUBSECTION_RESCORE_EVENT_TYPE
-        from .tasks import recalculate_subsection_grade_v3
+        from .signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE
 
         course_key = _get_key(course_key_or_id, CourseKey)
         usage_key = _get_key(usage_key_or_id, UsageKey)
@@ -89,22 +89,20 @@ class GradesService(object):
             earned_graded_override=earned_graded
         )
 
-        # Recalculation will call PersistentSubsectionGrade.update_or_create_grade which will use the above override
-        # to update the grade before writing to the table.
-        event_transaction_id = create_new_event_transaction_id()
-        set_event_transaction_type(SUBSECTION_RESCORE_EVENT_TYPE)
-        recalculate_subsection_grade_v3.apply_async(
-            kwargs=dict(
-                user_id=user_id,
-                course_id=unicode(course_key),
-                usage_id=unicode(usage_key),
-                only_if_higher=False,
-                expected_modified_time=to_timestamp(override.modified),
-                score_deleted=False,
-                event_transaction_id=unicode(event_transaction_id),
-                event_transaction_type=SUBSECTION_RESCORE_EVENT_TYPE,
-                score_db_table=ScoreDatabaseTableEnum.overrides
-            )
+        # Cache a new event id and event type which the signal handler will use to emit a tracking log event.
+        create_new_event_transaction_id()
+        set_event_transaction_type(SUBSECTION_OVERRIDE_EVENT_TYPE)
+
+        # Signal will trigger subsection recalculation which will call PersistentSubsectionGrade.update_or_create_grade
+        # which will use the above override to update the grade before writing to the table.
+        SUBSECTION_OVERRIDE_CHANGED.send(
+            user_id=user_id,
+            course_id=unicode(course_key),
+            usage_id=unicode(usage_key),
+            only_if_higher=False,
+            modified=override.modified,
+            score_deleted=False,
+            score_db_table=ScoreDatabaseTableEnum.overrides
         )
 
     def undo_override_subsection_grade(self, user_id, course_key_or_id, usage_key_or_id):
@@ -115,8 +113,7 @@ class GradesService(object):
         override does not exist, no error is raised, it just triggers the recalculation.
         """
         # prevent circular imports:
-        from .signals.handlers import SUBSECTION_RESCORE_EVENT_TYPE
-        from .tasks import recalculate_subsection_grade_v3
+        from .signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE
 
         course_key = _get_key(course_key_or_id, CourseKey)
         usage_key = _get_key(usage_key_or_id, UsageKey)
@@ -126,23 +123,24 @@ class GradesService(object):
         if override is not None:
             override.delete()
 
-        event_transaction_id = create_new_event_transaction_id()
-        set_event_transaction_type(SUBSECTION_RESCORE_EVENT_TYPE)
-        recalculate_subsection_grade_v3.apply_async(
-            kwargs=dict(
-                user_id=user_id,
-                course_id=unicode(course_key),
-                usage_id=unicode(usage_key),
-                only_if_higher=False,
-                # Not used when score_deleted=True:
-                expected_modified_time=to_timestamp(datetime.now().replace(tzinfo=pytz.UTC)),
-                score_deleted=True,
-                event_transaction_id=unicode(event_transaction_id),
-                event_transaction_type=SUBSECTION_RESCORE_EVENT_TYPE,
-                score_db_table=ScoreDatabaseTableEnum.overrides
-            )
+        # Cache a new event id and event type which the signal handler will use to emit a tracking log event.
+        create_new_event_transaction_id()
+        set_event_transaction_type(SUBSECTION_OVERRIDE_EVENT_TYPE)
+
+        # Signal will trigger subsection recalculation which will call PersistentSubsectionGrade.update_or_create_grade
+        # which will no longer use the above deleted override, and instead return the grade to the original score from
+        # the actual problem responses before writing to the table.
+        SUBSECTION_OVERRIDE_CHANGED.send(
+            user_id=user_id,
+            course_id=unicode(course_key),
+            usage_id=unicode(usage_key),
+            only_if_higher=False,
+            modified=datetime.now().replace(tzinfo=pytz.UTC),  # Not used when score_deleted=True
+            score_deleted=True,
+            score_db_table=ScoreDatabaseTableEnum.overrides
         )
 
-    def should_override_grade_on_rejected_exam(self, course_key):
+    def should_override_grade_on_rejected_exam(self, course_key_or_id):
         """Convienence function to return the state of the CourseWaffleFlag REJECTED_EXAM_OVERRIDES_GRADE"""
+        course_key = _get_key(course_key_or_id, CourseKey)
         return waffle_flags()[REJECTED_EXAM_OVERRIDES_GRADE].is_enabled(course_key)

lms/djangoapps/grades/signals/handlers.py

@@ -30,7 +30,8 @@ from .signals import (
     PROBLEM_RAW_SCORE_CHANGED,
     PROBLEM_WEIGHTED_SCORE_CHANGED,
     SCORE_PUBLISHED,
-    SUBSECTION_SCORE_CHANGED
+    SUBSECTION_SCORE_CHANGED,
+    SUBSECTION_OVERRIDE_CHANGED
 )
 
 log = getLogger(__name__)
@@ -38,7 +39,7 @@ log = getLogger(__name__)
 # define values to be used in grading events
 GRADES_RESCORE_EVENT_TYPE = 'edx.grades.problem.rescored'
 PROBLEM_SUBMITTED_EVENT_TYPE = 'edx.grades.problem.submitted'
-SUBSECTION_RESCORE_EVENT_TYPE = 'edx.grades.subsection.rescored'
+SUBSECTION_OVERRIDE_EVENT_TYPE = 'edx.grades.subsection.score_overridden'
 
 
 @receiver(score_set)
@@ -210,9 +211,10 @@ def problem_raw_score_changed_handler(sender, **kwargs):  # pylint: disable=unus
 
 
 @receiver(PROBLEM_WEIGHTED_SCORE_CHANGED)
+@receiver(SUBSECTION_OVERRIDE_CHANGED)
 def enqueue_subsection_update(sender, **kwargs):  # pylint: disable=unused-argument
     """
-    Handles the PROBLEM_WEIGHTED_SCORE_CHANGED signal by
+    Handles the PROBLEM_WEIGHTED_SCORE_CHANGED or SUBSECTION_OVERRIDE_CHANGED signals by
     enqueueing a subsection update operation to occur asynchronously.
     """
     _emit_event(kwargs)
@@ -294,9 +296,9 @@ def _emit_event(kwargs):
             }
         )
 
-    if root_type in [SUBSECTION_RESCORE_EVENT_TYPE]:
+    if root_type in [SUBSECTION_OVERRIDE_EVENT_TYPE]:
         tracker.emit(
-            unicode(SUBSECTION_RESCORE_EVENT_TYPE),
+            unicode(SUBSECTION_OVERRIDE_EVENT_TYPE),
             {
                 'course_id': unicode(kwargs['course_id']),
                 'user_id': unicode(kwargs['user_id']),

lms/djangoapps/grades/signals/signals.py

@@ -81,3 +81,23 @@ SUBSECTION_SCORE_CHANGED = Signal(
         'subsection_grade',  # SubsectionGrade object
     ]
 )
+
+# Signal that indicates that a user's score for a subsection has been overridden.
+# This signal is generated when an admin changes a user's exam attempt state to
+# rejected or to verified from rejected. This signal may also be generated by any
+# other client using the GradesService to override subsections in the future.
+SUBSECTION_OVERRIDE_CHANGED = Signal(
+    providing_args=[
+        'user_id',  # Integer User ID
+        'course_id',  # Unicode string representing the course
+        'usage_id',  # Unicode string indicating the courseware instance
+        'only_if_higher',   # Boolean indicating whether updates should be
+                            # made only if the new score is higher than previous.
+        'modified',  # A datetime indicating when the database representation of
+                     # this subsection override score was saved.
+        'score_deleted',  # Boolean indicating whether the override score was
+                          # deleted in this event.
+        'score_db_table',  # The database table that houses the subsection override
+                           # score that was created.
+    ]
+)

lms/djangoapps/grades/tests/test_services.py

@@ -4,11 +4,10 @@ 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 lms.djangoapps.grades.signals.handlers import SUBSECTION_RESCORE_EVENT_TYPE
-from mock import patch
+from lms.djangoapps.grades.signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE
+from mock import patch, call
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from student.tests.factories import UserFactory
-from util.date_utils import to_timestamp
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 
@@ -46,8 +45,8 @@ class GradesServiceTests(ModuleStoreTestCase):
             earned_graded=5.0,
             possible_graded=5.0
         )
-        self.recalc_patcher = patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.apply_async')
-        self.mock_recalculate = self.recalc_patcher.start()
+        self.signal_patcher = patch('lms.djangoapps.grades.signals.signals.SUBSECTION_OVERRIDE_CHANGED.send')
+        self.mock_signal = self.signal_patcher.start()
         self.id_patcher = patch('lms.djangoapps.grades.services.create_new_event_transaction_id')
         self.mock_create_id = self.id_patcher.start()
         self.mock_create_id.return_value = 1
@@ -60,7 +59,7 @@ class GradesServiceTests(ModuleStoreTestCase):
         }
 
     def tearDown(self):
-        self.recalc_patcher.stop()
+        self.signal_patcher.stop()
         self.id_patcher.stop()
         self.type_patcher.stop()
         self.flag_patcher.stop()
@@ -177,17 +176,15 @@ class GradesServiceTests(ModuleStoreTestCase):
         self.assertEqual(override_obj.earned_all_override, override['earned_all'])
         self.assertEqual(override_obj.earned_graded_override, override['earned_graded'])
 
-        self.assertDictEqual(
-            self.mock_recalculate.call_args[1]['kwargs'],
-            dict(
+        self.assertEqual(
+            self.mock_signal.call_args,
+            call(
                 user_id=self.user.id,
                 course_id=unicode(self.course.id),
                 usage_id=unicode(self.subsection.location),
                 only_if_higher=False,
-                expected_modified_time=to_timestamp(override_obj.modified),
+                modified=override_obj.modified,
                 score_deleted=False,
-                event_transaction_id=unicode(self.mock_create_id.return_value),
-                event_transaction_type=SUBSECTION_RESCORE_EVENT_TYPE,
                 score_db_table=ScoreDatabaseTableEnum.overrides
             )
         )
@@ -205,17 +202,15 @@ class GradesServiceTests(ModuleStoreTestCase):
         override = self.service.get_subsection_grade_override(self.user.id, self.course.id, self.subsection.location)
         self.assertIsNone(override)
 
-        self.assertDictEqual(
-            self.mock_recalculate.call_args[1]['kwargs'],
-            dict(
+        self.assertEqual(
+            self.mock_signal.call_args,
+            call(
                 user_id=self.user.id,
                 course_id=unicode(self.course.id),
                 usage_id=unicode(self.subsection.location),
                 only_if_higher=False,
-                expected_modified_time=to_timestamp(datetime.now().replace(tzinfo=pytz.UTC)),
+                modified=datetime.now().replace(tzinfo=pytz.UTC),
                 score_deleted=True,
-                event_transaction_id=unicode(self.mock_create_id.return_value),
-                event_transaction_type=SUBSECTION_RESCORE_EVENT_TYPE,
                 score_db_table=ScoreDatabaseTableEnum.overrides
             )
         )