Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
27.0
-----
* [*] You can now browse Google Photos (albums, collections, and search) when adding photos or videos from your device.
* [*] 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]


26.9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,14 @@ class StatsListFragment : ViewPagerFragment(R.layout.stats_list_fragment), PullT
@Suppress("DEPRECATION")
setHasOptionsMenu(statsSection == StatsSection.INSIGHTS)
(parentFragment as? StatsPullToRefreshListener.PullToRefreshReceiverListener)?.setPullToRefreshReceiver(this)
// Re-fetch when the user returns to the screen so newly-available (or previously stale) stats
// appear without a manual pull-to-refresh. The stats use cases are process-lifetime singletons
// that otherwise keep serving their last in-memory result. This uses the non-forced refresh
// path, so StatsRequestSqlUtils.STALE_PERIOD throttles it to at most one network request per
// 5 minutes; resumes within that window are served from cache.
if (::viewModel.isInitialized) {
viewModel.onRefresh()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️ The first run will be making two refresh calls now. One in viewmodel.start() called from onViewCreated()and one in onResume()

}
}

override fun onDestroyView() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ class UiModelMapper
)
LOADING -> StatsBlock.Loading(
useCaseModel.type,
useCaseModel.stateData ?: useCaseModel.data ?: listOf()
// Keep already-loaded rows visible during a refresh; fall back to the
// loading placeholder only on first load (no data yet).
useCaseModel.data ?: useCaseModel.stateData ?: listOf()
)
EMPTY -> StatsBlock.EmptyBlock(
useCaseModel.type,
Expand Down Expand Up @@ -70,9 +72,12 @@ class UiModelMapper
} else if (!allFailing && !allFailingWithoutData) {
val data = useCaseModels.mapNotNull { useCaseModel ->
when (useCaseModel.state) {
LOADING -> useCaseModel.stateData?.let {
StatsBlock.Loading(useCaseModel.type, useCaseModel.stateData)
}
// Keep already-loaded rows visible during a refresh; fall back to the loading
// placeholder only on first load (no data yet).
LOADING -> StatsBlock.Loading(
useCaseModel.type,
useCaseModel.data ?: useCaseModel.stateData ?: listOf()
)

SUCCESS -> StatsBlock.Success(useCaseModel.type, useCaseModel.data ?: listOf())
ERROR -> useCaseModel.stateData?.let {
Expand Down Expand Up @@ -114,9 +119,12 @@ class UiModelMapper
UiModel.Success(
useCaseModels.mapNotNull { useCaseModel ->
when {
useCaseModel.state == LOADING -> useCaseModel.stateData?.let {
StatsBlock.Loading(useCaseModel.type, useCaseModel.stateData)
}
// Keep already-loaded rows visible during a refresh; fall back to the
// loading placeholder only on first load (no data yet).
useCaseModel.state == LOADING -> StatsBlock.Loading(
useCaseModel.type,
useCaseModel.data ?: useCaseModel.stateData ?: listOf()
)

useCaseModel.type == overViewType && useCaseModel.data != null -> StatsBlock.Success(
useCaseModel.type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.wordpress.android.ui.stats.refresh.lists.sections

import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import java.util.concurrent.atomic.AtomicBoolean
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
Expand Down Expand Up @@ -44,6 +45,7 @@ abstract class BaseStatsUseCase<DOMAIN_MODEL, UI_STATE>(
private var domainModel: DOMAIN_MODEL? = null
private var uiState: UI_STATE = defaultUiState
private var updateJob: Job? = null
private val fetchInProgress = AtomicBoolean(false)

private val _liveData = MutableLiveData<UseCaseModel>()
val liveData: LiveData<UseCaseModel> = _liveData
Expand Down Expand Up @@ -72,9 +74,22 @@ abstract class BaseStatsUseCase<DOMAIN_MODEL, UI_STATE>(
}
}
if (refresh || domainState != SUCCESS || emptyDb) {
// Guard against duplicate concurrent loads — e.g. the initial start() load racing the
// onResume refresh. A forced refresh (pull-to-refresh) still proceeds so it can bypass
// the STALE_PERIOD cache.
val startedFetch = fetchInProgress.compareAndSet(false, true)
if (!startedFetch && !forced) {
return
}
updateUseCaseState(LOADING)
val state = fetchRemoteData(forced)
evaluateState(state)
try {
val state = fetchRemoteData(forced)
evaluateState(state)
} finally {
if (startedFetch) {
fetchInProgress.set(false)
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import org.wordpress.android.BaseUnitTest
import org.wordpress.android.R
import org.wordpress.android.fluxc.store.StatsStore.InsightType.TOTAL_FOLLOWERS
import org.wordpress.android.fluxc.store.StatsStore.ManagementType
import org.wordpress.android.fluxc.store.StatsStore.SubscriberType.EMAILS
import org.wordpress.android.ui.stats.refresh.lists.StatsListViewModel.UiModel
import org.wordpress.android.ui.stats.refresh.lists.sections.BaseStatsUseCase.UseCaseModel
import org.wordpress.android.ui.stats.refresh.lists.sections.BaseStatsUseCase.UseCaseModel.UseCaseState.LOADING
import org.wordpress.android.ui.stats.refresh.lists.sections.BaseStatsUseCase.UseCaseModel.UseCaseState.SUCCESS
import org.wordpress.android.ui.stats.refresh.lists.sections.BlockListItem
import org.wordpress.android.util.NetworkUtilsWrapper

@ExperimentalCoroutinesApi
Expand Down Expand Up @@ -66,4 +69,34 @@ import org.wordpress.android.util.NetworkUtilsWrapper
assertThat(model.showButton).isTrue()
assertThat(error).isNull()
}

@Test
fun `mapSubscribers keeps loaded rows visible while a block is refreshing`() {
val dataRows = listOf(BlockListItem.Divider)
val loadingPlaceholder = listOf(BlockListItem.Divider)

val uiModel = mapper.mapSubscribers(
listOf(UseCaseModel(EMAILS, data = dataRows, stateData = loadingPlaceholder, state = LOADING))
) {}

val model = uiModel as UiModel.Success
assertThat(model.data).hasSize(1)
assertThat(model.data[0].type).isEqualTo(StatsBlock.Type.LOADING)
// The already-loaded rows stay on screen during the refresh, not the loading placeholder.
assertThat(model.data[0].data).isSameAs(dataRows)
}

@Test
fun `mapSubscribers shows the loading placeholder on first load when there is no data yet`() {
val loadingPlaceholder = listOf(BlockListItem.Divider)

val uiModel = mapper.mapSubscribers(
listOf(UseCaseModel(EMAILS, data = null, stateData = loadingPlaceholder, state = LOADING))
) {}

val model = uiModel as UiModel.Success
assertThat(model.data).hasSize(1)
assertThat(model.data[0].type).isEqualTo(StatsBlock.Type.LOADING)
assertThat(model.data[0].data).isSameAs(loadingPlaceholder)
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package org.wordpress.android.ui.stats.refresh.lists.sections

import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.advanceUntilIdle
import org.assertj.core.api.Assertions.assertThat
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.mockito.Mock
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import org.wordpress.android.BaseUnitTest
import org.wordpress.android.R
Expand Down Expand Up @@ -95,6 +99,40 @@ class BaseStatsUseCaseTest : BaseUnitTest() {
assertThat(block.liveData.value?.state).isEqualTo(UseCaseState.LOADING)
}

@Test
fun `concurrent non-forced fetches only trigger one remote load`() = test {
whenever(localDataProvider.get()).thenReturn(null)
val gate = CompletableDeferred<Unit>()
val gatedBlock = TestUseCase(localDataProvider, remoteDataProvider, loadingData, gate)

launch { gatedBlock.fetch(false, false) } // acquires the in-flight guard, suspends at the gate
advanceUntilIdle()
launch { gatedBlock.fetch(true, false) } // a load is already in flight -> should be skipped
advanceUntilIdle()
gate.complete(Unit)
advanceUntilIdle()

verify(remoteDataProvider, times(1)).get()
gatedBlock.clear()
}

@Test
fun `a forced fetch is not skipped while a load is in flight`() = test {
whenever(localDataProvider.get()).thenReturn(null)
val gate = CompletableDeferred<Unit>()
val gatedBlock = TestUseCase(localDataProvider, remoteDataProvider, loadingData, gate)

launch { gatedBlock.fetch(false, false) } // non-forced load in flight
advanceUntilIdle()
launch { gatedBlock.fetch(true, true) } // forced (pull-to-refresh) must still hit remote
advanceUntilIdle()
gate.complete(Unit)
advanceUntilIdle()

verify(remoteDataProvider, times(2)).get()
gatedBlock.clear()
}

@After
fun tearDown() {
block.clear()
Expand All @@ -109,7 +147,8 @@ class BaseStatsUseCaseTest : BaseUnitTest() {
class TestUseCase(
private val localDataProvider: Provider<String?>,
private val remoteDataProvider: Provider<String?>,
private val loadingItems: List<BlockListItem>
private val loadingItems: List<BlockListItem>,
private val remoteGate: CompletableDeferred<Unit>? = null
) : BaseStatsUseCase<String, Int>(
ALL_TIME_STATS,
UnconfinedTestDispatcher(),
Expand All @@ -130,6 +169,7 @@ class BaseStatsUseCaseTest : BaseUnitTest() {
}

override suspend fun fetchRemoteData(forced: Boolean): State<String> {
remoteGate?.await()
val domainModel = remoteDataProvider.get()
return if (domainModel != null) {
State.Data(domainModel)
Expand Down
Loading