[ShopperInsights] Add EligiblePayments API Call #1171
Conversation
| return | ||
| } | ||
|
|
||
| let customer = lastPostParameters["customer"] as! [String: Any] |
There was a problem hiding this comment.
🏈 Would love for us to implement a cleaner way to verify JSON structure across our SDKs test suite
There was a problem hiding this comment.
On Android, we use https://github.com/skyscreamer/JSONassert.
Maybe you could try using https://github.com/iwill/generic-json-swift on iOS?
There was a problem hiding this comment.
👀 Thanks for those resources! Cool that it plays easy w/ Codable, too.
Sources/BraintreeShopperInsights/BTEligiblePaymentsRequest.swift
Outdated
Show resolved
Hide resolved
| @@ -178,6 +179,14 @@ import Foundation | |||
| graphQLHTTP = BTGraphQLHTTP(url: graphQLBaseURL, tokenizationKey: tokenizationKey) | |||
| } | |||
| } | |||
|
|
|||
| if paypalHTTP == nil { | |||
| let paypalBaseURL: URL? = paypalAPIURL(forEnvironment: configuration?.environment ?? "") | |||
There was a problem hiding this comment.
Do we want to make this method return a non-optional URL? Based on this method it seems like the default is the prod URL. Based on the defaulting behavior it may also make sense to make the environment optional instead of passing an empty string here if we don't get one from the config. Thoughts?
There was a problem hiding this comment.
Ah - good ❓. I was following the existing pattern that we have for the GraphQL API implementation (see here).
I agree though, it's definitely funky.
Wrote this in the PR notes a bit, but this PR revealed how tightly coupled our networking layer is to BT APIs. Even the GraphQL stuff feels very tacked-on. I think we could benefit from a re-design, but wanted to 🏈 on it.
There was a problem hiding this comment.
For sure down to 🏈 on it. Agree the network layer as a whole needs an overhaul, so we can certainly tackle it in a future PR.
Co-authored-by: agedd <[email protected]>
|
Marking this as "Blocked" since there is talks that the BT GW will wrap this API call, versus us having to hit the PP API directly. |
|
Removing blocked label, as the API team has updated the API to send mock response values back given certain emails. Follow-up PR to come with parsing logic! |
…al API prod env setting
Summary of changes
BTAPIClient&BTHTTPto support API calls to paypal.com APIsBTEligiblePaymentsRequeststruct to house POST body forv2/payments/find-eligible-methodsAPI callNotes
BTHTTP&BTGraphQLHTTPhave a lot of overlap & duplication (3 differentURLSessionconfigurations, 2 differentURLRequestcreation sites)Checklist
Added a changelog entryAuthors
@scannillo