From a3871cda2281fbf0aceee7a1ef998a9b3b3b65d3 Mon Sep 17 00:00:00 2001 From: Isaac Lee <124631592+ilee2u@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:16:29 -0400 Subject: [PATCH] API functions for the new generic VerificationAttempt model in the verify_student app (#35338) * feat: add VerificationAttempt model to verify_student application This commits adds a VerificationAttempt model to store implementation and provider agnostic information about identity verification attempts in the platform. * feat: add api for VerificationAttempt model * fix: error handling for update - added tests accordingly - also took care of some nits * chore: lint * chore: lint for equals spaces * feat: using generic update function instead - can now update name, status, and exp. date on generic attempts - changed tests accordingly - a few nits * chore: fix docstring args * fix: corrected status validation - reverted to old status validation method - fixed tests accordingly * fix: datetime, status, and annotation fixes - expiration_datetime can be updated to None - VerificationAttemptStatus is now StrEnum - Added type annotations for api functions --------- Co-authored-by: michaelroytman --- lms/djangoapps/verify_student/api.py | 92 +++++++++++ lms/djangoapps/verify_student/exceptions.py | 4 + .../commands/approve_id_verifications.py | 1 - lms/djangoapps/verify_student/models.py | 8 +- lms/djangoapps/verify_student/statuses.py | 11 +- .../verify_student/tests/test_api.py | 147 +++++++++++++++++- 6 files changed, 251 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/verify_student/api.py b/lms/djangoapps/verify_student/api.py index c974fa0c8e5f..f61b90d682ff 100644 --- a/lms/djangoapps/verify_student/api.py +++ b/lms/djangoapps/verify_student/api.py @@ -1,12 +1,25 @@ """ API module. """ +import logging + from django.conf import settings +from django.contrib.auth import get_user_model from django.utils.translation import gettext as _ +from datetime import datetime +from typing import Optional + from lms.djangoapps.verify_student.emails import send_verification_approved_email +from lms.djangoapps.verify_student.exceptions import VerificationAttemptInvalidStatus +from lms.djangoapps.verify_student.models import VerificationAttempt +from lms.djangoapps.verify_student.statuses import VerificationAttemptStatus from lms.djangoapps.verify_student.tasks import send_verification_status_email +log = logging.getLogger(__name__) + +User = get_user_model() + def send_approval_email(attempt): """ @@ -33,3 +46,82 @@ def send_approval_email(attempt): else: email_context = {'user': attempt.user, 'expiration_datetime': expiration_datetime.strftime("%m/%d/%Y")} send_verification_approved_email(context=email_context) + + +def create_verification_attempt(user: User, name: str, status: str, expiration_datetime: Optional[datetime] = None): + """ + Create a verification attempt. + + This method is intended to be used by IDV implementation plugins to create VerificationAttempt instances. + + Args: + user (User): the user (usually a learner) performing the verification attempt + name (string): the name being ID verified + status (string): the initial status of the verification attempt + expiration_datetime (datetime, optional): When the verification attempt expires. Defaults to None. + + Returns: + id (int): The id of the created VerificationAttempt instance + """ + verification_attempt = VerificationAttempt.objects.create( + user=user, + name=name, + status=status, + expiration_datetime=expiration_datetime, + ) + + return verification_attempt.id + + +def update_verification_attempt( + attempt_id: int, + name: Optional[str] = None, + status: Optional[str] = None, + expiration_datetime: Optional[datetime] = None +): + """ + Update a verification attempt. + + This method is intended to be used by IDV implementation plugins to update VerificationAttempt instances. + + Arguments: + * attempt_id (int): the verification attempt id of the attempt to update + * name (string, optional): the new name being ID verified + * status (string, optional): the new status of the verification attempt + * expiration_datetime (datetime, optional): The new expiration date and time + + Returns: + * None + """ + try: + attempt = VerificationAttempt.objects.get(id=attempt_id) + except VerificationAttempt.DoesNotExist: + log.error( + f'VerificationAttempt with id {attempt_id} was not found ' + f'when updating the attempt to status={status}', + ) + raise + + if name is not None: + attempt.name = name + + if status is not None: + attempt.status = status + + status_list = list(VerificationAttemptStatus) + if status not in status_list: + log.error( + 'Attempted to call update_verification_attempt called with invalid status: %(status)s. ' + 'Status must be one of: %(status_list)s', + { + 'status': status, + 'status_list': VerificationAttempt.STATUS_CHOICES, + }, + ) + raise VerificationAttemptInvalidStatus + + # NOTE: Generally, we only set the expiration date from the time that an IDV attempt is marked approved, + # so we allow expiration_datetime to = None for other status updates (e.g. pending). + attempt.expiration_datetime = expiration_datetime + + attempt.save() diff --git a/lms/djangoapps/verify_student/exceptions.py b/lms/djangoapps/verify_student/exceptions.py index 59e7d5623f05..d13e52d3e737 100644 --- a/lms/djangoapps/verify_student/exceptions.py +++ b/lms/djangoapps/verify_student/exceptions.py @@ -5,3 +5,7 @@ class WindowExpiredException(Exception): pass + + +class VerificationAttemptInvalidStatus(Exception): + pass diff --git a/lms/djangoapps/verify_student/management/commands/approve_id_verifications.py b/lms/djangoapps/verify_student/management/commands/approve_id_verifications.py index b87b2eee4559..3a08ede0aaf6 100644 --- a/lms/djangoapps/verify_student/management/commands/approve_id_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/approve_id_verifications.py @@ -8,7 +8,6 @@ import time from pprint import pformat -from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.management.base import BaseCommand, CommandError from lms.djangoapps.verify_student.api import send_approval_email diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 903d80bf9245..72b52d403849 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -1203,10 +1203,10 @@ class VerificationAttempt(TimeStampedModel): name = models.CharField(blank=True, max_length=255) STATUS_CHOICES = [ - VerificationAttemptStatus.created, - VerificationAttemptStatus.pending, - VerificationAttemptStatus.approved, - VerificationAttemptStatus.denied, + VerificationAttemptStatus.CREATED, + VerificationAttemptStatus.PENDING, + VerificationAttemptStatus.APPROVED, + VerificationAttemptStatus.DENIED, ] status = models.CharField(max_length=64, choices=[(status, status) for status in STATUS_CHOICES]) diff --git a/lms/djangoapps/verify_student/statuses.py b/lms/djangoapps/verify_student/statuses.py index b55a9042e0f6..41ef381cfe06 100644 --- a/lms/djangoapps/verify_student/statuses.py +++ b/lms/djangoapps/verify_student/statuses.py @@ -1,21 +1,22 @@ """ Status enums for verify_student. """ +from enum import StrEnum, auto -class VerificationAttemptStatus: +class VerificationAttemptStatus(StrEnum): """This class describes valid statuses for a verification attempt to be in.""" # This is the initial state of a verification attempt, before a learner has started IDV. - created = "created" + CREATED = auto() # A verification attempt is pending when it has been started but has not yet been completed. - pending = "pending" + PENDING = auto() # A verification attempt is approved when it has been approved by some mechanism (e.g. automatic review, manual # review, etc). - approved = "approved" + APPROVED = auto() # A verification attempt is denied when it has been denied by some mechanism (e.g. automatic review, manual review, # etc). - denied = "denied" + DENIED = auto() diff --git a/lms/djangoapps/verify_student/tests/test_api.py b/lms/djangoapps/verify_student/tests/test_api.py index acdebaa70c1c..747c76f82b61 100644 --- a/lms/djangoapps/verify_student/tests/test_api.py +++ b/lms/djangoapps/verify_student/tests/test_api.py @@ -3,14 +3,21 @@ """ from unittest.mock import patch +from datetime import datetime, timezone import ddt from django.conf import settings from django.core import mail from django.test import TestCase from common.djangoapps.student.tests.factories import UserFactory -from lms.djangoapps.verify_student.api import send_approval_email -from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification +from lms.djangoapps.verify_student.api import ( + create_verification_attempt, + send_approval_email, + update_verification_attempt, +) +from lms.djangoapps.verify_student.exceptions import VerificationAttemptInvalidStatus +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationAttempt +from lms.djangoapps.verify_student.statuses import VerificationAttemptStatus @ddt.ddt @@ -18,6 +25,7 @@ class TestSendApprovalEmail(TestCase): """ Test cases for the send_approval_email API method. """ + def setUp(self): super().setUp() @@ -41,3 +49,138 @@ def test_send_approval(self, use_ace): with patch.dict(settings.VERIFY_STUDENT, {'USE_DJANGO_MAIL': use_ace}): send_approval_email(self.attempt) self._assert_verification_approved_email(self.attempt.expiration_datetime) + + +@ddt.ddt +class CreateVerificationAttempt(TestCase): + """ + Test cases for the create_verification_attempt API method. + """ + + def setUp(self): + super().setUp() + + self.user = UserFactory.create() + self.attempt = VerificationAttempt( + user=self.user, + name='Tester McTest', + status=VerificationAttemptStatus.CREATED, + expiration_datetime=datetime(2024, 12, 31, tzinfo=timezone.utc) + ) + self.attempt.save() + + def test_create_verification_attempt(self): + expected_id = 2 + self.assertEqual( + create_verification_attempt( + user=self.user, + name='Tester McTest', + status=VerificationAttemptStatus.CREATED, + expiration_datetime=datetime(2024, 12, 31, tzinfo=timezone.utc) + ), + expected_id + ) + verification_attempt = VerificationAttempt.objects.get(id=expected_id) + + self.assertEqual(verification_attempt.user, self.user) + self.assertEqual(verification_attempt.name, 'Tester McTest') + self.assertEqual(verification_attempt.status, VerificationAttemptStatus.CREATED) + self.assertEqual(verification_attempt.expiration_datetime, datetime(2024, 12, 31, tzinfo=timezone.utc)) + + def test_create_verification_attempt_no_expiration_datetime(self): + expected_id = 2 + self.assertEqual( + create_verification_attempt( + user=self.user, + name='Tester McTest', + status=VerificationAttemptStatus.CREATED, + ), + expected_id + ) + verification_attempt = VerificationAttempt.objects.get(id=expected_id) + + self.assertEqual(verification_attempt.user, self.user) + self.assertEqual(verification_attempt.name, 'Tester McTest') + self.assertEqual(verification_attempt.status, VerificationAttemptStatus.CREATED) + self.assertEqual(verification_attempt.expiration_datetime, None) + + +@ddt.ddt +class UpdateVerificationAttempt(TestCase): + """ + Test cases for the update_verification_attempt API method. + """ + + def setUp(self): + super().setUp() + + self.user = UserFactory.create() + self.attempt = VerificationAttempt( + user=self.user, + name='Tester McTest', + status=VerificationAttemptStatus.CREATED, + expiration_datetime=datetime(2024, 12, 31, tzinfo=timezone.utc) + ) + self.attempt.save() + + @ddt.data( + ('Tester McTest', VerificationAttemptStatus.PENDING, datetime(2024, 12, 31, tzinfo=timezone.utc)), + ('Tester McTest2', VerificationAttemptStatus.APPROVED, datetime(2025, 12, 31, tzinfo=timezone.utc)), + ('Tester McTest3', VerificationAttemptStatus.DENIED, datetime(2026, 12, 31, tzinfo=timezone.utc)), + ) + @ddt.unpack + def test_update_verification_attempt(self, name, status, expiration_datetime): + update_verification_attempt( + attempt_id=self.attempt.id, + name=name, + status=status, + expiration_datetime=expiration_datetime, + ) + + verification_attempt = VerificationAttempt.objects.get(id=self.attempt.id) + + # Values should change as a result of this update. + self.assertEqual(verification_attempt.user, self.user) + self.assertEqual(verification_attempt.name, name) + self.assertEqual(verification_attempt.status, status) + self.assertEqual(verification_attempt.expiration_datetime, expiration_datetime) + + def test_update_verification_attempt_none_values(self): + update_verification_attempt( + attempt_id=self.attempt.id, + name=None, + status=None, + expiration_datetime=None, + ) + + verification_attempt = VerificationAttempt.objects.get(id=self.attempt.id) + + # Values should not change as a result of the values passed in being None, except for expiration_datetime. + self.assertEqual(verification_attempt.user, self.user) + self.assertEqual(verification_attempt.name, self.attempt.name) + self.assertEqual(verification_attempt.status, self.attempt.status) + self.assertEqual(verification_attempt.expiration_datetime, None) + + def test_update_verification_attempt_not_found(self): + self.assertRaises( + VerificationAttempt.DoesNotExist, + update_verification_attempt, + attempt_id=999999, + status=VerificationAttemptStatus.APPROVED, + ) + + @ddt.data( + 'completed', + 'failed', + 'submitted', + 'expired', + ) + def test_update_verification_attempt_invalid(self, status): + self.assertRaises( + VerificationAttemptInvalidStatus, + update_verification_attempt, + attempt_id=self.attempt.id, + name=None, + status=status, + expiration_datetime=None, + )