From 3035ba70f49da63d47d3157df5c725cb00c3a049 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Tue, 13 Aug 2024 15:58:33 -0400 Subject: [PATCH] feat: add new VerificationAttempt model to idv logic --- lms/djangoapps/verify_student/models.py | 10 ++++ lms/djangoapps/verify_student/services.py | 18 +++++-- .../verify_student/tests/test_services.py | 54 +++++++++++++++---- 3 files changed, 70 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 28d11fa7312..e503e350534 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -1215,3 +1215,13 @@ class VerificationAttempt(TimeStampedModel): blank=True, db_index=True, ) + + @property + def expiration_datetime(self): + """Backwards compatibility with existing IDVerification models""" + return self.expiration_date + + @property + def updated_at(self): + """Backwards compatibility with existing IDVerification models""" + return self.modified diff --git a/lms/djangoapps/verify_student/services.py b/lms/djangoapps/verify_student/services.py index bdfa31fee6d..5b88025719c 100644 --- a/lms/djangoapps/verify_student/services.py +++ b/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 @@ -244,6 +253,9 @@ def get_verification_details_by_id(cls, attempt_id): If the verification object cannot be found, returns None """ verification = None + + # TODO: Not updated for new model since we can't query multiple tables + # by id. verification_models = [ SoftwareSecurePhotoVerification, SSOVerification, diff --git a/lms/djangoapps/verify_student/tests/test_services.py b/lms/djangoapps/verify_student/tests/test_services.py index 56f388b7c97..19c85d9149a 100644 --- a/lms/djangoapps/verify_student/tests/test_services.py +++ b/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,21 @@ def test_user_is_verified(self): assert not IDVerificationService.user_is_verified(user), status attempt.status = "approved" + 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 +84,7 @@ 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 + attempt.expiration_date = now() + timedelta(days=19) attempt.save() assert IDVerificationService.user_has_valid_or_pending(user), status @@ -102,18 +117,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 +177,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_date=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': ''},