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

Update CVC Recollection API for Deferred Intent flow #9158

Merged
merged 19 commits into from
Sep 9, 2024

Conversation

tjclawson-stripe
Copy link
Collaborator

Summary

Update CVC Recollection API

Motivation

API Review
Integration Guide

Testing

  • Added tests
  • Modified tests
  • Manually verified

Changelog

Copy link
Contributor

github-actions bot commented Aug 29, 2024

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │   1.9 MiB │   1.9 MiB │  0 B │     4 MiB │     4 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 301.5 KiB │ 301.5 KiB │  0 B │   455 KiB │   455 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   6.8 KiB │   6.8 KiB │  0 B │   6.6 KiB │   6.6 KiB │  0 B 
    other │  85.5 KiB │  85.5 KiB │ +5 B │ 158.7 KiB │ 158.7 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.5 MiB │   9.5 MiB │ +5 B │  21.4 MiB │  21.4 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 20269 │ 20269 │ 0 (+0 -0) 
   types │  6106 │  6106 │ 0 (+0 -0) 
 classes │  4913 │  4913 │ 0 (+0 -0) 
 methods │ 29561 │ 29561 │ 0 (+0 -0) 
  fields │ 17371 │ 17371 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3608 │ 3608 │  0
APK
   compressed    │   uncompressed   │                        
──────────┬──────┼───────────┬──────┤                        
 size     │ diff │ size      │ diff │ path                   
──────────┼──────┼───────────┼──────┼────────────────────────
 28.3 KiB │ +7 B │  62.6 KiB │  0 B │ ∆ META-INF/CERT.SF     
  1.2 KiB │ -1 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA    
 25.1 KiB │ -1 B │  62.5 KiB │  0 B │ ∆ META-INF/MANIFEST.MF 
──────────┼──────┼───────────┼──────┼────────────────────────
 54.6 KiB │ +5 B │ 126.3 KiB │  0 B │ (total)

Copy link
Collaborator

@jaynewstrom-stripe jaynewstrom-stripe left a comment

Choose a reason for hiding this comment

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

Can you add a note to the changelog?

CHANGELOG.md Outdated
@@ -2,6 +2,8 @@

## XX.XX.XX - 20XX-XX-XX

*[ADDED][9158](https://github.com/stripe/stripe-android/pull/9158) Add `requireCvcRecollection` param to `IntentConfiguration`
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should mention this is still in private beta, it's a breaking API change, and how to migrate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, we have the migration docs ready but they're not deployed. Should I remove the changelog addition and add once we're ready to GA?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to mention the breaking change (removing the callback). Ideally we point to the migration guide as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added verbiage about breaking change and private beta. Will update to point to migration guide if we have approval by the next release

Copy link

emerge-tools bot commented Sep 5, 2024

3 builds increased size

Name Version Download Change Install Change Approval
Stripe Identity Example
com.stripe.android.identity.example.theme1
20.48.6-theme1 (20) 3.8 MB ⬆️ 2.7 kB (0.07%) 8.9 MB ⬆️ 9.0 kB (0.1%) N/A
⚠️ Financial Connections Example
com.stripe.android.financialconnections.example
20.48.6 (204806) 4.2 MB ⬆️ 650.7 kB (18.39%) 9.3 MB ⬆️ 1.5 MB (19.34%) N/A
PaymentSheet Example
com.stripe.android.paymentsheet.example
20.48.6 (11) 8.5 MB ⬆️ 11.2 kB (0.13%) 16.0 MB ⬆️ 28.7 kB (0.18%) N/A

Stripe Identity Example 20.48.6-theme1 (20)
com.stripe.android.identity.example.theme1

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 9.0 kB (0.1%)
Total download size change: ⬆️ 2.7 kB (0.07%)

Largest size changes

Item Install Size Change Download Size Change
🗑 androidx.compose.ui.text.android.selection.WordIterator$Companion ⬇️ -38.2 kB ⬇️ -18.3 kB
📝 androidx.fragment.app.FragmentAnim$AnimationOrAnimator ⬆️ 33.1 kB ⬆️ 15.9 kB
🗑 androidx.emoji2.viewsintegration.EmojiEditTextHelper$HelperIntern... ⬇️ -33.1 kB ⬇️ -15.9 kB
androidx.transition.ViewUtilsApi21 ⬆️ 32.9 kB ⬆️ 15.8 kB
📝 androidx.activity.compose.BackHandlerKt ⬆️ 28.6 kB ⬆️ 13.7 kB
View Treemap

Image of diff

Financial Connections Example 20.48.6 (204806)
com.stripe.android.financialconnections.example

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 1.5 MB (19.34%)
Total download size change: ⬆️ 650.7 kB (18.39%)

Largest size changes

Item Install Size Change Download Size Change
kotlin.sequences.SequencesKt__SequencesJVMKt ⬆️ 68.8 kB ⬆️ 32.5 kB
📝 androidx.fragment.app.SpecialEffectsController$Operation$State$Co... ⬆️ 65.3 kB ⬆️ 30.8 kB
androidx.work.impl.model.RawWorkInfoDao_Impl ⬆️ 53.9 kB ⬆️ 25.3 kB
📝 com.google.android.gms.dynamite.zzb ⬆️ 50.7 kB ⬆️ 23.9 kB
kotlin.collections.MapsKt__MapsKt ⬇️ -46.7 kB ⬇️ -22.3 kB
View Treemap

Image of diff

PaymentSheet Example 20.48.6 (11)
com.stripe.android.paymentsheet.example

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 28.7 kB (0.18%)
Total download size change: ⬆️ 11.2 kB (0.13%)

Largest size changes

Item Install Size Change Download Size Change
kotlin.collections.SetsKt ⬆️ 36.6 kB ⬆️ 16.7 kB
kotlin.collections.MapsKt__MapsKt ⬇️ -35.5 kB ⬇️ -16.2 kB
🗑 androidx.emoji2.text.MetadataListReader ⬇️ -28.7 kB ⬇️ -13.1 kB
📝 androidx.compose.foundation.layout.IntrinsicKt ⬆️ 25.8 kB ⬆️ 11.8 kB
📝 com.google.android.material.textfield.IconHelper ⬆️ 23.0 kB ⬆️ 10.5 kB
View Treemap

Image of diff


🛸 Powered by Emerge Tools

Comment trigger: Size diff threshold of 100.00kB exceeded

CHANGELOG.md Outdated
@@ -2,7 +2,7 @@

## XX.XX.XX - 20XX-XX-XX

*[ADDED][9158](https://github.com/stripe/stripe-android/pull/9158) Add `requireCvcRecollection` param to `IntentConfiguration`
*[ADDED][9158](https://github.com/stripe/stripe-android/pull/9158) Add `requireCvcRecollection` param to `IntentConfiguration`. Currently in private beta. This is a breaking change for merchants using `PaymentSheet.Builder.cvcRecollectionEnabledCallback` and `PaymentSheet.FlowController.Builder.cvcRecollectionEnabledCallback`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could reword this to be a little more clear.

A few things:

  • Let's update the tag from [ADDED] -> [BREAKING]
  • Let's start with something like "Updates to CVC recollection APIs, currently in private beta", then list the changes
  • Let's also mention (probably at the end) this is still in private beta, and requires the experimental API annotation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated 👍

@@ -328,7 +328,8 @@ class PaymentSheetEventTest {
"duration" to 5f,
"selected_lpm" to "none",
"intent_type" to "payment_intent",
"ordered_lpms" to "card,klarna"
"ordered_lpms" to "card,klarna",
"require_cvc_recollection" to false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please separate analytics into another PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted, will add in follow up PR

@tjclawson-stripe tjclawson-stripe merged commit 8821b19 into master Sep 9, 2024
13 checks passed
@tjclawson-stripe tjclawson-stripe deleted the cvc-api-update branch September 9, 2024 18:26
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.

2 participants