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

[samiuelson]

WOOMOB-1556

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.

[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.}

[wpmobilebot]

📲 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	699b021
	Direct Download	woocommerce-wear-prototype-build-pr14816-699b021.apk

[wpmobilebot]

📲 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.}

[codecov-commenter]

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.

[dangermattic]

	2 Warnings
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️	This PR is assigned to the milestone 23.6. The due date for this milestone has already passed. Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

[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.

[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.

[malinajirka]

Thanks @samiuelson !

As we discussed I haven't tested this PR since I'll test it when we update the behavior of cleaning cache on PTR.

I've left some suggestions/questions but it looks good overall and I believe can be merged as is.

[malinajirka]

💡 I think I understand the idea here, but I would consider doing it the other way round. The current implementation returns a result which can be either ProductsResult or VariationsResults (for hybrid approach). That is a bit misleading and forces us to ignore variationsResult in product VM and product result in Variations VM.

Instead, we could have two almost identical copies for local Catalog:

sealed class WooPosSyncProductResult
sealed class WooPosSyncVariationResult

Local:

sealed class PosLocalCatalogProductSyncResult : WooPosSyncProductResult() {
        val value: PosLocalCatalogSyncResult    
}

sealed class PosLocalCatalogSyncResult : WooPosSyncVariationResult() {
    val value: PosLocalCatalogSyncResult
}


sealed class PosLocalCatalogSyncResult {
    data class Success(
        val productsSynced: Int,
        val variationsSynced: Int,
        val syncDurationMs: Long
    ) : PosLocalCatalogSyncResult()

    sealed class Failure(val error: String) : PosLocalCatalogSyncResult() {
        class CatalogTooLarge(error: String, val totalPages: Int, val maxPages: Int) : Failure(error)
        class UnexpectedError(error: String) : Failure(error)
    }
}

Hybrid:

sealed class WooPosHybridProductSyncResult : WooPosSyncProductResult()
sealed class WooPosHybridVariationSyncResult : WooPosSyncVariationResult()

Wdyt?

[malinajirka]

No need to update this - the more I think about it the more I feel like perhaps the downside of the current approach aren't that significant to justify this refactoring.

[malinajirka]

Works as expected!