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: ⚡ Update payment details after calling initPaymentSheet #1653

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minawalphonce
Copy link

Summary

  • created a new function "updatePaymentSheet" which updates the intent config info (e.g. amount) after payment sheet is initialized

Motivation

updating payment sheet intent config, is needed for payment methods that requires second level of authentication.
so the amount can be displayed correctly on the 3rd party authentication.
the functionality already exists in IOS and android native code. the PR is to expose the same in react native sdk.

Testing

  • I tested this manually
  • I added automated tests

Documentation

new screen updated in the example app to test the scenario.

step 1

in your component, use the hook useStripe to retrieve the new "updatePaymentSheet" function.

const {
    updatePaymentSheet,
  } = useStripe();

step2

call the updatePaymentSheet with the PaymentSheet.IntentConfiguration object containing all the params

const { error, paymentOption } = await updatePaymentSheet({
        confirmHandler:  ...,
        mode: {
          amount: amount,
          currencyCode: 'SEK',
        },
      });

NOTE: the function replace the entier block, so be aware if you remove an attribute it will be removed.

Select one:

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

@CLAassistant
Copy link

CLAassistant commented May 4, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@charliecruzan-stripe charliecruzan-stripe left a comment

Choose a reason for hiding this comment

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

nice feature add!!! thanks so much for your contribution, this looks excellent so far! Special thanks for doing such an incredible job following the conventions we've used so far in Payment Sheet functions

I have a couple comments, but overall this looks really great

Comment on lines +296 to +308
val onFlowControllerConfigure = PaymentSheet.FlowController.ConfigCallback { _, _ ->
val result = flowController?.getPaymentOption()?.let {
val bitmap = getBitmapFromVectorDrawable(context, it.drawableResourceId)
val imageString = getBase64FromBitmap(bitmap)
val option: WritableMap = WritableNativeMap()
option.putString("label", it.label)
option.putString("image", imageString)
createResult("paymentOption", option)
} ?: run {
WritableNativeMap()
}
promise.resolve(result)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like this is copied from above, maybe we can extract this out to a helper function?

promise.resolve(result)
}

this.intentConfiguration = buildIntentConfiguration(bundle);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
this.intentConfiguration = buildIntentConfiguration(bundle);
this.intentConfiguration = buildIntentConfiguration(bundle);


this.intentConfiguration = buildIntentConfiguration(bundle);
this.flowController?.configureWithIntentConfiguration(
intentConfiguration = this.intentConfiguration!!,
Copy link
Collaborator

Choose a reason for hiding this comment

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

buildIntentConfiguration returns a nullable value so this !! seems dangerous, maybe we can add some null-handling before this

Comment on lines +5 to +6
// Address,
// BillingDetails,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Address,
// BillingDetails,

const initialisePaymentSheet = async () => {
setLoading(true);

try {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we can factor out this try catch

const updatePayment = async () => {
setLoading(true);

try {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, don't need try catch

setLoading(true);

try {
const { customer } = await fetchPaymentSheetParams();
Copy link
Collaborator

Choose a reason for hiding this comment

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

this might give us a new customer- should we save the old customer from the init step locally instead?

Comment on lines +201 to +214
guard let modeParams = params["mode"] as? NSDictionary else {
resolve(Errors.createError(ErrorType.Failed, "One of `paymentIntentClientSecret`, `setupIntentClientSecret`, or `intentConfiguration.mode` is required"))
return
}
if (params.object(forKey: "confirmHandler") == nil) {
resolve(Errors.createError(ErrorType.Failed, "You must provide `intentConfiguration.confirmHandler` if you are not passing an intent client secret"))
return
}
let captureMethodString = params["captureMethod"] as? String
let intentConfig = buildIntentConfiguration(
modeParams: modeParams,
paymentMethodTypes: params["paymentMethodTypes"] as? [String],
captureMethod: mapCaptureMethod(captureMethodString)
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this logic is also present in preparePaymentSheetInstance, maybe we could extract it out to avoid repeating it in two places?

)
self.paymentSheetFlowController?.update(intentConfiguration: intentConfig){ [weak self] error in
if error != nil {

Copy link
Collaborator

Choose a reason for hiding this comment

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

add error handling

Copy link
Collaborator

Choose a reason for hiding this comment

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

most likely: resolve(Errors.createError(ErrorType.Failed, error as NSError?))

@objc(updatePaymentSheet:resolver:rejecter:)
func updatePaymentSheet(params: NSDictionary, resolver resolve: @escaping RCTPromiseResolveBlock,
rejecter reject: @escaping RCTPromiseRejectBlock) -> Void {
DispatchQueue.main.async {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why does the entire function need to be wrapped in DispatchQueue.main.async?

@alexdieudonne
Copy link

Any update on this pr pls ?

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