diff --git a/openassessment/__init__.py b/openassessment/__init__.py index 0eea6b16d1..85f6daf7a8 100644 --- a/openassessment/__init__.py +++ b/openassessment/__init__.py @@ -2,4 +2,4 @@ Initialization Information for Open Assessment Module """ -__version__ = '6.0.3' +__version__ = '6.0.4' diff --git a/openassessment/templates/openassessmentblock/base.html b/openassessment/templates/openassessmentblock/base.html index 2bc378f73c..c1c62acb92 100644 --- a/openassessment/templates/openassessmentblock/base.html +++ b/openassessment/templates/openassessmentblock/base.html @@ -61,7 +61,7 @@

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()" > {% if leaderboard_ui_model %} @@ -98,14 +98,18 @@

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; } diff --git a/openassessment/xblock/apis/submissions/submissions_actions.py b/openassessment/xblock/apis/submissions/submissions_actions.py index e52c99281d..245fd459fe 100644 --- a/openassessment/xblock/apis/submissions/submissions_actions.py +++ b/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,7 +65,7 @@ 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 @@ -65,7 +73,7 @@ def submit(data, 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 diff --git a/openassessment/xblock/openassessmentblock.py b/openassessment/xblock/openassessmentblock.py index eb2e506b54..a91854dc0c 100644 --- a/openassessment/xblock/openassessmentblock.py +++ b/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 diff --git a/openassessment/xblock/test/data/assessment_steps_reordered.xml b/openassessment/xblock/test/data/assessment_steps_reordered.xml index 2430415926..e132ef29e0 100644 --- a/openassessment/xblock/test/data/assessment_steps_reordered.xml +++ b/openassessment/xblock/test/data/assessment_steps_reordered.xml @@ -43,7 +43,7 @@ - + diff --git a/openassessment/xblock/test/test_openassessment.py b/openassessment/xblock/test/test_openassessment.py index 9d8f7ff26d..136664c33b 100644 --- a/openassessment/xblock/test/test_openassessment.py +++ b/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) diff --git a/openassessment/xblock/ui_mixins/legacy/handlers_mixin.py b/openassessment/xblock/ui_mixins/legacy/handlers_mixin.py index 5e201c4bcd..56f067bc99 100644 --- a/openassessment/xblock/ui_mixins/legacy/handlers_mixin.py +++ b/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"), diff --git a/openassessment/xblock/ui_mixins/mfe/assessment_serializers.py b/openassessment/xblock/ui_mixins/mfe/assessment_serializers.py index f37b6d3cd1..eca6421609 100644 --- a/openassessment/xblock/ui_mixins/mfe/assessment_serializers.py +++ b/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): diff --git a/openassessment/xblock/ui_mixins/mfe/mixin.py b/openassessment/xblock/ui_mixins/mfe/mixin.py index 9edfc8ad31..628d040531 100644 --- a/openassessment/xblock/ui_mixins/mfe/mixin.py +++ b/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() diff --git a/openassessment/xblock/ui_mixins/mfe/page_context_serializer.py b/openassessment/xblock/ui_mixins/mfe/page_context_serializer.py index cad32e865b..a988bd7532 100644 --- a/openassessment/xblock/ui_mixins/mfe/page_context_serializer.py +++ b/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 diff --git a/openassessment/xblock/ui_mixins/mfe/test_mfe_mixin.py b/openassessment/xblock/ui_mixins/mfe/test_mfe_mixin.py index 56a284fffb..23bf577978 100644 --- a/openassessment/xblock/ui_mixins/mfe/test_mfe_mixin.py +++ b/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,7 +947,7 @@ 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") @@ -955,7 +955,7 @@ 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( diff --git a/openassessment/xblock/ui_mixins/mfe/test_page_context_serializer.py b/openassessment/xblock/ui_mixins/mfe/test_page_context_serializer.py index 184b40e9b3..a5cf1e7364 100644 --- a/openassessment/xblock/ui_mixins/mfe/test_page_context_serializer.py +++ b/openassessment/xblock/ui_mixins/mfe/test_page_context_serializer.py @@ -74,6 +74,20 @@ def test_missing_context(self, xblock, missing_context_entry): with self.assertRaises(ValidationError): _ = PageDataSerializer(xblock, context=context).data + @patch("openassessment.xblock.ui_mixins.mfe.page_context_serializer.AssessmentResponseSerializer") + @patch("openassessment.xblock.ui_mixins.mfe.page_context_serializer.SubmissionSerializer") + @scenario("data/basic_scenario.xml", user_id="Alan") + def test_no_requested_step(self, xblock, mock_submission_serializer, mock_assessment_serializer): + # Given I don't request a step (allowed for asking progress data) + context = {"requested_step": None, "current_workflow_step": "peer"} + + # When I ask for my submission data + _ = PageDataSerializer(xblock, context=context).data + + # Then I load page data, without any response data + mock_submission_serializer.assert_not_called() + mock_assessment_serializer.assert_not_called() + @ddt.ddt class TestPageDataSerializerAssessment(XBlockHandlerTestCase, SubmitAssessmentsMixin): @@ -83,7 +97,7 @@ class TestPageDataSerializerAssessment(XBlockHandlerTestCase, SubmitAssessmentsM def setUp(self): """For these tests, we are always in assessment view""" - self.context = {"view": "assessment"} + self.context = {"requested_step": "done"} return super().setUp() @scenario("data/student_training.xml", user_id="Alan") diff --git a/package-lock.json b/package-lock.json index 0bbdd5d5f1..e74fe01e1a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "edx-ora2", - "version": "6.0.3", + "version": "6.0.4", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "edx-ora2", - "version": "6.0.2", + "version": "6.0.4", "dependencies": { "@edx/frontend-build": "^6.1.1", "@edx/paragon": "^20.9.2", diff --git a/package.json b/package.json index 8e96f53867..3afba926d7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "edx-ora2", - "version": "6.0.3", + "version": "6.0.4", "repository": "https://github.com/openedx/edx-ora2.git", "dependencies": { "@edx/frontend-build": "^6.1.1",