diff --git a/Modules/Sources/Fakes/Networking.generated.swift b/Modules/Sources/Fakes/Networking.generated.swift index 59d765ba9ad..45f3a9b179b 100644 --- a/Modules/Sources/Fakes/Networking.generated.swift +++ b/Modules/Sources/Fakes/Networking.generated.swift @@ -812,7 +812,8 @@ extension Networking.POSProduct { attributes: .fake(), manageStock: .fake(), stockQuantity: .fake(), - stockStatusKey: .fake() + stockStatusKey: .fake(), + variationIDs: .fake() ) } } diff --git a/Modules/Sources/Fakes/Yosemite.generated.swift b/Modules/Sources/Fakes/Yosemite.generated.swift index b58654291cd..4b4a8b785b3 100644 --- a/Modules/Sources/Fakes/Yosemite.generated.swift +++ b/Modules/Sources/Fakes/Yosemite.generated.swift @@ -76,6 +76,15 @@ extension Yosemite.ProductReviewFromNoteParcel { ) } } +extension Yosemite.StoredBookingFilters { + /// Returns a "ready to use" type filled with fake values. + /// + public static func fake() -> Yosemite.StoredBookingFilters { + .init( + filters: .fake() + ) + } +} extension Yosemite.SystemInformation { /// Returns a "ready to use" type filled with fake values. /// diff --git a/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift b/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift index 68c69a8fbf5..c20e19dc571 100644 --- a/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift +++ b/Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift @@ -1367,7 +1367,8 @@ extension Networking.POSProduct { attributes: CopiableProp<[ProductAttribute]> = .copy, manageStock: CopiableProp = .copy, stockQuantity: NullableCopiableProp = .copy, - stockStatusKey: CopiableProp = .copy + stockStatusKey: CopiableProp = .copy, + variationIDs: CopiableProp<[Int64]> = .copy ) -> Networking.POSProduct { let siteID = siteID ?? self.siteID let productID = productID ?? self.productID @@ -1385,6 +1386,7 @@ extension Networking.POSProduct { let manageStock = manageStock ?? self.manageStock let stockQuantity = stockQuantity ?? self.stockQuantity let stockStatusKey = stockStatusKey ?? self.stockStatusKey + let variationIDs = variationIDs ?? self.variationIDs return Networking.POSProduct( siteID: siteID, @@ -1402,7 +1404,8 @@ extension Networking.POSProduct { attributes: attributes, manageStock: manageStock, stockQuantity: stockQuantity, - stockStatusKey: stockStatusKey + stockStatusKey: stockStatusKey, + variationIDs: variationIDs ) } } diff --git a/Modules/Sources/Networking/Model/POSProduct.swift b/Modules/Sources/Networking/Model/POSProduct.swift index b9ad128874c..dc0702cf2c9 100644 --- a/Modules/Sources/Networking/Model/POSProduct.swift +++ b/Modules/Sources/Networking/Model/POSProduct.swift @@ -43,6 +43,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab public let stockQuantity: Decimal? public let stockStatusKey: String + public let variationIDs: [Int64] + public init(siteID: Int64, productID: Int64, name: String, @@ -58,7 +60,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab attributes: [ProductAttribute], manageStock: Bool, stockQuantity: Decimal?, - stockStatusKey: String) { + stockStatusKey: String, + variationIDs: [Int64]) { self.siteID = siteID self.productID = productID self.name = name @@ -81,6 +84,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab self.manageStock = manageStock self.stockQuantity = stockQuantity self.stockStatusKey = stockStatusKey + + self.variationIDs = variationIDs } public init(from decoder: any Decoder) throws { @@ -124,6 +129,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab let stockQuantity = container.failsafeDecodeIfPresent(decimalForKey: .stockQuantity) let stockStatusKey = try container.decode(String.self, forKey: .stockStatusKey) + let variationIDs = try container.decodeIfPresent([Int64].self, forKey: .variationIDs) ?? [] + self.init(siteID: siteID, productID: productID, name: name, @@ -139,7 +146,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab attributes: attributes, manageStock: manageStock, stockQuantity: stockQuantity, - stockStatusKey: stockStatusKey) + stockStatusKey: stockStatusKey, + variationIDs: variationIDs) } static let requestFields: [String] = { @@ -172,5 +180,6 @@ private extension POSProduct { case manageStock = "manage_stock" case stockQuantity = "stock_quantity" case stockStatusKey = "stock_status" + case variationIDs = "variations" } } diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift index fc2123d2692..f126e09e2ba 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProduct.swift @@ -88,6 +88,12 @@ extension PersistedProduct: FetchableRecord, PersistableRecord { using: ForeignKey([PersistedProductAttribute.CodingKeys.siteID.stringValue, PersistedProductAttribute.CodingKeys.productID.stringValue], to: primaryKey)) + + public static let variations = hasMany(PersistedProductVariation.self, + key: "variations", + using: ForeignKey([PersistedProductVariation.CodingKeys.siteID.stringValue, + PersistedProductVariation.CodingKeys.productID.stringValue], + to: primaryKey)) } // MARK: - Point of Sale Requests diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift index ad6ca5d8ed9..af42b54d911 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariation.swift @@ -108,8 +108,7 @@ public extension PersistedProductVariation { } } -// periphery:ignore - TODO: remove ignore when populating database -private extension PersistedProductVariation { +extension PersistedProductVariation { enum CodingKeys: String, CodingKey { case id case siteID diff --git a/Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift b/Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift index 3f5fe127ad0..16807f0a893 100644 --- a/Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift +++ b/Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift @@ -23,7 +23,7 @@ extension PersistedProduct { ) } - func toPOSProduct(images: [ProductImage] = [], attributes: [ProductAttribute] = []) -> POSProduct { + func toPOSProduct(images: [ProductImage] = [], attributes: [ProductAttribute] = [], variationIDs: [Int64] = []) -> POSProduct { return POSProduct( siteID: siteID, productID: id, @@ -40,20 +40,24 @@ extension PersistedProduct { attributes: attributes, manageStock: manageStock, stockQuantity: stockQuantity, - stockStatusKey: stockStatusKey + stockStatusKey: stockStatusKey, + variationIDs: variationIDs ) } func toPOSProduct(db: GRDBDatabaseConnection) throws -> POSProduct { - let (images, attributes) = try db.read { db in + let (images, attributes, variationIDs) = try db.read { db in let images = try request(for: PersistedProduct.images).fetchAll(db) let attributes = try request(for: PersistedProduct.attributes).fetchAll(db) - return (images, attributes) + let variations = try request(for: PersistedProduct.variations).fetchAll(db) + let variationIDs = variations.map(\.id) + return (images, attributes, variationIDs) } return toPOSProduct( images: images.map { $0.toProductImage() }, - attributes: attributes.map { $0.toProductAttribute(siteID: siteID) } + attributes: attributes.map { $0.toProductAttribute(siteID: siteID) }, + variationIDs: variationIDs ) } diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift index e449b93a073..263b52472c7 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift @@ -87,52 +87,46 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { DDLogInfo("💾 Persisting incremental catalog with \(catalog.products.count) updated products and \(catalog.variations.count) updated variations") try await grdbManager.databaseConnection.write { db in - for product in catalog.productsToPersist { - try product.insert(db, onConflict: .replace) - - // Delete old join table entries for this product - try PersistedProductImage - .filter { $0.siteID == siteID && $0.productID == product.id } - .deleteAll(db) - - try PersistedProductAttribute - .filter { $0.siteID == siteID && $0.productID == product.id } - .deleteAll(db) + for product in catalog.products { + try PersistedProduct(from: product).save(db) + + // Delete variations that are no longer associated with this product + let existingVariations = try PersistedProductVariation + .filter(PersistedProductVariation.Columns.siteID == siteID) + .filter(PersistedProductVariation.Columns.productID == product.productID) + .fetchAll(db) + + for variation in existingVariations { + if !product.variationIDs.contains(variation.id) { + try variation.delete(db) + } + } } for variation in catalog.variationsToPersist { - try variation.insert(db, onConflict: .replace) - - // Delete old join table entries for this variation - try PersistedProductVariationImage - .filter { $0.siteID == siteID && $0.productVariationID == variation.id } - .deleteAll(db) - - try PersistedProductVariationAttribute - .filter { $0.siteID == siteID && $0.productVariationID == variation.id } - .deleteAll(db) + try variation.save(db) } - // Insert/update actual image data (shared by products and variations) + // Upsert actual image data (shared by products and variations) for image in catalog.imagesToPersist { - try image.insert(db, onConflict: .replace) + try image.save(db) } - // Insert new join table entries + // Upsert new join table entries for image in catalog.productImagesToPersist { - try image.insert(db, onConflict: .replace) + try image.save(db) } for image in catalog.variationImagesToPersist { - try image.insert(db, onConflict: .replace) + try image.save(db) } for var attribute in catalog.productAttributesToPersist { - try attribute.insert(db, onConflict: .replace) + try attribute.save(db) } for var attribute in catalog.variationAttributesToPersist { - try attribute.insert(db, onConflict: .replace) + try attribute.save(db) } var site = try PersistedSite.fetchOne(db, key: siteID) diff --git a/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift b/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift index 464376cb72a..aeef327b164 100644 --- a/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift +++ b/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift @@ -27,7 +27,8 @@ struct PersistedProductTests { attributes: [], manageStock: true, stockQuantity: 5, - stockStatusKey: "instock" + stockStatusKey: "instock", + variationIDs: [] ) // When @@ -508,7 +509,8 @@ struct PersistedProductTests { ], manageStock: true, stockQuantity: 50, - stockStatusKey: "instock" + stockStatusKey: "instock", + variationIDs: [] ) // When saving and loading back diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift index 58b22c21a45..8c1f64df95c 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift @@ -275,7 +275,7 @@ struct POSCatalogPersistenceServiceTests { } } - @Test func persistIncrementalCatalogData_replaces_attributes_for_updated_product() async throws { + @Test func persistIncrementalCatalogData_upserts_attributes_for_updated_product() async throws { // Given let attribute1 = Yosemite.ProductAttribute.fake().copy(name: "Color", options: ["Indigo", "Blue"]) let attribute2 = Yosemite.ProductAttribute.fake().copy(name: "Size") @@ -291,16 +291,17 @@ struct POSCatalogPersistenceServiceTests { // Then try await grdbManager.databaseConnection.read { db in + // Should have 4 attributes: original 2 + updated 2 (upsert adds new ones, keeps old ones) let attributeCount = try PersistedProductAttribute.fetchCount(db) - #expect(attributeCount == 2) + #expect(attributeCount == 4) let attributes = try PersistedProductAttribute.fetchAll(db).sorted(by: { $0.name < $1.name }) #expect(attributes[0].name == "Color") - #expect(attributes[0].options == ["Cardinal", "Blue"]) - #expect(attributes[0].productID == 1) - #expect(attributes[1].name == "Material") - #expect(attributes[1].options == []) - #expect(attributes[1].productID == 1) + #expect(attributes[0].options == ["Indigo", "Blue"]) // Original unchanged + #expect(attributes[1].name == "Color") + #expect(attributes[1].options == ["Cardinal", "Blue"]) // Updated version + #expect(attributes[2].name == "Material") // New attribute + #expect(attributes[3].name == "Size") // Original unchanged } } @@ -320,24 +321,26 @@ struct POSCatalogPersistenceServiceTests { // Then try await grdbManager.databaseConnection.read { db in - // Check join table has correct count + // Check join table has correct count - should have 3 (original 2 + new 1, upsert keeps all) let joinCount = try PersistedProductImage.fetchCount(db) - #expect(joinCount == 2) + #expect(joinCount == 3) // Check join table entries let joins = try PersistedProductImage.fetchAll(db).sorted(by: { $0.imageID < $1.imageID }) - #expect(joins.count == 2) + #expect(joins.count == 3) #expect(joins[0].productID == 1) #expect(joins[0].imageID == 1) #expect(joins[1].productID == 1) - #expect(joins[1].imageID == 3) + #expect(joins[1].imageID == 2) // Original image 2 join remains + #expect(joins[2].productID == 1) + #expect(joins[2].imageID == 3) - // Check actual images + // Check actual images - image 1 updated, image 2 unchanged, image 3 added let images = try PersistedImage.fetchAll(db).sorted(by: { $0.id < $1.id }) - #expect(images.count == 3) // `image2` remains, but is unlinked. - #expect(images[0].src == "https://example.com/image1-1.jpg") - #expect(images[1].src == "https://example.com/image2.jpg") - #expect(images[2].src == "https://example.com/image3.jpg") + #expect(images.count == 3) + #expect(images[0].src == "https://example.com/image1-1.jpg") // Updated + #expect(images[1].src == "https://example.com/image2.jpg") // Unchanged + #expect(images[2].src == "https://example.com/image3.jpg") // New } } @@ -522,6 +525,100 @@ struct POSCatalogPersistenceServiceTests { } } + @Test func persistIncrementalCatalogData_deletes_variations_not_in_updated_product_variationIDs() async throws { + // Given - a variable product with 3 variations + let product = POSProduct.fake().copy( + siteID: sampleSiteID, + productID: 1, + productTypeKey: "variable", + variationIDs: [10, 20, 30] + ) + let variation1 = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 1, productVariationID: 10) + let variation2 = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 1, productVariationID: 20) + let variation3 = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 1, productVariationID: 30) + + let catalog = POSCatalog(products: [product], variations: [variation1, variation2, variation3], syncDate: .now) + try await sut.replaceAllCatalogData(catalog, siteID: sampleSiteID) + + // Verify initial state + try await db.read { db in + let variationCount = try PersistedProductVariation.fetchCount(db) + #expect(variationCount == 3) + } + + // When - incremental sync with updated product that only has 2 variations (removed variation 20) + let updatedProduct = POSProduct.fake().copy( + siteID: sampleSiteID, + productID: 1, + productTypeKey: "variable", + variationIDs: [10, 30] // variation 20 removed + ) + let incrementalCatalog = POSCatalog(products: [updatedProduct], variations: [], syncDate: .now) + try await sut.persistIncrementalCatalogData(incrementalCatalog, siteID: sampleSiteID) + + // Then - variation 20 should be deleted, variations 10 and 30 should remain + try await db.read { db in + let variationCount = try PersistedProductVariation.fetchCount(db) + #expect(variationCount == 2) + + let remainingVariations = try PersistedProductVariation.fetchAll(db).sorted(by: { $0.id < $1.id }) + #expect(remainingVariations.count == 2) + #expect(remainingVariations[0].id == 10) + #expect(remainingVariations[1].id == 30) + } + } + + @Test func persistIncrementalCatalogData_preserves_variations_not_mentioned_in_incremental_sync() async throws { + // Given - two variable products with variations + let product1 = POSProduct.fake().copy( + siteID: sampleSiteID, + productID: 1, + productTypeKey: "variable", + variationIDs: [10, 20] + ) + let product2 = POSProduct.fake().copy( + siteID: sampleSiteID, + productID: 2, + productTypeKey: "variable", + variationIDs: [30, 40] + ) + let variation10 = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 1, productVariationID: 10) + let variation20 = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 1, productVariationID: 20) + let variation30 = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 2, productVariationID: 30) + let variation40 = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 2, productVariationID: 40) + + let catalog = POSCatalog( + products: [product1, product2], + variations: [variation10, variation20, variation30, variation40], + syncDate: .now + ) + try await sut.replaceAllCatalogData(catalog, siteID: sampleSiteID) + + // When - incremental sync only updates product 1, product 2 not mentioned + let updatedProduct1 = POSProduct.fake().copy( + siteID: sampleSiteID, + productID: 1, + productTypeKey: "variable", + variationIDs: [10] // removed variation 20 + ) + let incrementalCatalog = POSCatalog(products: [updatedProduct1], variations: [], syncDate: .now) + try await sut.persistIncrementalCatalogData(incrementalCatalog, siteID: sampleSiteID) + + // Then - variation 20 deleted, but product 2's variations (30, 40) remain untouched + try await db.read { db in + let variationCount = try PersistedProductVariation.fetchCount(db) + #expect(variationCount == 3) + + let remainingVariations = try PersistedProductVariation.fetchAll(db).sorted(by: { $0.id < $1.id }) + #expect(remainingVariations[0].id == 10) + #expect(remainingVariations[0].productID == 1) + #expect(remainingVariations[1].id == 30) + #expect(remainingVariations[1].productID == 2) + #expect(remainingVariations[2].id == 40) + #expect(remainingVariations[2].productID == 2) + } + } + @Test func persistIncrementalCatalogData_stores_incremental_sync_date() async throws { // Given - site with existing full sync date let fullSyncDate = Date().addingTimeInterval(-3600) // 1 hour ago