WIP: Self Criterion Feedback API change

Conflicts:
	openassessment/assessment/api/self.py
	openassessment/xblock/self_assessment_mixin.py
	openassessment/xblock/test/test_self.py

openassessment/assessment/api/self.py

@@ -156,8 +156,17 @@ def create_assessment(
         rubric = rubric_from_dict(rubric_dict)
 
         # Create the self assessment
-        assessment = Assessment.create(rubric, user_id, submission_uuid, SELF_TYPE, scored_at=scored_at)
-        AssessmentPart.create_from_option_names(assessment, options_selected)
+        assessment = Assessment.create(
+            rubric,
+            user_id,
+            submission_uuid,
+            SELF_TYPE,
+            scored_at=scored_at,
+            feedback=overall_feedback
+        )
+
+        # 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)
         _log_assessment(assessment, submission)
     except InvalidRubric:
         msg = "Invalid rubric definition"

openassessment/assessment/test/test_self.py

@@ -51,6 +51,14 @@ class TestSelfApi(CacheResetTest):
         "accuracy": "very accurate",
     }
 
+    CRITERION_FEEDBACK = {
+        "clarity": "Like a morning in the restful city of San Fransisco, the piece was indescribable, beautiful, and too foggy to properly comprehend.",
+        "accuracy": "Like my sister's cutting comments about my weight, I may not have enjoyed the piece, but I cannot fault it for its factual nature."
+    }
+
+    OVERALL_FEEDBACK = "Unfortunately, the nature of being is too complex to comment, judge, or discern any one" + \
+                       "arbitrary set of things over another."
+
     def test_create_assessment(self):
         # Initially, there should be no submission or self assessment
         self.assertEqual(get_assessment("5"), None)
@@ -66,7 +74,7 @@ class TestSelfApi(CacheResetTest):
         # Create a self-assessment for the submission
         assessment = create_assessment(
             submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-            self.OPTIONS_SELECTED, self.RUBRIC,
+            self.OPTIONS_SELECTED, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
             scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
         )
 
@@ -82,7 +90,7 @@ class TestSelfApi(CacheResetTest):
         self.assertEqual(assessment['submission_uuid'], submission['uuid'])
         self.assertEqual(assessment['points_earned'], 8)
         self.assertEqual(assessment['points_possible'], 10)
-        self.assertEqual(assessment['feedback'], u'')
+        self.assertEqual(assessment['feedback'], u'' + self.OVERALL_FEEDBACK)
         self.assertEqual(assessment['score_type'], u'SE')
 
     def test_create_assessment_no_submission(self):
@@ -90,7 +98,7 @@ class TestSelfApi(CacheResetTest):
         with self.assertRaises(SelfAssessmentRequestError):
             create_assessment(
                 'invalid_submission_uuid', u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-                self.OPTIONS_SELECTED, self.RUBRIC,
+                self.OPTIONS_SELECTED, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
                 scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
             )
 
@@ -102,7 +110,22 @@ class TestSelfApi(CacheResetTest):
         with self.assertRaises(SelfAssessmentRequestError):
             create_assessment(
                 'invalid_submission_uuid', u'another user',
-                self.OPTIONS_SELECTED, self.RUBRIC,
+                self.OPTIONS_SELECTED, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
+                scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
+            )
+
+    def test_create_assessment_invalid_criterion_feedback(self):
+        # Create a submission
+        submission = create_submission(self.STUDENT_ITEM, "Test answer")
+
+        # Mutate the criterion feedback to not include all the appropriate criteria.
+        criterion_feedback = {"clarify": "not", "accurate": "sure"}
+
+        # Attempt to create a self-assessment with criterion_feedback that do not match the rubric
+        with self.assertRaises(SelfAssessmentRequestError):
+            create_assessment(
+                submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
+                self.OPTIONS_SELECTED, criterion_feedback, self.OVERALL_FEEDBACK, self.RUBRIC,
                 scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
             )
 
@@ -118,7 +141,7 @@ class TestSelfApi(CacheResetTest):
         with self.assertRaises(SelfAssessmentRequestError):
             create_assessment(
                 submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-                options, self.RUBRIC,
+                options, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
                 scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
             )
 
@@ -134,7 +157,7 @@ class TestSelfApi(CacheResetTest):
         with self.assertRaises(SelfAssessmentRequestError):
             create_assessment(
                 submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-                options, self.RUBRIC,
+                options, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
                 scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
             )
 
@@ -146,11 +169,13 @@ class TestSelfApi(CacheResetTest):
         options = copy.deepcopy(self.OPTIONS_SELECTED)
         del options['clarity']
 
+        import pudb,sys as __sys;__sys.stdout=__sys.__stdout__;pudb.set_trace() # -={XX}=-={XX}=-={XX}=
+
         # Attempt to create a self-assessment with options that do not match the rubric
         with self.assertRaises(SelfAssessmentRequestError):
             create_assessment(
                 submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-                options, self.RUBRIC,
+                options, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
                 scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
             )
 
@@ -165,7 +190,7 @@ class TestSelfApi(CacheResetTest):
         # Do not override the scored_at timestamp, so it should be set to the current time
         assessment = create_assessment(
             submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-            self.OPTIONS_SELECTED, self.RUBRIC,
+            self.OPTIONS_SELECTED, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
         )
 
         # Retrieve the self-assessment
@@ -183,14 +208,14 @@ class TestSelfApi(CacheResetTest):
         # Self assess once
         assessment = create_assessment(
             submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-            self.OPTIONS_SELECTED, self.RUBRIC,
+            self.OPTIONS_SELECTED, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
         )
 
         # Attempt to self-assess again, which should raise an exception
         with self.assertRaises(SelfAssessmentRequestError):
             create_assessment(
                 submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-                self.OPTIONS_SELECTED, self.RUBRIC,
+                self.OPTIONS_SELECTED, self.CRITERION_FEEDBACK, self.OVERALL_FEEDBACK, self.RUBRIC,
             )
 
         # Expect that we still have the original assessment
@@ -213,17 +238,20 @@ class TestSelfApi(CacheResetTest):
             "options": []
         })
 
+        criterion_feedback = copy.deepcopy(self.CRITERION_FEEDBACK)
+        criterion_feedback['feedback only'] = "This is the feedback for the Zero Option Criterion."
+
         # Create a self-assessment for the submission
         assessment = create_assessment(
             submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-            self.OPTIONS_SELECTED, rubric,
+            self.OPTIONS_SELECTED, criterion_feedback, self.OVERALL_FEEDBACK, rubric,
             scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
         )
 
         # The self-assessment should have set the feedback for
         # the criterion with no options to an empty string
         self.assertEqual(assessment["parts"][2]["option"], None)
-        self.assertEqual(assessment["parts"][2]["feedback"], u"")
+        self.assertEqual(assessment["parts"][2]["feedback"], u"This is the feedback for the Zero Option Criterion.")
 
     def test_create_assessment_all_criteria_have_zero_options(self):
         # Create a submission to self-assess
@@ -237,14 +265,25 @@ class TestSelfApi(CacheResetTest):
         # Create a self-assessment for the submission
         # We don't select any options, since none of the criteria have options
         options_selected = {}
+
+        # However, because they don't have options, they need to have criterion feedback.
+        criterion_feedback = {
+            'clarity': 'I thought it was about as accurate as Scrubs is to the medical profession.',
+            'accuracy': 'I thought it was about as accurate as Scrubs is to the medical profession.'
+        }
+
+        overall_feedback = ""
+
         assessment = create_assessment(
             submission['uuid'], u'𝖙𝖊𝖘𝖙 𝖚𝖘𝖊𝖗',
-            options_selected, rubric,
-            scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
+            options_selected,  criterion_feedback, overall_feedback,
+            rubric, scored_at=datetime.datetime(2014, 4, 1).replace(tzinfo=pytz.utc)
         )
 
         # The self-assessment should have set the feedback for
         # all criteria to an empty string.
         for part in assessment["parts"]:
             self.assertEqual(part["option"], None)
-            self.assertEqual(part["feedback"], u"")
+            self.assertEqual(
+                part["feedback"], u'I thought it was about as accurate as Scrubs is to the medical profession.'
+            )

openassessment/management/commands/create_oa_submissions.py

@@ -107,7 +107,7 @@ class Command(BaseCommand):
             print "-- Creating self assessment"
             self_api.create_assessment(
                 submission_uuid, student_item['student_id'],
-                options_selected, rubric
+                options_selected, {}, "  ".join(loremipsum.get_paragraphs(2)), rubric
             )
 
     @property

openassessment/xblock/test/test_grade.py

@@ -115,9 +115,16 @@ class TestGrade(XBlockHandlerTestCase):
                 u'𝖋𝖊𝖊𝖉𝖇𝖆𝖈𝖐 𝖔𝖓𝖑𝖞': u"Ṫḧïṡ ïṡ ṡöṁë ḟëëḋḅäċḳ."
             }
 
+        self_assessment = copy.deepcopy(self.ASSESSMENTS[0])
+        self_assessment['criterion_feedback'] = {
+            u'𝖋𝖊𝖊𝖉𝖇𝖆𝖈𝖐 𝖔𝖓𝖑𝖞': "Feedback here",
+            u'Form': 'lots of feedback yes"',
+            u'𝓒𝓸𝓷𝓬𝓲𝓼𝓮': "such feedback"
+        }
+
         # Submit, assess, and render the grade view
         self._create_submission_and_assessments(
-            xblock, self.SUBMISSION, self.PEERS, peer_assessments, self.ASSESSMENTS[0]
+            xblock, self.SUBMISSION, self.PEERS, peer_assessments, self_assessment
         )
 
         # Render the grade section
@@ -365,5 +372,6 @@ class TestGrade(XBlockHandlerTestCase):
         if self_assessment is not None:
             self_api.create_assessment(
                 submission['uuid'], student_id, self_assessment['options_selected'],
+                self_assessment['criterion_feedback'], self_assessment['overall_feedback'],
                 {'criteria': xblock.rubric_criteria}
             )

openassessment/xblock/test/test_self.py

@@ -90,7 +90,10 @@ class TestSelfAssessment(XBlockHandlerTestCase):
         # Submit a self assessment for a rubric with a feedback-only criterion
         assessment_dict = {
             'options_selected': {u'vocabulary': u'good'},
-            'criterion_feedback': {u'vocabulary': 'Awesome job!'},
+            'criterion_feedback': {
+                u'vocabulary': 'Awesome job!',
+                u'𝖋𝖊𝖊𝖉𝖇𝖆𝖈𝖐 𝖔𝖓𝖑𝖞': 'fairly illegible.'
+            },
             'overall_feedback': u''
         }
         resp = self.request(xblock, 'self_assess', json.dumps(assessment_dict), response_format='json')
@@ -105,7 +108,7 @@ class TestSelfAssessment(XBlockHandlerTestCase):
         # Check the feedback-only criterion score/feedback
         self.assertEqual(assessment['parts'][1]['criterion']['name'], u'𝖋𝖊𝖊𝖉𝖇𝖆𝖈𝖐 𝖔𝖓𝖑𝖞')
         self.assertIs(assessment['parts'][1]['option'], None)
-        self.assertEqual(assessment['parts'][1]['feedback'], u'Awesome job!')
+        self.assertEqual(assessment['parts'][1]['feedback'], u'fairly illegible.')
 
     @scenario('data/self_assessment_scenario.xml', user_id='Bob')
     def test_self_assess_workflow_error(self, xblock):

openassessment/xblock/test/test_staff_info.py

@@ -32,6 +32,12 @@ ASSESSMENT_DICT = {
         "Clear-headed": "Yogi Berra",
         "Form": "Reddit",
     },
+    'criterion_feedback': {
+        "Concise": "Not very.",
+        "Clear-headed": "Indubitably",
+        "Form": "s ka tter ed"
+    }
+
 }
 
 
@@ -209,6 +215,8 @@ class TestCourseStaff(XBlockHandlerTestCase):
             submission['uuid'],
             STUDENT_ITEM["student_id"],
             ASSESSMENT_DICT['options_selected'],
+            ASSESSMENT_DICT['criterion_feedback'],
+            ASSESSMENT_DICT['overall_feedback'],
             {'criteria': xblock.rubric_criteria},
         )
 
@@ -235,6 +243,13 @@ class TestCourseStaff(XBlockHandlerTestCase):
             "Content": "Poor",
         }
 
+        criterion_feedback = {
+            "Ideas": "Dear diary: Lots of creativity from my dream journal last night at 2 AM,",
+            "Content": "Not as insightful as I had thought in the wee hours of the morning!"
+        }
+
+        overall_feedback = "I think I should tell more people about how important worms are for the ecosystem."
+
         bob_item = STUDENT_ITEM.copy()
         bob_item["item_id"] = xblock.scope_ids.usage_id
 
@@ -265,6 +280,8 @@ class TestCourseStaff(XBlockHandlerTestCase):
             submission['uuid'],
             STUDENT_ITEM["student_id"],
             options_selected,
+            criterion_feedback,
+            overall_feedback,
             {'criteria': xblock.rubric_criteria},
         )