diff --git a/Modules/Sources/Yosemite/PointOfSale/Items/GRDBObservableDataSource.swift b/Modules/Sources/Yosemite/PointOfSale/Items/GRDBObservableDataSource.swift index 2ee50e12ff8..21400cbd0d4 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Items/GRDBObservableDataSource.swift +++ b/Modules/Sources/Yosemite/PointOfSale/Items/GRDBObservableDataSource.swift @@ -160,9 +160,47 @@ public final class GRDBObservableDataSource: POSObservableDataSourceProtocol { private func setupVariationObservation(parentProduct: POSVariableParentProduct) { let currentPage = currentVariationPage + let parentProductID = parentProduct.productID + + struct ObservationResult { + let variations: [POSProductVariation] + let parentProduct: POSVariableParentProduct + } + let observation = ValueObservation - .tracking { [weak self] database -> [POSProductVariation] in - guard let self else { return [] } + .tracking { [weak self] database -> ObservationResult in + guard let self else { return ObservationResult(variations: [], parentProduct: parentProduct) } + + // Fetch parent product with updated attributes + struct ParentProductWithAttributes: Decodable, FetchableRecord { + let product: PersistedProduct + let attributes: [PersistedProductAttribute] + } + + let parentWithAttributes = try PersistedProduct + .filter(PersistedProduct.Columns.siteID == self.siteID) + .filter(PersistedProduct.Columns.id == parentProductID) + .including(all: PersistedProduct.attributes) + .asRequest(of: ParentProductWithAttributes.self) + .fetchOne(database) + + // Create updated parent product with fresh attributes + let updatedParentProduct: POSVariableParentProduct + if let parentWithAttributes { + let attributes = (parentWithAttributes.attributes).map { + $0.toProductAttribute(siteID: parentWithAttributes.product.siteID) + } + let product = parentWithAttributes.product + updatedParentProduct = POSVariableParentProduct( + id: parentProduct.id, + name: product.name, + productImageSource: nil, // Image not needed for variation name generation + productID: product.id, + allAttributes: attributes + ) + } else { + updatedParentProduct = parentProduct + } struct VariationWithRelations: Decodable, FetchableRecord { let persistedProductVariation: PersistedProductVariation @@ -171,36 +209,42 @@ public final class GRDBObservableDataSource: POSObservableDataSourceProtocol { } let variationsWithRelations = try PersistedProductVariation - .posVariationsRequest(siteID: self.siteID, parentProductID: parentProduct.productID) + .posVariationsRequest(siteID: self.siteID, parentProductID: parentProductID) .limit(self.pageSize * currentPage) .including(all: PersistedProductVariation.attributes) .including(optional: PersistedProductVariation.image) .asRequest(of: VariationWithRelations.self) .fetchAll(database) - return variationsWithRelations.map { record in + let variations = variationsWithRelations.map { record in record.persistedProductVariation.toPOSProductVariation( attributes: (record.attributes ?? []).map { $0.toProductVariationAttribute() }, image: record.image?.toProductImage() ) } + + return ObservationResult(variations: variations, parentProduct: updatedParentProduct) } variationObservationCancellable = observation .publisher(in: grdbManager.databaseConnection) .receive(on: DispatchQueue.main) .sink( - receiveCompletion: { [weak self] completion in + receiveCompletion: { [weak self] (completion: Subscribers.Completion) in if case .failure(let error) = completion { self?.variationError = error self?.isLoadingVariations = false } }, - receiveValue: { [weak self] observedVariations in + receiveValue: { [weak self] (result: ObservationResult) in guard let self else { return } + + // Update current parent product with fresh attributes + self.currentParentProduct = result.parentProduct + let posItems = itemMapper.mapVariationsToPOSItems( - variations: observedVariations, - parentProduct: parentProduct + variations: result.variations, + parentProduct: result.parentProduct ) variationItems = posItems variationError = nil diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift index e7626b53e6b..3efa8a64fc4 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift @@ -116,6 +116,11 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { } for variation in catalog.variationsToPersist { + // Delete attributes for updated variations, the remaining set will be recreated later in the save + try PersistedProductVariationAttribute + .filter(PersistedProductVariationAttribute.Columns.productVariationID == variation.id) + .deleteAll(db) + try variation.save(db) } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift index cc9a27d32e9..6fe1dc29ccf 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift @@ -472,6 +472,36 @@ struct POSCatalogPersistenceServiceTests { } } + @Test func persistIncrementalCatalogData_prevents_duplicate_variation_attributes_on_multiple_syncs() async throws { + // Given - variation with attributes + let parentProduct = POSProduct.fake().copy(siteID: sampleSiteID, productID: 10) + let attribute1 = Yosemite.ProductVariationAttribute.fake().copy(name: "Color", option: "Red") + let attribute2 = Yosemite.ProductVariationAttribute.fake().copy(name: "Size", option: "L") + let variation = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 10, productVariationID: 1, attributes: [attribute1, attribute2]) + try await insertProduct(parentProduct) + try await insertVariation(variation) + + // When - perform multiple incremental syncs with the same variation/attributes + let catalog = POSCatalog(products: [parentProduct], variations: [variation], syncDate: .now) + try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID) + try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID) + try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID) + + // Then - should have exactly 2 attributes, not duplicates + try await grdbManager.databaseConnection.read { db in + let attributeCount = try PersistedProductVariationAttribute.fetchCount(db) + #expect(attributeCount == 2) + + let attributes = try PersistedProductVariationAttribute.fetchAll(db).sorted(by: { $0.name < $1.name }) + #expect(attributes[0].name == "Color") + #expect(attributes[0].option == "Red") + #expect(attributes[0].productVariationID == 1) + #expect(attributes[1].name == "Size") + #expect(attributes[1].option == "L") + #expect(attributes[1].productVariationID == 1) + } + } + @Test func persistIncrementalCatalogData_replaces_image_for_updated_variation() async throws { // Given let parentProduct = POSProduct.fake().copy(siteID: sampleSiteID, productID: 10)