diff --git a/app/src/androidTest/kotlin/org/libremail/ui/reader/ReaderScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/reader/ReaderScreenTest.kt index e7928b1..654aa3f 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/reader/ReaderScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/reader/ReaderScreenTest.kt @@ -11,6 +11,7 @@ import androidx.compose.ui.test.performClick import androidx.lifecycle.SavedStateHandle import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry +import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -18,6 +19,7 @@ import org.libremail.R import org.libremail.data.settings.SettingsRepository import org.libremail.domain.model.Attachment import org.libremail.domain.model.Message +import org.libremail.domain.model.ReplyMode import org.libremail.ui.FakeMailRepository import org.libremail.ui.navigation.Routes import org.libremail.ui.theme.LibreMailTheme @@ -44,8 +46,17 @@ class ReaderScreenTest { private fun attachment(partIndex: Int, filename: String) = Attachment(messageId, partIndex, filename, "application/pdf", 1_000L) - /** Renders [ReaderScreen] for the fixed [message] with the given [attachments] and awaits load. */ - private fun renderReader(attachments: List, downloadedParts: Set = emptySet()) { + /** The draft id the reader last asked to open compose on (via OpenCompose), or null. */ + private var openedDraftId: String? = null + + /** + * Renders [ReaderScreen] for the fixed [message] with the given [attachments], awaits load, and + * returns the backing [FakeMailRepository] so a test can assert which reply drafts were built. + */ + private fun renderReader( + attachments: List = emptyList(), + downloadedParts: Set = emptySet(), + ): FakeMailRepository { val context = InstrumentationRegistry.getInstrumentation().targetContext.applicationContext val repo = FakeMailRepository( messages = listOf(message), @@ -59,12 +70,14 @@ class ReaderScreenTest { ) composeTestRule.setContent { LibreMailTheme(darkTheme = false, dynamicColor = false) { - ReaderScreen(onBack = {}, onReply = { _, _, _ -> }, viewModel = viewModel) + ReaderScreen(onBack = {}, onOpenCompose = { openedDraftId = it }, viewModel = viewModel) } } + // The Reply action appears once the message loads, regardless of whether it has attachments. composeTestRule.waitUntil(5_000) { - composeTestRule.onAllNodesWithText(attachments.first().filename).fetchSemanticsNodes().isNotEmpty() + composeTestRule.onAllNodesWithText(string(R.string.reader_reply)).fetchSemanticsNodes().isNotEmpty() } + return repo } @Test @@ -111,4 +124,39 @@ class ReaderScreenTest { composeTestRule.onNodeWithText(seeMore(1)).assertIsDisplayed() composeTestRule.onNodeWithText("b.pdf").assertDoesNotExist() } + + @Test + fun reader_reply_buildsQuotedReplyDraftAndOpensCompose() { + val repo = renderReader() + + composeTestRule.onNodeWithText(string(R.string.reader_reply)).performClick() + composeTestRule.waitUntil(5_000) { openedDraftId != null } + + // The reader routes through buildReplyDraft (quotes the original + bakes the signature), + // not a bare compose prefill (#303), and opens compose on the resulting draft. + assertEquals(listOf(messageId to ReplyMode.REPLY), repo.replyDrafts) + assertEquals("draft-$messageId", openedDraftId) + } + + @Test + fun reader_forward_viaOverflow_buildsForwardDraft() { + val repo = renderReader() + + composeTestRule.onNodeWithContentDescription(string(R.string.action_more)).performClick() + composeTestRule.onNodeWithText(string(R.string.action_forward)).performClick() + composeTestRule.waitUntil(5_000) { openedDraftId != null } + + assertEquals(listOf(messageId to ReplyMode.FORWARD), repo.replyDrafts) + } + + @Test + fun reader_replyAll_viaOverflow_buildsReplyAllDraft() { + val repo = renderReader() + + composeTestRule.onNodeWithContentDescription(string(R.string.action_more)).performClick() + composeTestRule.onNodeWithText(string(R.string.action_reply_all)).performClick() + composeTestRule.waitUntil(5_000) { openedDraftId != null } + + assertEquals(listOf(messageId to ReplyMode.REPLY_ALL), repo.replyDrafts) + } } diff --git a/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt b/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt index 9ee1125..7363c07 100644 --- a/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt +++ b/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt @@ -126,7 +126,7 @@ fun LibreMailApp( ) { ReaderScreen( onBack = navController::popBackStack, - onReply = { to, subject, from -> navController.navigate(Routes.compose(to, subject, from)) }, + onOpenCompose = { draftId -> navController.navigate(Routes.composeDraft(draftId)) }, ) } composable( diff --git a/app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt b/app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt index 16fe041..a53d204 100644 --- a/app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt @@ -28,8 +28,11 @@ import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.Check import androidx.compose.material.icons.filled.Delete import androidx.compose.material.icons.filled.KeyboardArrowDown +import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material.icons.filled.Star import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.DropdownMenu +import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon @@ -66,13 +69,14 @@ import org.libremail.R 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 java.io.File @OptIn(ExperimentalMaterial3Api::class) @Composable fun ReaderScreen( onBack: () -> Unit, - onReply: (to: String, subject: String, from: String) -> Unit, + onOpenCompose: (draftId: String) -> Unit, viewModel: ReaderViewModel = hiltViewModel(), ) { val state by viewModel.state.collectAsStateWithLifecycle() @@ -80,6 +84,7 @@ fun ReaderScreen( val snackbarHostState = remember { SnackbarHostState() } val noAppMessage = stringResource(R.string.attachment_no_app) val downloadFailedTemplate = stringResource(R.string.attachment_download_failed) + val replyFailedMessage = stringResource(R.string.reader_reply_failed) LaunchedEffect(state.deleted) { if (state.deleted) onBack() @@ -95,6 +100,11 @@ fun ReaderScreen( is ReaderEvent.DownloadFailed -> snackbarHostState.showSnackbar(downloadFailedTemplate.format(event.name)) + + is ReaderEvent.OpenCompose -> onOpenCompose(event.draftId) + + is ReaderEvent.ComposeFailed -> + snackbarHostState.showSnackbar(event.message ?: replyFailedMessage) } } } @@ -115,11 +125,17 @@ fun ReaderScreen( actions = { val message = state.message if (message != null) { - TextButton(onClick = { - onReply(message.senderEmail, "Re: ${message.subject}", message.accountId) - }) { + TextButton( + onClick = { viewModel.reply(ReplyMode.REPLY) }, + enabled = !state.composing, + ) { Text(stringResource(R.string.reader_reply)) } + ReplyOverflow( + enabled = !state.composing, + onReplyAll = { viewModel.reply(ReplyMode.REPLY_ALL) }, + onForward = { viewModel.reply(ReplyMode.FORWARD) }, + ) IconButton(onClick = viewModel::toggleStar) { Icon( Icons.Filled.Star, @@ -164,6 +180,35 @@ fun ReaderScreen( } } +/** + * Overflow menu holding the reader's secondary reply actions — Reply All and Forward — so the app bar + * keeps Reply as its one prominent action (#303). Disabled while a draft is being built so a rapid tap + * can't kick off a second one before the first navigates to compose. + */ +@Composable +private fun ReplyOverflow(enabled: Boolean, onReplyAll: () -> Unit, onForward: () -> Unit) { + var expanded by remember { mutableStateOf(false) } + IconButton(onClick = { expanded = true }, enabled = enabled) { + Icon(Icons.Filled.MoreVert, contentDescription = stringResource(R.string.action_more)) + } + DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { + DropdownMenuItem( + text = { Text(stringResource(R.string.action_reply_all)) }, + onClick = { + expanded = false + onReplyAll() + }, + ) + DropdownMenuItem( + text = { Text(stringResource(R.string.action_forward)) }, + onClick = { + expanded = false + onForward() + }, + ) + } +} + @Composable private fun MessageBody( message: Message, diff --git a/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt b/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt index 5c79508..341305e 100644 --- a/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt @@ -17,6 +17,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 @@ -33,13 +34,21 @@ data class ReaderUiState( val downloaded: Set = emptySet(), val loadRemoteImages: Boolean = false, val deleted: Boolean = false, + /** True while a reply/forward draft is being built (a brief network round-trip); gates re-entry. */ + val composing: Boolean = false, val error: String? = null, ) -/** One-shot effects the reader screen acts on (launching a viewer, showing a message). */ +/** One-shot effects the reader screen acts on (launching a viewer, opening compose, showing a message). */ sealed interface ReaderEvent { data class OpenFile(val file: File, val mimeType: String, val name: String) : ReaderEvent data class DownloadFailed(val name: String) : ReaderEvent + + /** A reply/forward draft was built; the screen opens compose on it. */ + data class OpenCompose(val draftId: String) : ReaderEvent + + /** Building the reply/forward draft failed; the screen surfaces [message] (or a generic fallback). */ + data class ComposeFailed(val message: String?) : ReaderEvent } @HiltViewModel @@ -122,6 +131,24 @@ class ReaderViewModel @Inject constructor( fun loadRemoteImages() = _state.update { it.copy(loadRemoteImages = true) } + /** + * Builds a reply/reply-all/forward draft from the open message and opens compose on it — the same + * high-fidelity path the mailbox uses (quotes the original into a `
`, bakes the + * signature, prefixes Re:/Fwd:), instead of a bare prefill (#303). [composing] is flipped + * synchronously before the launch so a double-tap can't build two drafts. + */ + fun reply(mode: ReplyMode) { + if (_state.value.composing) return + _state.update { it.copy(composing = true) } + viewModelScope.launch { + repository.buildReplyDraft(messageId, mode).fold( + onSuccess = { draftId -> _events.send(ReaderEvent.OpenCompose(draftId)) }, + onFailure = { e -> _events.send(ReaderEvent.ComposeFailed(e.message)) }, + ) + _state.update { it.copy(composing = false) } + } + } + fun delete() { viewModelScope.launch { repository.deleteMessage(messageId) diff --git a/app/src/main/kotlin/org/libremail/ui/reporting/ProblemReportsScreen.kt b/app/src/main/kotlin/org/libremail/ui/reporting/ProblemReportsScreen.kt index ce12a1f..8aaaf3a 100644 --- a/app/src/main/kotlin/org/libremail/ui/reporting/ProblemReportsScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/reporting/ProblemReportsScreen.kt @@ -43,6 +43,7 @@ fun ProblemReportsScreen( viewModel: ProblemReportsViewModel = hiltViewModel(), ) { val reports by viewModel.reports.collectAsStateWithLifecycle() + val creating by viewModel.creating.collectAsStateWithLifecycle() // A newly created manual report opens straight into review. LaunchedEffect(Unit) { @@ -71,6 +72,7 @@ fun ProblemReportsScreen( ) { Button( onClick = viewModel::createManualReport, + enabled = !creating, modifier = Modifier .fillMaxWidth() .padding(16.dp), diff --git a/app/src/main/kotlin/org/libremail/ui/reporting/ProblemReportsViewModel.kt b/app/src/main/kotlin/org/libremail/ui/reporting/ProblemReportsViewModel.kt index 0b5c175..ddc9bfe 100644 --- a/app/src/main/kotlin/org/libremail/ui/reporting/ProblemReportsViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/reporting/ProblemReportsViewModel.kt @@ -5,9 +5,11 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -34,11 +36,24 @@ class ProblemReportsViewModel @Inject constructor( private val _created = MutableSharedFlow(extraBufferCapacity = 1) val created: SharedFlow = _created + /** True while a manual report is being collected; gates the button against a double-tap (#304). */ + private val _creating = MutableStateFlow(false) + val creating: StateFlow = _creating.asStateFlow() + fun createManualReport() { + // Flip [_creating] synchronously before the launch so a double-tap can't collect and save two + // reports (and emit two "open review" navigations). Reset on completion — unlike a save-and-pop + // screen, this list stays put, so the user may legitimately create another later (#304). + if (_creating.value) return + _creating.value = true viewModelScope.launch { - val report = collector.collectManual() - store.save(report) - _created.tryEmit(report.id) + try { + val report = collector.collectManual() + store.save(report) + _created.tryEmit(report.id) + } finally { + _creating.value = false + } } } diff --git a/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewViewModel.kt b/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewViewModel.kt index a374a26..980a775 100644 --- a/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/reporting/ReportReviewViewModel.kt @@ -111,19 +111,20 @@ class ReportReviewViewModel @Inject constructor( * still considers actionable. */ fun submit() { + // Re-entry guard + validation run SYNCHRONOUSLY, and SUBMITTING is flipped BEFORE the async + // save/enqueue, so a double-tap can't launch two uploads before the first flips the flag (#304). + if (submitState.value == SubmitUiState.SUBMITTING) return + if (!ReportSubmissionRules.isCommentLongEnough(comment.value)) return + if (!ReportSubmissionRules.isValidEmail(email.value)) return + val report = store.find(reportId) ?: return + submitState.value = SubmitUiState.SUBMITTING viewModelScope.launch { - val currentSubmit = submitState.value - if (currentSubmit == SubmitUiState.SUBMITTING) return@launch - if (!ReportSubmissionRules.isCommentLongEnough(comment.value)) return@launch - if (!ReportSubmissionRules.isValidEmail(email.value)) return@launch - val report = store.find(reportId) ?: return@launch store.save(report.copy(userComment = comment.value, userEmail = email.value)) if (!submitter.isEnabled) { submitState.value = SubmitUiState.UNAVAILABLE return@launch } submitter.submit(reportId) - submitState.value = SubmitUiState.SUBMITTING submitter.status(reportId).collect { submitState.value = it.toUi() } } } diff --git a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsScreen.kt b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsScreen.kt index 361febc..4b5353b 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsScreen.kt @@ -40,6 +40,7 @@ fun AccountSettingsScreen( val signatureCount by viewModel.signatureCount.collectAsStateWithLifecycle() val defaultSignatureName by viewModel.defaultSignatureName.collectAsStateWithLifecycle() val isDefaultAccount by viewModel.isDefaultAccount.collectAsStateWithLifecycle() + val removing by viewModel.removing.collectAsStateWithLifecycle() val context = LocalContext.current val fallbackTitle = stringResource(R.string.settings_account_title) @@ -116,6 +117,7 @@ fun AccountSettingsScreen( ClickRow( title = stringResource(R.string.account_remove), titleColor = MaterialTheme.colorScheme.error, + enabled = !removing, onClick = { viewModel.removeAccount(onBack) }, ) } diff --git a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt index ab1b5a2..6f66a27 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt @@ -5,8 +5,10 @@ import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -60,6 +62,10 @@ class AccountSettingsViewModel @Inject constructor( /** This account's notification channel id, for deep-linking into Android's system settings. */ val notificationChannelId: String = MailNotifier.channelId(accountId) + /** True once account removal is in flight; gates the button so a double-tap can't over-pop (#304). */ + private val _removing = MutableStateFlow(false) + val removing: StateFlow = _removing.asStateFlow() + fun setSignatureEnabled(value: Boolean) { viewModelScope.launch { accountSettingsRepository.setSignatureEnabled(accountId, value) } } @@ -101,6 +107,10 @@ class AccountSettingsViewModel @Inject constructor( } fun removeAccount(onRemoved: () -> Unit) { + // [_removing] is flipped synchronously before the launch so a rapid double-tap can't delete + // twice and fire [onRemoved] (a back-navigation) twice, over-popping past the mailbox (#304). + if (_removing.value) return + _removing.value = true viewModelScope.launch { accountRepository.deleteAccount(accountId) // Don't strand the preference on a deleted account; only clears it if this account was diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SettingsComponents.kt b/app/src/main/kotlin/org/libremail/ui/settings/SettingsComponents.kt index 196e01e..58247a0 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SettingsComponents.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SettingsComponents.kt @@ -62,11 +62,12 @@ internal fun ClickRow( onClick: () -> Unit, subtitle: String? = null, titleColor: Color = Color.Unspecified, + enabled: Boolean = true, ) { Row( modifier = Modifier .fillMaxWidth() - .clickable(onClick = onClick) + .clickable(enabled = enabled, onClick = onClick) .padding(horizontal = 16.dp, vertical = 16.dp), verticalAlignment = Alignment.CenterVertically, ) { diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SignatureEditScreen.kt b/app/src/main/kotlin/org/libremail/ui/settings/SignatureEditScreen.kt index a73946f..7429e6b 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SignatureEditScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SignatureEditScreen.kt @@ -52,7 +52,7 @@ fun SignatureEditScreen(onBack: () -> Unit, viewModel: SignatureEditViewModel = } }, actions = { - TextButton(onClick = { viewModel.save(onBack) }) { + TextButton(onClick = { viewModel.save(onBack) }, enabled = !state.saving) { Text(stringResource(R.string.signature_save)) } }, diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SignatureEditViewModel.kt b/app/src/main/kotlin/org/libremail/ui/settings/SignatureEditViewModel.kt index 3bfe833..55822dd 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SignatureEditViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SignatureEditViewModel.kt @@ -21,6 +21,8 @@ data class SignatureEditUiState( val body: String = "", val bodyHtml: String? = null, val loaded: Boolean = false, + /** True once a save is in flight; gates the Save button so a double-tap can't create two signatures. */ + val saving: Boolean = false, ) @HiltViewModel @@ -59,12 +61,18 @@ class SignatureEditViewModel @Inject constructor( fun onBodyChange(plain: String, html: String?) = _state.update { it.copy(body = plain, bodyHtml = html) } - /** Persists the signature (create or update), then invokes [onSaved]. */ + /** + * Persists the signature (create or update), then invokes [onSaved]. Re-entrant taps are dropped: + * [SignatureEditUiState.saving] is flipped synchronously before the launch, so a rapid double-tap on + * a new signature can't create two rows (and fire [onSaved] twice) (#304). + */ fun save(onSaved: () -> Unit) { + if (_state.value.saving) return val s = _state.value val name = s.name.trim().ifBlank { DEFAULT_NAME } // Store real HTML so the signature round-trips; derive it from the plaintext when unformatted. val html = s.bodyHtml ?: RichTextHtml.toHtml(RichTextContent(s.body)) + _state.update { it.copy(saving = true) } viewModelScope.launch { if (signatureId == null) { signatureRepository.create(accountId, name, html) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a0c039d..32d2b55 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -150,6 +150,7 @@ Reply + Couldn\'t start the reply Attachments Downloaded Available offline diff --git a/app/src/test/kotlin/org/libremail/ui/reader/ReaderViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/reader/ReaderViewModelTest.kt index 6822e83..3e8dd57 100644 --- a/app/src/test/kotlin/org/libremail/ui/reader/ReaderViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/reader/ReaderViewModelTest.kt @@ -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(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(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(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(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) + } } diff --git a/app/src/test/kotlin/org/libremail/ui/reporting/ProblemReportsViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/reporting/ProblemReportsViewModelTest.kt index 9356947..99ebe0a 100644 --- a/app/src/test/kotlin/org/libremail/ui/reporting/ProblemReportsViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/reporting/ProblemReportsViewModelTest.kt @@ -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(relaxed = true) + every { store.reports } returns MutableStateFlow(emptyList()) + every { store.save(any()) } just Runs + val collector = mockk() + 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(relaxed = true) diff --git a/app/src/test/kotlin/org/libremail/ui/reporting/ReportReviewViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/reporting/ReportReviewViewModelTest.kt index f592c1b..6a647bc 100644 --- a/app/src/test/kotlin/org/libremail/ui/reporting/ReportReviewViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/reporting/ReportReviewViewModelTest.kt @@ -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 diff --git a/app/src/test/kotlin/org/libremail/ui/settings/AccountSettingsViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/settings/AccountSettingsViewModelTest.kt index b71f509..ef1a164 100644 --- a/app/src/test/kotlin/org/libremail/ui/settings/AccountSettingsViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/settings/AccountSettingsViewModelTest.kt @@ -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(relaxed = true) + every { accounts.observeAccounts() } returns MutableStateFlow(listOf(account)) + val settingsRepository = mockk(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" } diff --git a/app/src/test/kotlin/org/libremail/ui/settings/SignatureEditViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/settings/SignatureEditViewModelTest.kt index ccc6683..f9cc488 100644 --- a/app/src/test/kotlin/org/libremail/ui/settings/SignatureEditViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/settings/SignatureEditViewModelTest.kt @@ -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", "body") } } + @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(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(relaxed = true)