Skip to content

Commit dbfd5b0

Browse files
samiuelsonsamiuelson
authored
Merge pull request #14894 from woocommerce/update-status-checker-states
[Woo POS][Local Catalog] Refine sync status checker states
2 parents d41963c + d24aeea commit dbfd5b0

File tree

6 files changed

+79
-49
lines changed

6 files changed

+79
-49
lines changed

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsViewModel.kt‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,20 @@ class WooPosItemsViewModel @Inject constructor(
6363
preferencesRepository.setWasOpenedOnce(true)
6464
}
6565

66-
checkSyncStatusAndUpdateBanner()
66+
refreshSyncOverdueBannerState()
6767
}
6868

69-
private fun checkSyncStatusAndUpdateBanner() {
69+
private fun refreshSyncOverdueBannerState() {
7070
viewModelScope.launch {
7171
val requirement = syncStatusChecker.checkSyncRequirement()
7272
_catalogSyncOverdueBannerState.value = when (requirement) {
73-
is WooPosFullSyncRequirement.Overdue -> CatalogSyncOverdueBannerState.Visible
73+
is WooPosFullSyncRequirement.NonBlockingRequired -> {
74+
if (requirement.isOverdue) {
75+
CatalogSyncOverdueBannerState.Visible
76+
} else {
77+
CatalogSyncOverdueBannerState.Hidden
78+
}
79+
}
7480
else -> CatalogSyncOverdueBannerState.Hidden
7581
}
7682
}

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSource.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class WooPosProductsDataSource @Inject constructor(
7474
}
7575

7676
is WooPosFullSyncRequirement.NotRequired,
77-
is WooPosFullSyncRequirement.Overdue -> {
77+
is WooPosFullSyncRequirement.NonBlockingRequired -> {
7878
activeSource = localDbDataSource
7979
emit(WooPosPrepopulatingDataStatus.Completed)
8080
}

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosFullSyncStatusChecker.kt‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import org.wordpress.android.fluxc.model.LocalOrRemoteId
99
import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosLocalCatalogStore
1010
import javax.inject.Inject
1111
import kotlin.time.Duration.Companion.days
12+
import kotlin.time.Duration.Companion.hours
1213

1314
class WooPosFullSyncStatusChecker @Inject constructor(
1415
private val syncTimestampManager: WooPosSyncTimestampManager,
@@ -18,7 +19,8 @@ class WooPosFullSyncStatusChecker @Inject constructor(
1819
private val prefsRepo: WooPosPreferencesRepository,
1920
private val checkCatalogSizeAction: WooPosCheckCatalogSizeAction,
2021
private val isLocalCatalogSupported: WooPosIsLocalCatalogSupported,
21-
private val wooPosLogWrapper: WooPosLogWrapper
22+
private val wooPosLogWrapper: WooPosLogWrapper,
23+
private val time: DateTimeProvider,
2224
) {
2325
@Suppress("ReturnCount")
2426
suspend fun checkSyncRequirement(): WooPosFullSyncRequirement {
@@ -57,43 +59,45 @@ class WooPosFullSyncStatusChecker @Inject constructor(
5759
return WooPosFullSyncRequirement.BlockingRequired
5860
}
5961

60-
return when {
61-
isFullSyncOverdue(lastFullSyncTimestamp) -> {
62-
if (!networkStatus.isConnected()) {
63-
wooPosLogWrapper.d(
64-
"Full sync overdue but offline - allowing POS to load with cached data " +
65-
"(${if (catalogIsEmpty) "empty catalog" else "$productCount products"})"
66-
)
67-
}
68-
wooPosLogWrapper.d("Full sync overdue (last sync: $lastFullSyncTimestamp)")
69-
WooPosFullSyncRequirement.Overdue
70-
}
62+
val now = time.now()
63+
val timeElapsedSinceLastSync = now - lastFullSyncTimestamp
7164

72-
else -> {
65+
return when {
66+
timeElapsedSinceLastSync < FULL_SYNC_NOT_REQUIRED_THRESHOLD -> {
7367
wooPosLogWrapper.d(
7468
"Full sync not required: Recent sync at $lastFullSyncTimestamp " +
7569
"(${if (catalogIsEmpty) "empty catalog" else "$productCount products"})"
7670
)
7771
WooPosFullSyncRequirement.NotRequired(lastFullSyncTimestamp)
7872
}
73+
else -> {
74+
if (!networkStatus.isConnected()) {
75+
wooPosLogWrapper.d(
76+
"Full sync required but offline - allowing POS to load with cached data " +
77+
"(${if (catalogIsEmpty) "empty catalog" else "$productCount products"})"
78+
)
79+
}
80+
val isFullSyncOverdue = timeElapsedSinceLastSync > FULL_SYNC_OVERDUE_THRESHOLD
81+
if (isFullSyncOverdue) {
82+
wooPosLogWrapper.d("Full sync overdue (last sync: $lastFullSyncTimestamp)")
83+
}
84+
WooPosFullSyncRequirement.NonBlockingRequired(lastFullSyncTimestamp, isFullSyncOverdue)
85+
}
7986
}
8087
}
8188

82-
private fun isFullSyncOverdue(lastSyncTimestamp: Long): Boolean {
83-
val currentTime = System.currentTimeMillis()
84-
val timeSinceLastSync = currentTime - lastSyncTimestamp
85-
val overdueThreshold = FULL_SYNC_OVERDUE_THRESHOLD.inWholeMilliseconds
86-
return timeSinceLastSync >= overdueThreshold
87-
}
88-
8989
companion object {
90-
private val FULL_SYNC_OVERDUE_THRESHOLD = 7.days
90+
private val FULL_SYNC_OVERDUE_THRESHOLD = 7.days.inWholeMilliseconds
91+
private val FULL_SYNC_NOT_REQUIRED_THRESHOLD = 23.hours.inWholeMilliseconds
9192
}
9293
}
9394

9495
sealed class WooPosFullSyncRequirement {
9596
data class NotRequired(val lastSyncTimestamp: Long) : WooPosFullSyncRequirement()
96-
data object Overdue : WooPosFullSyncRequirement()
97+
data class NonBlockingRequired(
98+
val lastSyncTimestamp: Long,
99+
val isOverdue: Boolean
100+
) : WooPosFullSyncRequirement()
97101
data object BlockingRequired : WooPosFullSyncRequirement()
98102
data class Error(val message: String) : WooPosFullSyncRequirement()
99103
data class LocalCatalogDisabled(val message: String) : WooPosFullSyncRequirement()

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorker.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ constructor(
109109
Result.retry()
110110
}
111111
is WooPosFullSyncRequirement.BlockingRequired,
112-
is WooPosFullSyncRequirement.Overdue -> {
112+
is WooPosFullSyncRequirement.NonBlockingRequired -> {
113113
logger.d("Proceeding with sync (requirement: $syncRequirement)")
114114
null
115115
}

‎WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSourceTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class WooPosProductsDataSourceTest {
7575
fun `given sync overdue, when prepopulate cache, then uses local db data source`() = runTest {
7676
// GIVEN
7777
whenever(syncStatusChecker.checkSyncRequirement()).thenReturn(
78-
WooPosFullSyncRequirement.Overdue
78+
WooPosFullSyncRequirement.NonBlockingRequired(lastSyncTimestamp = 0L, isOverdue = true)
7979
)
8080
whenever(localDbDataSource.fetchFirstProductsPage(false)).thenReturn(
8181
flowOf(ProductsResult.Remote(Result.success(emptyList())))

‎WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosFullSyncStatusCheckerTest.kt‎

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import org.wordpress.android.fluxc.model.SiteModel
2121
import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosLocalCatalogStore
2222
import kotlin.test.Test
2323
import kotlin.time.Duration.Companion.days
24+
import kotlin.time.Duration.Companion.hours
2425

2526
@ExperimentalCoroutinesApi
2627
class WooPosFullSyncStatusCheckerTest {
@@ -36,12 +37,17 @@ class WooPosFullSyncStatusCheckerTest {
3637
private val prefsRepo: WooPosPreferencesRepository = mock()
3738
private val checkCatalogSizeAction: WooPosCheckCatalogSizeAction = mock()
3839
private val isLocalCatalogSupported: WooPosIsLocalCatalogSupported = mock()
40+
private val time: DateTimeProvider = mock()
3941

4042
private val siteModel = SiteModel().apply {
4143
id = 123
4244
siteId = 456L
4345
}
4446

47+
companion object {
48+
private const val NOW = 1609459200000L // 2021-01-01 00:00:00 UTC
49+
}
50+
4551
private fun createSut() = WooPosFullSyncStatusChecker(
4652
syncTimestampManager = syncTimestampManager,
4753
selectedSite = selectedSite,
@@ -50,16 +56,18 @@ class WooPosFullSyncStatusCheckerTest {
5056
prefsRepo = prefsRepo,
5157
checkCatalogSizeAction = checkCatalogSizeAction,
5258
isLocalCatalogSupported = isLocalCatalogSupported,
53-
wooPosLogWrapper = wooPosLogWrapper
59+
wooPosLogWrapper = wooPosLogWrapper,
60+
time = time
5461
)
5562

5663
@Before
5764
fun setup() = runTest {
58-
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
65+
val recentTimestamp = NOW - 1.days.inWholeMilliseconds
5966
whenever(selectedSite.getOrNull()).thenReturn(siteModel)
6067
whenever(isLocalCatalogSupported(siteModel.localId())).thenReturn(true)
6168
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)
6269
whenever(networkStatus.isConnected()).thenReturn(true)
70+
whenever(time.now()).thenReturn(NOW)
6371
whenever(localCatalogStore.getProductCount(LocalOrRemoteId.LocalId(siteModel.id)))
6472
.thenReturn(Result.success(15))
6573
whenever(checkCatalogSizeAction.execute(siteModel, null, 1000))
@@ -129,10 +137,11 @@ class WooPosFullSyncStatusCheckerTest {
129137
}
130138

131139
@Test
132-
fun `given sync overdue and network connected, when checkSyncRequirement called, then should return Overdue`() =
140+
fun `given sync overdue and network connected, when checkSyncRequirement called, then should return NonBlockingRequired with isOverdue true`() =
133141
runTest {
134142
// GIVEN
135-
val overdueTimestamp = System.currentTimeMillis() - 8.days.inWholeMilliseconds
143+
val overdueTimestamp = NOW - 8.days.inWholeMilliseconds
144+
whenever(time.now()).thenReturn(NOW)
136145
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(overdueTimestamp)
137146
whenever(networkStatus.isConnected()).thenReturn(true)
138147

@@ -142,14 +151,17 @@ class WooPosFullSyncStatusCheckerTest {
142151
val result = sut.checkSyncRequirement()
143152

144153
// THEN
145-
assertThat(result).isEqualTo(WooPosFullSyncRequirement.Overdue)
154+
assertThat(
155+
result
156+
).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(overdueTimestamp, isOverdue = true))
146157
}
147158

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

@@ -159,14 +171,17 @@ class WooPosFullSyncStatusCheckerTest {
159171
val result = sut.checkSyncRequirement()
160172

161173
// THEN
162-
assertThat(result).isEqualTo(WooPosFullSyncRequirement.Overdue)
174+
assertThat(
175+
result
176+
).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(overdueTimestamp, isOverdue = true))
163177
}
164178

165179
@Test
166-
fun `given sync overdue with empty catalog and no network, when checkSyncRequirement called, then should return Overdue`() =
180+
fun `given sync overdue with empty catalog and no network, when checkSyncRequirement called, then should return NonBlockingRequired with isOverdue true`() =
167181
runTest {
168182
// GIVEN
169-
val overdueTimestamp = System.currentTimeMillis() - 8.days.inWholeMilliseconds
183+
val overdueTimestamp = NOW - 8.days.inWholeMilliseconds
184+
whenever(time.now()).thenReturn(NOW)
170185
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(overdueTimestamp)
171186
whenever(networkStatus.isConnected()).thenReturn(false)
172187
whenever(localCatalogStore.getProductCount(LocalOrRemoteId.LocalId(siteModel.id)))
@@ -178,14 +193,16 @@ class WooPosFullSyncStatusCheckerTest {
178193
val result = sut.checkSyncRequirement()
179194

180195
// THEN
181-
assertThat(result).isEqualTo(WooPosFullSyncRequirement.Overdue)
196+
assertThat(
197+
result
198+
).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(overdueTimestamp, isOverdue = true))
182199
}
183200

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

191208
val sut = createSut()
@@ -198,10 +215,11 @@ class WooPosFullSyncStatusCheckerTest {
198215
}
199216

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

207225
val sut = createSut()
@@ -210,7 +228,9 @@ class WooPosFullSyncStatusCheckerTest {
210228
val result = sut.checkSyncRequirement()
211229

212230
// THEN
213-
assertThat(result).isEqualTo(WooPosFullSyncRequirement.Overdue)
231+
assertThat(
232+
result
233+
).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(exactThresholdTimestamp, isOverdue = false))
214234
}
215235

216236
@Test
@@ -226,7 +246,7 @@ class WooPosFullSyncStatusCheckerTest {
226246
val result = sut.checkSyncRequirement()
227247

228248
// THEN
229-
assertThat(result).isInstanceOf(WooPosFullSyncRequirement.NotRequired::class.java)
249+
assertThat(result).isInstanceOf(WooPosFullSyncRequirement.NonBlockingRequired::class.java)
230250
}
231251

232252
@Test
@@ -258,7 +278,7 @@ class WooPosFullSyncStatusCheckerTest {
258278
.thenReturn(Result.success(0))
259279
whenever(checkCatalogSizeAction.execute(siteModel, null, 1000))
260280
.thenReturn(WooPosCheckCatalogSizeAction.WooPosCheckCatalogSizeResult.SizeAcceptable)
261-
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
281+
val recentTimestamp = NOW - 1.days.inWholeMilliseconds
262282
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)
263283

264284
val sut = createSut()
@@ -267,7 +287,7 @@ class WooPosFullSyncStatusCheckerTest {
267287
val result = sut.checkSyncRequirement()
268288

269289
// THEN
270-
assertThat(result).isEqualTo(WooPosFullSyncRequirement.NotRequired(recentTimestamp))
290+
assertThat(result).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(recentTimestamp, false))
271291
}
272292

273293
@Test
@@ -278,7 +298,7 @@ class WooPosFullSyncStatusCheckerTest {
278298
.thenReturn(Result.success(0))
279299
whenever(checkCatalogSizeAction.execute(siteModel, null, 1000))
280300
.thenReturn(WooPosCheckCatalogSizeAction.WooPosCheckCatalogSizeResult.SizeUnknown)
281-
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
301+
val recentTimestamp = NOW - 1.days.inWholeMilliseconds
282302
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)
283303

284304
val sut = createSut()
@@ -287,7 +307,7 @@ class WooPosFullSyncStatusCheckerTest {
287307
val result = sut.checkSyncRequirement()
288308

289309
// THEN
290-
assertThat(result).isEqualTo(WooPosFullSyncRequirement.NotRequired(recentTimestamp))
310+
assertThat(result).isEqualTo(WooPosFullSyncRequirement.NonBlockingRequired(recentTimestamp, false))
291311
}
292312

293313
@Test
@@ -296,7 +316,7 @@ class WooPosFullSyncStatusCheckerTest {
296316
// GIVEN
297317
whenever(localCatalogStore.getProductCount(LocalOrRemoteId.LocalId(siteModel.id)))
298318
.thenReturn(Result.success(100))
299-
val recentTimestamp = System.currentTimeMillis() - 1.days.inWholeMilliseconds
319+
val recentTimestamp = NOW - 1.days.inWholeMilliseconds
300320
whenever(syncTimestampManager.getFullSyncLastCompletedTimestamp()).thenReturn(recentTimestamp)
301321

302322
val sut = createSut()

0 commit comments

Comments
 (0)