MBL-1285: Implement CompleteOnSessionCheckoutMutation for new and existing cards #2005
Conversation
amy-at-kickstarter
force-pushed
the
feat/adyer/MBL-1285/completeonsessioncheckout-new-and-existing-cards
branch
2 times, most recently
from
978d902 to
f68029a
Compare
amy-at-kickstarter
changed the title
|
|
||
| #if DEBUG | ||
| let serverError = error.errorMessages.first ?? "" | ||
| let message = "\(Strings.Something_went_wrong_please_try_again())\n\(serverError)" |
| @@ -1,6 +1,6 @@ | |||
| import Foundation | |||
|
|
|||
| extension GraphAPI.ApplePayInput { | |||
| public extension GraphAPI.ApplePayInput { | |||
There was a problem hiding this comment.
nit: Should this change be in this pr?
There was a problem hiding this comment.
Nope! Fixed.
| @@ -335,7 +343,57 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, | |||
|
|
|||
| // MARK: CompleteOnSessionCheckout | |||
|
|
|||
| // TODO: Call .completeOnSessionCheckout when self.confirmPaymentSuccessfulProperty.signal is called | |||
| let completeCheckoutWithExistingCardInput: Signal<GraphAPI.CompleteOnSessionCheckoutInput, Never> = Signal | |||
| .combineLatest(self.confirmPaymentSuccessfulProperty.signal.skipNil(), checkoutId, selectedExistingCard) | |||
There was a problem hiding this comment.
I think this is going to have the same issue as before. Let's say I first select an existing card, then add a new card. Now both selectedExistingCard and selectedNewCard will have values set (the existing one is outdated, but it doesn't know that). Once confirmPaymentSuccessfulProperty has an updated value, both this and the new card version of it will fire. I'm pretty sure that, in order to prevent this from happening, we need to add a filter after the combineLatest.
Alternatively, if we want to be able to just use selectedNewCard/selectedExistingCard we could map the opposite version to nil and filter on that (or return early/map to nil in the map/ect). Ie make sure that these signals won't both have valid cards at the same time.
For this specific call, though, I think it'd be easiest to just have one completeCheckoutWithCardInput method and switch on card.isNewPaymentMethod for the one field that's different.
| AppEnvironment.current.apiService.completeOnSessionCheckout(input: input).materialize() | ||
| } | ||
|
|
||
| self.checkoutComplete = checkoutCompleteSignal.signal.ignoreValues() |
There was a problem hiding this comment.
nit: I'm not sure about this, but I think this might need to be checkoutCompleteSignal.signal.values().ignoreValues() in order to prevent it from also firing if there's an error. Will you test it if you haven't already?
There was a problem hiding this comment.
Ha, yup, you're right. Happened to me a few times.
amy-at-kickstarter
force-pushed
the
feat/adyer/MBL-1285/completeonsessioncheckout-new-and-existing-cards
branch
from
cbd6aa2 to
a7390cb
Compare
amy-at-kickstarter
force-pushed
the
feat/adyer/MBL-1285/completeonsessioncheckout-new-and-existing-cards
branch
from
a7390cb to
0dae26b
Compare
amy-at-kickstarter
deleted the
feat/adyer/MBL-1285/completeonsessioncheckout-new-and-existing-cards
branch
📲 What
Implement
CompleteOnSessionCheckoutMutationfor new and existing cards in the late-pledge flow🤔 Why
This is the last mutation call required to actually complete the pledge!
This PR is intentionally incomplete, to keep the changes a bit more manageable. Follow-up PRs will do the following:
CompleteOnSessionCheckoutMutationfor ApplePay👀 See