feat: BFF integration work (#2115)

* fix: fix bad requested step validation

* fix: fix request context for grades view

* feat: default ora iframe height

* fix: correct draft save data shape

* refactor: change submission action args

* fix: allow null / blank feedback for assessments

* fix: update error code to contract

* fix: check for peer step when asking for peers

* refactor: always return grades when ORA complete

* fix: step order and forwarding modal close events

* fix: correctly get workflow from frontend step

* refactor: update step ordering logic

* fix: no student training selections when complete

* refactor: Return none response for end of training

* fix: remove rather than hide unused ORA view (#2119)

* fix: enable author view

* chore: bump ORA to 6.0.4

---------

Co-authored-by: Ben Warzeski <[email protected]>
Co-authored-by: Jansen Kantor <[email protected]>

openassessment/__init__.py

@@ -2,4 +2,4 @@
 Initialization Information for Open Assessment Module
 """
 
-__version__ = '6.0.3'
+__version__ = '6.0.4'

openassessment/templates/openassessmentblock/base.html

@@ -61,7 +61,7 @@ <h4 class="step__title">
           frameborder="0"
           id="ora-iframe-{{xblock_id}}"
           src="http://localhost:1992/xblock/{{course_id}}/{{xblock_id}}"
-          style="width: 100%"
+          style="width: 100%; height: 1000px"
           onload="loadResizeEvent()"
         ></iframe>
         {% if leaderboard_ui_model %}
@@ -98,14 +98,18 @@ <h4 class="step__title">
   const isMobile = window.navigator.userAgent.includes("org.edx.mobile");
   const mfe_views = ("{{mfe_views}}" === 'True') || isMobile;
   const toHide = mfe_views ? 'ora-legacy-view' : 'ora-view';
-  document.getElementById(toHide).style.display = 'none';
+  const elementToRemove = document.getElementById(toHide);
+  elementToRemove.parentNode.removeChild(elementToRemove);
   function loadResizeEvent() {
     window.addEventListener('message', function(event) {
+      const iframe = document.getElementById('ora-iframe-{{xblock_id}}');
       if (event.data.type === 'plugin.modal') {
         window.parent.postMessage(event.data, document.referrer);
       }
+      if (event.data.type === 'plugin.modal-close') {
+        iframe.contentWindow.postMessage(event.data, '*');
+      }
       if (event.data.type === 'ora-resize') {
-        const iframe = document.getElementById('ora-iframe-{{xblock_id}}');
         const { height } = event.data.payload;
         iframe.height = height;
       }

openassessment/xblock/apis/submissions/submissions_actions.py

@@ -31,10 +31,18 @@
 logger = logging.getLogger(__name__)  # pylint: disable=invalid-name
 
 
-def submit(data, block_config_data, block_submission_data, block_workflow_data):
-    student_sub_data = data["submission"]
+def submit(text_responses, block_config_data, block_submission_data, block_workflow_data):
+    """
+    Submit response for a student.
+
+    Args:
+    * text_responses - List of text responses for submission
+    * block_config_data - ORAConfigAPI
+    * block_submission_data - SubmissionAPI
+    * block_workflow_data - WorkflowAPI
+    """
     success, msg = validate_submission(
-        student_sub_data,
+        text_responses,
         block_config_data.prompts,
         block_config_data.translate,
         block_config_data.text_response,
@@ -57,15 +65,15 @@ def submit(data, block_config_data, block_submission_data, block_workflow_data):
         if block_config_data.is_team_assignment():
             return create_team_submission(
                 student_item_dict,
-                student_sub_data,
+                text_responses,
                 block_config_data,
                 block_submission_data,
                 block_workflow_data
             )
         else:
             return create_submission(
                 student_item_dict,
-                student_sub_data,
+                text_responses,
                 block_config_data,
                 block_submission_data,
                 block_workflow_data

openassessment/xblock/openassessmentblock.py

@@ -108,7 +108,7 @@ class OpenAssessmentBlock(
         "student-training",
         "peer-assessment",
         "self-assessment",
-        "staff-assessment",
+        "staff-assessment"
     ]
 
     VALID_ASSESSMENT_TYPES_FOR_TEAMS = [  # pylint: disable=invalid-name
@@ -524,6 +524,9 @@ def get_student_item_dict(self, anonymous_user_id=None):
         }
         return student_item_dict
 
+    # You need to tell studio that there is an author view, it won't go searching for it
+    has_author_view = True
+
     @togglable_mobile_support
     def author_view(self, context=None):  # pylint: disable=unused-argument
         """The main view of OpenAssessmentBlock, displayed when viewing courses.
@@ -622,9 +625,11 @@ def uses_default_assessment_order(self):
         """
         Determine if our steps have been reordered (omission of steps is fine)
         """
+        mfe_supported_step_ordering = ['student-training', 'self-assessment', 'peer-assessment', 'staff-assessment']
+
         last_step_index = 0
         for assessment_step in self.assessment_steps:
-            step_index = self.VALID_ASSESSMENT_TYPES.index(assessment_step)
+            step_index = mfe_supported_step_ordering.index(assessment_step)
 
             if step_index < last_step_index:
                 return False

openassessment/xblock/test/data/assessment_steps_reordered.xml

@@ -43,7 +43,7 @@
         </criterion>
     </rubric>
     <assessments>
-        <assessment name="self-assessment" />
+        <assessment name="staff-assessment" required="true"/>
         <assessment name="peer-assessment" must_grade="5" must_be_graded_by="3" />
     </assessments>
 </openassessment>

openassessment/xblock/test/test_openassessment.py

@@ -706,7 +706,7 @@ def test_custom_file_upload_loads_file_allow_list(self, xblock):
         self.assertEqual(xblock.white_listed_file_types, ["pdf"])
 
     @ddt.data(True, False)
-    @scenario('data/basic_scenario.xml')
+    @scenario('data/simple_self_staff_scenario.xml')
     def test_mfe_views_supported__teams(self, xblock, mock_teams_assignment):
         # Given I'm on / not on a team assignment
         xblock.is_team_assignment = Mock(return_value=mock_teams_assignment)

openassessment/xblock/ui_mixins/legacy/handlers_mixin.py

@@ -73,7 +73,12 @@ def submit(self, data, suffix=""):  # pylint: disable=unused-argument
                 self.config_data.translate('"submission" required to submit answer.'),
             )
         try:
-            submission = submissions_actions.submit(data, self.config_data, self.submission_data, self.workflow_data)
+            submission = submissions_actions.submit(
+                data["submission"],
+                self.config_data,
+                self.submission_data,
+                self.workflow_data
+            )
             return (
                 True,
                 submission.get("student_item"),

openassessment/xblock/ui_mixins/mfe/assessment_serializers.py

@@ -203,7 +203,7 @@ class MfeAssessmentCriterionSerializer(Serializer):
     }
     """
     selectedOption = IntegerField()
-    feedback = CharField()
+    feedback = CharField(allow_blank=True, allow_null=True)
 
 
 class MfeAssessmentDataSerializer(Serializer):

openassessment/xblock/ui_mixins/mfe/mixin.py

@@ -32,7 +32,6 @@
 from openassessment.xblock.ui_mixins.mfe.constants import error_codes, handler_suffixes
 from openassessment.xblock.ui_mixins.mfe.ora_config_serializer import OraBlockInfoSerializer
 from openassessment.xblock.ui_mixins.mfe.page_context_serializer import PageDataSerializer
-from openassessment.xblock.ui_mixins.mfe.serializer_utils import STEP_NAME_MAPPINGS
 from openassessment.xblock.ui_mixins.mfe.submission_serializers import (
     AddFileRequestSerializer,
     FileUploadCallbackRequestSerializer
@@ -48,12 +47,23 @@ def __init__(self, status_code, error_code, error_context=''):
         super().__init__(
             status_code,
             {
-                'error_code': error_code,
-                'error_context': error_context
+                'errorCode': error_code,
+                'errorContext': error_context
             }
         )
 
 
+# Map requested ORA app step to workflow step name
+MFE_STEP_TO_WORKFLOW_MAPPINGS = {
+    "submission": "submission",
+    "studentTraining": "training",
+    "peer": "peer",
+    "self": "self",
+    "staff": "staff",
+    "done": "done",
+}
+
+
 class MfeMixin:
     @XBlock.json_handler
     def get_block_info(self, data, suffix=""):  # pylint: disable=unused-argument
@@ -83,7 +93,7 @@ def get_learner_data(self, data, suffix=""):  # pylint: disable=unused-argument
             return PageDataSerializer(self, context=serializer_context).data
 
         # Check to see if user can access this workflow step
-        requested_workflow_step = STEP_NAME_MAPPINGS[requested_step]
+        requested_workflow_step = MFE_STEP_TO_WORKFLOW_MAPPINGS[requested_step]
         if not self.workflow_data.has_reached_given_step(
             requested_workflow_step,
             current_workflow_step=current_workflow_step
@@ -96,7 +106,7 @@ def get_learner_data(self, data, suffix=""):  # pylint: disable=unused-argument
 
     def _submission_draft_handler(self, data):
         try:
-            student_submission_data = data['response']['text_responses']
+            student_submission_data = data['response']['textResponses']
             submissions_actions.save_submission_draft(student_submission_data, self.config_data, self.submission_data)
         except KeyError as e:
             raise OraApiException(400, error_codes.INCORRECT_PARAMETERS) from e
@@ -108,7 +118,8 @@ def _submission_draft_handler(self, data):
     def _submission_create_handler(self, data):
         from submissions import api as submission_api
         try:
-            submissions_actions.submit(data, self.config_data, self.submission_data, self.workflow_data)
+            text_responses = data["submission"]["textResponses"]
+            submissions_actions.submit(text_responses, self.config_data, self.submission_data, self.workflow_data)
         except KeyError as e:
             raise OraApiException(400, error_codes.INCORRECT_PARAMETERS) from e
         except SubmissionValidationException as e:
@@ -324,6 +335,11 @@ def _assessment_submit_handler(self, data):
         return MfeAssessmentDataSerializer(data).data
 
     def _assessment_get_peer_handler(self):
+
+        # Raise an exception if we don't have a peer step
+        if "peer-assessment" not in self.assessment_steps:
+            raise OraApiException(400, error_codes.INACCESSIBLE_STEP, error_context="No peer step for ORA")
+
         # Call get_peer_submission to grab a new peer submission
         self.peer_assessment_data().get_peer_submission()
 

openassessment/xblock/ui_mixins/mfe/page_context_serializer.py

@@ -150,6 +150,9 @@ def get_expectedRubricSelections(self, instance):
             ... etc.
         }
         """
+        if not instance.example:
+            return None
+
         criteria = instance.example["rubric"]['criteria']
         options_selected = instance.example["options_selected"]
 
@@ -252,7 +255,7 @@ def get_activeStepName(self, instance):
             return "submission"
         elif instance.workflow_data.is_waiting:
             # If we are waiting, we are either waiting on a peer or a staff grade.
-            # Staff takes precidence, so if a required (not automatically inserted) staff
+            # Staff takes precedence, so if a required (not automatically inserted) staff
             # step exists, we are considered to be in "staff". If there is a peer, we are
             # considered to be in "peer"
             workflow_requirements = instance.workflow_data.workflow_requirements
@@ -290,12 +293,16 @@ class PageDataSerializer(Serializer):
     assessment = SerializerMethodField()
 
     def to_representation(self, instance):
+
+        # Check required context
         if "requested_step" not in self.context:
             raise ValidationError("Missing required context: requested_step")
         if "current_workflow_step" not in self.context:
             raise ValidationError("Missing required context: current_workflow_step")
 
-        if self.context.get("requested_step", "submission") not in handler_suffixes.STEP_SUFFIXES:
+        # validate step values
+        requested_step = self.context.get("requested_step")
+        if requested_step is not None and requested_step not in handler_suffixes.STEP_SUFFIXES:
             raise ValidationError(f"Bad step name: {self.context.get('requested_step')}")
 
         return super().to_representation(instance)
@@ -328,9 +335,11 @@ def get_response(self, instance):
             # Submitted response
             return SubmissionSerializer(learner_submission_data).data
 
-        # Student Training - return next example to practice
+        # Student Training - return next example to practice or None
         elif requested_step == "studentTraining":
             response = instance.student_training_data.example
+            if response is None:
+                return None
 
         # Peer
         elif requested_step == "peer":
@@ -357,10 +366,8 @@ def get_response(self, instance):
 
     def get_assessment(self, instance):
         """
-         we get an assessment for the current assessment step.
+        Get my assessments (grades) when my ORA is complete.
         """
-        # Assessment Views
-        if self.context.get("view") == "assessment":
+        if instance.workflow_data.is_done:
             return AssessmentGradeSerializer(instance.api_data, context=self.context).data
-        else:
-            return None
+        return None

openassessment/xblock/ui_mixins/mfe/test_mfe_mixin.py

@@ -67,8 +67,8 @@ def mock_get_url(self, expected_file_urls=None):
                 mock_unsubmitted_urls.side_effect = expected_file_urls.get
                 yield
 
-    DEFAULT_DRAFT_VALUE = {'response': {'text_responses': ['hi']}}
-    DEFAULT_SUBMIT_VALUE = {'response': {'text_responses': ['Hello World', 'Goodbye World']}}
+    DEFAULT_DRAFT_VALUE = {'response': {'textResponses': ['hi']}}
+    DEFAULT_SUBMIT_VALUE = {'submission': {'textResponses': ['Hello World', 'Goodbye World']}}
     DEFAULT_DELETE_FILE_VALUE = {'fileIndex': 1}
 
     DEFAULT_ASSESSMENT_SUBMIT_VALUE = {
@@ -174,8 +174,8 @@ def request_assessment_get_peer(self, xblock):
 def assert_error_response(response, status_code, error_code, context=''):
     assert response.status_code == status_code
     assert response.json['error'] == {
-        'error_code': error_code,
-        'error_context': context
+        'errorCode': error_code,
+        'errorContext': context
     }
 
 
@@ -277,8 +277,8 @@ def test_jump_to_inaccessible_step(self, xblock, inaccessible_step, mock_seriali
 
         expected_body = {
             'error': {
-                'error_code': 'ERR_INACCESSIBLE_STEP',
-                'error_context': f'Inaccessible step: {inaccessible_step}'
+                'errorCode': 'ERR_INACCESSIBLE_STEP',
+                'errorContext': f'Inaccessible step: {inaccessible_step}'
             }
         }
         self.assertDictEqual(expected_body, json.loads(response.body))
@@ -612,7 +612,7 @@ def test_draft_save(self, xblock):
         with self._mock_save_submission_draft() as mock_draft:
             resp = self.request_save_draft(xblock)
             assert resp.status_code == 200
-            assert_called_once_with_helper(mock_draft, self.DEFAULT_DRAFT_VALUE['response']['text_responses'], 2)
+            assert_called_once_with_helper(mock_draft, self.DEFAULT_DRAFT_VALUE['response']['textResponses'], 2)
 
 
 class SubmissionCreateTest(MFEHandlersTestBase):
@@ -685,7 +685,7 @@ def test_create_submission(self, xblock):
         with self._mock_create_submission() as mock_submit:
             resp = self.request_create_submission(xblock)
             assert resp.status_code == 200
-            assert_called_once_with_helper(mock_submit, self.DEFAULT_SUBMIT_VALUE, 3)
+            assert_called_once_with_helper(mock_submit, self.DEFAULT_SUBMIT_VALUE["submission"]["textResponses"], 3)
 
 
 @ddt.ddt
@@ -740,7 +740,7 @@ def _mock_delete_uploaded_file(self, **kwargs):
     def test_bad_inputs(self, xblock, payload):
         resp = self.request_upload_files(xblock, payload)
         assert resp.status_code == 400
-        assert resp.json['error']['error_code'] == error_codes.INCORRECT_PARAMETERS
+        assert resp.json['error']['errorCode'] == error_codes.INCORRECT_PARAMETERS
 
     @scenario("data/basic_scenario.xml")
     def test_file_upload_error(self, xblock):
@@ -869,7 +869,7 @@ def _mock_get_download_url(self, **kwargs):
     def test_bad_params(self, xblock, payload):
         resp = self.request_file_callback(xblock, payload)
         assert resp.status_code == 400
-        assert resp.json['error']['error_code'] == error_codes.INCORRECT_PARAMETERS
+        assert resp.json['error']['errorCode'] == error_codes.INCORRECT_PARAMETERS
 
     @scenario("data/basic_scenario.xml")
     def test_success(self, xblock):
@@ -947,15 +947,15 @@ def mock_assess_functions(self, self_kwargs=None, training_kwargs=None, peer_kwa
     def test_incorrect_params(self, xblock, payload):
         resp = self.request_assessment_submit(xblock, payload)
         assert resp.status_code == 400
-        assert resp.json['error']['error_code'] == error_codes.INCORRECT_PARAMETERS
+        assert resp.json['error']['errorCode'] == error_codes.INCORRECT_PARAMETERS
 
     @ddt.data(None, 'cancelled', 'done', 'ai')
     @scenario("data/basic_scenario.xml")
     def test_not_allowed_step_error(self, xblock, status):
         with self.mock_workflow_status(status):
             resp = self.request_assessment_submit(xblock)
         assert resp.status_code == 400
-        assert resp.json['error']['error_code'] == error_codes.INVALID_STATE_TO_ASSESS
+        assert resp.json['error']['errorCode'] == error_codes.INVALID_STATE_TO_ASSESS
 
     @ddt.unpack
     @ddt.data(