diff --git a/lms/djangoapps/grades/course_grades.py b/lms/djangoapps/grades/course_grades.py index 0607ff289c..cc5ab1f5e2 100644 --- a/lms/djangoapps/grades/course_grades.py +++ b/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): diff --git a/lms/djangoapps/grades/migrations/0001_initial.py b/lms/djangoapps/grades/migrations/0001_initial.py new file mode 100644 index 0000000000..20f218dc3f --- /dev/null +++ b/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')]), + ), + ] diff --git a/lms/djangoapps/grades/migrations/__init__.py b/lms/djangoapps/grades/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py new file mode 100644 index 0000000000..8012286f46 --- /dev/null +++ b/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() diff --git a/lms/djangoapps/grades/tests/test_models.py b/lms/djangoapps/grades/tests/test_models.py new file mode 100644 index 0000000000..1238507fd9 --- /dev/null +++ b/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) diff --git a/lms/envs/common.py b/lms/envs/common.py index 7d801363b0..dd95060382 100644 --- a/lms/envs/common.py +++ b/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',