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 issue where continue button is disabled when canceling account selection #7464

Merged
merged 8 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -27,6 +28,7 @@ object PaymentIntentFactory {
unactivatedPaymentMethods = emptyList(),
setupFutureUsage = setupFutureUsage,
confirmationMethod = confirmationMethod,
paymentMethodOptionsJsonString = paymentMethodOptionsJsonString,
)

private fun createCardPaymentMethod(): PaymentMethod = PaymentMethod(
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
}
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ internal class PlaygroundTestDriver(
fun confirmNewOrGuestComplete(
testParameters: TestParameters,
values: FieldPopulator.Values = FieldPopulator.Values(),
afterAuthorization: (Selectors) -> Unit = {},
populateCustomLpmFields: () -> Unit = {},
): PlaygroundState? {
setup(testParameters)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
4 changes: 3 additions & 1 deletion paymentsheet/detekt-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down Expand Up @@ -343,6 +346,7 @@ internal class USBankAccountFormViewModel @Inject internal constructor(
primaryButtonText = application.getString(
StripeUiCoreR.string.stripe_continue_button_label
),
isProcessing = false,
)
}
}
Expand All @@ -366,6 +370,7 @@ internal class USBankAccountFormViewModel @Inject internal constructor(
primaryButtonText = application.getString(
StripeUiCoreR.string.stripe_continue_button_label
),
isProcessing = false,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ private fun USBankAccountFormArguments.updatePrimaryButton(
shouldShowProcessingWhenClicked: Boolean,
enabled: Boolean,
) {
onUpdatePrimaryButtonState(PrimaryButton.State.Ready)
onUpdatePrimaryButtonUIState {
PrimaryButton.UIState(
label = text,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ internal fun PaymentElement(
USBankAccountForm(
formArgs = formArguments,
usBankAccountFormArgs = usBankAccountFormArguments,
isProcessing = !enabled,
modifier = Modifier.padding(horizontal = horizontalPadding),
)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ class USBankAccountFormViewModelTest {
val screenStates = listOf(
USBankAccountFormScreenState.BillingDetailsCollection(
primaryButtonText = "Continue",
isProcessing = false,
),
USBankAccountFormScreenState.MandateCollection(
financialConnectionsSessionId = "session_1234",
Expand Down Expand Up @@ -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 {
Expand Down
Loading