diff --git a/app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt b/app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt index 9bf93c8..3c196dd 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt @@ -107,6 +107,11 @@ fun ComposeScreen(onBack: () -> Unit, viewModel: ComposeViewModel = hiltViewMode PackageManager.PERMISSION_GRANTED, ) } + // Flush the pending debounced draft save when the app is backgrounded (#177), so the latest edits + // aren't lost if the process is killed before the autosave debounce fires. + LifecycleEventEffect(Lifecycle.Event.ON_STOP) { + viewModel.flushDraft() + } LaunchedEffect(Unit) { viewModel.finished.collect { onBack() } } BackHandler { viewModel.onExit() } LaunchedEffect(state.error) { diff --git a/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt b/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt index 1760d10..c0cacd3 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt @@ -5,13 +5,18 @@ import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update @@ -57,6 +62,21 @@ data class ComposeUiState( val highlightAttach: Boolean = false, ) +/** The persisted draft fields; autosave fires only when one of these actually changes (#177). */ +private data class DraftContent( + val to: String, + val cc: String, + val bcc: String, + val subject: String, + val body: String, + val bodyHtml: String?, + val attachments: List, +) + +/** Idle window after the last edit before an in-progress compose draft is autosaved (#177). */ +private const val DRAFT_AUTOSAVE_DEBOUNCE_MS = 1_500L + +@OptIn(FlowPreview::class) @HiltViewModel class ComposeViewModel @Inject constructor( savedStateHandle: SavedStateHandle, @@ -71,6 +91,17 @@ class ComposeViewModel @Inject constructor( private val draftId: String? = savedStateHandle.get(Routes.COMPOSE_ARG_DRAFT)?.takeIf { it.isNotBlank() } + /** + * The id every save in this session writes under: the resumed draft's [draftId], or — for a + * brand-new composition — a single id generated once and reused for every autosave (#177). Without + * this, `id = draftId ?: UUID.randomUUID()` minted a fresh id on each call, so periodic autosave + * would insert a new duplicate draft row per tick instead of updating the same one. + */ + private val persistedDraftId: String by lazy { draftId ?: UUID.randomUUID().toString() } + + /** Whether a draft row currently exists for this session (resumed, or autosaved at least once). */ + private var draftPersisted: Boolean = draftId != null + private val _state = MutableStateFlow( ComposeUiState( to = savedStateHandle.get(Routes.COMPOSE_ARG_TO).orEmpty(), @@ -140,6 +171,18 @@ class ComposeViewModel @Inject constructor( seedRememberedFont(settings.lastFontCss, settings.lastFontSizePt) } } + // Debounced periodic autosave (#177): coalesce rapid edits to the persisted fields into one + // write after a short idle window, so an in-progress draft survives a background-kill without + // waiting for the explicit back-press save. Keyed only off the fields saveOrDeleteDraft() writes + // (from-account and transient UI flags are intentionally excluded). + viewModelScope.launch { + _state + .map { DraftContent(it.to, it.cc, it.bcc, it.subject, it.body, it.bodyHtml, it.attachments) } + .distinctUntilChanged() + .drop(1) + .debounce(DRAFT_AUTOSAVE_DEBOUNCE_MS) + .collect { autosaveDraft() } + } } fun onToChange(value: String) { @@ -289,6 +332,23 @@ class ComposeViewModel @Inject constructor( } } + /** + * Immediately flushes any pending draft save — called from ComposeScreen on `ON_STOP` so the last + * keystrokes within the debounce window aren't lost if the app is killed while backgrounded (#177). + */ + fun flushDraft() { + viewModelScope.launch { autosaveDraft() } + } + + /** + * Persists (or deletes, when emptied) the draft now — unless a send is in flight or we've already + * navigated away. Mirrors onExit()'s "don't save mid-send" guard and never re-creates a draft for + * an already-sent message (see [navigated]). Shared by the debounce collector and [flushDraft]. + */ + private suspend fun autosaveDraft() { + if (!_state.value.sending && !navigated) saveOrDeleteDraft() + } + /** Closes the screen exactly once, so send() and a stray back-press can't double-pop. */ private suspend fun finish() { if (navigated) return @@ -305,21 +365,29 @@ class ComposeViewModel @Inject constructor( s.body.isNotBlank() || s.attachments.isNotEmpty() when { - hasContent -> mailRepository.saveDraft( - Draft( - id = draftId ?: UUID.randomUUID().toString(), - accountId = s.fromAccountId, - to = s.to, - cc = s.cc, - bcc = s.bcc, - subject = s.subject, - body = s.body, - updatedAt = System.currentTimeMillis(), - bodyHtml = s.bodyHtml, - attachments = s.attachments, - ), - ) - draftId != null -> mailRepository.deleteDraft(draftId) // an existing draft was emptied out + hasContent -> { + mailRepository.saveDraft( + Draft( + id = persistedDraftId, + accountId = s.fromAccountId, + to = s.to, + cc = s.cc, + bcc = s.bcc, + subject = s.subject, + body = s.body, + updatedAt = System.currentTimeMillis(), + bodyHtml = s.bodyHtml, + attachments = s.attachments, + ), + ) + draftPersisted = true + } + // The draft was emptied out: drop the persisted row — a resumed draft, or one an earlier + // autosave tick created this session — so an empty orphan is never left behind. + draftPersisted -> { + mailRepository.deleteDraft(persistedDraftId) + draftPersisted = false + } } } @@ -377,7 +445,10 @@ class ComposeViewModel @Inject constructor( ).fold( onSuccess = { rememberFontPreference(s.bodyHtml) - draftId?.let { mailRepository.deleteDraft(it) } + // Delete the draft this message was composed from — whether resumed from the drafts list + // or created by an autosave tick this session (#177) — so sending never orphans it. + if (draftPersisted) mailRepository.deleteDraft(persistedDraftId) + draftPersisted = false _state.update { it.copy(sending = false) } finish() }, diff --git a/app/src/test/kotlin/org/libremail/ui/compose/ComposeViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/compose/ComposeViewModelTest.kt index 2ba791d..ae3aaf0 100644 --- a/app/src/test/kotlin/org/libremail/ui/compose/ComposeViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/compose/ComposeViewModelTest.kt @@ -9,12 +9,15 @@ import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.slot +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf 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 import kotlinx.coroutines.test.setMain import org.junit.After @@ -534,4 +537,97 @@ class ComposeViewModelTest { assertFalse(vm.state.value.highlightAttach) coVerify(exactly = 0) { mailRepository.sendMessage(any()) } } + + @Test + fun `rapid edits coalesce into a single debounced autosave`() = runTest(testDispatcher) { + val mailRepository = mockk(relaxed = true) + val vm = viewModel(mailRepository = mailRepository) + advanceUntilIdle() // let init (signature/font seeding) settle before observing autosaves + + vm.onToChange("bob@example.org") + vm.onSubjectChange("Hi") + vm.onBodyChange("Hello", null) + vm.onBodyChange("Hello world", null) + + // Still inside the debounce window: nothing persisted yet. + coVerify(exactly = 0) { mailRepository.saveDraft(any()) } + + advanceUntilIdle() // cross the debounce window + coVerify(exactly = 1) { mailRepository.saveDraft(any()) } + } + + @Test + fun `repeated autosaves of a brand-new draft update a single row`() = runTest(testDispatcher) { + // Regression test for the draft-id-reuse bug: a brand-new compose (draftId == null) must not + // mint a fresh UUID per autosave tick, or each tick would insert a duplicate draft row. + val saved = mutableListOf() + val mailRepository = mockk(relaxed = true) + coEvery { mailRepository.saveDraft(capture(saved)) } just Runs + val vm = viewModel(mailRepository = mailRepository) + advanceUntilIdle() + + vm.onBodyChange("First", null) + advanceUntilIdle() // autosave #1 + + vm.onBodyChange("First, expanded", null) + advanceUntilIdle() // autosave #2 + + assertEquals(2, saved.size, "expected two autosaves") + assertEquals(saved[0].id, saved[1].id, "autosaves must reuse one draft id, not insert per tick") + } + + @Test + fun `exiting saves immediately without waiting for the debounce`() = runTest(testDispatcher) { + val mailRepository = mockk(relaxed = true) + val vm = viewModel(mailRepository = mailRepository) + advanceUntilIdle() + + vm.onBodyChange("Draft body", null) + vm.onExit() + runCurrent() // run onExit's coroutine, but do NOT advance past the debounce window + + coVerify(exactly = 1) { mailRepository.saveDraft(any()) } + } + + @Test + fun `does not autosave while a send is in flight`() = runTest(testDispatcher) { + val mailRepository = mockk(relaxed = true) + val gate = CompletableDeferred>() + coEvery { mailRepository.sendMessage(any()) } coAnswers { gate.await() } + val vm = viewModel(mailRepository = mailRepository) + advanceUntilIdle() + + vm.onToChange("bob@example.org") + vm.onBodyChange("Body", null) + vm.send() // performSend sets sending = true and suspends on the gate + + // An edit lands mid-send; even past the debounce window it must not autosave. + vm.onBodyChange("Body extended", null) + advanceUntilIdle() + coVerify(exactly = 0) { mailRepository.saveDraft(any()) } + + gate.complete(Result.success(Unit)) // let the send finish cleanly + advanceUntilIdle() + } + + @Test + fun `sending after an autosave deletes the autosaved draft`() = runTest(testDispatcher) { + // A brand-new compose that autosaved a row must not orphan it once the message is actually sent. + val saved = mutableListOf() + val mailRepository = mockk(relaxed = true) + coEvery { mailRepository.saveDraft(capture(saved)) } just Runs + coEvery { mailRepository.sendMessage(any()) } returns Result.success(Unit) + val vm = viewModel(mailRepository = mailRepository) + advanceUntilIdle() + + vm.onToChange("bob@example.org") + vm.onBodyChange("Body", null) + advanceUntilIdle() // autosave persists the draft + val autosavedId = saved.single().id + + vm.send() + advanceUntilIdle() + + coVerify(exactly = 1) { mailRepository.deleteDraft(autosavedId) } + } }