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

Fix issue with country dropdown not updating country #5431

Merged

Conversation

jameswoo-stripe
Copy link
Contributor

Summary

Fix an issue where the country dropdown was not updating correctly. This was broken in this previous PR

To test the change, use the payment sheet playground and play around with different customer types, default billing address enabled/disabled, and update the country dropdown to different countries. Make sure that the fields get updated, for example, US should show zip code, Canada should show postal code, other countries may not have other fields for billing address.

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

@github-actions
Copy link
Contributor

github-actions bot commented Aug 19, 2022

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 │  15.4 MiB │  15.4 MiB │ +432 B │  51.9 MiB │  51.9 MiB │ +1.8 KiB 
     arsc │   1.8 MiB │   1.8 MiB │    0 B │   1.8 MiB │   1.8 MiB │      0 B 
 manifest │     4 KiB │     4 KiB │    0 B │  18.5 KiB │  18.5 KiB │      0 B 
      res │ 873.3 KiB │ 873.3 KiB │    0 B │   1.4 MiB │   1.4 MiB │      0 B 
   native │   2.5 MiB │   2.5 MiB │    0 B │   5.9 MiB │   5.9 MiB │      0 B 
    asset │     3 MiB │     3 MiB │  -12 B │     3 MiB │     3 MiB │    -12 B 
    other │  81.7 KiB │  81.7 KiB │    0 B │ 155.6 KiB │ 155.6 KiB │      0 B 
──────────┼───────────┼───────────┼────────┼───────────┼───────────┼──────────
    total │  23.6 MiB │  23.6 MiB │ +420 B │  64.1 MiB │  64.1 MiB │ +1.8 KiB 

         │          raw           │             unique             
         ├────────┬────────┬──────┼────────┬────────┬──────────────
 DEX     │ old    │ new    │ diff │ old    │ new    │ diff         
─────────┼────────┼────────┼──────┼────────┼────────┼──────────────
   files │      4 │      4 │    0 │        │        │              
 strings │ 251014 │ 251019 │   +5 │ 214041 │ 214046 │ +5 (+20 -15) 
   types │  44292 │  44295 │   +3 │  40660 │  40663 │ +3 (+9 -6)   
 classes │  37859 │  37862 │   +3 │  37859 │  37862 │ +3 (+9 -6)   
 methods │ 221531 │ 221537 │   +6 │ 213539 │ 213545 │ +6 (+24 -18) 
  fields │ 162866 │ 162874 │   +8 │ 161826 │ 161834 │ +8 (+18 -10) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  292 │  292 │  0   
 entries │ 6232 │ 6232 │  0
APK
    compressed    │    uncompressed    │                               
─────────┬────────┼─────────┬──────────┤                               
 size    │ diff   │ size    │ diff     │ path                          
─────────┼────────┼─────────┼──────────┼───────────────────────────────
 3.2 MiB │ +432 B │ 9.1 MiB │ +1.8 KiB │ ∆ classes4.dex                
 8.7 KiB │  -12 B │ 8.5 KiB │    -12 B │ ∆ assets/dexopt/baseline.prof 
─────────┼────────┼─────────┼──────────┼───────────────────────────────
 3.2 MiB │ +420 B │ 9.1 MiB │ +1.8 KiB │ (total)
DEX
STRINGS:

   old    │ new    │ diff         
  ────────┼────────┼──────────────
   214041 │ 214046 │ +5 (+20 -15) 
  + Lcom/stripe/android/ui/core/elements/AddressElement_getFormFieldValueFlow_lambda-8__inlined_combine_1_2;
  + Lcom/stripe/android/ui/core/elements/AddressElement_getFormFieldValueFlow_lambda-8__inlined_combine_1_3;
  + Lcom/stripe/android/ui/core/elements/AddressElement_getFormFieldValueFlow_lambda-8__inlined_combine_1;
  + Lcom/stripe/android/ui/core/elements/AddressElement_getTextFieldIdentifiers_lambda-11__inlined_combine_1_2;
  + Lcom/stripe/android/ui/core/elements/AddressElement_getTextFieldIdentifiers_lambda-11__inlined_combine_1_3;
  + Lcom/stripe/android/ui/core/elements/AddressElement_getTextFieldIdentifiers_lambda-11__inlined_combine_1;
  + Lcom/stripe/android/ui/core/elements/AddressElement_special__inlined_map_2_2_1;
  + Lcom/stripe/android/ui/core/elements/AddressElement_special__inlined_map_2_2;
  + Lcom/stripe/android/ui/core/elements/AddressElement_special__inlined_map_2;
  + SMAP
  AddressElement.kt
  Kotlin
  *S Kotlin
  *F
  + 1 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement
  + 2 Maps.kt
  kotlin/collections/MapsKt__MapsKt
  + 3 _Collections.kt
  kotlin/collections/CollectionsKt___CollectionsKt
  + 4 Transform.kt
  kotlinx/coroutines/flow/FlowKt__TransformKt
  + 5 Emitters.kt
  kotlinx/coroutines/flow/FlowKt__EmittersKt
  + 6 SafeCollector.common.kt
  kotlinx/coroutines/flow/internal/SafeCollector_commonKt
  + 7 Merge.kt
  kotlinx/coroutines/flow/FlowKt__MergeKt
  *L
  1#1,169:1
  438#2:170
  388#2:171
  438#2:176
  388#2:177
  1238#3,4:172
  1238#3,4:178
  47#4:182
  49#4:186
  47#4:187
  49#4:191
  50#5:183
  55#5:185
  50#5:188
  55#5:190
  106#6:184
  106#6:189
  190#7:192
  190#7:193
  *S KotlinDebug
  *F
  + 1 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement
  *L
  59#1:170
  59#1:171
  60#1:176
  60#1:177
  59#1:172,4
  60#1:178,4
  64#1:182
  64#1:186
  71#1:187
  71#1:191
  64#1:183
  64#1:185
  71#1:188
  71#1:190
  64#1:184
  71#1:189
  136#1:192
  147#1:193
  *E
  
  + SMAP
  AddressElement.kt
  Kotlin
  *S Kotlin
  *F
  + 1 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement_fields_1
  + 2 _Collections.kt
  kotlin/collections/CollectionsKt___CollectionsKt
  *L
  1#1,169:1
  1851#2,2:170
  *S KotlinDebug
  *F
  + 1 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement_fields_1
  *L
  120#1:170,2
  *E
  
  + SMAP
  Emitters.kt
  Kotlin
  *S Kotlin
  *F
  + 1 Emitters.kt
  kotlinx/coroutines/flow/FlowKt__EmittersKt_unsafeTransform_1_1
  + 2 Transform.kt
  kotlinx/coroutines/flow/FlowKt__TransformKt
  + 3 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement
  + 4 _Collections.kt
  kotlin/collections/CollectionsKt___CollectionsKt
  *L
  1#1,222:1
  48#2:223
  72#3:224
  73#3,2:226
  75#3:229
  1851#4:225
  1852#4:228
  *S KotlinDebug
  *F
  + 1 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement
  *L
  72#1:225
  72#1:228
  *E
  
  + SMAP
  Merge.kt
  Kotlin
  *S Kotlin
  *F
  + 1 Merge.kt
  kotlinx/coroutines/flow/FlowKt__MergeKt_flatMapLatest_1
  + 2 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement
  + 3 _Collections.kt
  kotlin/collections/CollectionsKt___CollectionsKt
  + 4 Zip.kt
  kotlinx/coroutines/flow/FlowKt__ZipKt
  + 5 ArraysJVM.kt
  kotlin/collections/ArraysKt__ArraysJVMKt
  + 6 SafeCollector.common.kt
  kotlinx/coroutines/flow/internal/SafeCollector_commonKt
  *L
  1#1,215:1
  138#2,2:216
  140#2:221
  137#2:223
  1549#3:218
  1620#3,2:219
  1622#3:222
  287#4:224
  288#4:229
  37#5:225
  36#5,3:226
  106#6:230
  *S KotlinDebug
  *F
  + 1 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement
  *L
  139#1:218
  139#1:219,2
  139#1:222
  137#1:224
  137#1:229
  137#1:225
  137#1:226,3
  137#1:230
  *E
  
  + SMAP
  Merge.kt
  Kotlin
  *S Kotlin
  *F
  + 1 Merge.kt
  kotlinx/coroutines/flow/FlowKt__MergeKt_flatMapLatest_1
  + 2 AddressElement.kt
  com/stripe/android/ui/core/elements/AddressElement
  + 3 _Collections.kt
  kotlin/collections/CollectionsKt___CollectionsKt
  + 4 Zip.kt
  kotlinx/coroutines/flow/FlowKt__ZipKt
  + 5 ArraysJVM.kt
  kotlin/collections/ArraysKt__ArraysJVMKt
  + 6 SafeCollector.common.kt
  kotlinx/coroutines/flow/internal/SafeCollector_commonKt
  *L
  1#1,215:1
  149#2,2:216
  151#2:221
  148#2:223
  1549#3:218
  1620#3,2:219
  1622#3:222
  287#4:224
  288#4:229
  37#5:225
  36#5,3:22
...✂

tillh-stripe
tillh-stripe previously approved these changes Aug 22, 2022
Copy link
Collaborator

@tillh-stripe tillh-stripe left a comment

Choose a reason for hiding this comment

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

Fix looks good to me. Added two small comments.

@@ -68,6 +68,12 @@ open class AddressElement constructor(
addressRepository.get(countryCode)
?: emptyList()
}
.map { fields ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of mapping, you could use an onEach { … } here. I find it shows the side-effect-y intention better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 152 to 154
addressElement.fields.first().map {
it.getFormFieldValueFlow().first()[0].second.value
}.first()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you end up just using the first element from the mapped list, you could avoid mapping the entire list and just access the first element directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@jameswoo-stripe jameswoo-stripe enabled auto-merge (squash) August 22, 2022 17:05
@jameswoo-stripe jameswoo-stripe merged commit 1579321 into master Aug 22, 2022
@jameswoo-stripe jameswoo-stripe deleted the jameswoo/shipping-address-element-fix-country-dropdown branch August 22, 2022 17:10
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.

None yet

3 participants