-
Notifications
You must be signed in to change notification settings - Fork 136
[Woo POS] Fix variation lookup in WooPosProductsDataSource
#14918
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
base: trunk
Are you sure you want to change the base?
[Woo POS] Fix variation lookup in WooPosProductsDataSource
#14918
Conversation
This commit refactors the `getVariationById` method in `WooPosProductsDataSource` to prioritize fetching product variations from the cache. Key changes: - If a variation is found in the cache, it is returned immediately without a network request. - If the variation is not in the cache, but other variations for the same product exist, a network request is triggered to fetch all variations for that product. The cache is then updated with the fresh data. - If the product's variations cache is empty, the method returns null to avoid unnecessary network calls. - New unit tests have been added to cover these scenarios, including cache hits, cache misses, and network failures.
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.
Pull Request Overview
This PR adds enhanced caching logic for the getVariationById method in WooPosProductsRemoteDataSource. The implementation now checks the cache before making remote calls and refreshes the entire variation cache if a requested variation is not found but the cache already contains other variations for that product.
- Implements cache-first logic for variation retrieval
- Adds automatic cache refresh when variations exist but the requested one is not cached
- Returns
nullwithout remote call when cache is empty (indicating no variations exist)
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| WooPosProductsDataSource.kt | Implements new caching logic in getVariationById to check cache first, refresh from remote if variation not found but cache has variations, and return null for empty cache |
| WooPosProductsRemoteDataSourceTest.kt | Adds comprehensive test coverage for the new caching behavior including cache hit, cache miss with refresh, empty cache, error handling, and cache update scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...st/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosProductsRemoteDataSourceTest.kt
Show resolved
Hide resolved
...st/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosProductsRemoteDataSourceTest.kt
Show resolved
Hide resolved
📲 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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14918 +/- ##
============================================
+ Coverage 38.33% 38.35% +0.01%
- Complexity 10146 10147 +1
============================================
Files 2145 2145
Lines 121313 121324 +11
Branches 16653 16656 +3
============================================
+ Hits 46500 46528 +28
+ Misses 70088 70070 -18
- Partials 4725 4726 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…oryincache-uses-store
…oryincache-uses-store
WOOMOB-1573
Description
This PR removes dependency on
WCProductStorefromWooPosProductsRemoteDataSource::getVariationById. Instead of looking for variation inWCProductStore, it switches to in-memory variations cache used in POS, and falls back to the rest client in case the cache doesn't hold the variation.Before, the
getVariationByIdwas usingWCProductStore(used by store management) to get variation instance in POS Totals screen. This was incorrect, because the POS, in case local catalog is disabled, uses in-memory cache to render items, andWCProductStoreis not really a good fit there because it is not synced in any way with the in-memory variations cache in POS.Test Steps
Smoke test POS, focusing on the Totals and Checkout screens.
Images/gif
N/A
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.