feat: Add VerificationAttempt model to IDVerificationService logic (#…

…35311)

the new VerificationAttempt model (#35304) will now be taken into account when determining a user's verification status.

lms/djangoapps/instructor_task/tests/test_tasks_helper.py

@@ -406,7 +406,7 @@ def test_query_counts(self):
 
         with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
             with check_mongo_calls(2):
-                with self.assertNumQueries(53):
+                with self.assertNumQueries(54):
                     CourseGradeReport.generate(None, None, course.id, {}, 'graded')
 
     def test_inactive_enrollments(self):

lms/djangoapps/verify_student/models.py

@@ -1215,6 +1215,11 @@ class VerificationAttempt(TimeStampedModel):
         blank=True,
     )
 
+    @property
+    def updated_at(self):
+        """Backwards compatibility with existing IDVerification models"""
+        return self.modified
+
     @classmethod
     def retire_user(cls, user_id):
         """

lms/djangoapps/verify_student/services.py

@@ -17,7 +17,7 @@
 from lms.djangoapps.verify_student.utils import is_verification_expiring_soon
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 
-from .models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification
+from .models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification, VerificationAttempt
 from .utils import most_recent_verification
 
 log = logging.getLogger(__name__)
@@ -75,7 +75,8 @@ def verifications_for_user(cls, user):
         Return a list of all verifications associated with the given user.
         """
         verifications = []
-        for verification in chain(SoftwareSecurePhotoVerification.objects.filter(user=user).order_by('-created_at'),
+        for verification in chain(VerificationAttempt.objects.filter(user=user).order_by('-created'),
+                                  SoftwareSecurePhotoVerification.objects.filter(user=user).order_by('-created_at'),
                                   SSOVerification.objects.filter(user=user).order_by('-created_at'),
                                   ManualVerification.objects.filter(user=user).order_by('-created_at')):
             verifications.append(verification)
@@ -92,6 +93,11 @@ def get_verified_user_ids(cls, users):
             'created_at__gt': now() - timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])
         }
         return chain(
+            VerificationAttempt.objects.filter(**{
+                'user__in': users,
+                'status': 'approved',
+                'created__gt': now() - timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])
+            }).values_list('user_id', flat=True),
             SoftwareSecurePhotoVerification.objects.filter(**filter_kwargs).values_list('user_id', flat=True),
             SSOVerification.objects.filter(**filter_kwargs).values_list('user_id', flat=True),
             ManualVerification.objects.filter(**filter_kwargs).values_list('user_id', flat=True)
@@ -117,11 +123,14 @@ def get_expiration_datetime(cls, user, statuses):
             'status__in': statuses,
         }
 
+        id_verifications = VerificationAttempt.objects.filter(**filter_kwargs)
         photo_id_verifications = SoftwareSecurePhotoVerification.objects.filter(**filter_kwargs)
         sso_id_verifications = SSOVerification.objects.filter(**filter_kwargs)
         manual_id_verifications = ManualVerification.objects.filter(**filter_kwargs)
 
-        attempt = most_recent_verification((photo_id_verifications, sso_id_verifications, manual_id_verifications))
+        attempt = most_recent_verification(
+            (photo_id_verifications, sso_id_verifications, manual_id_verifications, id_verifications)
+        )
         return attempt and attempt.expiration_datetime
 
     @classmethod
@@ -242,8 +251,18 @@ def get_verification_details_by_id(cls, attempt_id):
         """
         Returns a verification attempt object by attempt_id
         If the verification object cannot be found, returns None
+
+        This method does not take into account verifications stored in the
+        VerificationAttempt model used for pluggable IDV implementations.
+
+        As part of the work to implement pluggable IDV, this method's use
+        will be deprecated: https://openedx.atlassian.net/browse/OSPR-1011
         """
         verification = None
+
+        # This does not look at the VerificationAttempt model since the provided id would become
+        # ambiguous between tables. The verification models in this list all inherit from the same
+        # base class and share the same id space.
         verification_models = [
             SoftwareSecurePhotoVerification,
             SSOVerification,

lms/djangoapps/verify_student/tests/test_services.py

@@ -2,8 +2,8 @@
 Tests for the service classes in verify_student.
 """
 
-from datetime import datetime, timedelta, timezone
 import itertools
+from datetime import datetime, timedelta, timezone
 from random import randint
 from unittest.mock import patch
 
@@ -16,10 +16,16 @@
 from pytz import utc
 
 from common.djangoapps.student.tests.factories import UserFactory
-from lms.djangoapps.verify_student.models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification
+from lms.djangoapps.verify_student.models import (
+    ManualVerification,
+    SoftwareSecurePhotoVerification,
+    SSOVerification,
+    VerificationAttempt
+)
 from lms.djangoapps.verify_student.services import IDVerificationService
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
-from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore.tests.django_utils import \
+    ModuleStoreTestCase  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.tests.factories import CourseFactory  # lint-amnesty, pylint: disable=wrong-import-order
 
 FAKE_SETTINGS = {
@@ -34,12 +40,15 @@ class TestIDVerificationService(ModuleStoreTestCase):
     Tests for IDVerificationService.
     """
 
-    def test_user_is_verified(self):
+    @ddt.data(
+        SoftwareSecurePhotoVerification, VerificationAttempt
+    )
+    def test_user_is_verified(self, verification_model):
         """
         Test to make sure we correctly answer whether a user has been verified.
         """
         user = UserFactory.create()
-        attempt = SoftwareSecurePhotoVerification(user=user)
+        attempt = verification_model(user=user)
         attempt.save()
 
         # If it's any of these, they're not verified...
@@ -49,16 +58,24 @@ def test_user_is_verified(self):
             assert not IDVerificationService.user_is_verified(user), status
 
         attempt.status = "approved"
+        if verification_model == VerificationAttempt:
+            attempt.expiration_datetime = now() + timedelta(days=19)
+        else:
+            attempt.expiration_date = now() + timedelta(days=19)
         attempt.save()
+
         assert IDVerificationService.user_is_verified(user), attempt.status
 
-    def test_user_has_valid_or_pending(self):
+    @ddt.data(
+        SoftwareSecurePhotoVerification, VerificationAttempt
+    )
+    def test_user_has_valid_or_pending(self, verification_model):
         """
         Determine whether we have to prompt this user to verify, or if they've
         already at least initiated a verification submission.
         """
         user = UserFactory.create()
-        attempt = SoftwareSecurePhotoVerification(user=user)
+        attempt = verification_model(user=user)
 
         # If it's any of these statuses, they don't have anything outstanding
         for status in ["created", "ready", "denied"]:
@@ -70,6 +87,10 @@ def test_user_has_valid_or_pending(self):
         # -- must_retry, and submitted both count until we hear otherwise
         for status in ["submitted", "must_retry", "approved"]:
             attempt.status = status
+            if verification_model == VerificationAttempt:
+                attempt.expiration_datetime = now() + timedelta(days=19)
+            else:
+                attempt.expiration_date = now() + timedelta(days=19)
             attempt.save()
             assert IDVerificationService.user_has_valid_or_pending(user), status
 
@@ -102,18 +123,22 @@ def test_get_verified_user_ids(self):
         user_a = UserFactory.create()
         user_b = UserFactory.create()
         user_c = UserFactory.create()
+        user_d = UserFactory.create()
         user_unverified = UserFactory.create()
         user_denied = UserFactory.create()
+        user_denied_b = UserFactory.create()
 
         SoftwareSecurePhotoVerification.objects.create(user=user_a, status='approved')
         ManualVerification.objects.create(user=user_b, status='approved')
         SSOVerification.objects.create(user=user_c, status='approved')
+        VerificationAttempt.objects.create(user=user_d, status='approved')
         SSOVerification.objects.create(user=user_denied, status='denied')
+        VerificationAttempt.objects.create(user=user_denied_b, status='denied')
 
         verified_user_ids = set(IDVerificationService.get_verified_user_ids([
-            user_a, user_b, user_c, user_unverified, user_denied
+            user_a, user_b, user_c, user_d, user_unverified, user_denied
         ]))
-        expected_user_ids = {user_a.id, user_b.id, user_c.id}
+        expected_user_ids = {user_a.id, user_b.id, user_c.id, user_d.id}
 
         assert expected_user_ids == verified_user_ids
 
@@ -158,6 +183,23 @@ def test_get_expiration_datetime(self):
         expiration_datetime = IDVerificationService.get_expiration_datetime(user_a, ['approved'])
         assert expiration_datetime == newer_record.expiration_datetime
 
+    def test_get_expiration_datetime_mixed_models(self):
+        """
+        Test that the latest expiration datetime is returned if there are both instances of
+        IDVerification models and VerificationAttempt models
+        """
+        user = UserFactory.create()
+
+        SoftwareSecurePhotoVerification.objects.create(
+            user=user, status='approved', expiration_date=datetime(2021, 11, 12, 0, 0, tzinfo=timezone.utc)
+        )
+        newest = VerificationAttempt.objects.create(
+            user=user, status='approved', expiration_datetime=datetime(2022, 1, 12, 0, 0, tzinfo=timezone.utc)
+        )
+
+        expiration_datetime = IDVerificationService.get_expiration_datetime(user, ['approved'])
+        assert expiration_datetime == newest.expiration_datetime
+
     @ddt.data(
         {'status': 'denied', 'error_msg': '[{"generalReasons": ["Name mismatch"]}]'},
         {'status': 'approved', 'error_msg': ''},