From a55e91eff8f298c06ecfcfe6695fd648ad465a78 Mon Sep 17 00:00:00 2001 From: Scott Clampet <110618242+scottkicks@users.noreply.github.com> Date: Wed, 3 Apr 2024 09:24:43 -0500 Subject: [PATCH 1/3] cta should remain disabled until payment method has been selected --- .../PostCampaignCheckoutViewModel.swift | 60 +++++++++++++++++-- 1 file changed, 54 insertions(+), 6 deletions(-) diff --git a/Library/ViewModels/PostCampaignCheckoutViewModel.swift b/Library/ViewModels/PostCampaignCheckoutViewModel.swift index c67c4815d2..d149c1bb21 100644 --- a/Library/ViewModels/PostCampaignCheckoutViewModel.swift +++ b/Library/ViewModels/PostCampaignCheckoutViewModel.swift @@ -86,8 +86,9 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, .skipNil() let context = initialData.map(\.context) + let project = initialData.map(\.project) let checkoutId = initialData.map(\.checkoutId) - let baseReward = initialData.map(\.rewards).map(\.first) + let baseReward = initialData.map(\.rewards).map(\.first).skipNil() let configurePaymentMethodsData = Signal.merge( initialData, @@ -109,14 +110,40 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, .map { _ in AppEnvironment.current.currentUser } .map(isNotNil) + let notChangingPaymentMethod = context.map { context in + context.isUpdating && context != .changePaymentMethod + } + .filter(isTrue) + + let paymentMethodSelected = self.creditCardSelectedProperty.signal.skipNil() + + let paymentMethodChangedAndValid = Signal.merge( + notChangingPaymentMethod.mapConst(false), + Signal.combineLatest( + project, + baseReward, + paymentMethodSelected.map { (source: PaymentSourceSelected, _, _) in source }, + context + ) + .map(paymentMethodValid) + ) + + let isEnabled = Signal.merge( + self.viewDidLoadProperty.signal.mapConst(false) + .take(until: paymentMethodChangedAndValid.ignoreValues()), + paymentMethodChangedAndValid + ) + .skipRepeats() + self.configurePledgeViewCTAContainerView = Signal.combineLatest( isLoggedIn, + isEnabled, context ) - .map { isLoggedIn, context in + .map { isLoggedIn, isEnabled, context in PledgeViewCTAContainerViewData( isLoggedIn: isLoggedIn, - isEnabled: true, // Pledge button never needs to be disabled on checkout page. + isEnabled: isEnabled, context: context, willRetryPaymentMethod: false // Only retry in the `fixPaymentMethod` context. ) @@ -394,9 +421,7 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, let thanksPageData = Signal.combineLatest(initialData, baseReward) .map { initialData, baseReward -> ThanksPageData? in - guard let reward = baseReward else { return nil } - - return (initialData.project, reward, nil, initialData.total) + (initialData.project, baseReward, nil, initialData.total) } self.checkoutComplete = thanksPageData.skipNil() @@ -514,3 +539,26 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, public var inputs: PostCampaignCheckoutViewModelInputs { return self } public var outputs: PostCampaignCheckoutViewModelOutputs { return self } } + +private func paymentMethodValid( + project: Project, + reward: Reward, + paymentSource: PaymentSourceSelected, + context: PledgeViewContext +) -> Bool { + guard + let backedPaymentSourceId = project.personalization.backing?.paymentSource?.id, + context.isUpdating, + userIsBacking(reward: reward, inProject: project) + else { + return true + } + + if project.personalization.backing?.status == .errored { + return true + } else if backedPaymentSourceId != paymentSource.savedCreditCardId { + return true + } + + return false +} From c928d9432a239accaf27179188f4727a3817bbe4 Mon Sep 17 00:00:00 2001 From: Scott Clampet <110618242+scottkicks@users.noreply.github.com> Date: Wed, 3 Apr 2024 10:02:27 -0500 Subject: [PATCH 2/3] add tests --- .../PostCampaignCheckoutViewModelTests.swift | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/Library/ViewModels/PostCampaignCheckoutViewModelTests.swift b/Library/ViewModels/PostCampaignCheckoutViewModelTests.swift index 804cbbf378..4573079296 100644 --- a/Library/ViewModels/PostCampaignCheckoutViewModelTests.swift +++ b/Library/ViewModels/PostCampaignCheckoutViewModelTests.swift @@ -14,6 +14,10 @@ final class PostCampaignCheckoutViewModelTests: TestCase { fileprivate let checkoutComplete = TestObserver() fileprivate let processingViewIsHidden = TestObserver() fileprivate let validateCheckoutSuccess = TestObserver() + + private let configurePledgeViewCTAContainerViewIsLoggedIn = TestObserver() + private let configurePledgeViewCTAContainerViewIsEnabled = TestObserver() + private let configurePledgeViewCTAContainerViewContext = TestObserver() override func setUp() { super.setUp() @@ -21,6 +25,13 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.vm.checkoutComplete.observe(self.checkoutComplete.observer) self.vm.processingViewIsHidden.observe(self.processingViewIsHidden.observer) self.vm.validateCheckoutSuccess.observe(self.validateCheckoutSuccess.observer) + + self.vm.outputs.configurePledgeViewCTAContainerView.map { $0.0 } + .observe(self.configurePledgeViewCTAContainerViewIsLoggedIn.observer) + self.vm.outputs.configurePledgeViewCTAContainerView.map { $0.1 } + .observe(self.configurePledgeViewCTAContainerViewIsEnabled.observer) + self.vm.outputs.configurePledgeViewCTAContainerView.map { $0.2 } + .observe(self.configurePledgeViewCTAContainerViewContext.observer) } func testApplePayAuthorization_noReward_isCorrect() { @@ -385,8 +396,14 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.vm.viewDidLoad() let paymentSource = PaymentSourceSelected.paymentIntentClientSecret("123") + self.vm.inputs .creditCardSelected(source: paymentSource, paymentMethodId: "123", isNewPaymentMethod: true) + + self.configurePledgeViewCTAContainerViewIsLoggedIn.assertValues([false, false]) + self.configurePledgeViewCTAContainerViewIsEnabled.assertValues([false, true]) + self.configurePledgeViewCTAContainerViewContext.assertValues([.latePledge, .latePledge]) + self.vm.inputs.submitButtonTapped() self.processingViewIsHidden.assertLastValue(false) @@ -400,6 +417,7 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.checkoutComplete.assertDidEmitValue() self.processingViewIsHidden.assertLastValue(true) + } } @@ -431,6 +449,11 @@ final class PostCampaignCheckoutViewModelTests: TestCase { let paymentSource = PaymentSourceSelected.paymentIntentClientSecret("123") self.vm.inputs .creditCardSelected(source: paymentSource, paymentMethodId: "123", isNewPaymentMethod: true) + + self.configurePledgeViewCTAContainerViewIsLoggedIn.assertValues([false, false]) + self.configurePledgeViewCTAContainerViewIsEnabled.assertValues([false, true]) + self.configurePledgeViewCTAContainerViewContext.assertValues([.latePledge, .latePledge]) + self.vm.inputs.submitButtonTapped() self.processingViewIsHidden.assertLastValue(false) @@ -471,6 +494,11 @@ final class PostCampaignCheckoutViewModelTests: TestCase { let paymentSource = PaymentSourceSelected.paymentIntentClientSecret("123") self.vm.inputs .creditCardSelected(source: paymentSource, paymentMethodId: "123", isNewPaymentMethod: true) + + self.configurePledgeViewCTAContainerViewIsLoggedIn.assertValues([false, false]) + self.configurePledgeViewCTAContainerViewIsEnabled.assertValues([false, true]) + self.configurePledgeViewCTAContainerViewContext.assertValues([.latePledge, .latePledge]) + self.vm.inputs.submitButtonTapped() self.processingViewIsHidden.assertLastValue(false) @@ -485,4 +513,36 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.processingViewIsHidden.assertLastValue(true) } } + + func testPledgeViewCTA_LoggedIn_State() { + let mockService = MockService(serverConfig: ServerConfig.staging) + + withEnvironment(apiService: mockService, currentUser: .template) { + let project = Project.cosmicSurgery + let reward = Reward.noReward |> Reward.lens.minimum .~ 5 + + let data = PostCampaignCheckoutData( + project: project, + rewards: [reward], + selectedQuantities: [:], + bonusAmount: 0, + total: 5, + shipping: nil, + refTag: nil, + context: .latePledge, + checkoutId: "0" + ) + + self.vm.inputs.configure(with: data) + self.vm.inputs.viewDidLoad() + + let paymentSource = PaymentSourceSelected.paymentIntentClientSecret("123") + self.vm.inputs + .creditCardSelected(source: paymentSource, paymentMethodId: "123", isNewPaymentMethod: true) + + self.configurePledgeViewCTAContainerViewIsLoggedIn.assertValues([true, true]) + self.configurePledgeViewCTAContainerViewIsEnabled.assertValues([false, true]) + self.configurePledgeViewCTAContainerViewContext.assertValues([.latePledge, .latePledge]) + } + } } From 6a49bab01cc37596d178ba6ca5c13080db7a744f Mon Sep 17 00:00:00 2001 From: Scott Clampet <110618242+scottkicks@users.noreply.github.com> Date: Wed, 3 Apr 2024 11:13:57 -0500 Subject: [PATCH 3/3] simplify based on pr feedback --- .../PostCampaignCheckoutViewModel.swift | 60 ++++--------------- .../PostCampaignCheckoutViewModelTests.swift | 23 ++++--- 2 files changed, 22 insertions(+), 61 deletions(-) diff --git a/Library/ViewModels/PostCampaignCheckoutViewModel.swift b/Library/ViewModels/PostCampaignCheckoutViewModel.swift index d149c1bb21..4054606087 100644 --- a/Library/ViewModels/PostCampaignCheckoutViewModel.swift +++ b/Library/ViewModels/PostCampaignCheckoutViewModel.swift @@ -88,7 +88,7 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, let context = initialData.map(\.context) let project = initialData.map(\.project) let checkoutId = initialData.map(\.checkoutId) - let baseReward = initialData.map(\.rewards).map(\.first).skipNil() + let baseReward = initialData.map(\.rewards).map(\.first) let configurePaymentMethodsData = Signal.merge( initialData, @@ -110,40 +110,23 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, .map { _ in AppEnvironment.current.currentUser } .map(isNotNil) - let notChangingPaymentMethod = context.map { context in - context.isUpdating && context != .changePaymentMethod - } - .filter(isTrue) - - let paymentMethodSelected = self.creditCardSelectedProperty.signal.skipNil() - - let paymentMethodChangedAndValid = Signal.merge( - notChangingPaymentMethod.mapConst(false), - Signal.combineLatest( - project, - baseReward, - paymentMethodSelected.map { (source: PaymentSourceSelected, _, _) in source }, - context - ) - .map(paymentMethodValid) - ) + let shouldEnablePledgeButton = self.creditCardSelectedProperty.signal.skipNil().mapConst(true) - let isEnabled = Signal.merge( - self.viewDidLoadProperty.signal.mapConst(false) - .take(until: paymentMethodChangedAndValid.ignoreValues()), - paymentMethodChangedAndValid + let pledgeButtonEnabled = Signal.merge( + self.viewDidLoadProperty.signal.mapConst(false), + shouldEnablePledgeButton ) .skipRepeats() self.configurePledgeViewCTAContainerView = Signal.combineLatest( isLoggedIn, - isEnabled, + pledgeButtonEnabled, context ) - .map { isLoggedIn, isEnabled, context in + .map { isLoggedIn, pledgeButtonEnabled, context in PledgeViewCTAContainerViewData( isLoggedIn: isLoggedIn, - isEnabled: isEnabled, + isEnabled: pledgeButtonEnabled, context: context, willRetryPaymentMethod: false // Only retry in the `fixPaymentMethod` context. ) @@ -421,7 +404,9 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, let thanksPageData = Signal.combineLatest(initialData, baseReward) .map { initialData, baseReward -> ThanksPageData? in - (initialData.project, baseReward, nil, initialData.total) + guard let reward = baseReward else { return nil } + + return (initialData.project, reward, nil, initialData.total) } self.checkoutComplete = thanksPageData.skipNil() @@ -539,26 +524,3 @@ public class PostCampaignCheckoutViewModel: PostCampaignCheckoutViewModelType, public var inputs: PostCampaignCheckoutViewModelInputs { return self } public var outputs: PostCampaignCheckoutViewModelOutputs { return self } } - -private func paymentMethodValid( - project: Project, - reward: Reward, - paymentSource: PaymentSourceSelected, - context: PledgeViewContext -) -> Bool { - guard - let backedPaymentSourceId = project.personalization.backing?.paymentSource?.id, - context.isUpdating, - userIsBacking(reward: reward, inProject: project) - else { - return true - } - - if project.personalization.backing?.status == .errored { - return true - } else if backedPaymentSourceId != paymentSource.savedCreditCardId { - return true - } - - return false -} diff --git a/Library/ViewModels/PostCampaignCheckoutViewModelTests.swift b/Library/ViewModels/PostCampaignCheckoutViewModelTests.swift index 4573079296..229532f004 100644 --- a/Library/ViewModels/PostCampaignCheckoutViewModelTests.swift +++ b/Library/ViewModels/PostCampaignCheckoutViewModelTests.swift @@ -14,7 +14,7 @@ final class PostCampaignCheckoutViewModelTests: TestCase { fileprivate let checkoutComplete = TestObserver() fileprivate let processingViewIsHidden = TestObserver() fileprivate let validateCheckoutSuccess = TestObserver() - + private let configurePledgeViewCTAContainerViewIsLoggedIn = TestObserver() private let configurePledgeViewCTAContainerViewIsEnabled = TestObserver() private let configurePledgeViewCTAContainerViewContext = TestObserver() @@ -25,7 +25,7 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.vm.checkoutComplete.observe(self.checkoutComplete.observer) self.vm.processingViewIsHidden.observe(self.processingViewIsHidden.observer) self.vm.validateCheckoutSuccess.observe(self.validateCheckoutSuccess.observer) - + self.vm.outputs.configurePledgeViewCTAContainerView.map { $0.0 } .observe(self.configurePledgeViewCTAContainerViewIsLoggedIn.observer) self.vm.outputs.configurePledgeViewCTAContainerView.map { $0.1 } @@ -396,14 +396,14 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.vm.viewDidLoad() let paymentSource = PaymentSourceSelected.paymentIntentClientSecret("123") - + self.vm.inputs .creditCardSelected(source: paymentSource, paymentMethodId: "123", isNewPaymentMethod: true) - + self.configurePledgeViewCTAContainerViewIsLoggedIn.assertValues([false, false]) self.configurePledgeViewCTAContainerViewIsEnabled.assertValues([false, true]) self.configurePledgeViewCTAContainerViewContext.assertValues([.latePledge, .latePledge]) - + self.vm.inputs.submitButtonTapped() self.processingViewIsHidden.assertLastValue(false) @@ -417,7 +417,6 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.checkoutComplete.assertDidEmitValue() self.processingViewIsHidden.assertLastValue(true) - } } @@ -449,11 +448,11 @@ final class PostCampaignCheckoutViewModelTests: TestCase { let paymentSource = PaymentSourceSelected.paymentIntentClientSecret("123") self.vm.inputs .creditCardSelected(source: paymentSource, paymentMethodId: "123", isNewPaymentMethod: true) - + self.configurePledgeViewCTAContainerViewIsLoggedIn.assertValues([false, false]) self.configurePledgeViewCTAContainerViewIsEnabled.assertValues([false, true]) self.configurePledgeViewCTAContainerViewContext.assertValues([.latePledge, .latePledge]) - + self.vm.inputs.submitButtonTapped() self.processingViewIsHidden.assertLastValue(false) @@ -494,11 +493,11 @@ final class PostCampaignCheckoutViewModelTests: TestCase { let paymentSource = PaymentSourceSelected.paymentIntentClientSecret("123") self.vm.inputs .creditCardSelected(source: paymentSource, paymentMethodId: "123", isNewPaymentMethod: true) - + self.configurePledgeViewCTAContainerViewIsLoggedIn.assertValues([false, false]) self.configurePledgeViewCTAContainerViewIsEnabled.assertValues([false, true]) self.configurePledgeViewCTAContainerViewContext.assertValues([.latePledge, .latePledge]) - + self.vm.inputs.submitButtonTapped() self.processingViewIsHidden.assertLastValue(false) @@ -513,7 +512,7 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.processingViewIsHidden.assertLastValue(true) } } - + func testPledgeViewCTA_LoggedIn_State() { let mockService = MockService(serverConfig: ServerConfig.staging) @@ -535,7 +534,7 @@ final class PostCampaignCheckoutViewModelTests: TestCase { self.vm.inputs.configure(with: data) self.vm.inputs.viewDidLoad() - + let paymentSource = PaymentSourceSelected.paymentIntentClientSecret("123") self.vm.inputs .creditCardSelected(source: paymentSource, paymentMethodId: "123", isNewPaymentMethod: true)