fix: error handling for update

- added tests accordingly
- also took care of some nits

lms/djangoapps/verify_student/api.py

@@ -1,13 +1,19 @@
 """
 API module.
 """
+import logging
+
 from django.conf import settings
 from django.utils.translation import gettext as _
 
 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__)
+
 
 def send_approval_email(attempt):
     """
@@ -40,14 +46,16 @@ def create_verification_attempt(user, name, status, expiration_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 verfication attempt
-        name (string): the name of the user
+        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:
-        VerificationAttempt (VerificationAttempt): The created VerificationAttempt instance
+        id (int): The id of the created VerificationAttempt instance
     """
     verification_attempt = VerificationAttempt.objects.create(
         user=user,
@@ -56,20 +64,40 @@ def create_verification_attempt(user, name, status, expiration_datetime=None):
         expiration_datetime=expiration_datetime,
     )
 
+
     return verification_attempt.id
 
 
-def update_verification_status(attempt_id, status):
+def update_verification_attempt_status(attempt_id, status):
     """
-    Update the VerificationAttempt status.
+    Update the status of a verification attempt.
+
+    This method is intended to be used by IDV implementation plugins to update VerificationAttempt instances.
 
     Arguments:
-        * id (str): the verification attempt id
+        * id (str): the verification attempt id of the attempt to update
         * status (str): the new status
 
     Returns:
         * None
     """
-    attempt = VerificationAttempt.objects.get(id=attempt_id)
+    status_list = [attr for attr in dir(VerificationAttemptStatus) if not attr.startswith('__')]
+
+    if status not in status_list:
+        log.error(
+            f'update_verification_attempt_status called with invalid status. '
+            f'Status must be one of: {status_list}',
+        )
+        raise VerificationAttemptInvalidStatus
+
+    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 VerificationAttempt.DoesNotExist  # pylint: disable=raise-missing-from
+
     attempt.status = status
     attempt.save()

lms/djangoapps/verify_student/exceptions.py

@@ -5,3 +5,6 @@
 
 class WindowExpiredException(Exception):
     pass
+
+class VerificationAttemptInvalidStatus(Exception):
+    pass

lms/djangoapps/verify_student/tests/test_api.py

@@ -13,8 +13,9 @@
 from lms.djangoapps.verify_student.api import (
     create_verification_attempt,
     send_approval_email,
-    update_verification_status,
+    update_verification_attempt_status,
 )
+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
 
@@ -53,7 +54,7 @@ def test_send_approval(self, use_ace):
 @ddt.ddt
 class CreateVerificationAttempt(TestCase):
     """
-    Test cases for the send_approval_email API method.
+    Test cases for the create_verification_attempt API method.
     """
 
     def setUp(self):
@@ -86,11 +87,28 @@ def test_create_verification_attempt(self):
         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 UpdateVerificationStatus(TestCase):
+class UpdateVerificationAttemptStatus(TestCase):
     """
-    Test cases for the update_verification_status API method.
+    Test cases for the update_verification_attempt_status API method.
     """
 
     def setUp(self):
@@ -110,8 +128,8 @@ def setUp(self):
         VerificationAttemptStatus.approved,
         VerificationAttemptStatus.denied,
     )
-    def test_update_verification_status(self, to_status):
-        update_verification_status(attempt_id=self.attempt.id, status=to_status)
+    def test_update_verification_attempt_status(self, to_status):
+        update_verification_attempt_status(attempt_id=self.attempt.id, status=to_status)
 
         verification_attempt = VerificationAttempt.objects.get(id=self.attempt.id)
 
@@ -122,3 +140,27 @@ def test_update_verification_status(self, to_status):
 
         # This field's value should change as a result of this update.
         self.assertEqual(verification_attempt.status, to_status)
+
+    # These are statuses used in edx-name-affirmation's VerifiedName model and persona-integration's unique
+    # VerificationAttempt model, and not by verify_student's VerificationAttempt model.
+    @ddt.data(
+        'completed',
+        'failed',
+        'submitted',
+        'expired',
+    )
+    def test_update_verification_attempt_status_invalid(self, to_status):
+        self.assertRaises(
+            VerificationAttemptInvalidStatus,
+            update_verification_attempt_status,
+            attempt_id=self.attempt.id,
+            status=to_status,
+        )
+
+    def test_update_verification_attempt_status_not_found(self):
+        self.assertRaises(
+            VerificationAttempt.DoesNotExist,
+            update_verification_attempt_status,
+            attempt_id=999999,
+            status=VerificationAttemptStatus.approved,
+        )