[Woo POS][Barcodes] Show an error row for short and incomplete scans (#15823)

joshheald · web-flow · commit 30fea92eb4ff · 2025-06-27T10:59:50.000+01:00
diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleBarcodeScanService.swift b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleBarcodeScanService.swift
@@ -18,6 +18,9 @@ public enum PointOfSaleBarcodeScanError: Error {
     case notFound(scannedCode: String)
     case loadingError(scannedCode: String, underlyingError: Error)
     case mappingError(scannedCode: String, underlyingError: Error)
+    case scanTooShort(scannedCode: String)
+    case timedOut(scannedCode: String)
+    case parsingError(underlyingError: Error)
 }
 
 /// Service for handling barcode scanning in Point of Sale
diff --git a/Modules/Tests/YosemiteTests/PointOfSale/POSProductOrVariationResolverTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/POSProductOrVariationResolverTests.swift
@@ -276,6 +276,10 @@ extension PointOfSaleBarcodeScanError: @retroactive Equatable {
             return lhsScannedCode == rhsScannedCode
         case let (.mappingError(lhsScannedCode, _), .mappingError(rhsScannedCode, _)):
             return lhsScannedCode == rhsScannedCode
+        case let (.scanTooShort(lhsScannedCode), .scanTooShort(rhsScannedCode)):
+            return lhsScannedCode == rhsScannedCode
+        case let (.timedOut(lhsScannedCode), .timedOut(rhsScannedCode)):
+            return lhsScannedCode == rhsScannedCode
         default:
             return false
         }
diff --git a/WooCommerce/Classes/POS/Models/Cart+BarcodeScanError.swift b/WooCommerce/Classes/POS/Models/Cart+BarcodeScanError.swift
@@ -6,16 +6,27 @@ extension Cart {
     mutating func updateLoadingItem(id: UUID, with error: PointOfSaleBarcodeScanError) -> Cart.PurchasableItem? {
         guard let index = purchasableItems.firstIndex(where: { $0.id == id }) else { return nil }
 
-        purchasableItems[index] = Cart.PurchasableItem(
+        purchasableItems[index] = errorItem(id: id, error: error)
+
+        return purchasableItems[index]
+    }
+
+    @discardableResult
+    mutating func addErrorItem(error: PointOfSaleBarcodeScanError) -> Cart.PurchasableItem {
+        let errorItem = errorItem(id: UUID(), error: error)
+        purchasableItems.insert(errorItem, at: purchasableItems.startIndex)
+        return errorItem
+    }
+
+    private func errorItem(id: UUID, error: PointOfSaleBarcodeScanError) -> Cart.PurchasableItem {
+        Cart.PurchasableItem(
             id: id,
             title: title(for: error),
             subtitle: subtitle(for: error),
             quantity: 1,
             state: .error,
             accessibilityLabel: accessibilityLabel(for: error)
         )
-
-        return purchasableItems[index]
     }
 
     private func title(for error: PointOfSaleBarcodeScanError) -> String {
@@ -25,11 +36,15 @@ extension Cart {
                 .variationCouldNotBeConverted(let scannedCode),
                 .notFound(let scannedCode),
                 .loadingError(let scannedCode, _),
-                .mappingError(let scannedCode, _):
+                .mappingError(let scannedCode, _),
+                .scanTooShort(let scannedCode),
+                .timedOut(let scannedCode):
             return scannedCode
         case .unsupportedProductType(_, let productName, _),
                 .downloadableProduct(_, let productName):
             return productName
+        case .parsingError:
+            return Localization.scanFailed
         }
     }
 
@@ -59,6 +74,12 @@ extension PointOfSaleBarcodeScanError {
             } else {
                 return Localization.networkRequestFailed
             }
+        case .scanTooShort:
+            return Localization.barcodeTooShort
+        case .timedOut:
+            return Localization.incompleteScan
+        case .parsingError:
+            return Localization.parsingError
         }
     }
 
@@ -92,5 +113,33 @@ extension PointOfSaleBarcodeScanError {
             value: "Parent product not found for variation",
             comment: "Error message shown when parent product is not found for a variation."
         )
+
+        static let barcodeTooShort = NSLocalizedString(
+            "pointOfSale.barcodeScan.error.barcodeTooShort",
+            value: "Barcode too short",
+            comment: "Error message shown when scan is too short."
+        )
+
+        static let incompleteScan = NSLocalizedString(
+            "pointOfSale.barcodeScan.error.incompleteScan",
+            value: "Partial barcode scan",
+            comment: "Error message shown when scan is incomplete."
+        )
+
+        static let parsingError = NSLocalizedString(
+            "pointOfSale.barcodeScan.error.parsingError",
+            value: "Couldn't read barcode",
+            comment: "Error message shown when parsing barcode data fails."
+        )
+    }
+}
+
+private extension Cart {
+    enum Localization {
+        static let scanFailed = NSLocalizedString(
+            "pointOfSale.barcodeScan.error.scanFailed",
+            value: "Scan failed",
+            comment: "Error message when scanning a barcode fails for an unknown reason, before lookup."
+        )
     }
 }
diff --git a/WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift b/WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift
@@ -34,7 +34,7 @@ protocol PointOfSaleAggregateModelProtocol {
     var couponsSearchController: PointOfSaleSearchingItemsControllerProtocol { get }
 
     var cart: Cart { get }
-    func barcodeScanned(_ barcode: String)
+    func barcodeScanned(_ result: Result<String, Error>)
     func addToCart(_ item: POSItem)
     func remove(cartItem: CartItem)
     func removeAllItemsFromCart(types: [CartItemType])
@@ -185,49 +185,77 @@ extension PointOfSaleAggregateModel {
 // MARK: - Barcode Scanning
 @available(iOS 17.0, *)
 extension PointOfSaleAggregateModel {
-    func barcodeScanned(_ barcode: String) {
+    func barcodeScanned(_ result: Result<String, Error>) {
         Task { @MainActor [weak self] in
             guard let self else { return }
-            let placeholderItemID = cart.addLoadingItem().id
+            switch result {
+            case .success(let barcode):
+                await handleSuccessfulScan(barcode: barcode)
+            case .failure(let error):
+                await handleFailedScan(error: error)
+            }
+        }
+    }
 
-            analytics.track(
-                event: .PointOfSale.addItemToCart(
-                    sourceViewType: .scanner,
-                    itemType: .loading
-                )
+    @MainActor
+    private func handleSuccessfulScan(barcode: String) async {
+        let placeholderItemID = cart.addLoadingItem().id
+
+        analytics.track(
+            event: .PointOfSale.addItemToCart(
+                sourceViewType: .scanner,
+                itemType: .loading
             )
+        )
 
-            do throws(PointOfSaleBarcodeScanError) {
-                let item = try await barcodeScanService.getItem(barcode: barcode)
-                if let cartItem = cart.updateLoadingItem(id: placeholderItemID, with: item) {
-                    analytics.track(
-                        event: .PointOfSale.addItemToCart(
-                            sourceViewType: .scanner,
-                            itemType: .product,
-                            productType: .init(cartItem: cartItem)
-                        )
+        do throws(PointOfSaleBarcodeScanError) {
+            let item = try await barcodeScanService.getItem(barcode: barcode)
+            if let cartItem = cart.updateLoadingItem(id: placeholderItemID, with: item) {
+                analytics.track(
+                    event: .PointOfSale.addItemToCart(
+                        sourceViewType: .scanner,
+                        itemType: .product,
+                        productType: .init(cartItem: cartItem)
                     )
+                )
 
-                    cart.accessibilityFocusedItemID = cartItem.id
-                }
-            } catch {
-                DDLogInfo("Failed to find item by barcode: \(error)")
-                if let errorItem = cart.updateLoadingItem(id: placeholderItemID, with: error) {
-                    // Only play a sound and track analytics if the item still exists in the cart.
-                    await soundPlayer.playSound(.barcodeScanFailure, completion: { [weak self] in
-                        self?.cart.accessibilityFocusedItemID = errorItem.id
-                    })
-
-                    analytics.track(
-                        event: .PointOfSale.addItemToCart(
-                            sourceViewType: .scanner,
-                            itemType: .error,
-                            error: error
-                        )
-                    )
-                }
+                cart.accessibilityFocusedItemID = cartItem.id
             }
+        } catch {
+            DDLogInfo("Failed to find item by barcode: \(error)")
+            if let _ = cart.updateLoadingItem(id: placeholderItemID, with: error) {
+                await handleErrorItemAdded(error)
+            }
+        }
+    }
+
+    @MainActor
+    private func handleFailedScan(error: Error) async {
+        let scanError = switch error {
+        case HIDBarcodeParserError.scanTooShort(let barcode):
+            PointOfSaleBarcodeScanError.scanTooShort(scannedCode: barcode)
+        case HIDBarcodeParserError.timedOut(let barcode):
+            PointOfSaleBarcodeScanError.timedOut(scannedCode: barcode)
+        default:
+            PointOfSaleBarcodeScanError.parsingError(underlyingError: error)
         }
+
+        cart.addErrorItem(error: scanError)
+        await handleErrorItemAdded(scanError)
+    }
+
+    @MainActor
+    private func handleErrorItemAdded(_ error: PointOfSaleBarcodeScanError) async {
+        // Only play a sound and track analytics if the item still exists in the cart.
+        await soundPlayer.playSound(.barcodeScanFailure)
+
+        analytics.track(
+            event: .PointOfSale.addItemToCart(
+                sourceViewType: .scanner,
+                itemType: .error,
+                error: error
+            )
+        )
     }
 }
 
diff --git a/WooCommerce/Classes/POS/Presentation/Barcode Scanning/BarcodeScannerContainer.swift b/WooCommerce/Classes/POS/Presentation/Barcode Scanning/BarcodeScannerContainer.swift
@@ -10,12 +10,12 @@ import SwiftUI
 struct BarcodeScannerContainer: View {
     /// Configuration for the barcode scanner
     let configuration: HIDBarcodeParserConfiguration
-    /// Callback that is triggered when a barcode is successfully scanned
-    let onScan: (String) -> Void
+    /// Callback that is triggered when a barcode scan completes (success or failure)
+    let onScan: (Result<String, Error>) -> Void
 
     init(
         configuration: HIDBarcodeParserConfiguration = .default,
-        onScan: @escaping (String) -> Void
+        onScan: @escaping (Result<String, Error>) -> Void
     ) {
         self.configuration = configuration
         self.onScan = onScan
@@ -37,7 +37,7 @@ struct BarcodeScannerContainer: View {
 /// keyboard input for barcode scanning.
 struct BarcodeScannerContainerRepresentable: UIViewControllerRepresentable {
     let configuration: HIDBarcodeParserConfiguration
-    let onScan: (String) -> Void
+    let onScan: (Result<String, Error>) -> Void
 
     func makeUIViewController(context: Context) -> BarcodeScannerHostingController {
         let controller = BarcodeScannerHostingController(
@@ -47,28 +47,23 @@ struct BarcodeScannerContainerRepresentable: UIViewControllerRepresentable {
         return controller
     }
 
-    func updateUIViewController(_ uiViewController: BarcodeScannerHostingController, context: Context) {
-        uiViewController.configuration = configuration
-        uiViewController.onScan = onScan
-    }
+    func updateUIViewController(_ uiViewController: BarcodeScannerHostingController, context: Context) {}
 }
 
 /// A UIHostingController that handles keyboard input events for barcode scanning.
 /// This controller captures keyboard input and interprets it as barcode data when a terminating
 /// character is detected.
 class BarcodeScannerHostingController: UIHostingController<EmptyView> {
-    var configuration: HIDBarcodeParserConfiguration
-    var onScan: (String) -> Void
-
+    private let configuration: HIDBarcodeParserConfiguration
     private let scanner: HIDBarcodeParser
 
     init(
         configuration: HIDBarcodeParserConfiguration,
-        onScan: @escaping (String) -> Void
+        onScan: @escaping (Result<String, Error>) -> Void
     ) {
         self.configuration = configuration
-        self.onScan = onScan
-        self.scanner = HIDBarcodeParser(configuration: configuration, onScan: onScan)
+        self.scanner = HIDBarcodeParser(configuration: configuration,
+                                        onScan: onScan)
         super.init(rootView: EmptyView())
     }
 
diff --git a/WooCommerce/Classes/POS/Presentation/Barcode Scanning/BarcodeScanningModifier.swift b/WooCommerce/Classes/POS/Presentation/Barcode Scanning/BarcodeScanningModifier.swift
@@ -7,7 +7,7 @@ struct BarcodeScanningModifier: ViewModifier {
     /// Whether barcode scanning is enabled
     @Binding var enabled: Bool
     /// Callback that is triggered when a barcode is successfully scanned
-    let onScan: (String) -> Void
+    let onScan: (Result<String, Error>) -> Void
 
     private var isBarcodeScani1FeatureEnabled: Bool {
         ServiceLocator.featureFlagService.isFeatureFlagEnabled(.pointOfSaleBarcodeScanningi1)
@@ -18,9 +18,7 @@ struct BarcodeScanningModifier: ViewModifier {
             content
 
             if enabled && isBarcodeScani1FeatureEnabled {
-                BarcodeScannerContainer() { scannedCode in
-                    onScan(scannedCode)
-                }
+                BarcodeScannerContainer(onScan: onScan)
             }
         }
     }
@@ -33,7 +31,8 @@ extension View {
     ///   - enabled: A binding to control whether barcode scanning is enabled. Defaults to a constant true binding.
     ///   - onScan: Callback that is triggered when a barcode is successfully scanned.
     /// - Returns: A view with barcode scanning capability.
-    func barcodeScanning(enabled: Binding<Bool> = .constant(true), onScan: @escaping (String) -> Void) -> some View {
+    func barcodeScanning(enabled: Binding<Bool> = .constant(true),
+                         onScan: @escaping (Result<String, Error>) -> Void) -> some View {
         modifier(BarcodeScanningModifier(enabled: enabled, onScan: onScan))
     }
 }
diff --git a/WooCommerce/Classes/POS/Presentation/Barcode Scanning/HIDBarcodeParser.swift b/WooCommerce/Classes/POS/Presentation/Barcode Scanning/HIDBarcodeParser.swift
@@ -7,16 +7,16 @@ import UIKit
 final class HIDBarcodeParser {
     /// Configuration for the barcode scanner
     let configuration: HIDBarcodeParserConfiguration
-    /// Callback that is triggered when a barcode is successfully scanned
-    let onScan: (String) -> Void
+    /// Callback that is triggered when a barcode scan completes (success or failure)
+    let onScan: (Result<String, Error>) -> Void
 
     private let timeProvider: TimeProvider
 
     private var buffer = ""
     private var lastKeyPressTime: Date?
 
     init(configuration: HIDBarcodeParserConfiguration,
-         onScan: @escaping (String) -> Void,
+         onScan: @escaping (Result<String, Error>) -> Void,
          timeProvider: TimeProvider = DefaultTimeProvider()) {
         self.configuration = configuration
         self.onScan = onScan
@@ -31,6 +31,7 @@ final class HIDBarcodeParser {
         // 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()
         }
 
@@ -138,7 +139,9 @@ final class HIDBarcodeParser {
 
     private func processScan() {
         if buffer.count >= configuration.minimumBarcodeLength {
-            onScan(buffer)
+            onScan(.success(buffer))
+        } else {
+            onScan(.failure(HIDBarcodeParserError.scanTooShort(barcode: buffer)))
         }
         resetScan()
     }
@@ -159,7 +162,12 @@ struct HIDBarcodeParserConfiguration {
     /// Default configuration suitable for most barcode scanners
     static let `default` = HIDBarcodeParserConfiguration(
         terminatingStrings: ["\r", "\n"],
-        minimumBarcodeLength: 4,
+        minimumBarcodeLength: 6,
         maximumInterCharacterTime: 0.2
     )
 }
+
+enum HIDBarcodeParserError: Error {
+    case scanTooShort(barcode: String)
+    case timedOut(barcode: String)
+}
diff --git a/WooCommerce/Classes/POS/Presentation/ItemListView.swift b/WooCommerce/Classes/POS/Presentation/ItemListView.swift
@@ -322,7 +322,7 @@ private extension ItemListView {
             }
 
             Button {
-                posModel.barcodeScanned(barcodeScanSimulatorText)
+                posModel.barcodeScanned(.success(barcodeScanSimulatorText))
             } label: {
                 Text("Scan!")
             }
diff --git a/WooCommerce/WooCommerceTests/POS/Mocks/MockPointOfSaleAggregateModel.swift b/WooCommerce/WooCommerceTests/POS/Mocks/MockPointOfSaleAggregateModel.swift
@@ -62,7 +62,7 @@ final class MockPointOfSaleAggregateModel: PointOfSaleAggregateModelProtocol {
 
     var cart: Cart = .init()
 
-    func barcodeScanned(_ barcode: String) { }
+    func barcodeScanned(_ result: Result<String, Error>) { }
 
     func addToCart(_ item: POSItem) { }
 
diff --git a/WooCommerce/WooCommerceTests/POS/Models/PointOfSaleAggregateModelTests.swift b/WooCommerce/WooCommerceTests/POS/Models/PointOfSaleAggregateModelTests.swift
diff --git a/WooCommerce/WooCommerceTests/POS/Presentation/Barcode Scanning/HIDBarcodeParserTests.swift b/WooCommerce/WooCommerceTests/POS/Presentation/Barcode Scanning/HIDBarcodeParserTests.swift

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,9 @@ public enum PointOfSaleBarcodeScanError: Error {`
`18`	`18`	`case notFound(scannedCode: String)`
`19`	`19`	`case loadingError(scannedCode: String, underlyingError: Error)`
`20`	`20`	`case mappingError(scannedCode: String, underlyingError: Error)`
	`21`	`+ case scanTooShort(scannedCode: String)`
	`22`	`+ case timedOut(scannedCode: String)`
	`23`	`+ case parsingError(underlyingError: Error)`
`21`	`24`	`}`
`22`	`25`
`23`	`26`	`/// Service for handling barcode scanning in Point of Sale`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ struct BarcodeScanningModifier: ViewModifier {`
`7`	`7`	`/// Whether barcode scanning is enabled`
`8`	`8`	`@Binding var enabled: Bool`
`9`	`9`	`/// Callback that is triggered when a barcode is successfully scanned`
`10`		`- let onScan: (String) -> Void`
	`10`	`+ let onScan: (Result<String, Error>) -> Void`
`11`	`11`
`12`	`12`	`private var isBarcodeScani1FeatureEnabled: Bool {`
`13`	`13`	`ServiceLocator.featureFlagService.isFeatureFlagEnabled(.pointOfSaleBarcodeScanningi1)`
`@@ -18,9 +18,7 @@ struct BarcodeScanningModifier: ViewModifier {`
`18`	`18`	`content`
`19`	`19`
`20`	`20`	`if enabled && isBarcodeScani1FeatureEnabled {`
`21`		`- BarcodeScannerContainer() { scannedCode in`
`22`		`- onScan(scannedCode)`
`23`		`- }`
	`21`	`+ BarcodeScannerContainer(onScan: onScan)`
`24`	`22`	`}`
`25`	`23`	`}`
`26`	`24`	`}`
`@@ -33,7 +31,8 @@ extension View {`
`33`	`31`	`/// - enabled: A binding to control whether barcode scanning is enabled. Defaults to a constant true binding.`
`34`	`32`	`/// - onScan: Callback that is triggered when a barcode is successfully scanned.`
`35`	`33`	`/// - Returns: A view with barcode scanning capability.`
`36`		`- func barcodeScanning(enabled: Binding<Bool> = .constant(true), onScan: @escaping (String) -> Void) -> some View {`
	`34`	`+ func barcodeScanning(enabled: Binding<Bool> = .constant(true),`
	`35`	`+ onScan: @escaping (Result<String, Error>) -> Void) -> some View {`
`37`	`36`	`modifier(BarcodeScanningModifier(enabled: enabled, onScan: onScan))`
`38`	`37`	`}`
`39`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -322,7 +322,7 @@ private extension ItemListView {`
`322`	`322`	`}`
`323`	`323`
`324`	`324`	`Button {`
`325`		`- posModel.barcodeScanned(barcodeScanSimulatorText)`
	`325`	`+ posModel.barcodeScanned(.success(barcodeScanSimulatorText))`
`326`	`326`	`} label: {`
`327`	`327`	`Text("Scan!")`
`328`	`328`	`}`