Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Oct 31, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
57f4048
Refactor and fix pull-to-refresh logic in products and variations vie…
samiuelson Oct 24, 2025
a656b2f
Merge branch `trunk` and resolve conflicts
samiuelson Oct 24, 2025
5e911ae
Merge branch 'trunk' into woomob-1566-woo-poslocal-catalog-ptr-runs-i…
samiuelson Oct 24, 2025
4df733c
Remove comment
samiuelson Oct 27, 2025
9e08a48
Rename function
samiuelson Oct 27, 2025
801786d
Fix implementation of `refreshProducts` and `refreshVariations` in `W…
samiuelson Oct 27, 2025
a7fd7db
Refactor: Unify sync result types and observe product/variation lists
samiuelson Oct 27, 2025
1e0e98c
Merge branch 'trunk' into woomob-1566-woo-poslocal-catalog-ptr-runs-i…
samiuelson Oct 27, 2025
eb180f2
Remove unreachable code
samiuelson Oct 28, 2025
22bb958
Remove redundant code
samiuelson Oct 28, 2025
e238460
Remove unreachable code
samiuelson Oct 28, 2025
8c895ce
Update tests
samiuelson Oct 28, 2025
fa6161a
Clean up code
samiuelson Oct 31, 2025
699b021
Return failure if site is not selected
samiuelson Oct 31, 2025
5cefb66
Introduce specific sync result types for products and variations
samiuelson Oct 31, 2025
7cfbb4e
Merge branch 'trunk' into woomob-1566-woo-poslocal-catalog-ptr-runs-i…
samiuelson Oct 31, 2025
3988c8f
Refactor WooPosSyncResult to remove unused sealed class
samiuelson Oct 31, 2025
5611e9d
Satisfy detekt's complaints
samiuelson Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 57 additions & 17 deletions .../kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSource.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,17 @@ import com.woocommerce.android.ui.woopos.common.data.models.WooPosProductModel
import com.woocommerce.android.ui.woopos.common.data.models.WooPosProductModelMapper
import com.woocommerce.android.ui.woopos.common.data.models.WooPosWCProductToWooPosProductModelMapper
import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation
import com.woocommerce.android.ui.woopos.home.items.products.WooPosProductsDataSource.VariationsResult
import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsLRUCache
import com.woocommerce.android.ui.woopos.localcatalog.PosLocalCatalogProductSyncResult
import com.woocommerce.android.ui.woopos.localcatalog.PosLocalCatalogVariationSyncResult
import com.woocommerce.android.ui.woopos.localcatalog.ProductsResult
import com.woocommerce.android.ui.woopos.localcatalog.VariationsResult
import com.woocommerce.android.ui.woopos.localcatalog.WooPosFullSyncRequirement
import com.woocommerce.android.ui.woopos.localcatalog.WooPosFullSyncStatusChecker
import com.woocommerce.android.ui.woopos.localcatalog.WooPosLocalCatalogSyncRepository
import com.woocommerce.android.ui.woopos.localcatalog.WooPosPerformInstantCatalogFullSync
import com.woocommerce.android.ui.woopos.localcatalog.WooPosSyncProductResult
import com.woocommerce.android.ui.woopos.localcatalog.WooPosSyncVariationResult
import com.woocommerce.android.util.WooLog
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
Expand All @@ -27,6 +33,7 @@ import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.last
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.take
import kotlinx.coroutines.withContext
Expand Down Expand Up @@ -127,6 +134,14 @@ class WooPosProductsDataSource @Inject constructor(
activeSource?.canLoadMoreVariations(numOfVariations)
?: error("canLoadMoreVariations - Data source not selected")

suspend fun refreshProducts(): Result<WooPosSyncProductResult> =
activeSource?.refreshProducts()
?: error("refreshProducts - Data source not selected")

suspend fun refreshVariations(productId: Long): Result<WooPosSyncVariationResult> =
activeSource?.refreshVariations(productId)
?: error("refreshVariations - Data source not selected")

suspend fun getVariationById(
productId: Long,
variationId: Long
Expand All @@ -135,16 +150,6 @@ class WooPosProductsDataSource @Inject constructor(
return activeSource?.getVariationById(productId, variationId)
}

sealed class ProductsResult {
data class Cached(val products: List<WooPosProductModel>) : ProductsResult()
data class Remote(val productsResult: Result<List<WooPosProductModel>>) : ProductsResult()
}

sealed class VariationsResult {
data class Cached(val data: List<WooPosVariation>) : VariationsResult()
data class Remote(val result: Result<List<WooPosVariation>>) : VariationsResult()
}

sealed class WooPosPrepopulatingDataStatus {
data object Syncing : WooPosPrepopulatingDataStatus()
data object Completed : WooPosPrepopulatingDataStatus()
Expand All @@ -158,6 +163,7 @@ class WooPosProductsInDbDataSource @Inject constructor(
private val productMapper: WooPosProductModelMapper,
private val variationMapper: WooPosVariationMapper,
private val performInstantCatalogFullSync: WooPosPerformInstantCatalogFullSync,
private val localCatalogSyncRepository: WooPosLocalCatalogSyncRepository,
) : WooPosProductsDataSourceInterface {

private fun getProductsFromDatabaseFlow(): Flow<List<WooPosProductModel>> {
Expand All @@ -174,9 +180,9 @@ class WooPosProductsInDbDataSource @Inject constructor(

override fun fetchFirstProductsPage(
forceRefresh: Boolean
): Flow<WooPosProductsDataSource.ProductsResult> = getProductsFromDatabaseFlow()
): Flow<ProductsResult> = getProductsFromDatabaseFlow()
.map { products ->
WooPosProductsDataSource.ProductsResult.Remote(Result.success(products))
ProductsResult.Remote(Result.success(products))
}
.flowOn(Dispatchers.IO)

Expand Down Expand Up @@ -232,6 +238,22 @@ class WooPosProductsInDbDataSource @Inject constructor(
)
return result.getOrNull()?.toWooPosVariation(variationMapper)
}

override suspend fun refreshProducts(): Result<WooPosSyncProductResult> = withContext(Dispatchers.IO) {
selectedSite.getOrNull()?.let { site ->
val syncResult = localCatalogSyncRepository.syncLocalCatalogIncremental(site)
Result.success(PosLocalCatalogProductSyncResult(syncResult))
} ?: Result.failure(IllegalStateException("No site selected"))
}

override suspend fun refreshVariations(productId: Long): Result<WooPosSyncVariationResult> = withContext(
Dispatchers.IO
) {
selectedSite.getOrNull()?.let { site ->
val syncResult = localCatalogSyncRepository.syncLocalCatalogIncremental(site)
Result.success(PosLocalCatalogVariationSyncResult(syncResult))
} ?: Result.failure(IllegalStateException("No site selected"))
}
}

class WooPosProductsRemoteDataSource @Inject constructor(
Expand Down Expand Up @@ -297,22 +319,22 @@ class WooPosProductsRemoteDataSource @Inject constructor(

override fun fetchFirstProductsPage(
forceRefresh: Boolean
): Flow<WooPosProductsDataSource.ProductsResult> = flow {
): Flow<ProductsResult> = flow {
offset.set(0)
productsIndex.clearCache()

if (!forceRefresh) {
val cachedProducts = sortProducts(productsCache.getAll()).take(NORMAL_PAGE_SIZE)
emit(WooPosProductsDataSource.ProductsResult.Cached(cachedProducts))
emit(ProductsResult.Cached(cachedProducts))
}

val fetchResult = fetchProducts()

if (fetchResult.isSuccess) {
emit(WooPosProductsDataSource.ProductsResult.Remote(Result.success(fetchResult.getOrThrow())))
emit(ProductsResult.Remote(Result.success(fetchResult.getOrThrow())))
} else {
emit(
WooPosProductsDataSource.ProductsResult.Remote(
ProductsResult.Remote(
Result.failure(
fetchResult.exceptionOrNull() ?: Exception("Unknown error")
)
Expand Down Expand Up @@ -488,6 +510,24 @@ class WooPosProductsRemoteDataSource @Inject constructor(
?.toWooPosVariation(variationMapper)
}

override suspend fun refreshProducts(): Result<WooPosSyncProductResult> = withContext(
Dispatchers.IO
) {
// We can return the last-emitted result which will always be the instance of [ProductsResult.Remote] because
// when forceRefresh = true the in-memory cache gets wiped out and the flow fetches from remote only.
val productsResult = fetchFirstProductsPage(forceRefresh = true).last()
Result.success(productsResult)
}

override suspend fun refreshVariations(
productId: Long
): Result<WooPosSyncVariationResult> = withContext(Dispatchers.IO) {
// We can return the last-emitted result which will always be the instance of [VariationsResult.Remote] because
// when forceRefresh = true the in-memory cache gets wiped out and the flow fetches from remote only.
val variationsResult = fetchFirstVariationsPage(productId, forceRefresh = true).last()
Result.success(variationsResult)
}

companion object {
private const val NORMAL_PAGE_SIZE = 25
private const val PRE_POPULATION_PAGE_SIZE = 100
Expand Down
12 changes: 10 additions & 2 deletions ...om/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSourceInterface.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ package com.woocommerce.android.ui.woopos.home.items.products

import com.woocommerce.android.ui.woopos.common.data.WooPosVariation
import com.woocommerce.android.ui.woopos.common.data.models.WooPosProductModel
import com.woocommerce.android.ui.woopos.localcatalog.ProductsResult
import com.woocommerce.android.ui.woopos.localcatalog.VariationsResult
import com.woocommerce.android.ui.woopos.localcatalog.WooPosSyncProductResult
import com.woocommerce.android.ui.woopos.localcatalog.WooPosSyncVariationResult
import kotlinx.coroutines.flow.Flow
import org.wordpress.android.fluxc.model.LocalOrRemoteId

interface WooPosProductsDataSourceInterface {
fun fetchFirstProductsPage(
forceRefresh: Boolean
): Flow<WooPosProductsDataSource.ProductsResult>
): Flow<ProductsResult>

suspend fun loadMoreProducts(): Result<List<WooPosProductModel>>

Expand All @@ -23,11 +27,15 @@ interface WooPosProductsDataSourceInterface {
fun fetchFirstVariationsPage(
productId: Long,
forceRefresh: Boolean
): Flow<WooPosProductsDataSource.VariationsResult>
): Flow<VariationsResult>

suspend fun loadMoreVariations(productId: Long): Result<List<WooPosVariation>>

fun canLoadMoreVariations(numOfVariations: Int): Boolean

suspend fun getVariationById(productId: Long, variationId: Long): WooPosVariation?

suspend fun refreshProducts(): Result<WooPosSyncProductResult>

suspend fun refreshVariations(productId: Long): Result<WooPosSyncVariationResult>
}
115 changes: 51 additions & 64 deletions ...n/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ package com.woocommerce.android.ui.woopos.home.items.products
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.woocommerce.android.R
import com.woocommerce.android.tools.SelectedSite
import com.woocommerce.android.ui.woopos.common.data.models.WooPosProductModel
import com.woocommerce.android.ui.woopos.featureflags.WooPosLocalCatalogM1Enabled
import com.woocommerce.android.ui.woopos.home.ChildToParentEvent
import com.woocommerce.android.ui.woopos.home.ParentToChildrenEvent
import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender
Expand All @@ -15,8 +13,9 @@ import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel.ItemCli
import com.woocommerce.android.ui.woopos.home.items.WooPosPaginationState
import com.woocommerce.android.ui.woopos.home.items.WooPosProductsViewState
import com.woocommerce.android.ui.woopos.home.items.WooPosPullToRefreshState
import com.woocommerce.android.ui.woopos.localcatalog.PosLocalCatalogProductSyncResult
import com.woocommerce.android.ui.woopos.localcatalog.PosLocalCatalogSyncResult
import com.woocommerce.android.ui.woopos.localcatalog.WooPosLocalCatalogSyncRepository
import com.woocommerce.android.ui.woopos.localcatalog.ProductsResult
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent.Event.PullToRefreshTriggered
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant
Expand All @@ -39,9 +38,6 @@ class WooPosProductsViewModel @Inject constructor(
private val parentToChildrenEventReceiver: WooPosParentToChildrenEventReceiver,
private val priceFormat: WooPosFormatPrice,
private val analyticsTracker: WooPosAnalyticsTracker,
private val wooPosLocalCatalogM1Enabled: WooPosLocalCatalogM1Enabled,
private val localCatalogSyncRepository: WooPosLocalCatalogSyncRepository,
private val selectedSite: SelectedSite,
private val resourceProvider: ResourceProvider,
) : ViewModel() {

Expand All @@ -62,22 +58,14 @@ class WooPosProductsViewModel @Inject constructor(

init {
listenEventsFromParent()
loadProducts(
forceRefreshProducts = false,
withPullToRefresh = false,
)
loadProducts(forceRefreshProducts = false)
}

private fun listenEventsFromParent() {
viewModelScope.launch {
parentToChildrenEventReceiver.events.collect { event ->
when (event) {
ParentToChildrenEvent.RefreshProductList -> {
loadProducts(
forceRefreshProducts = true,
withPullToRefresh = false,
)
}
ParentToChildrenEvent.RefreshProductList -> loadProducts(forceRefreshProducts = true)

ParentToChildrenEvent.BackFromCheckoutToCartClicked,
is ParentToChildrenEvent.BarcodeEvent,
Expand Down Expand Up @@ -109,17 +97,7 @@ class WooPosProductsViewModel @Inject constructor(
}

WooPosProductsUIEvent.PullToRefreshTriggered -> {
when {
wooPosLocalCatalogM1Enabled() -> {
performIncrementalSync()
}
else -> {
loadProducts(
forceRefreshProducts = true,
withPullToRefresh = true,
)
}
}
handlePullToRefresh()
viewModelScope.launch {
analyticsTracker.track(
PullToRefreshTriggered(
Expand All @@ -130,12 +108,7 @@ class WooPosProductsViewModel @Inject constructor(
}
}

WooPosProductsUIEvent.ProductsLoadingErrorRetryButtonClicked -> {
loadProducts(
forceRefreshProducts = false,
withPullToRefresh = false,
)
}
WooPosProductsUIEvent.ProductsLoadingErrorRetryButtonClicked -> loadProducts(forceRefreshProducts = false)
}
}

Expand Down Expand Up @@ -170,28 +143,21 @@ class WooPosProductsViewModel @Inject constructor(
}
}

private fun loadProducts(
forceRefreshProducts: Boolean,
withPullToRefresh: Boolean,
) {
private fun loadProducts(forceRefreshProducts: Boolean) {
loadProductsJob?.cancel()
loadMoreProductsJob?.cancel()
loadProductsJob = viewModelScope.launch {
_viewState.value = if (withPullToRefresh) {
buildReloadingState()
} else {
WooPosProductsViewState.Loading()
}
_viewState.value = WooPosProductsViewState.Loading()

dataSource.fetchFirstPage(forceRefresh = forceRefreshProducts).collect { result ->
when (result) {
is WooPosProductsDataSource.ProductsResult.Cached -> {
is ProductsResult.Cached -> {
if (result.products.isNotEmpty()) {
_viewState.value = result.products.toContentState()
}
}

is WooPosProductsDataSource.ProductsResult.Remote -> {
is ProductsResult.Remote -> {
_viewState.value = when {
result.productsResult.isSuccess -> {
val products = result.productsResult.getOrThrow()
Expand Down Expand Up @@ -337,32 +303,53 @@ class WooPosProductsViewModel @Inject constructor(
viewModelScope.launch { fromChildToParentEventSender.sendToParent(event) }
}

private fun performIncrementalSync() {
_viewState.value = buildReloadingState()

private fun handlePullToRefresh() {
viewModelScope.launch {
selectedSite.getOrNull()?.let { site ->
val syncResult = localCatalogSyncRepository.syncLocalCatalogIncremental(site)
_viewState.value = buildViewStateForSyncResult(syncResult)
_viewState.value = buildReloadingState()

val result = dataSource.refreshProducts()
result.onSuccess { syncResult ->
when (syncResult) {
is PosLocalCatalogProductSyncResult -> {
when (syncResult.value) {
is PosLocalCatalogSyncResult.Success -> {
_viewState.value = hidePTRIndicator()
}
is PosLocalCatalogSyncResult.Failure -> {
handlePTRError()
}
}
}
is ProductsResult.Cached -> Unit // PTR is not expected to deliver cached result
is ProductsResult.Remote -> {
if (syncResult.productsResult.isSuccess) {
val products = syncResult.productsResult.getOrThrow()
_viewState.value = if (products.isNotEmpty()) {
products.toContentState()
} else {
WooPosProductsViewState.Empty()
}
} else {
handlePTRError()
}
}
}
}.onFailure { exception ->
handlePTRError()
}
}
}

private fun buildViewStateForSyncResult(syncResult: PosLocalCatalogSyncResult): WooPosProductsViewState =
when (syncResult) {
is PosLocalCatalogSyncResult.Success -> {
hidePTRIndicator()
}

is PosLocalCatalogSyncResult.Failure -> {
sendEventToParent(
ChildToParentEvent.ToastMessageDisplayed(
message = resourceProvider.getString(R.string.something_went_wrong_try_again)
)
private fun handlePTRError() {
sendEventToParent(
ChildToParentEvent.ToastMessageDisplayed(
message = resourceProvider.getString(
R.string.something_went_wrong_try_again
)
hidePTRIndicator()
}
}
)
)
_viewState.value = hidePTRIndicator()
}

private fun hidePTRIndicator(): WooPosProductsViewState = when (val currentState = _viewState.value) {
is WooPosProductsViewState.Content -> currentState.copy(
Expand Down
Loading