-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Determine "Apple Pay capable" based on available card types from the project #805
Conversation
…apple-pay-capable
…apple-pay-capable
"VISA", | ||
"DISCOVER", | ||
"JCB", | ||
"DINERS", |
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.
Note that DINERS
is actually not a supportedNetwork
according to Apple Pay, so it's filtered out.
@@ -4,6 +4,7 @@ import Prelude | |||
import Runes | |||
|
|||
public struct Project { | |||
public var availableCardTypes: [String]? |
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.
This is optional because the project JSON object we get from the /discover
endpoint doesn't have this field (it's only a partial project), and we use the same Project
model to decode that payload.
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 suppose we could also consider coalescing an empty array if we wanted this to be non-optional 🤔 but I guess having nil
here is more explicit about not having the information rather than there are no available card types.
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.
Yeah I'd rather keep it nil
to make it clear that the info is not available.
} | ||
|
||
public static func supportedNetworks(for project: Project) -> [PKPaymentNetwork] { | ||
guard let availableCardTypes = project.availableCardTypes else { |
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.
availableCardTypes
should never be nil
in this context, but decided to add a fallback just in case 🤷♀
Generated by 🚫 Danger |
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.
Looking good, minor comments
Kickstarter-iOS/Views/Controllers/PledgePaymentMethodsViewController.swift
Outdated
Show resolved
Hide resolved
@@ -4,6 +4,7 @@ import Prelude | |||
import Runes | |||
|
|||
public struct Project { | |||
public var availableCardTypes: [String]? |
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 suppose we could also consider coalescing an empty array if we wanted this to be non-optional 🤔 but I guess having nil
here is more explicit about not having the information rather than there are no available card types.
…apple-pay-capable # Conflicts: # KsApi/models/lenses/ProjectLenses.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.
One last question
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.
Nice lgtm!
📲 What
Previously, we were determining Apple Pay capabilities from hardcoded logic. This PR updates that logic to instead use the
availableCardTypes
field on the project, and also adds the business logic of showing/hiding the Apple Pay button in the newPledgePaymentMethodsViewController
. The logic for determining supported networks is shared, and is now fully tested.🤔 Why
Part of Apple Pay implementation on the new Pledge screen.
🛠 How
We now have a new
availableCardTypes
field on theProject
model. We can use this array to determine whichsupportedNetworks
to request from the method PassKit functionPKPaymentAuthorizationViewController.canMakePayments(usingNetworks:)
. There is still fallback logic which mimics the backend logic, in caseavailableCardTypes
isnil
.✅ Acceptance criteria
⏰ TODO
availableCardType
strings that are being returned to match the GraphQLCreditCardType
schema names so this feature will work