Skip to content

Commit a4b330d

Browse files
authored
Merge branch 'trunk' into test/local-catalog-v-23.8-for-testing
2 parents 9016f09 + 0ca913c commit a4b330d

File tree

8 files changed

+128
-27
lines changed

8 files changed

+128
-27
lines changed

.buildkite/commands/build-for-testing.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ echo "--- :hammer_and_wrench: Building"
1818
bundle exec fastlane build_for_testing
1919

2020
echo "--- :arrow_up: Upload Build Products"
21-
tar -cf build-products.tar DerivedData/Build/Products/ DerivedData/Index.noindex/DataStore/
21+
tar -cf build-products.tar DerivedData/Build/Products/
2222
buildkite-agent artifact upload build-products.tar

.buildkite/pipeline.yml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,6 @@ steps:
7979
agents:
8080
queue: linter
8181

82-
- label: "🧟 Detect unused code"
83-
command: ".buildkite/commands/run-periphery.sh"
84-
depends_on: build
85-
plugins: [$CI_TOOLKIT]
86-
notify:
87-
- github_commit_status:
88-
context: Periphery - Detect unused code
8982

9083
- label: 🧹 Lint Translations
9184
command: gplint /workdir/WooCommerce/Resources/AppStoreStrings.pot

Modules/Sources/Yosemite/PointOfSale/Items/GRDBObservableDataSource.swift

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,47 @@ public final class GRDBObservableDataSource: POSObservableDataSourceProtocol {
160160

161161
private func setupVariationObservation(parentProduct: POSVariableParentProduct) {
162162
let currentPage = currentVariationPage
163+
let parentProductID = parentProduct.productID
164+
165+
struct ObservationResult {
166+
let variations: [POSProductVariation]
167+
let parentProduct: POSVariableParentProduct
168+
}
169+
163170
let observation = ValueObservation
164-
.tracking { [weak self] database -> [POSProductVariation] in
165-
guard let self else { return [] }
171+
.tracking { [weak self] database -> ObservationResult in
172+
guard let self else { return ObservationResult(variations: [], parentProduct: parentProduct) }
173+
174+
// Fetch parent product with updated attributes
175+
struct ParentProductWithAttributes: Decodable, FetchableRecord {
176+
let product: PersistedProduct
177+
let attributes: [PersistedProductAttribute]
178+
}
179+
180+
let parentWithAttributes = try PersistedProduct
181+
.filter(PersistedProduct.Columns.siteID == self.siteID)
182+
.filter(PersistedProduct.Columns.id == parentProductID)
183+
.including(all: PersistedProduct.attributes)
184+
.asRequest(of: ParentProductWithAttributes.self)
185+
.fetchOne(database)
186+
187+
// Create updated parent product with fresh attributes
188+
let updatedParentProduct: POSVariableParentProduct
189+
if let parentWithAttributes {
190+
let attributes = (parentWithAttributes.attributes).map {
191+
$0.toProductAttribute(siteID: parentWithAttributes.product.siteID)
192+
}
193+
let product = parentWithAttributes.product
194+
updatedParentProduct = POSVariableParentProduct(
195+
id: parentProduct.id,
196+
name: product.name,
197+
productImageSource: nil, // Image not needed for variation name generation
198+
productID: product.id,
199+
allAttributes: attributes
200+
)
201+
} else {
202+
updatedParentProduct = parentProduct
203+
}
166204

167205
struct VariationWithRelations: Decodable, FetchableRecord {
168206
let persistedProductVariation: PersistedProductVariation
@@ -171,36 +209,42 @@ public final class GRDBObservableDataSource: POSObservableDataSourceProtocol {
171209
}
172210

173211
let variationsWithRelations = try PersistedProductVariation
174-
.posVariationsRequest(siteID: self.siteID, parentProductID: parentProduct.productID)
212+
.posVariationsRequest(siteID: self.siteID, parentProductID: parentProductID)
175213
.limit(self.pageSize * currentPage)
176214
.including(all: PersistedProductVariation.attributes)
177215
.including(optional: PersistedProductVariation.image)
178216
.asRequest(of: VariationWithRelations.self)
179217
.fetchAll(database)
180218

181-
return variationsWithRelations.map { record in
219+
let variations = variationsWithRelations.map { record in
182220
record.persistedProductVariation.toPOSProductVariation(
183221
attributes: (record.attributes ?? []).map { $0.toProductVariationAttribute() },
184222
image: record.image?.toProductImage()
185223
)
186224
}
225+
226+
return ObservationResult(variations: variations, parentProduct: updatedParentProduct)
187227
}
188228

189229
variationObservationCancellable = observation
190230
.publisher(in: grdbManager.databaseConnection)
191231
.receive(on: DispatchQueue.main)
192232
.sink(
193-
receiveCompletion: { [weak self] completion in
233+
receiveCompletion: { [weak self] (completion: Subscribers.Completion<Error>) in
194234
if case .failure(let error) = completion {
195235
self?.variationError = error
196236
self?.isLoadingVariations = false
197237
}
198238
},
199-
receiveValue: { [weak self] observedVariations in
239+
receiveValue: { [weak self] (result: ObservationResult) in
200240
guard let self else { return }
241+
242+
// Update current parent product with fresh attributes
243+
self.currentParentProduct = result.parentProduct
244+
201245
let posItems = itemMapper.mapVariationsToPOSItems(
202-
variations: observedVariations,
203-
parentProduct: parentProduct
246+
variations: result.variations,
247+
parentProduct: result.parentProduct
204248
)
205249
variationItems = posItems
206250
variationError = nil

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol {
9595

9696
try await grdbManager.databaseConnection.write { db in
9797
for product in catalog.products {
98+
// Delete attributes for updated products, the remaining set will be recreated later in the save
99+
try PersistedProductAttribute
100+
.filter(PersistedProductAttribute.Columns.productID == product.productID)
101+
.deleteAll(db)
102+
98103
try PersistedProduct(from: product).save(db)
99104

100105
// Delete variations that are no longer associated with this product
@@ -111,6 +116,11 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol {
111116
}
112117

113118
for variation in catalog.variationsToPersist {
119+
// Delete attributes for updated variations, the remaining set will be recreated later in the save
120+
try PersistedProductVariationAttribute
121+
.filter(PersistedProductVariationAttribute.Columns.productVariationID == variation.id)
122+
.deleteAll(db)
123+
114124
try variation.save(db)
115125
}
116126

@@ -129,7 +139,7 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol {
129139
}
130140

131141
for var attribute in catalog.productAttributesToPersist {
132-
try attribute.save(db)
142+
try attribute.insert(db)
133143
}
134144

135145
for var attribute in catalog.variationAttributesToPersist {

Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ struct POSCatalogPersistenceServiceTests {
275275
}
276276
}
277277

278-
@Test func persistIncrementalCatalogData_upserts_attributes_for_updated_product() async throws {
278+
@Test func persistIncrementalCatalogData_replaces_attributes_for_updated_product() async throws {
279279
// Given
280280
let attribute1 = Yosemite.ProductAttribute.fake().copy(name: "Color", options: ["Indigo", "Blue"])
281281
let attribute2 = Yosemite.ProductAttribute.fake().copy(name: "Size")
@@ -291,17 +291,40 @@ struct POSCatalogPersistenceServiceTests {
291291

292292
// Then
293293
try await grdbManager.databaseConnection.read { db in
294-
// Should have 4 attributes: original 2 + updated 2 (upsert adds new ones, keeps old ones)
294+
// Should have 2 attributes - old ones deleted, new ones added (no duplicates)
295295
let attributeCount = try PersistedProductAttribute.fetchCount(db)
296-
#expect(attributeCount == 4)
296+
#expect(attributeCount == 2)
297297

298298
let attributes = try PersistedProductAttribute.fetchAll(db).sorted(by: { $0.name < $1.name })
299299
#expect(attributes[0].name == "Color")
300-
#expect(attributes[0].options == ["Indigo", "Blue"]) // Original unchanged
301-
#expect(attributes[1].name == "Color")
302-
#expect(attributes[1].options == ["Cardinal", "Blue"]) // Updated version
303-
#expect(attributes[2].name == "Material") // New attribute
304-
#expect(attributes[3].name == "Size") // Original unchanged
300+
#expect(attributes[0].options == ["Cardinal", "Blue"]) // Updated version
301+
#expect(attributes[1].name == "Material") // New attribute
302+
}
303+
}
304+
305+
@Test func persistIncrementalCatalogData_prevents_duplicate_attributes_on_multiple_syncs() async throws {
306+
// Given - product with attributes
307+
let attribute1 = Yosemite.ProductAttribute.fake().copy(name: "Color", options: ["Red", "Blue"])
308+
let attribute2 = Yosemite.ProductAttribute.fake().copy(name: "Size", options: ["S", "M", "L"])
309+
let product = POSProduct.fake().copy(siteID: sampleSiteID, productID: 1, attributes: [attribute1, attribute2])
310+
try await insertProduct(product)
311+
312+
// When - perform multiple incremental syncs with the same product/attributes
313+
let catalog = POSCatalog(products: [product], variations: [], syncDate: .now)
314+
try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID)
315+
try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID)
316+
try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID)
317+
318+
// Then - should have exactly 2 attributes, not duplicates
319+
try await grdbManager.databaseConnection.read { db in
320+
let attributeCount = try PersistedProductAttribute.fetchCount(db)
321+
#expect(attributeCount == 2)
322+
323+
let attributes = try PersistedProductAttribute.fetchAll(db).sorted(by: { $0.name < $1.name })
324+
#expect(attributes[0].name == "Color")
325+
#expect(attributes[0].options == ["Red", "Blue"])
326+
#expect(attributes[1].name == "Size")
327+
#expect(attributes[1].options == ["S", "M", "L"])
305328
}
306329
}
307330

@@ -449,6 +472,36 @@ struct POSCatalogPersistenceServiceTests {
449472
}
450473
}
451474

475+
@Test func persistIncrementalCatalogData_prevents_duplicate_variation_attributes_on_multiple_syncs() async throws {
476+
// Given - variation with attributes
477+
let parentProduct = POSProduct.fake().copy(siteID: sampleSiteID, productID: 10)
478+
let attribute1 = Yosemite.ProductVariationAttribute.fake().copy(name: "Color", option: "Red")
479+
let attribute2 = Yosemite.ProductVariationAttribute.fake().copy(name: "Size", option: "L")
480+
let variation = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 10, productVariationID: 1, attributes: [attribute1, attribute2])
481+
try await insertProduct(parentProduct)
482+
try await insertVariation(variation)
483+
484+
// When - perform multiple incremental syncs with the same variation/attributes
485+
let catalog = POSCatalog(products: [parentProduct], variations: [variation], syncDate: .now)
486+
try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID)
487+
try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID)
488+
try await sut.persistIncrementalCatalogData(catalog, siteID: sampleSiteID)
489+
490+
// Then - should have exactly 2 attributes, not duplicates
491+
try await grdbManager.databaseConnection.read { db in
492+
let attributeCount = try PersistedProductVariationAttribute.fetchCount(db)
493+
#expect(attributeCount == 2)
494+
495+
let attributes = try PersistedProductVariationAttribute.fetchAll(db).sorted(by: { $0.name < $1.name })
496+
#expect(attributes[0].name == "Color")
497+
#expect(attributes[0].option == "Red")
498+
#expect(attributes[0].productVariationID == 1)
499+
#expect(attributes[1].name == "Size")
500+
#expect(attributes[1].option == "L")
501+
#expect(attributes[1].productVariationID == 1)
502+
}
503+
}
504+
452505
@Test func persistIncrementalCatalogData_replaces_image_for_updated_variation() async throws {
453506
// Given
454507
let parentProduct = POSProduct.fake().copy(siteID: sampleSiteID, productID: 10)

RELEASE-NOTES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
- [*] Update products on the Top Performers card when there are changes made to displayed products [https://github.com/woocommerce/woocommerce-ios/pull/16380]
99
- [*] Fixed issue unregistering device from push notifications [https://github.com/woocommerce/woocommerce-ios/pull/16386]
1010
- [Internal] Fix broken navigation to a variable product selector [https://github.com/woocommerce/woocommerce-ios/pull/16363]
11+
-[***] POS uses a local on-device product catalog for fast barcode scanning, search, and product selection [https://github.com/woocommerce/woocommerce-ios/pull/16395]
1112

1213
23.7
1314
-----

fastlane/metadata/default/name.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
WooCommerce
1+
WooCommerce: Store Management & POS
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Your store in your pocket
1+
All-in-one mobile ecommerce

0 commit comments

Comments
 (0)