fix: a task or note never shows twice while its state changes

CHANGELOG.md

@@ -48,6 +48,7 @@ versionCode 30000.
 ### Fixed
 
 - Fingerprint unlock did not start, and its button did nothing, after Encly locked itself in the background ([#104](https://github.com/pasichDev/Encly/issues/104)).
+- Checking or unchecking a task, or archiving a note in a hidden tag's view, could close the app.
 - A new note lost its creation date at the first autosave. Notes that already lost it now sort by their last edit ([#103](https://github.com/pasichDev/Encly/issues/103)).
 - Stepping through matches in Find in note now scrolls to each match, also inside long paragraphs and lists ([#76](https://github.com/pasichDev/Encly/issues/76)).
 - Scrolling over a list no longer leaves its toolbar button active ([#76](https://github.com/pasichDev/Encly/issues/76)).

core/database/api/android/database.api

@@ -156,10 +156,10 @@ public abstract interface class com/pasich/encly/data/database/dao/NotesDao {
 	public abstract fun deleteTrashedBefore (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public abstract fun getAllNotes ()Lkotlinx/coroutines/flow/Flow;
 	public abstract fun getAllNotesWithTags ()Lkotlinx/coroutines/flow/Flow;
-	public abstract fun getArchivedNotesByTagId (J)Lkotlinx/coroutines/flow/Flow;
 	public abstract fun getArchivedNotesWithTags ()Lkotlinx/coroutines/flow/Flow;
 	public abstract fun getNoteById (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public abstract fun getNotesByTagId (Ljava/lang/Long;)Lkotlinx/coroutines/flow/Flow;
+	public abstract fun getNotesOfTagWithArchived (J)Lkotlinx/coroutines/flow/Flow;
 	public abstract fun getTrashNotes ()Lkotlinx/coroutines/flow/Flow;
 	public abstract fun insertNote (Lcom/pasich/encly/data/model/Note;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public abstract fun observeNoteWithTag (J)Lkotlinx/coroutines/flow/Flow;
@@ -181,10 +181,10 @@ public final class com/pasich/encly/data/database/dao/NotesDao_Impl : com/pasich
 	public fun deleteTrashedBefore (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public fun getAllNotes ()Lkotlinx/coroutines/flow/Flow;
 	public fun getAllNotesWithTags ()Lkotlinx/coroutines/flow/Flow;
-	public fun getArchivedNotesByTagId (J)Lkotlinx/coroutines/flow/Flow;
 	public fun getArchivedNotesWithTags ()Lkotlinx/coroutines/flow/Flow;
 	public fun getNoteById (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public fun getNotesByTagId (Ljava/lang/Long;)Lkotlinx/coroutines/flow/Flow;
+	public fun getNotesOfTagWithArchived (J)Lkotlinx/coroutines/flow/Flow;
 	public fun getTrashNotes ()Lkotlinx/coroutines/flow/Flow;
 	public fun insertNote (Lcom/pasich/encly/data/model/Note;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public fun observeNoteWithTag (J)Lkotlinx/coroutines/flow/Flow;

core/database/api/database.klib.api

@@ -36,9 +36,9 @@ abstract interface com.pasich.encly.data.database.dao/BackupDao { // com.pasich.
 abstract interface com.pasich.encly.data.database.dao/NotesDao { // com.pasich.encly.data.database.dao/NotesDao|null[0]
     abstract fun getAllNotes(): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/Note>> // com.pasich.encly.data.database.dao/NotesDao.getAllNotes|getAllNotes(){}[0]
     abstract fun getAllNotesWithTags(): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao.getAllNotesWithTags|getAllNotesWithTags(){}[0]
-    abstract fun getArchivedNotesByTagId(kotlin/Long): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao.getArchivedNotesByTagId|getArchivedNotesByTagId(kotlin.Long){}[0]
     abstract fun getArchivedNotesWithTags(): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao.getArchivedNotesWithTags|getArchivedNotesWithTags(){}[0]
     abstract fun getNotesByTagId(kotlin/Long?): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao.getNotesByTagId|getNotesByTagId(kotlin.Long?){}[0]
+    abstract fun getNotesOfTagWithArchived(kotlin/Long): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao.getNotesOfTagWithArchived|getNotesOfTagWithArchived(kotlin.Long){}[0]
     abstract fun getTrashNotes(): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/Note>> // com.pasich.encly.data.database.dao/NotesDao.getTrashNotes|getTrashNotes(){}[0]
     abstract fun observeNoteWithTag(kotlin/Long): kotlinx.coroutines.flow/Flow<com.pasich.encly.data.model/NoteWithTag?> // com.pasich.encly.data.database.dao/NotesDao.observeNoteWithTag|observeNoteWithTag(kotlin.Long){}[0]
     abstract suspend fun deleteNoteById(kotlin/Long): kotlin/Int // com.pasich.encly.data.database.dao/NotesDao.deleteNoteById|deleteNoteById(kotlin.Long){}[0]
@@ -142,9 +142,9 @@ final class com.pasich.encly.data.database.dao/NotesDao_Impl : com.pasich.encly.
 
     final fun getAllNotes(): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/Note>> // com.pasich.encly.data.database.dao/NotesDao_Impl.getAllNotes|getAllNotes(){}[0]
     final fun getAllNotesWithTags(): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao_Impl.getAllNotesWithTags|getAllNotesWithTags(){}[0]
-    final fun getArchivedNotesByTagId(kotlin/Long): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao_Impl.getArchivedNotesByTagId|getArchivedNotesByTagId(kotlin.Long){}[0]
     final fun getArchivedNotesWithTags(): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao_Impl.getArchivedNotesWithTags|getArchivedNotesWithTags(){}[0]
     final fun getNotesByTagId(kotlin/Long?): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao_Impl.getNotesByTagId|getNotesByTagId(kotlin.Long?){}[0]
+    final fun getNotesOfTagWithArchived(kotlin/Long): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/NoteWithTag>> // com.pasich.encly.data.database.dao/NotesDao_Impl.getNotesOfTagWithArchived|getNotesOfTagWithArchived(kotlin.Long){}[0]
     final fun getTrashNotes(): kotlinx.coroutines.flow/Flow<kotlin.collections/List<com.pasich.encly.data.model/Note>> // com.pasich.encly.data.database.dao/NotesDao_Impl.getTrashNotes|getTrashNotes(){}[0]
     final fun observeNoteWithTag(kotlin/Long): kotlinx.coroutines.flow/Flow<com.pasich.encly.data.model/NoteWithTag?> // com.pasich.encly.data.database.dao/NotesDao_Impl.observeNoteWithTag|observeNoteWithTag(kotlin.Long){}[0]
     final suspend fun deleteNoteById(kotlin/Long): kotlin/Int // com.pasich.encly.data.database.dao/NotesDao_Impl.deleteNoteById|deleteNoteById(kotlin.Long){}[0]

core/database/api/desktop/database.api

@@ -155,10 +155,10 @@ public abstract interface class com/pasich/encly/data/database/dao/NotesDao {
 	public abstract fun deleteTrashedBefore (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public abstract fun getAllNotes ()Lkotlinx/coroutines/flow/Flow;
 	public abstract fun getAllNotesWithTags ()Lkotlinx/coroutines/flow/Flow;
-	public abstract fun getArchivedNotesByTagId (J)Lkotlinx/coroutines/flow/Flow;
 	public abstract fun getArchivedNotesWithTags ()Lkotlinx/coroutines/flow/Flow;
 	public abstract fun getNoteById (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public abstract fun getNotesByTagId (Ljava/lang/Long;)Lkotlinx/coroutines/flow/Flow;
+	public abstract fun getNotesOfTagWithArchived (J)Lkotlinx/coroutines/flow/Flow;
 	public abstract fun getTrashNotes ()Lkotlinx/coroutines/flow/Flow;
 	public abstract fun insertNote (Lcom/pasich/encly/data/model/Note;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public abstract fun observeNoteWithTag (J)Lkotlinx/coroutines/flow/Flow;
@@ -180,10 +180,10 @@ public final class com/pasich/encly/data/database/dao/NotesDao_Impl : com/pasich
 	public fun deleteTrashedBefore (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public fun getAllNotes ()Lkotlinx/coroutines/flow/Flow;
 	public fun getAllNotesWithTags ()Lkotlinx/coroutines/flow/Flow;
-	public fun getArchivedNotesByTagId (J)Lkotlinx/coroutines/flow/Flow;
 	public fun getArchivedNotesWithTags ()Lkotlinx/coroutines/flow/Flow;
 	public fun getNoteById (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public fun getNotesByTagId (Ljava/lang/Long;)Lkotlinx/coroutines/flow/Flow;
+	public fun getNotesOfTagWithArchived (J)Lkotlinx/coroutines/flow/Flow;
 	public fun getTrashNotes ()Lkotlinx/coroutines/flow/Flow;
 	public fun insertNote (Lcom/pasich/encly/data/model/Note;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
 	public fun observeNoteWithTag (J)Lkotlinx/coroutines/flow/Flow;

core/database/src/androidHostTest/kotlin/com/pasich/encly/data/database/NotesDaoTest.kt

@@ -257,7 +257,7 @@ class NotesDaoTest {
     }
 
     @Test
-    fun aTagsArchivedNotesAreReadByTag() = runBlocking {
+    fun aTagsNotesAreReadWithItsArchivedOnesInOneQuery() = runBlocking {
         insert("live", tagId = TAG)
         insert("old", tagId = TAG) { isArchived = true }
         insert("trashed", tagId = TAG) {
@@ -266,7 +266,7 @@ class NotesDaoTest {
         }
         insert("other", tagId = OTHER_TAG) { isArchived = true }
 
-        assertEquals(listOf("old"), titles(dao.getArchivedNotesByTagId(TAG)))
+        assertEquals(listOf("live", "old"), titles(dao.getNotesOfTagWithArchived(TAG)))
     }
 
     @Test

core/database/src/commonMain/kotlin/com/pasich/encly/data/database/dao/NotesDao.kt

@@ -34,10 +34,13 @@ public interface NotesDao {
     )
     public fun getNotesByTagId(tagId: Long?): Flow<List<NoteWithTag>>
 
-    /** The archived notes (not in the trash) of one tag: the way to a hidden tag's archived notes. */
+    /**
+     * The notes (not in the trash) of one tag, archived ones included: the way to a hidden tag's
+     * archived notes. One query, so a note being archived or unarchived is never read twice.
+     */
     @Transaction
-    @Query("SELECT * FROM notes WHERE isTrash = 0 AND isArchived = 1 AND tagId = :tagId")
-    public fun getArchivedNotesByTagId(tagId: Long): Flow<List<NoteWithTag>>
+    @Query("SELECT * FROM notes WHERE isTrash = 0 AND tagId = :tagId")
+    public fun getNotesOfTagWithArchived(tagId: Long): Flow<List<NoteWithTag>>
 
     @Transaction
     @Query("SELECT * FROM notes WHERE id = :noteId")

shared/src/commonMain/kotlin/com/pasich/encly/data/repository/NotesRepositoryImpl.kt

@@ -24,8 +24,8 @@ class NotesRepositoryImpl(private val databaseProvider: DatabaseProvider) : Note
 
     override fun getArchivedNotesWithTag(): Flow<List<NoteWithTag>> = daoFlow { dao().getArchivedNotesWithTags() }
 
-    override fun getArchivedNotesByTagId(tagId: Long): Flow<List<NoteWithTag>> =
-        daoFlow { dao().getArchivedNotesByTagId(tagId) }
+    override fun getNotesOfTagWithArchived(tagId: Long): Flow<List<NoteWithTag>> =
+        daoFlow { dao().getNotesOfTagWithArchived(tagId) }
 
     override fun observeNote(noteId: Long): Flow<NoteWithTag?> = daoFlow { dao().observeNoteWithTag(noteId) }
 

shared/src/commonMain/kotlin/com/pasich/encly/domain/repository/NotesRepository.kt

@@ -13,7 +13,7 @@ interface NotesRepository {
     fun getAllNotesWithTag(): Flow<List<NoteWithTag>>
     fun getNotesByTagId(tagId: Long): Flow<List<NoteWithTag>>
     fun getArchivedNotesWithTag(): Flow<List<NoteWithTag>>
-    fun getArchivedNotesByTagId(tagId: Long): Flow<List<NoteWithTag>>
+    fun getNotesOfTagWithArchived(tagId: Long): Flow<List<NoteWithTag>>
     fun getTrashNotes(): Flow<List<Note>>
 
     /** The note as stored now (null once it is gone), following every change to it. */

shared/src/commonMain/kotlin/com/pasich/encly/domain/usecase/note/ObserveNotesUseCase.kt

@@ -9,7 +9,6 @@ import com.pasich.encly.domain.repository.NotesRepository
 import dev.zacsweers.metro.Inject
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.map
 
@@ -38,10 +37,11 @@ class ObserveNotesUseCase(
 
             tagId == null -> repository.getAllNotesWithTag().map { all -> all.filter { it.isShown() } }
 
-            else -> combine(repository.getNotesByTagId(tagId), repository.getArchivedNotesByTagId(tagId)) { live, old ->
-                // A hidden tag's archived notes are in no other list (the archive leaves hidden tags
-                // out too), so its own view is the way to unarchive them. A visible tag's are in the archive.
-                live + old.filter { it.tag?.isVisible == false }
+            // A hidden tag's archived notes are in no other list (the archive leaves hidden tags out
+            // too), so its own view is the way to unarchive them. A visible tag's are in the archive.
+            // One read for both: two flows combined could hold a note that is being archived twice.
+            else -> repository.getNotesOfTagWithArchived(tagId).map { all ->
+                all.filter { !it.note.isArchived || it.tag?.isVisible == false }
             }
         }
         return notes.map { list -> list.toSortedItems(sortOption) }.flowOn(dispatcher)

shared/src/commonMain/kotlin/com/pasich/encly/presentation/viewmodel/TasksViewModel.kt

@@ -207,14 +207,12 @@ class TasksViewModel(
 
     private fun observeTasks() {
         tasksJob = viewModelScope.launch {
-            combine(
-                tasksRepository.getAllActiveTasks(),
-                tasksRepository.getAllCompletedTasks(),
-                tasksRepository.getActiveTasksCount(),
-                tasksRepository.getCompletedTasksCount(),
-                tasksRepository.observeSubtasks(),
-            ) { activeTasks, completedTasks, activeCount, completedCount, subtasks ->
-                TaskFilterEngine.reduce(_uiState.value, activeTasks, completedTasks, activeCount, completedCount)
+            // Open and done tasks come from one query: two queries combined could hold a task that
+            // was just checked or unchecked in both lists at once, and the list keys tasks by id.
+            combine(tasksRepository.getAllTasks(), tasksRepository.observeSubtasks()) { tasks, subtasks ->
+                val (completedTasks, activeTasks) = tasks.partition { it.isCompleted }
+                val doneNewestFirst = completedTasks.sortedByDescending { it.completedDate }
+                TaskFilterEngine.reduce(_uiState.value, activeTasks, doneNewestFirst, activeTasks.size, completedTasks.size)
                     .copy(subtasks = subtasks.groupBy { it.taskId })
             }.catch {
                 // The vault closed under the list (locked; [clearOnLock] resets the screen right

shared/src/commonTest/kotlin/com/pasich/encly/presentation/viewmodel/TasksViewModelTest.kt

@@ -64,6 +64,27 @@ class TasksViewModelTest {
         assertEquals(TaskFilterEngine.ALL_FILTER_ID, state.selectedActiveFilter?.id)
     }
 
+    @Test
+    fun aTaskBeingCheckedOrUncheckedIsNeverInBothListsAtOnce() = runTest(dispatcher) {
+        advanceUntilIdle()
+        // Every state the screen sees: the list keys tasks by id, so a task in both the open and
+        // the done list at once crashes it.
+        val states = mutableListOf<TasksUiState>()
+        backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { viewModel.uiState.collect(states::add) }
+
+        viewModel.toggleTaskCompletion(taskId = 1, isCompleted = true)
+        advanceUntilIdle()
+        viewModel.toggleTaskCompletion(taskId = 3, isCompleted = false)
+        advanceUntilIdle()
+
+        assertTrue(states.size > 1)
+        states.forEach { state ->
+            val open = state.activeTasks.map { it.id }.toSet()
+            assertTrue(state.completedTasks.none { it.id in open }, "a task is in both lists: $state")
+        }
+        assertEquals(listOf(2L, 3L), viewModel.uiState.value.activeTasks.map { it.id })
+    }
+
     @Test
     fun aReadFailingAfterTheVaultClosedKeepsTheListInsteadOfCrashing() = runTest(dispatcher) {
         advanceUntilIdle()

shared/src/commonTest/kotlin/com/pasich/encly/testutil/TestNotesRepository.kt

@@ -73,8 +73,10 @@ internal class TestNotesRepository : NotesRepository {
 
     override fun getArchivedNotesWithTag(): Flow<List<NoteWithTag>> = archivedNotesWithTags
 
-    override fun getArchivedNotesByTagId(tagId: Long): Flow<List<NoteWithTag>> =
-        archivedNotesWithTags.map { list -> list.filter { it.note.tagId == tagId } }
+    override fun getNotesOfTagWithArchived(tagId: Long): Flow<List<NoteWithTag>> =
+        combine(tagFlow(tagId), archivedNotesWithTags) { live, archived ->
+            live + archived.filter { it.note.tagId == tagId }
+        }
 
     override fun observeNote(noteId: Long): Flow<NoteWithTag?> = combine(allNotesWithTags, archivedNotesWithTags) {
             a,

shared/src/commonTest/kotlin/com/pasich/encly/testutil/TestTasksRepository.kt

@@ -37,7 +37,7 @@ internal class TestTasksRepository(initial: List<Task> = emptyList(), initialSub
         tasks.map { list -> read { list.filter { it.isCompleted } } }
 
     private fun <T> read(block: () -> T): T = if (failReads) error("Database session expired") else block()
-    override fun getAllTasks(): Flow<List<Task>> = tasks
+    override fun getAllTasks(): Flow<List<Task>> = tasks.map { list -> read { list } }
     override fun getActiveTasksCount(): Flow<Int> = getAllActiveTasks().map { it.size }
     override fun getCompletedTasksCount(): Flow<Int> = getAllCompletedTasks().map { it.size }