Merge pull request #13093 from edx/beryl/grading

Use Persistent Subsection Scores in Grading

cms/envs/bok_choy.py

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

cms/envs/common.py

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

cms/envs/test.py

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

common/lib/xmodule/xmodule/graders.py

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

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

@@ -227,5 +227,6 @@ def expected_settings_names(self):
             'instructor_info',
             'create_zendesk_tickets',
             'ccx_connector',
-            'enable_ccx'
+            'enable_ccx',
+            'enable_subsection_grades_saved',
         ]

lms/djangoapps/ccx/views.py

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

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

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

lms/djangoapps/grades/models.py

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

lms/djangoapps/grades/new/course_grade.py

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