Override submitter requirements on staff override assessment

TNL-4696

When a staff override assessment is recorded, we now treat all the submitter's
requirements as being fulfilled. This allows ORA tooling to match the progress
page reported grade for the student by placing the AssessmentWorkflow into a
DONE state.

Acceptance tests have been updated, and are now also included in quality checks

openassessment/assessment/api/peer.py

@@ -436,6 +436,8 @@ def get_assessment_median_scores(submission_uuid):
         assessments = [item.assessment for item in items]
         scores = Assessment.scores_by_criterion(assessments)
         return Assessment.get_median_score_dict(scores)
+    except PeerWorkflow.DoesNotExist:
+        return {}
     except DatabaseError:
         error_message = (
             u"Error getting assessment median scores for submission {uuid}"

openassessment/workflow/api.py

@@ -177,7 +177,7 @@ def get_workflow_for_submission(submission_uuid, assessment_requirements):
     return update_from_assessments(submission_uuid, assessment_requirements)
 
 
-def update_from_assessments(submission_uuid, assessment_requirements):
+def update_from_assessments(submission_uuid, assessment_requirements, override_submitter_requirements=False):
     """
     Update our workflow status based on the status of the underlying assessments.
 
@@ -259,7 +259,7 @@ def update_from_assessments(submission_uuid, assessment_requirements):
     workflow = _get_workflow_model(submission_uuid)
 
     try:
-        workflow.update_from_assessments(assessment_requirements)
+        workflow.update_from_assessments(assessment_requirements, override_submitter_requirements)
         logger.info((
             u"Updated workflow for submission UUID {uuid} "
             u"with requirements {reqs}"

openassessment/workflow/models.py

@@ -171,7 +171,7 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
 
                 # If we auto-added a staff step, it is optional and should be marked complete immediately
                 if step.name == "staff" and staff_auto_added:
-                    step.assessment_completed_at=now()
+                    step.assessment_completed_at = now()
                     step.save()
 
                 # For the first valid step, update the workflow status
@@ -209,6 +209,12 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
         return score
 
     def status_details(self):
+        """
+        Returns workflow status in the form of a dictionary. Each step in the
+        workflow is a key, and each key maps to a dictionary defining whether
+        the step is complete (submitter requirements fulfilled) and graded (the
+        submission has been assessed).
+        """
         status_dict = {}
         steps = self._get_steps()
         for step in steps:
@@ -259,7 +265,7 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
 
         return score
 
-    def update_from_assessments(self, assessment_requirements):
+    def update_from_assessments(self, assessment_requirements, override_submitter_requirements=False):
         """Query assessment APIs and change our status if appropriate.
 
         If the status is done, we do nothing. Once something is done, we never
@@ -320,7 +326,9 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
                 # Update the assessment_completed_at field for all steps
                 # All steps are considered "assessment complete", as the staff score will override all
                 for step in steps:
-                    step.assessment_completed_at=now()
+                    step.assessment_completed_at = now()
+                    if override_submitter_requirements:
+                        step.submitter_completed_at = now()
                     step.save()
 
         if self.status == self.STATUS.done:
@@ -346,8 +354,10 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
 
         # 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)):
+        if (
+                new_status == self.STATUS.waiting and
+                all(step.assessment_completed_at for step in steps)
+        ):
 
             score = self.get_score(assessment_requirements, step_for_name)
             # If we found a score, then we're done
@@ -398,7 +408,7 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
 
         return steps
 
-    def set_staff_score(self, score, is_override=False, reason=None):
+    def set_staff_score(self, score, reason=None):
         """
         Set a staff score for the workflow.
 
@@ -425,9 +435,9 @@ class AssessmentWorkflow(TimeStampedModel, StatusModel):
             self.submission_uuid,
             score["points_earned"],
             score["points_possible"],
-            annotation_creator = score["staff_id"],
-            annotation_type = annotation_type,
-            annotation_reason = reason
+            annotation_creator=score["staff_id"],
+            annotation_type=annotation_type,
+            annotation_reason=reason
         )
 
     def set_score(self, score):
@@ -602,9 +612,16 @@ class AssessmentWorkflowStep(models.Model):
         ordering = ["workflow", "order_num"]
 
     def is_submitter_complete(self):
+        """
+        Used to determine whether the submitter of the response has completed
+        their required actions.
+        """
         return self.submitter_completed_at is not None
 
     def is_assessment_complete(self):
+        """
+        Used to determine whether the response has been assessed at this step.
+        """
         return self.assessment_completed_at is not None
 
     def api(self):
@@ -660,12 +677,12 @@ class AssessmentWorkflowStep(models.Model):
         assessment_finished = getattr(self.api(), 'assessment_is_finished', default_finished)
 
         # Has the user completed their obligations for this step?
-        if (not self.is_submitter_complete() and submitter_finished(submission_uuid, step_reqs)):
+        if not self.is_submitter_complete() and submitter_finished(submission_uuid, step_reqs):
             self.submitter_completed_at = now()
             step_changed = True
 
         # Has the step received a score?
-        if (not self.is_assessment_complete() and assessment_finished(submission_uuid, step_reqs)):
+        if not self.is_assessment_complete() and assessment_finished(submission_uuid, step_reqs):
             self.assessment_completed_at = now()
             step_changed = True
 

openassessment/xblock/grade_mixin.py

@@ -567,15 +567,16 @@ class GradeMixin(object):
         # If criteria/options in the problem definition do NOT have a "label" field
         # (because they were created before this change),
         # we create a new label that has the same value as "name".
-        for part in assessment['parts']:
-            criterion_label_key = part['criterion']['name']
-            part['criterion']['label'] = criterion_labels.get(criterion_label_key, part['criterion']['name'])
-
-            # We need to be a little bit careful here: some assessment parts
-            # have only written feedback, so they're not associated with any options.
-            # If that's the case, we don't need to add the label field.
-            if part.get('option') is not None:
-                option_label_key = (part['criterion']['name'], part['option']['name'])
-                part['option']['label'] = option_labels.get(option_label_key, part['option']['name'])
+        if assessment is not None:
+            for part in assessment['parts']:
+                criterion_label_key = part['criterion']['name']
+                part['criterion']['label'] = criterion_labels.get(criterion_label_key, part['criterion']['name'])
+
+                # We need to be a little bit careful here: some assessment parts
+                # have only written feedback, so they're not associated with any options.
+                # If that's the case, we don't need to add the label field.
+                if part.get('option') is not None:
+                    option_label_key = (part['criterion']['name'], part['option']['name'])
+                    part['option']['label'] = option_labels.get(option_label_key, part['option']['name'])
 
         return assessment

openassessment/xblock/staff_assessment_mixin.py

@@ -53,7 +53,11 @@ class StaffAssessmentMixin(object):
             )
             assess_type = data.get('assess_type', 'regrade')
             self.publish_assessment_event("openassessmentblock.staff_assess", assessment, type=assess_type)
-            workflow_api.update_from_assessments(assessment["submission_uuid"], None)
+            workflow_api.update_from_assessments(
+                assessment["submission_uuid"],
+                None,
+                override_submitter_requirements=(assess_type == 'regrade')
+            )
 
         except StaffAssessmentRequestError:
             logger.warning(

scripts/run-pep8.sh

 #!/usr/bin/env bash
 
-MAX_PEP8_VIOLATIONS=110
+MAX_PEP8_VIOLATIONS=111
 
 mkdir -p test/logs
 PEP8_VIOLATIONS=test/logs/pep8.txt
 touch $PEP8_VIOLATIONS
 
-pep8 --config=.pep8 openassessment > $PEP8_VIOLATIONS
+pep8 --config=.pep8 openassessment test > $PEP8_VIOLATIONS
 NUM_PEP8_VIOLATIONS=$(cat $PEP8_VIOLATIONS | wc -l)
 
 echo "Found" $NUM_PEP8_VIOLATIONS "pep8 violations, threshold is" $MAX_PEP8_VIOLATIONS

scripts/run-pylint.sh

 #!/usr/bin/env bash
 
-MAX_PYLINT_VIOLATIONS=519
+MAX_PYLINT_VIOLATIONS=504
 
 mkdir -p test/logs
 PYLINT_VIOLATIONS=test/logs/pylint.txt
 touch $PYLINT_VIOLATIONS
 
-pylint --rcfile=pylintrc openassessment --msg-template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"'> $PYLINT_VIOLATIONS
+pylint --rcfile=pylintrc openassessment test --msg-template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"'> $PYLINT_VIOLATIONS
 ./scripts/run-pylint.py $PYLINT_VIOLATIONS $MAX_PYLINT_VIOLATIONS

test/acceptance/tests.py

@@ -8,13 +8,13 @@ import time
 from functools import wraps
 from pyinstrument import Profiler
 
-from nose.plugins.attrib import attr
-from bok_choy.web_app_test import WebAppTest
-from bok_choy.promise import BrokenPromise, EmptyPromise
-from auto_auth import AutoAuthPage
-from pages import (
+from acceptance.auto_auth import AutoAuthPage
+from acceptance.pages import (
     SubmissionPage, AssessmentPage, GradePage, StaffAreaPage
 )
+from bok_choy.web_app_test import WebAppTest
+from bok_choy.promise import BrokenPromise, EmptyPromise
+from nose.plugins.attrib import attr
 
 # This value is generally used in jenkins, but not locally
 PROFILING_ENABLED = os.environ.get('ORA_PROFILING_ENABLED', False)
@@ -89,7 +89,6 @@ class OpenAssessmentTest(WebAppTest):
     STAFF_OVERRIDE_OPTIONS_SELECTED = [0, 1]
     STAFF_OVERRIDE_SCORE = 1
     STAFF_GRADE_EXISTS = "COMPLETE"
-    STAFF_OVERRIDE_LEARNER_STEPS_NOT_COMPLETE = "You Must Complete the Steps Above to View Your Grade"
     STAFF_AREA_SCORE = "Final grade: {} out of 8"
     STAFF_OVERRIDE_STAFF_AREA_NOT_COMPLETE = "The problem has not been completed."
     EXPECTED_SCORE = 6
@@ -179,7 +178,7 @@ class OpenAssessmentTest(WebAppTest):
         self.self_asmnt_page.assess(options).wait_for_complete()
         self.assertTrue(self.self_asmnt_page.is_complete)
 
-    def _verify_staff_grade_section(self, expected_status, expected_message_title):
+    def _verify_staff_grade_section(self, expected_status):
         """
         Verifies the expected status and message text in the Staff Grade section
         (as shown to the learner).
@@ -187,8 +186,6 @@ class OpenAssessmentTest(WebAppTest):
         self.staff_asmnt_page.wait_for_page()
         self.assertEqual("Staff Grade", self.staff_asmnt_page.label)
         self.staff_asmnt_page.verify_status_value(expected_status)
-        message_title = self.staff_asmnt_page.open_step().message_title
-        self.assertEqual(expected_message_title, message_title)
 
     def do_training(self):
         """
@@ -273,7 +270,7 @@ class OpenAssessmentTest(WebAppTest):
                 self.staff_area_page.verify_available_checked_out_numbers((ungraded, checked_out-1))
                 break
             else:
-                ungraded -=1
+                ungraded -= 1
                 self.staff_area_page.verify_available_checked_out_numbers((ungraded, checked_out))
 
     def refresh_page(self):
@@ -337,14 +334,14 @@ class StaffAssessmentTest(OpenAssessmentTest):
         self.submission_page.visit()
 
         # Verify that staff grade step is shown initially
-        self._verify_staff_grade_section("NOT AVAILABLE", None)
+        self._verify_staff_grade_section("NOT AVAILABLE")
 
         # User submits a response
         self.submission_page.submit_response(self.SUBMISSION)
         self.assertTrue(self.submission_page.has_submitted)
 
         # Verify staff grade section appears as expected
-        self._verify_staff_grade_section("NOT AVAILABLE", "Waiting for a Staff Grade")
+        self._verify_staff_grade_section("NOT AVAILABLE")
 
         # Perform staff assessment
         self.staff_area_page = StaffAreaPage(self.browser, self.problem_loc)
@@ -352,7 +349,7 @@ class StaffAssessmentTest(OpenAssessmentTest):
 
         # Verify staff grade section appears as expected
         self.staff_asmnt_page.visit()
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, None)
+        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS)
         self.assertEqual(self.EXPECTED_SCORE, self.grade_page.wait_for_page().score)
 
         # Verify that staff scores can be overriden
@@ -414,15 +411,11 @@ class PeerAssessmentTestStaffOverride(OpenAssessmentTest):
         Then the Staff Grade section is marked complete with no message
         And I can see my final grade, even though no peers have assessed me
         """
-        # Create two students with a submission each so that there are 2 submissions to assess.
-        for _ in range(0, 2):
-            self.auto_auth_page.visit()
-            self.submission_page.visit().submit_response(self.SUBMISSION)
-
-        # Create a submission for the third student (used for the remainder of the test).
+        # Create a submission
         self.auto_auth_page.visit()
         username, _ = self.auto_auth_page.get_username_and_email()
         self.submission_page.visit().submit_response(self.SUBMISSION)
+
         # Staff Grade field should not be visible yet.
         self.assertFalse(self.staff_asmnt_page.is_browser_on_page())
 
@@ -431,17 +424,7 @@ class PeerAssessmentTestStaffOverride(OpenAssessmentTest):
 
         # Refresh the page so the learner sees the Staff Grade section.
         self.refresh_page()
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, self.STAFF_OVERRIDE_LEARNER_STEPS_NOT_COMPLETE)
-
-        # Verify no final grade yet.
-        self.assertIsNone(self.grade_page.wait_for_page().score)
-
-        # Assess two submissions
-        self.do_peer_assessment(count=2)
-
-        # Staff grade section is now marked complete, even though no students have submitted
-        # assessments for this particular student (no longer required since staff grade exists).
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, None)
+        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS)
 
         # Verify the staff override grade
         self.assertEqual(self.STAFF_OVERRIDE_SCORE, self.grade_page.wait_for_page().score)
@@ -522,8 +505,14 @@ class StaffAreaTest(OpenAssessmentTest):
         self.assertEqual(self.staff_area_page.visible_staff_panels, [])
 
         for panel_name, button_label in [
-            ("staff-tools", "MANAGE INDIVIDUAL LEARNERS"),
-            ("staff-info", "VIEW ASSIGNMENT STATISTICS"),
+                (
+                    "staff-tools",
+                    "MANAGE INDIVIDUAL LEARNERS"
+                ),
+                (
+                    "staff-info",
+                    "VIEW ASSIGNMENT STATISTICS"
+                ),
         ]:
             # Click on the button and verify that the panel has opened
             self.staff_area_page.click_staff_toolbar_button(panel_name)
@@ -708,22 +697,7 @@ class StaffAreaTest(OpenAssessmentTest):
 
         # Refresh the page so the learner sees the Staff Grade section.
         self.refresh_page()
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, self.STAFF_OVERRIDE_LEARNER_STEPS_NOT_COMPLETE)
-
-        # Verify no final grade yet.
-        self.assertIsNone(self.grade_page.wait_for_page().score)
-
-        # Verify required staff grading section not available
-        self.staff_area_page = StaffAreaPage(self.browser, self.problem_loc)
-        self.assertFalse(self.staff_area_page.is_button_visible('staff-grading'))
-
-        # Learner does required self-assessment
-        self.self_asmnt_page.wait_for_page().wait_for_response()
-        self.assertIn(self.SUBMISSION, self.self_asmnt_page.response_text)
-        self.self_asmnt_page.assess(self.OPTIONS_SELECTED).wait_for_complete()
-        self.assertTrue(self.self_asmnt_page.is_complete)
-
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, None)
+        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS)
 
         # Verify the staff override grade
         self.assertEqual(self.STAFF_OVERRIDE_SCORE, self.grade_page.wait_for_page().score)
@@ -750,7 +724,7 @@ class StaffAreaTest(OpenAssessmentTest):
 
         # Refresh the page so the learner sees the Staff Grade section shows the submission has been cancelled.
         self.refresh_page()
-        self._verify_staff_grade_section("CANCELLED", None)
+        self._verify_staff_grade_section("CANCELLED")
         self.assertIsNone(self.grade_page.wait_for_page().score)
 
 
@@ -881,7 +855,7 @@ class FullWorkflowMixin(object):
 
         # At this point, the learner sees the score (1).
         self.refresh_page()
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, None)
+        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS)
         self.assertEqual(self.STAFF_OVERRIDE_SCORE, self.grade_page.wait_for_page().score)
 
         if peer_grades_me:
@@ -1035,7 +1009,7 @@ class FullWorkflowOverrideTest(OpenAssessmentTest, FullWorkflowMixin):
         self.do_staff_override(learner)
 
         self.refresh_page()
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, None)
+        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS)
         self.assertEqual(self.STAFF_OVERRIDE_SCORE, self.grade_page.wait_for_page().score)
         self.verify_staff_area_fields(
             learner, self.STAFF_AREA_PEER_ASSESSMENT, self.STAFF_AREA_SUBMITTED, self.STAFF_AREA_SELF_ASSESSMENT
@@ -1086,47 +1060,25 @@ class FullWorkflowOverrideTest(OpenAssessmentTest, FullWorkflowMixin):
 
         # Refresh the page so the learner sees the Staff Grade section.
         self.refresh_page()
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, self.STAFF_OVERRIDE_LEARNER_STEPS_NOT_COMPLETE)
-
-        # Now create a second learner so that "learner" has someone to assess.
-        self.do_submission()
-
-        # Go back to the original learner to complete her workflow and view score.
-        self.login_user(learner, learner_email)
-
-        # Do training exercise and self assessment
-        self.student_training_page.visit()
-        self.do_training()
-        self.submit_self_assessment(self.SELF_ASSESSMENT)
+        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS)
 
-        # Verify staff grade still not available, as learner has not done peer assessment.
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, self.STAFF_OVERRIDE_LEARNER_STEPS_NOT_COMPLETE)
-        self.assertIsNone(self.grade_page.wait_for_page().score)
-        self.verify_staff_area_fields(learner, [], [], self.STAFF_AREA_SELF_ASSESSMENT)
-        self.staff_area_page.verify_learner_final_score(self.STAFF_OVERRIDE_STAFF_AREA_NOT_COMPLETE)
-
-        # Now do the final required step-- peer grading.
-        self.do_peer_assessment(options=self.SUBMITTED_ASSESSMENT)
-
-        # Grade is now visible to the learner (even though no student has graded the learner).
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, None)
+        # Grade is now visible to the learner despite not having made any assessments
         self.assertEqual(self.STAFF_OVERRIDE_SCORE, self.grade_page.wait_for_page().score)
-        self.verify_staff_area_fields(learner, [], self.STAFF_AREA_SUBMITTED, self.STAFF_AREA_SELF_ASSESSMENT)
+        self.verify_staff_area_fields(learner, [], [], [])
         self.staff_area_page.verify_learner_final_score(self.STAFF_AREA_SCORE.format(self.STAFF_OVERRIDE_SCORE))
         self.assertEquals(
-            ['CRITERION', 'STAFF GRADE', 'PEER MEDIAN GRADE', 'SELF ASSESSMENT GRADE'],
+            ['CRITERION', 'STAFF GRADE', 'PEER MEDIAN GRADE'],
             self.staff_area_page.learner_final_score_table_headers
         )
         self.assertEquals(
-            ['Poor - 0 points', 'Waiting for peer reviews', 'Good',
-             'Fair - 1 point', 'Waiting for peer reviews', 'Excellent'],
+            ['Poor - 0 points', 'Waiting for peer reviews',
+             'Fair - 1 point', 'Waiting for peer reviews'],
             self.staff_area_page.learner_final_score_table_values
         )
 
         self.verify_grade_entries([
             [(u"STAFF GRADE - 0 POINTS", u"Poor"), (u"STAFF GRADE - 1 POINT", u"Fair")],
             [(u'PEER MEDIAN GRADE', u'Waiting for peer reviews'), (u'PEER MEDIAN GRADE', u'Waiting for peer reviews')],
-            [(u"YOUR SELF ASSESSMENT", u"Good"), (u"YOUR SELF ASSESSMENT", u"Excellent")]
         ])
 
 
@@ -1163,7 +1115,8 @@ class FullWorkflowRequiredTest(OpenAssessmentTest, FullWorkflowMixin):
 @ddt.ddt
 class FeedbackOnlyTest(OpenAssessmentTest, FullWorkflowMixin):
     """
-    Test for a problem that containing a criterion that only accepts feedback. Will make and verify self and staff assessments.
+    Test for a problem that containing a criterion that only accepts feedback. Will make and verify self and staff
+    assessments.
     """
     def setUp(self):
         super(FeedbackOnlyTest, self).setUp("feedback_only", staff=True)
@@ -1196,7 +1149,7 @@ class FeedbackOnlyTest(OpenAssessmentTest, FullWorkflowMixin):
 
         # Staff assess all available submissions
         self.do_staff_assessment(
-            options_selected = [0],  # Select the 0-th option (Yes) on the single scored criterion
+            options_selected=[0],  # Select the 0-th option (Yes) on the single scored criterion
             feedback=lambda feedback_type: self.generate_feedback("staff", feedback_type)
         )