Merge pull request #164 from edx/cdodge/dont-allow-err-transition-when-completed

add some protection about not transition from a completed state to an…

edx_proctoring/api.py

@@ -552,6 +552,21 @@ def update_attempt_status(exam_id, user_id, to_status, raise_if_not_found=True,
         )
         raise ProctoredExamIllegalStatusTransition(err_msg)
 
+    # special case logic, if we are in a completed status we shouldn't allow
+    # for a transition to 'Error' state
+    if in_completed_status and to_status == ProctoredExamStudentAttemptStatus.error:
+        err_msg = (
+            'A status transition from {from_status} to {to_status} was attempted '
+            'on exam_id {exam_id} for user_id {user_id}. This is not '
+            'allowed!'.format(
+                from_status=exam_attempt_obj.status,
+                to_status=to_status,
+                exam_id=exam_id,
+                user_id=user_id
+            )
+        )
+        raise ProctoredExamIllegalStatusTransition(err_msg)
+
     # OK, state transition is fine, we can proceed
     exam_attempt_obj.status = to_status
     exam_attempt_obj.save()
@@ -591,6 +606,13 @@ def update_attempt_status(exam_id, user_id, to_status, raise_if_not_found=True,
         )
 
     if cascade_effects and ProctoredExamStudentAttemptStatus.is_a_cascadable_failure(to_status):
+        if to_status == ProctoredExamStudentAttemptStatus.declined:
+            # if user declines attempt, make sure we clear out the external_id and
+            # taking_as_proctored fields
+            exam_attempt_obj.taking_as_proctored = False
+            exam_attempt_obj.external_id = None
+            exam_attempt_obj.save()
+
         # some state transitions (namely to a rejected or declined status)
         # will mark other exams as declined because once we fail or decline
         # one exam all other (un-completed) proctored exams will be likewise

edx_proctoring/tests/test_api.py

@@ -1230,6 +1230,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
         (ProctoredExamStudentAttemptStatus.rejected, ProctoredExamStudentAttemptStatus.started),
         (ProctoredExamStudentAttemptStatus.not_reviewed, ProctoredExamStudentAttemptStatus.started),
         (ProctoredExamStudentAttemptStatus.error, ProctoredExamStudentAttemptStatus.started),
+        (ProctoredExamStudentAttemptStatus.submitted, ProctoredExamStudentAttemptStatus.error),
     )
     @ddt.unpack
     @patch.dict('django.conf.settings.PROCTORING_SETTINGS', {'ALLOW_TIMED_OUT_STATE': True})

edx_proctoring/tests/test_views.py

@@ -580,7 +580,61 @@ class TestStudentProctoredExamAttempt(LoggedInTestCase):
         )
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
-        self.assertEqual(response_data['status'], 'started')
+        self.assertEqual(response_data['status'], ProctoredExamStudentAttemptStatus.started)
+        attempt_code = response_data['attempt_code']
+
+        # test the polling callback point
+        response = self.client.get(
+            reverse(
+                'edx_proctoring.anonymous.proctoring_poll_status',
+                args=[attempt_code]
+            )
+        )
+        self.assertEqual(response.status_code, 200)
+
+        # now reset the time to 2 minutes in the future.
+        reset_time = datetime.now(pytz.UTC) + timedelta(minutes=2)
+        with freeze_time(reset_time):
+            response = self.client.get(
+                reverse('edx_proctoring.proctored_exam.attempt', args=[attempt_id])
+            )
+            self.assertEqual(response.status_code, 200)
+            response_data = json.loads(response.content)
+            self.assertEqual(response_data['status'], ProctoredExamStudentAttemptStatus.error)
+
+    def test_attempt_status_stickiness(self):
+        """
+        Test to confirm that a status timeout error will not alter a completed state
+        """
+        # Create an exam.
+        proctored_exam = ProctoredExam.objects.create(
+            course_id='a/b/c',
+            content_id='test_content',
+            exam_name='Test Exam',
+            external_id='123aXqe3',
+            time_limit_mins=90
+        )
+        attempt_data = {
+            'exam_id': proctored_exam.id,
+            'external_id': proctored_exam.external_id,
+            'start_clock': True,
+        }
+        response = self.client.post(
+            reverse('edx_proctoring.proctored_exam.attempt.collection'),
+            attempt_data
+        )
+
+        self.assertEqual(response.status_code, 200)
+        response_data = json.loads(response.content)
+        attempt_id = response_data['exam_attempt_id']
+        self.assertEqual(attempt_id, 1)
+
+        response = self.client.get(
+            reverse('edx_proctoring.proctored_exam.attempt', args=[attempt_id])
+        )
+        self.assertEqual(response.status_code, 200)
+        response_data = json.loads(response.content)
+        self.assertEqual(response_data['status'], ProctoredExamStudentAttemptStatus.started)
         attempt_code = response_data['attempt_code']
 
         # test the polling callback point
@@ -592,6 +646,13 @@ class TestStudentProctoredExamAttempt(LoggedInTestCase):
         )
         self.assertEqual(response.status_code, 200)
 
+        # now switched to a submitted state
+        update_attempt_status(
+            proctored_exam.id,
+            self.user.id,
+            ProctoredExamStudentAttemptStatus.submitted
+        )
+
         # now reset the time to 2 minutes in the future.
         reset_time = datetime.now(pytz.UTC) + timedelta(minutes=2)
         with freeze_time(reset_time):
@@ -600,7 +661,11 @@ class TestStudentProctoredExamAttempt(LoggedInTestCase):
             )
             self.assertEqual(response.status_code, 200)
             response_data = json.loads(response.content)
-            self.assertEqual(response_data['status'], 'error')
+            # make sure the submitted status is sticky
+            self.assertEqual(
+                response_data['status'],
+                ProctoredExamStudentAttemptStatus.submitted
+            )
 
     @ddt.data(
         ProctoredExamStudentAttemptStatus.created,

edx_proctoring/views.py

@@ -37,6 +37,7 @@ from edx_proctoring.exceptions import (
     UserNotFoundException,
     ProctoredExamPermissionDenied,
     StudentExamAttemptDoesNotExistsException,
+    ProctoredExamIllegalStatusTransition,
 )
 from edx_proctoring.serializers import ProctoredExamSerializer, ProctoredExamStudentAttemptSerializer
 from edx_proctoring.models import ProctoredExamStudentAttemptStatus, ProctoredExamStudentAttempt
@@ -285,12 +286,16 @@ class StudentProctoredExamAttempt(AuthenticatedAPIView):
             last_poll_timestamp = attempt['last_poll_timestamp']
             if last_poll_timestamp is not None \
                     and (datetime.now(pytz.UTC) - last_poll_timestamp).total_seconds() > SOFTWARE_SECURE_CLIENT_TIMEOUT:
-                attempt['status'] = 'error'
-                update_attempt_status(
-                    attempt['proctored_exam']['id'],
-                    attempt['user']['id'],
-                    ProctoredExamStudentAttemptStatus.error
-                )
+                try:
+                    update_attempt_status(
+                        attempt['proctored_exam']['id'],
+                        attempt['user']['id'],
+                        ProctoredExamStudentAttemptStatus.error
+                    )
+                    attempt['status'] = ProctoredExamStudentAttemptStatus.error
+                except ProctoredExamIllegalStatusTransition:
+                    # don't transition a completed state to an error state
+                    pass
 
             # add in the computed time remaining as a helper to a client app
             time_remaining_seconds = get_time_remaining_for_attempt(attempt)

setup.py

@@ -34,7 +34,7 @@ def load_requirements(*requirements_paths):
 
 setup(
     name='edx-proctoring',
-    version='0.9.6b',
+    version='0.9.6c',
     description='Proctoring subsystem for Open edX',
     long_description=open('README.md').read(),
     author='edX',