Merge pull request #8560 from woocommerce/issue/refactor-results-controller

Variations: Refactor how we access all variations.

Author: Ecarrion

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

@@ -699,7 +699,7 @@ extension ProductVariationsViewController: SyncingCoordinatorDelegate {
         }
     }
 
-    /// Generates all possible variations for the product attibutes.
+    /// Generates all possible variations for the product attributes.
     ///
     private func generateAllVariations() {
         viewModel.generateAllVariations(for: product) { [weak self] result in

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

@@ -28,33 +28,31 @@ final class ProductVariationsViewModel {
     /// Generates all missing variations for a product. Up to 100 variations.
     ///
     func generateAllVariations(for product: Product, onCompletion: @escaping (Result<Void, GenerationError>) -> Void) {
-        let action = ProductVariationAction.synchronizeAllProductVariations(siteID: product.siteID, productID: product.productID) { [weak self] result in
-            // TODO: Fetch this via a results controller
-            let existingVariations = ServiceLocator.storageManager.viewStorage.loadProductVariations(siteID: product.siteID, productID: product.productID)?
-                .map {
-                    $0.toReadOnly()
-                } ?? []
-
-            let variationsToGenerate = ProductVariationGenerator.generateVariations(for: product, excluding: existingVariations)
-
-            // Guard for 100 variation limit
-            guard variationsToGenerate.count <= 100 else {
-                return onCompletion(.failure(.tooManyVariations(variationCount: variationsToGenerate.count)))
-            }
 
-            guard variationsToGenerate.count > 0 else {
-                // TODO: Inform user that no variation will be created
-                return onCompletion(.success(()))
-            }
+        fetchAllVariations(of: product) { [weak self] result in
+            guard let self else { return }
+            switch result {
+            case .success(let existingVariations):
 
-            self?.createVariationsRemotely(for: product, variations: variationsToGenerate, onCompletion: onCompletion)
+                let variationsToGenerate = ProductVariationGenerator.generateVariations(for: product, excluding: existingVariations)
 
-        }
-        stores.dispatch(action)
+                // Guard for 100 variation limit
+                guard variationsToGenerate.count <= 100 else {
+                    return onCompletion(.failure(.tooManyVariations(variationCount: variationsToGenerate.count)))
+                }
+
+                guard variationsToGenerate.count > 0 else {
+                    // TODO: Inform user that no variation will be created
+                    return onCompletion(.success(()))
+                }
 
-        // TODO:
-        // - Alert if there are more than 100 variations to create
-        // - Create variations remotely
+                self.createVariationsRemotely(for: product, variations: variationsToGenerate, onCompletion: onCompletion)
+
+            case .failure:
+                // TODO: Log and inform error
+                break
+            }
+        }
     }
 
     /// Updates the internal `formType` to `edit` if  the given product exists remotely and previous formType was `.add`
@@ -66,6 +64,13 @@ final class ProductVariationsViewModel {
         formType = .edit
     }
 
+    /// Fetches all remote variations.
+    ///
+    private func fetchAllVariations(of product: Product, onCompletion: @escaping (Result<[ProductVariation], Error>) -> Void) {
+        let action = ProductVariationAction.synchronizeAllProductVariations(siteID: product.siteID, productID: product.productID, onCompletion: onCompletion)
+        stores.dispatch(action)
+    }
+
     /// Creates the provided variations remotely.
     ///
     private func createVariationsRemotely(for product: Product,

WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/ProductVariationsViewModelTests.swift

@@ -127,7 +127,7 @@ final class ProductVariationsViewModelTests: XCTestCase {
         stores.whenReceivingAction(ofType: ProductVariationAction.self) { action in
             switch action {
             case .synchronizeAllProductVariations(_, _, let onCompletion):
-                onCompletion(.success(()))
+                onCompletion(.success([]))
             default:
                 break
             }
@@ -159,7 +159,7 @@ final class ProductVariationsViewModelTests: XCTestCase {
         stores.whenReceivingAction(ofType: ProductVariationAction.self) { action in
             switch action {
             case .synchronizeAllProductVariations(_, _, let onCompletion):
-                onCompletion(.success(()))
+                onCompletion(.success([]))
             case .createProductVariations(_, _, _, let onCompletion):
                 onCompletion(.success([]))
             default:

Yosemite/Yosemite/Actions/ProductVariationAction.swift

@@ -8,7 +8,7 @@ public enum ProductVariationAction: Action {
 
     /// Synchronizes all the ProductVariation's available in the store.
     ///
-    case synchronizeAllProductVariations(siteID: Int64, productID: Int64, onCompletion: (Result<Void, Error>) -> Void)
+    case synchronizeAllProductVariations(siteID: Int64, productID: Int64, onCompletion: (Result<[ProductVariation], Error>) -> Void)
 
     /// Synchronizes the ProductVariation's matching the specified criteria.
     /// If successful, the result boolean value, will indicate weather there are more variations to fetch or not.

Yosemite/Yosemite/Stores/ProductVariationStore.swift

@@ -67,14 +67,21 @@ private extension ProductVariationStore {
 
     /// Synchronizes all the product reviews associated with a given Site ID (if any!).
     ///
-    func synchronizeAllProductVariations(siteID: Int64, productID: Int64, onCompletion: @escaping (Result<Void, Error>) -> Void) {
+    func synchronizeAllProductVariations(siteID: Int64, productID: Int64, onCompletion: @escaping (Result<[ProductVariation], Error>) -> Void) {
         let maxPageSize = 100 // API only allows to fetch a max of 100 variations at a time
         recursivelySyncAllVariations(siteID: siteID,
                                      productID: productID,
                                      pageNumber: Default.firstPageNumber,
-                                     pageSize: maxPageSize) { result in
-            // Transforms Result<Bool, Error> -> Result<Void, Error> as the bool value is not needed anymore.
-            onCompletion(result.map { _ in () }) //
+                                     pageSize: maxPageSize) { [weak self] result in
+            guard let self else { return }
+            switch result {
+            case .success:
+                let storedVariations = self.storageManager.viewStorage.loadProductVariations(siteID: siteID, productID: productID) ?? []
+                let readOnlyVariations = storedVariations.map { $0.toReadOnly() }
+                onCompletion(.success(readOnlyVariations))
+            case .failure(let error):
+                onCompletion(.failure(error))
+            }
         }
     }