[Mobile Payments] Show network error when onboarding requests fail

Modules/Sources/Yosemite/Stores/CardPresentPaymentStore.swift

@@ -444,6 +444,7 @@ private extension CardPresentPaymentStore {
     ///
     func loadAccounts(siteID: Int64, onCompletion: @escaping (Result<Void, Error>) -> Void) {
         var error: Error? = nil
+        var hasSuccess: Bool? = nil
 
         let group = DispatchGroup()
         group.enter()
@@ -453,7 +454,7 @@ private extension CardPresentPaymentStore {
                 DDLogError("⛔️ Error synchronizing WCPay Account: \(loadError)")
                 error = loadError
             case .success:
-                break
+                hasSuccess = true
             }
             group.leave()
         })
@@ -465,17 +466,23 @@ private extension CardPresentPaymentStore {
                 DDLogError("⛔️ Error synchronizing Stripe Account: \(loadError)")
                 error = loadError
             case .success:
-                break
+                hasSuccess = true
             }
             group.leave()
         })
 
         group.notify(queue: .main) {
-            guard let error = error else {
+            switch (hasSuccess, error) {
+            case (true, _):
+                // If either succeeds, the load is successful
                 onCompletion(.success(()))
-                return
+            case (_, .some(let error)):
+                // If we have an error, and no success, the load fails
+                onCompletion(.failure(error))
+            case (_, .none):
+                // This... shouldn't really happen.
+                onCompletion(.failure(CardPresentPaymentStoreError.unknownErrorFetchingAccounts))
             }
-            onCompletion(.failure(error))
         }
     }
 
@@ -676,6 +683,11 @@ public enum ServerSidePaymentCaptureError: Error, LocalizedError {
     }
 }
 
+//periphery:ignore - logging this error detail in WooCommerce is useful, if it ever happens. It's part of the public API here.
+public enum CardPresentPaymentStoreError: Error {
+    case unknownErrorFetchingAccounts
+}
+
 private extension PaymentGatewayAccount {
     var isWCPay: Bool {
         self.gatewayID == WCPayAccount.gatewayID

Modules/Tests/YosemiteTests/Stores/CardPresentPaymentStoreTests.swift

@@ -252,38 +252,40 @@ final class CardPresentPaymentStoreTests: XCTestCase {
     /// Verifies that the PaymentGatewayAccountStore hits the network when loading a WCPay Account and places nothing in storage in case of error.
     ///
     func test_loadAccounts_handles_failure() throws {
-        let expectation = self.expectation(description: "Load Account error response")
         network.simulateResponse(requestUrlSuffix: "payments/accounts",
                                  filename: "generic_error")
         network.simulateResponse(requestUrlSuffix: "wc_stripe/account/summary",
                                  filename: "generic_error")
 
-        let action = CardPresentPaymentAction.loadAccounts(siteID: sampleSiteID, onCompletion: { result in
-            XCTAssertTrue(result.isFailure)
-            expectation.fulfill()
-        })
+        // When
+        let result = waitFor { promise in
+            self.cardPresentStore.onAction(CardPresentPaymentAction.loadAccounts(siteID: self.sampleSiteID, onCompletion: { result in
+                promise(result)
+            }))
+        }
 
-        cardPresentStore.onAction(action)
-        wait(for: [expectation], timeout: Constants.expectationTimeout)
+        // Then
+        XCTAssertTrue(result.isFailure)
 
         XCTAssertNil(viewStorage.firstObject(ofType: Storage.PaymentGatewayAccount.self, matching: nil))
     }
 
     /// Verifies that the PaymentGatewayAccountStore hits the network when loading a WCPay Account, propagates success and upserts the account into storage.
     ///
     func test_loadAccounts_returns_expected_data() throws {
-        let expectation = self.expectation(description: "Load Account fetch response")
         network.simulateResponse(requestUrlSuffix: "payments/accounts",
                                  filename: "wcpay-account-complete")
         network.simulateResponse(requestUrlSuffix: "wc_stripe/account/summary",
                                  filename: "stripe-account-complete")
-        let action = CardPresentPaymentAction.loadAccounts(siteID: sampleSiteID, onCompletion: { result in
-            XCTAssertTrue(result.isSuccess)
-            expectation.fulfill()
-        })
+        // When
+        let result = waitFor { promise in
+            self.cardPresentStore.onAction(CardPresentPaymentAction.loadAccounts(siteID: self.sampleSiteID, onCompletion: { result in
+                promise(result)
+            }))
+        }
 
-        cardPresentStore.onAction(action)
-        wait(for: [expectation], timeout: Constants.expectationTimeout)
+        // Then
+        XCTAssertTrue(result.isSuccess)
 
         XCTAssert(viewStorage.countObjects(ofType: Storage.PaymentGatewayAccount.self, matching: nil) == 2)
 
@@ -297,6 +299,89 @@ final class CardPresentPaymentStoreTests: XCTestCase {
         XCTAssert(storageAccount?.status == "complete")
     }
 
+    /// Verifies that loadAccounts succeeds when only WCPay succeeds and Stripe fails.
+    ///
+    func test_loadAccounts_succeeds_when_only_wcpay_succeeds() throws {
+        // Given
+        network.simulateResponse(requestUrlSuffix: "payments/accounts",
+                                 filename: "wcpay-account-complete")
+        network.simulateResponse(requestUrlSuffix: "wc_stripe/account/summary",
+                                 filename: "generic_error")
+
+        // When
+        let result = waitFor { promise in
+            self.cardPresentStore.onAction(CardPresentPaymentAction.loadAccounts(siteID: self.sampleSiteID, onCompletion: { result in
+                promise(result)
+            }))
+        }
+
+        // Then
+        XCTAssertTrue(result.isSuccess)
+
+        XCTAssert(viewStorage.countObjects(ofType: Storage.PaymentGatewayAccount.self, matching: nil) == 1)
+
+        let storageAccount = viewStorage.loadPaymentGatewayAccount(
+            siteID: sampleSiteID,
+            gatewayID: WCPayAccount.gatewayID
+        )
+
+        XCTAssert(storageAccount?.siteID == sampleSiteID)
+        XCTAssert(storageAccount?.gatewayID == WCPayAccount.gatewayID)
+        XCTAssert(storageAccount?.status == "complete")
+    }
+
+    /// Verifies that loadAccounts succeeds when only Stripe succeeds and WCPay fails.
+    ///
+    func test_loadAccounts_succeeds_when_only_stripe_succeeds() throws {
+        // Given
+        network.simulateResponse(requestUrlSuffix: "payments/accounts",
+                                 filename: "generic_error")
+        network.simulateResponse(requestUrlSuffix: "wc_stripe/account/summary",
+                                 filename: "stripe-account-complete")
+
+        // When
+        let result = waitFor { promise in
+            self.cardPresentStore.onAction(CardPresentPaymentAction.loadAccounts(siteID: self.sampleSiteID, onCompletion: { result in
+                promise(result)
+            }))
+        }
+
+        // Then
+        XCTAssertTrue(result.isSuccess)
+
+        XCTAssert(viewStorage.countObjects(ofType: Storage.PaymentGatewayAccount.self, matching: nil) == 1)
+
+        let storageAccount = viewStorage.loadPaymentGatewayAccount(
+            siteID: sampleSiteID,
+            gatewayID: StripeAccount.gatewayID
+        )
+
+        XCTAssert(storageAccount?.siteID == sampleSiteID)
+        XCTAssert(storageAccount?.gatewayID == StripeAccount.gatewayID)
+        XCTAssert(storageAccount?.status == "complete")
+    }
+
+    /// Verifies that loadAccounts returns an error when both WCPay and Stripe fail.
+    ///
+    func test_loadAccounts_fails_when_both_wcpay_and_stripe_fail() throws {
+        // Given
+        network.simulateResponse(requestUrlSuffix: "payments/accounts",
+                                 filename: "generic_error")
+        network.simulateResponse(requestUrlSuffix: "wc_stripe/account/summary",
+                                 filename: "generic_error")
+        // When
+        let result = waitFor { promise in
+            self.cardPresentStore.onAction(CardPresentPaymentAction.loadAccounts(siteID: self.sampleSiteID, onCompletion: { result in
+                promise(result)
+            }))
+        }
+
+        // Then
+        XCTAssertTrue(result.isFailure)
+
+        XCTAssertNil(viewStorage.firstObject(ofType: Storage.PaymentGatewayAccount.self, matching: nil))
+    }
+
     /// Verifies that the store hits the network when fetching a charge, and propagates success.
     ///
     func test_fetchWCPayCharge_returns_expected_data() throws {

RELEASE-NOTES.txt

@@ -3,7 +3,7 @@
 
 23.7
 -----
-
+- [*] Improve card payments onboarding error handling to show network errors correctly [https://github.com/woocommerce/woocommerce-ios/pull/16304]
 
 23.6
 -----

WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/CardPresentPaymentsOnboardingUseCase.swift

@@ -4,6 +4,7 @@ import Storage
 import Yosemite
 import Experiments
 import WooFoundation
+import enum Alamofire.AFError
 
 private typealias SystemPlugin = Yosemite.SystemPlugin
 private typealias PaymentGatewayAccount = Yosemite.PaymentGatewayAccount
@@ -180,8 +181,17 @@ final class CardPresentPaymentsOnboardingUseCase: CardPresentPaymentsOnboardingU
                 return
             }
 
-            self.updateState()
-            CardPresentPaymentOnboardingStateCache.shared.update(self.state)
+            switch result {
+            case .success:
+                updateState()
+                CardPresentPaymentOnboardingStateCache.shared.update(state)
+            case .failure(let error):
+                if isNetworkError(error) {
+                    state = .noConnectionError
+                } else {
+                    state = .genericError
+                }
+            }
         }
         stores.dispatch(paymentGatewayAccountsAction)
     }
@@ -548,7 +558,11 @@ private extension CardPresentPaymentsOnboardingUseCase {
     }
 
     func isNetworkError(_ error: Error) -> Bool {
-        (error as NSError).domain == NSURLErrorDomain
+        if let afError = error as? AFError {
+            return true
+        } else {
+            return (error as NSError).domain == NSURLErrorDomain
+        }
     }
 }
 

WooCommerce/WooCommerceTests/ViewRelated/CardPresentPayments/CardPresentPaymentsOnboardingUseCaseTests.swift

@@ -1,6 +1,7 @@
 import XCTest
 import Fakes
 import Yosemite
+import enum Alamofire.AFError
 @testable import WooCommerce
 
 class CardPresentPaymentsOnboardingUseCaseTests: XCTestCase {
@@ -1226,6 +1227,62 @@ class CardPresentPaymentsOnboardingUseCaseTests: XCTestCase {
         XCTAssertEqual(CardPresentPaymentsPlugin.stripe.plugin, .stripe)
         XCTAssertEqual(CardPresentPaymentsPlugin.stripe.fileNameWithPathExtension, "woocommerce-gateway-stripe/woocommerce-gateway-stripe")
     }
+
+    // MARK: - loadAccounts error handling tests
+
+    func test_onboarding_handles_network_error_when_loading_accounts() {
+        // Given
+        setupCountry(country: .us)
+        setupWCPayPlugin(status: .active, version: WCPayPluginVersion.minimumSupportedVersion)
+
+        // Use AFError.sessionTaskFailed which is what Alamofire returns for network errors
+        let urlError = URLError(.notConnectedToInternet)
+        let networkError = AFError.sessionTaskFailed(error: urlError)
+        stores.whenReceivingAction(ofType: CardPresentPaymentAction.self) { action in
+            switch action {
+            case let .loadAccounts(_, onCompletion):
+                onCompletion(.failure(networkError))
+            default:
+                break
+            }
+        }
+
+        // When
+        let useCase = CardPresentPaymentsOnboardingUseCase(storageManager: storageManager,
+                                                           stores: stores,
+                                                           cardPresentPaymentOnboardingStateCache: onboardingStateCache)
+        useCase.updateAccounts()
+
+        // Then - Should show no connection error
+        XCTAssertEqual(useCase.state, .noConnectionError)
+    }
+
+    func test_onboarding_handles_generic_error_when_both_accounts_fail_to_load() {
+        // Given
+        setupCountry(country: .us)
+        setupWCPayPlugin(status: .active, version: WCPayPluginVersion.minimumSupportedVersion)
+        setupStripePlugin(status: .active, version: StripePluginVersion.minimumSupportedVersion)
+
+        let genericError = NSError(domain: "test.error", code: 500, userInfo: nil)
+        stores.whenReceivingAction(ofType: CardPresentPaymentAction.self) { action in
+            switch action {
+            case let .loadAccounts(_, onCompletion):
+                // Both accounts fail to load
+                onCompletion(.failure(genericError))
+            default:
+                break
+            }
+        }
+
+        // When
+        let useCase = CardPresentPaymentsOnboardingUseCase(storageManager: storageManager,
+                                                           stores: stores,
+                                                           cardPresentPaymentOnboardingStateCache: onboardingStateCache)
+        useCase.updateAccounts()
+
+        // Then - Should show generic error
+        XCTAssertEqual(useCase.state, .genericError)
+    }
 }
 
 // MARK: - Settings helpers