diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 63e991f0bd..1a16eb7c82 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,13 +14,17 @@ Change Log Unreleased ~~~~~~~~~~ +[4.18.2] - 2024-10-03 +~~~~~~~~~~~~~~~~~~~~~ +* fix various bugs related to exams configured with removed proctoring backend + [4.17.0] ~~~~~~~~~~~~~~~~~~~~~ * Add support for Python 3.11 & 3.12 [4.16.1] - 2023-08-8 ~~~~~~~~~~~~~~~~~~~~~ -* Updated django-simple-history package to 3.3.0 +* Updated django-simple-history package to 3.3.0 * Created no-op migrations needed for new django-simple-history package version [4.16.0] - 2023-06-22 diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index f34458f811..f5cb65ad4b 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -3,4 +3,4 @@ """ # Be sure to update the version number in edx_proctoring/package.json -__version__ = '4.18.1' +__version__ = '4.18.2' diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 135e0874e0..117b725576 100644 --- a/edx_proctoring/api.py +++ b/edx_proctoring/api.py @@ -1669,7 +1669,7 @@ def update_attempt_status(attempt_id, to_status, if email: try: email.send() - except Exception as err: # pylint: disable=broad-except + except Exception as err: log.exception( ('Exception occurred while trying to send proctoring attempt ' 'status email for user_id=%(user_id)s in course_id=%(course_id)s -- %(err)s'), @@ -2943,7 +2943,7 @@ def get_student_view(user_id, course_id, content_id, is_proctored_exam = exam['is_proctored'] and not exam['is_practice_exam'] is_timed_exam = not exam['is_proctored'] and not exam['is_practice_exam'] - exam_backend = get_backend_provider(name=exam['backend']) + exam_backend = get_backend_provider(exam=exam) sub_view_func = None if is_timed_exam: @@ -3019,8 +3019,17 @@ def is_backend_dashboard_available(course_id): active_only=True ) for exam in exams: - if get_backend_provider(name=exam.backend).has_dashboard: - return True + try: + if get_backend_provider(name=exam.backend).has_dashboard: + return True + except NotImplementedError: + log.exception( + 'No proctoring backend configured for backend=%(backend)s', + { + 'backend': exam.backend, + } + ) + return False diff --git a/edx_proctoring/backends/rest.py b/edx_proctoring/backends/rest.py index ae6ceae7bf..963fc1d46a 100644 --- a/edx_proctoring/backends/rest.py +++ b/edx_proctoring/backends/rest.py @@ -274,7 +274,7 @@ def on_exam_saved(self, exam): try: response = self.session.post(url, json=exam) data = response.json() - except Exception as exc: # pylint: disable=broad-except + except Exception as exc: if response: if hasattr(exc, 'response') and exc.response is not None: content = exc.response.content diff --git a/edx_proctoring/backends/software_secure.py b/edx_proctoring/backends/software_secure.py index 68b0f16897..3c1f2b81a8 100644 --- a/edx_proctoring/backends/software_secure.py +++ b/edx_proctoring/backends/software_secure.py @@ -409,7 +409,7 @@ def get_instructor_url( # reformat video url as per MST-871 findings reformatted_url = decrypted_video_url.replace('DirectLink-Generic', 'DirectLink-HTML5') return reformatted_url - except Exception as err: # pylint: disable=broad-except + except Exception as err: log.exception( 'Could not decrypt video url for attempt_id=%(attempt_id)s ' 'due to the following error: %(error_string)s', diff --git a/edx_proctoring/handlers.py b/edx_proctoring/handlers.py index a9dbdc7fc3..44fecc7c9a 100644 --- a/edx_proctoring/handlers.py +++ b/edx_proctoring/handlers.py @@ -27,8 +27,16 @@ def check_for_category_switch(sender, instance, **kwargs): exam = ProctoredExamJSONSafeSerializer(instance).data # from the perspective of the backend, the exam is now inactive. exam['is_active'] = False - backend = get_backend_provider(name=exam['backend']) - backend.on_exam_saved(exam) + try: + backend = get_backend_provider(name=exam['backend']) + backend.on_exam_saved(exam) + except NotImplementedError: + log.exception( + 'No proctoring backend configured for backend=%(backend)s', + { + 'backend': exam['backend'], + } + ) @receiver(post_save, sender=models.ProctoredExamReviewPolicy) @@ -127,15 +135,16 @@ def on_attempt_changed(sender, instance, signal, **kwargs): 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_id=%s from backend=%s', - instance.id, - instance.proctored_exam.backend, - ) + if instance.proctored_exam.is_proctored: + 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_id=%s from backend=%s', + instance.id, + instance.proctored_exam.backend, + ) @receiver(post_delete, sender=models.ProctoredExamStudentAttempt) diff --git a/edx_proctoring/rules.py b/edx_proctoring/rules.py index 0ebde0d0e7..81a084fe57 100644 --- a/edx_proctoring/rules.py +++ b/edx_proctoring/rules.py @@ -12,5 +12,5 @@ def is_in_reviewer_group(user, attempt): return user.groups.filter(name=backend_group).exists() -# pylint: disable=unsupported-binary-operation +# pylint: disable-next=unsupported-binary-operation rules.add_perm('edx_proctoring.can_review_attempt', is_in_reviewer_group | rules.is_staff) diff --git a/edx_proctoring/tests/test_api.py b/edx_proctoring/tests/test_api.py index d2f74889ab..f6d20a863e 100644 --- a/edx_proctoring/tests/test_api.py +++ b/edx_proctoring/tests/test_api.py @@ -2896,6 +2896,11 @@ def test_dashboard_availability(self): # backend with a dashboard self.assertTrue(is_backend_dashboard_available(self.course_id)) + @patch('edx_proctoring.api.get_backend_provider') + def test_dashboard_availability_no_provider(self, mock_get_backend): + mock_get_backend.side_effect = NotImplementedError() + self.assertFalse(is_backend_dashboard_available(self.course_id)) + def test_does_provider_support_onboarding(self): self.assertTrue(does_backend_support_onboarding('test')) self.assertFalse(does_backend_support_onboarding('mock')) diff --git a/edx_proctoring/tests/test_handlers.py b/edx_proctoring/tests/test_handlers.py index 70d91458b3..62cf04dbc2 100644 --- a/edx_proctoring/tests/test_handlers.py +++ b/edx_proctoring/tests/test_handlers.py @@ -27,7 +27,7 @@ def setUp(self): self.proctored_exam = ProctoredExam.objects.create( course_id='x/y/z', content_id=self.content_id, exam_name=self.exam_name, time_limit_mins=self.default_time_limit, external_id=self.external_id, - backend=self.backend_name + backend=self.backend_name, is_proctored=True, ) self.attempt = ProctoredExamStudentAttempt.objects.create( proctored_exam=self.proctored_exam, user=self.user, attempt_code='12345', diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index c9cf38d99a..a6cb2728d5 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -792,6 +792,18 @@ def test_no_onboarding_exam(self): message = 'There is no onboarding exam related to this course id.' self.assertEqual(response_data['detail'], message) + @patch('edx_proctoring.views.get_backend_provider') + def test_invalid_backend(self, mock_get_backend): + mock_get_backend.side_effect = NotImplementedError() + response = self.client.get( + reverse('edx_proctoring:user_onboarding.status') + + f'?course_id={self.course_id}' + ) + self.assertEqual(response.status_code, 404) + response_data = json.loads(response.content.decode('utf-8')) + message = 'There is no onboarding exam related to this course id.' + self.assertEqual(response_data['detail'], message) + @override_settings(LEARNING_MICROFRONTEND_URL='https://learningmfe') def test_onboarding_mfe_link(self): """ @@ -1518,6 +1530,16 @@ def test_backend_does_not_support_onboarding(self): self.assertEqual(response.status_code, 404) test_backend.supports_onboarding = previous_value + @patch('edx_proctoring.views.get_backend_provider') + def test_invalid_backend(self, mock_get_backend): + mock_get_backend.side_effect = NotImplementedError() + response = self.client.get(reverse( + 'edx_proctoring:user_onboarding.status.course', + kwargs={'course_id': 'a/b/c'} + ) + ) + self.assertEqual(response.status_code, 404) + def test_multiple_onboarding_exams(self): onboarding_exam_2_id = create_exam( course_id=self.course_id, diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 3d8560eb4c..061bc4cdca 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -640,7 +640,7 @@ class StudentOnboardingStatusView(ProctoredAPIView): * 'onboarding_past_due': Whether the onboarding exam is past due. All onboarding exams in the course must be past due in order for onboarding_past_due to be true. """ - def get(self, request): + def get(self, request): # pylint: disable=too-many-statements """ HTTP GET handler. Returns the learner's onboarding status. """ @@ -677,7 +677,17 @@ def get(self, request): # If there are multiple onboarding exams, use the last created exam accessible to the user onboarding_exams = list(ProctoredExam.get_practice_proctored_exams_for_course(course_id).order_by('-created')) - if not onboarding_exams or not get_backend_provider(name=onboarding_exams[0].backend).supports_onboarding: + provider = None + try: + provider = get_backend_provider(name=onboarding_exams[0].backend) if onboarding_exams else None + except NotImplementedError: + logging.exception( + 'No proctoring backend configured for backend=%(backend)s', + { + 'backend': onboarding_exams[0].backend, + } + ) + if not onboarding_exams or not (provider and provider.supports_onboarding): return Response( status=404, data={'detail': _('There is no onboarding exam related to this course id.')} @@ -848,7 +858,17 @@ def get(self, request, course_id): # get last created onboarding exam if there are multiple onboarding_exam = (ProctoredExam.get_practice_proctored_exams_for_course(course_id) .order_by('-created').first()) - if not onboarding_exam or not get_backend_provider(name=onboarding_exam.backend).supports_onboarding: + provider = None + try: + provider = get_backend_provider(name=onboarding_exam.backend) if onboarding_exam else None + except NotImplementedError: + logging.exception( + 'No proctoring backend configured for backend=%(backend)s', + { + 'backend': onboarding_exam.backend, + } + ) + if not onboarding_exam or not (provider and provider.supports_onboarding): return Response( status=404, data={'detail': _('There is no onboarding exam related to this course id.')} diff --git a/package.json b/package.json index 255bd3e632..56b2bade77 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@edx/edx-proctoring", "//": "Note that the version format is slightly different than that of the Python version when using prereleases.", - "version": "4.18.1", + "version": "4.18.2", "main": "edx_proctoring/static/index.js", "scripts": { "test": "gulp test"