diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 45d1a5810be..1c0bdc61f3a 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -5,6 +5,6 @@ from __future__ import absolute_import # Be sure to update the version number in edx_proctoring/package.json -__version__ = '1.5.0rc3' +__version__ = '1.5.0rc4' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/backends/tests/test_backend.py b/edx_proctoring/backends/tests/test_backend.py index 0ce25dcb859..742a6c74ac2 100644 --- a/edx_proctoring/backends/tests/test_backend.py +++ b/edx_proctoring/backends/tests/test_backend.py @@ -67,6 +67,8 @@ def get_instructor_url(self, course_id, user, exam_id=None, attempt_id=None): url = '/instructor/%s/' % course_id if exam_id: url += '?exam=%s' % exam_id + if attempt_id: + url += '&attempt=%s' % attempt_id return url diff --git a/edx_proctoring/tests/test_reviews.py b/edx_proctoring/tests/test_reviews.py index 21093f29e18..72b44e019e9 100644 --- a/edx_proctoring/tests/test_reviews.py +++ b/edx_proctoring/tests/test_reviews.py @@ -8,6 +8,7 @@ import ddt from mock import patch +from django.test import RequestFactory from django.urls import reverse from edx_proctoring import constants @@ -33,6 +34,7 @@ class ReviewTests(LoggedInTestCase): """ def setUp(self): super(ReviewTests, self).setUp() + self.dummy_request = RequestFactory().get('/') self.exam_id = create_exam( course_id='foo/bar/baz', content_id='content', @@ -113,12 +115,11 @@ def test_psi_review_callback(self, psi_review_status, review_status, credit_requ )) # test_payload = self.get_review_payload(review_status) self.attempt['proctored_exam']['backend'] = 'software_secure' - if review_status is None: with self.assertRaises(ProctoredExamBadReviewStatus): ProctoredExamReviewCallback().make_review(self.attempt, test_payload) else: - ProctoredExamReviewCallback().make_review(self.attempt, test_payload) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) # make sure that what we have in the Database matches what we expect review = ProctoredExamSoftwareSecureReview.get_review_by_attempt_code(self.attempt['attempt_code']) @@ -150,11 +151,13 @@ def test_psi_review_callback(self, psi_review_status, review_status, credit_requ # check to see whether the zendesk ticket was created self.assertEqual(len(notifications), 1) exam = self.attempt['proctored_exam'] + review_url = 'http://testserver/edx_proctoring/v1/instructor/foo/bar/baz/1?attempt=testexternalid' self.assertEqual(notifications, [(exam['course_id'], exam['exam_name'], self.attempt['user']['username'], - review.review_status)]) + review.review_status, + review_url)]) else: self.assertEqual(len(notifications), 0) @@ -242,7 +245,7 @@ def test_allow_review_resubmission(self): # now call again, this will not throw exception test_payload['status'] = ReviewStatus.suspicious - ProctoredExamReviewCallback().make_review(self.attempt, test_payload) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) # make sure that what we have in the Database matches what we expect review = ProctoredExamSoftwareSecureReview.get_review_by_attempt_code(self.attempt['attempt_code']) @@ -274,9 +277,10 @@ def test_failure_submission(self): don't automatically update the status to failure """ test_payload = self.get_review_payload(ReviewStatus.suspicious) + allow_rejects = not constants.REQUIRE_FAILURE_SECOND_REVIEWS # submit a Suspicious review payload - ProctoredExamReviewCallback().make_review(self.attempt, test_payload) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) # now look at the attempt and make sure it did not # transition to failure on the callback, @@ -306,7 +310,7 @@ def test_failure_submission_rejected(self): test_payload = self.get_review_payload(ReviewStatus.suspicious) allow_rejects = not constants.REQUIRE_FAILURE_SECOND_REVIEWS # submit a Suspicious review payload - ProctoredExamReviewCallback().make_review(self.attempt, test_payload) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) # now look at the attempt and make sure it did not # transition to failure on the callback, @@ -359,7 +363,7 @@ def test_update_archived_attempt(self): # now we'll make another review for the archived attempt. It should NOT update the status test_payload = self.get_review_payload(ReviewStatus.suspicious) self.attempt['is_archived'] = True - ProctoredExamReviewCallback().make_review(self.attempt, test_payload) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) attempt, is_archived = locate_attempt_by_attempt_code(self.attempt['attempt_code']) self.assertTrue(is_archived) self.assertEqual(attempt.status, 'verified') diff --git a/edx_proctoring/tests/test_services.py b/edx_proctoring/tests/test_services.py index 8a87086ba7b..37cfbe7b254 100644 --- a/edx_proctoring/tests/test_services.py +++ b/edx_proctoring/tests/test_services.py @@ -145,11 +145,11 @@ def is_course_staff(self, user, course_id): """ return self.is_user_course_staff - def send_support_notification(self, course_id, exam_name, student_username, review_status): + def send_support_notification(self, course_id, exam_name, student_username, review_status, review_url): """ Mocked implementation of send_support_notification """ - self.notifications.append((course_id, exam_name, student_username, review_status)) + self.notifications.append((course_id, exam_name, student_username, review_status, review_url)) class TestProctoringService(unittest.TestCase): diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index f47c160cfff..5867f1ada3f 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -2528,10 +2528,16 @@ def test_launch_for_exam(self): ) exam_id = proctored_exam.id - expected_url = '/instructor/%s/?exam=%s' % (course_id, exam_id) - response = self.client.get( - reverse('edx_proctoring:instructor_dashboard_exam', kwargs={'course_id': course_id, 'exam_id': exam_id}) - ) + expected_url = '/instructor/%s/?exam=%s' % (course_id, proctored_exam.external_id) + dashboard_url = reverse('edx_proctoring:instructor_dashboard_exam', + kwargs={'course_id': 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 + attempt_frag = 'attempt=abcde' + expected_url += '&%s' % attempt_frag + dashboard_url += '?%s' % attempt_frag + response = self.client.get(dashboard_url) self.assertRedirects(response, expected_url, fetch_redirect_response=False) def test_error_with_multiple_backends(self): diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 7dc388a9db6..e90ce383e9e 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -780,7 +780,7 @@ class BaseReviewCallback(object): Base class for review callbacks. make_review handles saving reviews and review comments. """ - def make_review(self, attempt, data, backend=None): + def make_review(self, attempt, data, request=None, backend=None): """ Save the review and review comments """ @@ -862,11 +862,19 @@ def make_review(self, attempt, data, backend=None): if review.review_status in SoftwareSecureReviewStatus.notify_support_for_status: instructor_service = get_runtime_service('instructor') if instructor_service: + course_id = attempt['proctored_exam']['course_id'] + exam_id = attempt['proctored_exam']['id'] + review_url = request.build_absolute_uri( + u'{}?attempt={}'.format( + reverse('edx_proctoring:instructor_dashboard_exam', args=[course_id, exam_id]), + attempt['external_id'] + )) instructor_service.send_support_notification( course_id=attempt['proctored_exam']['course_id'], exam_name=attempt['proctored_exam']['exam_name'], student_username=attempt['user']['username'], - review_status=review.review_status + review_status=review.review_status, + review_url=review_url, ) if not attempt.get('is_archived', False): @@ -900,7 +908,7 @@ def post(self, request, external_id): attempt = get_exam_attempt_by_external_id(external_id) if attempt is None: raise StudentExamAttemptDoesNotExistsException('not found') - self.make_review(attempt, request.data) + self.make_review(attempt, request.data, request) return Response(data='OK') @@ -961,6 +969,7 @@ def post(self, request): serialized['is_archived'] = is_archived self.make_review(serialized, request.data, + request, backend=provider) return Response('OK') @@ -975,8 +984,14 @@ 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 + ext_exam_id = None if exam_id: exam = get_exam_by_id(exam_id) + # 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) else: found_backend = None for exam in get_all_exams_for_course(course_id, True): @@ -1001,7 +1016,7 @@ def get(self, request, course_id, exam_id=None): 'full_name': request.user.get_full_name(), 'email': request.user.email } - url = backend.get_instructor_url(exam['course_id'], user, exam_id=exam_id) + url = backend.get_instructor_url(exam['course_id'], user, exam_id=ext_exam_id, attempt_id=attempt_id) if url: return redirect(url) else: diff --git a/package.json b/package.json index dc3bbed56eb..5526a156f68 100644 --- a/package.json +++ b/package.json @@ -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": "1.5.0-rc.3", + "version": "1.5.0-rc.4", "main": "edx_proctoring/static/index.js", "repository": { "type": "git",