From 231b31021fcef9fe32d341b328afbc86b22faa75 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 20 Oct 2025 14:16:28 +0100 Subject: [PATCH 01/17] Add single source of truth for catalog mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces `POSCatalogModeProvider` to centralize all eligibility checks for determining whether local GRDB catalog should be used. The provider consolidates and evaluates: - Feature flag (.pointOfSaleLocalCatalogi1) - Country support (US, GB only) - Currency support (USD for US, GBP for GB) - Catalog size limits (≤1000 items) Eligibility is checked once per site and cached, since these criteria don't change during a session. This provides a single source of truth that can be used for catalog sync decisions, barcode scanning, and other local catalog features. --- .../PointOfSale/POSCatalogModeProvider.swift | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 Modules/Sources/Yosemite/PointOfSale/POSCatalogModeProvider.swift diff --git a/Modules/Sources/Yosemite/PointOfSale/POSCatalogModeProvider.swift b/Modules/Sources/Yosemite/PointOfSale/POSCatalogModeProvider.swift new file mode 100644 index 00000000000..eaf9f802cbe --- /dev/null +++ b/Modules/Sources/Yosemite/PointOfSale/POSCatalogModeProvider.swift @@ -0,0 +1,108 @@ +import Foundation +import WooFoundation + +/// Provides information about which catalog mode is being used for Point of Sale +public protocol POSCatalogModeProviderProtocol { + /// Checks eligibility and determines whether the local GRDB catalog should be used. + /// + /// This performs a one-time async check of all eligibility criteria and caches the result. + /// Subsequent calls return the cached value immediately. + /// + /// - Parameter siteID: The site ID to check + /// - Returns: True if local catalog should be used based on all eligibility criteria + func shouldUseLocalCatalog(for siteID: Int64) async -> Bool +} + +/// Determines catalog mode based on comprehensive eligibility criteria. +/// +/// This provider evaluates all requirements for local catalog functionality: +/// - Feature flag enablement (gates the entire feature) +/// - Country support (US, GB only) +/// - Currency support (USD for US, GBP for GB) +/// - Catalog size limits (≤1000 items: products + variations) +/// +/// The eligibility check is performed once per site and cached, as these criteria +/// don't change during a session (except via app restart or site change). +public final class POSCatalogModeProvider: POSCatalogModeProviderProtocol { + private let isFeatureFlagEnabled: Bool + private let catalogSizeChecker: POSCatalogSizeCheckerProtocol + private let currencySettings: CurrencySettings + private let siteAddress: SiteAddress + + // Cache eligibility result per site + private var eligibilityCache: [Int64: Bool] = [:] + + public init(isFeatureFlagEnabled: Bool, + catalogSizeChecker: POSCatalogSizeCheckerProtocol, + currencySettings: CurrencySettings, + siteAddress: SiteAddress) { + self.isFeatureFlagEnabled = isFeatureFlagEnabled + self.catalogSizeChecker = catalogSizeChecker + self.currencySettings = currencySettings + self.siteAddress = siteAddress + } + + public func shouldUseLocalCatalog(for siteID: Int64) async -> Bool { + // Check cache first + if let cached = eligibilityCache[siteID] { + return cached + } + + // Perform comprehensive eligibility check + let isEligible = await checkEligibility(for: siteID) + + // Cache the result + eligibilityCache[siteID] = isEligible + + return isEligible + } + + private func checkEligibility(for siteID: Int64) async -> Bool { + // 1. Feature flag must be enabled + guard isFeatureFlagEnabled else { + DDLogInfo("📋 POSCatalogModeProvider: Local catalog disabled - feature flag not enabled") + return false + } + + // 2. Country must be supported (US or GB) + let supportedCountries: [CountryCode] = [.US, .GB] + guard supportedCountries.contains(siteAddress.countryCode) else { + DDLogInfo("📋 POSCatalogModeProvider: Local catalog disabled - unsupported country: \(siteAddress.countryCode)") + return false + } + + // 3. Currency must match country requirements + let supportedCurrencies: [CountryCode: [CurrencyCode]] = [ + .US: [.USD], + .GB: [.GBP] + ] + let expectedCurrencies = supportedCurrencies[siteAddress.countryCode] ?? [] + guard expectedCurrencies.contains(currencySettings.currencyCode) else { + DDLogInfo("📋 POSCatalogModeProvider: Local catalog disabled - unsupported currency: \(currencySettings.currencyCode) for country: \(siteAddress.countryCode)") + return false + } + + // 4. Catalog size must be within limits + do { + let catalogSize = try await catalogSizeChecker.checkCatalogSize(for: siteID) + let sizeLimit = Constants.defaultSizeLimitForPOSCatalog + guard catalogSize.totalCount <= sizeLimit else { + DDLogInfo("📋 POSCatalogModeProvider: Local catalog disabled - catalog too large: \(catalogSize.totalCount) > \(sizeLimit)") + return false + } + } catch { + DDLogError("⚠️ POSCatalogModeProvider: Failed to check catalog size for site \(siteID): \(error)") + return false + } + + DDLogInfo("✅ POSCatalogModeProvider: Local catalog enabled for site \(siteID)") + return true + } +} + +private extension POSCatalogModeProvider { + enum Constants { + /// Maximum number of items (products + variations) supported for local catalog + static let defaultSizeLimitForPOSCatalog = 1000 + } +} From 90c6c25826e13f7e6e0760a3d155a884f9df5aa9 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 20 Oct 2025 14:18:27 +0100 Subject: [PATCH 02/17] Add GRDB query methods for barcode lookup Adds query methods to PersistedProduct and PersistedProductVariation for searching by barcode (global unique ID or SKU). Product queries: - posProductByGlobalUniqueID: Search by global unique ID - posProductBySKU: Search by SKU Variation queries: - posVariationByGlobalUniqueID: Search by global unique ID - posVariationBySKU: Search by SKU All queries apply the same POS filters as the existing posProductsRequest and posVariationsRequest methods (non-downloadable, supported product types). This ensures barcode scanning respects the same product visibility rules as the POS catalog display. The service layer will use these methods sequentially: first searching by global unique ID, then falling back to SKU if no match is found. --- .../Storage/GRDB/Model/PersistedProduct.swift | 22 +++++++++++++++++ .../Model/PersistedProductVariation.swift | 24 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift index c6d53530204..c13721cf627 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift @@ -100,6 +100,28 @@ public extension PersistedProduct { .filter(Columns.downloadable == false) .order(Columns.name.collating(.localizedCaseInsensitiveCompare)) } + + /// Searches for a POS-supported product by global unique ID + /// - Parameters: + /// - siteID: The site ID + /// - globalUniqueID: The global unique ID (barcode) to search for + /// - Returns: A query request that matches products with the given global unique ID + static func posProductByGlobalUniqueID(siteID: Int64, globalUniqueID: String) -> QueryInterfaceRequest { + return PersistedProduct + .posProductsRequest(siteID: siteID) + .filter(Columns.globalUniqueID == globalUniqueID) + } + + /// Searches for a POS-supported product by SKU + /// - Parameters: + /// - siteID: The site ID + /// - sku: The SKU to search for + /// - Returns: A query request that matches products with the given SKU + static func posProductBySKU(siteID: Int64, sku: String) -> QueryInterfaceRequest { + return PersistedProduct + .posProductsRequest(siteID: siteID) + .filter(Columns.sku == sku) + } } // periphery:ignore - TODO: remove ignore when populating database diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift index 545919004b8..2c82f6f451f 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift @@ -89,6 +89,30 @@ public extension PersistedProductVariation { .filter(Columns.downloadable == false) .order(Columns.id) } + + /// Searches for a POS-supported variation by global unique ID + /// - Parameters: + /// - siteID: The site ID + /// - globalUniqueID: The global unique ID (barcode) to search for + /// - Returns: A query request that matches variations with the given global unique ID + static func posVariationByGlobalUniqueID(siteID: Int64, globalUniqueID: String) -> QueryInterfaceRequest { + return PersistedProductVariation + .filter(Columns.siteID == siteID) + .filter(Columns.downloadable == false) + .filter(Columns.globalUniqueID == globalUniqueID) + } + + /// Searches for a POS-supported variation by SKU + /// - Parameters: + /// - siteID: The site ID + /// - sku: The SKU to search for + /// - Returns: A query request that matches variations with the given SKU + static func posVariationBySKU(siteID: Int64, sku: String) -> QueryInterfaceRequest { + return PersistedProductVariation + .filter(Columns.siteID == siteID) + .filter(Columns.downloadable == false) + .filter(Columns.sku == sku) + } } // periphery:ignore - TODO: remove ignore when populating database From f0fda1647124bdd0d3f22ebc948fc7db4923b942 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 20 Oct 2025 14:31:09 +0100 Subject: [PATCH 03/17] Add local barcode scanning service Introduces `PointOfSaleLocalBarcodeScanService` that implements barcode scanning using the local GRDB catalog instead of remote API calls. Also adds a `parentProduct` relationship to `PersistedProductVariation` to fetch variations with their parent products in a single query. Key features: - Implements `PointOfSaleBarcodeScanServiceProtocol` for drop-in compatibility - Sequential search: globalUniqueID first, then SKU fallback (for both products and variations) - Uses GRDB relationships to fetch variations with parent products efficiently - Reuses existing `PointOfSaleItemMapper` for POSItem conversion - Returns same error types as remote service for consistent UX - Validates products are POS-compatible (non-downloadable, supported types) The search strategy: 1. Search products by globalUniqueID 2. Search variations by globalUniqueID (with parent product) 3. Search products by SKU 4. Search variations by SKU (with parent product) 5. Throw notFound error if no matches This allows barcode scanning to work entirely offline when local catalog is synced, matching the same filters as the POS product list. --- .../Model/PersistedProductVariation.swift | 5 + .../PointOfSaleLocalBarcodeScanService.swift | 161 ++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift index 2c82f6f451f..e4c50b79fa3 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift @@ -78,6 +78,11 @@ extension PersistedProductVariation: FetchableRecord, PersistableRecord { through: productVariationImage, using: PersistedProductVariationImage.image, key: "image") + + // Relationship to parent product + public static let parentProduct = belongsTo(PersistedProduct.self, + using: ForeignKey([Columns.siteID, Columns.productID], + to: [PersistedProduct.Columns.siteID, PersistedProduct.Columns.id])) } // MARK: - Point of Sale Requests diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift new file mode 100644 index 00000000000..4fd64c8df65 --- /dev/null +++ b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift @@ -0,0 +1,161 @@ +import Foundation +import protocol Storage.GRDBManagerProtocol +import Storage +import class WooFoundation.CurrencySettings + +/// Service for handling barcode scanning using local GRDB catalog +public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanServiceProtocol { + private let grdbManager: GRDBManagerProtocol + private let siteID: Int64 + private let itemMapper: PointOfSaleItemMapperProtocol + + public init(siteID: Int64, + grdbManager: GRDBManagerProtocol, + currencySettings: CurrencySettings, + itemMapper: PointOfSaleItemMapperProtocol? = nil) { + self.siteID = siteID + self.grdbManager = grdbManager + self.itemMapper = itemMapper ?? PointOfSaleItemMapper(currencySettings: currencySettings) + } + + /// Looks up a POSItem using a barcode scan string from the local GRDB catalog + /// - Parameter barcode: The barcode string from a scan (global unique identifier or SKU) + /// - Returns: A POSItem if found, or throws an error + public func getItem(barcode: String) async throws(PointOfSaleBarcodeScanError) -> POSItem { + // Search for product or variation by barcode + // Try globalUniqueID first, then fall back to SKU + do { + if let product = try searchProductByGlobalUniqueID(barcode) { + return try convertProductToItem(product, scannedCode: barcode) + } + + if let variationAndParent = try searchVariationByGlobalUniqueID(barcode) { + return try await convertVariationToItem(variationAndParent.variation, parentProduct: variationAndParent.parentProduct, scannedCode: barcode) + } + + if let product = try searchProductBySKU(barcode) { + return try convertProductToItem(product, scannedCode: barcode) + } + + if let variationAndParent = try searchVariationBySKU(barcode) { + 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 + } catch { + throw PointOfSaleBarcodeScanError.loadingError(scannedCode: barcode, underlyingError: error) + } + } + + // MARK: - Product Search + + private func searchProductByGlobalUniqueID(_ globalUniqueID: String) throws -> PersistedProduct? { + try grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) + } + } + + private func searchProductBySKU(_ sku: String) throws -> PersistedProduct? { + try grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductBySKU(siteID: siteID, sku: sku).fetchOne(db) + } + } + + // MARK: - Variation Search + + private func searchVariationByGlobalUniqueID(_ globalUniqueID: String) throws -> (variation: PersistedProductVariation, parentProduct: PersistedProduct)? { + try grdbManager.databaseConnection.read { db in + guard let variation = try PersistedProductVariation.posVariationByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) else { + return nil + } + // Fetch parent product using the relationship + guard let parentProduct = try variation.request(for: PersistedProductVariation.parentProduct).fetchOne(db) else { + return nil + } + return (variation, parentProduct) + } + } + + private func searchVariationBySKU(_ sku: String) throws -> (variation: PersistedProductVariation, parentProduct: PersistedProduct)? { + try grdbManager.databaseConnection.read { db in + guard let variation = try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: sku).fetchOne(db) else { + return nil + } + // Fetch parent product using the relationship + guard let parentProduct = try variation.request(for: PersistedProductVariation.parentProduct).fetchOne(db) else { + return nil + } + return (variation, parentProduct) + } + } + + // MARK: - Conversion to POSItem + + private func convertProductToItem(_ persistedProduct: PersistedProduct, scannedCode: String) throws(PointOfSaleBarcodeScanError) -> POSItem { + 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) + } + + // Validate product type + guard [.simple, .variable].contains(posProduct.productType) else { + throw PointOfSaleBarcodeScanError.unsupportedProductType( + scannedCode: scannedCode, + productName: posProduct.name, + productType: posProduct.productType + ) + } + + // Convert to POSItem + let items = itemMapper.mapProductsToPOSItems(products: [posProduct]) + guard let item = items.first else { + throw PointOfSaleBarcodeScanError.unknown(scannedCode: scannedCode) + } + + return item + } catch let error as PointOfSaleBarcodeScanError { + throw error + } catch { + throw PointOfSaleBarcodeScanError.mappingError(scannedCode: scannedCode, underlyingError: error) + } + } + + private func convertVariationToItem(_ persistedVariation: PersistedProductVariation, 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 { + 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) + } + + return item + } catch let error as PointOfSaleBarcodeScanError { + throw error + } catch { + throw PointOfSaleBarcodeScanError.mappingError(scannedCode: scannedCode, underlyingError: error) + } + } +} From f09ecf4a15bef135a8ba2780a598224d1c6570ae Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 20 Oct 2025 14:40:49 +0100 Subject: [PATCH 04/17] Wire up local barcode scanning Updates `POSTabCoordinator` to use local barcode scanning service when local catalog infrastructure is available, with automatic fallback to remote service. Changes: - Replace lazy `barcodeScanService` property with `createBarcodeScanService` method that chooses the appropriate service based on infrastructure availability - Checks for both `grdbManager` AND `catalogSyncCoordinator` to determine if local catalog is properly initialized - When both are available, creates `PointOfSaleLocalBarcodeScanService` for offline barcode scanning using local GRDB catalog - Otherwise, creates `PointOfSaleBarcodeScanService` for remote API-based barcode scanning - Service selection happens at POS view presentation time, ensuring the correct service is used based on current feature flag and infrastructure state This completes the local catalog barcode scanning feature. When the feature flag is enabled and infrastructure is available, barcode scanning works entirely offline using GRDB. When disabled or not available, it falls back to the existing remote API behavior seamlessly. --- .../POS/TabBar/POSTabCoordinator.swift | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift b/WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift index ad5d6d401a0..f399cea5dbd 100644 --- a/WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift +++ b/WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift @@ -71,13 +71,23 @@ final class POSTabCoordinator { storage: storageManager) }() - private lazy var barcodeScanService: PointOfSaleBarcodeScanService = { - PointOfSaleBarcodeScanService(siteID: siteID, - credentials: credentials, - selectedSite: defaultSitePublisher, - appPasswordSupportState: isAppPasswordSupported, - currencySettings: currencySettings) - }() + /// Creates the appropriate barcode scan service based on local catalog availability + private func createBarcodeScanService(grdbManager: GRDBManagerProtocol?, catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol?) -> any PointOfSaleBarcodeScanServiceProtocol { + // Use local barcode scanning if both GRDB manager and catalog sync coordinator are available + // This indicates the local catalog feature is properly initialized and can be used + if let grdbManager, catalogSyncCoordinator != nil { + return PointOfSaleLocalBarcodeScanService(siteID: siteID, + grdbManager: grdbManager, + currencySettings: currencySettings) + } else { + // Fall back to remote barcode scanning + return PointOfSaleBarcodeScanService(siteID: siteID, + credentials: credentials, + selectedSite: defaultSitePublisher, + appPasswordSupportState: isAppPasswordSupported, + currencySettings: currencySettings) + } + } /// Publisher to send to `AlamofireNetwork` for request authentication mode switching. private let defaultSitePublisher: AnyPublisher @@ -150,6 +160,11 @@ private extension POSTabCoordinator { let grdbManager: GRDBManagerProtocol? = serviceAdaptor.featureFlags.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1) ? ServiceLocator.grdbManager : nil let catalogSyncCoordinator = ServiceLocator.posCatalogSyncCoordinator + // Create appropriate barcode scan service based on local catalog availability + // Will use local GRDB-based scanning if both grdbManager and catalogSyncCoordinator are available, + // otherwise falls back to remote API-based scanning + let barcodeScanService = createBarcodeScanService(grdbManager: grdbManager, catalogSyncCoordinator: catalogSyncCoordinator) + if let receiptService = POSReceiptService(siteID: siteID, credentials: credentials, selectedSite: defaultSitePublisher, From 9a84ddbafb16231895ec64bb1330dc3cbe65dc6a Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 20 Oct 2025 14:45:09 +0100 Subject: [PATCH 05/17] Remove unused POSCatalogModeProvider The POSCatalogModeProvider was created but not used in the final implementation. Barcode service selection is done by checking for grdbManager and catalogSyncCoordinator availability directly. This can be properly refactored in a future issue to consolidate eligibility checking across POS features. --- .../PointOfSale/POSCatalogModeProvider.swift | 108 ------------------ 1 file changed, 108 deletions(-) delete mode 100644 Modules/Sources/Yosemite/PointOfSale/POSCatalogModeProvider.swift diff --git a/Modules/Sources/Yosemite/PointOfSale/POSCatalogModeProvider.swift b/Modules/Sources/Yosemite/PointOfSale/POSCatalogModeProvider.swift deleted file mode 100644 index eaf9f802cbe..00000000000 --- a/Modules/Sources/Yosemite/PointOfSale/POSCatalogModeProvider.swift +++ /dev/null @@ -1,108 +0,0 @@ -import Foundation -import WooFoundation - -/// Provides information about which catalog mode is being used for Point of Sale -public protocol POSCatalogModeProviderProtocol { - /// Checks eligibility and determines whether the local GRDB catalog should be used. - /// - /// This performs a one-time async check of all eligibility criteria and caches the result. - /// Subsequent calls return the cached value immediately. - /// - /// - Parameter siteID: The site ID to check - /// - Returns: True if local catalog should be used based on all eligibility criteria - func shouldUseLocalCatalog(for siteID: Int64) async -> Bool -} - -/// Determines catalog mode based on comprehensive eligibility criteria. -/// -/// This provider evaluates all requirements for local catalog functionality: -/// - Feature flag enablement (gates the entire feature) -/// - Country support (US, GB only) -/// - Currency support (USD for US, GBP for GB) -/// - Catalog size limits (≤1000 items: products + variations) -/// -/// The eligibility check is performed once per site and cached, as these criteria -/// don't change during a session (except via app restart or site change). -public final class POSCatalogModeProvider: POSCatalogModeProviderProtocol { - private let isFeatureFlagEnabled: Bool - private let catalogSizeChecker: POSCatalogSizeCheckerProtocol - private let currencySettings: CurrencySettings - private let siteAddress: SiteAddress - - // Cache eligibility result per site - private var eligibilityCache: [Int64: Bool] = [:] - - public init(isFeatureFlagEnabled: Bool, - catalogSizeChecker: POSCatalogSizeCheckerProtocol, - currencySettings: CurrencySettings, - siteAddress: SiteAddress) { - self.isFeatureFlagEnabled = isFeatureFlagEnabled - self.catalogSizeChecker = catalogSizeChecker - self.currencySettings = currencySettings - self.siteAddress = siteAddress - } - - public func shouldUseLocalCatalog(for siteID: Int64) async -> Bool { - // Check cache first - if let cached = eligibilityCache[siteID] { - return cached - } - - // Perform comprehensive eligibility check - let isEligible = await checkEligibility(for: siteID) - - // Cache the result - eligibilityCache[siteID] = isEligible - - return isEligible - } - - private func checkEligibility(for siteID: Int64) async -> Bool { - // 1. Feature flag must be enabled - guard isFeatureFlagEnabled else { - DDLogInfo("📋 POSCatalogModeProvider: Local catalog disabled - feature flag not enabled") - return false - } - - // 2. Country must be supported (US or GB) - let supportedCountries: [CountryCode] = [.US, .GB] - guard supportedCountries.contains(siteAddress.countryCode) else { - DDLogInfo("📋 POSCatalogModeProvider: Local catalog disabled - unsupported country: \(siteAddress.countryCode)") - return false - } - - // 3. Currency must match country requirements - let supportedCurrencies: [CountryCode: [CurrencyCode]] = [ - .US: [.USD], - .GB: [.GBP] - ] - let expectedCurrencies = supportedCurrencies[siteAddress.countryCode] ?? [] - guard expectedCurrencies.contains(currencySettings.currencyCode) else { - DDLogInfo("📋 POSCatalogModeProvider: Local catalog disabled - unsupported currency: \(currencySettings.currencyCode) for country: \(siteAddress.countryCode)") - return false - } - - // 4. Catalog size must be within limits - do { - let catalogSize = try await catalogSizeChecker.checkCatalogSize(for: siteID) - let sizeLimit = Constants.defaultSizeLimitForPOSCatalog - guard catalogSize.totalCount <= sizeLimit else { - DDLogInfo("📋 POSCatalogModeProvider: Local catalog disabled - catalog too large: \(catalogSize.totalCount) > \(sizeLimit)") - return false - } - } catch { - DDLogError("⚠️ POSCatalogModeProvider: Failed to check catalog size for site \(siteID): \(error)") - return false - } - - DDLogInfo("✅ POSCatalogModeProvider: Local catalog enabled for site \(siteID)") - return true - } -} - -private extension POSCatalogModeProvider { - enum Constants { - /// Maximum number of items (products + variations) supported for local catalog - static let defaultSizeLimitForPOSCatalog = 1000 - } -} From bf505b383f8677fa6c0fba654070a7387a9c7303 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 20 Oct 2025 14:59:36 +0100 Subject: [PATCH 06/17] Add tests for barcode scanning from local catalog Adds test coverage for local catalog barcode scanning functionality: 1. PointOfSaleLocalBarcodeScanServiceTests: - Tests finding products by global unique ID and SKU - Tests finding variations with parent products - Tests search priority (global unique ID before SKU) - Tests error cases (not found, downloadable, unsupported types) - Uses actual GRDBManager for integration testing 2. PersistedProductBarcodeQueryTests: - Tests posProductByGlobalUniqueID query method - Tests posProductBySKU query method - Verifies POS filters (non-downloadable, supported types) - Tests site isolation 3. PersistedProductVariationBarcodeQueryTests: - Tests posVariationByGlobalUniqueID query method - Tests posVariationBySKU query method - Verifies variation filters (non-downloadable) - Tests parent product relationship fetching - Tests site isolation --- .../PersistedProductBarcodeQueryTests.swift | 311 ++++++++++++++ ...tedProductVariationBarcodeQueryTests.swift | 406 ++++++++++++++++++ ...ntOfSaleLocalBarcodeScanServiceTests.swift | 345 +++++++++++++++ 3 files changed, 1062 insertions(+) create mode 100644 Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift create mode 100644 Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift create mode 100644 Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift diff --git a/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift b/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift new file mode 100644 index 00000000000..b47f3a20953 --- /dev/null +++ b/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift @@ -0,0 +1,311 @@ +import Foundation +import Testing +@testable import Storage + +@Suite("PersistedProduct Barcode Query Tests") +struct PersistedProductBarcodeQueryTests { + private let siteID: Int64 = 123 + private var grdbManager: GRDBManager! + + init() async throws { + grdbManager = try GRDBManager() + + // Initialize site + try await grdbManager.databaseConnection.write { db in + try PersistedSite(id: siteID).insert(db) + } + } + + // MARK: - Global Unique ID Query Tests + + @Test("posProductByGlobalUniqueID finds product with matching global unique ID") + func test_finds_product_by_global_unique_id() async throws { + // Given + let globalUniqueID = "UPC-123456" + let product = PersistedProduct( + id: 1, + siteID: siteID, + name: "Test Product", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: "SKU-001", + globalUniqueID: globalUniqueID, + price: "10.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(product) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) + } + + // Then + #expect(result != nil) + #expect(result?.id == 1) + #expect(result?.name == "Test Product") + #expect(result?.globalUniqueID == globalUniqueID) + } + + @Test("posProductByGlobalUniqueID returns nil when no match") + func test_returns_nil_when_no_global_unique_id_match() async throws { + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: "NONEXISTENT").fetchOne(db) + } + + // Then + #expect(result == nil) + } + + @Test("posProductByGlobalUniqueID filters out downloadable products") + func test_global_unique_id_query_filters_downloadable() async throws { + // Given + let globalUniqueID = "UPC-DOWNLOADABLE" + let downloadableProduct = PersistedProduct( + id: 2, + siteID: siteID, + name: "Downloadable Product", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: globalUniqueID, + price: "5.00", + downloadable: true, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(downloadableProduct) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) + } + + // Then + #expect(result == nil) + } + + @Test("posProductByGlobalUniqueID filters out unsupported product types") + func test_global_unique_id_query_filters_unsupported_types() async throws { + // Given + let globalUniqueID = "UPC-GROUPED" + let groupedProduct = PersistedProduct( + id: 3, + siteID: siteID, + name: "Grouped Product", + productTypeKey: "grouped", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: globalUniqueID, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(groupedProduct) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) + } + + // Then + #expect(result == nil) + } + + // MARK: - SKU Query Tests + + @Test("posProductBySKU finds product with matching SKU") + func test_finds_product_by_sku() async throws { + // Given + let sku = "SKU-ABC-123" + let product = PersistedProduct( + id: 4, + siteID: siteID, + name: "Product with SKU", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: sku, + globalUniqueID: nil, + price: "15.00", + downloadable: false, + parentID: 0, + manageStock: true, + stockQuantity: 10, + stockStatusKey: "instock" + ) + try await insertProduct(product) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductBySKU(siteID: siteID, sku: sku).fetchOne(db) + } + + // Then + #expect(result != nil) + #expect(result?.id == 4) + #expect(result?.sku == sku) + } + + @Test("posProductBySKU returns nil when no match") + func test_returns_nil_when_no_sku_match() async throws { + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductBySKU(siteID: siteID, sku: "NONEXISTENT-SKU").fetchOne(db) + } + + // Then + #expect(result == nil) + } + + @Test("posProductBySKU filters out downloadable products") + func test_sku_query_filters_downloadable() async throws { + // Given + let sku = "SKU-DOWNLOADABLE" + let downloadableProduct = PersistedProduct( + id: 5, + siteID: siteID, + name: "Downloadable Product", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: sku, + globalUniqueID: nil, + price: "5.00", + downloadable: true, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(downloadableProduct) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductBySKU(siteID: siteID, sku: sku).fetchOne(db) + } + + // Then + #expect(result == nil) + } + + @Test("posProductBySKU accepts variable products") + func test_sku_query_accepts_variable_products() async throws { + // Given + let sku = "SKU-VARIABLE" + let variableProduct = PersistedProduct( + id: 6, + siteID: siteID, + name: "Variable Product", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: sku, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(variableProduct) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductBySKU(siteID: siteID, sku: sku).fetchOne(db) + } + + // Then + #expect(result != nil) + #expect(result?.productTypeKey == "variable") + } + + // MARK: - Site Isolation Tests + + @Test("Queries only return products from specified site") + func test_queries_respect_site_isolation() async throws { + // Given + let otherSiteID: Int64 = 456 + let barcode = "SHARED-BARCODE" + + // Insert site + try await grdbManager.databaseConnection.write { db in + try PersistedSite(id: otherSiteID).insert(db) + } + + // Insert product for our site + let ourProduct = PersistedProduct( + id: 7, + siteID: siteID, + name: "Our Product", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: barcode, + globalUniqueID: barcode, + price: "10.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + + // Insert product for other site + let otherProduct = PersistedProduct( + id: 8, + siteID: otherSiteID, + name: "Other Site Product", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: barcode, + globalUniqueID: barcode, + price: "20.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + + try await insertProduct(ourProduct) + try await insertProduct(otherProduct) + + // When + let resultBySKU = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductBySKU(siteID: siteID, sku: barcode).fetchOne(db) + } + let resultByGlobalID = try await grdbManager.databaseConnection.read { db in + try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: barcode).fetchOne(db) + } + + // Then + #expect(resultBySKU?.siteID == siteID) + #expect(resultBySKU?.id == 7) + #expect(resultByGlobalID?.siteID == siteID) + #expect(resultByGlobalID?.id == 7) + } + + // MARK: - Helper Methods + + private func insertProduct(_ product: PersistedProduct) async throws { + try await grdbManager.databaseConnection.write { db in + try product.insert(db) + } + } +} diff --git a/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift b/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift new file mode 100644 index 00000000000..33fc59d744a --- /dev/null +++ b/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift @@ -0,0 +1,406 @@ +import Foundation +import Testing +@testable import Storage + +@Suite("PersistedProductVariation Barcode Query Tests") +struct PersistedProductVariationBarcodeQueryTests { + private let siteID: Int64 = 123 + private var grdbManager: GRDBManager! + + init() async throws { + grdbManager = try GRDBManager() + + // Initialize site + try await grdbManager.databaseConnection.write { db in + try PersistedSite(id: siteID).insert(db) + } + } + + // MARK: - Global Unique ID Query Tests + + @Test("posVariationByGlobalUniqueID finds variation with matching global unique ID") + func test_finds_variation_by_global_unique_id() async throws { + // Given + let globalUniqueID = "VAR-UPC-789" + + // Insert parent product first (required by foreign key) + let parentProduct = PersistedProduct( + id: 10, + siteID: siteID, + name: "Parent Product", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(parentProduct) + + let variation = PersistedProductVariation( + id: 100, + siteID: siteID, + productID: 10, + sku: "VAR-SKU-001", + globalUniqueID: globalUniqueID, + price: "12.50", + downloadable: false, + fullDescription: nil, + manageStock: true, + stockQuantity: 5, + stockStatusKey: "instock" + ) + try await insertVariation(variation) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.posVariationByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) + } + + // Then + #expect(result != nil) + #expect(result?.id == 100) + #expect(result?.globalUniqueID == globalUniqueID) + #expect(result?.price == "12.50") + } + + @Test("posVariationByGlobalUniqueID returns nil when no match") + func test_returns_nil_when_no_global_unique_id_match() async throws { + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.posVariationByGlobalUniqueID(siteID: siteID, globalUniqueID: "NONEXISTENT").fetchOne(db) + } + + // Then + #expect(result == nil) + } + + @Test("posVariationByGlobalUniqueID filters out downloadable variations") + func test_global_unique_id_query_filters_downloadable() async throws { + // Given + let globalUniqueID = "VAR-DOWNLOADABLE" + + // Insert parent product first + let parentProduct = PersistedProduct( + id: 10, + siteID: siteID, + name: "Parent Product", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(parentProduct) + + let downloadableVariation = PersistedProductVariation( + id: 101, + siteID: siteID, + productID: 10, + sku: nil, + globalUniqueID: globalUniqueID, + price: "5.00", + downloadable: true, + fullDescription: nil, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertVariation(downloadableVariation) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.posVariationByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) + } + + // Then + #expect(result == nil) + } + + // MARK: - SKU Query Tests + + @Test("posVariationBySKU finds variation with matching SKU") + func test_finds_variation_by_sku() async throws { + // Given + let sku = "VAR-SKU-XYZ" + + // Insert parent product first + let parentProduct = PersistedProduct( + id: 11, + siteID: siteID, + name: "Parent Product", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(parentProduct) + + let variation = PersistedProductVariation( + id: 102, + siteID: siteID, + productID: 11, + sku: sku, + globalUniqueID: nil, + price: "18.00", + downloadable: false, + fullDescription: "Test variation", + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertVariation(variation) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: sku).fetchOne(db) + } + + // Then + #expect(result != nil) + #expect(result?.id == 102) + #expect(result?.sku == sku) + } + + @Test("posVariationBySKU returns nil when no match") + func test_returns_nil_when_no_sku_match() async throws { + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: "NONEXISTENT-SKU").fetchOne(db) + } + + // Then + #expect(result == nil) + } + + @Test("posVariationBySKU filters out downloadable variations") + func test_sku_query_filters_downloadable() async throws { + // Given + let sku = "VAR-SKU-DOWNLOADABLE" + + // Insert parent product first + let parentProduct = PersistedProduct( + id: 12, + siteID: siteID, + name: "Parent Product", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(parentProduct) + + let downloadableVariation = PersistedProductVariation( + id: 103, + siteID: siteID, + productID: 12, + sku: sku, + globalUniqueID: nil, + price: "7.00", + downloadable: true, + fullDescription: nil, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertVariation(downloadableVariation) + + // When + let result = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: sku).fetchOne(db) + } + + // Then + #expect(result == nil) + } + + // MARK: - Site Isolation Tests + + @Test("Queries only return variations from specified site") + func test_queries_respect_site_isolation() async throws { + // Given + let otherSiteID: Int64 = 456 + let barcode = "SHARED-VAR-BARCODE" + + // Insert other site + try await grdbManager.databaseConnection.write { db in + try PersistedSite(id: otherSiteID).insert(db) + } + + // Insert parent products for both sites + let ourParentProduct = PersistedProduct( + id: 10, + siteID: siteID, + name: "Our Parent", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + let otherParentProduct = PersistedProduct( + id: 20, + siteID: otherSiteID, + name: "Other Parent", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(ourParentProduct) + try await insertProduct(otherParentProduct) + + // Insert variation for our site + let ourVariation = PersistedProductVariation( + id: 104, + siteID: siteID, + productID: 10, + sku: barcode, + globalUniqueID: barcode, + price: "10.00", + downloadable: false, + fullDescription: nil, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + + // Insert variation for other site + let otherVariation = PersistedProductVariation( + id: 105, + siteID: otherSiteID, + productID: 20, + sku: barcode, + globalUniqueID: barcode, + price: "20.00", + downloadable: false, + fullDescription: nil, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + + try await insertVariation(ourVariation) + try await insertVariation(otherVariation) + + // When + let resultBySKU = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: barcode).fetchOne(db) + } + let resultByGlobalID = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.posVariationByGlobalUniqueID(siteID: siteID, globalUniqueID: barcode).fetchOne(db) + } + + // Then + #expect(resultBySKU?.siteID == siteID) + #expect(resultBySKU?.id == 104) + #expect(resultByGlobalID?.siteID == siteID) + #expect(resultByGlobalID?.id == 104) + } + + // MARK: - Parent Product Relationship Test + + @Test("Can fetch variation with parent product using relationship") + func test_fetch_variation_with_parent_product() async throws { + // Given + let parentProduct = PersistedProduct( + id: 50, + siteID: siteID, + name: "Parent Variable Product", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(parentProduct) + + let variation = PersistedProductVariation( + id: 500, + siteID: siteID, + productID: 50, + sku: "TEST-VAR", + globalUniqueID: "TEST-GLOBAL", + price: "15.00", + downloadable: false, + fullDescription: nil, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertVariation(variation) + + // When + let result: (PersistedProductVariation, PersistedProduct?)? = try await grdbManager.databaseConnection.read { db in + guard let variation = try PersistedProductVariation.posVariationByGlobalUniqueID(siteID: siteID, globalUniqueID: "TEST-GLOBAL").fetchOne(db) else { + return nil + } + let parentProduct = try variation.request(for: PersistedProductVariation.parentProduct).fetchOne(db) + return (variation, parentProduct) + } + + // Then + #expect(result != nil) + #expect(result?.0.id == 500) + #expect(result?.1?.id == 50) + #expect(result?.1?.name == "Parent Variable Product") + } + + // MARK: - Helper Methods + + private func insertVariation(_ variation: PersistedProductVariation) async throws { + try await grdbManager.databaseConnection.write { db in + try variation.insert(db) + } + } + + private func insertProduct(_ product: PersistedProduct) async throws { + try await grdbManager.databaseConnection.write { db in + try product.insert(db) + } + } +} diff --git a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift new file mode 100644 index 00000000000..c25094e30f2 --- /dev/null +++ b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift @@ -0,0 +1,345 @@ +import Foundation +import Testing +import WooFoundation +@testable import Storage +@testable import Yosemite + +@Suite("PointOfSaleLocalBarcodeScanService Tests") +struct PointOfSaleLocalBarcodeScanServiceTests { + private let siteID: Int64 = 123 + private var grdbManager: GRDBManager! + private var sut: PointOfSaleLocalBarcodeScanService! + + init() async throws { + grdbManager = try GRDBManager() + + // Initialize site + try await grdbManager.databaseConnection.write { db in + try PersistedSite(id: siteID).insert(db) + } + + sut = PointOfSaleLocalBarcodeScanService( + siteID: siteID, + grdbManager: grdbManager, + currencySettings: CurrencySettings() + ) + } + + // MARK: - Simple Product Tests + + @Test("Returns simple product when found by global unique ID") + func test_returns_simple_product_by_global_unique_id() async throws { + // Given + let barcode = "1234567890" + let product = PersistedProduct( + id: 1, + siteID: siteID, + name: "Test Product", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: "SKU123", + globalUniqueID: barcode, + price: "10.00", + downloadable: false, + parentID: 0, + manageStock: true, + stockQuantity: 10, + stockStatusKey: "instock" + ) + try await insertProduct(product) + + // When + let item = try await sut.getItem(barcode: barcode) + + // Then + guard case let .simpleProduct(posProduct) = item else { + Issue.record("Expected simple product, got \(item)") + return + } + #expect(posProduct.name == "Test Product") + #expect(posProduct.price == "10.00") + #expect(posProduct.productID == 1) + } + + @Test("Returns simple product when found by SKU") + func test_returns_simple_product_by_sku() async throws { + // Given + let sku = "SKU-TEST-123" + let product = PersistedProduct( + id: 2, + siteID: siteID, + name: "Product with SKU", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: sku, + globalUniqueID: nil, + price: "20.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(product) + + // When + let item = try await sut.getItem(barcode: sku) + + // Then + guard case let .simpleProduct(posProduct) = item else { + Issue.record("Expected simple product, got \(item)") + return + } + #expect(posProduct.name == "Product with SKU") + #expect(posProduct.productID == 2) + } + + @Test("Prioritizes global unique ID over SKU when both match different products") + func test_prioritizes_global_unique_id_over_sku() async throws { + // Given + let barcode = "SHARED-CODE" + + // Product with matching global unique ID + let productWithGlobalID = PersistedProduct( + id: 3, + siteID: siteID, + name: "Product with Global ID", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: "DIFFERENT-SKU", + globalUniqueID: barcode, + price: "30.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + + // Product with matching SKU + let productWithSKU = PersistedProduct( + id: 4, + siteID: siteID, + name: "Product with SKU", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: barcode, + globalUniqueID: "DIFFERENT-GLOBAL-ID", + price: "40.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + + try await insertProduct(productWithGlobalID) + try await insertProduct(productWithSKU) + + // When + let item = try await sut.getItem(barcode: barcode) + + // Then + guard case let .simpleProduct(posProduct) = item else { + Issue.record("Expected simple product, got \(item)") + return + } + // Should find the one with matching global ID first + #expect(posProduct.name == "Product with Global ID") + #expect(posProduct.productID == 3) + } + + // MARK: - Variation Tests + + @Test("Returns variation when found by global unique ID") + func test_returns_variation_by_global_unique_id() async throws { + // Given + let barcode = "VAR-9876543210" + + // Insert parent product + let parentProduct = PersistedProduct( + id: 10, + siteID: siteID, + name: "Variable Parent", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(parentProduct) + + // Insert variation + let variation = PersistedProductVariation( + id: 100, + siteID: siteID, + productID: 10, + sku: "VAR-SKU", + globalUniqueID: barcode, + price: "15.00", + downloadable: false, + fullDescription: nil, + manageStock: true, + stockQuantity: 5, + stockStatusKey: "instock" + ) + try await insertVariation(variation) + + // When + let item = try await sut.getItem(barcode: barcode) + + // Then + guard case let .variation(variationItem) = item else { + Issue.record("Expected variation, got \(item)") + return + } + #expect(variationItem.productVariationID == 100) + #expect(variationItem.price == "15.00") + #expect(variationItem.parentProductName == "Variable Parent") + #expect(variationItem.productID == 10) + } + + @Test("Returns variation when found by SKU") + func test_returns_variation_by_sku() async throws { + // Given + let sku = "VAR-SKU-456" + + // Insert parent product + let parentProduct = PersistedProduct( + id: 11, + siteID: siteID, + name: "Another Variable Parent", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(parentProduct) + + // Insert variation + let variation = PersistedProductVariation( + id: 110, + siteID: siteID, + productID: 11, + sku: sku, + globalUniqueID: nil, + price: "25.00", + downloadable: false, + fullDescription: nil, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertVariation(variation) + + // When + let item = try await sut.getItem(barcode: sku) + + // Then + guard case let .variation(variationItem) = item else { + Issue.record("Expected variation, got \(item)") + return + } + #expect(variationItem.productVariationID == 110) + } + + // MARK: - Error Tests + + @Test("Throws notFound when barcode doesn't match any product or variation") + func test_throws_not_found_when_no_match() async throws { + // Given + let barcode = "NONEXISTENT-BARCODE" + + // When/Then + await #expect(throws: PointOfSaleBarcodeScanError.notFound(scannedCode: barcode)) { + _ = try await sut.getItem(barcode: barcode) + } + } + + @Test("Filters out downloadable products") + func test_filters_out_downloadable_products() async throws { + // Given + let barcode = "DOWNLOADABLE-123" + let downloadableProduct = PersistedProduct( + id: 20, + siteID: siteID, + name: "Downloadable Product", + productTypeKey: "simple", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: barcode, + price: "10.00", + downloadable: true, // Downloadable + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(downloadableProduct) + + // When/Then - Should not find the downloadable product + await #expect(throws: PointOfSaleBarcodeScanError.notFound(scannedCode: barcode)) { + _ = try await sut.getItem(barcode: barcode) + } + } + + @Test("Filters out unsupported product types") + func test_filters_out_unsupported_product_types() async throws { + // Given + let barcode = "GROUPED-123" + let groupedProduct = PersistedProduct( + id: 21, + siteID: siteID, + name: "Grouped Product", + productTypeKey: "grouped", // Unsupported type + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: barcode, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(groupedProduct) + + // When/Then - Should not find unsupported product type + await #expect(throws: PointOfSaleBarcodeScanError.notFound(scannedCode: barcode)) { + _ = try await sut.getItem(barcode: barcode) + } + } + + // MARK: - Helper Methods + + private func insertProduct(_ product: PersistedProduct) async throws { + try await grdbManager.databaseConnection.write { db in + try product.insert(db) + } + } + + private func insertVariation(_ variation: PersistedProductVariation) async throws { + try await grdbManager.databaseConnection.write { db in + try variation.insert(db) + } + } +} From 7fe5983e75a514c17c9938c28bae6f4415bdca03 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 21 Oct 2025 07:45:27 +0100 Subject: [PATCH 07/17] Use only global unique ID for barcode scanning Simplified barcode scanning to only use global unique IDs. The previous implementation also searched by SKU, but this was incorrect as barcode scanning should only match barcodes (global unique IDs), not SKUs. SKU matching is still available through search. Changes: - Removed posProductBySKU and posVariationBySKU query methods - Simplified PointOfSaleLocalBarcodeScanService to only search by global unique ID - Removed all SKU-related tests and updated site isolation tests --- .../Storage/GRDB/Model/PersistedProduct.swift | 11 -- .../Model/PersistedProductVariation.swift | 12 -- .../PointOfSaleLocalBarcodeScanService.swift | 32 +--- .../PersistedProductBarcodeQueryTests.swift | 114 -------------- ...tedProductVariationBarcodeQueryTests.swift | 116 -------------- ...ntOfSaleLocalBarcodeScanServiceTests.swift | 142 ------------------ 6 files changed, 2 insertions(+), 425 deletions(-) diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift index c13721cf627..84fb4755eb2 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift @@ -111,17 +111,6 @@ public extension PersistedProduct { .posProductsRequest(siteID: siteID) .filter(Columns.globalUniqueID == globalUniqueID) } - - /// Searches for a POS-supported product by SKU - /// - Parameters: - /// - siteID: The site ID - /// - sku: The SKU to search for - /// - Returns: A query request that matches products with the given SKU - static func posProductBySKU(siteID: Int64, sku: String) -> QueryInterfaceRequest { - return PersistedProduct - .posProductsRequest(siteID: siteID) - .filter(Columns.sku == sku) - } } // periphery:ignore - TODO: remove ignore when populating database diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift index e4c50b79fa3..ac91e34d578 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift @@ -106,18 +106,6 @@ public extension PersistedProductVariation { .filter(Columns.downloadable == false) .filter(Columns.globalUniqueID == globalUniqueID) } - - /// Searches for a POS-supported variation by SKU - /// - Parameters: - /// - siteID: The site ID - /// - sku: The SKU to search for - /// - Returns: A query request that matches variations with the given SKU - static func posVariationBySKU(siteID: Int64, sku: String) -> QueryInterfaceRequest { - return PersistedProductVariation - .filter(Columns.siteID == siteID) - .filter(Columns.downloadable == false) - .filter(Columns.sku == sku) - } } // periphery:ignore - TODO: remove ignore when populating database diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift index 4fd64c8df65..5c2315c7189 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift +++ b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift @@ -19,11 +19,10 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer } /// Looks up a POSItem using a barcode scan string from the local GRDB catalog - /// - Parameter barcode: The barcode string from a scan (global unique identifier or SKU) + /// - 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 barcode - // Try globalUniqueID first, then fall back to SKU + // Search for product or variation by global unique ID do { if let product = try searchProductByGlobalUniqueID(barcode) { return try convertProductToItem(product, scannedCode: barcode) @@ -33,14 +32,6 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer return try await convertVariationToItem(variationAndParent.variation, parentProduct: variationAndParent.parentProduct, scannedCode: barcode) } - if let product = try searchProductBySKU(barcode) { - return try convertProductToItem(product, scannedCode: barcode) - } - - if let variationAndParent = try searchVariationBySKU(barcode) { - return try await convertVariationToItem(variationAndParent.variation, parentProduct: variationAndParent.parentProduct, scannedCode: barcode) - } - // No match found throw PointOfSaleBarcodeScanError.notFound(scannedCode: barcode) } catch let error as PointOfSaleBarcodeScanError { @@ -58,12 +49,6 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer } } - private func searchProductBySKU(_ sku: String) throws -> PersistedProduct? { - try grdbManager.databaseConnection.read { db in - try PersistedProduct.posProductBySKU(siteID: siteID, sku: sku).fetchOne(db) - } - } - // MARK: - Variation Search private func searchVariationByGlobalUniqueID(_ globalUniqueID: String) throws -> (variation: PersistedProductVariation, parentProduct: PersistedProduct)? { @@ -79,19 +64,6 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer } } - private func searchVariationBySKU(_ sku: String) throws -> (variation: PersistedProductVariation, parentProduct: PersistedProduct)? { - try grdbManager.databaseConnection.read { db in - guard let variation = try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: sku).fetchOne(db) else { - return nil - } - // Fetch parent product using the relationship - guard let parentProduct = try variation.request(for: PersistedProductVariation.parentProduct).fetchOne(db) else { - return nil - } - return (variation, parentProduct) - } - } - // MARK: - Conversion to POSItem private func convertProductToItem(_ persistedProduct: PersistedProduct, scannedCode: String) throws(PointOfSaleBarcodeScanError) -> POSItem { diff --git a/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift b/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift index b47f3a20953..932a1e784d0 100644 --- a/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift +++ b/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift @@ -125,115 +125,6 @@ struct PersistedProductBarcodeQueryTests { #expect(result == nil) } - // MARK: - SKU Query Tests - - @Test("posProductBySKU finds product with matching SKU") - func test_finds_product_by_sku() async throws { - // Given - let sku = "SKU-ABC-123" - let product = PersistedProduct( - id: 4, - siteID: siteID, - name: "Product with SKU", - productTypeKey: "simple", - fullDescription: nil, - shortDescription: nil, - sku: sku, - globalUniqueID: nil, - price: "15.00", - downloadable: false, - parentID: 0, - manageStock: true, - stockQuantity: 10, - stockStatusKey: "instock" - ) - try await insertProduct(product) - - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProduct.posProductBySKU(siteID: siteID, sku: sku).fetchOne(db) - } - - // Then - #expect(result != nil) - #expect(result?.id == 4) - #expect(result?.sku == sku) - } - - @Test("posProductBySKU returns nil when no match") - func test_returns_nil_when_no_sku_match() async throws { - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProduct.posProductBySKU(siteID: siteID, sku: "NONEXISTENT-SKU").fetchOne(db) - } - - // Then - #expect(result == nil) - } - - @Test("posProductBySKU filters out downloadable products") - func test_sku_query_filters_downloadable() async throws { - // Given - let sku = "SKU-DOWNLOADABLE" - let downloadableProduct = PersistedProduct( - id: 5, - siteID: siteID, - name: "Downloadable Product", - productTypeKey: "simple", - fullDescription: nil, - shortDescription: nil, - sku: sku, - globalUniqueID: nil, - price: "5.00", - downloadable: true, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(downloadableProduct) - - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProduct.posProductBySKU(siteID: siteID, sku: sku).fetchOne(db) - } - - // Then - #expect(result == nil) - } - - @Test("posProductBySKU accepts variable products") - func test_sku_query_accepts_variable_products() async throws { - // Given - let sku = "SKU-VARIABLE" - let variableProduct = PersistedProduct( - id: 6, - siteID: siteID, - name: "Variable Product", - productTypeKey: "variable", - fullDescription: nil, - shortDescription: nil, - sku: sku, - globalUniqueID: nil, - price: "0.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(variableProduct) - - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProduct.posProductBySKU(siteID: siteID, sku: sku).fetchOne(db) - } - - // Then - #expect(result != nil) - #expect(result?.productTypeKey == "variable") - } - // MARK: - Site Isolation Tests @Test("Queries only return products from specified site") @@ -287,16 +178,11 @@ struct PersistedProductBarcodeQueryTests { try await insertProduct(otherProduct) // When - let resultBySKU = try await grdbManager.databaseConnection.read { db in - try PersistedProduct.posProductBySKU(siteID: siteID, sku: barcode).fetchOne(db) - } let resultByGlobalID = try await grdbManager.databaseConnection.read { db in try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: barcode).fetchOne(db) } // Then - #expect(resultBySKU?.siteID == siteID) - #expect(resultBySKU?.id == 7) #expect(resultByGlobalID?.siteID == siteID) #expect(resultByGlobalID?.id == 7) } diff --git a/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift b/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift index 33fc59d744a..75ca81cbc6d 100644 --- a/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift +++ b/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift @@ -128,117 +128,6 @@ struct PersistedProductVariationBarcodeQueryTests { #expect(result == nil) } - // MARK: - SKU Query Tests - - @Test("posVariationBySKU finds variation with matching SKU") - func test_finds_variation_by_sku() async throws { - // Given - let sku = "VAR-SKU-XYZ" - - // Insert parent product first - let parentProduct = PersistedProduct( - id: 11, - siteID: siteID, - name: "Parent Product", - productTypeKey: "variable", - fullDescription: nil, - shortDescription: nil, - sku: nil, - globalUniqueID: nil, - price: "0.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(parentProduct) - - let variation = PersistedProductVariation( - id: 102, - siteID: siteID, - productID: 11, - sku: sku, - globalUniqueID: nil, - price: "18.00", - downloadable: false, - fullDescription: "Test variation", - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertVariation(variation) - - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: sku).fetchOne(db) - } - - // Then - #expect(result != nil) - #expect(result?.id == 102) - #expect(result?.sku == sku) - } - - @Test("posVariationBySKU returns nil when no match") - func test_returns_nil_when_no_sku_match() async throws { - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: "NONEXISTENT-SKU").fetchOne(db) - } - - // Then - #expect(result == nil) - } - - @Test("posVariationBySKU filters out downloadable variations") - func test_sku_query_filters_downloadable() async throws { - // Given - let sku = "VAR-SKU-DOWNLOADABLE" - - // Insert parent product first - let parentProduct = PersistedProduct( - id: 12, - siteID: siteID, - name: "Parent Product", - productTypeKey: "variable", - fullDescription: nil, - shortDescription: nil, - sku: nil, - globalUniqueID: nil, - price: "0.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(parentProduct) - - let downloadableVariation = PersistedProductVariation( - id: 103, - siteID: siteID, - productID: 12, - sku: sku, - globalUniqueID: nil, - price: "7.00", - downloadable: true, - fullDescription: nil, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertVariation(downloadableVariation) - - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: sku).fetchOne(db) - } - - // Then - #expect(result == nil) - } - // MARK: - Site Isolation Tests @Test("Queries only return variations from specified site") @@ -322,16 +211,11 @@ struct PersistedProductVariationBarcodeQueryTests { try await insertVariation(otherVariation) // When - let resultBySKU = try await grdbManager.databaseConnection.read { db in - try PersistedProductVariation.posVariationBySKU(siteID: siteID, sku: barcode).fetchOne(db) - } let resultByGlobalID = try await grdbManager.databaseConnection.read { db in try PersistedProductVariation.posVariationByGlobalUniqueID(siteID: siteID, globalUniqueID: barcode).fetchOne(db) } // Then - #expect(resultBySKU?.siteID == siteID) - #expect(resultBySKU?.id == 104) #expect(resultByGlobalID?.siteID == siteID) #expect(resultByGlobalID?.id == 104) } diff --git a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift index c25094e30f2..3c541ca293b 100644 --- a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift +++ b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift @@ -62,97 +62,6 @@ struct PointOfSaleLocalBarcodeScanServiceTests { #expect(posProduct.productID == 1) } - @Test("Returns simple product when found by SKU") - func test_returns_simple_product_by_sku() async throws { - // Given - let sku = "SKU-TEST-123" - let product = PersistedProduct( - id: 2, - siteID: siteID, - name: "Product with SKU", - productTypeKey: "simple", - fullDescription: nil, - shortDescription: nil, - sku: sku, - globalUniqueID: nil, - price: "20.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(product) - - // When - let item = try await sut.getItem(barcode: sku) - - // Then - guard case let .simpleProduct(posProduct) = item else { - Issue.record("Expected simple product, got \(item)") - return - } - #expect(posProduct.name == "Product with SKU") - #expect(posProduct.productID == 2) - } - - @Test("Prioritizes global unique ID over SKU when both match different products") - func test_prioritizes_global_unique_id_over_sku() async throws { - // Given - let barcode = "SHARED-CODE" - - // Product with matching global unique ID - let productWithGlobalID = PersistedProduct( - id: 3, - siteID: siteID, - name: "Product with Global ID", - productTypeKey: "simple", - fullDescription: nil, - shortDescription: nil, - sku: "DIFFERENT-SKU", - globalUniqueID: barcode, - price: "30.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - - // Product with matching SKU - let productWithSKU = PersistedProduct( - id: 4, - siteID: siteID, - name: "Product with SKU", - productTypeKey: "simple", - fullDescription: nil, - shortDescription: nil, - sku: barcode, - globalUniqueID: "DIFFERENT-GLOBAL-ID", - price: "40.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - - try await insertProduct(productWithGlobalID) - try await insertProduct(productWithSKU) - - // When - let item = try await sut.getItem(barcode: barcode) - - // Then - guard case let .simpleProduct(posProduct) = item else { - Issue.record("Expected simple product, got \(item)") - return - } - // Should find the one with matching global ID first - #expect(posProduct.name == "Product with Global ID") - #expect(posProduct.productID == 3) - } - // MARK: - Variation Tests @Test("Returns variation when found by global unique ID") @@ -209,57 +118,6 @@ struct PointOfSaleLocalBarcodeScanServiceTests { #expect(variationItem.productID == 10) } - @Test("Returns variation when found by SKU") - func test_returns_variation_by_sku() async throws { - // Given - let sku = "VAR-SKU-456" - - // Insert parent product - let parentProduct = PersistedProduct( - id: 11, - siteID: siteID, - name: "Another Variable Parent", - productTypeKey: "variable", - fullDescription: nil, - shortDescription: nil, - sku: nil, - globalUniqueID: nil, - price: "0.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(parentProduct) - - // Insert variation - let variation = PersistedProductVariation( - id: 110, - siteID: siteID, - productID: 11, - sku: sku, - globalUniqueID: nil, - price: "25.00", - downloadable: false, - fullDescription: nil, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertVariation(variation) - - // When - let item = try await sut.getItem(barcode: sku) - - // Then - guard case let .variation(variationItem) = item else { - Issue.record("Expected variation, got \(item)") - return - } - #expect(variationItem.productVariationID == 110) - } - // MARK: - Error Tests @Test("Throws notFound when barcode doesn't match any product or variation") From 8455db8c4793d1b19e8638e9bb85522235000e04 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 21 Oct 2025 07:54:48 +0100 Subject: [PATCH 08/17] DRY out variation request definitions --- .../GRDB/Model/PersistedProductVariation.swift | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift index ac91e34d578..9faafe97219 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift @@ -90,8 +90,8 @@ public extension PersistedProductVariation { /// Returns a request for non-downloadable variations of a parent product, ordered by ID static func posVariationsRequest(siteID: Int64, parentProductID: Int64) -> QueryInterfaceRequest { return PersistedProductVariation - .filter(Columns.siteID == siteID && Columns.productID == parentProductID) - .filter(Columns.downloadable == false) + .basePOSVariationsRequest(siteID: siteID) + .filter(Columns.productID == parentProductID) .order(Columns.id) } @@ -101,10 +101,17 @@ public extension PersistedProductVariation { /// - globalUniqueID: The global unique ID (barcode) to search for /// - Returns: A query request that matches variations with the given global unique ID static func posVariationByGlobalUniqueID(siteID: Int64, globalUniqueID: String) -> QueryInterfaceRequest { + return PersistedProductVariation + .basePOSVariationsRequest(siteID: siteID) + .filter(Columns.globalUniqueID == globalUniqueID) + } +} + +private extension PersistedProductVariation { + static func basePOSVariationsRequest(siteID: Int64) -> QueryInterfaceRequest { return PersistedProductVariation .filter(Columns.siteID == siteID) .filter(Columns.downloadable == false) - .filter(Columns.globalUniqueID == globalUniqueID) } } From cfa925c7dfed5dfa017a7dfaf8078b81fa2697bc Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 21 Oct 2025 10:22:56 +0100 Subject: [PATCH 09/17] Throw unsupportedProductType when scanning variable parent barcode Variable parent products cannot be added to the cart directly - only their variations can be added. Updated the local barcode scan service to throw an unsupportedProductType error when a variable parent product barcode is scanned, matching the behavior of the remote barcode scan service. This ensures an error row is displayed in the cart UI when scanning a variable parent product's barcode. --- .../PointOfSaleLocalBarcodeScanService.swift | 5 ++- ...ntOfSaleLocalBarcodeScanServiceTests.swift | 37 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift index 5c2315c7189..725427e3bdb 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift +++ b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift @@ -75,8 +75,9 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer throw PointOfSaleBarcodeScanError.downloadableProduct(scannedCode: scannedCode, productName: posProduct.name) } - // Validate product type - guard [.simple, .variable].contains(posProduct.productType) else { + // Validate product type - only simple products can be scanned directly + // Variable parent products cannot be added to cart (only their variations can) + guard posProduct.productType == .simple else { throw PointOfSaleBarcodeScanError.unsupportedProductType( scannedCode: scannedCode, productName: posProduct.name, diff --git a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift index 3c541ca293b..0060deebdf5 100644 --- a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift +++ b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift @@ -187,6 +187,43 @@ struct PointOfSaleLocalBarcodeScanServiceTests { } } + @Test("Throws unsupportedProductType when scanning variable parent product barcode") + func test_throws_unsupported_product_type_for_variable_parent() async throws { + // Given + let barcode = "VARIABLE-PARENT-123" + let variableParentProduct = PersistedProduct( + id: 22, + siteID: siteID, + name: "Variable Parent Product", + productTypeKey: "variable", // Variable parent cannot be added to cart + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: barcode, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(variableParentProduct) + + // When/Then - Should throw unsupportedProductType error + do { + _ = try await sut.getItem(barcode: barcode) + Issue.record("Expected unsupportedProductType error but none was thrown") + } catch let error as PointOfSaleBarcodeScanError { + guard case .unsupportedProductType(let scannedCode, let productName, let productType) = error else { + Issue.record("Expected unsupportedProductType error, got \(error)") + return + } + #expect(scannedCode == barcode) + #expect(productName == "Variable Parent Product") + #expect(productType == .variable) + } + } + // MARK: - Helper Methods private func insertProduct(_ product: PersistedProduct) async throws { From 8377ed06ed30695b4dda7b1374d4ff3384b826ff Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 21 Oct 2025 13:00:25 +0100 Subject: [PATCH 10/17] Fix lint --- .../Items/PointOfSaleLocalBarcodeScanService.swift | 5 +++-- WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift index 725427e3bdb..94fc5310214 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift +++ b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift @@ -1,6 +1,5 @@ import Foundation import protocol Storage.GRDBManagerProtocol -import Storage import class WooFoundation.CurrencySettings /// Service for handling barcode scanning using local GRDB catalog @@ -99,7 +98,9 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer } } - private func convertVariationToItem(_ persistedVariation: PersistedProductVariation, parentProduct: PersistedProduct, scannedCode: String) async throws(PointOfSaleBarcodeScanError) -> POSItem { + private func convertVariationToItem(_ persistedVariation: PersistedProductVariation, + 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 { diff --git a/WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift b/WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift index f399cea5dbd..d6f65edd974 100644 --- a/WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift +++ b/WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift @@ -72,7 +72,8 @@ final class POSTabCoordinator { }() /// Creates the appropriate barcode scan service based on local catalog availability - private func createBarcodeScanService(grdbManager: GRDBManagerProtocol?, catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol?) -> any PointOfSaleBarcodeScanServiceProtocol { + private func createBarcodeScanService(grdbManager: GRDBManagerProtocol?, + catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol?) -> any PointOfSaleBarcodeScanServiceProtocol { // Use local barcode scanning if both GRDB manager and catalog sync coordinator are available // This indicates the local catalog feature is properly initialized and can be used if let grdbManager, catalogSyncCoordinator != nil { From 42fa5b20d82175116329f2bbf53f2b19b4f7ebb5 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 21 Oct 2025 13:38:34 +0100 Subject: [PATCH 11/17] Show specific error for scanned downloadable items --- .../Storage/GRDB/Model/PersistedProduct.swift | 13 ++++-- .../Model/PersistedProductVariation.swift | 2 +- .../PointOfSaleLocalBarcodeScanService.swift | 40 +++++++++++-------- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift index 84fb4755eb2..94ab32b5dd0 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift +++ b/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 { 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 { return PersistedProduct - .posProductsRequest(siteID: siteID) + .basePOSProductsRequest(siteID: siteID) .filter(Columns.globalUniqueID == globalUniqueID) } } +private extension PersistedProduct { + static func basePOSProductsRequest(siteID: Int64) -> QueryInterfaceRequest { + 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 { diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift index 9faafe97219..8e76a8210ac 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift +++ b/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 { return PersistedProductVariation .filter(Columns.siteID == siteID) - .filter(Columns.downloadable == false) } } diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift index 94fc5310214..aa2e8221883 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift +++ b/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") + } } From 61539f94afceaee98fe10460e8f778acf2efda06 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 21 Oct 2025 13:45:15 +0100 Subject: [PATCH 12/17] Remove unhelpful base requests --- .../Storage/GRDB/Model/PersistedProduct.swift | 13 +++---------- .../GRDB/Model/PersistedProductVariation.swift | 11 ++--------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift index 94ab32b5dd0..fc2123d2692 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift @@ -95,7 +95,8 @@ 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 { return PersistedProduct - .basePOSProductsRequest(siteID: siteID) + .filter(Columns.siteID == siteID) + .filter([ProductType.simple.rawValue, ProductType.variable.rawValue].contains(Columns.productTypeKey)) .filter(Columns.downloadable == false) .order(Columns.name.collating(.localizedCaseInsensitiveCompare)) } @@ -106,17 +107,9 @@ public extension PersistedProduct { /// - globalUniqueID: The global unique ID (barcode) to search for /// - Returns: A query request that matches products with the given global unique ID static func posProductByGlobalUniqueID(siteID: Int64, globalUniqueID: String) -> QueryInterfaceRequest { - return PersistedProduct - .basePOSProductsRequest(siteID: siteID) - .filter(Columns.globalUniqueID == globalUniqueID) - } -} - -private extension PersistedProduct { - static func basePOSProductsRequest(siteID: Int64) -> QueryInterfaceRequest { return PersistedProduct .filter(Columns.siteID == siteID) - .filter([ProductType.simple.rawValue, ProductType.variable.rawValue].contains(Columns.productTypeKey)) + .filter(Columns.globalUniqueID == globalUniqueID) } } diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift index 8e76a8210ac..ad6ca5d8ed9 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift @@ -90,7 +90,7 @@ public extension PersistedProductVariation { /// Returns a request for non-downloadable variations of a parent product, ordered by ID static func posVariationsRequest(siteID: Int64, parentProductID: Int64) -> QueryInterfaceRequest { return PersistedProductVariation - .basePOSVariationsRequest(siteID: siteID) + .filter(Columns.siteID == siteID) .filter(Columns.productID == parentProductID) .filter(Columns.downloadable == false) .order(Columns.id) @@ -102,16 +102,9 @@ public extension PersistedProductVariation { /// - globalUniqueID: The global unique ID (barcode) to search for /// - Returns: A query request that matches variations with the given global unique ID static func posVariationByGlobalUniqueID(siteID: Int64, globalUniqueID: String) -> QueryInterfaceRequest { - return PersistedProductVariation - .basePOSVariationsRequest(siteID: siteID) - .filter(Columns.globalUniqueID == globalUniqueID) - } -} - -private extension PersistedProductVariation { - static func basePOSVariationsRequest(siteID: Int64) -> QueryInterfaceRequest { return PersistedProductVariation .filter(Columns.siteID == siteID) + .filter(Columns.globalUniqueID == globalUniqueID) } } From 8f80c01b04be126cfaf429297281100fdec51d85 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 21 Oct 2025 13:52:06 +0100 Subject: [PATCH 13/17] Update downloadable product scanning test --- .../PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift index 0060deebdf5..ff92e607566 100644 --- a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift +++ b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift @@ -154,7 +154,7 @@ struct PointOfSaleLocalBarcodeScanServiceTests { try await insertProduct(downloadableProduct) // When/Then - Should not find the downloadable product - await #expect(throws: PointOfSaleBarcodeScanError.notFound(scannedCode: barcode)) { + await #expect(throws: PointOfSaleBarcodeScanError.downloadableProduct(scannedCode: barcode, productName: "Downloadable Product")) { _ = try await sut.getItem(barcode: barcode) } } From c66708dee43a002167b8f49f06f3dc5b63d28443 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 22 Oct 2025 09:28:22 +0100 Subject: [PATCH 14/17] Test fixes --- .../PersistedProductBarcodeQueryTests.swift | 62 ------------------- ...tedProductVariationBarcodeQueryTests.swift | 48 -------------- ...ntOfSaleLocalBarcodeScanServiceTests.swift | 17 ++--- 3 files changed, 6 insertions(+), 121 deletions(-) diff --git a/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift b/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift index 932a1e784d0..08bab734b2f 100644 --- a/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift +++ b/Modules/Tests/StorageTests/GRDB/PersistedProductBarcodeQueryTests.swift @@ -63,68 +63,6 @@ struct PersistedProductBarcodeQueryTests { #expect(result == nil) } - @Test("posProductByGlobalUniqueID filters out downloadable products") - func test_global_unique_id_query_filters_downloadable() async throws { - // Given - let globalUniqueID = "UPC-DOWNLOADABLE" - let downloadableProduct = PersistedProduct( - id: 2, - siteID: siteID, - name: "Downloadable Product", - productTypeKey: "simple", - fullDescription: nil, - shortDescription: nil, - sku: nil, - globalUniqueID: globalUniqueID, - price: "5.00", - downloadable: true, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(downloadableProduct) - - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) - } - - // Then - #expect(result == nil) - } - - @Test("posProductByGlobalUniqueID filters out unsupported product types") - func test_global_unique_id_query_filters_unsupported_types() async throws { - // Given - let globalUniqueID = "UPC-GROUPED" - let groupedProduct = PersistedProduct( - id: 3, - siteID: siteID, - name: "Grouped Product", - productTypeKey: "grouped", - fullDescription: nil, - shortDescription: nil, - sku: nil, - globalUniqueID: globalUniqueID, - price: "0.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(groupedProduct) - - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProduct.posProductByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) - } - - // Then - #expect(result == nil) - } - // MARK: - Site Isolation Tests @Test("Queries only return products from specified site") diff --git a/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift b/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift index 75ca81cbc6d..38641468837 100644 --- a/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift +++ b/Modules/Tests/StorageTests/GRDB/PersistedProductVariationBarcodeQueryTests.swift @@ -80,54 +80,6 @@ struct PersistedProductVariationBarcodeQueryTests { #expect(result == nil) } - @Test("posVariationByGlobalUniqueID filters out downloadable variations") - func test_global_unique_id_query_filters_downloadable() async throws { - // Given - let globalUniqueID = "VAR-DOWNLOADABLE" - - // Insert parent product first - let parentProduct = PersistedProduct( - id: 10, - siteID: siteID, - name: "Parent Product", - productTypeKey: "variable", - fullDescription: nil, - shortDescription: nil, - sku: nil, - globalUniqueID: nil, - price: "0.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertProduct(parentProduct) - - let downloadableVariation = PersistedProductVariation( - id: 101, - siteID: siteID, - productID: 10, - sku: nil, - globalUniqueID: globalUniqueID, - price: "5.00", - downloadable: true, - fullDescription: nil, - manageStock: false, - stockQuantity: nil, - stockStatusKey: "instock" - ) - try await insertVariation(downloadableVariation) - - // When - let result = try await grdbManager.databaseConnection.read { db in - try PersistedProductVariation.posVariationByGlobalUniqueID(siteID: siteID, globalUniqueID: globalUniqueID).fetchOne(db) - } - - // Then - #expect(result == nil) - } - // MARK: - Site Isolation Tests @Test("Queries only return variations from specified site") diff --git a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift index ff92e607566..0036eae9645 100644 --- a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift +++ b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift @@ -182,7 +182,9 @@ struct PointOfSaleLocalBarcodeScanServiceTests { try await insertProduct(groupedProduct) // When/Then - Should not find unsupported product type - await #expect(throws: PointOfSaleBarcodeScanError.notFound(scannedCode: barcode)) { + await #expect(throws: PointOfSaleBarcodeScanError.unsupportedProductType(scannedCode: barcode, + productName: "Grouped Product", + productType: .grouped)) { _ = try await sut.getItem(barcode: barcode) } } @@ -210,17 +212,10 @@ struct PointOfSaleLocalBarcodeScanServiceTests { try await insertProduct(variableParentProduct) // When/Then - Should throw unsupportedProductType error - do { + await #expect(throws: PointOfSaleBarcodeScanError.unsupportedProductType(scannedCode: barcode, + productName: "Variable Parent Product", + productType: .variable)) { _ = try await sut.getItem(barcode: barcode) - Issue.record("Expected unsupportedProductType error but none was thrown") - } catch let error as PointOfSaleBarcodeScanError { - guard case .unsupportedProductType(let scannedCode, let productName, let productType) = error else { - Issue.record("Expected unsupportedProductType error, got \(error)") - return - } - #expect(scannedCode == barcode) - #expect(productName == "Variable Parent Product") - #expect(productType == .variable) } } From 8136676649c5d9cf01d1d10ef4e44f367e4afc37 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 22 Oct 2025 09:38:15 +0100 Subject: [PATCH 15/17] Remove unnecessary `await` calls Co-authored-by: Povilas Staskus --- .../Items/PointOfSaleLocalBarcodeScanService.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift index aa2e8221883..1b2033a4f4f 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift +++ b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift @@ -27,7 +27,7 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer } if let variationAndParent = try searchVariationByGlobalUniqueID(barcode) { - return try await convertVariationToItem(variationAndParent.variation, parentProduct: variationAndParent.parentProduct, scannedCode: barcode) + return try convertVariationToItem(variationAndParent.variation, parentProduct: variationAndParent.parentProduct, scannedCode: barcode) } throw PointOfSaleBarcodeScanError.notFound(scannedCode: barcode) @@ -97,7 +97,7 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer private func convertVariationToItem(_ persistedVariation: PersistedProductVariation, parentProduct: PersistedProduct, - scannedCode: String) async throws(PointOfSaleBarcodeScanError) -> POSItem { + scannedCode: String) throws(PointOfSaleBarcodeScanError) -> POSItem { do { // Convert both variation and parent to POS models let posVariation = try persistedVariation.toPOSProductVariation(db: grdbManager.databaseConnection) From f01164d8c6bec34abf8fb1e313dde4018bef1da2 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 22 Oct 2025 09:43:46 +0100 Subject: [PATCH 16/17] Match the error for missing parent product --- .../PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift index 1b2033a4f4f..e3f3a0bd647 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift +++ b/Modules/Sources/Yosemite/PointOfSale/Items/PointOfSaleLocalBarcodeScanService.swift @@ -55,7 +55,7 @@ public final class PointOfSaleLocalBarcodeScanService: PointOfSaleBarcodeScanSer } // Fetch parent product using the relationship guard let parentProduct = try variation.request(for: PersistedProductVariation.parentProduct).fetchOne(db) else { - return nil + throw PointOfSaleBarcodeScanError.noParentProductForVariation(scannedCode: globalUniqueID) } return (variation, parentProduct) } From b5f0a1f6425538c91474087e7fa67db679abbcac Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 22 Oct 2025 10:09:07 +0100 Subject: [PATCH 17/17] Documenting CASCADE delete for orphaned variations Tests that variations are automatically deleted when their parent product is deleted due to foreign key constraints with CASCADE delete. This documents that the noParentProductForVariation error thrown in searchVariationByGlobalUniqueID is defensive code that won't be triggered in normal operation, since the database prevents orphaned variations from existing. --- ...ntOfSaleLocalBarcodeScanServiceTests.swift | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift index 0036eae9645..5e6db2e6f8f 100644 --- a/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift +++ b/Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleLocalBarcodeScanServiceTests.swift @@ -219,6 +219,67 @@ struct PointOfSaleLocalBarcodeScanServiceTests { } } + @Test("Foreign key constraint prevents orphaned variations") + func test_variations_cannot_be_orphaned() async throws { + // Given + let barcode = "ORPHAN-VAR-123" + + // Insert a parent product + let parentProduct = PersistedProduct( + id: 888, + siteID: siteID, + name: "Parent Product", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertProduct(parentProduct) + + // Insert the variation + let variation = PersistedProductVariation( + id: 999, + siteID: siteID, + productID: 888, + sku: nil, + globalUniqueID: barcode, + price: "20.00", + downloadable: false, + fullDescription: nil, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try await insertVariation(variation) + + // Verify the variation exists + let variationExists = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.fetchOne(db, key: ["siteID": siteID, "id": 999]) != nil + } + #expect(variationExists, "Variation should exist after insertion") + + // When - Delete the parent product (with foreign keys enabled) + try await grdbManager.databaseConnection.write { db in + try db.execute(sql: "DELETE FROM product WHERE id = ? AND siteID = ?", arguments: [888, siteID]) + } + + // Then - Variation should be automatically deleted by CASCADE + let variationStillExists = try await grdbManager.databaseConnection.read { db in + try PersistedProductVariation.fetchOne(db, key: ["siteID": siteID, "id": 999]) != nil + } + #expect(!variationStillExists, "Variation should be automatically deleted by CASCADE when parent is deleted") + + // This means the noParentProductForVariation error is defensive code that won't be triggered + // in normal operation due to foreign key constraints with CASCADE delete + } + // MARK: - Helper Methods private func insertProduct(_ product: PersistedProduct) async throws {