Skip to content

Commit

Permalink
fix: bff skippable step handling (#2147)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
nsprenkle authored Dec 19, 2023
1 parent f4610c1 commit 96a774e
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 12 deletions.
2 changes: 1 addition & 1 deletion openassessment/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Initialization Information for Open Assessment Module
"""

__version__ = '6.0.18'
__version__ = '6.0.19'
41 changes: 38 additions & 3 deletions openassessment/xblock/apis/workflow_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""

Expand All @@ -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):
Expand Down
1 change: 0 additions & 1 deletion openassessment/xblock/test/data/self_assessment_closed.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
</criterion>
</rubric>
<assessments>
<assessment name="peer-assessment" must_grade="5" must_be_graded_by="3" />
<assessment name="self-assessment" due="2000-01-01"/>
</assessments>
</openassessment>
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@
</criterion>
</rubric>
<assessments>
<assessment name="peer-assessment" must_grade="5" must_be_graded_by="3" />
<assessment name="self-assessment" start="5999-01-01"/>
</assessments>
</openassessment>
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,6 @@ def test_self_assessment_closed(self, xblock):
"cancelledBy": None,
"teamInfo": {},
},
"peer": None,
"self": {
"closed": True,
"closedReason": "pastDue",
Expand Down Expand Up @@ -587,7 +586,6 @@ def test_self_assessment_not_available(self, xblock):
"cancelledBy": None,
"teamInfo": {},
},
"peer": None,
"self": {
"closed": True,
"closedReason": "notAvailableYet",
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down

0 comments on commit 96a774e

Please sign in to comment.