From 3a0e9e3fffbe00bb05d1289bfbf1e7abc40df6b7 Mon Sep 17 00:00:00 2001 From: Tian Zhao Date: Fri, 17 Jan 2025 09:23:42 -0800 Subject: [PATCH] [MOBILESDK-2894] [Android] Make Default Payment Method first (#9928) * ordering of default payment methods * testOrderingOfPaymentMethodsWithDefaultPaymentMethodId * better ordering logic edge case where defaultPaymentMethodIndex not found and there is a saved selection Index, and we ignore the savedSelectionIndex * rename withLastUsedPaymentMethodFirst to withDefaultPaymentMethodOrLastUsedPaymentMethodFirst * use a hardcoded input and expected output * lint --- .../state/PaymentElementLoader.kt | 24 ++++-- .../state/DefaultPaymentElementLoaderTest.kt | 76 +++++++++++++++++++ 2 files changed, 92 insertions(+), 8 deletions(-) diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/PaymentElementLoader.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/PaymentElementLoader.kt index 2c6770e5896..3ae59f24020 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/PaymentElementLoader.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/PaymentElementLoader.kt @@ -348,7 +348,10 @@ internal class DefaultPaymentElementLoader @Inject constructor( return customerState?.let { state -> state.copy( paymentMethods = state.paymentMethods - .withLastUsedPaymentMethodFirst(savedSelection.await()).filter { cardBrandFilter.isAccepted(it) } + .withDefaultPaymentMethodOrLastUsedPaymentMethodFirst( + savedSelection = savedSelection.await(), + defaultPaymentMethodId = state.defaultPaymentMethodId + ).filter { cardBrandFilter.isAccepted(it) } ) } } @@ -715,19 +718,24 @@ internal class DefaultPaymentElementLoader @Inject constructor( } } -private fun List.withLastUsedPaymentMethodFirst( +private fun List.withDefaultPaymentMethodOrLastUsedPaymentMethodFirst( savedSelection: SavedSelection, + defaultPaymentMethodId: String? ): List { - val defaultPaymentMethodIndex = (savedSelection as? SavedSelection.PaymentMethod)?.let { + val defaultPaymentMethodIndex = defaultPaymentMethodId?.let { + indexOfFirst { it.id == defaultPaymentMethodId } + } + + val savedSelectionIndex = (savedSelection as? SavedSelection.PaymentMethod)?.let { indexOfFirst { it.id == savedSelection.id }.takeIf { it != -1 } } - return if (defaultPaymentMethodIndex != null) { - val primaryPaymentMethod = get(defaultPaymentMethodIndex) + val primaryPaymentMethodIndex = defaultPaymentMethodIndex ?: savedSelectionIndex + + return primaryPaymentMethodIndex?.let { + val primaryPaymentMethod = get(primaryPaymentMethodIndex) listOf(primaryPaymentMethod) + (this - primaryPaymentMethod) - } else { - this - } + } ?: this } private fun PaymentMethod.toPaymentSelection(): PaymentSelection.Saved { diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/DefaultPaymentElementLoaderTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/DefaultPaymentElementLoaderTest.kt index 2b696b3460c..dbac87bb3de 100644 --- a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/DefaultPaymentElementLoaderTest.kt +++ b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/DefaultPaymentElementLoaderTest.kt @@ -6,6 +6,7 @@ import com.stripe.android.common.model.asCommonConfiguration import com.stripe.android.core.Logger import com.stripe.android.core.exception.APIConnectionException import com.stripe.android.core.model.CountryCode +import com.stripe.android.core.utils.FeatureFlags import com.stripe.android.googlepaylauncher.GooglePayRepository import com.stripe.android.isInstanceOf import com.stripe.android.link.LinkConfiguration @@ -41,6 +42,7 @@ import com.stripe.android.paymentsheet.repositories.ElementsSessionRepository import com.stripe.android.paymentsheet.state.PaymentSheetLoadingException.PaymentIntentInTerminalState import com.stripe.android.paymentsheet.utils.FakeUserFacingLogger import com.stripe.android.testing.FakeErrorReporter +import com.stripe.android.testing.FeatureFlagTestRule import com.stripe.android.testing.PaymentMethodFactory import com.stripe.android.testing.PaymentMethodFactory.update import com.stripe.android.ui.core.cbc.CardBrandChoiceEligibility @@ -50,6 +52,7 @@ import com.stripe.android.utils.FakeElementsSessionRepository import kotlinx.coroutines.flow.flow import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest +import org.junit.Rule import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.eq import org.mockito.Captor @@ -80,6 +83,12 @@ internal class DefaultPaymentElementLoaderTest { private val readyGooglePayRepository = mock() private val unreadyGooglePayRepository = mock() + @get:Rule + val enableDefaultPaymentMethods = FeatureFlagTestRule( + featureFlag = FeatureFlags.enableDefaultPaymentMethods, + isEnabled = false, + ) + @BeforeTest fun setup() { MockitoAnnotations.openMocks(this) @@ -1944,6 +1953,22 @@ internal class DefaultPaymentElementLoaderTest { assertThat(repository.lastParams?.defaultPaymentMethodId).isNull() } + @Test + fun `When DefaultPaymentMethod not null, no saved selection, paymentMethod order correct`() = runTest { + enableDefaultPaymentMethods.setEnabled(true) + + testOrderingOfPaymentMethodsWithDefaultPaymentMethodId() + } + + @Test + fun `When DefaultPaymentMethod not null, and saved selection, paymentMethod order correct`() = runTest { + enableDefaultPaymentMethods.setEnabled(true) + + testOrderingOfPaymentMethodsWithDefaultPaymentMethodId( + setLastUsedIndex = 1 + ) + } + @Test fun `When using 'LegacyEphemeralKey' & has a default saved Stripe payment method, should not call 'ElementsSessionRepository' with default id`() = runTest { @@ -2466,4 +2491,55 @@ internal class DefaultPaymentElementLoaderTest { isReloadingAfterProcessDeath = isReloadingAfterProcessDeath, initializedViaCompose = initializedViaCompose, ) + + private val paymentMethodsForTestingOrdering = listOf( + PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "a1", customerId = "alice"), + PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "b2", customerId = "bob"), + PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "c3", customerId = "carol"), + PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "d4", customerId = "dan") + ) + + @OptIn(ExperimentalCustomerSessionApi::class) + private suspend fun testOrderingOfPaymentMethodsWithDefaultPaymentMethodId( + setLastUsedIndex: Int? = null + ) { + val defaultPaymentMethodIndex = 2 + val defaultPaymentMethod = paymentMethodsForTestingOrdering[defaultPaymentMethodIndex] + val defaultPaymentMethodId = defaultPaymentMethod.id + + if (setLastUsedIndex != null && setLastUsedIndex in paymentMethodsForTestingOrdering.indices) { + val lastUsed = paymentMethodsForTestingOrdering[setLastUsedIndex] + prefsRepository.savePaymentSelection(PaymentSelection.Saved(lastUsed)) + } + + val loader = createPaymentElementLoader( + customer = ElementsSession.Customer( + paymentMethods = paymentMethodsForTestingOrdering, + session = createElementsSessionCustomerSession( + mobilePaymentElementComponent = ElementsSession.Customer.Components.MobilePaymentElement.Disabled, + ), + defaultPaymentMethod = defaultPaymentMethodId, + ) + ) + + val result = loader.load( + initializationMode = PaymentElementLoader.InitializationMode.PaymentIntent("secret"), + paymentSheetConfiguration = mockConfiguration( + customer = PaymentSheet.CustomerConfiguration.createWithCustomerSession( + id = "id", + clientSecret = "cuss_1", + ), + ), + initializedViaCompose = false, + ).getOrThrow() + + val observedElements = result.customer?.paymentMethods + val expectedElements = listOf( + PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "c3", customerId = "carol"), + PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "a1", customerId = "alice"), + PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "b2", customerId = "bob"), + PaymentMethodFixtures.CARD_PAYMENT_METHOD.copy(id = "d4", customerId = "dan") + ) + assertThat(observedElements).containsExactlyElementsIn(expectedElements).inOrder() + } }