Skip to content

[Woo POS][Local Catalog] Refine sync status checker states #14894

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 6 commits into from
Nov 5, 2025
Merged

[Woo POS][Local Catalog] Refine sync status checker states #14894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4e66008
Introduce Non-Blocking Sync Requirement state
samiuelson Nov 4, 2025
d66674e
Update data source test
samiuelson Nov 4, 2025
6c56c04
Refactor WooPosFullSyncStatusCheckerTest to use a mock DateTimeProvider
samiuelson Nov 4, 2025
c93cf51
Satisfy detekt's complaints
samiuelson Nov 4, 2025
309cbbc
Fix FULL_SYNC_NOT_REQUIRED_THRESHOLD value
samiuelson Nov 5, 2025
d24aeea
Update tests
samiuelson Nov 5, 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
12 changes: 9 additions & 3 deletions ...erce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,20 @@ class WooPosItemsViewModel @Inject constructor(
preferencesRepository.setWasOpenedOnce(true)
}

checkSyncStatusAndUpdateBanner()
refreshSyncOverdueBannerState()
}

private fun checkSyncStatusAndUpdateBanner() {
private fun refreshSyncOverdueBannerState() {
viewModelScope.launch {
val requirement = syncStatusChecker.checkSyncRequirement()
_catalogSyncOverdueBannerState.value = when (requirement) {
is WooPosFullSyncRequirement.Overdue -> CatalogSyncOverdueBannerState.Visible
is WooPosFullSyncRequirement.NonBlockingRequired -> {
if (requirement.isOverdue) {
CatalogSyncOverdueBannerState.Visible
} else {
CatalogSyncOverdueBannerState.Hidden
}
}
else -> CatalogSyncOverdueBannerState.Hidden
}
}
Expand Down
2 changes: 1 addition & 1 deletion .../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 @@ -74,7 +74,7 @@ class WooPosProductsDataSource @Inject constructor(
}

is WooPosFullSyncRequirement.NotRequired,
is WooPosFullSyncRequirement.Overdue -> {
is WooPosFullSyncRequirement.NonBlockingRequired -> {
activeSource = localDbDataSource
emit(WooPosPrepopulatingDataStatus.Completed)
}
Expand Down
48 changes: 26 additions & 22 deletions ...main/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosFullSyncStatusChecker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.wordpress.android.fluxc.model.LocalOrRemoteId
import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosLocalCatalogStore
import javax.inject.Inject
import kotlin.time.Duration.Companion.days
import kotlin.time.Duration.Companion.hours

class WooPosFullSyncStatusChecker @Inject constructor(
private val syncTimestampManager: WooPosSyncTimestampManager,
Expand All @@ -18,7 +19,8 @@ class WooPosFullSyncStatusChecker @Inject constructor(
private val prefsRepo: WooPosPreferencesRepository,
private val checkCatalogSizeAction: WooPosCheckCatalogSizeAction,
private val isLocalCatalogSupported: WooPosIsLocalCatalogSupported,
private val wooPosLogWrapper: WooPosLogWrapper
private val wooPosLogWrapper: WooPosLogWrapper,
private val time: DateTimeProvider,
) {
@Suppress("ReturnCount")
suspend fun checkSyncRequirement(): WooPosFullSyncRequirement {
Expand Down Expand Up @@ -54,43 +56,45 @@ class WooPosFullSyncStatusChecker @Inject constructor(
return WooPosFullSyncRequirement.BlockingRequired
}

return when {
isFullSyncOverdue(lastFullSyncTimestamp) -> {
if (!networkStatus.isConnected()) {
wooPosLogWrapper.d(
"Full sync overdue but offline - allowing POS to load with cached data " +
"(${if (catalogIsEmpty) "empty catalog" else "$productCount products"})"
)
}
wooPosLogWrapper.d("Full sync overdue (last sync: $lastFullSyncTimestamp)")
WooPosFullSyncRequirement.Overdue
}
val now = time.now()
val timeElapsedSinceLastSync = now - lastFullSyncTimestamp

else -> {
return when {
timeElapsedSinceLastSync < FULL_SYNC_NOT_REQUIRED_THRESHOLD -> {
wooPosLogWrapper.d(
"Full sync not required: Recent sync at $lastFullSyncTimestamp " +
"(${if (catalogIsEmpty) "empty catalog" else "$productCount products"})"
)
WooPosFullSyncRequirement.NotRequired(lastFullSyncTimestamp)
}
else -> {
if (!networkStatus.isConnected()) {
wooPosLogWrapper.d(
"Full sync required but offline - allowing POS to load with cached data " +
"(${if (catalogIsEmpty) "empty catalog" else "$productCount products"})"
)
}
val isFullSyncOverdue = timeElapsedSinceLastSync > FULL_SYNC_OVERDUE_THRESHOLD
if (isFullSyncOverdue) {
wooPosLogWrapper.d("Full sync overdue (last sync: $lastFullSyncTimestamp)")
}
WooPosFullSyncRequirement.NonBlockingRequired(lastFullSyncTimestamp, isFullSyncOverdue)
}
}
}

private fun isFullSyncOverdue(lastSyncTimestamp: Long): Boolean {
val currentTime = System.currentTimeMillis()
val timeSinceLastSync = currentTime - lastSyncTimestamp
val overdueThreshold = FULL_SYNC_OVERDUE_THRESHOLD.inWholeMilliseconds
return timeSinceLastSync >= overdueThreshold
}

companion object {
private val FULL_SYNC_OVERDUE_THRESHOLD = 7.days
private val FULL_SYNC_OVERDUE_THRESHOLD = 7.days.inWholeMilliseconds
private val FULL_SYNC_NOT_REQUIRED_THRESHOLD = 23.hours.inWholeMilliseconds
}
}

sealed class WooPosFullSyncRequirement {
data class NotRequired(val lastSyncTimestamp: Long) : WooPosFullSyncRequirement()
data object Overdue : WooPosFullSyncRequirement()
data class NonBlockingRequired(
val lastSyncTimestamp: Long,
val isOverdue: Boolean
) : WooPosFullSyncRequirement()
data object BlockingRequired : WooPosFullSyncRequirement()
data class Error(val message: String) : WooPosFullSyncRequirement()
data class LocalCatalogDisabled(val message: String) : WooPosFullSyncRequirement()
Expand Down
2 changes: 1 addition & 1 deletion ...ain/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ constructor(
Result.retry()
}
is WooPosFullSyncRequirement.BlockingRequired,
is WooPosFullSyncRequirement.Overdue -> {
is WooPosFullSyncRequirement.NonBlockingRequired -> {
logger.d("Proceeding with sync (requirement: $syncRequirement)")
null
}
Expand Down
2 changes: 1 addition & 1 deletion ...lin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSourceTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class WooPosProductsDataSourceTest {
fun `given sync overdue, when prepopulate cache, then uses local db data source`() = runTest {
// GIVEN
whenever(syncStatusChecker.checkSyncRequirement()).thenReturn(
WooPosFullSyncRequirement.Overdue
WooPosFullSyncRequirement.NonBlockingRequired(lastSyncTimestamp = 0L, isOverdue = true)
)
whenever(localDbDataSource.fetchFirstProductsPage(false)).thenReturn(
flowOf(ProductsResult.Remote(Result.success(emptyList())))
Expand Down
62 changes: 41 additions & 21 deletions .../kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosFullSyncStatusCheckerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosLocalCatalogStore
import kotlin.test.Test
import kotlin.time.Duration.Companion.days
import kotlin.time.Duration.Companion.hours

@ExperimentalCoroutinesApi
class WooPosFullSyncStatusCheckerTest {
Expand All @@ -36,12 +37,17 @@ class WooPosFullSyncStatusCheckerTest {
private val prefsRepo: WooPosPreferencesRepository = mock()
private val checkCatalogSizeAction: WooPosCheckCatalogSizeAction = mock()
private val isLocalCatalogSupported: WooPosIsLocalCatalogSupported = mock()
private val time: DateTimeProvider = mock()

private val siteModel = SiteModel().apply {
id = 123
siteId = 456L
}

companion object {
private const val NOW = 1609459200000L // 2021-01-01 00:00:00 UTC
}

private fun createSut() = WooPosFullSyncStatusChecker(
syncTimestampManager = syncTimestampManager,
selectedSite = selectedSite,
Expand All @@ -50,16 +56,18 @@ class WooPosFullSyncStatusCheckerTest {
prefsRepo = prefsRepo,
checkCatalogSizeAction = checkCatalogSizeAction,
isLocalCatalogSupported = isLocalCatalogSupported,
wooPosLogWrapper = wooPosLogWrapper
wooPosLogWrapper = wooPosLogWrapper,
time = time
)

@Before
fun setup() = runTest {
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
val recentTimestamp = NOW - 1.days.inWholeMilliseconds
whenever(selectedSite.getOrNull()).thenReturn(siteModel)
whenever(isLocalCatalogSupported(siteModel.localId())).thenReturn(true)
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)
whenever(networkStatus.isConnected()).thenReturn(true)
whenever(time.now()).thenReturn(NOW)
whenever(localCatalogStore.getProductCount(LocalOrRemoteId.LocalId(siteModel.id)))
.thenReturn(Result.success(15))
whenever(checkCatalogSizeAction.execute(siteModel, null, 1000))
Expand Down Expand Up @@ -128,10 +136,11 @@ class WooPosFullSyncStatusCheckerTest {
}

@Test
fun `given sync overdue and network connected, when checkSyncRequirement called, then should return Overdue`() =
fun `given sync overdue and network connected, when checkSyncRequirement called, then should return NonBlockingRequired with isOverdue true`() =
runTest {
// GIVEN
val overdueTimestamp = System.currentTimeMillis() - 8.days.inWholeMilliseconds
val overdueTimestamp = NOW - 8.days.inWholeMilliseconds
whenever(time.now()).thenReturn(NOW)
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(overdueTimestamp)
whenever(networkStatus.isConnected()).thenReturn(true)

Expand All @@ -141,14 +150,17 @@ class WooPosFullSyncStatusCheckerTest {
val result = sut.checkSyncRequirement()

// THEN
assertThat(result).isEqualTo(WooPosFullSyncRequirement.Overdue)
assertThat(
result
).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(overdueTimestamp, isOverdue = true))
}

@Test
fun `given sync overdue and no network, when checkSyncRequirement called, then should return Overdue`() =
fun `given sync overdue and no network, when checkSyncRequirement called, then should return NonBlockingRequired with isOverdue true`() =
runTest {
// GIVEN
val overdueTimestamp = System.currentTimeMillis() - 8.days.inWholeMilliseconds
val overdueTimestamp = NOW - 8.days.inWholeMilliseconds
whenever(time.now()).thenReturn(NOW)
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(overdueTimestamp)
whenever(networkStatus.isConnected()).thenReturn(false)

Expand All @@ -158,14 +170,17 @@ class WooPosFullSyncStatusCheckerTest {
val result = sut.checkSyncRequirement()

// THEN
assertThat(result).isEqualTo(WooPosFullSyncRequirement.Overdue)
assertThat(
result
).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(overdueTimestamp, isOverdue = true))
}

@Test
fun `given sync overdue with empty catalog and no network, when checkSyncRequirement called, then should return Overdue`() =
fun `given sync overdue with empty catalog and no network, when checkSyncRequirement called, then should return NonBlockingRequired with isOverdue true`() =
runTest {
// GIVEN
val overdueTimestamp = System.currentTimeMillis() - 8.days.inWholeMilliseconds
val overdueTimestamp = NOW - 8.days.inWholeMilliseconds
whenever(time.now()).thenReturn(NOW)
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(overdueTimestamp)
whenever(networkStatus.isConnected()).thenReturn(false)
whenever(localCatalogStore.getProductCount(LocalOrRemoteId.LocalId(siteModel.id)))
Expand All @@ -177,14 +192,16 @@ class WooPosFullSyncStatusCheckerTest {
val result = sut.checkSyncRequirement()

// THEN
assertThat(result).isEqualTo(WooPosFullSyncRequirement.Overdue)
assertThat(
result
).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(overdueTimestamp, isOverdue = true))
}

@Test
fun `given sync not overdue, when checkSyncRequirement called, then should return NotRequired`() =
runTest {
// GIVEN
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
val recentTimestamp = NOW - 10.hours.inWholeMilliseconds
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)

val sut = createSut()
Expand All @@ -197,10 +214,11 @@ class WooPosFullSyncStatusCheckerTest {
}

@Test
fun `given sync at exact threshold, when checkSyncRequirement called, then should return Overdue`() =
fun `given sync at exact threshold, when checkSyncRequirement called, then should return NonBlockingRequired with isOverdue false`() =
runTest {
// GIVEN
val exactThresholdTimestamp = System.currentTimeMillis() - 7.days.inWholeMilliseconds
val exactThresholdTimestamp = NOW - 7.days.inWholeMilliseconds
whenever(time.now()).thenReturn(NOW)
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(exactThresholdTimestamp)

val sut = createSut()
Expand All @@ -209,7 +227,9 @@ class WooPosFullSyncStatusCheckerTest {
val result = sut.checkSyncRequirement()

// THEN
assertThat(result).isEqualTo(WooPosFullSyncRequirement.Overdue)
assertThat(
result
).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(exactThresholdTimestamp, isOverdue = false))
}

@Test
Expand All @@ -225,7 +245,7 @@ class WooPosFullSyncStatusCheckerTest {
val result = sut.checkSyncRequirement()

// THEN
assertThat(result).isInstanceOf(WooPosFullSyncRequirement.NotRequired::class.java)
assertThat(result).isInstanceOf(WooPosFullSyncRequirement.NonBlockingRequired::class.java)
}

@Test
Expand Down Expand Up @@ -257,7 +277,7 @@ class WooPosFullSyncStatusCheckerTest {
.thenReturn(Result.success(0))
whenever(checkCatalogSizeAction.execute(siteModel, null, 1000))
.thenReturn(WooPosCheckCatalogSizeAction.WooPosCheckCatalogSizeResult.SizeAcceptable)
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
val recentTimestamp = NOW - 1.days.inWholeMilliseconds
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)

val sut = createSut()
Expand All @@ -266,7 +286,7 @@ class WooPosFullSyncStatusCheckerTest {
val result = sut.checkSyncRequirement()

// THEN
assertThat(result).isEqualTo(WooPosFullSyncRequirement.NotRequired(recentTimestamp))
assertThat(result).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(recentTimestamp, false))
}

@Test
Expand All @@ -277,7 +297,7 @@ class WooPosFullSyncStatusCheckerTest {
.thenReturn(Result.success(0))
whenever(checkCatalogSizeAction.execute(siteModel, null, 1000))
.thenReturn(WooPosCheckCatalogSizeAction.WooPosCheckCatalogSizeResult.SizeUnknown)
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
val recentTimestamp = NOW - 1.days.inWholeMilliseconds
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)

val sut = createSut()
Expand All @@ -286,7 +306,7 @@ class WooPosFullSyncStatusCheckerTest {
val result = sut.checkSyncRequirement()

// THEN
assertThat(result).isEqualTo(WooPosFullSyncRequirement.NotRequired(recentTimestamp))
assertThat(result).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(recentTimestamp, false))
}

@Test
Expand All @@ -295,7 +315,7 @@ class WooPosFullSyncStatusCheckerTest {
// GIVEN
whenever(localCatalogStore.getProductCount(LocalOrRemoteId.LocalId(siteModel.id)))
.thenReturn(Result.success(100))
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
val recentTimestamp = NOW - 1.days.inWholeMilliseconds
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)

val sut = createSut()
Expand Down