Skip to content

Commit

Permalink
Merge pull request #633 from edx/schen/PROD-1089
Browse files Browse the repository at this point in the history 
PROD-1089 Bug fix for Instructor dashboard redirect URL get API call
  • Loading branch information
@schenedx
schenedx authored Dec 16, 2019
2 parents e41a777 + 7d97e9b commit 6cebd32
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 67 deletions.
2 changes: 1 addition & 1 deletion edx_proctoring/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
from __future__ import absolute_import

# Be sure to update the version number in edx_proctoring/package.json
__version__ = '2.1.9'
__version__ = '2.2.0'

default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name
97 changes: 66 additions & 31 deletions edx_proctoring/tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2589,6 +2589,7 @@ def test_get_active_exams_for_user(self):
self.assertEqual(response.status_code, 200)


@ddt.ddt
class TestInstructorDashboard(LoggedInTestCase):
"""
Tests for launching the instructor dashboard
Expand All @@ -2604,14 +2605,13 @@ def setUp(self):
self.second_user = User(username='tester2', email='[email protected]')
self.second_user.save()
self.client.login_user(self.user)
self.course_id = 'a/b/c'

set_runtime_service('instructor', MockInstructorService(is_user_course_staff=True))

def test_launch_for_course(self):
course_id = 'a/b/c'

ProctoredExam.objects.create(
course_id=course_id,
course_id=self.course_id,
content_id='test_content',
exam_name='Test Exam',
external_id='123aXqe3',
Expand All @@ -2620,17 +2620,16 @@ def test_launch_for_course(self):
is_proctored=True,
)

expected_url = '/instructor/%s/' % course_id
expected_url = '/instructor/%s/' % self.course_id
response = self.client.get(
reverse('edx_proctoring:instructor_dashboard_course', args=[course_id])
reverse('edx_proctoring:instructor_dashboard_course',
kwargs={'course_id': self.course_id})
)
self.assertRedirects(response, expected_url, fetch_redirect_response=False)

def test_launch_for_exam(self):
course_id = 'a/b/c'

proctored_exam = ProctoredExam.objects.create(
course_id=course_id,
course_id=self.course_id,
content_id='test_content',
exam_name='Test Exam',
external_id='123aXqe3',
Expand All @@ -2640,9 +2639,9 @@ def test_launch_for_exam(self):
)
exam_id = proctored_exam.id

expected_url = '/instructor/%s/?exam=%s' % (course_id, proctored_exam.external_id)
expected_url = '/instructor/%s/?exam=%s' % (self.course_id, proctored_exam.external_id)
dashboard_url = reverse('edx_proctoring:instructor_dashboard_exam',
kwargs={'course_id': course_id, 'exam_id': exam_id})
kwargs={'course_id': self.course_id, 'exam_id': exam_id})
response = self.client.get(dashboard_url)
self.assertRedirects(response, expected_url, fetch_redirect_response=False)
# try with an attempt
Expand All @@ -2653,10 +2652,8 @@ def test_launch_for_exam(self):
self.assertRedirects(response, expected_url, fetch_redirect_response=False)

def test_error_with_multiple_backends(self):
course_id = 'a/b/c'

ProctoredExam.objects.create(
course_id=course_id,
course_id=self.course_id,
content_id='test_content',
exam_name='Test Exam',
external_id='123aXqe3',
Expand All @@ -2666,7 +2663,7 @@ def test_error_with_multiple_backends(self):
backend='test',
)
ProctoredExam.objects.create(
course_id=course_id,
course_id=self.course_id,
content_id='test_content2',
exam_name='Test Exam',
external_id='123aXqe4',
Expand All @@ -2677,61 +2674,56 @@ def test_error_with_multiple_backends(self):
)
response = self.client.get(
reverse('edx_proctoring:instructor_dashboard_course',
kwargs={'course_id': course_id})
kwargs={'course_id': self.course_id})
)
self.assertEqual(response.status_code, 400)
self.assertIn('Multiple backends for course', response.data)

def test_error_with_no_exams(self):
course_id = 'a/b/c'
response = self.client.get(
reverse('edx_proctoring:instructor_dashboard_course',
kwargs={'course_id': course_id})
kwargs={'course_id': self.course_id})
)
self.assertEqual(response.status_code, 404)

# test the case of no PROCTORED exams
ProctoredExam.objects.create(
course_id=course_id,
course_id=self.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',
backend='mock',
)
response = self.client.get(
reverse('edx_proctoring:instructor_dashboard_course',
kwargs={'course_id': course_id})
kwargs={'course_id': self.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,
course_id=self.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',
backend='mock',
)
response = self.client.get(
reverse('edx_proctoring:instructor_dashboard_course',
kwargs={'course_id': course_id})
kwargs={'course_id': self.course_id})
)
self.assertEqual(response.status_code, 404)
self.assertEqual('No instructor dashboard for RPNow', response.data)
self.assertEqual('No instructor dashboard for Mock Backend', response.data)

def test_launch_for_configuration_dashboard(self):
course_id = 'a/b/c'

proctored_exam = ProctoredExam.objects.create(
course_id=course_id,
course_id=self.course_id,
content_id='test_content',
exam_name='Test Exam',
external_id='123aXqe3',
Expand All @@ -2741,12 +2733,55 @@ def test_launch_for_configuration_dashboard(self):
)
exam_id = proctored_exam.id

expected_url = '/instructor/%s/?exam=%s&config=true' % (course_id, proctored_exam.external_id)
expected_url = '/instructor/%s/?exam=%s&config=true' % (self.course_id, proctored_exam.external_id)
dashboard_url = reverse('edx_proctoring:instructor_dashboard_exam',
kwargs={'course_id': course_id, 'exam_id': exam_id})
kwargs={'course_id': self.course_id, 'exam_id': exam_id})
response = self.client.get(dashboard_url, {'config': 'true'})
self.assertRedirects(response, expected_url, fetch_redirect_response=False)

@ddt.data(
(True, True),
(True, False),
(False, True),
(False, False)
)
@ddt.unpack
def test_multiple_exams_returns_correct_dashboard(self, exam_1_is_proctored, exam_2_is_proctored):
ProctoredExam.objects.create(
course_id=self.course_id,
content_id='test_content',
exam_name='Test Exam',
external_id='123aXqe3',
time_limit_mins=90,
is_active=True,
is_proctored=exam_1_is_proctored,
backend='test',
)
ProctoredExam.objects.create(
course_id=self.course_id,
content_id='test_content2',
exam_name='Test Exam',
external_id='123aXqe4',
time_limit_mins=90,
is_active=True,
is_proctored=exam_2_is_proctored,
backend='test',
)

expected_url = '/instructor/%s/' % self.course_id
response = self.client.get(
reverse('edx_proctoring:instructor_dashboard_course',
kwargs={'course_id': self.course_id})
)
if not exam_1_is_proctored and not exam_2_is_proctored:
self.assertEqual(response.status_code, 404)
self.assertEqual(
u'No proctored exams in course {}'.format(self.course_id),
response.data
)
else:
self.assertRedirects(response, expected_url, fetch_redirect_response=False)


class TestBackendUserDeletion(LoggedInTestCase):
"""
Expand Down
83 changes: 49 additions & 34 deletions edx_proctoring/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -971,62 +971,77 @@ def get(self, request, course_id, exam_id=None):
Redirect to dashboard for a given course and optional exam_id
"""
exam = None
attempt_id = None
backend = None
ext_exam_id = None
attempt_id = None
show_configuration_dashboard = False

if exam_id:
exam = get_exam_by_id(exam_id)
backend = get_backend_provider(exam=exam)
# the exam_id in the url is our database id (for ease of lookups)
# but the backend needs its external id for the instructor dashboard
ext_exam_id = exam['external_id']
attempt_id = request.GET.get('attempt', None)

# only show the configuration dashboard if an exam_id is passed in
show_configuration_dashboard = request.GET.get('config', '').lower() == 'true'

else:
found_backend = None
existing_backend_name = None
for exam in get_all_exams_for_course(course_id, True):
exam_backend = exam['backend'] or settings.PROCTORING_BACKENDS.get('DEFAULT', None)
if found_backend and exam_backend != found_backend:
if not exam.get('is_proctored'):
# We should only get backends of exams which are configured to be proctored
continue

exam_backend_name = exam.get('backend')
backend = get_backend_provider(name=exam_backend_name)
if existing_backend_name and exam_backend_name != existing_backend_name:
# In this case, what are we supposed to do?!
# It should not be possible to get in this state, because
# course teams will be prevented from updating the backend after the course start date
error_message = u"Multiple backends for course %r %r != %r" % (
course_id,
found_backend,
exam['backend']
existing_backend_name,
exam_backend_name
)
return Response(data=error_message, status=400)
else:
found_backend = exam_backend
if exam is None:
error = _(u'No exams in course {course_id}.').format(course_id=course_id)
else:
backend = get_backend_provider(exam)
if backend:
user = {
'id': obscured_user_id(request.user.id, exam['backend']),
'full_name': request.user.profile.name,
'email': request.user.email
}

url = backend.get_instructor_url(
exam['course_id'],
user,
exam_id=ext_exam_id,
attempt_id=attempt_id,
show_configuration_dashboard=show_configuration_dashboard
)
if url:
return redirect(url)
else:
error = _(u'No instructor dashboard for {proctor_service}').format(
proctor_service=backend.verbose_name)
else:
error = _(u'No proctored exams in course {course_id}').format(course_id=course_id)
return Response(data=error, status=404, headers={'X-Frame-Options': 'sameorigin'})
existing_backend_name = exam_backend_name

if not exam:
return Response(
data=_(u'No exams in course {course_id}.').format(course_id=course_id),
status=404,
headers={'X-Frame-Options': 'sameorigin'}
)
if not backend:
return Response(
data=_(u'No proctored exams in course {course_id}').format(course_id=course_id),
status=404,
headers={'X-Frame-Options': 'sameorigin'}
)
user = {
'id': obscured_user_id(request.user.id, exam['backend']),
'full_name': request.user.profile.name,
'email': request.user.email
}

url = backend.get_instructor_url(
exam['course_id'],
user,
exam_id=ext_exam_id,
attempt_id=attempt_id,
show_configuration_dashboard=show_configuration_dashboard
)
if not url:
return Response(
data=_(u'No instructor dashboard for {proctor_service}').format(
proctor_service=backend.verbose_name
),
status=404,
headers={'X-Frame-Options': 'sameorigin'}
)
return redirect(url)


class BackendUserManagementAPI(AuthenticatedAPIView):
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "@edx/edx-proctoring",
"//": "Be sure to update the version number in edx_proctoring/__init__.py",
"//": "Note that the version format is slightly different than that of the Python version when using prereleases.",
"version": "2.1.9",
"version": "2.2.0",
"main": "edx_proctoring/static/index.js",
"repository": {
"type": "git",
Expand Down

0 comments on commit 6cebd32

Please sign in to comment.