Skip to content

Commit a965ae4

Browse files
jaclyncjaclync
committed
Remove storage loadSystemPlugin(siteID:name:) and use case in order details.
1 parent d9d3800 commit a965ae4

File tree

9 files changed

+128
-126
lines changed

9 files changed

+128
-126
lines changed

‎Modules/Sources/Storage/Tools/StorageType+Extensions.swift‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -832,13 +832,6 @@ public extension StorageType {
832832
return allObjects(ofType: SystemPlugin.self, matching: predicate, sortedBy: [descriptor])
833833
}
834834

835-
/// Returns a system plugin with a specified `siteID` and `name`
836-
///
837-
func loadSystemPlugin(siteID: Int64, name: String) -> SystemPlugin? {
838-
let predicate = \SystemPlugin.siteID == siteID && \SystemPlugin.name == name
839-
return firstObject(ofType: SystemPlugin.self, matching: predicate)
840-
}
841-
842835
/// Returns a system plugin with a specified `siteID` and `path`.
843836
///
844837
/// - Parameters:

‎Modules/Sources/Yosemite/Tools/Plugin.swift‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import Foundation
33
public enum Plugin: Equatable, CaseIterable {
44
case wooCommerce
55
case wooSubscriptions
6-
6+
case wooShipmentTracking
7+
78
/// File name without extension in the plugin path.
89
/// Full plugin path is like `woocommerce/woocommerce.php`.
910
var fileNameWithoutExtension: String {
@@ -12,6 +13,8 @@ public enum Plugin: Equatable, CaseIterable {
1213
return "woocommerce"
1314
case .wooSubscriptions:
1415
return "woocommerce-subscriptions"
16+
case .wooShipmentTracking:
17+
return "woocommerce-shipment-tracking"
1518
}
1619
}
1720
}

‎Modules/Tests/StorageTests/Tools/StorageTypeExtensionsTests.swift‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1432,23 +1432,6 @@ final class StorageTypeExtensionsTests: XCTestCase {
14321432
XCTAssertEqual(storedSystemPlugins, [systemPlugin1, systemPlugin4])
14331433
}
14341434

1435-
func test_loadSystemPlugin_by_siteID_and_name() throws {
1436-
// Given
1437-
let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self)
1438-
systemPlugin1.name = "Plugin 1"
1439-
systemPlugin1.siteID = sampleSiteID
1440-
1441-
let systemPlugin2 = storage.insertNewObject(ofType: SystemPlugin.self)
1442-
systemPlugin2.name = "Plugin 2"
1443-
systemPlugin2.siteID = sampleSiteID
1444-
1445-
// When
1446-
let foundSystemPlugin = try XCTUnwrap(storage.loadSystemPlugin(siteID: sampleSiteID, name: "Plugin 2"))
1447-
1448-
// Then
1449-
XCTAssertEqual(foundSystemPlugin, systemPlugin2)
1450-
}
1451-
14521435
func test_loadSystemPlugin_by_siteID_and_path() throws {
14531436
// Given
14541437
let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self)

‎Modules/Tests/YosemiteTests/Stores/SystemStatusStoreTests.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ final class SystemStatusStoreTests: XCTestCase {
7979

8080
func test_synchronizeSystemInformation_removes_stale_systemPlugins_correctly() {
8181
// Given
82-
let staleSystemPluginName = "Stale System Plugin"
83-
let staleSystemPlugin = SystemPlugin.fake().copy(siteID: sampleSiteID, name: staleSystemPluginName)
82+
let staleSystemPluginPath = "folder/stale-plugin.php"
83+
let staleSystemPlugin = SystemPlugin.fake().copy(siteID: sampleSiteID, plugin: staleSystemPluginPath)
8484
let storedStaleSystemPlugin = viewStorage.insertNewObject(ofType: StorageSystemPlugin.self)
8585
storedStaleSystemPlugin.update(with: staleSystemPlugin)
8686
XCTAssertEqual(viewStorage.countObjects(ofType: StorageSystemPlugin.self), 1)
@@ -98,7 +98,7 @@ final class SystemStatusStoreTests: XCTestCase {
9898
// Then
9999
XCTAssertTrue(result.isSuccess)
100100
XCTAssertEqual(viewStorage.countObjects(ofType: StorageSystemPlugin.self), 6) // number of systemPlugins in json file
101-
XCTAssertNil(viewStorage.loadSystemPlugin(siteID: sampleSiteID, name: staleSystemPluginName))
101+
XCTAssertNil(viewStorage.loadSystemPlugin(siteID: sampleSiteID, fileNameWithoutExtension: "stale-plugin"))
102102
}
103103

104104
func test_fetchSystemPlugins_return_systemPlugins_correctly() {

‎Modules/Tests/YosemiteTests/Tools/Plugins/PluginsServiceTests.swift‎

Lines changed: 90 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ struct PluginsServiceTests {
1515
@Test func waitForPluginInStorage_returns_plugin_when_already_in_storage() async {
1616
// Given
1717
await storageManager.reset()
18-
storageManager.insertWCPlugin(siteID: siteID, isActive: true, version: "1.0.0")
18+
storageManager.insertPlugin(siteID: siteID, plugin: .wooCommerce, isActive: true, version: "1.0.0")
1919

2020
// When
21-
let result = await sut.waitForPluginInStorage(siteID: siteID, pluginPath: PluginConstants.plugin, isActive: true)
21+
let result = await sut.waitForPluginInStorage(siteID: siteID, pluginPath: "woocommerce/woocommerce.php", isActive: true)
2222

2323
// Then
2424
#expect(result.siteID == siteID)
25-
#expect(result.plugin == PluginConstants.plugin)
25+
#expect(result.plugin == "woocommerce/woocommerce.php")
2626
#expect(result.active == true)
2727
#expect(result.version == "1.0.0")
2828
}
@@ -33,31 +33,103 @@ struct PluginsServiceTests {
3333
await storageManager.reset()
3434

3535
// When
36-
async let plugin = sut.waitForPluginInStorage(siteID: siteID, pluginPath: PluginConstants.plugin, isActive: true)
36+
async let plugin = sut.waitForPluginInStorage(siteID: siteID, pluginPath: "woocommerce/woocommerce.php", isActive: true)
3737
#expect(storageManager.viewStorage.loadSystemPlugins(siteID: siteID).count == 0)
38-
storageManager.insertWCPlugin(siteID: siteID, isActive: true, version: "2.0.0")
38+
storageManager.insertPlugin(siteID: siteID, plugin: .wooCommerce, isActive: true, version: "2.0.0")
3939
#expect(storageManager.viewStorage.loadSystemPlugins(siteID: siteID).count == 1)
4040

4141
// Then
4242
let result = await plugin
4343
#expect(result.siteID == siteID)
44-
#expect(result.plugin == PluginConstants.plugin)
45-
#expect(result.name == PluginConstants.pluginName)
44+
#expect(result.plugin == "woocommerce/woocommerce.php")
4645
#expect(result.active == true)
4746
#expect(result.version == "2.0.0")
4847
}
48+
49+
// MARK: - `loadPluginInStorage`
50+
51+
@Test(arguments: [(Plugin.wooCommerce, true, "1.5.0"),
52+
(Plugin.wooCommerce, false, "2.1.0"),
53+
(Plugin.wooSubscriptions, true, "3.0.0"),
54+
(Plugin.wooShipmentTracking, false, "3.0.0")])
55+
func loadPluginInStorage_returns_plugin_when_exists_in_storage(plugin: Plugin, isActive: Bool, version: String) async throws {
56+
// Given
57+
await storageManager.reset()
58+
storageManager.insertPlugin(siteID: siteID, plugin: plugin, isActive: isActive, version: version)
59+
60+
// When
61+
let result = sut.loadPluginInStorage(siteID: siteID, plugin: plugin, isActive: isActive)
62+
63+
// Then
64+
let unwrappedResult = try #require(result)
65+
#expect(unwrappedResult.siteID == siteID)
66+
#expect(unwrappedResult.plugin == plugin.pluginPath)
67+
#expect(unwrappedResult.active == isActive)
68+
#expect(unwrappedResult.version == version)
69+
}
70+
71+
@Test func loadPluginInStorage_returns_plugin_when_exists_in_storage_with_any_active_state() async throws {
72+
// Given
73+
await storageManager.reset()
74+
storageManager.insertPlugin(siteID: siteID, plugin: .wooCommerce, isActive: true, version: "3.0.0")
75+
76+
// When
77+
let result = sut.loadPluginInStorage(siteID: siteID, plugin: .wooCommerce, isActive: nil)
78+
79+
// Then
80+
let unwrappedResult = try #require(result)
81+
#expect(unwrappedResult.siteID == siteID)
82+
#expect(unwrappedResult.plugin == "woocommerce/woocommerce.php")
83+
#expect(unwrappedResult.active == true)
84+
#expect(unwrappedResult.version == "3.0.0")
85+
}
86+
87+
@Test func loadPluginInStorage_returns_nil_when_plugin_does_not_exist() async {
88+
// Given
89+
await storageManager.reset()
90+
91+
// When
92+
let result = sut.loadPluginInStorage(siteID: siteID, plugin: .wooCommerce, isActive: true)
93+
94+
// Then
95+
#expect(result == nil)
96+
}
97+
98+
@Test func loadPluginInStorage_returns_nil_when_plugin_exists_but_active_state_does_not_match() async {
99+
// Given
100+
await storageManager.reset()
101+
storageManager.insertPlugin(siteID: siteID, plugin: .wooCommerce, isActive: true, version: "1.0.0")
102+
103+
// When
104+
let result = sut.loadPluginInStorage(siteID: siteID, plugin: .wooCommerce, isActive: false)
105+
106+
// Then
107+
#expect(result == nil)
108+
}
109+
110+
@Test func loadPluginInStorage_returns_nil_when_plugin_exists_for_different_site() async {
111+
// Given
112+
await storageManager.reset()
113+
let differentSiteID: Int64 = 999
114+
storageManager.insertPlugin(siteID: differentSiteID, plugin: .wooCommerce, isActive: true, version: "1.0.0")
115+
116+
// When
117+
let result = sut.loadPluginInStorage(siteID: siteID, plugin: .wooCommerce, isActive: true)
118+
119+
// Then
120+
#expect(result == nil)
121+
}
49122
}
50123

51124
private extension MockStorageManager {
52-
func insertWCPlugin(siteID: Int64, isActive: Bool, version: String? = nil) {
125+
func insertPlugin(siteID: Int64, plugin: Plugin, isActive: Bool, version: String? = nil) {
53126
performAndSave({ storage in
54-
let plugin = SystemPlugin.fake().copy(siteID: siteID,
55-
plugin: PluginConstants.plugin,
56-
name: PluginConstants.pluginName,
57-
version: version,
58-
active: isActive)
127+
let systemPlugin = SystemPlugin.fake().copy(siteID: siteID,
128+
plugin: plugin.pluginPath,
129+
version: version,
130+
active: isActive)
59131
let newPlugin = storage.insertNewObject(ofType: StorageSystemPlugin.self)
60-
newPlugin.update(with: plugin)
132+
newPlugin.update(with: systemPlugin)
61133
}, completion: nil, on: .main)
62134
}
63135

@@ -70,8 +142,8 @@ private extension MockStorageManager {
70142
}
71143
}
72144

73-
// MARK: - Constants
74-
private enum PluginConstants {
75-
static let plugin = "example-plugin/example-plugin.php"
76-
static let pluginName = "Example Plugin"
145+
extension Plugin {
146+
var pluginPath: String {
147+
"\(fileNameWithoutExtension)/\(fileNameWithoutExtension).php"
148+
}
77149
}

‎WooCommerce/Classes/Extensions/SitePlugin+Woo.swift‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import Yosemite
44
///
55
extension SitePlugin {
66
enum SupportedPlugin {
7-
public static let WCTracking = "WooCommerce Shipment Tracking"
87
public static let WCSubscriptions = ["WooCommerce Subscriptions", "Woo Subscriptions"]
98
public static let WCProductBundles = ["WooCommerce Product Bundles", "Woo Product Bundles"]
109
public static let WCCompositeProducts = "WooCommerce Composite Products"

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

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ final class OrderDetailsViewModel {
1515
private let stores: StoresManager
1616
private let storageManager: StorageManagerType
1717
private let currencyFormatter: CurrencyFormatter
18+
private let pluginsService: PluginsServiceProtocol
1819
let featureFlagService: FeatureFlagService
1920

2021
private(set) var order: Order
@@ -35,7 +36,8 @@ final class OrderDetailsViewModel {
3536
currencyFormatter: CurrencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings),
3637
featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService,
3738
syncStateController: OrderDetailsSyncStateControlling = OrderDetailsSyncStateController(syncState: .notSynced),
38-
receiptEligibilityUseCase: ReceiptEligibilityUseCaseProtocol = ReceiptEligibilityUseCase()) {
39+
receiptEligibilityUseCase: ReceiptEligibilityUseCaseProtocol = ReceiptEligibilityUseCase(),
40+
pluginsService: PluginsServiceProtocol? = nil) {
3941
self.order = order
4042
self.stores = stores
4143
self.storageManager = storageManager
@@ -46,6 +48,7 @@ final class OrderDetailsViewModel {
4648
self.dataSource = OrderDetailsDataSource(order: order,
4749
cardPresentPaymentsConfiguration: configurationLoader.configuration)
4850
self.receiptEligibilityUseCase = receiptEligibilityUseCase
51+
self.pluginsService = pluginsService ?? PluginsService(storageManager: storageManager)
4952
}
5053

5154
func update(order newOrder: Order) {
@@ -228,6 +231,7 @@ final class OrderDetailsViewModel {
228231
extension OrderDetailsViewModel {
229232
/// Syncs all data related to the current order.
230233
///
234+
@MainActor
231235
func syncEverything(onReloadSections: (() -> ())? = nil, onCompletion: (() -> ())? = nil) {
232236
let group = DispatchGroup()
233237

@@ -309,7 +313,7 @@ extension OrderDetailsViewModel {
309313
defer {
310314
group.leave()
311315
}
312-
trackingIsReachable = await isShipmentTrackingEnabled()
316+
trackingIsReachable = isShipmentTrackingEnabled()
313317
guard trackingIsReachable else {
314318
return
315319
}
@@ -371,9 +375,9 @@ extension OrderDetailsViewModel {
371375
/// Checks if shipment tracking is enabled for the order.
372376
/// - Returns: Whether shipment tracking is enabled for the user by checking the products and if the Shipment Tracking plugin is active.
373377
@MainActor
374-
func isShipmentTrackingEnabled() async -> Bool {
378+
func isShipmentTrackingEnabled() -> Bool {
375379
guard orderContainsOnlyVirtualProducts == false,
376-
await isPluginActive(SitePlugin.SupportedPlugin.WCTracking) else {
380+
isPluginActive(.wooShipmentTracking) else {
377381
return false
378382
}
379383
return true
@@ -749,9 +753,10 @@ extension OrderDetailsViewModel {
749753
stores.dispatch(action)
750754
}
751755

756+
@MainActor
752757
func syncSubscriptions(onCompletion: ((Error?) -> ())? = nil) {
753758
// If the plugin is not active, there is no point in continuing with a request that will fail.
754-
isPluginActive(SitePlugin.SupportedPlugin.WCSubscriptions) { [weak self] isActive in
759+
isPluginActive(.wooSubscriptions) { [weak self] isActive in
755760

756761
guard let self, isActive else {
757762
onCompletion?(nil)
@@ -892,40 +897,33 @@ extension OrderDetailsViewModel {
892897
stores.dispatch(action)
893898
}
894899

895-
/// Helper function that returns `true` in its callback if the provided plugin name is active on the order's store.
900+
/// Helper function that returns `true` in its callback if the provided plugin is active on the order's store.
896901
/// 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.
897902
///
898-
private func isPluginActive(_ plugin: String, completion: @escaping (Bool) -> (Void)) {
899-
isPluginActive([plugin], completion: completion)
903+
@MainActor
904+
private func isPluginActive(_ plugin: Plugin) -> Bool {
905+
let plugin = fetchPlugin(plugin, isActive: true)
906+
return plugin != nil && plugin?.active == true
900907
}
901908

902-
/// Helper function that returns `true` in its callback if any of the the provided plugin names are active on the order's store.
903-
/// 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.
904-
/// Useful for when a plugin has had many names.
905-
///
906-
private func isPluginActive(_ pluginNames: [String], completion: @escaping (Bool) -> (Void)) {
907-
Task { @MainActor in
908-
let plugin = await fetchPluginByNames(pluginNames)
909-
completion(plugin?.active == true)
910-
}
909+
/// Legacy helper function that returns plugin active value in a completion closure.
910+
@MainActor
911+
private func isPluginActive(_ plugin: Plugin, completion: @escaping (Bool) -> (Void)) {
912+
completion(isPluginActive(plugin))
911913
}
912914

913915
/// Fetches a plugin from storage, based on the provided list of plugin names.
914916
/// 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.
915917
///
916918
@MainActor
917-
private func fetchPluginByNames(_ pluginNames: [String]) async -> SystemPlugin? {
919+
private func fetchPlugin(_ plugin: Plugin, isActive: Bool? = nil) -> SystemPlugin? {
918920
guard arePluginsSynced() else {
919921
DDLogError("⚠️ SystemPlugins accessed without being in sync.")
920922
ServiceLocator.analytics.track(event: WooAnalyticsEvent.Orders.pluginsNotSyncedYet())
921923
return nil
922924
}
923925

924-
return await withCheckedContinuation { continuation in
925-
stores.dispatch(SystemStatusAction.fetchSystemPluginListWithNameList(siteID: order.siteID, systemPluginNameList: pluginNames, onCompletion: { plugin in
926-
continuation.resume(returning: plugin)
927-
}))
928-
}
926+
return pluginsService.loadPluginInStorage(siteID: order.siteID, plugin: plugin, isActive: isActive)
929927
}
930928

931929
/// Fetches a plugin from storage, based on the provided plugin path.
@@ -1039,20 +1037,6 @@ private extension OrderDetailsViewModel {
10391037
}
10401038
}
10411039

1042-
@MainActor
1043-
func isPluginActive(_ plugin: String) async -> Bool {
1044-
return await isPluginActive([plugin])
1045-
}
1046-
1047-
@MainActor
1048-
func isPluginActive(_ pluginNames: [String]) async -> Bool {
1049-
await withCheckedContinuation { continuation in
1050-
isPluginActive(pluginNames) { isActive in
1051-
continuation.resume(returning: isActive)
1052-
}
1053-
}
1054-
}
1055-
10561040
@MainActor
10571041
func isPluginActive(pluginPath: String) async -> Bool {
10581042
let plugin = await fetchPluginByPath(pluginPath)

0 commit comments

Comments
 (0)