Fix issue where continue button is disabled when canceling account se…

…lection (#7464) * Fix issue where pay button is disabled when canceling account selection
stripe · Oct 30, 2023 · a47ee61 · a47ee61
1 parent 4c5c732
commit a47ee61
Show file tree

Hide file tree

Showing 16 changed files with 190 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### PaymentSheet
 * [FIXED][7499](https://github.com/stripe/stripe-android/pull/7499) Fixed an issue with incorrect error messages when encountering a failure after 3D Secure authentication.
+* [FIXED][7464](https://github.com/stripe/stripe-android/pull/7464) Fixed an issue where canceling the US Bank Account selection flow prevents the user from launching it again.
 * [FIXED][7529](https://github.com/stripe/stripe-android/pull/7529) PaymentSheet no longer displays saved cards that originated from Apple Pay or Google Pay.
 
 ## 20.34.1 - 2023-10-24

diff --git a/payments-core-testing/src/main/java/com/stripe/android/testing/PaymentIntentFactory.kt b/payments-core-testing/src/main/java/com/stripe/android/testing/PaymentIntentFactory.kt
@@ -13,6 +13,7 @@ object PaymentIntentFactory {
         setupFutureUsage: StripeIntent.Usage? = null,
         confirmationMethod: PaymentIntent.ConfirmationMethod = PaymentIntent.ConfirmationMethod.Automatic,
         status: StripeIntent.Status = StripeIntent.Status.RequiresConfirmation,
+        paymentMethodOptionsJsonString: String? = null,
     ): PaymentIntent = PaymentIntent(
         created = 500L,
         amount = 1000L,
@@ -27,6 +28,7 @@ object PaymentIntentFactory {
         unactivatedPaymentMethods = emptyList(),
         setupFutureUsage = setupFutureUsage,
         confirmationMethod = confirmationMethod,
+        paymentMethodOptionsJsonString = paymentMethodOptionsJsonString,
     )
 
     private fun createCardPaymentMethod(): PaymentMethod = PaymentMethod(

diff --git a/paymentsheet-example/src/androidTest/java/com/stripe/android/lpm/TestUSBankAccount.kt b/paymentsheet-example/src/androidTest/java/com/stripe/android/lpm/TestUSBankAccount.kt
@@ -0,0 +1,40 @@
+package com.stripe.android.lpm
+
+import androidx.compose.ui.test.assertIsEnabled
+import androidx.compose.ui.test.hasTestTag
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.stripe.android.BasePlaygroundTest
+import com.stripe.android.paymentsheet.example.playground.settings.Country
+import com.stripe.android.paymentsheet.example.playground.settings.CountrySettingsDefinition
+import com.stripe.android.paymentsheet.example.playground.settings.Currency
+import com.stripe.android.paymentsheet.example.playground.settings.CurrencySettingsDefinition
+import com.stripe.android.paymentsheet.example.playground.settings.DelayedPaymentMethodsSettingsDefinition
+import com.stripe.android.paymentsheet.ui.PAYMENT_SHEET_PRIMARY_BUTTON_TEST_TAG
+import com.stripe.android.test.core.AuthorizeAction
+import com.stripe.android.test.core.TestParameters
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+internal class TestUSBankAccount : BasePlaygroundTest() {
+    private val testParameters = TestParameters.create(
+        paymentMethodCode = "us_bank_account",
+    ) { settings ->
+        settings[CountrySettingsDefinition] = Country.US
+        settings[CurrencySettingsDefinition] = Currency.USD
+        settings[DelayedPaymentMethodsSettingsDefinition] = true
+    }
+
+    @Test
+    fun testUSBankAccountCancelAllowsUserToContinue() {
+        testDriver.confirmUSBankAccount(
+            testParameters = testParameters.copy(
+                authorizationAction = AuthorizeAction.Cancel,
+            ),
+            afterAuthorization = {
+                rules.compose.onNode(hasTestTag(PAYMENT_SHEET_PRIMARY_BUTTON_TEST_TAG))
+                    .assertIsEnabled()
+            }
+        )
+    }
+}
diff --git a/...ntsheet-example/src/androidTest/java/com/stripe/android/test/core/PlaygroundTestDriver.kt b/...ntsheet-example/src/androidTest/java/com/stripe/android/test/core/PlaygroundTestDriver.kt
@@ -258,6 +258,7 @@ internal class PlaygroundTestDriver(
     fun confirmNewOrGuestComplete(
         testParameters: TestParameters,
         values: FieldPopulator.Values = FieldPopulator.Values(),
+        afterAuthorization: (Selectors) -> Unit = {},
         populateCustomLpmFields: () -> Unit = {},
     ): PlaygroundState? {
         setup(testParameters)
@@ -285,6 +286,46 @@ internal class PlaygroundTestDriver(
 
         doAuthorization()
 
+        afterAuthorization(selectors)
+
+        teardown()
+
+        return result
+    }
+
+    fun confirmUSBankAccount(
+        testParameters: TestParameters,
+        values: FieldPopulator.Values = FieldPopulator.Values(),
+        afterAuthorization: (Selectors) -> Unit = {},
+        populateCustomLpmFields: () -> Unit = {},
+    ): PlaygroundState? {
+        setup(testParameters)
+        launchComplete()
+
+        selectors.paymentSelection.click()
+
+        FieldPopulator(
+            selectors,
+            testParameters,
+            populateCustomLpmFields,
+            values = values,
+        ).populateFields()
+
+        // Verify device requirements are met prior to attempting confirmation.  Do this
+        // after we have had the chance to capture a screenshot.
+        verifyDeviceSupportsTestAuthorization(
+            testParameters.authorizationAction,
+            testParameters.useBrowser
+        )
+
+        val result = playgroundState
+
+        pressBuy()
+
+        doUSBankAccountAuthorization()
+
+        afterAuthorization(selectors)
+
         teardown()
 
         return result
@@ -560,6 +601,31 @@ internal class PlaygroundTestDriver(
         }
     }
 
+    private fun doUSBankAccountAuthorization() {
+        selectors.apply {
+            if (testParameters.authorizationAction != null) {
+                if (testParameters.authorizationAction?.requiresBrowser == true) {
+                    // If a specific browser is requested we will use it, otherwise, we will
+                    // select the first browser found
+                    val selectedBrowser = getBrowser(BrowserUI.convert(testParameters.useBrowser))
+
+                    // If there are multiple browser there is a browser selector window
+                    selectBrowserPrompt.wait(4000)
+                    if (selectBrowserPrompt.exists()) {
+                        browserIconAtPrompt(selectedBrowser).click()
+                    }
+
+                    assertThat(browserWindow(selectedBrowser)?.exists()).isTrue()
+
+                    blockUntilUSBankAccountPageLoaded()
+                }
+                if (testParameters.authorizationAction == AuthorizeAction.Cancel) {
+                    selectors.authorizeAction?.click()
+                }
+            }
+        }
+    }
+
     internal fun setup(testParameters: TestParameters) {
         this.testParameters = testParameters
         this.selectors = Selectors(device, composeTestRule, testParameters)

diff --git a/paymentsheet-example/src/androidTest/java/com/stripe/android/test/core/ui/Selectors.kt b/paymentsheet-example/src/androidTest/java/com/stripe/android/test/core/ui/Selectors.kt
@@ -91,6 +91,18 @@ internal class Selectors(
         device.waitForIdle()
     }
 
+    fun blockUntilUSBankAccountPageLoaded() {
+        assertThat(
+            device.wait(
+                Until.findObject(
+                    By.textContains("Agree and continue")
+                ),
+                HOOKS_PAGE_LOAD_TIMEOUT * 1000
+            )
+        ).isNotNull()
+        device.waitForIdle()
+    }
+
     fun getInstalledBrowsers() = getInstalledPackages()
         .mapNotNull {
             when (it.packageName) {

diff --git a/paymentsheet-example/src/androidTest/java/com/stripe/android/utils/LpmRepositoryKtx.kt b/paymentsheet-example/src/androidTest/java/com/stripe/android/utils/LpmRepositoryKtx.kt
@@ -7,12 +7,22 @@ import com.stripe.android.ui.core.forms.resources.LpmRepository
 
 internal fun initializedLpmRepository(context: Context): LpmRepository {
     val repository = LpmRepository(
-        LpmRepository.LpmRepositoryArguments(context.resources)
+        LpmRepository.LpmRepositoryArguments(
+            resources = context.resources,
+            isFinancialConnectionsAvailable = { true }
+        )
     )
 
     repository.update(
         stripeIntent = PaymentIntentFactory.create(
             paymentMethodTypes = PaymentMethod.Type.values().map { it.code },
+            paymentMethodOptionsJsonString = """
+                {
+                    "us_bank_account": {
+                        "verification_method": "automatic"
+                    }
+              }
+            """.trimIndent()
         ),
         serverLpmSpecs = null,
     )

diff --git a/paymentsheet/detekt-baseline.xml b/paymentsheet/detekt-baseline.xml
@@ -4,6 +4,7 @@
   <CurrentIssues>
     <ID>CyclomaticComplexMethod:CustomerSheetViewModel.kt$CustomerSheetViewModel$fun handleViewAction(viewAction: CustomerSheetViewAction)</ID>
     <ID>CyclomaticComplexMethod:PlaceholderHelper.kt$PlaceholderHelper$@VisibleForTesting internal fun specForPlaceholderField( field: PlaceholderField, placeholderOverrideList: List&lt;IdentifierSpec>, requiresMandate: Boolean, configuration: PaymentSheet.BillingDetailsCollectionConfiguration, )</ID>
+    <ID>CyclomaticComplexMethod:USBankAccountFormScreenState.kt$USBankAccountFormScreenState$fun copy( error: Int? = null, primaryButtonText: String? = null, mandateText: String? = null, isProcessing: Boolean? = null, ): USBankAccountFormScreenState</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>
     <ID>FunctionNaming:PaymentSheetTopBar.kt$@Preview @Composable internal fun PaymentSheetTopBar_Preview()</ID>
@@ -30,8 +31,9 @@
     <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:PlaceholderHelperTest.kt$PlaceholderHelperTest$@Test fun `Test correct placeholder is removed for placeholder spec`()</ID>
-    <ID>LongMethod:USBankAccountForm.kt$@Composable internal fun USBankAccountForm( formArgs: FormArguments, usBankAccountFormArgs: USBankAccountFormArguments, isProcessing: Boolean, modifier: Modifier = Modifier, )</ID>
+    <ID>LongMethod:USBankAccountForm.kt$@Composable internal fun USBankAccountForm( formArgs: FormArguments, usBankAccountFormArgs: USBankAccountFormArguments, modifier: Modifier = Modifier, )</ID>
     <ID>LongMethod:USBankAccountForm.kt$@Composable private fun AccountDetailsForm( formArgs: FormArguments, isProcessing: Boolean, bankName: String?, last4: String?, saveForFutureUseElement: SaveForFutureUseElement, onRemoveAccount: () -> Unit, )</ID>
+    <ID>LongMethod:USBankAccountFormViewModelTest.kt$USBankAccountFormViewModelTest$@Test fun `Restores screen state when re-opening screen`()</ID>
     <ID>MagicNumber:AutocompleteScreen.kt$0.07f</ID>
     <ID>MagicNumber:BaseSheetActivity.kt$BaseSheetActivity$30</ID>
     <ID>MagicNumber:BottomSheet.kt$BottomSheetState$10</ID>

diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/model/PaymentSelection.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/model/PaymentSelection.kt
@@ -143,6 +143,14 @@ internal sealed class PaymentSelection : Parcelable {
             override val paymentMethodOptionsParams: PaymentMethodOptionsParams? = null,
         ) : New() {
 
+            override fun mandateText(
+                context: Context,
+                merchantName: String,
+                isSaveForFutureUseSelected: Boolean,
+            ): String? {
+                return screenState.mandateText
+            }
+
             @Parcelize
             data class Input(
                 val name: String,

diff --git a/...n/java/com/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountEmitters.kt b/...n/java/com/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountEmitters.kt
@@ -58,7 +58,7 @@ internal fun USBankAccountEmitters(
         usBankAccountFormArgs.handleScreenStateChanged(
             context = context,
             screenState = screenState,
-            enabled = hasRequiredFields,
+            enabled = hasRequiredFields && !screenState.isProcessing,
             merchantName = viewModel.formattedMerchantName(),
             onPrimaryButtonClick = viewModel::handlePrimaryButtonClick,
         )

diff --git a/.../main/java/com/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountForm.kt b/.../main/java/com/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountForm.kt
@@ -53,7 +53,6 @@ import com.stripe.android.ui.core.R as PaymentsUiCoreR
 internal fun USBankAccountForm(
     formArgs: FormArguments,
     usBankAccountFormArgs: USBankAccountFormArguments,
-    isProcessing: Boolean,
     modifier: Modifier = Modifier,
 ) {
     val viewModel = viewModel<USBankAccountFormViewModel>(
@@ -84,7 +83,7 @@ internal fun USBankAccountForm(
             is USBankAccountFormScreenState.BillingDetailsCollection -> {
                 BillingDetailsCollectionScreen(
                     formArgs = formArgs,
-                    isProcessing = isProcessing,
+                    isProcessing = screenState.isProcessing,
                     nameController = viewModel.nameController,
                     emailController = viewModel.emailController,
                     phoneController = viewModel.phoneController,
@@ -96,7 +95,7 @@ internal fun USBankAccountForm(
             is USBankAccountFormScreenState.MandateCollection -> {
                 MandateCollectionScreen(
                     formArgs = formArgs,
-                    isProcessing = isProcessing,
+                    isProcessing = screenState.isProcessing,
                     screenState = screenState,
                     nameController = viewModel.nameController,
                     emailController = viewModel.emailController,
@@ -111,7 +110,7 @@ internal fun USBankAccountForm(
             is USBankAccountFormScreenState.VerifyWithMicrodeposits -> {
                 VerifyWithMicrodepositsScreen(
                     formArgs = formArgs,
-                    isProcessing = isProcessing,
+                    isProcessing = screenState.isProcessing,
                     screenState = screenState,
                     nameController = viewModel.nameController,
                     emailController = viewModel.emailController,
@@ -126,7 +125,7 @@ internal fun USBankAccountForm(
             is USBankAccountFormScreenState.SavedAccount -> {
                 SavedAccountScreen(
                     formArgs = formArgs,
-                    isProcessing = isProcessing,
+                    isProcessing = screenState.isProcessing,
                     screenState = screenState,
                     nameController = viewModel.nameController,
                     emailController = viewModel.emailController,

diff --git a/...com/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountFormScreenState.kt b/...com/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountFormScreenState.kt
@@ -7,7 +7,8 @@ import com.stripe.android.financialconnections.model.FinancialConnectionsAccount
 import kotlinx.parcelize.Parcelize
 
 internal sealed class USBankAccountFormScreenState(
-    @StringRes open val error: Int? = null
+    @StringRes open val error: Int? = null,
+    open val isProcessing: Boolean = false
 ) : Parcelable {
     abstract val primaryButtonText: String
     abstract val mandateText: String?
@@ -16,6 +17,7 @@ internal sealed class USBankAccountFormScreenState(
     data class BillingDetailsCollection(
         @StringRes override val error: Int? = null,
         override val primaryButtonText: String,
+        override val isProcessing: Boolean,
     ) : USBankAccountFormScreenState() {
 
         override val mandateText: String?

diff --git a/...a/com/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountFormViewModel.kt b/...a/com/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountFormViewModel.kt
@@ -308,6 +308,9 @@ internal class USBankAccountFormViewModel @Inject internal constructor(
     fun handlePrimaryButtonClick(screenState: USBankAccountFormScreenState) {
         when (screenState) {
             is USBankAccountFormScreenState.BillingDetailsCollection -> {
+                _currentScreenState.update {
+                    screenState.copy(isProcessing = true)
+                }
                 collectBankAccount(args.clientSecret)
             }
             is USBankAccountFormScreenState.MandateCollection ->
@@ -343,6 +346,7 @@ internal class USBankAccountFormViewModel @Inject internal constructor(
                 primaryButtonText = application.getString(
                     StripeUiCoreR.string.stripe_continue_button_label
                 ),
+                isProcessing = false,
             )
         }
     }
@@ -366,6 +370,7 @@ internal class USBankAccountFormViewModel @Inject internal constructor(
                 primaryButtonText = application.getString(
                     StripeUiCoreR.string.stripe_continue_button_label
                 ),
+                isProcessing = false,
             )
         }
     }

diff --git a/...om/stripe/android/paymentsheet/paymentdatacollection/ach/UsBankAccountFormArgumentsKtx.kt b/...om/stripe/android/paymentsheet/paymentdatacollection/ach/UsBankAccountFormArgumentsKtx.kt
@@ -39,7 +39,6 @@ private fun USBankAccountFormArguments.updatePrimaryButton(
     shouldShowProcessingWhenClicked: Boolean,
     enabled: Boolean,
 ) {
-    onUpdatePrimaryButtonState(PrimaryButton.State.Ready)
     onUpdatePrimaryButtonUIState {
         PrimaryButton.UIState(
             label = text,

diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/PaymentElement.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/PaymentElement.kt
@@ -69,7 +69,6 @@ internal fun PaymentElement(
                 USBankAccountForm(
                     formArgs = formArguments,
                     usBankAccountFormArgs = usBankAccountFormArguments,
-                    isProcessing = !enabled,
                     modifier = Modifier.padding(horizontal = horizontalPadding),
                 )
             } else {

diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/model/PaymentSelectionTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/model/PaymentSelectionTest.kt
@@ -133,6 +133,24 @@ class PaymentSelectionTest {
         )
     }
 
+    @Test
+    fun `Displays the correct mandate for US Bank Account`() {
+        val paymentSelection = PaymentSelection.Saved(
+            paymentMethod = PaymentMethodFactory.usBankAccount(),
+        )
+
+        val result = paymentSelection.mandateText(
+            context = context,
+            merchantName = "Merchant",
+            isSaveForFutureUseSelected = false,
+        )
+
+        assertThat(result).isEqualTo(
+            "By continuing, you agree to authorize payments pursuant to " +
+                "<a href=\"https://stripe.com/ach-payments/authorization\">these terms</a>."
+        )
+    }
+
     @Test
     fun `Doesn't display a mandate for a saved payment method that isn't US bank account`() =
         runAllConfigurations { isSaveForFutureUseSelected ->

diff --git a/...m/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountFormViewModelTest.kt b/...m/stripe/android/paymentsheet/paymentdatacollection/ach/USBankAccountFormViewModelTest.kt
@@ -353,6 +353,7 @@ class USBankAccountFormViewModelTest {
         val screenStates = listOf(
             USBankAccountFormScreenState.BillingDetailsCollection(
                 primaryButtonText = "Continue",
+                isProcessing = false,
             ),
             USBankAccountFormScreenState.MandateCollection(
                 financialConnectionsSessionId = "session_1234",
@@ -835,6 +836,21 @@ class USBankAccountFormViewModelTest {
         }
     }
 
+    @Test
+    fun `When the primary button is pressed, the primary button state moves to processing`() = runTest {
+        val viewModel = createViewModel()
+
+        viewModel.currentScreenState.test {
+            assertThat(awaitItem().isProcessing)
+                .isFalse()
+
+            viewModel.handlePrimaryButtonClick(viewModel.currentScreenState.value)
+
+            assertThat(awaitItem().isProcessing)
+                .isTrue()
+        }
+    }
+
     private fun createViewModel(
         args: USBankAccountFormViewModel.Args = defaultArgs
     ): USBankAccountFormViewModel {