From 9a67a6d6b3d9be7dae222c9c8519b59221a775c7 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 4 Oct 2026 03:05:24 +0200 Subject: [PATCH 01/10] feat: Create new consistency tests for the task editor refactorization Added some test that check some actions that should work after the refactorization. --- .../flow/tasks/TaskEditorViewModelTest.kt | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt index 46c889c9..43edaf73 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt @@ -22,6 +22,7 @@ import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test @@ -560,4 +561,153 @@ class TaskEditorViewModelTest { coVerify(exactly = 0) { repository.upsertTask(any()) } } + // ── Editor consistency tests ──────────────────────────────── + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun categoryChange_followedImmediatelyByAutosave_neverPersistsPreviousCategory() = runTest { + // GIVEN — A task loaded in category GENERAL_ID + val original = Task( + id = 10, + title = "Original", + isDone = false, + categoryId = Category.GENERAL_ID + ) + val persistedTasks = mutableListOf() + every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.upsertTask(capture(persistedTasks)) } returns 10L + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(10) + advanceUntilIdle() + + // WHEN — Category changes to 2 and an autosave is triggered immediately + viewModel.updateCategory(2) + viewModel.updateTask( + newTitle = "Edited", + newDescription = "", + newPriority = Priority.NOTHING, + newDueDate = null, + newReminderTime = null, + newSubTasks = emptyList() + ) + advanceUntilIdle() + + // THEN — The state and all persisted copies use category 2 + assertEquals(2, viewModel.editor.value.task?.categoryId) + assertTrue(persistedTasks.isNotEmpty()) + assertTrue(persistedTasks.all { it.categoryId == 2 }) + assertTrue(persistedTasks.any { it.title == "Edited" }) + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun rapidCategoryChanges_latestCategoryWins() = runTest { + // GIVEN — A loaded task in category 1 + val original = Task(id = 10, title = "Task", isDone = false, categoryId = 1) + val persistedTasks = mutableListOf() + every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.upsertTask(capture(persistedTasks)) } returns 10L + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(10) + advanceUntilIdle() + + // WHEN — Rapid consecutive category updates occur + viewModel.updateCategory(2) + viewModel.updateCategory(3) + advanceUntilIdle() + + // THEN — Only the latest category snapshot (3) is persisted and held in state + assertEquals(3, viewModel.editor.value.task?.categoryId) + assertTrue(persistedTasks.isNotEmpty()) + assertEquals(3, persistedTasks.last().categoryId) + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun pastReminder_isNormalizedToNullInStateAndPersistence() = runTest { + // GIVEN — A task with a future reminder loaded in the editor + val futureReminder = System.currentTimeMillis() + 60_000L + val original = Task( + id = 10, + title = "Task", + isDone = false, + reminderTime = futureReminder + ) + every { repository.getTasks() } returns flowOf(listOf(original)) + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(10) + advanceUntilIdle() + + // WHEN — The reminder is updated to a time in the past + viewModel.updateReminderTime(System.currentTimeMillis() - 1_000L) + advanceUntilIdle() + + // THEN — The reminder is normalized to null in state and repository, and no alarm is scheduled + assertNull(viewModel.editor.value.task?.reminderTime) + coVerify { repository.upsertTask(match { it.id == 10 && it.reminderTime == null }) } + coVerify(exactly = 0) { reminderScheduler.schedule(any(), any(), any(), any()) } + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun toggleFollowedImmediatelyByAutosave_preservesCompletedState() = runTest { + // GIVEN — A task loaded in the editor + val original = Task(id = 10, title = "Task", isDone = false) + val completed = original.copy( + isDone = true, + completedAt = 1_234L, + reminderTime = null + ) + val persistedTasks = mutableListOf() + every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { toggleTaskUseCase(original) } returns completed + coEvery { repository.upsertTask(capture(persistedTasks)) } returns 10L + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(10) + advanceUntilIdle() + + // WHEN — The task is toggled done and immediately followed by an autosave update + viewModel.toggleDone() + viewModel.updateTask( + newTitle = "Edited after toggle", + newDescription = "", + newPriority = Priority.NOTHING, + newDueDate = null, + newReminderTime = null, + newSubTasks = emptyList() + ) + advanceUntilIdle() + + // THEN — The completed state is preserved in state and in the persisted updates + assertEquals(true, viewModel.editor.value.task?.isDone) + assertTrue(persistedTasks.any { it.title == "Edited after toggle" && it.isDone }) + } } \ No newline at end of file From 72c4c80d0289c69e8bea3834cccb87c24ce5f5a8 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 4 Oct 2026 04:16:12 +0200 Subject: [PATCH 02/10] refactor: use task as single editor state snapshot Avoid using some duplicated code, now everything is managed from the same variable and makes it easier to keep everything synchronized. Updated tests to match the new logic. --- .../feature/flow/tasks/TaskEditorViewModel.kt | 67 ++++--------------- .../flow/tasks/components/TaskEditorScreen.kt | 13 ++-- .../flow/tasks/TaskEditorViewModelTest.kt | 38 ++++++----- 3 files changed, 42 insertions(+), 76 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index 1ae18f54..545b9037 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -37,12 +37,7 @@ import javax.inject.Inject * With a dedicated ViewModel per screen, each lives and dies with its NavBackStackEntry and solves that problem. */ data class TaskEditorState( - val task: Task? = null, - val priority: Priority = Priority.NOTHING, - val dueDate: Long? = null, - val reminderTime: Long? = null, - val isDone: Boolean = false, - val categoryId: Int? = Category.GENERAL_ID + val task: Task? = null ) @HiltViewModel class TaskEditorViewModel @Inject constructor( @@ -87,14 +82,7 @@ class TaskEditorViewModel @Inject constructor( .firstOrNull { it.id == taskId } if (task != null) { - _editor.value = TaskEditorState( - task = task, - priority = task.priority, - dueDate = task.dueDate, - reminderTime = task.reminderTime, - isDone = task.isDone, - categoryId = task.categoryId - ) + _editor.value = TaskEditorState(task = task) } } } @@ -117,46 +105,31 @@ class TaskEditorViewModel @Inject constructor( viewModelScope.launch { taskMutationMutex.withLock { - val state = _editor.value - val currentTask = state.task ?: return@withLock + val currentTask = _editor.value.task ?: return@withLock val updatedTask = currentTask.copy( title = newTitle, description = newDescription, priority = newPriority, dueDate = newDueDate, reminderTime = newReminderTime, - categoryId = state.categoryId ?: Category.GENERAL_ID, - isDone = state.isDone, + categoryId = currentTask.categoryId ?: Category.GENERAL_ID, + isDone = currentTask.isDone, subTasks = newSubTasks ) repository.upsertTask(updatedTask) reconcileSubTaskAlarms(original = currentTask, updated = updatedTask) - _editor.update { - it.copy( - task = updatedTask, - priority = updatedTask.priority, - dueDate = updatedTask.dueDate, - reminderTime = updatedTask.reminderTime, - categoryId = updatedTask.categoryId - ) - } + _editor.update { it.copy(task = updatedTask) } } } } fun updatePriority(value: Priority) = _editor.update { state -> - state.copy( - priority = value, - task = state.task?.copy(priority = value) - ) + state.copy(task = state.task?.copy(priority = value)) } fun updateDueDate(value: Long?) = _editor.update { state -> - state.copy( - dueDate = value, - task = state.task?.copy(dueDate = value) - ) + state.copy(task = state.task?.copy(dueDate = value)) } /** @@ -169,10 +142,7 @@ class TaskEditorViewModel @Inject constructor( fun updateCategory(categoryId: Int?) { val normalizedCategoryId = categoryId ?: Category.GENERAL_ID _editor.update { state -> - state.copy( - categoryId = normalizedCategoryId, - task = state.task?.copy(categoryId = normalizedCategoryId) - ) + state.copy(task = state.task?.copy(categoryId = normalizedCategoryId)) } viewModelScope.launch { @@ -186,26 +156,24 @@ class TaskEditorViewModel @Inject constructor( fun updateReminderTime(value: Long?) { val effectiveValue = if (value != null && value > System.currentTimeMillis()) value else null _editor.update { state -> - state.copy( - reminderTime = effectiveValue, - task = state.task?.copy(reminderTime = effectiveValue) - ) + state.copy(task = state.task?.copy(reminderTime = effectiveValue)) } viewModelScope.launch { taskMutationMutex.withLock { val task = _editor.value.task ?: return@withLock + val currentReminderTime = task.reminderTime // Cancel the old alarm regardless of whether we're setting a new one. reminderScheduler.cancel(task.id) repository.upsertTask(task) - if (effectiveValue != null) { + if (currentReminderTime != null) { reminderScheduler.schedule( task.id, task.title, task.description, - effectiveValue + currentReminderTime ) } } @@ -220,14 +188,7 @@ class TaskEditorViewModel @Inject constructor( val current = _editor.value.task ?: return@withLock val updatedTask = toggleTaskUseCase(current) - _editor.update { - it.copy( - task = updatedTask, - isDone = updatedTask.isDone, - reminderTime = updatedTask.reminderTime, - categoryId = updatedTask.categoryId - ) - } + _editor.update { it.copy(task = updatedTask) } if (updatedTask.isDone) { reminderScheduler.cancel(updatedTask.id) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt index b87e8a14..e56212ec 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt @@ -14,6 +14,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.markel.flowstate.core.designsystem.ui.TaskSharedKeys +import com.markel.flowstate.core.domain.Priority import com.markel.flowstate.core.designsystem.ui.sharedDetailBounds import com.markel.flowstate.feature.flow.tasks.TaskEditorViewModel import com.markel.flowstate.feature.flow.tasks.components.TaskEditorSheetContent @@ -51,9 +52,9 @@ fun TaskEditorScreen( contentWindowInsets = WindowInsets(0.dp), topBar = { TaskEditorTopBar( - priority = editor.priority, + priority = editor.task?.priority ?: Priority.NOTHING, onPriorityChange = { viewModel.updatePriority(it) }, - isDone = editor.isDone, + isDone = editor.task?.isDone == true, onComplete = { viewModel.toggleDone() }, onDelete = { viewModel.deleteTask(editor.task!!) @@ -73,9 +74,9 @@ fun TaskEditorScreen( editor.task?.let { task -> TaskEditorSheetContent( task = task, - priority = editor.priority, - dueDate = editor.dueDate, - remTime = editor.reminderTime, + priority = task.priority, + dueDate = task.dueDate, + remTime = task.reminderTime, onDueDateChange = { viewModel.updateDueDate(it) }, onReminderTimeChange = { viewModel.updateReminderTime(it) }, onAutoUpdate = { title, desc, prio, date, remTime, subTasks -> @@ -90,7 +91,7 @@ fun TaskEditorScreen( }, categories = categories, categoriesEnabled = categoriesEnabled, - categoryId = editor.categoryId, + categoryId = task.categoryId, onCategoryChange = { viewModel.updateCategory(it) }, generalCategoryName = generalCategoryName ) diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt index 43edaf73..0b335b80 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt @@ -48,8 +48,6 @@ class TaskEditorViewModelTest { viewModel.editor.test { val state = awaitItem() assertNull(state.task) - assertEquals(Priority.NOTHING, state.priority) - assertNull(state.dueDate) } } @@ -68,9 +66,9 @@ class TaskEditorViewModelTest { viewModel.editor.test { val state = awaitItem() assertEquals(task, state.task) - assertEquals(Priority.HIGH, state.priority) - assertEquals(1000L, state.dueDate) - assertEquals(false, state.isDone) + assertEquals(Priority.HIGH, state.task?.priority) + assertEquals(1000L, state.task?.dueDate) + assertEquals(false, state.task?.isDone) } } @@ -93,32 +91,38 @@ class TaskEditorViewModelTest { } @Test - fun updatePriority_updatesStateCorrectly() = runTest { + fun updatePriority_updatesTaskSnapshotCorrectly() = runTest { // GIVEN + val task = Task(id = 1, title = "Task", isDone = false) + every { repository.getTasks() } returns flowOf(listOf(task)) viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) - + viewModel.loadTask(1) + advanceUntilIdle() // WHEN viewModel.updatePriority(Priority.MEDIUM) - // THEN + // THEN - the task snapshot is the only source of truth viewModel.editor.test { - assertEquals(Priority.MEDIUM, awaitItem().priority) + assertEquals(Priority.MEDIUM, awaitItem().task?.priority) } } @Test - fun updateDueDate_updatesStateCorrectly() = runTest { + fun updateDueDate_updatesTaskSnapshotCorrectly() = runTest { // GIVEN + val task = Task(id = 1, title = "Task", isDone = false) + every { repository.getTasks() } returns flowOf(listOf(task)) viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) - + viewModel.loadTask(1) + advanceUntilIdle() // WHEN viewModel.updateDueDate(9999L) - // THEN + // THEN - the task snapshot is the only source of truth viewModel.editor.test { - assertEquals(9999L, awaitItem().dueDate) + assertEquals(9999L, awaitItem().task?.dueDate) } } @@ -187,7 +191,7 @@ class TaskEditorViewModelTest { // THEN - State updates optimistically viewModel.editor.test { - assertEquals(true, awaitItem().isDone) + assertEquals(true, awaitItem().task?.isDone) } // AND - Use case is called coVerify { toggleTaskUseCase(task) } @@ -431,7 +435,7 @@ class TaskEditorViewModelTest { // THEN — the editor state reflects the task's category viewModel.editor.test { val state = awaitItem() - assertEquals(5, state.categoryId) + assertEquals(5, state.task?.categoryId) } } @@ -446,7 +450,7 @@ class TaskEditorViewModelTest { viewModel.editor.test { val state = awaitItem() - assertEquals(Category.GENERAL_ID, state.categoryId) + assertEquals(Category.GENERAL_ID, state.task?.categoryId) } } @@ -464,7 +468,7 @@ class TaskEditorViewModelTest { // THEN — the editor state reflects the new category synchronously viewModel.editor.test { val state = awaitItem() - assertEquals(2, state.categoryId) + assertEquals(2, state.task?.categoryId) } } From 0230dbdc13ec8819117842f5161378163db7f0ef Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 4 Oct 2026 21:46:09 +0200 Subject: [PATCH 03/10] refactor: model task editor loading states - Replace nullable `TaskEditorState` with a sealed interface (`Loading`, `Ready`, `NotFound`, `Error`) to cleanly handle multiple loading and error states. - Add `taskOrNull` extension property for safe access to the task only when in the `Ready` state. - Refactor task loading to use `repository.getTaskById(taskId)` directly instead of querying and filtering the entire list. - Prevent race conditions by cancelling previous load jobs (`loadTaskJob?.cancel()`) when a new task request comes in. - Properly propagate `CancellationException` while converting other repository failures into the `Error` state. - Centralize state updates via `updateReadyTask` to ensure editing actions only execute when the editor state is `Ready`. - Remove non-null assertions (`!!`) and safely handle deletion with `task?.let`. - Introduce `TaskEditorTopBar.actionsEnabled` parameter and disable action buttons while the task is loading. - Implement exhaustive `when` branches in `TaskEditorScreen` to render loading indicators, error views, retry buttons, and the main editor content. - Add localized resource strings for task-not-found, load errors, and retry actions across base, English, and Spanish values. - Update unit tests to mock `getTaskById` and explicitly verify new editor states (`Loading`, `NotFound`, `Error`, `taskOrNull`). --- .../feature/flow/tasks/TaskEditorViewModel.kt | 86 +++++++------ .../flow/tasks/components/TaskEditorScreen.kt | 80 +++++++++--- .../flow/tasks/components/TaskEditorTopBar.kt | 27 ++-- .../flow/src/main/res/values-en/strings.xml | 3 + .../flow/src/main/res/values-es/strings.xml | 3 + feature/flow/src/main/res/values/strings.xml | 3 + .../flow/tasks/TaskEditorViewModelTest.kt | 117 +++++++++++------- 7 files changed, 208 insertions(+), 111 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index 545b9037..7c19b2c5 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -13,11 +13,12 @@ import com.markel.flowstate.core.domain.usecase.tasks.DeleteTaskUseCase import com.markel.flowstate.core.domain.usecase.tasks.ToggleTaskUseCase import com.markel.flowstate.core.notifications.ReminderScheduler import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch @@ -36,9 +37,16 @@ import javax.inject.Inject * * With a dedicated ViewModel per screen, each lives and dies with its NavBackStackEntry and solves that problem. */ -data class TaskEditorState( - val task: Task? = null -) +sealed interface TaskEditorState { + data object Loading : TaskEditorState + data class Ready(val task: Task) : TaskEditorState + data object NotFound : TaskEditorState + data class Error(val cause: Throwable) : TaskEditorState +} + +val TaskEditorState.taskOrNull: Task? + get() = (this as? TaskEditorState.Ready)?.task + @HiltViewModel class TaskEditorViewModel @Inject constructor( private val repository: TaskRepository, @@ -49,9 +57,11 @@ class TaskEditorViewModel @Inject constructor( private val userPreferencesRepository: UserPreferencesRepository ) : ViewModel() { - private val _editor = MutableStateFlow(TaskEditorState()) + private val _editor = MutableStateFlow(TaskEditorState.Loading) val editor: StateFlow = _editor.asStateFlow() + private var loadTaskJob: Job? = null + /** * Serializes task writes coming from autosave and immediate actions such * as changing a category or reminder. Each write reads the latest editor @@ -71,18 +81,22 @@ class TaskEditorViewModel @Inject constructor( val generalCategoryName: StateFlow = userPreferencesRepository.generalCategoryName .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null) - /** - * Loads the task from the repository by ID. - * Uses .first() to get the current Flow value without staying subscribed. - */ + /** Loads one task and represents every outcome explicitly in editor state. */ fun loadTask(taskId: Int) { - viewModelScope.launch { - val task = repository.getTasks() - .first() - .firstOrNull { it.id == taskId } - - if (task != null) { - _editor.value = TaskEditorState(task = task) + loadTaskJob?.cancel() + _editor.value = TaskEditorState.Loading + loadTaskJob = viewModelScope.launch { + try { + val task = repository.getTaskById(taskId) + _editor.value = if (task == null) { + TaskEditorState.NotFound + } else { + TaskEditorState.Ready(task) + } + } catch (cancellation: CancellationException) { + throw cancellation + } catch (cause: Throwable) { + _editor.value = TaskEditorState.Error(cause) } } } @@ -105,7 +119,7 @@ class TaskEditorViewModel @Inject constructor( viewModelScope.launch { taskMutationMutex.withLock { - val currentTask = _editor.value.task ?: return@withLock + val currentTask = _editor.value.taskOrNull ?: return@withLock val updatedTask = currentTask.copy( title = newTitle, description = newDescription, @@ -119,18 +133,14 @@ class TaskEditorViewModel @Inject constructor( repository.upsertTask(updatedTask) reconcileSubTaskAlarms(original = currentTask, updated = updatedTask) - _editor.update { it.copy(task = updatedTask) } + updateReadyTask { updatedTask } } } } - fun updatePriority(value: Priority) = _editor.update { state -> - state.copy(task = state.task?.copy(priority = value)) - } + fun updatePriority(value: Priority) = updateReadyTask { it.copy(priority = value) } - fun updateDueDate(value: Long?) = _editor.update { state -> - state.copy(task = state.task?.copy(dueDate = value)) - } + fun updateDueDate(value: Long?) = updateReadyTask { it.copy(dueDate = value) } /** * Moves the task being edited to a different category. @@ -141,13 +151,11 @@ class TaskEditorViewModel @Inject constructor( */ fun updateCategory(categoryId: Int?) { val normalizedCategoryId = categoryId ?: Category.GENERAL_ID - _editor.update { state -> - state.copy(task = state.task?.copy(categoryId = normalizedCategoryId)) - } + updateReadyTask { it.copy(categoryId = normalizedCategoryId) } viewModelScope.launch { taskMutationMutex.withLock { - val task = _editor.value.task ?: return@withLock + val task = _editor.value.taskOrNull ?: return@withLock repository.upsertTask(task) } } @@ -155,13 +163,11 @@ class TaskEditorViewModel @Inject constructor( fun updateReminderTime(value: Long?) { val effectiveValue = if (value != null && value > System.currentTimeMillis()) value else null - _editor.update { state -> - state.copy(task = state.task?.copy(reminderTime = effectiveValue)) - } + updateReadyTask { it.copy(reminderTime = effectiveValue) } viewModelScope.launch { taskMutationMutex.withLock { - val task = _editor.value.task ?: return@withLock + val task = _editor.value.taskOrNull ?: return@withLock val currentReminderTime = task.reminderTime // Cancel the old alarm regardless of whether we're setting a new one. @@ -181,14 +187,14 @@ class TaskEditorViewModel @Inject constructor( } fun toggleDone() { - if (_editor.value.task == null) return + if (_editor.value.taskOrNull == null) return viewModelScope.launch { taskMutationMutex.withLock { - val current = _editor.value.task ?: return@withLock + val current = _editor.value.taskOrNull ?: return@withLock val updatedTask = toggleTaskUseCase(current) - _editor.update { it.copy(task = updatedTask) } + updateReadyTask { updatedTask } if (updatedTask.isDone) { reminderScheduler.cancel(updatedTask.id) @@ -212,6 +218,16 @@ class TaskEditorViewModel @Inject constructor( // ── Internal ────────────────────────────────────────────────────────────── + private fun updateReadyTask(transform: (Task) -> Task) { + _editor.update { state -> + if (state is TaskEditorState.Ready) { + state.copy(task = transform(state.task)) + } else { + state + } + } + } + /** * Diffs old vs new subtask list to cancel removed alarms and schedule new ones. * Only touches alarms for subtasks whose reminderTime actually changed. diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt index e56212ec..c418e50c 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt @@ -1,24 +1,32 @@ package com.markel.flowstate.feature.flow.tasks.components +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding +import androidx.compose.material3.Button +import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold +import androidx.compose.material3.Text import androidx.compose.runtime.* +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.markel.flowstate.core.designsystem.ui.TaskSharedKeys import com.markel.flowstate.core.domain.Priority import com.markel.flowstate.core.designsystem.ui.sharedDetailBounds +import com.markel.flowstate.feature.flow.tasks.TaskEditorState import com.markel.flowstate.feature.flow.tasks.TaskEditorViewModel -import com.markel.flowstate.feature.flow.tasks.components.TaskEditorSheetContent -import com.markel.flowstate.feature.flow.tasks.components.TaskEditorTopBar +import com.markel.flowstate.feature.flow.tasks.taskOrNull +import com.markel.flowstate.feature.tasks.R /** * Full screen for task editing. @@ -44,6 +52,7 @@ fun TaskEditorScreen( val categories by viewModel.categories.collectAsStateWithLifecycle() val categoriesEnabled by viewModel.categoriesEnabled.collectAsStateWithLifecycle() val generalCategoryName by viewModel.generalCategoryName.collectAsStateWithLifecycle() + val task = editor.taskOrNull Scaffold( modifier = Modifier.sharedDetailBounds( @@ -52,15 +61,18 @@ fun TaskEditorScreen( contentWindowInsets = WindowInsets(0.dp), topBar = { TaskEditorTopBar( - priority = editor.task?.priority ?: Priority.NOTHING, - onPriorityChange = { viewModel.updatePriority(it) }, - isDone = editor.task?.isDone == true, - onComplete = { viewModel.toggleDone() }, + priority = task?.priority ?: Priority.NOTHING, + onPriorityChange = viewModel::updatePriority, + isDone = task?.isDone == true, + onComplete = viewModel::toggleDone, onDelete = { - viewModel.deleteTask(editor.task!!) - onBack() + task?.let { + viewModel.deleteTask(it) + onBack() + } }, - onBack = onBack + onBack = onBack, + actionsEnabled = task != null ) }, containerColor = MaterialTheme.colorScheme.surface @@ -71,14 +83,23 @@ fun TaskEditorScreen( .imePadding() .navigationBarsPadding() ) { - editor.task?.let { task -> - TaskEditorSheetContent( - task = task, - priority = task.priority, - dueDate = task.dueDate, - remTime = task.reminderTime, - onDueDateChange = { viewModel.updateDueDate(it) }, - onReminderTimeChange = { viewModel.updateReminderTime(it) }, + when (val state = editor) { + TaskEditorState.Loading -> EditorLoading() + TaskEditorState.NotFound -> EditorLoadFailure( + message = stringResource(R.string.task_editor_not_found), + onRetry = { viewModel.loadTask(taskId) } + ) + is TaskEditorState.Error -> EditorLoadFailure( + message = stringResource(R.string.task_editor_load_error), + onRetry = { viewModel.loadTask(taskId) } + ) + is TaskEditorState.Ready -> TaskEditorSheetContent( + task = state.task, + priority = state.task.priority, + dueDate = state.task.dueDate, + remTime = state.task.reminderTime, + onDueDateChange = viewModel::updateDueDate, + onReminderTimeChange = viewModel::updateReminderTime, onAutoUpdate = { title, desc, prio, date, remTime, subTasks -> viewModel.updateTask( newTitle = title, @@ -91,11 +112,32 @@ fun TaskEditorScreen( }, categories = categories, categoriesEnabled = categoriesEnabled, - categoryId = task.categoryId, - onCategoryChange = { viewModel.updateCategory(it) }, + categoryId = state.task.categoryId, + onCategoryChange = viewModel::updateCategory, generalCategoryName = generalCategoryName ) } } } } + +@Composable +private fun EditorLoading() { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + CircularProgressIndicator() + } +} + +@Composable +private fun EditorLoadFailure(message: String, onRetry: () -> Unit) { + Column( + modifier = Modifier.fillMaxSize(), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center + ) { + Text(text = message, style = MaterialTheme.typography.bodyLarge) + Button(onClick = onRetry) { + Text(stringResource(R.string.retry)) + } + } +} diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorTopBar.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorTopBar.kt index 0ae384cc..2a613f37 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorTopBar.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorTopBar.kt @@ -44,7 +44,8 @@ fun TaskEditorTopBar( isDone: Boolean, onComplete: () -> Unit, onDelete: () -> Unit, - onBack: () -> Unit + onBack: () -> Unit, + actionsEnabled: Boolean = true ) { var menuExpanded by remember { mutableStateOf(false) } @@ -63,15 +64,18 @@ fun TaskEditorTopBar( horizontalArrangement = Arrangement.spacedBy(4.dp), verticalAlignment = Alignment.CenterVertically ) { - IconButton(onClick = { - val nextPriority = when (priority) { - Priority.NOTHING -> Priority.LOW - Priority.LOW -> Priority.MEDIUM - Priority.MEDIUM -> Priority.HIGH - Priority.HIGH -> Priority.NOTHING + IconButton( + enabled = actionsEnabled, + onClick = { + val nextPriority = when (priority) { + Priority.NOTHING -> Priority.LOW + Priority.LOW -> Priority.MEDIUM + Priority.MEDIUM -> Priority.HIGH + Priority.HIGH -> Priority.NOTHING + } + onPriorityChange(nextPriority) } - onPriorityChange(nextPriority) - }) { + ) { Icon( imageVector = ImageVector.vectorResource(priority.flagIconRes()), contentDescription = "Priority", @@ -81,7 +85,10 @@ fun TaskEditorTopBar( } Box { - IconButton(onClick = { menuExpanded = true }) { + IconButton( + enabled = actionsEnabled, + onClick = { menuExpanded = true } + ) { Icon( imageVector = ImageVector.vectorResource(R.drawable.more_vert_24px), contentDescription = "More options" diff --git a/feature/flow/src/main/res/values-en/strings.xml b/feature/flow/src/main/res/values-en/strings.xml index 2bd59d51..2d338dfb 100644 --- a/feature/flow/src/main/res/values-en/strings.xml +++ b/feature/flow/src/main/res/values-en/strings.xml @@ -64,4 +64,7 @@ Date Reminder + Task not found + Could not load the task + Retry \ No newline at end of file diff --git a/feature/flow/src/main/res/values-es/strings.xml b/feature/flow/src/main/res/values-es/strings.xml index 0da6df3d..d69886b5 100644 --- a/feature/flow/src/main/res/values-es/strings.xml +++ b/feature/flow/src/main/res/values-es/strings.xml @@ -64,4 +64,7 @@ Fecha Recordatorio + Tarea no encontrada + No se pudo cargar la tarea + Reintentar \ No newline at end of file diff --git a/feature/flow/src/main/res/values/strings.xml b/feature/flow/src/main/res/values/strings.xml index 2bd59d51..2d338dfb 100644 --- a/feature/flow/src/main/res/values/strings.xml +++ b/feature/flow/src/main/res/values/strings.xml @@ -64,4 +64,7 @@ Date Reminder + Task not found + Could not load the task + Retry \ No newline at end of file diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt index 0b335b80..4245637d 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt @@ -17,7 +17,6 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals @@ -47,7 +46,8 @@ class TaskEditorViewModelTest { // THEN - editor state should be empty viewModel.editor.test { val state = awaitItem() - assertNull(state.task) + assertEquals(TaskEditorState.Loading, state) + assertNull(state.taskOrNull) } } @@ -55,7 +55,7 @@ class TaskEditorViewModelTest { fun loadTask_whenTaskExists_populatesEditorState() = runTest { // GIVEN - A task in the repository val task = Task(id = 42, title = "Buy milk", isDone = false, priority = Priority.HIGH, dueDate = 1000L) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) @@ -65,17 +65,17 @@ class TaskEditorViewModelTest { // THEN - Editor state should reflect the task viewModel.editor.test { val state = awaitItem() - assertEquals(task, state.task) - assertEquals(Priority.HIGH, state.task?.priority) - assertEquals(1000L, state.task?.dueDate) - assertEquals(false, state.task?.isDone) + assertEquals(task, state.taskOrNull) + assertEquals(Priority.HIGH, state.taskOrNull?.priority) + assertEquals(1000L, state.taskOrNull?.dueDate) + assertEquals(false, state.taskOrNull?.isDone) } } @Test fun loadTask_whenTaskDoesNotExist_keepsEmptyState() = runTest { // GIVEN - Repository returns a list that does NOT contain the requested ID - every { repository.getTasks() } returns flowOf(emptyList()) + coEvery { repository.getTaskById(any()) } returns null viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) @@ -86,7 +86,30 @@ class TaskEditorViewModelTest { // THEN - State should remain null (no crash, no phantom data) viewModel.editor.test { val state = awaitItem() - assertNull(state.task) + assertEquals(TaskEditorState.NotFound, state) + assertNull(state.taskOrNull) + } + } + + @Test + fun loadTask_whenRepositoryFails_exposesErrorState() = runTest { + val failure = IllegalStateException("Database unavailable") + coEvery { repository.getTaskById(42) } throws failure + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + + viewModel.loadTask(42) + + viewModel.editor.test { + val state = awaitItem() + assertTrue(state is TaskEditorState.Error) + assertEquals(failure, (state as TaskEditorState.Error).cause) } } @@ -94,7 +117,7 @@ class TaskEditorViewModelTest { fun updatePriority_updatesTaskSnapshotCorrectly() = runTest { // GIVEN val task = Task(id = 1, title = "Task", isDone = false) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(1) advanceUntilIdle() @@ -104,7 +127,7 @@ class TaskEditorViewModelTest { // THEN - the task snapshot is the only source of truth viewModel.editor.test { - assertEquals(Priority.MEDIUM, awaitItem().task?.priority) + assertEquals(Priority.MEDIUM, awaitItem().taskOrNull?.priority) } } @@ -112,7 +135,7 @@ class TaskEditorViewModelTest { fun updateDueDate_updatesTaskSnapshotCorrectly() = runTest { // GIVEN val task = Task(id = 1, title = "Task", isDone = false) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(1) advanceUntilIdle() @@ -122,7 +145,7 @@ class TaskEditorViewModelTest { // THEN - the task snapshot is the only source of truth viewModel.editor.test { - assertEquals(9999L, awaitItem().task?.dueDate) + assertEquals(9999L, awaitItem().taskOrNull?.dueDate) } } @@ -130,7 +153,7 @@ class TaskEditorViewModelTest { fun updateTask_callsRepositoryWithUpdatedData() = runTest { // GIVEN - A task loaded in the editor val original = Task(id = 5, title = "Old title", isDone = false, priority = Priority.NOTHING) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(5) @@ -161,7 +184,7 @@ class TaskEditorViewModelTest { fun updateTask_withBlankTitle_doesNotCallRepository() = runTest { // GIVEN val original = Task(id = 1, title = "Original", isDone = false) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) @@ -176,7 +199,7 @@ class TaskEditorViewModelTest { fun toggleDone_callsToggleUseCaseAndUpdatesState() = runTest { // GIVEN val task = Task(id = 1, title = "Task", isDone = false) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(1) @@ -191,7 +214,7 @@ class TaskEditorViewModelTest { // THEN - State updates optimistically viewModel.editor.test { - assertEquals(true, awaitItem().task?.isDone) + assertEquals(true, awaitItem().taskOrNull?.isDone) } // AND - Use case is called coVerify { toggleTaskUseCase(task) } @@ -201,7 +224,7 @@ class TaskEditorViewModelTest { fun deleteTask_cancelsAlarmsAndCallsDeleteUseCase() = runTest { // GIVEN val task = Task(id = 3, title = "Task to delete", isDone = false) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(3) @@ -218,7 +241,7 @@ class TaskEditorViewModelTest { @Test fun updateReminderTime_with_future_time_cancels_old_and_schedules_new() = runTest { val task = Task(id = 10, title = "My Task", description = "Desc", isDone = false) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) @@ -238,7 +261,7 @@ class TaskEditorViewModelTest { @Test fun updateReminderTime_with_past_time_cancels_old_and_does_not_schedule() = runTest { val task = Task(id = 10, title = "My Task", description = "Desc", isDone = false, reminderTime = System.currentTimeMillis() + 60_000L) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) @@ -256,7 +279,7 @@ class TaskEditorViewModelTest { @Test fun updateReminderTime_with_null_cancels_old_alarm() = runTest { val task = Task(id = 10, title = "My Task", isDone = false, reminderTime = System.currentTimeMillis() + 60_000L) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) @@ -275,7 +298,7 @@ class TaskEditorViewModelTest { val sub1 = SubTask(id = "s1", title = "Sub1", reminderTime = System.currentTimeMillis() + 60_000L) val sub2 = SubTask(id = "s2", title = "Sub2", reminderTime = null) // no reminder → should NOT be canceled val task = Task(id = 10, title = "Task", isDone = false, subTasks = listOf(sub1, sub2)) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) coEvery { toggleTaskUseCase(task) } returns task.copy( @@ -302,7 +325,7 @@ class TaskEditorViewModelTest { @Test fun toggleDone_uncompleting_does_not_cancel_alarms() = runTest { val task = Task(id = 10, title = "Done task", isDone = true) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) @@ -319,7 +342,7 @@ class TaskEditorViewModelTest { fun updateTask_cancels_alarm_for_removed_subtask_with_reminder() = runTest { val subWithReminder = SubTask(id = "s1", title = "Sub1", reminderTime = System.currentTimeMillis() + 60_000L) val original = Task(id = 5, title = "Original", isDone = false, subTasks = listOf(subWithReminder)) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(5) @@ -333,7 +356,7 @@ class TaskEditorViewModelTest { @Test fun updateTask_schedules_alarm_for_new_subtask_with_future_reminder() = runTest { val original = Task(id = 5, title = "Original", isDone = false, subTasks = emptyList()) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(5) @@ -349,7 +372,7 @@ class TaskEditorViewModelTest { @Test fun updateTask_does_not_schedule_alarm_for_new_subtask_with_past_reminder() = runTest { val original = Task(id = 5, title = "Original", isDone = false, subTasks = emptyList()) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(5) @@ -367,7 +390,7 @@ class TaskEditorViewModelTest { val futureTime = System.currentTimeMillis() + 60_000L val sub = SubTask(id = "s1", title = "Sub1", reminderTime = futureTime) val original = Task(id = 5, title = "Original", isDone = false, subTasks = listOf(sub)) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(5) @@ -384,7 +407,7 @@ class TaskEditorViewModelTest { val oldTime = System.currentTimeMillis() + 30_000L val subOld = SubTask(id = "s1", title = "Sub1", reminderTime = oldTime) val original = Task(id = 5, title = "Original", isDone = false, subTasks = listOf(subOld)) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(5) @@ -405,7 +428,7 @@ class TaskEditorViewModelTest { val sub1 = SubTask(id = "s1", title = "Sub1", reminderTime = System.currentTimeMillis() + 60_000L) val sub2 = SubTask(id = "s2", title = "Sub2", reminderTime = null) // even without reminder val task = Task(id = 10, title = "Task", isDone = false, subTasks = listOf(sub1, sub2)) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) @@ -426,7 +449,7 @@ class TaskEditorViewModelTest { fun loadTask_populatesEditorStateWithTaskCategoryId() = runTest { // GIVEN — a task that belongs to category 5 val task = Task(id = 1, title = "Buy milk", isDone = false, categoryId = 5) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) // WHEN @@ -435,7 +458,7 @@ class TaskEditorViewModelTest { // THEN — the editor state reflects the task's category viewModel.editor.test { val state = awaitItem() - assertEquals(5, state.task?.categoryId) + assertEquals(5, state.taskOrNull?.categoryId) } } @@ -443,14 +466,14 @@ class TaskEditorViewModelTest { fun loadTask_withGeneralTask_populatesGeneralCategoryId() = runTest { // GIVEN — a task in the General category val task = Task(id = 1, title = "Buy milk", isDone = false, categoryId = Category.GENERAL_ID) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(1) viewModel.editor.test { val state = awaitItem() - assertEquals(Category.GENERAL_ID, state.task?.categoryId) + assertEquals(Category.GENERAL_ID, state.taskOrNull?.categoryId) } } @@ -458,7 +481,7 @@ class TaskEditorViewModelTest { fun updateCategory_updatesEditorStateImmediately() = runTest { // GIVEN — a loaded task with categoryId = 1 val task = Task(id = 10, title = "T", isDone = false, categoryId = 1) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) @@ -468,7 +491,7 @@ class TaskEditorViewModelTest { // THEN — the editor state reflects the new category synchronously viewModel.editor.test { val state = awaitItem() - assertEquals(2, state.task?.categoryId) + assertEquals(2, state.taskOrNull?.categoryId) } } @@ -476,7 +499,7 @@ class TaskEditorViewModelTest { fun updateCategory_persistsTaskCopyWithNewCategoryId() = runTest { // GIVEN — a loaded task with categoryId = 1 val task = Task(id = 10, title = "T", isDone = false, categoryId = 1) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) @@ -501,7 +524,7 @@ class TaskEditorViewModelTest { isDone = false, categoryId = Category.GENERAL_ID ) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel( repository, toggleTaskUseCase, @@ -538,7 +561,7 @@ class TaskEditorViewModelTest { fun updateCategory_toNull_movesTaskToGeneral() = runTest { // GIVEN — a loaded task that belongs to category 1 val task = Task(id = 10, title = "T", isDone = false, categoryId = 1) - every { repository.getTasks() } returns flowOf(listOf(task)) + coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) @@ -561,7 +584,7 @@ class TaskEditorViewModelTest { // WHEN viewModel.updateCategory(5) - // THEN — the VM early-returns when state.task == null, so no upsert + // THEN — the VM early-returns when state.taskOrNull == null, so no upsert coVerify(exactly = 0) { repository.upsertTask(any()) } } @@ -578,7 +601,7 @@ class TaskEditorViewModelTest { categoryId = Category.GENERAL_ID ) val persistedTasks = mutableListOf() - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original coEvery { repository.upsertTask(capture(persistedTasks)) } returns 10L viewModel = TaskEditorViewModel( repository, @@ -604,7 +627,7 @@ class TaskEditorViewModelTest { advanceUntilIdle() // THEN — The state and all persisted copies use category 2 - assertEquals(2, viewModel.editor.value.task?.categoryId) + assertEquals(2, viewModel.editor.value.taskOrNull?.categoryId) assertTrue(persistedTasks.isNotEmpty()) assertTrue(persistedTasks.all { it.categoryId == 2 }) assertTrue(persistedTasks.any { it.title == "Edited" }) @@ -616,7 +639,7 @@ class TaskEditorViewModelTest { // GIVEN — A loaded task in category 1 val original = Task(id = 10, title = "Task", isDone = false, categoryId = 1) val persistedTasks = mutableListOf() - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original coEvery { repository.upsertTask(capture(persistedTasks)) } returns 10L viewModel = TaskEditorViewModel( repository, @@ -635,7 +658,7 @@ class TaskEditorViewModelTest { advanceUntilIdle() // THEN — Only the latest category snapshot (3) is persisted and held in state - assertEquals(3, viewModel.editor.value.task?.categoryId) + assertEquals(3, viewModel.editor.value.taskOrNull?.categoryId) assertTrue(persistedTasks.isNotEmpty()) assertEquals(3, persistedTasks.last().categoryId) } @@ -651,7 +674,7 @@ class TaskEditorViewModelTest { isDone = false, reminderTime = futureReminder ) - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel( repository, toggleTaskUseCase, @@ -668,7 +691,7 @@ class TaskEditorViewModelTest { advanceUntilIdle() // THEN — The reminder is normalized to null in state and repository, and no alarm is scheduled - assertNull(viewModel.editor.value.task?.reminderTime) + assertNull(viewModel.editor.value.taskOrNull?.reminderTime) coVerify { repository.upsertTask(match { it.id == 10 && it.reminderTime == null }) } coVerify(exactly = 0) { reminderScheduler.schedule(any(), any(), any(), any()) } } @@ -684,7 +707,7 @@ class TaskEditorViewModelTest { reminderTime = null ) val persistedTasks = mutableListOf() - every { repository.getTasks() } returns flowOf(listOf(original)) + coEvery { repository.getTaskById(any()) } returns original coEvery { toggleTaskUseCase(original) } returns completed coEvery { repository.upsertTask(capture(persistedTasks)) } returns 10L viewModel = TaskEditorViewModel( @@ -711,7 +734,7 @@ class TaskEditorViewModelTest { advanceUntilIdle() // THEN — The completed state is preserved in state and in the persisted updates - assertEquals(true, viewModel.editor.value.task?.isDone) + assertEquals(true, viewModel.editor.value.taskOrNull?.isDone) assertTrue(persistedTasks.any { it.title == "Edited after toggle" && it.isDone }) } } \ No newline at end of file From 2dd68f14e9607d55831036eb9818cb8776b499b5 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sat, 10 Oct 2026 02:34:56 +0200 Subject: [PATCH 04/10] refactor: hoist task editor draft into viewmodel - Update TaskEditorState.Ready to include an autosaveRevision counter, allowing precise tracking of field modifications that require persistence. - Refactor TaskEditorViewModel with granular update methods (title, description, priority, dueDate, subtasks) that increment the autosave revision. - Re-architect TaskEditorSheetContent to use a revision-triggered LaunchedEffect for a cleaner 600ms debounce mechanism, avoiding full draft copy passing. - Refactor a DisposableEffect in the UI to safely flush any pending revisions on disposal. - Ensure robust concurrency control and alarm reconciliation via Mutex and persistedTask snapshots. --- .../feature/flow/tasks/TaskEditorViewModel.kt | 149 ++++++++------- .../flow/tasks/components/TaskEditorScreen.kt | 18 +- .../components/TaskEditorSheetContent.kt | 172 +++++------------- .../flow/tasks/TaskEditorViewModelTest.kt | 74 ++++++-- 4 files changed, 196 insertions(+), 217 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index 7c19b2c5..e1db1f90 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -39,7 +39,11 @@ import javax.inject.Inject */ sealed interface TaskEditorState { data object Loading : TaskEditorState - data class Ready(val task: Task) : TaskEditorState + data class Ready( + val task: Task, + /** Increments only for fields handled by the debounced autosave. */ + val autosaveRevision: Long = 0 + ) : TaskEditorState data object NotFound : TaskEditorState data class Error(val cause: Throwable) : TaskEditorState } @@ -70,6 +74,9 @@ class TaskEditorViewModel @Inject constructor( */ private val taskMutationMutex = Mutex() + /** Last snapshot successfully written, used as the alarm reconciliation baseline. */ + private var persistedTask: Task? = null + /** User categories, exposed so the editor can populate the category selector. */ val categories: StateFlow> = categoryRepository.getCategories() .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) @@ -84,13 +91,16 @@ class TaskEditorViewModel @Inject constructor( /** Loads one task and represents every outcome explicitly in editor state. */ fun loadTask(taskId: Int) { loadTaskJob?.cancel() + persistedTask = null _editor.value = TaskEditorState.Loading loadTaskJob = viewModelScope.launch { try { val task = repository.getTaskById(taskId) _editor.value = if (task == null) { + persistedTask = null TaskEditorState.NotFound } else { + persistedTask = task TaskEditorState.Ready(task) } } catch (cancellation: CancellationException) { @@ -101,46 +111,24 @@ class TaskEditorViewModel @Inject constructor( } } - /** - * Saves the latest draft fields on top of the canonical task snapshot - * held by this ViewModel. The composable deliberately does not pass an - * "original task" back here because that object may be stale after an - * immediate mutation such as a category change. - */ - fun updateTask( - newTitle: String, - newDescription: String, - newPriority: Priority, - newDueDate: Long?, - newReminderTime: Long?, - newSubTasks: List - ) { - if (newTitle.isBlank()) return - - viewModelScope.launch { - taskMutationMutex.withLock { - val currentTask = _editor.value.taskOrNull ?: return@withLock - val updatedTask = currentTask.copy( - title = newTitle, - description = newDescription, - priority = newPriority, - dueDate = newDueDate, - reminderTime = newReminderTime, - categoryId = currentTask.categoryId ?: Category.GENERAL_ID, - isDone = currentTask.isDone, - subTasks = newSubTasks - ) - - repository.upsertTask(updatedTask) - reconcileSubTaskAlarms(original = currentTask, updated = updatedTask) - updateReadyTask { updatedTask } - } - } + /** Persists the latest draft owned by this ViewModel. */ + fun saveTask() { + viewModelScope.launch { persistCurrentDraft() } } - fun updatePriority(value: Priority) = updateReadyTask { it.copy(priority = value) } + fun updateTitle(value: String) = updateReadyTask(autosave = true) { it.copy(title = value) } + + fun updateDescription(value: String) = + updateReadyTask(autosave = true) { it.copy(description = value) } + + fun updatePriority(value: Priority) = + updateReadyTask(autosave = true) { it.copy(priority = value) } + + fun updateDueDate(value: Long?) = + updateReadyTask(autosave = true) { it.copy(dueDate = value) } - fun updateDueDate(value: Long?) = updateReadyTask { it.copy(dueDate = value) } + fun updateSubTasks(value: List) = + updateReadyTask(autosave = true) { it.copy(subTasks = value) } /** * Moves the task being edited to a different category. @@ -153,37 +141,14 @@ class TaskEditorViewModel @Inject constructor( val normalizedCategoryId = categoryId ?: Category.GENERAL_ID updateReadyTask { it.copy(categoryId = normalizedCategoryId) } - viewModelScope.launch { - taskMutationMutex.withLock { - val task = _editor.value.taskOrNull ?: return@withLock - repository.upsertTask(task) - } - } + viewModelScope.launch { persistCurrentDraft() } } fun updateReminderTime(value: Long?) { val effectiveValue = if (value != null && value > System.currentTimeMillis()) value else null updateReadyTask { it.copy(reminderTime = effectiveValue) } - viewModelScope.launch { - taskMutationMutex.withLock { - val task = _editor.value.taskOrNull ?: return@withLock - val currentReminderTime = task.reminderTime - - // Cancel the old alarm regardless of whether we're setting a new one. - reminderScheduler.cancel(task.id) - repository.upsertTask(task) - - if (currentReminderTime != null) { - reminderScheduler.schedule( - task.id, - task.title, - task.description, - currentReminderTime - ) - } - } - } + viewModelScope.launch { persistCurrentDraft() } } fun toggleDone() { @@ -194,6 +159,7 @@ class TaskEditorViewModel @Inject constructor( val current = _editor.value.taskOrNull ?: return@withLock val updatedTask = toggleTaskUseCase(current) + persistedTask = updatedTask updateReadyTask { updatedTask } if (updatedTask.isDone) { @@ -218,13 +184,58 @@ class TaskEditorViewModel @Inject constructor( // ── Internal ────────────────────────────────────────────────────────────── - private fun updateReadyTask(transform: (Task) -> Task) { + private fun updateReadyTask( + autosave: Boolean = false, + transform: (Task) -> Task + ) { _editor.update { state -> - if (state is TaskEditorState.Ready) { - state.copy(task = transform(state.task)) - } else { - state - } + if (state !is TaskEditorState.Ready) return@update state + val updated = transform(state.task) + if (updated == state.task) state else state.copy( + task = updated, + autosaveRevision = if (autosave) { + state.autosaveRevision + 1 + } else { + state.autosaveRevision + } + ) + } + } + + /** + * Reads the draft after acquiring the lock. Persistence never publishes + * its snapshot back into editor state, so an older save cannot replace an + * edit made while Room was suspended. + */ + private suspend fun persistCurrentDraft() { + taskMutationMutex.withLock { + val task = _editor.value.taskOrNull ?: return@withLock + if (task.title.isBlank()) return@withLock + val original = persistedTask ?: task + + repository.upsertTask(task) + reconcileTaskAlarm(original = original, updated = task) + reconcileSubTaskAlarms(original = original, updated = task) + persistedTask = task + } + } + + /** Replaces the task alarm when its time or displayed content changed. */ + private fun reconcileTaskAlarm(original: Task, updated: Task) { + val alarmChanged = original.reminderTime != updated.reminderTime || + original.title != updated.title || + original.description != updated.description + if (!alarmChanged) return + + reminderScheduler.cancel(updated.id) + val reminderTime = updated.reminderTime + if (!updated.isDone && reminderTime != null && reminderTime > System.currentTimeMillis()) { + reminderScheduler.schedule( + updated.id, + updated.title, + updated.description, + reminderTime + ) } } diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt index c418e50c..f1ec221b 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt @@ -95,21 +95,13 @@ fun TaskEditorScreen( ) is TaskEditorState.Ready -> TaskEditorSheetContent( task = state.task, - priority = state.task.priority, - dueDate = state.task.dueDate, - remTime = state.task.reminderTime, + autosaveRevision = state.autosaveRevision, + onTitleChange = viewModel::updateTitle, + onDescriptionChange = viewModel::updateDescription, + onSubTasksChange = viewModel::updateSubTasks, onDueDateChange = viewModel::updateDueDate, onReminderTimeChange = viewModel::updateReminderTime, - onAutoUpdate = { title, desc, prio, date, remTime, subTasks -> - viewModel.updateTask( - newTitle = title, - newDescription = desc, - newPriority = prio, - newDueDate = date, - newReminderTime = remTime, - newSubTasks = subTasks, - ) - }, + onAutoUpdate = viewModel::saveTask, categories = categories, categoriesEnabled = categoriesEnabled, categoryId = state.task.categoryId, diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt index d1ea8b70..1511d252 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt @@ -21,8 +21,6 @@ import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.rotate -import androidx.compose.ui.focus.FocusRequester -import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.graphics.SolidColor import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.pluralStringResource @@ -45,11 +43,12 @@ import java.util.UUID @OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3ExpressiveApi::class) @Composable fun TaskEditorSheetContent( - task: Task?, - priority: Priority, // Get it from the parent - dueDate: Long?, - remTime: Long?, - onAutoUpdate: (String, String, Priority, Long?, Long?, List) -> Unit, + task: Task, + autosaveRevision: Long, + onTitleChange: (String) -> Unit, + onDescriptionChange: (String) -> Unit, + onSubTasksChange: (List) -> Unit, + onAutoUpdate: () -> Unit, onDueDateChange: (Long?) -> Unit = {}, onReminderTimeChange: (Long?) -> Unit = {}, categories: List = emptyList(), @@ -58,13 +57,18 @@ fun TaskEditorSheetContent( onCategoryChange: (Int?) -> Unit = {}, generalCategoryName: String? = null ) { - val isNewTask = remember { task == null } - var title by remember { mutableStateOf(task?.title ?: "") } - var description by remember { mutableStateOf(task?.description ?: "") } - val subTasks = remember { - mutableStateListOf().apply { - addAll(task?.subTasks ?: emptyList()) - } + val subTasks = task.subTasks + + fun replaceSubTask(updated: SubTask) { + onSubTasksChange(subTasks.map { if (it.id == updated.id) updated else it }) + } + + fun toggleSubTask(subTask: SubTask) { + replaceSubTask(subTask.copy(isDone = !subTask.isDone)) + } + + fun removeSubTask(subTaskId: String) { + onSubTasksChange(subTasks.filterNot { it.id == subTaskId }) } // Track which subtask is expanded for inline editing @@ -87,74 +91,25 @@ fun TaskEditorSheetContent( var draftSubDueDate by rememberSaveable { mutableStateOf(null) } var draftSubReminder by rememberSaveable { mutableStateOf(null) } - // Checkpoints to track what was actually last saved - var lastSavedTitle by remember { mutableStateOf(title) } - var lastSavedDesc by remember { mutableStateOf(description) } - var lastSavedPriority by remember { mutableStateOf(priority) } - var lastSavedDueDate by remember { mutableStateOf(dueDate) } - var lastSavedReminder by remember { mutableStateOf(remTime) } - var lastSavedSubTasksHash by remember { mutableIntStateOf(subTasks.toList().hashCode()) } - - val focusRequester = remember { FocusRequester() } - - val currentPriority by rememberUpdatedState(priority) - val currentDueDate by rememberUpdatedState(dueDate) - val currentReminder by rememberUpdatedState(remTime) - val currentTitle by rememberUpdatedState(title) - val currentDescription by rememberUpdatedState(description) - val currentSubTasksList by rememberUpdatedState(subTasks.toList()) + // Compose only tracks whether a revision still needs to be flushed + var lastSavedRevision by remember { mutableLongStateOf(autosaveRevision) } + val currentRevision by rememberUpdatedState(autosaveRevision) + val currentOnAutoUpdate by rememberUpdatedState(onAutoUpdate) // TIME-BASED AUTOSAVE (DEBOUNCE) - if (!isNewTask) { - LaunchedEffect(title, description, priority, dueDate, remTime, subTasks.toList().hashCode()) { - val currentSubTasksHash = subTasks.toList().hashCode() - - val hasChanges = title != lastSavedTitle || - description != lastSavedDesc || - priority != lastSavedPriority || - dueDate != lastSavedDueDate || - remTime != lastSavedReminder || - currentSubTasksHash != lastSavedSubTasksHash - - if (hasChanges && title.isNotBlank()) { - delay(600) - // Save - onAutoUpdate(title, description, priority, dueDate, remTime, subTasks.toList()) - // Update references - lastSavedTitle = title - lastSavedDesc = description - lastSavedPriority = priority - lastSavedDueDate = dueDate - lastSavedReminder = remTime - lastSavedSubTasksHash = currentSubTasksHash - } + LaunchedEffect(autosaveRevision) { + if (autosaveRevision != lastSavedRevision) { + delay(600) + currentOnAutoUpdate() + lastSavedRevision = autosaveRevision } } - // EMERGENCY SAVE ON CLOSE (DISPOSE) - // This runs if the user closes the sheet before the delay finishes + // Flush a revision whose debounce did not finish before leaving the screen. DisposableEffect(Unit) { onDispose { - // Only autosave on exit if it's an ALREADY EXISTING task (Editing) - if (!isNewTask) { - val currentSubTasksHash = currentSubTasksList.hashCode() - val hasPendingChanges = currentTitle != lastSavedTitle || - currentDescription != lastSavedDesc || - currentPriority != lastSavedPriority || - currentDueDate != lastSavedDueDate || - currentReminder != lastSavedReminder || - currentSubTasksHash != lastSavedSubTasksHash - - if (hasPendingChanges && currentTitle.isNotBlank()) { - onAutoUpdate( - currentTitle, - currentDescription, - currentPriority, - currentDueDate, - currentReminder, - currentSubTasksList - ) - } + if (currentRevision != lastSavedRevision) { + currentOnAutoUpdate() } } } @@ -206,7 +161,7 @@ fun TaskEditorSheetContent( .fillMaxWidth() .padding(vertical = 6.dp) ) { - if (title.isEmpty()) { + if (task.title.isEmpty()) { Text( text = stringResource(R.string.edit_task_placeholder), style = MaterialTheme.typography.headlineSmall.copy( @@ -218,11 +173,9 @@ fun TaskEditorSheetContent( ) } BasicTextField( - value = title, - onValueChange = { title = it }, - modifier = Modifier - .fillMaxWidth() - .focusRequester(focusRequester), + value = task.title, + onValueChange = onTitleChange, + modifier = Modifier.fillMaxWidth(), textStyle = MaterialTheme.typography.headlineSmall.copy( fontSize = 23.sp, fontWeight = FontWeight.SemiBold, @@ -243,7 +196,7 @@ fun TaskEditorSheetContent( .fillMaxWidth() .padding(vertical = 4.dp) ) { - if (description.isEmpty()) { + if (task.description.isEmpty()) { Text( text = stringResource(R.string.edit_task_desc_placeholder), style = MaterialTheme.typography.bodyLarge, @@ -251,8 +204,8 @@ fun TaskEditorSheetContent( ) } BasicTextField( - value = description, - onValueChange = { description = it }, + value = task.description, + onValueChange = onDescriptionChange, modifier = Modifier.fillMaxWidth(), textStyle = MaterialTheme.typography.bodyLarge.copy( lineHeight = 22.sp, @@ -268,19 +221,19 @@ fun TaskEditorSheetContent( // ── Metadata: due date · reminder as outlined chips ─────────── Spacer(modifier = Modifier.height(10.dp)) TaskMetadataChips( - dueDate = dueDate, + dueDate = task.dueDate, onDueDateChange = onDueDateChange, - reminderTime = remTime, + reminderTime = task.reminderTime, onReminderTimeChange = onReminderTimeChange ) Spacer(modifier = Modifier.height(24.dp)) // SUBTASKS — computed lists & completed-chevron state - val pendingSubTasks = remember(subTasks.toList()) { + val pendingSubTasks = remember(subTasks) { subTasks.filter { !it.isDone } } - val completedSubTasks = remember(subTasks.toList()) { + val completedSubTasks = remember(subTasks) { subTasks.filter { it.isDone } } @@ -335,23 +288,10 @@ fun TaskEditorSheetContent( onExpandChange = { shouldExpand -> expandedSubTaskId = if (shouldExpand) subTask.id else null }, - onUpdate = { updatedSubTask -> - val realIndex = subTasks.indexOfFirst { it.id == updatedSubTask.id } - if (realIndex != -1) { - subTasks[realIndex] = updatedSubTask - } - }, - onCheckedChange = { - // We need to find the index in the REAL list, not the visible list - val realIndex = subTasks.indexOfFirst { it.id == subTask.id } - if (realIndex != -1) { - subTasks[realIndex] = subTask.copy(isDone = !subTask.isDone) - } - }, + onUpdate = ::replaceSubTask, + onCheckedChange = { toggleSubTask(subTask) }, onDelete = { - val realIndex = subTasks.indexOfFirst { it.id == subTask.id } - if (realIndex != -1) subTasks.removeAt(realIndex) - // Close expansion if this was the expanded item + removeSubTask(subTask.id) if (expandedSubTaskId == subTask.id) expandedSubTaskId = null } ) @@ -451,21 +391,10 @@ fun TaskEditorSheetContent( onExpandChange = { shouldExpand -> expandedSubTaskId = if (shouldExpand) subTask.id else null }, - onUpdate = { updatedSubTask -> - val realIndex = subTasks.indexOfFirst { it.id == updatedSubTask.id } - if (realIndex != -1) { - subTasks[realIndex] = updatedSubTask - } - }, - onCheckedChange = { - val realIndex = subTasks.indexOfFirst { it.id == subTask.id } - if (realIndex != -1) { - subTasks[realIndex] = subTask.copy(isDone = !subTask.isDone) - } - }, + onUpdate = ::replaceSubTask, + onCheckedChange = { toggleSubTask(subTask) }, onDelete = { - val realIndex = subTasks.indexOfFirst { it.id == subTask.id } - if (realIndex != -1) subTasks.removeAt(realIndex) + removeSubTask(subTask.id) if (expandedSubTaskId == subTask.id) expandedSubTaskId = null } ) @@ -506,7 +435,7 @@ fun TaskEditorSheetContent( position = subTasks.size, reminderTime = reminder ) - subTasks.add(newSubTask) + onSubTasksChange(subTasks + newSubTask) draftSubTitle = "" draftSubDescription = "" @@ -519,11 +448,6 @@ fun TaskEditorSheetContent( } } - LaunchedEffect(Unit) { - delay(100) - if (isNewTask) focusRequester.requestFocus() - } - // ── Category selector bottom sheet ─────────────────────────────────────── if (showCategorySelector) { CategorySelectorSheet( diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt index 4245637d..55dd6782 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt @@ -16,6 +16,7 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -38,6 +39,26 @@ class TaskEditorViewModelTest { private val userPreferencesRepository: UserPreferencesRepository = mockk(relaxed = true) private lateinit var viewModel: TaskEditorViewModel + /** Drives the same field-level API used by Compose, then requests a save. */ + private fun updateTaskDraft( + newTitle: String, + newDescription: String, + newPriority: Priority, + newDueDate: Long?, + newReminderTime: Long?, + newSubTasks: List + ) { + viewModel.updateTitle(newTitle) + viewModel.updateDescription(newDescription) + viewModel.updatePriority(newPriority) + viewModel.updateDueDate(newDueDate) + if (viewModel.editor.value.taskOrNull?.reminderTime != newReminderTime) { + viewModel.updateReminderTime(newReminderTime) + } + viewModel.updateSubTasks(newSubTasks) + viewModel.saveTask() + } + @Test fun initialState_isEmpty() = runTest { // GIVEN / WHEN - A fresh ViewModel with no loadTask call yet @@ -159,7 +180,7 @@ class TaskEditorViewModelTest { viewModel.loadTask(5) // WHEN - User edits the task - viewModel.updateTask( + updateTaskDraft( newTitle = "New title", newDescription = "New desc", newPriority = Priority.HIGH, @@ -186,10 +207,11 @@ class TaskEditorViewModelTest { val original = Task(id = 1, title = "Original", isDone = false) coEvery { repository.getTaskById(any()) } returns original viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) - + viewModel.loadTask(1) + advanceUntilIdle() // WHEN - Trying to save with a blank title - viewModel.updateTask(" ", "", Priority.NOTHING, null, null, emptyList()) + updateTaskDraft(" ", "", Priority.NOTHING, null, null, emptyList()) // THEN - Repository must NOT be called coVerify(exactly = 0) { repository.upsertTask(any()) } @@ -348,7 +370,7 @@ class TaskEditorViewModelTest { viewModel.loadTask(5) // Update: remove the subtask - viewModel.updateTask("Updated", "", Priority.NOTHING, null, null, emptyList()) + updateTaskDraft("Updated", "", Priority.NOTHING, null, null, emptyList()) coVerify { reminderScheduler.cancelSubTask("s1") } } @@ -364,7 +386,7 @@ class TaskEditorViewModelTest { val futureTime = System.currentTimeMillis() + 60_000L val newSub = SubTask(id = "s-new", title = "New Sub", reminderTime = futureTime) - viewModel.updateTask("Updated", "", Priority.NOTHING, null, null, listOf(newSub)) + updateTaskDraft("Updated", "", Priority.NOTHING, null, null, listOf(newSub)) coVerify { reminderScheduler.scheduleSubTask("s-new", "New Sub", futureTime) } } @@ -380,7 +402,7 @@ class TaskEditorViewModelTest { val pastTime = System.currentTimeMillis() - 60_000L val newSub = SubTask(id = "s-old", title = "Old Sub", reminderTime = pastTime) - viewModel.updateTask("Updated", "", Priority.NOTHING, null, null, listOf(newSub)) + updateTaskDraft("Updated", "", Priority.NOTHING, null, null, listOf(newSub)) coVerify(exactly = 0) { reminderScheduler.scheduleSubTask(any(), any(), any()) } } @@ -396,7 +418,7 @@ class TaskEditorViewModelTest { viewModel.loadTask(5) // Update with same subtask, same reminder → no cancel, no schedule - viewModel.updateTask("Updated", "", Priority.NOTHING, null, null, listOf(sub)) + updateTaskDraft("Updated", "", Priority.NOTHING, null, null, listOf(sub)) coVerify(exactly = 0) { reminderScheduler.cancelSubTask(any()) } coVerify(exactly = 0) { reminderScheduler.scheduleSubTask(any(), any(), any()) } @@ -415,7 +437,7 @@ class TaskEditorViewModelTest { val newTime = System.currentTimeMillis() + 90_000L val subUpdated = SubTask(id = "s1", title = "Sub1", reminderTime = newTime) - viewModel.updateTask("Updated", "", Priority.NOTHING, null, null, listOf(subUpdated)) + updateTaskDraft("Updated", "", Priority.NOTHING, null, null, listOf(subUpdated)) coVerify { reminderScheduler.cancelSubTask("s1") } coVerify { reminderScheduler.scheduleSubTask("s1", "Sub1", newTime) } @@ -540,7 +562,7 @@ class TaskEditorViewModelTest { advanceUntilIdle() // Simulate a later autosave after editing the title. - viewModel.updateTask( + updateTaskDraft( newTitle = "Edited", newDescription = "", newPriority = Priority.NOTHING, @@ -616,7 +638,7 @@ class TaskEditorViewModelTest { // WHEN — Category changes to 2 and an autosave is triggered immediately viewModel.updateCategory(2) - viewModel.updateTask( + updateTaskDraft( newTitle = "Edited", newDescription = "", newPriority = Priority.NOTHING, @@ -723,7 +745,7 @@ class TaskEditorViewModelTest { // WHEN — The task is toggled done and immediately followed by an autosave update viewModel.toggleDone() - viewModel.updateTask( + updateTaskDraft( newTitle = "Edited after toggle", newDescription = "", newPriority = Priority.NOTHING, @@ -737,4 +759,34 @@ class TaskEditorViewModelTest { assertEquals(true, viewModel.editor.value.taskOrNull?.isDone) assertTrue(persistedTasks.any { it.title == "Edited after toggle" && it.isDone }) } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun saveCompletingAfterNewEdit_doesNotReplaceNewerDraft() = runTest { + val original = Task(id = 10, title = "Original", isDone = false) + val allowSaveToFinish = CompletableDeferred() + coEvery { repository.getTaskById(10) } returns original + coEvery { repository.upsertTask(match { it.title == "First edit" }) } coAnswers { + allowSaveToFinish.await() + 10L + } + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(10) + advanceUntilIdle() + + viewModel.updateTitle("First edit") + viewModel.saveTask() // Suspends inside the repository. + viewModel.updateTitle("Newer edit") + allowSaveToFinish.complete(Unit) + advanceUntilIdle() + + assertEquals("Newer edit", viewModel.editor.value.taskOrNull?.title) + } } \ No newline at end of file From d414605ee5135dcff8c307537380beb550f603ed Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sat, 10 Oct 2026 18:59:39 +0200 Subject: [PATCH 05/10] refactor: move the debounce to the viewmodel The UI doesn't need to know anything about the debounce, now that is separated from the Compose file. --- .../feature/flow/tasks/TaskEditorViewModel.kt | 65 +++++++++++++++---- .../flow/tasks/components/TaskEditorScreen.kt | 1 + .../components/TaskEditorSheetContent.kt | 20 ++---- .../flow/tasks/TaskEditorViewModelTest.kt | 45 ++++++++++++- 4 files changed, 103 insertions(+), 28 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index e1db1f90..1ead9fcd 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -15,6 +15,7 @@ import com.markel.flowstate.core.notifications.ReminderScheduler import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -42,7 +43,9 @@ sealed interface TaskEditorState { data class Ready( val task: Task, /** Increments only for fields handled by the debounced autosave. */ - val autosaveRevision: Long = 0 + val autosaveRevision: Long = 0, + /** Highest autosave revision confirmed by the repository. */ + val savedRevision: Long = 0 ) : TaskEditorState data object NotFound : TaskEditorState data class Error(val cause: Throwable) : TaskEditorState @@ -65,6 +68,7 @@ class TaskEditorViewModel @Inject constructor( val editor: StateFlow = _editor.asStateFlow() private var loadTaskJob: Job? = null + private var autosaveJob: Job? = null /** * Serializes task writes coming from autosave and immediate actions such @@ -91,6 +95,8 @@ class TaskEditorViewModel @Inject constructor( /** Loads one task and represents every outcome explicitly in editor state. */ fun loadTask(taskId: Int) { loadTaskJob?.cancel() + autosaveJob?.cancel() + autosaveJob = null persistedTask = null _editor.value = TaskEditorState.Loading loadTaskJob = viewModelScope.launch { @@ -111,8 +117,13 @@ class TaskEditorViewModel @Inject constructor( } } - /** Persists the latest draft owned by this ViewModel. */ + /** + * Requests an immediate save. The debounce itself is owned by this ViewModel; + * this entry point remains temporarily for the screen's best-effort dispose flush. + */ fun saveTask() { + autosaveJob?.cancel() + autosaveJob = null viewModelScope.launch { persistCurrentDraft() } } @@ -141,14 +152,14 @@ class TaskEditorViewModel @Inject constructor( val normalizedCategoryId = categoryId ?: Category.GENERAL_ID updateReadyTask { it.copy(categoryId = normalizedCategoryId) } - viewModelScope.launch { persistCurrentDraft() } + saveTask() } fun updateReminderTime(value: Long?) { val effectiveValue = if (value != null && value > System.currentTimeMillis()) value else null updateReadyTask { it.copy(reminderTime = effectiveValue) } - viewModelScope.launch { persistCurrentDraft() } + saveTask() } fun toggleDone() { @@ -188,18 +199,33 @@ class TaskEditorViewModel @Inject constructor( autosave: Boolean = false, transform: (Task) -> Task ) { + var changed = false _editor.update { state -> if (state !is TaskEditorState.Ready) return@update state val updated = transform(state.task) - if (updated == state.task) state else state.copy( + if (updated == state.task) return@update state + + changed = true + state.copy( task = updated, - autosaveRevision = if (autosave) { - state.autosaveRevision + 1 - } else { - state.autosaveRevision - } + autosaveRevision = if (autosave) state.autosaveRevision + 1 else state.autosaveRevision ) } + + // Keep coroutine scheduling outside update(), whose reducer may retry. + if (autosave && changed) scheduleAutosave() + } + + /** Restarts the debounce so rapid edits result in one save of the latest draft. */ + private fun scheduleAutosave() { + autosaveJob?.cancel() + autosaveJob = viewModelScope.launch { + delay(AUTOSAVE_DEBOUNCE_MS) + // Clear the debounce handle before I/O so a new edit does not cancel + // an upsert that has already started. + autosaveJob = null + persistCurrentDraft() + } } /** @@ -209,7 +235,9 @@ class TaskEditorViewModel @Inject constructor( */ private suspend fun persistCurrentDraft() { taskMutationMutex.withLock { - val task = _editor.value.taskOrNull ?: return@withLock + val ready = _editor.value as? TaskEditorState.Ready ?: return@withLock + val task = ready.task + val revision = ready.autosaveRevision if (task.title.isBlank()) return@withLock val original = persistedTask ?: task @@ -217,6 +245,16 @@ class TaskEditorViewModel @Inject constructor( reconcileTaskAlarm(original = original, updated = task) reconcileSubTaskAlarms(original = original, updated = task) persistedTask = task + + // Mark only the revision represented by this persisted snapshot. If + // the user edited during the Room write, the newer draft stays dirty. + _editor.update { current -> + if (current is TaskEditorState.Ready && current.task.id == task.id) { + current.copy(savedRevision = maxOf(current.savedRevision, revision)) + } else { + current + } + } } } @@ -268,4 +306,7 @@ class TaskEditorViewModel @Inject constructor( } } -} \ No newline at end of file + private companion object { + const val AUTOSAVE_DEBOUNCE_MS = 600L + } +} diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt index f1ec221b..9e3ff4be 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt @@ -96,6 +96,7 @@ fun TaskEditorScreen( is TaskEditorState.Ready -> TaskEditorSheetContent( task = state.task, autosaveRevision = state.autosaveRevision, + savedRevision = state.savedRevision, onTitleChange = viewModel::updateTitle, onDescriptionChange = viewModel::updateDescription, onSubTasksChange = viewModel::updateSubTasks, diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt index 1511d252..e75f917a 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt @@ -37,7 +37,6 @@ import com.markel.flowstate.core.domain.SubTask import com.markel.flowstate.core.domain.Task import com.markel.flowstate.feature.flow.components.CategorySelectorSheet import com.markel.flowstate.feature.tasks.R -import kotlinx.coroutines.delay import java.util.UUID @OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3ExpressiveApi::class) @@ -45,6 +44,7 @@ import java.util.UUID fun TaskEditorSheetContent( task: Task, autosaveRevision: Long, + savedRevision: Long, onTitleChange: (String) -> Unit, onDescriptionChange: (String) -> Unit, onSubTasksChange: (List) -> Unit, @@ -91,24 +91,16 @@ fun TaskEditorSheetContent( var draftSubDueDate by rememberSaveable { mutableStateOf(null) } var draftSubReminder by rememberSaveable { mutableStateOf(null) } - // Compose only tracks whether a revision still needs to be flushed - var lastSavedRevision by remember { mutableLongStateOf(autosaveRevision) } + // The ViewModel owns the debounce. Keep only a temporary best-effort fallback + // for a dirty revision when this content leaves composition. A later mini-phase + // will replace this with an awaited flush before navigation. val currentRevision by rememberUpdatedState(autosaveRevision) + val currentSavedRevision by rememberUpdatedState(savedRevision) val currentOnAutoUpdate by rememberUpdatedState(onAutoUpdate) - // TIME-BASED AUTOSAVE (DEBOUNCE) - LaunchedEffect(autosaveRevision) { - if (autosaveRevision != lastSavedRevision) { - delay(600) - currentOnAutoUpdate() - lastSavedRevision = autosaveRevision - } - } - - // Flush a revision whose debounce did not finish before leaving the screen. DisposableEffect(Unit) { onDispose { - if (currentRevision != lastSavedRevision) { + if (currentRevision > currentSavedRevision) { currentOnAutoUpdate() } } diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt index 55dd6782..2d33d508 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt @@ -18,7 +18,9 @@ import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertNull @@ -760,14 +762,52 @@ class TaskEditorViewModelTest { assertTrue(persistedTasks.any { it.title == "Edited after toggle" && it.isDone }) } + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun fieldEdits_areDebouncedByViewModel_andPersistOnlyLatestDraft() = runTest { + val original = Task(id = 11, title = "Original", isDone = false) + val persistedTasks = mutableListOf() + coEvery { repository.getTaskById(11) } returns original + coEvery { repository.upsertTask(capture(persistedTasks)) } returns 11L + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + + viewModel.loadTask(11) + advanceUntilIdle() + + viewModel.updateTitle("First edit") + advanceTimeBy(300) + runCurrent() + viewModel.updateTitle("Latest edit") + advanceTimeBy(599) + runCurrent() + + coVerify(exactly = 0) { repository.upsertTask(any()) } + + advanceTimeBy(1) + runCurrent() + + assertEquals(listOf("Latest edit"), persistedTasks.map { it.title }) + assertEquals(2L, (viewModel.editor.value as TaskEditorState.Ready).savedRevision) + } + @OptIn(ExperimentalCoroutinesApi::class) @Test fun saveCompletingAfterNewEdit_doesNotReplaceNewerDraft() = runTest { val original = Task(id = 10, title = "Original", isDone = false) val allowSaveToFinish = CompletableDeferred() + val persistedTasks = mutableListOf() coEvery { repository.getTaskById(10) } returns original - coEvery { repository.upsertTask(match { it.title == "First edit" }) } coAnswers { - allowSaveToFinish.await() + coEvery { repository.upsertTask(capture(persistedTasks)) } coAnswers { + if (persistedTasks.last().title == "First edit") { + allowSaveToFinish.await() + } 10L } viewModel = TaskEditorViewModel( @@ -788,5 +828,6 @@ class TaskEditorViewModelTest { advanceUntilIdle() assertEquals("Newer edit", viewModel.editor.value.taskOrNull?.title) + assertEquals("Newer edit", persistedTasks.last().title) } } \ No newline at end of file From 5e90c9ae41b2d5fdce2cef5035af380909d4d2d8 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 11 Oct 2026 01:49:00 +0200 Subject: [PATCH 06/10] refactor: onBack logic The logic now cancels the debounced autosave and persists any pending changes. If the saving process fails the editor doesn't exit without showing a warning --- .../feature/flow/tasks/TaskEditorViewModel.kt | 87 +++++++++++---- .../flow/tasks/components/TaskEditorScreen.kt | 34 +++++- .../flow/src/main/res/values-en/strings.xml | 1 + .../flow/src/main/res/values-es/strings.xml | 1 + feature/flow/src/main/res/values/strings.xml | 1 + .../flow/tasks/TaskEditorViewModelTest.kt | 100 ++++++++++++++++++ 6 files changed, 202 insertions(+), 22 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index 1ead9fcd..7089d870 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -15,6 +15,7 @@ import com.markel.flowstate.core.notifications.ReminderScheduler import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Job +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted @@ -127,6 +128,42 @@ class TaskEditorViewModel @Inject constructor( viewModelScope.launch { persistCurrentDraft() } } + /** + * Cancels the pending debounce, waits for any write already in progress, and + * persists the latest draft before a normal back navigation. + * + * Returns false on persistence failure so the UI can remain in the editor. + */ + suspend fun flushPendingChanges(): Boolean { + return try { + while (true) { + cancelPendingAutosave() + + val flushed = taskMutationMutex.withLock { + val task = _editor.value.taskOrNull ?: return@withLock true + // A blank title is not persisted. + if (task.title.isBlank() || task == persistedTask) { + return@withLock true + } + + persistCurrentDraftLocked() + + // An edit may have arrived while the repository write was suspended. + // Loop and save that newer snapshot before allowing navigation. + val latestTask = _editor.value.taskOrNull + latestTask == null || latestTask.title.isBlank() || latestTask == persistedTask + } + + if (flushed) break + } + true + } catch (cancellation: CancellationException) { + throw cancellation + } catch (_: Exception) { + false + } + } + fun updateTitle(value: String) = updateReadyTask(autosave = true) { it.copy(title = value) } fun updateDescription(value: String) = @@ -234,30 +271,40 @@ class TaskEditorViewModel @Inject constructor( * edit made while Room was suspended. */ private suspend fun persistCurrentDraft() { - taskMutationMutex.withLock { - val ready = _editor.value as? TaskEditorState.Ready ?: return@withLock - val task = ready.task - val revision = ready.autosaveRevision - if (task.title.isBlank()) return@withLock - val original = persistedTask ?: task - - repository.upsertTask(task) - reconcileTaskAlarm(original = original, updated = task) - reconcileSubTaskAlarms(original = original, updated = task) - persistedTask = task - - // Mark only the revision represented by this persisted snapshot. If - // the user edited during the Room write, the newer draft stays dirty. - _editor.update { current -> - if (current is TaskEditorState.Ready && current.task.id == task.id) { - current.copy(savedRevision = maxOf(current.savedRevision, revision)) - } else { - current - } + taskMutationMutex.withLock { persistCurrentDraftLocked() } + } + + /** Must be called while holding [taskMutationMutex]. */ + private suspend fun persistCurrentDraftLocked() { + val ready = _editor.value as? TaskEditorState.Ready ?: return + val task = ready.task + val revision = ready.autosaveRevision + if (task.title.isBlank()) return + val original = persistedTask ?: task + + repository.upsertTask(task) + reconcileTaskAlarm(original = original, updated = task) + reconcileSubTaskAlarms(original = original, updated = task) + persistedTask = task + + // Mark only the revision represented by this persisted snapshot. If + // the user edited during the Room write, the newer draft stays dirty. + _editor.update { current -> + if (current is TaskEditorState.Ready && current.task.id == task.id) { + current.copy(savedRevision = maxOf(current.savedRevision, revision)) + } else { + current } } } + /** Cancels only a pending debounce; an active write is awaited through the Mutex. */ + private suspend fun cancelPendingAutosave() { + val pendingJob = autosaveJob + autosaveJob = null + pendingJob?.cancelAndJoin() + } + /** Replaces the task alarm when its time or displayed content changed. */ private fun reconcileTaskAlarm(original: Task, updated: Task) { val alarmChanged = original.reminderTime != updated.reminderTime || diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt index 9e3ff4be..c26c551b 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt @@ -1,5 +1,6 @@ package com.markel.flowstate.feature.flow.tasks.components +import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -12,6 +13,8 @@ import androidx.compose.material3.Button import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.runtime.* import androidx.compose.ui.Alignment @@ -27,6 +30,7 @@ import com.markel.flowstate.feature.flow.tasks.TaskEditorState import com.markel.flowstate.feature.flow.tasks.TaskEditorViewModel import com.markel.flowstate.feature.flow.tasks.taskOrNull import com.markel.flowstate.feature.tasks.R +import kotlinx.coroutines.launch /** * Full screen for task editing. @@ -54,11 +58,37 @@ fun TaskEditorScreen( val generalCategoryName by viewModel.generalCategoryName.collectAsStateWithLifecycle() val task = editor.taskOrNull + val scope = rememberCoroutineScope() + val snackbarHostState = remember { SnackbarHostState() } + val saveErrorMessage = stringResource(R.string.task_editor_save_error) + var isFlushing by remember { mutableStateOf(false) } + + val requestBack: () -> Unit = { + if (!isFlushing) { + isFlushing = true + scope.launch { + try { + if (viewModel.flushPendingChanges()) { + onBack() + } else { + // Let the user retry by pressing Back again after the message. + isFlushing = false + snackbarHostState.showSnackbar(saveErrorMessage) + } + } finally { + isFlushing = false + } + } + } + } + BackHandler(onBack = requestBack) + Scaffold( modifier = Modifier.sharedDetailBounds( key = TaskSharedKeys.container(taskId) ), contentWindowInsets = WindowInsets(0.dp), + snackbarHost = { SnackbarHost(hostState = snackbarHostState) }, topBar = { TaskEditorTopBar( priority = task?.priority ?: Priority.NOTHING, @@ -71,8 +101,8 @@ fun TaskEditorScreen( onBack() } }, - onBack = onBack, - actionsEnabled = task != null + onBack = requestBack, + actionsEnabled = task != null && !isFlushing ) }, containerColor = MaterialTheme.colorScheme.surface diff --git a/feature/flow/src/main/res/values-en/strings.xml b/feature/flow/src/main/res/values-en/strings.xml index 2d338dfb..7f9cd83a 100644 --- a/feature/flow/src/main/res/values-en/strings.xml +++ b/feature/flow/src/main/res/values-en/strings.xml @@ -66,5 +66,6 @@ Reminder Task not found Could not load the task + Could not save your changes. Please try again. Retry \ No newline at end of file diff --git a/feature/flow/src/main/res/values-es/strings.xml b/feature/flow/src/main/res/values-es/strings.xml index d69886b5..bcae0784 100644 --- a/feature/flow/src/main/res/values-es/strings.xml +++ b/feature/flow/src/main/res/values-es/strings.xml @@ -66,5 +66,6 @@ Recordatorio Tarea no encontrada No se pudo cargar la tarea + No se pudieron guardar los cambios. Inténtalo de nuevo. Reintentar \ No newline at end of file diff --git a/feature/flow/src/main/res/values/strings.xml b/feature/flow/src/main/res/values/strings.xml index 2d338dfb..7f9cd83a 100644 --- a/feature/flow/src/main/res/values/strings.xml +++ b/feature/flow/src/main/res/values/strings.xml @@ -66,5 +66,6 @@ Reminder Task not found Could not load the task + Could not save your changes. Please try again. Retry \ No newline at end of file diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt index 2d33d508..407a8f2f 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt @@ -18,11 +18,14 @@ import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.launch import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Rule @@ -797,6 +800,103 @@ class TaskEditorViewModelTest { assertEquals(2L, (viewModel.editor.value as TaskEditorState.Ready).savedRevision) } + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun flushPendingChanges_savesImmediately_andCancelsPendingDebounce() = runTest { + val original = Task(id = 12, title = "Original", isDone = false) + val persistedTasks = mutableListOf() + coEvery { repository.getTaskById(12) } returns original + coEvery { repository.upsertTask(capture(persistedTasks)) } returns 12L + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(12) + advanceUntilIdle() + + viewModel.updateTitle("Edited before leaving") + + assertTrue(viewModel.flushPendingChanges()) + assertEquals(listOf("Edited before leaving"), persistedTasks.map { it.title }) + + // The cancelled 600 ms timer must not issue a duplicate upsert. + advanceTimeBy(600) + runCurrent() + assertEquals(1, persistedTasks.size) + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun flushPendingChanges_waitsForInFlightWrite_andSavesNewerDraft() = runTest { + val original = Task(id = 13, title = "Original", isDone = false) + val allowFirstWriteToFinish = CompletableDeferred() + val persistedTasks = mutableListOf() + coEvery { repository.getTaskById(13) } returns original + coEvery { repository.upsertTask(capture(persistedTasks)) } coAnswers { + if (persistedTasks.last().title == "First edit") { + allowFirstWriteToFinish.await() + } + 13L + } + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(13) + advanceUntilIdle() + + viewModel.updateTitle("First edit") + advanceTimeBy(600) + runCurrent() // The first upsert is now suspended in the repository. + viewModel.updateTitle("Latest edit") + + val flushDeferred = async { + viewModel.flushPendingChanges() + } + runCurrent() + assertFalse(flushDeferred.isCompleted) + + allowFirstWriteToFinish.complete(Unit) + advanceUntilIdle() + + assertTrue(flushDeferred.await()) + assertEquals(listOf("First edit", "Latest edit"), persistedTasks.map { it.title }) + assertEquals(2L, (viewModel.editor.value as TaskEditorState.Ready).savedRevision) + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun flushPendingChanges_onRepositoryFailure_returnsFalseAndKeepsRevisionDirty() = runTest { + val original = Task(id = 14, title = "Original", isDone = false) + coEvery { repository.getTaskById(14) } returns original + coEvery { repository.upsertTask(any()) } throws IllegalStateException("Database unavailable") + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(14) + advanceUntilIdle() + + viewModel.updateTitle("Unsaved edit") + + assertFalse(viewModel.flushPendingChanges()) + val ready = viewModel.editor.value as TaskEditorState.Ready + assertEquals(1L, ready.autosaveRevision) + assertEquals(0L, ready.savedRevision) + } + @OptIn(ExperimentalCoroutinesApi::class) @Test fun saveCompletingAfterNewEdit_doesNotReplaceNewerDraft() = runTest { From 90642122f750c1aed4ba2277e9be543aa2fba8cf Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 11 Oct 2026 04:01:43 +0200 Subject: [PATCH 07/10] refactor: delete task logic Now the logic is hardened so if a pending update was waiting the lock after deleting a task doesn't recreate it. --- .../feature/flow/tasks/TaskEditorViewModel.kt | 42 +++++++-- .../flow/tasks/components/TaskEditorScreen.kt | 30 +++++-- .../flow/src/main/res/values-en/strings.xml | 1 + .../flow/src/main/res/values-es/strings.xml | 1 + feature/flow/src/main/res/values/strings.xml | 1 + .../flow/tasks/TaskEditorViewModelTest.kt | 90 ++++++++++++++++++- 6 files changed, 147 insertions(+), 18 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index 7089d870..466cd05f 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -82,6 +82,8 @@ class TaskEditorViewModel @Inject constructor( /** Last snapshot successfully written, used as the alarm reconciliation baseline. */ private var persistedTask: Task? = null + private var isDeleted = false + /** User categories, exposed so the editor can populate the category selector. */ val categories: StateFlow> = categoryRepository.getCategories() .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) @@ -98,6 +100,7 @@ class TaskEditorViewModel @Inject constructor( loadTaskJob?.cancel() autosaveJob?.cancel() autosaveJob = null + isDeleted = false persistedTask = null _editor.value = TaskEditorState.Loading loadTaskJob = viewModelScope.launch { @@ -136,6 +139,7 @@ class TaskEditorViewModel @Inject constructor( */ suspend fun flushPendingChanges(): Boolean { return try { + if (isDeleted) return true while (true) { cancelPendingAutosave() @@ -200,10 +204,12 @@ class TaskEditorViewModel @Inject constructor( } fun toggleDone() { + if (isDeleted) return // avoids to recreate a task if is deleted if (_editor.value.taskOrNull == null) return viewModelScope.launch { taskMutationMutex.withLock { + if (isDeleted) return@withLock // double safeguard if the task was deleted waiting for lock val current = _editor.value.taskOrNull ?: return@withLock val updatedTask = toggleTaskUseCase(current) @@ -221,14 +227,39 @@ class TaskEditorViewModel @Inject constructor( } } } - fun deleteTask(task: Task) { - viewModelScope.launch { - reminderScheduler.cancel(task.id) - task.subTasks.forEach { reminderScheduler.cancelSubTask(it.id) } // Cancel all subtask alarms - deleteTaskUseCase(task) + + /** Leaves Ready, cancels the debounce, and serializes deletion with task writes. */ + suspend fun deleteCurrentTask(): Boolean { + val ready = _editor.value as? TaskEditorState.Ready ?: return false + + return try { + cancelPendingAutosave() + taskMutationMutex.withLock { + val task = _editor.value.taskOrNull ?: return@withLock false + reminderScheduler.cancel(task.id) + (task.subTasks + persistedTask?.subTasks.orEmpty()) + .distinctBy { it.id } + .forEach { reminderScheduler.cancelSubTask(it.id) } + + deleteTaskUseCase(task) + persistedTask = null + isDeleted = true + } + true + } catch (cancellation: CancellationException) { + restoreAfterDeleteFailure(ready) + throw cancellation + } catch (_: Exception) { + restoreAfterDeleteFailure(ready) + false } } + private fun restoreAfterDeleteFailure(ready: TaskEditorState.Ready) { + val task = _editor.value.taskOrNull ?: return + if (task != persistedTask) scheduleAutosave() + } + // ── Internal ────────────────────────────────────────────────────────────── @@ -276,6 +307,7 @@ class TaskEditorViewModel @Inject constructor( /** Must be called while holding [taskMutationMutex]. */ private suspend fun persistCurrentDraftLocked() { + if (isDeleted) return val ready = _editor.value as? TaskEditorState.Ready ?: return val task = ready.task val revision = ready.autosaveRevision diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt index c26c551b..64ebb78c 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt @@ -61,22 +61,23 @@ fun TaskEditorScreen( val scope = rememberCoroutineScope() val snackbarHostState = remember { SnackbarHostState() } val saveErrorMessage = stringResource(R.string.task_editor_save_error) - var isFlushing by remember { mutableStateOf(false) } + val deleteErrorMessage = stringResource(R.string.task_editor_delete_error) + var isExitInProgress by remember { mutableStateOf(false) } val requestBack: () -> Unit = { - if (!isFlushing) { - isFlushing = true + if (!isExitInProgress) { + isExitInProgress = true scope.launch { try { if (viewModel.flushPendingChanges()) { onBack() } else { // Let the user retry by pressing Back again after the message. - isFlushing = false + isExitInProgress = false snackbarHostState.showSnackbar(saveErrorMessage) } } finally { - isFlushing = false + isExitInProgress = false } } } @@ -96,13 +97,24 @@ fun TaskEditorScreen( isDone = task?.isDone == true, onComplete = viewModel::toggleDone, onDelete = { - task?.let { - viewModel.deleteTask(it) - onBack() + if (!isExitInProgress) { + isExitInProgress = true + scope.launch { + try { + if (viewModel.deleteCurrentTask()) { + onBack() + } else { + isExitInProgress = false + snackbarHostState.showSnackbar(deleteErrorMessage) + } + } finally { + isExitInProgress = false + } + } } }, onBack = requestBack, - actionsEnabled = task != null && !isFlushing + actionsEnabled = task != null && !isExitInProgress ) }, containerColor = MaterialTheme.colorScheme.surface diff --git a/feature/flow/src/main/res/values-en/strings.xml b/feature/flow/src/main/res/values-en/strings.xml index 7f9cd83a..2999a9da 100644 --- a/feature/flow/src/main/res/values-en/strings.xml +++ b/feature/flow/src/main/res/values-en/strings.xml @@ -67,5 +67,6 @@ Task not found Could not load the task Could not save your changes. Please try again. + Could not delete the task. Please try again. Retry \ No newline at end of file diff --git a/feature/flow/src/main/res/values-es/strings.xml b/feature/flow/src/main/res/values-es/strings.xml index bcae0784..c97f8664 100644 --- a/feature/flow/src/main/res/values-es/strings.xml +++ b/feature/flow/src/main/res/values-es/strings.xml @@ -67,5 +67,6 @@ Tarea no encontrada No se pudo cargar la tarea No se pudieron guardar los cambios. Inténtalo de nuevo. + No se pudo eliminar la tarea. Inténtalo de nuevo. Reintentar \ No newline at end of file diff --git a/feature/flow/src/main/res/values/strings.xml b/feature/flow/src/main/res/values/strings.xml index 7f9cd83a..2999a9da 100644 --- a/feature/flow/src/main/res/values/strings.xml +++ b/feature/flow/src/main/res/values/strings.xml @@ -67,5 +67,6 @@ Task not found Could not load the task Could not save your changes. Please try again. + Could not delete the task. Please try again. Retry \ No newline at end of file diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt index 407a8f2f..63c3d565 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt @@ -19,7 +19,6 @@ import io.mockk.mockk import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.async -import kotlinx.coroutines.launch import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runCurrent @@ -247,6 +246,7 @@ class TaskEditorViewModelTest { coVerify { toggleTaskUseCase(task) } } + @OptIn(ExperimentalCoroutinesApi::class) @Test fun deleteTask_cancelsAlarmsAndCallsDeleteUseCase() = runTest { // GIVEN @@ -255,15 +255,95 @@ class TaskEditorViewModelTest { viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(3) + advanceUntilIdle() // WHEN - viewModel.deleteTask(task) + assertTrue(viewModel.deleteCurrentTask()) // THEN coVerify { reminderScheduler.cancel(3) } coVerify { deleteTaskUseCase(task) } } + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun delete_waitsForInFlightWrite_andPreventsLaterUpsert() = runTest { + val original = Task(id = 15, title = "Original", isDone = false) + val allowWriteToFinish = CompletableDeferred() + val persistedTasks = mutableListOf() + val deletedTasks = mutableListOf() + val operations = mutableListOf() + coEvery { repository.getTaskById(15) } returns original + coEvery { repository.upsertTask(capture(persistedTasks)) } coAnswers { + val savedTask = persistedTasks.last() + operations += "upsert:${savedTask.title}" + if (savedTask.title == "Pending edit") allowWriteToFinish.await() + 15L + } + coEvery { deleteTaskUseCase(capture(deletedTasks)) } coAnswers { + operations += "delete" + Unit + } + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(15) + advanceUntilIdle() + + viewModel.updateTitle("Pending edit") + advanceTimeBy(600) + runCurrent() // The autosave is now suspended inside the repository. + viewModel.updateTitle("Newest draft") // Its debounce must be canceled by deletion. + + val deleteDeferred = async { viewModel.deleteCurrentTask() } + runCurrent() + assertFalse(deleteDeferred.isCompleted) + + allowWriteToFinish.complete(Unit) + advanceUntilIdle() + + assertTrue(deleteDeferred.await()) + assertEquals(listOf("upsert:Pending edit", "delete"), operations) + assertEquals("Newest draft", deletedTasks.single().title) + assertTrue(viewModel.editor.value is TaskEditorState.Ready) + + // Matches the editor's disposal fallback: it must not recreate the deleted task. + viewModel.saveTask() + advanceUntilIdle() + assertEquals(listOf("upsert:Pending edit", "delete"), operations) + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun deleteFailure_restoresEditorAndReschedulesUnsavedDraft() = runTest { + val original = Task(id = 16, title = "Original", isDone = false) + val persistedTasks = mutableListOf() + coEvery { repository.getTaskById(16) } returns original + coEvery { repository.upsertTask(capture(persistedTasks)) } returns 16L + coEvery { deleteTaskUseCase(any()) } throws IllegalStateException("Delete failed") + viewModel = TaskEditorViewModel( + repository, + toggleTaskUseCase, + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + viewModel.loadTask(16) + advanceUntilIdle() + viewModel.updateTitle("Unsaved edit") + + assertFalse(viewModel.deleteCurrentTask()) + assertEquals("Unsaved edit", viewModel.editor.value.taskOrNull?.title) + advanceUntilIdle() + assertEquals(listOf("Unsaved edit"), persistedTasks.map { it.title }) + } + // ── updateReminderTime tests ────────────────────────────────────────────── @Test fun updateReminderTime_with_future_time_cancels_old_and_schedules_new() = runTest { @@ -450,6 +530,7 @@ class TaskEditorViewModelTest { // ── deleteTask with subtask alarms ──────────────────────────────────────── + @OptIn(ExperimentalCoroutinesApi::class) @Test fun deleteTask_cancels_task_alarm_and_all_subtask_alarms() = runTest { val sub1 = SubTask(id = "s1", title = "Sub1", reminderTime = System.currentTimeMillis() + 60_000L) @@ -459,8 +540,9 @@ class TaskEditorViewModelTest { viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) + advanceUntilIdle() - viewModel.deleteTask(task) + assertTrue(viewModel.deleteCurrentTask()) coVerify { reminderScheduler.cancel(10) } // deleteTask cancels ALL subtask alarms (not just ones with reminders) @@ -823,7 +905,7 @@ class TaskEditorViewModelTest { assertTrue(viewModel.flushPendingChanges()) assertEquals(listOf("Edited before leaving"), persistedTasks.map { it.title }) - // The cancelled 600 ms timer must not issue a duplicate upsert. + // The canceled 600 ms timer must not issue a duplicate upsert. advanceTimeBy(600) runCurrent() assertEquals(1, persistedTasks.size) From 9c04edd095ca1ad0f745d82d850e6d9a88348b86 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 11 Oct 2026 17:08:43 +0200 Subject: [PATCH 08/10] refactor: add safeguard when flushing pending changes A deletion may occur before flushed gets the lock, that's why I added a check inside, to return in that case and don't try persisting any change if the task was deleted. --- .../markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index 466cd05f..9ae2b499 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -144,6 +144,9 @@ class TaskEditorViewModel @Inject constructor( cancelPendingAutosave() val flushed = taskMutationMutex.withLock { + // Deletion may have completed while this flush waited for the mutex. + if (isDeleted) return@withLock true + val task = _editor.value.taskOrNull ?: return@withLock true // A blank title is not persisted. if (task.title.isBlank() || task == persistedTask) { From 0b63c07af549b1df09bccf41327cbbe4713869ca Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 11 Oct 2026 19:19:53 +0200 Subject: [PATCH 09/10] refactor: delete disposableEffect The effect is no longer needed, if an emergency autosave is needed is managed by the flushPendingChanges method safely. That method is better because guarantees that the save is made before navigating back if fails shows an error instead of exiting without a warning. --- .../feature/flow/tasks/TaskEditorViewModel.kt | 5 +---- .../flow/tasks/components/TaskEditorScreen.kt | 3 --- .../tasks/components/TaskEditorSheetContent.kt | 18 ------------------ 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index 9ae2b499..3eef7131 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -121,10 +121,7 @@ class TaskEditorViewModel @Inject constructor( } } - /** - * Requests an immediate save. The debounce itself is owned by this ViewModel; - * this entry point remains temporarily for the screen's best-effort dispose flush. - */ + /** Immediately persists editor changes that are not handled by the autosave debounce. */ fun saveTask() { autosaveJob?.cancel() autosaveJob = null diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt index 64ebb78c..7cd9637e 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorScreen.kt @@ -137,14 +137,11 @@ fun TaskEditorScreen( ) is TaskEditorState.Ready -> TaskEditorSheetContent( task = state.task, - autosaveRevision = state.autosaveRevision, - savedRevision = state.savedRevision, onTitleChange = viewModel::updateTitle, onDescriptionChange = viewModel::updateDescription, onSubTasksChange = viewModel::updateSubTasks, onDueDateChange = viewModel::updateDueDate, onReminderTimeChange = viewModel::updateReminderTime, - onAutoUpdate = viewModel::saveTask, categories = categories, categoriesEnabled = categoriesEnabled, categoryId = state.task.categoryId, diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt index e75f917a..abcbef3a 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/components/TaskEditorSheetContent.kt @@ -43,12 +43,9 @@ import java.util.UUID @Composable fun TaskEditorSheetContent( task: Task, - autosaveRevision: Long, - savedRevision: Long, onTitleChange: (String) -> Unit, onDescriptionChange: (String) -> Unit, onSubTasksChange: (List) -> Unit, - onAutoUpdate: () -> Unit, onDueDateChange: (Long?) -> Unit = {}, onReminderTimeChange: (Long?) -> Unit = {}, categories: List = emptyList(), @@ -91,21 +88,6 @@ fun TaskEditorSheetContent( var draftSubDueDate by rememberSaveable { mutableStateOf(null) } var draftSubReminder by rememberSaveable { mutableStateOf(null) } - // The ViewModel owns the debounce. Keep only a temporary best-effort fallback - // for a dirty revision when this content leaves composition. A later mini-phase - // will replace this with an awaited flush before navigation. - val currentRevision by rememberUpdatedState(autosaveRevision) - val currentSavedRevision by rememberUpdatedState(savedRevision) - val currentOnAutoUpdate by rememberUpdatedState(onAutoUpdate) - - DisposableEffect(Unit) { - onDispose { - if (currentRevision > currentSavedRevision) { - currentOnAutoUpdate() - } - } - } - Column ( modifier = Modifier .fillMaxSize() From 66f5955c94df3e1fe5d3a33eee2b97a50ff882d8 Mon Sep 17 00:00:00 2001 From: Markel15 Date: Sun, 11 Oct 2026 21:53:46 +0200 Subject: [PATCH 10/10] refactor: simplify duplicated logic. Use the created methods to reconcile alarms. --- .../feature/flow/tasks/TaskEditorViewModel.kt | 12 +-- .../flow/tasks/TaskEditorViewModelTest.kt | 76 ++++++++++++++++++- 2 files changed, 77 insertions(+), 11 deletions(-) diff --git a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt index 3eef7131..c0aece9c 100644 --- a/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt +++ b/feature/flow/src/main/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModel.kt @@ -211,19 +211,13 @@ class TaskEditorViewModel @Inject constructor( taskMutationMutex.withLock { if (isDeleted) return@withLock // double safeguard if the task was deleted waiting for lock val current = _editor.value.taskOrNull ?: return@withLock + val original = persistedTask ?: current val updatedTask = toggleTaskUseCase(current) persistedTask = updatedTask updateReadyTask { updatedTask } - - if (updatedTask.isDone) { - reminderScheduler.cancel(updatedTask.id) - current.subTasks - .filter { it.reminderTime != null } - .forEach { subTask -> - reminderScheduler.cancelSubTask(subTask.id) - } - } + reconcileTaskAlarm(original = original, updated = updatedTask) + reconcileSubTaskAlarms(original = original, updated = updatedTask) } } } diff --git a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt index 63c3d565..ae9f2075 100644 --- a/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt +++ b/feature/flow/src/test/java/com/markel/flowstate/feature/flow/tasks/TaskEditorViewModelTest.kt @@ -404,7 +404,13 @@ class TaskEditorViewModelTest { fun toggleDone_completing_cancels_task_alarm_and_subtask_alarms_with_reminders() = runTest { val sub1 = SubTask(id = "s1", title = "Sub1", reminderTime = System.currentTimeMillis() + 60_000L) val sub2 = SubTask(id = "s2", title = "Sub2", reminderTime = null) // no reminder → should NOT be canceled - val task = Task(id = 10, title = "Task", isDone = false, subTasks = listOf(sub1, sub2)) + val task = Task( + id = 10, + title = "Task", + isDone = false, + reminderTime = System.currentTimeMillis() + 60_000L, + subTasks = listOf(sub1, sub2) + ) coEvery { repository.getTaskById(any()) } returns task viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) @@ -429,15 +435,81 @@ class TaskEditorViewModelTest { coVerify(exactly = 0) { reminderScheduler.cancelSubTask("s2") } } + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun toggleDone_waitsForInFlightAutosave_andPersistsItsDraft() = runTest { + val reminderTime = System.currentTimeMillis() + 60_000L + val subTask = SubTask(id = "s1", title = "Subtask", reminderTime = reminderTime) + val original = Task( + id = 17, + title = "Original", + isDone = false, + reminderTime = reminderTime, + subTasks = listOf(subTask) + ) + val allowAutosaveToFinish = CompletableDeferred() + val persistedTasks = mutableListOf() + val operations = mutableListOf() + coEvery { repository.getTaskById(17) } returns original + coEvery { repository.upsertTask(capture(persistedTasks)) } coAnswers { + val task = persistedTasks.last() + operations += if (task.isDone) "toggle:${task.title}" else "autosave:${task.title}" + if (task.title == "Edited before toggle" && !task.isDone) { + allowAutosaveToFinish.await() + } + 17L + } + viewModel = TaskEditorViewModel( + repository, + ToggleTaskUseCase(repository), + deleteTaskUseCase, + reminderScheduler, + categoryRepository, + userPreferencesRepository + ) + + viewModel.loadTask(17) + advanceUntilIdle() + + viewModel.updateTitle("Edited before toggle") + advanceTimeBy(600) + runCurrent() // The autosave is now suspended inside the repository. + assertEquals(listOf("autosave:Edited before toggle"), operations) + + viewModel.toggleDone() + runCurrent() // The toggle must wait for the in-flight autosave's mutex. + assertEquals(listOf("autosave:Edited before toggle"), operations) + + allowAutosaveToFinish.complete(Unit) + advanceUntilIdle() + + assertEquals( + listOf("autosave:Edited before toggle", "toggle:Edited before toggle"), + operations + ) + assertEquals(2, persistedTasks.size) + val completedTask = persistedTasks.last() + assertEquals("Edited before toggle", completedTask.title) + assertTrue(completedTask.isDone) + assertNull(completedTask.reminderTime) + assertNull(completedTask.subTasks.single().reminderTime) + assertTrue(viewModel.editor.value.taskOrNull?.isDone == true) + coVerify { reminderScheduler.cancel(17) } + coVerify { reminderScheduler.cancelSubTask("s1") } + } + + @OptIn(ExperimentalCoroutinesApi::class) @Test fun toggleDone_uncompleting_does_not_cancel_alarms() = runTest { val task = Task(id = 10, title = "Done task", isDone = true) - coEvery { repository.getTaskById(any()) } returns task + coEvery { toggleTaskUseCase(task) } returns task.copy(isDone = false, completedAt = null) viewModel = TaskEditorViewModel(repository, toggleTaskUseCase, deleteTaskUseCase, reminderScheduler, categoryRepository, userPreferencesRepository) viewModel.loadTask(10) + advanceUntilIdle() viewModel.toggleDone() + advanceUntilIdle() coVerify(exactly = 0) { reminderScheduler.cancel(any()) } coVerify(exactly = 0) { reminderScheduler.cancelSubTask(any()) }