Merge pull request #489 from edx/dcs/review-link

Include a direct link to the review in support tickets

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

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
 
 

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')

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):

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):

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:

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",