Merge pull request #13437 from edx/cdyer/keep-course-run

Ensure usage keys have course run in locators.

lms/djangoapps/grades/models.py

@@ -17,7 +17,7 @@ from django.db.utils import IntegrityError
 from model_utils.models import TimeStampedModel
 
 from coursewarehistoryextended.fields import UnsignedBigIntAutoField
-from opaque_keys.edx.locator import BlockUsageLocator
+from opaque_keys.edx.keys import CourseKey, UsageKey
 from xmodule_django.models import CourseKeyField, UsageKeyField
 
 
@@ -39,6 +39,17 @@ class BlockRecordSet(frozenset):
         self._json = None
         self._hash = None
 
+    def _get_course_key_string(self):
+        """
+        Get the course key as a string.  All blocks are from the same course,
+        so just grab one arbitrarily.  If no blocks are present, return None.
+        """
+        if self:
+            a_block = next(iter(self))
+            return unicode(a_block.locator.course_key)
+        else:
+            return None
+
     def to_json(self):
         """
         Return a JSON-serialized version of the list of block records, using a
@@ -47,12 +58,18 @@ class BlockRecordSet(frozenset):
         if self._json is None:
             sorted_blocks = sorted(self, key=attrgetter('locator'))
             list_of_block_dicts = [block._asdict() for block in sorted_blocks]
+            course_key_string = self._get_course_key_string()  # all blocks are from the same course
+
             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
+            data = {
+                'course_key': course_key_string,
+                'blocks': list_of_block_dicts,
+            }
+
             self._json = json.dumps(
-                list_of_block_dicts,
-                separators=(',', ':'),
+                data,
+                separators=(',', ':'),  # Remove spaces from separators for more compact representation
                 sort_keys=True,
             )
         return self._json
@@ -62,10 +79,17 @@ class BlockRecordSet(frozenset):
         """
         Return a BlockRecordSet from a json list.
         """
-        block_dicts = json.loads(blockrecord_json)
+        data = json.loads(blockrecord_json)
+        course_key = data['course_key']
+        if course_key is not None:
+            course_key = CourseKey.from_string(course_key)
+        else:
+            # If there was no course key, there are no blocks.
+            assert len(data['blocks']) == 0
+        block_dicts = data['blocks']
         record_generator = (
             BlockRecord(
-                locator=BlockUsageLocator.from_string(block["locator"]),
+                locator=UsageKey.from_string(block["locator"]).replace(course_key=course_key),
                 weight=block["weight"],
                 max_score=block["max_score"],
             )

lms/djangoapps/grades/tests/test_models.py

@@ -20,6 +20,25 @@ from lms.djangoapps.grades.models import (
 )
 
 
+class BlockRecordSetTestCase(TestCase):
+    """
+    Verify the behavior of BlockRecordSets, particularly around edge cases
+    """
+    empty_json = '{"blocks":[],"course_key":null}'
+
+    def test_empty_block_record_set(self):
+        brs = BlockRecordSet()
+        self.assertFalse(brs)
+        self.assertEqual(
+            brs.to_json(),
+            self.empty_json
+        )
+        self.assertEqual(
+            BlockRecordSet.from_json(self.empty_json),
+            brs
+        )
+
+
 class GradesModelTestCase(TestCase):
     """
     Base class for common setup of grades model tests.
@@ -41,8 +60,8 @@ class GradesModelTestCase(TestCase):
             block_type='problem',
             block_id='block_id_b'
         )
-        self.record_a = BlockRecord(self.locator_a, 1, 10)
-        self.record_b = BlockRecord(self.locator_b, 1, 10)
+        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)
 
 
 @ddt.ddt
@@ -97,8 +116,15 @@ class VisibleBlocksTest(GradesModelTestCase):
         list_of_block_dicts = [self.record_a._asdict()]
         for block_dict in list_of_block_dicts:
             block_dict['locator'] = unicode(block_dict['locator'])  # BlockUsageLocator is not json-serializable
-        expected_json = json.dumps(list_of_block_dicts, separators=(',', ':'), sort_keys=True)
+        expected_data = {
+            'course_key': unicode(self.record_a.locator.course_key),
+            'blocks': [
+                {'locator': unicode(self.record_a.locator), 'max_score': 10, 'weight': 1},
+            ],
+        }
+        expected_json = json.dumps(expected_data, separators=(',', ':'), sort_keys=True)
         expected_hash = b64encode(sha1(expected_json).digest())
+        self.assertEqual(expected_data, json.loads(vblocks.blocks_json))
         self.assertEqual(expected_json, vblocks.blocks_json)
         self.assertEqual(expected_hash, vblocks.hashed)