-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Fix Rapyd error on billing address #5472
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is a good candidate for a unit test that validates the presence or absence of fields, based on what we expect should happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of suggestions for you.
I also see now we have 2 different methods for handling the address, def address
and def add_address
. Do you think there is an opportunity to refactor this to consolidate the logic of adding address details in the requests?
8a4a7c1
to
16dd170
Compare
Understood! Unit test added |
16dd170
to
9149e16
Compare
9149e16
to
0d3d012
Compare
0d3d012
to
8b88d5f
Compare
8b88d5f
to
55a7230
Compare
OPPS-397
Summary:
Fix Incorrect Handling of Phone Number in Customer Address Object Leads to Address Validation Errors in Rapyd
Unit Tests:
Finished in 0.067795 seconds.
54 tests, 254 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
796.52 tests/s, 3746.59 assertions/s
Remote Tests:
Finished in 71.616667 seconds.
55 tests, 140 assertions, 18 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
67.2727% passed
0.77 tests/s, 1.95 assertions/s
** Failing tests are not related to change, several payment methods are not available anymore, need to be validated with rapyd support **