Merge main into fix-reporting-pii-mainthread

This commit is contained in:
Jason Ross
2026-07-04 03:43:43 -05:00
committed by GitHub
18 changed files with 353 additions and 22 deletions
@@ -3,10 +3,12 @@ package org.libremail.ui.reader
import androidx.lifecycle.SavedStateHandle
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.mockk
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.advanceUntilIdle
@@ -21,6 +23,7 @@ import org.libremail.data.settings.SettingsRepository
import org.libremail.domain.model.Attachment
import org.libremail.domain.model.InlineImage
import org.libremail.domain.model.Message
import org.libremail.domain.model.ReplyMode
import org.libremail.domain.repository.MailRepository
import org.libremail.ui.navigation.Routes
import java.io.File
@@ -104,4 +107,72 @@ class ReaderViewModelTest {
assertTrue(0 in vm.state.value.downloaded)
}
/** Builds a reader whose message loads and whose [MailRepository.buildReplyDraft] returns [draftId]. */
private fun replyViewModel(repo: MailRepository, mode: ReplyMode, draftId: String = "draft-x"): ReaderViewModel {
coEvery { repo.openMessage(messageId) } returns Result.success(message)
every { repo.observeAttachments(messageId) } returns flowOf(emptyList())
coEvery { repo.downloadedAttachmentParts(messageId) } returns emptySet()
coEvery { repo.buildReplyDraft(messageId, mode) } returns Result.success(draftId)
return viewModel(repo)
}
@Test
fun `reply routes through buildReplyDraft and emits OpenCompose`() = runTest(dispatcher) {
// #303: the open-email reply must reuse the mailbox's high-fidelity draft (quoted original +
// signature), not a bare prefill — so it goes through buildReplyDraft and opens that draft.
val repo = mockk<MailRepository>(relaxed = true)
val vm = replyViewModel(repo, ReplyMode.REPLY, draftId = "draft-42")
advanceUntilIdle()
vm.reply(ReplyMode.REPLY)
advanceUntilIdle()
assertEquals(ReaderEvent.OpenCompose("draft-42"), vm.events.first())
coVerify(exactly = 1) { repo.buildReplyDraft(messageId, ReplyMode.REPLY) }
}
@Test
fun `forward passes the FORWARD mode to buildReplyDraft`() = runTest(dispatcher) {
val repo = mockk<MailRepository>(relaxed = true)
val vm = replyViewModel(repo, ReplyMode.FORWARD)
advanceUntilIdle()
vm.reply(ReplyMode.FORWARD)
advanceUntilIdle()
coVerify(exactly = 1) { repo.buildReplyDraft(messageId, ReplyMode.FORWARD) }
}
@Test
fun `a rapid double-tap on reply builds only one draft`() = runTest(dispatcher) {
// #304: composing is flipped synchronously, so the second tap (before the first draft resolves)
// is dropped rather than building a second draft and opening compose twice.
val repo = mockk<MailRepository>(relaxed = true)
val vm = replyViewModel(repo, ReplyMode.REPLY)
advanceUntilIdle()
vm.reply(ReplyMode.REPLY)
vm.reply(ReplyMode.REPLY)
advanceUntilIdle()
coVerify(exactly = 1) { repo.buildReplyDraft(messageId, ReplyMode.REPLY) }
}
@Test
fun `a failed reply emits ComposeFailed and clears the composing flag`() = runTest(dispatcher) {
val repo = mockk<MailRepository>(relaxed = true)
coEvery { repo.openMessage(messageId) } returns Result.success(message)
every { repo.observeAttachments(messageId) } returns flowOf(emptyList())
coEvery { repo.downloadedAttachmentParts(messageId) } returns emptySet()
coEvery { repo.buildReplyDraft(messageId, ReplyMode.REPLY) } returns Result.failure(RuntimeException("boom"))
val vm = viewModel(repo)
advanceUntilIdle()
vm.reply(ReplyMode.REPLY)
advanceUntilIdle()
assertEquals(ReaderEvent.ComposeFailed("boom"), vm.events.first())
assertEquals(false, vm.state.value.composing)
}
}
@@ -4,6 +4,7 @@ package org.libremail.ui.reporting
import app.cash.turbine.test
import io.mockk.Runs
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
@@ -12,6 +13,7 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.resetMain
@@ -92,6 +94,29 @@ class ProblemReportsViewModelTest {
verify(exactly = 1) { store.save(match { it.id == "fresh" }) }
}
@Test
fun `a rapid double-tap on Create makes only one report`() {
// Runs on a StandardTestDispatcher so the collect coroutine is queued: the second tap lands
// before it runs, exercising the synchronous `creating` guard so only one report is made (#304).
val standardMain = StandardTestDispatcher()
Dispatchers.setMain(standardMain)
val store = mockk<ReportStore>(relaxed = true)
every { store.reports } returns MutableStateFlow(emptyList())
every { store.save(any()) } just Runs
val collector = mockk<DiagnosticsCollector>()
coEvery { collector.collectManual() } returns report("fresh")
runTest(standardMain) {
val vm = ProblemReportsViewModel(store, collector)
vm.createManualReport()
vm.createManualReport() // double-tap before the first collect runs
advanceUntilIdle()
coVerify(exactly = 1) { collector.collectManual() }
verify(exactly = 1) { store.save(match { it.id == "fresh" }) }
}
}
@Test
fun `discard deletes the report from the store`() = runTest(dispatcher) {
val store = mockk<ReportStore>(relaxed = true)
@@ -11,7 +11,9 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.test.setMain
@@ -112,6 +114,31 @@ class ReportReviewViewModelTest {
verify(exactly = 1) { submitter.submit("rid") }
}
@Test
fun `a rapid double-tap on Submit enqueues the upload only once`() {
// Runs on a StandardTestDispatcher so the submit coroutine is queued: the second tap lands
// before it runs. The fix flips SUBMITTING synchronously (before the async save), so the
// second tap is dropped instead of enqueueing a second upload (#304).
val standardMain = StandardTestDispatcher()
Dispatchers.setMain(standardMain)
every { submitter.isEnabled } returns true
every { submitter.submit("rid") } just Runs
every { submitter.status("rid") } returns emptyFlow()
every { store.save(any()) } just Runs
runTest(standardMain) {
val vm = viewModel()
vm.updateComment(VALID_COMMENT)
vm.updateEmail(VALID_EMAIL)
vm.submit()
vm.submit() // double-tap before the first submit is dispatched
advanceUntilIdle()
verify(exactly = 1) { submitter.submit("rid") }
verify(exactly = 1) { store.save(any()) }
}
}
@Test
fun `submit with no endpoint configured never transmits but still persists`() = runTest(testDispatcher) {
every { submitter.isEnabled } returns false
@@ -10,6 +10,7 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.resetMain
@@ -253,6 +254,30 @@ class AccountSettingsViewModelTest {
assertEquals(true, removed)
}
@Test
fun `a rapid double-tap on Remove deletes the account and navigates back only once`() {
// Runs on a StandardTestDispatcher so the delete coroutine is queued: the second tap lands
// before it runs, exercising the synchronous `removing` guard. Otherwise a double onBack would
// over-pop past the mailbox (#304).
val standardMain = StandardTestDispatcher()
Dispatchers.setMain(standardMain)
val accounts = mockk<AccountRepository>(relaxed = true)
every { accounts.observeAccounts() } returns MutableStateFlow(listOf(account))
val settingsRepository = mockk<SettingsRepository>(relaxed = true)
every { settingsRepository.settings } returns MutableStateFlow(AppSettings())
var removedCount = 0
runTest(standardMain) {
val vm = viewModel(accountRepository = accounts, settingsRepository = settingsRepository)
vm.removeAccount { removedCount++ }
vm.removeAccount { removedCount++ } // double-tap before the first delete runs
advanceUntilIdle()
coVerify(exactly = 1) { accounts.deleteAccount(ACCOUNT) }
assertEquals(1, removedCount)
}
}
private companion object {
const val ACCOUNT = "imap:me@example.org"
}
@@ -9,6 +9,7 @@ import io.mockk.just
import io.mockk.mockk
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.resetMain
@@ -148,6 +149,28 @@ class SignatureEditViewModelTest {
coVerify { repo.create(ACCOUNT, "Trimmed", "<i>body</i>") }
}
@Test
fun `a rapid double-tap on Save creates the signature only once`() {
// Runs on a StandardTestDispatcher so the save coroutine is queued (not run eagerly): the
// second tap lands before it runs, exercising the synchronous `saving` guard (#304).
val standardMain = StandardTestDispatcher()
Dispatchers.setMain(standardMain)
val repo = mockk<SignatureRepository>(relaxed = true)
coEvery { repo.create(any(), any(), any()) } returns "new-id"
var savedCount = 0
runTest(standardMain) {
val vm = viewModel(repo)
vm.onBodyChange("body", null)
vm.save { savedCount++ }
vm.save { savedCount++ } // double-tap before the first save is dispatched
advanceUntilIdle()
coVerify(exactly = 1) { repo.create(any(), any(), any()) }
assertEquals(1, savedCount)
}
}
@Test
fun `saving an existing signature updates it and invokes the callback`() = runTest(dispatcher) {
val repo = mockk<SignatureRepository>(relaxed = true)