[Woo POS][Barcodes] Ignore empty scanner input (#15843)

Author: joshheald

WooCommerce/Classes/POS/Presentation/Barcode Scanning/HIDBarcodeParser.swift

@@ -26,26 +26,51 @@ final class HIDBarcodeParser {
     /// Process a key press event
     /// - Parameter key: The key that was pressed
     func processKeyPress(_ key: UIKey) {
-        let currentTime = timeProvider.now()
-
-        // If characters are entered too slowly, it's probably typing and we should ignore it
-        if let lastTime = lastKeyPressTime,
-           currentTime.timeIntervalSince(lastTime) > configuration.maximumInterCharacterTime {
-            onScan(.failure(HIDBarcodeParserError.timedOut(barcode: buffer)))
-            resetScan()
+        guard shouldRecogniseAsScanKeystroke(key) else {
+            return
         }
 
-        lastKeyPressTime = currentTime
-
         let character = key.characters
         if configuration.terminatingStrings.contains(character) {
             processScan()
         } else {
             guard !excludedKeys.contains(key.keyCode) else { return }
+            checkForTimeoutBetweenKeystrokes()
             buffer.append(character)
         }
     }
 
+    private func shouldRecogniseAsScanKeystroke(_ key: UIKey) -> Bool {
+        guard key.characters.isNotEmpty else {
+            // This prevents a double-trigger-pull on a Star scanner from adding an error row –
+            // Star use this as a shortcut to switch to the software keyboard. They send keycode 174 0xAE, which is
+            // undefined and reserved in UIKeyboardHIDUsage. The scanner doesn't send a character with the code.
+            // There seems to be no reason to handle empty input when considering scans.
+            return false
+        }
+
+        if buffer.isEmpty && configuration.terminatingStrings.contains(key.characters) {
+            // We prefer to show all partial scans, but if we just get an enter with no numbers, ignoring it makes testing easier
+            return false
+        }
+
+        return true
+    }
+
+    private func checkForTimeoutBetweenKeystrokes() {
+        // If characters are entered too slowly, it's probably typing and we should ignore the old input.
+        // The key we just received is still considered for adding to the buffer – we may simply reset the buffer first.
+        let currentTime = timeProvider.now()
+
+        if let lastTime = lastKeyPressTime,
+           currentTime.timeIntervalSince(lastTime) > configuration.maximumInterCharacterTime {
+            onScan(.failure(HIDBarcodeParserError.timedOut(barcode: buffer)))
+            resetScan()
+        }
+
+        lastKeyPressTime = currentTime
+    }
+
     private let excludedKeys: [UIKeyboardHIDUsage] = [
         .keyboardCapsLock,
         .keyboardF1,
@@ -138,6 +163,7 @@ final class HIDBarcodeParser {
     }
 
     private func processScan() {
+        checkForTimeoutBetweenKeystrokes()
         if buffer.count >= configuration.minimumBarcodeLength {
             onScan(.success(buffer))
         } else {

WooCommerce/WooCommerceTests/POS/Presentation/Barcode Scanning/HIDBarcodeParserTests.swift

@@ -334,6 +334,65 @@ struct HIDBarcodeParserTests {
             Issue.record("Expected failure result")
         }
     }
+
+    @Test("Parser does not show an error row for empty scan")
+    func testEmptyScanDoesntError() {
+        var results: [Result<String, Error>] = []
+        let parser = HIDBarcodeParser(
+            configuration: testConfiguration,
+            onScan: { result in
+                results.append(result)
+            }
+        )
+
+        // Just send the terminator, no scan input
+        parser.processKeyPress(MockUIKey(character: "\r"))
+
+        #expect(results.isEmpty)
+    }
+
+    @Test("Parser does not start a timeout for an ignored character")
+    func testEmptyScanDoesntStartTimeoutForIgnoredCharacter() {
+        var results: [Result<String, Error>] = []
+        let mockTimeProvider = MockTimeProvider()
+        let parser = HIDBarcodeParser(
+            configuration: testConfiguration,
+            onScan: { result in
+                results.append(result)
+            },
+            timeProvider: mockTimeProvider
+        )
+
+        // Scan a barcode with two terminators, then scan another barcode – only the two codes should be parsed
+        parser.processKeyPress(MockUIKey(character: "1"))
+        parser.processKeyPress(MockUIKey(character: "2"))
+        parser.processKeyPress(MockUIKey(character: "3"))
+        parser.processKeyPress(MockUIKey(character: "\r")) // Scan is recognised here
+        parser.processKeyPress(MockUIKey(character: "\n", keyCode: .keyboardDownArrow)) // This is ignored
+
+        // Time between scans
+        mockTimeProvider.advance(by: 1.5)
+
+        // Scan the second barcode
+        parser.processKeyPress(MockUIKey(character: "4")) // Risk of an error row here if `\n` isn't ignored
+        parser.processKeyPress(MockUIKey(character: "5"))
+        parser.processKeyPress(MockUIKey(character: "6"))
+        parser.processKeyPress(MockUIKey(character: "\r"))
+        parser.processKeyPress(MockUIKey(character: "\n", keyCode: .keyboardDownArrow))
+
+
+        #expect(results.count == 2)
+        if case .success(let barcode1) = results[0] {
+            #expect(barcode1 == "123")
+        } else {
+            Issue.record("Expected success result for first scan")
+        }
+        if case .success(let barcode2) = results[1] {
+            #expect(barcode2 == "456")
+        } else {
+            Issue.record("Expected success result for second scan")
+        }
+    }
 }
 
 // MARK: - Test Helpers