Merge pull request #13093 from edx/beryl/grading

Use Persistent Subsection Scores in Grading

cms/envs/bok_choy.py

@@ -93,6 +93,9 @@ FEATURES['LICENSING'] = True
 FEATURES['ENABLE_MOBILE_REST_API'] = True  # Enable video bumper in Studio
 FEATURES['ENABLE_VIDEO_BUMPER'] = True  # Enable video bumper in Studio settings
 
+# Enable persistent subsection grades, so that feature can be tested.
+FEATURES['ENABLE_SUBSECTION_GRADES_SAVED'] = True
+
 # Enable partner support link in Studio footer
 PARTNER_SUPPORT_EMAIL = 'partner-support@example.com'
 

cms/envs/common.py

@@ -921,6 +921,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',
 
     # Microsite configuration application
     'microsite_configuration',

cms/envs/test.py

@@ -322,6 +322,9 @@ SEARCH_ENGINE = "search.tests.mock_search_engine.MockSearchEngine"
 # teams feature
 FEATURES['ENABLE_TEAMS'] = True
 
+# Enable persistent subsection grades, so that feature can be tested.
+FEATURES['ENABLE_SUBSECTION_GRADES_SAVED'] = True
+
 # Dummy secret key for dev/test
 SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
 

common/lib/xmodule/xmodule/graders.py

@@ -26,9 +26,10 @@ def float_sum(iterable):
     return float(sum(iterable))
 
 
-def aggregate_scores(scores, section_name="summary"):
+def aggregate_scores(scores, section_name="summary", location=None):
     """
     scores: A list of Score objects
+    location: The location under which all objects in scores are located
     returns: A tuple (all_total, graded_total).
         all_total: A Score representing the total score summed over all input scores
         graded_total: A Score representing the score summed over all graded input scores
@@ -45,7 +46,7 @@ def aggregate_scores(scores, section_name="summary"):
         total_possible,
         False,
         section_name,
-        None
+        location,
     )
     #selecting only graded things
     graded_total = Score(
@@ -53,7 +54,7 @@ def aggregate_scores(scores, section_name="summary"):
         total_possible_graded,
         True,
         section_name,
-        None
+        location,
     )
 
     return all_total, graded_total

common/test/acceptance/pages/studio/settings_advanced.py

@@ -227,5 +227,6 @@ class AdvancedSettingsPage(CoursePage):
             'instructor_info',
             'create_zendesk_tickets',
             'ccx_connector',
-            'enable_ccx'
+            'enable_ccx',
+            'enable_subsection_grades_saved',
         ]

lms/djangoapps/ccx/views.py

@@ -31,7 +31,7 @@ from courseware.courses import get_course_by_id
 
 from courseware.field_overrides import disable_overrides
 from edxmako.shortcuts import render_to_response
-from grades.course_grades import iterate_grades_for
+from lms.djangoapps.grades.course_grades import iterate_grades_for
 from opaque_keys.edx.keys import CourseKey
 from ccx_keys.locator import CCXLocator
 from student.roles import CourseCcxCoachRole

lms/djangoapps/grades/migrations/0002_rename_last_edited_field.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('grades', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name='persistentsubsectiongrade',
+            old_name='subtree_edited_date',
+            new_name='subtree_edited_timestamp',
+        ),
+    ]

lms/djangoapps/grades/models.py

@@ -45,9 +45,12 @@ class BlockRecordSet(frozenset):
         """
         if self._json is None:
             sorted_blocks = sorted(self, key=attrgetter('locator'))
+            list_of_block_dicts = [block._asdict() for block in sorted_blocks]
+            for block_dict in list_of_block_dicts:
+                block_dict['locator'] = unicode(block_dict['locator'])  # BlockUsageLocator is not json-serializable
             # Remove spaces from separators for more compact representation
             self._json = json.dumps(
-                [block._asdict() for block in sorted_blocks],
+                list_of_block_dicts,
                 separators=(',', ':'),
                 sort_keys=True,
             )
@@ -119,7 +122,7 @@ class VisibleBlocks(models.Model):
     A django model used to track the state of a set of visible blocks under a
     given subsection at the time they are used for grade calculation.
 
-    This state is represented using an array of serialized BlockRecords, stored
+    This state is represented using an array of BlockRecord, stored
     in the blocks_json field. A hash of this json array is used for lookup
     purposes.
     """
@@ -156,7 +159,7 @@ class PersistentSubsectionGradeQuerySet(models.QuerySet):
             user_id (int)
             usage_key (serialized UsageKey)
             course_version (str)
-            subtree_edited_date (datetime)
+            subtree_edited_timestamp (datetime)
             earned_all (float)
             possible_all (float)
             earned_graded (float)
@@ -164,10 +167,12 @@ class PersistentSubsectionGradeQuerySet(models.QuerySet):
             visible_blocks (iterable of BlockRecord)
         """
         visible_blocks = kwargs.pop('visible_blocks')
+        kwargs['course_version'] = kwargs.get('course_version', None) or ""
+        if not kwargs.get('course_id', None):
+            kwargs['course_id'] = kwargs['usage_key'].course_key
 
         visible_blocks_hash = VisibleBlocks.objects.hash_from_blockrecords(blocks=visible_blocks)
         grade = self.model(
-            course_id=kwargs['usage_key'].course_key,
             visible_blocks_id=visible_blocks_hash,
             **kwargs
         )
@@ -198,7 +203,7 @@ class PersistentSubsectionGrade(TimeStampedModel):
     usage_key = UsageKeyField(blank=False, max_length=255)
 
     # Information relating to the state of content when grade was calculated
-    subtree_edited_date = models.DateTimeField('last content edit timestamp', blank=False)
+    subtree_edited_timestamp = models.DateTimeField('last content edit timestamp', blank=False)
     course_version = models.CharField('guid of latest course version', blank=True, max_length=255)
 
     # earned/possible refers to the number of points achieved and available to achieve.
@@ -239,11 +244,17 @@ class PersistentSubsectionGrade(TimeStampedModel):
         user_id = kwargs.pop('user_id')
         usage_key = kwargs.pop('usage_key')
         try:
-            grade, is_created = cls.objects.get_or_create(user_id=user_id, usage_key=usage_key, defaults=kwargs)
+            grade, is_created = cls.objects.get_or_create(
+                user_id=user_id,
+                course_id=usage_key.course_key,
+                usage_key=usage_key,
+                defaults=kwargs,
+            )
         except IntegrityError:
-            is_created = False
-        if not is_created:
-            grade.update(**kwargs)
+            cls.update_grade(user_id=user_id, usage_key=usage_key, **kwargs)
+        else:
+            if not is_created:
+                grade.update(**kwargs)
 
     @classmethod
     def read_grade(cls, user_id, usage_key):
@@ -268,7 +279,7 @@ class PersistentSubsectionGrade(TimeStampedModel):
             user_id,
             usage_key,
             course_version,
-            subtree_edited_date,
+            subtree_edited_timestamp,
             earned_all,
             possible_all,
             earned_graded,
@@ -294,7 +305,7 @@ class PersistentSubsectionGrade(TimeStampedModel):
 
         grade.update(
             course_version=course_version,
-            subtree_edited_date=subtree_edited_date,
+            subtree_edited_timestamp=subtree_edited_timestamp,
             earned_all=earned_all,
             possible_all=possible_all,
             earned_graded=earned_graded,
@@ -305,7 +316,7 @@ class PersistentSubsectionGrade(TimeStampedModel):
     def update(
             self,
             course_version,
-            subtree_edited_date,
+            subtree_edited_timestamp,
             earned_all,
             possible_all,
             earned_graded,
@@ -318,8 +329,8 @@ class PersistentSubsectionGrade(TimeStampedModel):
         """
         visible_blocks_hash = VisibleBlocks.objects.hash_from_blockrecords(blocks=visible_blocks)
 
-        self.course_version = course_version
-        self.subtree_edited_date = subtree_edited_date
+        self.course_version = course_version or ""
+        self.subtree_edited_timestamp = subtree_edited_timestamp
         self.earned_all = earned_all
         self.possible_all = possible_all
         self.earned_graded = earned_graded

lms/djangoapps/grades/new/course_grade.py

@@ -42,15 +42,15 @@ class CourseGrade(object):
         return subsections_by_format
 
     @lazy
-    def locations_to_scores(self):
+    def locations_to_weighted_scores(self):
         """
         Returns a dict of problem scores keyed by their locations.
         """
-        locations_to_scores = {}
+        locations_to_weighted_scores = {}
         for chapter in self.chapter_grades:
             for subsection_grade in chapter['sections']:
-                locations_to_scores.update(subsection_grade.locations_to_scores)
-        return locations_to_scores
+                locations_to_weighted_scores.update(subsection_grade.locations_to_weighted_scores)
+        return locations_to_weighted_scores
 
     @property
     def has_access_to_course(self):
@@ -77,7 +77,7 @@ class CourseGrade(object):
         grade_summary['percent'] = round(grade_summary['percent'] * 100 + 0.05) / 100
         grade_summary['grade'] = self._compute_letter_grade(grade_summary['percent'])
         grade_summary['totaled_scores'] = self.subsection_grade_totals_by_format
-        grade_summary['raw_scores'] = list(self.locations_to_scores.itervalues())
+        grade_summary['raw_scores'] = list(self.locations_to_weighted_scores.itervalues())
 
         return grade_summary
 
@@ -115,8 +115,8 @@ class CourseGrade(object):
         composite module (a vertical or section ) the scores will be the sums of
         all scored problems that are children of the chosen location.
         """
-        if location in self.locations_to_scores:
-            score = self.locations_to_scores[location]
+        if location in self.locations_to_weighted_scores:
+            score = self.locations_to_weighted_scores[location]
             return score.earned, score.possible
         children = self.course_structure.get_children(location)
         earned = 0.0

lms/djangoapps/grades/new/subsection_grade.py

@@ -8,7 +8,8 @@ from django.conf import settings
 
 from courseware.model_data import ScoresClient
 from lms.djangoapps.grades.scores import get_score, possibly_scored
-from student.models import anonymous_id_for_user
+from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade
+from student.models import anonymous_id_for_user, User
 from submissions import api as submissions_api
 from xmodule import block_metadata_utils, graders
 from xmodule.graders import Score
@@ -29,14 +30,14 @@ class SubsectionGrade(object):
 
         self.graded_total = None  # aggregated grade for all graded problems
         self.all_total = None  # aggregated grade for all problems, regardless of whether they are graded
-        self.locations_to_scores = OrderedDict()  # dict of problem locations to their Score objects
+        self.locations_to_weighted_scores = OrderedDict()  # dict of problem locations to (Score, weight) tuples
 
     @lazy
     def scores(self):
         """
         List of all problem scores in the subsection.
         """
-        return list(self.locations_to_scores.itervalues())
+        return [score for score, _ in self.locations_to_weighted_scores.itervalues()]
 
     def compute(self, student, course_structure, scores_client, submissions_scores):
         """
@@ -46,34 +47,103 @@ class SubsectionGrade(object):
                 filter_func=possibly_scored,
                 start_node=self.location,
         ):
-            descendant = course_structure[descendant_key]
+            self._compute_block_score(student, descendant_key, course_structure, scores_client, submissions_scores)
 
-            if not getattr(descendant, 'has_score', False):
-                continue
+        self.all_total, self.graded_total = graders.aggregate_scores(self.scores, self.display_name, self.location)
 
+    def save(self, student, subsection, course):
+        """
+        Persist the SubsectionGrade.
+        """
+        visible_blocks = [
+            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,
+            course_version=getattr(course, 'course_version', None),
+            subtree_edited_timestamp=subsection.subtree_edited_on,
+            earned_all=self.all_total.earned,
+            possible_all=self.all_total.possible,
+            earned_graded=self.graded_total.earned,
+            possible_graded=self.graded_total.possible,
+            visible_blocks=visible_blocks,
+        )
+
+    def load_from_data(self, model, course_structure, scores_client, submissions_scores):
+        """
+        Load the subsection grade from the persisted model.
+        """
+        for block in model.visible_blocks.blocks:
+            persisted_values = {'weight': block.weight, 'possible': block.max_score}
+            self._compute_block_score(
+                User.objects.get(id=model.user_id),
+                block.locator,
+                course_structure,
+                scores_client,
+                submissions_scores,
+                persisted_values
+            )
+
+        self.graded_total = Score(
+            earned=model.earned_graded,
+            possible=model.possible_graded,
+            graded=True,
+            section=self.display_name,
+            module_id=self.location,
+        )
+        self.all_total = Score(
+            earned=model.earned_all,
+            possible=model.possible_all,
+            graded=False,
+            section=self.display_name,
+            module_id=self.location,
+        )
+
+    def _compute_block_score(
+            self,
+            student,
+            block_key,
+            course_structure,
+            scores_client,
+            submissions_scores,
+            persisted_values=None,
+    ):
+        """
+        Compute score for the given block. If persisted_values is provided, it will be used for possible and weight.
+        """
+        block = course_structure[block_key]
+
+        if getattr(block, 'has_score', False):
             (earned, possible) = get_score(
                 student,
-                descendant,
+                block,
                 scores_client,
                 submissions_scores,
             )
-            if earned is None and possible is None:
-                continue
-
-            # cannot grade a problem with a denominator of 0
-            descendant_graded = descendant.graded if possible > 0 else False
-
-            self.locations_to_scores[descendant.location] = Score(
-                earned,
-                possible,
-                descendant_graded,
-                block_metadata_utils.display_name_with_default_escaped(descendant),
-                descendant.location,
-            )
 
-        self.all_total, self.graded_total = graders.aggregate_scores(
-            self.scores, self.display_name,
-        )
+            # 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 = block.weight
+            if persisted_values:
+                possible = persisted_values.get('possible', possible)
+                weight = persisted_values.get('weight', weight)
+
+            if earned is not None or possible is not None:
+                # cannot grade a problem with a denominator of 0
+                block_graded = block.graded if possible > 0 else False
+
+                self.locations_to_weighted_scores[block.location] = (
+                    Score(
+                        earned,
+                        possible,
+                        block_graded,
+                        block_metadata_utils.display_name_with_default_escaped(block),
+                        block.location,
+                    ),
+                    weight,
+                )
 
 
 class SubsectionGradeFactory(object):
@@ -90,36 +160,43 @@ class SubsectionGradeFactory(object):
         """
         Returns the SubsectionGrade object for the student and subsection.
         """
+        self._prefetch_scores(course_structure, course)
         return (
-            self._get_saved_grade(subsection, course) or
-            self._compute_and_update_grade(subsection, course_structure, course)
+            self._get_saved_grade(subsection, course_structure, course) or
+            self._compute_and_save_grade(subsection, course_structure, course)
         )
 
-    def _compute_and_update_grade(self, 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.
         """
-        self._prefetch_scores(course_structure, course)
         subsection_grade = SubsectionGrade(subsection)
         subsection_grade.compute(self.student, course_structure, self._scores_client, self._submissions_scores)
-        self._update_saved_grade(subsection_grade, subsection, course)
+        self._save_grade(subsection_grade, subsection, course)
         return subsection_grade
 
-    def _get_saved_grade(self, subsection, course):  # pylint: disable=unused-argument
+    def _get_saved_grade(self, subsection, course_structure, course):  # pylint: disable=unused-argument
         """
-        Returns the saved grade for the given course and student.
+        Returns the saved grade for the student and subsection.
         """
         if settings.FEATURES.get('ENABLE_SUBSECTION_GRADES_SAVED') and course.enable_subsection_grades_saved:
-            # TODO Retrieve the saved grade for the subsection, if it exists.
-            pass
-
-    def _update_saved_grade(self, subsection_grade, subsection, course):  # pylint: disable=unused-argument
+            try:
+                model = PersistentSubsectionGrade.read_grade(
+                    user_id=self.student.id,
+                    usage_key=subsection.location,
+                )
+                subsection_grade = SubsectionGrade(subsection)
+                subsection_grade.load_from_data(model, course_structure, self._scores_client, self._submissions_scores)
+                return subsection_grade
+            except PersistentSubsectionGrade.DoesNotExist:
+                return None
+
+    def _save_grade(self, subsection_grade, subsection, course):  # pylint: disable=unused-argument
         """
-        Returns the saved grade for the given course and student.
+        Updates the saved grade for the student and subsection.
         """
         if settings.FEATURES.get('ENABLE_SUBSECTION_GRADES_SAVED') and course.enable_subsection_grades_saved:
-            # TODO Update the saved grade for the subsection.
-            _pretend_to_save_subsection_grades()
+            subsection_grade.save(self.student, subsection, course)
 
     def _prefetch_scores(self, course_structure, course):
         """
@@ -133,10 +210,3 @@ class SubsectionGradeFactory(object):
             self._submissions_scores = submissions_api.get_scores(
                 unicode(course.id), anonymous_id_for_user(self.student, course.id)
             )
-
-
-def _pretend_to_save_subsection_grades():
-    """
-    Stub to facilitate testing feature flag until robust grade work lands.
-    """
-    pass

lms/djangoapps/grades/scores.py

@@ -35,7 +35,7 @@ def weighted_score(raw_earned, raw_possible, weight):
     # If there is no weighting, or weighting can't be applied, return input.
     if weight is None or raw_possible == 0:
         return (raw_earned, raw_possible)
-    return (float(raw_earned) * weight / raw_possible, float(weight))
+    return float(raw_earned) * weight / raw_possible, float(weight)
 
 
 def get_score(user, block, scores_client, submissions_scores_cache):

lms/djangoapps/grades/tests/test_grades.py

@@ -208,7 +208,7 @@ class TestProgressSummary(TestCase):
         course_structure = MagicMock()
         course_structure.get_children = lambda location: locations_to_scored_children[location]
         self.course_grade = CourseGrade(student=None, course=None, course_structure=course_structure)
-        self.course_grade.locations_to_scores = weighted_scores
+        self.course_grade.locations_to_weighted_scores = weighted_scores
 
     def create_score(self, earned, possible):
         """
@@ -273,90 +273,6 @@ class TestProgressSummary(TestCase):
         self.assertEqual(possible, 0)
 
 
-@ddt.ddt
-class TestCourseGradeFactory(SharedModuleStoreTestCase):
-    """
-    Test that CourseGrades are calculated properly
-    """
-    @classmethod
-    def setUpClass(cls):
-        super(TestCourseGradeFactory, cls).setUpClass()
-        cls.course = CourseFactory.create()
-        cls.chapter = ItemFactory.create(
-            parent=cls.course,
-            category="chapter",
-            display_name="Test Chapter"
-        )
-        cls.sequence = ItemFactory.create(
-            parent=cls.chapter,
-            category='sequential',
-            display_name="Test Sequential 1",
-            graded=True
-        )
-        cls.vertical = ItemFactory.create(
-            parent=cls.sequence,
-            category='vertical',
-            display_name='Test Vertical 1'
-        )
-        problem_xml = MultipleChoiceResponseXMLFactory().build_xml(
-            question_text='The correct answer is Choice 3',
-            choices=[False, False, True, False],
-            choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3']
-        )
-        cls.problem = ItemFactory.create(
-            parent=cls.vertical,
-            category="problem",
-            display_name="Test Problem",
-            data=problem_xml
-        )
-
-    def setUp(self):
-        """
-        Set up test course
-        """
-        super(TestCourseGradeFactory, self).setUp()
-        self.request = get_request_for_user(UserFactory())
-        self.client.login(username=self.request.user.username, password="test")
-        CourseEnrollment.enroll(self.request.user, self.course.id)
-
-    @ddt.data(
-        (True, True),
-        (True, False),
-        (False, True),
-        (False, False),
-    )
-    @ddt.unpack
-    def test_course_grade_feature_gating(self, feature_flag, course_setting):
-        # Grades are only saved if the feature flag and the advanced setting are
-        # both set to True.
-        grade_factory = CourseGradeFactory(self.request.user)
-        with patch('lms.djangoapps.grades.new.course_grade._pretend_to_save_course_grades') as mock_save_grades:
-            with patch.dict(settings.FEATURES, {'ENABLE_SUBSECTION_GRADES_SAVED': feature_flag}):
-                with patch.object(self.course, 'enable_subsection_grades_saved', new=course_setting):
-                    grade_factory.create(self.course)
-        self.assertEqual(mock_save_grades.called, feature_flag and course_setting)
-
-    @ddt.data(
-        (True, True),
-        (True, False),
-        (False, True),
-        (False, False),
-    )
-    @ddt.unpack
-    def test_subsection_grade_feature_gating(self, feature_flag, course_setting):
-        # Grades are only saved if the feature flag and the advanced setting are
-        # both set to True.
-        grade_factory = SubsectionGradeFactory(self.request.user)
-        course_structure = get_course_blocks(self.request.user, self.course.location)
-        with patch(
-            'lms.djangoapps.grades.new.subsection_grade._pretend_to_save_subsection_grades'
-        ) as mock_save_grades:
-            with patch.dict(settings.FEATURES, {'ENABLE_SUBSECTION_GRADES_SAVED': feature_flag}):
-                with patch.object(self.course, 'enable_subsection_grades_saved', new=course_setting):
-                    grade_factory.create(self.sequence, course_structure, self.course)
-        self.assertEqual(mock_save_grades.called, feature_flag and course_setting)
-
-
 class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
     """
     Test get_module_score

lms/djangoapps/grades/tests/test_models.py

@@ -111,8 +111,9 @@ class VisibleBlocksTest(GradesModelTestCase):
 
     def test_blocks_property(self):
         """
-        Ensures that, given an array of BlockRecord, creating visible_blocks and accessing visible_blocks.blocks yields
-        a copy of the initial array. Also, trying to set the blocks property should raise an exception.
+        Ensures that, given an array of BlockRecord, creating visible_blocks and accessing
+        visible_blocks.blocks yields a copy of the initial array. Also, trying to set the blocks property should raise
+        an exception.
         """
         expected_blocks = [self.record_a, self.record_b]
         visible_blocks = VisibleBlocks.objects.create_from_blockrecords(expected_blocks)
@@ -137,7 +138,7 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
             "user_id": 12345,
             "usage_key": self.usage_key,
             "course_version": "deadbeef",
-            "subtree_edited_date": "2016-08-01 18:53:24.354741",
+            "subtree_edited_timestamp": "2016-08-01 18:53:24.354741",
             "earned_all": 6,
             "possible_all": 12,
             "earned_graded": 6,

lms/djangoapps/grades/tests/test_new.py

+"""
+Test saved subsection grade functionality.
+"""
+
+import ddt
+from django.conf import settings
+from mock import patch
+
+from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
+from courseware.tests.helpers import get_request_for_user
+from lms.djangoapps.course_blocks.api import get_course_blocks
+from student.models import CourseEnrollment
+from student.tests.factories import UserFactory
+from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+
+from ..models import PersistentSubsectionGrade
+from ..new.course_grade import CourseGradeFactory
+from ..new.subsection_grade import SubsectionGrade, SubsectionGradeFactory
+
+
+class GradeTestBase(SharedModuleStoreTestCase):
+    """
+    Base class for Course- and SubsectionGradeFactory tests.
+    """
+    @classmethod
+    def setUpClass(cls):
+        super(GradeTestBase, cls).setUpClass()
+        cls.course = CourseFactory.create()
+        cls.chapter = ItemFactory.create(
+            parent=cls.course,
+            category="chapter",
+            display_name="Test Chapter"
+        )
+        cls.sequence = ItemFactory.create(
+            parent=cls.chapter,
+            category='sequential',
+            display_name="Test Sequential 1",
+            graded=True
+        )
+        cls.vertical = ItemFactory.create(
+            parent=cls.sequence,
+            category='vertical',
+            display_name='Test Vertical 1'
+        )
+        problem_xml = MultipleChoiceResponseXMLFactory().build_xml(
+            question_text='The correct answer is Choice 3',
+            choices=[False, False, True, False],
+            choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3']
+        )
+        cls.problem = ItemFactory.create(
+            parent=cls.vertical,
+            category="problem",
+            display_name="Test Problem",
+            data=problem_xml
+        )
+
+    def setUp(self):
+        super(GradeTestBase, self).setUp()
+        self.request = get_request_for_user(UserFactory())
+        self.client.login(username=self.request.user.username, password="test")
+        self.subsection_grade_factory = SubsectionGradeFactory(self.request.user)
+        self.course_structure = get_course_blocks(self.request.user, self.course.location)
+        self.course.enable_subsection_grades_saved = True
+        CourseEnrollment.enroll(self.request.user, self.course.id)
+
+
+@ddt.ddt
+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),
+        (False, True),
+        (False, False),
+    )
+    @ddt.unpack
+    def test_course_grade_feature_gating(self, feature_flag, course_setting):
+        # Grades are only saved if the feature flag and the advanced setting are
+        # both set to True.
+        grade_factory = CourseGradeFactory(self.request.user)
+        with patch('lms.djangoapps.grades.new.course_grade._pretend_to_save_course_grades') as mock_save_grades:
+            with patch.dict(settings.FEATURES, {'ENABLE_SUBSECTION_GRADES_SAVED': feature_flag}):
+                with patch.object(self.course, 'enable_subsection_grades_saved', new=course_setting):
+                    grade_factory.create(self.course)
+        self.assertEqual(mock_save_grades.called, feature_flag and course_setting)
+
+
+@ddt.ddt
+class SubsectionGradeFactoryTest(GradeTestBase):
+    """
+    Tests for SubsectionGradeFactory functionality.
+
+    Ensures that SubsectionGrades are created and updated properly, that
+    persistent grades are functioning as expected, and that the flag to
+    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.
+        """
+        with patch(
+            'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._save_grade',
+            wraps=self.subsection_grade_factory._save_grade  # pylint: disable=protected-access
+        ) as mock_save_grades:
+            with patch(
+                'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._get_saved_grade',
+                wraps=self.subsection_grade_factory._get_saved_grade  # pylint: disable=protected-access
+            ) as mock_get_saved_grade:
+                with self.assertNumQueries(17):
+                    grade_a = self.subsection_grade_factory.create(self.sequence, self.course_structure, self.course)
+                self.assertTrue(mock_get_saved_grade.called)
+                self.assertTrue(mock_save_grades.called)
+
+                mock_get_saved_grade.reset_mock()
+                mock_save_grades.reset_mock()
+
+                with self.assertNumQueries(3):
+                    grade_b = self.subsection_grade_factory.create(self.sequence, self.course_structure, self.course)
+                self.assertTrue(mock_get_saved_grade.called)
+                self.assertFalse(mock_save_grades.called)
+
+        self.assertEqual(grade_a.url_name, grade_b.url_name)
+        self.assertEqual(grade_a.all_total, grade_b.all_total)
+
+    @ddt.data(
+        (True, True),
+        (True, False),
+        (False, True),
+        (False, False),
+    )
+    @ddt.unpack
+    def test_subsection_grade_feature_gating(self, feature_flag, course_setting):
+        # Grades are only saved if the feature flag and the advanced setting are
+        # both set to True.
+        with patch(
+            'lms.djangoapps.grades.models.PersistentSubsectionGrade.read_grade'
+        ) as mock_read_saved_grade:
+            with patch.dict(settings.FEATURES, {'ENABLE_SUBSECTION_GRADES_SAVED': feature_flag}):
+                with patch.object(self.course, 'enable_subsection_grades_saved', new=course_setting):
+                    self.subsection_grade_factory.create(self.sequence, self.course_structure, self.course)
+        self.assertEqual(mock_read_saved_grade.called, feature_flag and course_setting)
+
+
+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.
+        """
+        grade = self.subsection_grade_factory.create(self.sequence, self.course_structure, self.course)
+        with patch('lms.djangoapps.grades.new.subsection_grade.get_score', return_value=(0, 1)):
+            # The final 2 parameters are only passed through to our mocked-out get_score method
+            grade.compute(self.request.user, self.course_structure, None, None)
+        self.assertEqual(grade.all_total.earned, 0)
+        self.assertEqual(grade.all_total.possible, 1)
+
+    def test_save_and_load(self):
+        """
+        Test that grades are persisted to the database properly, and that loading saved grades returns the same data.
+        """
+        # Create a grade that *isn't* saved to the database
+        self.subsection_grade_factory._prefetch_scores(self.course_structure, self.course)  # pylint: disable=protected-access
+        input_grade = SubsectionGrade(self.sequence)
+        input_grade.compute(
+            self.request.user,
+            self.course_structure,
+            self.subsection_grade_factory._scores_client,  # pylint: disable=protected-access
+            self.subsection_grade_factory._submissions_scores,  # pylint: disable=protected-access
+        )
+        self.assertEqual(PersistentSubsectionGrade.objects.count(), 0)
+
+        # save to db, and verify object is in database
+        input_grade.save(self.request.user, self.sequence, self.course)
+        self.assertEqual(PersistentSubsectionGrade.objects.count(), 1)
+
+        # load from db, and ensure output matches input
+        loaded_grade = SubsectionGrade(self.sequence)
+        saved_model = PersistentSubsectionGrade.read_grade(
+            user_id=self.request.user.id,
+            usage_key=self.sequence.location,
+        )
+        loaded_grade.load_from_data(
+            saved_model,
+            self.course_structure,
+            self.subsection_grade_factory._scores_client,  # pylint: disable=protected-access
+            self.subsection_grade_factory._submissions_scores,  # pylint: disable=protected-access
+        )
+
+        self.assertEqual(input_grade.url_name, loaded_grade.url_name)
+        self.assertEqual(input_grade.all_total, loaded_grade.all_total)

lms/djangoapps/instructor_analytics/basic.py

@@ -23,7 +23,7 @@ from courseware.models import StudentModule
 from certificates.models import GeneratedCertificate
 from django.db.models import Count
 from certificates.models import CertificateStatuses
-from grades.context import grading_context_for_course
+from lms.djangoapps.grades.context import grading_context_for_course
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 
 

lms/djangoapps/instructor_task/tasks_helper.py

@@ -46,7 +46,7 @@ from certificates.models import (
 )
 from certificates.api import generate_user_certificates
 from courseware.courses import get_course_by_id, get_problems_in_section
-from grades.course_grades import iterate_grades_for
+from lms.djangoapps.grades.course_grades import iterate_grades_for
 from courseware.models import StudentModule
 from courseware.model_data import DjangoKeyValueStore, FieldDataCache
 from courseware.module_render import get_module_for_descriptor_internal
@@ -948,7 +948,7 @@ def upload_problem_grade_report(_xmodule_instance_args, _entry_id, course_id, _t
 
         final_grade = gradeset['percent']
         # Only consider graded problems
-        problem_scores = {unicode(score.module_id): score for score in gradeset['raw_scores'] if score.graded}
+        problem_scores = {unicode(score.module_id): score for score, _ in gradeset['raw_scores'] if score.graded}
         earned_possible_values = list()
         for problem_id in problems:
             try:

lms/envs/bok_choy.py

@@ -124,6 +124,9 @@ FEATURES['ENABLE_TEAMS'] = True
 # Enable custom content licensing
 FEATURES['LICENSING'] = True
 
+# Enable persistent subsection grades, so that feature can be tested.
+FEATURES['ENABLE_SUBSECTION_GRADES_SAVED'] = True
+
 # Use the auto_auth workflow for creating users and logging them in
 FEATURES['AUTOMATIC_AUTH_FOR_TESTING'] = True
 

lms/envs/test.py

@@ -78,6 +78,9 @@ FEATURES['ENABLE_COMBINED_LOGIN_REGISTRATION'] = True
 # Enable the milestones app in tests to be consistent with it being enabled in production
 FEATURES['MILESTONES_APP'] = True
 
+# Enable persistent subsection grades, so that feature can be tested.
+FEATURES['ENABLE_SUBSECTION_GRADES_SAVED'] = True
+
 # Need wiki for courseware views to work. TODO (vshnayder): shouldn't need it.
 WIKI_ENABLED = True