From 3ffdb8358561d74c2dc56e424a13a9a6f2b1a397 Mon Sep 17 00:00:00 2001 From: tianzhao-stripe Date: Mon, 6 Jan 2025 13:05:14 -0500 Subject: [PATCH 1/6] Feature flag --- .../example/playground/settings/PlaygroundSettings.kt | 1 + .../src/main/java/com/stripe/android/core/utils/FeatureFlags.kt | 1 + 2 files changed, 2 insertions(+) diff --git a/paymentsheet-example/src/main/java/com/stripe/android/paymentsheet/example/playground/settings/PlaygroundSettings.kt b/paymentsheet-example/src/main/java/com/stripe/android/paymentsheet/example/playground/settings/PlaygroundSettings.kt index 40625216c39..b86a0ebd992 100644 --- a/paymentsheet-example/src/main/java/com/stripe/android/paymentsheet/example/playground/settings/PlaygroundSettings.kt +++ b/paymentsheet-example/src/main/java/com/stripe/android/paymentsheet/example/playground/settings/PlaygroundSettings.kt @@ -450,6 +450,7 @@ internal class PlaygroundSettings private constructor( CardBrandAcceptanceSettingsDefinition, FeatureFlagSettingsDefinition(FeatureFlags.useNewUpdateCardScreen), FeatureFlagSettingsDefinition(FeatureFlags.instantDebitsIncentives), + FeatureFlagSettingsDefinition(FeatureFlags.enableDefaultPaymentMethods), ) private val nonUiSettingDefinitions: List> = listOf( diff --git a/stripe-core/src/main/java/com/stripe/android/core/utils/FeatureFlags.kt b/stripe-core/src/main/java/com/stripe/android/core/utils/FeatureFlags.kt index 133f2b476db..f2c85c3747a 100644 --- a/stripe-core/src/main/java/com/stripe/android/core/utils/FeatureFlags.kt +++ b/stripe-core/src/main/java/com/stripe/android/core/utils/FeatureFlags.kt @@ -9,6 +9,7 @@ object FeatureFlags { val nativeLinkEnabled = FeatureFlag("Native Link") val useNewUpdateCardScreen = FeatureFlag("Enable new update card screen") val instantDebitsIncentives = FeatureFlag("Instant Bank Payments Incentives") + val enableDefaultPaymentMethods = FeatureFlag("Default Payment Method") } @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) From 683d8c1e394e251968fc5c516f7fc68e90b86fb0 Mon Sep 17 00:00:00 2001 From: tianzhao-stripe Date: Mon, 6 Jan 2025 13:57:50 -0500 Subject: [PATCH 2/6] add feature flag check when getting the defaultPaymentMethodId --- .../paymentsheet/SavedPaymentMethodMutator.kt | 2 +- .../paymentsheet/state/CustomerState.kt | 11 ++++++++++- .../viewmodels/PaymentOptionsItemsMapper.kt | 2 +- .../paymentsheet/state/CustomerStateTest.kt | 18 +++++++++++++++++- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt index ec9fd28dfe0..f5652cdeb3a 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt @@ -59,7 +59,7 @@ internal class SavedPaymentMethodMutator( isNotPaymentFlow: Boolean, ) { val defaultPaymentMethodId: StateFlow = customerStateHolder.customer.mapAsStateFlow { customerState -> - customerState?.defaultPaymentMethodId + customerState?.getDefaultPaymentMethodId() } private val paymentOptionsItemsMapper: PaymentOptionsItemsMapper by lazy { diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/CustomerState.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/CustomerState.kt index 6b378ebea18..e76d51c95bf 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/CustomerState.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/CustomerState.kt @@ -2,6 +2,7 @@ package com.stripe.android.paymentsheet.state import android.os.Parcelable import com.stripe.android.common.model.CommonConfiguration +import com.stripe.android.core.utils.FeatureFlags import com.stripe.android.model.ElementsSession import com.stripe.android.model.PaymentMethod import com.stripe.android.paymentsheet.PaymentSheet @@ -14,7 +15,7 @@ internal data class CustomerState( val customerSessionClientSecret: String?, val paymentMethods: List, val permissions: Permissions, - val defaultPaymentMethodId: String? + private val defaultPaymentMethodId: String? ) : Parcelable { @Parcelize data class Permissions( @@ -23,6 +24,14 @@ internal data class CustomerState( val canRemoveDuplicates: Boolean, ) : Parcelable + fun getDefaultPaymentMethodId(): String? { + return if (FeatureFlags.enableDefaultPaymentMethods.isEnabled) { + this.defaultPaymentMethodId + } else { + null + } + } + internal companion object { /** * Creates a [CustomerState] instance using an [ElementsSession.Customer] response. diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/viewmodels/PaymentOptionsItemsMapper.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/viewmodels/PaymentOptionsItemsMapper.kt index 3b45deae66f..aad920e4916 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/viewmodels/PaymentOptionsItemsMapper.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/viewmodels/PaymentOptionsItemsMapper.kt @@ -28,7 +28,7 @@ internal class PaymentOptionsItemsMapper( paymentMethods = customerState?.paymentMethods ?: listOf(), isLinkEnabled = isLinkEnabled, isGooglePayReady = isGooglePayReady, - defaultPaymentMethodId = customerState?.defaultPaymentMethodId + defaultPaymentMethodId = customerState?.getDefaultPaymentMethodId() ) ?: emptyList() } } diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt index be4772577ef..7d2a18a4607 100644 --- a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt +++ b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt @@ -3,16 +3,26 @@ package com.stripe.android.paymentsheet.state 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.utils.FeatureFlags import com.stripe.android.model.ElementsSession import com.stripe.android.model.PaymentMethod import com.stripe.android.model.PaymentMethodFixtures import com.stripe.android.paymentsheet.PaymentSheet import com.stripe.android.paymentsheet.PaymentSheetFixtures +import com.stripe.android.testing.FeatureFlagTestRule import com.stripe.android.testing.PaymentMethodFactory import kotlinx.coroutines.test.runTest +import org.junit.Rule import org.junit.Test class CustomerStateTest { + + @get:Rule + val enableDefaultPaymentMethods = FeatureFlagTestRule( + featureFlag = FeatureFlags.enableDefaultPaymentMethods, + isEnabled = true, + ) + @Test fun `Should create 'CustomerState' for customer session properly with permissions disabled`() { val paymentMethods = PaymentMethodFactory.cards(4) @@ -163,7 +173,13 @@ class CustomerStateTest { canRemoveDuplicates = true, ) ) - assertThat(customerState.defaultPaymentMethodId).isEqualTo(defaultPaymentMethodId) + assertThat(customerState.getDefaultPaymentMethodId()).isEqualTo(defaultPaymentMethodId) + + enableDefaultPaymentMethods.setEnabled(false) + assertThat(customerState.getDefaultPaymentMethodId()).isNull() + + enableDefaultPaymentMethods.setEnabled(true) + assertThat(customerState.getDefaultPaymentMethodId()).isEqualTo(defaultPaymentMethodId) } @Test From becd0fc3b82c03bcba2291b2412a801a95aa345d Mon Sep 17 00:00:00 2001 From: tianzhao-stripe Date: Mon, 6 Jan 2025 19:58:53 -0500 Subject: [PATCH 3/6] use defaultPaymentMethodId directly --- .../paymentsheet/SavedPaymentMethodMutator.kt | 2 +- .../android/paymentsheet/state/CustomerState.kt | 16 ++++++---------- .../viewmodels/PaymentOptionsItemsMapper.kt | 2 +- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt index f5652cdeb3a..ec9fd28dfe0 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/SavedPaymentMethodMutator.kt @@ -59,7 +59,7 @@ internal class SavedPaymentMethodMutator( isNotPaymentFlow: Boolean, ) { val defaultPaymentMethodId: StateFlow = customerStateHolder.customer.mapAsStateFlow { customerState -> - customerState?.getDefaultPaymentMethodId() + customerState?.defaultPaymentMethodId } private val paymentOptionsItemsMapper: PaymentOptionsItemsMapper by lazy { diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/CustomerState.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/CustomerState.kt index e76d51c95bf..8b73b3585c9 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/CustomerState.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/state/CustomerState.kt @@ -15,7 +15,7 @@ internal data class CustomerState( val customerSessionClientSecret: String?, val paymentMethods: List, val permissions: Permissions, - private val defaultPaymentMethodId: String? + val defaultPaymentMethodId: String? ) : Parcelable { @Parcelize data class Permissions( @@ -24,14 +24,6 @@ internal data class CustomerState( val canRemoveDuplicates: Boolean, ) : Parcelable - fun getDefaultPaymentMethodId(): String? { - return if (FeatureFlags.enableDefaultPaymentMethods.isEnabled) { - this.defaultPaymentMethodId - } else { - null - } - } - internal companion object { /** * Creates a [CustomerState] instance using an [ElementsSession.Customer] response. @@ -75,7 +67,11 @@ internal data class CustomerState( // Should always remove duplicates when using `customer_session` canRemoveDuplicates = true, ), - defaultPaymentMethodId = customer.defaultPaymentMethod + defaultPaymentMethodId = if (FeatureFlags.enableDefaultPaymentMethods.isEnabled) { + customer.defaultPaymentMethod + } else { + null + } ) } diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/viewmodels/PaymentOptionsItemsMapper.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/viewmodels/PaymentOptionsItemsMapper.kt index aad920e4916..3b45deae66f 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/viewmodels/PaymentOptionsItemsMapper.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/viewmodels/PaymentOptionsItemsMapper.kt @@ -28,7 +28,7 @@ internal class PaymentOptionsItemsMapper( paymentMethods = customerState?.paymentMethods ?: listOf(), isLinkEnabled = isLinkEnabled, isGooglePayReady = isGooglePayReady, - defaultPaymentMethodId = customerState?.getDefaultPaymentMethodId() + defaultPaymentMethodId = customerState?.defaultPaymentMethodId ) ?: emptyList() } } From b6c54df378f57f4c3b104f655648643b5d8fc9ab Mon Sep 17 00:00:00 2001 From: tianzhao-stripe Date: Mon, 6 Jan 2025 19:59:04 -0500 Subject: [PATCH 4/6] break apart the scenarios for testing --- .../paymentsheet/state/CustomerStateTest.kt | 62 ++++++++++++++----- 1 file changed, 46 insertions(+), 16 deletions(-) diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt index 7d2a18a4607..5fd0e406116 100644 --- a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt +++ b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt @@ -135,18 +135,17 @@ class CustomerStateTest { ) } - @Test - fun `Should create 'CustomerState' for customer session properly with nonnull defaultPaymentMethodId`() { + private fun createCustomerSessionForTestingDefaultPaymentMethod(defaultPaymentMethodId: String?): CustomerState { val customerId = "cus_3" val ephemeralKeySecret = "ek_3" val paymentMethods = PaymentMethodFactory.cards(3) + val mobilePaymentElementComponent = ElementsSession.Customer.Components.MobilePaymentElement.Enabled( isPaymentMethodSaveEnabled = false, isPaymentMethodRemoveEnabled = false, canRemoveLastPaymentMethod = false, allowRedisplayOverride = null, ) - val defaultPaymentMethodId = "aaa111" val customer = createElementsSessionCustomer( customerId = customerId, ephemeralKeySecret = ephemeralKeySecret, @@ -162,24 +161,55 @@ class CustomerStateTest { customerSessionClientSecret = "cuss_123", ) - assertThat(customerState.id).isEqualTo(customerId) - assertThat(customerState.ephemeralKeySecret).isEqualTo(ephemeralKeySecret) - assertThat(customerState.paymentMethods).isEqualTo(paymentMethods) - assertThat(customerState.permissions).isEqualTo( - CustomerState.Permissions( - canRemovePaymentMethods = false, - canRemoveLastPaymentMethod = false, - // Always true for `customer_session` - canRemoveDuplicates = true, - ) + return customerState + } + + @Test + fun `Should create 'CustomerState' for customer session properly with nonnull defaultPaymentMethodId and feature flag on`() { + val defaultPaymentMethodId = "aaa111" + + enableDefaultPaymentMethods.setEnabled(true) + val customerState = createCustomerSessionForTestingDefaultPaymentMethod( + defaultPaymentMethodId = defaultPaymentMethodId ) - assertThat(customerState.getDefaultPaymentMethodId()).isEqualTo(defaultPaymentMethodId) + + assertThat(customerState.defaultPaymentMethodId).isEqualTo(defaultPaymentMethodId) + } + + @Test + fun `Should create 'CustomerState' for customer session properly with nonnull defaultPaymentMethodId and feature flag off`() { + val defaultPaymentMethodId = "aaa111" enableDefaultPaymentMethods.setEnabled(false) - assertThat(customerState.getDefaultPaymentMethodId()).isNull() + val customerState = createCustomerSessionForTestingDefaultPaymentMethod( + defaultPaymentMethodId = defaultPaymentMethodId + ) + + assertThat(customerState.defaultPaymentMethodId).isNull() + } + + @Test + fun `Should create 'CustomerState' for customer session properly with null defaultPaymentMethodId and feature flag on`() { + val defaultPaymentMethodId = null enableDefaultPaymentMethods.setEnabled(true) - assertThat(customerState.getDefaultPaymentMethodId()).isEqualTo(defaultPaymentMethodId) + val customerState = createCustomerSessionForTestingDefaultPaymentMethod( + defaultPaymentMethodId = defaultPaymentMethodId + ) + + assertThat(customerState.defaultPaymentMethodId).isNull() + } + + @Test + fun `Should create 'CustomerState' for customer session properly with null defaultPaymentMethodId and feature flag off`() { + val defaultPaymentMethodId = null + + enableDefaultPaymentMethods.setEnabled(false) + val customerState = createCustomerSessionForTestingDefaultPaymentMethod( + defaultPaymentMethodId = defaultPaymentMethodId + ) + + assertThat(customerState.defaultPaymentMethodId).isNull() } @Test From 30eb4e3a7f53a216a56fa9b77078d17cae77b6d6 Mon Sep 17 00:00:00 2001 From: tianzhao-stripe Date: Mon, 6 Jan 2025 21:16:20 -0500 Subject: [PATCH 5/6] shorter method names for lint --- .../android/paymentsheet/state/CustomerStateTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt index 5fd0e406116..a88e2de9d60 100644 --- a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt +++ b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/state/CustomerStateTest.kt @@ -165,7 +165,7 @@ class CustomerStateTest { } @Test - fun `Should create 'CustomerState' for customer session properly with nonnull defaultPaymentMethodId and feature flag on`() { + fun `Create 'CustomerState' for customer session with nonnull defaultPaymentMethodId & flag on`() { val defaultPaymentMethodId = "aaa111" enableDefaultPaymentMethods.setEnabled(true) @@ -177,7 +177,7 @@ class CustomerStateTest { } @Test - fun `Should create 'CustomerState' for customer session properly with nonnull defaultPaymentMethodId and feature flag off`() { + fun `Create 'CustomerState' for customer session with nonnull defaultPaymentMethodId & flag off`() { val defaultPaymentMethodId = "aaa111" enableDefaultPaymentMethods.setEnabled(false) @@ -189,7 +189,7 @@ class CustomerStateTest { } @Test - fun `Should create 'CustomerState' for customer session properly with null defaultPaymentMethodId and feature flag on`() { + fun `Create 'CustomerState' for customer session with null defaultPaymentMethodId & flag on`() { val defaultPaymentMethodId = null enableDefaultPaymentMethods.setEnabled(true) @@ -201,7 +201,7 @@ class CustomerStateTest { } @Test - fun `Should create 'CustomerState' for customer session properly with null defaultPaymentMethodId and feature flag off`() { + fun `Create 'CustomerState' for customer session with null defaultPaymentMethodId & flag off`() { val defaultPaymentMethodId = null enableDefaultPaymentMethods.setEnabled(false) From 7ad2fb94c9eb0b7b466fdfdcbbf270dd13b4013b Mon Sep 17 00:00:00 2001 From: tianzhao-stripe Date: Tue, 7 Jan 2025 10:20:25 -0500 Subject: [PATCH 6/6] Update FeatureFlags.kt --- .../src/main/java/com/stripe/android/core/utils/FeatureFlags.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stripe-core/src/main/java/com/stripe/android/core/utils/FeatureFlags.kt b/stripe-core/src/main/java/com/stripe/android/core/utils/FeatureFlags.kt index f2c85c3747a..e82bb203ec6 100644 --- a/stripe-core/src/main/java/com/stripe/android/core/utils/FeatureFlags.kt +++ b/stripe-core/src/main/java/com/stripe/android/core/utils/FeatureFlags.kt @@ -9,7 +9,7 @@ object FeatureFlags { val nativeLinkEnabled = FeatureFlag("Native Link") val useNewUpdateCardScreen = FeatureFlag("Enable new update card screen") val instantDebitsIncentives = FeatureFlag("Instant Bank Payments Incentives") - val enableDefaultPaymentMethods = FeatureFlag("Default Payment Method") + val enableDefaultPaymentMethods = FeatureFlag("Enable Default Payment Methods") } @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)