make status transitions to completed more formalized

edx_proctoring/api.py

@@ -18,6 +18,7 @@ from edx_proctoring.exceptions import (
     StudentExamAttemptAlreadyExistsException,
     StudentExamAttemptDoesNotExistsException,
     StudentExamAttemptedAlreadyStarted,
+    ProctoredExamIllegalStatusTransition,
 )
 from edx_proctoring.models import (
     ProctoredExam,
@@ -382,7 +383,11 @@ def _start_exam_attempt(existing_attempt):
 
         raise StudentExamAttemptedAlreadyStarted(err_msg)
 
-    existing_attempt.start_exam_attempt()
+    update_attempt_status(
+        existing_attempt.proctored_exam_id,
+        existing_attempt.user_id,
+        ProctoredExamStudentAttemptStatus.started
+    )
 
     return existing_attempt.id
 
@@ -391,7 +396,7 @@ def stop_exam_attempt(exam_id, user_id):
     """
     Marks the exam attempt as completed (sets the completed_at field and updates the record)
     """
-    return update_attempt_status(exam_id, user_id, ProctoredExamStudentAttemptStatus.completed)
+    return update_attempt_status(exam_id, user_id, ProctoredExamStudentAttemptStatus.ready_to_submit)
 
 
 def mark_exam_attempt_timeout(exam_id, user_id):
@@ -417,6 +422,37 @@ def update_attempt_status(exam_id, user_id, to_status):
     if exam_attempt_obj is None:
         raise StudentExamAttemptDoesNotExistsException('Error. Trying to look up an exam that does not exist.')
 
+    #
+    # don't allow state transitions from a completed state to an incomplete state
+    # if a re-attempt is desired then the current attempt must be deleted
+    #
+    in_completed_status = exam_attempt_obj.status in [
+        ProctoredExamStudentAttemptStatus.verified, ProctoredExamStudentAttemptStatus.rejected,
+        ProctoredExamStudentAttemptStatus.declined, ProctoredExamStudentAttemptStatus.not_reviewed,
+        ProctoredExamStudentAttemptStatus.submitted, ProctoredExamStudentAttemptStatus.error,
+        ProctoredExamStudentAttemptStatus.timed_out
+    ]
+
+    to_incompleted_status = to_status in [
+        ProctoredExamStudentAttemptStatus.eligible, ProctoredExamStudentAttemptStatus.created,
+        ProctoredExamStudentAttemptStatus.ready_to_start, ProctoredExamStudentAttemptStatus.started,
+        ProctoredExamStudentAttemptStatus.ready_to_submit
+    ]
+
+    if in_completed_status and to_incompleted_status:
+        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()
 
@@ -427,7 +463,7 @@ def update_attempt_status(exam_id, user_id, to_status):
     update_credit = to_status in [
         ProctoredExamStudentAttemptStatus.verified, ProctoredExamStudentAttemptStatus.rejected,
         ProctoredExamStudentAttemptStatus.declined, ProctoredExamStudentAttemptStatus.not_reviewed,
-        ProctoredExamStudentAttemptStatus.submitted
+        ProctoredExamStudentAttemptStatus.submitted, ProctoredExamStudentAttemptStatus.error
     ]
 
     if update_credit:
@@ -446,6 +482,16 @@ def update_attempt_status(exam_id, user_id, to_status):
             status=verification
         )
 
+    if to_status == ProctoredExamStudentAttemptStatus.submitted:
+        # also mark the exam attempt completed_at timestamp
+        # after we submit the attempt
+        exam_attempt_obj.completed_at = datetime.now(pytz.UTC)
+        exam_attempt_obj.save()
+
+    if to_status == ProctoredExamStudentAttemptStatus.started:
+        exam_attempt_obj.started_at = datetime.now(pytz.UTC)
+        exam_attempt_obj.save()
+
     return exam_attempt_obj.id
 
 
@@ -681,11 +727,11 @@ def get_student_view(user_id, course_id, content_id,
         student_view_template = 'proctoring/seq_proctored_exam_verified.html'
     elif attempt['status'] == ProctoredExamStudentAttemptStatus.rejected:
         student_view_template = 'proctoring/seq_proctored_exam_rejected.html'
-    elif attempt['status'] == ProctoredExamStudentAttemptStatus.completed:
+    elif attempt['status'] == ProctoredExamStudentAttemptStatus.ready_to_submit:
         if is_proctored:
-            student_view_template = 'proctoring/seq_proctored_exam_completed.html'
+            student_view_template = 'proctoring/seq_proctored_exam_ready_to_submit.html'
         else:
-            student_view_template = 'proctoring/seq_timed_exam_completed.html'
+            student_view_template = 'proctoring/seq_timed_exam_ready_to_submit.html'
 
     if student_view_template:
         template = loader.get_template(student_view_template)

edx_proctoring/exceptions.py

@@ -75,3 +75,9 @@ class ProctoredExamBadReviewStatus(ProctoredBaseException):
     """
     Raised if we get an unexpected status back from the Proctoring attempt review status
     """
+
+
+class ProctoredExamIllegalStatusTransition(ProctoredBaseException):
+    """
+    Raised if a state transition is not allowed, e.g. going from submitted to started
+    """

edx_proctoring/models.py

 """
 Data models for the proctoring subsystem
 """
-import pytz
-from datetime import datetime
 from django.db import models
 from django.db.models import Q
 from django.db.models.signals import post_save, pre_delete
@@ -80,6 +78,60 @@ class ProctoredExam(TimeStampedModel):
         return cls.objects.filter(course_id=course_id)
 
 
+class ProctoredExamStudentAttemptStatus(object):
+    """
+    A class to enumerate the various status that an attempt can have
+
+    IMPORTANT: Since these values are stored in a database, they are system
+    constants and should not be language translated, since translations
+    might change over time.
+    """
+
+    # the student is eligible to decide if he/she wants to persue credit
+    eligible = 'eligible'
+
+    # the attempt record has been created, but the exam has not yet
+    # been started
+    created = 'created'
+
+    # the attempt is ready to start but requires
+    # user to acknowledge that he/she wants to start the exam
+    ready_to_start = 'ready_to_start'
+
+    # the student has started the exam and is
+    # in the process of completing the exam
+    started = 'started'
+
+    # the student has completed the exam
+    ready_to_submit = 'ready_to_submit'
+
+    #
+    # The follow statuses below are considered in a 'completed' state
+    # and we will not allow transitions to status above this mark
+    #
+
+    # the student declined to take the exam as a proctored exam
+    declined = 'declined'
+
+    # the exam has timed out
+    timed_out = 'timed_out'
+
+    # the student has submitted the exam for proctoring review
+    submitted = 'submitted'
+
+    # the exam has been verified and approved
+    verified = 'verified'
+
+    # the exam has been rejected
+    rejected = 'rejected'
+
+    # the exam was not reviewed
+    not_reviewed = 'not_reviewed'
+
+    # the exam is believed to be in error
+    error = 'error'
+
+
 class ProctoredExamStudentAttemptManager(models.Manager):
     """
     Custom manager
@@ -137,62 +189,13 @@ class ProctoredExamStudentAttemptManager(models.Manager):
         """
         Returns the active student exams (user in-progress exams)
         """
-        filtered_query = Q(user_id=user_id) & Q(started_at__isnull=False) & Q(completed_at__isnull=True)
+        filtered_query = Q(user_id=user_id) & Q(status=ProctoredExamStudentAttemptStatus.started)
         if course_id is not None:
             filtered_query = filtered_query & Q(proctored_exam__course_id=course_id)
 
         return self.filter(filtered_query).order_by('-created')
 
 
-class ProctoredExamStudentAttemptStatus(object):
-    """
-    A class to enumerate the various status that an attempt can have
-
-    IMPORTANT: Since these values are stored in a database, they are system
-    constants and should not be language translated, since translations
-    might change over time.
-    """
-
-    # the student is eligible to decide if he/she wants to persue credit
-    eligible = 'eligible'
-
-    # the student declined to take the exam as a proctored exam
-    declined = 'declined'
-
-    # the attempt record has been created, but the exam has not yet
-    # been started
-    created = 'created'
-
-    # the attempt is ready to start but requires
-    # user to acknowledge that he/she wants to start the exam
-    ready_to_start = 'ready_to_start'
-
-    # the student has started the exam and is
-    # in the process of completing the exam
-    started = 'started'
-
-    # the exam has timed out
-    timed_out = 'timed_out'
-
-    # the student has completed the exam
-    completed = 'completed'
-
-    # the student has submitted the exam for proctoring review
-    submitted = 'submitted'
-
-    # the exam has been verified and approved
-    verified = 'verified'
-
-    # the exam has been rejected
-    rejected = 'rejected'
-
-    # the exam was not reviewed
-    not_reviewed = 'not_reviewed'
-
-    # the exam is believed to be in error
-    error = 'error'
-
-
 class ProctoredExamStudentAttempt(TimeStampedModel):
     """
     Information about the Student Attempt on a
@@ -206,6 +209,8 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
 
     # started/completed date times
     started_at = models.DateTimeField(null=True)
+
+    # completed_at means when the attempt was 'submitted'
     completed_at = models.DateTimeField(null=True)
 
     last_poll_timestamp = models.DateTimeField(null=True)
@@ -240,11 +245,6 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
         verbose_name = 'proctored exam attempt'
         unique_together = (('user', 'proctored_exam'),)
 
-    @property
-    def is_active(self):
-        """ returns boolean if this attempt is considered active """
-        return self.started_at and not self.completed_at
-
     @classmethod
     def create_exam_attempt(cls, exam_id, user_id, student_name, allowed_time_limit_mins,
                             attempt_code, taking_as_proctored, is_sample_attempt, external_id):
@@ -265,14 +265,6 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
             status=ProctoredExamStudentAttemptStatus.created,
         )
 
-    def start_exam_attempt(self):
-        """
-        sets the model's state when an exam attempt has started
-        """
-        self.started_at = datetime.now(pytz.UTC)
-        self.status = ProctoredExamStudentAttemptStatus.started
-        self.save()
-
     def delete_exam_attempt(self):
         """
         deletes the exam attempt object and archives it to the ProctoredExamStudentAttemptHistory table.

edx_proctoring/templates/proctoring/proctoring_launch_callback.html

@@ -213,8 +213,8 @@
         var url = '{{exam_attempt_status_url}}';
 
         $.ajax(url).success(function(data){
-          // has the end user completed the exam in the LMS?!?
-          if (data.status === 'completed' || data.status === 'submitted' || data.status === 'verified') {
+          // has the end user completed and submitted the exam in the LMS?!?
+          if (data.status === 'submitted') {
             // Signal that the desktop software should terminate
             // NOTE: This is per the API documentation from SoftwareSecure
             window.external.quitApplication();

edx_proctoring/templates/proctoring/seq_proctored_exam_completed.html → edx_proctoring/templates/proctoring/seq_proctored_exam_ready_to_submit.html

@@ -11,11 +11,18 @@
       Your worked will then be graded and your proctored session will be reviewed separately.
     {% endblocktrans %}
   </p>
-  <button type="button" name="submit-proctored-exam" >
+  <button type="button" name="submit-proctored-exam" class="exam-action-button" data-action="submit" data-exam-id="{{exam_id}}" data-change-state-url="{{data-change-state-url}}">
     {% blocktrans %}
-      I'm ready! Submit my answers and end my proctored exam
+      I'm ready! Submit my answers and end my proctored exam.
     {% endblocktrans %}
   </button>
+  {% if does_time_remain %}
+    <button type="button" name="goback-proctored-exam" class="exam-action-button" data-action="start" data-exam-id="{{exam_id}}" data-change-state-url="{{data-change-state-url}}">
+      {% blocktrans %}
+        No, I am not ready! I'd like to continue my work.
+      {% endblocktrans %}
+    </button>
+  {% endif %}
 </div>
 <div class="footer-sequence border-b-0 padding-b-0">
   <span> {% trans "What happens next ?" %} </span>
@@ -27,3 +34,25 @@
     {% endblocktrans %}
   </p>
 </div>
+<script type="text/javascript">
+  $('.exam-action-button').click(
+    function(event) {
+      var action_url = $(this).data('data-change-state-url');
+      var exam_id = $(this).data('exam-id');
+      var action = $(this).data('data-action')
+
+      // Update the state of the attempt
+      $.ajax({
+        url: url,
+        type: 'PUT',
+        data: {
+          action: action
+        },
+        success: function() {
+          // Reloading page will reflect the new state of the attempt
+          location.reload()
+        }
+      });
+    }
+  );
+</script>

edx_proctoring/tests/test_api.py

@@ -3,6 +3,7 @@
 """
 All tests for the models.py
 """
+import ddt
 from datetime import datetime, timedelta
 from mock import patch
 import pytz
@@ -39,7 +40,8 @@ from edx_proctoring.exceptions import (
     StudentExamAttemptAlreadyExistsException,
     StudentExamAttemptDoesNotExistsException,
     StudentExamAttemptedAlreadyStarted,
-    UserNotFoundException
+    UserNotFoundException,
+    ProctoredExamIllegalStatusTransition
 )
 from edx_proctoring.models import (
     ProctoredExam,
@@ -56,6 +58,7 @@ from edx_proctoring.tests.test_services import MockCreditService
 from edx_proctoring.runtime import set_runtime_service, get_runtime_service
 
 
+@ddt.ddt
 class ProctoredExamApiTests(LoggedInTestCase):
     """
     All tests for the models.py
@@ -497,8 +500,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
         Test to get the all the active
         exams for the user.
         """
-        active_exam_attempt = self._create_started_exam_attempt()
-        self.assertEqual(active_exam_attempt.is_active, True)
+        self._create_started_exam_attempt()
         exam_id = create_exam(
             course_id=self.course_id,
             content_id='test_content_2',
@@ -938,7 +940,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
         Test for get_student_view proctored exam which has been completed.
         """
         exam_attempt = self._create_started_exam_attempt()
-        exam_attempt.status = "completed"
+        exam_attempt.status = ProctoredExamStudentAttemptStatus.ready_to_submit
         exam_attempt.save()
 
         rendered_response = get_student_view(
@@ -1022,7 +1024,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
         Test for get_student_view timed exam which has completed.
         """
         exam_attempt = self._create_started_exam_attempt(is_proctored=False)
-        exam_attempt.status = "completed"
+        exam_attempt.status = ProctoredExamStudentAttemptStatus.ready_to_submit
         exam_attempt.save()
 
         rendered_response = get_student_view(
@@ -1057,3 +1059,55 @@ class ProctoredExamApiTests(LoggedInTestCase):
             credit_status['credit_requirement_status'][0]['status'],
             'submitted'
         )
+
+    def test_error_credit_state(self):
+        """
+        Verify that putting an attempt into the error state will also mark
+        the credit requirement as failed
+        """
+        exam_attempt = self._create_started_exam_attempt()
+        update_attempt_status(
+            exam_attempt.proctored_exam_id,
+            self.user.id,
+            ProctoredExamStudentAttemptStatus.error
+        )
+
+        credit_service = get_runtime_service('credit')
+        credit_status = credit_service.get_credit_state(self.user.id, exam_attempt.proctored_exam.course_id)
+
+        self.assertEqual(len(credit_status['credit_requirement_status']), 1)
+        self.assertEqual(
+            credit_status['credit_requirement_status'][0]['status'],
+            'failed'
+        )
+
+    @ddt.data(
+        (ProctoredExamStudentAttemptStatus.declined, ProctoredExamStudentAttemptStatus.eligible),
+        (ProctoredExamStudentAttemptStatus.timed_out, ProctoredExamStudentAttemptStatus.created),
+        (ProctoredExamStudentAttemptStatus.submitted, ProctoredExamStudentAttemptStatus.ready_to_start),
+        (ProctoredExamStudentAttemptStatus.verified, ProctoredExamStudentAttemptStatus.started),
+        (ProctoredExamStudentAttemptStatus.rejected, ProctoredExamStudentAttemptStatus.started),
+        (ProctoredExamStudentAttemptStatus.not_reviewed, ProctoredExamStudentAttemptStatus.started),
+        (ProctoredExamStudentAttemptStatus.error, ProctoredExamStudentAttemptStatus.started),
+    )
+    @ddt.unpack
+    def test_illegal_status_transition(self, from_status, to_status):
+        """
+        Verify that we cannot reset backwards an attempt status
+        once it is in a completed state
+        """
+
+        exam_attempt = self._create_started_exam_attempt()
+        update_attempt_status(
+            exam_attempt.proctored_exam_id,
+            self.user.id,
+            from_status
+        )
+
+        with self.assertRaises(ProctoredExamIllegalStatusTransition):
+            print '*** from = {}  to {}'.format(from_status, to_status)
+            update_attempt_status(
+                exam_attempt.proctored_exam_id,
+                self.user.id,
+                to_status
+            )

edx_proctoring/tests/test_views.py

@@ -13,7 +13,12 @@ from django.test.client import Client
 from django.core.urlresolvers import reverse, NoReverseMatch
 from django.contrib.auth.models import User
 
-from edx_proctoring.models import ProctoredExam, ProctoredExamStudentAttempt, ProctoredExamStudentAllowance
+from edx_proctoring.models import (
+    ProctoredExam,
+    ProctoredExamStudentAttempt,
+    ProctoredExamStudentAllowance,
+    ProctoredExamStudentAttemptStatus,
+)
 from edx_proctoring.views import require_staff
 from edx_proctoring.api import (
     create_exam,
@@ -706,6 +711,67 @@ class TestStudentProctoredExamAttempt(LoggedInTestCase):
         response_data = json.loads(response.content)
         self.assertEqual(response_data['exam_attempt_id'], old_attempt_id)
 
+    def test_submit_exam_attempt(self):
+        """
+        Tries to submit an exam
+        """
+        # 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,
+            'user_id': self.student_taking_exam.id,
+            'external_id': proctored_exam.external_id
+        }
+        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)
+        self.assertGreater(response_data['exam_attempt_id'], 0)
+        old_attempt_id = response_data['exam_attempt_id']
+
+        response = self.client.put(
+            reverse('edx_proctoring.proctored_exam.attempt', args=[old_attempt_id]),
+            {
+                'action': 'submit',
+            }
+        )
+
+        self.assertEqual(response.status_code, 200)
+        response_data = json.loads(response.content)
+        self.assertEqual(response_data['exam_attempt_id'], old_attempt_id)
+
+        attempt = get_exam_attempt_by_id(response_data['exam_attempt_id'])
+
+        self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.submitted)
+
+        # we should not be able to restart it
+        response = self.client.put(
+            reverse('edx_proctoring.proctored_exam.attempt', args=[old_attempt_id]),
+            {
+                'action': 'start',
+            }
+        )
+
+        self.assertEqual(response.status_code, 400)
+
+        response = self.client.put(
+            reverse('edx_proctoring.proctored_exam.attempt', args=[old_attempt_id]),
+            {
+                'action': 'stop',
+            }
+        )
+
+        self.assertEqual(response.status_code, 400)
+
     def test_get_exam_attempts(self):
         """
         Test to get the exam attempts in a course.

edx_proctoring/views.py

@@ -30,7 +30,6 @@ from edx_proctoring.api import (
     get_all_exam_attempts,
     remove_exam_attempt,
     get_filtered_exam_attempts,
-    update_exam_attempt,
     update_attempt_status
 )
 from edx_proctoring.exceptions import (
@@ -283,7 +282,11 @@ class StudentProctoredExamAttempt(AuthenticatedAPIView):
             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_exam_attempt(attempt_id, status='error')
+                update_attempt_status(
+                    attempt['proctored_exam']['id'],
+                    attempt['user']['id'],
+                    ProctoredExamStudentAttemptStatus.error
+                )
 
             return Response(
                 data=attempt,
@@ -334,6 +337,12 @@ class StudentProctoredExamAttempt(AuthenticatedAPIView):
                     exam_id=attempt['proctored_exam']['id'],
                     user_id=request.user.id
                 )
+            elif action == 'submit':
+                exam_attempt_id = update_attempt_status(
+                    attempt['proctored_exam']['id'],
+                    request.user.id,
+                    ProctoredExamStudentAttemptStatus.submitted
+                )
             return Response({"exam_attempt_id": exam_attempt_id})
 
         except ProctoredBaseException, ex: