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

feat(router): use only card number for card duplication check #57

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

ShankarSinghC
Copy link
Contributor

@ShankarSinghC ShankarSinghC commented Dec 11, 2023

Currently we are taking the hash of entire card details (payment method data) for card duplication check. In this case if a add card request is made for same card number but with a different card holder name, cvv or expiry it would pass the card duplication check and the card would get added even though the card number is same.
So this pr treats the card number as a unique field and performs the duplication based on it.

Test case :-

  1. Payment method create.
image 2) Payment method create for same merchant and customer, change `card_holder_name` keeping the the card number same.

image
3) Locker db still has only one card inserted in locker table.
image
4) Also there is only one entry in the hash table as well.
image

@ShankarSinghC ShankarSinghC self-assigned this Dec 11, 2023
Comment on lines +16 to 19
pub card_number: masking::StrongSecret<String>,
name_on_card: Option<String>,
card_exp_month: Option<String>,
card_exp_year: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

nit: in the future, we shouldn't have any of the fields on Card as public. We should either expose an interface for accessing the data as mutation

@NishantJoshi00 NishantJoshi00 merged commit 5781603 into main Dec 12, 2023
3 checks passed
@NishantJoshi00 NishantJoshi00 deleted the refactor-hash-check branch December 12, 2023 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants