14602 Cancel payment when order marked complete

Adds an async `cancelPayment` function to the CardPresentPaymentFacade. This isn’t as comprehensive as the fire-and-forget version, which remains, as it can’t cancel if it’s called during non-payment events, e.g. connection and searching.

We use the new async cancel to stop card payments from happening when a cash payment is accepted; it happens right before we mark the order complete.

Currently failure to cancel is unhandled; it would be good to improve that!

The state representation is incomplete with this approach. We should keep hold of the card payment state, if it’s still live in the background… and maybe even if it’s not. I’ve made suggestions for this in the comments.

Another (maybe better?) option (commented out in this commit) is to cancel card payments as soon as the cash option is selected.

WooCommerce/Classes/POS/Card Present Payments/CardPresentPaymentFacade.swift

@@ -42,4 +42,7 @@ protocol CardPresentPaymentFacade {
 
     /// Cancels any in-progress payment.
     func cancelPayment()
+
+    /// Cancels any in-progress payment, returning when complete
+    func cancelPayment() async throws
 }

WooCommerce/Classes/POS/Card Present Payments/CardPresentPaymentService.swift

@@ -104,7 +104,7 @@ final class CardPresentPaymentService: CardPresentPaymentFacade {
     func disconnectReader() async {
         readerConnectionStatusSubject.send(.disconnecting)
 
-        cancelPayment()
+        try? await cancelPayment()
 
         connectionControllerManager.knownReaderProvider.forgetCardReader()
 
@@ -161,6 +161,18 @@ final class CardPresentPaymentService: CardPresentPaymentFacade {
     func cancelPayment() {
         paymentTask?.cancel()
     }
+
+    @MainActor
+    func cancelPayment() async throws {
+        try await withCheckedThrowingContinuation { continuation in
+            var nillableContinuation: CheckedContinuation<Void, any Error>? = continuation
+            let action = CardPresentPaymentAction.cancelPayment { result in
+                nillableContinuation?.resume(with: result)
+                nillableContinuation = nil
+            }
+            stores.dispatch(action)
+        }
+    }
 }
 
 private extension CardPresentPaymentService {
@@ -213,4 +225,5 @@ enum CardPresentPaymentServiceError: Error {
     case invalidAmount
     case unknownPaymentError(underlyingError: Error)
     case incompleteAddressConnectionError
+    case couldNotCancelPayment
 }

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift

@@ -61,7 +61,6 @@ class PointOfSaleAggregateModel: ObservableObject, PointOfSaleAggregateModelProt
     private let orderController: PointOfSaleOrderControllerProtocol
     private let analytics: Analytics
 
-    private var isCashPaymentInProgress: Bool = false
     private var startPaymentOnCardReaderConnection: AnyCancellable?
     private var cardReaderDisconnection: AnyCancellable?
 
@@ -130,7 +129,6 @@ extension PointOfSaleAggregateModel {
     }
 
     func startNewCart() {
-        isCashPaymentInProgress = false
         removeAllItemsFromCart()
         orderController.clearOrder()
         setStateForEditing()
@@ -202,13 +200,38 @@ extension PointOfSaleAggregateModel {
     }
 
     func startCashPayment() {
-        isCashPaymentInProgress = true
-        paymentState = .cash(.collectingCash)
+        // Uncomment the lines below to prevent card payments from as soon as the button to open cash payment entry is tapped.
+//        Task { @MainActor [weak self] in
+//            guard let self else { return }
+//            try? await cardPresentPaymentService.cancelPayment()
+            paymentState = .cash(.collectingCash)
+//        }
     }
 
     func cancelCashPayment() {
-        isCashPaymentInProgress = false
-        paymentState = .card(.idle)
+        Task { @MainActor [weak self] in
+            guard let self else { return }
+            // Because we don't know the previous card payment state, we have to cancel then collect again.
+            // If the reader's not connected, we don't want to call collect because it'll start a connection attempt.
+            // This is bad.
+            // To improve this, if we keep allowing card payments while cash payment is showing, we should improve the
+            // paymentState representation to allow two payment states to be known. It would need to be a struct with something like:
+            /*
+
+             struct PointOfSalePaymentState {
+                 let activePaymentMethod: PointOfSaleActivePaymentMethod //(enum for just `.card`, `.cash` without associated type)
+                 let cardPaymentState: PointOfSaleCardPaymentState
+                 let cashPaymentState: PointOfSaleCashPaymentState
+             }
+
+             */
+            if case .connected = cardReaderConnectionStatus {
+                try? await cardPresentPaymentService.cancelPayment()
+                await collectPayment()
+            } else {
+                paymentState = .card(.idle)
+            }
+        }
     }
 
     private func cashPaymentSuccess() {
@@ -217,6 +240,9 @@ extension PointOfSaleAggregateModel {
 
     @MainActor
     func collectCashPayment() async throws {
+        // Currently, we allow card payments right up until the `Mark order complete` button is tapped.
+        // Delete the following row if we decide to cancel at the outset of cash payments.
+        try? await cardPresentPaymentService.cancelPayment()
         try await orderController.collectCashPayment()
         cashPaymentSuccess()
     }
@@ -232,9 +258,10 @@ extension PointOfSaleAggregateModel {
     }
 
     func cancelThenCollectPayment() {
-        cardPresentPaymentService.cancelPayment()
         Task { [weak self] in
-            await self?.collectPayment()
+            guard let self else { return }
+            try await cardPresentPaymentService.cancelPayment()
+            await collectPayment()
         }
     }
 
@@ -314,10 +341,6 @@ private extension PointOfSaleAggregateModel {
                 let newPaymentState = PointOfSalePaymentState(from: paymentEvent,
                                                               using: presentationStyleDeterminerDependencies)
 
-                if self.isCashPaymentInProgress, case .card = newPaymentState {
-                    cardPresentPaymentService.cancelPayment()
-                    return .cash(.paymentSuccess)
-                }
                 return newPaymentState
             }
             .assign(to: &$paymentState)