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

bump native SDKs to 3.1 #558

Merged
merged 2 commits into from
Oct 20, 2023
Merged

bump native SDKs to 3.1 #558

merged 2 commits into from
Oct 20, 2023

Conversation

ianlin-bbpos
Copy link
Collaborator

Summary

bump native SDKs to 3.1.0.

Motivation

add user cancellation for internet readers.

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.

add user cancellation for internet readers.
chr-stripe
chr-stripe previously approved these changes Oct 19, 2023
Copy link
Contributor

@chr-stripe chr-stripe left a comment

Choose a reason for hiding this comment

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

Android changes LGTM 👍

@@ -537,12 +541,13 @@ class StripeTerminalReactNativeModule(reactContext: ReactApplicationContext) :
}

val customerConsentCollected = getBoolean(params, "customerConsentCollected")
val enableCustomerCancellation = getBoolean(params, "enableCustomerCancellation")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does getBoolean() default to false if it's not provided?

Copy link
Collaborator

Choose a reason for hiding this comment

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

bric-stripe
bric-stripe previously approved these changes Oct 19, 2023
Copy link
Collaborator

@bric-stripe bric-stripe left a comment

Choose a reason for hiding this comment

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

iOS changes lgtm 👍

@@ -4,6 +4,7 @@ import android.annotation.SuppressLint
import android.app.Application
import android.content.ComponentCallbacks2
import android.content.res.Configuration
import android.util.Log
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems unused ✂️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

got it.

Comment on lines 189 to 193
export type CollectSetupIntentPaymentMethodParams = {
customerConsentCollected?: boolean;
enableCustomerCancellation?: boolean;
setupIntent: SetupIntent.Type;
};

Choose a reason for hiding this comment

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

Does order matter?

Also in the other SDKs, we create a new "config" class that contained the enableCustomerCancellation param.

https://site-admin.stripe.com/docs/terminal/references/api/js-sdk#collect-setup-intent-payment-method

Choose a reason for hiding this comment

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

(also 👋 Ian!)

Copy link
Collaborator Author

@ianlin-bbpos ianlin-bbpos Oct 20, 2023

Choose a reason for hiding this comment

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

Hi, Michael, the order doesnt matter, we just need to match them well. right? @nazli-stripe

@ianlin-bbpos ianlin-bbpos dismissed stale reviews from bric-stripe and chr-stripe via 05cce13 October 20, 2023 02:25
@nazli-stripe nazli-stripe merged commit 188225e into main Oct 20, 2023
@nazli-stripe nazli-stripe deleted the bbpos/bump-native-sdks-to-3.1 branch September 7, 2024 17:13
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.

5 participants