-
Notifications
You must be signed in to change notification settings - Fork 991
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
Enable bank payments in native Link #4534
Conversation
c56d0a9
to
27466bf
Compare
27466bf
to
6132647
Compare
|
||
var expectedPaymentMethodTypeForPassthroughMode: String? { | ||
switch type { | ||
case .card, .unparsable: |
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.
Just checking, should we not send this for .card
?
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.
As far as I know, that’s not necessary. Let me double-check!
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.
The backend will fall back to card
, but I added it anyway since that’s what Web is doing 🤷♂️
@@ -330,7 +335,7 @@ extension STPAPIClient { | |||
let parameters: [String: Any] = [ | |||
"credentials": ["consumer_session_client_secret": consumerSessionClientSecret], | |||
"request_surface": "ios_payment_element", | |||
"types": ["card"], | |||
"types": supportedPaymentDetailsTypes.map(\.rawValue), |
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.
Why do we use a Set instead of an Array for supportedPaymentDetailsType? It's a little clunkier in Swift, and I'm not sure if the underlying JSON response can really represent a Set. It was causing issues with ordering in our tests: #4549
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’ll change it to a list, now that I’ve learned about the test issues it can cause.
- Use list instead of set - Align with Web logic for ACH-enabled Elements sessions
var supportedPaymentMethodTypes = Set(ConsumerPaymentDetails.DetailsType.allCases) { | ||
didSet { | ||
// TODO(tillh-stripe) Update this as soon as adding bank accounts is supported | ||
addPaymentMethodButton.isHidden = !supportedPaymentMethodTypes.contains(.card) |
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 noticed that pressing Add payment method
in a bank-only session (orderedPaymentMethodTypes == [.USBankAccount]
) does nothing, so let’s hide it, @davidme-stripe?
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, it's not hooked up yet, I have a JIRA for this somewhere. We should plug it in once all this is working!
let canAcceptACH = elementsSession.orderedPaymentMethodTypes.contains(.USBankAccount) | ||
let isLinkCardBrand = elementsSession.linkSettings?.linkMode?.isPantherPayment ?? false | ||
return isLinkCardBrand && !canAcceptACH ? "card" : "bank_account" |
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 mimics behavior on the Web: If we’re in the Link card brand mode, but the merchant accepts ACH, then we want the payment method to be a full bank account.
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.
Interesting, okay!
Summary
This pull request adds support for bank payments to the native Link flow.
["card"]
, we now pass the supported payment details types through to thelistPaymentDetails
call.sharePaymentDetails
method to take in anexpectedPaymentMethodType
when sharing a bank account in passthrough mode.Motivation
Parity with the web flow it replaces.
Testing
Added end-to-end tests.
Changelog