From 647636a96de90b3096ead66e76c1fb1de4be4cfd Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Wed, 16 Jan 2019 15:53:54 -0500 Subject: [PATCH] Expose error state to proctoring rest API implementers JIRA:EDUCATOR-3887 --- docs/backends.rst | 2 +- edx_proctoring/__init__.py | 2 +- edx_proctoring/api.py | 13 ++++++--- edx_proctoring/backends/backend.py | 8 ++++++ edx_proctoring/backends/mock.py | 8 ++++++ edx_proctoring/backends/null.py | 8 ++++++ edx_proctoring/backends/rest.py | 12 +++++++++ edx_proctoring/backends/software_secure.py | 8 ++++++ edx_proctoring/backends/tests/test_backend.py | 24 +++++++++++++++++ edx_proctoring/backends/tests/test_rest.py | 27 +++++++++---------- .../backends/tests/test_software_secure.py | 9 +++++++ package.json | 2 +- 12 files changed, 101 insertions(+), 22 deletions(-) diff --git a/docs/backends.rst b/docs/backends.rst index d893d199833..70d2f69d0b0 100644 --- a/docs/backends.rst +++ b/docs/backends.rst @@ -125,7 +125,7 @@ The PS system should respond with an object containing at least the following fi "status": "submitted", } -Open edX will issue a ``PATCH`` request with a ``started`` status when the learner starts the proctored exam, and a ``submitted`` status when the learner finishes the exam. +Open edX will issue a ``PATCH`` request with a ``started`` status when the learner starts the proctored exam, and a ``submitted`` status when the learner finishes the exam. A status of ``error`` may be used in case of a technical error being associated with a learner's proctoring session. ``GET``: returns PS information about the attempt diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index baa7c623e39..f4a742ee4c9 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -5,6 +5,6 @@ from __future__ import absolute_import # Be sure to update the version number in edx_proctoring/package.json -__version__ = '1.5.5' +__version__ = '1.5.6' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 1029f2e9221..7254ca44a2a 100644 --- a/edx_proctoring/api.py +++ b/edx_proctoring/api.py @@ -1034,11 +1034,16 @@ def update_attempt_status(exam_id, user_id, to_status, if backend: # only proctored exams have a backend # timed exams have no backend + backend_method = None if to_status == ProctoredExamStudentAttemptStatus.started: - backend.start_exam_attempt(exam['external_id'], attempt['external_id']) - if to_status == ProctoredExamStudentAttemptStatus.submitted: - backend.stop_exam_attempt(exam['external_id'], attempt['external_id']) - # we user the 'status' field as the name of the event 'verb' + backend_method = backend.start_exam_attempt + elif to_status == ProctoredExamStudentAttemptStatus.submitted: + backend_method = backend.stop_exam_attempt + elif to_status == ProctoredExamStudentAttemptStatus.error: + backend_method = backend.mark_erroneous_exam_attempt + if backend_method: + backend_method(exam['external_id'], attempt['external_id']) + # we use the 'status' field as the name of the event 'verb' emit_event(exam, attempt['status'], attempt=attempt) return attempt['id'] diff --git a/edx_proctoring/backends/backend.py b/edx_proctoring/backends/backend.py index dc30ab3a51f..ee31662eda7 100644 --- a/edx_proctoring/backends/backend.py +++ b/edx_proctoring/backends/backend.py @@ -42,6 +42,14 @@ def stop_exam_attempt(self, exam, attempt): """ raise NotImplementedError() + @abc.abstractmethod + def mark_erroneous_exam_attempt(self, exam, attempt): + """ + Method that is responsible for communicating with the backend provider + to establish a new proctored exam + """ + raise NotImplementedError() + @abc.abstractmethod def get_software_download_url(self): """ diff --git a/edx_proctoring/backends/mock.py b/edx_proctoring/backends/mock.py index d0ccd5d3051..1cb87875b5c 100644 --- a/edx_proctoring/backends/mock.py +++ b/edx_proctoring/backends/mock.py @@ -66,6 +66,14 @@ def stop_exam_attempt(self, exam, attempt): """ return None + def mark_erroneous_exam_attempt(self, exam, attempt): + """ + Method that would be responsible for communicating with the + backend provider to mark a proctored session as having + encountered a technical error + """ + return None + def get_software_download_url(self): """ Returns diff --git a/edx_proctoring/backends/null.py b/edx_proctoring/backends/null.py index b20c0c2213f..03f313f6748 100644 --- a/edx_proctoring/backends/null.py +++ b/edx_proctoring/backends/null.py @@ -33,6 +33,14 @@ def stop_exam_attempt(self, exam, attempt): """ return None + def mark_erroneous_exam_attempt(self, exam, attempt): + """ + Method that would be responsible for communicating with the + backend provider to mark a proctored session as having + encountered a technical error + """ + return None + def get_software_download_url(self): """ Returns the URL that the user needs to go to in order to download diff --git a/edx_proctoring/backends/rest.py b/edx_proctoring/backends/rest.py index 8328cae7055..c898fb49e80 100644 --- a/edx_proctoring/backends/rest.py +++ b/edx_proctoring/backends/rest.py @@ -196,6 +196,18 @@ def stop_exam_attempt(self, exam, attempt): method='PATCH') return response.get('status') + def mark_erroneous_exam_attempt(self, exam, attempt): + """ + Method that is responsible for communicating with the backend provider + to mark an unfinished exam to be in error + """ + response = self._make_attempt_request( + exam, + attempt, + status=ProctoredExamStudentAttemptStatus.error, + method='PATCH') + return response.get('status') + def on_review_callback(self, attempt, payload): """ Called when the reviewing 3rd party service posts back the results diff --git a/edx_proctoring/backends/software_secure.py b/edx_proctoring/backends/software_secure.py index 90b841e1126..fc74dd38466 100644 --- a/edx_proctoring/backends/software_secure.py +++ b/edx_proctoring/backends/software_secure.py @@ -113,6 +113,14 @@ def stop_exam_attempt(self, exam, attempt): """ return None + def mark_erroneous_exam_attempt(self, exam, attempt): + """ + Method that would be responsible for communicating with the + backend provider to mark a proctored session as having + encountered a technical error + """ + return None + def get_software_download_url(self): """ Returns the URL that the user needs to go to in order to download diff --git a/edx_proctoring/backends/tests/test_backend.py b/edx_proctoring/backends/tests/test_backend.py index 69a629b328d..985ca849ebc 100644 --- a/edx_proctoring/backends/tests/test_backend.py +++ b/edx_proctoring/backends/tests/test_backend.py @@ -45,6 +45,14 @@ def stop_exam_attempt(self, exam, attempt): """ return None + def mark_erroneous_exam_attempt(self, exam, attempt): + """ + Method that would be responsible for communicating with the + backend provider to mark a proctored session as having + encountered a technical error + """ + return None + def get_software_download_url(self): """ Returns the URL that the user needs to go to in order to download @@ -111,6 +119,17 @@ def stop_exam_attempt(self, exam, attempt): attempt ) + def mark_erroneous_exam_attempt(self, exam, attempt): + """ + Method that would be responsible for communicating with the + backend provider to mark a proctored session as having + encountered a technical error + """ + return super(PassthroughBackendProvider, self).mark_erroneous_exam_attempt( + exam, + attempt + ) + def get_software_download_url(self): """ Returns the URL that the user needs to go to in order to download @@ -155,6 +174,9 @@ def test_raises_exception(self): with self.assertRaises(NotImplementedError): provider.on_review_callback(None, None) + with self.assertRaises(NotImplementedError): + provider.mark_erroneous_exam_attempt(None, None) + with self.assertRaises(NotImplementedError): provider.on_exam_saved(None) @@ -170,6 +192,7 @@ def test_null_provider(self): self.assertIsNone(provider.register_exam_attempt(None, None)) self.assertIsNone(provider.start_exam_attempt(None, None)) self.assertIsNone(provider.stop_exam_attempt(None, None)) + self.assertIsNone(provider.mark_erroneous_exam_attempt(None, None)) self.assertIsNone(provider.get_software_download_url()) self.assertIsNone(provider.on_review_callback(None, None)) self.assertIsNone(provider.on_exam_saved(None)) @@ -196,6 +219,7 @@ def test_mock_provider(self): ) self.assertIsNone(provider.start_exam_attempt(None, None)) self.assertIsNone(provider.stop_exam_attempt(None, None)) + self.assertIsNone(provider.mark_erroneous_exam_attempt(None, None)) self.assertIsNone(provider.on_review_callback(None, None)) self.assertIsNone(provider.on_exam_saved(None)) diff --git a/edx_proctoring/backends/tests/test_rest.py b/edx_proctoring/backends/tests/test_rest.py index e2efecb3f85..c48782c117f 100644 --- a/edx_proctoring/backends/tests/test_rest.py +++ b/edx_proctoring/backends/tests/test_rest.py @@ -3,6 +3,7 @@ """ import json +import ddt import jwt import responses @@ -16,6 +17,7 @@ from edx_proctoring.exceptions import BackendProviderCannotRegisterAttempt +@ddt.ddt class RESTBackendTests(TestCase): """ Tests for the REST backend @@ -197,27 +199,22 @@ def test_register_exam_attempt_failure(self): with self.assertRaises(BackendProviderCannotRegisterAttempt): self.provider.register_exam_attempt(self.backend_exam, context) + @ddt.data( + ['start_exam_attempt', 'start'], + ['stop_exam_attempt', 'stop'], + ['mark_erroneous_exam_attempt', 'error'], + ) + @ddt.unpack @responses.activate - def test_start_exam_attempt(self): + def test_update_exam_attempt_status(self, provider_method_name, corresponding_status): attempt_id = 2 responses.add( responses.PATCH, url=self.provider.exam_attempt_url.format(exam_id=self.backend_exam['external_id'], attempt_id=attempt_id), - json={'id': 2, 'status': 'start'} + json={'id': 2, 'status': corresponding_status} ) - status = self.provider.start_exam_attempt(self.backend_exam['external_id'], attempt_id) - self.assertEqual(status, 'start') - - @responses.activate - def test_stop_exam_attempt(self): - attempt_id = 2 - responses.add( - responses.PATCH, - url=self.provider.exam_attempt_url.format(exam_id=self.backend_exam['external_id'], attempt_id=attempt_id), - json={'id': 2, 'status': 'stop'} - ) - status = self.provider.stop_exam_attempt(self.backend_exam['external_id'], attempt_id) - self.assertEqual(status, 'stop') + status = getattr(self.provider, provider_method_name)(self.backend_exam['external_id'], attempt_id) + self.assertEqual(status, corresponding_status) def test_on_review_callback(self): """ diff --git a/edx_proctoring/backends/tests/test_software_secure.py b/edx_proctoring/backends/tests/test_software_secure.py index 11df5832915..46c94a19f0a 100644 --- a/edx_proctoring/backends/tests/test_software_secure.py +++ b/edx_proctoring/backends/tests/test_software_secure.py @@ -542,6 +542,15 @@ def test_stop_proctored_exam(self): provider = get_backend_provider() self.assertIsNone(provider.stop_exam_attempt(None, None)) + def test_mark_erroneous_proctored_exam(self): + """ + Test that SoftwareSecure's implementation returns None, because there is no + work that needs to happen right now + """ + + provider = get_backend_provider() + self.assertIsNone(provider.mark_erroneous_exam_attempt(None, None)) + def test_split_fullname(self): """ Make sure we are splitting up full names correctly diff --git a/package.json b/package.json index 4f2006a49b1..4d3aed57632 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "@edx/edx-proctoring", "//": "Be sure to update the version number in edx_proctoring/__init__.py", "//": "Note that the version format is slightly different than that of the Python version when using prereleases.", - "version": "1.5.5", + "version": "1.5.6", "main": "edx_proctoring/static/index.js", "repository": { "type": "git",