Skip to content

Commit f4c711e

Browse files
joshhealdjoshheald
committed
Use upsert for incremental updates
This commit fixes an issue with incremental sync Previously, if variations changed, any unchanged variations for the same product would be deleted during an incremental sync. This was because we also get the parent product back, and we used `insert(onConflict: .replace)` – this triggered a cascade delete of variations (and other relations.) Then we inserted the changed variations, so any that were unchanged remained deleted. To avoid these issues, we move to `save`, which is GRDB’s upsert method. Additionally, with the fix deleted variations would not be deleted during an incremental sync. To improve that, we check the `variationIDs` array for which variations to leave in, after an incremental sync.
1 parent c48b5cb commit f4c711e

File tree

9 files changed

+69
-42
lines changed

9 files changed

+69
-42
lines changed

‎Modules/Sources/Fakes/Networking.generated.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,8 @@ extension Networking.POSProduct {
812812
attributes: .fake(),
813813
manageStock: .fake(),
814814
stockQuantity: .fake(),
815-
stockStatusKey: .fake()
815+
stockStatusKey: .fake(),
816+
variationIDs: .fake()
816817
)
817818
}
818819
}

‎Modules/Sources/Fakes/Yosemite.generated.swift‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ extension Yosemite.ProductReviewFromNoteParcel {
7676
)
7777
}
7878
}
79+
extension Yosemite.StoredBookingFilters {
80+
/// Returns a "ready to use" type filled with fake values.
81+
///
82+
public static func fake() -> Yosemite.StoredBookingFilters {
83+
.init(
84+
filters: .fake()
85+
)
86+
}
87+
}
7988
extension Yosemite.SystemInformation {
8089
/// Returns a "ready to use" type filled with fake values.
8190
///

‎Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,7 +1367,8 @@ extension Networking.POSProduct {
13671367
attributes: CopiableProp<[ProductAttribute]> = .copy,
13681368
manageStock: CopiableProp<Bool> = .copy,
13691369
stockQuantity: NullableCopiableProp<Decimal> = .copy,
1370-
stockStatusKey: CopiableProp<String> = .copy
1370+
stockStatusKey: CopiableProp<String> = .copy,
1371+
variationIDs: CopiableProp<[Int64]> = .copy
13711372
) -> Networking.POSProduct {
13721373
let siteID = siteID ?? self.siteID
13731374
let productID = productID ?? self.productID
@@ -1385,6 +1386,7 @@ extension Networking.POSProduct {
13851386
let manageStock = manageStock ?? self.manageStock
13861387
let stockQuantity = stockQuantity ?? self.stockQuantity
13871388
let stockStatusKey = stockStatusKey ?? self.stockStatusKey
1389+
let variationIDs = variationIDs ?? self.variationIDs
13881390

13891391
return Networking.POSProduct(
13901392
siteID: siteID,
@@ -1402,7 +1404,8 @@ extension Networking.POSProduct {
14021404
attributes: attributes,
14031405
manageStock: manageStock,
14041406
stockQuantity: stockQuantity,
1405-
stockStatusKey: stockStatusKey
1407+
stockStatusKey: stockStatusKey,
1408+
variationIDs: variationIDs
14061409
)
14071410
}
14081411
}

‎Modules/Sources/Networking/Model/POSProduct.swift‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab
4343
public let stockQuantity: Decimal?
4444
public let stockStatusKey: String
4545

46+
public let variationIDs: [Int64]
47+
4648
public init(siteID: Int64,
4749
productID: Int64,
4850
name: String,
@@ -58,7 +60,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab
5860
attributes: [ProductAttribute],
5961
manageStock: Bool,
6062
stockQuantity: Decimal?,
61-
stockStatusKey: String) {
63+
stockStatusKey: String,
64+
variationIDs: [Int64]) {
6265
self.siteID = siteID
6366
self.productID = productID
6467
self.name = name
@@ -81,6 +84,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab
8184
self.manageStock = manageStock
8285
self.stockQuantity = stockQuantity
8386
self.stockStatusKey = stockStatusKey
87+
88+
self.variationIDs = variationIDs
8489
}
8590

8691
public init(from decoder: any Decoder) throws {
@@ -124,6 +129,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab
124129
let stockQuantity = container.failsafeDecodeIfPresent(decimalForKey: .stockQuantity)
125130
let stockStatusKey = try container.decode(String.self, forKey: .stockStatusKey)
126131

132+
let variationIDs = try container.decodeIfPresent([Int64].self, forKey: .variationIDs) ?? []
133+
127134
self.init(siteID: siteID,
128135
productID: productID,
129136
name: name,
@@ -139,7 +146,8 @@ public struct POSProduct: Codable, Equatable, GeneratedCopiable, GeneratedFakeab
139146
attributes: attributes,
140147
manageStock: manageStock,
141148
stockQuantity: stockQuantity,
142-
stockStatusKey: stockStatusKey)
149+
stockStatusKey: stockStatusKey,
150+
variationIDs: variationIDs)
143151
}
144152

145153
static let requestFields: [String] = {
@@ -172,5 +180,6 @@ private extension POSProduct {
172180
case manageStock = "manage_stock"
173181
case stockQuantity = "stock_quantity"
174182
case stockStatusKey = "stock_status"
183+
case variationIDs = "variations"
175184
}
176185
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,12 @@ extension PersistedProduct: FetchableRecord, PersistableRecord {
8888
using: ForeignKey([PersistedProductAttribute.CodingKeys.siteID.stringValue,
8989
PersistedProductAttribute.CodingKeys.productID.stringValue],
9090
to: primaryKey))
91+
92+
public static let variations = hasMany(PersistedProductVariation.self,
93+
key: "variations",
94+
using: ForeignKey([PersistedProductVariation.CodingKeys.siteID.stringValue,
95+
PersistedProductVariation.CodingKeys.productID.stringValue],
96+
to: primaryKey))
9197
}
9298

9399
// MARK: - Point of Sale Requests

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,7 @@ public extension PersistedProductVariation {
108108
}
109109
}
110110

111-
// periphery:ignore - TODO: remove ignore when populating database
112-
private extension PersistedProductVariation {
111+
extension PersistedProductVariation {
113112
enum CodingKeys: String, CodingKey {
114113
case id
115114
case siteID

‎Modules/Sources/Yosemite/Model/Storage/PersistedProduct+Conversions.swift‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ extension PersistedProduct {
2323
)
2424
}
2525

26-
func toPOSProduct(images: [ProductImage] = [], attributes: [ProductAttribute] = []) -> POSProduct {
26+
func toPOSProduct(images: [ProductImage] = [], attributes: [ProductAttribute] = [], variationIDs: [Int64] = []) -> POSProduct {
2727
return POSProduct(
2828
siteID: siteID,
2929
productID: id,
@@ -40,20 +40,24 @@ extension PersistedProduct {
4040
attributes: attributes,
4141
manageStock: manageStock,
4242
stockQuantity: stockQuantity,
43-
stockStatusKey: stockStatusKey
43+
stockStatusKey: stockStatusKey,
44+
variationIDs: variationIDs
4445
)
4546
}
4647

4748
func toPOSProduct(db: GRDBDatabaseConnection) throws -> POSProduct {
48-
let (images, attributes) = try db.read { db in
49+
let (images, attributes, variationIDs) = try db.read { db in
4950
let images = try request(for: PersistedProduct.images).fetchAll(db)
5051
let attributes = try request(for: PersistedProduct.attributes).fetchAll(db)
51-
return (images, attributes)
52+
let variations = try request(for: PersistedProduct.variations).fetchAll(db)
53+
let variationIDs = variations.map(\.id)
54+
return (images, attributes, variationIDs)
5255
}
5356

5457
return toPOSProduct(
5558
images: images.map { $0.toProductImage() },
56-
attributes: attributes.map { $0.toProductAttribute(siteID: siteID) }
59+
attributes: attributes.map { $0.toProductAttribute(siteID: siteID) },
60+
variationIDs: variationIDs
5761
)
5862
}
5963

‎Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -87,52 +87,46 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol {
8787
DDLogInfo("💾 Persisting incremental catalog with \(catalog.products.count) updated products and \(catalog.variations.count) updated variations")
8888

8989
try await grdbManager.databaseConnection.write { db in
90-
for product in catalog.productsToPersist {
91-
try product.insert(db, onConflict: .replace)
92-
93-
// Delete old join table entries for this product
94-
try PersistedProductImage
95-
.filter { $0.siteID == siteID && $0.productID == product.id }
96-
.deleteAll(db)
97-
98-
try PersistedProductAttribute
99-
.filter { $0.siteID == siteID && $0.productID == product.id }
100-
.deleteAll(db)
90+
for product in catalog.products {
91+
try PersistedProduct(from: product).save(db)
92+
93+
// Delete variations that are no longer associated with this product
94+
let existingVariations = try PersistedProductVariation
95+
.filter(PersistedProductVariation.Columns.siteID == siteID)
96+
.filter(PersistedProductVariation.Columns.productID == product.productID)
97+
.fetchAll(db)
98+
99+
for variation in existingVariations {
100+
if !product.variationIDs.contains(variation.id) {
101+
try variation.delete(db)
102+
}
103+
}
101104
}
102105

103106
for variation in catalog.variationsToPersist {
104-
try variation.insert(db, onConflict: .replace)
105-
106-
// Delete old join table entries for this variation
107-
try PersistedProductVariationImage
108-
.filter { $0.siteID == siteID && $0.productVariationID == variation.id }
109-
.deleteAll(db)
110-
111-
try PersistedProductVariationAttribute
112-
.filter { $0.siteID == siteID && $0.productVariationID == variation.id }
113-
.deleteAll(db)
107+
try variation.save(db)
114108
}
115109

116-
// Insert/update actual image data (shared by products and variations)
110+
// Upsert actual image data (shared by products and variations)
117111
for image in catalog.imagesToPersist {
118-
try image.insert(db, onConflict: .replace)
112+
try image.save(db)
119113
}
120114

121-
// Insert new join table entries
115+
// Upsert new join table entries
122116
for image in catalog.productImagesToPersist {
123-
try image.insert(db, onConflict: .replace)
117+
try image.save(db)
124118
}
125119

126120
for image in catalog.variationImagesToPersist {
127-
try image.insert(db, onConflict: .replace)
121+
try image.save(db)
128122
}
129123

130124
for var attribute in catalog.productAttributesToPersist {
131-
try attribute.insert(db, onConflict: .replace)
125+
try attribute.save(db)
132126
}
133127

134128
for var attribute in catalog.variationAttributesToPersist {
135-
try attribute.insert(db, onConflict: .replace)
129+
try attribute.save(db)
136130
}
137131

138132
var site = try PersistedSite.fetchOne(db, key: siteID)

‎Modules/Tests/YosemiteTests/Storage/PersistedProductTests.swift‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ struct PersistedProductTests {
2727
attributes: [],
2828
manageStock: true,
2929
stockQuantity: 5,
30-
stockStatusKey: "instock"
30+
stockStatusKey: "instock",
31+
variationIDs: []
3132
)
3233

3334
// When
@@ -508,7 +509,8 @@ struct PersistedProductTests {
508509
],
509510
manageStock: true,
510511
stockQuantity: 50,
511-
stockStatusKey: "instock"
512+
stockStatusKey: "instock",
513+
variationIDs: []
512514
)
513515

514516
// When saving and loading back

0 commit comments

Comments
 (0)