From 022be8f894db38aac8e978b7b8643c73b73ce942 Mon Sep 17 00:00:00 2001 From: Michelle Brubaker Date: Fri, 17 Sep 2021 16:50:53 -0400 Subject: [PATCH 1/3] If postal code is required it should be reported in the invalidFields. --- .../activity/CreateCardSourceActivity.kt | 17 ++-- .../stripe/android/view/CardInputWidget.kt | 50 +++++++---- .../android/view/CardInputWidgetTest.kt | 82 +++++++++++++++---- 3 files changed, 115 insertions(+), 34 deletions(-) diff --git a/example/src/main/java/com/stripe/example/activity/CreateCardSourceActivity.kt b/example/src/main/java/com/stripe/example/activity/CreateCardSourceActivity.kt index b17eb78ff6d..1391921cd86 100644 --- a/example/src/main/java/com/stripe/example/activity/CreateCardSourceActivity.kt +++ b/example/src/main/java/com/stripe/example/activity/CreateCardSourceActivity.kt @@ -2,6 +2,7 @@ package com.stripe.example.activity import android.content.Intent import android.os.Bundle +import android.util.Log import android.view.View import androidx.activity.viewModels import androidx.appcompat.app.AlertDialog @@ -10,11 +11,7 @@ import androidx.recyclerview.widget.LinearLayoutManager import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.stripe.android.ApiResultCallback import com.stripe.android.Stripe -import com.stripe.android.model.CardBrand -import com.stripe.android.model.CardParams -import com.stripe.android.model.Source -import com.stripe.android.model.SourceParams -import com.stripe.android.model.SourceTypeModel +import com.stripe.android.model.* import com.stripe.example.R import com.stripe.example.StripeFactory import com.stripe.example.adapter.SourcesAdapter @@ -50,11 +47,21 @@ class CreateCardSourceActivity : AppCompatActivity() { setContentView(viewBinding.root) viewBinding.createButton.setOnClickListener { + // If a field is invalid this will shift focus to that field viewBinding.cardWidget.cardParams?.let { createCardSource(it) } ?: showSnackbar("Enter a valid card.") } + viewBinding.cardWidget.setCardValidCallback { isValid, invalidFields -> // We will not call cardParams unless it is valid because + // this will cause the inFocus field to change. + if (isValid) { + Log.e("STRIPE", "Validity: $isValid, ${viewBinding.cardWidget.cardParams}") + } else { + Log.e("STRIPE", "Validity: $isValid, invalidField: ${invalidFields}") + } + } + viewBinding.recyclerView.setHasFixedSize(true) viewBinding.recyclerView.layoutManager = LinearLayoutManager(this) viewBinding.recyclerView.adapter = sourcesAdapter diff --git a/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt b/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt index cb68b05837c..f7e6746fbc1 100644 --- a/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt +++ b/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt @@ -106,6 +106,10 @@ class CardInputWidget @JvmOverloads constructor( }, CardValidCallback.Fields.Cvc.takeIf { this.cvc == null + }, + CardValidCallback.Fields.Postal.takeIf { + (postalCodeRequired || usZipCodeRequired) && + postalCodeEditText.postalCode.isNullOrBlank() } ).toSet() } @@ -152,8 +156,8 @@ class CardInputWidget @JvmOverloads constructor( @VisibleForTesting @JvmSynthetic - internal val requiredFields: List - private val allFields: List + internal val requiredFields: MutableSet = mutableSetOf() + private val allFields: Set /** * The [StripeEditText] fields that are currently enabled and active in the UI. @@ -169,7 +173,7 @@ class CardInputWidget @JvmOverloads constructor( /** * A [PaymentMethodCreateParams.Card] representing the card details if all fields are valid; - * otherwise `null` + * otherwise `null`. If a field is invalid focus will shift to the invalid field. */ override val paymentMethodCard: PaymentMethodCreateParams.Card? get() { @@ -197,7 +201,7 @@ class CardInputWidget @JvmOverloads constructor( /** * A [PaymentMethodCreateParams] representing the card details and postal code if all fields - * are valid; otherwise `null` + * are valid; otherwise `null`. If a field is invalid focus will shift to the invalid field */ override val paymentMethodCreateParams: PaymentMethodCreateParams? get() { @@ -208,7 +212,7 @@ class CardInputWidget @JvmOverloads constructor( /** * A [CardParams] representing the card details and postal code if all fields are valid; - * otherwise `null` + * otherwise `null`. If a field is invalid focus will shift to the invalid field. */ override val cardParams: CardParams? get() { @@ -221,7 +225,7 @@ class CardInputWidget @JvmOverloads constructor( cvcEditText.shouldShowError = cvc == null postalCodeEditText.shouldShowError = (postalCodeRequired || usZipCodeRequired) && - postalCodeEditText.postalCode.isNullOrBlank() + postalCodeEditText.postalCode.isNullOrBlank() // Announce error messages for accessibility currentFields @@ -275,7 +279,7 @@ class CardInputWidget @JvmOverloads constructor( /** * The postal code field is enabled by default. Disabling the postal code field may impact * auth success rates, so it is discouraged to disable it unless you are collecting the postal - * code outside of this form. + * code outside of this form. If the postal code is disabled it will not be shown in the view. */ var postalCodeEnabled: Boolean by Delegates.observable( CardWidget.DEFAULT_POSTAL_CODE_ENABLED @@ -291,6 +295,7 @@ class CardInputWidget @JvmOverloads constructor( cvcEditText.imeOptions = EditorInfo.IME_ACTION_DONE } + updatePostalRequired() } /** @@ -302,7 +307,11 @@ class CardInputWidget @JvmOverloads constructor( * Note that some countries do not have postal codes, so requiring postal code will prevent * those users from submitting this form successfully. */ - var postalCodeRequired: Boolean = CardWidget.DEFAULT_POSTAL_CODE_REQUIRED + var postalCodeRequired: Boolean by Delegates.observable( + CardWidget.DEFAULT_POSTAL_CODE_REQUIRED + ) { _, _, _ -> + updatePostalRequired() + } /** * If [postalCodeEnabled] is true and [usZipCodeRequired] is true, then postal code is a @@ -318,6 +327,17 @@ class CardInputWidget @JvmOverloads constructor( } else { postalCodeEditText.config = PostalCodeEditText.Config.Global } + + updatePostalRequired() + } + + private fun updatePostalRequired() { + val isPostalCodeCurrentlyRequired = requiredFields.contains(postalCodeEditText) + if ((postalCodeRequired || usZipCodeRequired) && postalCodeEnabled) { + requiredFields.add(postalCodeEditText) + } else { + requiredFields.remove(postalCodeEditText) + } } private val frameStart: Int @@ -342,10 +362,12 @@ class CardInputWidget @JvmOverloads constructor( frameWidthSupplier = { containerLayout.width } - requiredFields = listOf( - cardNumberEditText, - cvcEditText, - expiryDateEditText + requiredFields.addAll( + setOf( + cardNumberEditText, + cvcEditText, + expiryDateEditText + ) ) allFields = requiredFields.plus(postalCodeEditText) @@ -481,8 +503,8 @@ class CardInputWidget @JvmOverloads constructor( * Override of [View.isEnabled] that returns `true` only * if all three sub-controls are enabled. * - * @return `true` if the card number field, expiry field, and cvc field are enabled, - * `false` otherwise + * @return `true` if the card number field, expiry field, cvc field, and postal (if required) + * are enabled, `false` otherwise */ override fun isEnabled(): Boolean { return requiredFields.all { it.isEnabled } diff --git a/payments-core/src/test/java/com/stripe/android/view/CardInputWidgetTest.kt b/payments-core/src/test/java/com/stripe/android/view/CardInputWidgetTest.kt index f4e1f69b9fa..9883b062c9a 100644 --- a/payments-core/src/test/java/com/stripe/android/view/CardInputWidgetTest.kt +++ b/payments-core/src/test/java/com/stripe/android/view/CardInputWidgetTest.kt @@ -17,12 +17,7 @@ import com.stripe.android.CardNumberFixtures.VISA_WITH_SPACES import com.stripe.android.PaymentConfiguration import com.stripe.android.cards.AccountRangeFixtures import com.stripe.android.cards.DefaultCardAccountRangeStore -import com.stripe.android.model.Address -import com.stripe.android.model.BinFixtures -import com.stripe.android.model.CardBrand -import com.stripe.android.model.CardParams -import com.stripe.android.model.PaymentMethod -import com.stripe.android.model.PaymentMethodCreateParams +import com.stripe.android.model.* import com.stripe.android.testharness.ViewTestUtils import com.stripe.android.utils.TestUtils.idleLooper import com.stripe.android.view.CardInputWidget.Companion.LOGGING_TOKEN @@ -33,7 +28,7 @@ import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.resetMain import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -import java.util.Calendar +import java.util.* import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -1297,25 +1292,24 @@ internal class CardInputWidgetTest { idleLooper() assertThat(cardInputWidget.requiredFields) - .isEqualTo(cardInputWidget.currentFields) + .isEqualTo(cardInputWidget.currentFields.toHashSet()) } @Test fun currentFields_notEquals_requiredFields_withPostalCodeEnabled() { cardInputWidget.postalCodeEnabled = true assertThat(cardInputWidget.requiredFields) - .isNotEqualTo(cardInputWidget.currentFields) + .isNotEqualTo(cardInputWidget.currentFields.toHashSet()) } @Test - fun testCardValidCallback() { + fun testCardValidCallback_withPostalCodeDefaultDisabled() { var currentIsValid = false var currentInvalidFields = emptySet() cardInputWidget.setCardValidCallback { isValid, invalidFields -> currentIsValid = isValid currentInvalidFields = invalidFields } - assertThat(currentIsValid) .isFalse() assertThat(currentInvalidFields) @@ -1362,6 +1356,60 @@ internal class CardInputWidgetTest { .containsExactly(CardValidCallback.Fields.Cvc) } + @Test + fun testCardValidCallback_withPostalCodeEnabledNotRequired() { + var currentIsValid = false + var currentInvalidFields = emptySet() + cardInputWidget.postalCodeEnabled = true + cardInputWidget.setCardValidCallback { isValid, invalidFields -> + currentIsValid = isValid + currentInvalidFields = invalidFields + } + assertThat(currentIsValid) + .isFalse() + assertThat(currentInvalidFields) + .containsExactly( + CardValidCallback.Fields.Number, + CardValidCallback.Fields.Expiry, + CardValidCallback.Fields.Cvc + ) + } + + @Test + fun testCardValidCallback_withPostalCodeEnabledAndRequired() { + var currentIsValid = false + var currentInvalidFields = emptySet() + cardInputWidget.postalCodeEnabled = true + cardInputWidget.usZipCodeRequired = true + cardInputWidget.setCardValidCallback { isValid, invalidFields -> + currentIsValid = isValid + currentInvalidFields = invalidFields + } + assertThat(currentIsValid) + .isFalse() + assertThat(currentInvalidFields) + .containsExactly( + CardValidCallback.Fields.Number, + CardValidCallback.Fields.Expiry, + CardValidCallback.Fields.Cvc, + CardValidCallback.Fields.Postal + ) + + cardInputWidget.setCardNumber(VISA_NO_SPACES) + expiryEditText.setText("1250") + cvcEditText.setText("123") + assertThat(currentIsValid).isFalse() + assertThat(currentInvalidFields).containsExactly(CardValidCallback.Fields.Postal) + + postalCodeEditText.setText("12345") + assertThat(currentIsValid).isTrue() + assertThat(currentInvalidFields).isEmpty() + + postalCodeEditText.setText("123") + assertThat(currentIsValid).isFalse() + assertThat(currentInvalidFields).containsExactly(CardValidCallback.Fields.Postal) + } + @Test fun shouldShowErrorIcon_shouldBeUpdatedCorrectly() { cardInputWidget.setExpiryDate(12, 2030) @@ -1480,7 +1528,8 @@ internal class CardInputWidgetTest { @Test fun `Verify on postal code focus change listeners trigger the callback`() { - cardInputWidget.postalCodeEditText.getParentOnFocusChangeListener().onFocusChange(cardInputWidget.postalCodeEditText, true) + cardInputWidget.postalCodeEditText.getParentOnFocusChangeListener() + .onFocusChange(cardInputWidget.postalCodeEditText, true) assertThat(cardInputListener.focusedFields) .contains(CardInputListener.FocusField.PostalCode) @@ -1488,7 +1537,8 @@ internal class CardInputWidgetTest { @Test fun `Verify on cvc focus change listeners trigger the callback`() { - cardInputWidget.cvcEditText.getParentOnFocusChangeListener().onFocusChange(cardInputWidget.cvcEditText, true) + cardInputWidget.cvcEditText.getParentOnFocusChangeListener() + .onFocusChange(cardInputWidget.cvcEditText, true) assertThat(cardInputListener.focusedFields) .contains(CardInputListener.FocusField.Cvc) @@ -1496,7 +1546,8 @@ internal class CardInputWidgetTest { @Test fun `Verify on expiration date focus change listeners trigger the callback`() { - cardInputWidget.expiryDateEditText.getParentOnFocusChangeListener().onFocusChange(cardInputWidget.expiryDateEditText, true) + cardInputWidget.expiryDateEditText.getParentOnFocusChangeListener() + .onFocusChange(cardInputWidget.expiryDateEditText, true) assertThat(cardInputListener.focusedFields) .contains(CardInputListener.FocusField.ExpiryDate) @@ -1504,7 +1555,8 @@ internal class CardInputWidgetTest { @Test fun `Verify on card number focus change listeners trigger the callback`() { - cardInputWidget.cardNumberEditText.getParentOnFocusChangeListener().onFocusChange(cardInputWidget.cardNumberEditText, true) + cardInputWidget.cardNumberEditText.getParentOnFocusChangeListener() + .onFocusChange(cardInputWidget.cardNumberEditText, true) assertThat(cardInputListener.focusedFields) .contains(CardInputListener.FocusField.CardNumber) From 79b8aff6e13f6d904e7c4a7ae0f84c3cfde6d680 Mon Sep 17 00:00:00 2001 From: Michelle Brubaker Date: Sat, 18 Sep 2021 11:58:51 -0400 Subject: [PATCH 2/3] If postal code is required it should be reported in the invalidFields. --- .../example/activity/CreateCardSourceActivity.kt | 8 ++++++-- .../com/stripe/android/view/CardInputWidget.kt | 14 ++++++-------- .../com/stripe/android/view/CardInputWidgetTest.kt | 10 ++++++++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/example/src/main/java/com/stripe/example/activity/CreateCardSourceActivity.kt b/example/src/main/java/com/stripe/example/activity/CreateCardSourceActivity.kt index 1391921cd86..a34f635acb9 100644 --- a/example/src/main/java/com/stripe/example/activity/CreateCardSourceActivity.kt +++ b/example/src/main/java/com/stripe/example/activity/CreateCardSourceActivity.kt @@ -11,7 +11,11 @@ import androidx.recyclerview.widget.LinearLayoutManager import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.stripe.android.ApiResultCallback import com.stripe.android.Stripe -import com.stripe.android.model.* +import com.stripe.android.model.CardBrand +import com.stripe.android.model.CardParams +import com.stripe.android.model.Source +import com.stripe.android.model.SourceParams +import com.stripe.android.model.SourceTypeModel import com.stripe.example.R import com.stripe.example.StripeFactory import com.stripe.example.adapter.SourcesAdapter @@ -58,7 +62,7 @@ class CreateCardSourceActivity : AppCompatActivity() { if (isValid) { Log.e("STRIPE", "Validity: $isValid, ${viewBinding.cardWidget.cardParams}") } else { - Log.e("STRIPE", "Validity: $isValid, invalidField: ${invalidFields}") + Log.e("STRIPE", "Validity: $isValid, invalidField: $invalidFields") } } diff --git a/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt b/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt index f7e6746fbc1..b42db19b888 100644 --- a/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt +++ b/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt @@ -108,8 +108,7 @@ class CardInputWidget @JvmOverloads constructor( this.cvc == null }, CardValidCallback.Fields.Postal.takeIf { - (postalCodeRequired || usZipCodeRequired) && - postalCodeEditText.postalCode.isNullOrBlank() + isPostalRequired() && postalCodeEditText.postalCode.isNullOrBlank() } ).toSet() } @@ -156,7 +155,7 @@ class CardInputWidget @JvmOverloads constructor( @VisibleForTesting @JvmSynthetic - internal val requiredFields: MutableSet = mutableSetOf() + internal val requiredFields: MutableSet private val allFields: Set /** @@ -332,14 +331,15 @@ class CardInputWidget @JvmOverloads constructor( } private fun updatePostalRequired() { - val isPostalCodeCurrentlyRequired = requiredFields.contains(postalCodeEditText) - if ((postalCodeRequired || usZipCodeRequired) && postalCodeEnabled) { + if (isPostalRequired()) { requiredFields.add(postalCodeEditText) } else { requiredFields.remove(postalCodeEditText) } } + private fun isPostalRequired() = (postalCodeRequired || usZipCodeRequired) && postalCodeEnabled + private val frameStart: Int get() { val isLtr = context.resources.configuration.layoutDirection == View.LAYOUT_DIRECTION_LTR @@ -362,13 +362,11 @@ class CardInputWidget @JvmOverloads constructor( frameWidthSupplier = { containerLayout.width } - requiredFields.addAll( - setOf( + requiredFields = mutableSetOf( cardNumberEditText, cvcEditText, expiryDateEditText ) - ) allFields = requiredFields.plus(postalCodeEditText) initView(attrs) diff --git a/payments-core/src/test/java/com/stripe/android/view/CardInputWidgetTest.kt b/payments-core/src/test/java/com/stripe/android/view/CardInputWidgetTest.kt index 9883b062c9a..19ca790991c 100644 --- a/payments-core/src/test/java/com/stripe/android/view/CardInputWidgetTest.kt +++ b/payments-core/src/test/java/com/stripe/android/view/CardInputWidgetTest.kt @@ -17,7 +17,12 @@ import com.stripe.android.CardNumberFixtures.VISA_WITH_SPACES import com.stripe.android.PaymentConfiguration import com.stripe.android.cards.AccountRangeFixtures import com.stripe.android.cards.DefaultCardAccountRangeStore -import com.stripe.android.model.* +import com.stripe.android.model.Address +import com.stripe.android.model.BinFixtures +import com.stripe.android.model.CardBrand +import com.stripe.android.model.CardParams +import com.stripe.android.model.PaymentMethod +import com.stripe.android.model.PaymentMethodCreateParams import com.stripe.android.testharness.ViewTestUtils import com.stripe.android.utils.TestUtils.idleLooper import com.stripe.android.view.CardInputWidget.Companion.LOGGING_TOKEN @@ -28,7 +33,7 @@ import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.resetMain import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -import java.util.* +import java.util.Calendar import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -1310,6 +1315,7 @@ internal class CardInputWidgetTest { currentIsValid = isValid currentInvalidFields = invalidFields } + assertThat(currentIsValid) .isFalse() assertThat(currentInvalidFields) From 9f0b58dc69b3f7a143777c1fb533fcd00bfda998 Mon Sep 17 00:00:00 2001 From: Michelle Brubaker Date: Sat, 18 Sep 2021 12:01:09 -0400 Subject: [PATCH 3/3] ktlintFormat --- .../java/com/stripe/android/view/CardInputWidget.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt b/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt index b42db19b888..617c216dda7 100644 --- a/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt +++ b/payments-core/src/main/java/com/stripe/android/view/CardInputWidget.kt @@ -224,7 +224,7 @@ class CardInputWidget @JvmOverloads constructor( cvcEditText.shouldShowError = cvc == null postalCodeEditText.shouldShowError = (postalCodeRequired || usZipCodeRequired) && - postalCodeEditText.postalCode.isNullOrBlank() + postalCodeEditText.postalCode.isNullOrBlank() // Announce error messages for accessibility currentFields @@ -363,10 +363,10 @@ class CardInputWidget @JvmOverloads constructor( frameWidthSupplier = { containerLayout.width } requiredFields = mutableSetOf( - cardNumberEditText, - cvcEditText, - expiryDateEditText - ) + cardNumberEditText, + cvcEditText, + expiryDateEditText + ) allFields = requiredFields.plus(postalCodeEditText) initView(attrs)