Skip to content

Commit

Permalink
fix: block submission and assessments when steps closed (openedx#2168)
Browse files Browse the repository at this point in the history
* fix: block submitting or assessing on closed steps

* test: add tests for blocking submit/assess when step closed

* fix: block loading of page data for closed step with 400 error

* refactor: more consistent step data accessor names

* chore: bump ORA to 6.0.31
  • Loading branch information
nsprenkle authored Jan 31, 2024
1 parent 9ed4f62 commit 78fa034
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 11 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.30'
__version__ = '6.0.31'
2 changes: 1 addition & 1 deletion openassessment/staffgrader/staff_grader_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ def submit_staff_assessment(self, submission_uuid, data, suffix=''): # pylint:
data['overall_feedback'],
data.get('assess_type', 'regrade'),
self.config_data,
self.staff_data,
self.staff_assessment_data,
)
if self.is_team_assignment():
do_team_staff_assessment(*args, team_submission_uuid=submission_uuid)
Expand Down
4 changes: 2 additions & 2 deletions openassessment/xblock/openassessmentblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,11 @@ def peer_assessment_data(self, continue_grading=False):
return PeerAssessmentAPI(self, continue_grading)

@property
def self_data(self):
def self_assessment_data(self):
return SelfAssessmentAPI(self)

@property
def staff_data(self):
def staff_assessment_data(self):
return StaffAssessmentAPI(self)

@property
Expand Down
4 changes: 2 additions & 2 deletions openassessment/xblock/ui_mixins/legacy/handlers_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ def failure_response(reason):
data['overall_feedback'],
self.config_data,
self.workflow_data,
self.self_data,
self.self_assessment_data,
)
except ReviewerMustHaveSubmittedException:
return failure_response('You must submit a response before you can perform a self-assessment.')
Expand Down Expand Up @@ -376,7 +376,7 @@ def failure_response(reason):
data['overall_feedback'],
data.get('assess_type', 'regrade'),
self.config_data,
self.staff_data,
self.staff_assessment_data,
)
except (StaffAssessmentRequestError, StaffAssessmentInternalError):
return failure_response('Your team assessment could not be submitted.')
Expand Down
48 changes: 47 additions & 1 deletion openassessment/xblock/ui_mixins/mfe/mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ def get_learner_data(self, data, suffix=""): # pylint: disable=unused-argument
if not requested_step:
return PageDataSerializer(self, context=serializer_context).data

# Raise error if step is closed
elif not self.is_step_open(requested_step):
raise OraApiException(400, error_codes.INACCESSIBLE_STEP, f"Inaccessible step: {requested_step}")

# Check to see if user can access this workflow step
requested_workflow_step = MFE_STEP_TO_WORKFLOW_MAPPINGS[requested_step]
if not self.workflow_data.has_reached_given_step(
Expand All @@ -106,6 +110,39 @@ def get_learner_data(self, data, suffix=""): # pylint: disable=unused-argument
serializer_context["requested_step"] = requested_step
return PageDataSerializer(self, context=serializer_context).data

def is_step_open(self, step_name):
"""
Determine whether or not the requested step is open
Return: If the problem is open or not (Bool)
Raises: OraApiException if the step name is invalid
"""
step_data = None

# Users can always view a submission they've previously submitted
if step_name == "submission" and self.submission_data.has_submitted:
return True
# And whether they can get to grades, depends on the workflow being "done"
elif step_name == "done":
return self.workflow_data.is_done

# Otherwise, get the info for the current step to determine access
if step_name == "submission":
step_data = self.submission_data
elif step_name == "studentTraining":
step_data = self.student_training_data
elif step_name == "peer":
step_data = self.peer_assessment_data()
elif step_name == "self":
step_data = self.self_assessment_data
elif step_name == "staff":
step_data = self.staff_assessment_data
else:
raise OraApiException(400, error_codes.UNKNOWN_SUFFIX, error_context=f"Bad step name: {step_name}")

# Return if the step is currently open
return not step_data.problem_closed

def _submission_draft_handler(self, data):
try:
student_submission_data = data['response']['textResponses']
Expand Down Expand Up @@ -146,6 +183,9 @@ def submission(self, data, suffix=""):
if suffix == handler_suffixes.SUBMISSION_DRAFT:
return self._submission_draft_handler(data)
elif suffix == handler_suffixes.SUBMISSION_SUBMIT:
# Return an error if the submission step is not open
if not self.is_step_open("submission"):
raise OraApiException(400, error_codes.INACCESSIBLE_STEP)
return self._submission_create_handler(data)
else:
raise OraApiException(404, error_codes.UNKNOWN_SUFFIX)
Expand Down Expand Up @@ -302,8 +342,14 @@ def _assessment_submit_handler(self, data):
assessment_data = serializer.to_legacy_format(self)
requested_step = serializer.data['step']
try:
# Block assessing a closed step
if not self.is_step_open(requested_step):
raise OraApiException(400, error_codes.INACCESSIBLE_STEP, f"Inaccessible step: {requested_step}")

# Block assessing a cancelled submission
if self.workflow_data.is_cancelled:
raise InvalidStateToAssess()

if requested_step == 'peer':
peer_assess(
assessment_data['options_selected'],
Expand All @@ -320,7 +366,7 @@ def _assessment_submit_handler(self, data):
assessment_data['feedback'],
self.config_data,
self.workflow_data,
self.self_data
self.self_assessment_data
)
elif requested_step == 'studentTraining':
corrections = training_assess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class StepInfoSerializer(Serializer):
submission = SubmissionStepInfoSerializer(source="submission_data")
studentTraining = StudentTrainingStepInfoSerializer(source="student_training_data")
peer = PeerStepInfoSerializer(source="peer_assessment_data")
_self = SelfStepInfoSerializer(source="self_data")
_self = SelfStepInfoSerializer(source="self_assessment_data")

def get_fields(self):
# Hack to name one of the output fields "self", a reserved word
Expand Down
22 changes: 22 additions & 0 deletions openassessment/xblock/ui_mixins/mfe/test_mfe_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,14 @@ def test_create_submission(self, xblock):
assert resp.status_code == 200
assert_called_once_with_helper(mock_submit, self.DEFAULT_SUBMIT_VALUE["submission"]["textResponses"], 3)

@patch("openassessment.xblock.ui_mixins.mfe.mixin.MfeMixin.is_step_open")
@scenario("data/basic_scenario.xml")
def test_blocks_submit_when_step_closed(self, xblock, mock_is_step_open):
mock_is_step_open.return_value = False
with self._mock_create_submission():
resp = self.request_create_submission(xblock)
assert_error_response(resp, 400, error_codes.INACCESSIBLE_STEP)


@ddt.ddt
class FileUploadTest(MFEHandlersTestBase):
Expand Down Expand Up @@ -1012,3 +1020,17 @@ def test_training_assess_corrections(self, xblock):
resp = self.request_assessment_submit(xblock, step='studentTraining')

assert_error_response(resp, 400, error_codes.TRAINING_ANSWER_INCORRECT, corrections)

@patch("openassessment.xblock.ui_mixins.mfe.mixin.MfeMixin.is_step_open")
@ddt.data('self', 'studentTraining', 'peer')
@scenario("data/basic_scenario.xml")
def test_blocks_assess_when_step_closed(self, xblock, mfe_step, mock_is_step_open):
mock_is_step_open.return_value = False
with self.mock_assess_functions() as assess_mocks:
with self.mock_workflow_status(mfe_step):
resp = self.request_assessment_submit(xblock, step=mfe_step)

assert_error_response(resp, 400, error_codes.INACCESSIBLE_STEP, f"Inaccessible step: {mfe_step}")
assess_mocks.self.assert_not_called()
assess_mocks.training.assert_not_called()
assess_mocks.peer.assert_not_called()
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.30",
"version": "6.0.31",
"repository": "https://github.com/openedx/edx-ora2.git",
"dependencies": {
"@edx/frontend-build": "8.0.6",
Expand Down

0 comments on commit 78fa034

Please sign in to comment.