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

Show blocked brands as disabled in CBC drop down #9979

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 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
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package com.stripe.android.cards

import androidx.annotation.RestrictTo
import androidx.annotation.VisibleForTesting
import com.stripe.android.CardBrandFilter
import com.stripe.android.DefaultCardBrandFilter
import com.stripe.android.model.AccountRange
import com.stripe.android.model.CardBrand
import kotlinx.coroutines.CoroutineScope
Expand All @@ -22,8 +20,7 @@ class CardAccountRangeService(
private val workContext: CoroutineContext,
val staticCardAccountRanges: StaticCardAccountRanges,
private val accountRangeResultListener: AccountRangeResultListener,
private val isCbcEligible: () -> Boolean,
private val cardBrandFilter: CardBrandFilter = DefaultCardBrandFilter
private val isCbcEligible: () -> Boolean
) {

val isLoading: StateFlow<Boolean> = cardAccountRangeRepository.loading
Expand Down Expand Up @@ -104,7 +101,7 @@ class CardAccountRangeService(
}

fun updateAccountRangesResult(accountRanges: List<AccountRange>) {
this.accountRanges = accountRanges.filter { cardBrandFilter.isAccepted(it.brand) }
this.accountRanges = accountRanges
accountRangeResultListener.onAccountRangesResult(this.accountRanges)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ import com.stripe.android.uicore.elements.SingleChoiceDropdownItem

internal data class CardBrandChoice(
override val label: ResolvableString,
override val icon: Int?
override val icon: Int?,
override val enabled: Boolean
) : SingleChoiceDropdownItem
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,7 @@ class CardNumberEditText internal constructor(
possibleCardBrands = brands
}
}
},
cardBrandFilter = cardBrandFilter
}
)

@JvmSynthetic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.test.core.app.ApplicationProvider
import com.google.common.truth.Truth.assertThat
import com.stripe.android.ApiKeyFixtures
import com.stripe.android.CardBrandFilter
import com.stripe.android.DefaultCardBrandFilter
import com.stripe.android.cards.DefaultStaticCardAccountRanges.Companion.ACCOUNTS
import com.stripe.android.cards.DefaultStaticCardAccountRanges.Companion.CARTES_BANCAIRES_ACCOUNT_RANGES
import com.stripe.android.cards.DefaultStaticCardAccountRanges.Companion.UNIONPAY16_ACCOUNTS
Expand All @@ -15,7 +13,6 @@ import com.stripe.android.core.networking.ApiRequest
import com.stripe.android.core.networking.DefaultAnalyticsRequestExecutor
import com.stripe.android.model.AccountRange
import com.stripe.android.model.BinRange
import com.stripe.android.model.CardBrand
import com.stripe.android.networking.PaymentAnalyticsRequestFactory
import com.stripe.android.networking.StripeApiRepository
import com.stripe.android.uicore.utils.stateFlowOf
Expand All @@ -24,7 +21,6 @@ import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runTest
import kotlinx.parcelize.Parcelize
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TestRule
Expand Down Expand Up @@ -178,8 +174,7 @@ class CardAccountRangeServiceTest {
private suspend fun testBehavior(
cardNumber: String,
isCbcEligible: Boolean,
mockRemoteCardAccountRangeSource: CardAccountRangeSource? = null,
cardBrandFilter: CardBrandFilter = DefaultCardBrandFilter
mockRemoteCardAccountRangeSource: CardAccountRangeSource? = null
): List<AccountRange> {
val completable = CompletableDeferred<List<AccountRange>>()

Expand All @@ -197,57 +192,14 @@ class CardAccountRangeServiceTest {
completable.complete(accountRanges)
}
},
isCbcEligible = { isCbcEligible },
cardBrandFilter = cardBrandFilter
isCbcEligible = { isCbcEligible }
)

service.onCardNumberChanged(CardNumber.Unvalidated(cardNumber))

return completable.await()
}

@Test
fun `Brands that are disallowed by the card brand filter are filtered out`() =
runTest {
// Disallow Mastercard
val disallowedBrands = setOf(CardBrand.MasterCard)
val cardBrandFilter = FakeCardBrandFilter(disallowedBrands)

// Use a card number that matches Mastercard (starts with '2')
val cardNumber = "2"

// Call testBehavior with the custom CardBrandFilter
val accountRanges = testBehavior(
cardNumber = cardNumber,
isCbcEligible = false,
cardBrandFilter = cardBrandFilter
)

// Since Mastercard is disallowed, the accountRanges should be empty
assertThat(accountRanges).isEmpty()
}

@Test
fun `Brands that are not disallowed by the card brand filter are filtered out`() =
runTest {
// Disallow Visa
val disallowedBrands = setOf(CardBrand.Visa)
val cardBrandFilter = FakeCardBrandFilter(disallowedBrands)

// Use a card number that matches Mastercard (starts with '2')
val cardNumber = "2"

// Call testBehavior with the custom CardBrandFilter
val accountRanges = testBehavior(
cardNumber = cardNumber,
isCbcEligible = false,
cardBrandFilter = cardBrandFilter
)

// Since Mastercard is allow, the accountRanges should have contents
assertThat(accountRanges).isNotEmpty()
}

private fun createMockRemoteDefaultCardAccountRangeRepository(
mockRemoteCardAccountRangeSource: CardAccountRangeSource
): CardAccountRangeRepository {
Expand Down Expand Up @@ -311,12 +263,3 @@ class CardAccountRangeServiceTest {
)
}
}

@Parcelize
private class FakeCardBrandFilter(
private val disallowedBrands: Set<CardBrand>
) : CardBrandFilter {
override fun isAccepted(cardBrand: CardBrand): Boolean {
return !disallowedBrands.contains(cardBrand)
}
}
2 changes: 2 additions & 0 deletions payments-ui-core/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,6 @@
<string name="stripe_setup_button_label">Set up</string>
<!-- Label for UPI ID number field on form -->
<string name="stripe_upi_id_label">UPI ID</string>
<!-- Shown in a dropdown picker next to a card brand that is not accepted by a merchant. E.g. "Visa (not accepted)" -->
<string name="stripe_card_brand_not_accepted">(not accepted)</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be a parameterized string (I know some languages won't have the same ordering as english).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's a good idea. I should do this on iOS too.

</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ import com.stripe.android.cards.CardAccountRangeService
import com.stripe.android.cards.CardNumber
import com.stripe.android.cards.DefaultStaticCardAccountRanges
import com.stripe.android.cards.StaticCardAccountRanges
import com.stripe.android.core.strings.plus
import com.stripe.android.core.strings.resolvableString
import com.stripe.android.model.AccountRange
import com.stripe.android.model.CardBrand
import com.stripe.android.stripecardscan.cardscan.CardScanSheetResult
import com.stripe.android.ui.core.R
import com.stripe.android.ui.core.asIndividualDigits
import com.stripe.android.ui.core.elements.events.LocalCardBrandDisallowedReporter
import com.stripe.android.ui.core.elements.events.LocalCardNumberCompletedEventReporter
Expand Down Expand Up @@ -178,8 +180,7 @@ internal class DefaultCardNumberController(
brandChoices.value = newBrandChoices
}
},
isCbcEligible = { isEligibleForCardBrandChoice },
cardBrandFilter = cardBrandFilter
isCbcEligible = { isEligibleForCardBrandChoice }
)

override val trailingIcon: StateFlow<TextFieldIcon?> = combineAsStateFlow(
Expand Down Expand Up @@ -214,10 +215,18 @@ internal class DefaultCardNumberController(
}

val items = brands.map { brand ->
val enabled = cardBrandFilter.isAccepted(brand)
TextFieldIcon.Dropdown.Item(
id = brand.code,
label = brand.displayName.resolvableString,
icon = brand.icon
label = if (enabled) {
brand.displayName.resolvableString
} else {
brand.displayName.resolvableString +
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be pretty easy to parameterize, let me know if you want to pair on it!

" ".resolvableString +
resolvableString(R.string.stripe_card_brand_not_accepted)
},
icon = brand.icon,
enabled = enabled
)
}

Expand Down
2 changes: 2 additions & 0 deletions paymentsheet/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,6 @@
<!-- A text notice shown when the user selects an expired card. -->
<string name="stripe_wallet_update_expired_card_error">This card has expired. Update your card info or choose a different payment method.</string>
<string name="stripe_wallet_expand_accessibility">Change selection</string>
<!-- Shown in a dropdown picker next to a card brand that is not accepted by a merchant. E.g. "Visa (not accepted)" -->
<string name="stripe_card_brand_not_accepted">(not accepted)</string>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there one file where I can put these rather than in two strings.xml files?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can access the one in payments-core-ui in paymentsheet module. It'll just require a different import.

</resources>
Original file line number Diff line number Diff line change
@@ -1,16 +1,27 @@
package com.stripe.android.paymentsheet.ui

import com.stripe.android.core.strings.ResolvableString
import com.stripe.android.core.strings.plus
import com.stripe.android.core.strings.resolvableString
import com.stripe.android.model.CardBrand
import com.stripe.android.paymentsheet.R
import com.stripe.android.uicore.elements.SingleChoiceDropdownItem

internal data class CardBrandChoice(
val brand: CardBrand
val brand: CardBrand,
override val enabled: Boolean
) : SingleChoiceDropdownItem {

override val icon: Int
get() = brand.icon

override val label: ResolvableString
get() = brand.displayName.resolvableString
get() = if (enabled) {
brand.displayName.resolvableString
} else {
// If it's disabled, append "not accepted" to the end of the brand display name
brand.displayName.resolvableString +
" ".resolvableString +
resolvableString(R.string.stripe_card_brand_not_accepted)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ import com.stripe.android.CardBrandFilter
import com.stripe.android.model.CardBrand
import com.stripe.android.model.PaymentMethod

internal fun PaymentMethod.Card.getPreferredChoice(): CardBrandChoice {
return CardBrand.fromCode(displayBrand).toChoice()
internal fun PaymentMethod.Card.getPreferredChoice(cardBrandFilter: CardBrandFilter): CardBrandChoice {
return CardBrand.fromCode(displayBrand).toChoice(cardBrandFilter)
}

internal fun PaymentMethod.Card.getAvailableNetworks(cardBrandFilter: CardBrandFilter): List<CardBrandChoice> {
return networks?.available?.let { brandCodes ->
brandCodes.map { code ->
CardBrand.fromCode(code).toChoice()
}.filter { cardBrandFilter.isAccepted(it.brand) }
CardBrand.fromCode(code).toChoice(cardBrandFilter)
}
} ?: listOf()
}

private fun CardBrand.toChoice(): CardBrandChoice {
return CardBrandChoice(brand = this)
private fun CardBrand.toChoice(cardBrandFilter: CardBrandFilter): CardBrandChoice {
return CardBrandChoice(brand = this, enabled = cardBrandFilter.isAccepted(this))
}
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ internal class DefaultUpdatePaymentMethodInteractor(
val updateResult = updateExecutor(displayableSavedPaymentMethod.paymentMethod, newCardBrand)

updateResult.onSuccess {
savedCardBrand.emit(CardBrandChoice(brand = newCardBrand))
savedCardBrand.emit(CardBrandChoice(brand = newCardBrand, enabled = true))
cardBrandHasBeenChanged.emit(false)
}.onFailure {
error.emit(it.stripeErrorMessage())
Expand All @@ -173,8 +173,8 @@ internal class DefaultUpdatePaymentMethodInteractor(

private fun getInitialCardBrandChoice(): CardBrandChoice {
return when (val savedPaymentMethod = displayableSavedPaymentMethod.savedPaymentMethod) {
is SavedPaymentMethod.Card -> savedPaymentMethod.card.getPreferredChoice()
else -> CardBrandChoice(brand = CardBrand.Unknown)
is SavedPaymentMethod.Card -> savedPaymentMethod.card.getPreferredChoice(cardBrandFilter)
else -> CardBrandChoice(brand = CardBrand.Unknown, enabled = true)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2592,7 +2592,8 @@ class CustomerSheetViewModelTest {
editViewState.updatePaymentMethodInteractor.handleViewAction(
UpdatePaymentMethodInteractor.ViewAction.BrandChoiceChanged(
CardBrandChoice(
brand = CardBrand.Visa
brand = CardBrand.Visa,
enabled = true
)
)
)
Expand Down Expand Up @@ -2780,7 +2781,9 @@ class CustomerSheetViewModelTest {
editViewState.updatePaymentMethodInteractor.handleViewAction(
UpdatePaymentMethodInteractor.ViewAction.BrandChoiceChanged(
CardBrandChoice(
brand = CardBrand.Visa
brand = CardBrand.Visa,
enabled = true

)
)
)
Expand Down Expand Up @@ -2875,7 +2878,10 @@ class CustomerSheetViewModelTest {
val editViewState = awaitViewState<CustomerSheetViewState.UpdatePaymentMethod>()
editViewState.updatePaymentMethodInteractor.handleViewAction(
UpdatePaymentMethodInteractor.ViewAction.BrandChoiceChanged(
CardBrandChoice(brand = CardBrand.Visa)
CardBrandChoice(
brand = CardBrand.Visa,
enabled = true
)
)
)

Expand Down Expand Up @@ -2906,7 +2912,10 @@ class CustomerSheetViewModelTest {
val editViewState = awaitViewState<CustomerSheetViewState.UpdatePaymentMethod>()
editViewState.updatePaymentMethodInteractor.handleViewAction(
UpdatePaymentMethodInteractor.ViewAction.BrandChoiceChanged(
CardBrandChoice(brand = CardBrand.Visa)
CardBrandChoice(
brand = CardBrand.Visa,
enabled = true
)
)
)

Expand Down Expand Up @@ -3457,7 +3466,8 @@ class CustomerSheetViewModelTest {
editViewState.updatePaymentMethodInteractor.handleViewAction(
UpdatePaymentMethodInteractor.ViewAction.BrandChoiceChanged(
CardBrandChoice(
brand = CardBrand.Visa
brand = CardBrand.Visa,
enabled = true
)
)
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.stripe.android.paymentsheet

import androidx.compose.ui.test.SemanticsNodeInteraction
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsNotEnabled
import androidx.compose.ui.test.junit4.ComposeTestRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.performClick
Expand Down Expand Up @@ -28,15 +30,16 @@ internal class EditPage(
.performClick()
}

fun assertNotInDropdown(cardBrand: String) {
fun assertInDropdownButDisabled(cardBrand: String) {
// Click on the dropdown menu to expand it
composeTestRule.onNodeWithTag(DROPDOWN_MENU_CLICKABLE_TEST_TAG)
.performClick()

// Attempt to find the node with the specified cardBrand
// and assert that it does not exist
// Attempt to find the node with the specified cardBrand,
// assert that it is present (displayed) and disabled
composeTestRule.onNodeWithTag("${TEST_TAG_DROP_DOWN_CHOICE}_$cardBrand")
.assertDoesNotExist()
.assertIsDisplayed()
.assertIsNotEnabled()
jaynewstrom-stripe marked this conversation as resolved.
Show resolved Hide resolved

// Optionally, close the dropdown menu if it's still open
composeTestRule.onNodeWithTag(DROPDOWN_MENU_CLICKABLE_TEST_TAG)
Expand Down
Loading
Loading