diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6dab65f..86e2b15 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,7 @@ Change Log Unreleased ~~~~~~~~~~ * Add platform verification id field to the VerifiedName model +* Integrate platform verification id into app [2.4.0] - 2024-04-23 ~~~~~~~~~~~~~~~~~~~~ diff --git a/edx_name_affirmation/admin.py b/edx_name_affirmation/admin.py index 1380390..d1aeeb9 100644 --- a/edx_name_affirmation/admin.py +++ b/edx_name_affirmation/admin.py @@ -14,10 +14,13 @@ class VerifiedNameAdmin(admin.ModelAdmin): """ list_display = ( 'id', 'user', 'verified_name', 'verification_attempt_id', 'proctored_exam_attempt_id', - 'status', 'created', 'modified', + 'platform_verification_attempt_id', 'status', 'created', 'modified', ) readonly_fields = ('id',) - search_fields = ('user__username', 'verification_attempt_id', 'proctored_exam_attempt_id',) + search_fields = ( + 'user__username', 'verification_attempt_id', 'proctored_exam_attempt_id', + 'platform_verification_attempt_id' + ) raw_id_fields = ('user', ) diff --git a/edx_name_affirmation/api.py b/edx_name_affirmation/api.py index b23c1aa..0ddf303 100644 --- a/edx_name_affirmation/api.py +++ b/edx_name_affirmation/api.py @@ -20,7 +20,8 @@ def create_verified_name( user, verified_name, profile_name, verification_attempt_id=None, - proctored_exam_attempt_id=None, status=VerifiedNameStatus.PENDING, + proctored_exam_attempt_id=None, platform_verification_attempt_id=None, + status=VerifiedNameStatus.PENDING, ): """ Create a new `VerifiedName` for the given user. @@ -34,6 +35,8 @@ def create_verified_name( attempt. * `proctored_exam_attempt_id` (int): Optional reference to an external proctored exam attempt. + * `platform_verification_attempt_id` (int): Optional reference to a platform defined + verification attempt. * `is_verified` (bool): Optional, defaults False. This should determine whether the verified_name is valid for use with ID verification, exams, etc. """ @@ -44,15 +47,17 @@ def create_verified_name( raise VerifiedNameEmptyString('profile_name', user.id) # Only link to one attempt - if verification_attempt_id and proctored_exam_attempt_id: + if sum(map(bool, [proctored_exam_attempt_id, verification_attempt_id, platform_verification_attempt_id])) > 1: err_msg = ( - 'Attempted to create VerifiedName for user_id={user_id}, but two different ' + 'Attempted to create VerifiedName for user_id={user_id}, but at least two different ' 'external attempt IDs were given. Only one may be used. ' 'verification_attempt_id={verification_attempt_id}, ' 'proctored_exam_attempt_id={proctored_exam_attempt_id}, ' + 'platform_verification_attempt_id={platform_verification_attempt_id}, ' 'status={status}'.format( user_id=user.id, verification_attempt_id=verification_attempt_id, proctored_exam_attempt_id=proctored_exam_attempt_id, status=status, + platform_verification_attempt_id=platform_verification_attempt_id, ) ) raise VerifiedNameMultipleAttemptIds(err_msg) @@ -63,6 +68,7 @@ def create_verified_name( profile_name=profile_name, verification_attempt_id=verification_attempt_id, proctored_exam_attempt_id=proctored_exam_attempt_id, + platform_verification_attempt_id=platform_verification_attempt_id, status=status, ) @@ -128,89 +134,44 @@ def get_verified_name_history(user): return VerifiedName.objects.filter(user=user).order_by('-created') -def update_verification_attempt_id(user, verification_attempt_id): - """ - Update the `verification_attempt_id` for the user's most recent VerifiedName. - - If the VerifiedName already has a linked verification or proctored exam attempt, create a new - VerifiedName instead, using the same `verified_name` and `profile_name`. - - This will raise an exception if the user does not have an existing VerifiedName. - - Arguments: - * `user` (User object) - * `verification_attempt_id` (int) - """ - verified_name_obj = get_verified_name(user) - - if not verified_name_obj: - err_msg = ( - 'Attempted to update most recent VerifiedName for user_id={user_id} with ' - 'verification_attempt_id={verification_attempt_id}, but this user does not have ' - 'an existing VerifiedName.'.format( - user_id=user.id, verification_attempt_id=verification_attempt_id - ) - ) - raise VerifiedNameDoesNotExist(err_msg) - - if verified_name_obj.verification_attempt_id or verified_name_obj.proctored_exam_attempt_id: - log_msg = ( - 'Attempted to update VerifiedName id={id} with ' - 'verification_attempt_id={verification_attempt_id}, but it already has a linked attempt. ' - 'Creating a new VerifiedName for user_id={user_id}'.format( - id=verified_name_obj.id, verification_attempt_id=verification_attempt_id, user_id=user.id, - ) - ) - log.warning(log_msg) - - create_verified_name( - user=user, - verified_name=verified_name_obj.verified_name, - profile_name=verified_name_obj.profile_name, - verification_attempt_id=verification_attempt_id, - ) - - verified_name_obj.verification_attempt_id = verification_attempt_id - verified_name_obj.save() - - log_msg = ( - 'Updated VerifiedName id={id} with verification_attempt_id={verification_attempt_id} ' - 'for user_id={user_id}'.format( - id=verified_name_obj.id, verification_attempt_id=verification_attempt_id, user_id=user.id, - ) - ) - log.info(log_msg) - - def update_verified_name_status( - user, status, verification_attempt_id=None, proctored_exam_attempt_id=None + user, status, verification_attempt_id=None, proctored_exam_attempt_id=None, platform_verification_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. + Update the status of a VerifiedName using the linked ID verification, exam attempt ID, or platform defined + verification attempt ID. Only one of these should be specified. Arguments: * user (User object) * status (Verified Name Status) - * verification_attempt_id (int) - * proctored_exam_attempt_id (int) + * verification_attempt_id (int): Optional reference to an external ID verification + attempt. + * proctored_exam_attempt_id (int): Optional reference to an external proctored exam + attempt. + * platform_verification_attempt_id (int): Optional reference to a platform defined + verification attempt. """ filters = {'user': user} - if verification_attempt_id: - if proctored_exam_attempt_id: - err_msg = ( - '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, - proctored_exam_attempt_id=proctored_exam_attempt_id, - ) + if sum(map(bool, [proctored_exam_attempt_id, verification_attempt_id, platform_verification_attempt_id])) > 1: + err_msg = ( + 'Attempted to update the status for a VerifiedName, but at least two different ' + 'attempt IDs were given. verification_attempt_id={verification_attempt_id}, ' + 'proctored_exam_attempt_id={proctored_exam_attempt_id},' + 'platform_verification_attempt_id={platform_verification_attempt_id}'.format( + verification_attempt_id=verification_attempt_id, + proctored_exam_attempt_id=proctored_exam_attempt_id, + platform_verification_attempt_id=platform_verification_attempt_id, ) - raise VerifiedNameMultipleAttemptIds(err_msg) + ) + raise VerifiedNameMultipleAttemptIds(err_msg) + + if verification_attempt_id: filters['verification_attempt_id'] = verification_attempt_id elif proctored_exam_attempt_id: filters['proctored_exam_attempt_id'] = proctored_exam_attempt_id + elif platform_verification_attempt_id: + filters['platform_verification_attempt_id'] = platform_verification_attempt_id else: err_msg = ( 'Attempted to update the status for a VerifiedName, but no ' diff --git a/edx_name_affirmation/models.py b/edx_name_affirmation/models.py index d6fe988..bf4ef2a 100644 --- a/edx_name_affirmation/models.py +++ b/edx_name_affirmation/models.py @@ -14,8 +14,10 @@ try: from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification + from lms.djangoapps.verify_studnet.models import VerificationAttempt as PlatformVerificationAttempt except ImportError: SoftwareSecurePhotoVerification = None + PlatformVerificationAttempt = None User = get_user_model() @@ -77,6 +79,21 @@ def verification_attempt_status(self): except ObjectDoesNotExist: return None + @property + def platform_verification_attempt_status(self): + """ + Returns the status associated with its platform VerificationAttempt + """ + if not self.platform_verification_attempt_id or not PlatformVerificationAttempt: + return None + + try: + verification = PlatformVerificationAttempt.objects.get(id=self.platform_verification_attempt_id) + return verification.status + + except ObjectDoesNotExist: + return None + def save(self, *args, **kwargs): """ Validate only one of `verification_attempt_id` or `platform_verification_attempt_id` diff --git a/edx_name_affirmation/serializers.py b/edx_name_affirmation/serializers.py index 464519d..b244196 100644 --- a/edx_name_affirmation/serializers.py +++ b/edx_name_affirmation/serializers.py @@ -22,6 +22,8 @@ class VerifiedNameSerializer(serializers.ModelSerializer): verification_attempt_status = serializers.CharField(required=False, allow_null=True) proctored_exam_attempt_id = serializers.IntegerField(required=False, allow_null=True) status = serializers.CharField(required=False, allow_null=True) + platform_verification_attempt_id = serializers.IntegerField(required=False, allow_null=True) + platform_verification_attempt_status = serializers.CharField(required=False, allow_null=True) class Meta: """ @@ -31,7 +33,8 @@ class Meta: fields = ( "id", "created", "username", "verified_name", "profile_name", "verification_attempt_id", - "verification_attempt_status", "proctored_exam_attempt_id", "status" + "verification_attempt_status", "proctored_exam_attempt_id", "platform_verification_attempt_id", + "platform_verification_attempt_status", "status" ) def validate_verified_name(self, verified_name): diff --git a/edx_name_affirmation/tests/test_api.py b/edx_name_affirmation/tests/test_api.py index d8719de..bc5a74f 100644 --- a/edx_name_affirmation/tests/test_api.py +++ b/edx_name_affirmation/tests/test_api.py @@ -14,7 +14,6 @@ get_verified_name, get_verified_name_history, should_use_verified_name_for_certs, - update_verification_attempt_id, update_verified_name_status ) from edx_name_affirmation.exceptions import ( @@ -23,7 +22,7 @@ VerifiedNameEmptyString, VerifiedNameMultipleAttemptIds ) -from edx_name_affirmation.models import VerifiedName, VerifiedNameConfig +from edx_name_affirmation.models import VerifiedNameConfig from edx_name_affirmation.statuses import VerifiedNameStatus User = get_user_model() @@ -38,6 +37,7 @@ class TestVerifiedNameAPI(TestCase): PROFILE_NAME = 'Jon Doe' VERIFICATION_ATTEMPT_ID = 123 PROCTORED_EXAM_ATTEMPT_ID = 456 + PLATFORM_VERIFICATION_ATTEMPT_ID = 789 def setUp(self): super().setUp() @@ -62,25 +62,36 @@ def test_create_verified_name_defaults(self): self.assertEqual(verified_name_obj.status, VerifiedNameStatus.PENDING.value) @ddt.data( - (123, None, VerifiedNameStatus.APPROVED), - (None, 456, VerifiedNameStatus.SUBMITTED), + (123, None, None, VerifiedNameStatus.APPROVED), + (None, 456, None, VerifiedNameStatus.SUBMITTED), + (None, None, 789, VerifiedNameStatus.SUBMITTED), ) @ddt.unpack def test_create_verified_name_with_optional_arguments( - self, verification_attempt_id, proctored_exam_attempt_id, status, + self, verification_attempt_id, proctored_exam_attempt_id, platform_verification_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, status, + verification_attempt_id, proctored_exam_attempt_id, platform_verification_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.platform_verification_attempt_id, platform_verification_attempt_id) self.assertEqual(verified_name_obj.status, status.value) - def test_create_verified_name_two_ids(self): + @ddt.data( + (123, 456, None), + (123, None, 789), + (None, 456, 789), + (123, 456, 789), + ) + @ddt.unpack + def test_create_verified_name_two_ids( + self, verification_attempt_id, proctored_exam_attempt_id, platform_verification_attempt_id + ): """ Test that a verified name cannot be created with both a verification_attempt_id and a proctored_exam_attempt_id. @@ -90,8 +101,9 @@ def test_create_verified_name_two_ids(self): self.user, self.VERIFIED_NAME, self.PROFILE_NAME, - self.VERIFICATION_ATTEMPT_ID, - self.PROCTORED_EXAM_ATTEMPT_ID, + verification_attempt_id, + proctored_exam_attempt_id, + platform_verification_attempt_id, ) @ddt.data( @@ -202,65 +214,25 @@ def test_get_verified_name_history(self): self.assertEqual(verified_name_qs[1].id, verified_name_first.id) self.assertEqual(verified_name_qs[0].id, verified_name_second.id) - def test_update_verification_attempt_id(self): - """ - Test that the most recent VerifiedName is updated with a verification_attempt_id if - it does not already have one. - """ - first_verified_name_id = self._create_verified_name().id - second_verified_name_id = self._create_verified_name().id - - update_verification_attempt_id(self.user, self.VERIFICATION_ATTEMPT_ID) - - first_verified_name_obj = VerifiedName.objects.get(id=first_verified_name_id) - second_verified_name_obj = VerifiedName.objects.get(id=second_verified_name_id) - - self.assertIsNone(first_verified_name_obj.verification_attempt_id) - self.assertEqual(second_verified_name_obj.verification_attempt_id, self.VERIFICATION_ATTEMPT_ID) - - @ddt.data( - (VERIFICATION_ATTEMPT_ID, None), - (None, PROCTORED_EXAM_ATTEMPT_ID), - ) - @ddt.unpack - def test_update_verification_attempt_id_already_exists( - self, verification_attempt_id, proctored_exam_attempt_id, - ): - """ - Test that if the most recent VerifiedName already has a linked verification or - proctored exam attempt, a new VerifiedName will be created when updating the - `verification_attempt_id`. - """ - self._create_verified_name( - verification_attempt_id=verification_attempt_id, - proctored_exam_attempt_id=proctored_exam_attempt_id, - ) - update_verification_attempt_id(self.user, 789) - verified_name_qs = VerifiedName.objects.all() - self.assertEqual(len(verified_name_qs), 2) - - def test_update_verification_attempt_id_none_exist(self): - """ - Test that if the user does not have an existing VerifiedName, - `update_verification_attempt_id` will raise an exception. - """ - with self.assertRaises(VerifiedNameDoesNotExist): - update_verification_attempt_id(self.user, self.VERIFICATION_ATTEMPT_ID) - @ddt.data( - (VERIFICATION_ATTEMPT_ID, None), - (None, PROCTORED_EXAM_ATTEMPT_ID) + (VERIFICATION_ATTEMPT_ID, None, None), + (None, PROCTORED_EXAM_ATTEMPT_ID, None), + (None, None, PLATFORM_VERIFICATION_ATTEMPT_ID) ) @ddt.unpack def test_update_is_verified_status( - self, verification_attempt_id, proctored_exam_attempt_id, + self, verification_attempt_id, proctored_exam_attempt_id, platform_verification_attempt_id, ): """ Test that VerifiedName status can be updated with a given attempt ID. """ - self._create_verified_name(verification_attempt_id, proctored_exam_attempt_id) + self._create_verified_name(verification_attempt_id, proctored_exam_attempt_id, platform_verification_attempt_id) update_verified_name_status( - self.user, VerifiedNameStatus.DENIED, verification_attempt_id, proctored_exam_attempt_id, + self.user, + VerifiedNameStatus.DENIED, + verification_attempt_id, + proctored_exam_attempt_id, + platform_verification_attempt_id, ) verified_name_obj = get_verified_name(self.user) self.assertEqual(VerifiedNameStatus.DENIED.value, verified_name_obj.status) @@ -273,14 +245,23 @@ def test_update_is_verified_no_attempt_id(self): with self.assertRaises(VerifiedNameAttemptIdNotGiven): update_verified_name_status(self.user, True) - def test_update_is_verified_multiple_attempt_ids(self): + @ddt.data( + (123, 456, None), + (123, None, 789), + (None, 456, 789), + (123, 456, 789), + ) + @ddt.unpack + def test_update_is_verified_multiple_attempt_ids( + self, verification_attempt_id, proctored_exam_attempt_id, platform_verification_attempt_id + ): """ Test that `update_is_verified_by_attempt_id` will raise an exception with multiple attempt IDs given. """ with self.assertRaises(VerifiedNameMultipleAttemptIds): update_verified_name_status( - self.user, True, self.VERIFICATION_ATTEMPT_ID, self.PROCTORED_EXAM_ATTEMPT_ID, + self.user, True, verification_attempt_id, proctored_exam_attempt_id, platform_verification_attempt_id, ) def test_update_is_verified_does_not_exist(self): @@ -292,14 +273,15 @@ def test_update_is_verified_does_not_exist(self): 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, status=VerifiedNameStatus.PENDING, + self, verification_attempt_id=None, proctored_exam_attempt_id=None, + platform_verification_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, status + proctored_exam_attempt_id, platform_verification_attempt_id, status ) return get_verified_name(self.user) diff --git a/edx_name_affirmation/tests/test_models.py b/edx_name_affirmation/tests/test_models.py index 8a316c8..94d6116 100644 --- a/edx_name_affirmation/tests/test_models.py +++ b/edx_name_affirmation/tests/test_models.py @@ -67,6 +67,22 @@ def test_verification_status(self, sspv_mock): self.verified_name.verification_attempt_id = self.idv_attempt_id assert self.verified_name.verification_attempt_status is self.idv_attempt_status + @patch('edx_name_affirmation.models.PlatformVerificationAttempt') + def test_platform_verification_attempt_status(self, platform_attempt_mock): + """ + Test that the platform verification status is updated as expected + """ + + idv_attempt_id_notfound_status = None + + platform_attempt_mock.objects.get = self._mocked_model_get + + self.verified_name.platform_verification_attempt_id = self.idv_attempt_id_notfound + assert self.verified_name.platform_verification_attempt_status is idv_attempt_id_notfound_status + + self.verified_name.platform_verification_attempt_id = self.idv_attempt_id + assert self.verified_name.platform_verification_attempt_status is self.idv_attempt_status + def test_verification_id_exclusivity(self): """ Test that only one verification ID can be set at a time @@ -84,7 +100,9 @@ def _obj(self, dictionary): return type('obj', (object,), dictionary) def _mocked_model_get(self, id): # pylint: disable=redefined-builtin - "Helper method to mock the behavior of SoftwareSecurePhotoVerification model. Used to mock below." + """ + Helper method to mock the behavior of SoftwareSecurePhotoVerification and PlatformVerificationAttempt models. + """ if id == self.idv_attempt_id_notfound: raise ObjectDoesNotExist diff --git a/edx_name_affirmation/tests/test_views.py b/edx_name_affirmation/tests/test_views.py index eb1eb86..97bbc3a 100644 --- a/edx_name_affirmation/tests/test_views.py +++ b/edx_name_affirmation/tests/test_views.py @@ -329,7 +329,7 @@ def test_delete(self, is_staff, verified_name_exists, expected_response): def _create_verified_name( self, user, verification_attempt_id=None, - proctored_exam_attempt_id=None, status=VerifiedNameStatus.PENDING, + proctored_exam_attempt_id=None, platform_verification_attempt_id=None, status=VerifiedNameStatus.PENDING, ): """ Create and return a verified name object. @@ -340,6 +340,7 @@ def _create_verified_name( self.PROFILE_NAME, verification_attempt_id, proctored_exam_attempt_id, + platform_verification_attempt_id, status ) return get_verified_name(user) @@ -362,6 +363,8 @@ def _get_expected_data( 'proctored_exam_attempt_id': verified_name_obj.proctored_exam_attempt_id, 'status': verified_name_obj.status, 'use_verified_name_for_certs': use_verified_name_for_certs, + 'platform_verification_attempt_id': verified_name_obj.platform_verification_attempt_id, + 'platform_verification_attempt_status': None, } @@ -479,7 +482,9 @@ def _get_expected_response( 'verification_attempt_id': verified_name_obj.verification_attempt_id, 'verification_attempt_status': None, 'proctored_exam_attempt_id': verified_name_obj.proctored_exam_attempt_id, - 'status': verified_name_obj.status + 'status': verified_name_obj.status, + 'platform_verification_attempt_id': verified_name_obj.platform_verification_attempt_id, + 'platform_verification_attempt_status': None, } expected_response['results'].append(data) diff --git a/edx_name_affirmation/views.py b/edx_name_affirmation/views.py index ae79377..36e21ef 100644 --- a/edx_name_affirmation/views.py +++ b/edx_name_affirmation/views.py @@ -111,6 +111,7 @@ def post(self, request): "profile_name": "Jon Doe" "verification_attempt_id": (Optional) "proctored_exam_attempt_id": (Optional) + "platform_verification_attempt_id": (Optional) "status": (Optional) } """ @@ -131,6 +132,7 @@ def post(self, request): request.data.get('profile_name'), verification_attempt_id=request.data.get('verification_attempt_id', None), proctored_exam_attempt_id=request.data.get('proctored_exam_attempt_id', None), + platform_verification_attempt_id=request.data.get('platform_verification_attempt_id', None), status=request.data.get('status', VerifiedNameStatus.PENDING) ) response_status = http_status.HTTP_200_OK