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: Add ACHv2 support for iOS #861

Merged
merged 35 commits into from
Apr 1, 2022

Conversation

charliecruzan-stripe
Copy link
Collaborator

@charliecruzan-stripe charliecruzan-stripe commented Mar 24, 2022

Summary

This exposes 4 methods that React Native devs can use:

  • collectBankAccountForPayment: opens a modal for customers to provide their US Bank account info and attach it to the specified payment intent
  • collectBankAccountForSetup: opens a modal for customers to provide their US Bank account info and attach it to the specified setup intent
  • verifyMicrodepositsForPayment: verifies microdeposits for a payment intent
  • verifyMicrodepositsForSetup: verifies microdeposits for a setup intent

These all match methods exposed in stripe.js (in naming and parameters):

Motivation

add support for ACHv2 via bindings on iOS since GA was yesterday,

Testing

  • I tested this manually
  • I added automated tests

Tests are failing because Webdriver isn't playing nicely with macos-latest on GHA, but we need to upgrade to support the newer version of stripe-ios :/ will look into that, but the tests are passing locally on my machine.

Documentation

Select one:

  • I have added relevant documentation for my changes
  • This PR does not result in any developer-facing changes.

@charliecruzan-stripe charliecruzan-stripe changed the title feat: Add ACHv2 support feat: Add ACHv2 support for iOS Mar 29, 2022
@charliecruzan-stripe
Copy link
Collaborator Author

Tests are failing because Webdriver isn't playing nicely with macos-latest on GHA, but we need to upgrade to support the newer version of stripe-ios :/ will look into that, but the tests are passing locally on my machine.

@@ -47,6 +47,8 @@ class PaymentMethodFactory {
return try createAfterpayClearpayPaymentMethodParams()
case STPPaymentMethodType.klarna:
return try createKlarnaPaymentMethodParams()
case STPPaymentMethodType.USBankAccount:
return nil

Choose a reason for hiding this comment

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

why does this return nil?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for the USBankAccount payment method type, the payment method is created/attached to the intent during collectBankAccountForPayment, so the STPPaymentIntentParams.paymentMethodParams aren't required in this case. Unless we want to allow folks to pass in account number, routing number, etc. directly (after collecting in their own UI, for instance)?

Choose a reason for hiding this comment

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

I believe that is a use case we want to support (it's supported on iOS currently)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah okay, thought that was a security issue and that was why we had a hosted modal for collecting bank account info. I can add that in easily then

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@csabol-stripe added in 9115057

Following this change, it's made me realize we should probably change the confirmPayment (and other similar methods) type to include two separate fields: payment method type and payment method data (right now it's just payment method data, which includes the type). That will make future additions like this one easier. That is out of scope for this PR though, but I will most likely land that change before our V1 release.

@charliecruzan-stripe charliecruzan-stripe merged commit e2d0a08 into master Apr 1, 2022
@charliecruzan-stripe charliecruzan-stripe deleted the charliecruzan-achv2-bindings branch April 1, 2022 17: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.

4 participants