Merge pull request #25 from edx/diana/score-annotations

Add the ability to annotate scores.

AUTHORS

@@ -3,3 +3,4 @@ Will Daly <will@edx.org>
 David Ormsbee <dave@edx.org>
 Stephen Sanchez <steve@edx.org>
 Phil McGachey <phil_mcgachey@harvard.edu>
+Diana Huang <dkh@edx.org>

submissions/api.py

@@ -10,13 +10,13 @@ import json
 
 from django.conf import settings
 from django.core.cache import cache
-from django.db import IntegrityError, DatabaseError
+from django.db import IntegrityError, DatabaseError, transaction
 from dogapi import dog_stats_api
 
 from submissions.serializers import (
     SubmissionSerializer, StudentItemSerializer, ScoreSerializer
 )
-from submissions.models import Submission, StudentItem, Score, ScoreSummary, score_set, score_reset
+from submissions.models import Submission, StudentItem, Score, ScoreSummary, ScoreAnnotation, score_set, score_reset
 
 logger = logging.getLogger("submissions.api")
 
@@ -698,7 +698,8 @@ def reset_score(student_id, course_id, item_id):
         logger.info(msg)
 
 
-def set_score(submission_uuid, points_earned, points_possible):
+def set_score(submission_uuid, points_earned, points_possible,
+              annotation_creator=None, annotation_type=None, annotation_reason=None):
     """Set a score for a particular submission.
 
     Sets the score for a particular submission. This score is calculated
@@ -709,6 +710,11 @@ def set_score(submission_uuid, points_earned, points_possible):
         points_earned (int): The earned points for this submission.
         points_possible (int): The total points possible for this particular student item.
 
+        annotation_creator (str): An optional field for recording who gave this particular score
+        annotation_type (str): An optional field for recording what type of annotation should be created,
+                                e.g. "staff_override".
+        annotation_reason (str): An optional field for recording why this score was set to its value.
+
     Returns:
         None
 
@@ -761,9 +767,19 @@ def set_score(submission_uuid, points_earned, points_possible):
     # even though we cannot retrieve it.
     # In this case, we assume that someone else has already created
     # a score summary and ignore the error.
+    # TODO: once we're using Django 1.8, use transactions to ensure that these
+    # two models are saved at the same time.
     try:
         score_model = score.save()
         _log_score(score_model)
+        if annotation_creator is not None:
+            score_annotation = ScoreAnnotation(
+                score=score_model,
+                creator=annotation_creator,
+                annotation_type=annotation_type,
+                reason=annotation_reason
+            )
+            score_annotation.save()
         # Send a signal out to any listeners who are waiting for scoring events.
         score_set.send(
             sender=None,

submissions/migrations/0006_auto__add_scoreannotation__chg_field_studentitem_student_id.py

+# -*- coding: utf-8 -*-
+from south.utils import datetime_utils as datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding model 'ScoreAnnotation'
+        db.create_table('submissions_scoreannotation', (
+            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+            ('score', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['submissions.Score'])),
+            ('annotation_type', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)),
+            ('creator', self.gf('submissions.models.AnonymizedUserIDField')(max_length=255, db_index=True)),
+            ('reason', self.gf('django.db.models.fields.TextField')()),
+        ))
+        db.send_create_signal('submissions', ['ScoreAnnotation'])
+
+
+        # Changing field 'StudentItem.student_id'
+        db.alter_column('submissions_studentitem', 'student_id', self.gf('submissions.models.AnonymizedUserIDField')(max_length=255))
+
+    def backwards(self, orm):
+        # Deleting model 'ScoreAnnotation'
+        db.delete_table('submissions_scoreannotation')
+
+
+        # Changing field 'StudentItem.student_id'
+        db.alter_column('submissions_studentitem', 'student_id', self.gf('django.db.models.fields.CharField')(max_length=255))
+
+    models = {
+        'submissions.score': {
+            'Meta': {'object_name': 'Score'},
+            'created_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'points_earned': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
+            'points_possible': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
+            'reset': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'student_item': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.StudentItem']"}),
+            'submission': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.Submission']", 'null': 'True'})
+        },
+        'submissions.scoreannotation': {
+            'Meta': {'object_name': 'ScoreAnnotation'},
+            'annotation_type': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'creator': ('submissions.models.AnonymizedUserIDField', [], {'max_length': '255', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'reason': ('django.db.models.fields.TextField', [], {}),
+            'score': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.Score']"})
+        },
+        'submissions.scoresummary': {
+            'Meta': {'object_name': 'ScoreSummary'},
+            'highest': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['submissions.Score']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'latest': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['submissions.Score']"}),
+            'student_item': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.StudentItem']", 'unique': 'True'})
+        },
+        'submissions.studentitem': {
+            'Meta': {'unique_together': "(('course_id', 'student_id', 'item_id'),)", 'object_name': 'StudentItem'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'item_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'item_type': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'student_id': ('submissions.models.AnonymizedUserIDField', [], {'max_length': '255', 'db_index': 'True'})
+        },
+        'submissions.submission': {
+            'Meta': {'ordering': "['-submitted_at', '-id']", 'object_name': 'Submission'},
+            'answer': ('jsonfield.fields.JSONField', [], {'db_column': "'raw_answer'", 'blank': 'True'}),
+            'attempt_number': ('django.db.models.fields.PositiveIntegerField', [], {}),
+            'created_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'student_item': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['submissions.StudentItem']"}),
+            'submitted_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
+            'uuid': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '36', 'blank': 'True'})
+        }
+    }
+
+    complete_apps = ['submissions']
\ No newline at end of file

submissions/models.py

@@ -11,6 +11,7 @@ need to then generate a matching migration for it using:
 """
 import logging
 
+from south.modelsinspector import add_introspection_rules
 from django.db import models, DatabaseError
 from django.db.models.signals import post_save
 from django.dispatch import receiver, Signal
@@ -21,6 +22,9 @@ from jsonfield import JSONField
 
 logger = logging.getLogger(__name__)
 
+
+add_introspection_rules([], ["submissions\.models\.AnonymizedUserIDField"])
+
 # Signal to inform listeners that a score has been changed
 score_set = Signal(providing_args=[
     'points_possible', 'points_earned', 'anonymous_user_id',
@@ -33,6 +37,16 @@ score_reset = Signal(
 )
 
 
+class AnonymizedUserIDField(models.CharField):
+    """ Field for storing anonymized user ids. """
+    description = "The anonymized User ID that the XBlock sees"
+
+    def __init__(self, *args, **kwargs):
+        kwargs['max_length'] = 255
+        kwargs['db_index'] = True
+        super(AnonymizedUserIDField, self).__init__(*args, **kwargs)
+
+
 class StudentItem(models.Model):
     """Represents a single item for a single course for a single user.
 
@@ -41,7 +55,7 @@ class StudentItem(models.Model):
 
     """
     # The anonymized Student ID that the XBlock sees, not their real ID.
-    student_id = models.CharField(max_length=255, blank=False, db_index=True)
+    student_id = AnonymizedUserIDField()
 
     # Not sure yet whether these are legacy course_ids or new course_ids
     course_id = models.CharField(max_length=255, blank=False, db_index=True)
@@ -274,3 +288,15 @@ class ScoreSummary(models.Model):
                 u"Error while updating score summary for student item {}"
                 .format(score.student_item)
             )
+
+
+class ScoreAnnotation(models.Model):
+    """ Annotate individual scores with extra information if necessary. """
+
+    score = models.ForeignKey(Score)
+    # A string that will represent the 'type' of annotation,
+    # e.g. staff_override, etc.
+    annotation_type = models.CharField(max_length=255, blank=False, db_index=True)
+
+    creator = AnonymizedUserIDField()
+    reason = models.TextField()

submissions/tests/test_api.py

+# -*- coding: utf-8 -*-
+
 import datetime
 import copy
 
@@ -10,7 +12,7 @@ from mock import patch
 import pytz
 
 from submissions import api as api
-from submissions.models import ScoreSummary, Submission, StudentItem, score_set
+from submissions.models import ScoreSummary, ScoreAnnotation, Submission, StudentItem, score_set
 from submissions.serializers import StudentItemSerializer
 
 STUDENT_ITEM = dict(
@@ -252,6 +254,7 @@ class TestSubmissionsApi(TestCase):
         api.set_score(submission["uuid"], 11, 12)
         score = api.get_latest_score_for_submission(submission["uuid"])
         self._assert_score(score, 11, 12)
+        self.assertFalse(ScoreAnnotation.objects.all().exists())
 
     @patch.object(score_set, 'send')
     def test_set_score_signal(self, send_mock):
@@ -268,6 +271,28 @@ class TestSubmissionsApi(TestCase):
             item_id=STUDENT_ITEM['item_id']
         )
 
+    @ddt.data(u"First score was incorrect", u"☃")
+    def test_set_score_with_annotation(self, reason):
+        submission = api.create_submission(STUDENT_ITEM, ANSWER_ONE)
+        creator_uuid = "Bob"
+        annotation_type = "staff_override"
+        api.set_score(submission["uuid"], 11, 12, creator_uuid, annotation_type, reason)
+        score = api.get_latest_score_for_submission(submission["uuid"])
+        self._assert_score(score, 11, 12)
+
+        # We need to do this to verify that one score annotation exists and was
+        # created for this score. We do not have an api point for retrieving
+        # annotations, and it doesn't make sense to expose them, since they're
+        # for auditing purposes.
+        annotations = ScoreAnnotation.objects.all()
+        self.assertGreater(len(annotations), 0)
+        annotation = annotations[0]
+        self.assertEqual(annotation.score.points_earned, 11)
+        self.assertEqual(annotation.score.points_possible, 12)
+        self.assertEqual(annotation.annotation_type, annotation_type)
+        self.assertEqual(annotation.creator, creator_uuid)
+        self.assertEqual(annotation.reason, reason)
+
     def test_get_score(self):
         submission = api.create_submission(STUDENT_ITEM, ANSWER_ONE)
         api.set_score(submission["uuid"], 11, 12)
@@ -595,4 +620,3 @@ class TestSubmissionsApi(TestCase):
         self.assertIsNotNone(score)
         self.assertEqual(score["points_earned"], expected_points_earned)
         self.assertEqual(score["points_possible"], expected_points_possible)
-