diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 5d18d46fe43..aa1742751d4 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -13,6 +13,7 @@ - [Internal] Updated storage usage in CouponStore [https://github.com/woocommerce/woocommerce-ios/pull/14530] - [Internal] Update storage usage for BlazeStore [https://github.com/woocommerce/woocommerce-ios/pull/14532] - [Internal] Updated storage usage in ProductShippingClassStore [https://github.com/woocommerce/woocommerce-ios/pull/14520] +- [Internal] Updated storage usage in SystemStatusStore [https://github.com/woocommerce/woocommerce-ios/pull/14559] - [Internal] Updated storage usage in SitePluginStore [https://github.com/woocommerce/woocommerce-ios/pull/14560] - [Internal] Updated storage usage in ShippingLabelStore [https://github.com/woocommerce/woocommerce-ios/pull/14566] - [*] Fixed: Improved the error message displayed when Bluetooth permission is denied during the card reader connection process. [https://github.com/woocommerce/woocommerce-ios/pull/14561] diff --git a/Storage/Storage/Tools/StorageType+Extensions.swift b/Storage/Storage/Tools/StorageType+Extensions.swift index 5356a20ee91..0a1da1a748c 100644 --- a/Storage/Storage/Tools/StorageType+Extensions.swift +++ b/Storage/Storage/Tools/StorageType+Extensions.swift @@ -748,6 +748,14 @@ public extension StorageType { return allObjects(ofType: SystemPlugin.self, matching: predicate, sortedBy: [descriptor]) } + /// Returns stored system plugins for a provided `siteID` matching the given `names` + /// + func loadSystemPlugins(siteID: Int64, matching names: [String]) -> [SystemPlugin] { + let predicate = NSPredicate(format: "siteID == %lld && name in %@", siteID, names) + let descriptor = NSSortDescriptor(keyPath: \SystemPlugin.name, ascending: true) + return allObjects(ofType: SystemPlugin.self, matching: predicate, sortedBy: [descriptor]) + } + /// Returns a system plugin with a specified `siteID` and `name` /// func loadSystemPlugin(siteID: Int64, name: String) -> SystemPlugin? { diff --git a/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift b/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift index 9764d683471..dbab87bc5c5 100644 --- a/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift +++ b/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift @@ -1344,7 +1344,7 @@ final class StorageTypeExtensionsTests: XCTestCase { // MARK: - System plugins - func test_loadSystemPlugins_by_siteID_and_sorted_by_name() throws { + func test_loadSystemPlugins_by_siteID_and_sorted_by_name() { // Given let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self) systemPlugin1.name = "Plugin 1" @@ -1359,12 +1359,37 @@ final class StorageTypeExtensionsTests: XCTestCase { systemPlugin3.siteID = sampleSiteID // When - let storedSystemPlugins = try XCTUnwrap(storage.loadSystemPlugins(siteID: sampleSiteID)) + let storedSystemPlugins = storage.loadSystemPlugins(siteID: sampleSiteID) // Then XCTAssertEqual(storedSystemPlugins, [systemPlugin1, systemPlugin3]) } + func test_loadSystemPlugins_by_siteID_matching_names() { + // Given + let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self) + systemPlugin1.name = "Plugin 1" + systemPlugin1.siteID = sampleSiteID + + let systemPlugin2 = storage.insertNewObject(ofType: SystemPlugin.self) + systemPlugin2.name = "Plugin 1" + systemPlugin2.siteID = sampleSiteID + 1 + + let systemPlugin3 = storage.insertNewObject(ofType: SystemPlugin.self) + systemPlugin3.name = "Plugin 3" + systemPlugin3.siteID = sampleSiteID + + let systemPlugin4 = storage.insertNewObject(ofType: SystemPlugin.self) + systemPlugin4.name = "Plugin 4" + systemPlugin4.siteID = sampleSiteID + + // When + let storedSystemPlugins = storage.loadSystemPlugins(siteID: sampleSiteID, matching: ["Plugin 1", "Plugin 4"]) + + // Then + XCTAssertEqual(storedSystemPlugins, [systemPlugin1, systemPlugin4]) + } + func test_loadSystemPlugin_by_siteID_and_name() throws { // Given let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/Plugins/PluginListViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/Plugins/PluginListViewModel.swift index 5be123e1a2b..059ade8ee44 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/Plugins/PluginListViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/Plugins/PluginListViewModel.swift @@ -59,7 +59,14 @@ final class PluginListViewModel { /// Manually sync plugins. /// func syncPlugins(onCompletion: @escaping (Result) -> Void) { - let action = SitePluginAction.synchronizeSitePlugins(siteID: siteID, onCompletion: onCompletion) + let action = SystemStatusAction.synchronizeSystemInformation(siteID: siteID, onCompletion: { result in + switch result { + case .success: + onCompletion(.success(())) + case .failure(let error): + onCompletion(.failure(error)) + } + }) storesManager.dispatch(action) } } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Plugins/PluginListViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Plugins/PluginListViewModelTests.swift index 40d596216b8..8c4569fd6ab 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Plugins/PluginListViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Plugins/PluginListViewModelTests.swift @@ -31,9 +31,9 @@ class PluginListViewModelTests: XCTestCase { // Given let storesManager = MockStoresManager(sessionManager: .testingInstance) var triggeredSiteID: Int64? - storesManager.whenReceivingAction(ofType: SitePluginAction.self) { action in + storesManager.whenReceivingAction(ofType: SystemStatusAction.self) { action in switch action { - case .synchronizeSitePlugins(let siteID, _): + case .synchronizeSystemInformation(let siteID, _): triggeredSiteID = siteID default: break @@ -51,10 +51,10 @@ class PluginListViewModelTests: XCTestCase { func test_syncPlugins_returns_success_when_synchronizeSitePlugins_action_completes_successfully() { // Given let storesManager = MockStoresManager(sessionManager: .testingInstance) - storesManager.whenReceivingAction(ofType: SitePluginAction.self) { action in + storesManager.whenReceivingAction(ofType: SystemStatusAction.self) { action in switch action { - case .synchronizeSitePlugins(_, let completion): - completion(.success(())) + case .synchronizeSystemInformation(_, let completion): + completion(.success(.fake())) default: break } @@ -75,9 +75,9 @@ class PluginListViewModelTests: XCTestCase { func test_syncPlugins_returns_error_when_synchronizeSitePlugins_action_fails() { // Given let storesManager = MockStoresManager(sessionManager: .testingInstance) - storesManager.whenReceivingAction(ofType: SitePluginAction.self) { action in + storesManager.whenReceivingAction(ofType: SystemStatusAction.self) { action in switch action { - case .synchronizeSitePlugins(_, let completion): + case .synchronizeSystemInformation(_, let completion): completion(.failure(MockPluginError.mockError)) default: break @@ -97,24 +97,6 @@ class PluginListViewModelTests: XCTestCase { } } -// MARK: - Storage helpers -// -private extension PluginListViewModelTests { - func insert(_ readOnlyPlugin: Yosemite.SitePlugin) { - let plugin = storage.insertNewObject(ofType: StorageSitePlugin.self) - plugin.update(with: readOnlyPlugin) - storage.saveIfNeeded() - } - - func updateStorage(with readOnlyPlugin: Yosemite.SitePlugin) { - guard let plugin = storage.loadPlugin(siteID: readOnlyPlugin.siteID, name: readOnlyPlugin.name) else { - return - } - plugin.update(with: readOnlyPlugin) - storage.saveIfNeeded() - } -} - // MARK: - Mock types // private extension PluginListViewModelTests { diff --git a/Yosemite/Yosemite/Stores/SystemStatusStore.swift b/Yosemite/Yosemite/Stores/SystemStatusStore.swift index 396a84dc1a7..31a1248f74f 100644 --- a/Yosemite/Yosemite/Stores/SystemStatusStore.swift +++ b/Yosemite/Yosemite/Stores/SystemStatusStore.swift @@ -52,7 +52,7 @@ private extension SystemStatusStore { switch result { case .success(let systemInformation): self.updateStoreID(siteID: siteID, readonlySystemInformation: systemInformation) - self.upsertSystemPluginsInBackground(siteID: siteID, readonlySystemInformation: systemInformation) { [weak self] _ in + self.upsertSystemPluginsInBackground(siteID: siteID, readonlySystemInformation: systemInformation) { [weak self] in guard let self else { return } let systemPlugins = self.storageManager.viewStorage.loadSystemPlugins(siteID: siteID).map { $0.toReadOnly() } completionHandler(.success(.init(storeID: systemInformation.environment?.storeID, systemPlugins: systemPlugins))) @@ -77,17 +77,10 @@ private extension SystemStatusStore { /// func upsertSystemPluginsInBackground(siteID: Int64, readonlySystemInformation: SystemStatus, - completionHandler: @escaping (Result) -> Void) { - let writerStorage = storageManager.writerDerivedStorage - writerStorage.perform { - self.upsertSystemPlugins(siteID: siteID, readonlySystemInformation: readonlySystemInformation, in: writerStorage) - } - - storageManager.saveDerivedType(derivedStorage: writerStorage) { - DispatchQueue.main.async { - completionHandler(.success(())) - } - } + completionHandler: @escaping () -> Void) { + storageManager.performAndSave({ [weak self] storage in + self?.upsertSystemPlugins(siteID: siteID, readonlySystemInformation: readonlySystemInformation, in: storage) + }, completion: completionHandler, on: .main) } /// Updates the store id from the system information. @@ -117,10 +110,11 @@ private extension SystemStatusStore { return activePlugins + inactivePlugins }() + let storedPlugins = storage.loadSystemPlugins(siteID: siteID, matching: readonlySystemPlugins.map { $0.name }) readonlySystemPlugins.forEach { readonlySystemPlugin in // load or create new StorageSystemPlugin matching the readonly one let storageSystemPlugin: StorageSystemPlugin = { - if let systemPlugin = storage.loadSystemPlugin(siteID: readonlySystemPlugin.siteID, name: readonlySystemPlugin.name) { + if let systemPlugin = storedPlugins.first(where: { $0.name == readonlySystemPlugin.name }) { return systemPlugin } return storage.insertNewObject(ofType: StorageSystemPlugin.self) @@ -138,13 +132,9 @@ private extension SystemStatusStore { /// Useful when a plugin has had multiple names. /// func fetchSystemPlugin(siteID: Int64, systemPluginNameList: [String], completionHandler: @escaping (SystemPlugin?) -> Void) { - let viewStorage = storageManager.viewStorage - for systemPluginName in systemPluginNameList { - if let systemPlugin = viewStorage.loadSystemPlugin(siteID: siteID, name: systemPluginName)?.toReadOnly() { - return completionHandler(systemPlugin) - } - } - completionHandler(nil) + let matchingPlugins = storageManager.viewStorage.loadSystemPlugins(siteID: siteID, matching: systemPluginNameList) + .map { $0.toReadOnly() } + completionHandler(matchingPlugins.first) } func fetchSystemPluginWithPath(siteID: Int64, pluginPath: String, onCompletion: @escaping (SystemPlugin?) -> Void) {