Merge pull request #13143 from edx/efischer/grades

Persistent Grades Data Model

lms/djangoapps/grades/course_grades.py

@@ -14,7 +14,7 @@ from .new.course_grade import CourseGradeFactory
 log = getLogger(__name__)
 
 
-GradeResult = namedtuple('StudentGrade', ['student', 'gradeset', 'err_msg'])
+GradeResult = namedtuple('GradeResult', ['student', 'gradeset', 'err_msg'])
 
 
 def iterate_grades_for(course_or_id, students):

lms/djangoapps/grades/migrations/0001_initial.py

@@ -0,0 +1,51 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import coursewarehistoryextended.fields
+import django.utils.timezone
+import model_utils.fields
+import xmodule_django.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='PersistentSubsectionGrade',
+            fields=[
+                ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, verbose_name='created', editable=False)),
+                ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, verbose_name='modified', editable=False)),
+                ('id', coursewarehistoryextended.fields.UnsignedBigIntAutoField(serialize=False, primary_key=True)),
+                ('user_id', models.IntegerField()),
+                ('course_id', xmodule_django.models.CourseKeyField(max_length=255)),
+                ('usage_key', xmodule_django.models.UsageKeyField(max_length=255)),
+                ('subtree_edited_date', models.DateTimeField(verbose_name=b'last content edit timestamp')),
+                ('course_version', models.CharField(max_length=255, verbose_name=b'guid of latest course version', blank=True)),
+                ('earned_all', models.FloatField()),
+                ('possible_all', models.FloatField()),
+                ('earned_graded', models.FloatField()),
+                ('possible_graded', models.FloatField()),
+            ],
+        ),
+        migrations.CreateModel(
+            name='VisibleBlocks',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('blocks_json', models.TextField()),
+                ('hashed', models.CharField(unique=True, max_length=100)),
+            ],
+        ),
+        migrations.AddField(
+            model_name='persistentsubsectiongrade',
+            name='visible_blocks',
+            field=models.ForeignKey(to='grades.VisibleBlocks', db_column=b'visible_blocks_hash', to_field=b'hashed'),
+        ),
+        migrations.AlterUniqueTogether(
+            name='persistentsubsectiongrade',
+            unique_together=set([('course_id', 'user_id', 'usage_key')]),
+        ),
+    ]

lms/djangoapps/grades/models.py

@@ -0,0 +1,328 @@
+"""
+Models used for robust grading.
+
+Robust grading allows student scores to be saved per-subsection independent
+of any changes that may occur to the course after the score is achieved.
+"""
+
+from base64 import b64encode
+from collections import namedtuple
+from hashlib import sha1
+import json
+import logging
+from operator import attrgetter
+
+from django.db import models
+from django.db.utils import IntegrityError
+from model_utils.models import TimeStampedModel
+
+from coursewarehistoryextended.fields import UnsignedBigIntAutoField
+from xmodule_django.models import CourseKeyField, UsageKeyField
+
+
+log = logging.getLogger(__name__)
+
+
+# Used to serialize information about a block at the time it was used in
+# grade calculation.
+BlockRecord = namedtuple('BlockRecord', ['locator', 'weight', 'max_score'])
+
+
+class BlockRecordSet(frozenset):
+    """
+    An immutable ordered collection of BlockRecord objects.
+    """
+
+    def __init__(self, *args, **kwargs):
+        super(BlockRecordSet, self).__init__(*args, **kwargs)
+        self._json = None
+        self._hash = None
+
+    def to_json(self):
+        """
+        Return a JSON-serialized version of the list of block records, using a
+        stable ordering.
+        """
+        if self._json is None:
+            sorted_blocks = sorted(self, key=attrgetter('locator'))
+            # Remove spaces from separators for more compact representation
+            self._json = json.dumps(
+                [block._asdict() for block in sorted_blocks],
+                separators=(',', ':'),
+                sort_keys=True,
+            )
+        return self._json
+
+    @classmethod
+    def from_json(cls, blockrecord_json):
+        """
+        Return a BlockRecordSet from a json list.
+        """
+        block_dicts = json.loads(blockrecord_json)
+        record_generator = (BlockRecord(**block) for block in block_dicts)
+        return cls(record_generator)
+
+    def to_hash(self):
+        """
+        Return a hashed version of the list of block records.
+
+        This currently hashes using sha1, and returns a base64 encoded version
+        of the binary digest.  In the future, different algorithms could be
+        supported by adding a label indicated which algorithm was used, e.g.,
+        "sha256$j0NDRmSPa5bfid2pAcUXaxCm2Dlh3TwayItZstwyeqQ=".
+        """
+        if self._hash is None:
+            self._hash = b64encode(sha1(self.to_json()).digest())
+        return self._hash
+
+
+class VisibleBlocksQuerySet(models.QuerySet):
+    """
+    A custom QuerySet representing VisibleBlocks.
+    """
+
+    def create_from_blockrecords(self, blocks):
+        """
+        Creates a new VisibleBlocks model object.
+
+        Argument 'blocks' should be a BlockRecordSet.
+        """
+
+        if not isinstance(blocks, BlockRecordSet):
+            blocks = BlockRecordSet(blocks)
+
+        model, _ = self.get_or_create(hashed=blocks.to_hash(), defaults={'blocks_json': blocks.to_json()})
+        return model
+
+    def hash_from_blockrecords(self, blocks):
+        """
+        Return the hash for a given BlockRecordSet, serializing the records if
+        possible, but returning the hash even if an IntegrityError occurs.
+        """
+
+        if not isinstance(blocks, BlockRecordSet):
+            blocks = BlockRecordSet(blocks)
+
+        try:
+            model = self.create_from_blockrecords(blocks)
+        except IntegrityError:
+            # If an integrity error occurs, the VisibleBlocks model we want to
+            # create already exists.  The hash is still the correct value.
+            return blocks.to_hash()
+        else:
+            # No error occurred
+            return model.hashed
+
+
+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
+    in the blocks_json field. A hash of this json array is used for lookup
+    purposes.
+    """
+    blocks_json = models.TextField()
+    hashed = models.CharField(max_length=100, unique=True)
+
+    objects = VisibleBlocksQuerySet.as_manager()
+
+    def __unicode__(self):
+        """
+        String representation of this model.
+        """
+        return u"VisibleBlocks object - hash:{}, raw json:'{}'".format(self.hashed, self.blocks_json)
+
+    @property
+    def blocks(self):
+        """
+        Returns the blocks_json data stored on this model as a list of
+        BlockRecords in the order they were provided.
+        """
+        return BlockRecordSet.from_json(self.blocks_json)
+
+
+class PersistentSubsectionGradeQuerySet(models.QuerySet):
+    """
+    A custom QuerySet, that handles creating a VisibleBlocks model on creation, and
+    extracts the course id from the provided usage_key.
+    """
+    def create(self, **kwargs):
+        """
+        Instantiates a new model instance after creating a VisibleBlocks instance.
+
+        Arguments:
+            user_id (int)
+            usage_key (serialized UsageKey)
+            course_version (str)
+            subtree_edited_date (datetime)
+            earned_all (float)
+            possible_all (float)
+            earned_graded (float)
+            possible_graded (float)
+            visible_blocks (iterable of BlockRecord)
+        """
+        visible_blocks = kwargs.pop('visible_blocks')
+
+        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
+        )
+        grade.full_clean()
+        grade.save()
+        return grade
+
+
+class PersistentSubsectionGrade(TimeStampedModel):
+    """
+    A django model tracking persistent grades at the subsection level.
+    """
+
+    class Meta(object):
+        unique_together = [
+            # * Specific grades can be pulled using all three columns,
+            # * Progress page can pull all grades for a given (course_id, user_id)
+            # * Course staff can see all grades for a course using (course_id,)
+            ('course_id', 'user_id', 'usage_key'),
+        ]
+
+    # primary key will need to be large for this table
+    id = UnsignedBigIntAutoField(primary_key=True)  # pylint: disable=invalid-name
+
+    # uniquely identify this particular grade object
+    user_id = models.IntegerField(blank=False)
+    course_id = CourseKeyField(blank=False, max_length=255)
+    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)
+    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.
+    # graded refers to the subset of all problems that are marked as being graded.
+    earned_all = models.FloatField(blank=False)
+    possible_all = models.FloatField(blank=False)
+    earned_graded = models.FloatField(blank=False)
+    possible_graded = models.FloatField(blank=False)
+
+    # track which blocks were visible at the time of grade calculation
+    visible_blocks = models.ForeignKey(VisibleBlocks, db_column='visible_blocks_hash', to_field='hashed')
+
+    # use custom manager
+    objects = PersistentSubsectionGradeQuerySet.as_manager()
+
+    def __unicode__(self):
+        """
+        Returns a string representation of this model.
+        """
+        return u"{} user: {}, course version: {}, subsection {} ({}). {}/{} graded, {}/{} all".format(
+            type(self).__name__,
+            self.user_id,
+            self.course_version,
+            self.usage_key,
+            self.visible_blocks.hashed,
+            self.earned_graded,
+            self.possible_graded,
+            self.earned_all,
+            self.possible_all,
+        )
+
+    @classmethod
+    def save_grade(cls, **kwargs):
+        """
+        Wrapper for create_grade or update_grade, depending on which applies.
+        Takes the same arguments as both of those methods.
+        """
+        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)
+        except IntegrityError:
+            is_created = False
+        if not is_created:
+            grade.update(**kwargs)
+
+    @classmethod
+    def read_grade(cls, user_id, usage_key):
+        """
+        Reads a grade from database
+
+        Arguments:
+            user_id: The user associated with the desired grade
+            usage_key: The location of the subsection associated with the desired grade
+
+        Raises PersistentSubsectionGrade.DoesNotExist if applicable
+        """
+        return cls.objects.get(
+            user_id=user_id,
+            course_id=usage_key.course_key,  # course_id is included to take advantage of db indexes
+            usage_key=usage_key,
+        )
+
+    @classmethod
+    def update_grade(
+            cls,
+            user_id,
+            usage_key,
+            course_version,
+            subtree_edited_date,
+            earned_all,
+            possible_all,
+            earned_graded,
+            possible_graded,
+            visible_blocks,
+    ):
+        """
+        Updates a previously existing grade.
+
+        This is distinct from update() in that `grade.update()` operates on an
+        existing grade object, while this is a classmethod that pulls the grade
+        from the database, and then updates it.  If you already have a grade
+        object, use the update() method on that object to avoid an extra
+        round-trip to the database.  Use this classmethod if all you have are a
+        user and the usage key of an existing grade.
+
+        Requires all the arguments listed in docstring for create_grade
+        """
+        grade = cls.read_grade(
+            user_id=user_id,
+            usage_key=usage_key,
+        )
+
+        grade.update(
+            course_version=course_version,
+            subtree_edited_date=subtree_edited_date,
+            earned_all=earned_all,
+            possible_all=possible_all,
+            earned_graded=earned_graded,
+            possible_graded=possible_graded,
+            visible_blocks=visible_blocks,
+        )
+
+    def update(
+            self,
+            course_version,
+            subtree_edited_date,
+            earned_all,
+            possible_all,
+            earned_graded,
+            possible_graded,
+            visible_blocks,
+    ):
+        """
+        Modify an existing PersistentSubsectionGrade object, saving the new
+        version.
+        """
+        visible_blocks_hash = VisibleBlocks.objects.hash_from_blockrecords(blocks=visible_blocks)
+
+        self.course_version = course_version
+        self.subtree_edited_date = subtree_edited_date
+        self.earned_all = earned_all
+        self.possible_all = possible_all
+        self.earned_graded = earned_graded
+        self.possible_graded = possible_graded
+        self.visible_blocks_id = visible_blocks_hash  # pylint: disable=attribute-defined-outside-init
+        self.save()

lms/djangoapps/grades/tests/test_models.py

@@ -0,0 +1,202 @@
+"""
+Unit tests for grades models.
+"""
+from base64 import b64encode
+from collections import OrderedDict
+import ddt
+from hashlib import sha1
+import json
+from mock import patch
+
+from django.core.exceptions import ValidationError
+from django.test import TestCase
+from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator
+
+from lms.djangoapps.grades.models import BlockRecord, BlockRecordSet, PersistentSubsectionGrade, VisibleBlocks
+
+
+class GradesModelTestCase(TestCase):
+    """
+    Base class for common setup of grades model tests.
+    """
+    def setUp(self):
+        super(GradesModelTestCase, self).setUp()
+        self.course_key = CourseLocator(
+            org='some_org',
+            course='some_course',
+            run='some_run'
+        )
+        self.locator_a = BlockUsageLocator(
+            course_key=self.course_key,
+            block_type='problem',
+            block_id='block_id_a'
+        )
+        self.locator_b = BlockUsageLocator(
+            course_key=self.course_key,
+            block_type='problem',
+            block_id='block_id_b'
+        )
+        self.record_a = BlockRecord(unicode(self.locator_a), 1, 10)
+        self.record_b = BlockRecord(unicode(self.locator_b), 1, 10)
+
+
+@ddt.ddt
+class BlockRecordTest(GradesModelTestCase):
+    """
+    Test the BlockRecord model.
+    """
+    def setUp(self):
+        super(BlockRecordTest, self).setUp()
+
+    def test_creation(self):
+        """
+        Tests creation of a BlockRecord.
+        """
+        weight = 1
+        max_score = 10
+        record = BlockRecord(
+            self.locator_a,
+            weight,
+            max_score,
+        )
+        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"),
+    )
+    @ddt.unpack
+    def test_serialization(self, weight, max_score, block_key):
+        """
+        Tests serialization of a BlockRecord using the to_dict() method.
+        """
+        record = BlockRecord(block_key, weight, max_score)
+        expected = OrderedDict([
+            ("locator", block_key),
+            ("weight", weight),
+            ("max_score", max_score),
+        ])
+        self.assertEqual(expected, record._asdict())
+
+
+class VisibleBlocksTest(GradesModelTestCase):
+    """
+    Test the VisibleBlocks model.
+    """
+    def test_creation(self):
+        """
+        Happy path test to ensure basic create functionality works as expected.
+        """
+        vblocks = VisibleBlocks.objects.create_from_blockrecords([self.record_a])
+        expected_json = json.dumps([self.record_a._asdict()], separators=(',', ':'), sort_keys=True)
+        expected_hash = b64encode(sha1(expected_json).digest())
+        self.assertEqual(expected_json, vblocks.blocks_json)
+        self.assertEqual(expected_hash, vblocks.hashed)
+
+    def test_ordering_does_not_matter(self):
+        """
+        When creating new vblocks, a different ordering of blocks produces the
+        same record in the database.
+        """
+        stored_vblocks = VisibleBlocks.objects.create_from_blockrecords([self.record_a, self.record_b])
+        repeat_vblocks = VisibleBlocks.objects.create_from_blockrecords([self.record_b, self.record_a])
+        new_vblocks = VisibleBlocks.objects.create_from_blockrecords([self.record_b])
+
+        self.assertEqual(stored_vblocks.pk, repeat_vblocks.pk)
+        self.assertEqual(stored_vblocks.hashed, repeat_vblocks.hashed)
+
+        self.assertNotEqual(stored_vblocks.pk, new_vblocks.pk)
+        self.assertNotEqual(stored_vblocks.hashed, new_vblocks.hashed)
+
+    def test_blocks_property(self):
+        """
+        Ensures that, given an array of BlockRecord, creating visible_blocks and accessing visible_blocks.blocks yields
+        a copy of the initial array. Also, trying to set the blocks property should raise an exception.
+        """
+        expected_blocks = [self.record_a, self.record_b]
+        visible_blocks = VisibleBlocks.objects.create_from_blockrecords(expected_blocks)
+        self.assertEqual(BlockRecordSet(expected_blocks), visible_blocks.blocks)
+        with self.assertRaises(AttributeError):
+            visible_blocks.blocks = expected_blocks
+
+
+@ddt.ddt
+class PersistentSubsectionGradeTest(GradesModelTestCase):
+    """
+    Test the PersistentSubsectionGrade model.
+    """
+    def setUp(self):
+        super(PersistentSubsectionGradeTest, self).setUp()
+        self.usage_key = BlockUsageLocator(
+            course_key=self.course_key,
+            block_type='subsection',
+            block_id='subsection_12345',
+        )
+        self.params = {
+            "user_id": 12345,
+            "usage_key": self.usage_key,
+            "course_version": "deadbeef",
+            "subtree_edited_date": "2016-08-01 18:53:24.354741",
+            "earned_all": 6,
+            "possible_all": 12,
+            "earned_graded": 6,
+            "possible_graded": 8,
+            "visible_blocks": [self.record_a, self.record_b],
+        }
+
+    def test_create(self):
+        """
+        Tests model creation, and confirms error when trying to recreate model.
+        """
+        created_grade = PersistentSubsectionGrade.objects.create(**self.params)
+        read_grade = PersistentSubsectionGrade.read_grade(
+            user_id=self.params["user_id"],
+            usage_key=self.params["usage_key"],
+        )
+        self.assertEqual(created_grade, read_grade)
+        with self.assertRaises(ValidationError):
+            created_grade = PersistentSubsectionGrade.objects.create(**self.params)
+
+    def test_create_bad_params(self):
+        """
+        Confirms create will fail if params are missing.
+        """
+        del self.params["earned_graded"]
+        with self.assertRaises(ValidationError):
+            PersistentSubsectionGrade.objects.create(**self.params)
+
+    def test_course_version_is_optional(self):
+        del self.params["course_version"]
+        PersistentSubsectionGrade.objects.create(**self.params)
+
+    def test_update_grade(self):
+        """
+        Tests model update, and confirms error when updating a nonexistent model.
+        """
+        with self.assertRaises(PersistentSubsectionGrade.DoesNotExist):
+            PersistentSubsectionGrade.update_grade(**self.params)
+        PersistentSubsectionGrade.objects.create(**self.params)
+        self.params['earned_all'] = 12
+        self.params['earned_graded'] = 8
+        PersistentSubsectionGrade.update_grade(**self.params)
+        read_grade = PersistentSubsectionGrade.read_grade(
+            user_id=self.params["user_id"],
+            usage_key=self.params["usage_key"],
+        )
+        self.assertEqual(read_grade.earned_all, 12)
+        self.assertEqual(read_grade.earned_graded, 8)
+
+    @ddt.data(True, False)
+    def test_save(self, already_created):
+        if already_created:
+            PersistentSubsectionGrade.objects.create(**self.params)
+        module_prefix = "lms.djangoapps.grades.models.PersistentSubsectionGrade."
+        with patch(
+            module_prefix + "objects.get_or_create",
+            wraps=PersistentSubsectionGrade.objects.get_or_create
+        ) as mock_get_or_create:
+            with patch(module_prefix + "update") as mock_update:
+                PersistentSubsectionGrade.save_grade(**self.params)
+                self.assertTrue(mock_get_or_create.called)
+                self.assertEqual(mock_update.called, already_created)

lms/envs/common.py

@@ -1891,7 +1891,7 @@ INSTALLED_APPS = (
     'openedx.core.djangoapps.course_groups',
     'bulk_email',
     'branding',
-    'grades',
+    'lms.djangoapps.grades',
 
     # Student support tools
     'support',