Update grades to use Block Structures

common/lib/xmodule/xmodule/course_module.py

@@ -11,12 +11,10 @@ from django.utils.timezone import UTC
 from lazy import lazy
 from lxml import etree
 from path import Path as path
-from xblock.core import XBlock
 from xblock.fields import Scope, List, String, Dict, Boolean, Integer, Float
 
 from xmodule import course_metadata_utils
 from xmodule.course_metadata_utils import DEFAULT_START_DATE
-from xmodule.exceptions import UndefinedContext
 from xmodule.graders import grader_from_conf
 from xmodule.mixin import LicenseMixin
 from xmodule.seq_module import SequenceDescriptor, SequenceModule
@@ -1183,83 +1181,6 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin):
         """
         return course_metadata_utils.sorting_score(self.start, self.advertised_start, self.announcement)
 
-    @lazy
-    def grading_context(self):
-        """
-        This returns a dictionary with keys necessary for quickly grading
-        a student. They are used by grades.grade()
-
-        The grading context has two keys:
-        graded_sections - This contains the sections that are graded, as
-            well as all possible children modules that can affect the
-            grading. This allows some sections to be skipped if the student
-            hasn't seen any part of it.
-
-            The format is a dictionary keyed by section-type. The values are
-            arrays of dictionaries containing
-                "section_descriptor" : The section descriptor
-                "xmoduledescriptors" : An array of xmoduledescriptors that
-                    could possibly be in the section, for any student
-
-        all_descriptors - This contains a list of all xmodules that can
-            effect grading a student. This is used to efficiently fetch
-            all the xmodule state for a FieldDataCache without walking
-            the descriptor tree again.
-
-
-        """
-        # If this descriptor has been bound to a student, return the corresponding
-        # XModule. If not, just use the descriptor itself
-        try:
-            module = getattr(self, '_xmodule', None)
-            if not module:
-                module = self
-        except UndefinedContext:
-            module = self
-
-        def possibly_scored(usage_key):
-            """Can this XBlock type can have a score or children?"""
-            return usage_key.block_type in self.block_types_affecting_grading
-
-        all_descriptors = []
-        graded_sections = {}
-
-        def yield_descriptor_descendents(module_descriptor):
-            for child in module_descriptor.get_children(usage_key_filter=possibly_scored):
-                yield child
-                for module_descriptor in yield_descriptor_descendents(child):
-                    yield module_descriptor
-
-        for chapter in self.get_children():
-            for section in chapter.get_children():
-                if section.graded:
-                    xmoduledescriptors = list(yield_descriptor_descendents(section))
-                    xmoduledescriptors.append(section)
-
-                    # The xmoduledescriptors included here are only the ones that have scores.
-                    section_description = {
-                        'section_descriptor': section,
-                        'xmoduledescriptors': [child for child in xmoduledescriptors if child.has_score]
-                    }
-
-                    section_format = section.format if section.format is not None else ''
-                    graded_sections[section_format] = graded_sections.get(section_format, []) + [section_description]
-
-                    all_descriptors.extend(xmoduledescriptors)
-                    all_descriptors.append(section)
-
-        return {'graded_sections': graded_sections,
-                'all_descriptors': all_descriptors, }
-
-    @lazy
-    def block_types_affecting_grading(self):
-        """Return all block types that could impact grading (i.e. scored, or having children)."""
-        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)
-            )
-        )
-
     @staticmethod
     def make_id(org, course, url_name):
         return '/'.join([org, course, url_name])

lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py

@@ -51,7 +51,7 @@ class Command(BaseCommand):
 
         for cert in ungraded:
             # grade the student
-            grade = grades.grade(cert.user, request, course)
+            grade = grades.grade(cert.user, course)
             print "grading {0} - {1}".format(cert.user, grade['percent'])
             cert.grade = grade['percent']
             if not options['noop']:

lms/djangoapps/certificates/queue.py

@@ -257,7 +257,7 @@ class XQueueCertInterface(object):
         self.request.session = {}
 
         is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists()
-        grade = grades.grade(student, self.request, course)
+        grade = grades.grade(student, course)
         enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id)
         mode_is_verified = enrollment_mode in GeneratedCertificate.VERIFIED_CERTS_MODES
         user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student)

lms/djangoapps/courseware/model_data.py

@@ -940,7 +940,6 @@ class ScoresClient(object):
     Score = namedtuple('Score', 'correct total')
 
     def __init__(self, course_key, user_id):
-        """Basic constructor. from_field_data_cache() is more appopriate for most uses."""
         self.course_key = course_key
         self.user_id = user_id
         self._locations_to_scores = {}
@@ -983,10 +982,10 @@ class ScoresClient(object):
         return self._locations_to_scores.get(location.replace(version=None, branch=None))
 
     @classmethod
-    def from_field_data_cache(cls, fd_cache):
-        """Create a ScoresClient from a populated FieldDataCache."""
-        client = cls(fd_cache.course_id, fd_cache.user.id)
-        client.fetch_scores(fd_cache.scorable_locations)
+    def create_for_locations(cls, course_id, user_id, scorable_locations):
+        """Create a ScoresClient with pre-fetched data for the given locations."""
+        client = cls(course_id, user_id)
+        client.fetch_scores(scorable_locations)
         return client
 
 

lms/djangoapps/courseware/tests/test_grades.py

@@ -11,7 +11,6 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator
 
 from courseware.grades import (
-    field_data_cache_for_grading,
     grade,
     iterate_grades_for,
     MaxScoresCache,
@@ -31,7 +30,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 
 
-def _grade_with_errors(student, request, course, keep_raw_scores=False):
+def _grade_with_errors(student, course, keep_raw_scores=False):
     """This fake grade method will throw exceptions for student3 and
     student4, but allow any other students to go through normal grading.
 
@@ -42,7 +41,7 @@ def _grade_with_errors(student, request, course, keep_raw_scores=False):
     if student.username in ['student3', 'student4']:
         raise Exception("I don't like {}".format(student.username))
 
-    return grade(student, request, course, keep_raw_scores=keep_raw_scores)
+    return grade(student, course, keep_raw_scores=keep_raw_scores)
 
 
 @attr('shard_1')
@@ -217,15 +216,6 @@ class TestFieldDataCacheScorableLocations(SharedModuleStoreTestCase):
 
         CourseEnrollment.enroll(self.student, self.course.id)
 
-    def test_field_data_cache_scorable_locations(self):
-        """Only scorable locations should be in FieldDataCache.scorable_locations."""
-        fd_cache = field_data_cache_for_grading(self.course, self.student)
-        block_types = set(loc.block_type for loc in fd_cache.scorable_locations)
-        self.assertNotIn('video', block_types)
-        self.assertNotIn('html', block_types)
-        self.assertNotIn('discussion', block_types)
-        self.assertIn('problem', block_types)
-
 
 class TestProgressSummary(TestCase):
     """

lms/djangoapps/courseware/tests/test_submitting_problems.py

@@ -256,13 +256,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
         - grade_breakdown : A breakdown of the major components that
             make up the final grade. (For display)
         """
-
-        fake_request = self.factory.get(
-            reverse('progress', kwargs={'course_id': self.course.id.to_deprecated_string()})
-        )
-
-        fake_request.user = self.student_user
-        return grades.grade(self.student_user, fake_request, self.course)
+        return grades.grade(self.student_user, self.course)
 
     def get_progress_summary(self):
         """
@@ -275,15 +269,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
         ungraded problems, and is good for displaying a course summary with due dates,
         etc.
         """
-
-        fake_request = self.factory.get(
-            reverse('progress', kwargs={'course_id': self.course.id.to_deprecated_string()})
-        )
-
-        progress_summary = grades.progress_summary(
-            self.student_user, fake_request, self.course
-        )
-        return progress_summary
+        return grades.progress_summary(self.student_user, self.course)
 
     def check_grade_percent(self, percent):
         """

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -482,23 +482,7 @@ class TestGetHtmlMethod(BaseTestXmodule):
         # it'll just fall back to the values in the VideoDescriptor.
         self.assertIn("example_source.mp4", self.item_descriptor.render(STUDENT_VIEW).content)
 
-    @patch('edxval.api.get_video_info')
-    def test_get_html_with_mocked_edx_video_id(self, mock_get_video_info):
-        mock_get_video_info.return_value = {
-            'url': '/edxval/video/example',
-            'edx_video_id': u'example',
-            'duration': 111.0,
-            'client_video_id': u'The example video',
-            'encoded_videos': [
-                {
-                    'url': u'http://www.meowmix.com',
-                    'file_size': 25556,
-                    'bitrate': 9600,
-                    'profile': u'desktop_mp4'
-                }
-            ]
-        }
-
+    def test_get_html_with_mocked_edx_video_id(self):
         SOURCE_XML = """
             <video show_captions="true"
             display_name="A Name"
@@ -558,6 +542,22 @@ class TestGetHtmlMethod(BaseTestXmodule):
             edx_video_id=data['edx_video_id']
         )
         self.initialize_module(data=DATA)
+
+        with patch('edxval.api.get_video_info') as mock_get_video_info:
+            mock_get_video_info.return_value = {
+                'url': '/edxval/video/example',
+                'edx_video_id': u'example',
+                'duration': 111.0,
+                'client_video_id': u'The example video',
+                'encoded_videos': [
+                    {
+                        'url': u'http://www.meowmix.com',
+                        'file_size': 25556,
+                        'bitrate': 9600,
+                        'profile': u'desktop_mp4'
+                    }
+                ]
+            }
             context = self.item_descriptor.render(STUDENT_VIEW).content
 
         expected_context = dict(initial_context)

lms/djangoapps/courseware/views/views.py

@@ -38,6 +38,7 @@ from instructor.views.api import require_global_staff
 import shoppingcart
 import survey.utils
 import survey.views
+from lms.djangoapps.ccx.utils import prep_course_for_grading
 from certificates import api as certs_api
 from openedx.core.djangoapps.models.course_details import CourseDetails
 from commerce.utils import EcommerceService
@@ -681,6 +682,7 @@ def _progress(request, course_key, student_id):
             raise Http404
 
     course = get_course_with_access(request.user, 'load', course_key, depth=None, check_if_enrolled=True)
+    prep_course_for_grading(course, request)
 
     # check to see if there is a required survey that must be taken before
     # the user can access the course.
@@ -714,16 +716,8 @@ def _progress(request, course_key, student_id):
     # additional DB lookup (this kills the Progress page in particular).
     student = User.objects.prefetch_related("groups").get(id=student.id)
 
-    with outer_atomic():
-        field_data_cache = grades.field_data_cache_for_grading(course, student)
-        scores_client = ScoresClient.from_field_data_cache(field_data_cache)
-
-    courseware_summary = grades.progress_summary(
-        student, request, course, field_data_cache=field_data_cache, scores_client=scores_client
-    )
-    grade_summary = grades.grade(
-        student, request, course, field_data_cache=field_data_cache, scores_client=scores_client
-    )
+    courseware_summary = grades.progress_summary(student, course)
+    grade_summary = grades.grade(student, course)
     studio_url = get_studio_url(course, 'settings/grading')
 
     if courseware_summary is None:
@@ -1056,7 +1050,7 @@ def is_course_passed(course, grade_summary=None, student=None, request=None):
     success_cutoff = min(nonzero_cutoffs) if nonzero_cutoffs else None
 
     if grade_summary is None:
-        grade_summary = grades.grade(student, request, course)
+        grade_summary = grades.grade(student, course)
 
     return success_cutoff and grade_summary['percent'] >= success_cutoff
 

lms/djangoapps/instructor/features/data_download.py

@@ -63,7 +63,7 @@ Graded sections:
 Listing grading context for course {}
 graded sections:
 []
-all descriptors:
+all graded blocks:
 length=0""".format(world.course_key)
     assert_in(expected_config, world.css_text('#data-grade-config-text'))
 

lms/djangoapps/instructor/offline_gradecalc.py

@@ -50,7 +50,7 @@ def offline_grade_calculation(course_key):
         request.user = student
         request.session = {}
 
-        gradeset = grades.grade(student, request, course, keep_raw_scores=True)
+        gradeset = grades.grade(student, course, keep_raw_scores=True)
         # Convert Score namedtuples to dicts:
         totaled_scores = gradeset['totaled_scores']
         for section in totaled_scores:
@@ -89,7 +89,7 @@ def student_grades(student, request, course, keep_raw_scores=False, use_offline=
     as use_offline.  If use_offline is True then this will look for an offline computed gradeset in the DB.
     '''
     if not use_offline:
-        return grades.grade(student, request, course, keep_raw_scores=keep_raw_scores)
+        return grades.grade(student, course, keep_raw_scores=keep_raw_scores)
 
     try:
         ocg = models.OfflineComputedGrade.objects.get(user=student, course_id=course.id)

lms/djangoapps/instructor/tests/test_offline_gradecalc.py

@@ -16,7 +16,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
 from ..offline_gradecalc import offline_grade_calculation, student_grades
 
 
-def mock_grade(_student, _request, course, **_kwargs):
+def mock_grade(_student, course, **_kwargs):
     """ Return some fake grade data to mock grades.grade() """
     return {
         'grade': u'Pass',
@@ -104,4 +104,4 @@ class TestOfflineGradeCalc(ModuleStoreTestCase):
         offline_grade_calculation(self.course.id)
         with patch('courseware.grades.grade', side_effect=AssertionError('Should not re-grade')):
             result = student_grades(self.user, None, self.course, use_offline=True)
-        self.assertEqual(result, mock_grade(self.user, None, self.course))
+        self.assertEqual(result, mock_grade(self.user, self.course))

lms/djangoapps/instructor_analytics/basic.py

@@ -24,6 +24,7 @@ from courseware.models import StudentModule
 from certificates.models import GeneratedCertificate
 from django.db.models import Count
 from certificates.models import CertificateStatuses
+from courseware.grades import grading_context_for_course
 
 
 STUDENT_FEATURES = ('id', 'username', 'first_name', 'last_name', 'is_staff', 'email')
@@ -490,14 +491,14 @@ def dump_grading_context(course):
     msg += hbar
     msg += "Listing grading context for course %s\n" % course.id.to_deprecated_string()
 
-    gcontext = course.grading_context
+    gcontext = grading_context_for_course(course)
     msg += "graded sections:\n"
 
-    msg += '%s\n' % gcontext['graded_sections'].keys()
-    for (gsomething, gsvals) in gcontext['graded_sections'].items():
+    msg += '%s\n' % gcontext['all_graded_sections'].keys()
+    for (gsomething, gsvals) in gcontext['all_graded_sections'].items():
         msg += "--> Section %s:\n" % (gsomething)
         for sec in gsvals:
-            sdesc = sec['section_descriptor']
+            sdesc = sec['section_block']
             frmat = getattr(sdesc, 'format', None)
             aname = ''
             if frmat in graders:
@@ -512,7 +513,7 @@ def dump_grading_context(course):
                 notes = ', score by attempt!'
             msg += "      %s (format=%s, Assignment=%s%s)\n"\
                 % (sdesc.display_name, frmat, aname, notes)
-    msg += "all descriptors:\n"
-    msg += "length=%d\n" % len(gcontext['all_descriptors'])
+    msg += "all graded blocks:\n"
+    msg += "length=%d\n" % len(gcontext['all_graded_blocks'])
     msg = '<pre>%s</pre>' % msg.replace('<', '&lt;')
     return msg

lms/djangoapps/instructor_task/tests/test_tasks_helper.py

@@ -285,7 +285,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
             user_b.username,
             course.id,
             cohort_name_header,
-            ''
+            u'Default Group',
         )
 
     @patch('instructor_task.tasks_helper._get_current_task')
@@ -685,7 +685,7 @@ class TestProblemReportSplitTestContent(TestReportMixin, TestConditionalContent,
 
     def test_problem_grade_report(self):
         """
-        Test that we generate the correct the correct grade report when dealing with A/B tests.
+        Test that we generate the correct grade report when dealing with A/B tests.
 
         In order to verify that the behavior of the grade report is correct, we submit answers for problems
         that the student won't have access to. A/B tests won't restrict access to the problems, but it should

openedx/core/lib/cache_utils.py

 """
 Utilities related to caching.
 """
+import collections
 import cPickle as pickle
 import functools
 import zlib
@@ -40,6 +41,48 @@ def memoize_in_request_cache(request_cache_attr_name=None):
     return _decorator
 
 
+class memoized(object):  # pylint: disable=invalid-name
+    """
+    Decorator. Caches a function's return value each time it is called.
+    If called later with the same arguments, the cached value is returned
+    (not reevaluated).
+    https://wiki.python.org/moin/PythonDecoratorLibrary#Memoize
+
+    WARNING: Only use this memoized decorator for caching data that
+    is constant throughout the lifetime of a gunicorn worker process,
+    is costly to compute, and is required often.  Otherwise, it can lead to
+    unwanted memory leakage.
+    """
+
+    def __init__(self, func):
+        self.func = func
+        self.cache = {}
+
+    def __call__(self, *args):
+        if not isinstance(args, collections.Hashable):
+            # uncacheable. a list, for instance.
+            # better to not cache than blow up.
+            return self.func(*args)
+        if args in self.cache:
+            return self.cache[args]
+        else:
+            value = self.func(*args)
+            self.cache[args] = value
+            return value
+
+    def __repr__(self):
+        """
+        Return the function's docstring.
+        """
+        return self.func.__doc__
+
+    def __get__(self, obj, objtype):
+        """
+        Support instance methods.
+        """
+        return functools.partial(self.__call__, obj)
+
+
 def hashvalue(arg):
     """
     If arg is an xblock, use its location. otherwise just turn it into a string