Various code improvements

tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/ui/component/TaskMenu.kt

@@ -152,7 +152,7 @@ fun TaskMenu(
             text = {
                 Text(stringResource(Res.string.task_menu_move_to), style = MaterialTheme.typography.titleSmall)
             },
-            enabled = false, // TODO support task move to list
+            enabled = false,
             onClick = {}
         )
 
@@ -166,22 +166,19 @@ fun TaskMenu(
 
         // FIXME not ideal when a lot of list, maybe ask for a dialog or bottom sheet in which to choose?
         //  or using a submenu?
-        val enableMoveTaskList = true // TODO should it be hidden when 1 list only?
-        if (enableMoveTaskList) {
-            taskLists.forEach { taskList ->
-                val isCurrentList = taskList.id == currentTaskList?.id
-                DropdownMenuItem(
-                    text = {
-                        RowWithIcon(
-                            icon = LucideIcons.Check.takeIf { isCurrentList },
-                            text = taskList.title,
-                        )
-                    },
-                    modifier = Modifier.testTag(MOVE_TO_LIST),
-                    enabled = !isCurrentList,
-                    onClick = { onAction(TaskAction.MoveToList(task, taskList)) }
-                )
-            }
+        taskLists.forEach { taskList ->
+            val isCurrentList = taskList.id == currentTaskList?.id
+            DropdownMenuItem(
+                text = {
+                    RowWithIcon(
+                        icon = LucideIcons.Check.takeIf { isCurrentList },
+                        text = taskList.title,
+                    )
+                },
+                modifier = Modifier.testTag(MOVE_TO_LIST),
+                enabled = !isCurrentList,
+                onClick = { onAction(TaskAction.MoveToList(task, taskList)) }
+            )
         }
 
         HorizontalDivider()

tasks-app-shared/src/commonTest/kotlin/net/opatry/tasks/data/TaskRepositoryCRUDTest.kt

@@ -30,6 +30,7 @@ import net.opatry.tasks.data.model.TaskListDataModel
 import net.opatry.tasks.data.util.runTaskRepositoryTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
@@ -84,6 +85,13 @@ class TaskRepositoryCRUDTest {
         assertEquals("My renamed list", taskListRenamed.title, "Updated name is invalid")
     }
 
+    @Test
+    fun `rename unavailable task list should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task list id 42") {
+            repository.renameTaskList(42L, "toto")
+        }
+    }
+
     @Test
     fun `delete task list`() = runTaskRepositoryTest { repository ->
         val taskList = repository.createAndGetTaskList("My tasks")
@@ -95,6 +103,13 @@ class TaskRepositoryCRUDTest {
         assertEquals(0, taskLists.size, "No task list expected")
     }
 
+    @Test
+    fun `delete unavailable task list should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task list id 42") {
+            repository.deleteTaskList(42L)
+        }
+    }
+
     @Test
     fun `create task`() = runTaskRepositoryTest { repository ->
         val taskList = repository.createAndGetTaskList("My tasks")
@@ -127,6 +142,21 @@ class TaskRepositoryCRUDTest {
         assertEquals("task1", lastTask.title, "last task should be first created")
     }
 
+    @Test
+    fun `create task in an unavailable task list should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task list id 42") {
+            repository.createTask(42L, null, "toto")
+        }
+    }
+
+    @Test
+    fun `create task with an unavailable parent task should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        val taskList = repository.createAndGetTaskList("My tasks")
+        assertFailsWith<IllegalArgumentException>("Invalid parent task id 42") {
+            repository.createTask(taskList.id, 42L, "toto")
+        }
+    }
+
     @Test
     fun `rename task`() = runTaskRepositoryTest { repository ->
         val (taskList, task) = repository.createAndGetTask("My tasks", "My task")
@@ -139,6 +169,13 @@ class TaskRepositoryCRUDTest {
         assertEquals("My renamed task", tasks.first().title)
     }
 
+    @Test
+    fun `rename unavailable task should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.updateTaskTitle(42L, "toto")
+        }
+    }
+
     @Test
     fun `edit task with all parameters`() = runTaskRepositoryTest { repository ->
         val (taskList, task) = repository.createAndGetTask("My tasks", "My task")
@@ -154,6 +191,13 @@ class TaskRepositoryCRUDTest {
         assertEquals(updatedDate, tasks.first().dueDate)
     }
 
+    @Test
+    fun `edit unavailable task should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.updateTask(42L, "toto", "titi", null)
+        }
+    }
+
     @Test
     fun `edit task notes`() = runTaskRepositoryTest { repository ->
         val (taskList, task) = repository.createAndGetTask("My tasks", "My task")
@@ -166,6 +210,13 @@ class TaskRepositoryCRUDTest {
         assertEquals("These are some notes", tasks.first().notes)
     }
 
+    @Test
+    fun `edit unavailable task notes should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.updateTaskNotes(42L, "toto")
+        }
+    }
+
     @Test
     fun `edit task due date`() = runTaskRepositoryTest { repository ->
         val (taskList, task) = repository.createAndGetTask("My tasks", "My task")
@@ -191,6 +242,13 @@ class TaskRepositoryCRUDTest {
         assertEquals(null, tasks.first().dueDate)
     }
 
+    @Test
+    fun `edit unavailable task due date should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.updateTaskDueDate(42L, null)
+        }
+    }
+
     @Test
     fun `complete task`() = runTaskRepositoryTest { repository ->
         val (taskList, task) = repository.createAndGetTask("My tasks", "My task")
@@ -203,6 +261,13 @@ class TaskRepositoryCRUDTest {
         assertTrue(tasks.first().isCompleted)
     }
 
+    @Test
+    fun `toggle unavailable task completion state should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.toggleTaskCompletionState(42L)
+        }
+    }
+
     @Test
     fun `delete task`() = runTaskRepositoryTest { repository ->
         val (taskList, task) = repository.createAndGetTask("My tasks", "My task")
@@ -214,6 +279,13 @@ class TaskRepositoryCRUDTest {
         assertEquals(0, tasks.size, "Task should have been deleted")
     }
 
+    @Test
+    fun `delete unavailable task should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.deleteTask(42L)
+        }
+    }
+
     @Test
     fun `delete task should recompute remaining tasks positions`() = runTaskRepositoryTest { repository ->
         val (taskList, task1) = repository.createAndGetTask("My tasks", "task1")
@@ -244,6 +316,23 @@ class TaskRepositoryCRUDTest {
         assertEquals("00000000000000000001", tasks[1].position, "position should reflect new order")
     }
 
+    @Test
+    fun `move unavailable task to top should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.moveToTop(42L)
+        }
+    }
+
+    @Test
+    fun `move completed task to top should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        val (_, task1) = repository.createAndGetTask("tasks", "t1")
+        repository.toggleTaskCompletionState(task1.id)
+
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.moveToTop(task1.id)
+        }
+    }
+
     @Test
     fun `move task to list`() = runTaskRepositoryTest { repository ->
         val (taskList1, task1) = repository.createAndGetTask("list1", "t1")
@@ -270,6 +359,22 @@ class TaskRepositoryCRUDTest {
         assertEquals("00000000000000000001", updatedTaskList2.tasks[1].position, "task from second list should have their position updated")
     }
 
+    @Test
+    fun `move unavailable task to list should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        val taskList = repository.createAndGetTaskList("list1")
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.moveToList(42L, taskList.id)
+        }
+    }
+
+    @Test
+    fun `move task to unavailable list should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        val (_, task) = repository.createAndGetTask("list1", "task1")
+        assertFailsWith<IllegalArgumentException>("Invalid task list id 42") {
+            repository.moveToList(task.id, 42L)
+        }
+    }
+
     @Test
     fun `move task to new list`() = runTaskRepositoryTest { repository ->
         val (taskList1, task1) = repository.createAndGetTask("list1", "t1")
@@ -292,6 +397,13 @@ class TaskRepositoryCRUDTest {
         assertEquals("00000000000000000000", updatedTask.position, "task should be moved at first position")
     }
 
+    @Test
+    fun `move unavailable task to new list should throw IllegalArgumentException`() = runTaskRepositoryTest { repository ->
+        assertFailsWith<IllegalArgumentException>("Invalid task id 42") {
+            repository.moveToNewList(42L, "toto")
+        }
+    }
+
     @Test
     fun `sorting tasks by title should honor title (ignore case) and ignore parent task link`() =
         runTaskRepositoryTest { repository ->

tasks-app-shared/src/commonTest/kotlin/net/opatry/tasks/data/util/InMemoryTasksApi.kt

@@ -73,14 +73,14 @@ class InMemoryTasksApi(
                 if (tasks.isEmpty()) {
                     return@handleRequest
                 }
-                tasks.map { task ->
+
+                storage[taskListId] = tasks.map { task ->
                     if (task.isCompleted) {
                         task.copy(isHidden = true)
                     } else {
                         task
                     }
-                }
-                storage[taskListId] = tasks
+                }.toMutableList()
             }
         }
     }
@@ -92,14 +92,14 @@ class InMemoryTasksApi(
                 if (tasks.none { it.id == taskId }) {
                     error("Task ($taskId) not found in task list ($taskListId)")
                 }
-                tasks.map { task ->
+                val updatedTasks = tasks.map { task ->
                     if (task.id == taskId) {
                         task.copy(isDeleted = true, isHidden = true)
                     } else {
                         task
                     }
                 }
-                storage[taskListId] = recomputeTaskPositions(tasks).toMutableList()
+                storage[taskListId] = recomputeTaskPositions(updatedTasks).toMutableList()
             }
         }
     }
@@ -256,14 +256,13 @@ class InMemoryTasksApi(
         return handleRequest("update") {
             synchronized(this) {
                 val tasks = storage[taskListId] ?: error("Task list ($taskListId) not found")
-                tasks.map { task ->
-                    if (task.id == taskId) {
+                storage[taskListId] = tasks.map { initialTask ->
+                    if (initialTask.id == taskId) {
                         task.copy(updatedDate = Clock.System.now())
                     } else {
-                        task
+                        initialTask
                     }
-                }
-                storage[taskListId] = tasks
+                }.toMutableList()
                 task
             }
         }

tasks-app-shared/src/commonTest/kotlin/net/opatry/tasks/data/util/runTaskRepositoryTest.kt

@@ -23,6 +23,7 @@
 package net.opatry.tasks.data.util
 
 import androidx.sqlite.driver.bundled.BundledSQLiteDriver
+import kotlinx.coroutines.flow.firstOrNull
 import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runTest
 import kotlinx.datetime.Clock
@@ -32,6 +33,21 @@ import net.opatry.tasks.InMemoryTasksApi
 import net.opatry.tasks.NowProvider
 import net.opatry.tasks.data.TaskRepository
 
+internal suspend fun TaskRepository.printTaskTree() {
+    getTaskLists().firstOrNull()?.let { taskLists ->
+        if (taskLists.isEmpty()) {
+            println("No task lists found.")
+            return
+        }
+        for (taskList in taskLists) {
+            println("- ${taskList.title} (#${taskList.tasks.count()})")
+            taskList.tasks.forEach { task ->
+                val tabs = "  ".repeat(task.indent + 1)
+                println("$tabs- ${task.title} (@{${task.position}} >[${task.indent}])")
+            }
+        }
+    } ?: println("Task lists not ready.")
+}
 
 internal fun runTaskRepositoryTest(
     taskListsApi: TaskListsApi = InMemoryTaskListsApi(),
@@ -43,10 +59,12 @@ internal fun runTaskRepositoryTest(
         .setQueryCoroutineContext(backgroundScope.coroutineContext)
         .build()
 
+    val repository = TaskRepository(db.getTaskListDao(), db.getTaskDao(), taskListsApi, tasksApi, NowProvider(Clock.System::now))
     try {
-        val repository = TaskRepository(db.getTaskListDao(), db.getTaskDao(), taskListsApi, tasksApi, NowProvider(Clock.System::now))
-
         test(repository)
+    } catch (e: AssertionError) {
+        repository.printTaskTree()
+        throw e
     } finally {
         db.close()
     }

tasks-core/src/commonMain/kotlin/net/opatry/tasks/data/TaskRepository.kt

@@ -60,11 +60,16 @@ private fun TaskList.asTaskListEntity(localId: Long?, sorting: TaskListEntity.So
     )
 }
 
-private fun Task.asTaskEntity(parentLocalId: Long, localId: Long?, parentTaskLocalId: Long?): TaskEntity {
+// TODO invert taskId & parentTaskId parameters
+// Do it so that:
+//  no risk of tedious conflict
+//  replace call site with name=
+//  ensure call site order is properly switch accordingly (/!\ IDEA "flip ','" doesn't do it for us)
+private fun Task.asTaskEntity(parentListLocalId: Long, taskLocalId: Long?, parentTaskLocalId: Long?): TaskEntity {
     return TaskEntity(
-        id = localId ?: 0,
+        id = taskLocalId ?: 0,
         remoteId = id,
-        parentListLocalId = parentLocalId,
+        parentListLocalId = parentListLocalId,
         parentTaskLocalId = parentTaskLocalId,
         parentTaskRemoteId = parent,
         etag = etag,