Merge pull request #7065 from woocommerce/issue/6961-refactor-new-order-flow

ealeksandrov · web-flow · commit 3a67956a167b · 2022-06-23T09:20:14.000+03:00
[Unified Order Editing] Update New Order view and viewmodel to use existing order
diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/FlowCoordinator/AddOrderCoordinator.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/FlowCoordinator/AddOrderCoordinator.swift
@@ -79,7 +79,7 @@ private extension AddOrderCoordinator {
     ///
     func presentNewOrderController() {
         let viewModel = NewOrderViewModel(siteID: siteID)
-        viewModel.onOrderCreated = onOrderCreated
+        viewModel.onFinished = onOrderCreated
 
         let viewController = NewOrderHostingController(viewModel: viewModel)
         if ServiceLocator.featureFlagService.isFeatureFlagEnabled(.splitViewInOrdersTab) {
diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/NewOrder.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/NewOrder.swift
@@ -44,7 +44,7 @@ final class NewOrderHostingController: UIHostingController<NewOrder> {
 ///
 extension NewOrderHostingController {
     override func shouldPopOnBackButton() -> Bool {
-        guard !viewModel.hasChanges else {
+        guard viewModel.canBeDismissed else {
             presentDiscardChangesActionSheet(onDiscard: { [weak self] in
                 self?.discardOrderAndPop()
             })
@@ -62,7 +62,7 @@ extension NewOrderHostingController {
 ///
 extension NewOrderHostingController: UIAdaptivePresentationControllerDelegate {
     func presentationControllerShouldDismiss(_ presentationController: UIPresentationController) -> Bool {
-        return !viewModel.hasChanges
+        return viewModel.canBeDismissed
     }
 
     func presentationControllerDidAttemptToDismiss(_ presentationController: UIPresentationController) {
@@ -131,7 +131,7 @@ struct NewOrder: View {
                 .ignoresSafeArea(.container, edges: [.horizontal])
             }
         }
-        .navigationTitle(Localization.title)
+        .navigationTitle(viewModel.title)
         .navigationBarTitleDisplayMode(.inline)
         .toolbar {
             ToolbarItem(placement: .cancellationAction) {
@@ -150,7 +150,10 @@ struct NewOrder: View {
                     .id(navigationButtonID)
                     .accessibilityIdentifier(Accessibility.createButtonIdentifier)
                     .disabled(viewModel.disabled)
-
+                case .done:
+                    Button(Localization.doneButton) {
+                        dismissHandler()
+                    }
                 case .loading:
                     ProgressView()
                 }
@@ -242,8 +245,8 @@ private extension NewOrder {
     }
 
     enum Localization {
-        static let title = NSLocalizedString("New Order", comment: "Title for the order creation screen")
         static let createButton = NSLocalizedString("Create", comment: "Button to create an order on the New Order screen")
+        static let doneButton = NSLocalizedString("Done", comment: "Button to dismiss the Order Editing screen")
         static let cancelButton = NSLocalizedString("Cancel", comment: "Button to cancel the creation of an order on the New Order screen")
         static let products = NSLocalizedString("Products", comment: "Title text of the section that shows the Products when creating a new order")
         static let addProduct = NSLocalizedString("Add Product", comment: "Title text of the button that adds a product when creating a new order")
diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/NewOrderViewModel.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/NewOrderViewModel.swift
@@ -16,16 +16,50 @@ final class NewOrderViewModel: ObservableObject {
 
     private var cancellables: Set<AnyCancellable> = []
 
+    enum Flow: Equatable {
+        case creation
+        case editing(initialOrder: Order)
+    }
+
+    /// Current flow. For editing stores existing order state prior to applying any edits.
+    ///
+    let flow: Flow
+
     /// Indicates whether user has made any changes
     ///
     var hasChanges: Bool {
-        orderSynchronizer.order != OrderFactory.emptyNewOrder
+        switch flow {
+        case .creation:
+            return orderSynchronizer.order != OrderFactory.emptyNewOrder
+        case .editing(let initialOrder):
+            return orderSynchronizer.order != initialOrder
+        }
+    }
+
+    /// Indicates whether view can be dismissed.
+    ///
+    var canBeDismissed: Bool {
+        switch flow {
+        case .creation: // Creation can be dismissed when there aren't changes pending to commit.
+            return !hasChanges
+        case .editing: // Editing can always be dismissed because changes are committed instantly.
+            return true
+        }
     }
 
     /// Indicates whether the cancel button is visible.
     ///
     var shouldShowCancelButton: Bool {
-        featureFlagService.isFeatureFlagEnabled(.splitViewInOrdersTab)
+        featureFlagService.isFeatureFlagEnabled(.splitViewInOrdersTab) && flow == .creation
+    }
+
+    var title: String {
+        switch flow {
+        case .creation:
+            return Localization.titleForNewOrder
+        case .editing(let order):
+            return String.localizedStringWithFormat(Localization.titleWithOrderNumber, order.number)
+        }
     }
 
     /// Active navigation bar trailing item.
@@ -46,9 +80,15 @@ final class NewOrderViewModel: ObservableObject {
 
     /// Order creation date. For new order flow it's always current date.
     ///
-    let dateString: String = {
-        DateFormatter.mediumLengthLocalizedDateFormatter.string(from: Date())
-    }()
+    var dateString: String {
+        switch flow {
+        case .creation:
+            return DateFormatter.mediumLengthLocalizedDateFormatter.string(from: Date())
+        case .editing(let order):
+            let formatter = DateFormatter.dateAndTimeFormatter
+            return formatter.string(from: order.dateCreated)
+        }
+    }
 
     /// Representation of order status display properties.
     ///
@@ -195,17 +235,23 @@ final class NewOrderViewModel: ObservableObject {
     private let orderSynchronizer: OrderSynchronizer
 
     init(siteID: Int64,
+         flow: Flow = .creation,
          stores: StoresManager = ServiceLocator.stores,
          storageManager: StorageManagerType = ServiceLocator.storageManager,
          currencySettings: CurrencySettings = ServiceLocator.currencySettings,
          analytics: Analytics = ServiceLocator.analytics,
          featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) {
         self.siteID = siteID
+        self.flow = flow
         self.stores = stores
         self.storageManager = storageManager
         self.currencyFormatter = CurrencyFormatter(currencySettings: currencySettings)
         self.analytics = analytics
-        self.orderSynchronizer = RemoteOrderSynchronizer(siteID: siteID, stores: stores, currencySettings: currencySettings)
+        if case let .editing(initialOrder) = flow {
+            self.orderSynchronizer = RemoteOrderSynchronizer(siteID: siteID, initialOrder: initialOrder, stores: stores, currencySettings: currencySettings)
+        } else {
+            self.orderSynchronizer = RemoteOrderSynchronizer(siteID: siteID, initialOrder: nil, stores: stores, currencySettings: currencySettings)
+        }
         self.featureFlagService = featureFlagService
 
         // Set a temporary initial view model, as a workaround to avoid making it optional.
@@ -300,7 +346,7 @@ final class NewOrderViewModel: ObservableObject {
 
             switch result {
             case .success(let newOrder):
-                self.onOrderCreated(newOrder)
+                self.onFinished(newOrder)
                 self.trackCreateOrderSuccess()
             case .failure(let error):
                 self.notice = NoticeFactory.createOrderErrorNotice(error, order: self.orderSynchronizer.order)
@@ -311,9 +357,11 @@ final class NewOrderViewModel: ObservableObject {
         trackCreateButtonTapped()
     }
 
-    /// Assign this closure to be notified when a new order is created
+    /// Assign this closure to be notified when the flow has finished.
+    /// For creation it means that the order has been created.
+    /// For edition it means that the merchant has finished editing the order.
     ///
-    var onOrderCreated: (Order) -> Void = { _ in }
+    var onFinished: (Order) -> Void = { _ in }
 
     /// Updates the order status & tracks its event
     ///
@@ -349,6 +397,7 @@ extension NewOrderViewModel {
     ///
     enum NavigationItem: Equatable {
         case create
+        case done
         case loading
     }
 
@@ -500,13 +549,18 @@ private extension NewOrderViewModel {
     /// Calculates what navigation trailing item should be shown depending on our internal state.
     ///
     func configureNavigationTrailingItem() {
-        Publishers.CombineLatest(orderSynchronizer.orderPublisher, $performingNetworkRequest)
-            .map { order, performingNetworkRequest -> NavigationItem in
+        Publishers.CombineLatest3(orderSynchronizer.orderPublisher, $performingNetworkRequest, Just(flow))
+            .map { order, performingNetworkRequest, flow -> NavigationItem in
                 guard !performingNetworkRequest else {
                     return .loading
                 }
 
-                return .create
+                switch flow {
+                case .creation:
+                    return .create
+                case .editing:
+                    return .done
+                }
             }
             .assign(to: &$navigationTrailingItem)
     }
@@ -841,6 +895,8 @@ extension NewOrderViewModel {
 
 private extension NewOrderViewModel {
     enum Localization {
+        static let titleForNewOrder = NSLocalizedString("New Order", comment: "Title for the order creation screen")
+        static let titleWithOrderNumber = NSLocalizedString("Order #%1$@", comment: "Order number title. Parameters: %1$@ - order number")
         static let errorMessageOrderCreation = NSLocalizedString("Unable to create new order", comment: "Notice displayed when order creation fails")
         static let errorMessageOrderSync = NSLocalizedString("Unable to load taxes for order",
                                                              comment: "Notice displayed when taxes cannot be synced for new order")
diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/Synchronizer/RemoteOrderSynchronizer.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/Synchronizer/RemoteOrderSynchronizer.swift
@@ -63,11 +63,18 @@ final class RemoteOrderSynchronizer: OrderSynchronizer {
 
     // MARK: Initializers
 
-    init(siteID: Int64, stores: StoresManager = ServiceLocator.stores, currencySettings: CurrencySettings = ServiceLocator.currencySettings) {
+    init(siteID: Int64,
+         initialOrder: Order? = nil,
+         stores: StoresManager = ServiceLocator.stores,
+         currencySettings: CurrencySettings = ServiceLocator.currencySettings) {
         self.siteID = siteID
         self.stores = stores
         self.currencyFormatter = CurrencyFormatter(currencySettings: currencySettings)
 
+        if let initialOrder = initialOrder {
+            order = initialOrder
+        }
+
         updateBaseSyncOrderStatus()
         bindInputs()
         bindOrderSync()
diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Details/OrderDetailsViewController.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Details/OrderDetailsViewController.swift
@@ -353,7 +353,14 @@ private extension OrderDetailsViewController {
     /// Presents the order edit form
     ///
     private func editOrder() {
-        // TODO: Implement
+        let viewModel = NewOrderViewModel(siteID: viewModel.order.siteID, flow: .editing(initialOrder: viewModel.order))
+        viewModel.onFinished = { [weak self] order in
+            guard let self = self else { return }
+            self.dismiss(animated: true)
+        }
+        let viewController = NewOrderHostingController(viewModel: viewModel)
+        let navController = UINavigationController(rootViewController: viewController)
+        present(navController, animated: true)
     }
 }
 
diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/NewOrderViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/NewOrderViewModelTests.swift
@@ -6,6 +6,7 @@ import WooFoundation
 final class NewOrderViewModelTests: XCTestCase {
 
     let sampleSiteID: Int64 = 123
+    let sampleOrderID: Int64 = 1234
     let sampleProductID: Int64 = 5
 
     func test_view_model_inits_with_expected_values() {
@@ -21,6 +22,17 @@ final class NewOrderViewModelTests: XCTestCase {
         XCTAssertEqual(viewModel.productRows.count, 0)
     }
 
+    func test_edition_view_model_has_a_navigation_done_button() {
+        // Given
+        let stores = MockStoresManager(sessionManager: .testingInstance)
+
+        // When
+        let viewModel = NewOrderViewModel(siteID: sampleSiteID, flow: .editing(initialOrder: .fake()), stores: stores)
+
+        // Then
+        XCTAssertEqual(viewModel.navigationTrailingItem, .done)
+    }
+
     func test_loading_indicator_is_enabled_during_network_request() {
         // Given
         let stores = MockStoresManager(sessionManager: .testingInstance)
@@ -1012,6 +1024,46 @@ final class NewOrderViewModelTests: XCTestCase {
         XCTAssertEqual(viewModel.addressFormViewModel.fields.lastName, "",
                        "Pending change was not discarded when address form was reset")
     }
+
+    func test_canBeDismissed_is_true_when_creating_order_without_changes() {
+        // Given
+        let viewModel = NewOrderViewModel(siteID: sampleSiteID)
+
+        // Then
+        XCTAssertTrue(viewModel.canBeDismissed)
+    }
+
+    func test_canBeDismissed_is_false_when_creating_order_with_changes() {
+        // Given
+        let viewModel = NewOrderViewModel(siteID: sampleSiteID)
+
+        // When
+        viewModel.updateOrderStatus(newStatus: .failed)
+
+        // Then
+        XCTAssertFalse(viewModel.canBeDismissed)
+    }
+
+    func test_canBeDismissed_is_true_when_editing_order_without_changes() {
+        // Given
+        let order = Order.fake().copy(orderID: sampleOrderID)
+        let viewModel = NewOrderViewModel(siteID: sampleSiteID, flow: .editing(initialOrder: order))
+
+        // Then
+        XCTAssertTrue(viewModel.canBeDismissed)
+    }
+
+    func test_canBeDismissed_is_true_when_editing_order_with_changes() {
+        // Given
+        let order = Order.fake().copy(orderID: sampleOrderID)
+        let viewModel = NewOrderViewModel(siteID: sampleSiteID, flow: .editing(initialOrder: order))
+
+        // When
+        viewModel.updateOrderStatus(newStatus: .failed)
+
+        // Then
+        XCTAssertTrue(viewModel.canBeDismissed)
+    }
 }
 
 private extension MockStorageManager {
