-
Notifications
You must be signed in to change notification settings - Fork 659
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
Enable Cash App Pay for SI and PI+SFU #7264
Conversation
Diffuse output:
APK
DEX
ARSC
|
e376009
to
e58693a
Compare
c870f32
to
70da21b
Compare
70da21b
to
9073756
Compare
9073756
to
036125b
Compare
@@ -18,12 +18,12 @@ data class MandateTextSpec( | |||
@StringRes | |||
val stringResId: Int | |||
) : FormItemSpec() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we reuse MandateTextSpec, rather than adding a new CashAppPayMandateTextSpec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CashAppPayMandateTextSpec
is using MandateTextSpec
under the hood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why we need this class though, couldn't we just add MandateTextSpec with the specific arguments in LpmRepository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need CashAppPayMandateTextSpec
because it passes merchantName
to the underlying MandateSpec
twice (as there are two placeholders in the string). We can’t do that with the plain MandateSpec
.
paymentMethod.hasExpectedDetails() | ||
}.filter { paymentMethod -> | ||
// PayPal isn't supported yet as a saved payment method (backend limitation). | ||
paymentMethod.type != PaymentMethod.Type.PayPal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should (as a follow up) add this as a property to the LpmRepository or similar where we can define this stuff along side the PM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Summary
This pull request enables the usage of Cash App Pay for setup intents and payment intents with SFU.
Motivation
Testing
Screenshots
Changelog