Merge pull request #13734 from edx/efischer/async_grade

Update persistent grades asynchronously

lms/djangoapps/courseware/module_render.py

@@ -540,7 +540,7 @@ def get_module_system_for_user(user, student_data,  # TODO  # pylint: disable=to
             sender=None,
             points_possible=event['max_value'],
             points_earned=event['value'],
-            user=user,
+            user_id=user.id,
             course_id=unicode(course_id),
             usage_id=unicode(descriptor.location)
         )

lms/djangoapps/courseware/tests/test_module_render.py

@@ -1847,7 +1847,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems):
             'sender': None,
             'points_possible': self.grade_dict['max_value'],
             'points_earned': self.grade_dict['value'],
-            'user': self.student_user,
+            'user_id': self.student_user.id,
             'course_id': unicode(self.course.id),
             'usage_id': unicode(self.problem.location)
         }

lms/djangoapps/gating/signals.py

@@ -26,5 +26,5 @@ def handle_score_changed(**kwargs):
         gating_api.evaluate_prerequisite(
             course,
             UsageKey.from_string(kwargs.get('usage_id')),
-            kwargs.get('user').id,
+            kwargs.get('user_id'),
         )

lms/djangoapps/gating/tests/test_signals.py

@@ -19,7 +19,7 @@ class TestHandleScoreChanged(ModuleStoreTestCase):
     def setUp(self):
         super(TestHandleScoreChanged, self).setUp()
         self.course = CourseFactory.create(org='TestX', number='TS01', run='2016_Q1')
-        self.user = UserFactory()
+        self.user = UserFactory.create()
         self.test_usage_key = UsageKey.from_string('i4x://the/content/key/12345678')
 
     @patch('gating.signals.gating_api.evaluate_prerequisite')
@@ -31,7 +31,7 @@ class TestHandleScoreChanged(ModuleStoreTestCase):
             sender=None,
             points_possible=1,
             points_earned=1,
-            user=self.user,
+            user_id=self.user.id,
             course_id=unicode(self.course.id),
             usage_id=unicode(self.test_usage_key)
         )
@@ -44,7 +44,7 @@ class TestHandleScoreChanged(ModuleStoreTestCase):
             sender=None,
             points_possible=1,
             points_earned=1,
-            user=self.user,
+            user_id=self.user.id,
             course_id=unicode(self.course.id),
             usage_id=unicode(self.test_usage_key)
         )

lms/djangoapps/grades/signals/handlers.py

 """
 Grades related signals.
 """
-from logging import getLogger
 
 from django.dispatch import receiver
+from logging import getLogger
 
-from lms.djangoapps.course_blocks.api import get_course_blocks
-from lms.djangoapps.courseware.courses import get_course_by_id
-from opaque_keys.edx.locator import CourseLocator
-from opaque_keys.edx.keys import UsageKey
-from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
 from student.models import user_by_anonymous_id
 from submissions.models import score_set, score_reset
 
 from .signals import SCORE_CHANGED
-from ..config.models import PersistentGradesEnabledFlag
-from ..transformer import GradesTransformer
-from ..new.subsection_grade import SubsectionGradeFactory
+from ..tasks import recalculate_subsection_grade
 
 log = getLogger(__name__)
 
@@ -42,12 +35,14 @@ def submissions_score_set_handler(sender, **kwargs):  # pylint: disable=unused-a
     course_id = kwargs['course_id']
     usage_id = kwargs['item_id']
     user = user_by_anonymous_id(kwargs['anonymous_user_id'])
+    if user is None:
+        return
 
     SCORE_CHANGED.send(
         sender=None,
         points_possible=points_possible,
         points_earned=points_earned,
-        user=user,
+        user_id=user.id,
         course_id=course_id,
         usage_id=usage_id
     )
@@ -70,51 +65,22 @@ def submissions_score_reset_handler(sender, **kwargs):  # pylint: disable=unused
     course_id = kwargs['course_id']
     usage_id = kwargs['item_id']
     user = user_by_anonymous_id(kwargs['anonymous_user_id'])
+    if user is None:
+        return
 
     SCORE_CHANGED.send(
         sender=None,
         points_possible=0,
         points_earned=0,
-        user=user,
+        user_id=user.id,
         course_id=course_id,
         usage_id=usage_id
     )
 
 
 @receiver(SCORE_CHANGED)
-def recalculate_subsection_grade_handler(sender, **kwargs):  # pylint: disable=unused-argument
+def enqueue_update(sender, **kwargs):  # pylint: disable=unused-argument
     """
-    Consume the SCORE_CHANGED signal and trigger an update.
-    This method expects that the kwargs dictionary will contain the following
-    entries (See the definition of SCORE_CHANGED):
-       - points_possible: Maximum score available for the exercise
-       - points_earned: Score obtained by the user
-       - user: User object
-       - course_id: Unicode string representing the course
-       - usage_id: Unicode string indicating the courseware instance
+    Handles the SCORE_CHANGED signal by enqueueing an update operation to occur asynchronously.
     """
-    student = kwargs['user']
-    course_key = CourseLocator.from_string(kwargs['course_id'])
-    if not PersistentGradesEnabledFlag.feature_enabled(course_key):
-        return
-
-    scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key)
-    collected_block_structure = get_course_in_cache(course_key)
-    course = get_course_by_id(course_key, depth=0)
-
-    subsections_to_update = collected_block_structure.get_transformer_block_field(
-        scored_block_usage_key,
-        GradesTransformer,
-        'subsections',
-        set()
-    )
-    subsection_grade_factory = SubsectionGradeFactory(student, course, collected_block_structure)
-    for subsection_usage_key in subsections_to_update:
-        transformed_subsection_structure = get_course_blocks(
-            student,
-            subsection_usage_key,
-            collected_block_structure=collected_block_structure,
-        )
-        subsection_grade_factory.update(
-            transformed_subsection_structure[subsection_usage_key], transformed_subsection_structure
-        )
+    recalculate_subsection_grade.apply_async(args=(kwargs['user_id'], kwargs['course_id'], kwargs['usage_id']))

lms/djangoapps/grades/tasks.py

+"""
+This module contains tasks for asynchronous execution of grade updates.
+"""
+
+from celery import task
+from django.contrib.auth.models import User
+
+from lms.djangoapps.course_blocks.api import get_course_blocks
+from lms.djangoapps.courseware.courses import get_course_by_id
+from opaque_keys.edx.keys import UsageKey
+from opaque_keys.edx.locator import CourseLocator
+from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
+
+from .config.models import PersistentGradesEnabledFlag
+from .transformer import GradesTransformer
+from .new.subsection_grade import SubsectionGradeFactory
+
+
+@task()
+def recalculate_subsection_grade(user_id, course_id, usage_id):
+    """
+    Updates a saved subsection grade.
+    This method expects the following parameters:
+       - user_id: serialized id of applicable User object
+       - course_id: Unicode string representing the course
+       - usage_id: Unicode string indicating the courseware instance
+    """
+    course_key = CourseLocator.from_string(course_id)
+    if not PersistentGradesEnabledFlag.feature_enabled(course_key):
+        return
+
+    student = User.objects.get(id=user_id)
+    scored_block_usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key)
+
+    collected_block_structure = get_course_in_cache(course_key)
+    course = get_course_by_id(course_key, depth=0)
+    subsection_grade_factory = SubsectionGradeFactory(student, course, collected_block_structure)
+    subsections_to_update = collected_block_structure.get_transformer_block_field(
+        scored_block_usage_key,
+        GradesTransformer,
+        'subsections',
+        set()
+    )
+
+    for subsection_usage_key in subsections_to_update:
+        transformed_subsection_structure = get_course_blocks(
+            student,
+            subsection_usage_key,
+            collected_block_structure=collected_block_structure,
+        )
+        subsection_grade_factory.update(
+            transformed_subsection_structure[subsection_usage_key], transformed_subsection_structure
+        )

lms/djangoapps/grades/tests/test_tasks.py

+"""
+Tests for the functionality and infrastructure of grades tasks.
+"""
+
+import ddt
+from django.conf import settings
+from mock import patch
+from unittest import skip
+
+from student.models import anonymous_id_for_user
+from student.tests.factories import UserFactory
+from xmodule.modulestore import ModuleStoreEnum
+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.signals.signals import SCORE_CHANGED
+from lms.djangoapps.grades.tasks import recalculate_subsection_grade
+
+
+@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False})
+@ddt.ddt
+class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
+    """
+    Ensures that the recalculate subsection grade task functions as expected when run.
+    """
+    def setUp(self):
+        super(RecalculateSubsectionGradeTest, self).setUp()
+        self.user = UserFactory()
+        PersistentGradesEnabledFlag.objects.create(enabled_for_all_courses=True, enabled=True)
+
+    def set_up_course(self, enable_subsection_grades=True):
+        """
+        Configures the course for this test.
+        """
+        # pylint: disable=attribute-defined-outside-init,no-member
+        self.course = CourseFactory.create(
+            org='edx',
+            name='course',
+            run='run',
+        )
+        if not enable_subsection_grades:
+            PersistentGradesEnabledFlag.objects.create(enabled=False)
+
+        self.chapter = ItemFactory.create(parent=self.course, category="chapter", display_name="Chapter")
+        self.sequential = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Open Sequential")
+        self.problem = ItemFactory.create(parent=self.sequential, category='problem', display_name='problem')
+
+        self.score_changed_kwargs = {
+            'user_id': self.user.id,
+            'course_id': unicode(self.course.id),
+            'usage_id': unicode(self.problem.location),
+        }
+
+        # this call caches the anonymous id on the user object, saving 4 queries in all happy path tests
+        _ = anonymous_id_for_user(self.user, self.course.id)
+        # pylint: enable=attribute-defined-outside-init,no-member
+
+    def test_score_changed_signal_queues_task(self):
+        """
+        Ensures that the SCORE_CHANGED signal enqueues a recalculate subsection grade task.
+        """
+        self.set_up_course()
+        with patch(
+            'lms.djangoapps.grades.tasks.recalculate_subsection_grade.apply_async',
+            return_value=None
+        ) as mock_task_apply:
+            SCORE_CHANGED.send(sender=None, **self.score_changed_kwargs)
+            mock_task_apply.assert_called_once_with(
+                args=(
+                    self.score_changed_kwargs['user_id'],
+                    self.score_changed_kwargs['course_id'],
+                    self.score_changed_kwargs['usage_id'],
+                )
+            )
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_subsection_grade_updated(self, default_store):
+        with self.store.default_store(default_store):
+            self.set_up_course()
+            self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
+            with check_mongo_calls(2) and self.assertNumQueries(13):
+                recalculate_subsection_grade.apply(
+                    args=(
+                        self.score_changed_kwargs['user_id'],
+                        self.score_changed_kwargs['course_id'],
+                        self.score_changed_kwargs['usage_id'],
+                    )
+                )
+
+    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:
+            recalculate_subsection_grade.apply(
+                args=(
+                    self.score_changed_kwargs['user_id'],
+                    self.score_changed_kwargs['course_id'],
+                    self.score_changed_kwargs['usage_id'],
+                )
+            )
+            self.assertEquals(mock_block_structure_create.call_count, 1)
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_query_count_does_not_change_with_more_problems(self, default_store):
+        with self.store.default_store(default_store):
+            self.set_up_course()
+            self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
+            ItemFactory.create(parent=self.sequential, category='problem', display_name='problem2')
+            ItemFactory.create(parent=self.sequential, category='problem', display_name='problem3')
+            with check_mongo_calls(2) and self.assertNumQueries(13):
+                recalculate_subsection_grade.apply(
+                    args=(
+                        self.score_changed_kwargs['user_id'],
+                        self.score_changed_kwargs['course_id'],
+                        self.score_changed_kwargs['usage_id'],
+                    )
+                )
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_subsection_grades_not_enabled_on_course(self, default_store):
+        with self.store.default_store(default_store):
+            self.set_up_course(enable_subsection_grades=False)
+            self.assertFalse(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
+            with check_mongo_calls(2) and self.assertNumQueries(0):
+                recalculate_subsection_grade.apply(
+                    args=(
+                        self.score_changed_kwargs['user_id'],
+                        self.score_changed_kwargs['course_id'],
+                        self.score_changed_kwargs['usage_id'],
+                    )
+                )
+
+    @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) and self.assertNumQueries(3 if feature_flag else 2):
+                recalculate_subsection_grade.apply(
+                    args=(
+                        self.score_changed_kwargs['user_id'],
+                        self.score_changed_kwargs['course_id'],
+                        self.score_changed_kwargs['usage_id'],
+                    )
+                )

lms/djangoapps/instructor/enrollment.py

@@ -326,7 +326,7 @@ def _fire_score_changed_for_block(course_id, student, block, module_state_key):
         sender=None,
         points_possible=points_possible,
         points_earned=points_earned,
-        user=student,
+        user_id=student.id,
         course_id=unicode(course_id),
         usage_id=unicode(module_state_key)
     )

lms/djangoapps/lti_provider/tasks.py

@@ -27,13 +27,13 @@ def score_changed_handler(sender, **kwargs):  # pylint: disable=unused-argument
     """
     points_possible = kwargs.get('points_possible', None)
     points_earned = kwargs.get('points_earned', None)
-    user = kwargs.get('user', None)
+    user_id = kwargs.get('user_id', None)
     course_id = kwargs.get('course_id', None)
     usage_id = kwargs.get('usage_id', None)
 
-    if None not in (points_earned, points_possible, user.id, course_id, user.id):
+    if None not in (points_earned, points_possible, user_id, course_id):
         course_key, usage_key = parse_course_and_usage_keys(course_id, usage_id)
-        assignments = increment_assignment_versions(course_key, usage_key, user.id)
+        assignments = increment_assignment_versions(course_key, usage_key, user_id)
         for assignment in assignments:
             if assignment.usage_key == usage_key:
                 send_leaf_outcome.delay(
@@ -41,15 +41,15 @@ def score_changed_handler(sender, **kwargs):  # pylint: disable=unused-argument
                 )
             else:
                 send_composite_outcome.apply_async(
-                    (user.id, course_id, assignment.id, assignment.version_number),
+                    (user_id, course_id, assignment.id, assignment.version_number),
                     countdown=settings.LTI_AGGREGATE_SCORE_PASSBACK_DELAY
                 )
     else:
         log.error(
             "Outcome Service: Required signal parameter is None. "
-            "points_possible: %s, points_earned: %s, user: %s, "
+            "points_possible: %s, points_earned: %s, user_id: %s, "
             "course_id: %s, usage_id: %s",
-            points_possible, points_earned, unicode(user), course_id, usage_id
+            points_possible, points_earned, user_id, course_id, usage_id
         )