fix: Proper handling to review callback on Non-proctored exam attempts (

#1018)

Co-authored-by: Simon Chen <[email protected]>

CHANGELOG.rst

@@ -14,6 +14,11 @@ Change Log
 Unreleased
 ~~~~~~~~~~
 
+[4.8.4] - 2022-01-12
+~~~~~~~~~~~~~~~~~~~~
+* Return better http status when review callback resulted in the original
+  exam no longer being proctored 
+
 [4.8.3] - 2022-01-12
 ~~~~~~~~~~~~~~~~~~~~
 * Exclude verified name results with a "denied" status when registering a proctored

edx_proctoring/__init__.py

@@ -3,6 +3,6 @@
 """
 
 # Be sure to update the version number in edx_proctoring/package.json
-__version__ = '4.8.3'
+__version__ = '4.8.4'
 
 default_app_config = 'edx_proctoring.apps.EdxProctoringConfig'  # pylint: disable=invalid-name

edx_proctoring/tests/test_views.py

@@ -28,7 +28,8 @@
     get_exam_attempt_by_id,
     mark_exam_attempt_as_ready_to_resume,
     reset_practice_exam,
-    update_attempt_status
+    update_attempt_status,
+    update_exam
 )
 from edx_proctoring.backends.tests.test_backend import TestBackendProvider
 from edx_proctoring.backends.tests.test_review_payload import create_test_review_payload
@@ -3801,6 +3802,47 @@ def test_review_mismatch(self):
         )
         self.assertEqual(response.status_code, 400)
 
+    def test_review_callback_non_proctored_exam(self):
+        """
+        Simulates a callback from the proctoring service with the
+        review data but the matched attempt had the exam not proctored
+        """
+        exam_id = create_exam(
+            course_id='foo/bar/baz',
+            content_id='content',
+            exam_name='Sample Exam',
+            time_limit_mins=10,
+            is_proctored=True
+        )
+
+        # be sure to use the mocked out exam provider handlers
+        with HTTMock(mock_response_content):
+            attempt_id = create_exam_attempt(
+                exam_id,
+                self.user.id,
+                taking_as_proctored=True
+            )
+
+        attempt = get_exam_attempt_by_id(attempt_id)
+        self.assertIsNotNone(attempt['external_id'])
+
+        update_exam(exam_id, is_proctored=False)
+
+        test_payload = create_test_review_payload(
+            attempt_code=attempt['attempt_code'],
+            external_id=attempt['external_id'],
+        )
+        response = self.client.post(
+            reverse(
+                'edx_proctoring:proctored_exam.attempt.callback',
+                args=[attempt['external_id']]
+            ),
+            data=test_payload,
+            content_type='application/json'
+        )
+        self.assertEqual(response.status_code, 412)
+        self.assertEqual(response.data, 'Exam no longer proctored')
+
     def test_review_callback_get(self):
         """
         We don't support any http METHOD other than GET

edx_proctoring/views.py

@@ -1752,6 +1752,23 @@ def make_review(self, attempt, data, backend=None):
         if not backend:
             backend = get_backend_provider(attempt['proctored_exam'])
 
+        if not backend:
+            LOG.warning(
+                (
+                    'Tried to mark review on an exam attempt that is no longer a proctored exam.'
+                    'The exam id=%(exam_id)s, exam.is_proctored=%(is_proctored)s,'
+                    'exam.course_id=%(course_id)s and attempt_id=%(attempt_id)s'
+                    'The review status will not be preserved on this attempt'
+                ),
+                {
+                    'attempt_id': attempt['id'],
+                    'exam_id': attempt['proctored_exam']['id'],
+                    'is_proctored': attempt['proctored_exam']['is_proctored'],
+                    'course_id': attempt['proctored_exam']['course_id'],
+                }
+            )
+            return Response(data='Exam no longer proctored', status=412)
+
         # this method should convert the payload into a normalized format
         backend_review = backend.on_review_callback(attempt, data)
 
@@ -1847,6 +1864,7 @@ def make_review(self, attempt, data, backend=None):
                     review_status=review.review_status,
                     review_url=review_url,
                 )
+        return Response(data='OK')
 
 
 class ProctoredExamReviewCallback(ProctoredAPIView, BaseReviewCallback):
@@ -1866,8 +1884,7 @@ def post(self, request, external_id):
             )
             raise StudentExamAttemptDoesNotExistsException(err_msg)
         if request.user.has_perm('edx_proctoring.can_review_attempt', attempt):
-            self.make_review(attempt, request.data)
-            resp = Response(data='OK')
+            resp = self.make_review(attempt, request.data)
         else:
             resp = Response(status=403)
         return resp
@@ -1927,10 +1944,11 @@ def post(self, request):
             raise StudentExamAttemptDoesNotExistsException(err_msg)
         serialized = ProctoredExamStudentAttemptSerializer(attempt_obj).data
         serialized['is_archived'] = is_archived
-        self.make_review(serialized,
-                         request.data,
-                         backend=provider)
-        return Response('OK')
+        return self.make_review(
+            serialized,
+            request.data,
+            backend=provider
+        )
 
 
 class InstructorDashboard(AuthenticatedAPIView):

package.json

@@ -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.8.3",
+  "version": "4.8.4",
   "main": "edx_proctoring/static/index.js",
   "scripts": {
     "test": "gulp test"