Skip to content

Commit

Permalink
fix: BFF fix peer get and deleted files in grades (#2133)
Browse files Browse the repository at this point in the history
* fix: always return a response when requesting peer

* refactor: remove unused get_peer endpoint

* fix: remove unused import

* fix: hide deleted files from submission

* chore: bump ORA to version 6.0.11
  • Loading branch information
nsprenkle authored Dec 6, 2023
1 parent 95ea346 commit 4012e00
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 91 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.10'
__version__ = '6.0.11'
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
FILE_ADD = 'add'
FILE_DELETE = 'delete'
ASSESSMENT_SUBMIT = 'submit'
ASSESSMENT_GET_PEER = 'get_peer'
FILE_UPLOAD_CALLBACK = 'upload_response'

STEP_SUFFIXES = ("submission", "peer", "studentTraining", "self", "staff", "done")
19 changes: 0 additions & 19 deletions openassessment/xblock/ui_mixins/mfe/mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,28 +334,9 @@ def _assessment_submit_handler(self, data):
# Return assessment data for the frontend
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()

# Then, just return page data
serializer_context = {
"view": "assessment",
"step": "peer",
}
page_context = PageDataSerializer(self, context=serializer_context)
return page_context.data

@XBlock.json_handler
def assessment(self, data, suffix=""):
if suffix == handler_suffixes.ASSESSMENT_SUBMIT:
return self._assessment_submit_handler(data)
elif suffix == handler_suffixes.ASSESSMENT_GET_PEER:
return self._assessment_get_peer_handler()
else:
raise OraApiException(404, error_codes.UNKNOWN_SUFFIX)
10 changes: 1 addition & 9 deletions openassessment/xblock/ui_mixins/mfe/page_context_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,15 +343,7 @@ def get_response(self, instance):

# Peer
elif requested_step == "peer":

# If this is the step we're on (not continued grading), get a new submission to assess
if current_workflow_step == "peer":
response = instance.peer_assessment_data().get_peer_submission()

# We're revisiting the peer step, get me my active assessment, if I have one in progress...
# Otherwise, we're using a separate endpoint to request extra peer submissions to grade.
else:
response = instance.peer_assessment_data().get_active_assessment_submission()
response = instance.peer_assessment_data().get_peer_submission()

# Self / Done - Return your response to view / assess
elif requested_step in ("self", "done"):
Expand Down
3 changes: 3 additions & 0 deletions openassessment/xblock/ui_mixins/mfe/submission_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ class SubmissionSerializer(Serializer):
def get_uploadedFiles(self, response):
result = []
for index, uploaded_file in enumerate(response.get_file_uploads(generate_urls=True)):
# Don't serialize deleted / missing files
if uploaded_file.url is None:
continue
result.append(SubmissionFileSerializer(({'file': uploaded_file, 'file_index': index})).data)
return result

Expand Down
23 changes: 0 additions & 23 deletions openassessment/xblock/ui_mixins/mfe/test_mfe_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from contextlib import contextmanager
import json
from unittest.mock import Mock, PropertyMock, patch
from uuid import uuid4

import ddt
from django.contrib.auth import get_user_model
Expand Down Expand Up @@ -161,15 +160,6 @@ def request_assessment_submit(self, xblock, payload=None):
response_format='response'
)

def request_assessment_get_peer(self, xblock):
return super().request(
xblock,
'assessment',
'{}',
suffix=handler_suffixes.ASSESSMENT_GET_PEER,
response_format='response'
)


def assert_error_response(response, status_code, error_code, context=''):
assert response.status_code == status_code
Expand Down Expand Up @@ -1003,16 +993,3 @@ def test_training_assess_corrections(self, xblock):
resp = self.request_assessment_submit(xblock)

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


class AssessmentGetPeerTest(MFEHandlersTestBase):

@scenario("data/basic_scenario.xml")
def test_get_peer(self, xblock):
with patch.object(PeerAssessmentAPI, 'get_peer_submission') as mock_get_peer:
with patch('openassessment.xblock.ui_mixins.mfe.mixin.PageDataSerializer') as mock_serializer:
mock_serializer().data = str(uuid4())
resp = self.request_assessment_get_peer(xblock)
assert resp.status_code == 200
assert resp.json == mock_serializer().data
mock_get_peer.assert_called()
46 changes: 11 additions & 35 deletions openassessment/xblock/ui_mixins/mfe/test_page_context_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,19 +181,6 @@ def test_staff_response(self, xblock):
expected_response = {}
self.assertDictEqual(expected_response, response_data)

@scenario("data/staff_grade_scenario.xml", user_id="Alan")
def test_waiting_response(self, xblock):
# Given I'm on the staff step
self.create_test_submission(xblock)

# When I load my response
self.context = {"requested_step": "peer", "current_workflow_step": "waiting"}
response_data = PageDataSerializer(xblock, context=self.context).data["response"]

# Then I get an empty object
expected_response = {}
self.assertDictEqual(expected_response, response_data)

@scenario("data/self_assessment_scenario.xml", user_id="Alan")
def test_done_response(self, xblock):
# Given I'm on the done step
Expand All @@ -212,32 +199,21 @@ def test_done_response(self, xblock):
}
self.assertDictEqual(expected_response, response_data)

@scenario("data/grade_scenario_peer_only.xml", user_id="Bernard")
def test_jump_to_peer_response__no_active_assessment(self, xblock):
student_item = xblock.get_student_item_dict()

# Given responses available for peer grading
other_student_item = deepcopy(student_item)
other_student_item["student_id"] = "Joan"
other_text_responses = ["Answer 1", "Answer 2"]
self.create_test_submission(
xblock,
student_item=other_student_item,
submission_text=other_text_responses,
)

# ... and I have completed the peer step of an ORA
self.create_submission_and_assessments(xblock, self.SUBMISSION, self.PEERS, PEER_ASSESSMENTS, None)
@scenario("data/grade_scenario_peer_only.xml", user_id="Alan")
def test_jump_to_peer_not_available(self, xblock):
# Given I'm past the peer step
self.create_test_submission(xblock)

# When I try to jump back to that step
self.context = {"requested_step": "peer", "current_workflow_step": "done"}
response_data = PageDataSerializer(xblock, context=self.context).data
# When I ask for a peer response, but there are none available
self.context = {"requested_step": "peer", "current_workflow_step": "waiting"}
response_data = PageDataSerializer(xblock, context=self.context).data["response"]

# I receive an empty response because I have not yet requested a submission to assess
self.assertDictEqual({}, response_data["response"])
# Then I get an empty object
expected_response = {}
self.assertDictEqual(expected_response, response_data)

@scenario("data/grade_scenario_peer_only.xml", user_id="Bernard")
def test_jump_to_peer_response__active_assessment(self, xblock):
def test_jump_to_peer_available(self, xblock):
student_item = xblock.get_student_item_dict()

# Given responses available for peer grading
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.10",
"version": "6.0.11",
"repository": "https://github.com/openedx/edx-ora2.git",
"dependencies": {
"@edx/frontend-build": "8.0.6",
Expand Down

0 comments on commit 4012e00

Please sign in to comment.