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.

Sign up for GitHub

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 Link default card change ui states #10108

Merged
merged 7 commits into from
Feb 13, 2025
Merged

Fix Link default card change ui states #10108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d6871b3
Wallet default card fix
toluo-stripe Feb 7, 2025
a784b7b
Remove list reorder
toluo-stripe Feb 11, 2025
e5d7993
Update WalletScreen.kt
toluo-stripe Feb 11, 2025
1b2571a
Update paymentsheet.api
toluo-stripe Feb 11, 2025
788d76a
Update WalletScreenTest.kt
toluo-stripe Feb 12, 2025
e498268
Clean up loader
toluo-stripe Feb 12, 2025
fa3db55
Update WalletViewModelTest.kt
toluo-stripe Feb 12, 2025
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: 4 additions & 2 deletions paymentsheet/api/paymentsheet.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,9 +522,11 @@ public final class com/stripe/android/link/ui/verification/ComposableSingletons$

public final class com/stripe/android/link/ui/wallet/ComposableSingletons$PaymentDetailsKt {
public static final field INSTANCE Lcom/stripe/android/link/ui/wallet/ComposableSingletons$PaymentDetailsKt;
public static field lambda-1 Lkotlin/jvm/functions/Function2;
public static field lambda-1 Lkotlin/jvm/functions/Function3;
public static field lambda-2 Lkotlin/jvm/functions/Function2;
public fun <init> ()V
public final fun getLambda-1$paymentsheet_release ()Lkotlin/jvm/functions/Function2;
public final fun getLambda-1$paymentsheet_release ()Lkotlin/jvm/functions/Function3;
public final fun getLambda-2$paymentsheet_release ()Lkotlin/jvm/functions/Function2;
}

public final class com/stripe/android/link/ui/wallet/ComposableSingletons$WalletScreenKt {
Expand Down
8 changes: 6 additions & 2 deletions paymentsheet/src/main/java/com/stripe/android/link/ui/wallet/PaymentDetails.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.stripe.android.link.ui.wallet

import androidx.compose.animation.AnimatedVisibility
import androidx.compose.foundation.Image
import androidx.compose.foundation.background
import androidx.compose.foundation.clickable
Expand Down Expand Up @@ -80,7 +81,7 @@ internal fun PaymentDetailsListItem(
) {
PaymentDetails(paymentDetails = paymentDetails)

if (paymentDetails.isDefault) {
AnimatedVisibility(paymentDetails.isDefault) {
DefaultTag()
}

Expand Down Expand Up @@ -118,7 +119,9 @@ private fun MenuAndLoader(
) {
if (isUpdating) {
CircularProgressIndicator(
modifier = Modifier.size(24.dp),
modifier = Modifier
.testTag(WALLET_PAYMENT_DETAIL_ITEM_LOADING_INDICATOR)
.size(24.dp),
strokeWidth = 2.dp
)
} else {
Expand Down Expand Up @@ -262,3 +265,4 @@ private fun RowScope.BankAccountInfo(

internal const val WALLET_PAYMENT_DETAIL_ITEM_RADIO_BUTTON = "wallet_payment_detail_item_radio_button"
internal const val WALLET_PAYMENT_DETAIL_ITEM_MENU_BUTTON = "wallet_payment_detail_item_menu_button"
internal const val WALLET_PAYMENT_DETAIL_ITEM_LOADING_INDICATOR = "wallet_payment_detail_item_loading_indicator"
6 changes: 5 additions & 1 deletion paymentsheet/src/main/java/com/stripe/android/link/ui/wallet/WalletScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.stripe.android.link.ui.wallet

import androidx.compose.animation.AnimatedVisibility
import androidx.compose.animation.animateContentSize
import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.foundation.background
import androidx.compose.foundation.border
import androidx.compose.foundation.clickable
Expand Down Expand Up @@ -381,6 +382,7 @@ internal fun CollapsedPaymentDetails(
}
}

@OptIn(ExperimentalFoundationApi::class)
@Composable
private fun ExpandedPaymentDetails(
uiState: WalletUiState,
Expand Down Expand Up @@ -433,6 +435,7 @@ private fun ExpandedPaymentDetails(
}
}

@OptIn(ExperimentalFoundationApi::class)
@Composable
private fun PaymentDetailsList(
uiState: WalletUiState,
Expand All @@ -453,11 +456,12 @@ private fun PaymentDetailsList(
) { item ->
PaymentDetailsListItem(
modifier = Modifier
.animateItemPlacement()
.testTag(WALLET_SCREEN_PAYMENT_METHODS_LIST),
paymentDetails = item,
enabled = isEnabled,
isSelected = uiState.selectedItem?.id == item.id,
isUpdating = false,
isUpdating = uiState.cardBeingUpdated == item.id,
onClick = {
onItemSelected(item)
},
Expand Down
27 changes: 17 additions & 10 deletions paymentsheet/src/main/java/com/stripe/android/link/ui/wallet/WalletUiState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ internal data class WalletUiState(
val primaryButtonLabel: ResolvableString,
val hasCompleted: Boolean,
val canAddNewPaymentMethod: Boolean,
val cardBeingUpdated: String? = null,
val errorMessage: ResolvableString? = null,
val expiryDateInput: FormFieldEntry = FormFieldEntry(null),
val cvcInput: FormFieldEntry = FormFieldEntry(null),
Expand All @@ -35,16 +36,21 @@ internal data class WalletUiState(
val isMissingCvcInput = cvcInput.isComplete.not()

val disableButton = (isExpired && isMissingExpiryDateInput) ||
(requiresCvcRecollection && isMissingCvcInput)
(requiresCvcRecollection && isMissingCvcInput) || (cardBeingUpdated != null)

return if (hasCompleted) {
PrimaryButtonState.Completed
} else if (isProcessing) {
PrimaryButtonState.Processing
} else if (disableButton) {
PrimaryButtonState.Disabled
} else {
PrimaryButtonState.Enabled
return when {
hasCompleted -> {
PrimaryButtonState.Completed
}
isProcessing -> {
PrimaryButtonState.Processing
}
disableButton -> {
PrimaryButtonState.Disabled
}
else -> {
PrimaryButtonState.Enabled
}
}
}

Expand All @@ -66,7 +72,8 @@ internal data class WalletUiState(
return copy(
paymentDetailsList = response.paymentDetails,
selectedItem = selectedItem,
isProcessing = false
isProcessing = false,
cardBeingUpdated = null
)
}
}
68 changes: 43 additions & 25 deletions paymentsheet/src/main/java/com/stripe/android/link/ui/wallet/WalletViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,13 @@ internal class WalletViewModel @Inject constructor(
)

init {
loadPaymentDetails()
_uiState.update {
it.setProcessing()
}

viewModelScope.launch {
loadPaymentDetails(selectedItemId = null)
}

viewModelScope.launch {
expiryDateController.formFieldValue.collectLatest { input ->
Expand All @@ -93,28 +99,22 @@ internal class WalletViewModel @Inject constructor(
}
}

private fun loadPaymentDetails(selectedItemId: String? = null) {
_uiState.update {
it.setProcessing()
}

viewModelScope.launch {
linkAccountManager.listPaymentDetails(
paymentMethodTypes = stripeIntent.supportedPaymentMethodTypes(linkAccount)
).fold(
onSuccess = { response ->
_uiState.update {
it.updateWithResponse(response, selectedItemId = selectedItemId)
}
private suspend fun loadPaymentDetails(selectedItemId: String?) {
linkAccountManager.listPaymentDetails(
paymentMethodTypes = stripeIntent.supportedPaymentMethodTypes(linkAccount)
).fold(
onSuccess = { response ->
_uiState.update {
it.updateWithResponse(response, selectedItemId = selectedItemId)
}

if (response.paymentDetails.isEmpty()) {
navigateAndClearStack(LinkScreen.PaymentMethod)
}
},
// If we can't load the payment details there's nothing to see here
onFailure = ::onFatal
)
}
if (response.paymentDetails.isEmpty()) {
navigateAndClearStack(LinkScreen.PaymentMethod)
}
},
// If we can't load the payment details there's nothing to see here
onFailure = ::onFatal
)
}

private fun onFatal(fatalError: Throwable) {
Expand Down Expand Up @@ -255,7 +255,9 @@ internal class WalletViewModel @Inject constructor(

fun onSetDefaultClicked(item: ConsumerPaymentDetails.PaymentDetails) {
_uiState.update {
it.setProcessing()
it.copy(
cardBeingUpdated = item.id,
)
}
viewModelScope.launch {
val updateParams = ConsumerPaymentDetailsUpdateParams(
Expand All @@ -266,7 +268,22 @@ internal class WalletViewModel @Inject constructor(
linkAccountManager.updatePaymentDetails(updateParams)
.fold(
onSuccess = {
loadPaymentDetails()
_uiState.update { state ->
state.copy(
paymentDetailsList = state.paymentDetailsList.map { details ->
when (details) {
is ConsumerPaymentDetails.BankAccount -> {
details.copy(isDefault = item.id == details.id)
}
is ConsumerPaymentDetails.Card -> {
details.copy(isDefault = item.id == details.id)
}
is ConsumerPaymentDetails.Passthrough -> details
}
},
cardBeingUpdated = null
)
}
},
onFailure = { error ->
updateErrorMessageAndStopProcessing(
Expand Down Expand Up @@ -304,7 +321,8 @@ internal class WalletViewModel @Inject constructor(
_uiState.update {
it.copy(
alertMessage = error.stripeErrorMessage(),
isProcessing = false
isProcessing = false,
cardBeingUpdated = null
)
}
}
Expand Down
51 changes: 51 additions & 0 deletions paymentsheet/src/test/java/com/stripe/android/link/ui/wallet/WalletScreenTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,21 @@ import com.stripe.android.link.ui.PrimaryButtonState
import com.stripe.android.link.ui.PrimaryButtonTag
import com.stripe.android.model.CardBrand
import com.stripe.android.model.ConsumerPaymentDetails
import com.stripe.android.model.ConsumerPaymentDetailsUpdateParams
import com.stripe.android.model.CvcCheck
import com.stripe.android.testing.CoroutineTestRule
import com.stripe.android.testing.FakeLogger
import com.stripe.android.ui.core.elements.CvcController
import com.stripe.android.uicore.elements.DateConfig
import com.stripe.android.uicore.elements.SimpleTextFieldController
import com.stripe.android.uicore.utils.stateFlowOf
import kotlinx.coroutines.delay
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runTest
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import kotlin.time.Duration.Companion.seconds
import com.stripe.android.link.confirmation.Result as LinkConfirmationResult

@RunWith(AndroidJUnit4::class)
Expand Down Expand Up @@ -480,6 +483,51 @@ internal class WalletScreenTest {
onWalletPaymentMethodMenu().assertIsDisplayed()
}

@Test
fun `pay method row is loading when card is being updated`() = runTest(dispatcher) {
val linkAccountManager = object : FakeLinkAccountManager() {
override suspend fun updatePaymentDetails(
updateParams: ConsumerPaymentDetailsUpdateParams
): Result<ConsumerPaymentDetails> {
delay(1.seconds)
return super.updatePaymentDetails(updateParams)
}
}
val card1 = TestFactory.CONSUMER_PAYMENT_DETAILS_CARD.copy(id = "card1", isDefault = false)
val card2 = TestFactory.CONSUMER_PAYMENT_DETAILS_CARD.copy(id = "card2", isDefault = true)
linkAccountManager.listPaymentDetailsResult = Result.success(
ConsumerPaymentDetails(paymentDetails = listOf(card1, card2))
)

val viewModel = createViewModel(linkAccountManager)
composeTestRule.setContent {
WalletScreen(
viewModel = viewModel,
showBottomSheetContent = {},
hideBottomSheetContent = {}
)
}
composeTestRule.waitForIdle()

onCollapsedWalletRow()
.performClick()
composeTestRule.waitForIdle()

viewModel.onSetDefaultClicked(card1)

composeTestRule.waitForIdle()

onWalletPaymentMethodRowLoadingIndicator().assertIsDisplayed()
Comment on lines +518 to +520
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this introduce flakiness? Should we use waitUntilExists instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we have flakiness after waitForIdle, then the screen is not behaving as expected. The loader state is changed on the main thread before kicking of any coroutines to update the card

onWalletPayButton().assertIsNotEnabled()

dispatcher.scheduler.advanceTimeBy(1.1.seconds)

onWalletPaymentMethodRowLoadingIndicator().assertDoesNotExist()
onWalletPayButton()
.assertExists()
.assertIsEnabled()
}

@Test
fun `wallet menu is dismissed on cancel clicked`() = runTest(dispatcher) {
testMenu(
Expand Down Expand Up @@ -654,6 +702,9 @@ internal class WalletScreenTest {
private fun onWalletPaymentMethodRowMenuButton() =
composeTestRule.onAllNodes(hasTestTag(WALLET_PAYMENT_DETAIL_ITEM_MENU_BUTTON), useUnmergedTree = true)

private fun onWalletPaymentMethodRowLoadingIndicator() =
composeTestRule.onNodeWithTag(WALLET_PAYMENT_DETAIL_ITEM_LOADING_INDICATOR, useUnmergedTree = true)

private fun onWalletPaymentMethodMenu() =
composeTestRule.onNodeWithTag(WALLET_SCREEN_MENU_SHEET_TAG, useUnmergedTree = true)

Expand Down
12 changes: 12 additions & 0 deletions paymentsheet/src/test/java/com/stripe/android/link/ui/wallet/WalletUiStateTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ class WalletUiStateTest {
assertThat(state.primaryButtonState).isEqualTo(PrimaryButtonState.Enabled)
}

@Test
fun testDisabledButtonStateWhenCardIsBeingUpdated() {
val state = walletUiState(
selectedItem = TestFactory.CONSUMER_PAYMENT_DETAILS_CARD,
cardBeingUpdated = "id"
)

assertThat(state.primaryButtonState).isEqualTo(PrimaryButtonState.Disabled)
}

private fun walletUiState(
paymentDetailsList: List<ConsumerPaymentDetails.PaymentDetails> =
TestFactory.CONSUMER_PAYMENT_DETAILS.paymentDetails,
Expand All @@ -157,6 +167,7 @@ class WalletUiStateTest {
expiryDateInput: FormFieldEntry = FormFieldEntry(null),
cvcInput: FormFieldEntry = FormFieldEntry(null),
canAddNewPaymentMethod: Boolean = true,
cardBeingUpdated: String? = null
): WalletUiState {
return WalletUiState(
paymentDetailsList = paymentDetailsList,
Expand All @@ -167,6 +178,7 @@ class WalletUiStateTest {
expiryDateInput = expiryDateInput,
cvcInput = cvcInput,
canAddNewPaymentMethod = canAddNewPaymentMethod,
cardBeingUpdated = cardBeingUpdated
)
}
}
Loading
Loading