Refactor validation errors from Payment- & CustomerSheetLoader

paymentsheet/detekt-baseline.xml

@@ -3,7 +3,6 @@
   <ManuallySuppressedIssues/>
   <CurrentIssues>
     <ID>CyclomaticComplexMethod:CustomerSheetViewModel.kt$CustomerSheetViewModel$fun handleViewAction(viewAction: CustomerSheetViewAction)</ID>
-    <ID>CyclomaticComplexMethod:LpmRepository.kt$LpmRepository$private fun convertToSupportedPaymentMethod( stripeIntent: StripeIntent, sharedDataSpec: SharedDataSpec, billingDetailsCollectionConfiguration: BillingDetailsCollectionConfiguration, isDeferred: Boolean = false, )</ID>
     <ID>CyclomaticComplexMethod:PlaceholderHelper.kt$PlaceholderHelper$@VisibleForTesting internal fun specForPlaceholderField( field: PlaceholderField, placeholderOverrideList: List&lt;IdentifierSpec>, requiresMandate: Boolean, configuration: PaymentSheet.BillingDetailsCollectionConfiguration, )</ID>
     <ID>EmptyFunctionBlock:PrimaryButtonAnimator.kt$PrimaryButtonAnimator.&lt;no name provided>${ }</ID>
     <ID>ForbiddenComment:PaymentOptionFactory.kt$PaymentOptionFactory$// TODO: Should use labelResource paymentMethodCreateParams or extension function</ID>
@@ -31,11 +30,10 @@
     <ID>LongMethod:EditPaymentMethod.kt$@Composable internal fun EditPaymentMethodUi( viewState: EditPaymentMethodViewState, viewActionHandler: (action: EditPaymentMethodViewAction) -> Unit, modifier: Modifier = Modifier )</ID>
     <ID>LongMethod:FormViewModelTest.kt$FormViewModelTest$@Test fun `Verify params are set when element address fields are complete`()</ID>
     <ID>LongMethod:FormViewModelTest.kt$FormViewModelTest$@Test fun `Verify params are set when required address fields are complete`()</ID>
-    <ID>LongMethod:LpmRepository.kt$LpmRepository$private fun convertToSupportedPaymentMethod( stripeIntent: StripeIntent, sharedDataSpec: SharedDataSpec, billingDetailsCollectionConfiguration: BillingDetailsCollectionConfiguration, isDeferred: Boolean = false, )</ID>
     <ID>LongMethod:PaymentElement.kt$@Composable internal fun PaymentElement( formViewModelSubComponentBuilderProvider: Provider&lt;FormViewModelSubcomponent.Builder>, enabled: Boolean, supportedPaymentMethods: List&lt;SupportedPaymentMethod>, selectedItem: SupportedPaymentMethod, linkSignupMode: LinkSignupMode?, linkConfigurationCoordinator: LinkConfigurationCoordinator?, showCheckboxFlow: Flow&lt;Boolean>, onItemSelectedListener: (SupportedPaymentMethod) -> Unit, onLinkSignupStateChanged: (LinkConfiguration, InlineSignupViewState) -> Unit, formArguments: FormArguments, usBankAccountFormArguments: USBankAccountFormArguments, onFormFieldValuesChanged: (FormFieldValues?) -> Unit, )</ID>
     <ID>LongMethod:PaymentOptionFactory.kt$PaymentOptionFactory$fun create(selection: PaymentSelection): PaymentOption</ID>
     <ID>LongMethod:PaymentSheetConfigurationKtx.kt$internal fun PaymentSheet.Appearance.parseAppearance()</ID>
-    <ID>LongMethod:PaymentSheetLoader.kt$DefaultPaymentSheetLoader$private suspend fun create( elementsSession: ElementsSession, config: PaymentSheet.Configuration, isGooglePayReady: Boolean, ): PaymentSheetState.Full</ID>
+    <ID>LongMethod:PaymentSheetLoader.kt$DefaultPaymentSheetLoader$private suspend fun create( elementsSession: ElementsSession, config: PaymentSheet.Configuration, ): PaymentSheetState.Full</ID>
     <ID>LongMethod:PaymentSheetScreen.kt$@Composable internal fun PaymentSheetScreenContent( viewModel: BaseSheetViewModel, type: PaymentSheetFlowType, modifier: Modifier = Modifier, )</ID>
     <ID>LongMethod:PlaceholderHelperTest.kt$PlaceholderHelperTest$@Test fun `Test correct placeholder is removed for placeholder spec`()</ID>
     <ID>LongMethod:USBankAccountEmitters.kt$@Composable internal fun USBankAccountEmitters( viewModel: USBankAccountFormViewModel, usBankAccountFormArgs: USBankAccountFormArguments, )</ID>
@@ -69,7 +67,6 @@
     <ID>MaxLineLength:FlowControllerConfigurationHandlerTest.kt$FlowControllerConfigurationHandlerTest$.</ID>
     <ID>MaxLineLength:InjectableActivityScenario.kt$InjectableActivityScenario$delegate ?: throw IllegalStateException("Cannot move to state $newState since the activity hasn't been launched.")</ID>
     <ID>MaxLineLength:InjectableActivityScenario.kt$InjectableActivityScenario$val d = delegate ?: throw IllegalStateException("Cannot run onActivity since the activity hasn't been launched.")</ID>
-    <ID>MaxLineLength:LinkHandlerTest.kt$whenever(linkConfigurationCoordinator.attachNewCardToAccount(eq(linkConfiguration), any())).thenReturn(attachNewCardToAccountResult)</ID>
     <ID>MaxLineLength:LpmRepositoryTest.kt$LpmRepositoryTest$fun</ID>
     <ID>MaxLineLength:PaymentSheet.kt$PaymentSheet.Address$*</ID>
     <ID>MaxLineLength:PaymentSheet.kt$PaymentSheet.BillingDetails$*</ID>

paymentsheet/src/main/java/com/stripe/android/customersheet/CustomerSheetLoader.kt

@@ -12,7 +12,7 @@ import com.stripe.android.model.PaymentMethod
 import com.stripe.android.payments.financialconnections.IsFinancialConnectionsAvailable
 import com.stripe.android.paymentsheet.PaymentSheet
 import com.stripe.android.paymentsheet.model.PaymentSelection
-import com.stripe.android.paymentsheet.model.requireValidOrThrow
+import com.stripe.android.paymentsheet.model.validate
 import com.stripe.android.paymentsheet.repositories.ElementsSessionRepository
 import com.stripe.android.paymentsheet.state.toInternal
 import com.stripe.android.ui.core.cbc.CardBrandChoiceEligibility
@@ -60,7 +60,7 @@ internal class DefaultCustomerSheetLoader @Inject constructor(
                 mode = PaymentSheet.IntentConfiguration.Mode.Setup(),
             )
         )
-        return elementsSessionRepository.get(initializationMode).mapCatching { elementsSession ->
+        return elementsSessionRepository.get(initializationMode).onSuccess { elementsSession ->
             val billingDetailsCollectionConfig = configuration?.billingDetailsCollectionConfiguration.toInternal()
             val metadata = PaymentMethodMetadata(
                 stripeIntent = elementsSession.stripeIntent,
@@ -73,8 +73,6 @@ internal class DefaultCustomerSheetLoader @Inject constructor(
                 metadata = metadata,
                 serverLpmSpecs = elementsSession.paymentMethodSpecs,
             )
-
-            elementsSession.requireValidOrThrow()
         }
     }
 
@@ -154,6 +152,7 @@ internal class DefaultCustomerSheetLoader @Inject constructor(
                         } else {
                             CardBrandChoiceEligibility.Ineligible
                         },
+                        loadingError = elementsSession?.stripeIntent?.validate(),
                     )
                 )
             },

paymentsheet/src/main/java/com/stripe/android/customersheet/CustomerSheetState.kt

@@ -18,5 +18,6 @@ internal sealed interface CustomerSheetState {
         val isGooglePayReady: Boolean,
         val paymentSelection: PaymentSelection?,
         val cbcEligibility: CardBrandChoiceEligibility,
+        val loadingError: Throwable?,
     ) : CustomerSheetState
 }

paymentsheet/src/main/java/com/stripe/android/customersheet/CustomerSheetViewModel.kt

@@ -271,18 +271,24 @@ internal class CustomerSheetViewModel @Inject constructor(
 
         result.fold(
             onSuccess = { state ->
-                supportedPaymentMethods.clear()
-                supportedPaymentMethods.addAll(state.supportedPaymentMethods)
+                if (state.loadingError != null) {
+                    _result.update {
+                        InternalCustomerSheetResult.Error(state.loadingError)
+                    }
+                } else {
+                    supportedPaymentMethods.clear()
+                    supportedPaymentMethods.addAll(state.supportedPaymentMethods)
 
-                originalPaymentSelection = state.paymentSelection
-                isGooglePayReadyAndEnabled = state.isGooglePayReady
-                stripeIntent = state.stripeIntent
+                    originalPaymentSelection = state.paymentSelection
+                    isGooglePayReadyAndEnabled = state.isGooglePayReady
+                    stripeIntent = state.stripeIntent
 
-                transitionToInitialScreen(
-                    paymentMethods = state.customerPaymentMethods,
-                    paymentSelection = state.paymentSelection,
-                    cbcEligibility = state.cbcEligibility,
-                )
+                    transitionToInitialScreen(
+                        paymentMethods = state.customerPaymentMethods,
+                        paymentSelection = state.paymentSelection,
+                        cbcEligibility = state.cbcEligibility,
+                    )
+                }
             },
             onFailure = { cause ->
                 _result.update {

paymentsheet/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt

@@ -56,7 +56,6 @@ import com.stripe.android.paymentsheet.state.GooglePayState
 import com.stripe.android.paymentsheet.state.PaymentSheetLoader
 import com.stripe.android.paymentsheet.state.PaymentSheetState
 import com.stripe.android.paymentsheet.state.WalletsState
-import com.stripe.android.paymentsheet.state.asPaymentSheetLoadingException
 import com.stripe.android.paymentsheet.ui.HeaderTextFactory
 import com.stripe.android.paymentsheet.ui.ModifiableEditPaymentMethodViewInteractor
 import com.stripe.android.paymentsheet.ui.PrimaryButton
@@ -335,21 +334,39 @@ internal class PaymentSheetViewModel @Inject internal constructor(
         )
     }
 
-    private suspend fun handlePaymentSheetStateLoadFailure(error: Throwable) {
+    private fun handlePaymentSheetStateLoadFailure(error: Throwable) {
+        setStripeIntent(null)
+        onFatal(error)
+    }
+
+    private suspend fun handlePaymentSheetStateLoaded(state: PaymentSheetState.Full) {
+        if (state.loadingError != null) {
+            handlePaymentSheetStateLoadedWithInvalidIntent(
+                stripeIntent = state.stripeIntent,
+                error = state.loadingError,
+            )
+        } else {
+            initializeWithState(state)
+        }
+    }
+
+    private suspend fun handlePaymentSheetStateLoadedWithInvalidIntent(
+        stripeIntent: StripeIntent,
+        error: Throwable,
+    ) {
         val pendingResult = awaitPaymentResult()
 
         if (pendingResult is InternalPaymentResult.Completed) {
             // If we just received a transaction result after process death, we don't error. Instead, we dismiss
             // PaymentSheet and return a `Completed` result to the caller.
-            val usedPaymentMethod = error.asPaymentSheetLoadingException.usedPaymentMethod
+            val usedPaymentMethod = stripeIntent.paymentMethod
             handlePaymentCompleted(usedPaymentMethod, finishImmediately = true)
         } else {
-            setStripeIntent(null)
-            onFatal(error)
+            handlePaymentSheetStateLoadFailure(error)
         }
     }
 
-    private suspend fun handlePaymentSheetStateLoaded(state: PaymentSheetState.Full) {
+    private suspend fun initializeWithState(state: PaymentSheetState.Full) {
         cbcEligibility = when (state.isEligibleForCardBrandChoice) {
             true -> CardBrandChoiceEligibility.Eligible(
                 preferredNetworks = state.config.preferredNetworks

paymentsheet/src/main/java/com/stripe/android/paymentsheet/flowcontroller/FlowControllerConfigurationHandler.kt

@@ -88,9 +88,13 @@ internal class FlowControllerConfigurationHandler @Inject constructor(
 
         paymentSheetLoader.load(initializationMode, configuration).fold(
             onSuccess = { state ->
-                viewModel.previousConfigureRequest = configureRequest
-                onInitSuccess(state, configureRequest)
-                onConfigured()
+                if (state.loadingError != null) {
+                    onConfigured(state.loadingError)
+                } else {
+                    viewModel.previousConfigureRequest = configureRequest
+                    onInitSuccess(state, configureRequest)
+                    onConfigured()
+                }
             },
             onFailure = { error ->
                 onConfigured(error = error)

paymentsheet/src/main/java/com/stripe/android/paymentsheet/model/StripeIntentValidator.kt

@@ -1,6 +1,5 @@
 package com.stripe.android.paymentsheet.model
 
-import com.stripe.android.model.ElementsSession
 import com.stripe.android.model.PaymentIntent
 import com.stripe.android.model.PaymentIntent.ConfirmationMethod.Automatic
 import com.stripe.android.model.SetupIntent
@@ -9,10 +8,10 @@ import com.stripe.android.model.StripeIntent.Status.Canceled
 import com.stripe.android.model.StripeIntent.Status.RequiresCapture
 import com.stripe.android.model.StripeIntent.Status.Succeeded
 import com.stripe.android.paymentsheet.state.PaymentSheetLoadingException
+import com.stripe.android.paymentsheet.state.asPaymentSheetLoadingException
 
-internal fun ElementsSession.requireValidOrThrow(): ElementsSession {
-    StripeIntentValidator.requireValid(stripeIntent)
-    return this
+internal fun StripeIntent.validate(): PaymentSheetLoadingException? {
+    return runCatching { StripeIntentValidator.requireValid(this) }.exceptionOrNull()?.asPaymentSheetLoadingException
 }
 
 /**
@@ -23,20 +22,18 @@ internal object StripeIntentValidator {
     fun requireValid(
         stripeIntent: StripeIntent
     ): StripeIntent {
-        val paymentMethod = stripeIntent.paymentMethod
-
         val exception = when {
             stripeIntent is PaymentIntent && stripeIntent.confirmationMethod != Automatic -> {
                 PaymentSheetLoadingException.InvalidConfirmationMethod(stripeIntent.confirmationMethod)
             }
             stripeIntent is PaymentIntent && stripeIntent.isInTerminalState -> {
-                PaymentSheetLoadingException.PaymentIntentInTerminalState(paymentMethod, stripeIntent.status)
+                PaymentSheetLoadingException.PaymentIntentInTerminalState(stripeIntent.status)
             }
             stripeIntent is PaymentIntent && (stripeIntent.amount == null || stripeIntent.currency == null) -> {
                 PaymentSheetLoadingException.MissingAmountOrCurrency
             }
             stripeIntent is SetupIntent && stripeIntent.isInTerminalState -> {
-                PaymentSheetLoadingException.SetupIntentInTerminalState(paymentMethod, stripeIntent.status)
+                PaymentSheetLoadingException.SetupIntentInTerminalState(stripeIntent.status)
             }
             else -> {
                 // valid

paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/PaymentSheetLoader.kt

@@ -25,7 +25,7 @@ import com.stripe.android.paymentsheet.model.currency
 import com.stripe.android.paymentsheet.model.getPMAddForm
 import com.stripe.android.paymentsheet.model.getPMsToAdd
 import com.stripe.android.paymentsheet.model.getSupportedSavedCustomerPMs
-import com.stripe.android.paymentsheet.model.requireValidOrThrow
+import com.stripe.android.paymentsheet.model.validate
 import com.stripe.android.paymentsheet.repositories.CustomerRepository
 import com.stripe.android.paymentsheet.repositories.ElementsSessionRepository
 import com.stripe.android.ui.core.BillingDetailsCollectionConfiguration
@@ -173,6 +173,7 @@ internal class DefaultPaymentSheetLoader @Inject constructor(
                 linkState = linkState.await(),
                 isEligibleForCardBrandChoice = elementsSession.isEligibleForCardBrandChoice,
                 paymentSelection = initialPaymentSelection.await(),
+                loadingError = stripeIntent.validate(),
             )
         } else {
             val requested = stripeIntent.paymentMethodTypes.joinToString(separator = ", ")
@@ -212,7 +213,7 @@ internal class DefaultPaymentSheetLoader @Inject constructor(
         initializationMode: PaymentSheet.InitializationMode,
         configuration: PaymentSheet.Configuration,
     ): Result<ElementsSession> {
-        return elementsSessionRepository.get(initializationMode).mapCatching { elementsSession ->
+        return elementsSessionRepository.get(initializationMode).onSuccess { elementsSession ->
             val billingDetailsCollectionConfig =
                 configuration.billingDetailsCollectionConfiguration.toInternal()
 
@@ -230,8 +231,6 @@ internal class DefaultPaymentSheetLoader @Inject constructor(
             if (!didParseServerResponse) {
                 eventReporter.onLpmSpecFailure()
             }
-
-            elementsSession.requireValidOrThrow()
         }
     }
 

paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/PaymentSheetLoadingException.kt

@@ -2,21 +2,17 @@ package com.stripe.android.paymentsheet.state
 
 import com.stripe.android.core.exception.StripeException
 import com.stripe.android.model.PaymentIntent
-import com.stripe.android.model.PaymentMethod
 import com.stripe.android.model.StripeIntent
 import com.stripe.android.paymentsheet.state.PaymentSheetLoadingException.Unknown
 
 internal sealed class PaymentSheetLoadingException : Throwable() {
 
     abstract val type: String
-    abstract val usedPaymentMethod: PaymentMethod?
 
     data class InvalidConfirmationMethod(
         private val confirmationMethod: PaymentIntent.ConfirmationMethod,
     ) : PaymentSheetLoadingException() {
 
-        override val usedPaymentMethod: PaymentMethod? = null
-
         override val type: String = "invalidConfirmationMethod"
 
         override val message: String = """
@@ -31,8 +27,6 @@ internal sealed class PaymentSheetLoadingException : Throwable() {
         private val supported: String,
     ) : PaymentSheetLoadingException() {
 
-        override val usedPaymentMethod: PaymentMethod? = null
-
         override val type: String = "noPaymentMethodTypesAvailable"
 
         override val message: String
@@ -41,7 +35,6 @@ internal sealed class PaymentSheetLoadingException : Throwable() {
     }
 
     data class PaymentIntentInTerminalState(
-        override val usedPaymentMethod: PaymentMethod?,
         private val status: StripeIntent.Status?,
     ) : PaymentSheetLoadingException() {
 
@@ -55,7 +48,6 @@ internal sealed class PaymentSheetLoadingException : Throwable() {
     }
 
     data class SetupIntentInTerminalState(
-        override val usedPaymentMethod: PaymentMethod?,
         private val status: StripeIntent.Status?,
     ) : PaymentSheetLoadingException() {
 
@@ -69,7 +61,6 @@ internal sealed class PaymentSheetLoadingException : Throwable() {
     }
 
     object MissingAmountOrCurrency : PaymentSheetLoadingException() {
-        override val usedPaymentMethod: PaymentMethod? = null
         override val type: String = "missingAmountOrCurrency"
         override val message: String = "PaymentIntent must contain amount and currency."
     }
@@ -82,8 +73,6 @@ internal sealed class PaymentSheetLoadingException : Throwable() {
             get() = StripeException.create(cause).analyticsValue()
 
         override val message: String? = cause.message
-
-        override val usedPaymentMethod: PaymentMethod? = null
     }
 }
 

paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/PaymentSheetState.kt

@@ -21,6 +21,7 @@ internal sealed interface PaymentSheetState : Parcelable {
         val linkState: LinkState?,
         val isEligibleForCardBrandChoice: Boolean,
         val paymentSelection: PaymentSelection?,
+        val loadingError: PaymentSheetLoadingException?,
     ) : PaymentSheetState {
 
         val showSavedPaymentMethods: Boolean

paymentsheet/src/test/java/com/stripe/android/customersheet/state/DefaultCustomerSheetLoaderTest.kt

@@ -140,6 +140,7 @@ class DefaultCustomerSheetLoaderTest {
                     paymentMethod = PaymentMethodFixtures.CARD_PAYMENT_METHOD,
                 ),
                 cbcEligibility = CardBrandChoiceEligibility.Ineligible,
+                loadingError = null,
             )
         )
 
@@ -196,6 +197,7 @@ class DefaultCustomerSheetLoaderTest {
                 isGooglePayReady = false,
                 paymentSelection = null,
                 cbcEligibility = CardBrandChoiceEligibility.Ineligible,
+                loadingError = null,
             )
         )
     }
@@ -238,6 +240,7 @@ class DefaultCustomerSheetLoaderTest {
                     PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "pm_3")
                 ),
                 cbcEligibility = CardBrandChoiceEligibility.Ineligible,
+                loadingError = null,
             )
         )
     }
@@ -276,6 +279,7 @@ class DefaultCustomerSheetLoaderTest {
                 isGooglePayReady = false,
                 paymentSelection = null,
                 cbcEligibility = CardBrandChoiceEligibility.Ineligible,
+                loadingError = null,
             )
         )
     }

paymentsheet/src/test/java/com/stripe/android/customersheet/utils/FakeCustomerSheetLoader.kt

@@ -43,6 +43,7 @@ internal class FakeCustomerSheetLoader(
                     isGooglePayReady = isGooglePayAvailable,
                     paymentSelection = paymentSelection,
                     cbcEligibility = cbcEligibility,
+                    loadingError = null,
                 )
             )
         }