From 40f6ce89ff6dca12d33e3ed7470cc38b77e888aa Mon Sep 17 00:00:00 2001 From: Jay Newstrom Date: Wed, 29 Jan 2025 11:13:41 -0600 Subject: [PATCH] Update DefaultEmbeddedSelectionChooser to keep the previousConfiguration. (#10028) --- .../embedded/EmbeddedSelectionChooser.kt | 32 ++++++--- .../embedded/SharedPaymentElementViewModel.kt | 5 +- .../DefaultEmbeddedSelectionChooserTest.kt | 67 +++++++++++++------ .../SharedPaymentElementViewModelTest.kt | 2 +- 4 files changed, 70 insertions(+), 36 deletions(-) diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/EmbeddedSelectionChooser.kt b/paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/EmbeddedSelectionChooser.kt index 58dd2dda738..1a144c06f64 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/EmbeddedSelectionChooser.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/EmbeddedSelectionChooser.kt @@ -2,11 +2,11 @@ package com.stripe.android.paymentelement.embedded -import com.stripe.android.common.model.asCommonConfiguration +import androidx.lifecycle.SavedStateHandle +import com.stripe.android.common.model.CommonConfiguration import com.stripe.android.common.model.containsVolatileDifferences import com.stripe.android.lpmfoundations.paymentmethod.PaymentMethodMetadata import com.stripe.android.model.PaymentMethod -import com.stripe.android.paymentelement.EmbeddedPaymentElement import com.stripe.android.paymentelement.ExperimentalEmbeddedPaymentElementApi import com.stripe.android.paymentsheet.model.PaymentSelection import javax.inject.Inject @@ -18,29 +18,35 @@ internal fun interface EmbeddedSelectionChooser { paymentMethods: List?, previousSelection: PaymentSelection?, newSelection: PaymentSelection?, - previousConfiguration: EmbeddedPaymentElement.Configuration?, - newConfiguration: EmbeddedPaymentElement.Configuration, + newConfiguration: CommonConfiguration, ): PaymentSelection? } -internal class DefaultEmbeddedSelectionChooser @Inject constructor() : EmbeddedSelectionChooser { +internal class DefaultEmbeddedSelectionChooser @Inject constructor( + private val savedStateHandle: SavedStateHandle, +) : EmbeddedSelectionChooser { + private var previousConfiguration: CommonConfiguration? + get() = savedStateHandle[PREVIOUS_CONFIGURATION_KEY] + set(value) = savedStateHandle.set(PREVIOUS_CONFIGURATION_KEY, value) + override fun choose( paymentMethodMetadata: PaymentMethodMetadata, paymentMethods: List?, previousSelection: PaymentSelection?, newSelection: PaymentSelection?, - previousConfiguration: EmbeddedPaymentElement.Configuration?, - newConfiguration: EmbeddedPaymentElement.Configuration, + newConfiguration: CommonConfiguration, ): PaymentSelection? { - return previousSelection?.takeIf { selection -> + val result = previousSelection?.takeIf { selection -> canUseSelection( paymentMethodMetadata = paymentMethodMetadata, paymentMethods = paymentMethods, selection = selection, - ) && previousConfiguration?.asCommonConfiguration()?.containsVolatileDifferences( - newConfiguration.asCommonConfiguration() - ) != true + ) && previousConfiguration?.containsVolatileDifferences(newConfiguration) != true } ?: newSelection + + previousConfiguration = newConfiguration + + return result } private fun canUseSelection( @@ -72,4 +78,8 @@ internal class DefaultEmbeddedSelectionChooser @Inject constructor() : EmbeddedS } } } + + companion object { + const val PREVIOUS_CONFIGURATION_KEY = "DefaultEmbeddedSelectionChooser_PREVIOUS_CONFIGURATION_KEY" + } } diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/SharedPaymentElementViewModel.kt b/paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/SharedPaymentElementViewModel.kt index ba72a1fda9b..ebe0ccdabb8 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/SharedPaymentElementViewModel.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentelement/embedded/SharedPaymentElementViewModel.kt @@ -12,6 +12,7 @@ import androidx.lifecycle.viewModelScope import androidx.lifecycle.viewmodel.CreationExtras import com.stripe.android.cards.CardAccountRangeRepository import com.stripe.android.cards.DefaultCardAccountRangeRepositoryFactory +import com.stripe.android.common.model.asCommonConfiguration import com.stripe.android.core.injection.IOContext import com.stripe.android.core.utils.RealUserFacingLogger import com.stripe.android.core.utils.UserFacingLogger @@ -126,7 +127,6 @@ internal class SharedPaymentElementViewModel @Inject constructor( intentConfiguration: PaymentSheet.IntentConfiguration, configuration: EmbeddedPaymentElement.Configuration, ) { - val previousConfiguration = confirmationStateHolder.state?.configuration confirmationStateHolder.state = EmbeddedConfirmationStateHolder.State( paymentMethodMetadata = state.paymentMethodMetadata, selection = state.paymentSelection, @@ -142,8 +142,7 @@ internal class SharedPaymentElementViewModel @Inject constructor( paymentMethods = state.customer?.paymentMethods, previousSelection = selectionHolder.selection.value, newSelection = state.paymentSelection, - previousConfiguration = previousConfiguration, - newConfiguration = configuration, + newConfiguration = configuration.asCommonConfiguration(), ) ) embeddedContentHelper.dataLoaded( diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentelement/embedded/DefaultEmbeddedSelectionChooserTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentelement/embedded/DefaultEmbeddedSelectionChooserTest.kt index 17f0f8472cb..86d2daee1a7 100644 --- a/paymentsheet/src/test/java/com/stripe/android/paymentelement/embedded/DefaultEmbeddedSelectionChooserTest.kt +++ b/paymentsheet/src/test/java/com/stripe/android/paymentelement/embedded/DefaultEmbeddedSelectionChooserTest.kt @@ -1,6 +1,9 @@ package com.stripe.android.paymentelement.embedded +import androidx.lifecycle.SavedStateHandle import com.google.common.truth.Truth.assertThat +import com.stripe.android.common.model.CommonConfiguration +import com.stripe.android.common.model.asCommonConfiguration import com.stripe.android.core.strings.resolvableString import com.stripe.android.lpmfoundations.paymentmethod.PaymentMethodMetadataFactory import com.stripe.android.model.PaymentIntentFixtures @@ -8,6 +11,7 @@ import com.stripe.android.model.PaymentMethodCreateParamsFixtures import com.stripe.android.model.PaymentMethodFixtures import com.stripe.android.paymentelement.EmbeddedPaymentElement import com.stripe.android.paymentelement.ExperimentalEmbeddedPaymentElementApi +import com.stripe.android.paymentelement.embedded.DefaultEmbeddedSelectionChooser.Companion.PREVIOUS_CONFIGURATION_KEY import com.stripe.android.paymentsheet.PaymentSheet import com.stripe.android.paymentsheet.model.PaymentSelection import com.stripe.android.testing.PaymentMethodFactory @@ -16,6 +20,10 @@ import com.stripe.android.ui.core.R as StripeUiCoreR @OptIn(ExperimentalEmbeddedPaymentElementApi::class) internal class DefaultEmbeddedSelectionChooserTest { + private val defaultConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.") + .build() + .asCommonConfiguration() + @Test fun `Uses new payment selection if there's no existing one`() = runScenario { val selection = chooser.choose( @@ -23,8 +31,7 @@ internal class DefaultEmbeddedSelectionChooserTest { paymentMethods = null, previousSelection = null, newSelection = PaymentSelection.GooglePay, - previousConfiguration = null, - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isEqualTo(PaymentSelection.GooglePay) } @@ -51,8 +58,7 @@ internal class DefaultEmbeddedSelectionChooserTest { paymentMethods = PaymentMethodFixtures.createCards(3) + paymentMethod, previousSelection = previousSelection, newSelection = newSelection, - previousConfiguration = null, - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isEqualTo(previousSelection) } @@ -67,8 +73,7 @@ internal class DefaultEmbeddedSelectionChooserTest { paymentMethods = PaymentMethodFixtures.createCards(3) + paymentMethod, previousSelection = previousSelection, newSelection = PaymentSelection.GooglePay, - previousConfiguration = null, - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isEqualTo(previousSelection) } @@ -82,8 +87,7 @@ internal class DefaultEmbeddedSelectionChooserTest { paymentMethods = PaymentMethodFixtures.createCards(3) + PaymentMethodFixtures.SEPA_DEBIT_PAYMENT_METHOD, previousSelection = previousSelection, newSelection = null, - previousConfiguration = null, - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isNull() } @@ -101,8 +105,7 @@ internal class DefaultEmbeddedSelectionChooserTest { paymentMethods = PaymentMethodFixtures.createCards(3), previousSelection = previousSelection, newSelection = null, - previousConfiguration = null, - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isNull() } @@ -120,8 +123,7 @@ internal class DefaultEmbeddedSelectionChooserTest { paymentMethods = PaymentMethodFixtures.createCards(3), previousSelection = previousSelection, newSelection = PaymentSelection.GooglePay, - previousConfiguration = null, - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isEqualTo(previousSelection) } @@ -138,8 +140,7 @@ internal class DefaultEmbeddedSelectionChooserTest { paymentMethods = PaymentMethodFixtures.createCards(3), previousSelection = previousSelection, newSelection = null, - previousConfiguration = null, - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isNull() } @@ -150,14 +151,13 @@ internal class DefaultEmbeddedSelectionChooserTest { val paymentMethod = PaymentMethodFixtures.createCard() val newSelection = PaymentSelection.Saved(paymentMethod) + savedStateHandle[PREVIOUS_CONFIGURATION_KEY] = defaultConfiguration.copy(merchantDisplayName = "Hi") val selection = chooser.choose( paymentMethodMetadata = PaymentMethodMetadataFactory.create(isGooglePayReady = true), paymentMethods = PaymentMethodFixtures.createCards(3) + paymentMethod, previousSelection = previousSelection, newSelection = newSelection, - previousConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.") - .embeddedViewDisplaysMandateText(false).build(), - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isEqualTo(previousSelection) } @@ -168,25 +168,50 @@ internal class DefaultEmbeddedSelectionChooserTest { val paymentMethod = PaymentMethodFixtures.createCard() val newSelection = PaymentSelection.Saved(paymentMethod) + savedStateHandle[PREVIOUS_CONFIGURATION_KEY] = defaultConfiguration.copy( + defaultBillingDetails = PaymentSheet.BillingDetails(email = "jaynewstrom@example.com") + ) val selection = chooser.choose( paymentMethodMetadata = PaymentMethodMetadataFactory.create(isGooglePayReady = true), paymentMethods = PaymentMethodFixtures.createCards(3) + paymentMethod, previousSelection = previousSelection, newSelection = newSelection, - previousConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.") - .defaultBillingDetails(PaymentSheet.BillingDetails(email = "jaynewstrom@example.com")).build(), - newConfiguration = EmbeddedPaymentElement.Configuration.Builder("Example, Inc.").build(), + newConfiguration = defaultConfiguration, ) assertThat(selection).isEqualTo(newSelection) } + @Test + fun `previousConfig is set when calling choose`() = runScenario { + val previousSelection = PaymentSelection.GooglePay + val paymentMethod = PaymentMethodFixtures.createCard() + val newSelection = PaymentSelection.Saved(paymentMethod) + + assertThat(savedStateHandle.get(PREVIOUS_CONFIGURATION_KEY)).isNull() + val selection = chooser.choose( + paymentMethodMetadata = PaymentMethodMetadataFactory.create(isGooglePayReady = true), + paymentMethods = PaymentMethodFixtures.createCards(3) + paymentMethod, + previousSelection = previousSelection, + newSelection = newSelection, + newConfiguration = defaultConfiguration, + ) + assertThat(selection).isEqualTo(previousSelection) + assertThat(savedStateHandle.get(PREVIOUS_CONFIGURATION_KEY)) + .isEqualTo(defaultConfiguration) + } + private fun runScenario( block: Scenario.() -> Unit, ) { - Scenario(DefaultEmbeddedSelectionChooser()).block() + val savedStateHandle = SavedStateHandle() + Scenario( + chooser = DefaultEmbeddedSelectionChooser(savedStateHandle), + savedStateHandle = savedStateHandle, + ).block() } private class Scenario( val chooser: EmbeddedSelectionChooser, + val savedStateHandle: SavedStateHandle, ) } diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentelement/embedded/SharedPaymentElementViewModelTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentelement/embedded/SharedPaymentElementViewModelTest.kt index c7dafc46263..f96954a6b1c 100644 --- a/paymentsheet/src/test/java/com/stripe/android/paymentelement/embedded/SharedPaymentElementViewModelTest.kt +++ b/paymentsheet/src/test/java/com/stripe/android/paymentelement/embedded/SharedPaymentElementViewModelTest.kt @@ -382,7 +382,7 @@ internal class SharedPaymentElementViewModelTest { configurationHandler = configurationHandler, paymentOptionDisplayDataFactory = paymentOptionDisplayDataFactory, selectionHolder = selectionHolder, - selectionChooser = { _, _, _, newSelection, _, _ -> + selectionChooser = { _, _, _, newSelection, _ -> newSelection }, customerStateHolder = customerStateHolder,