Merge pull request #412 from edx/noraiz/EDUCATOR-2314

Proctored exam allowed time calculation clean up

edx_proctoring/api.py

@@ -553,25 +553,14 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
 
             raise StudentExamAttemptAlreadyExistsException(err_msg)
 
-    allowed_time_limit_mins = exam['time_limit_mins']
-
-    # add in the allowed additional time
-    allowance_extra_mins = ProctoredExamStudentAllowance.get_additional_time_granted(exam_id, user_id)
-    if allowance_extra_mins:
-        allowed_time_limit_mins += allowance_extra_mins
-
-    allowed_time_limit_mins, is_exam_past_due_date = _calculate_allowed_mins(
-        exam['due_date'],
-        allowed_time_limit_mins
-    )
-
+    allowed_time_limit_mins = _calculate_allowed_mins(exam, user_id)
     attempt_code = unicode(uuid.uuid4()).upper()
 
     external_id = None
     review_policy = ProctoredExamReviewPolicy.get_review_policy_for_exam(exam_id)
     review_policy_exception = ProctoredExamStudentAllowance.get_review_policy_exception(exam_id, user_id)
 
-    if not is_exam_past_due_date and taking_as_proctored:
+    if not has_due_date_passed(exam['due_date']) and taking_as_proctored:
         scheme = 'https' if getattr(settings, 'HTTPS', 'on') == 'on' else 'http'
         callback_url = '{scheme}://{hostname}{path}'.format(
             scheme=scheme,
@@ -626,7 +615,6 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
         exam_id,
         user_id,
         '',  # student name is TBD
-        allowed_time_limit_mins,
         attempt_code,
         taking_as_proctored,
         exam['is_practice_exam'],
@@ -640,12 +628,10 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
     log_msg = (
         'Created exam attempt ({attempt_id}) for exam_id {exam_id} for '
         'user_id {user_id} with taking as proctored = {taking_as_proctored} '
-        'with allowed time limit minutes of {allowed_time_limit_mins}. '
         'Attempt_code {attempt_code} was generated which has a '
         'external_id of {external_id}'.format(
             attempt_id=attempt.id, exam_id=exam_id, user_id=user_id,
             taking_as_proctored=taking_as_proctored,
-            allowed_time_limit_mins=allowed_time_limit_mins,
             attempt_code=attempt_code,
             external_id=external_id
         )
@@ -772,7 +758,6 @@ def update_attempt_status(exam_id, user_id, to_status,
     from_status = exam_attempt_obj.status
     exam = get_exam_by_id(exam_id)
 
-    #
     # 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
     #
@@ -811,13 +796,15 @@ def update_attempt_status(exam_id, user_id, to_status,
     exam_attempt_obj.status = to_status
 
     # if we have transitioned to started and haven't set our
-    # started_at timestamp, do so now
+    # started_at timestamp and calculate allowed minutes, do so now
     add_start_time = (
         to_status == ProctoredExamStudentAttemptStatus.started and
         not exam_attempt_obj.started_at
     )
     if add_start_time:
         exam_attempt_obj.started_at = datetime.now(pytz.UTC)
+        exam_attempt_obj.allowed_time_limit_mins = _calculate_allowed_mins(exam, exam_attempt_obj.user_id)
+
     elif treat_timeout_as_submitted:
         exam_attempt_obj.completed_at = timeout_timestamp
     elif to_status == ProctoredExamStudentAttemptStatus.submitted:
@@ -1603,24 +1590,12 @@ def _get_timed_exam_view(exam, context, exam_id, user_id, course_id):
 
     if student_view_template:
         template = loader.get_template(student_view_template)
-
-        allowed_time_limit_mins = attempt['allowed_time_limit_mins'] if attempt else None
+        allowed_time_limit_mins = attempt.get('allowed_time_limit_mins', None) if attempt else None
 
         if not allowed_time_limit_mins:
-            # no existing attempt, so compute the user's allowed
+            # no existing attempt or user has not started exam yet, so compute the user's allowed
             # time limit, including any accommodations
-
-            allowed_time_limit_mins = exam['time_limit_mins']
-            allowance_extra_mins = ProctoredExamStudentAllowance.get_additional_time_granted(exam_id, user_id)
-
-            if allowance_extra_mins:
-                allowed_time_limit_mins += int(allowance_extra_mins)
-
-            # apply any cut off times according to due dates
-            allowed_time_limit_mins, _ = _calculate_allowed_mins(
-                exam['due_date'],
-                allowed_time_limit_mins
-            )
+            allowed_time_limit_mins = _calculate_allowed_mins(exam, user_id)
 
         total_time = humanized_time(allowed_time_limit_mins)
 
@@ -1651,25 +1626,26 @@ def _get_timed_exam_view(exam, context, exam_id, user_id, course_id):
         return template.render(context)
 
 
-def _calculate_allowed_mins(due_datetime, allowed_mins):
+def _calculate_allowed_mins(exam, user_id):
     """
-    Returns the allowed minutes w.r.t due date and has due date pass
+    Returns the allowed minutes w.r.t due date
     """
+    due_datetime = exam['due_date']
+    allowed_time_limit_mins = exam['time_limit_mins']
 
-    current_datetime = datetime.now(pytz.UTC)
-    is_exam_past_due_date = False
-    actual_allowed_mins = allowed_mins
+    # add in the allowed additional time
+    allowance_extra_mins = ProctoredExamStudentAllowance.get_additional_time_granted(exam.get('id'), user_id)
+    if allowance_extra_mins:
+        allowed_time_limit_mins += allowance_extra_mins
 
     if due_datetime:
-        if has_due_date_passed(due_datetime):
-            is_exam_past_due_date = True
-        elif current_datetime + timedelta(minutes=allowed_mins) > due_datetime:
-
+        current_datetime = datetime.now(pytz.UTC)
+        if current_datetime + timedelta(minutes=allowed_time_limit_mins) > due_datetime:
             # e.g current_datetime=09:00, due_datetime=10:00 and allowed_mins=120(2hours)
             # then allowed_mins should be 60(1hour)
+            allowed_time_limit_mins = int((due_datetime - current_datetime).total_seconds() / 60)
 
-            actual_allowed_mins = int((due_datetime - current_datetime).total_seconds() / 60)
-    return actual_allowed_mins, is_exam_past_due_date
+    return allowed_time_limit_mins
 
 
 def _get_proctored_exam_context(exam, attempt, course_id, is_practice_exam=False):

edx_proctoring/migrations/0006_allowed_time_limit_mins.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('edx_proctoring', '0005_proctoredexam_hide_after_due'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='proctoredexamstudentattempt',
+            name='allowed_time_limit_mins',
+            field=models.IntegerField(null=True),
+        ),
+        migrations.AlterField(
+            model_name='proctoredexamstudentattempthistory',
+            name='allowed_time_limit_mins',
+            field=models.IntegerField(null=True),
+        ),
+    ]

edx_proctoring/models.py

@@ -463,7 +463,7 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
     external_id = models.CharField(max_length=255, null=True, db_index=True)
 
     # this is the time limit allowed to the student
-    allowed_time_limit_mins = models.IntegerField()
+    allowed_time_limit_mins = models.IntegerField(null=True)
 
     # what is the status of this attempt
     status = models.CharField(max_length=64)
@@ -494,19 +494,17 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
         unique_together = (('user', 'proctored_exam'),)
 
     @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,
+    def create_exam_attempt(cls, exam_id, user_id, student_name, attempt_code,
+                            taking_as_proctored, is_sample_attempt, external_id,
                             review_policy_id=None):
         """
         Create a new exam attempt entry for a given exam_id and
         user_id.
         """
-
         return cls.objects.create(
             proctored_exam_id=exam_id,
             user_id=user_id,
             student_name=student_name,
-            allowed_time_limit_mins=allowed_time_limit_mins,
             attempt_code=attempt_code,
             taking_as_proctored=taking_as_proctored,
             is_sample_attempt=is_sample_attempt,
@@ -547,7 +545,7 @@ class ProctoredExamStudentAttemptHistory(TimeStampedModel):
     external_id = models.CharField(max_length=255, null=True, db_index=True)
 
     # this is the time limit allowed to the student
-    allowed_time_limit_mins = models.IntegerField()
+    allowed_time_limit_mins = models.IntegerField(null=True)
 
     # what is the status of this attempt
     status = models.CharField(max_length=64)

edx_proctoring/tests/test_api.py

@@ -446,25 +446,26 @@ class ProctoredExamApiTests(ProctoredExamTestCase):
         remove_allowance_for_user(student_allowance.proctored_exam.id, self.user_id, self.key)
         self.assertEqual(len(ProctoredExamStudentAllowance.objects.filter()), 0)
 
-    def test_create_exam_attempt_with_due_datetime(self):
+    def test_exam_attempt_with_due_datetime(self):
         """
-        Create the exam attempt with due date
+        Test the exam attempt with due date
         """
 
         due_date = datetime.now(pytz.UTC) + timedelta(days=1)
 
         # exam is created with due datetime > current_datetime and due_datetime < current_datetime + allowed_mins
         exam_id = self._create_exam_with_due_time(due_date=due_date)
+        attempt_id = create_exam_attempt(exam_id, self.user_id)
 
         # due_date is exactly after 24 hours, our exam's allowed minutes are 21
         # student will get full allowed minutes if student will start exam within next 23 hours and 39 minutes
         # otherwise allowed minutes = due_datetime - exam_attempt_datetime
-        # so if students arrives after 23 hours and 45 minutes later then he will get only 15 minutes
-
+        # so if students starts exam after 23 hours and 45 minutes later then he will get only 15 minutes
         minutes_before_past_due_date = 15
         reset_time = due_date - timedelta(minutes=minutes_before_past_due_date)
+
         with freeze_time(reset_time):
-            attempt_id = create_exam_attempt(exam_id, self.user_id)
+            __ = start_exam_attempt(exam_id, self.user_id)
             attempt = get_exam_attempt_by_id(attempt_id)
             self.assertLessEqual(minutes_before_past_due_date - 1, attempt['allowed_time_limit_mins'])
             self.assertLessEqual(attempt['allowed_time_limit_mins'], minutes_before_past_due_date)
@@ -506,6 +507,7 @@ class ProctoredExamApiTests(ProctoredExamTestCase):
             str(allowed_extra_time)
         )
         attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id)
+        start_exam_attempt(self.proctored_exam_id, self.user_id)
         self.assertGreater(attempt_id, 0)
 
         attempt = get_exam_attempt_by_id(attempt_id)

edx_proctoring/tests/test_models.py

@@ -282,7 +282,7 @@ class ProctoredExamStudentAttemptTests(LoggedInTestCase):
         # create number of exam attempts
         for i in range(90):
             ProctoredExamStudentAttempt.create_exam_attempt(
-                proctored_exam.id, i, 'test_name{0}'.format(i), i + 1,
+                proctored_exam.id, i, 'test_name{0}'.format(i),
                 'test_attempt_code{0}'.format(i), True, False, 'test_external_id{0}'.format(i)
             )
 
@@ -314,7 +314,6 @@ class ProctoredExamStudentAttemptTests(LoggedInTestCase):
             proctored_exam.id,
             self.user.id,
             'test_name{0}'.format(self.user.id),
-            self.user.id + 1,
             'test_attempt_code{0}'.format(self.user.id),
             True,
             False,

edx_proctoring/tests/test_views.py

@@ -35,6 +35,7 @@ from edx_proctoring.api import (
     _calculate_allowed_mins
 )
 
+from edx_proctoring.serializers import ProctoredExamSerializer
 from edx_proctoring.backends.tests.test_review_payload import create_test_review_payload
 from edx_proctoring.backends.tests.test_software_secure import mock_response_content
 from edx_proctoring.runtime import set_runtime_service, get_runtime_service
@@ -700,7 +701,11 @@ class TestStudentProctoredExamAttempt(LoggedInTestCase):
             time_limit_mins=1800,
             due_date=datetime.now(pytz.UTC) + timedelta(minutes=expected_total_minutes),
         )
-        total_minutes, __ = _calculate_allowed_mins(proctored_exam.due_date, proctored_exam.time_limit_mins)
+        # _calculate_allowed_mins expects serialized object
+        serialized_exam_object = ProctoredExamSerializer(proctored_exam)
+        serialized_exam_object = serialized_exam_object.data
+
+        total_minutes = _calculate_allowed_mins(serialized_exam_object, self.user.id)
 
         # Check that timer has > 24 hours
         self.assertGreater(total_minutes / 60, 24)
@@ -722,7 +727,7 @@ class TestStudentProctoredExamAttempt(LoggedInTestCase):
             time_limit_mins=90
         )
         attempt = ProctoredExamStudentAttempt.create_exam_attempt(
-            proctored_exam.id, self.user.id, 'test_user', 1,
+            proctored_exam.id, self.user.id, 'test_user',
             'test_attempt_code', True, False, 'test_external_id'
         )
         attempt.status = ProctoredExamStudentAttemptStatus.ready_to_start
@@ -1379,7 +1384,7 @@ class TestStudentProctoredExamAttempt(LoggedInTestCase):
         # create number of exam attempts
         for i in range(90):
             ProctoredExamStudentAttempt.create_exam_attempt(
-                proctored_exam.id, i, 'test_name{0}'.format(i), i + 1,
+                proctored_exam.id, i, 'test_name{0}'.format(i),
                 'test_attempt_code{0}'.format(i), True, False, 'test_external_id{0}'.format(i)
             )