From 299dde336d7c02d627dfd417a7fa831ee114b898 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Fri, 12 Sep 2025 13:10:06 +0100 Subject: [PATCH 1/3] Add remote ID for attributes --- .../GRDB/Migrations/V001InitialSchema.swift | 8 ++++++-- .../Model/PersistedProductAttribute.swift | 5 +++++ .../PersistedProductVariationAttribute.swift | 5 +++++ .../PersistedProduct+Conversions.swift | 3 ++- ...ersistedProductVariation+Conversions.swift | 3 ++- .../Storage/PersistedProductTests.swift | 20 +++++++++++++++++-- .../PersistedProductVariationTests.swift | 15 ++++++++++++-- 7 files changed, 51 insertions(+), 8 deletions(-) diff --git a/Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift b/Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift index dc02ccff017..cca60a2d50a 100644 --- a/Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift +++ b/Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift @@ -55,7 +55,8 @@ struct V001InitialSchema { private static func createProductAttributeTable(_ db: Database) throws { try db.create(table: "productAttribute") { productAttributeTable in - // This table holds local product attributes only. Global attributes belong to a site. + // This table holds both local and global product attributes. + // Local attributes have remoteAttributeID = 0, global attributes have remoteAttributeID > 0 productAttributeTable.autoIncrementedPrimaryKey("id").notNull() productAttributeTable.column("siteID", .integer).notNull() productAttributeTable.column("productID", .integer).notNull() @@ -64,6 +65,7 @@ struct V001InitialSchema { columns: ["siteID", "id"], onDelete: .cascade) + productAttributeTable.column("remoteAttributeID", .integer).notNull() productAttributeTable.column("name", .text).notNull() productAttributeTable.column("position", .integer).notNull() productAttributeTable.column("visible", .boolean).notNull() @@ -119,7 +121,8 @@ struct V001InitialSchema { private static func createProductVariationAttributeTable(_ db: Database) throws { try db.create(table: "productVariationAttribute") { productVariationAttributeTable in - // This table holds local variation attributes only. Global attributes belong to a site. + // This table holds both local and global variation attributes. + // Local attributes have remoteAttributeID = 0, global attributes have remoteAttributeID > 0 productVariationAttributeTable.autoIncrementedPrimaryKey("id").notNull() productVariationAttributeTable.column("siteID", .integer).notNull() productVariationAttributeTable.column("productVariationID", .integer).notNull() @@ -128,6 +131,7 @@ struct V001InitialSchema { columns: ["siteID", "id"], onDelete: .cascade) + productVariationAttributeTable.column("remoteAttributeID", .integer).notNull() productVariationAttributeTable.column("name", .text).notNull() productVariationAttributeTable.column("option", .text).notNull() } diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductAttribute.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductAttribute.swift index 3b52f7f97d1..6b013f4ed7a 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductAttribute.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProductAttribute.swift @@ -6,6 +6,7 @@ public struct PersistedProductAttribute: Codable { public private(set) var id: Int64? public let siteID: Int64 public let productID: Int64 + public let remoteAttributeID: Int64 public let name: String public let position: Int64 public let visible: Bool @@ -15,6 +16,7 @@ public struct PersistedProductAttribute: Codable { public init(id: Int64? = nil, siteID: Int64, productID: Int64, + remoteAttributeID: Int64, name: String, position: Int64, visible: Bool, @@ -23,6 +25,7 @@ public struct PersistedProductAttribute: Codable { self.id = id self.siteID = siteID self.productID = productID + self.remoteAttributeID = remoteAttributeID self.name = name self.position = position self.visible = visible @@ -41,6 +44,7 @@ extension PersistedProductAttribute: FetchableRecord, MutablePersistableRecord { public static let id = Column(CodingKeys.id) public static let siteID = Column(CodingKeys.siteID) public static let productID = Column(CodingKeys.productID) + public static let remoteAttributeID = Column(CodingKeys.remoteAttributeID) public static let name = Column(CodingKeys.name) public static let position = Column(CodingKeys.position) public static let visible = Column(CodingKeys.visible) @@ -62,6 +66,7 @@ extension PersistedProductAttribute { case id case siteID case productID + case remoteAttributeID case name case position case visible diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariationAttribute.swift b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariationAttribute.swift index 09d50fd2b7e..340de185aff 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedProductVariationAttribute.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedProductVariationAttribute.swift @@ -6,17 +6,20 @@ public struct PersistedProductVariationAttribute: Codable { public private(set) var id: Int64? public let siteID: Int64 public let productVariationID: Int64 + public let remoteAttributeID: Int64 public let name: String public let option: String public init(id: Int64? = nil, siteID: Int64, productVariationID: Int64, + remoteAttributeID: Int64, name: String, option: String) { self.id = id self.siteID = siteID self.productVariationID = productVariationID + self.remoteAttributeID = remoteAttributeID self.name = name self.option = option } @@ -33,6 +36,7 @@ extension PersistedProductVariationAttribute: FetchableRecord, MutablePersistabl public static let id = Column(CodingKeys.id) public static let siteID = Column(CodingKeys.siteID) public static let productVariationID = Column(CodingKeys.productVariationID) + public static let remoteAttributeID = Column(CodingKeys.remoteAttributeID) public static let name = Column(CodingKeys.name) public static let option = Column(CodingKeys.option) } @@ -51,6 +55,7 @@ extension PersistedProductVariationAttribute { case id case siteID case productVariationID + case remoteAttributeID case name case option } diff --git a/Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift b/Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift index d8bc677094e..1d1a3442e11 100644 --- a/Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift +++ b/Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift @@ -89,6 +89,7 @@ extension PersistedProductAttribute { self.init( siteID: siteID, productID: productID, + remoteAttributeID: productAttribute.attributeID, name: productAttribute.name, position: Int64(productAttribute.position), visible: productAttribute.visible, @@ -100,7 +101,7 @@ extension PersistedProductAttribute { func toProductAttribute(siteID: Int64) -> ProductAttribute { return ProductAttribute( siteID: siteID, - attributeID: id ?? 0, + attributeID: remoteAttributeID, name: name, position: Int(position), visible: visible, diff --git a/Modules/Sources/Yosemite/Model/Storage/PersistedProductVariation+Conversions.swift b/Modules/Sources/Yosemite/Model/Storage/PersistedProductVariation+Conversions.swift index 739ab87eb6c..87beb417fe2 100644 --- a/Modules/Sources/Yosemite/Model/Storage/PersistedProductVariation+Conversions.swift +++ b/Modules/Sources/Yosemite/Model/Storage/PersistedProductVariation+Conversions.swift @@ -83,6 +83,7 @@ extension PersistedProductVariationAttribute { self.init( siteID: siteID, productVariationID: productVariationID, + remoteAttributeID: productVariationAttribute.id, name: productVariationAttribute.name, option: productVariationAttribute.option ) @@ -90,7 +91,7 @@ extension PersistedProductVariationAttribute { func toProductVariationAttribute() -> ProductVariationAttribute { return ProductVariationAttribute( - id: id ?? 0, + id: remoteAttributeID, name: name, option: option ) diff --git a/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift b/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift index 7e822fad176..6d27185a0b4 100644 --- a/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift +++ b/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift @@ -92,8 +92,22 @@ struct PersistedProductTests { ] let persistedAttributes = [ - PersistedProductAttribute(siteID: siteID, productID: productID, name: "Material", position: 0, visible: true, variation: false, options: ["Cotton"]), - PersistedProductAttribute(siteID: siteID, productID: productID, name: "Fit", position: 1, visible: true, variation: true, options: ["Slim", "Regular"]) + PersistedProductAttribute(siteID: siteID, + productID: productID, + remoteAttributeID: 501, + name: "Material", + position: 0, + visible: true, + variation: false, + options: ["Cotton"]), + PersistedProductAttribute(siteID: siteID, + productID: productID, + remoteAttributeID: 502, + name: "Fit", + position: 1, + visible: true, + variation: true, + options: ["Slim", "Regular"]) ] // When @@ -178,6 +192,7 @@ struct PersistedProductTests { var attribute1 = PersistedProductAttribute( siteID: 1, productID: 100, + remoteAttributeID: 501, name: "Color", position: 0, visible: true, @@ -187,6 +202,7 @@ struct PersistedProductTests { var attribute2 = PersistedProductAttribute( siteID: 1, productID: 100, + remoteAttributeID: 502, name: "Size", position: 1, visible: false, diff --git a/Modules/Tests/YosemiteTests/Storage/PersistedProductVariationTests.swift b/Modules/Tests/YosemiteTests/Storage/PersistedProductVariationTests.swift index 188370af5dc..ce369679d61 100644 --- a/Modules/Tests/YosemiteTests/Storage/PersistedProductVariationTests.swift +++ b/Modules/Tests/YosemiteTests/Storage/PersistedProductVariationTests.swift @@ -76,8 +76,16 @@ struct PersistedProductVariationTests { ) let varAttrs = [ - PersistedProductVariationAttribute(siteID: siteID, productVariationID: variationID, name: "Material", option: "Wool"), - PersistedProductVariationAttribute(siteID: siteID, productVariationID: variationID, name: "Fit", option: "Slim") + PersistedProductVariationAttribute(siteID: siteID, + productVariationID: variationID, + remoteAttributeID: 100, + name: "Material", + option: "Wool"), + PersistedProductVariationAttribute(siteID: siteID, + productVariationID: variationID, + remoteAttributeID: 101, + name: "Fit", + option: "Slim") ] let varImage = PersistedProductVariationImage( siteID: siteID, @@ -171,12 +179,14 @@ struct PersistedProductVariationTests { var attr1 = PersistedProductVariationAttribute( siteID: 2, productVariationID: 500, + remoteAttributeID: 502, name: "Color", option: "Purple" ) var attr2 = PersistedProductVariationAttribute( siteID: 2, productVariationID: 500, + remoteAttributeID: 503, name: "Material", option: "Cotton" ) @@ -282,6 +292,7 @@ struct PersistedProductVariationTests { var attr = PersistedProductVariationAttribute( siteID: 4, productVariationID: 700, + remoteAttributeID: 105, name: "Style", option: "Modern" ) From e5acf35360294aa8887afd67bfc59f0ad961b175 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 7 Oct 2025 14:30:33 +0100 Subject: [PATCH 2/3] Tests for local attributes --- .../Storage/PersistedProductTests.swift | 111 +++++++++++++++++ .../PersistedProductVariationTests.swift | 112 ++++++++++++++++++ 2 files changed, 223 insertions(+) diff --git a/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift b/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift index 6d27185a0b4..8f22d38f88a 100644 --- a/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift +++ b/Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift @@ -328,6 +328,117 @@ struct PersistedProductTests { #expect(back.options == attribute.options) } + @Test("PersistedProductAttribute with remoteAttributeID of 0 represents non-global attribute") + func product_attribute_with_zero_remote_id_is_non_global() throws { + // Given a non-global attribute (not shared across products) + let siteID: Int64 = 10 + let productID: Int64 = 100 + let attribute = ProductAttribute(siteID: siteID, + attributeID: 0, + name: "Custom Color", + position: 0, + visible: true, + variation: true, + options: ["Custom Red", "Custom Blue"]) + + // When + let persisted = PersistedProductAttribute(from: attribute, siteID: siteID, productID: productID) + + // Then the remoteAttributeID should be 0 for non-global attributes + #expect(persisted.remoteAttributeID == 0) + #expect(persisted.name == "Custom Color") + #expect(persisted.options == ["Custom Red", "Custom Blue"]) + + // When converting back to ProductAttribute + let back = persisted.toProductAttribute(siteID: siteID) + + // Then the attributeID should remain 0 + #expect(back.attributeID == 0) + #expect(back.name == "Custom Color") + #expect(back.options == ["Custom Red", "Custom Blue"]) + } + + @Test("Non-global product attribute persists and retrieves correctly with remoteAttributeID 0") + func non_global_product_attribute_persists_correctly() throws { + // Given + let grdbManager = try GRDBManager() + let db = grdbManager.databaseConnection + + try db.write { db in + let site = PersistedSite(id: 5) + try site.insert(db) + + let product = PersistedProduct( + id: 500, + siteID: 5, + name: "Custom Product", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "25.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try product.insert(db) + + // Insert a non-global attribute with remoteAttributeID = 0 + var nonGlobalAttribute = PersistedProductAttribute( + siteID: 5, + productID: 500, + remoteAttributeID: 0, + name: "Custom Texture", + position: 0, + visible: true, + variation: true, + options: ["Smooth", "Rough", "Textured"] + ) + try nonGlobalAttribute.insert(db) + + // Also insert a global attribute for comparison + var globalAttribute = PersistedProductAttribute( + siteID: 5, + productID: 500, + remoteAttributeID: 123, + name: "Standard Size", + position: 1, + visible: true, + variation: false, + options: ["Small", "Medium", "Large"] + ) + try globalAttribute.insert(db) + } + + // When + let fetchedProduct = try db.read { db in + try PersistedProduct.filter(PersistedProduct.Columns.id == 500).fetchOne(db) + } + + let product = try #require(fetchedProduct) + let posProduct = try product.toPOSProduct(db: db) + + // Then both attributes should be present + #expect(posProduct.attributes.count == 2) + + // Verify non-global attribute + let nonGlobalAttr = posProduct.attributes.first { $0.name == "Custom Texture" } + #expect(nonGlobalAttr != nil) + #expect(nonGlobalAttr?.attributeID == 0) + #expect(nonGlobalAttr?.options == ["Smooth", "Rough", "Textured"]) + #expect(nonGlobalAttr?.variation == true) + + // Verify global attribute + let globalAttr = posProduct.attributes.first { $0.name == "Standard Size" } + #expect(globalAttr != nil) + #expect(globalAttr?.attributeID == 123) + #expect(globalAttr?.options == ["Small", "Medium", "Large"]) + #expect(globalAttr?.variation == false) + } + @Test("PersistedProductImage init(from:) and toProductImage round-trip") func product_image_round_trip() throws { // Given diff --git a/Modules/Tests/YosemiteTests/Storage/PersistedProductVariationTests.swift b/Modules/Tests/YosemiteTests/Storage/PersistedProductVariationTests.swift index ce369679d61..cd3876da153 100644 --- a/Modules/Tests/YosemiteTests/Storage/PersistedProductVariationTests.swift +++ b/Modules/Tests/YosemiteTests/Storage/PersistedProductVariationTests.swift @@ -335,6 +335,118 @@ struct PersistedProductVariationTests { #expect(back.option == attr.option) } + @Test("ProductVariationAttribute with remoteAttributeID of 0 represents non-global attribute") + func variation_attribute_with_zero_remote_id_is_non_global() throws { + // Given a non-global variation attribute (not shared across variations) + let siteID: Int64 = 20 + let variationID: Int64 = 2000 + let attr = ProductVariationAttribute(id: 0, name: "Custom Finish", option: "Matte") + + // When + let persisted = PersistedProductVariationAttribute(from: attr, siteID: siteID, productVariationID: variationID) + + // Then the remoteAttributeID should be 0 for non-global attributes + #expect(persisted.remoteAttributeID == 0) + #expect(persisted.name == "Custom Finish") + #expect(persisted.option == "Matte") + + // When converting back to ProductVariationAttribute + let back = persisted.toProductVariationAttribute() + + // Then the id should remain 0 + #expect(back.id == 0) + #expect(back.name == "Custom Finish") + #expect(back.option == "Matte") + } + + @Test("Non-global variation attribute persists and retrieves correctly with remoteAttributeID 0") + func non_global_variation_attribute_persists_correctly() throws { + // Given + let grdbManager = try GRDBManager() + let db = grdbManager.databaseConnection + + try db.write { db in + let site = PersistedSite(id: 6) + try site.insert(db) + + let parentProduct = PersistedProduct( + id: 600, + siteID: 6, + name: "Parent Product with Variations", + productTypeKey: "variable", + fullDescription: nil, + shortDescription: nil, + sku: nil, + globalUniqueID: nil, + price: "0.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try parentProduct.insert(db) + + let variation = PersistedProductVariation( + id: 6000, + siteID: 6, + productID: 600, + sku: "VAR-6000", + globalUniqueID: nil, + price: "30.00", + downloadable: false, + fullDescription: nil, + manageStock: false, + stockQuantity: nil, + stockStatusKey: "instock" + ) + try variation.insert(db) + + // Insert a non-global variation attribute with remoteAttributeID = 0 + var nonGlobalAttribute = PersistedProductVariationAttribute( + siteID: 6, + productVariationID: 6000, + remoteAttributeID: 0, + name: "Custom Pattern", + option: "Striped" + ) + try nonGlobalAttribute.insert(db) + + // Also insert a global variation attribute for comparison + var globalAttribute = PersistedProductVariationAttribute( + siteID: 6, + productVariationID: 6000, + remoteAttributeID: 456, + name: "Standard Color", + option: "Navy" + ) + try globalAttribute.insert(db) + } + + // When + let fetchedVariation = try db.read { db in + try PersistedProductVariation.filter(PersistedProductVariation.Columns.id == 6000).fetchOne(db) + } + + let variation = try #require(fetchedVariation) + let posVariation = try variation.toPOSProductVariation(db: db) + + // Then both attributes should be present + #expect(posVariation.attributes.count == 2) + + // Verify non-global attribute + let nonGlobalAttr = posVariation.attributes.first { $0.name == "Custom Pattern" } + #expect(nonGlobalAttr != nil) + #expect(nonGlobalAttr?.id == 0) + #expect(nonGlobalAttr?.option == "Striped") + + // Verify global attribute + let globalAttr = posVariation.attributes.first { $0.name == "Standard Color" } + #expect(globalAttr != nil) + #expect(globalAttr?.id == 456) + #expect(globalAttr?.option == "Navy") + } + @Test("PersistedProductVariationImage init(from:) and toProductImage round-trip") func variation_image_round_trip() throws { // Given From 68f79cd01d04b0683d9bf139052f451da7a27278 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Tue, 7 Oct 2025 15:33:52 +0100 Subject: [PATCH 3/3] Fix broken tests --- Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift b/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift index d156ef5b7ab..969283639f5 100644 --- a/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift +++ b/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift @@ -193,6 +193,7 @@ struct GRDBManagerTests { let attribute = TestProductAttribute( siteID: 1, productID: 100, + remoteAttributeID: 0, name: "Color", position: 0, visible: true, @@ -247,6 +248,7 @@ struct GRDBManagerTests { let variationAttribute = TestProductVariationAttribute( siteID: 1, productVariationID: 200, + remoteAttributeID: 0, name: "Color", option: "Red" ) @@ -300,6 +302,7 @@ struct GRDBManagerTests { let productAttribute = TestProductAttribute( siteID: testSiteId, productID: 100, + remoteAttributeID: 50, name: "Color", position: 0, visible: true, @@ -311,6 +314,7 @@ struct GRDBManagerTests { let variationAttribute = TestProductVariationAttribute( siteID: testSiteId, productVariationID: 200, + remoteAttributeID: 60, name: "Size", option: "Large" ) @@ -563,6 +567,7 @@ struct GRDBManagerTests { let productAttribute = TestProductAttribute( siteID: siteID, productID: product.id, + remoteAttributeID: 50, name: "Color \(i)", position: i, visible: true, @@ -575,6 +580,7 @@ struct GRDBManagerTests { let variationAttribute = TestProductVariationAttribute( siteID: siteID, productVariationID: variation.id, + remoteAttributeID: 60, name: "Size \(i)", option: "Large" ) @@ -744,6 +750,7 @@ extension TestProductVariation: FetchableRecord, PersistableRecord { struct TestProductAttribute: Codable { let siteID: Int64 let productID: Int64 + let remoteAttributeID: Int64 let name: String let position: Int let visible: Bool @@ -758,6 +765,7 @@ extension TestProductAttribute: FetchableRecord, PersistableRecord { struct TestProductVariationAttribute: Codable { let siteID: Int64 let productVariationID: Int64 + let remoteAttributeID: Int64 let name: String let option: String }