feat(kanban): synchronize task mutations across columns and guard concurrent moves
This commit is contained in:
M57 committed
1 parent
c57e056d71
commit
6fd697f2b0
2 files changed
+339
-14
No files matched your search
@@ -26,6 +26,8 @@ import kotlinx.coroutines.launch
|
||||
|
||||
data class KanbanUiState(
|
||||
val isLoading: Boolean = false,
|
||||
val isCreatingTask: Boolean = false,
|
||||
val operatingTaskIds: Set<String> = emptySet(),
|
||||
val boards: List<KanbanBoard> = emptyList(),
|
||||
val selectedBoard: KanbanBoard? = null,
|
||||
val columns: List<KanbanColumn> = emptyList(),
|
||||
@@ -180,9 +182,11 @@ class KanbanViewModel(
|
||||
fun createTask(
|
||||
title: String,
|
||||
description: String?,
|
||||
status: String,
|
||||
status: String = "todo",
|
||||
) {
|
||||
val board = _uiState.value.selectedBoard ?: return
|
||||
if (_uiState.value.isCreatingTask) return
|
||||
_uiState.update { it.copy(isCreatingTask = true) }
|
||||
viewModelScope.launch {
|
||||
val result =
|
||||
repository.createTask(
|
||||
@@ -195,12 +199,24 @@ class KanbanViewModel(
|
||||
)
|
||||
when (result) {
|
||||
is NetworkResult.Success -> {
|
||||
_uiState.update { it.copy(toastMessage = "Task created successfully") }
|
||||
val warning = result.data.warning
|
||||
val msg =
|
||||
if (!warning.isNullOrBlank()) {
|
||||
"Task created: $warning"
|
||||
} else {
|
||||
"Task created successfully"
|
||||
}
|
||||
_uiState.update { it.copy(isCreatingTask = false, toastMessage = msg) }
|
||||
reloadBoardSilently()
|
||||
}
|
||||
|
||||
is NetworkResult.Failure -> {
|
||||
_uiState.update { it.copy(toastMessage = "Failed to create task: ${result.error.message}") }
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
isCreatingTask = false,
|
||||
toastMessage = "Failed to create task: ${result.error.message}",
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -212,14 +228,31 @@ class KanbanViewModel(
|
||||
summary: String? = null,
|
||||
) {
|
||||
val board = _uiState.value.selectedBoard ?: return
|
||||
if (task.id in _uiState.value.operatingTaskIds) return
|
||||
val originalStatus = task.status
|
||||
// Optimistically update, desktop-style
|
||||
|
||||
// Optimistically update both tasks and columns, desktop-style
|
||||
_uiState.update { state ->
|
||||
val updatedTasks =
|
||||
state.tasks.map {
|
||||
if (it.id == task.id) it.copy(status = action.targetStatus) else it
|
||||
}
|
||||
val updatedColumns =
|
||||
state.columns.map { col ->
|
||||
val filtered = col.tasks.filter { it.id != task.id }
|
||||
val newTasks =
|
||||
if (col.name.equals(action.targetStatus, ignoreCase = true)) {
|
||||
val moved = updatedTasks.find { it.id == task.id }
|
||||
if (moved != null) filtered + moved else filtered
|
||||
} else {
|
||||
filtered
|
||||
}
|
||||
col.copy(tasks = newTasks)
|
||||
}
|
||||
state.copy(
|
||||
tasks =
|
||||
state.tasks.map {
|
||||
if (it.id == task.id) it.copy(status = action.targetStatus) else it
|
||||
},
|
||||
tasks = updatedTasks,
|
||||
columns = updatedColumns,
|
||||
operatingTaskIds = state.operatingTaskIds + task.id,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -231,8 +264,38 @@ class KanbanViewModel(
|
||||
result = if (action.needsSummary && !summary.isNullOrBlank()) summary else null,
|
||||
)
|
||||
val result = repository.updateTask(task.id, board = board.id, body = body)
|
||||
if (result is NetworkResult.Failure) {
|
||||
revertTaskMove(task.id, originalStatus, "Move failed: ${result.error.message}")
|
||||
when (result) {
|
||||
is NetworkResult.Success -> {
|
||||
val returnedTask = result.data.task
|
||||
_uiState.update { state ->
|
||||
val finalStatus = returnedTask?.status ?: action.targetStatus
|
||||
val updatedTasks =
|
||||
state.tasks.map {
|
||||
if (it.id == task.id) (returnedTask ?: it.copy(status = finalStatus)) else it
|
||||
}
|
||||
val updatedColumns =
|
||||
state.columns.map { col ->
|
||||
val filtered = col.tasks.filter { it.id != task.id }
|
||||
val newTasks =
|
||||
if (col.name.equals(finalStatus, ignoreCase = true)) {
|
||||
val moved = updatedTasks.find { it.id == task.id }
|
||||
if (moved != null) filtered + moved else filtered
|
||||
} else {
|
||||
filtered
|
||||
}
|
||||
col.copy(tasks = newTasks)
|
||||
}
|
||||
state.copy(
|
||||
tasks = updatedTasks,
|
||||
columns = updatedColumns,
|
||||
operatingTaskIds = state.operatingTaskIds - task.id,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
is NetworkResult.Failure -> {
|
||||
revertTaskMove(task.id, originalStatus, "Move failed: ${result.error.message}")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -243,11 +306,26 @@ class KanbanViewModel(
|
||||
errorMsg: String,
|
||||
) {
|
||||
_uiState.update { state ->
|
||||
val updatedTasks =
|
||||
state.tasks.map {
|
||||
if (it.id == taskId) it.copy(status = originalStatus) else it
|
||||
}
|
||||
val updatedColumns =
|
||||
state.columns.map { col ->
|
||||
val filtered = col.tasks.filter { it.id != taskId }
|
||||
val newTasks =
|
||||
if (col.name.equals(originalStatus, ignoreCase = true)) {
|
||||
val moved = updatedTasks.find { it.id == taskId }
|
||||
if (moved != null) filtered + moved else filtered
|
||||
} else {
|
||||
filtered
|
||||
}
|
||||
col.copy(tasks = newTasks)
|
||||
}
|
||||
state.copy(
|
||||
tasks =
|
||||
state.tasks.map {
|
||||
if (it.id == taskId) it.copy(status = originalStatus) else it
|
||||
},
|
||||
tasks = updatedTasks,
|
||||
columns = updatedColumns,
|
||||
operatingTaskIds = state.operatingTaskIds - taskId,
|
||||
toastMessage = errorMsg,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,247 @@
|
||||
package com.m57.hermescontrol.ui.kanban
|
||||
|
||||
import com.m57.hermescontrol.data.local.InMemoryKanbanPreferencesStore
|
||||
import com.m57.hermescontrol.data.model.CreateTaskBody
|
||||
import com.m57.hermescontrol.data.model.CreateTaskResponse
|
||||
import com.m57.hermescontrol.data.model.KanbanBoard
|
||||
import com.m57.hermescontrol.data.model.KanbanBoardResponse
|
||||
import com.m57.hermescontrol.data.model.KanbanColumn
|
||||
import com.m57.hermescontrol.data.model.KanbanTask
|
||||
import com.m57.hermescontrol.data.model.UpdateTaskBody
|
||||
import com.m57.hermescontrol.data.model.UpdateTaskResponse
|
||||
import com.m57.hermescontrol.data.remote.NetworkError
|
||||
import com.m57.hermescontrol.data.remote.NetworkResult
|
||||
import com.m57.hermescontrol.data.repository.KanbanRepository
|
||||
import com.m57.hermescontrol.data.ws.KanbanEventsClient
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
import io.mockk.mockk
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.test.StandardTestDispatcher
|
||||
import kotlinx.coroutines.test.resetMain
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlinx.coroutines.test.setMain
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
class KanbanMutationTest {
|
||||
private val testDispatcher = StandardTestDispatcher()
|
||||
private val mockRepository = mockk<KanbanRepository>(relaxed = true)
|
||||
private val mockEventsClient = mockk<KanbanEventsClient>(relaxed = true)
|
||||
private val preferences = InMemoryKanbanPreferencesStore()
|
||||
|
||||
private fun createViewModel(): KanbanViewModel =
|
||||
KanbanViewModel(
|
||||
repository = mockRepository,
|
||||
preferences = preferences,
|
||||
eventsClientProvider = { mockEventsClient },
|
||||
endpointProvider = { "http://127.0.0.1:9119" },
|
||||
)
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
Dispatchers.setMain(testDispatcher)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
Dispatchers.resetMain()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testCreateTaskSuccessWithWarning() =
|
||||
runTest(testDispatcher) {
|
||||
val board = KanbanBoard(id = "dev", name = "Dev")
|
||||
val newTask = KanbanTask(id = "t_100", title = "New task", status = "todo")
|
||||
|
||||
coEvery { mockRepository.getBoard("dev") } returns
|
||||
NetworkResult.Success(KanbanBoardResponse(columns = listOf(KanbanColumn("todo", emptyList()))))
|
||||
coEvery { mockRepository.createTask("dev", any()) } returns
|
||||
NetworkResult.Success(CreateTaskResponse(task = newTask, warning = "Dispatcher not running"))
|
||||
|
||||
val vm = createViewModel()
|
||||
vm.selectBoard(board)
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
vm.createTask("New task", "Description", "todo")
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
coVerify {
|
||||
mockRepository.createTask(
|
||||
"dev",
|
||||
match { it.title == "New task" && it.body == "Description" },
|
||||
)
|
||||
}
|
||||
assertTrue(
|
||||
vm.uiState.value.toastMessage
|
||||
?.contains("Dispatcher not running") == true,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testCreateTaskFailurePreservesStateAndShowsToast() =
|
||||
runTest(testDispatcher) {
|
||||
val board = KanbanBoard(id = "dev", name = "Dev")
|
||||
coEvery { mockRepository.getBoard("dev") } returns
|
||||
NetworkResult.Success(KanbanBoardResponse(columns = listOf(KanbanColumn("todo", emptyList()))))
|
||||
coEvery { mockRepository.createTask("dev", any()) } returns
|
||||
NetworkResult.Failure(NetworkError.Http(400, "Validation failed"))
|
||||
|
||||
val vm = createViewModel()
|
||||
vm.selectBoard(board)
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
vm.createTask("Bad task", null, "todo")
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
assertTrue(
|
||||
vm.uiState.value.toastMessage
|
||||
?.contains("Validation failed") == true,
|
||||
)
|
||||
assertFalse(vm.uiState.value.isCreatingTask)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testMoveTaskUpdatesBothTasksAndColumnsOptimistically() =
|
||||
runTest(testDispatcher) {
|
||||
val board = KanbanBoard(id = "dev", name = "Dev")
|
||||
val task = KanbanTask(id = "t_1", title = "Task 1", status = "todo")
|
||||
val colTodo = KanbanColumn("todo", listOf(task))
|
||||
val colReady = KanbanColumn("ready", emptyList())
|
||||
|
||||
val deferred = CompletableDeferred<NetworkResult<UpdateTaskResponse>>()
|
||||
|
||||
coEvery { mockRepository.getBoard("dev") } returns
|
||||
NetworkResult.Success(KanbanBoardResponse(columns = listOf(colTodo, colReady)))
|
||||
coEvery { mockRepository.updateTask("t_1", "dev", any()) } coAnswers { deferred.await() }
|
||||
|
||||
val vm = createViewModel()
|
||||
vm.selectBoard(board)
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
// Initiate move
|
||||
vm.moveTask(task, KanbanTaskAction.READY)
|
||||
|
||||
// Optimistic check: both tasks AND columns must reflect "ready"!
|
||||
assertEquals(
|
||||
"ready",
|
||||
vm.uiState.value.tasks
|
||||
.first { it.id == "t_1" }
|
||||
.status,
|
||||
)
|
||||
assertTrue(
|
||||
vm.uiState.value.columns
|
||||
.first { it.name == "todo" }
|
||||
.tasks
|
||||
.none { it.id == "t_1" },
|
||||
)
|
||||
assertTrue(
|
||||
vm.uiState.value.columns
|
||||
.first { it.name == "ready" }
|
||||
.tasks
|
||||
.any { it.id == "t_1" },
|
||||
)
|
||||
assertTrue(
|
||||
vm.uiState.value.operatingTaskIds
|
||||
.contains("t_1"),
|
||||
)
|
||||
|
||||
// Complete network call
|
||||
deferred.complete(NetworkResult.Success(UpdateTaskResponse(task = task.copy(status = "ready"))))
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
assertFalse(
|
||||
vm.uiState.value.operatingTaskIds
|
||||
.contains("t_1"),
|
||||
)
|
||||
assertEquals(
|
||||
"ready",
|
||||
vm.uiState.value.tasks
|
||||
.first { it.id == "t_1" }
|
||||
.status,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testMoveTaskFailureRevertsTasksAndColumns() =
|
||||
runTest(testDispatcher) {
|
||||
val board = KanbanBoard(id = "dev", name = "Dev")
|
||||
val task = KanbanTask(id = "t_1", title = "Task 1", status = "todo")
|
||||
val colTodo = KanbanColumn("todo", listOf(task))
|
||||
val colReady = KanbanColumn("ready", emptyList())
|
||||
|
||||
coEvery { mockRepository.getBoard("dev") } returns
|
||||
NetworkResult.Success(KanbanBoardResponse(columns = listOf(colTodo, colReady)))
|
||||
coEvery { mockRepository.updateTask("t_1", "dev", any()) } returns
|
||||
NetworkResult.Failure(NetworkError.Http(409, "Parent blocked"))
|
||||
|
||||
val vm = createViewModel()
|
||||
vm.selectBoard(board)
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
vm.moveTask(task, KanbanTaskAction.READY)
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
// Reverted back to "todo" in both tasks and columns!
|
||||
assertEquals(
|
||||
"todo",
|
||||
vm.uiState.value.tasks
|
||||
.first { it.id == "t_1" }
|
||||
.status,
|
||||
)
|
||||
assertTrue(
|
||||
vm.uiState.value.columns
|
||||
.first { it.name == "todo" }
|
||||
.tasks
|
||||
.any { it.id == "t_1" },
|
||||
)
|
||||
assertTrue(
|
||||
vm.uiState.value.columns
|
||||
.first { it.name == "ready" }
|
||||
.tasks
|
||||
.none { it.id == "t_1" },
|
||||
)
|
||||
assertTrue(
|
||||
vm.uiState.value.toastMessage
|
||||
?.contains("Parent blocked") == true,
|
||||
)
|
||||
assertFalse(
|
||||
vm.uiState.value.operatingTaskIds
|
||||
.contains("t_1"),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testDuplicateConcurrentMoveIgnored() =
|
||||
runTest(testDispatcher) {
|
||||
val board = KanbanBoard(id = "dev", name = "Dev")
|
||||
val task = KanbanTask(id = "t_1", title = "Task 1", status = "todo")
|
||||
val colTodo = KanbanColumn("todo", listOf(task))
|
||||
|
||||
val deferred = CompletableDeferred<NetworkResult<UpdateTaskResponse>>()
|
||||
coEvery { mockRepository.getBoard("dev") } returns
|
||||
NetworkResult.Success(KanbanBoardResponse(columns = listOf(colTodo)))
|
||||
coEvery { mockRepository.updateTask("t_1", "dev", any()) } coAnswers { deferred.await() }
|
||||
|
||||
val vm = createViewModel()
|
||||
vm.selectBoard(board)
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
// First tap
|
||||
vm.moveTask(task, KanbanTaskAction.READY)
|
||||
// Second duplicate tap while in-flight
|
||||
vm.moveTask(task, KanbanTaskAction.ARCHIVE)
|
||||
|
||||
testDispatcher.scheduler.runCurrent()
|
||||
|
||||
// Only 1 updateTask call initiated
|
||||
coVerify(exactly = 1) { mockRepository.updateTask("t_1", "dev", any()) }
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user