-
Notifications
You must be signed in to change notification settings - Fork 266
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
base: master
Are you sure you want to change the base?
Conversation
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.
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
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) | ||
} |
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.
looks like this is copied from above, maybe we can extract this out to a helper function?
promise.resolve(result) | ||
} | ||
|
||
this.intentConfiguration = buildIntentConfiguration(bundle); |
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.intentConfiguration = buildIntentConfiguration(bundle); | |
this.intentConfiguration = buildIntentConfiguration(bundle); |
|
||
this.intentConfiguration = buildIntentConfiguration(bundle); | ||
this.flowController?.configureWithIntentConfiguration( | ||
intentConfiguration = this.intentConfiguration!!, |
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.
buildIntentConfiguration
returns a nullable value so this !!
seems dangerous, maybe we can add some null-handling before this
// Address, | ||
// BillingDetails, |
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.
// Address, | |
// BillingDetails, |
const initialisePaymentSheet = async () => { | ||
setLoading(true); | ||
|
||
try { |
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 think we can factor out this try catch
const updatePayment = async () => { | ||
setLoading(true); | ||
|
||
try { |
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.
same here, don't need try catch
setLoading(true); | ||
|
||
try { | ||
const { customer } = await fetchPaymentSheetParams(); |
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 might give us a new customer- should we save the old customer from the init step locally instead?
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) | ||
) |
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 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 { | ||
|
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.
add error handling
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.
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 { |
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 does the entire function need to be wrapped in DispatchQueue.main.async
?
Any update on this pr pls ? |
Summary
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
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.
step2
call the updatePaymentSheet with the
PaymentSheet.IntentConfiguration
object containing all the paramsNOTE: the function replace the entier block, so be aware if you remove an attribute it will be removed.
Select one: