8089 Don’t report success when payment fails

This commit prevents us from going through the success flow when Tap to Pay on iPhone payments fail. Previously, if you locked the phone while the Apple payment screen was showing, it would still show a success notice even though the payment hadn’t been taken.

Author: joshheald

WooCommerce/Classes/ViewRelated/Orders/Collect Payments/CollectOrderPaymentUseCase.swift

@@ -22,7 +22,7 @@ protocol CollectOrderPaymentProtocol {
     /// - Parameter onCollect: Closure Invoked after the collect process has finished.
     /// - Parameter onCompleted: Closure Invoked after the flow has been totally completed.
     /// - Parameter onCancel: Closure invoked after the flow is cancelled
-    func collectPayment(onCollect: @escaping (Result<Void, Error>) -> (), onCancel: @escaping () -> (), onCompleted: @escaping () -> ())
+    func collectPayment(onFailure: @escaping (Error) -> (), onCancel: @escaping () -> (), onCompleted: @escaping () -> ())
 }
 
 /// Use case to collect payments from an order.
@@ -129,15 +129,15 @@ final class CollectOrderPaymentUseCase: NSObject, CollectOrderPaymentProtocol {
     /// 7. Tracks payment analytics
     ///
     ///
-    /// - Parameter onCollect: Closure invoked after the collect process has finished.
+    /// - Parameter onFailure: Closure invoked after the payment process fails.
     /// - Parameter onCancel: Closure invoked after the flow is cancelled
     /// - Parameter onCompleted: Closure invoked after the flow has been totally completed, currently after merchant has handled the receipt.
-    func collectPayment(onCollect: @escaping (Result<Void, Error>) -> (),
+    func collectPayment(onFailure: @escaping (Error) -> (),
                         onCancel: @escaping () -> (),
                         onCompleted: @escaping () -> ()) {
         guard isTotalAmountValid() else {
             let error = totalAmountInvalidError()
-            onCollect(.failure(error))
+            onFailure(error)
             return handleTotalAmountInvalidError(totalAmountInvalidError(), onCompleted: onCancel)
         }
 
@@ -158,16 +158,14 @@ final class CollectOrderPaymentUseCase: NSObject, CollectOrderPaymentProtocol {
                     case .failure(CollectOrderPaymentUseCaseError.flowCanceledByUser):
                         self.rootViewController.presentedViewController?.dismiss(animated: true)
                         return onCancel()
-                    default:
-                        onCollect(result.map { _ in () }) // Transforms Result<CardPresentCapturedPaymentData, Error> to Result<Void, Error>
-                    }
-                    // Handle payment receipt
-                    guard let paymentData = try? result.get() else {
-                        return onCompleted()
+                    case .failure(let error):
+                        return onFailure(error)
+                    case .success(let paymentData):
+                        // Handle payment receipt
+                        self.presentReceiptAlert(receiptParameters: paymentData.receiptParameters,
+                                                 alertProvider: paymentAlertProvider,
+                                                 onCompleted: onCompleted)
                     }
-                    self.presentReceiptAlert(receiptParameters: paymentData.receiptParameters,
-                                             alertProvider: paymentAlertProvider,
-                                             onCompleted: onCompleted)
                 })
             case .canceled:
                 self.handlePaymentCancellation()

WooCommerce/Classes/ViewRelated/Orders/Collect Payments/LegacyCollectOrderPaymentUseCase.swift

@@ -5,10 +5,24 @@ import MessageUI
 import WooFoundation
 import protocol Storage.StorageManagerType
 
+
+/// Protocol to abstract the `LegacyCollectOrderPaymentUseCase`.
+/// Currently only used to facilitate unit tests.
+///
+protocol LegacyCollectOrderPaymentProtocol {
+    /// Starts the collect payment flow.
+    ///
+    ///
+    /// - Parameter onCollect: Closure Invoked after the collect process has finished.
+    /// - Parameter onCompleted: Closure Invoked after the flow has been totally completed.
+    /// - Parameter onCancel: Closure invoked after the flow is cancelled
+    func collectPayment(onCollect: @escaping (Result<Void, Error>) -> (), onCancel: @escaping () -> (), onCompleted: @escaping () -> ())
+}
+
 /// Use case to collect payments from an order.
 /// Orchestrates reader connection, payment, UI alerts, receipt handling and analytics.
 ///
-final class LegacyCollectOrderPaymentUseCase: NSObject, CollectOrderPaymentProtocol {
+final class LegacyCollectOrderPaymentUseCase: NSObject, LegacyCollectOrderPaymentProtocol {
     /// Currency Formatter
     ///
     private let currencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings)

WooCommerce/Classes/ViewRelated/Orders/Payment Methods/PaymentMethodsView.swift

@@ -60,7 +60,7 @@ struct PaymentMethodsView: View {
                             Divider()
 
                             MethodRow(icon: .creditCardImage, title: Localization.card, accessibilityID: Accessibility.cardMethod) {
-                                viewModel.collectPayment(on: rootViewController, onSuccess: dismiss)
+                                viewModel.collectPayment(on: rootViewController, onSuccess: dismiss, onFailure: dismiss)
                             }
                         }
 

WooCommerce/Classes/ViewRelated/Orders/Payment Methods/PaymentMethodsViewModel.swift

@@ -99,7 +99,7 @@ final class PaymentMethodsViewModel: ObservableObject {
 
     /// Retains the use-case so it can perform all of its async tasks.
     ///
-    private var legacyCollectPaymentsUseCase: CollectOrderPaymentProtocol?
+    private var legacyCollectPaymentsUseCase: LegacyCollectOrderPaymentProtocol?
 
     private var collectPaymentsUseCase: CollectOrderPaymentProtocol?
 
@@ -112,6 +112,8 @@ final class PaymentMethodsViewModel: ObservableObject {
               configuration: upsellCardReadersCampaign.configuration)
     }
 
+    private let isTapToPayOnIPhoneEnabled: Bool
+
     struct Dependencies {
         let presentNoticeSubject: PassthroughSubject<SimplePaymentsNotice, Never>
         let cardPresentPaymentsOnboardingPresenter: CardPresentPaymentsOnboardingPresenting
@@ -141,12 +143,14 @@ final class PaymentMethodsViewModel: ObservableObject {
          paymentLink: URL? = nil,
          formattedTotal: String,
          flow: WooAnalyticsEvent.PaymentsFlow.Flow,
+         isTapToPayOnIPhoneEnabled: Bool = ServiceLocator.generalAppSettings.settings.isTapToPayOnIPhoneSwitchEnabled,
          dependencies: Dependencies = Dependencies()) {
         self.siteID = siteID
         self.orderID = orderID
         self.paymentLink = paymentLink
         self.formattedTotal = formattedTotal
         self.flow = flow
+        self.isTapToPayOnIPhoneEnabled = isTapToPayOnIPhoneEnabled
         presentNoticeSubject = dependencies.presentNoticeSubject
         cardPresentPaymentsOnboardingPresenter = dependencies.cardPresentPaymentsOnboardingPresenter
         stores = dependencies.stores
@@ -199,19 +203,21 @@ final class PaymentMethodsViewModel: ObservableObject {
     /// - parameter useCase: Assign a custom useCase object for testing purposes. If not provided `CollectOrderPaymentUseCase` will be used.
     ///
     func collectPayment(on rootViewController: UIViewController?,
-                        useCase: CollectOrderPaymentProtocol? = nil,
-                        onSuccess: @escaping () -> ()) {
-        switch ServiceLocator.generalAppSettings.settings.isTapToPayOnIPhoneSwitchEnabled {
+                        useCase: LegacyCollectOrderPaymentProtocol? = nil,
+                        onSuccess: @escaping () -> (),
+                        onFailure: @escaping () -> ()) {
+        switch isTapToPayOnIPhoneEnabled {
         case true:
-            newCollectPayment(on: rootViewController, useCase: useCase, onSuccess: onSuccess)
+            newCollectPayment(on: rootViewController, onSuccess: onSuccess, onFailure: onFailure)
         case false:
             legacyCollectPayment(on: rootViewController, useCase: useCase, onSuccess: onSuccess)
         }
     }
 
     func newCollectPayment(on rootViewController: UIViewController?,
-                        useCase: CollectOrderPaymentProtocol? = nil,
-                        onSuccess: @escaping () -> ()) {
+                           useCase: CollectOrderPaymentProtocol? = nil,
+                           onSuccess: @escaping () -> (),
+                           onFailure: @escaping () -> ()) {
         trackCollectIntention(method: .card)
 
         guard let rootViewController = rootViewController else {
@@ -243,9 +249,12 @@ final class PaymentMethodsViewModel: ObservableObject {
                         configuration: CardPresentConfigurationLoader().configuration)
 
                     self.collectPaymentsUseCase?.collectPayment(
-                        onCollect: { [weak self] result in
-                            guard result.isFailure else { return }
+                        onFailure: { [weak self] error in
                             self?.trackFlowFailed()
+                            // Update order in case its status and/or other details are updated after a failed in-person payment
+                            self?.updateOrderAsynchronously()
+
+                            onFailure()
                         },
                         onCancel: {
                             // No tracking required because the flow remains on screen to choose other payment methods.
@@ -273,8 +282,8 @@ final class PaymentMethodsViewModel: ObservableObject {
     }
 
     func legacyCollectPayment(on rootViewController: UIViewController?,
-                        useCase: CollectOrderPaymentProtocol? = nil,
-                        onSuccess: @escaping () -> ()) {
+                              useCase: LegacyCollectOrderPaymentProtocol? = nil,
+                              onSuccess: @escaping () -> ()) {
         trackCollectIntention(method: .card)
 
         guard let rootViewController = rootViewController else {