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-15711] Pass billing address and email address to payment details API #4148

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mats-stripe
Copy link
Collaborator

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

Android equivalent: stripe/stripe-android#9446

Summary

This passes the billing address model and a billing email to the /consumers/payment_details/ API. More details: https://docs.google.com/document/d/1IP66DkxVK6DzeGCmuw00Gt9okFhEHQgM3lqBZSA-M4E/edit?usp=sharing

Motivation

BANKCON-15711

Testing

Unit tests, and I've verified the billing fields are correctly passed from MPE to the API call:

Screenshot 2024-10-16 at 3 12 25 PM

Changelog

N/a

}

if let billingEmail, !billingEmail.isEmpty {
parameters["billing_email_address"] = billingEmail
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines +38 to +44
case name
case line1 = "line_1"
case line2 = "line_2"
case city = "locality"
case state = "administrative_area"
case postalCode = "postal_code"
case countryCode = "country_code"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link

github-actions bot commented Oct 16, 2024

🚨 New dead code detected in this PR:

FinancialConnectionsAPIClient.swift:246 warning: Function 'paymentDetails(consumerSessionClientSecret:bankAccountId:billingAddress:billingEmail:)' is unused

Please remove the dead code before merging.

If this is intentional, you can bypass this check by adding the label skip dead code check to this PR.

ℹ️ If this comment appears to be left in error, double check that the flagged code is actually used and/or make sure your branch is up-to-date with master.

@mats-stripe mats-stripe marked this pull request as ready for review October 17, 2024 13:24
@mats-stripe mats-stripe requested review from a team as code owners October 17, 2024 13:24
@mats-stripe mats-stripe force-pushed the mats/pass_billing_address_to_payment_details branch 2 times, most recently from 905cca9 to dd16f78 Compare October 18, 2024 20:07
Copy link

⚠️ Public API changes detected:

StripePaymentSheet

- public var paymentMethodLayout: StripePaymentSheet.PaymentSheet.PaymentMethodLayout
- public enum PaymentMethodLayout {
- case horizontal
- case vertical
- case automatic
- public static func == (a: StripePaymentSheet.PaymentSheet.PaymentMethodLayout, b: StripePaymentSheet.PaymentSheet.PaymentMethodLayout) -> Swift.Bool
- public func hash(into hasher: inout Swift.Hasher)
- public var hashValue: Swift.Int {
- get
- }
- }

If you are adding a new public API consider the following:

  • Do these APIs need to be public or can they be protected with @_spi(STP)?
  • If these APIs need to be public, assess whether they require an API review.

If you are modifying or removing a public API:

  • Does this require a breaking version change?
  • Do these changes require API review?

If you confirm these APIs need to be added/updated and have undergone necessary review, add the label modifies public API to this PR to acknowledge and bypass this check.

ℹ️ If this comment appears to be left in error, make sure your branch is up-to-date with master.

@mats-stripe
Copy link
Collaborator Author

Moving this to draft for now, since this PR will change slightly with this work: #4162

@mats-stripe mats-stripe marked this pull request as draft October 21, 2024 13:52
@mats-stripe mats-stripe force-pushed the mats/pass_billing_address_to_payment_details branch from dd16f78 to 8e2d09b Compare October 21, 2024 17:47
@mats-stripe mats-stripe changed the base branch from master to mats/add_requested_fields_in_bank_tab October 21, 2024 17:47
@mats-stripe mats-stripe marked this pull request as ready for review October 21, 2024 17:48
Copy link

emerge-tools bot commented Oct 21, 2024

📸 Snapshot Test

No snapshots generated

Name Version Added Removed Modified Unchanged Errored Approval
StripePaymentsSize
com.stripe.StripePaymentsSize
1.0 (1) 0 0 0 0 0 N/A
StripeSize
com.stripe.StripeSize
1.0 (1) 0 0 0 0 0 N/A
StripeApplePaySize
com.stripe.StripeApplePaySize
1.0 (1) 0 0 0 0 0 N/A
StripePaymentSheetSize
com.stripe.StripePaymentSheetSize
1.0 (1) 0 0 0 0 0 N/A
StripeFinancialConnectionsSize
com.stripe.StripeFinancialConnectionsSize
1.0 (1) 0 0 0 0 0 N/A
StripePaymentsUISize
com.stripe.StripePaymentsUISize
1.0 (1) 0 0 0 0 0 N/A

🛸 Powered by Emerge Tools

@mats-stripe mats-stripe force-pushed the mats/add_requested_fields_in_bank_tab branch from 764ccf7 to 594cdd6 Compare October 22, 2024 14:39
@mats-stripe mats-stripe force-pushed the mats/pass_billing_address_to_payment_details branch 4 times, most recently from 9dfabf9 to cf1076b Compare October 24, 2024 14:14
@mats-stripe mats-stripe force-pushed the mats/add_requested_fields_in_bank_tab branch from aaf0034 to 73eda16 Compare October 24, 2024 14:54
Base automatically changed from mats/add_requested_fields_in_bank_tab to master October 24, 2024 17:49
@mats-stripe mats-stripe force-pushed the mats/pass_billing_address_to_payment_details branch from cf1076b to 9a0d88b Compare October 24, 2024 18:44
let billingAddress = BillingAddress(
name: "Bobby Tables",
line1: "123 Fake St",
line2: nil,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a test with line2: ""?

@@ -220,11 +220,13 @@ extension PaymentMethodFormViewController {
}()

let linkMode = elementsSession.linkSettings?.linkMode
let billingAddress = instantDebitsFormElement?.billingAddress
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like we’re sending the address even if attachDefaults is set to false and address collection is set to never. Can you take another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants