-
Notifications
You must be signed in to change notification settings - Fork 83
payments: update Wise implementation to match API changes #1847
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: main
Are you sure you want to change the base?
Conversation
apps/payments/wise.py
Outdated
| account.bankDetails.bankAddress.country, | ||
| ] | ||
| ) | ||
| bank_name, _, bank_address = bank_info.partition("\n") |
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.
This feels like it might come back to bite us at some point - perhaps we should remove the institution column from BankAccount and just throw this all in the address field.
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.
Yep, this does seem fragile. However: it does seem useful in the admin UI (and perhaps elsewhere?) to have a small descriptive text field with the institution name alone.
I'll think about it some more. Another option could be to add validation logic so that we detect any likely breakage early.
8040734 to
4cbf020
Compare
NB: Redacts a few Cloudflare/Envoy, cookie, etc-related response headers
…bles/attributes Relates-to commit 559cc5d.
Relates-to commit 0c469bd.
ab1a647 to
4bba914
Compare
As described in #1844, the Wise API has been updated and we need to update our codebase accordingly.
An open question is whether our
BankAccountrecords for Wise accounts should use the same account-per-currency design model as our Stripe accounts. Currently, they still do, but I would like to explore whether it would make sense to consolidate to a single account ID (on which we could, for example, populate all of sort-code, account-number, SWIFT and IBAN).Creating this pull request as a draft, initially.
Resolves #1844.