stripe / stripe-terminal-react-native

React Native SDK for Stripe Terminal
https://stripe.com/docs/terminal/payments/setup-integration?terminal-sdk-platform=react-native
MIT License
105 stars 50 forks source link

Should it be possible to connect to a simulated local mobile reader? #439

Closed mihaildu closed 1 year ago

mihaildu commented 1 year ago

Describe the bug I'm trying to connect to a simulated local mobile reader and I'm getting this error

const {reader: connectedReader, error} = await connectLocalMobileReader({
  reader,
  locationId,
});

// error = [TypeError: undefined is not a function]

To Reproduce Discover simulated local mobile readers & try to connect

const {error} = await discoverReaders({
  discoveryMethod: 'localMobile',
  simulated: true,
});
...
// discoveredReaders - I have simulated Stripe S700, wisePosE, Simulated Etna and verifoneP400 here
// connectToLocalMobileReader calls the above code
connectToLocalMobileReader(discoveredReaders[0])

Expected behavior Not sure, I'm just curious if this should work or not with simulated local mobile readers

Stripe Terminal React Native SDK version

Smartphone (please complete the following information):

billfinn-stripe commented 1 year ago

Good question -- no, connecting to simulated local mobile readers is not supported. (In fact, currently, local mobile (aka, tap-to-pay) is not supported by ReactNative at all.)

I'll leave this issue open as a reminder to improve the error message and/or documentation.

FaggioniHQ commented 1 year ago

Hello @billfinn-stripe ,

I'm implementing tap-to-pay on a react native app using the iOS terminal SDK. The only issue that I found is that the react SDK uses the StripeTerminal version 2.13.0, and for supporting iPhone payments we need to use the StripeTerminal version 2.17.1. This leads to a conflict between the SDKs.

My question is, Are you planning to bump the react native SDK to use StripeTerminal 2.17.1?

We support payments using Bluetooth readers with the react native SDK, we would like to keep both methods.

FaggioniHQ commented 1 year ago

Hello again @billfinn-stripe ,

I review the repo and noticed that you already working on it. That's perfect.

Any ETA for this?

Thanks in advance

billfinn-stripe commented 1 year ago

Hi @FaggioniHQ -- no ETA yet. Thanks for flagging it as a need for you and your team. Our current plan is to bump the native versions to our 3.x.x beta versions before releasing a new RN SDK. We're relying on open source PRs to move this along, so we don't have any firm time commitments. If we haven't gotten any momentum for bumping to 3.x.x in the next two weeks, we'll release a RN version that is pointing to the latest generally available native SDK versions.

FaggioniHQ commented 1 year ago

Hello @billfinn-stripe ,

Thanks for the quick response. Sounds good.

Thanks for your help.

mihaildu commented 1 year ago

Good question -- no, connecting to simulated local mobile readers is not supported. (In fact, currently, local mobile (aka, tap-to-pay) is not supported by ReactNative at all.)

Good to know, thank you for the answer!

I'll leave this issue open as a reminder to improve the error message and/or documentation.

I think the documentation is pretty clear that it only works with iOS SDK, but after looking through the example app in the github repo where it seems like it might work, I decided to give it a try and hope for outdated documentation :)

I have a follow up question: to make Tap to Pay work with RN is it just about bumping StripeTerminal dep to 2.14+ (already done on main branch) and also adding an extra function to StripeTerminalReactNative.swift called connectLocalMobileReader that should try to connect to a local mobile reader, similar to the example here? I might be able to submit this PR if it's only those changes.

Also, just wanted to say I would also like this feature implemented in RN for my team. Thank you!

billfinn-stripe commented 1 year ago

I have a follow up question: to make Tap to Pay work with RN is it just about bumping StripeTerminal dep to 2.14+ (already done on main branch) and also adding an extra function to StripeTerminalReactNative.swift called connectLocalMobileReader that should try to connect to a local mobile reader, similar to the example here? I might be able to submit this PR if it's only those changes.

Hmm, yeah, that may be all that is needed. I see that the native Android code already seems to support it. Did you happen to test if tap-to-pay already works for Android if you bump the underlying native SDK versions?

Also, we'll want to add localmobile to DiscoveryMethod here.

I confirmed internally, and we are working on adding support for tap-to-pay to RN, but I can't commit to a timeline.

Also, just wanted to say I would also like this feature implemented in RN for my team. Thank you!

Great, we appreciate the feedback. Thank you.

mihaildu commented 1 year ago

I see that the native Android code already seems to support it. Did you happen to test if tap-to-pay already works for Android if you bump the underlying native SDK versions?

Unfortunately I didn't get a chance to test anything with Android, we don't plan to support Android until later down the line.

Also, we'll want to add localmobile to DiscoveryMethod here.

Great, I created a fork here with the suggested changes which can be seen in this PR. I will give it a test when I have some time. Let me know if you want me to also open the PR against this repository!

I confirmed internally, and we are working on adding support for tap-to-pay to RN, but I can't commit to a timeline.

Ok cool! Thanks for letting me know

billfinn-stripe commented 1 year ago

Great, I created a fork here with the suggested changes whch can be seen in this PR. I will give it a test when I have some time. Let me know if you want me to also open the PR against this repository!

Great, thanks! We may also want to map more native errors to RN-friendly error strings (so we don't see "Unknown" for tap-to-pay specific errors that haven't been mapped yet) [0], but we can do that in a follow-up PR.

Let me know how testing goes, and, if it goes well, we'd welcome a PR to this repo too. Thanks!

[0] https://github.com/stripe/stripe-terminal-react-native/blob/61f9dc2d9c5f488f012ea2a016808ce57e35effa/ios/Errors.swift#L45

mihaildu commented 1 year ago

Let me know how testing goes, and, if it goes well

Well I managed to get it to work up until the Tap to Pay on iPhone screen, where you get to choose an Apple ID. However when calling collectPaymentMethod it errors out

Error collecting the payment intent: {“code”: “InvalidRequiredParameter”, “message”: “A required parameter was invalid or missing.“}

I would like to debug this but the code for Terminal.shared.collectPaymentMethod is not open source afaik. I set all the required params when creating the payment intent and when collecting it.

const {error, paymentIntent} = await createPaymentIntent({
  amount: 100,
  currency: 'usd',
  onBehalfOf: '<some-test-acct>',
  transferDataDestination: '<some-test-acct>',
  applicationFeeAmount: 20,
});
...
const {
  paymentIntent: collectedPaymentIntent,
  error,
} = await collectPaymentMethod({
  paymentIntentId: paymentIntent.id,
});

Maybe it's a dumb question, but does my Apple ID have to be from US? I tested using a device location from US, but I'm not physically from US.

we'd welcome a PR to this repo too

I am more than happy to open a PR from my fork, but I will need a little bit of guidance. This is the first time I'm writing any Swift code or do any mobile development, so I have a few questions. If this is OK with you, I will open a PR :)

billfinn-stripe commented 1 year ago

Great, thanks. I left a comment on the PR. When creating Payment Intents on behalf of other accounts, you'll also need to connect to the reader on behalf of the other account:

https://stripe.dev/stripe-terminal-ios/docs/Classes/SCPLocalMobileConnectionConfiguration.html#/c:objc(cs)SCPLocalMobileConnectionConfiguration(py)onBehalfOf

I am more than happy to open a PR from my fork, but I will need a little bit of guidance. This is the first time I'm writing any Swift code or do any mobile development, so I have a few questions. If this is OK with you, I will open a PR :)

Let's get it tested and working, and then we'd love a PR that includes the entire changeset. Thank you!

mihaildu commented 1 year ago

I replied in the same PR, already did the change in a different commit later. At this point I'm almost sure it doesn't work because of some Apple ID related issue and the code should be ok, with the exception of the local mobile reader handlers that I don't want to write myself (and re-use existing ones). Should I open a PR against this repository and discuss there?

billfinn-stripe commented 1 year ago

Should I open a PR against this repository and discuss there?

Sounds good. I still have a hunch it's related to on_behalf_of. Can you try creating a PI on your platform account and omitting it from both the connectLocalMobileReader and createPaymentIntent calls?

And, can you share how you're calling connectLocalMobileReader from your RN app?

mihaildu commented 1 year ago

Sounds good. I still have a hunch it's related to on_behalf_of

You are right, I forgot to add it when I call connectLocalMobileReader from RN. Now it all works!

I opened my PR here https://github.com/stripe/stripe-terminal-react-native/pull/441

Not related to this PR, I am also wondering if createPaymentIntent should accept a captureMethod param. I don't want to use manual and it would be nice if I can do all of this without backend support. Looking at API docs it says it has a default value of automatic, so something is explicitly setting it to manual

billfinn-stripe commented 1 year ago

Not related to this PR, I am also wondering if createPaymentIntent should accept a captureMethod param.

Yep, definitely we should add that. The underlying native SDKs do support this param [0].

I opened my PR here

Great! Thank you! Would you mind adding a few notes in the PR description what manual testing you performed? I'll pull that branch and do some testing on both Android and iOS before reviewing and approving. I also kicked off a CI build to make sure our automated tests all pass.

I don't want to use manual and it would be nice if I can do all of this without backend support. Looking at API docs it says it has a default value of automatic, so something is explicitly setting it to manual

Yeah, that's confusing. The API has a default value of automatic, but the SDKs have a default value of manual (for backwards compatibility reasons). We do have plans to require integrations to always specify a capture method (removing the default entirely from the SDK), but that's a breaking change, so we haven't implemented it quite yet.

[0] Android: https://stripe.dev/stripe-terminal-android//external/com.stripe.stripeterminal.external.models/-payment-intent-parameters/capture-method.html https://stripe.dev/stripe-terminal-android//external/com.stripe.stripeterminal.external.models/-capture-method/-companion/index.html

iOS: https://stripe.dev/stripe-terminal-ios/docs/Classes/SCPPaymentIntentParameters.html#/c:objc(cs)SCPPaymentIntentParameters(py)captureMethod

mihaildu commented 1 year ago

Yep, definitely we should add that

Do you have an ETA for this?

Would you mind adding a few notes in the PR description what manual testing you performed?

Sure, I'll add some description with the steps.

Awesome, thanks!

chenzesam commented 1 year ago

@billfinn-stripe Hi, I understand that you guys discussed publishing RN code for TTPOI two weeks ago. Are there any plans to release it recently, please?

liran-co commented 1 year ago

@billfinn-stripe Is there any update on this?

tenacioustechie commented 1 year ago

Any plans to release this? Very keen to start developing against this, otherwise we might need to start our own version off @mihaildu 's PR.

gperl27 commented 1 year ago

Has anyone had success using the beta version that includes #441 with a real device? Is the beta only supposed to work in a simulated environment? No matter what, discoverReaders returns an empty array.

At first I thought it was my provisioning profile and/or entitlements file, but I cross-checked that they are correct by installing the stripe iOS example app; here I was able to discover my device as a reader and initiate a TTPOI payment.

I also tried patching @stripe/stripe-terminal-react-native's podspec to Stripe SDK 2.14.0 and the latest (2.19.1 as of today) version with the same results.

chr-stripe commented 1 year ago

Hi folks, thanks for reaching out to us about getting support for Tap to Pay in the React Native SDK. Right now we’re targeting a new release for end of May at the latest. We’ll update this thread as well once the next release lands.

nazli-stripe commented 1 year ago

Hi folks, thanks for your patience. We are planning on releasing the new version with Tap to Pay Android and iOS support by end of this week.

nazli-stripe commented 1 year ago

Tap to Pay support added in the latest release: https://github.com/stripe/stripe-terminal-react-native/releases/tag/v0.0.1-beta.12

592da commented 1 year ago

I having this error as well, using beta.12

{code: InvalidRequiredParameter, message: A required parameter was invalid or missing.}

the payment intent is created in the server, important to note, works with WisePad3 with the same flow. might relate to the fact my apple id is registered in the UK.

(the merchant is set up in US, might not have location settings on as I'm debugging it from the UK) @nazli-stripe