-
Notifications
You must be signed in to change notification settings - Fork 649
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
Move Google Pay
to a ConfirmationDefinition
type
#9693
Move Google Pay
to a ConfirmationDefinition
type
#9693
Conversation
5ef6257
to
f7d8e5b
Compare
Diffuse output:
APK
|
f7d8e5b
to
24eb230
Compare
...a/com/stripe/android/paymentelement/confirmation/gpay/GooglePayConfirmationDefinitionTest.kt
Show resolved
Hide resolved
...t/src/main/java/com/stripe/android/paymentelement/confirmation/DefaultConfirmationHandler.kt
Show resolved
Hide resolved
...t/src/main/java/com/stripe/android/paymentelement/confirmation/DefaultConfirmationHandler.kt
Show resolved
Hide resolved
Also a large PR, about 200 lines of production code changes accompanied by about ~630 lines of test code changes |
24eb230
to
2a48304
Compare
...t/src/main/java/com/stripe/android/paymentelement/confirmation/DefaultConfirmationHandler.kt
Show resolved
Hide resolved
.../java/com/stripe/android/paymentelement/confirmation/gpay/GooglePayConfirmationDefinition.kt
Show resolved
Hide resolved
.../java/com/stripe/android/paymentelement/confirmation/gpay/GooglePayConfirmationDefinition.kt
Show resolved
Hide resolved
.../java/com/stripe/android/paymentelement/confirmation/gpay/GooglePayConfirmationDefinition.kt
Show resolved
Hide resolved
...a/com/stripe/android/paymentelement/confirmation/gpay/GooglePayConfirmationDefinitionTest.kt
Show resolved
Hide resolved
...a/com/stripe/android/paymentelement/confirmation/gpay/GooglePayConfirmationDefinitionTest.kt
Show resolved
Hide resolved
...a/com/stripe/android/paymentelement/confirmation/gpay/GooglePayConfirmationDefinitionTest.kt
Outdated
Show resolved
Hide resolved
...a/com/stripe/android/paymentelement/confirmation/gpay/GooglePayConfirmationDefinitionTest.kt
Show resolved
Hide resolved
...java/com/stripe/android/paymentsheet/utils/RecordingGooglePayPaymentMethodLauncherFactory.kt
Show resolved
Hide resolved
...java/com/stripe/android/paymentsheet/utils/RecordingGooglePayPaymentMethodLauncherFactory.kt
Outdated
Show resolved
Hide resolved
): ConfirmationDefinition.Result { | ||
return when (result) { | ||
is GooglePayPaymentMethodLauncher.Result.Completed -> { | ||
val nextConfirmationOption = PaymentMethodConfirmationOption.Saved( |
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'm not sure this is perfect. But at a minimum, so much better than modeling this as a PaymentSelection
!
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.
Agreed! I'd like to come up with a solution in which we don't model from one option to another. It is hard though because we have quite a few options that convert from one option to another or modify an option. Something we should think about after the architecture changes.
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.
Thinking more about this. It does kinda seem like the job of this definition to do this. So it feels right.
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 have a couple ideas that involve a solution using PaymentConfirmationMetadata
that may work.
Summary
Move
Google Pay
to aConfirmationDefinition
typeMotivation
Last confirmation type in
DefaultConfirmationHandler
that needs to be become a definition.Testing