Fix bug in `PaymentSheet` where newly save payment method is not set as default payment method. #7496

samer-stripe · 2023-10-25T02:22:40Z

Summary

Fix bug in PaymentSheet where newly save payment method is not set as default payment method.

Motivation

RUN_MOBILESDK-2667

Makes sure that newly added payment methods are selected by default.

Testing

Added tests
Modified tests
Manually verified

samer-stripe · 2023-10-25T02:24:52Z

paymentsheet/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt

+            is PaymentSheet.InitializationMode.DeferredIntent -> {
+                initializationMode.intentConfiguration.mode.setupFutureUse != null || requestedToSave
+            }
+        }


Marking this as especially important to review. Decides if the newly added card should be saved as the default selection.

@tillh-stripe can you also take a look (especially here).

github-actions · 2023-10-25T02:35:21Z

Diffuse output:

OLD: paymentsheet-example-release-master.apk (signature: V1, V2)
NEW: paymentsheet-example-release-pr.apk (signature: V1, V2)

          │           compressed           │           uncompressed           
          ├───────────┬───────────┬────────┼───────────┬───────────┬──────────
 APK      │ old       │ new       │ diff   │ old       │ new       │ diff     
──────────┼───────────┼───────────┼────────┼───────────┼───────────┼──────────
      dex │   3.6 MiB │   3.6 MiB │ -759 B │   7.8 MiB │   7.8 MiB │ +3.8 KiB 
     arsc │   2.3 MiB │   2.3 MiB │    0 B │   2.3 MiB │   2.3 MiB │      0 B 
 manifest │   5.1 KiB │   5.1 KiB │    0 B │  25.4 KiB │  25.4 KiB │      0 B 
      res │ 905.5 KiB │ 905.5 KiB │    0 B │   1.4 MiB │   1.4 MiB │      0 B 
   native │   2.6 MiB │   2.6 MiB │    0 B │     6 MiB │     6 MiB │      0 B 
    asset │     3 MiB │     3 MiB │  +33 B │     3 MiB │     3 MiB │    +33 B 
    other │ 204.5 KiB │ 204.5 KiB │   +2 B │ 460.3 KiB │ 460.3 KiB │      0 B 
──────────┼───────────┼───────────┼────────┼───────────┼───────────┼──────────
    total │  12.5 MiB │  12.5 MiB │ -724 B │  20.8 MiB │  20.8 MiB │ +3.9 KiB 

 DEX     │ old   │ new   │ diff              
─────────┼───────┼───────┼───────────────────
   files │     1 │     1 │   0               
 strings │ 38483 │ 38527 │ +44 (+146 -102)   
   types │ 12659 │ 12669 │ +10 (+108 -98)    
 classes │ 10645 │ 10652 │  +7 (+66 -59)     
 methods │ 55450 │ 55476 │ +26 (+3487 -3461) 
  fields │ 35656 │ 35667 │ +11 (+2201 -2190) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  292 │  292 │  0   
 entries │ 6936 │ 6936 │  0

APK

    compressed     │     uncompressed     │                                
──────────┬────────┼───────────┬──────────┤                                
 size     │ diff   │ size      │ diff     │ path                           
──────────┼────────┼───────────┼──────────┼────────────────────────────────
  3.6 MiB │ -759 B │   7.8 MiB │ +3.8 KiB │ ∆ classes.dex                  
  6.4 KiB │  +44 B │   6.3 KiB │    +44 B │ ∆ assets/dexopt/baseline.prof  
    762 B │  -11 B │     630 B │    -11 B │ ∆ assets/dexopt/baseline.profm 
 49.9 KiB │   +4 B │ 147.1 KiB │      0 B │ ∆ META-INF/MANIFEST.MF         
 65.6 KiB │   -3 B │ 147.1 KiB │      0 B │ ∆ META-INF/CERT.SF             
  1.2 KiB │   +1 B │   1.2 KiB │      0 B │ ∆ META-INF/CERT.RSA            
──────────┼────────┼───────────┼──────────┼────────────────────────────────
  3.7 MiB │ -724 B │   8.1 MiB │ +3.9 KiB │ (total)

DEX

STRINGS:

   old   │ new   │ diff            
  ───────┼───────┼─────────────────
   38483 │ 38527 │ +44 (+146 -102) 
  + Completed(intent=
  + E2
  + F2
  + Failed to get PaymentSheetResult from Intent
  + G2
  + H2
  + I2
  + J2
  + K2
  + L2
  + Ldj/b;
  + Le1/m8;
  + Lej/a;
  + Lej/b;
  + Lej/c;
  + Lej/d;
  + Lfj/c;
  + Lfj/d;
  + Lfj/e;
  + Lfj/f;
  + Lfj/g;
  + Lfj/h;
  + Lfj/i;
  + Lfj/j;
  + Lfj/k;
  + Lfj/l;
  + Lfj/m;
  + Lfj/n;
  + Lfj/o;
  + Lfj/p;
  + Lfj/q;
  + Lfj/r;
  + Lfj/s;
  + Lfj/t;
  + Lfj/u;
  + Lfj/v;
  + Lfj/w;
  + Lfj/x;
  + Lij/b;
  + Lij/c;
  + Lij/d;
  + Lmd/a0;
  + Lmd/b0;
  + Lmd/c0;
  + Lmd/z;
  + Lnd/a4;
  + Lnd/b4;
  + Lne/h0;
  + Lnj/c;
  + Lnj/d;
  + Lod/q0;
  + Lpj/e;
  + Lpj/f;
  + Lqj/c;
  + Lqj/d;
  + Lqj/e;
  + Lqj/f;
  + Lri/a;
  + Lsg/e4;
  + Lta/u2;
  + Lvi/b;
  + Lvi/c;
  + Lxi/a;
  + Lxi/b;
  + Lxi/c;
  + Lxi/d;
  + Lxi/e;
  + Lxi/f;
  + Lxi/g;
  + Lxi/h;
  + Lxi/i;
  + Lxi/j;
  + Lxi/k;
  + Lxi/l;
  + Lxi/m;
  + Lxi/n;
  + M2
  + N2
  + O2
  + P2
  + PaymentSheetSavedPaymentOption
  + Q2
  + R2
  + S2
  + T2
  + U2
  + V2
  + W2
  + X2
  + Y2
  + Z2
  + [La0/p;
  + [Lcj/a;
  + [Le1/c8;
  + [Lfj/s;
  + [Lhf/h;
  + [Lmd/a;
  + [Lmd/b;
  + [Lmd/c;
  + [Lmd/h;
  + [Lmd/i;
  + [Lmd/j;
  + [Lmd/x;
  + [Lmd/y;
  + [Lmd/z;
  + [Lmf/c0;
  + [Lmf/z;
  + [Lnd/a2;
  + [Lnd/c3;
  + [Lnd/g2;
  + [Lnd/h3;
  + [Lnd/j2;
  + [Lnd/n1;
  + [Lnd/p2;
  + [Lnd/q0;
  + [Lnd/s2;
  + [Lnd/u0;
  + [Lnd/x1;
  + [Lne/f0;
  + [Lne/h0;
  + [Lpj/e;
  + [Lqe/k;
  + [Lsg/c4;
  + [Lsg/d3;
  + [Lsg/o3;
  + [Lsg/q3;
  + [Lta/g2;
  + [Ltj/a;
  + [Lwe/f0;
  + [Lwe/q1;
  + [Lwe/s1;
  + [Lwe/u;
  + [Lyi/h;
  + a3
  + activityResultCaller.reg…LauncherResult)
          )
  + b3
  + c3
  + d3
  + e3
  + f3
  + g3
  + i3
  + j3
  + onInternalPaymentResult
  + onInternalPaymentResult(Lcom/stripe/android/payments/paymentlauncher/InternalPaymentResult;)V
  + ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:2d01024,r8-mode:full,version:8.1.65}
  
  - Failed to get PaymentResult from Intent
  - Laj/c;
  - Laj/d;
  - Laj/e;
  - Lcj/b;
  - Lgj/b;
  - Lgj/c;
  - Lgj/d;
  - Lhj/d;
  - Lhj/e;
  - Lhj/f;
  - Lhj/g;
  - Lhj/h;
  - Lhj/i;
  - Lhj/j;
  - Lhj/k;
  - Lhj/l;
  - Lhj/m;
  - Lhj/n;
  - Lhj/o;
  - Lhj/p;
  - Lhj/q;
  - Lhj/r;
  - Lhj/s;
  - Lhj/t;
  - Lhj/u;
  - Lhj/v;
  - Lhj/w;
  - Lhj/x;
  - Ljj/c;
  - Lkc/r;
  - Lkd/f;
  - Lkj/c;
  - Lkj/d;
  - Lpd/f;
  - Lrj/c;
  - Lrj/d;
  - Lrj/e;
  - Lrj/f;
  - Lsj/a;
  - Lsj/b;
  - Lsj/c;
  - Lsj/d;
  - Lsj/e;
  - Lsj/f;
  - Lvj/a;
  - Lvj/b;
  - Lwe/v1;
  - Lwi/a;
  - Lwi/b;
  - Lwi/c;
  - Lyi/m;
  - Lyi/n;
  - Lzi/f;
  - Lzi/g;
  - Lzi/h;
  - Lzi/i;
  - Lzi/j;
  - Lzi/k;
  - Lzi/l;
  - [Ldj/a;
  - [Le1/b8;
  - [Lhf/e;
  - [Lhj/s;
  - [Lmd/d;
  - [Lmd/e;
  - [Lmd/f;
  - [Lmd/t;
  - [Lmd/u;
  - [Lmd/v;
  - [Lmf/a0;
  - [Lmf/y;
  - [Lnd/a3;
  - [Lnd/c2;
  - [Lnd/e3;
  - [Lnd/h2;
  - [Lnd/k2;
  - [Lnd/l1;
  - [Lnd/n2;
  - [Lnd/p0;
  - [Lnd/q1;
  - [Lnd/r0;
  - [Lnd/y1;
  - [Lne/e0;
  - [Lne/g0;
  - [Lqe/o;
  - [Lrj/e;
  - [Lsg/b3;
  - [Lsg/b4;
  - [Lsg/n3;
  - [Lsg/p3;
  - [Lta/d2;
  - [Lvj/a;
  - [Lwe/g0;
  - [Lwe/r1;
  - [Lwe/u1;
  - [Lwe/v;
  - [Lx/e;
  - [Lzi/h;
  - activityResultCaller.reg…tLauncherResult
          )
  - onPaymentResult(Lcom/stripe/android/payments/paymentlauncher/PaymentResult;)V
  - ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:10c4e71,r8-mode:full,version:8.1.65}
  

TYPES:

   old   │ new   │ diff           
  ───────┼───────┼────────────────
   12659 │ 12669 │ +10 (+108 -98) 
  + Ldj/b;
  + Le1/m8;
  + Lej/a;
  + Lej/b;
  + Lej/c;
  + Lej/d;
  + Lfj/c;
  + Lfj/d;
  + Lfj/e;
  + Lfj/f;
  + Lfj/g;
  + Lfj/h;
  + Lfj/i;
  + Lfj/j;
  + Lfj/k;
  + Lfj/l;
  + Lfj/m;
  + Lfj/n;
  + Lfj/o;
  + Lfj/p;
  + Lfj/q;
  + Lfj/r;
  + Lfj/s;
  + Lfj/t;
  + Lfj/u;
  + Lfj/v;
  + Lfj/w;
  + Lfj/x;
  + Lij/b;
  + Lij/c;
  + Lij/d;
  + Lmd/a0;
  + Lmd/b0;
  + Lmd/c0;
  + Lmd/z;
  + Lnd/a4;
  + Lnd/b4;
  + Lne/h0;
  + Lnj/c;
  + Lnj/d;
  + Lod/q0;
  + Lpj/e;
  + Lpj/f;
  + Lqj/c;
  + Lqj/d;
  + Lqj/e;
  + Lqj/f;
  + Lri/a;
  + Lsg/e4;
  + Lta/u2;
  + Lvi/b;
  + Lvi/c;
  + Lxi/a;
  + Lxi/b;
  + Lxi/c;
  + Lxi/d;
  + Lxi/e;
  + Lxi/f;
  + Lxi/g;
  + Lxi/h;
  + Lxi/i;
  + Lxi/j;
  + Lxi/k;
  + Lxi/l;
  + Lxi/m;
  + Lxi/n;
  + [La0/p;
  + [Lcj/a;
  + [Le1/c8;
  + [Lfj/s;
  + [Lhf/h;
  + [Lmd/a;
  + [Lmd/b;
  + [Lmd/c;
  + [Lmd/h;
  + [Lmd/i;
  + [Lmd/j;
  + [
...✂

jaynewstrom-stripe

This is super well done, and is a huge win. Thanks for doing this @samer-stripe!

...ents-core/src/main/java/com/stripe/android/payments/paymentlauncher/PaymentLauncherResult.kt

paymentsheet/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt

jaynewstrom-stripe · 2023-10-25T14:31:08Z

paymentsheet/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt

+            is PaymentSheet.InitializationMode.DeferredIntent -> {
+                initializationMode.intentConfiguration.mode.setupFutureUse != null || requestedToSave
+            }
+        }


@tillh-stripe can you also take a look (especially here).

paymentsheet/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt

jaynewstrom-stripe

Chatted offline, I think it makes sense to add an E2E test or two. But overall looks great.

payments-core/src/main/java/com/stripe/android/payments/paymentlauncher/PaymentLauncher.kt

tillh-stripe · 2023-10-26T21:45:13Z

paymentsheet/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt

@@ -531,6 +533,32 @@ internal class PaymentSheetViewModel @Inject internal constructor(
        }
    }

+    private fun onPaymentLauncherResult(launcherResult: PaymentLauncherResult) {


With onPaymentResult now only being used by Link, should we switch it over too (and does that even give us any benefit)?

From what I can tell, Link only produces the payment method that was used by not a stripe intent. If we can pull the intent, then we can make it use PaymentLauncherResult. Otherwise, there are a few things we can do.

We could limit PaymentLauncherResult to just return the payment method then have Link use that result inside of PaymentResult.

Have Link return its own response type with the payment method and remove onPaymentResult.

Either way, there isn't any huge benefit to doing it since Link methods aren't saved. onPaymentResult is a BaseSheetViewModel method that is used in other places so we'd have to remove them there as well.

…as default payment method.

samer-stripe · 2023-11-10T05:16:54Z

paymentsheet-example/src/androidTest/java/com/stripe/android/lpm/TestCard.kt

+
+    /*
+     * TODO(samer-stripe): Once we update `PaymentResult` to return a `StripeIntent`, update the test
+     *  to check against the payment method IDs rather than the last 4 digits.


Highlighting this comment.

paymentsheet-example/src/androidTest/java/com/stripe/android/lpm/TestCard.kt

paymentsheet/src/main/java/com/stripe/android/paymentsheet/utils/SelectionUtils.kt

jaynewstrom-stripe

LGTM, I wouldn't mind a second set of eyes on the two places I called out. But not blocking.

paymentsheet/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt

paymentsheet/src/main/java/com/stripe/android/paymentsheet/utils/SelectionUtils.kt

samer-stripe requested review from a team as code owners October 25, 2023 02:22

samer-stripe requested review from awush-stripe and removed request for awush-stripe October 25, 2023 02:22

samer-stripe marked this pull request as draft October 25, 2023 02:23

samer-stripe commented Oct 25, 2023

View reviewed changes

jaynewstrom-stripe reviewed Oct 25, 2023

View reviewed changes

samer-stripe marked this pull request as ready for review October 25, 2023 15:26

samer-stripe force-pushed the samer/fix-default-payment-method-bug branch from 09b5c75 to 603dcb6 Compare October 25, 2023 22:55

samer-stripe requested a review from jaynewstrom-stripe October 25, 2023 22:55

jaynewstrom-stripe reviewed Oct 26, 2023

View reviewed changes

tillh-stripe reviewed Oct 26, 2023

View reviewed changes

payments-core/src/main/java/com/stripe/android/payments/paymentlauncher/PaymentLauncher.kt Outdated Show resolved Hide resolved

tillh-stripe reviewed Oct 26, 2023

View reviewed changes

samer-stripe force-pushed the samer/fix-default-payment-method-bug branch from 603dcb6 to a5b69c6 Compare October 30, 2023 14:50

samer-stripe added 3 commits November 9, 2023 17:01

Fix bug in PaymentSheet where newly save payment method is not set …

47e5ff7

…as default payment method.

Add improvements for PaymentSheet selection bug.

f3030e0

Add additional improvements for PaymentSheet selection bug.

16a7a3a

samer-stripe force-pushed the samer/fix-default-payment-method-bug branch from a5b69c6 to 06518f1 Compare November 9, 2023 22:01

samer-stripe requested review from tillh-stripe and jaynewstrom-stripe November 9, 2023 22:09

Add end-to-end tests and ktlint fixes.

eb30fb4

samer-stripe force-pushed the samer/fix-default-payment-method-bug branch from 06518f1 to eb30fb4 Compare November 10, 2023 05:16

samer-stripe commented Nov 10, 2023

View reviewed changes

jaynewstrom-stripe reviewed Nov 10, 2023

View reviewed changes

Add testing fixes for payment method default bug

c84d2ff

samer-stripe requested a review from jaynewstrom-stripe November 10, 2023 15:23

jaynewstrom-stripe approved these changes Nov 10, 2023

View reviewed changes

paymentsheet/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt Show resolved Hide resolved

paymentsheet/src/main/java/com/stripe/android/paymentsheet/utils/SelectionUtils.kt Show resolved Hide resolved

tillh-stripe approved these changes Nov 10, 2023

View reviewed changes

samer-stripe merged commit 5e8edad into master Nov 10, 2023
10 checks passed

samer-stripe deleted the samer/fix-default-payment-method-bug branch November 10, 2023 18:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bug in `PaymentSheet` where newly save payment method is not set as default payment method. #7496

Fix bug in `PaymentSheet` where newly save payment method is not set as default payment method. #7496

samer-stripe commented Oct 25, 2023 •

edited

Loading

samer-stripe Oct 25, 2023

jaynewstrom-stripe Oct 25, 2023

github-actions bot commented Oct 25, 2023 •

edited

Loading

jaynewstrom-stripe left a comment

jaynewstrom-stripe Oct 25, 2023

jaynewstrom-stripe left a comment

tillh-stripe Oct 26, 2023

samer-stripe Oct 26, 2023 •

edited

Loading

samer-stripe Nov 10, 2023

jaynewstrom-stripe left a comment

Fix bug in PaymentSheet where newly save payment method is not set as default payment method. #7496

Fix bug in PaymentSheet where newly save payment method is not set as default payment method. #7496

Conversation

samer-stripe commented Oct 25, 2023 • edited Loading

Summary

Motivation

Testing

samer-stripe Oct 25, 2023

Choose a reason for hiding this comment

jaynewstrom-stripe Oct 25, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 25, 2023 • edited Loading

jaynewstrom-stripe left a comment

Choose a reason for hiding this comment

jaynewstrom-stripe Oct 25, 2023

Choose a reason for hiding this comment

jaynewstrom-stripe left a comment

Choose a reason for hiding this comment

tillh-stripe Oct 26, 2023

Choose a reason for hiding this comment

samer-stripe Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

samer-stripe Nov 10, 2023

Choose a reason for hiding this comment

jaynewstrom-stripe left a comment

Choose a reason for hiding this comment

Fix bug in `PaymentSheet` where newly save payment method is not set as default payment method. #7496

Fix bug in `PaymentSheet` where newly save payment method is not set as default payment method. #7496

samer-stripe commented Oct 25, 2023 •

edited

Loading

github-actions bot commented Oct 25, 2023 •

edited

Loading

samer-stripe Oct 26, 2023 •

edited

Loading