Staff assessments and workflow status

Staff assessments can now mark workflow as being done, with proper handling
for blocking workflows (such as peer), to hide a score if the submitter
has not yet fulfilled their obligations. Includes tests.

openassessment/assessment/test/test_staff.py

@@ -78,8 +78,10 @@ class TestStaffAssessment(CacheResetTest):
         self.assertEqual(assessment["points_earned"], OPTIONS_SELECTED_DICT[key]["expected_points"])
         self.assertEqual(assessment["points_possible"], RUBRIC_POSSIBLE_POINTS)
 
-        # ensure submission is marked as finished
+        # Ensure submission and workflow are marked as finished
         self.assertTrue(staff_api.assessment_is_finished(tim_sub["uuid"], self.STEP_REQUIREMENTS))
+        workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], self.STEP_REQUIREMENTS)
+        self.assertEqual(workflow["status"], "done")
 
     @data(*ASSESSMENT_SCORES_DDT)
     def test_create_assessment_required(self, key):
@@ -104,6 +106,8 @@ class TestStaffAssessment(CacheResetTest):
         # Verify assesment made, score updated, and no longer waiting
         self.assertEqual(staff_assessment["points_earned"], OPTIONS_SELECTED_DICT[key]["expected_points"])
         self.assertTrue(staff_api.assessment_is_finished(tim_sub["uuid"], self.STEP_REQUIREMENTS_WITH_STAFF))
+        workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], self.STEP_REQUIREMENTS_WITH_STAFF)
+        self.assertEqual(workflow["status"], "done")
 
     @data(*ASSESSMENT_SCORES_DDT)
     def test_create_assessment_score_overrides(self, key):
@@ -182,7 +186,7 @@ class TestStaffAssessment(CacheResetTest):
 
         # Verify both assessment and workflow report correct score
         self.assertEqual(staff_assessment["points_earned"], OPTIONS_SELECTED_DICT[staff_score]["expected_points"])
-        workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], self.STEP_REQUIREMENTS)
+        workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], requirements)
         self.assertEqual(workflow["score"]["points_earned"], OPTIONS_SELECTED_DICT[staff_score]["expected_points"])
 
     @data(*ASSESSMENT_TYPES_DDT)
@@ -196,6 +200,10 @@ class TestStaffAssessment(CacheResetTest):
         if after_type == 'staff':
             return
 
+        requirements = self.STEP_REQUIREMENTS
+        if after_type == 'peer':
+            requirements = {"peer": {"must_grade": 0, "must_be_graded_by": 1}}
+
         # Create assessment
         tim_sub, tim = self._create_student_and_submission("Tim", "Tim's answer", override_steps=[after_type])
 
@@ -210,23 +218,71 @@ class TestStaffAssessment(CacheResetTest):
 
         # Verify both assessment and workflow report correct score
         self.assertEqual(staff_assessment["points_earned"], OPTIONS_SELECTED_DICT[staff_score]["expected_points"])
-        workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], self.STEP_REQUIREMENTS)
+        workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], requirements)
         self.assertEqual(workflow["score"]["points_earned"], OPTIONS_SELECTED_DICT[staff_score]["expected_points"])
 
         # Now, non-force asses with a 'most' value
         # This was selected to match the value that the ai test will set
         unscored_assessment = OPTIONS_SELECTED_DICT["most"]
         assessment = after_assess(tim_sub["uuid"], tim["student_id"], unscored_assessment["options"])
-        # and update workflow with new scores
-        requirements = self.STEP_REQUIREMENTS
-        if after_type == 'peer':
-            requirements = {"peer": {"must_grade": 0, "must_be_graded_by": 1}}
 
         # Verify both assessment and workflow report correct score (workflow should report previous value)
         self.assertEqual(assessment["points_earned"], unscored_assessment["expected_points"])
         workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], requirements)
         self.assertEqual(workflow["score"]["points_earned"], OPTIONS_SELECTED_DICT[staff_score]["expected_points"])
 
+    def test_provisionally_done(self):
+        """
+        Test to ensure that blocking steps, such as peer, are not considered done and do not display a score
+        if the submitter's requirements have not yet been met, even if a staff score has been recorded.
+
+        This test also ensures that a user may submit peer assessments after having been staff assessed, which was
+        a bug that had been previously present.
+        """
+        # Tim(student) makes a submission, for a problem that requires peer assessment
+        tim_sub, tim = TestStaffAssessment._create_student_and_submission("Tim", "Tim's answer", override_steps=['peer'])
+        # Bob(student) also makes a submission for that problem
+        bob_sub, bob = TestStaffAssessment._create_student_and_submission("Bob", "Bob's answer", override_steps=['peer'])
+
+        # Define peer requirements. Note that neither submission will fulfill must_be_graded_by
+        requirements = {"peer": {"must_grade": 1, "must_be_graded_by": 2}}
+
+        staff_score = "none"
+        # Dumbledore(staff) uses override ability to provide a score for both submissions
+        tim_assessment = staff_api.create_assessment(
+            tim_sub["uuid"],
+            "Dumbledore",
+            OPTIONS_SELECTED_DICT[staff_score]["options"], dict(), "",
+            RUBRIC,
+        )
+        bob_assessment = staff_api.create_assessment(
+            bob_sub["uuid"],
+            "Dumbledore",
+            OPTIONS_SELECTED_DICT[staff_score]["options"], dict(), "",
+            RUBRIC,
+        )
+
+        # Bob completes his peer assessment duties, Tim does not
+        peer_api.get_submission_to_assess(bob_sub["uuid"], 1)
+        peer_assess(
+            bob_sub["uuid"],
+            bob["student_id"],
+            OPTIONS_SELECTED_DICT["most"]["options"], dict(), "",
+            RUBRIC,
+            requirements["peer"]["must_be_graded_by"]
+        )
+
+
+        # Verify that Bob's submission is marked done and returns the proper score
+        bob_workflow = workflow_api.get_workflow_for_submission(bob_sub["uuid"], requirements)
+        self.assertEqual(bob_workflow["score"]["points_earned"], OPTIONS_SELECTED_DICT[staff_score]["expected_points"])
+        self.assertEqual(bob_workflow["status"], "done")
+
+        # Verify that Tim's submission is not marked done, and he cannot get his score
+        tim_workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], requirements)
+        self.assertEqual(tim_workflow["score"], None)
+        self.assertNotEqual(tim_workflow["status"], "done")
+
     def test_invalid_rubric_exception(self):
         # Create a submission
         tim_sub, tim = self._create_student_and_submission("Tim", "Tim's answer")

openassessment/workflow/models.py

@@ -82,6 +82,9 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
         DEFAULT_ASSESSMENT_SCORE_PRIORITY
     )
 
+    # Blocking steps are steps that must be completed by the submitter, even if a staff assesment is present
+    BLOCKING_STEPS = ["peer"]
+
     submission_uuid = models.CharField(max_length=36, db_index=True, unique=True)
     uuid = UUIDField(version=1, db_index=True, unique=True)
 
@@ -202,10 +205,13 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
     def score(self):
         """Latest score for the submission we're tracking.
 
-        Note that while it is usually the case that we're setting the score,
-        that may not always be the case. We may have some course staff override.
+        Returns:
+            score (dict): The latest score for this workflow, or None if the workflow is incomplete.
         """
-        return sub_api.get_latest_score_for_submission(self.submission_uuid)
+        score = None
+        if self.status == self.STATUS.done:
+            score = sub_api.get_latest_score_for_submission(self.submission_uuid)
+        return score
 
     def status_details(self, assessment_requirements):
         status_dict = {}
@@ -318,17 +324,23 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
 
         new_staff_score = self.get_score(assessment_requirements, {'staff': step_for_name.get('staff', None)})
         if new_staff_score:
-            old_score = self.score
+            old_score = sub_api.get_latest_score_for_submission(self.submission_uuid)
             if not old_score or old_score['points_earned'] != new_staff_score['points_earned']:
+                # Set the staff score using submissions api, and log that fact
                 self.set_staff_score(new_staff_score)
                 self.save()
                 logger.info((
                     u"Workflow for submission UUID {uuid} has updated score using staff assessment."
                 ).format(uuid=self.submission_uuid))
-                staff_step = step_for_name.get('staff')
-                staff_step.assessment_completed_at=now()
-                staff_step.save()
-                self.status = self.STATUS.done
+
+                # Update the assessment and submitter_completed_at fields for all steps
+                # All steps are considered "assessment complete", as the staff score will override all
+                # Steps in BLOCKING_STEPS may still require the submitter to fulfill their obligations
+                for step in steps:
+                    step.assessment_completed_at=now()
+                    if step.name not in self.BLOCKING_STEPS:
+                        step.submitter_completed_at=now()
+                    step.save()
 
         if self.status == self.STATUS.done:
             return
@@ -359,7 +371,8 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
             score = self.get_score(assessment_requirements, step_for_name)
             # If we found a score, then we're done
             if score is not None:
-                self.set_score(score)
+                if score.get("staff_id") is None:
+                    self.set_score(score)
                 new_status = self.STATUS.done
 
         # Finally save our changes if the status has changed