From 3cc89c644686c34121ed447b0408af0c9e528278 Mon Sep 17 00:00:00 2001 From: Zacharis278 Date: Fri, 27 Sep 2024 16:07:18 -0400 Subject: [PATCH] feat: refactor existing tests for new IDV source --- edx_name_affirmation/handlers.py | 14 +- edx_name_affirmation/tests/test_handlers.py | 134 +++++++++----------- edx_name_affirmation/tests/test_tasks.py | 10 +- 3 files changed, 73 insertions(+), 85 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/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): """