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

[BANKCON-14524] Allow pay by bank when linkCardBrand criteria is met & dedupe instant debits #4021

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

mats-stripe
Copy link
Collaborator

@mats-stripe mats-stripe commented Sep 16, 2024

Summary

Per the Panther project plan, this adds Link Card Brand as a payment method when the following criteria is met:

  • link_funding_sources contains BANK_ACCOUNT
  • US_BANK_ACCOUNT is not an available payment method.
  • link_mode is LINK_CARD_BRAND

This also makes sure that both Instant Debits and Link Card Brand won't both be shown at the same time, since they appear as identical payment methods to a user (same name, icon, and elements form).

Motivation

Building Panther support!

Testing

Added a unit test, and manually verified the new payment method shows up when the conditions are met:

Screen.Recording.2024-09-17.at.9.18.47.AM.mov

Changelog

N/a

Comment on lines +173 to +178
var eligibleForInstantDebits: Bool {
elementsSession.orderedPaymentMethodTypes.contains(.link) &&
!elementsSession.orderedPaymentMethodTypes.contains(.USBankAccount) &&
!intent.isDeferredIntent &&
elementsSession.linkFundingSources?.contains(.bankAccount) == true
{
}
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need to be a var instead of a let?

and out of curiosity/learning how does this compile?

I would have expected it to be something like:

            let eligibleForInstantDebits: Bool = {
                return elementsSession.orderedPaymentMethodTypes.contains(.link) &&
                !elementsSession.orderedPaymentMethodTypes.contains(.USBankAccount) &&
                !intent.isDeferredIntent &&
                elementsSession.linkFundingSources?.contains(.bankAccount) == true
            }()

I get the inferred return as they are no longer necessary.

But what are all the other pieces?

it almost looks like an in-line computed property but the declaration is inside of a function 🤔 so I am not immediately sure the step-by-step execution for it to compile into a boolean

Copy link
Contributor

Choose a reason for hiding this comment

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

Tried in Xcode and I guess it can't be a let because it is a computed property

Is there a name for computed properties inside of functions, and is this new or old? 🤔

kgaidis-stripe
kgaidis-stripe previously approved these changes Sep 16, 2024
Copy link
Contributor

@kgaidis-stripe kgaidis-stripe left a comment

Choose a reason for hiding this comment

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

💯

Base automatically changed from mats/add_link_mode_to_link_settings_model to master September 18, 2024 19:15
@mats-stripe mats-stripe dismissed kgaidis-stripe’s stale review September 18, 2024 19:15

The base branch was changed.

@mats-stripe
Copy link
Collaborator Author

Putting this PR on hold until we have guardrails preventing this feature be shown to users while it is being built.

tillh-stripe
tillh-stripe previously approved these changes Sep 25, 2024
@mats-stripe mats-stripe force-pushed the mats/show_link_card_brand_payment_type branch from 73c6171 to ccd9427 Compare September 25, 2024 14:46
@@ -196,7 +196,7 @@ class PaymentSheetStandardUITests: PaymentSheetUITestCase {
// `mc_load_succeeded` event `selected_lpm` should be "apple_pay", the default payment method.
XCTAssertEqual(analyticsLog[2][string: "selected_lpm"], "apple_pay")
app.buttons["+ Add"].waitForExistenceAndTap()
XCTAssertTrue(app.staticTexts["Add a card"].waitForExistence(timeout: 2))
XCTAssertTrue(app.staticTexts["Card information"].waitForExistence(timeout: 2))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to update this UI test. With Panther enabled there's a second payment method available in this scenario now, and so the Add a card text was no longer shown. Here's the changes to this screen with Panther:

Before (without Panther) Now (with Panther)
Simulator Screenshot - iPhone 15 - 2024-09-25 at 11 28 33 Simulator Screenshot - iPhone 15 - 2024-09-25 at 11 28 03

@mats-stripe mats-stripe merged commit 3b8e1f6 into master Sep 25, 2024
5 checks passed
@mats-stripe mats-stripe deleted the mats/show_link_card_brand_payment_type branch September 25, 2024 16:07
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.

3 participants