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.

Sign up for GitHub

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

Jump to bottom

Add Multibanco support to PaymentSheet and bindings #8433

Merged
merged 9 commits into from
May 10, 2024
Merged

Add Multibanco support to PaymentSheet and bindings #8433

merged 9 commits into from
May 10, 2024

Conversation

sfriedman-stripe
Copy link
Collaborator

Summary

Added Multibanco support on PaymentIntents to bindings, and added support to PaymentSheet as well

Motivation

https://docs.google.com/document/d/1QVT2zuwkz9IFTBNyjsYV_zzfeOsWva8gpSz2vXPHfW8/edit

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Before After
before screenshot after screenshot

Changelog

Updated CHANGELOG.md

@sfriedman-stripe sfriedman-stripe requested review from a team as code owners May 8, 2024 04:42
sfriedman-stripe
sfriedman-stripe commented May 8, 2024
View reviewed changes
payments-ui-core/src/main/resources/lpms.json Outdated
Comment on lines 1072 to 1086
"next_action_spec": {
"confirm_response_status_specs": {
"requires_action": {
"type": "multibanco_display_details"
}
},
"post_confirm_handling_pi_status_specs": {
"succeeded": {
"type": "finished"
},
"requires_action": {
"type": "canceled"
}
}
}
Copy link
Collaborator Author

@sfriedman-stripe sfriedman-stripe May 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Wasn't clear on whether we actually need the next_action_spec given #8053

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've been leaving the JSON exactly as it is on the server to try to make things consistent and not confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

On iOS leaving this next_action_spec would probably make handling the next action fail FWIW.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this after Jay and I chatted. It's not in the server spec anyway

@sfriedman-stripe sfriedman-stripe requested review from jaynewstrom-stripe and porter-stripe and removed request for awush-stripe May 8, 2024 04:45
@sfriedman-stripe sfriedman-stripe force-pushed the sfriedman/multibanco-ps branch from 28fb0e5 to d64471b Compare May 8, 2024 04:49
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 8, 2024 •
edited
Loading

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.9 MiB │   3.9 MiB │   -363 B │   8.6 MiB │   8.6 MiB │ +2.6 KiB 
     arsc │   2.2 MiB │   2.2 MiB │   +904 B │   2.2 MiB │   2.2 MiB │   +904 B 
 manifest │     5 KiB │     5 KiB │      0 B │  25.4 KiB │  25.4 KiB │      0 B 
      res │ 907.5 KiB │ 910.2 KiB │ +2.7 KiB │   1.4 MiB │   1.4 MiB │   +6 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 KiB │ 194.3 KiB │   +289 B │ 423.6 KiB │ 424.7 KiB │ +1.1 KiB 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
    total │  12.7 MiB │  12.7 MiB │ +4.9 KiB │  21.6 MiB │  21.6 MiB │  +12 KiB 

 DEX     │ old   │ new   │ diff                
─────────┼───────┼───────┼─────────────────────
   files │     1 │     1 │   0                 
 strings │ 43023 │ 43058 │ +35 (+1164 -1129)   
   types │ 14767 │ 14773 │  +6 (+1133 -1127)   
 classes │ 12467 │ 12472 │  +5 (+1008 -1003)   
 methods │ 61080 │ 61102 │ +22 (+13088 -13066) 
  fields │ 40304 │ 40314 │ +10 (+6600 -6590)   

 ARSC    │ old  │ new  │ diff       
─────────┼──────┼──────┼────────────
 configs │  242 │  242 │  0         
 entries │ 6028 │ 6030 │ +2 (+2 -0)
APK 
     compressed      │     uncompressed     │                                           
──────────┬──────────┼───────────┬──────────┤                                           
 size     │ diff     │ size      │ diff     │ path                                      
──────────┼──────────┼───────────┼──────────┼───────────────────────────────────────────
  7.3 KiB │ +1.4 KiB │   7.2 KiB │ +1.4 KiB │ ∆ assets/dexopt/baseline.prof             
  1.3 KiB │ +1.3 KiB │     3 KiB │   +3 KiB │ + res/5U.xml                              
  1.3 KiB │ +1.3 KiB │     3 KiB │   +3 KiB │ + res/rh.xml                              
  2.2 MiB │   +904 B │   2.2 MiB │   +904 B │ ∆ resources.arsc                          
  3.9 MiB │   -363 B │   8.6 MiB │ +2.6 KiB │ ∆ classes.dex                             
  3.2 KiB │   +138 B │  25.6 KiB │   +805 B │ ∆ lpms.json                               
    127 B │   +127 B │       5 B │     +5 B │ + META-INF/services/N8.z                  
    127 B │   +127 B │       5 B │     +5 B │ + META-INF/services/S8.m                  
          │   -127 B │           │     -5 B │ - META-INF/services/O8.z                  
          │   -127 B │           │     -5 B │ - META-INF/services/T8.n                  
 49.5 KiB │    +85 B │ 116.8 KiB │   +164 B │ ∆ META-INF/MANIFEST.MF                    
 52.8 KiB │    +67 B │ 116.9 KiB │   +164 B │ ∆ META-INF/CERT.SF                        
    894 B │    +15 B │     762 B │    +15 B │ ∆ assets/dexopt/baseline.profm            
    271 B │     -1 B │     120 B │      0 B │ ∆ META-INF/version-control-info.textproto 
    953 B │     -1 B │   2.5 KiB │      0 B │ ∆ res/de.xml                              
──────────┼──────────┼───────────┼──────────┼───────────────────────────────────────────
  6.3 MiB │ +4.9 KiB │  11.1 MiB │  +12 KiB │ (total)
DEX 
STRINGS:

   old   │ new   │ diff              
  ───────┼───────┼───────────────────
   43023 │ 43058 │ +35 (+1164 -1129) 
  
  + A2
  + B2
  + C2
  + D2
  + DisplayMultibancoDetails
  + DisplayMultibancoDetails(hostedVoucherUrl=
  + E2
  + F2
  + G2
  + H2
  + I2
  + J2
  + K2
  + L2
  + LA6/v;
  + LB8/b;
  + LB8/c;
  + LB8/d;
  + LB8/e;
  + LB8/f;
  + LB8/g;
  + LB8/h;
  + LB8/i;
  + LC8/j;
  + LC8/k;
  + LC8/l;
  + LC8/m;
  + LC8/n;
  + LC8/o;
  + LC8/p;
  + LC8/q;
  + LC8/r;
  + LC8/s;
  + LC8/t;
  + LC8/u;
  + LC8/v;
  + LC8/w;
  + LC8/x;
  + LC9/b;
  + LD9/c;
  + LD9/d;
  + LE9/e;
  + LE9/f;
  + LE9/g;
  + LE9/h;
  + LE9/i;
  + LE9/j;
  + LE9/k;
  + LE9/l;
  + LE9/m;
  + LE9/n;
  + LE9/o;
  + LE9/p;
  + LE9/q;
  + LE9/r;
  + LE9/s;
  + LE9/t;
  + LE9/u;
  + LE9/v;
  + LE9/w;
  + LE9/x;
  + LE9/y;
  + LG8/b;
  + LG8/c;
  + LG8/d;
  + LG9/b;
  + LG9/c;
  + LH9/d;
  + LI8/b;
  + LI8/c;
  + LI8/d;
  + LI8/e;
  + LI8/f;
  + LI8/g;
  + LI8/h;
  + LI8/i;
  + LI8/j;
  + LK8/h;
  + LK8/i;
  + LK8/j;
  + LK8/k;
  + LK8/l;
  + LK8/m;
  + LK8/n;
  + LL8/o;
  + LL8/p;
  + LL8/q;
  + LM9/c;
  + LM9/d;
  + LN8/A0;
  + LN8/A;
  + LN8/B0;
  + LN8/B;
  + LN8/C0;
  + LN8/C;
  + LN8/D0;
  + LN8/D;
  + LN8/E0;
  + LN8/E;
  + LN8/F;
  + LN8/G;
  + LN8/H;
  + LN8/I;
  + LN8/J;
  + LN8/K;
  + LN8/L;
  + LN8/M;
  + LN8/N;
  + LN8/O;
  + LN8/P;
  + LN8/Q;
  + LN8/S;
  + LN8/T;
  + LN8/U;
  + LN8/V;
  + LN8/W;
  + LN8/X;
  + LN8/Y;
  + LN8/Z;
  + LN8/a0;
  + LN8/b0;
  + LN8/c0;
  + LN8/d0;
  + LN8/e0;
  + LN8/f0;
  + LN8/g0;
  + LN8/h0;
  + LN8/i0;
  + LN8/i;
  + LN8/j0;
  + LN8/j;
  + LN8/k0;
  + LN8/k;
  + LN8/l0;
  + LN8/l;
  + LN8/m0;
  + LN8/m;
  + LN8/n0;
  + LN8/n;
  + LN8/o0;
  + LN8/o;
  + LN8/p0;
  + LN8/p;
  + LN8/q0;
  + LN8/q;
  + LN8/r0;
  + LN8/r;
  + LN8/s0;
  + LN8/s;
  + LN8/t0;
  + LN8/t;
  + LN8/u0;
  + LN8/u;
  + LN8/v0;
  + LN8/v;
  + LN8/w0;
  + LN8/w;
  + LN8/x0;
  + LN8/x;
  + LN8/y0;
  + LN8/y;
  + LN8/z0;
  + LN8/z;
  + LO9/c;
  + LO9/d;
  + LO9/e;
  + LP8/g;
  + LP8/h;
  + LP8/i;
  + LP8/j;
  + LP8/k;
  + LP8/l;
  + LP8/m;
  + LP8/n;
  + LP8/o;
  + LP8/p;
  + LP8/q;
  + LP8/r;
  + LP8/s;
  + LP8/t;
  + LP8/u;
  + LP8/v;
  + LP8/w;
  + LP8/x;
  + LP9/f;
  + LQ8/A;
  + LQ8/B;
  + LQ8/C;
  + LQ8/D;
  + LQ8/E;
  + LQ8/F;
  + LQ8/G;
  + LQ8/H;
  + LQ8/I;
  + LQ8/J;
  + LQ8/K;
  + LQ8/L;
  + LQ8/M;
  + LQ8/N;
  + LQ8/O;
  + LQ8/P;
  + LQ8/Q;
  + LQ8/S;
  + LQ8/T;
  + LQ8/U;
  + LQ8/V;
  + LQ8/W;
  + LQ8/X;
  + LQ8/Y;
  + LQ8/Z;
  + LQ8/a0;
  + LQ8/b0;
  + LQ8/c0;
  + LQ8/d0;
  + LQ8/e0;
  + LQ8/f0;
  + LQ8/g0;
  + LQ8/h0;
  + LQ8/i0;
  + LQ8/j0;
  + LQ8/k0;
  + LQ8/l0;
  + LQ8/m0;
  + LQ8/n0;
  + LQ8/o0;
  + LQ8/p0;
  + LQ8/y;
  + LQ8/z;
  + LS9/a;
  + LS9/b;
  + LT0/k;
  + LV7/a;
  + LV7/b;
  + LV7/c;
  + LV8/j;
  + LX7/A0;
  + LX7/A1;
  + LX7/A;
  + LX7/B0;
  + LX7/B1;
  + LX7/B;
  + LX7/C0;
  + LX7/C1;
  + LX7/C;
  + LX7/D0;
  + LX7/D1;
  + LX7/D;
  + LX7/E0;
  + LX7/E1;
  + LX7/E;
  + LX7/F0;
  + LX7/F1;
  + LX7/F;
  + LX7/G0;
  + LX7/G1;
  + LX7/G;
  + LX7/H0;
  + LX7/H1;
  + LX7/H;
  + LX7/I0;
  + LX7/I1;
  + LX7/I;
  + LX7/J0;
  + LX7/J1;
  + LX7/J;
  + LX7/K0;
  + LX7/K1;
  + LX7/K;
  + LX7/L0;
  + LX7/L1;
  + LX7/L;
  + LX7/M0;
  + LX7/M1;
  + LX7/M;
  + LX7/N0;
  + LX7/N1;
  + LX7/N;
  + LX7/O0;
  + LX7/O1;
  + LX7/O;
  + LX7/P0;
  + LX7/P1;
  + LX7/P;
  + LX7/Q0;
  + LX7/Q1;
  + LX7/Q;
  + LX7/R0;
  + LX7/R1;
  + LX7/S0;
  + LX7/S1;
  + LX7/S;
  + LX7/T0;
  + LX7/T1;
  + LX7/T;
  + LX7/U0;
  + LX7/U1;
  + LX7/U;
  + LX7/V0;
  + LX7/V1;
  + LX7/V;
  + LX7/W0;
  + LX7/W1;
  + LX7/W;
  + LX7/X0;
  + LX7/X1;
  + LX7/X;
  + LX7/Y0;
  + LX7/Y1;
  + LX7/Y;
  + LX7/Z0;
  + LX7/Z1;
  + LX7/Z;
  + LX7/a0;
  + LX7/a1;
  + LX7/a2;
  + LX7/b0;
  + LX7/b1;
  + LX7/b2;
  + LX7/b;
  + LX7/c0;
  + LX7/c1;
  + LX7/c2;
  + LX7/c;
  + LX7/d0;
  + LX7/d1;
  + LX7/d2;
  + LX7/d;
  + LX7/e0;
  + LX7/e1;
  + LX7/e2;
  + LX7/e;
  + LX7/f0;
  + LX7/f1;
  + LX7/f2;
  + LX7/f;
  + LX7/g0;
  + LX7/g1;
  + LX7/g2;
  + LX7/g;
  + LX7/h0;
  + LX7/h1;
  + LX7/h;
  + LX7/i0;
  + LX7/i1;
  + LX7/i;
  + LX7/j0;
  + LX7/j1;
  + LX7/j;
  + LX7/k0;
  + LX7/k1;
  + LX7/k;
  + LX7/l0;
  + LX7/l1;
  + LX7/l;
  + LX7/m0;
  + LX7/m1;
  + LX7/m;
  + LX7/n0;
  + LX7/n1;
  + LX7/n;
  + LX7/o0;
  + LX7/o1;
  + LX7/o;
  + LX7/p0;
  + LX7/p1;
  + LX7/p;
  + LX7/q0;
  + LX7/q1;
  + LX7/q;
  + LX7/r0;
  + LX7/r1;
  + LX7/r;
  + LX7/s0;
  + LX7/s1;
  + LX7/s;
  + LX7/t0;
  + LX7/t1;
  + LX7/t;
  + LX7/u0;
  + LX7/u1;
  + LX7/u;
  + LX7/v0;
  + LX7/v1;
  + 
...✂
ARSC 
ENTRIES:

   old  │ new  │ diff       
  ──────┼──────┼────────────
   6028 │ 6030 │ +2 (+2 -0) 
  + drawable/stripe_ic_paymentsheet_pm_multibanco
  + string/stripe_paymentsheet_payment_method_multibanco

@sfriedman-stripe
Copy link
Collaborator Author

Still need to run the Lokalize script and apiDump, but having some issues with my Java env. Will pair with someone

sfriedman-stripe
sfriedman-stripe commented May 8, 2024
View reviewed changes
example/src/main/java/com/stripe/example/activity/MultibancoActivity.kt
import com.stripe.android.model.PaymentMethod
import com.stripe.android.model.PaymentMethodCreateParams

class MultibancoActivity : StripeIntentActivity() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is heavily cargo culted from other Activities in the Example app; please lmk if I've faltered

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I think this is the right thing to do for now. I added this as an improvement we can make, but it seems low bar since it's example code.

@sfriedman-stripe sfriedman-stripe mentioned this pull request May 10, 2024
Merged
3 tasks
jaynewstrom-stripe
jaynewstrom-stripe reviewed May 10, 2024
View reviewed changes
payments-ui-core/res/values/strings.xml Outdated
@@ -93,4 +93,5 @@
<string name="stripe_setup_button_label">Set up</string>
<!-- Label for UPI ID number field on form -->
<string name="stripe_upi_id_label">UPI ID</string>
<string name="stripe_paymentsheet_payment_method_multibanco">Multibanco</string>
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 normally put these in donottranslate, as most of the time these are names, and don't get translated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The lokalise build won't fail if you do that too. See others here:

stripe-android/payments-ui-core/res/values/donottranslate.xml

Line 31 in c766796

<string name="stripe_paymentsheet_payment_method_twint">TWINT</string>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh nice! Android Studio warns against adding anything to anything but the translations editor, so I didn't realize some were in different files

@sfriedman-stripe sfriedman-stripe force-pushed the sfriedman/multibanco-ps branch from d653adf to ccd56c2 Compare May 10, 2024 18:37
@sfriedman-stripe sfriedman-stripe enabled auto-merge (squash) May 10, 2024 20:45
@sfriedman-stripe sfriedman-stripe merged commit e554dde into master May 10, 2024
15 checks passed
@sfriedman-stripe sfriedman-stripe deleted the sfriedman/multibanco-ps branch May 10, 2024 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaynewstrom-stripe jaynewstrom-stripe jaynewstrom-stripe approved these changes

@porter-stripe porter-stripe Awaiting requested review from porter-stripe

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfriedman-stripe @porter-stripe @jaynewstrom-stripe