From 22db84a62b2312be970008c49bf299f9c9f10af4 Mon Sep 17 00:00:00 2001 From: jameswoo-stripe <99316447+jameswoo-stripe@users.noreply.github.com> Date: Wed, 13 Sep 2023 10:21:30 -0700 Subject: [PATCH] Fix issue where sepa mandate text is incorrectly being displayed --- CHANGELOG.md | 1 + paymentsheet/detekt-baseline.xml | 1 + .../paymentsheet/forms/PlaceholderHelper.kt | 5 -- .../forms/PlaceholderHelperTest.kt | 89 ++++++++++++++++++- 4 files changed, 89 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 220b1cd98c4..39f1ca48aa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [ADDED] PaymentSheet now supports the following payment methods for PaymentIntents: * [7281](https://github.com/stripe/stripe-android/pull/7281) OXXO * [ADDED][7282](https://github.com/stripe/stripe-android/pull/7282) PaymentSheet now supports Boleto for PaymentIntents, SetupIntents, and PaymentIntents with setup for future usage. +* [FIXED][7303](https://github.com/stripe/stripe-android/pull/7303) Fixed an issue where SEPA mandate texts were being displayed for payment methods when they shouldn't be. ## 20.30.1 - 2023-09-11 diff --git a/paymentsheet/detekt-baseline.xml b/paymentsheet/detekt-baseline.xml index 053055a04cb..e84ee391dd1 100644 --- a/paymentsheet/detekt-baseline.xml +++ b/paymentsheet/detekt-baseline.xml @@ -29,6 +29,7 @@ LongMethod:PaymentOptionFactory.kt$PaymentOptionFactory$fun create(selection: PaymentSelection): PaymentOption LongMethod:PaymentSheetConfigurationKtx.kt$internal fun PaymentSheet.Appearance.parseAppearance() LongMethod:PaymentSheetLoader.kt$DefaultPaymentSheetLoader$private suspend fun create( elementsSession: ElementsSession, config: PaymentSheet.Configuration?, isGooglePayReady: Boolean, ): PaymentSheetState.Full + LongMethod:PlaceholderHelperTest.kt$PlaceholderHelperTest$@Test fun `Test correct placeholder is removed for placeholder spec`() LongMethod:USBankAccountForm.kt$@Composable internal fun USBankAccountForm( formArgs: FormArguments, usBankAccountFormArgs: USBankAccountFormArguments, isProcessing: Boolean, modifier: Modifier = Modifier, ) LongMethod:USBankAccountForm.kt$@Composable private fun AccountDetailsForm( formArgs: FormArguments, isProcessing: Boolean, bankName: String?, last4: String?, saveForFutureUseElement: SaveForFutureUseElement, onRemoveAccount: () -> Unit, ) MagicNumber:AutocompleteScreen.kt$0.07f diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/forms/PlaceholderHelper.kt b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/forms/PlaceholderHelper.kt index 5065cc9b439..ed6986d587c 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentsheet/forms/PlaceholderHelper.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentsheet/forms/PlaceholderHelper.kt @@ -39,7 +39,6 @@ internal object PlaceholderHelper { PlaceholderField.Email, PlaceholderField.Phone, PlaceholderField.BillingAddress, - PlaceholderField.SepaMandate, ) val modifiedSpecs = specs.mapNotNull { @@ -61,10 +60,6 @@ internal object PlaceholderHelper { configuration.address == AddressCollectionMode.Never } - is SepaMandateTextSpec -> it.takeIf { - requiresMandate - } - is PlaceholderSpec -> specForPlaceholderField( it.field, placeholderOverrideList, diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/forms/PlaceholderHelperTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/forms/PlaceholderHelperTest.kt index 96612b8e089..c3ace2b38ae 100644 --- a/paymentsheet/src/test/java/com/stripe/android/paymentsheet/forms/PlaceholderHelperTest.kt +++ b/paymentsheet/src/test/java/com/stripe/android/paymentsheet/forms/PlaceholderHelperTest.kt @@ -40,7 +40,6 @@ class PlaceholderHelperTest { EmailSpec(), PhoneSpec(), AddressSpec(), - SepaMandateTextSpec(), ), ) assertThat(specs).isEmpty() @@ -109,7 +108,54 @@ class PlaceholderHelperTest { ) } - @Suppress("LongMethod") + @Test + fun `Test when requiresMandate is true, SepaMandateSpec is only added when specified`() { + val specs = specsForConfiguration( + configuration = PaymentSheet.BillingDetailsCollectionConfiguration(), + placeholderOverrideList = emptyList(), + requiresMandate = true, + specs = listOf( + NameSpec(), + ), + ) + + assertThat(specs).containsExactly( + NameSpec(), + ) + + val specsWithSepa = specsForConfiguration( + configuration = PaymentSheet.BillingDetailsCollectionConfiguration(), + placeholderOverrideList = emptyList(), + requiresMandate = true, + specs = listOf( + NameSpec(), + SepaMandateTextSpec() + ), + ) + + assertThat(specsWithSepa).containsExactly( + NameSpec(), + SepaMandateTextSpec() + ) + + val specsWithSepaPlaceholder = specsForConfiguration( + configuration = PaymentSheet.BillingDetailsCollectionConfiguration(), + placeholderOverrideList = emptyList(), + requiresMandate = true, + specs = listOf( + NameSpec(), + PlaceholderSpec( + field = PlaceholderSpec.PlaceholderField.SepaMandate, + ) + ), + ) + + assertThat(specsWithSepaPlaceholder).containsExactly( + NameSpec(), + SepaMandateTextSpec() + ) + } + @Test fun `Test correct spec is returned for placeholder fields`() { val billingDetailsCollectionConfiguration = PaymentSheet.BillingDetailsCollectionConfiguration( @@ -160,6 +206,14 @@ class PlaceholderHelperTest { configuration = billingDetailsCollectionConfiguration, ) ).isEqualTo(AddressSpec(hideCountry = true)) + assertThat( + specForPlaceholderField( + field = PlaceholderField.SepaMandate, + placeholderOverrideList = emptyList(), + requiresMandate = true, + configuration = billingDetailsCollectionConfiguration, + ) + ).isEqualTo(SepaMandateTextSpec()) } @Test @@ -222,6 +276,7 @@ class PlaceholderHelperTest { PlaceholderField.Email, PlaceholderField.Phone, PlaceholderField.BillingAddress, + PlaceholderField.SepaMandate, ) placeholders = basePlaceholders() @@ -230,6 +285,7 @@ class PlaceholderHelperTest { PlaceholderField.Name, PlaceholderField.Phone, PlaceholderField.BillingAddress, + PlaceholderField.SepaMandate, ) placeholders = basePlaceholders() @@ -238,6 +294,7 @@ class PlaceholderHelperTest { PlaceholderField.Name, PlaceholderField.Email, PlaceholderField.BillingAddress, + PlaceholderField.SepaMandate, ) placeholders = basePlaceholders() @@ -246,6 +303,16 @@ class PlaceholderHelperTest { PlaceholderField.Name, PlaceholderField.Email, PlaceholderField.Phone, + PlaceholderField.SepaMandate, + ) + + placeholders = basePlaceholders() + removeCorrespondingPlaceholder(placeholders, SepaMandateTextSpec()) + assertThat(placeholders).containsExactly( + PlaceholderField.Name, + PlaceholderField.Email, + PlaceholderField.Phone, + PlaceholderField.BillingAddress, ) } @@ -257,6 +324,7 @@ class PlaceholderHelperTest { PlaceholderField.Email, PlaceholderField.Phone, PlaceholderField.BillingAddress, + PlaceholderField.SepaMandate, ) placeholders = basePlaceholders() @@ -268,6 +336,7 @@ class PlaceholderHelperTest { PlaceholderField.Name, PlaceholderField.Phone, PlaceholderField.BillingAddress, + PlaceholderField.SepaMandate, ) placeholders = basePlaceholders() @@ -279,6 +348,7 @@ class PlaceholderHelperTest { PlaceholderField.Name, PlaceholderField.Email, PlaceholderField.BillingAddress, + PlaceholderField.SepaMandate, ) placeholders = basePlaceholders() @@ -290,6 +360,7 @@ class PlaceholderHelperTest { PlaceholderField.Name, PlaceholderField.Email, PlaceholderField.Phone, + PlaceholderField.SepaMandate, ) placeholders = basePlaceholders() @@ -301,6 +372,19 @@ class PlaceholderHelperTest { PlaceholderField.Name, PlaceholderField.Email, PlaceholderField.Phone, + PlaceholderField.SepaMandate, + ) + + placeholders = basePlaceholders() + removeCorrespondingPlaceholder( + placeholders, + PlaceholderSpec(field = PlaceholderField.SepaMandate) + ) + assertThat(placeholders).containsExactly( + PlaceholderField.Name, + PlaceholderField.Email, + PlaceholderField.Phone, + PlaceholderField.BillingAddress, ) } @@ -383,5 +467,6 @@ class PlaceholderHelperTest { PlaceholderField.Email, PlaceholderField.Phone, PlaceholderField.BillingAddress, + PlaceholderField.SepaMandate, ) }