From 40d5f5740a31b1844f48d1a33c309336baa0a03c Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 14 Jul 2025 10:33:29 -0400 Subject: [PATCH 1/8] Refresh plugin ineligible cases by making one system status API request. --- .../POS/POSTabEligibilityChecker.swift | 82 +++++++- .../POS/POSTabEligibilityCheckerTests.swift | 186 +++++++++++++++--- 2 files changed, 229 insertions(+), 39 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift index f22ce4799cf..044a2d12bc7 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift @@ -13,6 +13,8 @@ import enum Yosemite.FeatureFlagAction import enum Yosemite.SettingAction import protocol Yosemite.PluginsServiceProtocol import class Yosemite.PluginsService +import protocol Yosemite.POSSystemStatusServiceProtocol +import class Yosemite.POSSystemStatusService /// Represents the reasons why a site may be ineligible for POS. enum POSIneligibleReason: Equatable { @@ -51,6 +53,7 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { private let eligibilityService: POSEligibilityServiceProtocol private let stores: StoresManager private let featureFlagService: FeatureFlagService + private let systemStatusService: POSSystemStatusServiceProtocol init(siteID: Int64, userInterfaceIdiom: UIUserInterfaceIdiom = UIDevice.current.userInterfaceIdiom, @@ -58,7 +61,8 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { pluginsService: PluginsServiceProtocol = PluginsService(storageManager: ServiceLocator.storageManager), eligibilityService: POSEligibilityServiceProtocol = POSEligibilityService(), stores: StoresManager = ServiceLocator.stores, - featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) { + featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService, + systemStatusService: POSSystemStatusServiceProtocol = POSSystemStatusService(credentials: ServiceLocator.stores.sessionManager.defaultCredentials)) { self.siteID = siteID self.userInterfaceIdiom = userInterfaceIdiom self.siteSettings = siteSettings @@ -66,6 +70,7 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { self.eligibilityService = eligibilityService self.stores = stores self.featureFlagService = featureFlagService + self.systemStatusService = systemStatusService } /// Checks the initial visibility of the POS tab without dependance on network requests. @@ -75,12 +80,19 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { /// Determines whether the POS entry point can be shown based on the selected store and feature gates. func checkEligibility() async -> POSEligibilityState { + await checkEligibility(pluginEligibility: nil) + } + + /// Checks the eligibility for POS based on site settings and plugin eligibility. + /// - Parameter pluginEligibility: An optional parameter that can provide pre-fetched plugin eligibility state. + /// - Returns: The eligibility state for POS. + private func checkEligibility(pluginEligibility: POSEligibilityState?) async -> POSEligibilityState { guard #available(iOS 17.0, *) else { return .ineligible(reason: .unsupportedIOSVersion) } async let siteSettingsEligibility = checkSiteSettingsEligibility() - async let pluginEligibility = checkPluginEligibility() + async let pluginEligibility = checkPluginEligibility(pluginEligibility: pluginEligibility) switch await siteSettingsEligibility { case .eligible: @@ -131,9 +143,8 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { throw error } case .unsupportedWooCommerceVersion, .wooCommercePluginNotFound: - // TODO: WOOMOB-799 - sync the WooCommerce plugin then check eligibility again. - // For now, it requires relaunching the app or switching stores to refresh the plugin info. - return await checkEligibility() + let pluginEligibility = await refreshPluginEligibility() + return await checkEligibility(pluginEligibility: pluginEligibility) case .featureSwitchDisabled: // TODO: WOOMOB-759 - enable feature switch via API and check eligibility again // For now, just checks eligibility again. @@ -147,8 +158,59 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { // MARK: - WC Plugin Related Eligibility Check private extension POSTabEligibilityChecker { - func checkPluginEligibility() async -> POSEligibilityState { - let wcPlugin = await fetchWooCommercePlugin(siteID: siteID) + /// Checks the eligibility of the WooCommerce plugin and plugin version based POS feature switch value. + /// When a pre-fetched eligibility state is not provided, the plugin is the first matching WC plugin found in the storage for performance reason, which is + /// fetched remotely outside of the eligibility checker during site initialization. + /// The feature switch value is fetched remotely if the plugin version supports it. + /// + /// - Parameter pluginEligibility: An optional parameter that can provide pre-fetched plugin eligibility state. + /// - Returns: The eligibility state for POS based on the WooCommerce plugin and POS feature switch. + func checkPluginEligibility(pluginEligibility: POSEligibilityState? = nil) async -> POSEligibilityState { + if let pluginEligibility { + return pluginEligibility + } + async let wcPlugin = fetchWooCommercePlugin(siteID: siteID) + let wcPluginEligibility = checkWooCommercePluginEligibility(wcPlugin: await wcPlugin) + switch wcPluginEligibility { + case .eligible: + return .eligible + case .ineligible(let reason): + return .ineligible(reason: reason) + case .pendingFeatureSwitchCheck: + return await checkFeatureSwitchRemotely(siteID: siteID) + } + } + + /// Refreshes the eligibility state of the WooCommerce plugin and POS feature switch by making a system status API request. + /// - Returns: The eligibility state for POS based on the WooCommerce plugin and POS feature switch from remote sync. + func refreshPluginEligibility() async -> POSEligibilityState { + do { + async let info = systemStatusService.loadWooCommercePluginAndPOSFeatureSwitch(siteID: siteID) + let wcPluginEligibility = checkWooCommercePluginEligibility(wcPlugin: try await info.wcPlugin) + switch wcPluginEligibility { + case .eligible: + return .eligible + case .ineligible(let reason): + return .ineligible(reason: reason) + case .pendingFeatureSwitchCheck: + let isFeatureSwitchEnabled = try await info.featureValue == true + return isFeatureSwitchEnabled ? .eligible : .ineligible(reason: .featureSwitchDisabled) + } + } catch { + return .ineligible(reason: .wooCommercePluginNotFound) + } + } + + enum PluginEligibilityState { + case eligible + case ineligible(reason: POSIneligibleReason) + case pendingFeatureSwitchCheck + } + + func checkWooCommercePluginEligibility(wcPlugin: SystemPlugin?) -> PluginEligibilityState { + guard let wcPlugin else { + return .ineligible(reason: .wooCommercePluginNotFound) + } guard VersionHelpers.isVersionSupported(version: wcPlugin.version, minimumRequired: Constants.wcPluginMinimumVersion) else { @@ -163,8 +225,8 @@ private extension POSTabEligibilityChecker { return .eligible } - // For versions that support the feature switch, checks if the feature switch is enabled. - return await checkFeatureSwitchEnabled(siteID: siteID) + // For versions that support the feature switch, checks if the feature switch is enabled separately. + return .pendingFeatureSwitchCheck } @MainActor @@ -173,7 +235,7 @@ private extension POSTabEligibilityChecker { } @MainActor - func checkFeatureSwitchEnabled(siteID: Int64) async -> POSEligibilityState { + func checkFeatureSwitchRemotely(siteID: Int64) async -> POSEligibilityState { await withCheckedContinuation { [weak self] continuation in guard let self else { return continuation.resume(returning: .ineligible(reason: .selfDeallocated)) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift index d6c16344124..276d4a058aa 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift @@ -12,6 +12,7 @@ struct POSTabEligibilityCheckerTests { private var siteSettings: MockSelectedSiteSettings! private var pluginsService: MockPluginsService! private var eligibilityService: MockPOSEligibilityService! + private var mockSystemStatusService: MockPOSSystemStatusService! private let siteID: Int64 = 2 init() async throws { @@ -22,6 +23,7 @@ struct POSTabEligibilityCheckerTests { eligibilityService = MockPOSEligibilityService() setupWooCommerceVersion() siteSettings = MockSelectedSiteSettings() + mockSystemStatusService = MockPOSSystemStatusService() } // MARK: `checkVisibility` @@ -656,10 +658,11 @@ struct POSTabEligibilityCheckerTests { #expect(syncCalled == true) // Called during the attempt } - @Test func refreshEligibility_checks_eligibility_for_unsupportedWooCommerceVersion() async throws { + @Test func refreshEligibility_checks_eligibility_for_featureSwitchDisabled() async throws { // Given setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("9.5.0") // Still below minimum + setupWooCommerceVersion("10.0.0") // Version that supports feature switch + setupPOSFeatureEnabled(.success(true)) // Now enabled let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, @@ -667,16 +670,34 @@ struct POSTabEligibilityCheckerTests { stores: stores) // When - let result = try await checker.refreshEligibility(ineligibleReason: .unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta")) + let result = try await checker.refreshEligibility(ineligibleReason: .featureSwitchDisabled) - // Then - Should check eligibility again (still ineligible due to version) - #expect(result == .ineligible(reason: .unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"))) + // Then - Should check eligibility again (now eligible) + #expect(result == .eligible) } - @Test func refreshEligibility_checks_eligibility_for_wooCommercePluginNotFound() async throws { + @Test func refreshEligibility_checks_eligibility_for_featureSwitchSyncFailure() async throws { // Given setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("9.6.0") // Now eligible version + setupWooCommerceVersion("10.0.0") + setupPOSFeatureEnabled(.failure(NSError(domain: "test", code: 0))) // Still failing + + let checker = POSTabEligibilityChecker(siteID: siteID, + siteSettings: siteSettings, + pluginsService: pluginsService, + stores: stores) + + // When + let result = try await checker.refreshEligibility(ineligibleReason: .featureSwitchSyncFailure) + + // Then - Should check eligibility again (still failing) + #expect(result == .ineligible(reason: .featureSwitchSyncFailure)) + } + + @Test func refreshEligibility_checks_eligibility_for_selfDeallocated() async throws { + // Given + setupCountry(country: .us, currency: .USD) + setupWooCommerceVersion("9.6.0") setupPOSFeatureEnabled(.success(true)) let checker = POSTabEligibilityChecker(siteID: siteID, @@ -685,64 +706,154 @@ struct POSTabEligibilityCheckerTests { stores: stores) // When - let result = try await checker.refreshEligibility(ineligibleReason: .wooCommercePluginNotFound) + let result = try await checker.refreshEligibility(ineligibleReason: .selfDeallocated) // Then - Should check eligibility again (now eligible) #expect(result == .eligible) } - @Test func refreshEligibility_checks_eligibility_for_featureSwitchDisabled() async throws { + // MARK: - `refreshEligibility` with System Status Service Tests + + @Test(arguments: [ + POSIneligibleReason.unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"), + POSIneligibleReason.wooCommercePluginNotFound + ]) + func refreshEligibility_returns_eligible_when_plugin_refreshed_with_valid_version_below_10(ineligibleReason: POSIneligibleReason) async throws { // Given + let mockSystemStatusService = MockPOSSystemStatusService() + let wcPlugin = createWooCommercePlugin(version: "9.9.9") // Valid version below feature switch threshold + mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: nil)) + setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("10.0.0") // Version that supports feature switch - setupPOSFeatureEnabled(.success(true)) // Now enabled + let checker = POSTabEligibilityChecker(siteID: siteID, + siteSettings: siteSettings, + pluginsService: pluginsService, + stores: stores, + systemStatusService: mockSystemStatusService) + + // When + let result = try await checker.refreshEligibility(ineligibleReason: ineligibleReason) + + // Then + #expect(result == .eligible) + } + @Test(arguments: [ + POSIneligibleReason.unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"), + POSIneligibleReason.wooCommercePluginNotFound + ]) + fileprivate func refreshEligibility_returns_eligible_when_plugin_with_version_10_and_feature_enabled(ineligibleReason: POSIneligibleReason) async throws { + // Given + let mockSystemStatusService = MockPOSSystemStatusService() + let wcPlugin = createWooCommercePlugin(version: "10.0.0") + mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: true)) + + setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, pluginsService: pluginsService, - stores: stores) + stores: stores, + systemStatusService: mockSystemStatusService) // When - let result = try await checker.refreshEligibility(ineligibleReason: .featureSwitchDisabled) + let result = try await checker.refreshEligibility(ineligibleReason: ineligibleReason) - // Then - Should check eligibility again (now eligible) + // Then #expect(result == .eligible) } - @Test func refreshEligibility_checks_eligibility_for_featureSwitchSyncFailure() async throws { + @Test(arguments: [ + POSIneligibleReason.unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"), + POSIneligibleReason.wooCommercePluginNotFound + ]) + fileprivate func refreshEligibility_returns_ineligible_when_plugin_not_found_in_system_status(ineligibleReason: POSIneligibleReason) async throws { // Given + let mockSystemStatusService = MockPOSSystemStatusService() + mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: nil, featureValue: nil)) + setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("10.0.0") - setupPOSFeatureEnabled(.failure(NSError(domain: "test", code: 0))) // Still failing + let checker = POSTabEligibilityChecker(siteID: siteID, + siteSettings: siteSettings, + pluginsService: pluginsService, + stores: stores, + systemStatusService: mockSystemStatusService) + + // When + let result = try await checker.refreshEligibility(ineligibleReason: ineligibleReason) + + // Then + #expect(result == .ineligible(reason: .wooCommercePluginNotFound)) + } + + @Test(arguments: [ + POSIneligibleReason.unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"), + POSIneligibleReason.wooCommercePluginNotFound + ]) + fileprivate func refreshEligibility_returns_ineligible_when_plugin_version_still_below_minimum(ineligibleReason: POSIneligibleReason) async throws { + // Given + let mockSystemStatusService = MockPOSSystemStatusService() + let wcPlugin = createWooCommercePlugin(version: "9.5.0") // Still below minimum 9.6.0-beta + mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: nil)) + setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, pluginsService: pluginsService, - stores: stores) + stores: stores, + systemStatusService: mockSystemStatusService) // When - let result = try await checker.refreshEligibility(ineligibleReason: .featureSwitchSyncFailure) + let result = try await checker.refreshEligibility(ineligibleReason: ineligibleReason) - // Then - Should check eligibility again (still failing) - #expect(result == .ineligible(reason: .featureSwitchSyncFailure)) + // Then + #expect(result == .ineligible(reason: .unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"))) } - @Test func refreshEligibility_checks_eligibility_for_selfDeallocated() async throws { + @Test(arguments: [ + POSIneligibleReason.unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"), + POSIneligibleReason.wooCommercePluginNotFound + ]) + fileprivate func refreshEligibility_returns_ineligible_when_feature_switch_still_disabled(ineligibleReason: POSIneligibleReason) async throws { // Given + let mockSystemStatusService = MockPOSSystemStatusService() + let wcPlugin = createWooCommercePlugin(version: "10.0.0") + mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: nil)) + setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("9.6.0") - setupPOSFeatureEnabled(.success(true)) + let checker = POSTabEligibilityChecker(siteID: siteID, + siteSettings: siteSettings, + pluginsService: pluginsService, + stores: stores, + systemStatusService: mockSystemStatusService) + + // When + let result = try await checker.refreshEligibility(ineligibleReason: ineligibleReason) + + // Then + #expect(result == .ineligible(reason: .featureSwitchDisabled)) + } + + @Test(arguments: [ + POSIneligibleReason.unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"), + POSIneligibleReason.wooCommercePluginNotFound + ]) + fileprivate func refreshEligibility_returns_ineligible_when_system_status_request_fails(ineligibleReason: POSIneligibleReason) async throws { + // Given + let mockSystemStatusService = MockPOSSystemStatusService() + mockSystemStatusService.resultToReturn = .failure(NSError(domain: "test", code: 500)) + setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, pluginsService: pluginsService, - stores: stores) + stores: stores, + systemStatusService: mockSystemStatusService) // When - let result = try await checker.refreshEligibility(ineligibleReason: .selfDeallocated) + let result = try await checker.refreshEligibility(ineligibleReason: ineligibleReason) - // Then - Should check eligibility again (now eligible) - #expect(result == .eligible) + // Then + #expect(result == .ineligible(reason: .wooCommercePluginNotFound)) } } @@ -759,7 +870,11 @@ private extension POSTabEligibilityCheckerTests { } func setupWooCommerceVersion(_ version: String = "9.6.0-beta") { - pluginsService.pluginToReturn = .fake().copy( + pluginsService.pluginToReturn = createWooCommercePlugin(version: version) + } + + func createWooCommercePlugin(version: String) -> SystemPlugin { + SystemPlugin.fake().copy( siteID: siteID, plugin: "woocommerce/woocommerce.php", version: version, @@ -839,3 +954,16 @@ private final class MockSelectedSiteSettings: SelectedSiteSettingsProtocol { // Mock implementation - no action needed. } } + +private final class MockPOSSystemStatusService: POSSystemStatusServiceProtocol { + var resultToReturn: Result = .success(POSPluginAndFeatureInfo(wcPlugin: nil, featureValue: nil)) + + func loadWooCommercePluginAndPOSFeatureSwitch(siteID: Int64) async throws -> POSPluginAndFeatureInfo { + switch resultToReturn { + case .success(let info): + return info + case .failure(let error): + throw error + } + } +} From d6116a76592f103627c0770f7168b36920160cf2 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 14 Jul 2025 10:36:13 -0400 Subject: [PATCH 2/8] Use `LegacyPOSTabEligibilityChecker` in HubMenu entry point when `pointOfSaleOrdersi1` is disabled (it's currently enabled in production). Will be removed when `pointOfSaleOrdersi1` feature flag is removed. --- WooCommerce/Classes/ViewRelated/Hub Menu/HubMenu.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/ViewRelated/Hub Menu/HubMenu.swift b/WooCommerce/Classes/ViewRelated/Hub Menu/HubMenu.swift index 8394022eb2b..4a9d96886c5 100644 --- a/WooCommerce/Classes/ViewRelated/Hub Menu/HubMenu.swift +++ b/WooCommerce/Classes/ViewRelated/Hub Menu/HubMenu.swift @@ -65,7 +65,7 @@ struct HubMenu: View { itemProvider: PointOfSaleItemService(currencySettings: ServiceLocator.currencySettings), itemFetchStrategyFactory: viewModel.posPopularItemFetchStrategyFactory), barcodeScanService: viewModel.barcodeScanService, - posEligibilityChecker: POSTabEligibilityChecker(siteID: viewModel.siteID)) + posEligibilityChecker: LegacyPOSTabEligibilityChecker(siteID: viewModel.siteID)) } else { // TODO: When we have a singleton for the card payment service, this should not be required. Text("Error creating card payment service") From fd0462d4f96f26596235aeafc5c9c007a94345e9 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 14 Jul 2025 11:47:03 -0400 Subject: [PATCH 3/8] Update `POSSystemStatusService` to also fetch inactive plugins and upsert all plugins to storage so that later use cases have the latest value. --- .../Protocols/StorageManagerType.swift | 13 ++++ .../Eligibility/POSSystemStatusService.swift | 60 ++++++++++++++++--- ...us-wc-plugin-and-pos-feature-disabled.json | 1 + ...tus-wc-plugin-and-pos-feature-enabled.json | 1 + .../system-status-wc-plugin-missing.json | 1 + .../POSSystemStatusServiceTests.swift | 30 +++++++++- .../POS/POSTabEligibilityChecker.swift | 5 +- 7 files changed, 100 insertions(+), 11 deletions(-) diff --git a/Modules/Sources/Storage/Protocols/StorageManagerType.swift b/Modules/Sources/Storage/Protocols/StorageManagerType.swift index 7c996d22654..bf858470d5c 100644 --- a/Modules/Sources/Storage/Protocols/StorageManagerType.swift +++ b/Modules/Sources/Storage/Protocols/StorageManagerType.swift @@ -57,4 +57,17 @@ public extension StorageManagerType { func reset() { reset(onCompletion: nil) } + + /// Async/await version of `performAndSave`. + /// + /// - Parameters: + /// - operation: A closure which uses the given `StorageType` to make data changes in background. + /// - queue: A queue on which to execute the completion closure. + func performAndSaveAsync(_ operation: @escaping (StorageType) -> Void, on queue: DispatchQueue = .main) async { + await withCheckedContinuation { continuation in + performAndSave(operation, completion: { + continuation.resume() + }, on: queue) + } + } } diff --git a/Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift b/Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift index ee4c09b8979..d496d2068a8 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift +++ b/Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift @@ -1,5 +1,6 @@ import Foundation import Networking +import Storage public protocol POSSystemStatusServiceProtocol { /// Loads WooCommerce plugin and POS feature switch value remotely for eligibility checks. @@ -23,27 +24,36 @@ public struct POSPluginAndFeatureInfo { /// Service for fetching POS-related system status information. public final class POSSystemStatusService: POSSystemStatusServiceProtocol { private let remote: SystemStatusRemote + private let storageManager: StorageManagerType - public init(credentials: Credentials?) { + public init(credentials: Credentials?, storageManager: StorageManagerType) { let network = AlamofireNetwork(credentials: credentials) - remote = SystemStatusRemote(network: network) + self.remote = SystemStatusRemote(network: network) + self.storageManager = storageManager } /// Test-friendly initializer that accepts a network implementation. - init(network: Network) { - remote = SystemStatusRemote(network: network) + init(network: Network, storageManager: StorageManagerType) { + self.remote = SystemStatusRemote(network: network) + self.storageManager = storageManager } + @MainActor public func loadWooCommercePluginAndPOSFeatureSwitch(siteID: Int64) async throws -> POSPluginAndFeatureInfo { let mapper = SingleItemMapper(siteID: siteID) let systemStatus: POSPluginEligibilitySystemStatus = try await remote.loadSystemStatus( for: siteID, - fields: [.activePlugins, .settings], + fields: [.activePlugins, .inactivePlugins, .settings], mapper: mapper ) - // Finds WooCommerce plugin from active plugins response. - guard let wcPlugin = systemStatus.activePlugins.first(where: { $0.plugin == Constants.wcPluginPath }) else { + // Upserts all plugins in storage. + await storageManager.performAndSaveAsync({ [weak self] storage in + self?.upsertSystemPlugins(siteID: siteID, systemStatus: systemStatus, in: storage) + }) + + // Loads WooCommerce plugin from storage. + guard let wcPlugin = storageManager.viewStorage.loadSystemPlugin(siteID: siteID, path: Constants.wcPluginPath)?.toReadOnly() else { return POSPluginAndFeatureInfo(wcPlugin: nil, featureValue: nil) } @@ -57,16 +67,52 @@ private extension POSSystemStatusService { enum Constants { static let wcPluginPath = "woocommerce/woocommerce.php" } + + /// Updates or inserts system plugins in storage. + func upsertSystemPlugins(siteID: Int64, systemStatus: POSPluginEligibilitySystemStatus, in storage: StorageType) { + // Active and inactive plugins share identical structure, but are stored in separate parts of the remote response + // (and without an active attribute in the response). So we apply the correct value for active (or not) + let readonlySystemPlugins: [SystemPlugin] = { + let activePlugins = systemStatus.activePlugins.map { + $0.copy(active: true) + } + + let inactivePlugins = systemStatus.inactivePlugins.map { + $0.copy(active: false) + } + + return activePlugins + inactivePlugins + }() + + let storedPlugins = storage.loadSystemPlugins(siteID: siteID, matching: readonlySystemPlugins.map { $0.name }) + readonlySystemPlugins.forEach { readonlySystemPlugin in + // Loads or creates new StorageSystemPlugin matching the readonly one. + let storageSystemPlugin: StorageSystemPlugin = { + if let systemPlugin = storedPlugins.first(where: { $0.name == readonlySystemPlugin.name }) { + return systemPlugin + } + return storage.insertNewObject(ofType: StorageSystemPlugin.self) + }() + + storageSystemPlugin.update(with: readonlySystemPlugin) + } + + // Removes stale system plugins. + let currentSystemPlugins = readonlySystemPlugins.map(\.name) + storage.deleteStaleSystemPlugins(siteID: siteID, currentSystemPlugins: currentSystemPlugins) + } } // MARK: - Network Response Structs private struct POSPluginEligibilitySystemStatus: Decodable { let activePlugins: [SystemPlugin] + let inactivePlugins: [SystemPlugin] let settings: POSEligibilitySystemStatusSettings enum CodingKeys: String, CodingKey { case activePlugins = "active_plugins" + case inactivePlugins = "inactive_plugins" case settings } } diff --git a/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-and-pos-feature-disabled.json b/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-and-pos-feature-disabled.json index 146bdf85529..77dd7476873 100644 --- a/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-and-pos-feature-disabled.json +++ b/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-and-pos-feature-disabled.json @@ -1,5 +1,6 @@ { "data": { + "inactive_plugins": [], "active_plugins":[ { "plugin": "woocommerce/woocommerce.php", diff --git a/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-and-pos-feature-enabled.json b/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-and-pos-feature-enabled.json index 72c3e66d6cf..cae5047619e 100644 --- a/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-and-pos-feature-enabled.json +++ b/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-and-pos-feature-enabled.json @@ -1,5 +1,6 @@ { "data": { + "inactive_plugins": [], "active_plugins":[ { "plugin": "woocommerce/woocommerce.php", diff --git a/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-missing.json b/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-missing.json index 2bfadb84427..1d961aae24e 100644 --- a/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-missing.json +++ b/Modules/Tests/NetworkingTests/Responses/system-status-wc-plugin-missing.json @@ -1,5 +1,6 @@ { "data": { + "inactive_plugins": [], "active_plugins":[ ], "settings": { diff --git a/Modules/Tests/YosemiteTests/PointOfSale/POSSystemStatusServiceTests.swift b/Modules/Tests/YosemiteTests/PointOfSale/POSSystemStatusServiceTests.swift index aa769b933ab..82edceade1e 100644 --- a/Modules/Tests/YosemiteTests/PointOfSale/POSSystemStatusServiceTests.swift +++ b/Modules/Tests/YosemiteTests/PointOfSale/POSSystemStatusServiceTests.swift @@ -2,22 +2,24 @@ import Foundation import Testing import TestKit @testable import Networking +@testable import Storage @testable import Yosemite @MainActor struct POSSystemStatusServiceTests { private let network = MockNetwork() + private let storageManager = MockStorageManager() private let sampleSiteID: Int64 = 134 private let sut: POSSystemStatusService init() async throws { network.removeAllSimulatedResponses() - sut = POSSystemStatusService(network: network) + sut = POSSystemStatusService(network: network, storageManager: storageManager) } // MARK: - loadWooCommercePluginAndPOSFeatureSwitch Tests - @Test func loadWooCommercePluginAndPOSFeatureSwitch_returns_plugin_and_nil_feature_when_settings_response_does_not_include_enabled_featuers() async throws { + @Test func loadWooCommercePluginAndPOSFeatureSwitch_returns_plugin_and_nil_feature_when_settings_response_does_not_include_enabled_features() async throws { // Given network.simulateResponse(requestUrlSuffix: "system_status", filename: "systemStatus") @@ -37,6 +39,10 @@ struct POSSystemStatusServiceTests { // Given network.simulateResponse(requestUrlSuffix: "system_status", filename: "system-status-wc-plugin-and-pos-feature-enabled") + // Inserts WooCommerce plugin into storage with an older version and inactive. + let storageWCPlugin = createWCPlugin(version: "9.5.2", active: false) + storageManager.insertSampleSystemPlugin(readOnlySystemPlugin: storageWCPlugin) + // When let result = try await sut.loadWooCommercePluginAndPOSFeatureSwitch(siteID: sampleSiteID) @@ -53,6 +59,10 @@ struct POSSystemStatusServiceTests { // Given network.simulateResponse(requestUrlSuffix: "system_status", filename: "system-status-wc-plugin-and-pos-feature-disabled") + // Inserts WooCommerce plugin into storage with an older version and inactive. + let storageWCPlugin = createWCPlugin(version: "9.5.2", active: false) + storageManager.insertSampleSystemPlugin(readOnlySystemPlugin: storageWCPlugin) + // When let result = try await sut.loadWooCommercePluginAndPOSFeatureSwitch(siteID: sampleSiteID) @@ -69,6 +79,10 @@ struct POSSystemStatusServiceTests { // Given network.simulateResponse(requestUrlSuffix: "system_status", filename: "system-status-wc-plugin-missing") + // Inserts WooCommerce plugin eligible for POS into storage. + let storageWCPlugin = createWCPlugin(version: "9.9.0", active: true) + storageManager.insertSampleSystemPlugin(readOnlySystemPlugin: storageWCPlugin) + // When let result = try await sut.loadWooCommercePluginAndPOSFeatureSwitch(siteID: sampleSiteID) @@ -87,3 +101,15 @@ struct POSSystemStatusServiceTests { } } } + +private extension POSSystemStatusServiceTests { + func createWCPlugin(version: String = "5.8.0", active: Bool = true) -> Yosemite.SystemPlugin { + .fake().copy( + siteID: sampleSiteID, + plugin: "woocommerce/woocommerce.php", + version: version, + versionLatest: version, + active: active + ) + } +} diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift index 044a2d12bc7..b7b3d972506 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift @@ -62,7 +62,8 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { eligibilityService: POSEligibilityServiceProtocol = POSEligibilityService(), stores: StoresManager = ServiceLocator.stores, featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService, - systemStatusService: POSSystemStatusServiceProtocol = POSSystemStatusService(credentials: ServiceLocator.stores.sessionManager.defaultCredentials)) { + systemStatusService: POSSystemStatusServiceProtocol = POSSystemStatusService(credentials: ServiceLocator.stores.sessionManager.defaultCredentials, + storageManager: ServiceLocator.storageManager)) { self.siteID = siteID self.userInterfaceIdiom = userInterfaceIdiom self.siteSettings = siteSettings @@ -208,7 +209,7 @@ private extension POSTabEligibilityChecker { } func checkWooCommercePluginEligibility(wcPlugin: SystemPlugin?) -> PluginEligibilityState { - guard let wcPlugin else { + guard let wcPlugin, wcPlugin.active else { return .ineligible(reason: .wooCommercePluginNotFound) } From 2f26bc29bb8572b2b38e62e726dbe87dffebf1f1 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 14 Jul 2025 13:12:47 -0400 Subject: [PATCH 4/8] Nit: fix lint error. --- Modules/Sources/Storage/Protocols/StorageManagerType.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/Sources/Storage/Protocols/StorageManagerType.swift b/Modules/Sources/Storage/Protocols/StorageManagerType.swift index bf858470d5c..4d83d4582b5 100644 --- a/Modules/Sources/Storage/Protocols/StorageManagerType.swift +++ b/Modules/Sources/Storage/Protocols/StorageManagerType.swift @@ -57,7 +57,7 @@ public extension StorageManagerType { func reset() { reset(onCompletion: nil) } - + /// Async/await version of `performAndSave`. /// /// - Parameters: From dd73d5d103c7d5f85e8e86b7d5bd9a36ad2a7516 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 14 Jul 2025 14:16:19 -0400 Subject: [PATCH 5/8] Always make system status API request for plugin and feature switch to fix an issue where the plugin from storage is outdated before it is synced externally with a tradeoff of an extra API request when POS tab is tapped for the first time. --- .../POS/POSTabEligibilityChecker.swift | 64 ++----------------- 1 file changed, 6 insertions(+), 58 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift index b7b3d972506..0f9685c9ca9 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift @@ -81,19 +81,12 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { /// Determines whether the POS entry point can be shown based on the selected store and feature gates. func checkEligibility() async -> POSEligibilityState { - await checkEligibility(pluginEligibility: nil) - } - - /// Checks the eligibility for POS based on site settings and plugin eligibility. - /// - Parameter pluginEligibility: An optional parameter that can provide pre-fetched plugin eligibility state. - /// - Returns: The eligibility state for POS. - private func checkEligibility(pluginEligibility: POSEligibilityState?) async -> POSEligibilityState { guard #available(iOS 17.0, *) else { return .ineligible(reason: .unsupportedIOSVersion) } async let siteSettingsEligibility = checkSiteSettingsEligibility() - async let pluginEligibility = checkPluginEligibility(pluginEligibility: pluginEligibility) + async let pluginEligibility = checkPluginEligibility() switch await siteSettingsEligibility { case .eligible: @@ -144,8 +137,7 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { throw error } case .unsupportedWooCommerceVersion, .wooCommercePluginNotFound: - let pluginEligibility = await refreshPluginEligibility() - return await checkEligibility(pluginEligibility: pluginEligibility) + return await checkEligibility() case .featureSwitchDisabled: // TODO: WOOMOB-759 - enable feature switch via API and check eligibility again // For now, just checks eligibility again. @@ -160,41 +152,20 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { private extension POSTabEligibilityChecker { /// Checks the eligibility of the WooCommerce plugin and plugin version based POS feature switch value. - /// When a pre-fetched eligibility state is not provided, the plugin is the first matching WC plugin found in the storage for performance reason, which is - /// fetched remotely outside of the eligibility checker during site initialization. - /// The feature switch value is fetched remotely if the plugin version supports it. /// /// - Parameter pluginEligibility: An optional parameter that can provide pre-fetched plugin eligibility state. /// - Returns: The eligibility state for POS based on the WooCommerce plugin and POS feature switch. - func checkPluginEligibility(pluginEligibility: POSEligibilityState? = nil) async -> POSEligibilityState { - if let pluginEligibility { - return pluginEligibility - } - async let wcPlugin = fetchWooCommercePlugin(siteID: siteID) - let wcPluginEligibility = checkWooCommercePluginEligibility(wcPlugin: await wcPlugin) - switch wcPluginEligibility { - case .eligible: - return .eligible - case .ineligible(let reason): - return .ineligible(reason: reason) - case .pendingFeatureSwitchCheck: - return await checkFeatureSwitchRemotely(siteID: siteID) - } - } - - /// Refreshes the eligibility state of the WooCommerce plugin and POS feature switch by making a system status API request. - /// - Returns: The eligibility state for POS based on the WooCommerce plugin and POS feature switch from remote sync. - func refreshPluginEligibility() async -> POSEligibilityState { + func checkPluginEligibility() async -> POSEligibilityState { do { - async let info = systemStatusService.loadWooCommercePluginAndPOSFeatureSwitch(siteID: siteID) - let wcPluginEligibility = checkWooCommercePluginEligibility(wcPlugin: try await info.wcPlugin) + let info = try await systemStatusService.loadWooCommercePluginAndPOSFeatureSwitch(siteID: siteID) + let wcPluginEligibility = checkWooCommercePluginEligibility(wcPlugin: info.wcPlugin) switch wcPluginEligibility { case .eligible: return .eligible case .ineligible(let reason): return .ineligible(reason: reason) case .pendingFeatureSwitchCheck: - let isFeatureSwitchEnabled = try await info.featureValue == true + let isFeatureSwitchEnabled = info.featureValue == true return isFeatureSwitchEnabled ? .eligible : .ineligible(reason: .featureSwitchDisabled) } } catch { @@ -229,29 +200,6 @@ private extension POSTabEligibilityChecker { // For versions that support the feature switch, checks if the feature switch is enabled separately. return .pendingFeatureSwitchCheck } - - @MainActor - func fetchWooCommercePlugin(siteID: Int64) async -> SystemPlugin { - await pluginsService.waitForPluginInStorage(siteID: siteID, pluginPath: Constants.wcPlugin, isActive: true) - } - - @MainActor - func checkFeatureSwitchRemotely(siteID: Int64) async -> POSEligibilityState { - await withCheckedContinuation { [weak self] continuation in - guard let self else { - return continuation.resume(returning: .ineligible(reason: .selfDeallocated)) - } - let action = SettingAction.isFeatureEnabled(siteID: siteID, feature: .pointOfSale) { result in - switch result { - case .success(let isEnabled): - continuation.resume(returning: isEnabled ? .eligible : .ineligible(reason: .featureSwitchDisabled)) - case .failure: - continuation.resume(returning: .ineligible(reason: .featureSwitchSyncFailure)) - } - } - stores.dispatch(action) - } - } } // MARK: - Site Settings Related Eligibility Check From ff00f980f3c7573e4bd82ec3527f7f1543b2b443 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 14 Jul 2025 16:13:54 -0400 Subject: [PATCH 6/8] Remove `POSIneligibleReason.featureSwitchSyncFailure` now that POS feature switch loading is combined with WC plugin. Update test cases. --- .../POS/TabBar/POSIneligibleView.swift | 13 -- .../POS/POSTabEligibilityChecker.swift | 8 +- .../POS/POSTabEligibilityCheckerTests.swift | 160 ++++-------------- 3 files changed, 37 insertions(+), 144 deletions(-) diff --git a/WooCommerce/Classes/POS/TabBar/POSIneligibleView.swift b/WooCommerce/Classes/POS/TabBar/POSIneligibleView.swift index 84dea444012..979ea8d9b24 100644 --- a/WooCommerce/Classes/POS/TabBar/POSIneligibleView.swift +++ b/WooCommerce/Classes/POS/TabBar/POSIneligibleView.swift @@ -97,10 +97,6 @@ struct POSIneligibleView: View { value: "Point of Sale must be enabled to proceed. " + "Please enable the POS feature from your WordPress admin under WooCommerce settings > Advanced > Features.", comment: "Suggestion for disabled feature switch: enable feature in WooCommerce settings") - case .featureSwitchSyncFailure: - return NSLocalizedString("pos.ineligible.suggestion.featureSwitchSyncFailure", - value: "Please check your internet connection and try again.", - comment: "Suggestion for feature switch sync failure: check connection and retry") case let .unsupportedCurrency(supportedCurrencies): let currencyList = supportedCurrencies.map { $0.rawValue } let formattedCurrencyList = ListFormatter.localizedString(byJoining: currencyList) @@ -194,15 +190,6 @@ private extension POSIneligibleView { } } -#Preview("Feature switch sync failure") { - if #available(iOS 17.0, *) { - POSIneligibleView( - reason: .featureSwitchSyncFailure, - onRefresh: {} - ) - } -} - #Preview("Unsupported WooCommerce version") { if #available(iOS 17.0, *) { POSIneligibleView( diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift index 0f9685c9ca9..ffd8059c2cf 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift @@ -11,8 +11,6 @@ import class Yosemite.POSEligibilityService import struct Yosemite.SystemPlugin import enum Yosemite.FeatureFlagAction import enum Yosemite.SettingAction -import protocol Yosemite.PluginsServiceProtocol -import class Yosemite.PluginsService import protocol Yosemite.POSSystemStatusServiceProtocol import class Yosemite.POSSystemStatusService @@ -23,7 +21,6 @@ enum POSIneligibleReason: Equatable { case siteSettingsNotAvailable case wooCommercePluginNotFound case featureSwitchDisabled - case featureSwitchSyncFailure case unsupportedCurrency(supportedCurrencies: [CurrencyCode]) case selfDeallocated } @@ -49,7 +46,6 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { private let siteID: Int64 private let userInterfaceIdiom: UIUserInterfaceIdiom private let siteSettings: SelectedSiteSettingsProtocol - private let pluginsService: PluginsServiceProtocol private let eligibilityService: POSEligibilityServiceProtocol private let stores: StoresManager private let featureFlagService: FeatureFlagService @@ -58,7 +54,6 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { init(siteID: Int64, userInterfaceIdiom: UIUserInterfaceIdiom = UIDevice.current.userInterfaceIdiom, siteSettings: SelectedSiteSettingsProtocol = ServiceLocator.selectedSiteSettings, - pluginsService: PluginsServiceProtocol = PluginsService(storageManager: ServiceLocator.storageManager), eligibilityService: POSEligibilityServiceProtocol = POSEligibilityService(), stores: StoresManager = ServiceLocator.stores, featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService, @@ -67,7 +62,6 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { self.siteID = siteID self.userInterfaceIdiom = userInterfaceIdiom self.siteSettings = siteSettings - self.pluginsService = pluginsService self.eligibilityService = eligibilityService self.stores = stores self.featureFlagService = featureFlagService @@ -142,7 +136,7 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol { // TODO: WOOMOB-759 - enable feature switch via API and check eligibility again // For now, just checks eligibility again. return await checkEligibility() - case .featureSwitchSyncFailure, .selfDeallocated: + case .selfDeallocated: return await checkEligibility() } } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift index 276d4a058aa..65472054c82 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift @@ -10,7 +10,6 @@ struct POSTabEligibilityCheckerTests { private var stores: MockStoresManager! private var storageManager: MockStorageManager! private var siteSettings: MockSelectedSiteSettings! - private var pluginsService: MockPluginsService! private var eligibilityService: MockPOSEligibilityService! private var mockSystemStatusService: MockPOSSystemStatusService! private let siteID: Int64 = 2 @@ -19,11 +18,10 @@ struct POSTabEligibilityCheckerTests { stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true)) stores.updateDefaultStore(storeID: siteID) storageManager = MockStorageManager() - pluginsService = MockPluginsService() eligibilityService = MockPOSEligibilityService() - setupWooCommerceVersion() siteSettings = MockSelectedSiteSettings() mockSystemStatusService = MockPOSSystemStatusService() + setupWooCommerceVersion() } // MARK: `checkVisibility` @@ -40,7 +38,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -63,7 +60,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -89,7 +85,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -110,7 +105,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -126,12 +120,10 @@ struct POSTabEligibilityCheckerTests { let featureFlagService = MockFeatureFlagService(isPointOfSaleAsATabi2Enabled: true) setupCountry(country: .us) accountWhitelistedInBackend(true) - setupWooCommerceVersion("10.0.0") - setupPOSFeatureEnabled(.success(true)) + setupWooCommerceVersion("10.0.0", featureSwitchEnabled: true) let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -148,12 +140,10 @@ struct POSTabEligibilityCheckerTests { let featureFlagService = MockFeatureFlagService(isPointOfSaleAsATabi2Enabled: true) setupCountry(country: .us) accountWhitelistedInBackend(true) - setupWooCommerceVersion("10.0.0") - setupPOSFeatureEnabled(.success(false)) + setupWooCommerceVersion("10.0.0", featureSwitchEnabled: false) let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -170,12 +160,10 @@ struct POSTabEligibilityCheckerTests { let featureFlagService = MockFeatureFlagService(isPointOfSaleAsATabi2Enabled: true) setupCountry(country: .us) accountWhitelistedInBackend(true) - setupWooCommerceVersion("10.0.0") - setupPOSFeatureEnabled(.failure(NSError(domain: "test", code: 0))) + setupWooCommerceVersion("10.0.0", featureSwitchEnabled: nil) let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -191,12 +179,10 @@ struct POSTabEligibilityCheckerTests { let featureFlagService = MockFeatureFlagService(isPointOfSaleAsATabi2Enabled: true) setupCountry(country: .us) accountWhitelistedInBackend(true) - setupWooCommerceVersion("9.9.9") - setupPOSFeatureEnabled(.success(false)) + setupWooCommerceVersion("9.9.9", featureSwitchEnabled: false) let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -232,12 +218,13 @@ struct POSTabEligibilityCheckerTests { ].publisher.eraseToAnyPublisher() accountWhitelistedInBackend(true) + setupWooCommerceVersion("9.6.0") let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, - featureFlagService: featureFlagService) + featureFlagService: featureFlagService, + systemStatusService: mockSystemStatusService) // When let result = await checker.checkEligibility() @@ -254,7 +241,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -290,7 +276,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -309,7 +294,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .phone, // Not iPad siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -331,12 +315,13 @@ struct POSTabEligibilityCheckerTests { let settingsSubject = PassthroughSubject<(siteID: Int64, settings: [SiteSetting], source: SettingsUpdateSource), Never>() siteSettings.mockSettingsStream = settingsSubject.eraseToAnyPublisher() + setupWooCommerceVersion("9.6.0") let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, - featureFlagService: featureFlagService) + featureFlagService: featureFlagService, + systemStatusService: mockSystemStatusService) // When - Call checkVisibility and checkEligibility concurrently before site settings are available async let visibilityTask = checker.checkVisibility() @@ -408,9 +393,9 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, - featureFlagService: featureFlagService) + featureFlagService: featureFlagService, + systemStatusService: mockSystemStatusService) // When let result = await checker.checkEligibility() @@ -431,7 +416,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -458,7 +442,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, featureFlagService: featureFlagService) @@ -478,9 +461,9 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, - featureFlagService: featureFlagService) + featureFlagService: featureFlagService, + systemStatusService: mockSystemStatusService) // When let result = await checker.checkEligibility() @@ -494,14 +477,13 @@ struct POSTabEligibilityCheckerTests { let featureFlagService = MockFeatureFlagService(isPointOfSaleAsATabi2Enabled: true) setupCountry(country: .us) accountWhitelistedInBackend(true) - setupWooCommerceVersion("10.0.0") - setupPOSFeatureEnabled(.success(true)) + setupWooCommerceVersion("10.0.0", featureSwitchEnabled: true) let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, - featureFlagService: featureFlagService) + featureFlagService: featureFlagService, + systemStatusService: mockSystemStatusService) // When let result = await checker.checkEligibility() @@ -515,14 +497,13 @@ struct POSTabEligibilityCheckerTests { let featureFlagService = MockFeatureFlagService(isPointOfSaleAsATabi2Enabled: true) setupCountry(country: .us) accountWhitelistedInBackend(true) - setupWooCommerceVersion("10.0.0") - setupPOSFeatureEnabled(.success(false)) + setupWooCommerceVersion("10.0.0", featureSwitchEnabled: false) let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, - featureFlagService: featureFlagService) + featureFlagService: featureFlagService, + systemStatusService: mockSystemStatusService) // When let result = await checker.checkEligibility() @@ -531,40 +512,18 @@ struct POSTabEligibilityCheckerTests { #expect(result == .ineligible(reason: .featureSwitchDisabled)) } - func is_ineligible_when_core_version_is_10_0_0_and_POS_feature_check_fails() async throws { - // Given - let featureFlagService = MockFeatureFlagService(isPointOfSaleAsATabi2Enabled: true) - setupCountry(country: .us) - accountWhitelistedInBackend(true) - setupWooCommerceVersion("10.0.0") - setupPOSFeatureEnabled(.failure(NSError(domain: "test", code: 0))) - let checker = POSTabEligibilityChecker(siteID: siteID, - userInterfaceIdiom: .pad, - siteSettings: siteSettings, - pluginsService: pluginsService, - stores: stores, - featureFlagService: featureFlagService) - - // When - let result = await checker.checkEligibility() - - // Then - #expect(result == .ineligible(reason: .featureSwitchSyncFailure)) - } - func is_eligible_when_core_version_is_below_10_0_0_and_POS_feature_disabled() async throws { // Given let featureFlagService = MockFeatureFlagService(isPointOfSaleAsATabi2Enabled: true) setupCountry(country: .us) accountWhitelistedInBackend(true) - setupWooCommerceVersion("9.9.9") - setupPOSFeatureEnabled(.success(false)) + setupWooCommerceVersion("9.9.9", featureSwitchEnabled: false) let checker = POSTabEligibilityChecker(siteID: siteID, userInterfaceIdiom: .pad, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, - featureFlagService: featureFlagService) + featureFlagService: featureFlagService, + systemStatusService: mockSystemStatusService) // When let result = await checker.checkEligibility() @@ -579,7 +538,6 @@ struct POSTabEligibilityCheckerTests { // Given let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores) // When @@ -596,8 +554,7 @@ struct POSTabEligibilityCheckerTests { fileprivate func refreshEligibility_syncs_site_settings_and_checks_eligibility_for_site_settings_issues(ineligibleReason: POSIneligibleReason) async throws { // Given setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("9.6.0") - setupPOSFeatureEnabled(.success(true)) + setupWooCommerceVersion("9.6.0", featureSwitchEnabled: true) var syncCalled = false stores.whenReceivingAction(ofType: SettingAction.self) { action in @@ -614,8 +571,8 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, - stores: stores) + stores: stores, + systemStatusService: mockSystemStatusService) // When let result = try await checker.refreshEligibility(ineligibleReason: ineligibleReason) @@ -647,7 +604,6 @@ struct POSTabEligibilityCheckerTests { let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores) // When & Then - Should throw the network error @@ -661,13 +617,12 @@ struct POSTabEligibilityCheckerTests { @Test func refreshEligibility_checks_eligibility_for_featureSwitchDisabled() async throws { // Given setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("10.0.0") // Version that supports feature switch - setupPOSFeatureEnabled(.success(true)) // Now enabled + setupWooCommerceVersion("10.0.0", featureSwitchEnabled: true) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, - stores: stores) + stores: stores, + systemStatusService: mockSystemStatusService) // When let result = try await checker.refreshEligibility(ineligibleReason: .featureSwitchDisabled) @@ -676,34 +631,15 @@ struct POSTabEligibilityCheckerTests { #expect(result == .eligible) } - @Test func refreshEligibility_checks_eligibility_for_featureSwitchSyncFailure() async throws { - // Given - setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("10.0.0") - setupPOSFeatureEnabled(.failure(NSError(domain: "test", code: 0))) // Still failing - - let checker = POSTabEligibilityChecker(siteID: siteID, - siteSettings: siteSettings, - pluginsService: pluginsService, - stores: stores) - - // When - let result = try await checker.refreshEligibility(ineligibleReason: .featureSwitchSyncFailure) - - // Then - Should check eligibility again (still failing) - #expect(result == .ineligible(reason: .featureSwitchSyncFailure)) - } - @Test func refreshEligibility_checks_eligibility_for_selfDeallocated() async throws { // Given setupCountry(country: .us, currency: .USD) - setupWooCommerceVersion("9.6.0") - setupPOSFeatureEnabled(.success(true)) + setupWooCommerceVersion("9.6.0", featureSwitchEnabled: true) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, - stores: stores) + stores: stores, + systemStatusService: mockSystemStatusService) // When let result = try await checker.refreshEligibility(ineligibleReason: .selfDeallocated) @@ -727,7 +663,6 @@ struct POSTabEligibilityCheckerTests { setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, systemStatusService: mockSystemStatusService) @@ -751,7 +686,6 @@ struct POSTabEligibilityCheckerTests { setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, systemStatusService: mockSystemStatusService) @@ -774,7 +708,6 @@ struct POSTabEligibilityCheckerTests { setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, systemStatusService: mockSystemStatusService) @@ -798,7 +731,6 @@ struct POSTabEligibilityCheckerTests { setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, systemStatusService: mockSystemStatusService) @@ -822,7 +754,6 @@ struct POSTabEligibilityCheckerTests { setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, systemStatusService: mockSystemStatusService) @@ -845,7 +776,6 @@ struct POSTabEligibilityCheckerTests { setupCountry(country: .us, currency: .USD) let checker = POSTabEligibilityChecker(siteID: siteID, siteSettings: siteSettings, - pluginsService: pluginsService, stores: stores, systemStatusService: mockSystemStatusService) @@ -869,8 +799,9 @@ private extension POSTabEligibilityCheckerTests { ].publisher.eraseToAnyPublisher() } - func setupWooCommerceVersion(_ version: String = "9.6.0-beta") { - pluginsService.pluginToReturn = createWooCommercePlugin(version: version) + func setupWooCommerceVersion(_ version: String = "9.6.0-beta", featureSwitchEnabled: Bool? = nil) { + let wcPlugin = createWooCommercePlugin(version: version) + mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: featureSwitchEnabled)) } func createWooCommercePlugin(version: String) -> SystemPlugin { @@ -891,17 +822,6 @@ private extension POSTabEligibilityCheckerTests { } } - func setupPOSFeatureEnabled(_ result: Result) { - stores.whenReceivingAction(ofType: SettingAction.self) { action in - switch action { - case .isFeatureEnabled(_, _, let completion): - completion(result) - default: - break - } - } - } - func setupPOSTabVisibility(siteID: Int64, isVisible: Bool?) { eligibilityService.cachedTabVisibility[siteID] = isVisible } @@ -934,14 +854,6 @@ private extension POSTabEligibilityCheckerTests { } } -private final class MockPluginsService: PluginsServiceProtocol { - var pluginToReturn: SystemPlugin = .fake() - - func waitForPluginInStorage(siteID: Int64, pluginPath: String, isActive: Bool) async -> SystemPlugin { - pluginToReturn - } -} - private final class MockSelectedSiteSettings: SelectedSiteSettingsProtocol { var mockSettingsStream: AnyPublisher<(siteID: Int64, settings: [SiteSetting], source: SettingsUpdateSource), Never>? var siteSettings: [SiteSetting] = [] From ae1fbd64c5c8ecf78cef8c457fce22cd27451ede Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 14 Jul 2025 16:22:47 -0400 Subject: [PATCH 7/8] Remove unnecessary `MockPOSSystemStatusService` instance now that it is initialized in test setup. --- .../Settings/POS/POSTabEligibilityCheckerTests.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift index 65472054c82..dff3cf2b6cd 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Settings/POS/POSTabEligibilityCheckerTests.swift @@ -656,7 +656,6 @@ struct POSTabEligibilityCheckerTests { ]) func refreshEligibility_returns_eligible_when_plugin_refreshed_with_valid_version_below_10(ineligibleReason: POSIneligibleReason) async throws { // Given - let mockSystemStatusService = MockPOSSystemStatusService() let wcPlugin = createWooCommercePlugin(version: "9.9.9") // Valid version below feature switch threshold mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: nil)) @@ -679,7 +678,6 @@ struct POSTabEligibilityCheckerTests { ]) fileprivate func refreshEligibility_returns_eligible_when_plugin_with_version_10_and_feature_enabled(ineligibleReason: POSIneligibleReason) async throws { // Given - let mockSystemStatusService = MockPOSSystemStatusService() let wcPlugin = createWooCommercePlugin(version: "10.0.0") mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: true)) @@ -702,7 +700,6 @@ struct POSTabEligibilityCheckerTests { ]) fileprivate func refreshEligibility_returns_ineligible_when_plugin_not_found_in_system_status(ineligibleReason: POSIneligibleReason) async throws { // Given - let mockSystemStatusService = MockPOSSystemStatusService() mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: nil, featureValue: nil)) setupCountry(country: .us, currency: .USD) @@ -724,7 +721,6 @@ struct POSTabEligibilityCheckerTests { ]) fileprivate func refreshEligibility_returns_ineligible_when_plugin_version_still_below_minimum(ineligibleReason: POSIneligibleReason) async throws { // Given - let mockSystemStatusService = MockPOSSystemStatusService() let wcPlugin = createWooCommercePlugin(version: "9.5.0") // Still below minimum 9.6.0-beta mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: nil)) @@ -747,7 +743,6 @@ struct POSTabEligibilityCheckerTests { ]) fileprivate func refreshEligibility_returns_ineligible_when_feature_switch_still_disabled(ineligibleReason: POSIneligibleReason) async throws { // Given - let mockSystemStatusService = MockPOSSystemStatusService() let wcPlugin = createWooCommercePlugin(version: "10.0.0") mockSystemStatusService.resultToReturn = .success(POSPluginAndFeatureInfo(wcPlugin: wcPlugin, featureValue: nil)) @@ -770,7 +765,6 @@ struct POSTabEligibilityCheckerTests { ]) fileprivate func refreshEligibility_returns_ineligible_when_system_status_request_fails(ineligibleReason: POSIneligibleReason) async throws { // Given - let mockSystemStatusService = MockPOSSystemStatusService() mockSystemStatusService.resultToReturn = .failure(NSError(domain: "test", code: 500)) setupCountry(country: .us, currency: .USD) From 1098a341906aa7925d479a64f048181bacfb76f7 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 14 Jul 2025 16:30:11 -0400 Subject: [PATCH 8/8] Move constants enum to be below other private extensions. --- .../Eligibility/POSSystemStatusService.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift b/Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift index d496d2068a8..6980c8b11a1 100644 --- a/Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift +++ b/Modules/Sources/Yosemite/PointOfSale/Eligibility/POSSystemStatusService.swift @@ -64,10 +64,6 @@ public final class POSSystemStatusService: POSSystemStatusServiceProtocol { } private extension POSSystemStatusService { - enum Constants { - static let wcPluginPath = "woocommerce/woocommerce.php" - } - /// Updates or inserts system plugins in storage. func upsertSystemPlugins(siteID: Int64, systemStatus: POSPluginEligibilitySystemStatus, in storage: StorageType) { // Active and inactive plugins share identical structure, but are stored in separate parts of the remote response @@ -103,6 +99,12 @@ private extension POSSystemStatusService { } } +private extension POSSystemStatusService { + enum Constants { + static let wcPluginPath = "woocommerce/woocommerce.php" + } +} + // MARK: - Network Response Structs private struct POSPluginEligibilitySystemStatus: Decodable {