[Woo POS][Local Catalog] Refactor and fix pull-to-refresh logic in products and variations #14816

samiuelson · 2025-10-24T09:35:03Z

Description

This PR refactors pull-to-refresh logic for both products and variations in the WooCommerce POS local catalog feature. The changes consolidate different refresh behaviors by introducing new refreshProducts() and refreshVariations() methods that delegate to either local catalog incremental sync or remote data source refresh based on feature flags and sync requirements.

Test Steps

Crucial: Verify that there's no regression to the existing PTR behavior in the hybrid data source.
Verify that local catalog incremental sync is performed for both variations and products.
Verify that local catalog incremental sync is not performed if the local catalog is not enabled.

Images/gif

N/A

I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

…w models Key changes: - A new `refreshProducts` and `refreshVariations` method has been introduced in the `WooPosProductsDataSource` layer to encapsulate the refresh logic. - The view models now call these new methods, removing the need for feature flag checks and direct interaction with sync repositories. - The `WooPosProductsInDbDataSource` now handles incremental sync logic when pull-to-refresh is triggered, falling back to a remote fetch if incremental sync isn't applicable.

Copilot

Pull Request Overview

This PR refactors pull-to-refresh logic for both products and variations in the WooCommerce POS local catalog feature. The changes consolidate different refresh behaviors by introducing new refreshProducts() and refreshVariations() methods that intelligently delegate to either local catalog incremental sync or remote data source refresh based on feature flags and sync requirements.

Key Changes:

Introduced refreshProducts() and refreshVariations() methods in the data source interface to standardize pull-to-refresh behavior
Removed feature flag checks and related dependencies from ViewModels, moving the logic into data sources
Updated all tests to use the new refresh methods instead of fetchFirstPage(forceRefresh = true)

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
WooPosProductsDataSourceInterface.kt	Added new interface methods `refreshProducts()` and `refreshVariations()` with documentation
WooPosProductsDataSource.kt	Implemented refresh methods in main data source and both local/remote implementations
WooPosProductsViewModel.kt	Refactored `handlePullToRefresh()` to use new `refreshProducts()` method, removed feature flag dependencies
WooPosVariationsViewModel.kt	Refactored `handlePullToRefresh()` to use new `refreshVariations()` method, removed feature flag dependencies
WooPosProductsViewModelTest.kt	Updated tests to use `refreshProducts()` and removed unused mocks
WooPosVariationsViewModelTest.kt	Updated tests to use `refreshVariations()` and removed unused mocks
WooPosProductsDataSourceTest.kt	Added test coverage for new refresh methods
WooPosProductsInDbDataSourceTest.kt	Added missing constructor parameters for test setup

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

.../kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt

...main/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsViewModel.kt

wpmobilebot · 2025-10-24T10:30:29Z

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`8c895ce`
	Direct Download	`woocommerce-wear-prototype-build-pr14816-8c895ce.apk`

wpmobilebot · 2025-10-24T10:46:00Z

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`8c895ce`
	Direct Download	`woocommerce-prototype-build-pr14816-8c895ce.apk`

Copilot

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsViewModel.kt:1

Missing closing brace for the WooPosProductsUIEvent.PullToRefreshTriggered when block. The handlePullToRefresh call and analytics tracking should be wrapped in braces.

package com.woocommerce.android.ui.woopos.home.items.products

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

…ncremental-sync-even-on

codecov-commenter · 2025-10-24T13:03:40Z

Codecov Report

❌ Patch coverage is 30.50847% with 82 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.26%. Comparing base (3e9f4c3) to head (8c895ce).
⚠️ Report is 46 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...os/home/items/products/WooPosProductsDataSource.kt	13.15%	28 Missing and 5 partials ⚠️
...home/items/variations/WooPosVariationsViewModel.kt	33.33%	15 Missing and 11 partials ⚠️
...pos/home/items/products/WooPosProductsViewModel.kt	30.30%	12 Missing and 11 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #14816      +/-   ##
============================================
- Coverage     38.27%   38.26%   -0.02%     
- Complexity    10103    10108       +5     
============================================
  Files          2132     2133       +1     
  Lines        120582   120617      +35     
  Branches      16530    16547      +17     
============================================
  Hits          46158    46158              
- Misses        69747    69764      +17     
- Partials       4677     4695      +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

malinajirka · 2025-10-24T17:20:11Z

...ain/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSource.kt

+        }
+    }.flowOn(Dispatchers.IO)
+
+    private suspend fun shouldUseIncrementalSync(): Boolean {


❓ I'm not sure I follow the code - why do we invoke fetchFirstVariationsPage from refreshVariations 😕 ?

Fixed: 801786d

malinajirka · 2025-10-24T17:23:04Z

...ain/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSource.kt


    override fun canLoadMoreVariations(numOfVariations: Int): Boolean = false

+    override suspend fun refreshProducts(): Flow<WooPosProductsDataSource.ProductsResult> = flow {


❓ It feels a bit off that refresh returns a flow, especially since it always emits just one response. Have you considered returning Result<Unit> and relying on Room updating getProductsFromDatabaseFlow that is being observed? Alternatively, we could return Result which isn't optimal but might simplify implementing the remote approach.

@malinajirka, thanks for your question. This is not obvious how to marry the Hybrid and LocalCatalog incremental sync results. Technically, the Hybrid returns a flow of Cached (intermediate) and Remote (final) results. However, in case forceRefresh=true, the cache gets cleared, I believe we can be interested solely in the last emitted result and can return it as a result. Could you please take a look at my refactor commit here: a7fd7db and let me know what you think about the approach of updating/consolidating the data sync result models?

This is not obvious how to marry the Hybrid and LocalCatalog incremental sync results.

Yeah you are right, it's not trivial. I think ideally both approaches would return data from cache - the only difference should be whether it's persistent db or in-memory cache. The fact that we return the response directly isn't optimal, as it means we have multiple sources of truth. Having said that, I understand that's out of scope. Moreover, the VM logic currently determines whether to hide/show progress/error-screen, based on whether the returned data are cache/remote, so it's not something we could easily refactor.

Could you please take a look at my refactor commit here: a7fd7db and let me know what you think about the approach of updating/consolidating the data sync result models?

I like it!

…ooPosProductsInDbDataSource`

dangermattic · 2025-10-27T10:38:01Z

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

This commit refactors the data synchronization and observation logic within the WooPOS items list. Key changes: - Introduced `WooPosSyncResult` as a common sealed interface for different synchronization results. - Moved `ProductsResult` and `VariationsResult` into a new `WooPosHybridSyncResult` file, which inherits from `WooPosSyncResult`, to centralize result type definitions. - Modified `refreshProducts` and `refreshVariations` in data sources and ViewModels to return `Result<WooPosSyncResult>`, simplifying the handling of refresh outcomes. - Implemented continuous observation of products and variations in `WooPosProductsViewModel` and `WooPosVariationsViewModel` to ensure the UI immediately reflects data from the cache upon initialization.

…ncremental-sync-even-on

Copilot

Pull Request Overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.../kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt

...main/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsViewModel.kt

.../kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt

...main/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsViewModel.kt

...ain/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSource.kt

Copilot

Pull Request Overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

samiuelson requested a review from Copilot October 24, 2025 09:35

Copilot AI reviewed Oct 24, 2025

View reviewed changes

.../kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt Outdated Show resolved Hide resolved

...main/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsViewModel.kt Outdated Show resolved Hide resolved

samiuelson added the type: task An internally driven task. label Oct 24, 2025

samiuelson added this to the 23.6 milestone Oct 24, 2025

Merge branch trunk and resolve conflicts

a656b2f

samiuelson requested a review from Copilot October 24, 2025 10:48

Copilot AI reviewed Oct 24, 2025

View reviewed changes

samiuelson requested a review from malinajirka October 24, 2025 12:25

Merge branch 'trunk' into woomob-1566-woo-poslocal-catalog-ptr-runs-i…

5e911ae

…ncremental-sync-even-on

samiuelson marked this pull request as ready for review October 24, 2025 12:26

malinajirka reviewed Oct 24, 2025

View reviewed changes

samiuelson added 2 commits October 27, 2025 11:12

Remove comment

4df733c

Rename function

9e08a48

samiuelson marked this pull request as draft October 27, 2025 10:18

Fix implementation of refreshProducts and refreshVariations in `W…

801786d

…ooPosProductsInDbDataSource`

samiuelson added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Oct 27, 2025

Merge branch 'trunk' into woomob-1566-woo-poslocal-catalog-ptr-runs-i…

1e0e98c

…ncremental-sync-even-on

samiuelson marked this pull request as ready for review October 27, 2025 17:17

samiuelson requested a review from Copilot October 28, 2025 07:53

Copilot AI reviewed Oct 28, 2025

View reviewed changes

samiuelson added 3 commits October 28, 2025 13:03

Remove unreachable code

eb180f2

Remove redundant code

22bb958

Remove unreachable code

e238460

samiuelson requested a review from Copilot October 28, 2025 12:58

Copilot AI reviewed Oct 28, 2025

View reviewed changes

samiuelson removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Oct 28, 2025

samiuelson requested a review from malinajirka October 28, 2025 12:59

Update tests

8c895ce


		override fun canLoadMoreVariations(numOfVariations: Int): Boolean = false

		override suspend fun refreshProducts(): Flow<WooPosProductsDataSource.ProductsResult> = flow {

Uh oh!

[Woo POS][Local Catalog] Refactor and fix pull-to-refresh logic in products and variations #14816

Are you sure you want to change the base?

[Woo POS][Local Catalog] Refactor and fix pull-to-refresh logic in products and variations #14816

Conversation

samiuelson commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Steps

Images/gif

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

wpmobilebot commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

codecov-commenter commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

malinajirka Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

samiuelson Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

malinajirka Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

samiuelson Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

malinajirka Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

dangermattic commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

samiuelson commented Oct 24, 2025 •

edited

Loading

wpmobilebot commented Oct 24, 2025 •

edited

Loading

wpmobilebot commented Oct 24, 2025 •

edited

Loading

codecov-commenter commented Oct 24, 2025 •

edited

Loading

dangermattic commented Oct 27, 2025 •

edited

Loading