From cbaceac60a39a4cc58376aed7f9966a21c4c2acb Mon Sep 17 00:00:00 2001 From: Alie Langston Date: Fri, 9 Sep 2022 08:42:47 -0400 Subject: [PATCH] fix: allow updates to proctoring verified name if it already exists --- CHANGELOG.rst | 4 ++ edx_name_affirmation/__init__.py | 2 +- edx_name_affirmation/tasks.py | 28 +++++----- edx_name_affirmation/tests/test_handlers.py | 61 ++++++++++++++++++++- 4 files changed, 79 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 621b710..ab7166a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,10 @@ Change Log Unreleased ~~~~~~~~~~ +[2.3.5] - 2022-09-09 +~~~~~~~~~~~~~~~~~~~~ +* Fix bug that prevents a verified name from being updated if the user already has an approved verified name associated with a proctored exam attempt + [2.3.4] - 2022-05-17 ~~~~~~~~~~~~~~~~~~~~ * Fix bug that prevents new verified names from being created if the user is trying to verify the same name diff --git a/edx_name_affirmation/__init__.py b/edx_name_affirmation/__init__.py index 06ef5fe..0fee05f 100644 --- a/edx_name_affirmation/__init__.py +++ b/edx_name_affirmation/__init__.py @@ -2,6 +2,6 @@ Django app housing name affirmation logic. """ -__version__ = '2.3.4' +__version__ = '2.3.5' default_app_config = 'edx_name_affirmation.apps.EdxNameAffirmationConfig' # pylint: disable=invalid-name diff --git a/edx_name_affirmation/tasks.py b/edx_name_affirmation/tasks.py index dfafd49..d76eb15 100644 --- a/edx_name_affirmation/tasks.py +++ b/edx_name_affirmation/tasks.py @@ -119,31 +119,33 @@ def proctoring_update_verified_name_task( Celery task for updating a verified name based on a proctoring attempt """ - # check if approved VerifiedName already exists for the user - verified_name = VerifiedName.objects.filter( + approved_verified_name = VerifiedName.objects.filter( user__id=user_id, status=VerifiedNameStatus.APPROVED ).order_by('-created').first() - if verified_name: - approved_verified_name = verified_name.verified_name - is_full_name_approved = approved_verified_name == full_name + + verified_name_for_exam = VerifiedName.objects.filter( + user__id=user_id, + proctored_exam_attempt_id=attempt_id + ).order_by('-created').first() + + # check if approved VerifiedName already exists for the user, and skip + # update if no VerifiedName has already been created for this specific exam + if approved_verified_name and not verified_name_for_exam: + is_full_name_approved = approved_verified_name.verified_name == full_name if not is_full_name_approved: log.warning( 'Full name for proctored_exam_attempt_id={attempt_id} is not equal ' 'to the most recent verified name verified_name_id={name_id}.'.format( attempt_id=attempt_id, - name_id=verified_name.id + name_id=approved_verified_name.id ) ) return - verified_name = VerifiedName.objects.filter( - user__id=user_id, - proctored_exam_attempt_id=attempt_id - ).order_by('-created').first() - if verified_name: - verified_name.status = name_affirmation_status - verified_name.save() + if verified_name_for_exam: + verified_name_for_exam.status = name_affirmation_status + verified_name_for_exam.save() log.info( 'Updated VerifiedName for user={user_id} with proctored_exam_attempt_id={attempt_id} ' 'to have status={status}'.format( diff --git a/edx_name_affirmation/tests/test_handlers.py b/edx_name_affirmation/tests/test_handlers.py index 7c764ba..6b2f3df 100644 --- a/edx_name_affirmation/tests/test_handlers.py +++ b/edx_name_affirmation/tests/test_handlers.py @@ -434,8 +434,9 @@ def test_proctoring_log_with_existing_approved_verified_name(self, should_names_ status=VerifiedNameStatus.APPROVED ) + additional_attempt_id = self.proctoring_attempt_id + 1 proctoring_attempt_handler( - self.proctoring_attempt_id, + additional_attempt_id, self.user.id, 'created', ('John' if should_names_differ else self.verified_name), @@ -449,7 +450,7 @@ def test_proctoring_log_with_existing_approved_verified_name(self, should_names_ 'Full name for proctored_exam_attempt_id={attempt_id} is not equal to the most recent verified ' 'name verified_name_id={verified_name_id}.' ).format( - attempt_id=self.proctoring_attempt_id, + attempt_id=additional_attempt_id, verified_name_id=verified_name.id ) @@ -499,3 +500,59 @@ def test_proctoring_delete_handler(self, mock_task): ) mock_task.assert_called_with(None, mock_proctoring_object.id) + + def test_proctoring_multiple_approved(self): + # create task for submitted exam + proctoring_attempt_handler( + self.proctoring_attempt_id, + self.user.id, + 'submitted', + 'John', + 'John', + True, + True, + True + ) + + # create task for submitted on another exam + other_attempt_id = self.proctoring_attempt_id + 1 + proctoring_attempt_handler( + other_attempt_id, + self.user.id, + 'submitted', + 'John', + 'John', + True, + True, + True + ) + + self.assertEqual(len(VerifiedName.objects.filter()), 2) + self.assertEqual(len(VerifiedName.objects.filter(status=VerifiedNameStatus.SUBMITTED)), 2) + + # create task for approved exam 1 + proctoring_attempt_handler( + self.proctoring_attempt_id, + self.user.id, + 'verified', + 'John', + 'John', + True, + True, + True + ) + + # create task for approved exam 2 + proctoring_attempt_handler( + other_attempt_id, + self.user.id, + 'verified', + 'John', + 'John', + True, + True, + True + ) + + self.assertEqual(len(VerifiedName.objects.filter()), 2) + self.assertEqual(len(VerifiedName.objects.filter(status=VerifiedNameStatus.APPROVED)), 2)