Refactoring the Staff override functionality.

TNL-898 & TNL-899

openassessment/assessment/api/peer.py

@@ -4,6 +4,7 @@ The Peer Assessment Workflow API exposes all public actions required to complete
 the workflow for a given submission.
 
 """
+import copy
 import logging
 
 from django.utils import timezone
@@ -12,8 +13,7 @@ from django.db import DatabaseError, IntegrityError, transaction
 from dogapi import dog_stats_api
 from openassessment.assessment.models import (
     Assessment, AssessmentFeedback, AssessmentPart,
-    InvalidRubricSelection, PeerWorkflow, PeerWorkflowItem,
-    AssessmentOverride)
+    InvalidRubricSelection, PeerWorkflow, PeerWorkflowItem)
 from openassessment.assessment.serializers import (
     AssessmentFeedbackSerializer, RubricSerializer,
     full_assessment_dict, rubric_from_dict, serialize_assessments,
@@ -308,7 +308,7 @@ def _complete_assessment(
         scorer_workflow,
         overall_feedback,
         num_required_grades,
-        scored_at
+        scored_at,
 ):
     """
     Internal function for atomic assessment creation. Creates a peer assessment
@@ -672,6 +672,8 @@ def get_submission_to_assess(submission_uuid, graded_by):
             u"with submission UUID {}".format(submission_uuid)
         )
     open_item = workflow.find_active_assessments()
+    import pudb;pu.db
+
     peer_submission_uuid = open_item.submission_uuid if open_item else None
     # If there is an active assessment for this user, get that submission,
     # otherwise, get the first assessment for review, otherwise,
@@ -952,7 +954,8 @@ def _log_workflow(submission_uuid, workflow):
     dog_stats_api.increment('openassessment.assessment.peer_workflow.count', tags=tags)
 
 
-def create_overridden_assessment(assessment_id, points, scorer_id, comments=None, scored_at=None):
+@transaction.commit_on_success
+def create_overridden_assessment(assessment_id, overridden_name, overridden_points, scorer_id, overridden_feedback=None, scored_at=None):
     """
     Create a new override assessment.
 
@@ -969,15 +972,45 @@ def create_overridden_assessment(assessment_id, points, scorer_id, comments=None
         AssessmentOverride
     """
     try:
-        points = int(points)
+        points = int(overridden_points)
 
         # Get the particular assessment to Override/Regrade
         assessment = Assessment.objects.get(pk=assessment_id)
-        if points > assessment.points_possible:
-            raise PeerAssessmentRequestError(u'New grade points must be less than or equal to possible points.')
 
-        return AssessmentOverride.create(assessment=assessment, points=points, comments=comments,
-                                         scorer_id=scorer_id, scored_at=scored_at)
+        #
+        # if points > assessment.points_possible:
+        #     raise PeerAssessmentRequestError(u'New grade points must be less than or equal to possible points.')
+
+        # Create the peer assessment
+        overridden_assessment = Assessment.create(
+            assessment.rubric,
+            scorer_id,
+            assessment.submission_uuid,
+            score_type='ST',
+            scored_at=scored_at,
+            feedback=assessment.feedback
+        )
+        options_selected = {'Idea': 'Good'}
+        criterion_feedback = {'Idea': overridden_feedback}
+        overridden_assessment_part = AssessmentPart.create_from_option_names(overridden_assessment, options_selected, feedback=criterion_feedback)
+        # TODO should we create criteria option with new points????
+        #
+        # for part in assessment.parts.all():
+        #     overridden_part = copy.deepcopy(part)
+        #     if part.criterion.name == overridden_name:
+        #         overridden_part.criterion.feedback = overridden_feedback
+        #         overridden_part.option.points = overridden_points
+
+
+
+        # assessment.overridden_by = overridden_assessment
+        assessment.save()
+        # Create assessment parts for each criterion in the rubric
+        # This will raise an `InvalidRubricSelection` if the selected options do not
+        # match the rubric.
+        # AssessmentPart.create_from_option_names(assessment, options_selected, feedback=criterion_feedback)
+
+        return overridden_assessment
     except ValueError:
         raise PeerAssessmentRequestError(u'New grade must be an integer.')
 

openassessment/assessment/migrations/0026_auto__del_field_assessmentoverride_comment__add_field_assessmentoverri.py → openassessment/assessment/migrations/0025_auto__add_field_assessment_overridden_by.py

@@ -8,23 +8,15 @@ from django.db import models
 class Migration(SchemaMigration):
 
     def forwards(self, orm):
-        # Deleting field 'AssessmentOverride.comment'
-        db.delete_column('assessment_assessmentoverride', 'comment')
-
-        # Adding field 'AssessmentOverride.comments'
-        db.add_column('assessment_assessmentoverride', 'comments',
-                      self.gf('django.db.models.fields.TextField')(default='', max_length=10000, blank=True),
+        # Adding field 'Assessment.overridden_by'
+        db.add_column('assessment_assessment', 'overridden_by',
+                      self.gf('django.db.models.fields.related.ForeignKey')(default=None, to=orm['assessment.Assessment'], blank=True),
                       keep_default=False)
 
 
     def backwards(self, orm):
-        # Adding field 'AssessmentOverride.comment'
-        db.add_column('assessment_assessmentoverride', 'comment',
-                      self.gf('django.db.models.fields.TextField')(default='', max_length=10000, blank=True),
-                      keep_default=False)
-
-        # Deleting field 'AssessmentOverride.comments'
-        db.delete_column('assessment_assessmentoverride', 'comments')
+        # Deleting field 'Assessment.overridden_by'
+        db.delete_column('assessment_assessment', 'overridden_by_id')
 
 
     models = {
@@ -76,6 +68,7 @@ class Migration(SchemaMigration):
             'Meta': {'ordering': "['-scored_at', '-id']", 'object_name': 'Assessment'},
             'feedback': ('django.db.models.fields.TextField', [], {'default': "''", 'max_length': '10000', 'blank': 'True'}),
             'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'overridden_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['assessment.Assessment']", 'blank': 'True'}),
             'rubric': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['assessment.Rubric']"}),
             'score_type': ('django.db.models.fields.CharField', [], {'max_length': '2'}),
             'scored_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
@@ -95,15 +88,6 @@ class Migration(SchemaMigration):
             'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
             'text': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'})
         },
-        'assessment.assessmentoverride': {
-            'Meta': {'object_name': 'AssessmentOverride'},
-            'assessment': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'override'", 'to': "orm['assessment.Assessment']"}),
-            'comments': ('django.db.models.fields.TextField', [], {'default': "''", 'max_length': '10000', 'blank': 'True'}),
-            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
-            'points': ('django.db.models.fields.IntegerField', [], {'db_index': 'True'}),
-            'scored_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
-            'scorer_id': ('django.db.models.fields.CharField', [], {'max_length': '40', 'db_index': 'True'})
-        },
         'assessment.assessmentpart': {
             'Meta': {'object_name': 'AssessmentPart'},
             'assessment': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'parts'", 'to': "orm['assessment.Assessment']"}),

openassessment/assessment/migrations/0025_auto__add_assessmentoverride.py → openassessment/assessment/migrations/0026_auto__chg_field_assessment_overridden_by.py

@@ -8,22 +8,14 @@ from django.db import models
 class Migration(SchemaMigration):
 
     def forwards(self, orm):
-        # Adding model 'AssessmentOverride'
-        db.create_table('assessment_assessmentoverride', (
-            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
-            ('assessment', self.gf('django.db.models.fields.related.ForeignKey')(related_name='override', to=orm['assessment.Assessment'])),
-            ('scored_at', self.gf('django.db.models.fields.DateTimeField')(default=datetime.datetime.now, db_index=True)),
-            ('scorer_id', self.gf('django.db.models.fields.CharField')(max_length=40, db_index=True)),
-            ('points', self.gf('django.db.models.fields.IntegerField')(db_index=True)),
-            ('comment', self.gf('django.db.models.fields.TextField')(default='', max_length=10000, blank=True)),
-        ))
-        db.send_create_signal('assessment', ['AssessmentOverride'])
 
+        # Changing field 'Assessment.overridden_by'
+        db.alter_column('assessment_assessment', 'overridden_by_id', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['assessment.Assessment'], null=True))
 
     def backwards(self, orm):
-        # Deleting model 'AssessmentOverride'
-        db.delete_table('assessment_assessmentoverride')
 
+        # Changing field 'Assessment.overridden_by'
+        db.alter_column('assessment_assessment', 'overridden_by_id', self.gf('django.db.models.fields.related.ForeignKey')(default=None, to=orm['assessment.Assessment']))
 
     models = {
         'assessment.aiclassifier': {
@@ -74,6 +66,7 @@ class Migration(SchemaMigration):
             'Meta': {'ordering': "['-scored_at', '-id']", 'object_name': 'Assessment'},
             'feedback': ('django.db.models.fields.TextField', [], {'default': "''", 'max_length': '10000', 'blank': 'True'}),
             'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'overridden_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['assessment.Assessment']", 'null': 'True', 'blank': 'True'}),
             'rubric': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['assessment.Rubric']"}),
             'score_type': ('django.db.models.fields.CharField', [], {'max_length': '2'}),
             'scored_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
@@ -93,15 +86,6 @@ class Migration(SchemaMigration):
             'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
             'text': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'})
         },
-        'assessment.assessmentoverride': {
-            'Meta': {'object_name': 'AssessmentOverride'},
-            'assessment': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'override'", 'to': "orm['assessment.Assessment']"}),
-            'comment': ('django.db.models.fields.TextField', [], {'default': "''", 'max_length': '10000', 'blank': 'True'}),
-            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
-            'points': ('django.db.models.fields.IntegerField', [], {'db_index': 'True'}),
-            'scored_at': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}),
-            'scorer_id': ('django.db.models.fields.CharField', [], {'max_length': '40', 'db_index': 'True'})
-        },
         'assessment.assessmentpart': {
             'Meta': {'object_name': 'AssessmentPart'},
             'assessment': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'parts'", 'to': "orm['assessment.Assessment']"}),

openassessment/assessment/models/base.py

@@ -413,6 +413,7 @@ class Assessment(models.Model):
 
     submission_uuid = models.CharField(max_length=128, db_index=True)
     rubric = models.ForeignKey(Rubric)
+    overridden_by = models.ForeignKey('self', blank=True, null=True)
 
     scored_at = models.DateTimeField(default=now, db_index=True)
     scorer_id = models.CharField(max_length=40, db_index=True)
@@ -826,57 +827,3 @@ class AssessmentPart(models.Model):
         if len(missing_criteria) > 0:
             msg = u"Missing selections for criteria: {missing}".format(missing=', '.join(missing_criteria))
             raise InvalidRubricSelection(msg)
\ No newline at end of file
-
-
-class AssessmentOverride(models.Model):
-    """Override/Regrade an Assessment.
-
-    This is an assessment state information and is created when a staff overrides
-    an assessment of some student.
-
-    """
-    MAX_COMMENT_SIZE = 1000 * 10
-
-    assessment = models.ForeignKey(Assessment, related_name='overridden')
-
-    scored_at = models.DateTimeField(default=now, db_index=True)
-    scorer_id = models.CharField(max_length=40, db_index=True)
-
-    points = models.IntegerField(db_index=True)
-    comments = models.TextField(max_length=10000, default="", blank=True)
-
-    class Meta:
-        app_label = "assessment"
-
-    @classmethod
-    def create(cls, assessment, points, scorer_id, comments=None, scored_at=None):
-        """
-        Create a new override assessment.
-
-        Args:
-            assessment (Assessment): The assessment associated with this override.
-            scorer_id (unicode): The ID of the staff.
-            points (int): The points given by staff.
-
-        Keyword Arguments:
-            feedback (unicode): Overall feedback on the submission.
-            scored_at (datetime): The time the assessment override was created.  Defaults to the current time.
-
-        Returns:
-            AssessmentOverride
-
-        """
-        assessment_params = {
-            'points': points,
-            'scorer_id': scorer_id,
-            'assessment': assessment,
-        }
-
-        if scored_at is not None:
-            assessment_params['scored_at'] = scored_at
-
-        # Truncate the comment if it exceeds the maximum size
-        if comments is not None:
-            assessment_params['comments'] = comments[0:cls.MAX_COMMENT_SIZE]
-
-        return cls.objects.create(**assessment_params)

openassessment/assessment/serializers/base.py

@@ -235,7 +235,7 @@ def full_assessment_dict(assessment, rubric_dict=None):
     assessment_dict["id"] = assessment.id
 
     # Get the latest overridden staff grade
-    assessment_dict["staff_overridden_grade"] = assessment.latest_overridden_assessment
+    # assessment_dict["staff_overridden_grade"] = assessment.latest_overridden_assessment
 
     cache.set(assessment_cache_key, assessment_dict)
 

openassessment/xblock/grade_mixin.py

@@ -271,7 +271,8 @@ class GradeMixin(object):
 
         for assessment in peer_assessments:
             for part in assessment['parts']:
-                feedback = None if assessment['staff_overridden_grade'] else part.get('feedback')
+                # feedback = None if assessment['staff_overridden_grade'] else part.get('feedback')
+                feedback = part.get('feedback')
                 if feedback:
                     part_criterion_name = part['criterion']['name']
                     peer_criteria_feedback[part_criterion_name].append(feedback)

openassessment/xblock/staff_info_mixin.py

@@ -5,13 +5,10 @@ determine the flow of the problem.
 import copy
 from functools import wraps
 import logging
-from django.template import Context
-from django.template.loader import get_template
 
 from xblock.core import XBlock
 from openassessment.assessment.errors import PeerAssessmentInternalError, PeerAssessmentRequestError
 from openassessment.assessment.errors.ai import AIError
-from openassessment.assessment.models import Assessment, AssessmentOverride
 from openassessment.xblock.resolve_dates import DISTANT_PAST, DISTANT_FUTURE
 from openassessment.xblock.data_conversion import (
     create_rubric_dict, convert_training_examples_list_to_dict
@@ -39,6 +36,7 @@ def require_global_admin(error_key):
         Decorated function
 
     """
+
     def _decorator(func):  # pylint: disable=C0111
         @wraps(func)
         def _wrapped(xblock, *args, **kwargs):  # pylint: disable=C0111
@@ -50,7 +48,9 @@ def require_global_admin(error_key):
                 return {'success': False, 'msg': permission_errors[error_key]}
             else:
                 return func(xblock, *args, **kwargs)
+
         return _wrapped
+
     return _decorator
 
 
@@ -67,6 +67,7 @@ def require_course_staff(error_key):
         decorated function
 
     """
+
     def _decorator(func):  # pylint: disable=C0111
         @wraps(func)
         def _wrapped(xblock, *args, **kwargs):  # pylint: disable=C0111
@@ -79,7 +80,9 @@ def require_course_staff(error_key):
                 return xblock.render_error(permission_errors[error_key])
             else:
                 return func(xblock, *args, **kwargs)
+
         return _wrapped
+
     return _decorator
 
 
@@ -286,17 +289,23 @@ class StaffInfoMixin(object):
     @XBlock.json_handler
     @require_course_staff("STAFF_INFO")
     def staff_override_assessment(self, data, suffix=''):
-        assessment_id = data.get('assessment_id', '')
-        points = data.get('points', '')
-        comments = data.get('comments', '')
+        assessment_id = data.get('assessment_id')
+        overridden_points = data.get('points')
+        overridden_feedback = data.get('overridden_feedback')
+        overridden_name = data.get('overridden_name')
+        overridden_criterion = data.get('overridden_criterion')
 
-        if points in ['', None]:
+        if overridden_points in ['', None]:
             return {"success": False, "msg": self._(u'Please enter integer value for new grade.')}
             # return self.render_error(self._(u'"override_assessment" required new grade value.'))
 
         try:
-            overridden_assessment = peer_api.create_overridden_assessment(assessment_id=assessment_id, points=points, comments=comments,
-                                                  scorer_id=self.get_student_item_dict()["student_id"])
+            overridden_assessment = peer_api.create_overridden_assessment(assessment_id=assessment_id,
+                                                                          overridden_name=overridden_name,
+                                                                          overridden_points=overridden_points,
+                                                                          overridden_feedback=overridden_feedback,
+                                                                          scorer_id=self.get_student_item_dict()[
+                                                                              "student_id"])
 
             path = 'openassessmentblock/staff_debug/staff_regrade_info.html'
             context_dict = {