diff --git a/openassessment/__init__.py b/openassessment/__init__.py index 2d1e42453b..f4a42b0cee 100644 --- a/openassessment/__init__.py +++ b/openassessment/__init__.py @@ -2,4 +2,4 @@ Initialization Information for Open Assessment Module """ -__version__ = '6.0.24' +__version__ = '6.0.25' diff --git a/openassessment/xblock/apis/assessments/peer_assessment_api.py b/openassessment/xblock/apis/assessments/peer_assessment_api.py index 5a33e217ef..fcdb4eb812 100644 --- a/openassessment/xblock/apis/assessments/peer_assessment_api.py +++ b/openassessment/xblock/apis/assessments/peer_assessment_api.py @@ -156,10 +156,20 @@ def get_peer_submission(self): logger.exception(err) return peer_submission - def assert_assessed_submission_uuid_matches(self, uuid_client): - submission = self.get_peer_submission() or {} + def assert_assessing_valid_submission(self, uuid_client): + """ + Validate that the forntend and backend agree on which submission is being assessed + """ + # Get the current active submission without pulling a new one + submission = self.get_active_assessment_submission() + + # If there is no active submission (expired or never got one somehow), raise + if submission is None: + raise ServerClientUUIDMismatchException() + uuid_server = submission.get("uuid", None) + # If the server and client don't agree, raise if uuid_server != uuid_client: logger.warning( "Irrelevant assessment submission: expected '%s', got '%s'", @@ -207,8 +217,7 @@ def peer_assess( if not workflow_data.has_workflow: raise ReviewerMustHaveSubmittedException() - if assessed_submission_uuid: - peer_step_data.assert_assessed_submission_uuid_matches(assessed_submission_uuid) + peer_step_data.assert_assessing_valid_submission(assessed_submission_uuid) try: assessment = peer_api.create_assessment( diff --git a/openassessment/xblock/test/test_peer.py b/openassessment/xblock/test/test_peer.py index af49158590..8af38b1972 100644 --- a/openassessment/xblock/test/test_peer.py +++ b/openassessment/xblock/test/test_peer.py @@ -14,7 +14,10 @@ import pytz from openassessment.assessment.api import peer as peer_api +from openassessment.assessment.models.peer import PeerWorkflowItem from openassessment.workflow import api as workflow_api +from openassessment.xblock.apis.assessments.errors import ServerClientUUIDMismatchException +from openassessment.xblock.apis.assessments.peer_assessment_api import peer_assess from openassessment.xblock.utils.data_conversion import create_submission_dict from .base import SubmissionTestMixin, XBlockHandlerTestCase, scenario @@ -22,6 +25,7 @@ logger = logging.getLogger(__name__) +@ddt.ddt class TestPeerAssessment(XBlockHandlerTestCase, SubmissionTestMixin): """ Test integration of the OpenAssessment XBlock with the peer assessment API. @@ -308,6 +312,49 @@ def test_turbo_grading(self, xblock): self.assertNotIn(submission["answer"]["parts"][1]["text"], peer_response.body.decode('utf-8')) self.assertIn("You have successfully completed", peer_response.body.decode('utf-8')) + @scenario('data/peer_assessment_scenario.xml', user_id='Sally') + @ddt.data(None, "nonmatchinguuid") + def test_assess_expired_peer_item(self, xblock, client_uuid): + + # Create two submissions, one for sally and one for Hal + student_item = xblock.get_student_item_dict() + sally_student_item = copy.deepcopy(student_item) + sally_student_item['student_id'] = "Sally" + sally_submission = self.create_test_submission( + xblock, student_item=sally_student_item, submission_text=("Sally's answer 1", "Sally's answer 2") + ) + + hal_student_item = copy.deepcopy(student_item) + hal_student_item['student_id'] = "Hal" + hal_submission = self.create_test_submission( + xblock, student_item=hal_student_item, submission_text=("Hal's answer 1", "Hal's answer 2") + ) + + # Sally is given Hal to assess + hal_sub = peer_api.get_submission_to_assess(sally_submission['uuid'], 1) + assert hal_sub['uuid'] == hal_submission['uuid'] + assert peer_api.get_active_assessment_submission(sally_submission['uuid']) == hal_sub + + # Modify the peer workflow item to be too old to be "active" + pwi = PeerWorkflowItem.objects.last() + assert pwi.submission_uuid == hal_submission['uuid'] + pwi.started_at = pwi.started_at - dt.timedelta(days=1) + pwi.save() + assert peer_api.get_active_assessment_submission(sally_submission['uuid']) is None + + # An error should be raised when sally tried to submit an assessment + # because she doesn't have an active submission + with self.assertRaises(ServerClientUUIDMismatchException): + peer_assess( + self.ASSESSMENT['options_selected'], + self.ASSESSMENT['overall_feedback'], + self.ASSESSMENT['criterion_feedback'], + xblock.config_data, + xblock.workflow_data, + xblock.peer_assessment_data(), + assessed_submission_uuid=client_uuid + ) + @ddt.ddt class TestPeerAssessmentRender(XBlockHandlerTestCase, SubmissionTestMixin): diff --git a/package-lock.json b/package-lock.json index 4496bc78bf..ee5dff1efa 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "edx-ora2", - "version": "6.0.24", + "version": "6.0.25", "lockfileVersion": 3, "requires": true, "packages": { diff --git a/package.json b/package.json index edae2cb717..8892c37007 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "edx-ora2", - "version": "6.0.24", + "version": "6.0.25", "repository": "https://github.com/openedx/edx-ora2.git", "dependencies": { "@edx/frontend-build": "8.0.6",