fix: VerificationAttempt.expiration_datetime field may be None.

This commit fixes a bug introduced by the new VerificationAttempt model. The expiration_datetime field is nullable. Other IDV models in the verify_student application have a default value for expiration_date, so they are typically not null, despite being nullable.

This commit updates code that queries this field to function correctly when the value of expiration_datetime is None. In this case, a None expiration_datetime indicates a non-expiring attempt.

lms/djangoapps/verify_student/models.py

@@ -1229,6 +1229,22 @@ def should_display_status_to_user(self):
         """When called, returns true or false based on the type of VerificationAttempt"""
         return not self.hide_status_from_user
 
+    def active_at_datetime(self, deadline):
+        """Check whether the verification was active at a particular datetime.
+
+        Arguments:
+            deadline (datetime): The date at which the verification was active
+                (created before and expiration datetime is after today).
+
+        Returns:
+            bool
+
+        """
+        return (
+            self.created_at <= deadline and
+            (self.expiration_datetime is None or self.expiration_datetime > now())
+        )
+
     @classmethod
     def retire_user(cls, user_id):
         """

lms/djangoapps/verify_student/services.py

@@ -173,13 +173,35 @@ def user_status(cls, user):
 
         verifications = cls.verifications_for_user(user)
 
+        # Get the first verification with an expiration date.
+        with_expiration_index = next(
+            (index for index, verification in enumerate(verifications) if verification.expiration_datetime),
+            None
+        )
+
         if verifications:
             attempt = verifications[0]
             for verification in verifications:
-                if verification.expiration_datetime > now() and verification.status == 'approved':
-                    # Always select the LATEST non-expired approved verification if there is such
-                    if attempt.status != 'approved' or (
-                        attempt.expiration_datetime < verification.expiration_datetime
+
+                # # A verification may not have an expiration date. If it doesn't, ignore it for the purpose of
+                # # determining the latest non-expired approved verification.
+                # if not verification.expiration_datetime:
+                #     continue
+
+                # If a verification has no expiration_datetime, it's implied that it never expires, so we should still
+                # consider verifications in the approved state that have no expiration date.
+                if (
+                    not verification.expiration_datetime or
+                        verification.expiration_datetime > now()
+                ) and verification.status == 'approved':
+                    # Always select the LATEST non-expired approved verification if there is such.
+                    if (
+                        attempt.status != 'approved' or
+                        (
+                            attempt.expiration_datetime and
+                            verification.expiration_datetime and
+                            attempt.expiration_datetime < verification.expiration_datetime
+                        )
                     ):
                         attempt = verification
 
@@ -188,7 +210,7 @@ def user_status(cls, user):
 
         user_status['should_display'] = attempt.should_display_status_to_user()
 
-        if attempt.expiration_datetime < now() and attempt.status == 'approved':
+        if attempt.expiration_datetime and attempt.expiration_datetime < now() and attempt.status == 'approved':
             if user_status['should_display']:
                 user_status['status'] = 'expired'
                 user_status['error'] = _("Your {platform_name} verification has expired.").format(

lms/djangoapps/verify_student/tests/__init__.py

@@ -9,7 +9,7 @@
 from django.utils.timezone import now
 
 from common.djangoapps.student.tests.factories import UserFactory
-from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
+from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationAttempt
 
 
 class TestVerificationBase(TestCase):
@@ -52,11 +52,14 @@ def verification_active_at_datetime(self, attempt):
 
         # Active immediately before expiration date
         expiration = attempt.expiration_datetime
-        before_expiration = expiration - timedelta(seconds=1)
-        assert attempt.active_at_datetime(before_expiration)
+        if expiration:
+            before_expiration = expiration - timedelta(seconds=1)
+            assert attempt.active_at_datetime(before_expiration)
 
         # Not active after the expiration date
-        attempt.expiration_date = now() - timedelta(days=1)
+        field = 'expiration_datetime' if isinstance(attempt, VerificationAttempt) else 'expiration_date'
+        setattr(attempt, field, now() - timedelta(days=1))
+        # attempt.expiration_date = now() - timedelta(days=1)
         attempt.save()
         assert not attempt.active_at_datetime(now())
 

lms/djangoapps/verify_student/tests/test_models.py

@@ -20,6 +20,7 @@
     PhotoVerification,
     SoftwareSecurePhotoVerification,
     SSOVerification,
+    VerificationAttempt,
     VerificationException
 )
 from lms.djangoapps.verify_student.tests import TestVerificationBase
@@ -437,3 +438,14 @@ def test_active_at_datetime(self):
         user = UserFactory.create()
         verification = ManualVerification.objects.create(user=user)
         self.verification_active_at_datetime(verification)
+
+
+class VerificationAttemptTest(TestVerificationBase):
+    """
+    Tests for the VerificationAttempt model
+    """
+
+    def test_active_at_datetime(self):
+        user = UserFactory.create()
+        attempt = VerificationAttempt.objects.create(user=user)
+        self.verification_active_at_datetime(attempt)

lms/djangoapps/verify_student/tests/test_services.py

@@ -2,6 +2,7 @@
 Tests for the service classes in verify_student.
 """
 
+import itertools
 from datetime import datetime, timedelta, timezone
 from unittest.mock import patch
 
@@ -273,6 +274,29 @@ def test_denied_software_secure_verification(self):
             }
             self.assertDictEqual(status, expected_status)
 
+    def test_approved_verification_attempt_verification(self):
+        with freeze_time('2015-01-02'):
+            # test for when Verification Attempt verification has been created
+            VerificationAttempt.objects.create(user=self.user, status='approved')
+            status = IDVerificationService.user_status(self.user)
+            expected_status = {'status': 'approved', 'error': '', 'should_display': True, 'verification_expiry': '',
+                               'status_date': datetime.now(utc)}
+            self.assertDictEqual(status, expected_status)
+
+    def test_denied_verification_attempt_verification(self):
+        with freeze_time('2015-2-02'):
+            # create denied photo verification for the user, make sure the denial
+            # is handled properly
+            VerificationAttempt.objects.create(
+                user=self.user, status='denied'
+            )
+            status = IDVerificationService.user_status(self.user)
+            expected_status = {
+                'status': 'must_reverify', 'error': '',
+                'should_display': True, 'verification_expiry': '', 'status_date': '',
+            }
+            self.assertDictEqual(status, expected_status)
+
     def test_approved_sso_verification(self):
         with freeze_time('2015-03-02'):
             # test for when sso verification has been created
@@ -303,22 +327,19 @@ def test_manual_verification(self):
                                'status_date': datetime.now(utc)}
             self.assertDictEqual(status, expected_status)
 
-    @ddt.data(
-        'submitted',
-        'denied',
-        'approved',
-        'created',
-        'ready',
-        'must_retry'
+    @ddt.idata(itertools.product(
+        [SoftwareSecurePhotoVerification, VerificationAttempt],
+        ['submitted', 'denied', 'approved', 'created', 'ready', 'must_retry'])
     )
-    def test_expiring_software_secure_verification(self, new_status):
+    def test_expiring_software_secure_verification(self, value):
+        verification_model, new_status = value
         with freeze_time('2015-07-11') as frozen_datetime:
             # create approved photo verification for the user
-            SoftwareSecurePhotoVerification.objects.create(user=self.user, status='approved')
+            verification_model.objects.create(user=self.user, status='approved')
             expiring_datetime = datetime.now(utc)
             frozen_datetime.move_to('2015-07-14')
             # create another according to status passed in.
-            SoftwareSecurePhotoVerification.objects.create(user=self.user, status=new_status)
+            verification_model.objects.create(user=self.user, status=new_status)
             status_date = expiring_datetime
             if new_status == 'approved':
                 status_date = datetime.now(utc)
@@ -327,13 +348,16 @@ def test_expiring_software_secure_verification(self, new_status):
             status = IDVerificationService.user_status(self.user)
             self.assertDictEqual(status, expected_status)
 
-    def test_expired_verification(self):
+    @ddt.data(SoftwareSecurePhotoVerification, VerificationAttempt)
+    def test_expired_verification(self, verification_model):
         with freeze_time('2015-07-11') as frozen_datetime:
-            # create approved photo verification for the user
-            SoftwareSecurePhotoVerification.objects.create(
+            key = 'expiration_datetime' if verification_model == VerificationAttempt else 'expiration_date'
+
+            # create approved verification for the user
+            verification_model.objects.create(
                 user=self.user,
                 status='approved',
-                expiration_date=now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])
+                **{key: now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])},
             )
             frozen_datetime.move_to('2016-07-11')
             expected_status = {
@@ -348,28 +372,28 @@ def test_expired_verification(self):
             status = IDVerificationService.user_status(self.user)
             self.assertDictEqual(status, expected_status)
 
-    @ddt.data(
-        'submitted',
-        'denied',
-        'approved',
-        'created',
-        'ready',
-        'must_retry'
+    @ddt.idata(itertools.product(
+        [SoftwareSecurePhotoVerification, VerificationAttempt],
+        ['submitted', 'denied', 'approved', 'created', 'ready', 'must_retry'])
     )
-    def test_reverify_after_expired(self, new_status):
+    def test_reverify_after_expired(self, value):
+        verification_model, new_status = value
         with freeze_time('2015-07-11') as frozen_datetime:
-            # create approved photo verification for the user
-            SoftwareSecurePhotoVerification.objects.create(
+            key = 'expiration_datetime' if verification_model == VerificationAttempt else 'expiration_date'
+
+            # create approved verification for the user
+            verification_model.objects.create(
                 user=self.user,
                 status='approved',
-                expiration_date=now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])
+                **{key: now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])},
             )
+
             frozen_datetime.move_to('2016-07-12')
             # create another according to status passed in.
-            SoftwareSecurePhotoVerification.objects.create(
+            verification_model.objects.create(
                 user=self.user,
                 status=new_status,
-                expiration_date=now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])
+                **{key: now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])},
             )
 
             check_status = new_status
@@ -390,9 +414,10 @@ def test_reverify_after_expired(self, new_status):
 
     @ddt.data(
         SSOVerification,
-        ManualVerification
+        ManualVerification,
+        VerificationAttempt
     )
-    def test_override_verification(self, verification_type):
+    def test_override_verification(self, verification_model):
         with freeze_time('2015-07-11') as frozen_datetime:
             # create approved photo verification for the user
             SoftwareSecurePhotoVerification.objects.create(
@@ -401,19 +426,27 @@ def test_override_verification(self, verification_type):
                 expiration_date=now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])
             )
             frozen_datetime.move_to('2015-07-14')
-            verification_type.objects.create(
+
+            key = 'expiration_datetime' if verification_model == VerificationAttempt else 'expiration_date'
+            verification_model.objects.create(
                 user=self.user,
                 status='approved',
-                expiration_date=now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])
+                **{key: now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])},
             )
             expected_status = {
-                'status': 'approved', 'error': '', 'should_display': False,
+                'status': 'approved', 'error': '',
+                'should_display': verification_model == VerificationAttempt,
                 'verification_expiry': '', 'status_date': now()
             }
             status = IDVerificationService.user_status(self.user)
             self.assertDictEqual(status, expected_status)
 
-    def test_denied_after_approved_verification(self):
+    @ddt.data(
+        SSOVerification,
+        ManualVerification,
+        VerificationAttempt
+    )
+    def test_denied_after_approved_verification(self, verification_model):
         with freeze_time('2015-07-11') as frozen_datetime:
             # create approved photo verification for the user
             SoftwareSecurePhotoVerification.objects.create(
@@ -423,10 +456,12 @@ def test_denied_after_approved_verification(self):
             )
             expected_date = now()
             frozen_datetime.move_to('2015-07-14')
-            SoftwareSecurePhotoVerification.objects.create(
+
+            key = 'expiration_datetime' if verification_model == VerificationAttempt else 'expiration_date'
+            verification_model.objects.create(
                 user=self.user,
                 status='denied',
-                expiration_date=now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])
+                **{key: now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])},
             )
             expected_status = {
                 'status': 'approved', 'error': '', 'should_display': True,