Merge main into test-275-lane5-followup-screens
This commit is contained in:
@@ -83,6 +83,10 @@ class FakeMailRepository(
|
||||
private val unreadCounts: List<UnreadCount> = emptyList(),
|
||||
drafts: List<Draft> = emptyList(),
|
||||
outbox: List<OutboxMessage> = emptyList(),
|
||||
// When set, every paged query returns this instead of a static page over [messages]. Lets a UI test
|
||||
// drive an explicit LoadState (e.g. refresh = Loading) through collectAsLazyPagingItems to exercise
|
||||
// the empty-state gate (issue #219).
|
||||
private val pagedOverride: Flow<PagingData<Message>>? = null,
|
||||
) : MailRepository {
|
||||
|
||||
val sentMessages = mutableListOf<OutgoingMessage>()
|
||||
@@ -104,19 +108,21 @@ class FakeMailRepository(
|
||||
private val outboxFlow = MutableStateFlow(outbox)
|
||||
|
||||
override fun pagedUnifiedFolderMessages(folder: String): Flow<PagingData<Message>> =
|
||||
flowOf(PagingData.from(messages.filter { it.folder == folder && it.inInbox }))
|
||||
pagedOverride ?: flowOf(PagingData.from(messages.filter { it.folder == folder && it.inInbox }))
|
||||
|
||||
override fun pagedFolderMessages(accountId: String, folder: String): Flow<PagingData<Message>> =
|
||||
flowOf(PagingData.from(messages.filter { it.accountId == accountId && it.folder == folder && it.inInbox }))
|
||||
pagedOverride ?: flowOf(
|
||||
PagingData.from(messages.filter { it.accountId == accountId && it.folder == folder && it.inInbox }),
|
||||
)
|
||||
|
||||
override fun pagedUnifiedSearchMessages(folder: String, query: String): Flow<PagingData<Message>> =
|
||||
flowOf(PagingData.from(messages.filter { it.folder == folder && it.matchesSearch(query) }))
|
||||
pagedOverride ?: flowOf(PagingData.from(messages.filter { it.folder == folder && it.matchesSearch(query) }))
|
||||
|
||||
override fun pagedFolderSearchMessages(
|
||||
accountId: String,
|
||||
folder: String,
|
||||
query: String,
|
||||
): Flow<PagingData<Message>> = flowOf(
|
||||
): Flow<PagingData<Message>> = pagedOverride ?: flowOf(
|
||||
PagingData.from(
|
||||
messages.filter { it.accountId == accountId && it.folder == folder && it.matchesSearch(query) },
|
||||
),
|
||||
|
||||
@@ -14,7 +14,11 @@ import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performTouchInput
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.paging.LoadState
|
||||
import androidx.paging.LoadStates
|
||||
import androidx.paging.PagingData
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
@@ -284,4 +288,53 @@ class MailboxScreenTest {
|
||||
|
||||
composeTestRule.onNodeWithContentDescription(string(R.string.message_available_offline)).assertIsDisplayed()
|
||||
}
|
||||
|
||||
// Issue #219: on return from the reader the inbox's LazyPagingItems is cold — it presents
|
||||
// itemCount == 0 with refresh == Loading before the window repopulates. The empty-state gate must
|
||||
// hold "No messages yet" back through that window, so the empty state never flashes.
|
||||
@Test
|
||||
fun emptyState_isHidden_whileTheInboxPagerIsStillLoading() {
|
||||
val loadingEmpty = flowOf(
|
||||
PagingData.from(
|
||||
emptyList<Message>(),
|
||||
LoadStates(
|
||||
refresh = LoadState.Loading,
|
||||
prepend = LoadState.NotLoading(endOfPaginationReached = false),
|
||||
append = LoadState.NotLoading(endOfPaginationReached = false),
|
||||
),
|
||||
),
|
||||
)
|
||||
setContent(FakeMailRepository(pagedOverride = loadingEmpty))
|
||||
|
||||
// Assert only the gate — no positive "screen composed" anchor. Under a never-completing
|
||||
// refresh == Loading pager the compose tree never settles deterministically, so no node (not even
|
||||
// the always-present FAB) is reliable to assert *present* here: it flaked as not-displayed
|
||||
// (assertIsDisplayed), not-found (assertExists), and waitForText-timeout across CI runs.
|
||||
// The gate under test (issue #219): the empty state stays hidden while refresh == Loading, so
|
||||
// "No messages yet" never flashes on return from the reader. This is the stable, meaningful
|
||||
// assertion — NoMessagesState is composed only once the pager is "settled" (refresh NotLoading
|
||||
// AND append end-of-pagination reached), which this frozen Loading state never reaches, so the
|
||||
// string can never appear regardless of when the tree happens to settle.
|
||||
composeTestRule.onNodeWithText(string(R.string.mailbox_empty)).assertDoesNotExist()
|
||||
}
|
||||
|
||||
// The flip side of the gate: once the pager settles (refresh done, no further page) with no rows,
|
||||
// the inbox really is empty and "No messages yet" must show.
|
||||
@Test
|
||||
fun emptyState_isShown_onceTheInboxPagerSettlesEmpty() {
|
||||
val settledEmpty = flowOf(
|
||||
PagingData.from(
|
||||
emptyList<Message>(),
|
||||
LoadStates(
|
||||
refresh = LoadState.NotLoading(endOfPaginationReached = false),
|
||||
prepend = LoadState.NotLoading(endOfPaginationReached = true),
|
||||
append = LoadState.NotLoading(endOfPaginationReached = true),
|
||||
),
|
||||
),
|
||||
)
|
||||
setContent(FakeMailRepository(pagedOverride = settledEmpty))
|
||||
|
||||
waitForText(string(R.string.mailbox_empty))
|
||||
composeTestRule.onNodeWithText(string(R.string.mailbox_empty)).assertIsDisplayed()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,8 @@ import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import androidx.paging.PagingData
|
||||
import androidx.paging.PagingDataEvent
|
||||
import androidx.paging.PagingDataPresenter
|
||||
import androidx.paging.cachedIn
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
@@ -15,6 +17,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.collectLatest
|
||||
import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.debounce
|
||||
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||
@@ -150,6 +153,20 @@ class MailboxViewModel @Inject constructor(
|
||||
}
|
||||
.cachedIn(viewModelScope)
|
||||
|
||||
/**
|
||||
* An always-on, invisible presenter that holds [pagedMessages]'s cached pager open across a reader
|
||||
* visit (issue #219). `collectAsLazyPagingItems` is the mailbox's only presenter, so it is torn down
|
||||
* the instant the user opens a message — leaving the `cachedIn` pager with no downstream collector.
|
||||
* Opening an unread message writes `setRead`, invalidating the Room `PagingSource`; with nothing
|
||||
* collecting, the fresh generation would cold-load only once the mailbox re-enters composition,
|
||||
* flashing the empty state and stalling for a frame. Keeping this second collector subscribed lets
|
||||
* that generation load in the background while the reader is up, so the return replays a full window
|
||||
* with no empty frame. It renders nothing — the UI's `LazyPagingItems` still drives the visible list.
|
||||
*/
|
||||
private val keepAlivePresenter = object : PagingDataPresenter<Message>() {
|
||||
override suspend fun presentPagingDataEvent(event: PagingDataEvent<Message>) = Unit
|
||||
}
|
||||
|
||||
val draftCount: StateFlow<Int> = mailRepository.observeDrafts()
|
||||
.map { it.size }
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), 0)
|
||||
@@ -319,6 +336,13 @@ class MailboxViewModel @Inject constructor(
|
||||
}
|
||||
|
||||
init {
|
||||
// Keep the cached pager warm across reader visits (issue #219). collectLatest hands each new
|
||||
// generation — including the one produced when opening a message invalidates the source via
|
||||
// setRead — to the presenter, which drives its initial load in the background so the return to
|
||||
// the inbox never cold-reloads or flashes the empty state.
|
||||
viewModelScope.launch {
|
||||
pagedMessages.collectLatest { keepAlivePresenter.collectFrom(it) }
|
||||
}
|
||||
// Fall back to the unified inbox if the filtered account is removed. The list.isNotEmpty()
|
||||
// guard avoids clobbering a seeded account filter during the initial empty emission (before
|
||||
// the account list first loads from the database).
|
||||
|
||||
@@ -2,7 +2,11 @@
|
||||
package org.libremail.ui.mailbox
|
||||
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.paging.Pager
|
||||
import androidx.paging.PagingConfig
|
||||
import androidx.paging.PagingData
|
||||
import androidx.paging.PagingSource
|
||||
import androidx.paging.PagingState
|
||||
import app.cash.turbine.test
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
@@ -18,6 +22,7 @@ import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.UnconfinedTestDispatcher
|
||||
import kotlinx.coroutines.test.advanceUntilIdle
|
||||
import kotlinx.coroutines.test.resetMain
|
||||
import kotlinx.coroutines.test.runCurrent
|
||||
import kotlinx.coroutines.test.runTest
|
||||
@@ -38,6 +43,7 @@ import org.libremail.domain.model.UnreadCount
|
||||
import org.libremail.domain.repository.AccountRepository
|
||||
import org.libremail.domain.repository.MailRepository
|
||||
import org.libremail.ui.navigation.Routes
|
||||
import java.util.concurrent.atomic.AtomicInteger
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
@@ -88,6 +94,44 @@ class MailboxViewModelTest {
|
||||
verify { repo.pagedFolderSearchMessages("imap:a", "INBOX", "world") }
|
||||
}
|
||||
|
||||
// Issue #219: returning from the reader must not cold-reload the inbox. The mailbox's only paging
|
||||
// presenter (collectAsLazyPagingItems) is gone while a message is open, so opening an unread message
|
||||
// — which writes setRead and invalidates the Room PagingSource — would otherwise leave the fresh
|
||||
// generation to cold-load on return, flashing "No messages". The ViewModel keeps its own always-on
|
||||
// collector on the cached pager so that generation instead loads in the background while reading.
|
||||
@Test
|
||||
fun `keeps the cached pager warm so a post-setRead invalidation reloads with no UI collector`() =
|
||||
runTest(testDispatcher) {
|
||||
val loads = AtomicInteger(0)
|
||||
val sources = mutableListOf<PagingSource<Int, Message>>()
|
||||
val warmInbox = Pager(PagingConfig(pageSize = 20, enablePlaceholders = false)) {
|
||||
countingSource(listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX")), loads).also { sources += it }
|
||||
}.flow
|
||||
val repo = mockk<MailRepository>(relaxed = true)
|
||||
every { repo.observeDrafts() } returns flowOf(emptyList())
|
||||
every { repo.observeOutbox() } returns flowOf(emptyList())
|
||||
every { repo.observeUnreadCounts() } returns MutableStateFlow(emptyList())
|
||||
every { repo.observeFolders(any()) } returns MutableStateFlow(emptyList())
|
||||
every { repo.pagedUnifiedFolderMessages(any()) } returns warmInbox
|
||||
val accountRepository = mockk<AccountRepository>(relaxed = true)
|
||||
every { accountRepository.observeAccounts() } returns MutableStateFlow(listOf(alice))
|
||||
|
||||
// Construct with NO collector on vm.pagedMessages — exactly the reader-is-on-top state.
|
||||
val vm = MailboxViewModel(repo, accountRepository, mockk(relaxed = true), SavedStateHandle())
|
||||
advanceUntilIdle()
|
||||
|
||||
// The keep-alive presenter drove the initial page load with no LazyPagingItems attached.
|
||||
assertEquals(1, loads.get())
|
||||
|
||||
// Opening an unread message writes setRead, invalidating the source → a new generation.
|
||||
sources.last().invalidate()
|
||||
advanceUntilIdle()
|
||||
|
||||
// That generation loaded in the background, so the return replays a full window: no cold
|
||||
// reload, no empty-state flash.
|
||||
assertEquals(2, loads.get())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `selecting a concrete account pages that account's folder`() = runTest(testDispatcher) {
|
||||
val repo = mockk<MailRepository>(relaxed = true)
|
||||
@@ -653,6 +697,16 @@ class MailboxViewModelTest {
|
||||
return MailboxViewModel(repo, accountRepository, syncer, savedState)
|
||||
}
|
||||
|
||||
/** An in-memory [PagingSource] that counts loads, so a test can watch the pager (re)load a generation. */
|
||||
private fun countingSource(data: List<Message>, loads: AtomicInteger) = object : PagingSource<Int, Message>() {
|
||||
override suspend fun load(params: LoadParams<Int>): LoadResult<Int, Message> {
|
||||
loads.incrementAndGet()
|
||||
return LoadResult.Page(data = data, prevKey = null, nextKey = null)
|
||||
}
|
||||
|
||||
override fun getRefreshKey(state: PagingState<Int, Message>): Int? = null
|
||||
}
|
||||
|
||||
private fun account(id: String, email: String) = Account(
|
||||
id = id,
|
||||
email = email,
|
||||
|
||||
Reference in New Issue
Block a user