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

Preserve payment selection for guests #5480

Merged

Conversation

tillh-stripe
Copy link
Collaborator

@tillh-stripe tillh-stripe commented Aug 29, 2022

Summary

This pull request fixes a bug where the FlowController would show an incorrect payment method selection for guests that don’t have any saved payment methods.

Previously, we would use PrefsRepository.NoOp when no customer ID was provided to our SDK. Now, we use DefaultPrefsRepository and store a guest’s last saved payment method under a generic "guest" key. This is similar to iOS, which uses an empty string.

Motivation

Bugfix.

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screen recordings

Before
The payment options sheet would default to Google Pay, even if Link was available and had been previously selected.

old-behavior.mp4

After
The payment options sheet now correctly shows link as the current payment selection when opening the sheet.

new-behavior.mp4

Changelog

[Fixed] FlowController now correctly preserves the previously selected payment method for guests.

Comment on lines 144 to 117
private fun createWithoutCustomer(
clientSecret: ClientSecret,
stripeIntent: StripeIntent,
config: PaymentSheet.Configuration?,
isGooglePayReady: Boolean,
isLinkReady: Boolean
): FlowControllerInitializer.InitResult {
val savedSelection = if (isLinkReady) {
SavedSelection.Link
} else if (isGooglePayReady) {
SavedSelection.GooglePay
} else {
SavedSelection.None
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this logic for guests, because we can just use setLastSavedPaymentMethod() for this.

@@ -239,7 +239,7 @@ class LinkPaymentLauncher @AssistedInject internal constructor(

@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
companion object {
val LINK_ENABLED = BuildConfig.DEBUG
val LINK_ENABLED = false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Had to revert my earlier change because this breaks the PaymentOptionsActivityTest.

brnunes-stripe
brnunes-stripe previously approved these changes Sep 8, 2022
@@ -242,7 +242,7 @@ class LinkPaymentLauncher @AssistedInject internal constructor(

@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
companion object {
val LINK_ENABLED = BuildConfig.DEBUG
val LINK_ENABLED = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert this change?

Copy link
Collaborator Author

@tillh-stripe tillh-stripe Sep 9, 2022

Choose a reason for hiding this comment

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

Nope, unfortunately had to do this to stop PaymentOptionsActivityTest from failing.

@tillh-stripe tillh-stripe merged commit c65acef into master Sep 9, 2022
@tillh-stripe tillh-stripe deleted the tillh/initializer-guest-payment-method-selection branch September 9, 2022 18:40
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.

3 participants