From d59f7e6638b2f39d9a77ba6151d3dd1381cc21f7 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Fri, 27 Sep 2024 09:16:37 -0400 Subject: [PATCH 01/12] feat: initial approach --- edx_name_affirmation/apps.py | 8 +-- edx_name_affirmation/handlers.py | 84 ++++++++++++++++---------------- edx_name_affirmation/tasks.py | 28 ++++++----- 3 files changed, 61 insertions(+), 59 deletions(-) diff --git a/edx_name_affirmation/apps.py b/edx_name_affirmation/apps.py index dd4942d..e5a08fb 100644 --- a/edx_name_affirmation/apps.py +++ b/edx_name_affirmation/apps.py @@ -25,13 +25,9 @@ class EdxNameAffirmationConfig(AppConfig): 'relative_path': 'handlers', 'receivers': [ { - 'receiver_func_name': 'idv_attempt_handler', - 'signal_path': 'lms.djangoapps.verify_student.signals.signals.idv_update_signal', - }, - { - 'receiver_func_name': 'idv_delete_handler', + 'receiver_func_name': 'platform_verification_delete_handler', 'signal_path': 'django.db.models.signals.post_delete', - 'sender_path': 'lms.djangoapps.verify_student.models.SoftwareSecurePhotoVerification', + 'sender_path': 'lms.djangoapps.verify_student.models.VerificationAttempt', }, { 'receiver_func_name': 'proctoring_attempt_handler', diff --git a/edx_name_affirmation/handlers.py b/edx_name_affirmation/handlers.py index a276772..5e196ee 100644 --- a/edx_name_affirmation/handlers.py +++ b/edx_name_affirmation/handlers.py @@ -8,6 +8,12 @@ from django.db.models.signals import post_save from django.dispatch.dispatcher import receiver +from openedx_events.learning.signals import ( + IDV_ATTEMPT_APPROVED, + IDV_ATTEMPT_DENIED, + IDV_ATTEMPT_PENDING, +) + from edx_name_affirmation.models import VerifiedName from edx_name_affirmation.signals import VERIFIED_NAME_APPROVED from edx_name_affirmation.statuses import VerifiedNameStatus @@ -35,56 +41,52 @@ def verified_name_approved(sender, instance, **kwargs): # pylint: disable=unuse ) -def idv_attempt_handler(attempt_id, user_id, status, photo_id_name, full_name, **kwargs): - """ - Receiver for IDV attempt updates - - Args: - attempt_id(int): ID associated with the IDV attempt - user_id(int): ID associated with the IDV attempt's user - status(str): status in IDV language for the IDV attempt - photo_id_name(str): name to be used as verified name - full_name(str): user's pending name change or current profile name - """ - trigger_status = VerifiedNameStatus.trigger_state_change_from_idv(status) - - # only trigger celery task if status is relevant to name affirmation - if trigger_status: - log.info('VerifiedName: idv_attempt_handler triggering Celery task for user %(user_id)s ' - 'with photo_id_name %(photo_id_name)s and status %(status)s', - { - 'user_id': user_id, - 'photo_id_name': photo_id_name, - 'status': status - } - ) - idv_update_verified_name_task.delay(attempt_id, user_id, trigger_status, photo_id_name, full_name) +@receiver(IDV_ATTEMPT_APPROVED) +@receiver(IDV_ATTEMPT_DENIED) +@receiver(IDV_ATTEMPT_PENDING) +def handle_idv_attempt_approved(sender, signal, **kwargs): + event_data = kwargs.get('idv_attempt') + user = User.objects.get(id=event_data.user.id) + + # If the user has a pending name change, use that as the full name + try: + user_full_name = user.pending_name_change + except AttributeError: + user_full_name = user.profile.name + + status = None + if signal == IDV_ATTEMPT_APPROVED: + status = VerifiedNameStatus.APPROVED + elif signal == IDV_ATTEMPT_PENDING: + status = VerifiedNameStatus.PENDING + elif signal == IDV_ATTEMPT_DENIED: + status = VerifiedNameStatus.DENIED else: - log.info('VerifiedName: idv_attempt_handler will not trigger Celery task for user %(user_id)s ' - 'with photo_id_name %(photo_id_name)s because of status %(status)s', - { - 'user_id': user_id, - 'photo_id_name': photo_id_name, - 'status': status - } - ) + return + log.info(f'IDV_ATTEMPT {status} signal triggering Celery task for user {user.id} ' + f'with photo_id_name {event_data.name}') + idv_update_verified_name_task.delay( + event_data.attempt_id, + user.id, + status, + event_data.name, + user_full_name, + ) -def idv_delete_handler(sender, instance, signal, **kwargs): # pylint: disable=unused-argument - """ - Receiver for IDV attempt deletions - Args: - attempt_id(int): ID associated with the IDV attempt +def platform_verification_delete_handler(sender, instance, signal, **kwargs): # pylint: disable=unused-argument + """ """ - idv_attempt_id = instance.id + platform_verification_attempt_id = instance.id log.info( - 'VerifiedName: idv_delete_handler triggering Celery task for idv_attempt_id=%(idv_attempt_id)s', + 'VerifiedName: platform_verification_delete_handler triggering Celery task for ' + 'platform_verification_attempt_id=%(platform_verification_attempt_id)s', { - 'idv_attempt_id': idv_attempt_id, + 'platform_verification_attempt_id': platform_verification_attempt_id, } ) - delete_verified_name_task.delay(idv_attempt_id, None) + delete_verified_name_task.delay(platform_verification_attempt_id, None) def proctoring_attempt_handler( diff --git a/edx_name_affirmation/tasks.py b/edx_name_affirmation/tasks.py index d76eb15..3fbb491 100644 --- a/edx_name_affirmation/tasks.py +++ b/edx_name_affirmation/tasks.py @@ -43,7 +43,7 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st # want to grab all verified names for the same user and name combination, because # some of those records may already be associated with a different IDV attempt. verified_names = VerifiedName.objects.filter( - (Q(verification_attempt_id=attempt_id) | Q(verification_attempt_id__isnull=True)) + (Q(platform_verification_attempt_id=attempt_id) | Q(platform_verification_attempt_id__isnull=True)) & Q(user__id=user_id) & Q(verified_name=photo_id_name) ).order_by('-created') @@ -52,12 +52,12 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st # for each attempt with no attempt id (either proctoring or idv), update attempt id updated_for_attempt_id = verified_names.filter( proctored_exam_attempt_id=None, - verification_attempt_id=None - ).update(verification_attempt_id=attempt_id) + platform_verification_attempt_id=None + ).update(platform_verification_attempt_id=attempt_id) if updated_for_attempt_id: log.info( - 'Updated VerifiedNames for user={user_id} to verification_attempt_id={attempt_id}'.format( + 'Updated VerifiedNames for user={user_id} to platform_verification_attempt_id={attempt_id}'.format( user_id=user_id, attempt_id=attempt_id, ) @@ -65,7 +65,7 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st # then for all matching attempt ids, update the status verified_name_qs = verified_names.filter( - verification_attempt_id=attempt_id, + platform_verification_attempt_id=attempt_id, proctored_exam_attempt_id=None ) @@ -75,7 +75,7 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st verified_name_obj.save() log.info( - 'Updated VerifiedNames for user={user_id} with verification_attempt_id={attempt_id} to ' + 'Updated VerifiedNames for user={user_id} with platform_verification_attempt_id={attempt_id} to ' 'have status={status}'.format( user_id=user_id, attempt_id=attempt_id, @@ -89,12 +89,12 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st user=user, verified_name=photo_id_name, profile_name=full_name, - verification_attempt_id=attempt_id, + platform_verification_attempt_id=attempt_id, status=name_affirmation_status, ) log.error( 'Created VerifiedName for user={user_id} to have status={status} ' - 'and verification_attempt_id={attempt_id}, because no matching ' + 'and platform_verification_attempt_id={attempt_id}, because no matching ' 'attempt_id or verified_name were found.'.format( user_id=user_id, attempt_id=attempt_id, @@ -187,20 +187,24 @@ def proctoring_update_verified_name_task( bind=True, autoretry_for=(Exception,), default_retry_delay=DEFAULT_RETRY_SECONDS, max_retries=MAX_RETRIES, ) @set_code_owner_attribute -def delete_verified_name_task(self, idv_attempt_id, proctoring_attempt_id): +def delete_verified_name_task(self, platform_verification_attempt_id, idv_attempt_id, proctoring_attempt_id): """ Celery task to delete a verified name based on an idv or proctoring attempt """ # this case shouldn't happen, but should log as an error in case - if (idv_attempt_id and proctoring_attempt_id) or (not idv_attempt_id and not proctoring_attempt_id): + if (idv_attempt_id, proctoring_attempt_id, platform_verification_attempt_id).count(None) != 1: log.error( - 'A maximum of one attempt id should be provided for either a proctored exam attempt or IDV attempt.' + 'A maximum of one attempt id should be provided' ) return log_message = {'field_name': '', 'attempt_id': ''} - if idv_attempt_id: + if platform_verification_attempt_id: + verified_names = VerifiedName.objects.filter(platform_verification_attempt_id=platform_verification_attempt_id) + log_message['field_name'] = 'platform_verification_attempt_id' + log_message['attempt_id'] = platform_verification_attempt_id + elif idv_attempt_id: verified_names = VerifiedName.objects.filter(verification_attempt_id=idv_attempt_id) log_message['field_name'] = 'verification_attempt_id' log_message['attempt_id'] = idv_attempt_id From 23c42438d8a36e78b84c01a0bee52a314d8fa1e3 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Fri, 27 Sep 2024 12:53:24 -0400 Subject: [PATCH 02/12] feat: unused status function --- edx_name_affirmation/statuses.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/edx_name_affirmation/statuses.py b/edx_name_affirmation/statuses.py index 7358a51..1c3838d 100644 --- a/edx_name_affirmation/statuses.py +++ b/edx_name_affirmation/statuses.py @@ -32,21 +32,6 @@ class VerifiedNameStatus(str, Enum): APPROVED = "approved" DENIED = "denied" - @classmethod - def trigger_state_change_from_idv(cls, idv_status): - """ - Return the translated IDV status if it should trigger a state transition, otherwise return None - """ - # mapping from an idv status (key) to it's associated verified name status (value). We only want to - # include idv statuses that would cause a status transition for a verified name - idv_state_transition_mapping = { - 'created': cls.PENDING, - 'submitted': cls.SUBMITTED, - 'approved': cls.APPROVED, - 'denied': cls.DENIED - } - - return idv_state_transition_mapping.get(idv_status, None) @classmethod def trigger_state_change_from_proctoring(cls, proctoring_status): From aacb6cdcc64c49ed1c67167a4a29ef9ce2735102 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Fri, 27 Sep 2024 12:54:02 -0400 Subject: [PATCH 03/12] fix: task logic --- edx_name_affirmation/tasks.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/edx_name_affirmation/tasks.py b/edx_name_affirmation/tasks.py index 3fbb491..f6b8b46 100644 --- a/edx_name_affirmation/tasks.py +++ b/edx_name_affirmation/tasks.py @@ -52,6 +52,7 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st # for each attempt with no attempt id (either proctoring or idv), update attempt id updated_for_attempt_id = verified_names.filter( proctored_exam_attempt_id=None, + verification_attempt_id=None, platform_verification_attempt_id=None ).update(platform_verification_attempt_id=attempt_id) @@ -66,6 +67,7 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st # then for all matching attempt ids, update the status verified_name_qs = verified_names.filter( platform_verification_attempt_id=attempt_id, + verification_attempt_id=None, proctored_exam_attempt_id=None ) @@ -187,12 +189,12 @@ def proctoring_update_verified_name_task( bind=True, autoretry_for=(Exception,), default_retry_delay=DEFAULT_RETRY_SECONDS, max_retries=MAX_RETRIES, ) @set_code_owner_attribute -def delete_verified_name_task(self, platform_verification_attempt_id, idv_attempt_id, proctoring_attempt_id): +def delete_verified_name_task(self, platform_verification_attempt_id, proctoring_attempt_id): """ Celery task to delete a verified name based on an idv or proctoring attempt """ # this case shouldn't happen, but should log as an error in case - if (idv_attempt_id, proctoring_attempt_id, platform_verification_attempt_id).count(None) != 1: + if (proctoring_attempt_id, platform_verification_attempt_id).count(None) != 1: log.error( 'A maximum of one attempt id should be provided' ) @@ -204,22 +206,18 @@ def delete_verified_name_task(self, platform_verification_attempt_id, idv_attemp verified_names = VerifiedName.objects.filter(platform_verification_attempt_id=platform_verification_attempt_id) log_message['field_name'] = 'platform_verification_attempt_id' log_message['attempt_id'] = platform_verification_attempt_id - elif idv_attempt_id: - verified_names = VerifiedName.objects.filter(verification_attempt_id=idv_attempt_id) - log_message['field_name'] = 'verification_attempt_id' - log_message['attempt_id'] = idv_attempt_id else: verified_names = VerifiedName.objects.filter(proctored_exam_attempt_id=proctoring_attempt_id) log_message['field_name'] = 'proctored_exam_attempt_id' log_message['attempt_id'] = proctoring_attempt_id if verified_names: - log.info( + log.info( # there's a bug in this log message 'Deleting {num_names} VerifiedName(s) associated with {field_name}=' - '{verification_attempt_id}'.format( + '{platform_verification_attempt_id}'.format( num_names=len(verified_names), field_name=log_message['field_name'], - verification_attempt_id=log_message['attempt_id'], + platform_verification_attempt_id=log_message['attempt_id'], ) ) verified_names.delete() From 576f04c8f703b0f8bb2723659de070b2a1834bc7 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Fri, 27 Sep 2024 16:07:18 -0400 Subject: [PATCH 04/12] feat: refactor existing tests for new IDV source --- edx_name_affirmation/handlers.py | 14 +- edx_name_affirmation/statuses.py | 1 - edx_name_affirmation/tests/test_handlers.py | 134 +++++++++----------- edx_name_affirmation/tests/test_tasks.py | 10 +- 4 files changed, 73 insertions(+), 86 deletions(-) diff --git a/edx_name_affirmation/handlers.py b/edx_name_affirmation/handlers.py index 5e196ee..1512c6c 100644 --- a/edx_name_affirmation/handlers.py +++ b/edx_name_affirmation/handlers.py @@ -10,6 +10,7 @@ from openedx_events.learning.signals import ( IDV_ATTEMPT_APPROVED, + IDV_ATTEMPT_CREATED, IDV_ATTEMPT_DENIED, IDV_ATTEMPT_PENDING, ) @@ -42,9 +43,13 @@ def verified_name_approved(sender, instance, **kwargs): # pylint: disable=unuse @receiver(IDV_ATTEMPT_APPROVED) +@receiver(IDV_ATTEMPT_CREATED) @receiver(IDV_ATTEMPT_DENIED) @receiver(IDV_ATTEMPT_PENDING) -def handle_idv_attempt_approved(sender, signal, **kwargs): +def handle_idv_event(sender, signal, **kwargs): # pylint: disable=unused-argument + """ + Trigger update to verified names based on open edX IDV events. + """ event_data = kwargs.get('idv_attempt') user = User.objects.get(id=event_data.user.id) @@ -52,13 +57,15 @@ def handle_idv_attempt_approved(sender, signal, **kwargs): try: user_full_name = user.pending_name_change except AttributeError: - user_full_name = user.profile.name + user_full_name = event_data.user.pii.name status = None if signal == IDV_ATTEMPT_APPROVED: status = VerifiedNameStatus.APPROVED - elif signal == IDV_ATTEMPT_PENDING: + elif signal == IDV_ATTEMPT_CREATED: status = VerifiedNameStatus.PENDING + elif signal == IDV_ATTEMPT_PENDING: + status = VerifiedNameStatus.SUBMITTED elif signal == IDV_ATTEMPT_DENIED: status = VerifiedNameStatus.DENIED else: @@ -77,6 +84,7 @@ def handle_idv_attempt_approved(sender, signal, **kwargs): def platform_verification_delete_handler(sender, instance, signal, **kwargs): # pylint: disable=unused-argument """ + Receiver for VerificationAttempt deletions """ platform_verification_attempt_id = instance.id log.info( diff --git a/edx_name_affirmation/statuses.py b/edx_name_affirmation/statuses.py index 1c3838d..914af34 100644 --- a/edx_name_affirmation/statuses.py +++ b/edx_name_affirmation/statuses.py @@ -32,7 +32,6 @@ class VerifiedNameStatus(str, Enum): APPROVED = "approved" DENIED = "denied" - @classmethod def trigger_state_change_from_proctoring(cls, proctoring_status): """ diff --git a/edx_name_affirmation/tests/test_handlers.py b/edx_name_affirmation/tests/test_handlers.py index 0c237cc..70bdb54 100644 --- a/edx_name_affirmation/tests/test_handlers.py +++ b/edx_name_affirmation/tests/test_handlers.py @@ -8,9 +8,17 @@ from django.contrib.auth import get_user_model from django.test import TestCase +from openedx_events.learning.data import UserData, UserPersonalData, VerificationAttemptData +from openedx_events.learning.signals import ( + IDV_ATTEMPT_APPROVED, + IDV_ATTEMPT_CREATED, + IDV_ATTEMPT_DENIED, + IDV_ATTEMPT_PENDING, +) + from edx_name_affirmation.handlers import ( - idv_attempt_handler, - idv_delete_handler, + handle_idv_event, + platform_verification_delete_handler, proctoring_attempt_handler, proctoring_delete_handler ) @@ -73,34 +81,49 @@ class IDVSignalTests(SignalTestCase): """ Test for idv_attempt_handler """ + def _handle_idv_event(self, idv_signal, attempt_id): + """ Call IDV handler with a mock event """ + user_data = UserData( + id=self.user.id, + is_active=True, + pii=UserPersonalData( + username=self.user.username, + email=self.user.email, + name=self.profile_name, + ) + ) + event_data = VerificationAttemptData( + attempt_id=attempt_id, + user=user_data, + status='mock-platform-status', + name=self.verified_name, + ) + event_kwargs = { + 'idv_attempt': event_data + } + handle_idv_event(None, idv_signal, **event_kwargs) def test_idv_create_verified_name(self): """ Test that if no verified name exists for the name or attempt id, create one """ - idv_attempt_handler( - self.idv_attempt_id, - self.user.id, - 'created', - self.verified_name, - self.profile_name - ) + self._handle_idv_event(IDV_ATTEMPT_CREATED, self.idv_attempt_id) # make sure that verifiedname is created with relevant data - verified_name = VerifiedName.objects.get(verification_attempt_id=self.idv_attempt_id) + verified_name = VerifiedName.objects.get(platform_verification_attempt_id=self.idv_attempt_id) self.assertEqual(verified_name.status, VerifiedNameStatus.PENDING) - self.assertEqual(verified_name.verification_attempt_id, self.idv_attempt_id) + self.assertEqual(verified_name.platform_verification_attempt_id, self.idv_attempt_id) self.assertEqual(verified_name.verified_name, self.verified_name) self.assertEqual(verified_name.profile_name, self.profile_name) @ddt.data( - ('created', VerifiedNameStatus.PENDING), - ('submitted', VerifiedNameStatus.SUBMITTED), - ('approved', VerifiedNameStatus.APPROVED), - ('denied', VerifiedNameStatus.DENIED) + (IDV_ATTEMPT_CREATED, VerifiedNameStatus.PENDING), + (IDV_ATTEMPT_PENDING, VerifiedNameStatus.SUBMITTED), + (IDV_ATTEMPT_APPROVED, VerifiedNameStatus.APPROVED), + (IDV_ATTEMPT_DENIED, VerifiedNameStatus.DENIED) ) @ddt.unpack - def test_idv_update_multiple_verified_names(self, idv_status, expected_status): + def test_idv_update_multiple_verified_names(self, idv_signal, expected_status): """ If a VerifiedName(s) for a user and verified name exist, ensure that it is updated properly """ @@ -119,19 +142,13 @@ def test_idv_update_multiple_verified_names(self, idv_status, expected_status): user=self.user, verified_name=self.verified_name, profile_name=self.profile_name, - verification_attempt_id=self.idv_attempt_id + platform_verification_attempt_id=self.idv_attempt_id ) - idv_attempt_handler( - self.idv_attempt_id, - self.user.id, - idv_status, - self.verified_name, - self.profile_name - ) + self._handle_idv_event(idv_signal, self.idv_attempt_id) # check that the attempt id and status have been updated for all three VerifiedNames - self.assertEqual(len(VerifiedName.objects.filter(verification_attempt_id=self.idv_attempt_id)), 3) + self.assertEqual(len(VerifiedName.objects.filter(platform_verification_attempt_id=self.idv_attempt_id)), 3) self.assertEqual(len(VerifiedName.objects.filter(status=expected_status)), 3) def test_idv_create_with_existing_verified_names(self): @@ -144,23 +161,17 @@ def test_idv_create_with_existing_verified_names(self): user=self.user, verified_name=self.verified_name, profile_name=self.profile_name, - verification_attempt_id=previous_id, + platform_verification_attempt_id=previous_id, status='denied' ) # create an IDV attempt with the same user and names as above, but change the attempt ID to a unique value - idv_attempt_handler( - self.idv_attempt_id, - self.user.id, - 'submitted', - self.verified_name, - self.profile_name - ) + self._handle_idv_event(IDV_ATTEMPT_PENDING, self.idv_attempt_id) - verified_name = VerifiedName.objects.get(verification_attempt_id=self.idv_attempt_id) + verified_name = VerifiedName.objects.get(platform_verification_attempt_id=self.idv_attempt_id) self.assertEqual(verified_name.status, VerifiedNameStatus.SUBMITTED) - previous_name = VerifiedName.objects.get(verification_attempt_id=previous_id) + previous_name = VerifiedName.objects.get(platform_verification_attempt_id=previous_id) self.assertEqual(previous_name.status, VerifiedNameStatus.DENIED) def test_idv_does_not_update_verified_name_by_proctoring(self): @@ -181,27 +192,21 @@ def test_idv_does_not_update_verified_name_by_proctoring(self): profile_name=self.profile_name ) - idv_attempt_handler( - self.idv_attempt_id, - self.user.id, - 'submitted', - self.verified_name, - self.profile_name - ) + self._handle_idv_event(IDV_ATTEMPT_PENDING, self.idv_attempt_id) # check that the attempt id and status have only been updated for the record that does not have a proctored # exam attempt id - self.assertEqual(len(VerifiedName.objects.filter(verification_attempt_id=self.idv_attempt_id)), 1) + self.assertEqual(len(VerifiedName.objects.filter(platform_verification_attempt_id=self.idv_attempt_id)), 1) self.assertEqual(len(VerifiedName.objects.filter(status=VerifiedNameStatus.SUBMITTED)), 1) @ddt.data( - ('created', VerifiedNameStatus.PENDING), - ('submitted', VerifiedNameStatus.SUBMITTED), - ('approved', VerifiedNameStatus.APPROVED), - ('denied', VerifiedNameStatus.DENIED) + (IDV_ATTEMPT_CREATED, VerifiedNameStatus.PENDING), + (IDV_ATTEMPT_PENDING, VerifiedNameStatus.SUBMITTED), + (IDV_ATTEMPT_APPROVED, VerifiedNameStatus.APPROVED), + (IDV_ATTEMPT_DENIED, VerifiedNameStatus.DENIED) ) @ddt.unpack - def test_idv_update_one_verified_name(self, idv_status, expected_status): + def test_idv_update_one_verified_name(self, idv_signal, expected_status): """ If a VerifiedName(s) for a user and verified name exist, ensure that it is updated properly """ @@ -210,19 +215,13 @@ def test_idv_update_one_verified_name(self, idv_status, expected_status): user=self.user, verified_name=self.verified_name, profile_name=self.profile_name, - verification_attempt_id=self.idv_attempt_id + platform_verification_attempt_id=self.idv_attempt_id ) - idv_attempt_handler( - self.idv_attempt_id, - self.user.id, - idv_status, - self.verified_name, - self.profile_name - ) + self._handle_idv_event(idv_signal, self.idv_attempt_id) # check that the attempt id and status have been updated for all three VerifiedNames - self.assertEqual(len(VerifiedName.objects.filter(verification_attempt_id=self.idv_attempt_id)), 1) + self.assertEqual(len(VerifiedName.objects.filter(platform_verification_attempt_id=self.idv_attempt_id)), 1) self.assertEqual(len(VerifiedName.objects.filter(status=expected_status)), 1) # If the status is approved, ensure that the post_save signal is called @@ -231,25 +230,6 @@ def test_idv_update_one_verified_name(self, idv_status, expected_status): else: mock_signal.assert_not_called() - @ddt.data( - 'ready', - 'must_retry', - ) - @patch('edx_name_affirmation.tasks.idv_update_verified_name_task.delay') - def test_idv_non_trigger_status(self, status, mock_task): - """ - Test that a celery task is not triggered if a non-relevant status is received - """ - idv_attempt_handler( - self.idv_attempt_id, - self.user.id, - status, - self.verified_name, - self.profile_name - ) - - mock_task.assert_not_called() - @patch('edx_name_affirmation.tasks.delete_verified_name_task.delay') def test_idv_delete_handler(self, mock_task): """ @@ -257,7 +237,7 @@ def test_idv_delete_handler(self, mock_task): """ mock_idv_object = MagicMock() mock_idv_object.id = 'abcdef' - idv_delete_handler( + platform_verification_delete_handler( {}, mock_idv_object, '', diff --git a/edx_name_affirmation/tests/test_tasks.py b/edx_name_affirmation/tests/test_tasks.py index 58b4485..8c3aab7 100644 --- a/edx_name_affirmation/tests/test_tasks.py +++ b/edx_name_affirmation/tests/test_tasks.py @@ -63,7 +63,7 @@ def test_idv_delete(self): Assert that only relevant VerifiedNames are deleted for a given idv_attempt_id """ # associated test object with an idv attempt - self.verified_name_obj.verification_attempt_id = self.idv_attempt_id + self.verified_name_obj.platform_verification_attempt_id = self.idv_attempt_id self.verified_name_obj.save() other_attempt_id = 123456 @@ -73,7 +73,7 @@ def test_idv_delete(self): user=self.user, verified_name='Jonathan X Doe', profile_name='Jon D', - verification_attempt_id=self.idv_attempt_id + platform_verification_attempt_id=self.idv_attempt_id ).save() # create VerifiedName not associated with idv attempt @@ -81,7 +81,7 @@ def test_idv_delete(self): user=self.user, verified_name='Jonathan X Doe', profile_name='Jon D', - verification_attempt_id=other_attempt_id + platform_verification_attempt_id=other_attempt_id ) other_verified_name_obj.save() @@ -91,8 +91,8 @@ def test_idv_delete(self): ) # check that there is only VerifiedName object - self.assertEqual(len(VerifiedName.objects.filter(verification_attempt_id=self.idv_attempt_id)), 0) - self.assertEqual(len(VerifiedName.objects.filter(verification_attempt_id=other_attempt_id)), 1) + self.assertEqual(len(VerifiedName.objects.filter(platform_verification_attempt_id=self.idv_attempt_id)), 0) + self.assertEqual(len(VerifiedName.objects.filter(platform_verification_attempt_id=other_attempt_id)), 1) def test_proctoring_delete(self): """ From 81a41c31815d31fa458ffdbef71245f906f4b493 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Mon, 30 Sep 2024 10:11:24 -0400 Subject: [PATCH 05/12] test: new test case --- edx_name_affirmation/tasks.py | 2 ++ edx_name_affirmation/tests/test_handlers.py | 32 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/edx_name_affirmation/tasks.py b/edx_name_affirmation/tasks.py index f6b8b46..e91c2af 100644 --- a/edx_name_affirmation/tasks.py +++ b/edx_name_affirmation/tasks.py @@ -86,6 +86,8 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st ) else: # otherwise if there are no entries, we want to create one. + # TODO: There is an existing bug here where if a user has an existing verified name + # from proctoring and goes through idv again a new verified name is not created. user = User.objects.get(id=user_id) verified_name = VerifiedName.objects.create( user=user, diff --git a/edx_name_affirmation/tests/test_handlers.py b/edx_name_affirmation/tests/test_handlers.py index 70bdb54..678d409 100644 --- a/edx_name_affirmation/tests/test_handlers.py +++ b/edx_name_affirmation/tests/test_handlers.py @@ -199,6 +199,38 @@ def test_idv_does_not_update_verified_name_by_proctoring(self): self.assertEqual(len(VerifiedName.objects.filter(platform_verification_attempt_id=self.idv_attempt_id)), 1) self.assertEqual(len(VerifiedName.objects.filter(status=VerifiedNameStatus.SUBMITTED)), 1) + def test_idv_does_not_update_old_verification_types(self): + """ + The verfication_attempt_id field is no longer supported by edx-platform. These records should no be + updated by idv events. + """ + VerifiedName.objects.create( + user=self.user, + verified_name=self.verified_name, + profile_name=self.profile_name, + verification_attempt_id=123, + status=VerifiedNameStatus.APPROVED, + ) + VerifiedName.objects.create( + user=self.user, + verified_name=self.verified_name, + profile_name=self.profile_name, + verification_attempt_id=456, + status=VerifiedNameStatus.SUBMITTED, + ) + + VerifiedName.objects.create(user=self.user, verified_name=self.verified_name, profile_name=self.profile_name) + self._handle_idv_event(IDV_ATTEMPT_CREATED, self.idv_attempt_id) + # new name linked + self.assertEqual(len(VerifiedName.objects.filter( + status=VerifiedNameStatus.PENDING, + platform_verification_attempt_id=self.idv_attempt_id, + )), 1) + + # old records remain untouched + self.assertEqual(len(VerifiedName.objects.filter(status=VerifiedNameStatus.SUBMITTED)), 1) + self.assertEqual(len(VerifiedName.objects.filter(status=VerifiedNameStatus.APPROVED)), 1) + @ddt.data( (IDV_ATTEMPT_CREATED, VerifiedNameStatus.PENDING), (IDV_ATTEMPT_PENDING, VerifiedNameStatus.SUBMITTED), From 932e8170b3f8c87e7513dc28fffe4dede97e84f5 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Mon, 30 Sep 2024 10:15:58 -0400 Subject: [PATCH 06/12] feat: install openedx-events --- requirements/base.in | 2 ++ requirements/base.txt | 21 ++++++++++++++++++--- requirements/dev.txt | 29 ++--------------------------- requirements/doc.txt | 18 ++++++++++++++++-- requirements/quality.txt | 12 ------------ requirements/test.txt | 21 ++++++++++++++++++++- 6 files changed, 58 insertions(+), 45 deletions(-) diff --git a/requirements/base.in b/requirements/base.in index 1f7b965..8e1aa3f 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -12,3 +12,5 @@ edx-celeryutils edx-django-utils>=3.12.0 edx-drf-extensions edx-toggles # Feature toggles management + +openedx-events \ No newline at end of file diff --git a/requirements/base.txt b/requirements/base.txt index 86555cc..193167d 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -8,6 +8,8 @@ amqp==5.2.0 # via kombu asgiref==3.8.1 # via django +attrs==24.2.0 + # via openedx-events billiard==4.2.0 # via celery celery==5.4.0 @@ -57,6 +59,7 @@ django==4.2.16 # edx-drf-extensions # edx-toggles # jsonfield + # openedx-events django-config-models==2.7.0 # via -r requirements/base.in django-crum==0.7.9 @@ -92,6 +95,8 @@ drf-yasg==1.21.7 # via edx-api-doc-tools edx-api-doc-tools==1.8.0 # via -r requirements/base.in +edx-ccx-keys==1.3.0 + # via openedx-events edx-celeryutils==1.3.0 # via -r requirements/base.in edx-django-utils==5.15.0 @@ -100,12 +105,18 @@ edx-django-utils==5.15.0 # django-config-models # edx-drf-extensions # edx-toggles + # openedx-events edx-drf-extensions==10.4.0 # via -r requirements/base.in -edx-opaque-keys==2.11.0 - # via edx-drf-extensions +edx-opaque-keys[django]==2.11.0 + # via + # edx-ccx-keys + # edx-drf-extensions + # openedx-events edx-toggles==5.2.0 # via -r requirements/base.in +fastavro==1.9.7 + # via openedx-events idna==3.8 # via requests inflection==0.5.1 @@ -120,6 +131,8 @@ markupsafe==2.1.5 # via jinja2 newrelic==9.13.0 # via edx-django-utils +openedx-events==9.14.1 + # via -r requirements/base.in packaging==24.1 # via drf-yasg pbr==6.1.0 @@ -153,7 +166,9 @@ requests==2.32.3 semantic-version==2.10.0 # via edx-drf-extensions six==1.16.0 - # via python-dateutil + # via + # edx-ccx-keys + # python-dateutil sqlparse==0.5.1 # via django stevedore==5.3.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index bea86a5..392992b 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -33,10 +33,6 @@ certifi==2024.8.30 # via # -r requirements/quality.txt # requests -cffi==1.17.1 - # via - # -r requirements/quality.txt - # cryptography chardet==5.2.0 # via # -r requirements/ci.txt @@ -69,10 +65,6 @@ colorama==0.4.6 # tox coverage==7.6.1 # via -r requirements/ci.txt -cryptography==43.0.1 - # via - # -r requirements/quality.txt - # secretstorage diff-cover==9.1.1 # via -r requirements/dev.in dill==0.3.8 @@ -127,11 +119,6 @@ jaraco-functools==4.0.2 # via # -r requirements/quality.txt # keyring -jeepney==0.8.0 - # via - # -r requirements/quality.txt - # keyring - # secretstorage jinja2==3.1.4 # via # -r requirements/quality.txt @@ -141,12 +128,8 @@ keyring==25.3.0 # via # -r requirements/quality.txt # twine -lxml[html-clean,html_clean]==5.3.0 - # via - # edx-i18n-tools - # lxml-html-clean -lxml-html-clean==0.2.2 - # via lxml +lxml[html_clean]==5.3.0 + # via edx-i18n-tools markdown-it-py==3.0.0 # via # -r requirements/quality.txt @@ -207,10 +190,6 @@ polib==1.2.0 # via edx-i18n-tools pycodestyle==2.12.1 # via -r requirements/quality.txt -pycparser==2.22 - # via - # -r requirements/quality.txt - # cffi pydantic==2.9.0 # via # -r requirements/quality.txt @@ -293,10 +272,6 @@ rstcheck-core==1.2.1 # via # -r requirements/quality.txt # rstcheck -secretstorage==3.3.3 - # via - # -r requirements/quality.txt - # keyring shellingham==1.5.4 # via # -r requirements/quality.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index ab0eecb..a478942 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -10,6 +10,8 @@ amqp==5.2.0 # via kombu asgiref==3.8.1 # via django +attrs==24.2.0 + # via openedx-events babel==2.16.0 # via sphinx billiard==4.2.0 @@ -61,6 +63,7 @@ django==4.2.16 # edx-drf-extensions # edx-toggles # jsonfield + # openedx-events django-config-models==2.7.0 # via -r requirements/base.in django-crum==0.7.9 @@ -104,6 +107,8 @@ drf-yasg==1.21.7 # via edx-api-doc-tools edx-api-doc-tools==1.8.0 # via -r requirements/base.in +edx-ccx-keys==1.3.0 + # via openedx-events edx-celeryutils==1.3.0 # via -r requirements/base.in edx-django-utils==5.15.0 @@ -112,14 +117,20 @@ edx-django-utils==5.15.0 # django-config-models # edx-drf-extensions # edx-toggles + # openedx-events edx-drf-extensions==10.4.0 # via -r requirements/base.in -edx-opaque-keys==2.11.0 - # via edx-drf-extensions +edx-opaque-keys[django]==2.11.0 + # via + # edx-ccx-keys + # edx-drf-extensions + # openedx-events edx-sphinx-theme==3.1.0 # via -r requirements/doc.in edx-toggles==5.2.0 # via -r requirements/base.in +fastavro==1.9.7 + # via openedx-events idna==3.8 # via requests imagesize==1.4.1 @@ -140,6 +151,8 @@ newrelic==9.13.0 # via edx-django-utils nh3==0.2.18 # via readme-renderer +openedx-events==9.14.1 + # via -r requirements/base.in packaging==24.1 # via # drf-yasg @@ -189,6 +202,7 @@ semantic-version==2.10.0 # via edx-drf-extensions six==1.16.0 # via + # edx-ccx-keys # edx-sphinx-theme # pockets # python-dateutil diff --git a/requirements/quality.txt b/requirements/quality.txt index 932eade..a465ef4 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -16,8 +16,6 @@ backports-tarfile==1.2.0 # via jaraco-context certifi==2024.8.30 # via requests -cffi==1.17.1 - # via cryptography charset-normalizer==3.3.2 # via requests click==8.1.7 @@ -30,8 +28,6 @@ click-log==0.4.0 # via edx-lint code-annotations==1.8.0 # via edx-lint -cryptography==43.0.1 - # via secretstorage dill==0.3.8 # via pylint django==4.2.16 @@ -60,10 +56,6 @@ jaraco-context==6.0.1 # via keyring jaraco-functools==4.0.2 # via keyring -jeepney==0.8.0 - # via - # keyring - # secretstorage jinja2==3.1.4 # via code-annotations keyring==25.3.0 @@ -90,8 +82,6 @@ platformdirs==4.2.2 # via pylint pycodestyle==2.12.1 # via -r requirements/quality.in -pycparser==2.22 - # via cffi pydantic==2.9.0 # via rstcheck-core pydantic-core==2.23.2 @@ -138,8 +128,6 @@ rstcheck==6.2.4 # via -r requirements/quality.in rstcheck-core==1.2.1 # via rstcheck -secretstorage==3.3.3 - # via keyring shellingham==1.5.4 # via typer six==1.16.0 diff --git a/requirements/test.txt b/requirements/test.txt index 595e511..d14ef23 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -12,6 +12,10 @@ asgiref==3.8.1 # via # -r requirements/base.txt # django +attrs==24.2.0 + # via + # -r requirements/base.txt + # openedx-events billiard==4.2.0 # via # -r requirements/base.txt @@ -84,6 +88,7 @@ ddt==1.7.2 # edx-drf-extensions # edx-toggles # jsonfield + # openedx-events django-config-models==2.7.0 # via -r requirements/base.txt django-crum==0.7.9 @@ -127,6 +132,10 @@ drf-yasg==1.21.7 # edx-api-doc-tools edx-api-doc-tools==1.8.0 # via -r requirements/base.txt +edx-ccx-keys==1.3.0 + # via + # -r requirements/base.txt + # openedx-events edx-celeryutils==1.3.0 # via -r requirements/base.txt edx-django-utils==5.15.0 @@ -135,14 +144,21 @@ edx-django-utils==5.15.0 # django-config-models # edx-drf-extensions # edx-toggles + # openedx-events edx-drf-extensions==10.4.0 # via -r requirements/base.txt -edx-opaque-keys==2.11.0 +edx-opaque-keys[django]==2.11.0 # via # -r requirements/base.txt + # edx-ccx-keys # edx-drf-extensions + # openedx-events edx-toggles==5.2.0 # via -r requirements/base.txt +fastavro==1.9.7 + # via + # -r requirements/base.txt + # openedx-events idna==3.8 # via # -r requirements/base.txt @@ -175,6 +191,8 @@ newrelic==9.13.0 # via # -r requirements/base.txt # edx-django-utils +openedx-events==9.14.1 + # via -r requirements/base.txt packaging==24.1 # via # -r requirements/base.txt @@ -247,6 +265,7 @@ semantic-version==2.10.0 six==1.16.0 # via # -r requirements/base.txt + # edx-ccx-keys # python-dateutil sqlparse==0.5.1 # via From 220af767b94f8764a2adff06d271b77c6a6452e7 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Mon, 30 Sep 2024 10:21:29 -0400 Subject: [PATCH 07/12] style: isort --- edx_name_affirmation/handlers.py | 10 +++++----- edx_name_affirmation/tests/test_handlers.py | 9 ++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/edx_name_affirmation/handlers.py b/edx_name_affirmation/handlers.py index 1512c6c..75bc7da 100644 --- a/edx_name_affirmation/handlers.py +++ b/edx_name_affirmation/handlers.py @@ -4,17 +4,17 @@ import logging -from django.contrib.auth import get_user_model -from django.db.models.signals import post_save -from django.dispatch.dispatcher import receiver - from openedx_events.learning.signals import ( IDV_ATTEMPT_APPROVED, IDV_ATTEMPT_CREATED, IDV_ATTEMPT_DENIED, - IDV_ATTEMPT_PENDING, + IDV_ATTEMPT_PENDING ) +from django.contrib.auth import get_user_model +from django.db.models.signals import post_save +from django.dispatch.dispatcher import receiver + from edx_name_affirmation.models import VerifiedName from edx_name_affirmation.signals import VERIFIED_NAME_APPROVED from edx_name_affirmation.statuses import VerifiedNameStatus diff --git a/edx_name_affirmation/tests/test_handlers.py b/edx_name_affirmation/tests/test_handlers.py index 678d409..3bb07f8 100644 --- a/edx_name_affirmation/tests/test_handlers.py +++ b/edx_name_affirmation/tests/test_handlers.py @@ -4,18 +4,17 @@ import ddt from mock import MagicMock, patch - -from django.contrib.auth import get_user_model -from django.test import TestCase - from openedx_events.learning.data import UserData, UserPersonalData, VerificationAttemptData from openedx_events.learning.signals import ( IDV_ATTEMPT_APPROVED, IDV_ATTEMPT_CREATED, IDV_ATTEMPT_DENIED, - IDV_ATTEMPT_PENDING, + IDV_ATTEMPT_PENDING ) +from django.contrib.auth import get_user_model +from django.test import TestCase + from edx_name_affirmation.handlers import ( handle_idv_event, platform_verification_delete_handler, From 010ced2e7cc1f93dd567a1e17a7c8c164e9ff0cd Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Mon, 30 Sep 2024 12:45:51 -0400 Subject: [PATCH 08/12] feat: update version and changelog --- CHANGELOG.rst | 5 +++++ edx_name_affirmation/__init__.py | 2 +- edx_name_affirmation/tests/test_handlers.py | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 86e2b15..16e2707 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,8 +13,13 @@ Change Log Unreleased ~~~~~~~~~~ + +[3.0.0] - 2024-09-30 +~~~~~~~~~~~~~~~~~~~~ * Add platform verification id field to the VerifiedName model * Integrate platform verification id into app +* Added event handlers for new IDV events on the VerifiedName model +* Removed event handlers for SoftwareSecurePhotoVerification updates. This is a breaking change. [2.4.0] - 2024-04-23 ~~~~~~~~~~~~~~~~~~~~ diff --git a/edx_name_affirmation/__init__.py b/edx_name_affirmation/__init__.py index 89d2851..a600169 100644 --- a/edx_name_affirmation/__init__.py +++ b/edx_name_affirmation/__init__.py @@ -2,4 +2,4 @@ Django app housing name affirmation logic. """ -__version__ = '2.4.2' +__version__ = '3.0.0' diff --git a/edx_name_affirmation/tests/test_handlers.py b/edx_name_affirmation/tests/test_handlers.py index 3bb07f8..33dc6c1 100644 --- a/edx_name_affirmation/tests/test_handlers.py +++ b/edx_name_affirmation/tests/test_handlers.py @@ -63,7 +63,7 @@ def test_post_save_verified_name_approved(self, status, should_send): user=self.user, verified_name='Jonathan Doe', profile_name=self.profile_name, - verification_attempt_id=self.idv_attempt_id + platform_verification_attempt_id=self.idv_attempt_id ) verified_name_obj.status = status verified_name_obj.save() From 2596ec44ee05e90b4ca4a481c3ddcee8c5fb55cc Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Mon, 30 Sep 2024 15:18:15 -0400 Subject: [PATCH 09/12] fix: create verified name if none exists --- edx_name_affirmation/handlers.py | 1 + edx_name_affirmation/tasks.py | 41 +++++++++++---------- edx_name_affirmation/tests/test_handlers.py | 2 - 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/edx_name_affirmation/handlers.py b/edx_name_affirmation/handlers.py index 75bc7da..a976190 100644 --- a/edx_name_affirmation/handlers.py +++ b/edx_name_affirmation/handlers.py @@ -69,6 +69,7 @@ def handle_idv_event(sender, signal, **kwargs): # pylint: disable=unused-argume elif signal == IDV_ATTEMPT_DENIED: status = VerifiedNameStatus.DENIED else: + log.info(f'IDV_ATTEMPT {signal} signal not recognized') # driven by receiver decorator so should never happen return log.info(f'IDV_ATTEMPT {status} signal triggering Celery task for user {user.id} ' diff --git a/edx_name_affirmation/tasks.py b/edx_name_affirmation/tasks.py index e91c2af..ca6b370 100644 --- a/edx_name_affirmation/tasks.py +++ b/edx_name_affirmation/tasks.py @@ -84,27 +84,28 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st status=name_affirmation_status ) ) - else: - # otherwise if there are no entries, we want to create one. - # TODO: There is an existing bug here where if a user has an existing verified name - # from proctoring and goes through idv again a new verified name is not created. - user = User.objects.get(id=user_id) - verified_name = VerifiedName.objects.create( - user=user, - verified_name=photo_id_name, - profile_name=full_name, - platform_verification_attempt_id=attempt_id, - status=name_affirmation_status, - ) - log.error( - 'Created VerifiedName for user={user_id} to have status={status} ' - 'and platform_verification_attempt_id={attempt_id}, because no matching ' - 'attempt_id or verified_name were found.'.format( - user_id=user_id, - attempt_id=attempt_id, - status=verified_name.status - ) + + if updated_for_attempt_id or verified_name_qs: + return + + # if there are no entries to update, we want to create one. + user = User.objects.get(id=user_id) + verified_name = VerifiedName.objects.create( + user=user, + verified_name=photo_id_name, + profile_name=full_name, + platform_verification_attempt_id=attempt_id, + status=name_affirmation_status, + ) + log.error( + 'Created VerifiedName for user={user_id} to have status={status} ' + 'and platform_verification_attempt_id={attempt_id}, because no matching ' + 'attempt_id or verified_name were found.'.format( + user_id=user_id, + attempt_id=attempt_id, + status=verified_name.status ) + ) @shared_task( diff --git a/edx_name_affirmation/tests/test_handlers.py b/edx_name_affirmation/tests/test_handlers.py index 33dc6c1..db60fc9 100644 --- a/edx_name_affirmation/tests/test_handlers.py +++ b/edx_name_affirmation/tests/test_handlers.py @@ -218,9 +218,7 @@ def test_idv_does_not_update_old_verification_types(self): status=VerifiedNameStatus.SUBMITTED, ) - VerifiedName.objects.create(user=self.user, verified_name=self.verified_name, profile_name=self.profile_name) self._handle_idv_event(IDV_ATTEMPT_CREATED, self.idv_attempt_id) - # new name linked self.assertEqual(len(VerifiedName.objects.filter( status=VerifiedNameStatus.PENDING, platform_verification_attempt_id=self.idv_attempt_id, From c1ebde1fecbc0ffab9a5d107e996f087a14984f6 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Mon, 30 Sep 2024 15:18:41 -0400 Subject: [PATCH 10/12] style: newline --- requirements/base.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/base.in b/requirements/base.in index 8e1aa3f..0dd53f5 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -13,4 +13,4 @@ edx-django-utils>=3.12.0 edx-drf-extensions edx-toggles # Feature toggles management -openedx-events \ No newline at end of file +openedx-events From c0b518416b153729b84aeedc8964dcb92bd67f24 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Mon, 30 Sep 2024 15:30:43 -0400 Subject: [PATCH 11/12] style: fix bad style on return statement --- edx_name_affirmation/tasks.py | 39 ++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/edx_name_affirmation/tasks.py b/edx_name_affirmation/tasks.py index ca6b370..1e58b37 100644 --- a/edx_name_affirmation/tasks.py +++ b/edx_name_affirmation/tasks.py @@ -47,6 +47,7 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st & Q(user__id=user_id) & Q(verified_name=photo_id_name) ).order_by('-created') + verified_names_updated = False if verified_names: # if there are VerifiedName objects, we want to update existing entries # for each attempt with no attempt id (either proctoring or idv), update attempt id @@ -57,6 +58,7 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st ).update(platform_verification_attempt_id=attempt_id) if updated_for_attempt_id: + verified_names_updated = True log.info( 'Updated VerifiedNames for user={user_id} to platform_verification_attempt_id={attempt_id}'.format( user_id=user_id, @@ -75,6 +77,7 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st for verified_name_obj in verified_name_qs: verified_name_obj.status = name_affirmation_status verified_name_obj.save() + verified_names_updated = True log.info( 'Updated VerifiedNames for user={user_id} with platform_verification_attempt_id={attempt_id} to ' @@ -85,27 +88,25 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st ) ) - if updated_for_attempt_id or verified_name_qs: - return - # if there are no entries to update, we want to create one. - user = User.objects.get(id=user_id) - verified_name = VerifiedName.objects.create( - user=user, - verified_name=photo_id_name, - profile_name=full_name, - platform_verification_attempt_id=attempt_id, - status=name_affirmation_status, - ) - log.error( - 'Created VerifiedName for user={user_id} to have status={status} ' - 'and platform_verification_attempt_id={attempt_id}, because no matching ' - 'attempt_id or verified_name were found.'.format( - user_id=user_id, - attempt_id=attempt_id, - status=verified_name.status + if not verified_names_updated: + user = User.objects.get(id=user_id) + verified_name = VerifiedName.objects.create( + user=user, + verified_name=photo_id_name, + profile_name=full_name, + platform_verification_attempt_id=attempt_id, + status=name_affirmation_status, + ) + log.error( + 'Created VerifiedName for user={user_id} to have status={status} ' + 'and platform_verification_attempt_id={attempt_id}, because no matching ' + 'attempt_id or verified_name were found.'.format( + user_id=user_id, + attempt_id=attempt_id, + status=verified_name.status + ) ) - ) @shared_task( From 8e0d3d6b735c069cc2df542a4598b6f0647b8137 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Fri, 4 Oct 2024 13:26:11 -0400 Subject: [PATCH 12/12] style: review feedback --- edx_name_affirmation/handlers.py | 2 +- edx_name_affirmation/tasks.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/edx_name_affirmation/handlers.py b/edx_name_affirmation/handlers.py index a976190..6fa918b 100644 --- a/edx_name_affirmation/handlers.py +++ b/edx_name_affirmation/handlers.py @@ -73,7 +73,7 @@ def handle_idv_event(sender, signal, **kwargs): # pylint: disable=unused-argume return log.info(f'IDV_ATTEMPT {status} signal triggering Celery task for user {user.id} ' - f'with photo_id_name {event_data.name}') + f'with name {event_data.name}') idv_update_verified_name_task.delay( event_data.attempt_id, user.id, diff --git a/edx_name_affirmation/tasks.py b/edx_name_affirmation/tasks.py index 1e58b37..4475f8b 100644 --- a/edx_name_affirmation/tasks.py +++ b/edx_name_affirmation/tasks.py @@ -216,7 +216,7 @@ def delete_verified_name_task(self, platform_verification_attempt_id, proctoring log_message['attempt_id'] = proctoring_attempt_id if verified_names: - log.info( # there's a bug in this log message + log.info( 'Deleting {num_names} VerifiedName(s) associated with {field_name}=' '{platform_verification_attempt_id}'.format( num_names=len(verified_names),