diff --git a/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewController.swift b/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewController.swift index 103e0665b3d..243972ee29e 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewController.swift +++ b/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 diff --git a/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewModel.swift b/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewModel.swift index c001f345376..639bb4847b4 100644 --- a/WooCommerce/Classes/ViewRelated/Products/Variations/ProductVariationsViewModel.swift +++ b/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) { - 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, diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/ProductVariationsViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/ProductVariationsViewModelTests.swift index 8c33e1b75ef..f74fabf00d6 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Products/Variations/ProductVariationsViewModelTests.swift +++ b/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: diff --git a/Yosemite/Yosemite/Actions/ProductVariationAction.swift b/Yosemite/Yosemite/Actions/ProductVariationAction.swift index ab4fbbb077a..887431dac37 100644 --- a/Yosemite/Yosemite/Actions/ProductVariationAction.swift +++ b/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) + 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. diff --git a/Yosemite/Yosemite/Stores/ProductVariationStore.swift b/Yosemite/Yosemite/Stores/ProductVariationStore.swift index 9235aabe03f..70b1d8afadc 100644 --- a/Yosemite/Yosemite/Stores/ProductVariationStore.swift +++ b/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) { + 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 -> Result 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)) + } } }