Revert "Defining done in context of staff assessments"

openassessment/assessment/test/test_staff.py

@@ -78,10 +78,8 @@ class TestStaffAssessment(CacheResetTest):
         self.assertEqual(assessment["points_earned"], OPTIONS_SELECTED_DICT[key]["expected_points"])
         self.assertEqual(assessment["points_possible"], RUBRIC_POSSIBLE_POINTS)
 
-        # Ensure submission and workflow are marked as finished
+        # ensure submission is 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):
@@ -106,8 +104,6 @@ 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):
@@ -186,7 +182,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"], requirements)
+        workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], self.STEP_REQUIREMENTS)
         self.assertEqual(workflow["score"]["points_earned"], OPTIONS_SELECTED_DICT[staff_score]["expected_points"])
 
     @data(*ASSESSMENT_TYPES_DDT)
@@ -200,10 +196,6 @@ 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])
 
@@ -218,71 +210,23 @@ 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"], requirements)
+        workflow = workflow_api.get_workflow_for_submission(tim_sub["uuid"], self.STEP_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,9 +82,6 @@ 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)
 
@@ -205,13 +202,10 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
     def score(self):
         """Latest score for the submission we're tracking.
 
-        Returns:
-            score (dict): The latest score for this workflow, or None if the workflow is incomplete.
+        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.
         """
-        score = None
-        if self.status == self.STATUS.done:
-            score = sub_api.get_latest_score_for_submission(self.submission_uuid)
-        return score
+        return sub_api.get_latest_score_for_submission(self.submission_uuid)
 
     def status_details(self, assessment_requirements):
         status_dict = {}
@@ -324,23 +318,17 @@ 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 = sub_api.get_latest_score_for_submission(self.submission_uuid)
+            old_score = self.score
             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))
-
-                # 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()
+                staff_step = step_for_name.get('staff')
+                staff_step.assessment_completed_at=now()
+                staff_step.save()
+                self.status = self.STATUS.done
 
         if self.status == self.STATUS.done:
             return
@@ -371,8 +359,7 @@ 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:
-                if score.get("staff_id") is None:
-                    self.set_score(score)
+                self.set_score(score)
                 new_status = self.STATUS.done
 
         # Finally save our changes if the status has changed