Merge pull request #13324 from edx/efischer/grades_r_broke

Persistent Grade being saved for individual problems

cms/envs/common.py

@@ -911,7 +911,7 @@ INSTALLED_APPS = (
     # other apps that are.  Django 1.8 wants to have imported models supported
     # by installed apps.
     'lms.djangoapps.verify_student',
-    'lms.djangoapps.grades',
+    'lms.djangoapps.grades.apps.GradesConfig',
 
     # Microsite configuration application
     'microsite_configuration',

common/test/acceptance/pages/lms/progress.py

@@ -22,13 +22,35 @@ class ProgressPage(CoursePage):
     def grading_formats(self):
         return [label.replace(' Scores:', '') for label in self.q(css="div.scores h3").text]
 
+    def section_score(self, chapter, section):
+        """
+        Return a list of (points, max_points) tuples representing the
+        aggregate score for the section.
+
+        Example:
+            page.section_score('Week 1', 'Lesson 1') --> (2, 5)
+
+        Returns `None` if no such chapter and section can be found.
+        """
+        # Find the index of the section in the chapter
+        chapter_index = self._chapter_index(chapter)
+        if chapter_index is None:
+            return None
+
+        section_index = self._section_index(chapter_index, section)
+        if section_index is None:
+            return None
+
+        # Retrieve the scores for the section
+        return self._aggregate_section_score(chapter_index, section_index)
+
     def scores(self, chapter, section):
         """
         Return a list of (points, max_points) tuples representing the scores
         for the section.
 
         Example:
-            scores('Week 1', 'Lesson 1') --> [(2, 4), (0, 1)]
+            page.scores('Week 1', 'Lesson 1') --> [(2, 4), (0, 1)]
 
         Returns `None` if no such chapter and section can be found.
         """
@@ -86,6 +108,28 @@ class ProgressPage(CoursePage):
             self.warning("Could not find section '{0}'".format(title))
             return None
 
+    def _aggregate_section_score(self, chapter_index, section_index):
+        """
+        Return a tuple of the form `(points, max_points)` representing
+        the aggregate score for the specified chapter and section.
+        """
+        score_css = "div.chapters>section:nth-of-type({0}) div.sections>div:nth-of-type({1}) h3>span".format(
+            chapter_index, section_index
+
+        )
+        text_scores = self.q(css=score_css).text
+        assert len(text_scores) == 1
+        text_score = text_scores[0]
+        text_score = text_score.split()[0]  # strip off percentage, if present
+
+        assert (text_score[0], text_score[-1]) == ('(', ')')
+        text_score = text_score.strip('()')
+
+        assert '/' in text_score
+        score = tuple(int(x) for x in text_score.split('/'))
+        assert len(score) == 2
+        return score
+
     def _section_scores(self, chapter_index, section_index):
         """
         Return a list of `(points, max_points)` tuples representing

common/test/acceptance/tests/lms/test_lms_courseware.py

@@ -3,10 +3,12 @@
 End-to-end tests for the LMS.
 """
 
+from contextlib import contextmanager
 import json
-from nose.plugins.attrib import attr
 from datetime import datetime, timedelta
+
 import ddt
+from nose.plugins.attrib import attr
 
 from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
 from ..helpers import UniqueCourseTest, EventsTestMixin
@@ -908,3 +910,89 @@ class SubsectionHiddenAfterDueDateTest(UniqueCourseTest):
 
         self.progress_page.visit()
         self.assertEqual(self.progress_page.scores('Test Section 1', 'Test Subsection 1'), [(0, 1)])
+
+
+class ProgressPageTest(UniqueCourseTest):
+    """
+    Test that the progress page reports scores from completed assessments.
+    """
+    USERNAME = "STUDENT_TESTER"
+    EMAIL = "student101@example.com"
+
+    def setUp(self):
+        super(ProgressPageTest, self).setUp()
+
+        self.courseware_page = CoursewarePage(self.browser, self.course_id)
+        self.problem_page = ProblemPage(self.browser)  # pylint: disable=attribute-defined-outside-init
+        self.progress_page = ProgressPage(self.browser, self.course_id)
+        self.logout_page = LogoutPage(self.browser)
+
+        self.course_outline = CourseOutlinePage(
+            self.browser,
+            self.course_info['org'],
+            self.course_info['number'],
+            self.course_info['run']
+        )
+
+        # Install a course with sections/problems, tabs, updates, and handouts
+        course_fix = CourseFixture(
+            self.course_info['org'],
+            self.course_info['number'],
+            self.course_info['run'],
+            self.course_info['display_name']
+        )
+
+        course_fix.add_children(
+            XBlockFixtureDesc('chapter', 'Test Section 1').add_children(
+                XBlockFixtureDesc('sequential', 'Test Subsection 1').add_children(
+                    create_multiple_choice_problem('Test Problem 1')
+                )
+            )
+        ).install()
+
+        # Auto-auth register for the course.
+        _auto_auth(self.browser, self.USERNAME, self.EMAIL, False, self.course_id)
+
+    def test_progress_page_shows_scored_problems(self):
+        with self._logged_in_session():
+            self.assertEqual(self._get_scores(), [(0, 1)])
+            self.assertEqual(self._get_section_score(), (0, 1))
+            self.courseware_page.visit()
+            self._answer_problem_correctly()
+            self.assertEqual(self._get_scores(), [(1, 1)])
+            self.assertEqual(self._get_section_score(), (1, 1))
+
+    def _answer_problem_correctly(self):
+        """
+        Submit a correct answer to the problem.
+        """
+        self.courseware_page.go_to_sequential_position(1)
+        self.problem_page.click_choice('choice_choice_2')
+        self.problem_page.click_check()
+
+    def _get_section_score(self):
+        """
+        Return a list of scores from the progress page.
+        """
+        self.progress_page.visit()
+        return self.progress_page.section_score('Test Section 1', 'Test Subsection 1')
+
+    def _get_scores(self):
+        """
+        Return a list of scores from the progress page.
+        """
+        self.progress_page.visit()
+        return self.progress_page.scores('Test Section 1', 'Test Subsection 1')
+
+    @contextmanager
+    def _logged_in_session(self):
+        """
+        Ensure that the user is logged in and out appropriately at the beginning
+        and end of the current test.
+        """
+        self.logout_page.visit()
+        try:
+            _auto_auth(self.browser, self.USERNAME, self.EMAIL, False, self.course_id)
+            yield
+        finally:
+            self.logout_page.visit()

lms/djangoapps/course_blocks/transformers/tests/helpers.py

@@ -115,8 +115,9 @@ class CourseStructureTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase)
             # It would be re-added to the course if the course was
             # explicitly listed in parents.
             course = modulestore().get_item(block_map['course'].location)
-            course.children.remove(block_key)
-            block_map['course'] = update_block(course)
+            if block_key in course.children:
+                course.children.remove(block_key)
+                block_map['course'] = update_block(course)
 
             # Add this to block to each listed parent.
             for parent_ref in parents:

lms/djangoapps/course_blocks/transformers/utils.py

@@ -18,6 +18,43 @@ def get_field_on_block(block, field_name, default_value=None):
     return default_value
 
 
+def collect_unioned_set_field(block_structure, transformer, merged_field_name, filter_by):
+    """
+    Recursively union a set field on the block structure.
+
+    If a block matches filter_by, it will be added to the result set.
+    This (potentially empty) set is unioned with the sets contained in
+    merged_field_name for all parents of the block.
+
+    This set union operation takes place during a topological traversal
+    of the block_structure, so all sets are inherited by descendants.
+
+    Parameters:
+        block_structure: BlockStructure to traverse
+        transformer: transformer that will be used for get_ and
+            set_transformer_block_field
+        merged_field_name: name of the field to store
+        filter_by: a unary lambda that returns true if a given
+            block_key should be included in the result set
+    """
+    for block_key in block_structure.topological_traversal():
+        result_set = {block_key} if filter_by(block_key) else set()
+        for parent in block_structure.get_parents(block_key):
+            result_set |= block_structure.get_transformer_block_field(
+                parent,
+                transformer,
+                merged_field_name,
+                set(),
+            )
+
+        block_structure.set_transformer_block_field(
+            block_key,
+            transformer,
+            merged_field_name,
+            result_set,
+        )
+
+
 def collect_merged_boolean_field(
         block_structure,
         transformer,

lms/djangoapps/courseware/module_render.py

@@ -45,7 +45,7 @@ from courseware.masquerade import (
     setup_masquerade,
 )
 from courseware.model_data import DjangoKeyValueStore, FieldDataCache, set_score
-from lms.djangoapps.grades.signals import SCORE_CHANGED
+from lms.djangoapps.grades.signals.signals import SCORE_CHANGED
 from edxmako.shortcuts import render_to_string
 from lms.djangoapps.lms_xblock.field_data import LmsFieldData
 from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig

lms/djangoapps/gating/signals.py

@@ -4,7 +4,7 @@ Signal handlers for the gating djangoapp
 from django.dispatch import receiver
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from xmodule.modulestore.django import modulestore
-from lms.djangoapps.grades.signals import SCORE_CHANGED
+from lms.djangoapps.grades.signals.signals import SCORE_CHANGED
 from gating import api as gating_api
 
 

lms/djangoapps/grades/apps.py

+"""
+Grades Application Configuration
+
+Signal handlers are connected here.
+"""
+
+from django.apps import AppConfig
+
+
+class GradesConfig(AppConfig):
+    """
+    Application Configuration for Grades.
+    """
+    name = u'lms.djangoapps.grades'
+
+    def ready(self):
+        """
+        Connect handlers to recalculate grades.
+        """
+        # Can't import models at module level in AppConfigs, and models get
+        # included from the signal handlers
+        from .signals import handlers  # pylint: disable=unused-variable

lms/djangoapps/grades/new/subsection_grade.py

@@ -132,7 +132,7 @@ class SubsectionGrade(object):
 
             # There's a chance that the value of weight is not the same value used when the problem was scored,
             # since we can get the value from either block_structure or CSM/submissions.
-            weight = getattr(block, 'weight', 1.0)
+            weight = getattr(block, 'weight', None)
             if persisted_values:
                 possible = persisted_values.get('possible', possible)
                 weight = persisted_values.get('weight', weight)

lms/djangoapps/grades/signals.py → lms/djangoapps/grades/signals/handlers.py

 """
 Grades related signals.
 """
-from django.conf import settings
-from django.dispatch import receiver, Signal
-from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag
 from logging import getLogger
+
+from django.dispatch import receiver
+
 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
 
-log = getLogger(__name__)
-
+from .signals import SCORE_CHANGED
+from ..config.models import PersistentGradesEnabledFlag
+from ..transformer import GradesTransformer
+from ..new.subsection_grade import SubsectionGradeFactory
 
-# Signal that indicates that a user's score for a problem has been updated.
-# This signal is generated when a scoring event occurs either within the core
-# platform or in the Submissions module. Note that this signal will be triggered
-# regardless of the new and previous values of the score (i.e. it may be the
-# case that this signal is generated when a user re-attempts a problem but
-# receives the same score).
-SCORE_CHANGED = Signal(
-    providing_args=[
-        'points_possible',  # Maximum score available for the exercise
-        'points_earned',   # Score obtained by the user
-        'user_id',  # Integer User ID
-        'course_id',  # Unicode string representing the course
-        'usage_id'  # Unicode string indicating the courseware instance
-    ]
-)
+log = getLogger(__name__)
 
 
 @receiver(score_set)
@@ -46,31 +35,22 @@ def submissions_score_set_handler(sender, **kwargs):  # pylint: disable=unused-a
       - 'course_id': unicode,
       - 'item_id': unicode
     """
-    points_possible = kwargs.get('points_possible', None)
-    points_earned = kwargs.get('points_earned', None)
-    course_id = kwargs.get('course_id', None)
-    usage_id = kwargs.get('item_id', None)
-    user = None
-    if 'anonymous_user_id' in kwargs:
-        user = user_by_anonymous_id(kwargs.get('anonymous_user_id'))
+    points_possible = kwargs['points_possible']
+    points_earned = kwargs['points_earned']
+    course_id = kwargs['course_id']
+    usage_id = kwargs['item_id']
+    user = user_by_anonymous_id(kwargs['anonymous_user_id'])
 
     # If any of the kwargs were missing, at least one of the following values
     # will be None.
-    if all((user, points_possible, points_earned, course_id, usage_id)):
-        SCORE_CHANGED.send(
-            sender=None,
-            points_possible=points_possible,
-            points_earned=points_earned,
-            user=user,
-            course_id=course_id,
-            usage_id=usage_id
-        )
-    else:
-        log.exception(
-            u"Failed to process score_set signal from Submissions API. "
-            "points_possible: %s, points_earned: %s, user: %s, course_id: %s, "
-            "usage_id: %s", points_possible, points_earned, user, course_id, usage_id
-        )
+    SCORE_CHANGED.send(
+        sender=None,
+        points_possible=points_possible,
+        points_earned=points_earned,
+        user=user,
+        course_id=course_id,
+        usage_id=usage_id
+    )
 
 
 @receiver(score_reset)
@@ -87,28 +67,20 @@ def submissions_score_reset_handler(sender, **kwargs):  # pylint: disable=unused
       - 'course_id': unicode,
       - 'item_id': unicode
     """
-    course_id = kwargs.get('course_id', None)
-    usage_id = kwargs.get('item_id', None)
-    user = None
-    if 'anonymous_user_id' in kwargs:
-        user = user_by_anonymous_id(kwargs.get('anonymous_user_id'))
+    course_id = kwargs['course_id']
+    usage_id = kwargs['item_id']
+    user = user_by_anonymous_id(kwargs['anonymous_user_id'])
 
     # If any of the kwargs were missing, at least one of the following values
     # will be None.
-    if all((user, course_id, usage_id)):
-        SCORE_CHANGED.send(
-            sender=None,
-            points_possible=0,
-            points_earned=0,
-            user=user,
-            course_id=course_id,
-            usage_id=usage_id
-        )
-    else:
-        log.exception(
-            u"Failed to process score_reset signal from Submissions API. "
-            "user: %s, course_id: %s, usage_id: %s", user, course_id, usage_id
-        )
+    SCORE_CHANGED.send(
+        sender=None,
+        points_possible=0,
+        points_earned=0,
+        user=user,
+        course_id=course_id,
+        usage_id=usage_id
+    )
 
 
 @receiver(SCORE_CHANGED)
@@ -124,21 +96,32 @@ def recalculate_subsection_grade_handler(sender, **kwargs):  # pylint: disable=u
        - usage_id: Unicode string indicating the courseware instance
     """
     try:
-        course_id = kwargs.get('course_id', None)
-        usage_id = kwargs.get('usage_id', None)
-        student = kwargs.get('user', None)
+        course_id = kwargs['course_id']
+        usage_id = kwargs['usage_id']
+        student = kwargs['user']
+    except KeyError:
+        log.exception(
+            u"Failed to process SCORE_CHANGED signal, some arguments were missing."
+            "user: %s, course_id: %s, usage_id: %s.",
+            kwargs.get('user', None),
+            kwargs.get('course_id', None),
+            kwargs.get('usage_id', None),
+        )
+        return
 
-        course_key = CourseLocator.from_string(course_id)
-        if not PersistentGradesEnabledFlag.feature_enabled(course_key):
-            return
+    course_key = CourseLocator.from_string(course_id)
+    if not PersistentGradesEnabledFlag.feature_enabled(course_key):
+        return
 
-        usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key)
+    usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key)
+    block_structure = get_course_in_cache(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
-        )
+    subsections_to_update = block_structure.get_transformer_block_field(
+        usage_key,
+        GradesTransformer,
+        'subsections',
+        set()
+    )
+
+    for subsection in subsections_to_update:
+        SubsectionGradeFactory(student).update(subsection, course_key)

lms/djangoapps/grades/signals/signals.py

+"""
+Grades related signals.
+"""
+from django.dispatch import Signal
+
+
+# Signal that indicates that a user's score for a problem has been updated.
+# This signal is generated when a scoring event occurs either within the core
+# platform or in the Submissions module. Note that this signal will be triggered
+# regardless of the new and previous values of the score (i.e. it may be the
+# case that this signal is generated when a user re-attempts a problem but
+# receives the same score).
+SCORE_CHANGED = Signal(
+    providing_args=[
+        'points_possible',  # Maximum score available for the exercise
+        'points_earned',   # Score obtained by the user
+        'user_id',  # Integer User ID
+        'course_id',  # Unicode string representing the course
+        'usage_id'  # Unicode string indicating the courseware instance
+    ]
+)

lms/djangoapps/grades/tests/test_grades.py

@@ -382,10 +382,12 @@ class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
         self.client.login(username=self.request.user.username, password="test")
         CourseEnrollment.enroll(self.request.user, self.course.id)
 
+        self.course_structure = get_course_blocks(self.request.user, self.course.location)
+
         # warm up the score cache to allow accurate query counts, even if tests are run in random order
         get_module_score(self.request.user, self.course, self.seq1)
 
-    def test_get_module_score(self):
+    def test_subsection_scores(self):
         """
         Test test_get_module_score
         """
@@ -393,21 +395,30 @@ class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
         # then stored in the request).
         with self.assertNumQueries(1):
             score = get_module_score(self.request.user, self.course, self.seq1)
+        new_score = SubsectionGradeFactory(self.request.user).create(self.seq1, self.course_structure, self.course)
         self.assertEqual(score, 0)
+        self.assertEqual(new_score.all_total.earned, 0)
 
         answer_problem(self.course, self.request, self.problem1)
         answer_problem(self.course, self.request, self.problem2)
 
         with self.assertNumQueries(1):
             score = get_module_score(self.request.user, self.course, self.seq1)
+        new_score = SubsectionGradeFactory(self.request.user).create(self.seq1, self.course_structure, self.course)
         self.assertEqual(score, 1.0)
+        self.assertEqual(new_score.all_total.earned, 2.0)
+        # These differ because get_module_score normalizes the subsection score
+        # to 1, which can cause incorrect aggregation behavior that will be
+        # fixed by TNL-5062.
 
         answer_problem(self.course, self.request, self.problem1)
         answer_problem(self.course, self.request, self.problem2, 0)
 
         with self.assertNumQueries(1):
             score = get_module_score(self.request.user, self.course, self.seq1)
+        new_score = SubsectionGradeFactory(self.request.user).create(self.seq1, self.course_structure, self.course)
         self.assertEqual(score, .5)
+        self.assertEqual(new_score.all_total.earned, 1.0)
 
     def test_get_module_score_with_empty_score(self):
         """

lms/djangoapps/grades/tests/test_new.py

@@ -71,13 +71,6 @@ class TestCourseGradeFactory(GradeTestBase):
     Test that CourseGrades are calculated properly
     """
 
-    @classmethod
-    def setUpClass(cls):
-        super(TestCourseGradeFactory, cls).setUpClass()
-
-    def setUp(self):
-        super(TestCourseGradeFactory, self).setUp()
-
     @ddt.data(
         (True, True),
         (True, False),
@@ -110,16 +103,6 @@ class SubsectionGradeFactoryTest(GradeTestBase):
     enable saving subsection grades blocks/enables that feature as expected.
     """
 
-    @classmethod
-    def setUpClass(cls):
-        super(SubsectionGradeFactoryTest, cls).setUpClass()
-
-    def setUp(self):
-        """
-        Set up test course
-        """
-        super(SubsectionGradeFactoryTest, self).setUp()
-
     def test_create(self):
         """
         Tests to ensure that a persistent subsection grade is created, saved, then fetched on re-request.
@@ -190,13 +173,6 @@ class SubsectionGradeTest(GradeTestBase):
     Tests SubsectionGrade functionality.
     """
 
-    @classmethod
-    def setUpClass(cls):
-        super(SubsectionGradeTest, cls).setUpClass()
-
-    def setUp(self):
-        super(SubsectionGradeTest, self).setUp()
-
     def test_compute(self):
         """
         Assuming the underlying score reporting methods work, test that the score is calculated properly.

lms/djangoapps/grades/tests/test_signals.py

@@ -13,12 +13,12 @@ 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 (
+from ..signals.handlers import (
     submissions_score_set_handler,
     submissions_score_reset_handler,
     recalculate_subsection_grade_handler,
-    SCORE_CHANGED
 )
+from ..signals.signals import SCORE_CHANGED
 
 
 SUBMISSION_SET_KWARGS = {
@@ -50,10 +50,13 @@ class SubmissionSignalRelayTest(TestCase):
         Configure mocks for all the dependencies of the render method
         """
         super(SubmissionSignalRelayTest, self).setUp()
-        self.signal_mock = self.setup_patch('lms.djangoapps.grades.signals.SCORE_CHANGED.send', None)
+        self.signal_mock = self.setup_patch('lms.djangoapps.grades.signals.signals.SCORE_CHANGED.send', None)
         self.user_mock = MagicMock()
         self.user_mock.id = 42
-        self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.user_by_anonymous_id', self.user_mock)
+        self.get_user_mock = self.setup_patch(
+            'lms.djangoapps.grades.signals.handlers.user_by_anonymous_id',
+            self.user_mock
+        )
 
     def setup_patch(self, function_name, return_value):
         """
@@ -100,7 +103,8 @@ class SubmissionSignalRelayTest(TestCase):
             kwargs = SUBMISSION_SET_KWARGS.copy()
             del kwargs[missing]
 
-            submissions_score_set_handler(None, **kwargs)
+            with self.assertRaises(KeyError):
+                submissions_score_set_handler(None, **kwargs)
             self.signal_mock.assert_not_called()
 
     def test_score_set_bad_user(self):
@@ -109,7 +113,7 @@ class SubmissionSignalRelayTest(TestCase):
         that has an invalid user ID, the courseware model does not generate a
         signal.
         """
-        self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.user_by_anonymous_id', None)
+        self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.handlers.user_by_anonymous_id', None)
         submissions_score_set_handler(None, **SUBMISSION_SET_KWARGS)
         self.signal_mock.assert_not_called()
 
@@ -149,7 +153,8 @@ class SubmissionSignalRelayTest(TestCase):
             kwargs = SUBMISSION_RESET_KWARGS.copy()
             del kwargs[missing]
 
-            submissions_score_reset_handler(None, **kwargs)
+            with self.assertRaises(KeyError):
+                submissions_score_reset_handler(None, **kwargs)
             self.signal_mock.assert_not_called()
 
     def test_score_reset_bad_user(self):
@@ -158,7 +163,7 @@ class SubmissionSignalRelayTest(TestCase):
         that has an invalid user ID, the courseware model does not generate a
         signal.
         """
-        self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.user_by_anonymous_id', None)
+        self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.handlers.user_by_anonymous_id', None)
         submissions_score_reset_handler(None, **SUBMISSION_RESET_KWARGS)
         self.signal_mock.assert_not_called()
 
@@ -243,15 +248,15 @@ class ScoreChangedUpdatesSubsectionGradeTest(ModuleStoreTestCase):
     @ddt.data(
         ('points_possible', 2, 19),
         ('points_earned', 2, 19),
-        ('user', 0, 3),
+        ('user', 0, 0),
         ('course_id', 0, 0),
-        ('usage_id', 0, 2),
+        ('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 patch('lms.djangoapps.grades.signals.handlers.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/grades/tests/test_transformer.py

@@ -102,6 +102,98 @@ class GradesTransformerTestCase(CourseStructureTestCase):
             }
         ])
 
+    def build_complicated_hypothetical_course(self):
+        """
+        Create a test course with a very odd structure as a stress-test for various methods.
+
+        Current design is to test containing_subsection logic in collect_unioned_set_field.
+        I can't reasonably draw this in ascii art (due to intentional complexities), so here's an overview:
+            We have 1 course, containing 1 chapter, containing 2 subsections.
+
+            From here, it starts to get hairy. Call our subsections A and B.
+            Subsection A contains 3 verticals (call them 1, 2, and 3), and another subsection (C)
+            Subsection B contains vertical 3 and subsection C
+            Subsection C contains 1 problem (b)
+            Vertical 1 contains 1 vertical (11)
+            Vertical 2 contains no children
+            Vertical 3 contains no children
+            Vertical 11 contains 1 problem (aa) and vertical 2
+            Problem b contains no children
+        """
+        return self.build_course([
+            {
+                u'org': u'GradesTestOrg',
+                u'course': u'GB101',
+                u'run': u'cannonball',
+                u'metadata': {u'format': u'homework'},
+                u'#type': u'course',
+                u'#ref': u'course',
+                u'#children': [
+                    {
+                        u'#type': u'chapter',
+                        u'#ref': u'chapter',
+                        u'#children': [
+                            {
+                                u'#type': u'sequential',
+                                u'#ref': 'sub_A',
+                                u'#children': [
+                                    {
+                                        u'#type': u'vertical',
+                                        u'#ref': 'vert_1',
+                                        u'#children': [
+                                            {
+                                                u'#type': u'vertical',
+                                                u'#ref': u'vert_A11',
+                                                u'#children': [{u'#type': u'problem', u'#ref': u'prob_A1aa'}]
+                                            },
+                                        ]
+                                    },
+                                    {u'#type': u'vertical', u'#ref': 'vert_2', '#parents': [u'vert_A11']},
+                                ]
+                            },
+                            {
+                                u'#type': u'sequential',
+                                u'#ref': u'sub_B',
+                                u'#children': [
+                                    {u'#type': u'vertical', u'#ref': 'vert_3', '#parents': ['sub_A']},
+                                    {
+                                        u'#type': u'sequential',
+                                        u'#ref': 'sub_C',
+                                        '#parents': ['sub_A'],
+                                        u'#children': [{u'#type': u'problem', u'#ref': u'prob_BCb'}]
+                                    },
+                                ]
+                            },
+                        ]
+                    }
+                ]
+            }
+        ])
+
+    def test_collect_containing_subsection(self):
+        expected_subsections = {
+            'course': set(),
+            'chapter': set(),
+            'sub_A': {'sub_A'},
+            'sub_B': {'sub_B'},
+            'sub_C': {'sub_A', 'sub_B', 'sub_C'},
+            'vert_1': {'sub_A'},
+            'vert_2': {'sub_A'},
+            'vert_3': {'sub_A', 'sub_B'},
+            'vert_A11': {'sub_A'},
+            'prob_A1aa': {'sub_A'},
+            'prob_BCb': {'sub_A', 'sub_B', 'sub_C'},
+        }
+        blocks = self.build_complicated_hypothetical_course()
+        block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers)
+        for block_ref, expected_subsections in expected_subsections.iteritems():
+            actual_subsections = block_structure.get_transformer_block_field(
+                blocks[block_ref].location,
+                self.TRANSFORMER_CLASS_TO_TEST,
+                'subsections',
+            )
+            self.assertEqual(actual_subsections, {blocks[sub].location for sub in expected_subsections})
+
     def test_ungraded_block_collection(self):
         blocks = self.build_course_with_problems()
         block_structure = get_course_blocks(self.student, blocks[u'course'].location, self.transformers)

lms/djangoapps/grades/transformer.py

@@ -5,6 +5,7 @@ from django.test.client import RequestFactory
 
 from courseware.model_data import FieldDataCache
 from courseware.module_render import get_module_for_descriptor
+from lms.djangoapps.course_blocks.transformers.utils import collect_unioned_set_field
 from openedx.core.lib.block_structure.transformer import BlockStructureTransformer
 from openedx.core.djangoapps.util.user_utils import SystemUser
 
@@ -30,7 +31,7 @@ class GradesTransformer(BlockStructureTransformer):
 
         max_score: (numeric)
     """
-    VERSION = 2
+    VERSION = 3
     FIELDS_TO_COLLECT = [u'due', u'format', u'graded', u'has_score', u'weight', u'course_version', u'subtree_edited_on']
 
     @classmethod
@@ -49,6 +50,12 @@ class GradesTransformer(BlockStructureTransformer):
         """
         block_structure.request_xblock_fields(*cls.FIELDS_TO_COLLECT)
         cls._collect_max_scores(block_structure)
+        collect_unioned_set_field(
+            block_structure=block_structure,
+            transformer=cls,
+            merged_field_name='subsections',
+            filter_by=lambda block_key: block_key.block_type == 'sequential',
+        )
 
     def transform(self, block_structure, usage_context):
         """

lms/djangoapps/lti_provider/tasks.py

@@ -8,7 +8,7 @@ from django.dispatch import receiver
 import logging
 
 from lms.djangoapps.grades import progress
-from lms.djangoapps.grades.signals import SCORE_CHANGED
+from lms.djangoapps.grades.signals.signals import SCORE_CHANGED
 from lms import CELERY_APP
 from lti_provider.models import GradedAssignment
 import lti_provider.outcomes as outcomes

lms/envs/common.py

@@ -1912,7 +1912,7 @@ INSTALLED_APPS = (
     'openedx.core.djangoapps.course_groups',
     'bulk_email',
     'branding',
-    'lms.djangoapps.grades',
+    'lms.djangoapps.grades.apps.GradesConfig',
 
     # Student support tools
     'support',