From a45457b02265b039540b18f5b7fdef1dc384c303 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Fri, 30 Nov 2018 09:40:22 -0500 Subject: [PATCH 1/2] When there is no instructor dashboard for the backend, return a 404 instead of a 500 --- edx_proctoring/__init__.py | 2 +- edx_proctoring/apps.py | 7 +++++-- edx_proctoring/backends/backend.py | 7 +++++++ edx_proctoring/backends/tests/test_rest.py | 6 +++--- edx_proctoring/tests/test_views.py | 20 ++++++++++++++++++++ edx_proctoring/views.py | 6 +++++- 6 files changed, 41 insertions(+), 7 deletions(-) diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 4447fb67292..06a6c5fedb6 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -4,6 +4,6 @@ from __future__ import absolute_import -__version__ = '1.5.0b2' +__version__ = '1.5.0b3' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/apps.py b/edx_proctoring/apps.py index 7174283e340..810087bd99d 100644 --- a/edx_proctoring/apps.py +++ b/edx_proctoring/apps.py @@ -53,11 +53,14 @@ def ready(self): config = settings.PROCTORING_BACKENDS self.backends = {} # pylint: disable=W0201 + not_found = [] for extension in ExtensionManager(namespace='openedx.proctoring'): name = extension.name try: options = config[name] self.backends[name] = extension.plugin(**options) except KeyError: - warnings.warn("No proctoring backend configured for '{}'. " - "Available: {}".format(name, list(self.backends))) + not_found.append(name) + if not_found: # pragma: no branch + warnings.warn("No proctoring backend configured for '{}'. " + "Available: {}".format(not_found, list(self.backends))) diff --git a/edx_proctoring/backends/backend.py b/edx_proctoring/backends/backend.py index 6bdc792bfc7..d65ce1d3a2e 100644 --- a/edx_proctoring/backends/backend.py +++ b/edx_proctoring/backends/backend.py @@ -82,3 +82,10 @@ def get_attempt(self, attempt): dict: backend exam attempt object """ return attempt + + # pylint: disable=unused-argument + def get_instructor_url(self, course_id, user, exam_id=None, attempt_id=None): + """ + Returns the instructor dashboard url for reviews + """ + return None diff --git a/edx_proctoring/backends/tests/test_rest.py b/edx_proctoring/backends/tests/test_rest.py index 149bf96004d..f456eb1e9a9 100644 --- a/edx_proctoring/backends/tests/test_rest.py +++ b/edx_proctoring/backends/tests/test_rest.py @@ -8,7 +8,7 @@ import responses from django.test import TestCase -from django.utils.translation import activate +from django.utils import translation from edx_proctoring.backends.rest import BaseRestProctoringProvider @@ -93,7 +93,6 @@ def test_get_attempt(self): @responses.activate def test_get_attempt_i18n(self): - activate('es') attempt = { 'id': 1, 'external_id': 'abcd', @@ -107,7 +106,8 @@ def test_get_attempt_i18n(self): exam_id=self.backend_exam['external_id'], attempt_id=attempt['external_id']), json=attempt ) - external_attempt = self.provider.get_attempt(attempt) + with translation.override('es'): + external_attempt = self.provider.get_attempt(attempt) self.assertEqual(external_attempt, attempt) self.assertEqual(responses.calls[1].request.headers['Accept-Language'], 'es;en-us') diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index 7f4f8e1f145..ae2d237eb30 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -2569,3 +2569,23 @@ def test_error_with_no_exams(self): kwargs={'course_id': course_id}) ) self.assertEqual(response.status_code, 404) + + def test_error_with_no_dashboard(self): + course_id = 'a/b/d' + + ProctoredExam.objects.create( + course_id=course_id, + content_id='test_content', + exam_name='Test Exam', + external_id='123aXqe3', + time_limit_mins=90, + is_active=True, + is_proctored=True, + backend='software_secure', + ) + response = self.client.get( + reverse('edx_proctoring.instructor_dashboard_course', + kwargs={'course_id': course_id}) + ) + self.assertEqual(response.status_code, 404) + self.assertEqual('No instructor dashboard for RPNow', response.data) diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 3822094ade5..e07b189d3ae 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -986,4 +986,8 @@ def get(self, request, course_id, exam_id=None): 'email': request.user.email } url = backend.get_instructor_url(exam['course_id'], user, exam_id=exam_id) - return redirect(url) + if url: + resp = redirect(url) + else: + resp = Response(data='No instructor dashboard for %s' % backend.verbose_name, status=404) + return resp From 524d0ad4fa9ffc4fb73943d9eb4dbac6ff1c4507 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Fri, 30 Nov 2018 15:16:35 -0500 Subject: [PATCH 2/2] Handle another test case --- edx_proctoring/tests/test_views.py | 17 +++++++++++++++++ edx_proctoring/views.py | 29 +++++++++++++++++------------ 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index ae2d237eb30..3f09fdaebca 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -2570,6 +2570,23 @@ def test_error_with_no_exams(self): ) self.assertEqual(response.status_code, 404) + # test the case of no PROCTORED exams + ProctoredExam.objects.create( + course_id=course_id, + content_id='test_content', + exam_name='Timed Exam', + external_id='123aXqe3', + time_limit_mins=90, + is_active=True, + is_proctored=False, + backend='software_secure', + ) + response = self.client.get( + reverse('edx_proctoring.instructor_dashboard_course', + kwargs={'course_id': course_id}) + ) + self.assertEqual(response.status_code, 404) + def test_error_with_no_dashboard(self): course_id = 'a/b/d' diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index e07b189d3ae..ac3d3e09188 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -978,16 +978,21 @@ def get(self, request, course_id, exam_id=None): else: found_backend = exam_backend if exam is None: - return Response(data='No exam found for course %r.' % course_id, status=404) - backend = get_backend_provider(exam) - user = { - 'id': request.user.id, - 'full_name': request.user.get_full_name(), - 'email': request.user.email - } - url = backend.get_instructor_url(exam['course_id'], user, exam_id=exam_id) - if url: - resp = redirect(url) + error = _('No exams in course {course_id}.').format(course_id=course_id) else: - resp = Response(data='No instructor dashboard for %s' % backend.verbose_name, status=404) - return resp + backend = get_backend_provider(exam) + if backend: + user = { + 'id': request.user.id, + 'full_name': request.user.get_full_name(), + 'email': request.user.email + } + url = backend.get_instructor_url(exam['course_id'], user, exam_id=exam_id) + if url: + return redirect(url) + else: + error = _('No instructor dashboard for {proctor_service}').format( + proctor_service=backend.verbose_name) + else: + error = _('No proctored exams in course {course_id}').format(course_id=course_id) + return Response(data=error, status=404, headers={'X-Frame-Options': 'sameorigin'})