Skip to content

Commit 2732588

Browse files
jkmasseljkmassel
authored
Refresh Stats on resume so the latest data appears automatically (#23112)
* Refresh Stats on resume so the latest data appears automatically The Subscribers, Insights, and Traffic stats use cases are @singleton, so their in-memory domainModel lives for the whole app process. Nothing refetched when the screen was re-displayed — onResume didn't refresh, and start() no-ops against an already-SUCCESS singleton. Once a stale or incomplete result was loaded (e.g. during a server-side stats aggregation lag), it was pinned and re-shown on every return with no network call, so server-side recovery never surfaced without a manual pull-to-refresh or a cold start. Trigger the existing non-forced refresh from StatsListFragment.onResume(). The store-level STALE_PERIOD (5 min) throttles this to at most one network request per 5 minutes, and the 50ms updateState debounce keeps within-window resumes from flashing a loading state. * Keep Stats rows visible while refreshing instead of blanking to a placeholder The onResume refresh (and pull-to-refresh) drive blocks through a LOADING state during the network round-trip. mapSubscribers and mapStatsWithOverview rendered LOADING from stateData only (the loading placeholder = a bare title), and mapInsights used `stateData ?: data` — but stateData is always non-null (buildLoadingItem returns a title), so all three discarded the already-loaded rows and blanked the blocks for the duration of the fetch. Prefer data over the placeholder on LOADING (data ?: stateData ?: listOf()), matching StatsViewAllViewModel. First load still shows the placeholder (no data yet); a refresh of already-loaded data keeps the rows on screen. Add regression tests for mapSubscribers. * Prevent duplicate concurrent Stats loads with an in-flight guard On first display the initial start() load (from onViewCreated) and the onResume refresh both fire and race to the network before either records the StatsRequest timestamp, so STALE_PERIOD doesn't dedup them. Add a per-use-case AtomicBoolean guard in BaseStatsUseCase.fetch: a second concurrent non-forced fetch is skipped while one is in flight. A forced refresh (pull-to-refresh) still proceeds so it can bypass STALE_PERIOD. Keeps offscreen tab preloading (start() unchanged) while removing the first-load double-fetch; also covers other load races.
1 parent b4564b2 commit 2732588

6 files changed

Lines changed: 115 additions & 10 deletions

File tree

‎RELEASE-NOTES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
27.0
44
-----
55
* [*] You can now browse Google Photos (albums, collections, and search) when adding photos or videos from your device.
6+
* [*] Stats now refresh when you return to the screen, so your latest data appears without a manual pull-to-refresh. [https://github.com/wordpress-mobile/WordPress-Android/pull/23112]
67

78

89
26.9

‎WordPress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/StatsListFragment.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,14 @@ class StatsListFragment : ViewPagerFragment(R.layout.stats_list_fragment), PullT
212212
@Suppress("DEPRECATION")
213213
setHasOptionsMenu(statsSection == StatsSection.INSIGHTS)
214214
(parentFragment as? StatsPullToRefreshListener.PullToRefreshReceiverListener)?.setPullToRefreshReceiver(this)
215+
// Re-fetch when the user returns to the screen so newly-available (or previously stale) stats
216+
// appear without a manual pull-to-refresh. The stats use cases are process-lifetime singletons
217+
// that otherwise keep serving their last in-memory result. This uses the non-forced refresh
218+
// path, so StatsRequestSqlUtils.STALE_PERIOD throttles it to at most one network request per
219+
// 5 minutes; resumes within that window are served from cache.
220+
if (::viewModel.isInitialized) {
221+
viewModel.onRefresh()
222+
}
215223
}
216224

217225
override fun onDestroyView() {

‎WordPress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/UiModelMapper.kt‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ class UiModelMapper
3131
)
3232
LOADING -> StatsBlock.Loading(
3333
useCaseModel.type,
34-
useCaseModel.stateData ?: useCaseModel.data ?: listOf()
34+
// Keep already-loaded rows visible during a refresh; fall back to the
35+
// loading placeholder only on first load (no data yet).
36+
useCaseModel.data ?: useCaseModel.stateData ?: listOf()
3537
)
3638
EMPTY -> StatsBlock.EmptyBlock(
3739
useCaseModel.type,
@@ -70,9 +72,12 @@ class UiModelMapper
7072
} else if (!allFailing && !allFailingWithoutData) {
7173
val data = useCaseModels.mapNotNull { useCaseModel ->
7274
when (useCaseModel.state) {
73-
LOADING -> useCaseModel.stateData?.let {
74-
StatsBlock.Loading(useCaseModel.type, useCaseModel.stateData)
75-
}
75+
// Keep already-loaded rows visible during a refresh; fall back to the loading
76+
// placeholder only on first load (no data yet).
77+
LOADING -> StatsBlock.Loading(
78+
useCaseModel.type,
79+
useCaseModel.data ?: useCaseModel.stateData ?: listOf()
80+
)
7681

7782
SUCCESS -> StatsBlock.Success(useCaseModel.type, useCaseModel.data ?: listOf())
7883
ERROR -> useCaseModel.stateData?.let {
@@ -114,9 +119,12 @@ class UiModelMapper
114119
UiModel.Success(
115120
useCaseModels.mapNotNull { useCaseModel ->
116121
when {
117-
useCaseModel.state == LOADING -> useCaseModel.stateData?.let {
118-
StatsBlock.Loading(useCaseModel.type, useCaseModel.stateData)
119-
}
122+
// Keep already-loaded rows visible during a refresh; fall back to the
123+
// loading placeholder only on first load (no data yet).
124+
useCaseModel.state == LOADING -> StatsBlock.Loading(
125+
useCaseModel.type,
126+
useCaseModel.data ?: useCaseModel.stateData ?: listOf()
127+
)
120128

121129
useCaseModel.type == overViewType && useCaseModel.data != null -> StatsBlock.Success(
122130
useCaseModel.type,

‎WordPress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/sections/BaseStatsUseCase.kt‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package org.wordpress.android.ui.stats.refresh.lists.sections
22

33
import androidx.lifecycle.LiveData
44
import androidx.lifecycle.MutableLiveData
5+
import java.util.concurrent.atomic.AtomicBoolean
56
import kotlinx.coroutines.CoroutineDispatcher
67
import kotlinx.coroutines.CoroutineScope
78
import kotlinx.coroutines.Job
@@ -44,6 +45,7 @@ abstract class BaseStatsUseCase<DOMAIN_MODEL, UI_STATE>(
4445
private var domainModel: DOMAIN_MODEL? = null
4546
private var uiState: UI_STATE = defaultUiState
4647
private var updateJob: Job? = null
48+
private val fetchInProgress = AtomicBoolean(false)
4749

4850
private val _liveData = MutableLiveData<UseCaseModel>()
4951
val liveData: LiveData<UseCaseModel> = _liveData
@@ -72,9 +74,22 @@ abstract class BaseStatsUseCase<DOMAIN_MODEL, UI_STATE>(
7274
}
7375
}
7476
if (refresh || domainState != SUCCESS || emptyDb) {
77+
// Guard against duplicate concurrent loads — e.g. the initial start() load racing the
78+
// onResume refresh. A forced refresh (pull-to-refresh) still proceeds so it can bypass
79+
// the STALE_PERIOD cache.
80+
val startedFetch = fetchInProgress.compareAndSet(false, true)
81+
if (!startedFetch && !forced) {
82+
return
83+
}
7584
updateUseCaseState(LOADING)
76-
val state = fetchRemoteData(forced)
77-
evaluateState(state)
85+
try {
86+
val state = fetchRemoteData(forced)
87+
evaluateState(state)
88+
} finally {
89+
if (startedFetch) {
90+
fetchInProgress.set(false)
91+
}
92+
}
7893
}
7994
}
8095

‎WordPress/src/test/java/org/wordpress/android/ui/stats/refresh/lists/UiModelMapperTest.kt‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@ import org.wordpress.android.BaseUnitTest
99
import org.wordpress.android.R
1010
import org.wordpress.android.fluxc.store.StatsStore.InsightType.TOTAL_FOLLOWERS
1111
import org.wordpress.android.fluxc.store.StatsStore.ManagementType
12+
import org.wordpress.android.fluxc.store.StatsStore.SubscriberType.EMAILS
1213
import org.wordpress.android.ui.stats.refresh.lists.StatsListViewModel.UiModel
1314
import org.wordpress.android.ui.stats.refresh.lists.sections.BaseStatsUseCase.UseCaseModel
15+
import org.wordpress.android.ui.stats.refresh.lists.sections.BaseStatsUseCase.UseCaseModel.UseCaseState.LOADING
1416
import org.wordpress.android.ui.stats.refresh.lists.sections.BaseStatsUseCase.UseCaseModel.UseCaseState.SUCCESS
17+
import org.wordpress.android.ui.stats.refresh.lists.sections.BlockListItem
1518
import org.wordpress.android.util.NetworkUtilsWrapper
1619

1720
@ExperimentalCoroutinesApi
@@ -66,4 +69,34 @@ import org.wordpress.android.util.NetworkUtilsWrapper
6669
assertThat(model.showButton).isTrue()
6770
assertThat(error).isNull()
6871
}
72+
73+
@Test
74+
fun `mapSubscribers keeps loaded rows visible while a block is refreshing`() {
75+
val dataRows = listOf(BlockListItem.Divider)
76+
val loadingPlaceholder = listOf(BlockListItem.Divider)
77+
78+
val uiModel = mapper.mapSubscribers(
79+
listOf(UseCaseModel(EMAILS, data = dataRows, stateData = loadingPlaceholder, state = LOADING))
80+
) {}
81+
82+
val model = uiModel as UiModel.Success
83+
assertThat(model.data).hasSize(1)
84+
assertThat(model.data[0].type).isEqualTo(StatsBlock.Type.LOADING)
85+
// The already-loaded rows stay on screen during the refresh, not the loading placeholder.
86+
assertThat(model.data[0].data).isSameAs(dataRows)
87+
}
88+
89+
@Test
90+
fun `mapSubscribers shows the loading placeholder on first load when there is no data yet`() {
91+
val loadingPlaceholder = listOf(BlockListItem.Divider)
92+
93+
val uiModel = mapper.mapSubscribers(
94+
listOf(UseCaseModel(EMAILS, data = null, stateData = loadingPlaceholder, state = LOADING))
95+
) {}
96+
97+
val model = uiModel as UiModel.Success
98+
assertThat(model.data).hasSize(1)
99+
assertThat(model.data[0].type).isEqualTo(StatsBlock.Type.LOADING)
100+
assertThat(model.data[0].data).isSameAs(loadingPlaceholder)
101+
}
69102
}

‎WordPress/src/test/java/org/wordpress/android/ui/stats/refresh/lists/sections/BaseStatsUseCaseTest.kt‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
package org.wordpress.android.ui.stats.refresh.lists.sections
22

3+
import kotlinx.coroutines.CompletableDeferred
34
import kotlinx.coroutines.ExperimentalCoroutinesApi
5+
import kotlinx.coroutines.launch
46
import kotlinx.coroutines.test.UnconfinedTestDispatcher
57
import kotlinx.coroutines.test.advanceUntilIdle
68
import org.assertj.core.api.Assertions.assertThat
79
import org.junit.After
810
import org.junit.Before
911
import org.junit.Test
1012
import org.mockito.Mock
13+
import org.mockito.kotlin.times
14+
import org.mockito.kotlin.verify
1115
import org.mockito.kotlin.whenever
1216
import org.wordpress.android.BaseUnitTest
1317
import org.wordpress.android.R
@@ -95,6 +99,40 @@ class BaseStatsUseCaseTest : BaseUnitTest() {
9599
assertThat(block.liveData.value?.state).isEqualTo(UseCaseState.LOADING)
96100
}
97101

102+
@Test
103+
fun `concurrent non-forced fetches only trigger one remote load`() = test {
104+
whenever(localDataProvider.get()).thenReturn(null)
105+
val gate = CompletableDeferred<Unit>()
106+
val gatedBlock = TestUseCase(localDataProvider, remoteDataProvider, loadingData, gate)
107+
108+
launch { gatedBlock.fetch(false, false) } // acquires the in-flight guard, suspends at the gate
109+
advanceUntilIdle()
110+
launch { gatedBlock.fetch(true, false) } // a load is already in flight -> should be skipped
111+
advanceUntilIdle()
112+
gate.complete(Unit)
113+
advanceUntilIdle()
114+
115+
verify(remoteDataProvider, times(1)).get()
116+
gatedBlock.clear()
117+
}
118+
119+
@Test
120+
fun `a forced fetch is not skipped while a load is in flight`() = test {
121+
whenever(localDataProvider.get()).thenReturn(null)
122+
val gate = CompletableDeferred<Unit>()
123+
val gatedBlock = TestUseCase(localDataProvider, remoteDataProvider, loadingData, gate)
124+
125+
launch { gatedBlock.fetch(false, false) } // non-forced load in flight
126+
advanceUntilIdle()
127+
launch { gatedBlock.fetch(true, true) } // forced (pull-to-refresh) must still hit remote
128+
advanceUntilIdle()
129+
gate.complete(Unit)
130+
advanceUntilIdle()
131+
132+
verify(remoteDataProvider, times(2)).get()
133+
gatedBlock.clear()
134+
}
135+
98136
@After
99137
fun tearDown() {
100138
block.clear()
@@ -109,7 +147,8 @@ class BaseStatsUseCaseTest : BaseUnitTest() {
109147
class TestUseCase(
110148
private val localDataProvider: Provider<String?>,
111149
private val remoteDataProvider: Provider<String?>,
112-
private val loadingItems: List<BlockListItem>
150+
private val loadingItems: List<BlockListItem>,
151+
private val remoteGate: CompletableDeferred<Unit>? = null
113152
) : BaseStatsUseCase<String, Int>(
114153
ALL_TIME_STATS,
115154
UnconfinedTestDispatcher(),
@@ -130,6 +169,7 @@ class BaseStatsUseCaseTest : BaseUnitTest() {
130169
}
131170

132171
override suspend fun fetchRemoteData(forced: Boolean): State<String> {
172+
remoteGate?.await()
133173
val domainModel = remoteDataProvider.get()
134174
return if (domainModel != null) {
135175
State.Data(domainModel)

0 commit comments

Comments
 (0)