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

Fix StripeIntent status check #4115

Merged
merged 6 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions payments-core/api/payments-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -5199,11 +5199,11 @@ public final class com/stripe/android/paymentsheet/analytics/DefaultEventReporte
}

public final class com/stripe/android/paymentsheet/flowcontroller/DefaultFlowControllerInitializer_Factory : dagger/internal/Factory {
public fun <init> (Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;)V
public static fun create (Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;)Lcom/stripe/android/paymentsheet/flowcontroller/DefaultFlowControllerInitializer_Factory;
public fun <init> (Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;)V
public static fun create (Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;Ljavax/inject/Provider;)Lcom/stripe/android/paymentsheet/flowcontroller/DefaultFlowControllerInitializer_Factory;
public fun get ()Lcom/stripe/android/paymentsheet/flowcontroller/DefaultFlowControllerInitializer;
public synthetic fun get ()Ljava/lang/Object;
public static fun newInstance (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcom/stripe/android/paymentsheet/repositories/StripeIntentRepository;Lcom/stripe/android/paymentsheet/repositories/CustomerRepository;Lkotlin/coroutines/CoroutineContext;)Lcom/stripe/android/paymentsheet/flowcontroller/DefaultFlowControllerInitializer;
public static fun newInstance (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcom/stripe/android/paymentsheet/repositories/StripeIntentRepository;Lcom/stripe/android/paymentsheet/repositories/CustomerRepository;Lcom/stripe/android/Logger;Lkotlin/coroutines/CoroutineContext;)Lcom/stripe/android/paymentsheet/flowcontroller/DefaultFlowControllerInitializer;
}

public final class com/stripe/android/paymentsheet/flowcontroller/DefaultFlowController_Factory : dagger/internal/Factory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,19 +215,15 @@ internal class PaymentSheetViewModel @Inject internal constructor(
}

private fun onStripeIntentFetchResponse(stripeIntent: StripeIntent) {
if (stripeIntent.isConfirmed) {
onConfirmedStripeIntent(stripeIntent)
} else {
runCatching {
stripeIntentValidator.requireValid(stripeIntent)
}.fold(
onSuccess = {
updatePaymentMethods(stripeIntent)
resetViewState(stripeIntent)
},
onFailure = ::onFatal
)
}
runCatching {
stripeIntentValidator.requireValid(stripeIntent)
}.fold(
onSuccess = {
updatePaymentMethods(stripeIntent)
resetViewState(stripeIntent)
},
onFailure = ::onFatal
)
}

/**
Expand Down Expand Up @@ -271,22 +267,6 @@ internal class PaymentSheetViewModel @Inject internal constructor(
}
}

/**
* There's nothing left to be done in payment sheet if the [StripeIntent] is confirmed.
*
* See [How intents work](https://stripe.com/docs/payments/intents) for more details.
*/
private fun onConfirmedStripeIntent(stripeIntent: StripeIntent) {
logger.info(
"""
${stripeIntent.javaClass.simpleName} with id=${stripeIntent.id} has already been confirmed.
""".trimIndent()
)
_viewState.value = PaymentSheetViewState.FinishProcessing {
_paymentSheetResult.value = PaymentSheetResult.Completed
}
}

private fun resetViewState(stripeIntent: StripeIntent, @IntegerRes stringResId: Int?) {
resetViewState(
stripeIntent,
Expand Down Expand Up @@ -470,6 +450,7 @@ internal class PaymentSheetViewModel @Inject internal constructor(
}

override fun onFatal(throwable: Throwable) {
logger.error("Payment Sheet error", throwable)
_fatal.value = throwable
_paymentSheetResult.value = PaymentSheetResult.Failed(throwable)
}
Expand Down Expand Up @@ -501,6 +482,7 @@ internal class PaymentSheetViewModel @Inject internal constructor(
private val applicationSupplier: () -> Application,
private val starterArgsSupplier: () -> PaymentSheetContract.Args
) : ViewModelProvider.Factory {
@Suppress("UNCHECKED_CAST")
override fun <T : ViewModel?> create(modelClass: Class<T>): T {
return DaggerPaymentSheetViewModelComponent.builder()
.application(applicationSupplier())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.stripe.android.paymentsheet.flowcontroller

import com.stripe.android.Logger
import com.stripe.android.googlepaylauncher.GooglePayEnvironment
import com.stripe.android.googlepaylauncher.GooglePayRepository
import com.stripe.android.model.PaymentMethod
Expand All @@ -26,6 +27,7 @@ internal class DefaultFlowControllerInitializer @Inject constructor(
private val googlePayRepositoryFactory: @JvmSuppressWildcards (GooglePayEnvironment) -> GooglePayRepository,
private val stripeIntentRepository: StripeIntentRepository,
private val customerRepository: CustomerRepository,
private val logger: Logger,
@IOContext private val workContext: CoroutineContext
) : FlowControllerInitializer {
private val stripeIntentValidator = StripeIntentValidator()
Expand Down Expand Up @@ -104,6 +106,7 @@ internal class DefaultFlowControllerInitializer @Inject constructor(
}
},
onFailure = {
logger.error("Failure initializing FlowController", it)
FlowControllerInitializer.InitResult.Failure(it)
}
)
Expand Down Expand Up @@ -142,6 +145,7 @@ internal class DefaultFlowControllerInitializer @Inject constructor(
)
},
onFailure = {
logger.error("Failure initializing FlowController", it)
FlowControllerInitializer.InitResult.Failure(it)
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal class StripeIntentValidator {
error(
brnunes-stripe marked this conversation as resolved.
Show resolved Hide resolved
"""
PaymentIntent with confirmation_method='automatic' is required.
The current PaymentIntent has confirmation_method ${stripeIntent.confirmationMethod}.
The current PaymentIntent has confirmation_method '${stripeIntent.confirmationMethod}'.
See https://stripe.com/docs/api/payment_intents/object#payment_intent_object-confirmation_method.
""".trimIndent()
)
Expand All @@ -31,7 +31,7 @@ internal class StripeIntentValidator {
error(
"""
A PaymentIntent with status='requires_payment_method' or 'requires_action` is required.
The current PaymentIntent has status ${stripeIntent.status}.
The current PaymentIntent has status '${stripeIntent.status}'.
See https://stripe.com/docs/api/payment_intents/object#payment_intent_object-status.
""".trimIndent()
)
Expand All @@ -44,7 +44,7 @@ internal class StripeIntentValidator {
error(
"""
A SetupIntent with status='requires_payment_method' or 'requires_action` is required.
The current SetupIntent has status ${stripeIntent.status}.
The current SetupIntent has status '${stripeIntent.status}'.
See https://stripe.com/docs/api/setup_intents/object#setup_intent_object-status
""".trimIndent()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,8 @@ internal class PaymentSheetActivityTest {
scenario.getResult().resultCode,
scenario.getResult().resultData
)
).isEqualTo(
PaymentSheetResult.Completed
).isInstanceOf(
PaymentSheetResult.Failed::class.java
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ internal class PaymentSheetViewModelTest {
assertThat((result as? PaymentSheetResult.Failed)?.error?.message)
.isEqualTo(
"PaymentIntent with confirmation_method='automatic' is required.\n" +
"The current PaymentIntent has confirmation_method Manual.\n" +
"The current PaymentIntent has confirmation_method 'Manual'.\n" +
"See https://stripe.com/docs/api/payment_intents/object#payment_intent_object-confirmation_method."
)
}
Expand All @@ -604,7 +604,7 @@ internal class PaymentSheetViewModelTest {
fun `fetchPaymentIntent() should fail if status != requires_payment_method`() {
val viewModel = createViewModel(
stripeIntentRepository = StripeIntentRepository.Static(
PaymentIntentFixtures.PI_REQUIRES_MASTERCARD_3DS2
PaymentIntentFixtures.PI_SUCCEEDED
)
)
var result: PaymentSheetResult? = null
Expand All @@ -614,9 +614,9 @@ internal class PaymentSheetViewModelTest {
viewModel.fetchStripeIntent()
assertThat((result as? PaymentSheetResult.Failed)?.error?.message)
.isEqualTo(
"PaymentIntent with confirmation_method='automatic' is required.\n" +
"The current PaymentIntent has confirmation_method Manual.\n" +
"See https://stripe.com/docs/api/payment_intents/object#payment_intent_object-confirmation_method."
"A PaymentIntent with status='requires_payment_method' or 'requires_action` is required.\n" +
"The current PaymentIntent has status 'succeeded'.\n" +
"See https://stripe.com/docs/api/payment_intents/object#payment_intent_object-status."
)
}

Expand Down Expand Up @@ -737,39 +737,6 @@ internal class PaymentSheetViewModelTest {
.isFalse()
}

@Test
fun `viewState should emit FinishProcessing and ProcessResult if PaymentIntent is confirmed`() {
val viewModel = createViewModel(
stripeIntentRepository = StripeIntentRepository.Static(
PaymentIntentFixtures.PI_SUCCEEDED
)
)

val viewStates = mutableListOf<PaymentSheetViewState>()
viewModel.viewState.observeForever { viewState ->
if (viewState is PaymentSheetViewState.FinishProcessing) {
// force `onComplete` to be called
viewState.onComplete()
}
viewState?.let {
viewStates.add(it)
}
}

var paymentSheetResult: PaymentSheetResult? = null
viewModel.paymentSheetResult.observeForever {
paymentSheetResult = it
}

viewModel.fetchStripeIntent()

assertThat(viewStates)
.hasSize(1)
assertThat(viewStates[0])
.isInstanceOf(PaymentSheetViewState.FinishProcessing::class.java)
assertThat(paymentSheetResult).isEqualTo(PaymentSheetResult.Completed)
}

@Test
fun `When configuration is empty, merchant name should reflect the app name`() {
val viewModel = createViewModel(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.stripe.android.paymentsheet.flowcontroller

import com.google.common.truth.Truth.assertThat
import com.stripe.android.Logger
import com.stripe.android.googlepaylauncher.GooglePayRepository
import com.stripe.android.model.PaymentIntent
import com.stripe.android.model.PaymentIntentFixtures
Expand Down Expand Up @@ -369,6 +370,7 @@ internal class DefaultFlowControllerInitializerTest {
{ if (isGooglePayReady) readyGooglePayRepository else unreadyGooglePayRepository },
stripeIntentRepo,
customerRepo,
Logger.noop(),
testDispatcher
)
}
Expand Down