Skip to content

Commit

Permalink
fix: allow timed exams for providers that are no longer valid (#1237)
Browse files Browse the repository at this point in the history
* fix: handle case where backend provider no longer exists for timed exams

* fix: update other uses of get_backend_provider

* fix: update tests and lint

* docs: update changelog
  • Loading branch information
alangsto authored Oct 3, 2024
1 parent f4d81e5 commit 5384a26
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 25 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
"""

# Be sure to update the version number in edx_proctoring/package.json
__version__ = '4.18.1'
__version__ = '4.18.2'
17 changes: 13 additions & 4 deletions edx_proctoring/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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


Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/backends/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/backends/software_secure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
31 changes: 20 additions & 11 deletions edx_proctoring/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
5 changes: 5 additions & 0 deletions edx_proctoring/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
22 changes: 22 additions & 0 deletions edx_proctoring/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 23 additions & 3 deletions edx_proctoring/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down Expand Up @@ -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.')}
Expand Down Expand Up @@ -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.')}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down

0 comments on commit 5384a26

Please sign in to comment.