Skip to content

Commit

Permalink
Correctly validate country field on shipping address form
Browse files Browse the repository at this point in the history
- Validation was being incorrectly skipped in
  `PaymentFlowActivity`. Use
  `ShippingInfoWidget#shippingInformation` instead
  of `rawShippingInformation`.
- Implement a `AutoCompleteTextView.Validator` in
  `CountryAutoCompleteTextView` and show an error
  message if the text is an invalid country.
- When a user submits their shipping information in
  `PaymentFlowActivity`, perform validation using
  the `AutoCompleteTextView.Validator`.
- Add string resource `address_country_invalid`.
  Create JIRA for translation.

ANDROID-451
  • Loading branch information
mshafrir-stripe committed Nov 19, 2019
1 parent 43e70e3 commit 1cca47b
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 38 deletions.
7 changes: 3 additions & 4 deletions stripe/res/layout/country_autocomplete_textview.xml
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
<?xml version="1.0" encoding="utf-8"?>
<merge xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
>
android:layout_width="match_parent"
android:layout_height="wrap_content">

<com.google.android.material.textfield.TextInputLayout
android:id="@+id/tl_country_cat"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:hint="@string/address_label_country"
>
android:labelFor="@id/autocomplete_country_cat">

<AutoCompleteTextView
android:id="@+id/autocomplete_country_cat"
Expand Down
5 changes: 5 additions & 0 deletions stripe/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@
<string name="address_postal_code_invalid">Your postal code is invalid</string>
<!--Error text indicating zip/ postal code is invalid, used for international addresses-->
<string name="address_zip_postal_invalid">Your ZIP/Postal code is invalid</string>

<!-- TODO(mshafrir-stripe): translate string (ANDROID-450) -->
<!--Error text indicating country is invalid-->
<string name="address_country_invalid" tools:ignore="MissingTranslation">Your country is invalid</string>

<!--Label for input requesting province, used for canadian addresses-->
<string name="address_label_province">Province</string>
<!--Label for input requesting province where province is optional, used for canadian addresses-->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import java.util.Locale
*/
internal class CountryAdapter(
context: Context,
private var unfilteredCountries: List<Country>
internal var unfilteredCountries: List<Country>
) : ArrayAdapter<Country>(context, R.layout.country_text_view) {
private val countryFilter: CountryFilter = CountryFilter(
unfilteredCountries,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,21 @@ import androidx.annotation.VisibleForTesting
import androidx.core.os.ConfigurationCompat
import com.stripe.android.R
import java.util.Locale
import kotlinx.android.synthetic.main.country_autocomplete_textview.view.*

internal class CountryAutoCompleteTextView @JvmOverloads constructor(
context: Context,
attrs: AttributeSet? = null,
defStyleAttr: Int = 0
) : FrameLayout(context, attrs, defStyleAttr) {
private val countryAutocomplete: AutoCompleteTextView
@VisibleForTesting
internal val countryAutocomplete: AutoCompleteTextView

/**
* @return 2 digit country code of the country selected by this input.
*/
@VisibleForTesting
var selectedCountry: Country
var selectedCountry: Country?

@JvmSynthetic
internal var countryChangeCallback: (Country) -> Unit = {}
Expand Down Expand Up @@ -58,6 +60,30 @@ internal class CountryAutoCompleteTextView @JvmOverloads constructor(

selectedCountry = countryAdapter.firstItem
updateInitialCountry()

val errorMessage = resources.getString(R.string.address_country_invalid)
countryAutocomplete.validator = object : AutoCompleteTextView.Validator {
override fun fixText(invalidText: CharSequence?): CharSequence {
return invalidText ?: ""
}

override fun isValid(text: CharSequence?): Boolean {
val validCountry = countryAdapter.unfilteredCountries.firstOrNull {
it.name == text.toString()
}

return if (validCountry != null) {
selectedCountry = validCountry
clearError()
true
} else {
selectedCountry = null
tl_country_cat.error = errorMessage
tl_country_cat.isErrorEnabled = true
false
}
}
}
}

private fun updateInitialCountry() {
Expand Down Expand Up @@ -94,12 +120,13 @@ internal class CountryAutoCompleteTextView @JvmOverloads constructor(
val displayCountry = country?.let {
updatedSelectedCountryCode(it)
displayCountryEntered
} ?: selectedCountry.name
} ?: selectedCountry?.name

countryAutocomplete.setText(displayCountry)
}

private fun updatedSelectedCountryCode(country: Country) {
clearError()
if (selectedCountry != country) {
selectedCountry = country
countryChangeCallback(country)
Expand All @@ -110,4 +137,13 @@ internal class CountryAutoCompleteTextView @JvmOverloads constructor(
return CountryUtils.getCountryByCode(countryCode)?.name
?: Locale("", countryCode).displayCountry
}

internal fun validateCountry() {
countryAutocomplete.performValidation()
}

private fun clearError() {
tl_country_cat.error = null
tl_country_cat.isErrorEnabled = false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import com.stripe.android.model.Customer
import com.stripe.android.model.ShippingInformation
import com.stripe.android.model.ShippingMethod
import java.lang.ref.WeakReference
import kotlinx.android.synthetic.main.activity_enter_shipping_info.*
import kotlinx.android.synthetic.main.activity_shipping_flow.*

/**
Expand Down Expand Up @@ -189,8 +190,7 @@ class PaymentFlowActivity : StripeActivity() {

private val shippingInfo: ShippingInformation?
get() {
val shippingInfoWidget: ShippingInfoWidget = findViewById(R.id.shipping_info_widget)
return shippingInfoWidget.rawShippingInformation
return shipping_info_widget.shippingInformation
}

private val selectedShippingMethod: ShippingMethod?
Expand Down
33 changes: 18 additions & 15 deletions stripe/src/main/java/com/stripe/android/view/ShippingInfoWidget.kt
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,17 @@ class ShippingInfoWidget @JvmOverloads constructor(
/**
* Return [ShippingInformation] based on user input.
*/
internal val rawShippingInformation: ShippingInformation
private val rawShippingInformation: ShippingInformation
get() {
val address = Address.Builder()
.setCity(cityEditText.text?.toString())
.setCountry(countryAutoCompleteTextView.selectedCountry.code)
.setLine1(addressEditText.text?.toString())
.setLine2(addressEditText2.text?.toString())
.setPostalCode(postalCodeEditText.text?.toString())
.setState(stateEditText.text?.toString())
.build()
return ShippingInformation(
address,
Address.Builder()
.setCity(cityEditText.text?.toString())
.setCountry(countryAutoCompleteTextView.selectedCountry?.code)
.setLine1(addressEditText.text?.toString())
.setLine2(addressEditText2.text?.toString())
.setPostalCode(postalCodeEditText.text?.toString())
.setState(stateEditText.text?.toString())
.build(),
nameEditText.text?.toString(),
phoneNumberEditText.text?.toString()
)
Expand Down Expand Up @@ -136,7 +135,7 @@ class ShippingInfoWidget @JvmOverloads constructor(
optionalShippingInfoFields = optionalAddressFields.orEmpty()
renderLabels()

countryAutoCompleteTextView.selectedCountry.let(::renderCountrySpecificLabels)
countryAutoCompleteTextView.selectedCountry?.let(::renderCountrySpecificLabels)
}

/**
Expand All @@ -147,7 +146,7 @@ class ShippingInfoWidget @JvmOverloads constructor(
hiddenShippingInfoFields = hiddenAddressFields.orEmpty()
renderLabels()

countryAutoCompleteTextView.selectedCountry.let(::renderCountrySpecificLabels)
countryAutoCompleteTextView.selectedCountry?.let(::renderCountrySpecificLabels)
}

/**
Expand Down Expand Up @@ -191,9 +190,12 @@ class ShippingInfoWidget @JvmOverloads constructor(
val postalCode = postalCodeEditText.text?.toString() ?: return false
val phoneNumber = phoneNumberEditText.text?.toString() ?: return false

countryAutoCompleteTextView.validateCountry()
val selectedCountry = countryAutoCompleteTextView.selectedCountry

val isPostalCodeValid = shippingPostalCodeValidator.isValid(
postalCode,
countryAutoCompleteTextView.selectedCountry.code,
selectedCountry?.code,
optionalShippingInfoFields,
hiddenShippingInfoFields
)
Expand All @@ -219,7 +221,8 @@ class ShippingInfoWidget @JvmOverloads constructor(
phoneNumberEditText.shouldShowError = requiredPhoneNumberEmpty

return isPostalCodeValid && !requiredAddressLine1Empty && !requiredCityEmpty &&
!requiredStateEmpty && !requiredNameEmpty && !requiredPhoneNumberEmpty
!requiredStateEmpty && !requiredNameEmpty && !requiredPhoneNumberEmpty &&
selectedCountry != null
}

private fun isFieldRequired(@CustomizableShippingField field: String): Boolean {
Expand All @@ -241,7 +244,7 @@ class ShippingInfoWidget @JvmOverloads constructor(
setupErrorHandling()
renderLabels()

countryAutoCompleteTextView.selectedCountry.let(::renderCountrySpecificLabels)
countryAutoCompleteTextView.selectedCountry?.let(::renderCountrySpecificLabels)
}

private fun setupErrorHandling() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,26 @@ internal class ShippingPostalCodeValidator {

fun isValid(
postalCode: String,
countryCode: String,
countryCode: String?,
optionalShippingInfoFields: List<String>,
hiddenShippingInfoFields: List<String>
): Boolean {
if (countryCode == null) {
return false
}

val postalCodePattern = POSTAL_CODE_PATTERNS[countryCode]
return if (postalCode.isEmpty() &&
isPostalCodeOptional(optionalShippingInfoFields, hiddenShippingInfoFields)) {
true
} else postalCodePattern?.matcher(postalCode)?.matches()
?: if (CountryUtils.doesCountryUsePostalCode(countryCode)) {
postalCode.isNotEmpty()
} else {
true
}
} else {
postalCodePattern?.matcher(postalCode)?.matches()
?: if (CountryUtils.doesCountryUsePostalCode(countryCode)) {
postalCode.isNotEmpty()
} else {
true
}
}
}

private companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue
import org.junit.runner.RunWith
Expand Down Expand Up @@ -37,13 +38,14 @@ class CountryAutoCompleteTextViewTest : BaseViewTest<ShippingInfoTestActivity>(

@Test
fun countryAutoCompleteTextView_whenInitialized_displaysDefaultLocaleDisplayName() {
assertEquals(Locale.US.country, countryAutoCompleteTextView.selectedCountry.code)
assertEquals(Locale.US.country, countryAutoCompleteTextView.selectedCountry?.code)
assertEquals(Locale.US.displayCountry, autoCompleteTextView.text.toString())
}

@Test
fun updateUIForCountryEntered_whenInvalidCountry_revertsToLastCountry() {
val previousValidCountryCode = countryAutoCompleteTextView.selectedCountry.code
val previousValidCountryCode =
countryAutoCompleteTextView.selectedCountry?.code.orEmpty()
countryAutoCompleteTextView.setCountrySelected("FAKE COUNTRY CODE")
assertNull(autoCompleteTextView.error)
assertEquals(autoCompleteTextView.text.toString(),
Expand All @@ -56,9 +58,9 @@ class CountryAutoCompleteTextViewTest : BaseViewTest<ShippingInfoTestActivity>(

@Test
fun updateUIForCountryEntered_whenValidCountry_UIUpdates() {
assertEquals(Locale.US.country, countryAutoCompleteTextView.selectedCountry.code)
assertEquals(Locale.US.country, countryAutoCompleteTextView.selectedCountry?.code)
countryAutoCompleteTextView.setCountrySelected(Locale.UK.country)
assertEquals(Locale.UK.country, countryAutoCompleteTextView.selectedCountry.code)
assertEquals(Locale.UK.country, countryAutoCompleteTextView.selectedCountry?.code)
}

@Test
Expand All @@ -74,10 +76,26 @@ class CountryAutoCompleteTextViewTest : BaseViewTest<ShippingInfoTestActivity>(
countryAutoCompleteTextView.setAllowedCountryCodes(setOf("fr", "de"))
assertEquals(
"FR",
countryAutoCompleteTextView.selectedCountry.code
countryAutoCompleteTextView.selectedCountry?.code
)
}

@Test
fun validateCountry_withInvalidCountry_setsSelectedCountryToNull() {
assertNotNull(countryAutoCompleteTextView.selectedCountry)
countryAutoCompleteTextView.countryAutocomplete.setText("invalid country")
countryAutoCompleteTextView.validateCountry()
assertNull(countryAutoCompleteTextView.selectedCountry)
}

@Test
fun validateCountry_withValidCountry_setsSelectedCountry() {
assertNotNull(countryAutoCompleteTextView.selectedCountry)
countryAutoCompleteTextView.countryAutocomplete.setText("Canada")
countryAutoCompleteTextView.validateCountry()
assertEquals("Canada", countryAutoCompleteTextView.selectedCountry?.name)
}

@AfterTest
fun teardown() {
Locale.setDefault(Locale.US)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class ShippingInfoWidgetTest : BaseViewTest<ShippingInfoTestActivity>(
assertEquals(phoneEditText.text.toString(), "(123) 456 - 7890")
assertEquals(postalEditText.text.toString(), "12345")
assertEquals(nameEditText.text.toString(), "Fake Name")
assertEquals(countryAutoCompleteTextView.selectedCountry.code, "US")
assertEquals(countryAutoCompleteTextView.selectedCountry?.code, "US")
}

private companion object {
Expand Down

0 comments on commit 1cca47b

Please sign in to comment.