Show specific error for scanned downloadable items

Author: joshheald

Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift

@@ -95,8 +95,7 @@ public extension PersistedProduct {
     /// Returns a request for POS-supported products (simple and variable, non-downloadable) for a given site, ordered by name
     static func posProductsRequest(siteID: Int64) -> QueryInterfaceRequest<PersistedProduct> {
         return PersistedProduct
-            .filter(Columns.siteID == siteID)
-            .filter([ProductType.simple.rawValue, ProductType.variable.rawValue].contains(Columns.productTypeKey))
+            .basePOSProductsRequest(siteID: siteID)
             .filter(Columns.downloadable == false)
             .order(Columns.name.collating(.localizedCaseInsensitiveCompare))
     }
@@ -108,11 +107,19 @@ public extension PersistedProduct {
     /// - Returns: A query request that matches products with the given global unique ID
     static func posProductByGlobalUniqueID(siteID: Int64, globalUniqueID: String) -> QueryInterfaceRequest<PersistedProduct> {
         return PersistedProduct
-            .posProductsRequest(siteID: siteID)
+            .basePOSProductsRequest(siteID: siteID)
             .filter(Columns.globalUniqueID == globalUniqueID)
     }
 }
 
+private extension PersistedProduct {
+    static func basePOSProductsRequest(siteID: Int64) -> QueryInterfaceRequest<PersistedProduct> {
+        return PersistedProduct
+            .filter(Columns.siteID == siteID)
+            .filter([ProductType.simple.rawValue, ProductType.variable.rawValue].contains(Columns.productTypeKey))
+    }
+}
+
 // periphery:ignore - TODO: remove ignore when populating database
 private extension PersistedProduct {
     enum CodingKeys: String, CodingKey {

Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift

@@ -92,6 +92,7 @@ public extension PersistedProductVariation {
         return PersistedProductVariation
             .basePOSVariationsRequest(siteID: siteID)
             .filter(Columns.productID == parentProductID)
+            .filter(Columns.downloadable == false)
             .order(Columns.id)
     }
 
@@ -111,7 +112,6 @@ private extension PersistedProductVariation {
     static func basePOSVariationsRequest(siteID: Int64) -> QueryInterfaceRequest<PersistedProductVariation> {
         return PersistedProductVariation
             .filter(Columns.siteID == siteID)
-            .filter(Columns.downloadable == false)
     }
 }
 

Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift

@@ -21,7 +21,6 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer
     /// - Parameter barcode: The barcode string from a scan (global unique identifier)
     /// - Returns: A POSItem if found, or throws an error
     public func getItem(barcode: String) async throws(PointOfSaleBarcodeScanError) -> POSItem {
-        // Search for product or variation by global unique ID
         do {
             if let product = try searchProductByGlobalUniqueID(barcode) {
                 return try convertProductToItem(product, scannedCode: barcode)
@@ -31,7 +30,6 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer
                 return try await convertVariationToItem(variationAndParent.variation, parentProduct: variationAndParent.parentProduct, scannedCode: barcode)
             }
 
-            // No match found
             throw PointOfSaleBarcodeScanError.notFound(scannedCode: barcode)
         } catch let error as PointOfSaleBarcodeScanError {
             throw error
@@ -69,7 +67,6 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer
         do {
             let posProduct = try persistedProduct.toPOSProduct(db: grdbManager.databaseConnection)
 
-            // Validate product is not downloadable (should already be filtered by query, but double-check)
             guard !posProduct.downloadable else {
                 throw PointOfSaleBarcodeScanError.downloadableProduct(scannedCode: scannedCode, productName: posProduct.name)
             }
@@ -102,27 +99,20 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer
                                         parentProduct: PersistedProduct,
                                         scannedCode: String) async throws(PointOfSaleBarcodeScanError) -> POSItem {
         do {
-            // Validate variation is not downloadable (should already be filtered by query, but double-check)
-            guard !persistedVariation.downloadable else {
-                // We don't have the product name for variations, so use a generic message
-                throw PointOfSaleBarcodeScanError.downloadableProduct(scannedCode: scannedCode, productName: "Product variation")
-            }
-
             // Convert both variation and parent to POS models
             let posVariation = try persistedVariation.toPOSProductVariation(db: grdbManager.databaseConnection)
             let parentPOSProduct = try parentProduct.toPOSProduct(db: grdbManager.databaseConnection)
 
-            // Map parent to POSVariableParentProduct
-            let mappedParents = itemMapper.mapProductsToPOSItems(products: [parentPOSProduct])
-            guard let mappedParent = mappedParents.first,
-                  case .variableParentProduct(let variableParentProduct) = mappedParent else {
+            // Map to POSItem
+            guard let mappedParent = itemMapper.mapProductsToPOSItems(products: [parentPOSProduct]).first,
+                  case .variableParentProduct(let variableParentProduct) = mappedParent,
+                  let item = itemMapper.mapVariationsToPOSItems(variations: [posVariation], parentProduct: variableParentProduct).first else {
                 throw PointOfSaleBarcodeScanError.variationCouldNotBeConverted(scannedCode: scannedCode)
             }
 
-            // Convert to POSItem
-            let items = itemMapper.mapVariationsToPOSItems(variations: [posVariation], parentProduct: variableParentProduct)
-            guard let item = items.first else {
-                throw PointOfSaleBarcodeScanError.variationCouldNotBeConverted(scannedCode: scannedCode)
+            guard !persistedVariation.downloadable else {
+                throw PointOfSaleBarcodeScanError.downloadableProduct(scannedCode: scannedCode,
+                                                                      productName: variationName(for: item))
             }
 
             return item
@@ -132,4 +122,20 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer
             throw PointOfSaleBarcodeScanError.mappingError(scannedCode: scannedCode, underlyingError: error)
         }
     }
+
+    private func variationName(for item: POSItem) -> String {
+        guard case .variation(let posVariation) = item else {
+            return Localization.unknownVariationName
+        }
+        return posVariation.name
+    }
+}
+
+private extension PointOfSaleLocalBarcodeScanService {
+    enum Localization {
+        static let unknownVariationName = NSLocalizedString(
+            "pointOfSale.barcodeScanning.unresolved.variation.name",
+            value: "Unknown",
+            comment: "A placeholder name when we can't determine the name of a variation for an error message")
+    }
 }