From 645a0e333ca685dca13afcde25e0d78c265f5925 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Thu, 14 Feb 2019 12:15:28 -0500 Subject: [PATCH] Remove exam attempt on backend when it's deleted in the LMS --- docs/backends.rst | 8 ++++++++ edx_proctoring/backends/backend.py | 7 +++++++ edx_proctoring/backends/mock.py | 3 +++ edx_proctoring/backends/null.py | 3 +++ edx_proctoring/backends/rest.py | 10 ++++++++++ edx_proctoring/backends/software_secure.py | 3 +++ edx_proctoring/backends/tests/test_backend.py | 14 ++++++++++++++ edx_proctoring/backends/tests/test_rest.py | 11 +++++++++++ edx_proctoring/signals.py | 12 ++++++++++++ edx_proctoring/tests/test_api.py | 7 +++++++ 10 files changed, 78 insertions(+) diff --git a/docs/backends.rst b/docs/backends.rst index ddf6c7731c4..77936b6ca49 100644 --- a/docs/backends.rst +++ b/docs/backends.rst @@ -148,6 +148,14 @@ For convenience, the PS should return the exam instructions and the software dow "download_url": "http://my-proctoring.com/download" } +``DELETE``: removes attempt on PS server + +When an attempt is deleted on the Open edX server, it will make a ``DELETE`` request on the PS server. On success, return:: + + { + "status": "deleted" + } + User management endpoint ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/edx_proctoring/backends/backend.py b/edx_proctoring/backends/backend.py index ae26e958603..0b8228ab0f1 100644 --- a/edx_proctoring/backends/backend.py +++ b/edx_proctoring/backends/backend.py @@ -54,6 +54,13 @@ def mark_erroneous_exam_attempt(self, exam, attempt): """ raise NotImplementedError() + @abc.abstractmethod + def remove_exam_attempt(self, exam, attempt): + """ + Method that removes the exam attempt from the backend's system + """ + 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 1cb87875b5c..07a2d9d5217 100644 --- a/edx_proctoring/backends/mock.py +++ b/edx_proctoring/backends/mock.py @@ -74,6 +74,9 @@ def mark_erroneous_exam_attempt(self, exam, attempt): """ return None + def remove_exam_attempt(self, exam, attempt): + return True + def get_software_download_url(self): """ Returns diff --git a/edx_proctoring/backends/null.py b/edx_proctoring/backends/null.py index 03f313f6748..fdd60c6a67e 100644 --- a/edx_proctoring/backends/null.py +++ b/edx_proctoring/backends/null.py @@ -41,6 +41,9 @@ def mark_erroneous_exam_attempt(self, exam, attempt): """ return None + def remove_exam_attempt(self, exam, attempt): + return True + 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 edf50576ad7..e7f75964ecf 100644 --- a/edx_proctoring/backends/rest.py +++ b/edx_proctoring/backends/rest.py @@ -216,6 +216,16 @@ def stop_exam_attempt(self, exam, attempt): method='PATCH') return response.get('status') + def remove_exam_attempt(self, exam, attempt): + """ + Removes the exam attempt on the backend provider's server + """ + response = self._make_attempt_request( + exam, + attempt, + method='DELETE') + return response.get('status', None) == 'deleted' + def mark_erroneous_exam_attempt(self, exam, attempt): """ Method that is responsible for communicating with the backend provider diff --git a/edx_proctoring/backends/software_secure.py b/edx_proctoring/backends/software_secure.py index fc74dd38466..ab8b978c726 100644 --- a/edx_proctoring/backends/software_secure.py +++ b/edx_proctoring/backends/software_secure.py @@ -121,6 +121,9 @@ def mark_erroneous_exam_attempt(self, exam, attempt): """ return None + def remove_exam_attempt(self, exam, attempt): + 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 0bc8ad4d554..abd68aa6290 100644 --- a/edx_proctoring/backends/tests/test_backend.py +++ b/edx_proctoring/backends/tests/test_backend.py @@ -29,6 +29,7 @@ class TestBackendProvider(ProctoringBackendProvider): last_retire_user = None attempt_error = None + last_attempt_remove = None def register_exam_attempt(self, exam, context): """ @@ -60,6 +61,10 @@ def mark_erroneous_exam_attempt(self, exam, attempt): """ return None + def remove_exam_attempt(self, exam, attempt): + self.last_attempt_remove = (exam, attempt) + return True + def get_software_download_url(self): """ Returns the URL that the user needs to go to in order to download @@ -144,6 +149,12 @@ def mark_erroneous_exam_attempt(self, exam, attempt): attempt ) + def remove_exam_attempt(self, exam, attempt): + return super(PassthroughBackendProvider, self).remove_exam_attempt( + exam, + attempt + ) + def get_software_download_url(self): """ Returns the URL that the user needs to go to in order to download @@ -191,6 +202,9 @@ def test_raises_exception(self): with self.assertRaises(NotImplementedError): provider.mark_erroneous_exam_attempt(None, None) + with self.assertRaises(NotImplementedError): + provider.remove_exam_attempt(None, None) + with self.assertRaises(NotImplementedError): provider.on_exam_saved(None) diff --git a/edx_proctoring/backends/tests/test_rest.py b/edx_proctoring/backends/tests/test_rest.py index 9d9bafbfe84..2a660b7df0e 100644 --- a/edx_proctoring/backends/tests/test_rest.py +++ b/edx_proctoring/backends/tests/test_rest.py @@ -242,6 +242,17 @@ def test_update_exam_attempt_status(self, provider_method_name, corresponding_st status = getattr(self.provider, provider_method_name)(self.backend_exam['external_id'], attempt_id) self.assertEqual(status, corresponding_status) + @responses.activate + def test_remove_attempt(self): + attempt_id = 2 + responses.add( + responses.DELETE, + url=self.provider.exam_attempt_url.format(exam_id=self.backend_exam['external_id'], attempt_id=attempt_id), + json={'status': "deleted"} + ) + status = self.provider.remove_exam_attempt(self.backend_exam['external_id'], attempt_id) + self.assertTrue(status) + def test_on_review_callback(self): """ on_review_callback should just return the payload diff --git a/edx_proctoring/signals.py b/edx_proctoring/signals.py index ebe9701d25a..422abcb914b 100644 --- a/edx_proctoring/signals.py +++ b/edx_proctoring/signals.py @@ -1,4 +1,6 @@ "edx-proctoring signals" +import logging + from django.db.models.signals import pre_save, post_save, pre_delete from django.dispatch import receiver @@ -9,6 +11,8 @@ from edx_proctoring.utils import emit_event, locate_attempt_by_attempt_code from edx_proctoring.backends import get_backend_provider +log = logging.getLogger(__name__) + @receiver(pre_save, sender=models.ProctoredExam) def check_for_category_switch(sender, instance, **kwargs): # pylint: disable=unused-argument @@ -106,6 +110,14 @@ def on_attempt_changed(sender, instance, signal, **kwargs): # pylint: disable=u return else: return + else: + # remove the attempt on the backend + # timed exams have no backend + backend = get_backend_provider(name=instance.proctored_exam.backend) + if backend: + result = backend.remove_exam_attempt(instance.proctored_exam.external_id, instance.external_id) + if not result: + log.error('Failed to remove attempt %d from %s', instance.id, backend.verbose_name) models.archive_model(models.ProctoredExamStudentAttemptHistory, instance, id='attempt_id') diff --git a/edx_proctoring/tests/test_api.py b/edx_proctoring/tests/test_api.py index 4b741eed8a7..a98dd03822f 100644 --- a/edx_proctoring/tests/test_api.py +++ b/edx_proctoring/tests/test_api.py @@ -650,7 +650,14 @@ def test_remove_exam_attempt(self): proctored_exam_student_attempt = self._create_unstarted_exam_attempt() remove_exam_attempt(proctored_exam_student_attempt.id, requesting_user=self.user) + test_backend = get_backend_provider(name='test') + self.assertEqual( + test_backend.last_attempt_remove, ( + proctored_exam_student_attempt.proctored_exam.external_id, + proctored_exam_student_attempt.external_id + ) + ) with self.assertRaises(StudentExamAttemptDoesNotExistsException): remove_exam_attempt(proctored_exam_student_attempt.id, requesting_user=self.user)