Skip to content

Commit 2e322ea

Browse files
jaclyncjaclync
authored
[POS as a tab i2] Refactor plugins upsert so that plugins are matched by plugin path instead of name, and the first active WC plugin is returned for POS (#15915)
2 parents afd4650 + 4dde32e commit 2e322ea

File tree

8 files changed

+445
-92
lines changed

8 files changed

+445
-92
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,11 @@ public extension StorageType {
195195
}
196196
}
197197

198-
/// Deletes all of the stored SystemPlugins for the provided siteID whose name is not included in `currentSystemPlugins` array
198+
/// Deletes stored system plugins for the provided siteID that are not in the currentSystemPluginPaths list.
199199
///
200-
func deleteStaleSystemPlugins(siteID: Int64, currentSystemPlugins: [String]) {
200+
func deleteStaleSystemPlugins(siteID: Int64, currentSystemPluginPaths: [String]) {
201201
let systemPlugins = loadSystemPlugins(siteID: siteID).filter {
202-
!currentSystemPlugins.contains($0.name)
202+
!currentSystemPluginPaths.contains($0.plugin)
203203
}
204204
systemPlugins.forEach {
205205
deleteObject($0)

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -841,11 +841,28 @@ public extension StorageType {
841841

842842
/// Returns a system plugin with a specified `siteID` and `path`.
843843
///
844-
func loadSystemPlugin(siteID: Int64, path: String) -> SystemPlugin? {
845-
let predicate = \SystemPlugin.siteID == siteID && \SystemPlugin.plugin == path
844+
/// - Parameters:
845+
/// - siteID: The site ID to filter by.
846+
/// - path: The plugin path to match (e.g., "woocommerce/woocommerce.php").
847+
/// - active: Optional active state filter. If provided, only plugins with matching active state are returned. If nil, active state is ignored.
848+
/// - Returns: The matching system plugin, or nil if not found.
849+
func loadSystemPlugin(siteID: Int64, path: String, active: Bool? = nil) -> SystemPlugin? {
850+
let predicate = if let active {
851+
\SystemPlugin.siteID == siteID && \SystemPlugin.plugin == path && \SystemPlugin.active == active
852+
} else {
853+
\SystemPlugin.siteID == siteID && \SystemPlugin.plugin == path
854+
}
846855
return firstObject(ofType: SystemPlugin.self, matching: predicate)
847856
}
848857

858+
/// Returns stored system plugins for a provided `siteID` matching the given plugin `paths`.
859+
///
860+
func loadSystemPlugins(siteID: Int64, matchingPaths paths: [String]) -> [SystemPlugin] {
861+
let predicate = NSPredicate(format: "siteID == %lld && plugin in %@", siteID, paths)
862+
let descriptor = NSSortDescriptor(keyPath: \SystemPlugin.plugin, ascending: true)
863+
return allObjects(ofType: SystemPlugin.self, matching: predicate, sortedBy: [descriptor])
864+
}
865+
849866
// MARK: - Inbox Notes
850867

851868
/// Returns a single Inbox Note given a `siteID` and `id`

‎Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift‎

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,13 @@ public final class POSSystemStatusService: POSSystemStatusServiceProtocol {
4848
)
4949

5050
// Upserts all plugins in storage.
51-
await storageManager.performAndSaveAsync({ [weak self] storage in
52-
self?.upsertSystemPlugins(siteID: siteID, systemStatus: systemStatus, in: storage)
51+
await storageManager.performAndSaveAsync({ storage in
52+
let useCase = SystemPluginsUpsertUseCase(storage: storage)
53+
useCase.upsert(siteID: siteID, activePlugins: systemStatus.activePlugins, inactivePlugins: systemStatus.inactivePlugins)
5354
})
5455

5556
// Loads WooCommerce plugin from storage.
56-
guard let wcPlugin = storageManager.viewStorage.loadSystemPlugin(siteID: siteID, path: Constants.wcPluginPath)?.toReadOnly() else {
57+
guard let wcPlugin = storageManager.viewStorage.loadSystemPlugin(siteID: siteID, path: Constants.wcPluginPath, active: true)?.toReadOnly() else {
5758
return POSPluginAndFeatureInfo(wcPlugin: nil, featureValue: nil)
5859
}
5960

@@ -63,42 +64,6 @@ public final class POSSystemStatusService: POSSystemStatusServiceProtocol {
6364
}
6465
}
6566

66-
private extension POSSystemStatusService {
67-
/// Updates or inserts system plugins in storage.
68-
func upsertSystemPlugins(siteID: Int64, systemStatus: POSPluginEligibilitySystemStatus, in storage: StorageType) {
69-
// Active and inactive plugins share identical structure, but are stored in separate parts of the remote response
70-
// (and without an active attribute in the response). So we apply the correct value for active (or not)
71-
let readonlySystemPlugins: [SystemPlugin] = {
72-
let activePlugins = systemStatus.activePlugins.map {
73-
$0.copy(active: true)
74-
}
75-
76-
let inactivePlugins = systemStatus.inactivePlugins.map {
77-
$0.copy(active: false)
78-
}
79-
80-
return activePlugins + inactivePlugins
81-
}()
82-
83-
let storedPlugins = storage.loadSystemPlugins(siteID: siteID, matching: readonlySystemPlugins.map { $0.name })
84-
readonlySystemPlugins.forEach { readonlySystemPlugin in
85-
// Loads or creates new StorageSystemPlugin matching the readonly one.
86-
let storageSystemPlugin: StorageSystemPlugin = {
87-
if let systemPlugin = storedPlugins.first(where: { $0.name == readonlySystemPlugin.name }) {
88-
return systemPlugin
89-
}
90-
return storage.insertNewObject(ofType: StorageSystemPlugin.self)
91-
}()
92-
93-
storageSystemPlugin.update(with: readonlySystemPlugin)
94-
}
95-
96-
// Removes stale system plugins.
97-
let currentSystemPlugins = readonlySystemPlugins.map(\.name)
98-
storage.deleteStaleSystemPlugins(siteID: siteID, currentSystemPlugins: currentSystemPlugins)
99-
}
100-
}
101-
10267
private extension POSSystemStatusService {
10368
enum Constants {
10469
static let wcPluginPath = "woocommerce/woocommerce.php"

‎Modules/Sources/Yosemite/Stores/SystemPlugin/SystemPluginsUpsertUseCase.swift‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import Foundation
2+
import Storage
3+
import Networking
4+
5+
/// Upserts `Networking.SystemPlugin` objects into Storage.
6+
///
7+
/// This UseCase encapsulates the business logic for inserting or updating system plugins,
8+
/// including handling active/inactive state and removing stale plugins.
9+
struct SystemPluginsUpsertUseCase {
10+
private let storage: StorageType
11+
12+
/// Initializes a new UseCase.
13+
///
14+
/// - Parameter storage: A derived `StorageType`.
15+
init(storage: StorageType) {
16+
self.storage = storage
17+
}
18+
19+
/// Updates or inserts the given system plugins for a site.
20+
///
21+
/// - Parameters:
22+
/// - siteID: The site ID these plugins belong to.
23+
/// - activePlugins: Array of active system plugins.
24+
/// - inactivePlugins: Array of inactive system plugins.
25+
///
26+
func upsert(siteID: Int64, activePlugins: [SystemPlugin], inactivePlugins: [SystemPlugin]) {
27+
// Active and inactive plugins share identical structure, but are stored in separate parts of the remote response
28+
// (and without an active attribute in the response). So we apply the correct value for active (or not).
29+
let readonlySystemPlugins: [SystemPlugin] = {
30+
let activePluginsWithState = activePlugins.map {
31+
$0.copy(active: true)
32+
}
33+
34+
let inactivePluginsWithState = inactivePlugins.map {
35+
$0.copy(active: false)
36+
}
37+
38+
return activePluginsWithState + inactivePluginsWithState
39+
}()
40+
41+
let storedPlugins = storage.loadSystemPlugins(siteID: siteID, matchingPaths: readonlySystemPlugins.map { $0.plugin })
42+
readonlySystemPlugins.forEach { readonlySystemPlugin in
43+
// Loads or creates new StorageSystemPlugin matching the readonly one.
44+
let storageSystemPlugin: StorageSystemPlugin = {
45+
if let systemPlugin = storedPlugins.first(where: { $0.plugin == readonlySystemPlugin.plugin }) {
46+
return systemPlugin
47+
}
48+
return storage.insertNewObject(ofType: StorageSystemPlugin.self)
49+
}()
50+
51+
storageSystemPlugin.update(with: readonlySystemPlugin)
52+
}
53+
54+
// Removes stale system plugins.
55+
let currentSystemPluginPaths = readonlySystemPlugins.map(\.plugin)
56+
storage.deleteStaleSystemPlugins(siteID: siteID, currentSystemPluginPaths: currentSystemPluginPaths)
57+
}
58+
}

‎Modules/Sources/Yosemite/Stores/SystemStatusStore.swift‎

Lines changed: 3 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,9 @@ private extension SystemStatusStore {
7878
func upsertSystemPluginsInBackground(siteID: Int64,
7979
readonlySystemInformation: SystemStatus,
8080
completionHandler: @escaping () -> Void) {
81-
storageManager.performAndSave({ [weak self] storage in
82-
self?.upsertSystemPlugins(siteID: siteID, readonlySystemInformation: readonlySystemInformation, in: storage)
81+
storageManager.performAndSave({ storage in
82+
let useCase = SystemPluginsUpsertUseCase(storage: storage)
83+
useCase.upsert(siteID: siteID, activePlugins: readonlySystemInformation.activePlugins, inactivePlugins: readonlySystemInformation.inactivePlugins)
8384
}, completion: completionHandler, on: .main)
8485
}
8586

@@ -90,43 +91,6 @@ private extension SystemStatusStore {
9091
dispatcher.dispatch(action)
9192
}
9293

93-
/// Updates or inserts Readonly sistem plugins from the read only system information in specified storage.
94-
/// Also removes stale plugins that no longer exist in remote plugin list.
95-
///
96-
func upsertSystemPlugins(siteID: Int64, readonlySystemInformation: SystemStatus, in storage: StorageType) {
97-
/// Active and in-active plugins share identical structure, but are stored in separate parts of the remote response
98-
/// (and without an active attribute in the response). So... we use the same decoder for active and in-active plugins
99-
/// and here we apply the correct value for active (or not)
100-
///
101-
let readonlySystemPlugins: [SystemPlugin] = {
102-
let activePlugins = readonlySystemInformation.activePlugins.map {
103-
$0.copy(active: true)
104-
}
105-
106-
let inactivePlugins = readonlySystemInformation.inactivePlugins.map {
107-
$0.copy(active: false)
108-
}
109-
110-
return activePlugins + inactivePlugins
111-
}()
112-
113-
let storedPlugins = storage.loadSystemPlugins(siteID: siteID, matching: readonlySystemPlugins.map { $0.name })
114-
readonlySystemPlugins.forEach { readonlySystemPlugin in
115-
// load or create new StorageSystemPlugin matching the readonly one
116-
let storageSystemPlugin: StorageSystemPlugin = {
117-
if let systemPlugin = storedPlugins.first(where: { $0.name == readonlySystemPlugin.name }) {
118-
return systemPlugin
119-
}
120-
return storage.insertNewObject(ofType: StorageSystemPlugin.self)
121-
}()
122-
123-
storageSystemPlugin.update(with: readonlySystemPlugin)
124-
}
125-
126-
// remove stale system plugins
127-
let currentSystemPlugins = readonlySystemPlugins.map(\.name)
128-
storage.deleteStaleSystemPlugins(siteID: siteID, currentSystemPlugins: currentSystemPlugins)
129-
}
13094

13195
/// Retrieve a `SystemPlugin` entity from storage whose name matches any name from the provided name list.
13296
/// Useful when a plugin has had multiple names.

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -115,18 +115,18 @@ final class StorageTypeDeletionsTests: XCTestCase {
115115

116116
// MARK: - System plugins
117117

118-
func test_deleteStaleSystemPlugins_deletes_systemPlugins_not_included_in_currentSystemPlugins() throws {
118+
func test_deleteStaleSystemPlugins_by_plugin_paths_deletes_systemPlugins_not_included_in_currentSystemPluginPaths() throws {
119119
// Given
120-
_ = createSystemPlugin(name: "Plugin 1")
121-
_ = createSystemPlugin(name: "Plugin 2")
122-
let systemPlugin3 = createSystemPlugin(name: "Plugin 3")
120+
_ = createSystemPlugin(name: "WooCommerce", path: "woocommerce/woocommerce.php")
121+
_ = createSystemPlugin(name: "Jetpack", path: "jetpack/jetpack.php")
122+
let systemPlugin3 = createSystemPlugin(name: "WooCommerce", path: "wordpress-seo/wp-seo.php")
123123

124124
// When
125-
storage.deleteStaleSystemPlugins(siteID: sampleSiteID, currentSystemPlugins: ["Plugin 3"])
125+
storage.deleteStaleSystemPlugins(siteID: sampleSiteID, currentSystemPluginPaths: ["wordpress-seo/wp-seo.php"])
126126

127127
// Then
128-
let currrentSystemPlugin = storage.loadSystemPlugins(siteID: sampleSiteID)
129-
XCTAssertEqual(currrentSystemPlugin, [systemPlugin3])
128+
let currentSystemPlugins = storage.loadSystemPlugins(siteID: sampleSiteID)
129+
XCTAssertEqual(currentSystemPlugins, [systemPlugin3])
130130
}
131131

132132
func test_deleteBlazeTargetDevices_with_locale() throws {
@@ -242,12 +242,15 @@ private extension StorageTypeDeletionsTests {
242242
return plugin
243243
}
244244

245-
/// Creates and inserts a `SystemPlugin` entity with a given name
245+
/// Creates and inserts a `SystemPlugin` entity with a given name and optional path.
246246
///
247-
func createSystemPlugin(name: String) -> SystemPlugin {
247+
func createSystemPlugin(name: String, path: String? = nil) -> SystemPlugin {
248248
let systemPlugin = storage.insertNewObject(ofType: SystemPlugin.self)
249249
systemPlugin.siteID = sampleSiteID
250250
systemPlugin.name = name
251+
if let path {
252+
systemPlugin.plugin = path
253+
}
251254
return systemPlugin
252255
}
253256
}

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

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1466,6 +1466,107 @@ final class StorageTypeExtensionsTests: XCTestCase {
14661466
XCTAssertEqual(foundSystemPlugin, systemPlugin2)
14671467
}
14681468

1469+
func test_loadSystemPlugin_by_siteID_path_and_active_state() throws {
1470+
// Given
1471+
let inactivePlugin = storage.insertNewObject(ofType: SystemPlugin.self)
1472+
inactivePlugin.plugin = "woocommerce/woocommerce.php"
1473+
inactivePlugin.siteID = sampleSiteID
1474+
inactivePlugin.active = false
1475+
1476+
let activePlugin = storage.insertNewObject(ofType: SystemPlugin.self)
1477+
activePlugin.plugin = "woocommerce/woocommerce.php"
1478+
activePlugin.siteID = sampleSiteID
1479+
activePlugin.active = true
1480+
1481+
// When
1482+
let foundActivePlugin = try XCTUnwrap(storage.loadSystemPlugin(siteID: sampleSiteID, path: "woocommerce/woocommerce.php", active: true))
1483+
1484+
// Then
1485+
XCTAssertEqual(foundActivePlugin, activePlugin)
1486+
}
1487+
1488+
func test_loadSystemPlugin_by_siteID_path_and_inactive_state() throws {
1489+
// Given
1490+
let activePlugin = storage.insertNewObject(ofType: SystemPlugin.self)
1491+
activePlugin.plugin = "woocommerce/woocommerce.php"
1492+
activePlugin.siteID = sampleSiteID
1493+
activePlugin.active = true
1494+
1495+
let inactivePlugin = storage.insertNewObject(ofType: SystemPlugin.self)
1496+
inactivePlugin.plugin = "woocommerce/woocommerce.php"
1497+
inactivePlugin.siteID = sampleSiteID
1498+
inactivePlugin.active = false
1499+
1500+
// When
1501+
let foundInactivePlugin = try XCTUnwrap(storage.loadSystemPlugin(siteID: sampleSiteID, path: "woocommerce/woocommerce.php", active: false))
1502+
1503+
// Then
1504+
XCTAssertEqual(foundInactivePlugin, inactivePlugin)
1505+
}
1506+
1507+
func test_loadSystemPlugin_by_siteID_and_path_ignoring_active_state() throws {
1508+
// Given
1509+
let activePlugin = storage.insertNewObject(ofType: SystemPlugin.self)
1510+
activePlugin.plugin = "woocommerce/woocommerce.php"
1511+
activePlugin.siteID = sampleSiteID
1512+
activePlugin.active = true
1513+
1514+
let inactivePlugin = storage.insertNewObject(ofType: SystemPlugin.self)
1515+
inactivePlugin.plugin = "jetpack/jetpack.php"
1516+
inactivePlugin.siteID = sampleSiteID
1517+
inactivePlugin.active = false
1518+
1519+
// When
1520+
let foundPlugin = storage.loadSystemPlugin(siteID: sampleSiteID, path: "woocommerce/woocommerce.php", active: nil)
1521+
1522+
// Then
1523+
XCTAssertNotNil(foundPlugin)
1524+
XCTAssertEqual(foundPlugin, activePlugin)
1525+
}
1526+
1527+
func test_loadSystemPlugin_returns_nil_when_no_match_for_active_state() {
1528+
// Given
1529+
let activePlugin = storage.insertNewObject(ofType: SystemPlugin.self)
1530+
activePlugin.plugin = "woocommerce/woocommerce.php"
1531+
activePlugin.siteID = sampleSiteID
1532+
activePlugin.active = true
1533+
1534+
// When
1535+
let foundPlugin = storage.loadSystemPlugin(siteID: sampleSiteID, path: "woocommerce/woocommerce.php", active: false)
1536+
1537+
// Then
1538+
XCTAssertNil(foundPlugin)
1539+
}
1540+
1541+
func test_loadSystemPlugins_by_siteID_matching_paths() {
1542+
// Given
1543+
let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self)
1544+
systemPlugin1.plugin = "woocommerce/woocommerce.php"
1545+
systemPlugin1.siteID = sampleSiteID
1546+
1547+
let systemPlugin2 = storage.insertNewObject(ofType: SystemPlugin.self)
1548+
systemPlugin2.plugin = "woocommerce/woocommerce.php"
1549+
systemPlugin2.siteID = sampleSiteID + 1
1550+
1551+
let systemPlugin3 = storage.insertNewObject(ofType: SystemPlugin.self)
1552+
systemPlugin3.plugin = "wordpress-seo/wp-seo.php"
1553+
systemPlugin3.siteID = sampleSiteID
1554+
1555+
let systemPlugin4 = storage.insertNewObject(ofType: SystemPlugin.self)
1556+
systemPlugin4.plugin = "akismet/akismet.php"
1557+
systemPlugin4.siteID = sampleSiteID
1558+
1559+
// When
1560+
let storedSystemPlugins = storage.loadSystemPlugins(siteID: sampleSiteID, matchingPaths: ["woocommerce/woocommerce.php", "akismet/akismet.php"])
1561+
1562+
// Then
1563+
XCTAssertEqual(storedSystemPlugins.count, 2)
1564+
XCTAssertTrue(storedSystemPlugins.contains(systemPlugin1))
1565+
XCTAssertTrue(storedSystemPlugins.contains(systemPlugin4))
1566+
XCTAssertFalse(storedSystemPlugins.contains(systemPlugin2)) // Different siteID
1567+
XCTAssertFalse(storedSystemPlugins.contains(systemPlugin3)) // Not in matching paths
1568+
}
1569+
14691570
func test_load_WCPayCharge_by_siteID_and_chargeID() throws {
14701571
// Given
14711572
let charge1 = storage.insertNewObject(ofType: WCPayCharge.self)

0 commit comments

Comments
 (0)