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

SafeCharge: add card holder verification fields #5252

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

yunnydang
Copy link
Contributor

This change adds the optional middle_name and card_holder_verification fields
Local:
6017 tests, 80326 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Unit:
31 tests, 176 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:
35 tests, 93 assertions, 2 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
94.2857% passed

@yunnydang yunnydang requested a review from a team September 6, 2024 22:17
@yunnydang yunnydang self-assigned this Sep 6, 2024
Copy link
Contributor

@jcreiff jcreiff left a comment

Choose a reason for hiding this comment

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

Looks good to me!

I was curious to see if sending the card_holder_verification field would change the response sent back from SafeCharge, and it does look like they add a CardHolderNameVerification object to the response. You could add an assertion to the remote test to show that it's present, but I will ✅ this either way

@yunnydang yunnydang force-pushed the safecharge_add_card_holder_verification_fields branch from b3601df to 25410f8 Compare September 9, 2024 16:52
@yunnydang yunnydang force-pushed the safecharge_add_card_holder_verification_fields branch from 25410f8 to d615d35 Compare September 10, 2024 16:44
@yunnydang yunnydang merged commit d615d35 into master Sep 10, 2024
5 checks passed
@yunnydang yunnydang deleted the safecharge_add_card_holder_verification_fields branch September 10, 2024 16:45
This pull request was closed.
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.

2 participants