Use sdk_uuid for payment and setup intents #551

nazli-stripe · 2023-10-03T18:13:58Z

Summary

Updates PaymentIntent and SetupIntent methods to take the whole model and identify the PI based on a newly generated UUID.

Motivation

Offline mode prep

Testing

I tested this manually
I added automated tests

Documentation

Select one:

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

nazli-stripe · 2023-10-10T16:31:27Z

src/functions.ts

 ): Promise<SetupIntentResultType> {
-  return Logger.traceSdkMethod(async (innerSetupIntentId) => {
+  return Logger.traceSdkMethod(async (innerSetupInten) => {


typo, should be innerSetupIntent

nazli-stripe · 2023-10-10T16:32:23Z

src/StripeTerminalSdk.tsx

-    paymentIntentId: string
+    paymentIntentJson: PaymentIntent.Type
  ): Promise<PaymentIntentResultType>;
  // Create Setup Intent
  createSetupIntent(
    params: CreateSetupIntentParams
  ): Promise<SetupIntentResultType>;
  // Cancel Payment Intent
  cancelPaymentIntent(
-    paymentIntentId: string
+    paymentIntent: PaymentIntent.Type


we need these updates for cancelSetupIntent and confirmSetupIntent as well

nazli-stripe · 2023-10-11T16:59:00Z

src/StripeTerminalSdk.tsx

@@ -108,11 +108,15 @@ export interface StripeTerminalSdkType {
  clearReaderDisplay(): Promise<ClearReaderDisplayResultType>;
  retrieveSetupIntent(clientSecret: string): Promise<SetupIntentResultType>;
  // Cancel Setup Intent
-  cancelSetupIntent(paymentIntentId: string): Promise<SetupIntentResultType>;
+  cancelSetupIntent(
+    paymentIntent: PaymentIntent.Type


this should be setupIntent: SetupIntent.Type

nazli-stripe · 2023-10-11T16:59:07Z

src/StripeTerminalSdk.tsx

  // List of locations belonging to the merchant
  getLocations(params: GetLocationsParams): Promise<GetLocationsResultType>;
  // Confirm Setup Intent
-  confirmSetupIntent(paymentIntentId: string): Promise<SetupIntentResultType>;
+  confirmSetupIntent(
+    paymentIntent: PaymentIntent.Type


bric-stripe

iOS changes LGTM aside from a non-blocking request to improve the error messaging on the UUID checks to make those a little friendlier since the UUID is an internal detail

bric-stripe · 2023-10-13T17:25:16Z

ios/StripeTerminalReactNative.swift

+        guard let uuid = paymentIntentJSON["sdk_uuid"] as? String else {
+            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "You must provide sdk_uuid."))
            return
        }
-        guard let paymentIntent = self.paymentIntents[Optional(id)] else {
-            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "There is no associated paymentIntent with id \(id)"))
+        guard let paymentIntent = self.paymentIntents[uuid] else {
+            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "There is no associated paymentIntent with uuid \(uuid)"))


nit-ish: We should probably improve the error messaging here since the UUID is an internal detail that a user wouldn't know how to debug without digging in to the SDK more.

suggestion for if no UUID in the dictionary:

Unexpected PaymentIntent. collectPaymentMethod requires you to use the PaymentIntent that was returned from either createPaymentIntent or retrievePaymentIntent.

suggestion for if it has a UUID but we don't have a matching entry:

Unexpected PaymentIntent. The PaymentIntent provided must be re-retrieved with retrievePaymentIntent or a new PaymentIntent must be created with createPaymentIntent. This can happen if the reader disconnected, another PaymentIntent was confirmed, or the PaymentIntent was canceled.

^ got there from reviewing when we clear out self.paymentIntents and disconnect, post confirm, and cancel are the when we do that. So if someone

created a PI A

created a PI B

collected and confirmed PI A

attempted to collect PI B the SDK would error with the miss on paymentIntents[uuid] and they'd get this second error message telling to re-retrieve or create a new PI. (if I understand correctly this shouldn't be a new problem...before they would have gotten the "There is no associated paymentIntent with id (id)" error)

(and if we do improve the error messages we should do the same in the android layer too)

even though it's an internal detail I feel like it is good for users to know exactly what is wrong with the PI they provided. We often get user inquiries on parameters due to vague error messages that don't tell which param or field caused the issue.

The PaymentIntent is missing sdk_uuid field.

No PaymentIntent was found with the sdk_uuid {some_uuid}. The PaymentIntent provided must be re-retrieved with retrievePaymentIntent or a new PaymentIntent must be created with createPaymentIntent.

would you be okay if we changed it to this?

yep, sounds good to me

bric-stripe · 2023-10-13T17:26:22Z

ios/StripeTerminalReactNative.swift

+        guard let uuid = paymentIntentJson["sdk_uuid"] as? String else {
+            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "You must provide sdk_uuid."))
            return
        }
-
-        guard let paymentIntent = paymentIntents[Optional(id)] else {
-            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "There is no associated paymentIntent with id \(id)"))
+        guard let paymentIntent = self.paymentIntents[uuid] else {
+            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "There is no associated paymentIntent with uuid \(uuid)"))


similar request for improved error messaging here

bric-stripe · 2023-10-13T17:26:31Z

ios/StripeTerminalReactNative.swift

+        guard let uuid = paymentIntentJson["sdk_uuid"] as? String else {
+            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "You must provide sdk_uuid."))
            return
        }
-        guard let paymentIntent = paymentIntents[Optional(id)] else {
-            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "There is no associated paymentIntent with id \(id)"))
+        guard let paymentIntent = self.paymentIntents[uuid] else {
+            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "There is no associated paymentIntent with uuid \(uuid)"))


bric-stripe · 2023-10-13T17:26:59Z

ios/StripeTerminalReactNative.swift

+        guard let uuid = setupIntentJson["sdk_uuid"] as? String else {
+            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "You must provide sdk_uuid."))
            return
        }
-        guard let setupIntent = setupIntents[id] else {
-            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "There is no associated setupIntentId with id \(id)"))
+        guard let setupIntent = self.setupIntents[uuid] else {
+            resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "There is no associated setupIntentId with id \(uuid)"))


and for the 3 SetupIntent ones too

bric-stripe

👍 lgtm, thanks for the updates!

nazli-stripe and others added 5 commits October 3, 2023 08:13

pass pi for collect confirm

cb915ec

cancel payment intent

3b1ebfe

adnroid updates to use uuid

20babce

Pass si for confirmSetupIntent、cancelSetupIntent

e821369

android updates to use uuid for SetupIntent

ccb9611

ianlin-bbpos marked this pull request as ready for review October 10, 2023 02:10

ianlin-bbpos requested review from billfinn-stripe, bric-stripe, rv-stripe and chr-stripe as code owners October 10, 2023 02:10

ianlin-bbpos and others added 2 commits October 10, 2023 14:41

update Gemfile.

072731b

update ruby

7869c1a

nazli-stripe commented Oct 10, 2023

View reviewed changes

fix issues of review

b8072c2

nazli-stripe commented Oct 11, 2023

View reviewed changes

fix param issue.

7df132d

bric-stripe previously approved these changes Oct 13, 2023

View reviewed changes

improve the error messaging & remove cancelReadReusableCard.

1713a0d

ianlin-bbpos dismissed bric-stripe’s stale review via 1713a0d October 16, 2023 03:24

improve the error messaging.

b575c2a

ianlin-bbpos requested a review from bric-stripe October 16, 2023 07:20

update error message

c16bf76

bric-stripe approved these changes Oct 16, 2023

View reviewed changes

nazli-stripe merged commit f9aeda5 into main Oct 16, 2023
1 check passed

billfinn-stripe mentioned this pull request Apr 4, 2024

cancelPaymentIntent doesn’t cancel a payment in the requiresCapture state #655

Closed

nazli-stripe deleted the nazli/pi-id branch September 7, 2024 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use sdk_uuid for payment and setup intents #551

Use sdk_uuid for payment and setup intents #551

nazli-stripe commented Oct 3, 2023 •

edited

Loading

nazli-stripe Oct 10, 2023 •

edited

Loading

nazli-stripe Oct 10, 2023

nazli-stripe Oct 11, 2023

nazli-stripe Oct 11, 2023

bric-stripe left a comment

bric-stripe Oct 13, 2023

nazli-stripe Oct 13, 2023

bric-stripe Oct 13, 2023

bric-stripe Oct 13, 2023

bric-stripe Oct 13, 2023

bric-stripe Oct 13, 2023

bric-stripe left a comment

Use sdk_uuid for payment and setup intents #551

Use sdk_uuid for payment and setup intents #551

Conversation

nazli-stripe commented Oct 3, 2023 • edited Loading

Summary

Motivation

Testing

Documentation

nazli-stripe Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bric-stripe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bric-stripe left a comment

Choose a reason for hiding this comment

nazli-stripe commented Oct 3, 2023 •

edited

Loading

nazli-stripe Oct 10, 2023 •

edited

Loading