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

Introduce CardEditUIHandler for the CardEditUI #10462

Merged
merged 10 commits into from
Apr 3, 2025
Merged

Introduce CardEditUIHandler for the CardEditUI #10462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b73822a
Introduce CardEditUIHandler for the CardEditUI
toluo-stripe Mar 21, 2025
68d6fc1
Revert CommonTextField change
toluo-stripe Mar 31, 2025
138da16
Update PaymentMethodFixtures.kt
toluo-stripe Mar 31, 2025
22adff3
Rename to EditCardDetailsInteractor
toluo-stripe Mar 31, 2025
9c64789
Address comments
toluo-stripe Apr 1, 2025
5fa25cf
Integrate with UpdatePMInteractor
toluo-stripe Apr 1, 2025
d7918ea
Remove factory
toluo-stripe Apr 2, 2025
e0bf36c
Fix flaky tests
toluo-stripe Apr 3, 2025
662df22
Remove nullable payment method receiver
toluo-stripe Apr 3, 2025
6070548
Fallback to unknown card brand
toluo-stripe Apr 3, 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
33 changes: 33 additions & 0 deletions paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/CardDetailsEntry.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.stripe.android.paymentsheet.ui

import com.stripe.android.paymentsheet.CardUpdateParams

/**
* Represents the editable details of a card payment method.
*
* @property cardBrandChoice The currently selected card brand choice.
*/
internal data class CardDetailsEntry(
val cardBrandChoice: CardBrandChoice,
) {
/**
* Determines if the card details have changed compared to the provided values.
*
* @param originalCardBrandChoice The card brand choice to compare against.
* @return True if any of the card details have changed, false otherwise.
*/
fun hasChanged(
originalCardBrandChoice: CardBrandChoice,
): Boolean {
return originalCardBrandChoice != this.cardBrandChoice
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: Are we expecting to add parameters to CardDetailsEntry that we shouldn't compare?

  • If no, can we instead rely on the generated equals of data class instead of hasChanged?
  • If yes, is there anything we can remove or reduce to make it possible?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also should we just compare entries here?

fun hasChanged(
        cardDetailsEntry: CardDetailsEntry,
    ): Boolean

Copy link
Contributor Author

@toluo-stripe toluo-stripe Apr 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We're not comparing two versions of CardDetailsEntry, distinctUntilChanged would have sufficed in that scenario. We're comparing against the existing card and billing details (taking billingDetailsCollectionMode into account).

I have the full logic on my prototype branch here - https://github.com/stripe/stripe-android/pull/10366/files#r2025490382

}

/**
* Converts the CardDetailsEntry to CardUpdateParams.
*
* @return CardUpdateParams containing the updated card brand.
*/
internal fun CardDetailsEntry.toUpdateParams(): CardUpdateParams {
return CardUpdateParams(cardBrand = cardBrandChoice.brand)
}
31 changes: 26 additions & 5 deletions paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/CardDetailsUI.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import androidx.compose.material.Card
import androidx.compose.material.Divider
import androidx.compose.material.MaterialTheme
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
Expand All @@ -22,21 +23,41 @@ import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import com.stripe.android.CardBrandFilter
import com.stripe.android.R
import com.stripe.android.model.CardBrand
import com.stripe.android.model.PaymentMethod
import com.stripe.android.uicore.getBorderStroke
import com.stripe.android.uicore.stripeColors
import com.stripe.android.uicore.stripeShapes
import com.stripe.android.uicore.utils.collectAsState

@Composable
internal fun CardDetailsEditUI(
editCardDetailsInteractor: EditCardDetailsInteractor,
isExpired: Boolean,
) {
val state by editCardDetailsInteractor.state.collectAsState()

CardDetailsEditUI(
shouldShowCardBrandDropdown = state.shouldShowCardBrandDropdown,
selectedBrand = state.selectedCardBrand,
card = state.card,
isExpired = isExpired,
availableNetworks = state.availableNetworks,
paymentMethodIcon = state.paymentMethodIcon,
onBrandChoiceChanged = {
editCardDetailsInteractor.handleViewAction(EditCardDetailsInteractor.ViewAction.BrandChoiceChanged(it))
}
)
}

@Composable
private fun CardDetailsEditUI(
shouldShowCardBrandDropdown: Boolean,
selectedBrand: CardBrandChoice,
card: PaymentMethod.Card,
isExpired: Boolean,
cardBrandFilter: CardBrandFilter,
availableNetworks: List<CardBrandChoice>,
@DrawableRes paymentMethodIcon: Int,
onBrandChoiceChanged: (CardBrandChoice) -> Unit
) {
Expand All @@ -52,7 +73,7 @@ internal fun CardDetailsEditUI(
card = card,
selectedBrand = selectedBrand,
shouldShowCardBrandDropdown = shouldShowCardBrandDropdown,
cardBrandFilter = cardBrandFilter,
availableNetworks = availableNetworks,
savedPaymentMethodIcon = paymentMethodIcon,
onBrandChoiceChanged = onBrandChoiceChanged,
)
Expand Down Expand Up @@ -88,7 +109,7 @@ internal fun CardDetailsEditUI(
private fun CardNumberField(
card: PaymentMethod.Card,
selectedBrand: CardBrandChoice,
cardBrandFilter: CardBrandFilter,
availableNetworks: List<CardBrandChoice>,
shouldShowCardBrandDropdown: Boolean,
savedPaymentMethodIcon: Int,
onBrandChoiceChanged: (CardBrandChoice) -> Unit,
Expand All @@ -100,7 +121,7 @@ private fun CardNumberField(
if (shouldShowCardBrandDropdown) {
CardBrandDropdown(
selectedBrand = selectedBrand,
availableBrands = card.getAvailableNetworks(cardBrandFilter),
availableBrands = availableNetworks,
onBrandChoiceChanged = onBrandChoiceChanged,
)
} else {
Expand Down
147 changes: 147 additions & 0 deletions paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/EditCardDetailsInteractor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package com.stripe.android.paymentsheet.ui

import androidx.compose.runtime.Immutable
import com.stripe.android.CardBrandFilter
import com.stripe.android.DefaultCardBrandFilter
import com.stripe.android.core.utils.DateUtils
import com.stripe.android.model.CardBrand
import com.stripe.android.model.PaymentMethod
import com.stripe.android.paymentsheet.CardUpdateParams
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.mapLatest
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch

internal typealias CardUpdateParamsCallback = (CardUpdateParams?) -> Unit

internal typealias CardBrandCallback = (CardBrand) -> Unit

internal interface EditCardDetailsInteractor {
val state: StateFlow<State>

val onCardUpdateParamsChanged: CardUpdateParamsCallback
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this still need to be exposed? Is the UI going to call it somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yh, it needs to be exposed. It is used in the UpdatePaymentMethodInteractor to consume the latest CardUpdateParams

EditCardDetailsInteractor.create(
    card = displayableSavedPaymentMethod.savedPaymentMethod.card,
    onCardUpdateParamsChanged = {
        // CardParams update
        this.cardUpdateParams.value = it
    },
    coroutineScope = coroutineScope,
    isModifiable = displayableSavedPaymentMethod.isModifiable(),
    cardBrandFilter = cardBrandFilter,
    onBrandChoiceChanged = onBrandChoiceSelected,
)

The UpdatePaymentMethodInteractor uses this value to determine if the Save button should be enabled. It also uses this value to execute the update operation.

private suspend fun maybeUpdateCard(): Result<PaymentMethod>? {
    val cardUpdateParams = cardUpdateParams.value
    return if (cardUpdateParams != null) {
        updatePaymentMethodExecutor(
            displayableSavedPaymentMethod.paymentMethod,
            cardUpdateParams
        ).onSuccess {
            this.cardUpdateParams.value = null
        }
    } else {
        null
    }
}

Copy link
Collaborator

@samer-stripe samer-stripe Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Would passing it through the create method be enough? Is there ever a reason for a consumer to call interactor.onCardUpdateParamsChanged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Emitting CardUpdateParams is a core part of this interactor's behaviour and is used in testing. An example is this one:
We're testing that the save button is enabled when there is a non-null CardUpdateParams.

@Test
fun updatingCard_updatesStateAsExpected(: {
    var updatedPaymentMethod: PaymentMethod? = null
    val initialPaymentMethod = PaymentMethodFixtures.CARD_WITH_NETWORKS_PAYMENT_METHOD

    runScenario(
        displayableSavedPaymentMethod = initialPaymentMethod.toDisplayableSavedPaymentMethod(),
        updatePaymentMethodExecutor = { paymentMethod, _ ->
            updatedPaymentMethod = paymentMethod
            Result.success(paymentMethod)
        },
        onUpdateSuccess = {},
    ) {
        val editCardDetailsInteractor = interactor.editCardDetailsInteractorHelper()

        editCardDetailsInteractor.onCardUpdateParamsChanged(
            CardUpdateParams(
                cardBrand = CardBrand.CartesBancaires
            )
        )

        interactor.state.test {
            assertThat(awaitItem().isSaveButtonEnabled).isTrue()
        }
    }
}

We could call editCardDetailsInteractor.handleViewAction(...) to update the card brand and expect a new CardUpdateParams, but I feel like that would be testing the behaviour of the editCardDetailsInteractor. That's not what we're trying to do here. DefaultEditCardDetailsInteractorTest is doing that for us already.

Copy link
Collaborator

@samer-stripe samer-stripe Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I see what's going on here now. I think exposing an attribute for testing is the wrong solution. Re-introducing the Factory interface is probably better since we can then pass all these parameters through then just provide the factory as a parameter UpdatePaymentMethodInteractor.

I'm peaceful with keeping this for now! Can we follow-up with the factory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yh, I can do this in a follow-up. Thanks!


fun handleViewAction(viewAction: ViewAction)

@Immutable
data class State(
val card: PaymentMethod.Card,
val selectedCardBrand: CardBrandChoice,
val paymentMethodIcon: Int,
val shouldShowCardBrandDropdown: Boolean,
val availableNetworks: List<CardBrandChoice>
)

sealed interface ViewAction {
data class BrandChoiceChanged(val cardBrandChoice: CardBrandChoice) : ViewAction
}

companion object {
fun create(
coroutineScope: CoroutineScope,
isModifiable: Boolean,
cardBrandFilter: CardBrandFilter = DefaultCardBrandFilter,
card: PaymentMethod.Card,
onBrandChoiceChanged: CardBrandCallback,
onCardUpdateParamsChanged: CardUpdateParamsCallback
): EditCardDetailsInteractor {
return DefaultEditCardDetailsInteractor(
card = card,
cardBrandFilter = cardBrandFilter,
coroutineScope = coroutineScope,
onBrandChoiceChanged = onBrandChoiceChanged,
onCardUpdateParamsChanged = onCardUpdateParamsChanged,
isModifiable = isModifiable
)
}
}
}

private class DefaultEditCardDetailsInteractor(
private val card: PaymentMethod.Card,
private val cardBrandFilter: CardBrandFilter,
private val isModifiable: Boolean,
coroutineScope: CoroutineScope,
private val onBrandChoiceChanged: CardBrandCallback,
override val onCardUpdateParamsChanged: CardUpdateParamsCallback
) : EditCardDetailsInteractor {
private val cardDetailsEntry = MutableStateFlow(
value = buildDefaultCardEntry()
)

override val state: StateFlow<EditCardDetailsInteractor.State> = cardDetailsEntry.mapLatest { inputState ->
uiState(inputState.cardBrandChoice)
}.stateIn(
scope = coroutineScope,
started = SharingStarted.Eagerly,
initialValue = uiState()
)

init {
coroutineScope.launch(Dispatchers.Main) {
cardDetailsEntry.collectLatest { state ->
val newParams = state.takeIf {
it.hasChanged(
originalCardBrandChoice = defaultCardBrandChoice(),
)
}?.toUpdateParams()
onCardUpdateParamsChanged(newParams)
}
}
}

private fun onBrandChoiceChanged(cardBrandChoice: CardBrandChoice) {
if (cardBrandChoice != state.value.selectedCardBrand) {
onBrandChoiceChanged(cardBrandChoice.brand)
}
cardDetailsEntry.update {
it.copy(
cardBrandChoice = cardBrandChoice
)
}
}

override fun handleViewAction(viewAction: EditCardDetailsInteractor.ViewAction) {
when (viewAction) {
is EditCardDetailsInteractor.ViewAction.BrandChoiceChanged -> {
onBrandChoiceChanged(viewAction.cardBrandChoice)
}
}
}

private fun buildDefaultCardEntry(): CardDetailsEntry {
return CardDetailsEntry(
cardBrandChoice = defaultCardBrandChoice()
)
}

private fun defaultCardBrandChoice() = card.getPreferredChoice(cardBrandFilter)

private fun uiState(cardBrandChoice: CardBrandChoice = defaultCardBrandChoice()): EditCardDetailsInteractor.State {
return EditCardDetailsInteractor.State(
card = card,
selectedCardBrand = cardBrandChoice,
paymentMethodIcon = card.getSavedPaymentMethodIcon(forVerticalMode = true),
shouldShowCardBrandDropdown = isModifiable && isExpired().not(),
availableNetworks = card.getAvailableNetworks(cardBrandFilter)
)
}

private fun isExpired(): Boolean {
val cardExpiryMonth = card.expiryMonth
val cardExpiryYear = card.expiryYear
// If the card's expiration dates are missing, we can't conclude that it is expired, so we don't want to
// show the user an expired card error.
return cardExpiryMonth != null && cardExpiryYear != null &&
!DateUtils.isExpiryDataValid(
expiryMonth = cardExpiryMonth,
expiryYear = cardExpiryYear,
)
}
}
35 changes: 25 additions & 10 deletions paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/PaymentMethodsUiExtension.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,38 @@ internal fun PaymentMethod.getSavedPaymentMethodIcon(
): Int {
return when (type) {
PaymentMethod.Type.Card -> {
val brand = CardBrand.fromCode(card?.displayBrand).takeIf { it != Unknown } ?: card?.brand

// Vertical mode icons are the same for light & dark
if (forVerticalMode) {
brand?.getCardBrandIconForVerticalMode()
} else {
brand?.getCardBrandIconForHorizontalMode(
showNightIcon = showNightIcon,
)
}
card?.getSavedPaymentMethodIcon(
forVerticalMode = forVerticalMode,
showNightIcon = showNightIcon
)
}
PaymentMethod.Type.SepaDebit -> getSepaIcon(showNightIcon = showNightIcon)
PaymentMethod.Type.USBankAccount -> usBankAccount?.bankName?.let { TransformToBankIcon(it) }
else -> null
} ?: R.drawable.stripe_ic_paymentsheet_card_unknown_ref
}

@DrawableRes
internal fun PaymentMethod.Card?.getSavedPaymentMethodIcon(
forVerticalMode: Boolean = false,
showNightIcon: Boolean? = null,
): Int {
val brand = if (this != null) {
CardBrand.fromCode(displayBrand).takeIf { it != Unknown } ?: brand
} else {
Unknown
}

// Vertical mode icons are the same for light & dark
return if (forVerticalMode) {
brand.getCardBrandIconForVerticalMode()
} else {
brand.getCardBrandIconForHorizontalMode(
showNightIcon = showNightIcon,
)
}
}

@DrawableRes
internal fun CardBrand.getCardBrandIcon(): Int {
return this.getCardBrandIconRef()
Expand Down
Loading
Loading