Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow verified name to be created if user is trying to verify with the same name #93

Merged
merged 1 commit into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ Change Log

Unreleased
~~~~~~~~~~

[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

[2.3.3] - 2022-04-21
~~~~~~~~~~~~~~~~~~~~
* Leverage edx-api-doc-tools to provide better swagger documentation for the RESTFul API endpoints
Expand Down
2 changes: 1 addition & 1 deletion edx_name_affirmation/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
Django app housing name affirmation logic.
"""

__version__ = '2.3.3'
__version__ = '2.3.4'

default_app_config = 'edx_name_affirmation.apps.EdxNameAffirmationConfig' # pylint: disable=invalid-name
11 changes: 10 additions & 1 deletion edx_name_affirmation/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from edx_django_utils.monitoring import set_code_owner_attribute

from django.contrib.auth import get_user_model
from django.db.models import Q

from edx_name_affirmation.models import VerifiedName
from edx_name_affirmation.statuses import VerifiedNameStatus
Expand Down Expand Up @@ -37,7 +38,15 @@ def idv_update_verified_name_task(self, attempt_id, user_id, name_affirmation_st
'status': name_affirmation_status
}
)
verified_names = VerifiedName.objects.filter(user__id=user_id, verified_name=photo_id_name).order_by('-created')
# get all verified names that are either not associated with an IDV attempt, or
# only associated with the IDV attempt for which we received an update. We do not
# 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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#NIT add a comment here to explain that when multiple VerifiedName records exists, there might be records we want to update that is not the latest created.

(Q(verification_attempt_id=attempt_id) | Q(verification_attempt_id__isnull=True))
& Q(user__id=user_id)
& Q(verified_name=photo_id_name)
).order_by('-created')
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
Expand Down
29 changes: 29 additions & 0 deletions edx_name_affirmation/tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,35 @@ def test_idv_update_multiple_verified_names(self, idv_status, expected_status):
self.assertEqual(len(VerifiedName.objects.filter(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):
"""
Test that if a user attempts IDV again with the same name as previous attempts, we still create a new record
"""
previous_id = 1234567

VerifiedName.objects.create(
user=self.user,
verified_name=self.verified_name,
profile_name=self.profile_name,
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
)

verified_name = VerifiedName.objects.get(verification_attempt_id=self.idv_attempt_id)
self.assertEqual(verified_name.status, VerifiedNameStatus.SUBMITTED)

previous_name = VerifiedName.objects.get(verification_attempt_id=previous_id)
self.assertEqual(previous_name.status, VerifiedNameStatus.DENIED)

def test_idv_does_not_update_verified_name_by_proctoring(self):
"""
If the idv handler is triggered, ensure that the idv attempt info does not update any verified name
Expand Down