Update has_database_updated_with_new_score to handle all block types

TNL-6331

lms/djangoapps/courseware/tests/test_module_render.py

@@ -1848,7 +1848,8 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems):
             'course_id': unicode(self.course.id),
             'usage_id': unicode(self.problem.location),
             'only_if_higher': None,
-            'modified': datetime.now().replace(tzinfo=pytz.UTC)
+            'modified': datetime.now().replace(tzinfo=pytz.UTC),
+            'score_db_table': 'csm',
         }
         send_mock.assert_called_with(**expected_signal_kwargs)
 

lms/djangoapps/grades/constants.py

+"""
+Constants and Enums used by Grading.
+"""
+
+
+class ScoreDatabaseTableEnum(object):
+    """
+    The various database tables that store scores.
+    """
+    courseware_student_module = 'csm'
+    submissions = 'submissions'

lms/djangoapps/grades/signals/handlers.py

@@ -24,9 +24,10 @@ from .signals import (
     SUBSECTION_SCORE_CHANGED,
     SCORE_PUBLISHED,
 )
+from ..constants import ScoreDatabaseTableEnum
 from ..new.course_grade import CourseGradeFactory
 from ..scores import weighted_score
-from ..tasks import recalculate_subsection_grade_v2
+from ..tasks import recalculate_subsection_grade_v3
 
 log = getLogger(__name__)
 
@@ -62,9 +63,11 @@ def submissions_score_set_handler(sender, **kwargs):  # pylint: disable=unused-a
         weighted_earned=points_earned,
         weighted_possible=points_possible,
         user_id=user.id,
+        anonymous_user_id=kwargs['anonymous_user_id'],
         course_id=course_id,
         usage_id=usage_id,
         modified=kwargs['created_at'],
+        score_db_table=ScoreDatabaseTableEnum.submissions,
     )
 
 
@@ -93,9 +96,11 @@ def submissions_score_reset_handler(sender, **kwargs):  # pylint: disable=unused
         weighted_earned=0,
         weighted_possible=0,
         user_id=user.id,
+        anonymous_user_id=kwargs['anonymous_user_id'],
         course_id=course_id,
         usage_id=usage_id,
         modified=kwargs['created_at'],
+        score_db_table=ScoreDatabaseTableEnum.submissions,
     )
 
 
@@ -133,6 +138,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_
             usage_id=unicode(block.location),
             only_if_higher=only_if_higher,
             modified=score_modified_time,
+            score_db_table=ScoreDatabaseTableEnum.courseware_student_module,
         )
     return update_score
 
@@ -162,6 +168,7 @@ def problem_raw_score_changed_handler(sender, **kwargs):  # pylint: disable=unus
         only_if_higher=kwargs['only_if_higher'],
         score_deleted=kwargs.get('score_deleted', False),
         modified=kwargs['modified'],
+        score_db_table=kwargs['score_db_table'],
     )
 
 
@@ -172,9 +179,10 @@ def enqueue_subsection_update(sender, **kwargs):  # pylint: disable=unused-argum
     enqueueing a subsection update operation to occur asynchronously.
     """
     _emit_problem_submitted_event(kwargs)
-    result = recalculate_subsection_grade_v2.apply_async(
+    result = recalculate_subsection_grade_v3.apply_async(
         kwargs=dict(
             user_id=kwargs['user_id'],
+            anonymous_user_id=kwargs.get('anonymous_user_id'),
             course_id=kwargs['course_id'],
             usage_id=kwargs['usage_id'],
             only_if_higher=kwargs.get('only_if_higher'),
@@ -182,6 +190,7 @@ def enqueue_subsection_update(sender, **kwargs):  # pylint: disable=unused-argum
             score_deleted=kwargs.get('score_deleted', False),
             event_transaction_id=unicode(get_event_transaction_id()),
             event_transaction_type=unicode(get_event_transaction_type()),
+            score_db_table=kwargs['score_db_table'],
         )
     )
     log.info(

lms/djangoapps/grades/signals/signals.py

@@ -22,6 +22,7 @@ PROBLEM_RAW_SCORE_CHANGED = Signal(
                             # made only if the new score is higher than previous.
         'modified',  # A datetime indicating when the database representation of
                      # this the problem score was saved.
+        'score_db_table',  # The database table that houses the score that changed.
     ]
 )
 
@@ -35,6 +36,7 @@ PROBLEM_RAW_SCORE_CHANGED = Signal(
 PROBLEM_WEIGHTED_SCORE_CHANGED = Signal(
     providing_args=[
         'user_id',  # Integer User ID
+        'anonymous_user_id',  # Anonymous User ID
         'course_id',  # Unicode string representing the course
         'usage_id',  # Unicode string indicating the courseware instance
         'weighted_earned',   # Score obtained by the user
@@ -43,6 +45,7 @@ PROBLEM_WEIGHTED_SCORE_CHANGED = Signal(
                             # made only if the new score is higher than previous.
         'modified',  # A datetime indicating when the database representation of
                      # this the problem score was saved.
+        'score_db_table',  # The database table that houses the score that changed.
     ]
 )
 
@@ -59,6 +62,7 @@ SCORE_PUBLISHED = Signal(
         'raw_possible',  # Maximum score available for the exercise
         'only_if_higher',   # Boolean indicating whether updates should be
                             # made only if the new score is higher than previous.
+        'score_db_table',  # The database table that houses the score that changed.
     ]
 )
 

lms/djangoapps/grades/tasks.py

@@ -26,6 +26,7 @@ from util.date_utils import from_timestamp
 from xmodule.modulestore.django import modulestore
 
 from .config.models import PersistentGradesEnabledFlag
+from .constants import ScoreDatabaseTableEnum
 from .new.subsection_grade import SubsectionGradeFactory
 from .signals.signals import SUBSECTION_SCORE_CHANGED
 from .transformer import GradesTransformer
@@ -35,33 +36,31 @@ log = getLogger(__name__)
 KNOWN_RETRY_ERRORS = (DatabaseError, ValidationError)  # Errors we expect occasionally, should be resolved on retry
 
 
-@task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY)
-def recalculate_subsection_grade(
-        # pylint: disable=unused-argument
-        user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, **kwargs
-):
+# TODO (TNL-6373) DELETE ME once v3 is successfully deployed to Prod.
+@task(base=PersistOnFailureTask, default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY)
+def recalculate_subsection_grade_v2(**kwargs):
     """
-    Shim to allow us to modify this task's signature without blowing up production on deployment.
+    Shim to support tasks enqueued by older workers during initial deployment.
     """
-    recalculate_subsection_grade_v2.apply(
-        kwargs=dict(
-            user_id=user_id,
-            course_id=course_id,
-            usage_id=usage_id,
-            only_if_higher=only_if_higher,
-            expected_modified_time=kwargs.get('expected_modified_time', 0),  # Use the unix epoch as a default
-            score_deleted=kwargs['score_deleted'],
-        )
-    )
+    _recalculate_subsection_grade(recalculate_subsection_grade_v2, **kwargs)
 
 
 @task(base=PersistOnFailureTask, default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY)
-def recalculate_subsection_grade_v2(**kwargs):
+def recalculate_subsection_grade_v3(**kwargs):
+    """
+    Latest version of the recalculate_subsection_grade task.  See docstring
+    for _recalculate_subsection_grade for further description.
+    """
+    _recalculate_subsection_grade(recalculate_subsection_grade_v3, **kwargs)
+
+
+def _recalculate_subsection_grade(task_func, **kwargs):
     """
     Updates a saved subsection grade.
 
-    Arguments:
+    Keyword Arguments:
         user_id (int): id of applicable User object
+        anonymous_user_id (int, OPTIONAL): Anonymous ID of the User
         course_id (string): identifying the course
         usage_id (string): identifying the course block
         only_if_higher (boolean): indicating whether grades should
@@ -71,36 +70,44 @@ def recalculate_subsection_grade_v2(**kwargs):
             was queued so that we can verify the underlying data update.
         score_deleted (boolean): indicating whether the grade change is
             a result of the problem's score being deleted.
-        event_transaction_id(string): uuid identifying the current
+        event_transaction_id (string): uuid identifying the current
             event transaction.
-        event_transaction_type(string): human-readable type of the
+        event_transaction_type (string): human-readable type of the
             event at the root of the current event transaction.
+        score_db_table (ScoreDatabaseTableEnum): database table that houses
+            the changed score. Used in conjunction with expected_modified_time.
     """
     try:
         course_key = CourseLocator.from_string(kwargs['course_id'])
         if not PersistentGradesEnabledFlag.feature_enabled(course_key):
             return
 
-        score_deleted = kwargs['score_deleted']
         scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key)
-        expected_modified_time = from_timestamp(kwargs['expected_modified_time'])
 
         # The request cache is not maintained on celery workers,
         # where this code runs. So we take the values from the
         # main request cache and store them in the local request
         # cache. This correlates model-level grading events with
         # higher-level ones.
-        set_event_transaction_id(kwargs.pop('event_transaction_id', None))
-        set_event_transaction_type(kwargs.pop('event_transaction_type', None))
+        set_event_transaction_id(kwargs.get('event_transaction_id'))
+        set_event_transaction_type(kwargs.get('event_transaction_type'))
 
         # Verify the database has been updated with the scores when the task was
         # created. This race condition occurs if the transaction in the task
         # creator's process hasn't committed before the task initiates in the worker
         # process.
-        if not _has_database_updated_with_new_score(
-                kwargs['user_id'], scored_block_usage_key, expected_modified_time, score_deleted,
-        ):
-            raise _retry_recalculate_subsection_grade(**kwargs)
+        if task_func == recalculate_subsection_grade_v2:
+            has_database_updated = _has_db_updated_with_new_score_bwc_v2(
+                kwargs['user_id'],
+                scored_block_usage_key,
+                from_timestamp(kwargs['expected_modified_time']),
+                kwargs['score_deleted'],
+            )
+        else:
+            has_database_updated = _has_db_updated_with_new_score(scored_block_usage_key, **kwargs)
+
+        if not has_database_updated:
+            raise _retry_recalculate_subsection_grade(task_func, **kwargs)
 
         _update_subsection_grades(
             course_key,
@@ -115,13 +122,45 @@ def recalculate_subsection_grade_v2(**kwargs):
                 repr(exc),
                 kwargs
             ))
-        raise _retry_recalculate_subsection_grade(exc=exc, **kwargs)
+        raise _retry_recalculate_subsection_grade(task_func, exc=exc, **kwargs)
 
 
-def _has_database_updated_with_new_score(
+def _has_db_updated_with_new_score(scored_block_usage_key, **kwargs):
+    """
+    Returns whether the database has been updated with the
+    expected new score values for the given problem and user.
+    """
+    if kwargs['score_db_table'] == ScoreDatabaseTableEnum.courseware_student_module:
+        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
+        score = sub_api.get_score(
+            {
+                "student_id": kwargs['anonymous_user_id'],
+                "course_id": unicode(scored_block_usage_key.course_key),
+                "item_id": unicode(scored_block_usage_key),
+                "item_type": scored_block_usage_key.block_type,
+            }
+        )
+        found_modified_time = score['created_at'] if score is not None else None
+
+    if score is None:
+        # score should be None only if it was deleted.
+        # Otherwise, it hasn't yet been saved.
+        return kwargs['score_deleted']
+
+    return found_modified_time >= from_timestamp(kwargs['expected_modified_time'])
+
+
+# TODO (TNL-6373) DELETE ME once v3 is successfully deployed to Prod.
+def _has_db_updated_with_new_score_bwc_v2(
         user_id, scored_block_usage_key, expected_modified_time, score_deleted,
 ):
     """
+    DEPRECATED version for backward compatibility with v2 tasks.
+
     Returns whether the database has been updated with the
     expected new score values for the given problem and user.
     """
@@ -186,7 +225,7 @@ def _update_subsection_grades(
                     only_if_higher,
                 )
                 SUBSECTION_SCORE_CHANGED.send(
-                    sender=recalculate_subsection_grade,
+                    sender=None,
                     course=course,
                     course_structure=course_structure,
                     user=student,
@@ -194,29 +233,9 @@ def _update_subsection_grades(
                 )
 
 
-def _retry_recalculate_subsection_grade(
-        user_id,
-        course_id,
-        usage_id,
-        only_if_higher,
-        expected_modified_time,
-        score_deleted,
-        exc=None,
-):
+def _retry_recalculate_subsection_grade(task_func, exc=None, **kwargs):
     """
     Calls retry for the recalculate_subsection_grade task with the
     given inputs.
     """
-    recalculate_subsection_grade_v2.retry(
-        kwargs=dict(
-            user_id=user_id,
-            course_id=course_id,
-            usage_id=usage_id,
-            only_if_higher=only_if_higher,
-            expected_modified_time=expected_modified_time,
-            score_deleted=score_deleted,
-            event_transaction_id=unicode(get_event_transaction_id()),
-            event_transaction_type=unicode(get_event_transaction_type()),
-        ),
-        exc=exc,
-    )
+    task_func.retry(kwargs=kwargs, exc=exc)

lms/djangoapps/grades/tests/test_signals.py

@@ -11,6 +11,7 @@ from mock import patch, MagicMock
 import pytz
 from util.date_utils import to_timestamp
 
+from ..constants import ScoreDatabaseTableEnum
 from ..signals.handlers import (
     enqueue_subsection_update,
     submissions_score_set_handler,
@@ -32,7 +33,6 @@ SUBMISSION_SET_KWARGS = {
     'created_at': FROZEN_NOW_TIMESTAMP,
 }
 
-
 SUBMISSION_RESET_KWARGS = {
     'anonymous_user_id': 'anonymous_id',
     'course_id': 'CourseID',
@@ -50,6 +50,20 @@ PROBLEM_RAW_SCORE_CHANGED_KWARGS = {
     'only_if_higher': False,
     'score_deleted': True,
     'modified': FROZEN_NOW_TIMESTAMP,
+    'score_db_table': ScoreDatabaseTableEnum.courseware_student_module,
+}
+
+PROBLEM_WEIGHTED_SCORE_CHANGED_KWARGS = {
+    'sender': None,
+    'weighted_earned': 2.0,
+    'weighted_possible': 4.0,
+    'user_id': 'UserID',
+    'course_id': 'CourseID',
+    'usage_id': 'i4x://org/course/usage/123456',
+    'only_if_higher': False,
+    'score_deleted': True,
+    'modified': FROZEN_NOW_TIMESTAMP,
+    'score_db_table': ScoreDatabaseTableEnum.courseware_student_module,
 }
 
 
@@ -110,9 +124,11 @@ class ScoreChangedSignalRelayTest(TestCase):
             'weighted_possible': possible,
             'weighted_earned': earned,
             'user_id': self.user_mock.id,
+            'anonymous_user_id': 'anonymous_id',
             'course_id': 'CourseID',
             'usage_id': 'i4x://org/course/usage/123456',
             'modified': FROZEN_NOW_TIMESTAMP,
+            'score_db_table': 'submissions',
         }
         self.signal_mock.assert_called_once_with(**expected_set_kwargs)
         self.get_user_mock.assert_called_once_with(kwargs['anonymous_user_id'])
@@ -153,44 +169,26 @@ class ScoreChangedSignalRelayTest(TestCase):
 
     def test_raw_score_changed_signal_handler(self):
         problem_raw_score_changed_handler(None, **PROBLEM_RAW_SCORE_CHANGED_KWARGS)
-        expected_set_kwargs = {
-            'sender': None,
-            'weighted_earned': 2.0,
-            'weighted_possible': 4.0,
-            'user_id': 'UserID',
-            'course_id': 'CourseID',
-            'usage_id': 'i4x://org/course/usage/123456',
-            'only_if_higher': False,
-            'score_deleted': True,
-            'modified': FROZEN_NOW_TIMESTAMP
-        }
+        expected_set_kwargs = PROBLEM_WEIGHTED_SCORE_CHANGED_KWARGS.copy()
         self.signal_mock.assert_called_with(**expected_set_kwargs)
 
     def test_raw_score_changed_score_deleted_optional(self):
         local_kwargs = PROBLEM_RAW_SCORE_CHANGED_KWARGS.copy()
         del local_kwargs['score_deleted']
         problem_raw_score_changed_handler(None, **local_kwargs)
-        expected_set_kwargs = {
-            'sender': None,
-            'weighted_earned': 2.0,
-            'weighted_possible': 4.0,
-            'user_id': 'UserID',
-            'course_id': 'CourseID',
-            'usage_id': 'i4x://org/course/usage/123456',
-            'only_if_higher': False,
-            'score_deleted': False,
-            'modified': FROZEN_NOW_TIMESTAMP
-        }
+        expected_set_kwargs = PROBLEM_WEIGHTED_SCORE_CHANGED_KWARGS.copy()
+        expected_set_kwargs['score_deleted'] = False
         self.signal_mock.assert_called_with(**expected_set_kwargs)
 
     @patch('lms.djangoapps.grades.signals.handlers.log.info')
-    def test_problem_score_changed_logging(self, mocklog):
+    def test_subsection_update_logging(self, mocklog):
         enqueue_subsection_update(
             sender='test',
             user_id=1,
             course_id=u'course-v1:edX+Demo_Course+DemoX',
             usage_id=u'block-v1:block-key',
             modified=FROZEN_NOW_DATETIME,
+            score_db_table=ScoreDatabaseTableEnum.courseware_student_module,
         )
         log_statement = mocklog.call_args[0][0]
         log_statement = UUID_REGEX.sub(u'*UUID*', log_statement)
@@ -199,6 +197,7 @@ class ScoreChangedSignalRelayTest(TestCase):
             (
                 u'Grades: Request async calculation of subsection grades with args: '
                 u'course_id:course-v1:edX+Demo_Course+DemoX, modified:{time}, '
+                u'score_db_table:csm, '
                 u'usage_id:block-v1:block-key, user_id:1. Task [*UUID*]'
             ).format(time=FROZEN_NOW_DATETIME)
         )

lms/djangoapps/grades/tests/test_tasks.py

@@ -8,10 +8,10 @@ from datetime import datetime, timedelta
 import ddt
 from django.conf import settings
 from django.db.utils import IntegrityError
+import itertools
 from mock import patch, MagicMock
 import pytz
 from util.date_utils import to_timestamp
-from unittest import skip
 
 from student.models import anonymous_id_for_user
 from student.tests.factories import UserFactory
@@ -25,8 +25,9 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
 
 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_v2
+from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3
 
 
 @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False})
@@ -40,7 +41,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
         self.user = UserFactory()
         PersistentGradesEnabledFlag.objects.create(enabled_for_all_courses=True, enabled=True)
 
-    def set_up_course(self, enable_subsection_grades=True):
+    def set_up_course(self, enable_persistent_grades=True):
         """
         Configures the course for this test.
         """
@@ -50,7 +51,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
             name='course',
             run='run',
         )
-        if not enable_subsection_grades:
+        if not enable_persistent_grades:
             PersistentGradesEnabledFlag.objects.create(enabled=False)
 
         self.chapter = ItemFactory.create(parent=self.course, category="chapter", display_name="Chapter")
@@ -64,10 +65,12 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
             ('weighted_earned', 1.0),
             ('weighted_possible', 2.0),
             ('user_id', self.user.id),
+            ('anonymous_user_id', 5),
             ('course_id', unicode(self.course.id)),
             ('usage_id', unicode(self.problem.location)),
             ('only_if_higher', None),
             ('modified', self.frozen_now_datetime),
+            ('score_db_table', ScoreDatabaseTableEnum.courseware_student_module),
         ])
 
         create_new_event_transaction_id()
@@ -76,11 +79,13 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
             ('user_id', self.user.id),
             ('course_id', unicode(self.course.id)),
             ('usage_id', unicode(self.problem.location)),
+            ('anonymous_user_id', 5),
             ('only_if_higher', None),
             ('expected_modified_time', self.frozen_now_timestamp),
             ('score_deleted', False),
             ('event_transaction_id', unicode(get_event_transaction_id())),
             ('event_transaction_type', u'edx.grades.problem.submitted'),
+            ('score_db_table', ScoreDatabaseTableEnum.courseware_student_module),
         ])
 
         # this call caches the anonymous id on the user object, saving 4 queries in all happy path tests
@@ -96,7 +101,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
         with patch("lms.djangoapps.grades.tasks.get_score", return_value=score):
             yield
 
-    def test_problem_weighted_score_changed_queues_task(self):
+    def test_triggered_by_problem_weighted_score_change(self):
         """
         Ensures that the PROBLEM_WEIGHTED_SCORE_CHANGED signal enqueues the correct task.
         """
@@ -105,27 +110,37 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
         local_task_args = self.recalculate_subsection_grade_kwargs.copy()
         local_task_args['event_transaction_type'] = u'edx.grades.problem.submitted'
         with self.mock_get_score() and patch(
-            'lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.apply_async',
+            'lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.apply_async',
             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)
 
     @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send')
-    def test_subsection_update_triggers_signal(self, mock_subsection_signal):
+    def test_triggers_subsection_score_signal(self, mock_subsection_signal):
         """
-        Ensures that the subsection update operation triggers a signal.
+        Ensures that a subsection grade recalculation triggers a signal.
         """
         self.set_up_course()
         self._apply_recalculate_subsection_grade()
         self.assertTrue(mock_subsection_signal.called)
 
+    def test_block_structure_created_only_once(self):
+        self.set_up_course()
+        self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
+        with patch(
+            'openedx.core.lib.block_structure.factory.BlockStructureFactory.create_from_cache',
+            return_value=None,
+        ) as mock_block_structure_create:
+            self._apply_recalculate_subsection_grade()
+            self.assertEquals(mock_block_structure_create.call_count, 1)
+
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 1, 23),
         (ModuleStoreEnum.Type.split, 3, 22),
     )
     @ddt.unpack
-    def test_subsection_grade_updated(self, default_store, num_mongo_calls, num_sql_calls):
+    def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls):
         with self.store.default_store(default_store):
             self.set_up_course()
             self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
@@ -133,6 +148,30 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
                 with self.assertNumQueries(num_sql_calls):
                     self._apply_recalculate_subsection_grade()
 
+    # TODO (TNL-6225) Fix the number of SQL queries so they
+    # don't grow linearly with the number of sequentials.
+    @ddt.data(
+        (ModuleStoreEnum.Type.mongo, 1, 46),
+        (ModuleStoreEnum.Type.split, 3, 45),
+    )
+    @ddt.unpack
+    def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
+        with self.store.default_store(default_store):
+            self.set_up_course()
+            self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
+
+            num_problems = 10
+            for _ in range(num_problems):
+                ItemFactory.create(parent=self.sequential, category='problem')
+
+            num_sequentials = 10
+            for _ in range(num_sequentials):
+                ItemFactory.create(parent=self.chapter, category='sequential')
+
+            with check_mongo_calls(num_mongo_calls):
+                with self.assertNumQueries(num_sql_calls):
+                    self._apply_recalculate_subsection_grade()
+
     @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send')
     def test_other_inaccessible_subsection(self, mock_subsection_signal):
         self.set_up_course()
@@ -157,68 +196,29 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
             {self.sequential.location, accessible_seq.location},
         )
 
-    def test_single_call_to_create_block_structure(self):
-        self.set_up_course()
-        self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
-        with patch(
-            'openedx.core.lib.block_structure.factory.BlockStructureFactory.create_from_cache',
-            return_value=None,
-        ) as mock_block_structure_create:
-            self._apply_recalculate_subsection_grade()
-            self.assertEquals(mock_block_structure_create.call_count, 1)
-
-    # TODO (TNL-6225) Fix the number of SQL queries so they
-    # don't grow linearly with the number of sequentials.
-    @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 46),
-        (ModuleStoreEnum.Type.split, 3, 45),
-    )
-    @ddt.unpack
-    def test_query_count_does_not_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
-        with self.store.default_store(default_store):
-            self.set_up_course()
-            self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
-
-            num_problems = 10
-            for _ in range(num_problems):
-                ItemFactory.create(parent=self.sequential, category='problem')
-
-            num_sequentials = 10
-            for _ in range(num_sequentials):
-                ItemFactory.create(parent=self.chapter, category='sequential')
-
-            with check_mongo_calls(num_mongo_calls):
-                with self.assertNumQueries(num_sql_calls):
-                    self._apply_recalculate_subsection_grade()
-
     @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
-    def test_subsection_grades_not_enabled_on_course(self, default_store):
+    def test_persistent_grades_not_enabled_on_course(self, default_store):
         with self.store.default_store(default_store):
-            self.set_up_course(enable_subsection_grades=False)
+            self.set_up_course(enable_persistent_grades=False)
             self.assertFalse(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
             with check_mongo_calls(0):
                 with self.assertNumQueries(0):
                     self._apply_recalculate_subsection_grade()
 
-    @skip("Pending completion of TNL-5089")
-    @ddt.data(
-        (ModuleStoreEnum.Type.mongo, True),
-        (ModuleStoreEnum.Type.split, True),
-        (ModuleStoreEnum.Type.mongo, False),
-        (ModuleStoreEnum.Type.split, False),
-    )
-    @ddt.unpack
-    def test_query_counts_with_feature_flag(self, default_store, feature_flag):
-        PersistentGradesEnabledFlag.objects.create(enabled=feature_flag)
-        with self.store.default_store(default_store):
-            self.set_up_course()
-            with check_mongo_calls(0):
-                with self.assertNumQueries(3 if feature_flag else 2):
-                    self._apply_recalculate_subsection_grade()
+    @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send')
+    @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update')
+    def test_retry_first_time_only(self, mock_update, mock_course_signal):
+        """
+        Ensures that a task retry completes after a one-time failure.
+        """
+        self.set_up_course()
+        mock_update.side_effect = [IntegrityError("WHAMMY"), None]
+        self._apply_recalculate_subsection_grade()
+        self.assertEquals(mock_course_signal.call_count, 1)
 
-    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
+    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry')
     @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update')
-    def test_retry_subsection_update_on_integrity_error(self, mock_update, mock_retry):
+    def test_retry_on_integrity_error(self, mock_update, mock_retry):
         """
         Ensures that tasks will be retried if IntegrityErrors are encountered.
         """
@@ -227,55 +227,57 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
         self._apply_recalculate_subsection_grade()
         self._assert_retry_called(mock_retry)
 
-    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
-    def test_retry_subsection_grade_on_update_not_complete(self, mock_retry):
+    @ddt.data(ScoreDatabaseTableEnum.courseware_student_module, ScoreDatabaseTableEnum.submissions)
+    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry')
+    def test_retry_when_db_not_updated(self, score_db_table, mock_retry):
         self.set_up_course()
-        self._apply_recalculate_subsection_grade(
-            mock_score=MagicMock(modified=datetime.utcnow().replace(tzinfo=pytz.UTC) - timedelta(days=1))
-        )
-        self._assert_retry_called(mock_retry)
-
-    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
-    def test_retry_subsection_grade_on_update_not_complete_sub(self, mock_retry):
-        self.set_up_course()
-        with patch('lms.djangoapps.grades.tasks.sub_api.get_score') as mock_sub_score:
-            mock_sub_score.return_value = {
-                'created_at': datetime.utcnow().replace(tzinfo=pytz.UTC) - timedelta(days=1)
-            }
+        self.recalculate_subsection_grade_kwargs['score_db_table'] = score_db_table
+
+        modified_datetime = datetime.utcnow().replace(tzinfo=pytz.UTC) - timedelta(days=1)
+        if score_db_table == ScoreDatabaseTableEnum.submissions:
+            with patch('lms.djangoapps.grades.tasks.sub_api.get_score') as mock_sub_score:
+                mock_sub_score.return_value = {
+                    'created_at': modified_datetime
+                }
+                self._apply_recalculate_subsection_grade(
+                    mock_score=MagicMock(module_type='any_block_type')
+                )
+        else:
             self._apply_recalculate_subsection_grade(
-                mock_score=MagicMock(module_type='openassessment')
+                mock_score=MagicMock(modified=modified_datetime)
             )
-        self._assert_retry_called(mock_retry)
-
-    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
-    def test_retry_subsection_grade_on_no_score(self, mock_retry):
-        self.set_up_course()
-        self._apply_recalculate_subsection_grade(mock_score=None)
-        self._assert_retry_called(mock_retry)
 
-    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
-    def test_retry_subsection_grade_on_no_sub_score(self, mock_retry):
-        self.set_up_course()
-        with patch('lms.djangoapps.grades.tasks.sub_api.get_score') as mock_sub_score:
-            mock_sub_score.return_value = None
-            self._apply_recalculate_subsection_grade(
-                mock_score=MagicMock(module_type='openassessment')
-            )
         self._assert_retry_called(mock_retry)
 
-    @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send')
-    @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update')
-    def test_retry_first_time_only(self, mock_update, mock_course_signal):
-        """
-        Ensures that a task retry completes after a one-time failure.
-        """
+    @ddt.data(
+        *itertools.product(
+            (True, False),
+            (ScoreDatabaseTableEnum.courseware_student_module, ScoreDatabaseTableEnum.submissions),
+        )
+    )
+    @ddt.unpack
+    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry')
+    def test_when_no_score_found(self, score_deleted, score_db_table, mock_retry):
         self.set_up_course()
-        mock_update.side_effect = [IntegrityError("WHAMMY"), None]
-        self._apply_recalculate_subsection_grade()
-        self.assertEquals(mock_course_signal.call_count, 1)
+        self.recalculate_subsection_grade_kwargs['score_deleted'] = score_deleted
+        self.recalculate_subsection_grade_kwargs['score_db_table'] = score_db_table
+
+        if score_db_table == ScoreDatabaseTableEnum.submissions:
+            with patch('lms.djangoapps.grades.tasks.sub_api.get_score') as mock_sub_score:
+                mock_sub_score.return_value = None
+                self._apply_recalculate_subsection_grade(
+                    mock_score=MagicMock(module_type='any_block_type')
+                )
+        else:
+            self._apply_recalculate_subsection_grade(mock_score=None)
+
+        if score_deleted:
+            self._assert_retry_not_called(mock_retry)
+        else:
+            self._assert_retry_called(mock_retry)
 
     @patch('lms.djangoapps.grades.tasks.log')
-    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
+    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry')
     @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update')
     def test_log_unknown_error(self, mock_update, mock_retry, mock_log):
         """
@@ -288,7 +290,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
         self._assert_retry_called(mock_retry)
 
     @patch('lms.djangoapps.grades.tasks.log')
-    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
+    @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry')
     @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update')
     def test_no_log_known_error(self, mock_update, mock_retry, mock_log):
         """
@@ -309,7 +311,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
         mocking in place.
         """
         with self.mock_get_score(mock_score):
-            recalculate_subsection_grade_v2.apply(kwargs=self.recalculate_subsection_grade_kwargs)
+            recalculate_subsection_grade_v3.apply(kwargs=self.recalculate_subsection_grade_kwargs)
 
     def _assert_retry_called(self, mock_retry):
         """
@@ -318,3 +320,9 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
         """
         self.assertTrue(mock_retry.called)
         self.assertEquals(len(mock_retry.call_args[1]['kwargs']), len(self.recalculate_subsection_grade_kwargs))
+
+    def _assert_retry_not_called(self, mock_retry):
+        """
+        Verifies the task was not retried.
+        """
+        self.assertFalse(mock_retry.called)

lms/djangoapps/instructor/enrollment.py

@@ -16,10 +16,8 @@ from django.utils.translation import override as override_language
 from eventtracking import tracker
 import pytz
 
-from course_modes.models import CourseMode
-from courseware.models import StudentModule
-from edxmako.shortcuts import render_to_string
 from lms.djangoapps.grades.signals.signals import PROBLEM_RAW_SCORE_CHANGED
+from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum
 from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.user_api.models import UserPreference
@@ -329,19 +327,22 @@ def _fire_score_changed_for_block(
     The earned points are always zero. We must retrieve the possible points
     from the XModule, as noted below. The effective time is now().
     """
-    if block and block.has_score and block.max_score() is not None:
-        PROBLEM_RAW_SCORE_CHANGED.send(
-            sender=None,
-            raw_earned=0,
-            raw_possible=block.max_score(),
-            weight=getattr(block, 'weight', None),
-            user_id=student.id,
-            course_id=unicode(course_id),
-            usage_id=unicode(module_state_key),
-            score_deleted=True,
-            only_if_higher=False,
-            modified=datetime.now().replace(tzinfo=pytz.UTC),
-        )
+    if block and block.has_score:
+        max_score = block.max_score()
+        if max_score is not None:
+            PROBLEM_RAW_SCORE_CHANGED.send(
+                sender=None,
+                raw_earned=0,
+                raw_possible=max_score,
+                weight=getattr(block, 'weight', None),
+                user_id=student.id,
+                course_id=unicode(course_id),
+                usage_id=unicode(module_state_key),
+                score_deleted=True,
+                only_if_higher=False,
+                modified=datetime.now().replace(tzinfo=pytz.UTC),
+                score_db_table=ScoreDatabaseTableEnum.courseware_student_module,
+            )
 
 
 def get_email_params(course, auto_enroll, secure=True, course_key=None, display_name=None):