Merge pull request #7521 from woocommerce/issue/conditional-requests

[Orders Performance] Fire sync requests conditionally

Author: Ecarrion

WooCommerce/Classes/Analytics/WooAnalyticsEvent.swift

@@ -482,6 +482,12 @@ extension WooAnalyticsEvent {
             WooAnalyticsEvent(statName: .collectPaymentTapped,
                               properties: [:])
         }
+
+        /// Tracked when accessing the system plugin list without it being in sync.
+        ///
+        static func pluginsNotSyncedYet() -> WooAnalyticsEvent {
+            WooAnalyticsEvent(statName: .pluginsNotSyncedYet, properties: [:])
+        }
     }
 }
 

WooCommerce/Classes/Analytics/WooAnalyticsStat.swift

@@ -661,6 +661,9 @@ public enum WooAnalyticsStat: String {
     case paymentsMenuCardReadersManualsTapped = "payments_hub_card_readers_manuals_tapped"
     case paymentsMenuManageCardReadersTapped = "payments_hub_manage_card_readers_tapped"
     case paymentsMenuPaymentProviderTapped = "settings_card_present_select_payment_gateway_tapped"
+
+    // MARK: Payments Menu
+    case pluginsNotSyncedYet = "plugins_not_synced_yet"
 }
 
 public extension WooAnalyticsStat {

WooCommerce/Classes/Extensions/SitePlugin+Woo.swift

@@ -5,5 +5,6 @@ import Yosemite
 extension SitePlugin {
     enum SupportedPlugin {
         public static let WCShip = "WooCommerce Shipping & Tax"
+        public static let WCTracking = "WooCommerce Shipment Tracking"
     }
 }

WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift

@@ -8,10 +8,12 @@ import Experiments
 import WooFoundation
 import SwiftUI
 import enum Networking.DotcomError
+import protocol Storage.StorageManagerType
 
 final class OrderDetailsViewModel {
 
     private let stores: StoresManager
+    private let storageManager: StorageManagerType
     private let currencyFormatter: CurrencyFormatter
 
     private(set) var order: Order
@@ -26,9 +28,11 @@ final class OrderDetailsViewModel {
 
     init(order: Order,
          stores: StoresManager = ServiceLocator.stores,
+         storageManager: StorageManagerType = ServiceLocator.storageManager,
          currencyFormatter: CurrencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings)) {
         self.order = order
         self.stores = stores
+        self.storageManager = storageManager
         self.currencyFormatter = currencyFormatter
     }
 
@@ -265,12 +269,18 @@ extension OrderDetailsViewModel {
     }
 
     func syncTrackingsEnablingAddButtonIfReachable(onReloadSections: (() -> ())? = nil, onCompletion: (() -> Void)? = nil) {
-        syncTracking { [weak self] error in
-            if error == nil {
-                self?.trackingIsReachable = true
+        // If the plugin is not active, there is no point on continuing with a request that will fail.
+        isPluginActive(SitePlugin.SupportedPlugin.WCTracking) { [weak self] isActive in
+            guard let self = self, isActive else {
+                onCompletion?()
+                return
+            }
+
+            self.trackingIsReachable = true
+            self.syncTracking { error in
+                onReloadSections?()
+                onCompletion?()
             }
-            onReloadSections?()
-            onCompletion?()
         }
     }
 }
@@ -533,22 +543,31 @@ extension OrderDetailsViewModel {
     }
 
     func syncShippingLabels(onCompletion: ((Error?) -> ())? = nil) {
-        let action = ShippingLabelAction.synchronizeShippingLabels(siteID: order.siteID, orderID: order.orderID) { result in
-            switch result {
-            case .success:
-                ServiceLocator.analytics.track(event: .shippingLabelsAPIRequest(result: .success))
+        // If the plugin is not active, there is no point on continuing with a request that will fail.
+        isPluginActive(SitePlugin.SupportedPlugin.WCShip) { [weak self] isActive in
+            guard let self = self, isActive else {
                 onCompletion?(nil)
-            case .failure(let error):
-                ServiceLocator.analytics.track(event: .shippingLabelsAPIRequest(result: .failed(error: error)))
-                if error as? DotcomError == .noRestRoute {
-                    DDLogError("⚠️ Endpoint for synchronizing shipping labels is unreachable. WC Shipping plugin may be missing.")
-                } else {
-                    DDLogError("⛔️ Error synchronizing shipping labels: \(error)")
+                return
+            }
+
+            let action = ShippingLabelAction.synchronizeShippingLabels(siteID: self.order.siteID, orderID: self.order.orderID) { result in
+                switch result {
+                case .success:
+                    ServiceLocator.analytics.track(event: .shippingLabelsAPIRequest(result: .success))
+                    onCompletion?(nil)
+                case .failure(let error):
+                    ServiceLocator.analytics.track(event: .shippingLabelsAPIRequest(result: .failed(error: error)))
+                    if error as? DotcomError == .noRestRoute {
+                        DDLogError("⚠️ Endpoint for synchronizing shipping labels is unreachable. WC Shipping plugin may be missing.")
+                    } else {
+                        DDLogError("⛔️ Error synchronizing shipping labels: \(error)")
+                    }
+                    onCompletion?(error)
                 }
-                onCompletion?(error)
             }
+            self.stores.dispatch(action)
+
         }
-        stores.dispatch(action)
     }
 
     func syncSavedReceipts(onCompletion: ((Error?) -> ())? = nil) {
@@ -566,16 +585,24 @@ extension OrderDetailsViewModel {
     }
 
     func checkShippingLabelCreationEligibility(onCompletion: (() -> Void)? = nil) {
-        let action = ShippingLabelAction.checkCreationEligibility(siteID: order.siteID,
-                                                                  orderID: order.orderID) { [weak self] isEligible in
-            self?.dataSource.isEligibleForShippingLabelCreation = isEligible
-            if isEligible, let orderStatus = self?.orderStatus?.status.rawValue {
-                ServiceLocator.analytics.track(.shippingLabelOrderIsEligible,
-                                               withProperties: ["order_status": orderStatus])
+        // If the plugin is not active, there is no point on continuing with a request that will fail.
+        isPluginActive(SitePlugin.SupportedPlugin.WCShip) { [weak self] isActive in
+            guard let self = self, isActive else {
+                onCompletion?()
+                return
             }
-            onCompletion?()
+
+            let action = ShippingLabelAction.checkCreationEligibility(siteID: self.order.siteID,
+                                                                      orderID: self.order.orderID) { [weak self] isEligible in
+                self?.dataSource.isEligibleForShippingLabelCreation = isEligible
+                if isEligible, let orderStatus = self?.orderStatus?.status.rawValue {
+                    ServiceLocator.analytics.track(.shippingLabelOrderIsEligible,
+                                                   withProperties: ["order_status": orderStatus])
+                }
+                onCompletion?()
+            }
+            self.stores.dispatch(action)
         }
-        stores.dispatch(action)
     }
 
     func checkOrderAddOnFeatureSwitchState(onCompletion: (() -> Void)? = nil) {
@@ -618,6 +645,32 @@ extension OrderDetailsViewModel {
 
         stores.dispatch(deleteTrackingAction)
     }
+
+    /// Helper function that returns `true` in its callback if the provided plugin name is active on the order's store.
+    /// Additionally it logs to tracks if the plugin store is accessed without it being in sync so we can handle that edge-case if it happens recurrently.
+    ///
+    private func isPluginActive(_ plugin: String, completion: @escaping (Bool) -> (Void)) {
+        guard arePluginsSynced() else {
+            DDLogError("⚠️ SystemPlugins acceded without being in sync.")
+            ServiceLocator.analytics.track(event: WooAnalyticsEvent.Orders.pluginsNotSyncedYet())
+            return completion(false)
+        }
+
+        let action = SystemStatusAction.fetchSystemPlugin(siteID: order.siteID, systemPluginName: plugin) { plugin in
+            completion(plugin?.active == true)
+        }
+        stores.dispatch(action)
+    }
+
+    /// Function that checks for any existing system plugin in the order's store.
+    /// If there is none, we assume plugins are not synced because at least the`WooCommerce` plugin should be present.
+    ///
+    private func arePluginsSynced() -> Bool {
+        let predicate = NSPredicate(format: "siteID == %lld", order.siteID)
+        let resultsController = ResultsController<StorageSystemPlugin>(storageManager: storageManager, matching: predicate, sortedBy: [])
+        try? resultsController.performFetch()
+        return !resultsController.isEmpty
+    }
 }
 
 // MARK: Definitions

WooCommerce/WooCommerceTests/ViewRelated/OrderDetailsViewModelTests.swift

@@ -9,13 +9,15 @@ final class OrderDetailsViewModelTests: XCTestCase {
     private var viewModel: OrderDetailsViewModel!
 
     private var storesManager: MockStoresManager!
+    private var storageManager: MockStorageManager!
 
     override func setUp() {
         storesManager = MockStoresManager(sessionManager: SessionManager.makeForTesting())
+        storageManager = MockStorageManager()
 
         order = MockOrders().sampleOrder()
 
-        viewModel = OrderDetailsViewModel(order: order, stores: storesManager)
+        viewModel = OrderDetailsViewModel(order: order, stores: storesManager, storageManager: storageManager)
 
         let analytics = WooAnalytics(analyticsProvider: MockAnalyticsProvider())
         ServiceLocator.setAnalytics(analytics)
@@ -73,16 +75,35 @@ final class OrderDetailsViewModelTests: XCTestCase {
 
     func test_checkShippingLabelCreationEligibility_dispatches_correctly() throws {
         // Given
+
+        // Make sure the are plugins synced
+        let plugin = SystemPlugin.fake().copy(siteID: order.siteID, name: SitePlugin.SupportedPlugin.WCShip, active: true)
+        storageManager.insertSampleSystemPlugin(readOnlySystemPlugin: plugin)
+
         storesManager.reset()
         XCTAssertEqual(storesManager.receivedActions.count, 0)
 
         // When
-        viewModel.checkShippingLabelCreationEligibility()
+        waitForExpectation { exp in
+
+            // Return the active WCShip plugin.
+            storesManager.whenReceivingAction(ofType: SystemStatusAction.self) { action in
+                switch action {
+                case .fetchSystemPlugin(_, _, let onCompletion):
+                    onCompletion(plugin)
+                    exp.fulfill()
+                default:
+                    break
+                }
+            }
+
+            viewModel.checkShippingLabelCreationEligibility()
+        }
 
         // Then
-        XCTAssertEqual(storesManager.receivedActions.count, 1)
+        XCTAssertEqual(storesManager.receivedActions.count, 2)
 
-        let action = try XCTUnwrap(storesManager.receivedActions.first as? ShippingLabelAction)
+        let action = try XCTUnwrap(storesManager.receivedActions.last as? ShippingLabelAction)
         guard case let ShippingLabelAction.checkCreationEligibility(siteID: siteID,
                                                                     orderID: orderID,
                                                                     onCompletion: _) = action else {