Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove filter on postal codes when switching away from US #4385

Merged
merged 3 commits into from
Nov 17, 2021

Conversation

skyler-stripe
Copy link
Contributor

@skyler-stripe skyler-stripe commented Nov 16, 2021

Summary

This adds support for Canada style postal codes in PostalCodeEditText. Canadian postal codes are slightly different from US zip codes and it was previously impossible to input one. The rules for the regex are as follows:

* in the format A1A 1A1, where A is a letter and 1 is a digit.
* a space separates the third and fourth characters.
* do not include the letters D, F, I, O, Q or U.
* the first position does not make use of the letters W or Z.

I read online that it's better to allow users to input a dash instead of a space, or omit the space rather than enforcing it. So I did slightly break the rules.

EDIT: Michelle pointed out that this way of doing things has caused problems. I discovered the real source of the problem was that we were adding a filter of 5 characters when starting with US but not removing it when switching to other countries. Making the filter an empty list on global countries fixes this issue.

Motivation

https://jira.corp.stripe.com/browse/RUN_MOBILESDK-530

I think we should eventually add support for all country postal codes via regex, but I think this temporary band aid will get our merchant working.

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Before After
output output

@michelleb-stripe
Copy link
Contributor

@skyler-stripe We recently removed the CA postal code validation, and instead let any characters be entered as is the case for global configs. Can we just keep the validation more open for Canada, so they can center any character, number, dashes, etc, similar to other countries?

@michelleb-stripe
Copy link
Contributor

I would rather lean away from doing any postal code verification except for US. This matches iOS. We had recently removed the postal code validation for Canada for other issues that had come up.

I think the only change needed as a result of the user reported issue is in the PostalCodeEditText:

private fun configureForGlobal() {
    ...
    filters = arrayOf()
}

in order to remove the length restriction.

@skyler-stripe
Copy link
Contributor Author

I would rather lean away from doing any postal code verification except for US. This matches iOS. We had recently removed the postal code validation for Canada for other issues that had come up.

I think the only change needed as a result of the user reported issue is in the PostalCodeEditText:

private fun configureForGlobal() {
    ...
    filters = arrayOf()
}

in order to remove the length restriction.

Yeah it turns out when we we starting with US, the filter of 5 characters was being applied but not removed when switching to global. I removed the regex stuff and the filter and it works as expected.

@skyler-stripe
Copy link
Contributor Author

@skyler-stripe We recently removed the CA postal code validation, and instead let any characters be entered as is the case for global configs. Can we just keep the validation more open for Canada, so they can center any character, number, dashes, etc, similar to other countries?

I didn't realize we had validation for other countries but removed it. I updated the PR with your suggestion below.

@skyler-stripe skyler-stripe changed the title Add a case for Canada in PostalCodeEditText Remove filter on postal codes when switching away from US Nov 17, 2021
@github-actions
Copy link
Contributor

Diffuse output:

OLD: paymentsheet-example-release-master.apk (signature: none)
NEW: paymentsheet-example-release-pr.apk (signature: none)

          │         compressed          │         uncompressed          
          ├──────────┬──────────┬───────┼───────────┬───────────┬───────
 APK      │ old      │ new      │ diff  │ old       │ new       │ diff  
──────────┼──────────┼──────────┼───────┼───────────┼───────────┼───────
      dex │ 11.7 MiB │ 11.7 MiB │ +31 B │  39.8 MiB │  39.8 MiB │ +12 B 
     arsc │  1.4 MiB │  1.4 MiB │   0 B │   1.4 MiB │   1.4 MiB │   0 B 
 manifest │  2.6 KiB │  2.6 KiB │   0 B │  10.6 KiB │  10.6 KiB │   0 B 
      res │  651 KiB │  651 KiB │   0 B │     1 MiB │     1 MiB │   0 B 
    asset │ 77.8 KiB │ 77.8 KiB │  +1 B │ 109.3 KiB │ 109.3 KiB │  +1 B 
    other │   78 KiB │   78 KiB │   0 B │   154 KiB │   154 KiB │   0 B 
──────────┼──────────┼──────────┼───────┼───────────┼───────────┼───────
    total │ 13.9 MiB │ 13.9 MiB │ +32 B │  42.5 MiB │  42.5 MiB │ +13 B 


         │          raw           │           unique            
         ├────────┬────────┬──────┼────────┬────────┬───────────
 DEX     │ old    │ new    │ diff │ old    │ new    │ diff      
─────────┼────────┼────────┼──────┼────────┼────────┼───────────
   files │      3 │      3 │    0 │        │        │           
 strings │ 182291 │ 182291 │    0 │ 168439 │ 168439 │ 0 (+1 -1) 
   types │  32261 │  32261 │    0 │  30541 │  30541 │ 0 (+0 -0) 
 classes │  28215 │  28215 │    0 │  28215 │  28215 │ 0 (+0 -0) 
 methods │ 163635 │ 163635 │    0 │ 159116 │ 159116 │ 0 (+0 -0) 
  fields │ 109965 │ 109965 │    0 │ 109623 │ 109623 │ 0 (+0 -0) 


 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  287 │  287 │  0   
 entries │ 4362 │ 4362 │  0
APK
   compressed    │  uncompressed   │                               
─────────┬───────┼─────────┬───────┤                               
 size    │ diff  │ size    │ diff  │ path                          
─────────┼───────┼─────────┼───────┼───────────────────────────────
 3.4 MiB │ +31 B │ 9.9 MiB │ +12 B │ ∆ classes2.dex                
 5.6 KiB │  +1 B │ 5.4 KiB │  +1 B │ ∆ assets/dexopt/baseline.prof 
─────────┼───────┼─────────┼───────┼───────────────────────────────
 3.4 MiB │ +32 B │ 9.9 MiB │ +13 B │ (total)
DEX
STRINGS:

   old    │ new    │ diff      
  ────────┼────────┼───────────
   168439 │ 168439 │ 0 (+1 -1) 
  
  + SMAP
  PostalCodeEditText.kt
  Kotlin
  *S Kotlin
  *F
  + 1 PostalCodeEditText.kt
  com/stripe/android/view/PostalCodeEditText
  + 2 Delegates.kt
  kotlin/properties/Delegates
  + 3 TextView.kt
  androidx/core/widget/TextViewKt
  *L
  1#1,121:1
  33#2,3:122
  58#3,23:125
  93#3,3:148
  *S KotlinDebug
  *F
  + 1 PostalCodeEditText.kt
  com/stripe/android/view/PostalCodeEditText
  *L
  24#1:122,3
  48#1:125,23
  48#1:148,3
  *E
  
  
  - SMAP
  PostalCodeEditText.kt
  Kotlin
  *S Kotlin
  *F
  + 1 PostalCodeEditText.kt
  com/stripe/android/view/PostalCodeEditText
  + 2 Delegates.kt
  kotlin/properties/Delegates
  + 3 TextView.kt
  androidx/core/widget/TextViewKt
  *L
  1#1,120:1
  33#2,3:121
  58#3,23:124
  93#3,3:147
  *S KotlinDebug
  *F
  + 1 PostalCodeEditText.kt
  com/stripe/android/view/PostalCodeEditText
  *L
  24#1:121,3
  48#1:124,23
  48#1:147,3
  *E

Copy link
Contributor

@michelleb-stripe michelleb-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, thanks!

@skyler-stripe skyler-stripe merged commit 039ab18 into master Nov 17, 2021
@skyler-stripe skyler-stripe deleted the canadaPostalCodes branch November 17, 2021 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants