Merge pull request #13547 from edx/beryl/correct_weight

Fix Problem grades that have weight values

common/lib/xmodule/xmodule/graders.py

@@ -10,13 +10,67 @@ import logging
 import random
 import sys
 
-from collections import namedtuple
 
 log = logging.getLogger("edx.courseware")
 
-# This is a tuple for holding scores, either from problems or sections.
-# Section either indicates the name of the problem or the name of the section
-Score = namedtuple("Score", "earned possible graded section module_id")
+
+class ScoreBase(object):
+    """
+    Abstract base class for encapsulating fields of values scores.
+    Field common to all scores include:
+        display_name (string) - the display name of the module
+        module_id (UsageKey) - the location of the module
+        graded (boolean) - whether or not this module is graded
+    """
+    __metaclass__ = abc.ABCMeta
+
+    def __init__(self, graded, display_name, module_id):
+        self.graded = graded
+        self.display_name = display_name
+        self.module_id = module_id
+
+    def __eq__(self, other):
+        if type(other) is type(self):
+            return self.__dict__ == other.__dict__
+        return False
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __repr__(self):
+        return u"{class_name}({fields})".format(class_name=self.__class__.__name__, fields=self.__dict__)
+
+
+class ProblemScore(ScoreBase):
+    """
+    Encapsulates the fields of a Problem's score.
+    In addition to the fields in ScoreBase, also includes:
+        raw_earned (float) - raw points earned on this problem
+        raw_possible (float) - raw points possible to earn on this problem
+        weighted_earned = earned (float) - weighted value of the points earned
+        weighted_possible = possible (float) - weighted possible points on this problem
+        weight (float) - weight of this problem
+    """
+    def __init__(self, raw_earned, raw_possible, weighted_earned, weighted_possible, weight, *args, **kwargs):
+        super(ProblemScore, self).__init__(*args, **kwargs)
+        self.raw_earned = raw_earned
+        self.raw_possible = raw_possible
+        self.earned = weighted_earned
+        self.possible = weighted_possible
+        self.weight = weight
+
+
+class AggregatedScore(ScoreBase):
+    """
+    Encapsulates the fields of a Subsection's score.
+    In addition to the fields in ScoreBase, also includes:
+        tw_earned = earned - total aggregated sum of all weighted earned values
+        tw_possible = possible - total aggregated sum of all weighted possible values
+    """
+    def __init__(self, tw_earned, tw_possible, *args, **kwargs):
+        super(AggregatedScore, self).__init__(*args, **kwargs)
+        self.earned = tw_earned
+        self.possible = tw_possible
 
 
 def float_sum(iterable):
@@ -26,13 +80,14 @@ def float_sum(iterable):
     return float(sum(iterable))
 
 
-def aggregate_scores(scores, section_name="summary", location=None):
+def aggregate_scores(scores, display_name="summary", location=None):
     """
-    scores: A list of Score objects
+    scores: A list of ScoreBase objects
+    display_name: The display name for the score object
     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
+        all_total: A ScoreBase representing the total score summed over all input scores
+        graded_total: A ScoreBase representing the score summed over all graded input scores
     """
     total_correct_graded = float_sum(score.earned for score in scores if score.graded)
     total_possible_graded = float_sum(score.possible for score in scores if score.graded)
@@ -41,10 +96,10 @@ def aggregate_scores(scores, section_name="summary", location=None):
     total_possible = float_sum(score.possible for score in scores)
 
     #regardless of whether it is graded
-    all_total = Score(total_correct, total_possible, False, section_name, location)
+    all_total = AggregatedScore(total_correct, total_possible, False, display_name, location)
 
     #selecting only graded things
-    graded_total = Score(total_correct_graded, total_possible_graded, True, section_name, location)
+    graded_total = AggregatedScore(total_correct_graded, total_possible_graded, True, display_name, location)
 
     return all_total, graded_total
 
@@ -220,7 +275,7 @@ class SingleSectionGrader(CourseGrader):
         found_score = None
         if self.type in grade_sheet:
             for score in grade_sheet[self.type]:
-                if score.section == self.name:
+                if score.display_name == self.name:
                     found_score = score
                     break
 
@@ -342,7 +397,7 @@ class AssignmentFormatGrader(CourseGrader):
                 else:
                     earned = scores[i].earned
                     possible = scores[i].possible
-                    section_name = scores[i].section
+                    section_name = scores[i].display_name
 
                 percentage = earned / possible
                 summary_format = u"{section_type} {index} - {name} - {percent:.0%} ({earned:.3n}/{possible:.3n})"

common/lib/xmodule/xmodule/tests/test_graders.py

@@ -2,42 +2,67 @@
 import unittest
 
 from xmodule import graders
-from xmodule.graders import Score, aggregate_scores
+from xmodule.graders import ProblemScore, AggregatedScore, aggregate_scores
 
 
 class GradesheetTest(unittest.TestCase):
-    '''Tests the aggregate_scores method'''
+    """
+    Tests the aggregate_scores method
+    """
 
     def test_weighted_grading(self):
         scores = []
-        Score.__sub__ = lambda me, other: (me.earned - other.earned) + (me.possible - other.possible)
+        agg_fields = dict(display_name="aggregated_score", module_id=None)
+        prob_fields = dict(display_name="problem_score", module_id=None, raw_earned=0, raw_possible=0, weight=0)
 
-        all_total, graded_total = aggregate_scores(scores)
-        self.assertEqual(all_total, Score(earned=0, possible=0, graded=False, section="summary", module_id=None))
-        self.assertEqual(graded_total, Score(earned=0, possible=0, graded=True, section="summary", module_id=None))
+        all_total, graded_total = aggregate_scores(scores, display_name=agg_fields['display_name'])
+        self.assertEqual(
+            all_total,
+            AggregatedScore(tw_earned=0, tw_possible=0, graded=False, **agg_fields),
+        )
+        self.assertEqual(
+            graded_total,
+            AggregatedScore(tw_earned=0, tw_possible=0, graded=True, **agg_fields),
+        )
 
-        scores.append(Score(earned=0, possible=5, graded=False, section="summary", module_id=None))
-        all_total, graded_total = aggregate_scores(scores)
-        self.assertEqual(all_total, Score(earned=0, possible=5, graded=False, section="summary", module_id=None))
-        self.assertEqual(graded_total, Score(earned=0, possible=0, graded=True, section="summary", module_id=None))
+        scores.append(ProblemScore(weighted_earned=0, weighted_possible=5, graded=False, **prob_fields))
+        all_total, graded_total = aggregate_scores(scores, display_name=agg_fields['display_name'])
+        self.assertEqual(
+            all_total,
+            AggregatedScore(tw_earned=0, tw_possible=5, graded=False, **agg_fields),
+        )
+        self.assertEqual(
+            graded_total,
+            AggregatedScore(tw_earned=0, tw_possible=0, graded=True, **agg_fields),
+        )
 
-        scores.append(Score(earned=3, possible=5, graded=True, section="summary", module_id=None))
-        all_total, graded_total = aggregate_scores(scores)
-        self.assertAlmostEqual(all_total, Score(earned=3, possible=10, graded=False, section="summary", module_id=None))
+        scores.append(ProblemScore(weighted_earned=3, weighted_possible=5, graded=True, **prob_fields))
+        all_total, graded_total = aggregate_scores(scores, display_name=agg_fields['display_name'])
         self.assertAlmostEqual(
-            graded_total, Score(earned=3, possible=5, graded=True, section="summary", module_id=None)
+            all_total,
+            AggregatedScore(tw_earned=3, tw_possible=10, graded=False, **agg_fields),
+        )
+        self.assertAlmostEqual(
+            graded_total,
+            AggregatedScore(tw_earned=3, tw_possible=5, graded=True, **agg_fields),
         )
 
-        scores.append(Score(earned=2, possible=5, graded=True, section="summary", module_id=None))
-        all_total, graded_total = aggregate_scores(scores)
-        self.assertAlmostEqual(all_total, Score(earned=5, possible=15, graded=False, section="summary", module_id=None))
+        scores.append(ProblemScore(weighted_earned=2, weighted_possible=5, graded=True, **prob_fields))
+        all_total, graded_total = aggregate_scores(scores, display_name=agg_fields['display_name'])
+        self.assertAlmostEqual(
+            all_total,
+            AggregatedScore(tw_earned=5, tw_possible=15, graded=False, **agg_fields),
+        )
         self.assertAlmostEqual(
-            graded_total, Score(earned=5, possible=10, graded=True, section="summary", module_id=None)
+            graded_total,
+            AggregatedScore(tw_earned=5, tw_possible=10, graded=True, **agg_fields),
         )
 
 
 class GraderTest(unittest.TestCase):
-    '''Tests grader implementations'''
+    """
+    Tests grader implementations
+    """
 
     empty_gradesheet = {
     }
@@ -49,19 +74,25 @@ class GraderTest(unittest.TestCase):
     }
 
     test_gradesheet = {
-        'Homework': [Score(earned=2, possible=20.0, graded=True, section='hw1', module_id=None),
-                     Score(earned=16, possible=16.0, graded=True, section='hw2', module_id=None)],
-        # The dropped scores should be from the assignments that don't exist yet
-
-        'Lab': [Score(earned=1, possible=2.0, graded=True, section='lab1', module_id=None),  # Dropped
-                Score(earned=1, possible=1.0, graded=True, section='lab2', module_id=None),
-                Score(earned=1, possible=1.0, graded=True, section='lab3', module_id=None),
-                Score(earned=5, possible=25.0, graded=True, section='lab4', module_id=None),  # Dropped
-                Score(earned=3, possible=4.0, graded=True, section='lab5', module_id=None),  # Dropped
-                Score(earned=6, possible=7.0, graded=True, section='lab6', module_id=None),
-                Score(earned=5, possible=6.0, graded=True, section='lab7', module_id=None)],
+        'Homework': [
+            AggregatedScore(tw_earned=2, tw_possible=20.0, graded=True, display_name='hw1', module_id=None),
+            AggregatedScore(tw_earned=16, tw_possible=16.0, graded=True, display_name='hw2', module_id=None)
+        ],
 
-        'Midterm': [Score(earned=50.5, possible=100, graded=True, section="Midterm Exam", module_id=None), ],
+        # The dropped scores should be from the assignments that don't exist yet
+        'Lab': [
+            AggregatedScore(tw_earned=1, tw_possible=2.0, graded=True, display_name='lab1', module_id=None),  # Dropped
+            AggregatedScore(tw_earned=1, tw_possible=1.0, graded=True, display_name='lab2', module_id=None),
+            AggregatedScore(tw_earned=1, tw_possible=1.0, graded=True, display_name='lab3', module_id=None),
+            AggregatedScore(tw_earned=5, tw_possible=25.0, graded=True, display_name='lab4', module_id=None),  # Dropped
+            AggregatedScore(tw_earned=3, tw_possible=4.0, graded=True, display_name='lab5', module_id=None),  # Dropped
+            AggregatedScore(tw_earned=6, tw_possible=7.0, graded=True, display_name='lab6', module_id=None),
+            AggregatedScore(tw_earned=5, tw_possible=6.0, graded=True, display_name='lab7', module_id=None),
+        ],
+
+        'Midterm': [
+            AggregatedScore(tw_earned=50.5, tw_possible=100, graded=True, display_name="Midterm Exam", module_id=None),
+        ],
     }
 
     def test_single_section_grader(self):
@@ -69,9 +100,11 @@ class GraderTest(unittest.TestCase):
         lab4_grader = graders.SingleSectionGrader("Lab", "lab4")
         bad_lab_grader = graders.SingleSectionGrader("Lab", "lab42")
 
-        for graded in [midterm_grader.grade(self.empty_gradesheet),
-                       midterm_grader.grade(self.incomplete_gradesheet),
-                       bad_lab_grader.grade(self.test_gradesheet)]:
+        for graded in [
+                midterm_grader.grade(self.empty_gradesheet),
+                midterm_grader.grade(self.incomplete_gradesheet),
+                bad_lab_grader.grade(self.test_gradesheet),
+        ]:
             self.assertEqual(len(graded['section_breakdown']), 1)
             self.assertEqual(graded['percent'], 0.0)
 
@@ -91,10 +124,12 @@ class GraderTest(unittest.TestCase):
         lab_grader = graders.AssignmentFormatGrader("Lab", 7, 3)
 
         # Test the grading of an empty gradesheet
-        for graded in [homework_grader.grade(self.empty_gradesheet),
-                       no_drop_grader.grade(self.empty_gradesheet),
-                       homework_grader.grade(self.incomplete_gradesheet),
-                       no_drop_grader.grade(self.incomplete_gradesheet)]:
+        for graded in [
+                homework_grader.grade(self.empty_gradesheet),
+                no_drop_grader.grade(self.empty_gradesheet),
+                homework_grader.grade(self.incomplete_gradesheet),
+                no_drop_grader.grade(self.incomplete_gradesheet),
+        ]:
             self.assertAlmostEqual(graded['percent'], 0.0)
             # Make sure the breakdown includes 12 sections, plus one summary
             self.assertEqual(len(graded['section_breakdown']), 12 + 1)
@@ -118,8 +153,10 @@ class GraderTest(unittest.TestCase):
     def test_assignment_format_grader_on_single_section_entry(self):
         midterm_grader = graders.AssignmentFormatGrader("Midterm", 1, 0)
         # Test the grading on a section with one item:
-        for graded in [midterm_grader.grade(self.empty_gradesheet),
-                       midterm_grader.grade(self.incomplete_gradesheet)]:
+        for graded in [
+                midterm_grader.grade(self.empty_gradesheet),
+                midterm_grader.grade(self.incomplete_gradesheet),
+        ]:
             self.assertAlmostEqual(graded['percent'], 0.0)
             # Make sure the breakdown includes just the one summary
             self.assertEqual(len(graded['section_breakdown']), 0 + 1)
@@ -137,23 +174,31 @@ class GraderTest(unittest.TestCase):
         # will act like SingleSectionGraders on single sections.
         midterm_grader = graders.AssignmentFormatGrader("Midterm", 1, 0)
 
-        weighted_grader = graders.WeightedSubsectionsGrader([(homework_grader, homework_grader.category, 0.25),
-                                                             (lab_grader, lab_grader.category, 0.25),
-                                                             (midterm_grader, midterm_grader.category, 0.5)])
+        weighted_grader = graders.WeightedSubsectionsGrader([
+            (homework_grader, homework_grader.category, 0.25),
+            (lab_grader, lab_grader.category, 0.25),
+            (midterm_grader, midterm_grader.category, 0.5),
+        ])
 
-        over_one_weights_grader = graders.WeightedSubsectionsGrader([(homework_grader, homework_grader.category, 0.5),
-                                                                     (lab_grader, lab_grader.category, 0.5),
-                                                                     (midterm_grader, midterm_grader.category, 0.5)])
+        over_one_weights_grader = graders.WeightedSubsectionsGrader([
+            (homework_grader, homework_grader.category, 0.5),
+            (lab_grader, lab_grader.category, 0.5),
+            (midterm_grader, midterm_grader.category, 0.5),
+        ])
 
         # The midterm should have all weight on this one
-        zero_weights_grader = graders.WeightedSubsectionsGrader([(homework_grader, homework_grader.category, 0.0),
-                                                                 (lab_grader, lab_grader.category, 0.0),
-                                                                 (midterm_grader, midterm_grader.category, 0.5)])
+        zero_weights_grader = graders.WeightedSubsectionsGrader([
+            (homework_grader, homework_grader.category, 0.0),
+            (lab_grader, lab_grader.category, 0.0),
+            (midterm_grader, midterm_grader.category, 0.5),
+        ])
 
         # This should always have a final percent of zero
-        all_zero_weights_grader = graders.WeightedSubsectionsGrader([(homework_grader, homework_grader.category, 0.0),
-                                                                     (lab_grader, lab_grader.category, 0.0),
-                                                                     (midterm_grader, midterm_grader.category, 0.0)])
+        all_zero_weights_grader = graders.WeightedSubsectionsGrader([
+            (homework_grader, homework_grader.category, 0.0),
+            (lab_grader, lab_grader.category, 0.0),
+            (midterm_grader, midterm_grader.category, 0.0),
+        ])
 
         empty_grader = graders.WeightedSubsectionsGrader([])
 
@@ -177,10 +222,12 @@ class GraderTest(unittest.TestCase):
         self.assertEqual(len(graded['section_breakdown']), (12 + 1) + (7 + 1) + 1)
         self.assertEqual(len(graded['grade_breakdown']), 3)
 
-        for graded in [weighted_grader.grade(self.empty_gradesheet),
-                       weighted_grader.grade(self.incomplete_gradesheet),
-                       zero_weights_grader.grade(self.empty_gradesheet),
-                       all_zero_weights_grader.grade(self.empty_gradesheet)]:
+        for graded in [
+                weighted_grader.grade(self.empty_gradesheet),
+                weighted_grader.grade(self.incomplete_gradesheet),
+                zero_weights_grader.grade(self.empty_gradesheet),
+                all_zero_weights_grader.grade(self.empty_gradesheet),
+        ]:
             self.assertAlmostEqual(graded['percent'], 0.0)
             self.assertEqual(len(graded['section_breakdown']), (12 + 1) + (7 + 1) + 1)
             self.assertEqual(len(graded['grade_breakdown']), 3)

lms/djangoapps/gating/tests/test_api.py

@@ -166,8 +166,8 @@ class TestGatedContent(GatingTestCase, MilestonesTestCaseMixin):
         """
         course_grade = CourseGradeFactory(user).create(self.course)
         for prob in [self.gating_prob1, self.gated_prob2, self.prob3]:
-            self.assertIn(prob.location, course_grade.locations_to_weighted_scores)
-        self.assertNotIn(self.orphan.location, course_grade.locations_to_weighted_scores)
+            self.assertIn(prob.location, course_grade.locations_to_scores)
+        self.assertNotIn(self.orphan.location, course_grade.locations_to_scores)
 
         self.assertEquals(course_grade.percent, expected_percent)
 

lms/djangoapps/grades/models.py

@@ -27,7 +27,7 @@ BLOCK_RECORD_LIST_VERSION = 1
 
 # Used to serialize information about a block at the time it was used in
 # grade calculation.
-BlockRecord = namedtuple('BlockRecord', ['locator', 'weight', 'max_score'])
+BlockRecord = namedtuple('BlockRecord', ['locator', 'weight', 'raw_possible', 'graded'])
 
 
 class BlockRecordList(tuple):
@@ -98,7 +98,8 @@ class BlockRecordList(tuple):
             BlockRecord(
                 locator=UsageKey.from_string(block["locator"]).replace(course_key=course_key),
                 weight=block["weight"],
-                max_score=block["max_score"],
+                raw_possible=block["raw_possible"],
+                graded=block["graded"],
             )
             for block in block_dicts
         )

lms/djangoapps/grades/new/course_grade.py

@@ -43,15 +43,15 @@ class CourseGrade(object):
         return subsections_by_format
 
     @lazy
-    def locations_to_weighted_scores(self):
+    def locations_to_scores(self):
         """
         Returns a dict of problem scores keyed by their locations.
         """
-        locations_to_weighted_scores = {}
+        locations_to_scores = {}
         for chapter in self.chapter_grades:
             for subsection_grade in chapter['sections']:
-                locations_to_weighted_scores.update(subsection_grade.locations_to_weighted_scores)
-        return locations_to_weighted_scores
+                locations_to_scores.update(subsection_grade.locations_to_scores)
+        return locations_to_scores
 
     @lazy
     def grade_value(self):
@@ -113,7 +113,7 @@ class CourseGrade(object):
         grade_summary['percent'] = self.percent
         grade_summary['grade'] = self.letter_grade
         grade_summary['totaled_scores'] = self.subsection_grade_totals_by_format
-        grade_summary['raw_scores'] = list(self.locations_to_weighted_scores.itervalues())
+        grade_summary['raw_scores'] = list(self.locations_to_scores.itervalues())
 
         return grade_summary
 
@@ -141,7 +141,7 @@ class CourseGrade(object):
         subsections_total = sum(len(x) for x in self.subsection_grade_totals_by_format.itervalues())
         subsections_read = len(subsection_grade_factory._unsaved_subsection_grades)  # pylint: disable=protected-access
         subsections_created = subsections_total - subsections_read
-        blocks_total = len(self.locations_to_weighted_scores)
+        blocks_total = len(self.locations_to_scores)
         if not read_only:
             subsection_grade_factory.bulk_create_unsaved()
 
@@ -166,8 +166,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_weighted_scores:
-            score, _ = self.locations_to_weighted_scores[location]
+        if location in self.locations_to_scores:
+            score = self.locations_to_scores[location]
             return score.earned, score.possible
         children = self.course_structure.get_children(location)
         earned = 0.0

lms/djangoapps/grades/new/subsection_grade.py

@@ -11,12 +11,11 @@ from courseware.model_data import ScoresClient
 from lms.djangoapps.grades.scores import get_score, possibly_scored
 from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade
 from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag
-from lms.djangoapps.grades.transformer import GradesTransformer
 from student.models import anonymous_id_for_user, User
 from submissions import api as submissions_api
 from traceback import format_exc
 from xmodule import block_metadata_utils, graders
-from xmodule.graders import Score
+from xmodule.graders import AggregatedScore
 
 
 log = getLogger(__name__)
@@ -54,62 +53,47 @@ 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_weighted_scores = OrderedDict()  # dict of problem locations to (Score, weight) tuples
-
-        self._scores = None
+        self.locations_to_scores = OrderedDict()  # dict of problem locations to ProblemScore
 
     @property
     def scores(self):
         """
         List of all problem scores in the subsection.
         """
-        if self._scores is None:
-            self._scores = [score for score, _ in self.locations_to_weighted_scores.itervalues()]
-        return self._scores
+        return self.locations_to_scores.values()
 
-    def init_from_structure(self, student, course_structure, scores_client, submissions_scores):
+    def init_from_structure(self, student, course_structure, submissions_scores, csm_scores):
         """
         Compute the grade of this subsection for the given student and course.
         """
-        assert self._scores is None
         for descendant_key in course_structure.post_order_traversal(
                 filter_func=possibly_scored,
                 start_node=self.location,
         ):
-            self._compute_block_score(
-                student, descendant_key, course_structure, scores_client, submissions_scores, persisted_values={},
-            )
+            self._compute_block_score(descendant_key, course_structure, submissions_scores, csm_scores)
+
         self.all_total, self.graded_total = graders.aggregate_scores(self.scores, self.display_name, self.location)
         self._log_event(log.info, u"init_from_structure", student)
 
-    def init_from_model(self, student, model, course_structure, scores_client, submissions_scores):
+    def init_from_model(self, student, model, course_structure, submissions_scores, csm_scores):
         """
         Load the subsection grade from the persisted model.
         """
-        assert self._scores is None
         for block in model.visible_blocks.blocks:
-            persisted_values = {'weight': block.weight, 'possible': block.max_score}
-            self._compute_block_score(
-                student,
-                block.locator,
-                course_structure,
-                scores_client,
-                submissions_scores,
-                persisted_values
-            )
+            self._compute_block_score(block.locator, course_structure, submissions_scores, csm_scores, block)
 
-        self.graded_total = Score(
-            earned=model.earned_graded,
-            possible=model.possible_graded,
+        self.graded_total = AggregatedScore(
+            tw_earned=model.earned_graded,
+            tw_possible=model.possible_graded,
             graded=True,
-            section=self.display_name,
+            display_name=self.display_name,
             module_id=self.location,
         )
-        self.all_total = Score(
-            earned=model.earned_all,
-            possible=model.possible_all,
+        self.all_total = AggregatedScore(
+            tw_earned=model.earned_all,
+            tw_possible=model.possible_all,
             graded=False,
-            section=self.display_name,
+            display_name=self.display_name,
             module_id=self.location,
         )
         self._log_event(log.info, u"init_from_model", student)
@@ -140,12 +124,11 @@ class SubsectionGrade(object):
 
     def _compute_block_score(
             self,
-            student,
             block_key,
             course_structure,
-            scores_client,
             submissions_scores,
-            persisted_values,
+            csm_scores,
+            persisted_block=None,
     ):
         """
         Compute score for the given block. If persisted_values
@@ -154,54 +137,14 @@ class SubsectionGrade(object):
         block = course_structure[block_key]
 
         if getattr(block, 'has_score', False):
-
-            possible = persisted_values.get('possible', None)
-            weight = persisted_values.get('weight', getattr(block, 'weight', None))
-
-            (earned, possible) = get_score(
-                student,
-                block,
-                scores_client,
+            problem_score = get_score(
                 submissions_scores,
-                weight,
-                possible,
+                csm_scores,
+                persisted_block,
+                block,
             )
-
-            if earned is not None or possible is not None:
-                # There's a chance that the value of graded is not the same
-                # value when the problem was scored. Since we get the value
-                # from the block_structure.
-                #
-                # Cannot grade a problem with a denominator of 0.
-                # TODO: None > 0 is not python 3 compatible.
-                block_graded = self._get_explicit_graded(block, course_structure) 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,
-                )
-
-    def _get_explicit_graded(self, block, course_structure):
-        """
-        Returns the explicit graded field value for the given block
-        """
-        field_value = course_structure.get_transformer_block_field(
-            block.location,
-            GradesTransformer,
-            GradesTransformer.EXPLICIT_GRADED_FIELD_NAME
-        )
-
-        # Set to True if grading is not explicitly disabled for
-        # this block.  This allows us to include the block's score
-        # in the aggregated self.graded_total, regardless of the
-        # inherited graded value from the subsection. (TNL-5560)
-        return True if field_value is None else field_value
+            if problem_score:
+                self.locations_to_scores[block_key] = problem_score
 
     def _persisted_model_params(self, student):
         """
@@ -226,9 +169,9 @@ class SubsectionGrade(object):
         Returns the list of visible blocks.
         """
         return [
-            BlockRecord(location, weight, score.possible)
-            for location, (score, weight) in
-            self.locations_to_weighted_scores.iteritems()
+            BlockRecord(location, score.weight, score.raw_possible, score.graded)
+            for location, score in
+            self.locations_to_scores.iteritems()
         ]
 
     def _log_event(self, log_func, log_statement, student):
@@ -283,7 +226,7 @@ class SubsectionGradeFactory(object):
         if not subsection_grade:
             subsection_grade = SubsectionGrade(subsection, self.course)
             subsection_grade.init_from_structure(
-                self.student, block_structure, self._scores_client, self._submissions_scores
+                self.student, block_structure, self._submissions_scores, self._csm_scores,
             )
             if PersistentGradesEnabledFlag.feature_enabled(self.course.id):
                 if read_only:
@@ -313,7 +256,7 @@ class SubsectionGradeFactory(object):
         block_structure = self._get_block_structure(block_structure)
         subsection_grade = SubsectionGrade(subsection, self.course)
         subsection_grade.init_from_structure(
-            self.student, block_structure, self._scores_client, self._submissions_scores
+            self.student, block_structure, self._submissions_scores, self._csm_scores
         )
 
         if PersistentGradesEnabledFlag.feature_enabled(self.course.id):
@@ -323,7 +266,7 @@ class SubsectionGradeFactory(object):
         return subsection_grade
 
     @lazy
-    def _scores_client(self):
+    def _csm_scores(self):
         """
         Lazily queries and returns all the scores stored in the user
         state (in CSM) for the course, while caching the result.
@@ -351,7 +294,7 @@ class SubsectionGradeFactory(object):
         if saved_subsection_grade:
             subsection_grade = SubsectionGrade(subsection, self.course)
             subsection_grade.init_from_model(
-                self.student, saved_subsection_grade, block_structure, self._scores_client, self._submissions_scores
+                self.student, saved_subsection_grade, block_structure, self._submissions_scores, self._csm_scores,
             )
             return subsection_grade
 

lms/djangoapps/grades/scores.py

@@ -5,106 +5,246 @@ from logging import getLogger
 
 from openedx.core.lib.cache_utils import memoized
 from xblock.core import XBlock
+from xmodule.block_metadata_utils import display_name_with_default_escaped
+from xmodule.graders import ProblemScore
 from .transformer import GradesTransformer
 
 
 log = getLogger(__name__)
 
 
-@memoized
-def block_types_possibly_scored():
+def possibly_scored(usage_key):
     """
-    Returns the block types that could have a score.
+    Returns whether the given block could impact grading (i.e.
+    has_score or has_children).
+    """
+    return usage_key.block_type in _block_types_possibly_scored()
 
-    Something might be a scored item if it is capable of storing a score
-    (has_score=True). We also have to include anything that can have children,
-    since those children might have scores. We can avoid things like Videos,
-    which have state but cannot ever impact someone's grade.
+
+def get_score(submissions_scores, csm_scores, persisted_block, block):
     """
-    return frozenset(
-        cat for (cat, xblock_class) in XBlock.load_classes() if (
-            getattr(xblock_class, 'has_score', False) or getattr(xblock_class, 'has_children', False)
-        )
+    Returns the score for a problem, as a ProblemScore object.  It is
+    assumed that the provided storages have already been filtered for
+    a single user in question and have user-specific values.
+
+    The score is retrieved from the provided storages in the following
+    order of precedence.  If no value for the block is found in a
+    given storage, the next storage is checked.
+
+    submissions_scores (dict of {unicode(usage_key): (earned, possible)}):
+
+        A python dictionary of serialized UsageKeys to (earned, possible)
+        tuples. These values, retrieved using the Submissions API by the
+        caller (already filtered for the user and course), take precedence
+        above all other score storages.
+
+        When the score is found in this storage, it implies the user's score
+        for the block was persisted via the submissions API. Typically, this API
+        is used by ORA.
+
+        The returned score includes valid values for:
+            weighted_earned
+            weighted_possible
+            graded - retrieved from the persisted block, if found, else from
+                the latest block content.
+
+        Note: raw_earned and raw_possible are not required when submitting scores
+        via the submissions API, so those values (along with the unused weight)
+        are invalid and irrelevant.
+
+    csm_scores (ScoresClient):
+
+        The ScoresClient object (already filtered for the user and course),
+        from which a courseware.models.StudentModule object can be retrieved for
+        the block.
+
+        When the score is found from this storage, it implies the user's score
+        for the block was persisted in the Courseware Student Module. Typically,
+        this storage is used for all CAPA problems, including scores calculated
+        by external graders.
+
+        The returned score includes valid values for:
+            raw_earned, raw_possible - retrieved from CSM
+            weighted_earned, weighted_possible - calculated from the raw scores and weight
+            weight, graded - retrieved from the persisted block, if found,
+                else from the latest block content
+
+    persisted_block (.models.BlockRecord):
+        The block values as found in the grades persistence layer. These values
+        are used only if not found from an earlier storage, and take precedence
+        over values stored within the latest content-version of the block.
+
+        When the score is found from this storage, it implies the user has not
+        yet attempted this problem, but the user's grade _was_ persisted.
+
+        The returned score includes valid values for:
+            raw_earned - will equal 0.0 since the user's score was not found from
+                earlier storages
+            raw_possible - retrieved from the persisted block
+            weighted_earned, weighted_possible - calculated from the raw scores and weight
+            weight, graded - retrieved from the persisted block
+
+    block (block_structure.BlockData):
+        Values from the latest content-version of the block are used only if
+        they were not available from a prior storage.
+
+        When the score is found from this storage, it implies the user has not
+        yet attempted this problem and the user's grade was _not_ yet persisted.
+
+        The returned score includes valid values for:
+            raw_earned - will equal 0.0 since the user's score was not found from
+                earlier storages
+            raw_possible - retrieved from the latest block content
+            weighted_earned, weighted_possible - calculated from the raw scores and weight
+            weight, graded - retrieved from the latest block content
+    """
+    weight = _get_weight_from_block(persisted_block, block)
+
+    # Priority order for retrieving the scores:
+    # submissions API -> CSM -> grades persisted block -> latest block content
+    raw_earned, raw_possible, weighted_earned, weighted_possible = (
+        _get_score_from_submissions(submissions_scores, block) or
+        _get_score_from_csm(csm_scores, block, weight) or
+        _get_score_from_persisted_or_latest_block(persisted_block, block, weight)
     )
 
+    assert weighted_possible is not None
+    has_valid_denominator = weighted_possible > 0.0
+    graded = _get_graded_from_block(persisted_block, block) if has_valid_denominator else False
+
+    return ProblemScore(
+        raw_earned,
+        raw_possible,
+        weighted_earned,
+        weighted_possible,
+        weight,
+        graded,
+        display_name=display_name_with_default_escaped(block),
+        module_id=block.location,
+    )
 
-def possibly_scored(usage_key):
+
+def _get_score_from_submissions(submissions_scores, block):
     """
-    Returns whether the given block could impact grading (i.e. scored, or has children).
+    Returns the score values from the submissions API if found.
     """
-    return usage_key.block_type in block_types_possibly_scored()
+    if submissions_scores:
+        submission_value = submissions_scores.get(unicode(block.location))
+        if submission_value:
+            weighted_earned, weighted_possible = submission_value
+            assert weighted_earned >= 0.0 and weighted_possible > 0.0  # per contract from submissions API
+            return (None, None) + (weighted_earned, weighted_possible)
 
 
-def weighted_score(raw_earned, raw_possible, weight=None):
+def _get_score_from_csm(csm_scores, block, weight):
     """
-    Return a tuple that represents the weighted (earned, possible) score.
-    If weight is None or raw_possible is 0, returns the original values.
+    Returns the score values from the courseware student module, via
+    ScoresClient, if found.
+    """
+    # If an entry exists and has raw_possible (total) associated with it, we trust
+    # that value. This is important for cases where a student might have seen an
+    # older version of the problem -- they're still graded on what was possible
+    # when they tried the problem, not what it's worth now.
+    #
+    # Note: Storing raw_possible in CSM predates the implementation of the grades
+    # own persistence layer. Hence, we have duplicate storage locations for
+    # raw_possible, with potentially conflicting values, when a problem is
+    # attempted. Even though the CSM persistence for this value is now
+    # superfluous, for backward compatibility, we continue to use its value for
+    # raw_possible, giving it precedence over the one in the grades data model.
+    score = csm_scores.get(block.location)
+    has_valid_score = score and score.total is not None
+    if has_valid_score:
+        raw_earned = score.correct if score.correct is not None else 0.0
+        raw_possible = score.total
+        return (raw_earned, raw_possible) + _weighted_score(raw_earned, raw_possible, weight)
+
+
+def _get_score_from_persisted_or_latest_block(persisted_block, block, weight):
+    """
+    Returns the score values, now assuming the earned score is 0.0 - since a
+    score was not found in an earlier storage.
+    Uses the raw_possible value from the persisted_block if found, else from
+    the latest block content.
     """
-    if weight is None or raw_possible == 0:
-        return (raw_earned, raw_possible)
-    return float(raw_earned) * weight / raw_possible, float(weight)
+    raw_earned = 0.0
+
+    if persisted_block:
+        raw_possible = persisted_block.raw_possible
+    else:
+        raw_possible = block.transformer_data[GradesTransformer].max_score
+
+    return (raw_earned, raw_possible) + _weighted_score(raw_earned, raw_possible, weight)
 
 
-def get_score(user, block, scores_client, submissions_scores_cache, weight, possible=None):
+def _get_weight_from_block(persisted_block, block):
     """
-    Return the score for a user on a problem, as a tuple (earned, possible).
-    e.g. (5,7) if you got 5 out of 7 points.
+    Returns the weighted value from the persisted_block if found, else from
+    the latest block content.
+    """
+    if persisted_block:
+        return persisted_block.weight
+    else:
+        return getattr(block, 'weight', None)
 
-    If this problem doesn't have a score, or we couldn't load it, returns (None,
-    None).
 
-    user: a Student object
-    block: a BlockStructure's BlockData object
-    scores_client: an initialized ScoresClient
-    submissions_scores_cache: A dict of location names to (earned, possible)
-        point tuples.  If an entry is found in this cache, it takes precedence.
-    weight: The weight of the problem to use in the calculation.  A value of
-        None signifies that the weight should not be applied.
-    possible (optional): The possible maximum score of the problem to use in the
-        calculation.  If None, uses the value found either in scores_client or
-        from the block.
+def _get_graded_from_block(persisted_block, block):
+    """
+    Returns the graded value from the persisted_block if found, else from
+    the latest block content.
     """
-    submissions_scores_cache = submissions_scores_cache or {}
+    if persisted_block:
+        return persisted_block.graded
+    else:
+        return _get_explicit_graded(block)
 
-    if not user.is_authenticated():
-        return (None, None)
 
-    location_url = unicode(block.location)
-    if location_url in submissions_scores_cache:
-        return submissions_scores_cache[location_url]
+def _get_explicit_graded(block):
+    """
+    Returns the explicit graded field value for the given block.
+    """
+    field_value = getattr(
+        block.transformer_data[GradesTransformer],
+        GradesTransformer.EXPLICIT_GRADED_FIELD_NAME,
+        None,
+    )
 
-    if not getattr(block, 'has_score', False):
-        # These are not problems, and do not have a score
-        return (None, None)
+    # Set to True if grading is not explicitly disabled for
+    # this block.  This allows us to include the block's score
+    # in the aggregated self.graded_total, regardless of the
+    # inherited graded value from the subsection. (TNL-5560)
+    return True if field_value is None else field_value
 
-    # Check the score that comes from the ScoresClient (out of CSM).
-    # If an entry exists and has a total associated with it, we trust that
-    # value. This is important for cases where a student might have seen an
-    # older version of the problem -- they're still graded on what was possible
-    # when they tried the problem, not what it's worth now.
-    score = scores_client.get(block.location)
-    if score and score.total is not None:
-        # We have a valid score, just use it.
-        earned = score.correct if score.correct is not None else 0.0
-        if possible is None:
-            possible = score.total
-        elif possible != score.total:
-            log.error(
-                u"Persistent Grades: scores.get_score, possible value {} != score.total value {}".format(
-                    possible,
-                    score.total
-                )
-            )
+
+def _weighted_score(raw_earned, raw_possible, weight):
+    """
+    Returns a tuple that represents the weighted (earned, possible) score.
+    If weight is None or raw_possible is 0, returns the original values.
+
+    When weight is used, it defines the weighted_possible.  This allows
+    course authors to specify the exact maximum value for a problem when
+    they provide a weight.
+    """
+    assert raw_possible is not None
+    cannot_compute_with_weight = weight is None or raw_possible == 0
+    if cannot_compute_with_weight:
+        return raw_earned, raw_possible
     else:
-        # This means we don't have a valid score entry and we don't have a
-        # cached_max_score on hand. We know they've earned 0.0 points on this.
-        earned = 0.0
-        if possible is None:
-            possible = block.transformer_data[GradesTransformer].max_score
-
-        # Problem may be an error module (if something in the problem builder failed)
-        # In which case possible might be None
-        if possible is None:
-            return (None, None)
-
-    return weighted_score(earned, possible, weight)
+        return float(raw_earned) * weight / raw_possible, float(weight)
+
+
+@memoized
+def _block_types_possibly_scored():
+    """
+    Returns the block types that could have a score.
+
+    Something might be a scored item if it is capable of storing a score
+    (has_score=True). We also have to include anything that can have children,
+    since those children might have scores. We can avoid things like Videos,
+    which have state but cannot ever impact someone's grade.
+    """
+    return frozenset(
+        category for (category, xblock_class) in XBlock.load_classes() if (
+            getattr(xblock_class, 'has_score', False) or getattr(xblock_class, 'has_children', False)
+        )
+    )

lms/djangoapps/grades/tests/test_grades.py

@@ -2,24 +2,29 @@
 Test grade calculation.
 """
 
+import ddt
 from django.http import Http404
+import itertools
 from mock import patch
 from nose.plugins.attrib import attr
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
-from courseware.tests.helpers import (
-    LoginEnrollmentTestCase,
-    get_request_for_user
-)
+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.tests.factories import UserFactory
 from student.models import CourseEnrollment
+from xmodule.block_metadata_utils import display_name_with_default_escaped
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
+from xmodule.graders import ProblemScore
 
 from .utils import answer_problem
 from .. import course_grades
 from ..course_grades import summary as grades_summary
 from ..new.course_grade import CourseGradeFactory
+from ..new.subsection_grade import SubsectionGradeFactory
 
 
 def _grade_with_errors(student, course):
@@ -36,6 +41,17 @@ def _grade_with_errors(student, course):
     return grades_summary(student, course)
 
 
+def _create_problem_xml():
+    """
+    Creates and returns XML for a multiple choice response problem
+    """
+    return 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']
+    )
+
+
 @attr(shard=1)
 class TestGradeIteration(SharedModuleStoreTestCase):
     """
@@ -137,6 +153,101 @@ class TestGradeIteration(SharedModuleStoreTestCase):
         return students_to_gradesets, students_to_errors
 
 
+@ddt.ddt
+class TestWeightedProblems(SharedModuleStoreTestCase):
+    """
+    Test scores and grades with various problem weight values.
+    """
+    @classmethod
+    def setUpClass(cls):
+        super(TestWeightedProblems, cls).setUpClass()
+        cls.course = CourseFactory.create()
+        cls.chapter = ItemFactory.create(parent=cls.course, category="chapter", display_name="chapter")
+        cls.sequential = ItemFactory.create(parent=cls.chapter, category="sequential", display_name="sequential")
+        cls.vertical = ItemFactory.create(parent=cls.sequential, category="vertical", display_name="vertical1")
+        problem_xml = _create_problem_xml()
+        cls.problems = []
+        for i in range(2):
+            cls.problems.append(
+                ItemFactory.create(
+                    parent=cls.vertical,
+                    category="problem",
+                    display_name="problem_{}".format(i),
+                    data=problem_xml,
+                )
+            )
+
+    def setUp(self):
+        super(TestWeightedProblems, self).setUp()
+        self.user = UserFactory()
+        self.request = get_request_for_user(self.user)
+
+    def _verify_grades(self, raw_earned, raw_possible, weight, expected_score):
+        """
+        Verifies the computed grades are as expected.
+        """
+        with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred):
+            # pylint: disable=no-member
+            for problem in self.problems:
+                problem.weight = weight
+                self.store.update_item(problem, self.user.id)
+            self.store.publish(self.course.location, self.user.id)
+
+        course_structure = get_course_blocks(self.request.user, self.course.location)
+
+        # answer all problems
+        for problem in self.problems:
+            answer_problem(self.course, self.request, problem, score=raw_earned, max_value=raw_possible)
+
+        # get grade
+        subsection_grade = SubsectionGradeFactory(
+            self.request.user, self.course, course_structure
+        ).update(self.sequential)
+
+        # verify all problem grades
+        for problem in self.problems:
+            problem_score = subsection_grade.locations_to_scores[problem.location]
+            expected_score.display_name = display_name_with_default_escaped(problem)
+            expected_score.module_id = problem.location
+            self.assertEquals(problem_score, expected_score)
+
+        # verify subsection grades
+        self.assertEquals(subsection_grade.all_total.earned, expected_score.earned * len(self.problems))
+        self.assertEquals(subsection_grade.all_total.possible, expected_score.possible * len(self.problems))
+
+    @ddt.data(
+        *itertools.product(
+            (0.0, 0.5, 1.0, 2.0),  # raw_earned
+            (-2.0, -1.0, 0.0, 0.5, 1.0, 2.0),  # raw_possible
+            (-2.0, -1.0, -0.5, 0.0, 0.5, 1.0, 2.0, 50.0, None),  # weight
+        )
+    )
+    @ddt.unpack
+    def test_problem_weight(self, raw_earned, raw_possible, weight):
+
+        use_weight = weight is not None and raw_possible != 0
+        if use_weight:
+            expected_w_earned = raw_earned / raw_possible * weight
+            expected_w_possible = weight
+        else:
+            expected_w_earned = raw_earned
+            expected_w_possible = raw_possible
+
+        expected_graded = expected_w_possible > 0
+
+        expected_score = ProblemScore(
+            raw_earned=raw_earned,
+            raw_possible=raw_possible,
+            weighted_earned=expected_w_earned,
+            weighted_possible=expected_w_possible,
+            weight=weight,
+            graded=expected_graded,
+            display_name=None,  # problem-specific, filled in by _verify_grades
+            module_id=None,  # problem-specific, filled in by _verify_grades
+        )
+        self._verify_grades(raw_earned, raw_possible, weight, expected_score)
+
+
 class TestScoreForModule(SharedModuleStoreTestCase):
     """
     Test the method that calculates the score for a given block based on the

lms/djangoapps/grades/tests/test_models.py

@@ -71,8 +71,8 @@ class GradesModelTestCase(TestCase):
             block_type='problem',
             block_id='block_id_b'
         )
-        self.record_a = BlockRecord(locator=self.locator_a, weight=1, max_score=10)
-        self.record_b = BlockRecord(locator=self.locator_b, weight=1, max_score=10)
+        self.record_a = BlockRecord(locator=self.locator_a, weight=1, raw_possible=10, graded=False)
+        self.record_b = BlockRecord(locator=self.locator_b, weight=1, raw_possible=10, graded=True)
 
 
 @ddt.ddt
@@ -88,29 +88,31 @@ class BlockRecordTest(GradesModelTestCase):
         Tests creation of a BlockRecord.
         """
         weight = 1
-        max_score = 10
+        raw_possible = 10
         record = BlockRecord(
             self.locator_a,
             weight,
-            max_score,
+            raw_possible,
+            graded=False,
         )
         self.assertEqual(record.locator, self.locator_a)
 
     @ddt.data(
-        (0, 0, "0123456789abcdef"),
-        (1, 10, 'totally_a_real_block_key'),
-        ("BlockRecord is", "a dumb data store", "with no validation"),
+        (0, 0, "0123456789abcdef", True),
+        (1, 10, 'totally_a_real_block_key', False),
+        ("BlockRecord is", "a dumb data store", "with no validation", None),
     )
     @ddt.unpack
-    def test_serialization(self, weight, max_score, block_key):
+    def test_serialization(self, weight, raw_possible, block_key, graded):
         """
         Tests serialization of a BlockRecord using the _asdict() method.
         """
-        record = BlockRecord(block_key, weight, max_score)
+        record = BlockRecord(block_key, weight, raw_possible, graded)
         expected = OrderedDict([
             ("locator", block_key),
             ("weight", weight),
-            ("max_score", max_score),
+            ("raw_possible", raw_possible),
+            ("graded", graded),
         ])
         self.assertEqual(expected, record._asdict())
 
@@ -134,7 +136,12 @@ class VisibleBlocksTest(GradesModelTestCase):
         for block_dict in list_of_block_dicts:
             block_dict['locator'] = unicode(block_dict['locator'])  # BlockUsageLocator is not json-serializable
         expected_data = {
-            'blocks': [{'locator': unicode(self.record_a.locator), 'max_score': 10, 'weight': 1}],
+            'blocks': [{
+                'locator': unicode(self.record_a.locator),
+                'raw_possible': 10,
+                'weight': 1,
+                'graded': self.record_a.graded,
+            }],
             'course_key': unicode(self.record_a.locator.course_key),
             'version': BLOCK_RECORD_LIST_VERSION,
         }

lms/djangoapps/grades/tests/test_new.py

 """
 Test saved subsection grade functionality.
 """
-
+# pylint: disable=protected-access
 import ddt
 from django.conf import settings
 from django.db.utils import DatabaseError
@@ -116,7 +116,7 @@ class SubsectionGradeFactoryTest(GradeTestBase):
         ) as mock_create_grade:
             with patch(
                 'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._get_saved_grade',
-                wraps=self.subsection_grade_factory._get_saved_grade  # pylint: disable=protected-access
+                wraps=self.subsection_grade_factory._get_saved_grade
             ) as mock_get_saved_grade:
                 with self.assertNumQueries(14):
                     grade_a = self.subsection_grade_factory.create(self.sequence)
@@ -205,8 +205,8 @@ class SubsectionGradeTest(GradeTestBase):
         input_grade.init_from_structure(
             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.subsection_grade_factory._submissions_scores,
+            self.subsection_grade_factory._csm_scores,
         )
         self.assertEqual(PersistentSubsectionGrade.objects.count(), 0)
 
@@ -224,8 +224,8 @@ class SubsectionGradeTest(GradeTestBase):
             self.request.user,
             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.subsection_grade_factory._submissions_scores,
+            self.subsection_grade_factory._csm_scores,
         )
 
         self.assertEqual(input_grade.url_name, loaded_grade.url_name)

lms/djangoapps/grades/tests/test_scores.py

+"""
+Tests for grades.scores module.
+"""
+# pylint: disable=protected-access
+from collections import namedtuple
+import ddt
+from django.test import TestCase
+import itertools
+
+from lms.djangoapps.grades.models import BlockRecord
+import lms.djangoapps.grades.scores as scores
+from lms.djangoapps.grades.transformer import GradesTransformer
+from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
+from openedx.core.lib.block_structure.block_structure import BlockData
+from xmodule.graders import ProblemScore
+
+
+class TestScoredBlockTypes(TestCase):
+    """
+    Tests for the possibly_scored function.
+    """
+    possibly_scored_block_types = {
+        'course', 'chapter', 'sequential', 'vertical',
+        'library_content', 'split_test', 'conditional', 'library', 'randomize',
+        'problem', 'drag-and-drop-v2', 'openassessment', 'lti', 'lti_consumer',
+        'videosequence', 'problemset', 'acid_parent', 'done', 'wrapper', 'edx_sga',
+    }
+
+    def test_block_types_possibly_scored(self):
+        self.assertSetEqual(
+            self.possibly_scored_block_types,
+            scores._block_types_possibly_scored()
+        )
+
+    def test_possibly_scored(self):
+        course_key = CourseLocator(u'org', u'course', u'run')
+        for block_type in self.possibly_scored_block_types:
+            usage_key = BlockUsageLocator(course_key, block_type, 'mock_block_id')
+            self.assertTrue(scores.possibly_scored(usage_key))
+
+
+@ddt.ddt
+class TestGetScore(TestCase):
+    """
+    Tests for get_score
+    """
+    display_name = 'test_name'
+    location = 'test_location'
+
+    SubmissionValue = namedtuple('SubmissionValue', 'exists, weighted_earned, weighted_possible')
+    CSMValue = namedtuple('CSMValue', 'exists, raw_earned, raw_possible')
+    PersistedBlockValue = namedtuple('PersistedBlockValue', 'exists, raw_possible, weight, graded')
+    ContentBlockValue = namedtuple('ContentBlockValue', 'raw_possible, weight, explicit_graded')
+    ExpectedResult = namedtuple(
+        'ExpectedResult', 'raw_earned, raw_possible, weighted_earned, weighted_possible, weight, graded'
+    )
+
+    def _create_submissions_scores(self, submission_value):
+        """
+        Creates a stub result from the submissions API for the given values.
+        """
+        if submission_value.exists:
+            return {self.location: (submission_value.weighted_earned, submission_value.weighted_possible)}
+        else:
+            return {}
+
+    def _create_csm_scores(self, csm_value):
+        """
+        Creates a stub result from courseware student module for the given values.
+        """
+        if csm_value.exists:
+            stub_csm_record = namedtuple('stub_csm_record', 'correct, total')
+            return {self.location: stub_csm_record(correct=csm_value.raw_earned, total=csm_value.raw_possible)}
+        else:
+            return {}
+
+    def _create_persisted_block(self, persisted_block_value):
+        """
+        Creates and returns a minimal BlockRecord object with the give values.
+        """
+        if persisted_block_value.exists:
+            return BlockRecord(
+                self.location,
+                persisted_block_value.weight,
+                persisted_block_value.raw_possible,
+                persisted_block_value.graded,
+            )
+        else:
+            return None
+
+    def _create_block(self, content_block_value):
+        """
+        Creates and returns a minimal BlockData object with the give values.
+        """
+        block = BlockData(self.location)
+        block.display_name = self.display_name
+        block.weight = content_block_value.weight
+
+        block_grades_transformer_data = block.transformer_data.get_or_create(GradesTransformer)
+        block_grades_transformer_data.max_score = content_block_value.raw_possible
+        setattr(
+            block_grades_transformer_data,
+            GradesTransformer.EXPLICIT_GRADED_FIELD_NAME,
+            content_block_value.explicit_graded,
+        )
+        return block
+
+    @ddt.data(
+        # submissions _trumps_ other values; weighted and graded from persisted-block _trumps_ latest content values
+        (
+            SubmissionValue(exists=True, weighted_earned=50, weighted_possible=100),
+            CSMValue(exists=True, raw_earned=10, raw_possible=40),
+            PersistedBlockValue(exists=True, raw_possible=5, weight=40, graded=True),
+            ContentBlockValue(raw_possible=1, weight=20, explicit_graded=False),
+            ExpectedResult(
+                raw_earned=None, raw_possible=None, weighted_earned=50, weighted_possible=100, weight=40, graded=True
+            ),
+        ),
+        # same as above, except submissions doesn't exist; CSM values used
+        (
+            SubmissionValue(exists=False, weighted_earned=50, weighted_possible=100),
+            CSMValue(exists=True, raw_earned=10, raw_possible=40),
+            PersistedBlockValue(exists=True, raw_possible=5, weight=40, graded=True),
+            ContentBlockValue(raw_possible=1, weight=20, explicit_graded=False),
+            ExpectedResult(
+                raw_earned=10, raw_possible=40, weighted_earned=10, weighted_possible=40, weight=40, graded=True
+            ),
+        ),
+        # neither submissions nor CSM exist; Persisted values used
+        (
+            SubmissionValue(exists=False, weighted_earned=50, weighted_possible=100),
+            CSMValue(exists=False, raw_earned=10, raw_possible=40),
+            PersistedBlockValue(exists=True, raw_possible=5, weight=40, graded=True),
+            ContentBlockValue(raw_possible=1, weight=20, explicit_graded=False),
+            ExpectedResult(
+                raw_earned=0, raw_possible=5, weighted_earned=0, weighted_possible=40, weight=40, graded=True
+            ),
+        ),
+        # none of submissions, CSM, or persisted exist; Latest content values used
+        (
+            SubmissionValue(exists=False, weighted_earned=50, weighted_possible=100),
+            CSMValue(exists=False, raw_earned=10, raw_possible=40),
+            PersistedBlockValue(exists=False, raw_possible=5, weight=40, graded=True),
+            ContentBlockValue(raw_possible=1, weight=20, explicit_graded=False),
+            ExpectedResult(
+                raw_earned=0, raw_possible=1, weighted_earned=0, weighted_possible=20, weight=20, graded=False
+            ),
+        ),
+    )
+    @ddt.unpack
+    def test_get_score(self, submission_value, csm_value, persisted_block_value, block_value, expected_result):
+        score = scores.get_score(
+            self._create_submissions_scores(submission_value),
+            self._create_csm_scores(csm_value),
+            self._create_persisted_block(persisted_block_value),
+            self._create_block(block_value),
+        )
+        expected_score = ProblemScore(
+            display_name=self.display_name, module_id=self.location, **expected_result._asdict()
+        )
+        self.assertEquals(score, expected_score)
+
+
+@ddt.ddt
+class TestInternalWeightedScore(TestCase):
+    """
+    Tests the internal helper method: _weighted_score
+    """
+    @ddt.data(
+        (0, 0, 1),
+        (5, 0, 0),
+        (10, 0, None),
+        (0, 5, None),
+        (5, 10, None),
+        (10, 10, None),
+    )
+    @ddt.unpack
+    def test_cannot_compute(self, raw_earned, raw_possible, weight):
+        self.assertEquals(
+            scores._weighted_score(raw_earned, raw_possible, weight),
+            (raw_earned, raw_possible),
+        )
+
+    @ddt.data(
+        (0, 5, 0, (0, 0)),
+        (5, 5, 0, (0, 0)),
+        (2, 5, 1, (.4, 1)),
+        (5, 5, 1, (1, 1)),
+        (5, 5, 3, (3, 3)),
+        (2, 4, 6, (3, 6)),
+    )
+    @ddt.unpack
+    def test_computed(self, raw_earned, raw_possible, weight, expected_score):
+        self.assertEquals(
+            scores._weighted_score(raw_earned, raw_possible, weight),
+            expected_score,
+        )
+
+    def test_assert_on_invalid_r_possible(self):
+        with self.assertRaises(AssertionError):
+            scores._weighted_score(raw_earned=1, raw_possible=None, weight=1)
+
+
+@ddt.ddt
+class TestInternalGetGraded(TestCase):
+    """
+    Tests the internal helper method: _get_explicit_graded
+    """
+    def _create_block(self, explicit_graded_value):
+        """
+        Creates and returns a minimal BlockData object with the give value
+        for explicit_graded.
+        """
+        block = BlockData('any_key')
+        setattr(
+            block.transformer_data.get_or_create(GradesTransformer),
+            GradesTransformer.EXPLICIT_GRADED_FIELD_NAME,
+            explicit_graded_value,
+        )
+        return block
+
+    @ddt.data(None, True, False)
+    def test_with_no_persisted_block(self, explicitly_graded_value):
+        block = self._create_block(explicitly_graded_value)
+        self.assertEquals(
+            scores._get_graded_from_block(None, block),
+            explicitly_graded_value is not False,  # defaults to True unless explicitly False
+        )
+
+    @ddt.data(
+        *itertools.product((True, False), (True, False, None))
+    )
+    @ddt.unpack
+    def test_with_persisted_block(self, persisted_block_value, block_value):
+        block = self._create_block(block_value)
+        block_record = BlockRecord(block.location, 0, 0, persisted_block_value)
+        self.assertEquals(
+            scores._get_graded_from_block(block_record, block),
+            block_record.graded,  # persisted value takes precedence
+        )
+
+
+@ddt.ddt
+class TestInternalGetScoreFromBlock(TestCase):
+    """
+    Tests the internal helper method: _get_score_from_persisted_or_latest_block
+    """
+    def _create_block(self, raw_possible):
+        """
+        Creates and returns a minimal BlockData object with the give value
+        for raw_possible.
+        """
+        block = BlockData('any_key')
+        block.transformer_data.get_or_create(GradesTransformer).max_score = raw_possible
+        return block
+
+    def _verify_score_result(self, persisted_block, block, weight, expected_r_possible):
+        """
+        Verifies the result of _get_score_from_persisted_or_latest_block is as expected.
+        """
+        # pylint: disable=unbalanced-tuple-unpacking
+        raw_earned, raw_possible, weighted_earned, weighted_possible = scores._get_score_from_persisted_or_latest_block(
+            persisted_block, block, weight,
+        )
+        self.assertEquals(raw_earned, 0.0)
+        self.assertEquals(raw_possible, expected_r_possible)
+        self.assertEquals(weighted_earned, 0.0)
+        if weight is None or expected_r_possible == 0:
+            self.assertEquals(weighted_possible, expected_r_possible)
+        else:
+            self.assertEquals(weighted_possible, weight)
+
+    @ddt.data(
+        *itertools.product((0, 1, 5), (None, 0, 1, 5))
+    )
+    @ddt.unpack
+    def test_with_no_persisted_block(self, block_r_possible, weight):
+        block = self._create_block(block_r_possible)
+        self._verify_score_result(None, block, weight, block_r_possible)
+
+    @ddt.data(
+        *itertools.product((0, 1, 5), (None, 0, 1, 5), (None, 0, 1, 5))
+    )
+    @ddt.unpack
+    def test_with_persisted_block(self, persisted_block_r_possible, block_r_possible, weight):
+        block = self._create_block(block_r_possible)
+        block_record = BlockRecord(block.location, 0, persisted_block_r_possible, False)
+        self._verify_score_result(block_record, block, weight, persisted_block_r_possible)

lms/djangoapps/grades/tests/utils.py

@@ -5,6 +5,7 @@ from contextlib import contextmanager
 from mock import patch
 from courseware.module_render import get_module
 from courseware.model_data import FieldDataCache
+from xmodule.graders import ProblemScore
 
 
 @contextmanager
@@ -23,7 +24,7 @@ def mock_get_score(earned=0, possible=1):
     Mocks the get_score function to return a valid grade.
     """
     with patch('lms.djangoapps.grades.new.subsection_grade.get_score') as mock_score:
-        mock_score.return_value = (earned, possible)
+        mock_score.return_value = ProblemScore(earned, possible, earned, possible, 1, True, None, None)
         yield mock_score
 
 

lms/djangoapps/instructor_task/tasks_helper.py

@@ -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: