Merge pull request #31 from edx/ashultz0/verified-name-status

Switch verified name from boolean to status enumeration
edx · Aug 17, 2021 · 8f7e9ac · 8f7e9ac
2 parents d3bdb30 + 4295b59
commit 8f7e9ac
Show file tree

Hide file tree

Showing 11 changed files with 140 additions and 55 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -14,6 +14,10 @@ Change Log
 Unreleased
 ~~~~~~~~~~
 
+[0.6.0] - 2021-08-11
+~~~~~~~~~~~~~~~~~~~~
+* Add name verification status field, replacing single is_verified boolean.
+
 [0.5.0] - 2021-08-11
 ~~~~~~~~~~~~~~~~~~~~
 * Add API method and endpoint to return a complete list of the user's

diff --git a/edx_name_affirmation/__init__.py b/edx_name_affirmation/__init__.py
@@ -2,6 +2,6 @@
 Django app housing name affirmation logic.
 """
 
-__version__ = '0.5.0'
+__version__ = '0.6.0'
 
 default_app_config = 'edx_name_affirmation.apps.EdxNameAffirmationConfig'  # pylint: disable=invalid-name
diff --git a/edx_name_affirmation/api.py b/edx_name_affirmation/api.py
@@ -11,13 +11,14 @@
     VerifiedNameMultipleAttemptIds
 )
 from edx_name_affirmation.models import VerifiedName, VerifiedNameConfig
+from edx_name_affirmation.statuses import VerifiedNameStatus
 
 log = logging.getLogger(__name__)
 
 
 def create_verified_name(
     user, verified_name, profile_name, verification_attempt_id=None,
-    proctored_exam_attempt_id=None, is_verified=False,
+    proctored_exam_attempt_id=None, status=VerifiedNameStatus.PENDING,
 ):
     """
     Create a new `VerifiedName` for the given user.
@@ -47,9 +48,9 @@ def create_verified_name(
             'external attempt IDs were given. Only one may be used. '
             'verification_attempt_id={verification_attempt_id}, '
             'proctored_exam_attempt_id={proctored_exam_attempt_id}, '
-            'is_verified={is_verified}'.format(
+            'status={status}'.format(
                 user_id=user.id, verification_attempt_id=verification_attempt_id,
-                proctored_exam_attempt_id=proctored_exam_attempt_id, is_verified=is_verified,
+                proctored_exam_attempt_id=proctored_exam_attempt_id, status=status,
             )
         )
         raise VerifiedNameMultipleAttemptIds(err_msg)
@@ -60,16 +61,16 @@ def create_verified_name(
         profile_name=profile_name,
         verification_attempt_id=verification_attempt_id,
         proctored_exam_attempt_id=proctored_exam_attempt_id,
-        is_verified=is_verified,
+        status=status,
     )
 
     log_msg = (
         'VerifiedName created for user_id={user_id}. '
         'verification_attempt_id={verification_attempt_id}, '
         'proctored_exam_attempt_id={proctored_exam_attempt_id}, '
-        'is_verified={is_verified}'.format(
+        'status={status}'.format(
             user_id=user.id, verification_attempt_id=verification_attempt_id,
-            proctored_exam_attempt_id=proctored_exam_attempt_id, is_verified=is_verified,
+            proctored_exam_attempt_id=proctored_exam_attempt_id, status=status,
         )
     )
     log.info(log_msg)
@@ -89,7 +90,7 @@ def get_verified_name(user, is_verified=False):
     verified_name_qs = VerifiedName.objects.filter(user=user).order_by('-created')
 
     if is_verified:
-        return verified_name_qs.filter(is_verified=True).first()
+        return verified_name_qs.filter(status=VerifiedNameStatus.APPROVED.value).first()
 
     return verified_name_qs.first()
 
@@ -159,16 +160,16 @@ def update_verification_attempt_id(user, verification_attempt_id):
     log.info(log_msg)
 
 
-def update_is_verified_status(
-    user, is_verified, verification_attempt_id=None, proctored_exam_attempt_id=None
+def update_verified_name_status(
+    user, status, verification_attempt_id=None, proctored_exam_attempt_id=None
 ):
     """
     Update the status of a VerifiedName using the linked ID verification or exam attempt ID. Only one
     of these should be specified.
 
     Arguments:
         * user (User object)
-        * is_verified (bool)
+        * status (Verified Name Status)
         * verification_attempt_id (int)
         * proctored_exam_attempt_id (int)
     """
@@ -177,7 +178,7 @@ def update_is_verified_status(
     if verification_attempt_id:
         if proctored_exam_attempt_id:
             err_msg = (
-                'Attempted to update the is_verified status for a VerifiedName, but two different '
+                'Attempted to update the status for a VerifiedName, but two different '
                 'attempt IDs were given. verification_attempt_id={verification_attempt_id}, '
                 'proctored_exam_attempt_id={proctored_exam_attempt_id}'.format(
                     verification_attempt_id=verification_attempt_id,
@@ -190,7 +191,7 @@ def update_is_verified_status(
         filters['proctored_exam_attempt_id'] = proctored_exam_attempt_id
     else:
         err_msg = (
-            'Attempted to update the is_verified status for a VerifiedName, but no '
+            'Attempted to update the status for a VerifiedName, but no '
             'verification_attempt_id or proctored_exam_attempt_id was given.'
         )
         raise VerifiedNameAttemptIdNotGiven(err_msg)
@@ -199,24 +200,24 @@ def update_is_verified_status(
 
     if not verified_name_obj:
         err_msg = (
-            'Attempted to update is_verified={is_verified} for a VerifiedName, but one does '
+            'Attempted to update status={status} for a VerifiedName, but one does '
             'not exist for the given attempt ID. verification_attempt_id={verification_attempt_id}, '
             'proctored_exam_attempt_id={proctored_exam_attempt_id}'.format(
-                is_verified=is_verified,
+                status=status,
                 verification_attempt_id=verification_attempt_id,
                 proctored_exam_attempt_id=proctored_exam_attempt_id,
             )
         )
         raise VerifiedNameDoesNotExist(err_msg)
 
-    verified_name_obj.is_verified = is_verified
+    verified_name_obj.status = status.value
     verified_name_obj.save()
 
     log_msg = (
-        'Updated is_verified={is_verified} for VerifiedName belonging to user_id={user_id}. '
+        'Updated status={status} for VerifiedName belonging to user_id={user_id}. '
         'verification_attempt_id={verification_attempt_id}, '
         'proctored_exam_attempt_id={proctored_exam_attempt_id}'.format(
-            is_verified=is_verified,
+            status=status,
             user_id=verified_name_obj.user.id,
             verification_attempt_id=verification_attempt_id,
             proctored_exam_attempt_id=proctored_exam_attempt_id,

diff --git a/edx_name_affirmation/migrations/0003_verifiedname_status.py b/edx_name_affirmation/migrations/0003_verifiedname_status.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.2.24 on 2021-08-13 12:46
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('edx_name_affirmation', '0002_verifiednameconfig'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='verifiedname',
+            name='status',
+            field=models.CharField(choices=[('pending', 'pending'), ('submitted', 'submitted'), ('approved', 'approved'), ('denied', 'denied')], default='pending', max_length=32),
+        ),
+    ]
diff --git a/edx_name_affirmation/migrations/0004_auto_20210816_0958.py b/edx_name_affirmation/migrations/0004_auto_20210816_0958.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.2.24 on 2021-08-16 09:58
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('edx_name_affirmation', '0003_verifiedname_status'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='verifiedname',
+            name='is_verified',
+            field=models.BooleanField(default=False, null=True),
+        ),
+    ]
diff --git a/edx_name_affirmation/models.py b/edx_name_affirmation/models.py
@@ -8,6 +8,8 @@
 from django.contrib.auth import get_user_model
 from django.db import models
 
+from edx_name_affirmation.statuses import VerifiedNameStatus
+
 User = get_user_model()
 
 
@@ -30,7 +32,13 @@ class VerifiedName(TimeStampedModel):
     verification_attempt_id = models.PositiveIntegerField(null=True)
     proctored_exam_attempt_id = models.PositiveIntegerField(null=True)
 
-    is_verified = models.BooleanField(default=False)
+    status = models.CharField(
+        max_length=32,
+        choices=[(st.value, st.value) for st in VerifiedNameStatus],
+        default=VerifiedNameStatus.PENDING.value,
+    )
+    # is_verified is being removed
+    is_verified = models.BooleanField(default=False, null=True)
 
     class Meta:
         """ Meta class for this Django model """

diff --git a/edx_name_affirmation/serializers.py b/edx_name_affirmation/serializers.py
@@ -17,7 +17,7 @@ class VerifiedNameSerializer(serializers.ModelSerializer):
     profile_name = serializers.CharField(required=True)
     verification_attempt_id = serializers.IntegerField(required=False, allow_null=True)
     proctored_exam_attempt_id = serializers.IntegerField(required=False, allow_null=True)
-    is_verified = serializers.BooleanField(required=False, allow_null=True)
+    status = serializers.CharField(required=False, allow_null=True)
 
     class Meta:
         """
@@ -27,7 +27,7 @@ class Meta:
 
         fields = (
             "created", "username", "verified_name", "profile_name", "verification_attempt_id",
-            "proctored_exam_attempt_id", "is_verified"
+            "proctored_exam_attempt_id", "status"
         )
 
 

diff --git a/edx_name_affirmation/statuses.py b/edx_name_affirmation/statuses.py
@@ -0,0 +1,33 @@
+"""
+Statuses for edx_name_affirmation.
+"""
+
+from enum import Enum
+
+
+class VerifiedNameStatus(str, Enum):
+    """
+    Possible states for the verified name.
+
+    Pending: the verified name has been created
+
+    Submitted: the verified name has been submitted to a verification authority
+
+    Approved, Denied: resulting states from that authority
+
+    This is the status of the verified name attempt, which is related to
+    but separate from the status of the verifying process such as IDV or proctoring.
+    Status changes in the verifying processes are usually more fine grained.
+
+    For example when proctoring changes from ready to start to started the verified
+    name is still pending. Once proctoring is actually submitted the verified name
+    can be considered submitted.
+
+    The expected lifecycle is pending -> submitted -> approved/denied.
+
+    .. no_pii: This model has no PII.
+    """
+    PENDING = "pending"
+    SUBMITTED = "submitted"
+    APPROVED = "approved"
+    DENIED = "denied"
diff --git a/edx_name_affirmation/tests/test_api.py b/edx_name_affirmation/tests/test_api.py
@@ -14,8 +14,8 @@
     get_verified_name,
     get_verified_name_history,
     should_use_verified_name_for_certs,
-    update_is_verified_status,
-    update_verification_attempt_id
+    update_verification_attempt_id,
+    update_verified_name_status
 )
 from edx_name_affirmation.exceptions import (
     VerifiedNameAttemptIdNotGiven,
@@ -24,6 +24,7 @@
     VerifiedNameMultipleAttemptIds
 )
 from edx_name_affirmation.models import VerifiedName, VerifiedNameConfig
+from edx_name_affirmation.statuses import VerifiedNameStatus
 
 User = get_user_model()
 
@@ -58,26 +59,26 @@ def test_create_verified_name_defaults(self):
         self.assertEqual(verified_name_obj.user, self.user)
         self.assertIsNone(verified_name_obj.verification_attempt_id)
         self.assertIsNone(verified_name_obj.proctored_exam_attempt_id)
-        self.assertFalse(verified_name_obj.is_verified)
+        self.assertEqual(verified_name_obj.status, VerifiedNameStatus.PENDING.value)
 
     @ddt.data(
-        (123, None, False),
-        (None, 456, True),
+        (123, None, VerifiedNameStatus.APPROVED),
+        (None, 456, VerifiedNameStatus.SUBMITTED),
     )
     @ddt.unpack
     def test_create_verified_name_with_optional_arguments(
-        self, verification_attempt_id, proctored_exam_attempt_id, is_verified,
+        self, verification_attempt_id, proctored_exam_attempt_id, status,
     ):
         """
         Test to create a verified name with optional arguments supplied.
         """
         verified_name_obj = self._create_verified_name(
-            verification_attempt_id, proctored_exam_attempt_id, is_verified,
+            verification_attempt_id, proctored_exam_attempt_id, status,
         )
 
         self.assertEqual(verified_name_obj.verification_attempt_id, verification_attempt_id)
         self.assertEqual(verified_name_obj.proctored_exam_attempt_id, proctored_exam_attempt_id)
-        self.assertEqual(verified_name_obj.is_verified, is_verified)
+        self.assertEqual(verified_name_obj.status, status.value)
 
     def test_create_verified_name_two_ids(self):
         """
@@ -130,10 +131,10 @@ def test_get_verified_name_most_recent(self):
 
     def test_get_verified_name_only_verified(self):
         """
-        Test that VerifiedName entries with is_verified=False are ignored if is_verified
+        Test that VerifiedName entries with status != approved are ignored if is_verified
         argument is set to True.
         """
-        self._create_verified_name(is_verified=True)
+        self._create_verified_name(status=VerifiedNameStatus.APPROVED)
         create_verified_name(self.user, 'unverified name', 'unverified profile name')
 
         verified_name_obj = get_verified_name(self.user, True)
@@ -225,27 +226,27 @@ def test_update_is_verified_status(
         Test that VerifiedName status can be updated with a given attempt ID.
         """
         self._create_verified_name(verification_attempt_id, proctored_exam_attempt_id)
-        update_is_verified_status(
-            self.user, True, verification_attempt_id, proctored_exam_attempt_id,
+        update_verified_name_status(
+            self.user, VerifiedNameStatus.DENIED, verification_attempt_id, proctored_exam_attempt_id,
         )
         verified_name_obj = get_verified_name(self.user)
-        self.assertTrue(verified_name_obj.is_verified)
+        self.assertEqual(VerifiedNameStatus.DENIED.value, verified_name_obj.status)
 
     def test_update_is_verified_no_attempt_id(self):
         """
         Test that `update_is_verified_by_attempt_id` will raise an exception with no attempt
         ID given.
         """
         with self.assertRaises(VerifiedNameAttemptIdNotGiven):
-            update_is_verified_status(self.user, True)
+            update_verified_name_status(self.user, True)
 
     def test_update_is_verified_multiple_attempt_ids(self):
         """
         Test that `update_is_verified_by_attempt_id` will raise an exception with multiple attempt
         IDs given.
         """
         with self.assertRaises(VerifiedNameMultipleAttemptIds):
-            update_is_verified_status(
+            update_verified_name_status(
                 self.user, True, self.VERIFICATION_ATTEMPT_ID, self.PROCTORED_EXAM_ATTEMPT_ID,
             )
 
@@ -255,17 +256,17 @@ def test_update_is_verified_does_not_exist(self):
         not exist for the attempt ID given.
         """
         with self.assertRaises(VerifiedNameDoesNotExist):
-            update_is_verified_status(self.user, True, self.VERIFICATION_ATTEMPT_ID)
+            update_verified_name_status(self.user, True, self.VERIFICATION_ATTEMPT_ID)
 
     def _create_verified_name(
-        self, verification_attempt_id=None, proctored_exam_attempt_id=None, is_verified=False,
+        self, verification_attempt_id=None, proctored_exam_attempt_id=None, status=VerifiedNameStatus.PENDING,
     ):
         """
         Util to create and return a VerifiedName with default names.
         """
         create_verified_name(
             self.user, self.VERIFIED_NAME, self.PROFILE_NAME, verification_attempt_id,
-            proctored_exam_attempt_id, is_verified
+            proctored_exam_attempt_id, status
         )
         return get_verified_name(self.user)