[Woo POS][Local Catalog] Consolidate requirements for local catalog support #14821
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates the various requirements for local catalog support into a single, centralized check through the new WooPosIsLocalCatalogSupported class. This refactoring reduces duplication and simplifies the logic across multiple components.
Key Changes:
- Created
WooPosIsLocalCatalogSupportedclass to centralize all local catalog support checks (feature flag, variations endpoint availability, and periodic sync settings) - Refactored
WooPosLocalCatalogSyncWorkerto use the consolidated support check instead of multiple individual validations - Updated
WooPosFullSyncStatusCheckerto delegate support checks to the new centralized class
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| WooPosIsLocalCatalogSupported.kt | New class that consolidates all local catalog support checks in one place |
| WooPosIsLocalCatalogSupportedTest.kt | Test coverage for the new consolidated support checker |
| WooPosLocalCatalogSyncWorker.kt | Simplified to use consolidated support check, removing redundant individual checks |
| WooPosLocalCatalogSyncWorkerTest.kt | Updated tests to use new consolidated checker, removing tests for individual conditions |
| WooPosFullSyncStatusChecker.kt | Refactored to use consolidated support check instead of individual validations |
| WooPosFullSyncStatusCheckerTest.kt | Updated tests to reflect consolidated approach, removing redundant individual condition tests |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…osfullsyncstatuschecker-in
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #14821 +/- ##
============================================
- Coverage 38.24% 38.24% -0.01%
+ Complexity 10088 10087 -1
============================================
Files 2130 2131 +1
Lines 120423 120416 -7
Branches 16490 16488 -2
============================================
- Hits 46058 46048 -10
- Misses 69710 69711 +1
- Partials 4655 4657 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if (!prefsRepo.isPeriodicSyncEnabledForSite(siteId)) { | ||
| return false | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 We could consider having the catalog size check here as well - we could have a parameter, something like forceRefresh, that would indicate whether we actually check the catalog size or whether we use a cached value. Either case, it's not in scope of this PR, since it'd be more than just refactoring.
malinajirka
deleted the
woomob-1576-woo-poslocal-catalog-reuse-wooposfullsyncstatuschecker-in
branch
WOOMOB-1576
Description
This PR consolidates the requirements for local catalog support duplicated across different places into a single, centralized check through the new
WooPosIsLocalCatalogSupportedclass. This refactoring reduces duplication and simplifies the logic across multiple components.Test Steps
This is purely a refactoring PR. It's important to analyze the code to make sure the business logic didn't change and smoke test the POS local catalog syncing behavior.
Images/gif
N/A
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.