From 96a774e8f596f73e6c1520e41796a9cf82de627c Mon Sep 17 00:00:00 2001 From: Nathan Sprenkle Date: Tue, 19 Dec 2023 12:40:22 -0500 Subject: [PATCH] fix: bff skippable step handling (#2147) * feat: updates "has reached" check to account for skipped steps * feat: get next incomplete step Since new ORA UI uses "serial" instead of "parallel" progression, need to correctly progress to peer when skipped. Adds check for next incomplete step and uses for progress instead of naive pull from workflow status * chore: bump ORA to 6.0.19 * test: add test for skipped step * test: update some test scenarios to make testing easier --- openassessment/__init__.py | 2 +- openassessment/xblock/apis/workflow_api.py | 41 +++++++++++++++++-- .../test/data/self_assessment_closed.xml | 1 - .../test/data/self_assessment_unavailable.xml | 1 - .../ui_mixins/mfe/page_context_serializer.py | 2 +- .../mfe/test_page_context_serializer.py | 16 +++++++- package-lock.json | 4 +- package.json | 2 +- 8 files changed, 57 insertions(+), 12 deletions(-) diff --git a/openassessment/__init__.py b/openassessment/__init__.py index d005e0b20e..50c29bf661 100644 --- a/openassessment/__init__.py +++ b/openassessment/__init__.py @@ -2,4 +2,4 @@ Initialization Information for Open Assessment Module """ -__version__ = '6.0.18' +__version__ = '6.0.19' diff --git a/openassessment/xblock/apis/workflow_api.py b/openassessment/xblock/apis/workflow_api.py index 10de36e37f..4bbacd0961 100644 --- a/openassessment/xblock/apis/workflow_api.py +++ b/openassessment/xblock/apis/workflow_api.py @@ -34,12 +34,47 @@ def has_status(self): def status_details(self): return self.workflow.get("status_details", {}) + @property + def next_incomplete_step(self): + """ + Some steps (notably Peer) are "skipable" which means that the workflow auto-progresses + past this step if there are steps after it in the workflow. + + This is, for example, to allow working on assignment requirements while waiting for peer + grades. + + For certain circumstances, we'd like to just know the next incomplete / skipped step + instead of auto-progressing. + + Returns: + * "submission" when workflow doesn't exist + * Earliest incomplete step when workflow exists + * "done" when complete + * "cancelled" when cancelled + """ + step_order = self._block.rubric_assessments + status = self.status + status_details = self.status_details + + if not status_details: + return "submission" + + if status in ("done", "cancelled"): + return status + + for next_step in [step['name'] for step in step_order]: + workflow_step_name = WorkflowStep(next_step).workflow_step_name + if status_details[workflow_step_name].get('complete', False) is False: + return workflow_step_name + + return "done" + def has_reached_given_step(self, requested_step, current_workflow_step=None): """ Helper to determine if are far enough through a workflow to request data for a step. Returns: - True if we are on or have completed the requested step for this ORA. + True if we are on or have skipped / completed the requested step for this ORA. False otherwise. """ @@ -54,9 +89,9 @@ def has_reached_given_step(self, requested_step, current_workflow_step=None): if requested_step == current_workflow_step: return True - # Have reached any step you have completed + # Have reached any step you have completed / skipped step_status = self.status_details.get(requested_step, {}) - return step_status.get("complete", False) + return step_status.get("complete", False) or step_status.get("skipped", False) @property def is_peer_complete(self): diff --git a/openassessment/xblock/test/data/self_assessment_closed.xml b/openassessment/xblock/test/data/self_assessment_closed.xml index 9513510b62..37743eb860 100644 --- a/openassessment/xblock/test/data/self_assessment_closed.xml +++ b/openassessment/xblock/test/data/self_assessment_closed.xml @@ -43,7 +43,6 @@ - diff --git a/openassessment/xblock/test/data/self_assessment_unavailable.xml b/openassessment/xblock/test/data/self_assessment_unavailable.xml index 40a4717acf..adbfbbea78 100644 --- a/openassessment/xblock/test/data/self_assessment_unavailable.xml +++ b/openassessment/xblock/test/data/self_assessment_unavailable.xml @@ -87,7 +87,6 @@ - diff --git a/openassessment/xblock/ui_mixins/mfe/page_context_serializer.py b/openassessment/xblock/ui_mixins/mfe/page_context_serializer.py index b7eedb65f1..ce81b67632 100644 --- a/openassessment/xblock/ui_mixins/mfe/page_context_serializer.py +++ b/openassessment/xblock/ui_mixins/mfe/page_context_serializer.py @@ -266,7 +266,7 @@ def get_activeStepName(self, instance): else: raise UnknownActiveStepException("Workflow is in waiting but no staff or peer step is required.") else: - return STEP_NAME_MAPPINGS[instance.workflow_data.status] + return STEP_NAME_MAPPINGS[instance.workflow_data.next_incomplete_step] class PageDataSerializer(Serializer): 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 daa71ded16..e7cf7ca541 100644 --- a/openassessment/xblock/ui_mixins/mfe/test_page_context_serializer.py +++ b/openassessment/xblock/ui_mixins/mfe/test_page_context_serializer.py @@ -555,7 +555,6 @@ def test_self_assessment_closed(self, xblock): "cancelledBy": None, "teamInfo": {}, }, - "peer": None, "self": { "closed": True, "closedReason": "pastDue", @@ -587,7 +586,6 @@ def test_self_assessment_not_available(self, xblock): "cancelledBy": None, "teamInfo": {}, }, - "peer": None, "self": { "closed": True, "closedReason": "notAvailableYet", @@ -628,6 +626,20 @@ def test_peer_the_staff_assessment__waiting(self, xblock): # Expect active step to be staff instead of waiting or peer self.assertEqual('staff', progress_data['activeStepName']) + @scenario("data/peer_assessment_scenario.xml", user_id="Alan") + def test_peer_skipped(self, xblock): + # Given I have a skipped peer step + self.create_test_submission(xblock) + + self.assertTrue(xblock.workflow_data.is_peer_skipped) + self.assertTrue(xblock.workflow_data.is_self) + + # When I ask for progress + progress_data = ProgressSerializer(xblock).data + + # Expect active step to be staff instead of waiting or peer + self.assertEqual('peer', progress_data['activeStepName']) + @scenario("data/self_only_scenario.xml", user_id="Alan") def test_waiting_error(self, xblock): # Given I am waiting when I shouldn't be able to be waiting diff --git a/package-lock.json b/package-lock.json index 789b781fb2..07535d30bc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "edx-ora2", - "version": "6.0.18", + "version": "6.0.19", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "edx-ora2", - "version": "6.0.17", + "version": "6.0.19", "dependencies": { "@edx/frontend-build": "8.0.6", "@openedx/paragon": "^21.5.7", diff --git a/package.json b/package.json index 65803a0b02..89cefba30f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "edx-ora2", - "version": "6.0.18", + "version": "6.0.19", "repository": "https://github.com/openedx/edx-ora2.git", "dependencies": { "@edx/frontend-build": "8.0.6",