-
Notifications
You must be signed in to change notification settings - Fork 117
[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
[POS as a tab i2] Refresh POS eligibility in ineligible UI: WC plugin ineligible cases #15908
Conversation
…ntOfSaleOrdersi1` is disabled (it's currently enabled in production). Will be removed when `pointOfSaleOrdersi1` feature flag is removed.
|
…sert all plugins to storage so that later use cases have the latest value.
…o 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.
…ature switch loading is combined with WC plugin. Update test cases.
…is initialized in test setup.
// 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) |
There was a problem hiding this comment.
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
woocommerce-ios/Modules/Sources/Yosemite/Stores/SystemStatusStore.swift
Lines 96 to 129 in 0ea5d32
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good. Tested various cases with different WooCommerce versions and done additional testing with disabling other types of settings. The implementation holds well.
For WOOMOB-854
Just one review is required.
Apology for the larger diffs, close to 300 diffs were on the unit tests.
Description
This PR completes the POS eligibility refresh implementation for plugin reasons, utilizing the new
POSSystemStatusService
from #15906 to efficiently refresh both WooCommerce plugin information and POS feature switch values (for plugin version 10.0+) with a single API request. Now a system status API request is made for plugin & POS feature switch checks on every POS tab tap and refresh.Key changes:
checkPluginEligibility
method: Refactored to callPOSSystemStatusService
to load the WC plugin & POS feature switch value from one system status API request.refreshEligibility
method: Now handlesunsupportedWooCommerceVersion
andwooCommercePluginNotFound
cases by callingcheckEligibility
that includescheckPluginEligibility
to reload the WC plugin & POS feature switch value.PluginsService
dependency: Cleaned upPOSTabEligibilityChecker
by removing the unusedPluginsService
dependency since all plugin data now comes throughPOSSystemStatusService
. This simplifies the architecture and reduces the number of dependencies.POSIneligibleReason.featureSwitchSyncFailure
: Eliminated this reason since POS feature switch loading is now combined with WooCommerce plugin loading through the system status service, making separate sync failure handling unnecessary.POSSystemStatusService
to fetch both active and inactive plugins, upsert all plugins to storage using modern async/await patterns, and ensure data consistency by loading from storage after updates.PluginsService
dependencies and use onlyPOSSystemStatusService
.StorageManagerType.performAndSave
to enable cleaner asynchronous storage operations.LegacyPOSTabEligibilityChecker
when thepointOfSaleOrdersi1
feature flag is disabled. This feature flag will be removed in the next sprint.Steps to reproduce
Unsupported WooCommerce version
Prerequisite: for a store that is eligible for POS, downgrade the WooCommerce plugin to version 9.5.0 or lower. For easier testing, I'd recommend using the
WooCommerce Beta Tester
plugin on a store where the WC plugin is not managed by the host like a JN store (for WPCOM stores and default Pressable WC stores, WC plugin is automatically managed by the host. In Pressable, there is an option to manually manage the WC plugin when creating a site).Retry
--> the CTA should be in loading state, then back to normal state (the WC version hasn't been fixed)Retry
--> the CTA should be in loading state, then the app enters POS to the state where products are loadedTesting information
I tested the above case with both WC plugin version scenarios (below 10.0 and 10.0+) and verified the feature switch handling works correctly for version 10.0+. The storage improvements ensure that plugin data is properly synchronized and persisted for consistent eligibility checks and other use cases in the app like Menu > Settings > Plugins.
Example screencast
ineligible UI with WC version 9.5.2 -> refresh without changes -> update WC version to 10.0+ in wp-admin -> refresh -> POS -> exit and enter POS again -> relaunch app and tap to enter POS
ineligible-wc-version-v2.mp4
RELEASE-NOTES.txt
if necessary.