[Woo POS] Always create a new order when checking out (#15427)

staskus · web-flow · commit 5aa86247e329 · 2025-03-28T11:37:45.000+02:00
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -5,6 +5,7 @@
 -----
 - [*] Order Creation: Resolved an issue where the exit confirmation was not displayed when there were unsaved changes in a split view [https://github.com/woocommerce/woocommerce-ios/pull/15394].
 - [*] Order Creation: Fixed an issue where order recalculation would stop working after canceling a confirmation with unsaved changes [https://github.com/woocommerce/woocommerce-ios/pull/15392].
+- [internal] POS: Always create a new order during the checkout [https://github.com/woocommerce/woocommerce-ios/pull/15427].
 - [internal] Improve data fetching in Order Details, to avoid I/O performance on the main thread. [https://github.com/woocommerce/woocommerce-ios/pull/14999]
 
 22.0
diff --git a/WooCommerce/Classes/POS/Controllers/PointOfSaleOrderController.swift b/WooCommerce/Classes/POS/Controllers/PointOfSaleOrderController.swift
@@ -18,7 +18,6 @@ import protocol WooFoundation.Analytics
 
 enum SyncOrderState {
     case newOrder
-    case orderUpdated
     case orderNotChanged
 }
 
@@ -75,27 +74,20 @@ protocol PointOfSaleOrderControllerProtocol {
         }
 
         orderState = .syncing
-        let isNewOrder = order == nil
 
         do {
             let syncedOrder = try await orderService.syncOrder(cart: posCart,
-                                                               order: order,
                                                                currency: storeCurrency)
             self.order = syncedOrder
             orderState = .loaded(totals(for: syncedOrder), syncedOrder)
-            if isNewOrder {
-                analytics.track(.orderCreationSuccess)
-                return .success(.newOrder)
-            } else {
-                return .success(.orderUpdated)
-            }
+            analytics.track(.orderCreationSuccess)
+            return .success(.newOrder)
         } catch {
-            if isNewOrder {
-                analytics.track(event: WooAnalyticsEvent.Orders.orderCreationFailed(
-                    usesGiftCard: false,
-                    errorContext: String(describing: error),
-                    errorDescription: error.localizedDescription))
-            }
+            self.order = nil
+            analytics.track(event: WooAnalyticsEvent.Orders.orderCreationFailed(
+                usesGiftCard: false,
+                errorContext: String(describing: error),
+                errorDescription: error.localizedDescription))
             setOrderStateToError(error, retryHandler: retryHandler)
             return .failure(SyncOrderStateError.syncFailure)
         }
diff --git a/WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift b/WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift
@@ -172,13 +172,13 @@ private extension PointOfSaleAggregateModel {
         }
     }
 
-    // Tracks when the order is created or updated successfully
+    // Tracks when the order is created successfully
     // pdfdoF-6hn#comment-7625-p2
     func trackOrderSyncState(_ result: Result<SyncOrderState, Error>) {
         switch result {
         case .success(let syncState):
             switch syncState {
-            case .newOrder, .orderUpdated:
+            case .newOrder:
                 collectOrderPaymentAnalyticsTracker.trackOrderSyncSuccess()
             default:
                 break
diff --git a/WooCommerce/WooCommerceTests/POS/Controllers/PointOfSaleOrderControllerTests.swift b/WooCommerce/WooCommerceTests/POS/Controllers/PointOfSaleOrderControllerTests.swift
@@ -299,11 +299,10 @@ struct PointOfSaleOrderControllerTests {
     }
 
     @available(iOS 17.0, *)
-    @Test func syncOrder_when_updating_existing_order_returns_orderUpdated_result() async throws {
+    @Test func syncOrder_when_updating_existing_order_returns_newOrder_result() async throws {
         // Given
         let sut = PointOfSaleOrderController(orderService: mockOrderService,
                                              receiptService: mockReceiptService)
-        let fakeOrderItem = OrderItem.fake().copy(quantity: 1)
         let fakeOrder = Order.fake()
         mockOrderService.orderToReturn = fakeOrder
 
@@ -316,9 +315,9 @@ struct PointOfSaleOrderControllerTests {
 
         // Then
         if case .success(let state) = result {
-            #expect(state == .orderUpdated)
+            #expect(state == .newOrder)
         } else {
-            #expect(Bool(false), "Expected success result with order updated")
+            #expect(Bool(false), "Expected success result with new order")
         }
     }
 
@@ -524,6 +523,51 @@ struct PointOfSaleOrderControllerTests {
         ])
     }
 
+    @available(iOS 17.0, *)
+    @Test func syncOrder_when_fails_sets_order_to_nil() async throws {
+        // Given
+        let sut = PointOfSaleOrderController(orderService: mockOrderService,
+                                             receiptService: mockReceiptService)
+
+        // First create a successful order
+        let orderItem = OrderItem.fake().copy(quantity: 1)
+        let fakeOrder = Order.fake().copy(items: [orderItem])
+        let cartItem = makeItem(orderItemsToMatch: [orderItem])
+        mockOrderService.orderToReturn = fakeOrder
+
+        // Initial sync succeeds
+        let initialResult = await sut.syncOrder(for: .init(items: [cartItem]), retryHandler: {})
+        switch initialResult {
+        case .success(.newOrder):
+            break
+        default:
+            #expect(Bool(false), "Expected success with new order, got \(initialResult)")
+        }
+
+        // Then simulate a failure
+        mockOrderService.errorToReturn = SyncOrderStateError.syncFailure
+        let failureResult = await sut.syncOrder(for: .init(items: [cartItem, cartItem]), retryHandler: {})
+        switch failureResult {
+        case .failure(SyncOrderStateError.syncFailure):
+            break
+        default:
+            #expect(Bool(false), "Expected sync failure, got \(failureResult)")
+        }
+
+        // When - try syncing with the same cart again
+        mockOrderService.errorToReturn = nil
+        mockOrderService.orderToReturn = fakeOrder // Restore mock to return success
+        let subsequentResult = await sut.syncOrder(for: .init(items: [cartItem]), retryHandler: {})
+
+        // Then - should be treated as new order since previous order was cleared
+        switch subsequentResult {
+        case .success(.newOrder):
+            break
+        default:
+            #expect(Bool(false), "Expected new order after failure cleared previous order, got \(subsequentResult)")
+        }
+    }
+
     struct AnalyticsTests {
         private let analytics: WooAnalytics
         private let analyticsProvider = MockAnalyticsProvider()
diff --git a/WooCommerce/WooCommerceTests/POS/Mocks/MockPOSOrderService.swift b/WooCommerce/WooCommerceTests/POS/Mocks/MockPOSOrderService.swift
@@ -12,7 +12,6 @@ class MockPOSOrderService: POSOrderServiceProtocol {
     var spySyncOrderCurrency: CurrencyCode?
 
     func syncOrder(cart: Yosemite.POSCart,
-                   order: Yosemite.Order?,
                    currency: CurrencyCode) async throws -> Yosemite.Order {
         syncOrderWasCalled = true
         spySyncOrderCurrency = currency
diff --git a/Yosemite/Yosemite/Tools/POS/POSOrderService.swift b/Yosemite/Yosemite/Tools/POS/POSOrderService.swift
@@ -7,9 +7,8 @@ public protocol POSOrderServiceProtocol {
     /// Syncs order based on the cart.
     /// - Parameters:
     ///   - cart: Cart with different types of items and quantities.
-    ///   - order: Optional latest remotely synced order. Nil when syncing order for the first time.
     /// - Returns: Order from the remote sync.
-    func syncOrder(cart: POSCart, order: Order?, currency: CurrencyCode) async throws -> Order
+    func syncOrder(cart: POSCart, currency: CurrencyCode) async throws -> Order
     func updatePOSOrder(order: Order, recipientEmail: String) async throws
 }
 
@@ -36,19 +35,14 @@ public final class POSOrderService: POSOrderServiceProtocol {
     // MARK: - Protocol conformance
 
     public func syncOrder(cart: POSCart,
-                          order posOrder: Order?,
                           currency: CurrencyCode) async throws -> Order {
-        let initialOrder: Order = posOrder ?? OrderFactory.newOrder(currency: currency)
-            .copy(siteID: siteID,
-                  status: .autoDraft)
-        let order = updateOrder(initialOrder, cart: cart).sanitizingLocalItems()
-        let syncedOrder: Order
-        if posOrder != nil {
-            syncedOrder = try await ordersRemote.updatePOSOrder(siteID: siteID, order: order, fields: [.items, .couponLines])
-        } else {
-            syncedOrder = try await ordersRemote.createPOSOrder(siteID: siteID, order: order, fields: [.items, .status, .currency, .couponLines])
-        }
-        return syncedOrder
+        let order = OrderFactory
+            .newOrder(currency: currency)
+            .copy(siteID: siteID, status: .autoDraft)
+            .addItems(cart.items)
+            .addCoupons(cart.coupons)
+
+        return try await ordersRemote.createPOSOrder(siteID: siteID, order: order, fields: [.items, .status, .currency, .couponLines])
     }
 
     public func updatePOSOrder(order: Order, recipientEmail: String) async throws {
@@ -66,73 +60,19 @@ public final class POSOrderService: POSOrderServiceProtocol {
     }
 }
 
-private struct POSOrderSyncProductType: OrderSyncProductTypeProtocol, Hashable {
-    let productID: Int64
-    let price: String
-    // Not used in POS but have to be included for the app usage.
-    let productType: ProductType
-    let bundledItems: [ProductBundleItem]
-
-    init(productID: Int64, price: String, productType: ProductType, bundledItems: [ProductBundleItem] = []) {
-        self.productID = productID
-        self.price = price
-        self.productType = productType
-        self.bundledItems = bundledItems
-    }
-}
-
-private extension POSOrderService {
-    func updateOrder(_ order: Order, cart: POSCart) -> Order {
-        // Removes all existing items by setting quantity to 0.
-        let itemsToRemove = order.items.compactMap {
-            Self.removalProductInput(item: $0)
-        }
-
-        // Adds items from the latest cart grouping by item.
-        let itemsToAdd = cart.items.createGroupedOrderSyncProductInputs().values
-        let itemsToSync = itemsToRemove + itemsToAdd
-
-        var order = ProductInputTransformer.updateMultipleItems(with: itemsToSync, on: order, shouldUpdateOrDeleteZeroQuantities: .update)
-        order = updateCoupons(cart.coupons, on: order)
-
-        return order
+private extension Order {
+    func addItems(_ cartItems: [POSCartItem]) -> Order {
+        let itemsToAdd = Array(cartItems.createGroupedOrderSyncProductInputs().values)
+        return ProductInputTransformer
+            .updateMultipleItems(with: itemsToAdd, on: self, shouldUpdateOrDeleteZeroQuantities: .update)
+            .sanitizingLocalItems()
     }
 
-    func updateCoupons(_ coupons: [POSCoupon], on order: Order) -> Order {
-        // Get coupon codes from cart
-        let cartCouponCodes = Set(coupons.map { $0.code })
-
-        // Keep existing coupons that are still in the cart
-        let remainingCoupons = order.coupons.filter { orderCoupon in
-            cartCouponCodes.contains(orderCoupon.code)
-        }
-
-        // Find new coupons that need to be added (in cart but not in order)
-        let existingCouponCodes = Set(order.coupons.map { $0.code })
+    func addCoupons(_ coupons: [POSCoupon]) -> Order {
         let newCoupons = coupons
-            .filter { !existingCouponCodes.contains($0.code) }
             .map { OrderFactory.newOrderCouponLine(code: $0.code) }
 
-        // Update order with remaining + new coupons
-        return order.copy(coupons: remainingCoupons + newCoupons)
-    }
-
-    /// Creates a new `OrderSyncProductInput` type meant to remove an existing item from `OrderSynchronizer`
-    ///
-    static func removalProductInput(item: OrderItem) -> OrderSyncProductInput? {
-        let productForRemoval = POSProductForRemoval(productID: item.productID)
-        // Return a new input with the new quantity but with the same item id to properly reference the update.
-        return OrderSyncProductInput(id: item.itemID,
-                                     product: .product(productForRemoval),
-                                     quantity: 0)
-    }
-
-    /// A simplified product struct, intended to contain only the `productID`
-    struct POSProductForRemoval: OrderSyncProductTypeProtocol {
-        var price: String = ""
-        var productID: Int64
-        var productType: ProductType = .simple
-        var bundledItems: [ProductBundleItem] = []
+        return self.copy(coupons: newCoupons)
     }
 }
 
diff --git a/Yosemite/YosemiteTests/Tools/POS/POSOrderServiceTests.swift b/Yosemite/YosemiteTests/Tools/POS/POSOrderServiceTests.swift
