fix: Native code audit, mostly Android

[charliecruzan-stripe]

Summary / Motivation

Get rid of warnings, fix some code styling, better null checks, remove usage of deprecated Android features, better file naming, use PaymentLauncher instead of stripe.onPaymentResult

For reviewing- it's probably most efficient to review this PR commit by commit

Testing

• I tested this manually
• Existing automated tests should pass

Documentation

Select one:

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

[charliecruzan-stripe]

@michelleb-stripe requesting your review since you did the Android audit (here). Really the only commit that I think could use your attention is this one

[michelleb-stripe]

How is the result of of retrievePaymentIntent being used? I don't think it should be needed because the PaymentFlowFailureMessageFactory contains similar logic. It is called from the PaymentFlowResultProcessor in the PaymentLauncherViewModel.

We have an example of how to use the PaymentLauncher here.

It would also be good to have @ccen-stripe take a final look at it too

[charliecruzan-stripe]

retrievePaymentIntent allows us to get the relevant paymentIntent (or the error) and pass it back to the React Native app via promise.resolve. Went with stripe.retrievePaymentIntent based on this comment

[michelleb-stripe]

I am not sure I am understanding. It looks like the retrievePaymentIntent is only be retrieved in the "Complete" case and isn't being returned.

[charliecruzan-stripe]

That's correct- retrievePaymentIntent is only called in the complete case. In the case of cancellation or failure, we send an error object back to the user (L46 and L50)

In the case of a completed payment result, we retrieve the payment intent, and resolve the promise that was passed in via Javascript. We aren't explicitly calling return here with the payment intent object, but the promise.resolve call is passing the data back over the React Native bridge into "Javascript land".

The previous code functioned the same way, see here. But, if there's a better way to fetch the payment intent associated with the result then let me know!

[michelleb-stripe]

I gotchya, that is ok to preserve behavior. The PaymentSheet on Android and iOS do not return the payment intent though, so it is a difference of behavior there.

I don't think an immediate action is needed.

[ccen-stripe]

PaymentLauncher actually processes the status of the Intent internally and returns Completed/Failed/Cancelled according to the processed Outcomes.

Compared the logic with retrievePaymentIntent a bit, it seems we can just avoid the retrievePaymentIntent call and do the following mapping:

PaymentResult.Completed-> {
  val paymentIntent = createResult("paymentIntent", mapFromPaymentIntentResult(result))
  promise.resolve(paymentIntent)}
PaymentResult.Failed-> {
  val error = result.lastPaymentError
  promise.resolve(createError(ConfirmPaymentErrorType.Failed.toString(), error))
}
PaymentResult.Cancelled-> {
if (isPaymentIntentNextActionVoucherBased(result.nextActionType)) {
    val paymentIntent = createResult("paymentIntent", mapFromPaymentIntentResult(result))
    promise.resolve(paymentIntent)
  } else {
    (result.lastPaymentError)?.let {
      promise.resolve(createError(ConfirmPaymentErrorType.Canceled.toString(), it))
    } ?: run {
      promise.resolve(createError(ConfirmPaymentErrorType.Canceled.toString(), "The payment has been canceled"))
    }
  }
}

[charliecruzan-stripe]

Not sure I understand- mapFromPaymentIntentResult accepts a PaymentIntent, but the result we're getting here from PaymentLauncher is a PaymentResult. I thought in order to retrieve the full PaymentIntent object, we pretty much needed to use Stripe.retrievePaymentIntent

[michelleb-stripe]

yeah, @ccen-stripe I think this api is a little different in that it returns the full payment intent to clients.

[michelleb-stripe]

On line 278: setUrlSchemeOnAndroid, it appears this is the returnUrl on Android. I wonder if we can deprecate setUrlSchemeOnAndroid and move to setReturnUrlSchemeOnAndroid?

Let's check with @ccen-stripe, but I think we recommend that user's don't set this, but not sure where we indicate that to users. (Stripe.kt has some commentary on the effect of the returnUrl, but doesn't say anything about recommending it is not set.)

[charliecruzan-stripe]

renamed to setReturnUrlSchemeOnAndroid and deprecated setUrlSchemeOnAndroid.

[michelleb-stripe]

Lots of great changes and cleanup in here! Very excited to see this all implemented.

[michelleb-stripe]

I just sent @ccen-stripe a message this morning to get some feedback on one remaining comment: https://github.com/stripe/stripe-react-native/pull/832/files#r822856777