Merge pull request #8610 from woocommerce/issue/8536-error-handling

Variations: Error handling

Author: Ecarrion

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

@@ -64,6 +64,7 @@ final class ProductVariationsViewController: UIViewController, GhostableViewCont
     ///
     private let syncingCoordinator = SyncingCoordinator()
 
+
     private lazy var stateCoordinator: PaginatedListViewControllerStateCoordinator = {
         let stateCoordinator = PaginatedListViewControllerStateCoordinator(onLeavingState: { [weak self] state in
             self?.didLeave(state: state)
@@ -668,10 +669,18 @@ private extension ProductVariationsViewController {
     }
 
     /// Dismiss any `InProgressViewController` being presented.
+    /// By default retires the dismissal one time to overcome UIKit presentation delays.
     ///
-    private func dismissBlockingIndicator() {
+    private func dismissBlockingIndicator(retry: Bool = true) {
         if let inProgressViewController = self.presentedViewController as? InProgressViewController {
             inProgressViewController.dismiss(animated: true)
+        } else {
+            // When this method is invoked right after `InProgressViewController` is presented, chances are that it won't exists in the view controller
+            // hierarchy yet.
+            // Here we are retrying it with a small delay to give UIKit APIs time to finish its presentation.
+            DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) {
+                self.dismissBlockingIndicator(retry: false)
+            }
         }
     }
 

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

@@ -70,9 +70,9 @@ final class ProductVariationsViewModel {
                     }
                 }))
 
-            case .failure:
-                // TODO: Log and inform error
-                break
+            case .failure(let error):
+                onStateChanged(.error(.unableToFetchVariations))
+                DDLogError("⛔️ Failed to create variations: \(error)")
             }
         }
     }
@@ -104,9 +104,9 @@ final class ProductVariationsViewModel {
             switch result {
             case .success:
                 onCompletion(.success(()))
-            case .failure:
-                // TODO: Log Error
-                break
+            case .failure(let error):
+                onCompletion(.failure(.unableToCreateVariations))
+                DDLogError("⛔️ Failed to create variations: \(error)")
             }
         })
         stores.dispatch(action)
@@ -161,17 +161,27 @@ extension ProductVariationsViewModel {
     /// Type to represent known generation errors
     ///
     enum GenerationError: LocalizedError, Equatable {
+        case unableToFetchVariations
+        case unableToCreateVariations
         case tooManyVariations(variationCount: Int)
 
         var errorTitle: String {
             switch self {
+            case .unableToFetchVariations:
+                return NSLocalizedString("Unable to fetch variations", comment: "Error title for when we can't fetch existing variations.")
+            case .unableToCreateVariations:
+                return NSLocalizedString("Unable to create variations", comment: "Error title for when we can't create variations remotely.")
             case .tooManyVariations:
-                return NSLocalizedString("Generation limit exceeded", comment: "Error title for for when there are too many variations to generate.")
+                return NSLocalizedString("Generation limit exceeded", comment: "Error title for when there are too many variations to generate.")
             }
         }
 
         var errorDescription: String? {
             switch self {
+            case .unableToFetchVariations:
+                return NSLocalizedString("Something went wrong, please try again later.", comment: "Error message for when we can't fetch existing variations.")
+            case .unableToCreateVariations:
+                return NSLocalizedString("Something went wrong, please try again later.", comment: "Error message for when we can't create variations remotely")
             case .tooManyVariations(let variationCount):
                 let format = NSLocalizedString(
                     "Currently creation is supported for 100 variations maximum. Generating variations for this product would create %d variations.",

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

@@ -252,4 +252,73 @@ final class ProductVariationsViewModelTests: XCTestCase {
         // Then
         XCTAssertTrue(canceled)
     }
+
+    func test_failing_to_fetch_variations_sends_error_state() {
+        // Given
+        let product = Product.fake().copy(attributes: [
+            ProductAttribute.fake().copy(attributeID: 1, name: "Size", options: ["XS", "S", "M", "L", "XL"]),
+            ProductAttribute.fake().copy(attributeID: 2, name: "Color", options: ["Red", "Green", "Blue", "White", "Black"]),
+        ])
+
+        let stores = MockStoresManager(sessionManager: SessionManager.makeForTesting())
+        stores.whenReceivingAction(ofType: ProductVariationAction.self) { action in
+            switch action {
+            case .synchronizeAllProductVariations(_, _, let onCompletion):
+                onCompletion(.failure(NSError(domain: "", code: 0)))
+            default:
+                break
+            }
+        }
+
+        let viewModel = ProductVariationsViewModel(stores: stores, formType: .edit)
+
+        // When
+        let error = waitFor { promise in
+            viewModel.generateAllVariations(for: product) { state in
+                if case .error(let error) = state {
+                    promise(error)
+                }
+            }
+        }
+
+        // Then
+        XCTAssertEqual(error, .unableToFetchVariations)
+    }
+
+    func test_failing_to_create_variations_sends_error_state() {
+        // Given
+        let product = Product.fake().copy(attributes: [
+            ProductAttribute.fake().copy(attributeID: 1, name: "Size", options: ["XS", "S", "M", "L", "XL"]),
+            ProductAttribute.fake().copy(attributeID: 2, name: "Color", options: ["Red", "Green", "Blue", "White", "Black"]),
+        ])
+
+        let stores = MockStoresManager(sessionManager: SessionManager.makeForTesting())
+        stores.whenReceivingAction(ofType: ProductVariationAction.self) { action in
+            switch action {
+            case .synchronizeAllProductVariations(_, _, let onCompletion):
+                onCompletion(.success([]))
+            case .createProductVariations(_, _, _, let onCompletion):
+                onCompletion(.failure(NSError(domain: "", code: 0)))
+            default:
+                break
+            }
+        }
+
+        let viewModel = ProductVariationsViewModel(stores: stores, formType: .edit)
+
+        // When
+        let error = waitFor { promise in
+            viewModel.generateAllVariations(for: product) { state in
+                if case let .confirmation(_, onCompletion) = state {
+                    onCompletion(true)
+                }
+                if case .error(let error) = state {
+                    promise(error)
+                }
+            }
+        }
+
+        // Then
+        XCTAssertEqual(error, .unableToCreateVariations)
+    }
 }