feat: Add domain foundation + test data factory#1633
Conversation
Generated by 🚫 Danger Swift against 0e73d61 |
bthebladeprimer
force-pushed
the
bn/feature/cc-04-domain-foundation
branch
from
8e4c043 to
a9450d7
Compare
|
|
||
| return PaymentResult( | ||
| paymentId: paymentResponse.paymentId, | ||
| status: .success, |
| paymentId: paymentResponse.paymentId, | ||
| status: .success, | ||
| token: token, | ||
| amount: nil, |
|
|
||
| import Foundation | ||
|
|
||
| public struct PaymentResult { |
There was a problem hiding this comment.
This is public and is a much larger exposure than we currently offer today in Auto Flow, is this a conscious choice?
| self.cornerRadius = cornerRadius | ||
| } | ||
|
|
||
| static func == (lhs: InternalPaymentMethod, rhs: InternalPaymentMethod) -> Bool { |
There was a problem hiding this comment.
Thoughts on not comparing every attribute - is this an intentional choice?
| } | ||
|
|
||
| func execute() async throws -> PaymentResult { | ||
| let intent = PrimerInternal.shared.intent |
There was a problem hiding this comment.
This is one example (there are others,ill only comment once) of accessing a singleton without being in a @mainactor. Is this a potential issue? Perhaps @MainActor or MainActor.run {} ?
| ) | ||
|
|
||
| return PaymentResult( | ||
| paymentId: paymentResponse.id ?? "", |
There was a problem hiding this comment.
This string fallback stands out - is it required? What is the background if yes?
|
|
||
| import Foundation | ||
|
|
||
| protocol HeadlessRepository { |
There was a problem hiding this comment.
other repositories include @available(iOS 15.0, *) - should we have it here for consistency?
| @available(iOS 15.0, *) | ||
| struct ApplePayRequestBuilder { | ||
|
|
||
| static func build() throws -> ApplePayRequest { |
There was a problem hiding this comment.
We access 4 singletons here which makes this hard to test. Perhaps we can inject them
| } | ||
|
|
||
| @available(iOS 15.0, *) | ||
| struct AchStripeData { |
There was a problem hiding this comment.
Not sure where this can or cannot be printed around the codebase, but for sensitive types like this I would like to see a CustomStringConvertible which redacts stripeClientSecret and decodedJwtToken
| case requires3DS | ||
| case requiresAction | ||
|
|
||
| init(from apiStatus: Response.Body.Payment.Status) { |
There was a problem hiding this comment.
There are 8 statuses, but can only be initialised from 3 - conscious choice?
| Primer.shared.delegate?.primerDidResumeWith?(resumeToken) { decision in | ||
| continuation.resume(returning: decision) | ||
| } | ||
| } else if PrimerInternal.shared.sdkIntegrationType == .checkoutComponents { |
There was a problem hiding this comment.
I notice we bypass the delegate here - expected with the new integration method - curious how we surface these events to the merchant now?
There was a problem hiding this comment.
And critically allow them to interject/halt in the flow as they do today
bthebladeprimer
force-pushed
the
bn/feature/cc-03-sdk-refactors
branch
from
2e87ec4 to
9c8d8ab
Compare
bthebladeprimer
force-pushed
the
bn/feature/cc-04-domain-foundation
branch
from
708e244 to
0e73d61
Compare
2 participants
Summary
InternalPaymentMethod,PaymentResultfor internal payment state managementPaymentMethodMapperfor converting between API and domain modelsTestDatafactory with extensions for addresses, cards, config, contacts, errors, payment methods, and validationContext
PR 4 of 15 in the CheckoutComponents feature split. Establishes the domain layer foundation that all subsequent PRs depend on. Depends on PR 3 (#1632).
Tracker: https://www.notion.so/32fca65dc30e81dabf66f33e2b58c3a1
Test plan
InternalPaymentMethodTests,PaymentMethodMapperTests)ProcessPayPalPaymentInteractorTestspassesxcodebuild -workspace PrimerSDK.xcworkspace -scheme PrimerSDKTests -destination "platform=iOS Simulator,name=iPhone 16" test