[feat]: MST-817 allow exam attempt to be resumable through the new is…

…_resumable model field. Expose the is_resumable boolean field from the ProctoredExamStudentAttempt table through the proctoring API and restful web API. Update the UI to use the is_resumable property on the attempt model instead of 'error' status. This change will allow exams to be resumable even if the attempt is in reviewed status like 'verified' or 'rejected' (#839)
openedx · May 17, 2021 · 8b721e2 · 8b721e2
1 parent 98f9bf2
commit 8b721e2
Show file tree

Hide file tree

Showing 14 changed files with 246 additions and 10,393 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -14,6 +14,11 @@ Change Log
 Unreleased
 ~~~~~~~~~~
 
+[3.9.1] - 2021-05-17
+~~~~~~~~~~~~~~~~~~~~
+* Add the backend model field is_resumable to the ProctoredExamStudentAttempt model.
+* Expose the is_resumable property to the UI so users can resume exam attempts when that property is set
+
 [3.9.0] - 2021-05-07
 ~~~~~~~~~~~~~~~~~~~~
 * Add API endpoint which provides sequence exam data with current active attempt.

diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py
@@ -3,6 +3,6 @@
 """
 
 # Be sure to update the version number in edx_proctoring/package.json
-__version__ = '3.9.0'
+__version__ = '3.9.1'
 
 default_app_config = 'edx_proctoring.apps.EdxProctoringConfig'  # pylint: disable=invalid-name
diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py
@@ -966,14 +966,15 @@ def mark_exam_attempt_as_resumed(attempt_id):
     return update_attempt_status(attempt_id, ProctoredExamStudentAttemptStatus.resumed)
 
 
-def is_state_transition_legal(from_status, to_status):
+def is_state_transition_legal(from_status, to_status, attempt_obj):
     """
     Determine and return as a boolean whether a proctored exam attempt state transition
     from from_status to to_status is an allowed state transition.
 
     Arguments:
         from_status: original status of a proctored exam attempt
         to_status: future status of a proctored exam attempt
+        attempt_obj: the actual student exam attempt
     """
     in_completed_status = ProctoredExamStudentAttemptStatus.is_completed_status(from_status)
     to_incompleted_status = ProctoredExamStudentAttemptStatus.is_incomplete_status(to_status)
@@ -982,9 +983,9 @@ def is_state_transition_legal(from_status, to_status):
     # if a re-attempt is desired then the current attempt must be deleted
     if in_completed_status and to_incompleted_status:
         return False
-    # only allow a state transition to the ready_to_resume state from an error state
+    # only allow a state transition to the ready_to_resume state when the attempt is resumable
     if (to_status == ProctoredExamStudentAttemptStatus.ready_to_resume and
-            from_status != ProctoredExamStudentAttemptStatus.error):
+            not attempt_obj.is_resumable):
         return False
     # only allowed state transition to the resumed state from ready_to_resume (or resumed).
     # this accounts for cases where the previous attempt was marked as resumed, but a new
@@ -1036,6 +1037,28 @@ def can_update_credit_grades_and_email(attempts, to_status):
     return False
 
 
+def _is_attempt_resumable(attempt_obj, to_status):
+    """
+    Based on the attempt object and the status it's transitioning to,
+    return whether the attempt should be resumable, or not
+    """
+    status_to_reset_resumability = (
+        ProctoredExamStudentAttemptStatus.submitted,
+        ProctoredExamStudentAttemptStatus.resumed,
+        ProctoredExamStudentAttemptStatus.ready_to_resume,
+    )
+    if to_status in status_to_reset_resumability:
+        # Make sure we have resumable to be false in conditions where the
+        # attempt is either in the resume process, or it's successfully submitted
+        return False
+    if to_status == ProctoredExamStudentAttemptStatus.error:
+        # Only when the transition to status is "Error", the attempt is resumable
+        return True
+
+    # Otherwise, maintain resumability on the attempt
+    return attempt_obj.is_resumable
+
+
 # pylint: disable=inconsistent-return-statements
 def update_attempt_status(attempt_id, to_status,
                           raise_if_not_found=True, cascade_effects=True, timeout_timestamp=None,
@@ -1077,7 +1100,7 @@ def update_attempt_status(attempt_id, to_status,
     exam = get_exam_by_id(exam_id)
     backend = get_backend_provider(exam)
 
-    if not is_state_transition_legal(from_status, to_status):
+    if not is_state_transition_legal(from_status, to_status, exam_attempt_obj):
         illegal_status_transition_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 '
@@ -1112,6 +1135,9 @@ def update_attempt_status(attempt_id, to_status,
         # when the exam has been completed
         exam_attempt_obj.completed_at = datetime.now(pytz.UTC)
 
+    # Update the is_resumable flag of the attempt based on the status transition
+    exam_attempt_obj.is_resumable = _is_attempt_resumable(exam_attempt_obj, to_status)
+
     exam_attempt_obj.save()
 
     all_attempts = get_user_attempts_by_exam_id(user_id, exam_id)

diff --git a/edx_proctoring/migrations/0014_add_is_resumable_to_proctoredexamstudentattempt.py b/edx_proctoring/migrations/0014_add_is_resumable_to_proctoredexamstudentattempt.py
@@ -0,0 +1,23 @@
+# Generated by Django 2.2.17 on 2021-05-06 13:49
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('edx_proctoring', '0013_proctoredexamsoftwaresecurereview_is_active_attempt'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='proctoredexamstudentattempt',
+            name='is_resumable',
+            field=models.BooleanField(default=False, verbose_name='Is Resumable'),
+        ),
+        migrations.AddField(
+            model_name='proctoredexamstudentattempthistory',
+            name='is_resumable',
+            field=models.BooleanField(default=False),
+        ),
+    ]
diff --git a/edx_proctoring/models.py b/edx_proctoring/models.py
@@ -421,6 +421,11 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
     # be saved in order to allow the learner to resume
     time_remaining_seconds = models.IntegerField(null=True)
 
+    # marks whether the attempt is able to be resumed by user
+    # Only those attempts which had an error state before, but
+    # has not yet marked submitted is resumable.
+    is_resumable = models.BooleanField(default=False, verbose_name=ugettext_noop("Is Resumable"))
+
     class Meta:
         """ Meta class for this Django model """
         db_table = 'proctoring_proctoredexamstudentattempt'
@@ -510,6 +515,11 @@ class ProctoredExamStudentAttemptHistory(TimeStampedModel):
     last_poll_timestamp = models.DateTimeField(null=True)
     last_poll_ipaddr = models.CharField(max_length=32, null=True)
 
+    # Marks whether the attempt at this current state is able to be resumed by user
+    # Only those attempts which had an error state before, but
+    # has not yet marked submitted is resumable.
+    is_resumable = models.BooleanField(default=False)
+
     @classmethod
     def get_exam_attempt_by_code(cls, attempt_code):
         """

diff --git a/edx_proctoring/serializers.py b/edx_proctoring/serializers.py
@@ -97,7 +97,7 @@ class Meta:
             "external_id", "status", "proctored_exam", "allowed_time_limit_mins",
             "attempt_code", "is_sample_attempt", "taking_as_proctored", "last_poll_timestamp",
             "last_poll_ipaddr", "review_policy_id", "student_name", "is_status_acknowledged",
-            "time_remaining_seconds"
+            "time_remaining_seconds", "is_resumable"
         )
 
 

diff --git a/edx_proctoring/static/proctoring/spec/proctored_exam_attempt_spec.js b/edx_proctoring/static/proctoring/spec/proctored_exam_attempt_spec.js
@@ -38,6 +38,7 @@ describe('ProctoredExamAttemptView', function() {
                     modified: '2015-08-10T09:15:45Z',
                     started_at: '2015-08-10T09:15:45Z',
                     status: status,
+                    is_resumable: false,
                     taking_as_proctored: true,
                     proctored_exam: {
                         content_id: 'i4x://edX/DemoX/sequential/9f5e9b018a244ea38e5d157e0019e60c',
@@ -91,9 +92,11 @@ describe('ProctoredExamAttemptView', function() {
         );
     }
 
-    function getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson(status, isPracticeExam) {
+    function getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson(status, isPracticeExam, isResumable) {
         // eslint-disable-next-line no-param-reassign
         isPracticeExam = typeof isPracticeExam !== 'undefined' ? isPracticeExam : false;
+        // eslint-disable-next-line no-param-reassign
+        isResumable = typeof isResumable !== 'undefined' ? isResumable : false;
         return (
             [{
                 attempt_url: '/api/edx_proctoring/v1/proctored_exam/attempt/course_id/edX/DemoX/Demo_Course',
@@ -116,6 +119,7 @@ describe('ProctoredExamAttemptView', function() {
                     modified: '2015-08-10T09:15:45Z',
                     started_at: '2015-08-10T09:15:45Z',
                     status: status,
+                    is_resumable: isResumable,
                     taking_as_proctored: true,
                     proctored_exam: {
                         content_id: 'i4x://edX/DemoX/sequential/9f5e9b018a244ea38e5d157e0019e60c',
@@ -147,6 +151,7 @@ describe('ProctoredExamAttemptView', function() {
                             modified: '2015-08-10T09:15:45Z',
                             started_at: '2015-08-10T09:15:45Z',
                             status: status,
+                            is_resumable: isResumable,
                             taking_as_proctored: true,
                             proctored_exam: {
                                 content_id: 'i4x://edX/DemoX/sequential/9f5e9b018a244ea38e5d157e0019e60c',
@@ -178,6 +183,7 @@ describe('ProctoredExamAttemptView', function() {
                             modified: '2015-08-10T09:15:45Z',
                             started_at: '2015-08-10T09:15:45Z',
                             status: 'resumed',
+                            is_resumable: false,
                             taking_as_proctored: true,
                             proctored_exam: {
                                 content_id: 'i4x://edX/DemoX/sequential/9f5e9b018a244ea38e5d157e0019e60c',
@@ -292,7 +298,7 @@ describe('ProctoredExamAttemptView', function() {
         '<td>' +
         '<% if (proctored_exam_attempt.status){ %>' +
         '<% if (' +
-        'proctored_exam_attempt.status == "error" &&' +
+        'proctored_exam_attempt.is_resumable &&' +
         '!proctored_exam_attempt.proctored_exam.is_practice_exam' +
         ') { %>' +
         '<div class="wrapper-action-more">' +
@@ -528,7 +534,7 @@ describe('ProctoredExamAttemptView', function() {
                 {
                     'Content-Type': 'application/json'
                 },
-                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error'))
+                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error', false, true))
             ]
         );
         this.proctored_exam_attempt_view = new edx.instructor_dashboard.proctoring.ProctoredExamAttemptView();
@@ -606,7 +612,31 @@ describe('ProctoredExamAttemptView', function() {
                 {
                     'Content-Type': 'application/json'
                 },
-                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error', true))
+                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error', true, true))
+            ]
+        );
+        this.proctored_exam_attempt_view = new edx.instructor_dashboard.proctoring.ProctoredExamAttemptView();
+
+        // Process all requests so far
+        this.server.respond();
+        this.server.respond();
+
+        expect(this.proctored_exam_attempt_view.$el.find('tbody').html()).toContain('testuser1');
+        expect(this.proctored_exam_attempt_view.$el.find('tbody').html()).toContain('Normal Exam');
+        expect(this.proctored_exam_attempt_view.$el.find('tbody.accordion-panel').html()).toContain('Error');
+
+        expect(this.proctored_exam_attempt_view.$el.find('button.action').html()).toHaveLength(0);
+        expect(this.proctored_exam_attempt_view.$el.find('.actions-dropdown').html()).toHaveLength(0);
+    });
+
+    it('should not display actions dropdown for exam attempts not resumable', function() {
+        this.server.respondWith('GET', '/api/edx_proctoring/v1/proctored_exam/attempt/grouped/course_id/test_course_id',
+            [
+                200,
+                {
+                    'Content-Type': 'application/json'
+                },
+                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error', true, false))
             ]
         );
         this.proctored_exam_attempt_view = new edx.instructor_dashboard.proctoring.ProctoredExamAttemptView();
@@ -632,7 +662,7 @@ describe('ProctoredExamAttemptView', function() {
                 {
                     'Content-Type': 'application/json'
                 },
-                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error', false))
+                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error', false, true))
             ]
         );
         this.proctored_exam_attempt_view = new edx.instructor_dashboard.proctoring.ProctoredExamAttemptView();
@@ -664,7 +694,7 @@ describe('ProctoredExamAttemptView', function() {
                 {
                     'Content-Type': 'application/json'
                 },
-                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error', false))
+                JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('error', false, true))
             ]
         );
         this.proctored_exam_attempt_view = new edx.instructor_dashboard.proctoring.ProctoredExamAttemptView();

diff --git a/...proctoring/static/proctoring/templates/student-proctored-exam-attempts-grouped.underscore b/...proctoring/static/proctoring/templates/student-proctored-exam-attempts-grouped.underscore
@@ -200,7 +200,7 @@
                             <td>
                                 <% if (proctored_exam_attempt.status){ %>
                                     <% if (
-                                        proctored_exam_attempt.status == "error" &&
+                                        proctored_exam_attempt.is_resumable &&
                                         !proctored_exam_attempt.proctored_exam.is_practice_exam
                                     ) { %>
                                         <div class="wrapper-action-more">

diff --git a/edx_proctoring/tests/test_api.py b/edx_proctoring/tests/test_api.py
@@ -2021,7 +2021,13 @@ def test_update_exam_attempt_ready_to_resume_invalid_transition(self, initial_st
                 ProctoredExamStudentAttemptStatus.ready_to_resume
             )
 
-    def test_update_exam_attempt_ready_to_resume(self):
+    @ddt.data(
+        ProctoredExamStudentAttemptStatus.error,
+        ProctoredExamStudentAttemptStatus.verified,
+        ProctoredExamStudentAttemptStatus.second_review_required,
+        ProctoredExamStudentAttemptStatus.rejected
+    )
+    def test_update_exam_attempt_ready_to_resume(self, resumable_status):
         """
         Assert that an attempted transition of a proctored exam attempt from an error state
         to a ready_to_resume state completes successfully and does not raise a
@@ -2032,13 +2038,19 @@ def test_update_exam_attempt_ready_to_resume(self):
         attempt = get_exam_attempt_by_id(exam_attempt.id)
         self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.started)
 
+        # First we have to transition to error state to make the exam attempt resumable
         update_attempt_status(
             exam_attempt.id,
             ProctoredExamStudentAttemptStatus.error
         )
 
+        update_attempt_status(
+            exam_attempt.id,
+            resumable_status
+        )
+
         attempt = get_exam_attempt_by_id(exam_attempt.id)
-        self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.error)
+        self.assertEqual(attempt['status'], resumable_status)
 
         update_attempt_status(
             exam_attempt.id,
@@ -2092,6 +2104,58 @@ def test_update_exam_attempt_resumed(self, from_status):
         attempt = get_exam_attempt_by_id(exam_attempt.id)
         self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.resumed)
 
+    @ddt.data(
+        (
+            ProctoredExamStudentAttemptStatus.started,
+            ProctoredExamStudentAttemptStatus.error,
+            True
+        ),
+        (
+            ProctoredExamStudentAttemptStatus.started,
+            ProctoredExamStudentAttemptStatus.ready_to_submit,
+            False
+        ),
+        (
+            ProctoredExamStudentAttemptStatus.error,
+            ProctoredExamStudentAttemptStatus.ready_to_resume,
+            False
+        ),
+        (
+            ProctoredExamStudentAttemptStatus.ready_to_resume,
+            ProctoredExamStudentAttemptStatus.resumed,
+            False
+        ),
+        (
+            ProctoredExamStudentAttemptStatus.error,
+            ProctoredExamStudentAttemptStatus.verified,
+            True
+        ),
+        (
+            ProctoredExamStudentAttemptStatus.error,
+            ProctoredExamStudentAttemptStatus.second_review_required,
+            True
+        ),
+        (
+            ProctoredExamStudentAttemptStatus.error,
+            ProctoredExamStudentAttemptStatus.rejected,
+            True
+        ),
+    )
+    @ddt.unpack
+    def test_exam_attempt_is_resumable(self, from_status, to_status, expected_is_resumable):
+        exam_attempt = self._create_exam_attempt(self.proctored_exam_id, status=from_status)
+        if from_status == ProctoredExamStudentAttemptStatus.error:
+            self.assertTrue(exam_attempt.is_resumable)
+        else:
+            self.assertFalse(exam_attempt.is_resumable)
+
+        update_attempt_status(
+            exam_attempt.id,
+            to_status,
+        )
+        attempt = get_exam_attempt_by_id(exam_attempt.id)
+        self.assertEqual(attempt['is_resumable'], expected_is_resumable)
+
     def test_requirement_status_order(self):
         """
         Make sure that we get a correct ordered list of all statuses sorted in the correct