Merge pull request #5523 from woocommerce/issue/5483-error-notices-refactor

Author: Ecarrion

WooCommerce/Classes/ViewRelated/Orders/Order Creation/AddOrderCoordinator.swift

@@ -1,5 +1,6 @@
 import UIKit
 import Yosemite
+import Combine
 
 final class AddOrderCoordinator: Coordinator {
     var navigationController: UINavigationController
@@ -70,10 +71,11 @@ private extension AddOrderCoordinator {
     /// Presents `SimplePaymentsAmountHostingController`.
     ///
     func presentSimplePaymentsAmountController() {
-        let viewModel = SimplePaymentsAmountViewModel(siteID: siteID)
+        let presentNoticeSubject = PassthroughSubject<SimplePaymentsNotice, Never>()
+        let viewModel = SimplePaymentsAmountViewModel(siteID: siteID, presentNoticeSubject: presentNoticeSubject)
         viewModel.onOrderCreated = onOrderCreated
 
-        let viewController = SimplePaymentsAmountHostingController(viewModel: viewModel)
+        let viewController = SimplePaymentsAmountHostingController(viewModel: viewModel, presentNoticePublisher: presentNoticeSubject.eraseToAnyPublisher())
         let simplePaymentsNC = WooNavigationController(rootViewController: viewController)
         navigationController.present(simplePaymentsNC, animated: true)
 

WooCommerce/Classes/ViewRelated/Orders/Simple Payments/Amount/SimplePaymentsAmount.swift

@@ -14,33 +14,27 @@ final class SimplePaymentsAmountHostingController: UIHostingController<SimplePay
     ///
     private lazy var modalNoticePresenter: NoticePresenter = {
         let presenter = DefaultNoticePresenter()
-        presenter.presentingViewController = self
+        presenter.presentingViewController = self.navigationController ?? self
         return presenter
     }()
 
-    init(viewModel: SimplePaymentsAmountViewModel) {
+    init(viewModel: SimplePaymentsAmountViewModel, presentNoticePublisher: AnyPublisher<SimplePaymentsNotice, Never>) {
         super.init(rootView: SimplePaymentsAmount(viewModel: viewModel))
 
         // Needed because a `SwiftUI` cannot be dismissed when being presented by a UIHostingController
         rootView.dismiss = { [weak self] in
             self?.dismiss(animated: true, completion: nil)
         }
 
-        // Observe the present notice intent and set it back to `nil` after presented.
-        viewModel.$presentNotice
+        // Observe the present notice intent.
+        presentNoticePublisher
             .compactMap { $0 }
             .sink { [weak self] notice in
 
-                // To prevent keyboard to hide the notice
-                self?.view.endEditing(true)
-
                 switch notice {
-                case .error:
-                    self?.modalNoticePresenter.enqueue(notice: .init(title: SimplePaymentsAmount.Localization.error, feedbackType: .error))
+                case .error(let description):
+                    self?.modalNoticePresenter.enqueue(notice: .init(title: description, feedbackType: .error))
                 }
-
-                // Nullify the presentation intent.
-                viewModel.presentNotice = nil
             }
             .store(in: &subscriptions)
     }
@@ -153,7 +147,6 @@ private extension SimplePaymentsAmount {
         static let title = NSLocalizedString("Take Payment", comment: "Title for the simple payments screen")
         static let instructions = NSLocalizedString("Enter Amount", comment: "Short instructions label in the simple payments screen")
         static let cancelTitle = NSLocalizedString("Cancel", comment: "Title for the button to cancel the simple payments screen")
-        static let error = NSLocalizedString("There was an error creating the order", comment: "Notice text after failing to create a simple payments order.")
 
         static func buttonTitle() -> String {
             if ServiceLocator.featureFlagService.isFeatureFlagEnabled(.simplePaymentsPrototype) {
@@ -171,3 +164,9 @@ private extension SimplePaymentsAmount {
         }
     }
 }
+
+/// Representation of possible notices that can be displayed
+///
+enum SimplePaymentsNotice: Equatable {
+    case error(String)
+}

WooCommerce/Classes/ViewRelated/Orders/Simple Payments/Amount/SimplePaymentsAmountViewModel.swift

@@ -1,5 +1,6 @@
 import Foundation
 import Yosemite
+import Combine
 import Experiments
 
 /// View Model for the `SimplePaymentsAmount` view.
@@ -19,11 +20,6 @@ final class SimplePaymentsAmountViewModel: ObservableObject {
     ///
     @Published private(set) var loading: Bool = false
 
-    /// Defines the current notice that should be shown.
-    /// Defaults to `nil`.
-    ///
-    @Published var presentNotice: Notice?
-
     /// Defines if the view should navigate to the summary view.
     /// Setting it to `false` will `nil` the summary view model.
     ///
@@ -81,6 +77,10 @@ final class SimplePaymentsAmountViewModel: ObservableObject {
     ///
     private let userLocale: Locale
 
+    /// Transmits notice presentation intents.
+    ///
+    private let presentNoticeSubject: PassthroughSubject<SimplePaymentsNotice, Never>
+
     /// Current store currency settings
     ///
     private let storeCurrencySettings: CurrencySettings
@@ -100,12 +100,14 @@ final class SimplePaymentsAmountViewModel: ObservableObject {
     init(siteID: Int64,
          stores: StoresManager = ServiceLocator.stores,
          locale: Locale = Locale.autoupdatingCurrent,
+         presentNoticeSubject: PassthroughSubject<SimplePaymentsNotice, Never> = PassthroughSubject(),
          storeCurrencySettings: CurrencySettings = ServiceLocator.currencySettings,
          analytics: Analytics = ServiceLocator.analytics,
          isDevelopmentPrototype: Bool = ServiceLocator.featureFlagService.isFeatureFlagEnabled(FeatureFlag.simplePaymentsPrototype)) {
         self.siteID = siteID
         self.stores = stores
         self.userLocale = locale
+        self.presentNoticeSubject = presentNoticeSubject
         self.storeCurrencySettings = storeCurrencySettings
         self.storeCurrencySymbol = storeCurrencySettings.symbol(from: storeCurrencySettings.currencyCode)
         self.analytics = analytics
@@ -127,14 +129,16 @@ final class SimplePaymentsAmountViewModel: ObservableObject {
             switch result {
             case .success(let order):
                 if self.isDevelopmentPrototype {
-                    self.summaryViewModel = SimplePaymentsSummaryViewModel(order: order, providedAmount: self.amount)
+                    self.summaryViewModel = SimplePaymentsSummaryViewModel(order: order,
+                                                                           providedAmount: self.amount,
+                                                                           presentNoticeSubject: self.presentNoticeSubject)
                 } else {
                     self.onOrderCreated(order)
                 }
                 self.analytics.track(event: WooAnalyticsEvent.SimplePayments.simplePaymentsFlowCompleted(amount: order.total))
 
             case .failure(let error):
-                self.presentNotice = .error
+                self.presentNoticeSubject.send(.error(Localization.creationError))
                 self.analytics.track(event: WooAnalyticsEvent.SimplePayments.simplePaymentsFlowFailed())
                 DDLogError("⛔️ Error creating simple payments order: \(error)")
             }
@@ -190,10 +194,10 @@ private extension SimplePaymentsAmountViewModel {
     }
 }
 
-// MARK: Definitions
-extension SimplePaymentsAmountViewModel {
-    /// Representation of possible notices that can be displayed
-    enum Notice: Equatable {
-        case error
+// MARK: Constants
+private extension SimplePaymentsAmountViewModel {
+    enum Localization {
+        static let creationError = NSLocalizedString("There was an error creating the order",
+                                                     comment: "Notice text after failing to create a simple payments order.")
     }
 }

WooCommerce/Classes/ViewRelated/Orders/Simple Payments/Summary/SimplePaymentsSummaryViewModel.swift

@@ -1,5 +1,6 @@
 import Foundation
 import Yosemite
+import Combine
 
 /// `ViewModel` to drive the content of the `SimplePaymentsSummary` view.
 ///
@@ -61,6 +62,10 @@ final class SimplePaymentsSummaryViewModel: ObservableObject {
     ///
     private let feeID: Int64
 
+    /// Transmits notice presentation intents.
+    ///
+    private let presentNoticeSubject: PassthroughSubject<SimplePaymentsNotice, Never>
+
     /// Stores Manager.
     ///
     private let stores: StoresManager
@@ -76,11 +81,13 @@ final class SimplePaymentsSummaryViewModel: ObservableObject {
          siteID: Int64 = 0,
          orderID: Int64 = 0,
          feeID: Int64 = 0,
+         presentNoticeSubject: PassthroughSubject<SimplePaymentsNotice, Never> = PassthroughSubject(),
          currencyFormatter: CurrencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings),
          stores: StoresManager = ServiceLocator.stores) {
         self.siteID = siteID
         self.orderID = orderID
         self.feeID = feeID
+        self.presentNoticeSubject = presentNoticeSubject
         self.currencyFormatter = currencyFormatter
         self.stores = stores
         self.providedAmount = currencyFormatter.formatAmount(providedAmount) ?? providedAmount
@@ -109,6 +116,7 @@ final class SimplePaymentsSummaryViewModel: ObservableObject {
 
     convenience init(order: Order,
                      providedAmount: String,
+                     presentNoticeSubject: PassthroughSubject<SimplePaymentsNotice, Never> = PassthroughSubject(),
                      currencyFormatter: CurrencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings),
                      stores: StoresManager = ServiceLocator.stores) {
         self.init(providedAmount: providedAmount,
@@ -117,6 +125,7 @@ final class SimplePaymentsSummaryViewModel: ObservableObject {
                   siteID: order.siteID,
                   orderID: order.orderID,
                   feeID: order.fees.first?.feeID ?? 0,
+                  presentNoticeSubject: presentNoticeSubject,
                   currencyFormatter: currencyFormatter,
                   stores: stores)
     }
@@ -147,11 +156,18 @@ final class SimplePaymentsSummaryViewModel: ObservableObject {
                 // TODO: Analytics
                 break
             case .failure:
-                // TODO: Present notice
+                self.presentNoticeSubject.send(.error(Localization.updateError))
                 // TODO: Analytics
-                break
             }
         }
         stores.dispatch(action)
     }
 }
+
+// MARK: Constants
+private extension SimplePaymentsSummaryViewModel {
+    enum Localization {
+        static let updateError = NSLocalizedString("There was an error updating the order",
+                                                   comment: "Notice text after failing to update a simple payments order.")
+    }
+}

WooCommerce/WooCommerceTests/ViewRelated/Orders/Simple Payments/SimplePaymentsAmountViewModelTests.swift

@@ -1,5 +1,6 @@
 import Foundation
 import XCTest
+import Combine
 
 @testable import WooCommerce
 @testable import Yosemite
@@ -11,6 +12,8 @@ final class SimplePaymentsAmountViewModelTests: XCTestCase {
     private let usLocale = Locale(identifier: "en_US")
     private let usStoreSettings = CurrencySettings() // Default is US settings
 
+    private var subscriptions = Set<AnyCancellable>()
+
     func test_view_model_prepends_currency_symbol() {
         // Given
         let viewModel = SimplePaymentsAmountViewModel(siteID: sampleSiteID, locale: usLocale, storeCurrencySettings: usStoreSettings)
@@ -280,7 +283,8 @@ final class SimplePaymentsAmountViewModelTests: XCTestCase {
     func test_view_model_attempts_error_notice_presentation_when_failing_to_crete_order() {
         // Given
         let testingStore = MockStoresManager(sessionManager: .testingInstance)
-        let viewModel = SimplePaymentsAmountViewModel(siteID: sampleSiteID, stores: testingStore)
+        let noticeSubject = PassthroughSubject<SimplePaymentsNotice, Never>()
+        let viewModel = SimplePaymentsAmountViewModel(siteID: sampleSiteID, stores: testingStore, presentNoticeSubject: noticeSubject)
         testingStore.whenReceivingAction(ofType: OrderAction.self) { action in
             switch action {
             case let .createSimplePaymentsOrder(_, _, _, onCompletion):
@@ -291,13 +295,22 @@ final class SimplePaymentsAmountViewModelTests: XCTestCase {
         }
 
         // When
-        viewModel.createSimplePaymentsOrder()
+        let receivedError: Bool = waitFor { promise in
+            noticeSubject.sink { intent in
+                switch intent {
+                case .error:
+                    promise(true)
+                }
+            }
+            .store(in: &self.subscriptions)
+            viewModel.createSimplePaymentsOrder()
+        }
 
         // Then
-        XCTAssertEqual(viewModel.presentNotice, .error)
+        XCTAssertTrue(receivedError)
     }
 
-    func test_view_model_diable_cancel_button_while_creating_payment_order() {
+    func test_view_model_disable_cancel_button_while_creating_payment_order() {
         // Given
         let testingStore = MockStoresManager(sessionManager: .testingInstance)
         let viewModel = SimplePaymentsAmountViewModel(siteID: sampleSiteID, stores: testingStore)

WooCommerce/WooCommerceTests/ViewRelated/Orders/Simple Payments/SimplePaymentsSummaryViewModelTests.swift

@@ -108,4 +108,38 @@ final class SimplePaymentsSummaryViewModelTests: XCTestCase {
         // Then
         XCTAssertEqual(loadingStates, [true, false]) // Loading, then not loading.
     }
+
+    func test_view_model_attempts_error_notice_presentation_when_failing_to_update_order() {
+        // Given
+        let mockStores = MockStoresManager(sessionManager: .testingInstance)
+        let noticeSubject = PassthroughSubject<SimplePaymentsNotice, Never>()
+        let viewModel = SimplePaymentsSummaryViewModel(providedAmount: "1.0",
+                                                       totalWithTaxes: "1.0",
+                                                       taxAmount: "0.0",
+                                                       presentNoticeSubject: noticeSubject,
+                                                       stores: mockStores)
+        mockStores.whenReceivingAction(ofType: OrderAction.self) { action in
+            switch action {
+            case let .updateSimplePaymentsOrder(_, _, _, _, _, _, _, onCompletion):
+                onCompletion(.failure(NSError(domain: "Error", code: 0)))
+            default:
+                XCTFail("Received unsupported action: \(action)")
+            }
+        }
+
+        // When
+        let receivedError: Bool = waitFor { promise in
+            noticeSubject.sink { intent in
+                switch intent {
+                case .error:
+                    promise(true)
+                }
+            }
+            .store(in: &self.subscriptions)
+            viewModel.updateOrder()
+        }
+
+        // Then
+        XCTAssertTrue(receivedError)
+    }
 }