handler for SCORE_CHANGED signal

lms/djangoapps/courseware/module_render.py

@@ -539,7 +539,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_id=user_id,
+            user=user,
             course_id=unicode(course_id),
             usage_id=unicode(descriptor.location)
         )

lms/djangoapps/courseware/tests/test_module_render.py

@@ -1843,7 +1843,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems):
             'sender': None,
             'points_possible': self.grade_dict['max_value'],
             'points_earned': self.grade_dict['value'],
-            'user_id': self.student_user.id,
+            'user': self.student_user,
             '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

@@ -4,6 +4,7 @@ Unit tests for gating.signals module
 from mock import patch
 
 from opaque_keys.edx.keys import UsageKey
+from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.django import modulestore
@@ -18,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.test_user_id = 0
+        self.user = UserFactory()
         self.test_usage_key = UsageKey.from_string('i4x://the/content/key/12345678')
 
     @patch('gating.signals.gating_api.evaluate_prerequisite')
@@ -30,11 +31,11 @@ class TestHandleScoreChanged(ModuleStoreTestCase):
             sender=None,
             points_possible=1,
             points_earned=1,
-            user_id=self.test_user_id,
+            user=self.user,
             course_id=unicode(self.course.id),
             usage_id=unicode(self.test_usage_key)
         )
-        mock_evaluate.assert_called_with(self.course, self.test_usage_key, self.test_user_id)
+        mock_evaluate.assert_called_with(self.course, self.test_usage_key, self.user.id)  # pylint: disable=no-member
 
     @patch('gating.signals.gating_api.evaluate_prerequisite')
     def test_gating_disabled(self, mock_evaluate):
@@ -43,7 +44,7 @@ class TestHandleScoreChanged(ModuleStoreTestCase):
             sender=None,
             points_possible=1,
             points_earned=1,
-            user_id=self.test_user_id,
+            user=self.user,
             course_id=unicode(self.course.id),
             usage_id=unicode(self.test_usage_key)
         )

lms/djangoapps/grades/new/subsection_grade.py

@@ -6,6 +6,7 @@ from lazy import lazy
 
 from django.conf import settings
 
+from course_blocks.api import get_course_blocks
 from courseware.model_data import ScoresClient
 from lms.djangoapps.grades.scores import get_score, possibly_scored
 from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade
@@ -59,6 +60,7 @@ class SubsectionGrade(object):
             BlockRecord(location, weight, score.possible)
             for location, (score, weight) in self.locations_to_weighted_scores.iteritems()
         ]
+
         PersistentSubsectionGrade.save_grade(
             user_id=student.id,
             usage_key=self.location,
@@ -166,6 +168,22 @@ class SubsectionGradeFactory(object):
             self._compute_and_save_grade(subsection, course_structure, course)
         )
 
+    def update(self, usage_key, course_key):
+        """
+        Updates the SubsectionGrade object for the student and subsection
+        identified by the given usage key.
+        """
+        from courseware.courses import get_course_by_id  # avoids circular import with courseware.py
+        course = get_course_by_id(course_key, depth=0)
+        # save ourselves the extra queries if the course does not use subsection grades
+        if not course.enable_subsection_grades_saved:
+            return
+
+        course_structure = get_course_blocks(self.student, usage_key)
+        subsection = course_structure[usage_key]
+        self._prefetch_scores(course_structure, course)
+        return self._compute_and_save_grade(subsection, course_structure, course)
+
     def _compute_and_save_grade(self, subsection, course_structure, course):
         """
         Freshly computes and updates the grade for the student and subsection.

lms/djangoapps/grades/signals.py

 """
 Grades related signals.
 """
+from django.conf import settings
 from django.dispatch import receiver, Signal
 from logging import getLogger
+from opaque_keys.edx.locator import CourseLocator
+from opaque_keys.edx.keys import UsageKey
 from student.models import user_by_anonymous_id
 from submissions.models import score_set, score_reset
 
-
 log = getLogger(__name__)
 
 
@@ -58,7 +60,7 @@ def submissions_score_set_handler(sender, **kwargs):  # pylint: disable=unused-a
             sender=None,
             points_possible=points_possible,
             points_earned=points_earned,
-            user_id=user.id,
+            user=user,
             course_id=course_id,
             usage_id=usage_id
         )
@@ -97,7 +99,7 @@ def submissions_score_reset_handler(sender, **kwargs):  # pylint: disable=unused
             sender=None,
             points_possible=0,
             points_earned=0,
-            user_id=user.id,
+            user=user,
             course_id=course_id,
             usage_id=usage_id
         )
@@ -106,3 +108,35 @@ def submissions_score_reset_handler(sender, **kwargs):  # pylint: disable=unused
             u"Failed to process score_reset signal from Submissions API. "
             "user: %s, course_id: %s, usage_id: %s", user, course_id, usage_id
         )
+
+
+@receiver(SCORE_CHANGED)
+def recalculate_subsection_grade_handler(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
+    """
+    if not settings.FEATURES.get('ENABLE_SUBSECTION_GRADES_SAVED', False):
+        return
+    try:
+        course_id = kwargs.get('course_id', None)
+        usage_id = kwargs.get('usage_id', None)
+        student = kwargs.get('user', None)
+
+        course_key = CourseLocator.from_string(course_id)
+        usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key)
+
+        from lms.djangoapps.grades.new.subsection_grade import SubsectionGradeFactory
+        SubsectionGradeFactory(student).update(usage_key, course_key)
+    except Exception as ex:  # pylint: disable=broad-except
+        log.exception(
+            u"Failed to process SCORE_CHANGED signal. "
+            "user: %s, course_id: %s, "
+            "usage_id: %s. Exception: %s", unicode(student), course_id, usage_id, ex.message
+        )

lms/djangoapps/grades/tests/test_signals.py

@@ -2,10 +2,22 @@
 Tests for the score change signals defined in the courseware models module.
 """
 
+import ddt
+from unittest import skip
 from django.test import TestCase
 from mock import patch, MagicMock
+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 ..signals import submissions_score_set_handler, submissions_score_reset_handler
+from ..signals import (
+    submissions_score_set_handler,
+    submissions_score_reset_handler,
+    recalculate_subsection_grade_handler,
+    SCORE_CHANGED
+)
 
 
 SUBMISSION_SET_KWARGS = {
@@ -62,7 +74,7 @@ class SubmissionSignalRelayTest(TestCase):
             'sender': None,
             'points_possible': 10,
             'points_earned': 5,
-            'user_id': 42,
+            'user': self.user_mock,
             'course_id': 'CourseID',
             'usage_id': 'i4x://org/course/usage/123456'
         }
@@ -111,7 +123,7 @@ class SubmissionSignalRelayTest(TestCase):
             'sender': None,
             'points_possible': 0,
             'points_earned': 0,
-            'user_id': 42,
+            'user': self.user_mock,
             'course_id': 'CourseID',
             'usage_id': 'i4x://org/course/usage/123456'
         }
@@ -148,3 +160,95 @@ class SubmissionSignalRelayTest(TestCase):
         self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.user_by_anonymous_id', None)
         submissions_score_reset_handler(None, **SUBMISSION_RESET_KWARGS)
         self.signal_mock.assert_not_called()
+
+
+@ddt.ddt
+class ScoreChangedUpdatesSubsectionGradeTest(ModuleStoreTestCase):
+    """
+    Ensures that upon SCORE_CHANGED signals, the handler
+    initiates an update to the affected subsection grade.
+    """
+    def setUp(self):
+        super(ScoreChangedUpdatesSubsectionGradeTest, self).setUp()
+        self.user = UserFactory()
+
+    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',
+            metadata={'enable_subsection_grades_saved': enable_subsection_grades})
+
+        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 = {
+            'points_possible': 10,
+            'points_earned': 5,
+            'user': self.user,
+            '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
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_subsection_grade_updated_on_signal(self, default_store):
+        with self.store.default_store(default_store):
+            self.set_up_course()
+            with check_mongo_calls(2) and self.assertNumQueries(13):
+                recalculate_subsection_grade_handler(None, **self.score_changed_kwargs)
+
+    @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()
+            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_handler(None, **self.score_changed_kwargs)
+
+    @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)
+            with check_mongo_calls(2) and self.assertNumQueries(0):
+                recalculate_subsection_grade_handler(None, **self.score_changed_kwargs)
+
+    @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_score_changed_sent_with_feature_flag(self, default_store, feature_flag):
+        with patch.dict('django.conf.settings.FEATURES', {'ENABLE_SUBSECTION_GRADES_SAVED': feature_flag}):
+            with self.store.default_store(default_store):
+                self.set_up_course()
+                with check_mongo_calls(0) and self.assertNumQueries(19 if feature_flag else 1):
+                    SCORE_CHANGED.send(sender=None, **self.score_changed_kwargs)
+
+    @ddt.data(
+        ('points_possible', 2, 13),
+        ('points_earned', 2, 13),
+        ('user', 0, 0),
+        ('course_id', 0, 0),
+        ('usage_id', 0, 0),
+    )
+    @ddt.unpack
+    def test_missing_kwargs(self, kwarg, expected_mongo_calls, expected_sql_calls):
+        self.set_up_course()
+        del self.score_changed_kwargs[kwarg]
+        with patch('lms.djangoapps.grades.signals.log') as log_mock:
+            with check_mongo_calls(expected_mongo_calls) and self.assertNumQueries(expected_sql_calls):
+                recalculate_subsection_grade_handler(None, **self.score_changed_kwargs)
+            self.assertEqual(log_mock.exception.called, kwarg not in ['points_possible', 'points_earned'])

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_id = kwargs.get('user_id', None)
+    user = kwargs.get('user', 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, user.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_id: %s, "
+            "points_possible: %s, points_earned: %s, user: %s, "
             "course_id: %s, usage_id: %s",
-            points_possible, points_earned, user_id, course_id, usage_id
+            points_possible, points_earned, unicode(user), course_id, usage_id
         )