Skip to content

[POS as a tab i2] Refresh POS eligibility in ineligible UI: WC plugin ineligible cases #15908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

13 changes: 13 additions & 0 deletions Modules/Sources/Storage/Protocols/StorageManagerType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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<POSPluginEligibilitySystemStatus>(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)
}

Expand All @@ -53,6 +63,42 @@ public final class POSSystemStatusService: POSSystemStatusServiceProtocol {
}
}

private extension POSSystemStatusService {
/// 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)
Comment on lines +69 to +98
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗒️ This is mostly the same as the upsert logic in SystemStatusStore

func upsertSystemPlugins(siteID: Int64, readonlySystemInformation: SystemStatus, in storage: StorageType) {
/// Active and in-active plugins share identical structure, but are stored in separate parts of the remote response
/// (and without an active attribute in the response). So... we use the same decoder for active and in-active plugins
/// and here we apply the correct value for active (or not)
///
let readonlySystemPlugins: [SystemPlugin] = {
let activePlugins = readonlySystemInformation.activePlugins.map {
$0.copy(active: true)
}
let inactivePlugins = readonlySystemInformation.inactivePlugins.map {
$0.copy(active: false)
}
return activePlugins + inactivePlugins
}()
let storedPlugins = storage.loadSystemPlugins(siteID: siteID, matching: readonlySystemPlugins.map { $0.name })
readonlySystemPlugins.forEach { readonlySystemPlugin in
// load or create new StorageSystemPlugin matching the readonly one
let storageSystemPlugin: StorageSystemPlugin = {
if let systemPlugin = storedPlugins.first(where: { $0.name == readonlySystemPlugin.name }) {
return systemPlugin
}
return storage.insertNewObject(ofType: StorageSystemPlugin.self)
}()
storageSystemPlugin.update(with: readonlySystemPlugin)
}
// remove stale system plugins
let currentSystemPlugins = readonlySystemPlugins.map(\.name)
storage.deleteStaleSystemPlugins(siteID: siteID, currentSystemPlugins: currentSystemPlugins)
}

In WOOMOB-859, I plan to refactor the upsert logic for reuse with some enhancements to ensure we're matching plugins by the plugin path instead of name.

}
}

private extension POSSystemStatusService {
enum Constants {
static let wcPluginPath = "woocommerce/woocommerce.php"
Expand All @@ -63,10 +109,12 @@ private extension POSSystemStatusService {

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
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"data": {
"inactive_plugins": [],
"active_plugins":[
{
"plugin": "woocommerce/woocommerce.php",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"data": {
"inactive_plugins": [],
"active_plugins":[
{
"plugin": "woocommerce/woocommerce.php",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"data": {
"inactive_plugins": [],
"active_plugins":[
],
"settings": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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
)
}
}
13 changes: 0 additions & 13 deletions WooCommerce/Classes/POS/TabBar/POSIneligibleView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ 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

/// Represents the reasons why a site may be ineligible for POS.
enum POSIneligibleReason: Equatable {
Expand All @@ -21,7 +21,6 @@ enum POSIneligibleReason: Equatable {
case siteSettingsNotAvailable
case wooCommercePluginNotFound
case featureSwitchDisabled
case featureSwitchSyncFailure
case unsupportedCurrency(supportedCurrencies: [CurrencyCode])
case selfDeallocated
}
Expand All @@ -47,25 +46,26 @@ 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
private let systemStatusService: POSSystemStatusServiceProtocol

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) {
featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService,
systemStatusService: POSSystemStatusServiceProtocol = POSSystemStatusService(credentials: ServiceLocator.stores.sessionManager.defaultCredentials,
storageManager: ServiceLocator.storageManager)) {
self.siteID = siteID
self.userInterfaceIdiom = userInterfaceIdiom
self.siteSettings = siteSettings
self.pluginsService = pluginsService
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.
Expand Down Expand Up @@ -131,14 +131,12 @@ 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()
case .featureSwitchDisabled:
// 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()
}
}
Expand All @@ -147,8 +145,38 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol {
// MARK: - WC Plugin Related Eligibility Check

private extension POSTabEligibilityChecker {
/// Checks the eligibility of the WooCommerce plugin and plugin version based POS feature switch value.
///
/// - 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() async -> POSEligibilityState {
let wcPlugin = await fetchWooCommercePlugin(siteID: siteID)
do {
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 = 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, wcPlugin.active else {
return .ineligible(reason: .wooCommercePluginNotFound)
}

guard VersionHelpers.isVersionSupported(version: wcPlugin.version,
minimumRequired: Constants.wcPluginMinimumVersion) else {
Expand All @@ -163,31 +191,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)
}

@MainActor
func fetchWooCommercePlugin(siteID: Int64) async -> SystemPlugin {
await pluginsService.waitForPluginInStorage(siteID: siteID, pluginPath: Constants.wcPlugin, isActive: true)
}

@MainActor
func checkFeatureSwitchEnabled(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)
}
// For versions that support the feature switch, checks if the feature switch is enabled separately.
return .pendingFeatureSwitchCheck
}
}

Expand Down
2 changes: 1 addition & 1 deletion WooCommerce/Classes/ViewRelated/Hub Menu/HubMenu.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading