Course Grade reports: eliminate CSM calls, user-transformed structures

EDUCATOR-558

common/lib/xmodule/xmodule/graders.py

@@ -170,7 +170,6 @@ def grader_from_conf(conf):
         weight = subgraderconf.pop("weight", 0)
         try:
             if 'min_count' in subgraderconf:
-                #This is an AssignmentFormatGrader
                 subgrader_class = AssignmentFormatGrader
             else:
                 raise ValueError("Configuration has no appropriate grader class.")
@@ -344,28 +343,28 @@ class AssignmentFormatGrader(CourseGrader):
         self.starting_index = starting_index
         self.hide_average = hide_average
 
-    def grade(self, grade_sheet, generate_random_scores=False):
-        def total_with_drops(breakdown, drop_count):
-            """
-            Calculates total score for a section while dropping lowest scores
-            """
-            # Create an array of tuples with (index, mark), sorted by mark['percent'] descending
-            sorted_breakdown = sorted(enumerate(breakdown), key=lambda x: -x[1]['percent'])
-
-            # A list of the indices of the dropped scores
-            dropped_indices = []
-            if drop_count > 0:
-                dropped_indices = [x[0] for x in sorted_breakdown[-drop_count:]]
-            aggregate_score = 0
-            for index, mark in enumerate(breakdown):
-                if index not in dropped_indices:
-                    aggregate_score += mark['percent']
-
-            if len(breakdown) - drop_count > 0:
-                aggregate_score /= len(breakdown) - drop_count
-
-            return aggregate_score, dropped_indices
+    def total_with_drops(self, breakdown):
+        """
+        Calculates total score for a section while dropping lowest scores
+        """
+        # Create an array of tuples with (index, mark), sorted by mark['percent'] descending
+        sorted_breakdown = sorted(enumerate(breakdown), key=lambda x: -x[1]['percent'])
+
+        # A list of the indices of the dropped scores
+        dropped_indices = []
+        if self.drop_count > 0:
+            dropped_indices = [x[0] for x in sorted_breakdown[-self.drop_count:]]
+        aggregate_score = 0
+        for index, mark in enumerate(breakdown):
+            if index not in dropped_indices:
+                aggregate_score += mark['percent']
 
+        if len(breakdown) - self.drop_count > 0:
+            aggregate_score /= len(breakdown) - self.drop_count
+
+        return aggregate_score, dropped_indices
+
+    def grade(self, grade_sheet, generate_random_scores=False):
         scores = grade_sheet.get(self.type, {}).values()
         breakdown = []
         for i in range(max(self.min_count, len(scores))):
@@ -405,7 +404,7 @@ class AssignmentFormatGrader(CourseGrader):
             breakdown.append({'percent': percentage, 'label': short_label,
                               'detail': summary, 'category': self.category})
 
-        total_percent, dropped_indices = total_with_drops(breakdown, self.drop_count)
+        total_percent, dropped_indices = self.total_with_drops(breakdown)
 
         for dropped_index in dropped_indices:
             breakdown[dropped_index]['mark'] = {

lms/djangoapps/grades/context.py

@@ -5,18 +5,19 @@ from collections import OrderedDict
 
 from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
 
+from .course_grade import CourseGrade
 from .scores import possibly_scored
 
 
-def grading_context_for_course(course_key):
+def grading_context_for_course(course):
     """
     Same as grading_context, but takes in a course key.
     """
-    course_structure = get_course_in_cache(course_key)
-    return grading_context(course_structure)
+    course_structure = get_course_in_cache(course.id)
+    return grading_context(course, course_structure)
 
 
-def grading_context(course_structure):
+def grading_context(course, course_structure):
     """
     This returns a dictionary with keys necessary for quickly grading
     a student.
@@ -36,7 +37,7 @@ def grading_context(course_structure):
         the descriptor tree again.
 
     """
-    all_graded_blocks = []
+    count_all_graded_blocks = 0
     all_graded_subsections_by_type = OrderedDict()
 
     for chapter_key in course_structure.get_children(course_structure.root_block_usage_key):
@@ -64,9 +65,10 @@ def grading_context(course_structure):
                 if subsection_format not in all_graded_subsections_by_type:
                     all_graded_subsections_by_type[subsection_format] = []
                 all_graded_subsections_by_type[subsection_format].append(subsection_info)
-                all_graded_blocks.extend(scored_descendants_of_subsection)
+                count_all_graded_blocks += len(scored_descendants_of_subsection)
 
     return {
         'all_graded_subsections_by_type': all_graded_subsections_by_type,
-        'all_graded_blocks': all_graded_blocks,
+        'count_all_graded_blocks': count_all_graded_blocks,
+        'subsection_type_graders': CourseGrade.get_subsection_type_graders(course)
     }

lms/djangoapps/grades/course_data.py

@@ -32,14 +32,14 @@ class CourseData(object):
             if self._course:
                 self._course_key = self._course.id
             else:
-                structure = self._effective_structure
+                structure = self.effective_structure
                 self._course_key = structure.root_block_usage_key.course_key
         return self._course_key
 
     @property
     def location(self):
         if not self._location:
-            structure = self._effective_structure
+            structure = self.effective_structure
             if structure:
                 self._location = structure.root_block_usage_key
             elif self._course:
@@ -72,7 +72,7 @@ class CourseData(object):
 
     @property
     def grading_policy_hash(self):
-        structure = self._effective_structure
+        structure = self.effective_structure
         if structure:
             return structure.get_transformer_block_field(
                 structure.root_block_usage_key,
@@ -84,14 +84,14 @@ class CourseData(object):
 
     @property
     def version(self):
-        structure = self._effective_structure
+        structure = self.effective_structure
         course_block = structure[self.location] if structure else self.course
         return getattr(course_block, 'course_version', None)
 
     @property
     def edited_on(self):
         # get course block from structure only; subtree_edited_on field on modulestore's course block isn't optimized.
-        structure = self._effective_structure
+        structure = self.effective_structure
         if structure:
             course_block = structure[self.location]
             return getattr(course_block, 'subtree_edited_on', None)
@@ -100,7 +100,7 @@ class CourseData(object):
         return u'Course: course_key: {}'.format(self.course_key)
 
     def full_string(self):
-        if self._effective_structure:
+        if self.effective_structure:
             return u'Course: course_key: {}, version: {}, edited_on: {}, grading_policy: {}'.format(
                 self.course_key, self.version, self.edited_on, self.grading_policy_hash,
             )
@@ -108,5 +108,5 @@ class CourseData(object):
             return u'Course: course_key: {}, empty course structure'.format(self.course_key)
 
     @property
-    def _effective_structure(self):
+    def effective_structure(self):
         return self._structure or self._collected_block_structure

lms/djangoapps/grades/course_grade.py

@@ -10,6 +10,7 @@ from lazy import lazy
 from ccx_keys.locator import CCXLocator
 from xmodule import block_metadata_utils
 
+from .config import assume_zero_if_absent
 from .subsection_grade import ZeroSubsectionGrade
 from .subsection_grade_factory import SubsectionGradeFactory
 
@@ -46,17 +47,14 @@ class CourseGradeBase(object):
 
     def subsection_grade(self, subsection_key):
         """
-        Returns the subsection grade for given subsection usage key.
-        Raises KeyError if the user doesn't have access to that subsection.
-        """
-        return self._get_subsection_grade(self.course_data.structure[subsection_key])
+        Returns the subsection grade for the given subsection usage key.
 
-    @abstractmethod
-    def assignment_average(self, assignment_type):
-        """
-        Returns the average of all assignments of the given assignment type.
+        Note: does NOT check whether the user has access to the subsection.
+        Assumes that if a grade exists, the user has access to it.  If the
+        grade doesn't exist then either the user does not have access to
+        it or hasn't attempted any problems in the subsection.
         """
-        raise NotImplementedError
+        return self._get_subsection_grade(self.course_data.effective_structure[subsection_key])
 
     @lazy
     def graded_subsections_by_format(self):
@@ -148,7 +146,7 @@ class CourseGradeBase(object):
         """
         Returns the result from the course grader.
         """
-        course = self._prep_course_for_grading()
+        course = self._prep_course_for_grading(self.course_data.course)
         return course.grader.grade(
             self.graded_subsections_by_format,
             generate_random_scores=settings.GENERATE_PROFILE_SCORES,
@@ -166,7 +164,21 @@ class CourseGradeBase(object):
         grade_summary['grade'] = self.letter_grade
         return grade_summary
 
-    def _prep_course_for_grading(self):
+    @classmethod
+    def get_subsection_type_graders(cls, course):
+        """
+        Returns a dictionary mapping subsection types to their
+        corresponding configured graders, per grading policy.
+        """
+        course = cls._prep_course_for_grading(course)
+        return {
+            subsection_type: subsection_type_grader
+            for (subsection_type_grader, subsection_type, _)
+            in course.grader.subgraders
+        }
+
+    @classmethod
+    def _prep_course_for_grading(cls, course):
         """
         Make sure any overrides to the grading policy are used.
         This is most relevant for CCX courses.
@@ -176,8 +188,7 @@ class CourseGradeBase(object):
         this will no longer be needed - since BlockStructure correctly
         retrieves/uses all field overrides.
         """
-        course = self.course_data.course
-        if isinstance(self.course_data.course_key, CCXLocator):
+        if isinstance(course.id, CCXLocator):
             # clean out any field values that may have been set from the
             # parent course of the CCX course.
             course._field_data_cache = {}  # pylint: disable=protected-access
@@ -221,9 +232,6 @@ class ZeroCourseGrade(CourseGradeBase):
     Course Grade class for Zero-value grades when no problems were
     attempted in the course.
     """
-    def assignment_average(self, assignment_type):
-        return 0.0
-
     def _get_subsection_grade(self, subsection):
         return ZeroSubsectionGrade(subsection, self.course_data)
 
@@ -259,15 +267,15 @@ class CourseGrade(CourseGradeBase):
         Returns whether any of the subsections in this course
         have been attempted by the student.
         """
+        if assume_zero_if_absent(self.course_data.course_key):
+            return True
+
         for chapter in self.chapter_grades.itervalues():
             for subsection_grade in chapter['sections']:
                 if subsection_grade.all_total.first_attempted:
                     return True
         return False
 
-    def assignment_average(self, assignment_type):
-        return self.grader_result['grade_breakdown'].get(assignment_type, {}).get('percent')
-
     def _get_subsection_grade(self, subsection):
         if self.force_update_subsections:
             return self._subsection_grade_factory.update(subsection)

lms/djangoapps/grades/course_grade_factory.py

@@ -85,7 +85,7 @@ class CourseGradeFactory(object):
         If an error occurred, course_grade will be None and err_msg will be an
         exception message. If there was no error, err_msg is an empty string.
         """
-        # Pre-fetch the collected course_structure so:
+        # Pre-fetch the collected course_structure (in _iter_grade_result) so:
         # 1. Correctness: the same version of the course is used to
         #    compute the grade for all students.
         # 2. Optimization: the collected course_structure is not

lms/djangoapps/grades/subsection_grade.py

@@ -63,6 +63,13 @@ class SubsectionGradeBase(object):
         """
         raise NotImplementedError
 
+    @property
+    def percent_graded(self):
+        """
+        Returns the percent score of the graded problems in this subsection.
+        """
+        raise NotImplementedError
+
 
 class ZeroSubsectionGrade(SubsectionGradeBase):
     """
@@ -78,6 +85,10 @@ class ZeroSubsectionGrade(SubsectionGradeBase):
         return False
 
     @property
+    def percent_graded(self):
+        return 0.0
+
+    @property
     def all_total(self):
         return self._aggregate_scores[0]
 
@@ -128,6 +139,10 @@ class NonZeroSubsectionGrade(SubsectionGradeBase):
     def attempted_graded(self):
         return self.graded_total.first_attempted is not None
 
+    @property
+    def percent_graded(self):
+        return self.graded_total.earned / self.graded_total.possible
+
     @staticmethod
     def _compute_block_score(
             block_key,
@@ -157,7 +172,7 @@ class ReadSubsectionGrade(NonZeroSubsectionGrade):
     """
     Class for Subsection grades that are read from the database.
     """
-    def __init__(self, subsection, model, course_structure, submissions_scores, csm_scores):
+    def __init__(self, subsection, model, factory):
         all_total = AggregatedScore(
             tw_earned=model.earned_all,
             tw_possible=model.possible_all,
@@ -174,9 +189,7 @@ class ReadSubsectionGrade(NonZeroSubsectionGrade):
 
         # save these for later since we compute problem_scores lazily
         self.model = model
-        self.course_structure = course_structure
-        self.submissions_scores = submissions_scores
-        self.csm_scores = csm_scores
+        self.factory = factory
 
         super(ReadSubsectionGrade, self).__init__(subsection, all_total, graded_total, override)
 
@@ -185,7 +198,11 @@ class ReadSubsectionGrade(NonZeroSubsectionGrade):
         problem_scores = OrderedDict()
         for block in self.model.visible_blocks.blocks:
             problem_score = self._compute_block_score(
-                block.locator, self.course_structure, self.submissions_scores, self.csm_scores, block,
+                block.locator,
+                self.factory.course_data.structure,
+                self.factory._submissions_scores,
+                self.factory._csm_scores,
+                block,
             )
             if problem_score:
                 problem_scores[block.locator] = problem_score

lms/djangoapps/grades/subsection_grade_factory.py

@@ -80,9 +80,7 @@ class SubsectionGradeFactory(object):
                 except PersistentSubsectionGrade.DoesNotExist:
                     pass
                 else:
-                    orig_subsection_grade = ReadSubsectionGrade(
-                        subsection, grade_model, self.course_data.structure, self._submissions_scores, self._csm_scores,
-                    )
+                    orig_subsection_grade = ReadSubsectionGrade(subsection, grade_model, self)
                     if not is_score_higher_or_equal(
                             orig_subsection_grade.graded_total.earned,
                             orig_subsection_grade.graded_total.possible,
@@ -125,9 +123,7 @@ class SubsectionGradeFactory(object):
             saved_subsection_grades = self._get_bulk_cached_subsection_grades()
             grade = saved_subsection_grades.get(subsection.location)
             if grade:
-                return ReadSubsectionGrade(
-                    subsection, grade, self.course_data.structure, self._submissions_scores, self._csm_scores,
-                )
+                return ReadSubsectionGrade(subsection, grade, self)
 
     def _get_bulk_cached_subsection_grades(self):
         """

lms/djangoapps/grades/tests/base.py

@@ -6,6 +6,7 @@ from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 
+from ..course_data import CourseData
 from ..subsection_grade_factory import SubsectionGradeFactory
 
 
@@ -75,6 +76,7 @@ class GradeTestBase(SharedModuleStoreTestCase):
         self.client.login(username=self.request.user.username, password="test")
         self._set_grading_policy()
         self.course_structure = get_course_blocks(self.request.user, self.course.location)
+        self.course_data = CourseData(self.request.user, structure=self.course_structure)
         self.subsection_grade_factory = SubsectionGradeFactory(self.request.user, self.course, self.course_structure)
         CourseEnrollment.enroll(self.request.user, self.course.id)
 
@@ -91,6 +93,13 @@ class GradeTestBase(SharedModuleStoreTestCase):
                     "short_label": "HW",
                     "weight": 1.0,
                 },
+                {
+                    "type": "NoCredit",
+                    "min_count": 0,
+                    "drop_count": 0,
+                    "short_label": "NC",
+                    "weight": 0.0,
+                },
             ],
             "GRADE_CUTOFFS": {
                 "Pass": passing,

lms/djangoapps/grades/tests/test_course_grade_factory.py

@@ -97,19 +97,19 @@ class TestCourseGradeFactory(GradeTestBase):
         with self.assertNumQueries(29), mock_get_score(1, 2):
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
-        with self.assertNumQueries(4):
+        with self.assertNumQueries(2):
             _assert_read(expected_pass=True, expected_percent=0.5)  # updated to grade of .5
 
-        with self.assertNumQueries(6), mock_get_score(1, 4):
+        with self.assertNumQueries(4), mock_get_score(1, 4):
             grade_factory.update(self.request.user, self.course, force_update_subsections=False)
 
-        with self.assertNumQueries(4):
+        with self.assertNumQueries(2):
             _assert_read(expected_pass=True, expected_percent=0.5)  # NOT updated to grade of .25
 
         with self.assertNumQueries(12), mock_get_score(2, 2):
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
-        with self.assertNumQueries(4):
+        with self.assertNumQueries(2):
             _assert_read(expected_pass=True, expected_percent=1.0)  # updated to grade of 1.0
 
     @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
@@ -124,6 +124,35 @@ class TestCourseGradeFactory(GradeTestBase):
             else:
                 self.assertIsNone(course_grade)
 
+    def test_read_optimization(self):
+        grade_factory = CourseGradeFactory()
+        with patch('lms.djangoapps.grades.course_data.get_course_blocks') as mocked_course_blocks:
+            mocked_course_blocks.return_value = self.course_structure
+            with mock_get_score(1, 2):
+                grade_factory.update(self.request.user, self.course, force_update_subsections=True)
+                self.assertEquals(mocked_course_blocks.call_count, 1)
+
+        with patch('lms.djangoapps.grades.course_data.get_course_blocks') as mocked_course_blocks:
+            with patch('lms.djangoapps.grades.subsection_grade.get_score') as mocked_get_score:
+                course_grade = grade_factory.read(self.request.user, self.course)
+                self.assertEqual(course_grade.percent, 0.5)  # make sure it's not a zero-valued course grade
+                self.assertFalse(mocked_get_score.called)  # no calls to CSM/submissions tables
+                self.assertFalse(mocked_course_blocks.called)  # no user-specific transformer calculation
+
+    def test_subsection_grade(self):
+        grade_factory = CourseGradeFactory()
+        with mock_get_score(1, 2):
+            grade_factory.update(self.request.user, self.course, force_update_subsections=True)
+        course_grade = grade_factory.read(self.request.user, course_structure=self.course_structure)
+        subsection_grade = course_grade.subsection_grade(self.sequence.location)
+        self.assertEqual(subsection_grade.percent_graded, 0.5)
+
+    def test_subsection_type_graders(self):
+        graders = CourseGrade.get_subsection_type_graders(self.course)
+        self.assertEqual(len(graders), 2)
+        self.assertEqual(graders["Homework"].type, "Homework")
+        self.assertEqual(graders["NoCredit"].min_count, 0)
+
     def test_create_zero_subs_grade_for_nonzero_course_grade(self):
         subsection = self.course_structure[self.sequence.location]
         with mock_get_score(1, 2):
@@ -158,6 +187,11 @@ class TestCourseGradeFactory(GradeTestBase):
                     'category': 'Homework',
                     'percent': 0.25,
                     'detail': 'Homework = 25.00% of a possible 100.00%',
+                },
+                'NoCredit': {
+                    'category': 'NoCredit',
+                    'percent': 0.0,
+                    'detail': 'NoCredit = 0.00% of a possible 0.00%',
                 }
             },
             'percent': 0.25,
@@ -181,6 +215,13 @@ class TestCourseGradeFactory(GradeTestBase):
                     'percent': 0.25,
                     'prominent': True
                 },
+                {
+                    'category': 'NoCredit',
+                    'detail': u'NoCredit Average = 0%',
+                    'label': u'NC Avg',
+                    'percent': 0,
+                    'prominent': True
+                },
             ]
         }
         self.assertEqual(expected_summary, actual_summary)

lms/djangoapps/grades/tests/test_subsection_grade.py

@@ -15,6 +15,7 @@ class SubsectionGradeTest(GradeTestBase):
                 self.subsection_grade_factory._csm_scores,
             )
             self.assertEqual(PersistentSubsectionGrade.objects.count(), 0)
+            self.assertEqual(created_grade.percent_graded, 0.5)
 
             # save to db, and verify object is in database
             created_grade.update_or_create_model(self.request.user)
@@ -28,11 +29,10 @@ class SubsectionGradeTest(GradeTestBase):
             read_grade = ReadSubsectionGrade(
                 self.sequence,
                 saved_model,
-                self.course_structure,
-                self.subsection_grade_factory._submissions_scores,
-                self.subsection_grade_factory._csm_scores,
+                self.subsection_grade_factory
             )
 
             self.assertEqual(created_grade.url_name, read_grade.url_name)
             read_grade.all_total.first_attempted = created_grade.all_total.first_attempted = None
             self.assertEqual(created_grade.all_total, read_grade.all_total)
+            self.assertEqual(created_grade.percent_graded, 0.5)

lms/djangoapps/grades/tests/test_tasks.py

@@ -164,10 +164,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEquals(mock_block_structure_create.call_count, 1)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 25, True),
-        (ModuleStoreEnum.Type.mongo, 1, 25, False),
-        (ModuleStoreEnum.Type.split, 3, 25, True),
-        (ModuleStoreEnum.Type.split, 3, 25, False),
+        (ModuleStoreEnum.Type.mongo, 1, 23, True),
+        (ModuleStoreEnum.Type.mongo, 1, 23, False),
+        (ModuleStoreEnum.Type.split, 3, 23, True),
+        (ModuleStoreEnum.Type.split, 3, 23, False),
     )
     @ddt.unpack
     def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
@@ -179,8 +179,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
                     self._apply_recalculate_subsection_grade()
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 25),
-        (ModuleStoreEnum.Type.split, 3, 25),
+        (ModuleStoreEnum.Type.mongo, 1, 23),
+        (ModuleStoreEnum.Type.split, 3, 23),
     )
     @ddt.unpack
     def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
@@ -240,8 +240,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 26),
-        (ModuleStoreEnum.Type.split, 3, 26),
+        (ModuleStoreEnum.Type.mongo, 1, 24),
+        (ModuleStoreEnum.Type.split, 3, 24),
     )
     @ddt.unpack
     def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):

lms/djangoapps/instructor_analytics/basic.py

@@ -518,7 +518,7 @@ def dump_grading_context(course):
     msg += hbar
     msg += "Listing grading context for course %s\n" % course.id.to_deprecated_string()
 
-    gcontext = grading_context_for_course(course.id)
+    gcontext = grading_context_for_course(course)
     msg += "graded sections:\n"
 
     msg += '%s\n' % gcontext['all_graded_subsections_by_type'].keys()
@@ -541,6 +541,6 @@ def dump_grading_context(course):
             msg += "      %s (format=%s, Assignment=%s%s)\n"\
                 % (sdesc.display_name, frmat, aname, notes)
     msg += "all graded blocks:\n"
-    msg += "length=%d\n" % len(gcontext['all_graded_blocks'])
+    msg += "length=%d\n" % gcontext['count_all_graded_blocks']
     msg = '<pre>%s</pre>' % msg.replace('<', '&lt;')
     return msg

lms/djangoapps/instructor_task/tasks_helper/grades.py

@@ -103,7 +103,7 @@ class _CourseGradeReportContext(object):
         Returns an OrderedDict that maps an assignment type to a dict of
         subsection-headers and average-header.
         """
-        grading_cxt = grading_context(self.course_structure)
+        grading_cxt = grading_context(self.course, self.course_structure)
         graded_assignments_map = OrderedDict()
         for assignment_type_name, subsection_infos in grading_cxt['all_graded_subsections_by_type'].iteritems():
             graded_subsections_map = OrderedDict()
@@ -127,7 +127,8 @@ class _CourseGradeReportContext(object):
             graded_assignments_map[assignment_type_name] = {
                 'subsection_headers': graded_subsections_map,
                 'average_header': average_header,
-                'separate_subsection_avg_headers': separate_subsection_avg_headers
+                'separate_subsection_avg_headers': separate_subsection_avg_headers,
+                'grader': grading_cxt['subsection_type_graders'].get(assignment_type_name),
             }
         return graded_assignments_map
 
@@ -297,29 +298,56 @@ class CourseGradeReport(object):
         users = users.select_related('profile__allow_certificate')
         return grouper(users)
 
-    def _user_grade_results(self, course_grade, context):
+    def _user_grades(self, course_grade, context):
         """
         Returns a list of grade results for the given course_grade corresponding
         to the headers for this report.
         """
         grade_results = []
         for assignment_type, assignment_info in context.graded_assignments.iteritems():
-            for subsection_location in assignment_info['subsection_headers']:
-                try:
-                    subsection_grade = course_grade.subsection_grade(subsection_location)
-                except KeyError:
-                    grade_result = u'Not Available'
-                else:
-                    if subsection_grade.attempted_graded:
-                        grade_result = subsection_grade.graded_total.earned / subsection_grade.graded_total.possible
-                    else:
-                        grade_result = u'Not Attempted'
-                grade_results.append([grade_result])
-            if assignment_info['separate_subsection_avg_headers']:
-                assignment_average = course_grade.assignment_average(assignment_type)
+
+            subsection_grades, subsection_grades_results = self._user_subsection_grades(
+                course_grade,
+                assignment_info['subsection_headers'],
+            )
+            grade_results.extend(subsection_grades_results)
+
+            assignment_average = self._user_assignment_average(course_grade, subsection_grades, assignment_info)
+            if assignment_average is not None:
                 grade_results.append([assignment_average])
+
         return [course_grade.percent] + _flatten(grade_results)
 
+    def _user_subsection_grades(self, course_grade, subsection_headers):
+        """
+        Returns a list of grade results for the given course_grade corresponding
+        to the headers for this report.
+        """
+        subsection_grades = []
+        grade_results = []
+        for subsection_location in subsection_headers:
+            subsection_grade = course_grade.subsection_grade(subsection_location)
+            if subsection_grade.attempted_graded:
+                grade_result = subsection_grade.percent_graded
+            else:
+                grade_result = u'Not Attempted'
+            grade_results.append([grade_result])
+            subsection_grades.append(subsection_grade)
+        return subsection_grades, grade_results
+
+    def _user_assignment_average(self, course_grade, subsection_grades, assignment_info):
+        if assignment_info['separate_subsection_avg_headers']:
+            if assignment_info['grader']:
+                if course_grade.attempted:
+                    subsection_breakdown = [
+                        {'percent': subsection_grade.percent_graded}
+                        for subsection_grade in subsection_grades
+                    ]
+                    assignment_average, _ = assignment_info['grader'].total_with_drops(subsection_breakdown)
+                else:
+                    assignment_average = 0.0
+                return assignment_average
+
     def _user_cohort_group_names(self, user, context):
         """
         Returns a list of names of cohort groups in which the given user
@@ -411,7 +439,7 @@ class CourseGradeReport(object):
                 else:
                     success_rows.append(
                         [user.id, user.email, user.username] +
-                        self._user_grade_results(course_grade, context) +
+                        self._user_grades(course_grade, context) +
                         self._user_cohort_group_names(user, context) +
                         self._user_experiment_group_names(user, context) +
                         self._user_team_names(user, bulk_context.teams) +
@@ -440,7 +468,8 @@ class ProblemGradeReport(object):
         # as the keys.  It is structured in this way to keep the values related.
         header_row = OrderedDict([('id', 'Student ID'), ('email', 'Email'), ('username', 'Username')])
 
-        graded_scorable_blocks = cls._graded_scorable_blocks_to_header(course_id)
+        course = get_course_by_id(course_id)
+        graded_scorable_blocks = cls._graded_scorable_blocks_to_header(course)
 
         # Just generate the static fields for now.
         rows = [list(header_row.values()) + ['Enrollment Status', 'Grade'] + _flatten(graded_scorable_blocks.values())]
@@ -451,7 +480,6 @@ class ProblemGradeReport(object):
         # whether each user is currently enrolled in the course.
         CourseEnrollment.bulk_fetch_enrollment_states(enrolled_students, course_id)
 
-        course = get_course_by_id(course_id)
         for student, course_grade, error in CourseGradeFactory().iter(enrolled_students, course):
             student_fields = [getattr(student, field_name) for field_name in header_row]
             task_progress.attempted += 1
@@ -495,13 +523,13 @@ class ProblemGradeReport(object):
         return task_progress.update_task_state(extra_meta={'step': 'Uploading CSV'})
 
     @classmethod
-    def _graded_scorable_blocks_to_header(cls, course_key):
+    def _graded_scorable_blocks_to_header(cls, course):
         """
         Returns an OrderedDict that maps a scorable block's id to its
         headers in the final report.
         """
         scorable_blocks_map = OrderedDict()
-        grading_context = grading_context_for_course(course_key)
+        grading_context = grading_context_for_course(course)
         for assignment_type_name, subsection_infos in grading_context['all_graded_subsections_by_type'].iteritems():
             for subsection_index, subsection_info in enumerate(subsection_infos, start=1):
                 for scorable_block in subsection_info['scored_descendants']:

lms/djangoapps/instructor_task/tests/test_tasks_helper.py

@@ -1707,6 +1707,7 @@ class TestCohortStudents(TestReportMixin, InstructorTaskCourseTestCase):
         )
 
 
+@ddt.ddt
 @patch('lms.djangoapps.instructor_task.tasks_helper.misc.DefaultStorage', new=MockDefaultStorage)
 class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase):
     """
@@ -1782,7 +1783,7 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase):
                         u'Username': self.student.username,
                         u'Grade': '0.13',
                         u'Homework 1: Subsection': '0.5',
-                        u'Homework 2: Hidden': u'Not Available',
+                        u'Homework 2: Hidden': u'Not Attempted',
                         u'Homework 3: Unattempted': u'Not Attempted',
                         u'Homework 4: Empty': u'Not Attempted',
                         u'Homework (Avg)': '0.125',
@@ -1791,22 +1792,16 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase):
                 ignore_other_columns=True,
             )
 
-    def test_fast_generation_zero_grade(self):
+    @ddt.data(True, False)
+    def test_fast_generation(self, create_non_zero_grade):
+        if create_non_zero_grade:
+            self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
         with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
-            with patch('lms.djangoapps.grades.course_grade.CourseGradeBase._prep_course_for_grading') as mock_grader:
+            with patch('lms.djangoapps.grades.course_data.get_course_blocks') as mock_course_blocks:
                 with patch('lms.djangoapps.grades.subsection_grade.get_score') as mock_get_score:
                     CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
-                    self.assertFalse(mock_grader.called)
                     self.assertFalse(mock_get_score.called)
-
-    def test_slow_generation_nonzero_grade(self):
-        self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
-        with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
-            with patch('lms.djangoapps.grades.course_grade.CourseGradeBase._prep_course_for_grading') as mock_grader:
-                with patch('lms.djangoapps.grades.subsection_grade.get_score') as mock_get_score:
-                    CourseGradeReport.generate(None, None, self.course.id, None, 'graded')
-                    self.assertTrue(mock_grader.called)
-                    self.assertTrue(mock_get_score.called)
+                    self.assertFalse(mock_course_blocks.called)
 
 
 @ddt.ddt