Add associated value for connection error, expand unit tests

Author: allendav

WooCommerce/Classes/ViewRelated/CardPresentPayments/CardReaderConnectionController.swift

@@ -36,7 +36,7 @@ final class CardReaderConnectionController {
         /// A failure occurred while connecting. The search may continue or be canceled. At this time we
         /// do not present the detailed error from the service.
         ///
-        case connectingFailed
+        case connectingFailed(Error)
 
         /// User cancelled search/connecting to a card reader. The completion passed to `searchAndConnect`
         /// will be called with a `success` `Bool` `False` result. The view controller passed to `searchAndConnect` will be
@@ -127,8 +127,8 @@ private extension CardReaderConnectionController {
             onCancel()
         case .connectToReader:
             onConnectToReader()
-        case .connectingFailed:
-            onConnectingFailed()
+        case .connectingFailed(let error):
+            onConnectingFailed(error: error)
         case .discoveryFailed(let error):
             onDiscoveryFailed(error: error)
         }
@@ -319,7 +319,7 @@ private extension CardReaderConnectionController {
                 self.returnSuccess(connected: true)
             case .failure(let error):
                 ServiceLocator.analytics.track(.cardReaderConnectionFailed, withError: error)
-                self.state = .connectingFailed
+                self.state = .connectingFailed(error)
             }
         }
         ServiceLocator.stores.dispatch(action)
@@ -329,13 +329,14 @@ private extension CardReaderConnectionController {
 
     /// An error occurred while connecting
     ///
-    private func onConnectingFailed() {
+    private func onConnectingFailed(error: Error) {
         guard let from = fromController else {
             return
         }
 
-        /// Let's start over again - we don't want to try and connect to something obviously borked
-        /// right off the bat
+        /// Clear our copy of found readers to avoid connecting to a reader that isn't
+        /// there while we wait for `onReaderDiscovered` to receive an update.
+        /// See also https://github.com/stripe/stripe-terminal-ios/issues/104#issuecomment-916285167
         ///
         self.foundReaders = []
 

WooCommerce/WooCommerceTests/Mocks/MockCardPresentPaymentsStoresManager.swift

@@ -10,21 +10,24 @@ final class MockCardPresentPaymentsStoresManager: DefaultStoresManager {
     private var readerUpdateAvailable: Bool
     private var failReaderUpdateCheck: Bool
     private var failUpdate: Bool
+    private var failConnection: Bool
 
     init(connectedReaders: [CardReader],
          discoveredReader: CardReader? = nil,
          sessionManager: SessionManager,
          failDiscovery: Bool = false,
          readerUpdateAvailable: Bool = false,
          failReaderUpdateCheck: Bool = false,
-         failUpdate: Bool = false
+         failUpdate: Bool = false,
+         failConnection: Bool = false
     ) {
         self.connectedReaders = connectedReaders
         self.discoveredReader = discoveredReader
         self.failDiscovery = failDiscovery
         self.readerUpdateAvailable = readerUpdateAvailable
         self.failReaderUpdateCheck = failReaderUpdateCheck
         self.failUpdate = failUpdate
+        self.failConnection = failConnection
         super.init(sessionManager: sessionManager)
     }
 
@@ -50,6 +53,10 @@ final class MockCardPresentPaymentsStoresManager: DefaultStoresManager {
             }
             onReaderDiscovered([discoveredReader])
         case .connect(let reader, let onCompletion):
+            guard !failConnection else {
+                onCompletion(Result.failure(MockErrors.connectionFailure))
+                return
+            }
             onCompletion(Result.success(reader))
         case .cancelCardReaderDiscovery(let onCompletion):
             onCompletion(Result.success(()))
@@ -81,6 +88,7 @@ extension MockCardPresentPaymentsStoresManager {
         case discoveryFailure
         case readerUpdateCheckFailure
         case readerUpdateFailure
+        case connectionFailure
     }
 }
 

WooCommerce/WooCommerceTests/Mocks/MockCardReaderSettingsAlerts.swift

@@ -7,18 +7,30 @@ enum MockCardReaderSettingsAlertsMode {
     case closeScanFailure
     case continueSearching
     case connectFoundReader
+    case continueSearchingAfterConnectionFailure
+    case cancelSearchingAfterConnectionFailure
 }
 
 final class MockCardReaderSettingsAlerts: CardReaderSettingsAlertsProvider {
     private var mode: MockCardReaderSettingsAlertsMode
+    private var didPresentFoundReader: Bool
 
     init(mode: MockCardReaderSettingsAlertsMode) {
         self.mode = mode
+        self.didPresentFoundReader = false
     }
     func scanningForReader(from: UIViewController, cancel: @escaping () -> Void) {
         if mode == .cancelScanning {
             cancel()
         }
+
+        if mode == .continueSearchingAfterConnectionFailure {
+            /// If we've already presented a found reader once before, cancel this second search
+            ///
+            if didPresentFoundReader {
+                cancel()
+            }
+        }
     }
 
     func scanningFailed(from: UIViewController, error: Error, close: @escaping () -> Void) {
@@ -28,11 +40,13 @@ final class MockCardReaderSettingsAlerts: CardReaderSettingsAlertsProvider {
     }
 
     func foundReader(from: UIViewController, name: String, connect: @escaping () -> Void, continueSearch: @escaping () -> Void) {
+        didPresentFoundReader = true
+
         if mode == .continueSearching {
             continueSearch()
         }
 
-        if mode == .connectFoundReader {
+        if mode == .connectFoundReader || mode == .cancelSearchingAfterConnectionFailure || mode == .continueSearchingAfterConnectionFailure {
             connect()
         }
     }
@@ -42,7 +56,13 @@ final class MockCardReaderSettingsAlerts: CardReaderSettingsAlertsProvider {
     }
 
     func connectingFailed(from: UIViewController, continueSearch: @escaping () -> Void, cancelSearch: @escaping () -> Void) {
-        // GNDN
+        if mode == .continueSearchingAfterConnectionFailure {
+            continueSearch()
+        }
+
+        if mode == .cancelSearchingAfterConnectionFailure {
+            cancelSearch()
+        }
     }
 
     func dismiss() {

WooCommerce/WooCommerceTests/ViewRelated/CardPresentPayments/CardReaderConnectionControllerTests.swift

@@ -134,4 +134,73 @@ class CardReaderConnectionControllerTests: XCTestCase {
         wait(for: [expectation], timeout: Constants.expectationTimeout)
     }
 
+    func test_user_can_cancel_search_after_connection_error() {
+        // Given
+        let expectation = self.expectation(description: #function)
+
+        let discoveredReader = MockCardReader.bbposChipper2XBT()
+
+        let mockStoresManager = MockCardPresentPaymentsStoresManager(
+            connectedReaders: [],
+            discoveredReader: discoveredReader,
+            sessionManager: SessionManager.testingInstance,
+            failConnection: true
+        )
+        ServiceLocator.setStores(mockStoresManager)
+        let mockPresentingViewController = UIViewController()
+        let mockKnownReadersProvider = MockKnownReadersProvider(knownReaders: [])
+        let mockAlerts = MockCardReaderSettingsAlerts(mode: .cancelSearchingAfterConnectionFailure)
+        let controller = CardReaderConnectionController(
+            forSiteID: sampleSiteID,
+            knownReadersProvider: mockKnownReadersProvider,
+            alertsProvider: mockAlerts
+        )
+
+        // When
+        controller.searchAndConnect(from: mockPresentingViewController) { result in
+            XCTAssertTrue(result.isSuccess)
+            if case .success(let connected) = result {
+                XCTAssertFalse(connected)
+                expectation.fulfill()
+            }
+        }
+
+        // Then
+        wait(for: [expectation], timeout: Constants.expectationTimeout)
+    }
+
+    func test_user_can_continue_search_after_connection_error() {
+        // Given
+        let expectation = self.expectation(description: #function)
+
+        let discoveredReader = MockCardReader.bbposChipper2XBT()
+
+        let mockStoresManager = MockCardPresentPaymentsStoresManager(
+            connectedReaders: [],
+            discoveredReader: discoveredReader,
+            sessionManager: SessionManager.testingInstance,
+            failConnection: true
+        )
+        ServiceLocator.setStores(mockStoresManager)
+        let mockPresentingViewController = UIViewController()
+        let mockKnownReadersProvider = MockKnownReadersProvider(knownReaders: [])
+        let mockAlerts = MockCardReaderSettingsAlerts(mode: .continueSearchingAfterConnectionFailure)
+        let controller = CardReaderConnectionController(
+            forSiteID: sampleSiteID,
+            knownReadersProvider: mockKnownReadersProvider,
+            alertsProvider: mockAlerts
+        )
+
+        // When
+        controller.searchAndConnect(from: mockPresentingViewController) { result in
+            XCTAssertTrue(result.isSuccess)
+            if case .success(let connected) = result {
+                XCTAssertFalse(connected)
+                expectation.fulfill()
+            }
+        }
+
+        // Then
+        wait(for: [expectation], timeout: Constants.expectationTimeout)
+    }
 }