-
Notifications
You must be signed in to change notification settings - Fork 995
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
read from customersession setting instead of config #4517
base: master
Are you sure you want to change the base?
Changes from 20 commits
a149588
4660d4f
2057cb2
cf5fd54
b6cfbe9
cb9d01d
907a363
08087f6
9aa6307
23358dd
782ffec
d43d73e
68ae80e
56b7ff8
9c268e7
ac0fdf1
3aef64e
ea32e91
2fc6c05
4f2f131
1480972
f88219a
69d0483
f9bd545
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,6 +211,10 @@ extension STPElementsSession { | |
return customer?.customerSession.mobilePaymentElementComponent.features?.paymentMethodRemoveLast ?? true | ||
} | ||
|
||
var paymentMethodSetAsDefaultForPaymentSheet: Bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't need to be in this PR, but we should probably rename xForPaymentSheet to xForMPE |
||
return customer?.customerSession.mobilePaymentElementComponent.features?.paymentMethodSetAsDefault ?? false | ||
} | ||
|
||
func allowsRemovalOfPaymentMethodsForCustomerSheet() -> Bool { | ||
var allowsRemovalOfPaymentMethods = false | ||
if let customerSession = customer?.customerSession { | ||
|
@@ -227,6 +231,10 @@ extension STPElementsSession { | |
return customer?.customerSession.customerSheetComponent.features?.paymentMethodRemoveLast ?? true | ||
} | ||
|
||
var paymentMethodSyncDefaultForCustomerSheet: Bool { | ||
return customer?.customerSession.customerSheetComponent.features?.paymentMethodSyncDefault ?? false | ||
} | ||
|
||
var isLinkCardBrand: Bool { | ||
linkSettings?.linkMode == .linkCardBrand | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,10 +107,10 @@ extension CustomerSessionAdapter { | |
return stripePaymentMethodId | ||
} | ||
|
||
func fetchSelectedPaymentOption(for customerId: String, customer: ElementsCustomer? = nil) -> CustomerPaymentOption? { | ||
func fetchSelectedPaymentOption(for customerId: String, elementsSession: STPElementsSession? = nil) -> CustomerPaymentOption? { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly surprised we don't have tests for this, can you mention this class in the JIRA ticket you create. |
||
// if opted in to the "set as default" feature, try to get default payment method from elements session | ||
if configuration.allowsSetAsDefaultPM { | ||
guard let customer = customer, | ||
if let elementsSession = elementsSession, elementsSession.paymentMethodSyncDefaultForCustomerSheet { | ||
guard let customer = elementsSession.customer, | ||
let defaultPaymentMethod = customer.getDefaultOrFirstPaymentMethod() else { return nil } | ||
return CustomerPaymentOption.stripeId(defaultPaymentMethod.stripeId) | ||
} | ||
|
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.
Surprised we don't have any tests for this class? If so can we file a run ticket to get some tests created for this class.
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.
Created a ticket: RUN_MOBILESDK-3836
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 do have tests. They are in STPElementsSessionTest.swift
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.
Ah I see, it's a child of ElementsSession. Kinda hard to tell it's being tested might still be good to have standalone tests.