Integrate Sunbit on PaymentSheet #8595

egeniesse-stripe · 2024-06-06T17:16:21Z

Summary

Adding the Sunbit payment method to Payment Element and StripeJS. For information about Sunbit and why we're integrating it, check out its product brief and API review doc.

Motivation

Testing

Added tests
Manually verified

Screenshots

Screen.Recording.2024-06-06.at.8.06.39.PM.mov

Changelog

github-actions · 2024-06-06T17:22:43Z

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 │     4 MiB │     4 MiB │   +101 B │   8.7 MiB │  8.7 MiB │    +784 B 
     arsc │   2.2 MiB │   2.2 MiB │   +892 B │   2.2 MiB │  2.2 MiB │    +892 B 
 manifest │   5.1 KiB │   5.1 KiB │      0 B │  25.8 KiB │ 25.8 KiB │       0 B 
      res │ 910.1 KiB │ 913.8 KiB │ +3.7 KiB │   1.4 MiB │  1.4 MiB │  +9.8 KiB 
   native │   2.6 MiB │   2.6 MiB │      0 B │     6 MiB │    6 MiB │       0 B 
    asset │   2.9 MiB │   2.9 MiB │ +1.4 KiB │   2.9 MiB │  2.9 MiB │  +1.4 KiB 
    other │ 194.3 KiB │ 194.6 KiB │   +277 B │ 424.7 KiB │  426 KiB │  +1.3 KiB 
──────────┼───────────┼───────────┼──────────┼───────────┼──────────┼───────────
    total │  12.8 MiB │  12.8 MiB │ +6.3 KiB │  21.7 MiB │ 21.7 MiB │ +14.2 KiB 

 DEX     │ old   │ new   │ diff         
─────────┼───────┼───────┼──────────────
   files │     1 │     1 │   0          
 strings │ 43406 │ 43410 │  +4 (+5 -1)  
   types │ 14908 │ 14910 │  +2 (+2 -0)  
 classes │ 12598 │ 12600 │  +2 (+2 -0)  
 methods │ 61656 │ 61668 │ +12 (+20 -8) 
  fields │ 40643 │ 40647 │  +4 (+7 -3)  

 ARSC    │ old  │ new  │ diff       
─────────┼──────┼──────┼────────────
 configs │  242 │  242 │  0         
 entries │ 6033 │ 6035 │ +2 (+2 -0)

APK

     compressed      │     uncompressed      │                                
──────────┬──────────┼───────────┬───────────┤                                
 size     │ diff     │ size      │ diff      │ path                           
──────────┼──────────┼───────────┼───────────┼────────────────────────────────
  1.9 KiB │ +1.9 KiB │   5.2 KiB │  +5.2 KiB │ + res/8K.xml                   
  1.8 KiB │ +1.8 KiB │   4.7 KiB │  +4.7 KiB │ + res/M4.xml                   
  7.4 KiB │ +1.4 KiB │   7.2 KiB │  +1.4 KiB │ ∆ assets/dexopt/baseline.prof  
  2.2 MiB │   +892 B │   2.2 MiB │    +892 B │ ∆ resources.arsc               
  3.3 KiB │   +136 B │  26.6 KiB │    +1 KiB │ ∆ lpms.json                    
    4 MiB │   +101 B │   8.7 MiB │    +784 B │ ∆ classes.dex                  
 52.8 KiB │    +74 B │   117 KiB │    +164 B │ ∆ META-INF/CERT.SF             
 49.6 KiB │    +65 B │ 116.9 KiB │    +164 B │ ∆ META-INF/MANIFEST.MF         
  1.2 KiB │     +2 B │   1.2 KiB │       0 B │ ∆ META-INF/CERT.RSA            
    869 B │     +1 B │     737 B │      +1 B │ ∆ assets/dexopt/baseline.profm 
    848 B │     -1 B │   1.8 KiB │       0 B │ ∆ res/8Q.xml                   
    598 B │     -1 B │   1.1 KiB │       0 B │ ∆ res/JB.xml                   
──────────┼──────────┼───────────┼───────────┼────────────────────────────────
  6.3 MiB │ +6.3 KiB │  11.2 MiB │ +14.2 KiB │ (total)

DEX

STRINGS:

   old   │ new   │ diff       
  ───────┼───────┼────────────
   43406 │ 43410 │ +4 (+5 -1) 
  
  + Lm6/s0;
  + Lm6/t0;
  + Sunbit
  + sunbit
  + ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:2a8fc2b,r8-mode:full,version:8.3.37}
  
  - ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:00f25d8,r8-mode:full,version:8.3.37}
  

TYPES:

   old   │ new   │ diff       
  ───────┼───────┼────────────
   14908 │ 14910 │ +2 (+2 -0) 
  
  + Lm6/s0;
  + Lm6/t0;
  

METHODS:

   old   │ new   │ diff         
  ───────┼───────┼──────────────
   61656 │ 61668 │ +12 (+20 -8) 
  
  + m6.h0 e(j, Q1, j) → ArrayList
  + m6.h0 g(Q1) → i
  + m6.j0 c(j, n) → List
  + m6.j0 f() → i
  + m6.l0 e(j, Q1, j) → ArrayList
  + m6.l0 g(Q1) → i
  + m6.p0 c(j, n) → List
  + m6.p0 f() → i
  + m6.s0 <clinit>()
  + m6.s0 a() → S0
  + m6.s0 b(boolean) → Set
  + m6.s0 c(j) → boolean
  + m6.s0 d() → boolean
  + m6.s0 e() → q
  + m6.t0 <clinit>()
  + m6.t0 a(i, List) → i
  + m6.t0 b(i, j, List, n) → List
  + m6.t0 d(i, List) → boolean
  + m6.t0 e(j, Q1, j) → ArrayList
  + m6.t0 g(Q1) → i
  
  - m6.h0 c(j, n) → List
  - m6.h0 f() → i
  - m6.j0 e(j, Q1, j) → ArrayList
  - m6.j0 g(Q1) → i
  - m6.l0 c(j, n) → List
  - m6.l0 f() → i
  - m6.p0 e(j, Q1, j) → ArrayList
  - m6.p0 g(Q1) → i
  

FIELDS:

   old   │ new   │ diff       
  ───────┼───────┼────────────
   40643 │ 40647 │ +4 (+7 -3) 
  
  + m6.o0 c: boolean
  + m6.s0 a: s0
  + m6.s0 b: S0
  + m6.t0 a: t0
  + p6.S0 I0: S0
  + p6.S0 J0: S0[]
  + p6.S0 K0: b
  
  - m6.m0 c: boolean
  - p6.S0 I0: S0[]
  - p6.S0 J0: b

ARSC

ENTRIES:

   old  │ new  │ diff       
  ──────┼──────┼────────────
   6033 │ 6035 │ +2 (+2 -0) 
  + drawable/stripe_ic_paymentsheet_pm_sunbit
  + string/stripe_paymentsheet_payment_method_sunbit

amk-stripe · 2024-06-06T22:24:42Z

The Bitrise run on this PR failed, some tips to get it to pass:

for check: run ./gradlew ktlint detekt lintRelease apiCheck verifyReleaseResources locally and fix the errors (might need to run it, fix, run it, fix, etc until it passes)
for e2e tests: I think you need to gate sunbit into our test merchant to get these to pass, ping me and we can figure out how to do this?

amk-stripe · 2024-06-06T22:25:14Z

CHANGELOG.md

@@ -2,6 +2,12 @@

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

+### PaymentSheet
+* [ADDED][8595](https://github.com/stripe/stripe-android/pull/8595) PaymentSheet now supports Sunbit for PaymentIntents in private beta.


can/should we link to info about the beta in case merchants are interested in joining? Not sure if that makes sense for private beta or not

Also a nit, but can you match the wording we used for Multiblanco in the changelog for consistency?

Good question. Generally, we're the ones who reach out to users for Private betas, but if someone was excited to use it and wrote into Stripe, we could add them (assuming they met our criteria).

I removed the private beta wording to follow Multibanco since our release cycle isn't super relevant to this change.

amk-stripe · 2024-06-06T22:33:12Z

payments-core/src/main/java/com/stripe/android/model/PaymentMethodCreateParams.kt

@@ -1098,6 +1098,23 @@ data class PaymentMethodCreateParams internal constructor(
            )
        }

+                /**
+         * Helper method to create [PaymentMethodCreateParams] with [PaymentMethod.Type.Sunbit] as the payment


do you know why this is showing the fully qualified type name here (PaymentMethod.Type.Sunbit)? Can it show Sunbit instead?

At the time of this comment, we didn't have a Sunbit method. I added it in here (and piped it through in a few places), but I'm not sure if we get much value out of it.

Here's the commit, let me know if you think we should take this approach for the other payment methods.

Hm agreed that it doesn't seem like we get much value out of it and it looks like we didn't do this for Multibanco or Amazon Pay, so I don't think we need it here either or for the other payment methods. Sorry for the churn!

amk-stripe · 2024-06-06T22:35:27Z

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

+        settings[CurrencySettingsDefinition] = Currency.USD
+    }
+
+    @Ignore("Complex authorization handling required")


Does this need to be ignored? Using this annotation makes it so the test doesn't run

This is safe to enable! Just removed this line

amk-stripe · 2024-06-06T22:59:01Z

also can you update the PR description on this when you get a chance?

amk-stripe · 2024-06-07T16:31:28Z

....stripe.android.screenshot.TestPaymentSheetScreenshots_testDefaultBillingAndPhoneDetails.png

Rather than updating this screenshot, can we update the test so that Sunbit isn't shown? You should be able to do this by updating the settings here by setting the AutomaticPaymentMethodsSettingsDefinition to false and SupportedPaymentMethodsSettingsDefinition to card, amazon_pay, klarna.

Motivation for this is:

the point of this test is testing the payment sheet appearance in general, not LPMs, so Sunbit isn't needed here

I expect these tests might need updating on each LPM PR you did

most importantly, when payment method orders change it can lead to our screenshot tests flaking. In this case, the best way to avoid that is probably to just not display Sunbit here

If this isn't straightforward, you could also just add Sunbit to the existing payment method order so that at least we avoid the flakiness issue due to reordering

I'm glad that you're considering future flakiness! I'd love it if this is the last time a LPM integrating team needs to update this test 😅

I tried explicitly setting the payment method list and it caused other tests to fail (because the tests are pretty reliant on autoPM).

Looking forward, we have a couple of options to avoid flakiness:

We explicitly set the LPM list for each test individually

We update the screenshot here and add sunbit to the list ordering. Since there will only be 4 payment methods visible at one time, additional payment methods shouldn't affect this.

If we want these tests to also validate how we handle automatically selecting payment methods, it would be better to go with option 2. Otherwise, option 1 would be more durable

The only reason this test is failing is because there were only 3 visible payment methods up until now (it's the only test where the merchant is in the US). Sunbit will bring it up to 4 which should prevent failures like these from happening in the future

I think we're not interested in testing auto PMs in this test, so setting the LPM list for the test individually sounds good! It looks like that's what you did so thank you!!

egeniesse-stripe requested review from a team as code owners June 6, 2024 17:16

egeniesse-stripe requested a review from amk-stripe June 6, 2024 17:16

egeniesse-stripe force-pushed the egeniesse/add-sunbit-payment-method branch 2 times, most recently from bacb263 to 032dfe0 Compare June 6, 2024 17:26

amk-stripe reviewed Jun 6, 2024

View reviewed changes

egeniesse-stripe added 2 commits June 6, 2024 17:04

Integrate Sunbit on PaymentSheet

a3faa86

Fix lint and PR feedback

2455f36

egeniesse-stripe force-pushed the egeniesse/add-sunbit-payment-method branch from 032dfe0 to 2455f36 Compare June 7, 2024 00:04

egeniesse-stripe added 3 commits June 6, 2024 19:59

Fix screenshot test and lint

97ac925

Re-enable the test

50340ae

Remove unused import

e648287

amk-stripe reviewed Jun 7, 2024

View reviewed changes

update the screenshot tests to freeze the payment method list

dd29547

amk-stripe previously approved these changes Jun 7, 2024

View reviewed changes

Fix lint

99974f4

egeniesse-stripe dismissed amk-stripe’s stale review via 99974f4 June 7, 2024 21:55

amk-stripe previously approved these changes Jun 7, 2024

View reviewed changes

ensure amazon pay is ordered before Klarna

ef58bf7

egeniesse-stripe dismissed amk-stripe’s stale review via ef58bf7 June 10, 2024 15:49

Merge branch 'master' into egeniesse/add-sunbit-payment-method

66872b1

amk-stripe approved these changes Jun 10, 2024

View reviewed changes

egeniesse-stripe merged commit a979b03 into master Jun 10, 2024
15 checks passed

egeniesse-stripe deleted the egeniesse/add-sunbit-payment-method branch June 10, 2024 18:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate Sunbit on PaymentSheet #8595

Integrate Sunbit on PaymentSheet #8595

egeniesse-stripe commented Jun 6, 2024 •

edited

Loading

github-actions bot commented Jun 6, 2024 •

edited

Loading

amk-stripe commented Jun 6, 2024

amk-stripe Jun 6, 2024

egeniesse-stripe Jun 7, 2024

amk-stripe Jun 6, 2024

egeniesse-stripe Jun 7, 2024

amk-stripe Jun 7, 2024

amk-stripe Jun 6, 2024

egeniesse-stripe Jun 7, 2024

amk-stripe commented Jun 6, 2024

amk-stripe Jun 7, 2024

egeniesse-stripe Jun 7, 2024 •

edited

Loading

egeniesse-stripe Jun 7, 2024 •

edited

Loading

amk-stripe Jun 7, 2024

Integrate Sunbit on PaymentSheet #8595

Integrate Sunbit on PaymentSheet #8595

Conversation

egeniesse-stripe commented Jun 6, 2024 • edited Loading

Summary

Motivation

Testing

Screenshots

Changelog

github-actions bot commented Jun 6, 2024 • edited Loading

amk-stripe commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amk-stripe commented Jun 6, 2024

Choose a reason for hiding this comment

egeniesse-stripe Jun 7, 2024 • edited Loading

Choose a reason for hiding this comment

egeniesse-stripe Jun 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egeniesse-stripe commented Jun 6, 2024 •

edited

Loading

github-actions bot commented Jun 6, 2024 •

edited

Loading

egeniesse-stripe Jun 7, 2024 •

edited

Loading

egeniesse-stripe Jun 7, 2024 •

edited

Loading