Make peer assessment an optional step.

This also creates the AssessmentWorkflowStep model to track all
steps for new workflows. Some problems with it now:

* a lot of broken tests
* some display oddities on the peer step during (peer, self) workflows
* have not tested to see if there are concurrency issues with step
  model creation under load.

apps/openassessment/assessment/peer_api.py

@@ -72,6 +72,11 @@ class PeerAssessmentInternalError(PeerAssessmentError):
     pass
 
 
+def submitter_is_finished(submission_uuid, requirements):
+    """Temporary bridge method."""
+    return is_complete(submission_uuid, requirements)
+
+
 def is_complete(submission_uuid, requirements):
     try:
         workflow = PeerWorkflow.objects.get(submission_uuid=submission_uuid)
@@ -134,6 +139,9 @@ def get_score(submission_uuid, requirements):
         "points_possible": items[0].assessment.points_possible,
     }
 
+def assessment_is_finished(submission_uuid, requirements):
+    return bool(get_score(submission_uuid, requirements))
+
 
 def create_assessment(
         scorer_submission_uuid,

apps/openassessment/assessment/self_api.py

@@ -139,8 +139,11 @@ def get_assessment(submission_uuid):
 
     return serialized_assessment
 
+def submitter_is_finished(submission_uuid, requirements):
+    """Temporary bridge method."""
+    return is_complete(submission_uuid, requirements)
 
-def is_complete(submission_uuid):
+def is_complete(submission_uuid, requirements):
     """
     Check whether a self-assessment has been completed for a submission.
 
@@ -154,6 +157,48 @@ def is_complete(submission_uuid):
         score_type=SELF_TYPE, submission_uuid=submission_uuid
     ).exists()
 
+def assessment_is_finished(submission_uuid, requirements):
+    return is_complete(submission_uuid, requirements)
+
+def get_score(submission_uuid, requirements):
+    assessment = get_assessment(submission_uuid)
+    if not assessment:
+        return None
+
+    return {
+        "points_earned": assessment["points_earned"],
+        "points_possible": assessment["points_possible"]
+    }
+
+def get_assessment_scores_by_criteria(submission_uuid):
+    """Get the median score for each rubric criterion
+
+    Args:
+        submission_uuid (str): The submission uuid is used to get the
+            assessments used to score this submission, and generate the
+            appropriate median score.
+
+    Returns:
+        (dict): A dictionary of rubric criterion names, with a median score of
+            the peer assessments.
+
+    Raises:
+        SelfAssessmentInternalError: If any error occurs while retrieving
+            information to form the median scores, an error is raised.
+    """
+    try:
+        assessments = list(
+            Assessment.objects.filter(
+                score_type=SELF_TYPE, submission_uuid=submission_uuid
+            ).order_by('-scored_at')[:1]
+        )
+        scores = Assessment.scores_by_criterion(assessments)
+        return Assessment.get_median_score_dict(scores)
+    except DatabaseError:
+        error_message = _(u"Error getting self assessment scores for {}").format(submission_uuid)
+        logger.exception(error_message)
+        raise SelfAssessmentInternalError(error_message)
+
 
 def _log_assessment(assessment, submission):
     """

apps/openassessment/workflow/admin.py

 from django.contrib import admin
 
-from .models import AssessmentWorkflow
+from .models import AssessmentWorkflow, AssessmentWorkflowStep
+
+
+class AssessmentWorkflowStepInline(admin.StackedInline):
+    model = AssessmentWorkflowStep
+    extra = 0
 
 class AssessmentWorkflowAdmin(admin.ModelAdmin):
     """Admin for the user's overall workflow through open assessment.
@@ -15,5 +20,6 @@ class AssessmentWorkflowAdmin(admin.ModelAdmin):
     )
     list_filter = ('status',)
     search_fields = ('uuid', 'submission_uuid', 'course_id', 'item_id')
+    inlines = (AssessmentWorkflowStepInline,)
 
 admin.site.register(AssessmentWorkflow, AssessmentWorkflowAdmin)

apps/openassessment/workflow/api.py

@@ -9,7 +9,7 @@ from django.db import DatabaseError
 
 from openassessment.assessment import peer_api
 from submissions import api as sub_api
-from .models import AssessmentWorkflow
+from .models import AssessmentWorkflow, AssessmentWorkflowStep
 from .serializers import AssessmentWorkflowSerializer
 
 logger = logging.getLogger(__name__)
@@ -58,7 +58,7 @@ class AssessmentWorkflowNotFoundError(AssessmentWorkflowError):
     pass
 
 
-def create_workflow(submission_uuid):
+def create_workflow(submission_uuid, steps):
     """Begins a new assessment workflow.
 
     Create a new workflow that other assessments will record themselves against.
@@ -66,6 +66,8 @@ def create_workflow(submission_uuid):
     Args:
         submission_uuid (str): The UUID for the submission that all our
             assessments will be evaluating.
+        steps (list): List of steps that are part of the workflow, in the order
+            that the user must complete them. Example: `["peer", "self"]`
 
     Returns:
         dict: Assessment workflow information with the following
@@ -114,6 +116,15 @@ def create_workflow(submission_uuid):
             .format(submission_uuid, err)
         )
 
+    # Raise an error if they specify a step we don't recognize...
+    for step in steps:
+        if step not in AssessmentWorkflow.STEPS:
+            raise AssessmentWorkflowRequestError(
+                u"Step not recognized: {}; Must be one of: {}".format(
+                    step, AssessmentWorkflow.STEPS
+                )
+            )
+
     # We're not using a serializer to deserialize this because the only variable
     # we're getting from the outside is the submission_uuid, which is already
     # validated by this point.
@@ -125,6 +136,13 @@ def create_workflow(submission_uuid):
             course_id=submission_dict['student_item']['course_id'],
             item_id=submission_dict['student_item']['item_id'],
         )
+        workflow_steps = [
+            AssessmentWorkflowStep(
+                workflow=workflow, name=step, order_num=i
+            )
+            for i, step in enumerate(steps)
+        ]
+        workflow.steps.add(*workflow_steps)
     except (
         DatabaseError,
         peer_api.PeerAssessmentError,

apps/openassessment/workflow/migrations/0002_auto__add_field_assessmentworkflow_course_id__add_field_assessmentwork.py

@@ -40,7 +40,6 @@ class Migration(SchemaMigration):
             'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
             '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'}),
             'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
             'status': ('model_utils.fields.StatusField', [], {'default': "'peer'", 'max_length': '100', u'no_check_for_status': 'True'}),
             'status_changed': ('model_utils.fields.MonitorField', [], {'default': 'datetime.datetime.now', u'monitor': "u'status'"}),

apps/openassessment/workflow/migrations/0003_auto__add_assessmentworkflowstep.py

+# -*- coding: utf-8 -*-
+import 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 'AssessmentWorkflowStep'
+        db.create_table('workflow_assessmentworkflowstep', (
+            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+            ('workflow', self.gf('django.db.models.fields.related.ForeignKey')(related_name='steps', to=orm['workflow.AssessmentWorkflow'])),
+            ('name', self.gf('django.db.models.fields.CharField')(max_length=20)),
+            ('submitter_completed_at', self.gf('django.db.models.fields.DateTimeField')(default=None, null=True)),
+            ('assessment_completed_at', self.gf('django.db.models.fields.DateTimeField')(default=None, null=True)),
+            ('order_num', self.gf('django.db.models.fields.PositiveIntegerField')()),
+        ))
+        db.send_create_signal('workflow', ['AssessmentWorkflowStep'])
+
+
+    def backwards(self, orm):
+        # Deleting model 'AssessmentWorkflowStep'
+        db.delete_table('workflow_assessmentworkflowstep')
+
+
+    models = {
+        'workflow.assessmentworkflow': {
+            'Meta': {'ordering': "['-created']", 'object_name': 'AssessmentWorkflow'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'item_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'status': ('model_utils.fields.StatusField', [], {'default': "'peer'", 'max_length': '100', u'no_check_for_status': 'True'}),
+            'status_changed': ('model_utils.fields.MonitorField', [], {'default': 'datetime.datetime.now', u'monitor': "u'status'"}),
+            'submission_uuid': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '36', 'db_index': 'True'}),
+            'uuid': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'unique': 'True', 'max_length': '36', 'blank': 'True'})
+        },
+        'workflow.assessmentworkflowstep': {
+            'Meta': {'ordering': "['workflow', 'order_num']", 'object_name': 'AssessmentWorkflowStep'},
+            'assessment_completed_at': ('django.db.models.fields.DateTimeField', [], {'default': 'None', 'null': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '20'}),
+            'order_num': ('django.db.models.fields.PositiveIntegerField', [], {}),
+            'submitter_completed_at': ('django.db.models.fields.DateTimeField', [], {'default': 'None', 'null': 'True'}),
+            'workflow': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'steps'", 'to': "orm['workflow.AssessmentWorkflow']"})
+        }
+    }
+
+    complete_apps = ['workflow']
\ No newline at end of file

apps/openassessment/workflow/models.py

@@ -16,6 +16,7 @@ import importlib
 from django.conf import settings
 from django.db import models
 from django_extensions.db.fields import UUIDField
+from django.utils.timezone import now
 from model_utils import Choices
 from model_utils.models import StatusModel, TimeStampedModel
 
@@ -46,9 +47,12 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
     an after the fact recording of the last known state of that information so
     we can search easily.
     """
-    STATUS_VALUES = [
+    STEPS = [
         "peer",  # User needs to assess peer submissions
         "self",  # User needs to assess themselves
+    ]
+
+    STATUS_VALUES = STEPS + [
         "waiting",  # User has done all necessary assessment but hasn't been
                     # graded yet -- we're waiting for assessments of their
                     # submission by others.
@@ -81,23 +85,23 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
         return sub_api.get_latest_score_for_submission(self.submission_uuid)
 
     def status_details(self, assessment_requirements):
-        return {
-            "peer": {
-                "complete": self._is_peer_complete(assessment_requirements),
-            },
-            "self": {
-                "complete": self._is_self_complete(),
-            },
-        }
+        from openassessment.assessment import peer_api, self_api
+
+        status_dict = {}
 
-    def _is_peer_complete(self, assessment_requirements):
-        from openassessment.assessment import peer_api
-        peer_requirements = assessment_requirements["peer"]
-        return peer_api.is_complete(self.submission_uuid, peer_requirements)
+        if "peer" in assessment_requirements:
+            status_dict["peer"] = {
+                "complete": peer_api.submitter_is_finished(
+                    self.submission_uuid,
+                    assessment_requirements["peer"]
+                )
+            }
+        if "self" in assessment_requirements:
+            status_dict["self"] = {
+                "complete": self_api.submitter_is_finished(self.submission_uuid, {})
+            }
 
-    def _is_self_complete(self):
-        from openassessment.assessment import self_api
-        return self_api.is_complete(self.submission_uuid)
+        return status_dict
 
     def update_from_assessments(self, assessment_requirements):
         """Query self and peer APIs and change our status if appropriate.
@@ -130,65 +134,140 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
                 specific requirements in this dict.
 
         """
-        from openassessment.assessment import peer_api
+        from openassessment.assessment import peer_api, self_api
 
         # If we're done, we're done -- it doesn't matter if requirements have
         # changed because we've already written a score.
         if self.status == self.STATUS.done:
             return
 
-        # Have they completed the peer and self steps?
-        peer_complete = self._is_peer_complete(assessment_requirements)
-        self_complete = self._is_self_complete()
-
-        if peer_complete and self_complete:
-            # If they've completed both, they're at least waiting, possibly done
-            new_status = self.STATUS.waiting
-        elif peer_complete:
-            # If they haven't done self assessment yet, that's their status
-            new_status = self.STATUS.self
-        else:
-            # Default starting status is peer
-            new_status = self.STATUS.peer
-
-        # If we're at least waiting, let's check if we have a peer score and
-        # can move all the way to done
-        if new_status == self.STATUS.waiting:
-            score = peer_api.get_score(
-                self.submission_uuid, assessment_requirements["peer"]
-            )
-            if score:
-                sub_api.set_score(
+        # Update our AssessmentWorkflowStep models with the latest from our APIs
+        steps = self.update_steps(assessment_requirements)
+
+        # Fetch name of the first step that the submitter hasn't yet completed.
+        new_status = next(
+            (step.name for step in steps if step.submitter_completed_at is None),
+            self.STATUS.waiting  # if nothing's left to complete, we're waiting
+        )
+
+        # If the submitter has done all they need to do, let's check to see if
+        # all steps have been fully assessed (i.e. we can score it).
+        if (new_status == self.STATUS.waiting and
+            all(step.assessment_completed_at for step in steps)):
+
+            # At this point, we're trying to give a score. We currently have a
+            # very simple rule for this -- if it has a peer step, use that for
+            # scoring. If not, use the self step. Later on, we may put more
+            # interesting rules here.
+            step_names = [step.name for step in steps]
+            score = None
+            if self.STATUS.peer in step_names:
+                score = peer_api.get_score(
                     self.submission_uuid,
-                    score["points_earned"],
-                    score["points_possible"]
+                    assessment_requirements[self.STATUS.peer]
                 )
+            elif self.STATUS.self in step_names:
+                score = self_api.get_score(self.submission_uuid, {})
 
-                # This should be replaced by using the event tracking API, but
-                # that's not quite ready yet. So we're making this temp hack.
-                emit_event({
-                    "context": {
-                        "course_id": self.course_id
-                    },
-                    "event": {
-                        "submission_uuid": self.submission_uuid,
-                        "points_earned": score["points_earned"],
-                        "points_possible": score["points_possible"],
-                    },
-                    "event_source": "server",
-                    "event_type": "openassessment.workflow.score",
-                    "time": datetime.utcnow(),
-                })
-
+            if score:
+                self.set_score(score)
                 new_status = self.STATUS.done
 
-
         # Finally save our changes if the status has changed
         if self.status != new_status:
             self.status = new_status
             self.save()
 
 
+    def update_steps(self, assessment_requirements):
+        from openassessment.assessment import peer_api, self_api
+
+        steps = list(self.steps.all())
+        if not steps:
+            # If no steps exist for this AssessmentWorkflow, assume
+            # peer -> self for backwards compatibility
+            self.steps.add(
+                AssessmentWorkflowStep(name=self.STATUS.peer, order_num=0),
+                AssessmentWorkflowStep(name=self.STATUS.self, order_num=1)
+            )
+            steps = list(self.steps.all())
+
+        # Mapping of step names to the APIs that power them
+        steps_to_apis = {
+            self.STATUS.self: self_api,
+            self.STATUS.peer: peer_api
+        }
+
+        # Go through each step and update its status. Note that because we take
+        # the philosophy that once you're done, you're done. That means
+        for step in steps:
+            step_changed = False
+            step_api = steps_to_apis[step.name]
+            step_reqs = assessment_requirements.get(step.name, {})
+
+            # Has the user completed their obligations for this step?
+            if (step.submitter_completed_at is None and
+                step_api.submitter_is_finished(self.submission_uuid, step_reqs)):
+                step.submitter_completed_at = now()
+                step_changed = True
+
+            # Has the step received a score?
+            if (step.assessment_completed_at is None and
+                step_api.assessment_is_finished(self.submission_uuid, step_reqs)):
+                step.assessment_completed_at = now()
+                step_changed = True
+
+            if step_changed:
+                step.save()
+
+        return steps
+
+
+    def set_score(self, score):
+        sub_api.set_score(
+            self.submission_uuid,
+            score["points_earned"],
+            score["points_possible"]
+        )
+
+        # This should be replaced by using the event tracking API, but
+        # that's not quite ready yet. So we're making this temp hack.
+        emit_event({
+            "context": {
+                "course_id": self.course_id
+            },
+            "event": {
+                "submission_uuid": self.submission_uuid,
+                "points_earned": score["points_earned"],
+                "points_possible": score["points_possible"],
+            },
+            "event_source": "server",
+            "event_type": "openassessment.workflow.score",
+            "time": datetime.utcnow(),
+        })
+
+
+class AssessmentWorkflowStep(models.Model):
+    """An individual step in the overall workflow process.
+
+    Similar caveats apply to this class as apply to `AssessmentWorkflow`. What
+    we're storing in the database is usually but not always current information.
+    In particular, if the problem definition has changed the requirements for a
+    particular step in the workflow, then what is in the database will be out of
+    sync until someone views this problem again (which will trigger a workflow
+    update to occur).
+
+    """
+    workflow = models.ForeignKey(AssessmentWorkflow, related_name="steps")
+    name = models.CharField(max_length=20)
+    submitter_completed_at = models.DateTimeField(default=None, null=True)
+    assessment_completed_at = models.DateTimeField(default=None, null=True)
+    order_num = models.PositiveIntegerField()
+
+    # Store the score for this step as well?
+    class Meta:
+        ordering = ["workflow", "order_num"]
+
 
 # Just here to record thoughts for later:
 #

apps/openassessment/xblock/grade_mixin.py

@@ -70,12 +70,26 @@ class GradeMixin(object):
         Returns:
             tuple of context (dict), template_path (string)
         """
-        feedback = peer_api.get_assessment_feedback(self.submission_uuid)
+        # Peer specific stuff...
+        assessment_steps = [asmnt['name'] for asmnt in self.rubric_assessments]
+        submission_uuid = workflow['submission_uuid']
+
+        if "peer-assessment" in assessment_steps:
+            feedback = peer_api.get_assessment_feedback(submission_uuid)
+            peer_assessments = peer_api.get_assessments(submission_uuid)
+            has_submitted_feedback = peer_api.get_assessment_feedback(submission_uuid) is not None
+        else:
+            feedback = None
+            peer_assessments = []
+            has_submitted_feedback = False
+
+        if "self-assessment" in assessment_steps:
+            self_assessment = self_api.get_assessment(submission_uuid)
+        else:
+            self_assessment = None
+
         feedback_text = feedback.get('feedback', '') if feedback else ''
-        student_submission = sub_api.get_submission(workflow['submission_uuid'])
-        peer_assessments = peer_api.get_assessments(student_submission['uuid'])
-        self_assessment = self_api.get_assessment(student_submission['uuid'])
-        has_submitted_feedback = peer_api.get_assessment_feedback(workflow['submission_uuid']) is not None
+        student_submission = sub_api.get_submission(submission_uuid)
 
         # We retrieve the score from the workflow, which in turn retrieves
         # the score for our current submission UUID.
@@ -94,14 +108,23 @@ class GradeMixin(object):
         }
 
         # Update the scores we will display to the user
-        # Note that we are updating a *copy* of the rubric criteria stored in the XBlock field
-        max_scores = peer_api.get_rubric_max_scores(self.submission_uuid)
-        median_scores = peer_api.get_assessment_median_scores(student_submission["uuid"])
+        # Note that we are updating a *copy* of the rubric criteria stored in
+        # the XBlock field
+        max_scores = peer_api.get_rubric_max_scores(submission_uuid)
+        if "peer-assessment" in assessment_steps:
+            median_scores = peer_api.get_assessment_median_scores(submission_uuid)
+        elif "self-assessment" in assessment_steps:
+            median_scores = self_api.get_assessment_scores_by_criteria(submission_uuid)
+
         if median_scores is not None and max_scores is not None:
             for criterion in context["rubric_criteria"]:
                 criterion["median_score"] = median_scores[criterion["name"]]
                 criterion["total_value"] = max_scores[criterion["name"]]
 
+        from pprint import pprint
+
+        pprint(self_assessment)
+
         return ('openassessmentblock/grade/oa_grade_complete.html', context)
 
     def render_grade_incomplete(self, workflow):
@@ -114,10 +137,16 @@ class GradeMixin(object):
         Returns:
             tuple of context (dict), template_path (string)
         """
+        def _is_incomplete(step):
+            return (
+                step in workflow["status_details"] and
+                not workflow["status_details"][step]["complete"]
+            )
+
         incomplete_steps = []
-        if not workflow["status_details"]["peer"]["complete"]:
+        if _is_incomplete("peer"):
             incomplete_steps.append("Peer Assessment")
-        if not workflow["status_details"]["self"]["complete"]:
+        if _is_incomplete("self"):
             incomplete_steps.append("Self Assessment")
 
         return (
@@ -131,7 +160,8 @@ class GradeMixin(object):
         Submit feedback on an assessment.
 
         Args:
-            data (dict): Can provide keys 'feedback_text' (unicode) and 'feedback_options' (list of unicode).
+            data (dict): Can provide keys 'feedback_text' (unicode) and
+                'feedback_options' (list of unicode).
 
         Kwargs:
             suffix (str): Unused

apps/openassessment/xblock/openassessmentblock.py

@@ -313,6 +313,10 @@ class OpenAssessmentBlock(
                 load('static/xml/poverty_rubric_example.xml')
             ),
             (
+                "OpenAssessmentBlock (Self Only) Rubric",
+                load('static/xml/poverty_self_only_example.xml')
+            ),
+            (
                 "OpenAssessmentBlock Censorship Rubric",
                 load('static/xml/censorship_rubric_example.xml')
             ),
@@ -333,6 +337,10 @@ class OpenAssessmentBlock(
 
         return update_from_xml(block, node, validator=validator(block, strict_post_release=False))
 
+    @property
+    def assessment_steps(self):
+        return [asmnt['name'] for asmnt in self.rubric_assessments]
+
     def render_assessment(self, path, context_dict=None):
         """Render an Assessment Module's HTML
 
@@ -421,18 +429,19 @@ class OpenAssessmentBlock(
         ]
 
         # Resolve unspecified dates and date strings to datetimes
-        start, due, date_ranges = resolve_dates(self.start, self.due, [submission_range] + assessment_ranges)
+        start, due, date_ranges = resolve_dates(
+            self.start, self.due, [submission_range] + assessment_ranges
+        )
+        step_ranges = {"submission": date_ranges[0]}
+        for step_num, asmnt in enumerate(self.rubric_assessments, start=1):
+            step_ranges[asmnt["name"]] = date_ranges[step_num]
 
         # Based on the step, choose the date range to consider
         # We hard-code this to the submission -> peer -> self workflow for now;
         # later, we can revisit to make this more flexible.
         open_range = (start, due)
-        if step == "submission":
-            open_range = date_ranges[0]
-        if step == "peer-assessment":
-            open_range = date_ranges[1]
-        if step == "self-assessment":
-            open_range = date_ranges[2]
+        if step in ["submission", "peer-assessment", "self-assessment"]:
+            open_range = step_ranges[step]
 
         # Course staff always have access to the problem
         if course_staff is None:

apps/openassessment/xblock/peer_assessment_mixin.py

 import logging
+
 from django.utils.translation import ugettext as _
+from webob import Response
 from xblock.core import XBlock
+
 from openassessment.assessment import peer_api
 from openassessment.assessment.peer_api import (
     PeerAssessmentInternalError, PeerAssessmentRequestError,
@@ -114,6 +117,8 @@ class PeerAssessmentMixin(object):
                 number of assessments.
 
         """
+        if "peer-assessment" not in self.assessment_steps:
+            return Response(u"")
         continue_grading = data.params.get('continue_grading', False)
         path, context_dict = self.peer_path_and_context(continue_grading)
         return self.render_assessment(path, context_dict)

apps/openassessment/xblock/self_assessment_mixin.py

@@ -2,6 +2,8 @@ import logging
 from django.utils.translation import ugettext as _
 
 from xblock.core import XBlock
+from webob import Response
+
 from openassessment.assessment import self_api
 from openassessment.workflow import api as workflow_api
 from submissions import api as submission_api
@@ -24,6 +26,9 @@ class SelfAssessmentMixin(object):
 
     @XBlock.handler
     def render_self_assessment(self, data, suffix=''):
+        if "self-assessment" not in self.assessment_steps:
+            return Response(u"")
+
         try:
             path, context = self.self_path_and_context()
         except:

apps/openassessment/xblock/static/js/src/oa_base.js

@@ -181,14 +181,14 @@ OpenAssessment.BaseView.prototype = {
     toggleActionError: function(type, msg) {
         var element = this.element;
         var container = null;
-        if (type == 'save') { 
-            container = '.response__submission__actions'; 
+        if (type == 'save') {
+            container = '.response__submission__actions';
         }
-        else if (type == 'submit' || type == 'peer' || type == 'self') { 
-            container = '.step__actions'; 
+        else if (type == 'submit' || type == 'peer' || type == 'self') {
+            container = '.step__actions';
         }
-        else if (type == 'feedback_assess') { 
-            container = '.submission__feedback__actions'; 
+        else if (type == 'feedback_assess') {
+            container = '.submission__feedback__actions';
         }
 
         // If we don't have anywhere to put the message, just log it to the console
@@ -219,10 +219,10 @@ OpenAssessment.BaseView.prototype = {
         $(container + ' .step__status__value .copy').html(gettext('Unable to Load'));
     },
 
-    /** 
+    /**
      * Get the contents of the Step Actions error message box, for unit test validation.
      *
-     * Step Actions are the UX-level parts of the student interaction flow - 
+     * Step Actions are the UX-level parts of the student interaction flow -
      * Submission, Peer Assessment, and Self Assessment. Since steps are mutually
      * exclusive, only one error box should be rendered on screen at a time.
      *

apps/openassessment/xblock/static/xml/poverty_self_only_example.xml

+<?xml version="1.0" encoding="UTF-8" standalone="no" ?>
+<openassessment submission_due="2015-03-11T18:20">
+    <title>
+        Global Poverty
+    </title>
+    <rubric>
+        <prompt>
+            Given the state of the world today, what do you think should be done to combat poverty?
+
+            Read for conciseness, clarity of thought, and form.
+        </prompt>
+        <criterion>
+            <name>concise</name>
+            <prompt>How concise is it?</prompt>
+            <option points="0">
+                <name>Neal Stephenson (late)</name>
+                <explanation>
+                    In "Cryptonomicon", Stephenson spent multiple pages talking about breakfast cereal.
+                    While hilarious, in recent years his work has been anything but 'concise'.
+                </explanation>
+            </option>
+            <option points="1">
+                <name>HP Lovecraft</name>
+                <explanation>
+                    If the author wrote something cyclopean that staggers the mind, score it thus.
+                </explanation>
+            </option>
+            <option points="3">
+                <name>Robert Heinlein</name>
+                <explanation>
+                    Tight prose that conveys a wealth of information about the world in relatively
+                    few words. Example, "The door irised open and he stepped inside."
+                </explanation>
+            </option>
+            <option points="4">
+                <name>Neal Stephenson (early)</name>
+                <explanation>
+                    When Stephenson still had an editor, his prose was dense, with anecdotes about
+                    nitrox abuse implying main characters' whole life stories.
+                </explanation>
+            </option>
+            <option points="5">
+                <name>Earnest Hemingway</name>
+                <explanation>
+                    Score the work this way if it makes you weep, and the removal of a single
+                    word would make you sneer.
+                </explanation>
+            </option>
+        </criterion>
+        <criterion>
+            <name>clear-headed</name>
+            <prompt>How clear is the thinking?</prompt>
+            <option points="0">
+                <name>Yogi Berra</name>
+                <explanation></explanation>
+            </option>
+            <option points="1">
+                <name>Hunter S. Thompson</name>
+                <explanation></explanation>
+            </option>
+            <option points="2">
+                <name>Robert Heinlein</name>
+                <explanation></explanation>
+            </option>
+            <option points="3">
+                <name>Isaac Asimov</name>
+                <explanation></explanation>
+            </option>
+            <option points="10">
+                <name>Spock</name>
+                <explanation>
+                    Coolly rational, with a firm grasp of the main topics, a crystal-clear train of thought,
+                    and unemotional examination of the facts.  This is the only item explained in this category,
+                    to show that explained and unexplained items can be mixed.
+                </explanation>
+            </option>
+        </criterion>
+        <criterion>
+            <name>form</name>
+            <prompt>Lastly, how is its form? Punctuation, grammar, and spelling all count.</prompt>
+            <option points="0">
+                <name>lolcats</name>
+                <explanation></explanation>
+            </option>
+            <option points="1">
+                <name>Facebook</name>
+                <explanation></explanation>
+            </option>
+            <option points="2">
+                <name>Reddit</name>
+                <explanation></explanation>
+            </option>
+            <option points="3">
+                <name>metafilter</name>
+                <explanation></explanation>
+            </option>
+            <option points="4">
+                <name>Usenet, 1996</name>
+                <explanation></explanation>
+            </option>
+            <option points="5">
+                <name>The Elements of Style</name>
+                <explanation></explanation>
+            </option>
+        </criterion>
+    </rubric>
+    <assessments>
+        <assessment name="self-assessment" />
+    </assessments>
+</openassessment>

apps/openassessment/xblock/submission_mixin.py

@@ -124,7 +124,7 @@ class SubmissionMixin(object):
         student_sub_dict = {'text': student_sub}
 
         submission = api.create_submission(student_item_dict, student_sub_dict)
-        workflow_api.create_workflow(submission["uuid"])
+        self.create_workflow(submission["uuid"])
         self.submission_uuid = submission["uuid"]
 
         # Emit analytics event...

apps/openassessment/xblock/test/data/assessment_combo.json

@@ -23,7 +23,7 @@
         ]
     },
     "self_only": {
-        "valid": false,
+        "valid": true,
         "assessments": [
             {
                 "name": "self-assessment"

apps/openassessment/xblock/test/data/invalid_assessment_combo_order.xml

+<openassessment>
+    <title>Open Assessment Test</title>
+    <prompt>
+        Given the state of the world today, what do you think should be done to
+        combat poverty? Please answer in a short essay of 200-300 words.
+    </prompt>
+    <rubric>
+        <prompt>Read for conciseness, clarity of thought, and form.</prompt>
+        <criterion>
+            <name>Concise</name>
+            <prompt>How concise is it?</prompt>
+            <option points="0">
+                <name>Neal Stephenson (late)</name>
+                <explanation>Neal Stephenson explanation</explanation>
+            </option>
+        </criterion>
+    </rubric>
+    <assessments>
+        <assessment name="self-assessment" />
+        <assessment name="peer-assessment" />
+    </assessments>
+</openassessment>

apps/openassessment/xblock/test/data/invalid_assessment_combo_peer_only.xml

+<openassessment>
+    <title>Open Assessment Test</title>
+    <prompt>
+        Given the state of the world today, what do you think should be done to
+        combat poverty? Please answer in a short essay of 200-300 words.
+    </prompt>
+    <rubric>
+        <prompt>Read for conciseness, clarity of thought, and form.</prompt>
+        <criterion>
+            <name>Concise</name>
+            <prompt>How concise is it?</prompt>
+            <option points="0">
+                <name>Neal Stephenson (late)</name>
+                <explanation>Neal Stephenson explanation</explanation>
+            </option>
+        </criterion>
+    </rubric>
+    <assessments>
+        <assessment name="peer-assessment" />
+    </assessments>
+</openassessment>

apps/openassessment/xblock/test/data/invalid_assessment_combo.xml → apps/openassessment/xblock/test/data/optional_assessments_self_only.xml

 <openassessment>
-    <title>Open Assessment Test</title>
+    <title>Only Self Assessment</title>
     <prompt>
         Given the state of the world today, what do you think should be done to
         combat poverty? Please answer in a short essay of 200-300 words.

apps/openassessment/xblock/test/data/valid_assessments.json

 {
-    "peer": {
-        "assessment": {
+    "peer_then_self": [
+        {
             "name": "peer-assessment",
             "must_grade": 5,
             "must_be_graded_by": 3
+        },
+        {
+            "name": "self-assessment"
         }
-    },
-    "self": {
-        "assessment": {
-            "name": "self-assessment",
-            "must_grade": 2,
-            "must_be_graded_by": 1
+    ],
+    "self_only": [
+        {
+            "name": "self-assessment"
         }
-    },
-    "must_be_graded_by_equals_must_grade": {
-        "assessment": {
-            "name": "self-assessment",
+    ],
+    "must_be_graded_by_equals_must_grade": [
+        {
+            "name": "peer-assessment",
             "must_grade": 1,
             "must_be_graded_by": 1
+        },
+        {
+            "name": "self-assessment"
         }
-    }
+    ]
 }

apps/openassessment/xblock/test/test_studio.py

@@ -94,10 +94,17 @@ class StudioViewTest(XBlockHandlerTestCase):
     # If and when we remove this restriction, this test can be deleted.
     @scenario('data/basic_scenario.xml')
     def test_update_xml_invalid_assessment_combo(self, xblock):
-        request = json.dumps({'xml': self.load_fixture_str('data/invalid_assessment_combo.xml')})
-        resp = self.request(xblock, 'update_xml', request, response_format='json')
-        self.assertFalse(resp['success'])
-        self.assertIn("must have exactly two assessments", resp['msg'].lower())
+        invalid_workflows = [
+            'invalid_assessment_combo_order',
+            'invalid_assessment_combo_peer_only'
+        ]
+        for invalid_workflow in invalid_workflows:
+            request = json.dumps(
+                {'xml': self.load_fixture_str('data/{}.xml'.format(invalid_workflow))}
+            )
+            resp = self.request(xblock, 'update_xml', request, response_format='json')
+            self.assertFalse(resp['success'])
+            self.assertIn("supported assessment flows are", resp['msg'].lower())
 
     @data(('data/invalid_rubric.xml', 'rubric'), ('data/invalid_assessment.xml', 'assessment'))
     @scenario('data/basic_scenario.xml')

apps/openassessment/xblock/test/test_validation.py

@@ -14,7 +14,7 @@ class AssessmentValidationTest(TestCase):
 
     @ddt.file_data('data/valid_assessments.json')
     def test_valid_assessment(self, data):
-        success, msg = validate_assessments([data['assessment']])
+        success, msg = validate_assessments(data)
         self.assertTrue(success)
         self.assertEqual(msg, u'')
 
@@ -29,12 +29,11 @@ class AssessmentValidationTest(TestCase):
         self.assertFalse(success)
         self.assertGreater(len(msg), 0)
 
-    # Currently, we enforce the restriction that there must be
-    # exactly two assessments, in the order (a) peer, then (b) self.
-    # If and when we remove that restriction, this test can be deleted.
+    # Make sure only legal assessment combinations are allowed. For now, that's
+    # (peer -> self), and (self)
     @ddt.file_data('data/assessment_combo.json')
-    def test_enforce_peer_then_self(self, data):
-        success, msg = validate_assessments(data['assessments'], enforce_peer_then_self=True)
+    def test_enforce_assessment_combo_restrictions(self, data):
+        success, msg = validate_assessments(data['assessments'])
         self.assertEqual(success, data['valid'], msg=msg)
 
         if not success:

apps/openassessment/xblock/validation.py

@@ -43,29 +43,38 @@ def _duplicates(items):
     return set(x for x in items if counts[x] > 1)
 
 
-def validate_assessments(assessments, enforce_peer_then_self=False):
+def validate_assessments(assessments):
     """
     Check that the assessment dict is semantically valid.
 
+    Valid assessment steps are currently:
+    * peer, then self
+    * self only
+
     Args:
         assessments (list of dict): list of serialized assessment models.
 
-    Kwargs:
-        enforce_peer_then_self (bool): If True, enforce the requirement that there
-            must be exactly two assessments: first, a peer-assessment, then a self-assessment.
-
     Returns:
         tuple (is_valid, msg) where
             is_valid is a boolean indicating whether the assessment is semantically valid
             and msg describes any validation errors found.
     """
-    if enforce_peer_then_self:
-        if len(assessments) != 2:
-            return (False, _("This problem must have exactly two assessments."))
-        if assessments[0].get('name') != 'peer-assessment':
-            return (False, _("The first assessment must be a peer assessment."))
-        if assessments[1].get('name') != 'self-assessment':
-            return (False, _("The second assessment must be a self assessment."))
+    def _self_only(assessments):
+        return len(assessments) == 1 and assessments[0].get('name') == 'self-assessment'
+
+    def _peer_then_self(assessments):
+        return (
+            len(assessments) == 2 and
+            assessments[0].get('name') == 'peer-assessment' and
+            assessments[1].get('name') == 'self-assessment'
+        )
+
+    # Right now, there are two allowed scenarios: (peer -> self) and (self)
+    if not (_self_only(assessments) or _peer_then_self(assessments)):
+        return (
+            False,
+            _("The only supported assessment flows are (peer, self) or (self).")
+        )
 
     if len(assessments) == 0:
         return (False, _("This problem must include at least one assessment."))
@@ -188,7 +197,7 @@ def validator(oa_block, strict_post_release=True):
     """
 
     def _inner(rubric_dict, submission_dict, assessments):
-        success, msg = validate_assessments(assessments, enforce_peer_then_self=True)
+        success, msg = validate_assessments(assessments)
         if not success:
             return (False, msg)
 

apps/openassessment/xblock/workflow_mixin.py

@@ -8,6 +8,22 @@ class WorkflowMixin(object):
     def handle_workflow_info(self, data, suffix=''):
         return self.get_workflow_info()
 
+    def create_workflow(self, submission_uuid):
+        def _convert_rubric_assessment_name(ra_name):
+            """'self-assessment' -> 'self', 'peer-assessment' -> 'peer'"""
+            short_name, suffix = ra_name.split("-")
+            return short_name
+
+        # rubric_assessments stores names as "self-assessment",
+        # "peer-assessment", while the model is expecting "self", "peer".
+        # Therefore, this conversion step. We should refactor later to
+        # standardize.
+        steps = [
+            _convert_rubric_assessment_name(ra["name"])
+            for ra in self.rubric_assessments
+        ]
+        workflow_api.create_workflow(submission_uuid, steps)
+
     def workflow_requirements(self):
         """
         Retrieve the requirements from each assessment module