MBL-1972: Add PaymentMethodsUseCase and tests #2255
Conversation
amy-at-kickstarter
force-pushed
the
feat/adyer/mbl-1972
branch
from
37c6db3 to
ffcacd1
Compare
| * `selectedPaymentSource` - The currently selected credit card, or `nil` if no card is selected. Sent at least once after `initialData` is sent. | ||
| */ | ||
| public final class PaymentMethodsUseCase { | ||
| init(initialData: Signal<PledgeViewData, Never>, userSessionStarted: Signal<(), Never>) { |
There was a problem hiding this comment.
This is almost 100% copy-pasted out of the existing pledge flow.
There was a problem hiding this comment.
Splitting the comment up so that inputs and outputs have their own parts commented might make sense here
There was a problem hiding this comment.
Hmm, yeah, fair point. The only catch is that there's an extra input (creditCardSelected()) which doesn't live on init, so I don't want to document them there. Were you thinking the comments would make more sense to put on PaymentMethodsUseCaseInputs and PaymentMethodUseCaseOutputs?
| init(initialData: Signal<PledgeViewData, Never>, userSessionStarted: Signal<(), Never>) { | ||
| let project = initialData.map(\.project) | ||
| let baseReward = initialData.map(\.rewards).map(\.first).skipNil() | ||
| let refTag = initialData.map(\.refTag) |
There was a problem hiding this comment.
As a fun note - pulling this out into its own component makes it a much more obvious that we're passing around extra data we don't need. The pledge payment methods page doesn't need to know about refTag or baseReward, as far as I know. Once this is hooked up to both pledge controllers, refactoring those redundancies will be easier!
| .filter { !$3.paymentMethodsViewHidden } | ||
| .compactMap { project, reward, refTag, context -> PledgePaymentMethodsValue? in | ||
| guard let user = AppEnvironment.current.currentUser else { return nil } | ||
| return (user, project, "", reward, context, refTag) |
There was a problem hiding this comment.
I removed a defunct comment about NaN here.
| private let (creditCardSelectedSignal, creditCardSelectedObserver) = Signal<PaymentSourceSelected, Never> | ||
| .pipe() | ||
| public func creditCardSelected(with paymentSourceData: PaymentSourceSelected) { | ||
| self.creditCardSelectedObserver.send(value: paymentSourceData) |
There was a problem hiding this comment.
There's also some signals about stripe configuration, which I'm debating if they should live here or elsewhere. But we can always re-adjust later.
| self.useCase.selectedPaymentSource.observe(self.selectedPaymentSource.observer) | ||
| } | ||
|
|
||
| func test_LoggedInUser_SendsConfigValue_AndShowsPaymentMethods() { |
There was a problem hiding this comment.
To @ifosli's point, I'm trying to make these tests less of change detectors than our old pledge tests. They're not perfectly comprehensive, but they should exercise most of the important bits of the use case.
…ds signals in checkout
amy-at-kickstarter
force-pushed
the
feat/adyer/mbl-1972
branch
from
ffcacd1 to
b624703
Compare
| * `paymentMethodsViewHidden` - Whether or not to show the payment methods view. Sent at least once after `initialData` is sent. | ||
| * `selectedPaymentSource` - The currently selected credit card, or `nil` if no card is selected. Sent at least once after `initialData` is sent. | ||
| */ | ||
| public final class PaymentMethodsUseCase: PaymentMethodsUseCaseInputs, PaymentMethodsUseCaseOutputs { |
There was a problem hiding this comment.
Let's follow the VieModelType protocol pattern here
There was a problem hiding this comment.
Whoops! Missed that again, thanks for the eagle eyes
| * `selectedPaymentSource` - The currently selected credit card, or `nil` if no card is selected. Sent at least once after `initialData` is sent. | ||
| */ | ||
| public final class PaymentMethodsUseCase { | ||
| init(initialData: Signal<PledgeViewData, Never>, userSessionStarted: Signal<(), Never>) { |
There was a problem hiding this comment.
Splitting the comment up so that inputs and outputs have their own parts commented might make sense here
| .map { _ in AppEnvironment.current.currentUser } | ||
| .map { $0 != nil } |
There was a problem hiding this comment.
Nit: Could probably merge these
📲 What
Add
PaymentsMethodsUseCaseandPaymentMethodsUseCaseTests.n.B. that this does not hook up the use case - I'm holding off on installing it until our next release is cut.
🤔 Why
This puts all of the code related to creating/configuring/displaying a
PledgePaymentMethodsViewControllerin one place.More broadly: our goal is to simplify our checkout code. One way we can do this is by breaking it down into smaller, more understandable parts. We're following Android's lead here and building 'Use Cases', which are small chunks of code related to a single behavior or feature. These little pieces are much easier to understand, debug, and test.
This also means Late + Live pledges will be able to start sharing some of the code that's currently duplicated.