Skip to content

Commit e2797d7

Browse files
EcarrionEcarrion
authored
Merge pull request #8505 from woocommerce/issue/8487-fetch-all-variations
Variations: Fetch all variations before generating variations
2 parents 1e6ee6c + 01a2f9f commit e2797d7

File tree

10 files changed

+137
-46
lines changed

10 files changed

+137
-46
lines changed

‎WooCommerce/Classes/ViewRelated/Products/ProductSelector/ProductVariationSelectorViewModel.swift‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,20 +182,21 @@ extension ProductVariationSelectorViewModel: SyncingCoordinatorDelegate {
182182
let action = ProductVariationAction.synchronizeProductVariations(siteID: siteID,
183183
productID: productID,
184184
pageNumber: pageNumber,
185-
pageSize: pageSize) { [weak self] error in
185+
pageSize: pageSize) { [weak self] result in
186186
guard let self = self else { return }
187187

188-
if let error = error {
188+
switch result {
189+
case .success:
190+
self.updateProductVariationsResultsController()
191+
case .failure(let error):
189192
self.notice = NoticeFactory.productVariationSyncNotice() { [weak self] in
190193
self?.sync(pageNumber: pageNumber, pageSize: pageSize, onCompletion: nil)
191194
}
192195
DDLogError("⛔️ Error synchronizing product variations during order creation: \(error)")
193-
} else {
194-
self.updateProductVariationsResultsController()
195196
}
196197

197198
self.transitionToResultsUpdatedState()
198-
onCompletion?(error == nil)
199+
onCompletion?(result.isSuccess)
199200
}
200201
stores.dispatch(action)
201202
}

‎WooCommerce/Classes/ViewRelated/Products/Variations/Bulk Update/BulkUpdateViewModel.swift‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,23 +85,22 @@ final class BulkUpdateViewModel {
8585
// There is a limitof 100 objects for bulk update API
8686
let pageNumber = Constants.pageNumber
8787
let action = ProductVariationAction
88-
.synchronizeProductVariations(siteID: siteID, productID: productID, pageNumber: pageNumber, pageSize: numberOfObjects) { [weak self] error in
88+
.synchronizeProductVariations(siteID: siteID, productID: productID, pageNumber: pageNumber, pageSize: numberOfObjects) { [weak self] result in
8989
guard let self = self else { return }
9090

91-
if let error = error {
92-
self.syncState = .error
93-
94-
DDLogError("⛔️ Error synchronizing product variations: \(error)")
95-
} else {
91+
switch result {
92+
case .success:
9693
self.configureResultsControllerAndFetchData { [weak self] error in
9794
guard let self = self else { return }
98-
9995
if error != nil {
10096
self.syncState = .error
10197
} else {
10298
self.syncState = .synced(self.sections)
10399
}
104100
}
101+
case .failure(let error):
102+
self.syncState = .error
103+
DDLogError("⛔️ Error synchronizing product variations: \(error)")
105104
}
106105
}
107106

‎WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewController.swift‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -612,8 +612,7 @@ private extension ProductVariationsViewController {
612612
case .single:
613613
self.createVariation()
614614
case .all:
615-
// TODO: Start generate all variations flow
616-
break
615+
self.generateAllVariations()
617616
}
618617

619618
}
@@ -649,22 +648,23 @@ extension ProductVariationsViewController: SyncingCoordinatorDelegate {
649648
transitionToSyncingState(pageNumber: pageNumber)
650649

651650
let action = ProductVariationAction
652-
.synchronizeProductVariations(siteID: siteID, productID: productID, pageNumber: pageNumber, pageSize: pageSize) { [weak self] error in
651+
.synchronizeProductVariations(siteID: siteID, productID: productID, pageNumber: pageNumber, pageSize: pageSize) { [weak self] result in
653652
guard let self = self else {
654653
return
655654
}
656655

657-
if let error = error {
656+
switch result {
657+
case .success:
658+
ServiceLocator.analytics.track(.productVariationListLoaded)
659+
case .failure(let error):
658660
ServiceLocator.analytics.track(.productVariationListLoadError, withError: error)
659661

660662
DDLogError("⛔️ Error synchronizing product variations: \(error)")
661663
self.displaySyncingErrorNotice(pageNumber: pageNumber, pageSize: pageSize)
662-
} else {
663-
ServiceLocator.analytics.track(.productVariationListLoaded)
664664
}
665665

666666
self.transitionToResultsUpdatedState()
667-
onCompletion?(error == nil)
667+
onCompletion?(result.isSuccess)
668668
}
669669

670670
ServiceLocator.stores.dispatch(action)
@@ -691,6 +691,16 @@ extension ProductVariationsViewController: SyncingCoordinatorDelegate {
691691
}
692692
}
693693
}
694+
695+
/// Generates all possible variations for the product attibutes.
696+
///
697+
private func generateAllVariations() {
698+
viewModel.generateAllVariations(for: product)
699+
// TODO:
700+
// - Show Loading Indicator
701+
// - Alert if there are more than 100 variations to create
702+
// - Hide Loading Indicator
703+
}
694704
}
695705

696706
// MARK: - Finite State Machine Management

‎WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewModel.swift‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,23 @@ final class ProductVariationsViewModel {
2525
useCase.generateVariation(onCompletion: onCompletion)
2626
}
2727

28+
/// Generates all missing variations for a product. Up to 100 variations.
29+
///
30+
func generateAllVariations(for product: Product) {
31+
let action = ProductVariationAction.synchronizeAllProductVariations(siteID: product.siteID, productID: product.productID) { result in
32+
// Temp
33+
let fetched = ServiceLocator.storageManager.viewStorage.loadProductVariations(siteID: product.siteID, productID: product.productID)
34+
print("Synchronized \(fetched?.count ?? 0) variations")
35+
}
36+
stores.dispatch(action)
37+
38+
// TODO:
39+
// - Generate all variations locally
40+
// - Substract already created variations
41+
// - Alert if there are more than 100 variations to create
42+
// - Create variations remotely
43+
}
44+
2845
/// Updates the internal `formType` to `edit` if the given product exists remotely and previous formType was `.add`
2946
///
3047
func updatedFormTypeIfNeeded(newProduct: Product) {

‎WooCommerce/WooCommerceTests/ViewRelated/Orders/Order Creation/ProductVariationSelectorViewModelTests.swift‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ final class ProductVariationSelectorViewModelTests: XCTestCase {
7070
switch action {
7171
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
7272
XCTAssertTrue(viewModel.shouldShowScrollIndicator, "Scroll indicator is not enabled during sync")
73-
onCompletion(nil)
73+
onCompletion(.success(false))
7474
default:
7575
XCTFail("Unsupported Action")
7676
}
@@ -91,7 +91,7 @@ final class ProductVariationSelectorViewModelTests: XCTestCase {
9191
switch action {
9292
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
9393
XCTAssertEqual(viewModel.syncStatus, .firstPageSync)
94-
onCompletion(nil)
94+
onCompletion(.success(false))
9595
default:
9696
XCTFail("Unsupported Action")
9797
}
@@ -113,7 +113,7 @@ final class ProductVariationSelectorViewModelTests: XCTestCase {
113113
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
114114
XCTAssertEqual(viewModel.syncStatus, .firstPageSync)
115115
self.insert(self.sampleProductVariation)
116-
onCompletion(nil)
116+
onCompletion(.success(false))
117117
default:
118118
XCTFail("Unsupported Action")
119119
}
@@ -136,7 +136,7 @@ final class ProductVariationSelectorViewModelTests: XCTestCase {
136136
switch action {
137137
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
138138
XCTAssertEqual(viewModel.syncStatus, .results)
139-
onCompletion(nil)
139+
onCompletion(.success(false))
140140
default:
141141
XCTFail("Unsupported Action")
142142
}
@@ -192,7 +192,7 @@ final class ProductVariationSelectorViewModelTests: XCTestCase {
192192
stores.whenReceivingAction(ofType: ProductVariationAction.self) { action in
193193
switch action {
194194
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
195-
onCompletion(NSError(domain: "Error", code: 0))
195+
onCompletion(.failure(NSError(domain: "Error", code: 0)))
196196
default:
197197
XCTFail("Received unsupported action: \(action)")
198198
}

‎WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/Bulk Update/BulkUpdateViewControllerTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ final class BulkUpdateViewControllerTests: XCTestCase {
2323
storesManager.whenReceivingAction(ofType: ProductVariationAction.self) { action in
2424
switch action {
2525
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
26-
onCompletion(NSError.init(domain: "sample error", code: 0, userInfo: nil))
26+
onCompletion(.failure(NSError.init(domain: "sample error", code: 0, userInfo: nil)))
2727
default:
2828
XCTFail("Unsupported Action")
2929
}

‎WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/Bulk Update/BulkUpdateViewModelTests.swift‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ final class BulkUpdateViewModelTests: XCTestCase {
9898
storesManager.whenReceivingAction(ofType: ProductVariationAction.self) { action in
9999
switch action {
100100
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
101-
onCompletion(NSError.init(domain: "sample error", code: 0, userInfo: nil))
101+
onCompletion(.failure(NSError.init(domain: "sample error", code: 0, userInfo: nil)))
102102
default:
103103
XCTFail("Unsupported Action")
104104
}
@@ -122,7 +122,7 @@ final class BulkUpdateViewModelTests: XCTestCase {
122122
storesManager.whenReceivingAction(ofType: ProductVariationAction.self) { action in
123123
switch action {
124124
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
125-
onCompletion(nil)
125+
onCompletion(.success(false))
126126
default:
127127
XCTFail("Unsupported Action")
128128
}
@@ -144,7 +144,7 @@ final class BulkUpdateViewModelTests: XCTestCase {
144144
storesManager.whenReceivingAction(ofType: ProductVariationAction.self) { action in
145145
switch action {
146146
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
147-
onCompletion(nil)
147+
onCompletion(.success(false))
148148
default:
149149
XCTFail("Unsupported Action")
150150
}
@@ -180,7 +180,7 @@ final class BulkUpdateViewModelTests: XCTestCase {
180180
storesManager.whenReceivingAction(ofType: ProductVariationAction.self) { action in
181181
switch action {
182182
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
183-
onCompletion(nil)
183+
onCompletion(.success(false))
184184
default:
185185
XCTFail("Unsupported Action")
186186
}
@@ -218,7 +218,7 @@ final class BulkUpdateViewModelTests: XCTestCase {
218218
storesManager.whenReceivingAction(ofType: ProductVariationAction.self) { action in
219219
switch action {
220220
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
221-
onCompletion(nil)
221+
onCompletion(.success(false))
222222
default:
223223
XCTFail("Unsupported Action")
224224
}
@@ -255,7 +255,7 @@ final class BulkUpdateViewModelTests: XCTestCase {
255255
storesManager.whenReceivingAction(ofType: ProductVariationAction.self) { action in
256256
switch action {
257257
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
258-
onCompletion(nil)
258+
onCompletion(.success(false))
259259
default:
260260
XCTFail("Unsupported Action")
261261
}
@@ -292,7 +292,7 @@ final class BulkUpdateViewModelTests: XCTestCase {
292292
storesManager.whenReceivingAction(ofType: ProductVariationAction.self) { action in
293293
switch action {
294294
case let .synchronizeProductVariations(_, _, _, _, onCompletion):
295-
onCompletion(nil)
295+
onCompletion(.success(false))
296296
default:
297297
XCTFail("Unsupported Action")
298298
}

‎Yosemite/Yosemite/Actions/ProductVariationAction.swift‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@ import Networking
66
//
77
public enum ProductVariationAction: Action {
88

9+
/// Synchronizes all the ProductVariation's available in the store.
10+
///
11+
case synchronizeAllProductVariations(siteID: Int64, productID: Int64, onCompletion: (Result<Void, Error>) -> Void)
12+
913
/// Synchronizes the ProductVariation's matching the specified criteria.
14+
/// If successful, the result boolean value, will indicate weather there are more variations to fetch or not.
1015
///
11-
case synchronizeProductVariations(siteID: Int64, productID: Int64, pageNumber: Int, pageSize: Int, onCompletion: (Error?) -> Void)
16+
case synchronizeProductVariations(siteID: Int64, productID: Int64, pageNumber: Int, pageSize: Int, onCompletion: (Result<Bool, Error>) -> Void)
1217

1318
/// Retrieves the specified ProductVariation.
1419
///

‎Yosemite/Yosemite/Stores/ProductVariationStore.swift‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ public final class ProductVariationStore: Store {
3636
}
3737

3838
switch action {
39+
case .synchronizeAllProductVariations(let siteID, let productID, let onCompletion):
40+
synchronizeAllProductVariations(siteID: siteID, productID: productID, onCompletion: onCompletion)
3941
case .synchronizeProductVariations(let siteID, let productID, let pageNumber, let pageSize, let onCompletion):
4042
synchronizeProductVariations(siteID: siteID, productID: productID, pageNumber: pageNumber, pageSize: pageSize, onCompletion: onCompletion)
4143
case .retrieveProductVariation(let siteID, let productID, let variationID, let onCompletion):
@@ -61,16 +63,30 @@ public final class ProductVariationStore: Store {
6163
//
6264
private extension ProductVariationStore {
6365

66+
/// Synchronizes all the product reviews associated with a given Site ID (if any!).
67+
///
68+
func synchronizeAllProductVariations(siteID: Int64, productID: Int64, onCompletion: @escaping (Result<Void, Error>) -> Void) {
69+
let maxPageSize = 100 // API only allows to fetch a max of 100 variations at a time
70+
recursivelySyncAllVariations(siteID: siteID,
71+
productID: productID,
72+
pageNumber: Default.firstPageNumber,
73+
pageSize: maxPageSize) { result in
74+
// Transforms Result<Bool, Error> -> Result<Void, Error> as the bool value is not needed anymore.
75+
onCompletion(result.map { _ in () }) //
76+
}
77+
}
78+
6479
/// Synchronizes the product reviews associated with a given Site ID (if any!).
80+
/// If successful, the result boolean value, will indicate weather there are more variations to fetch or not.
6581
///
66-
func synchronizeProductVariations(siteID: Int64, productID: Int64, pageNumber: Int, pageSize: Int, onCompletion: @escaping (Error?) -> Void) {
82+
func synchronizeProductVariations(siteID: Int64, productID: Int64, pageNumber: Int, pageSize: Int, onCompletion: @escaping (Result<Bool, Error>) -> Void) {
6783
remote.loadAllProductVariations(for: siteID,
6884
productID: productID,
6985
context: nil,
7086
pageNumber: pageNumber,
7187
pageSize: pageSize) { [weak self] (productVariations, error) in
7288
guard let productVariations = productVariations else {
73-
onCompletion(error)
89+
onCompletion(.failure(error ?? NSError()))
7490
return
7591
}
7692

@@ -82,7 +98,8 @@ private extension ProductVariationStore {
8298
self?.upsertStoredProductVariationsInBackground(readOnlyProductVariations: productVariations,
8399
siteID: siteID,
84100
productID: productID) {
85-
onCompletion(nil)
101+
let couldBeMoreVariationsToFetch = productVariations.count == pageSize
102+
onCompletion(.success(couldBeMoreVariationsToFetch))
86103
}
87104
}
88105
}
@@ -407,6 +424,30 @@ private extension ProductVariationStore {
407424
storageVariation.image = newStorageImage
408425
}
409426
}
427+
428+
/// Recursively sync all product variations starting with the given page number and using a maximum page size.
429+
///
430+
private func recursivelySyncAllVariations(siteID: Int64,
431+
productID: Int64,
432+
pageNumber: Int,
433+
pageSize: Int,
434+
onCompletion: @escaping (Result<Bool, Error>) -> Void) {
435+
synchronizeProductVariations(siteID: siteID, productID: productID, pageNumber: pageNumber, pageSize: pageSize) { [weak self] result in
436+
switch result {
437+
case .success(let hasMoreVariationsToFetch):
438+
guard hasMoreVariationsToFetch else {
439+
return onCompletion(.success(false))
440+
}
441+
self?.recursivelySyncAllVariations(siteID: siteID,
442+
productID: productID,
443+
pageNumber: pageNumber + 1,
444+
pageSize: pageSize,
445+
onCompletion: onCompletion)
446+
case .failure(let error):
447+
onCompletion(.failure(error))
448+
}
449+
}
450+
}
410451
}
411452

412453
public enum ProductVariationLoadError: Error, Equatable {

0 commit comments

Comments
 (0)