From d3ae384104a13afb7b5faf3551cede9537738008 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Wed, 28 Nov 2018 13:48:32 -0500 Subject: [PATCH 1/2] Ensure that timed exams do not attempt to update the backend --- edx_proctoring/api.py | 11 +++++++---- edx_proctoring/backends/__init__.py | 8 ++++++-- edx_proctoring/tests/test_api.py | 19 +++++++++++++++++++ edx_proctoring/tests/test_views.py | 4 ++++ 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 21d196faf40..c7675480f63 100644 --- a/edx_proctoring/api.py +++ b/edx_proctoring/api.py @@ -1069,10 +1069,13 @@ def update_attempt_status(exam_id, user_id, to_status, # call back to the backend to register the end of the exam, if necessary backend = get_backend_provider(exam) - 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']) + if backend: + # only proctored exams have a backend + # timed exams have no backend + 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' emit_event(exam, attempt['status'], attempt=attempt) diff --git a/edx_proctoring/backends/__init__.py b/edx_proctoring/backends/__init__.py index 82ba7552292..6d163583131 100644 --- a/edx_proctoring/backends/__init__.py +++ b/edx_proctoring/backends/__init__.py @@ -9,6 +9,10 @@ def get_backend_provider(exam=None): Returns an instance of the configured backend provider """ backend_name = None - if exam and exam['backend']: - backend_name = exam['backend'] + if exam: + if exam['backend']: + backend_name = exam['backend'] + elif 'is_proctored' in exam and not exam['is_proctored']: + # timed exams don't have a backend + return None return apps.get_app_config('edx_proctoring').get_backend(name=backend_name) diff --git a/edx_proctoring/tests/test_api.py b/edx_proctoring/tests/test_api.py index 525f78e345d..de3c0392cf3 100644 --- a/edx_proctoring/tests/test_api.py +++ b/edx_proctoring/tests/test_api.py @@ -1654,6 +1654,25 @@ def test_status_summary_bad(self): self.assertIsNone(summary) + @patch('edx_proctoring.backends.tests.test_backend.TestBackendProvider') + def test_update_exam_attempt_timed(self, mock_backend): + """ + Make sure that timed exams do not try to update status in the backend + """ + attempt = self._create_unstarted_exam_attempt(is_proctored=False) + update_attempt_status( + attempt.proctored_exam_id, + self.user.id, + ProctoredExamStudentAttemptStatus.started + ) + mock_backend.start_exam_attempt.assert_not_called() + update_attempt_status( + attempt.proctored_exam_id, + self.user.id, + ProctoredExamStudentAttemptStatus.submitted + ) + mock_backend.stop_exam_attempt.assert_not_called() + def test_update_exam_attempt(self): """ Make sure we restrict which fields we can update diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index 8b7adc687bb..7f4f8e1f145 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -2503,6 +2503,7 @@ def test_launch_for_course(self): external_id='123aXqe3', time_limit_mins=90, is_active=True, + is_proctored=True, ) expected_url = '/instructor/%s/' % course_id @@ -2521,6 +2522,7 @@ def test_launch_for_exam(self): external_id='123aXqe3', time_limit_mins=90, is_active=True, + is_proctored=True, ) exam_id = proctored_exam.id @@ -2540,6 +2542,7 @@ def test_error_with_multiple_backends(self): external_id='123aXqe3', time_limit_mins=90, is_active=True, + is_proctored=True, backend='test', ) ProctoredExam.objects.create( @@ -2549,6 +2552,7 @@ def test_error_with_multiple_backends(self): external_id='123aXqe4', time_limit_mins=90, is_active=True, + is_proctored=True, backend='null', ) response = self.client.get( From 19add1bf9e052c2f59052ea9e2967cc148d74b0a Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Thu, 29 Nov 2018 16:00:21 -0500 Subject: [PATCH 2/2] Actually test the right thing --- edx_proctoring/backends/__init__.py | 6 +++--- edx_proctoring/backends/tests/test_backend.py | 11 +++++++++++ edx_proctoring/tests/test_api.py | 19 ------------------- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/edx_proctoring/backends/__init__.py b/edx_proctoring/backends/__init__.py index 6d163583131..d37388bb3f7 100644 --- a/edx_proctoring/backends/__init__.py +++ b/edx_proctoring/backends/__init__.py @@ -10,9 +10,9 @@ def get_backend_provider(exam=None): """ backend_name = None if exam: - if exam['backend']: - backend_name = exam['backend'] - elif 'is_proctored' in exam and not exam['is_proctored']: + if 'is_proctored' in exam and not exam['is_proctored']: # timed exams don't have a backend return None + elif exam['backend']: + backend_name = exam['backend'] return apps.get_app_config('edx_proctoring').get_backend(name=backend_name) diff --git a/edx_proctoring/backends/tests/test_backend.py b/edx_proctoring/backends/tests/test_backend.py index f6d722dccee..0ce25dcb859 100644 --- a/edx_proctoring/backends/tests/test_backend.py +++ b/edx_proctoring/backends/tests/test_backend.py @@ -226,6 +226,17 @@ def test_backend_choices(self): ] self.assertEqual(choices, expected) + def test_no_backend_for_timed_exams(self): + """ + Timed exams should not return a backend, even if one has accidentally been set + """ + exam = { + 'is_proctored': False, + 'backend': 'test' + } + backend = get_backend_provider(exam) + self.assertIsNone(backend) + def test_invalid_configurations(self): """ Test that invalid backends throw the right exceptions diff --git a/edx_proctoring/tests/test_api.py b/edx_proctoring/tests/test_api.py index de3c0392cf3..525f78e345d 100644 --- a/edx_proctoring/tests/test_api.py +++ b/edx_proctoring/tests/test_api.py @@ -1654,25 +1654,6 @@ def test_status_summary_bad(self): self.assertIsNone(summary) - @patch('edx_proctoring.backends.tests.test_backend.TestBackendProvider') - def test_update_exam_attempt_timed(self, mock_backend): - """ - Make sure that timed exams do not try to update status in the backend - """ - attempt = self._create_unstarted_exam_attempt(is_proctored=False) - update_attempt_status( - attempt.proctored_exam_id, - self.user.id, - ProctoredExamStudentAttemptStatus.started - ) - mock_backend.start_exam_attempt.assert_not_called() - update_attempt_status( - attempt.proctored_exam_id, - self.user.id, - ProctoredExamStudentAttemptStatus.submitted - ) - mock_backend.stop_exam_attempt.assert_not_called() - def test_update_exam_attempt(self): """ Make sure we restrict which fields we can update