fix: separate local create validation from server errors in board detail viewmodel

This commit is contained in:
2026-03-16 14:09:44 -04:00
parent 5e0eff37a6
commit 6a18d6679a
2 changed files with 106 additions and 47 deletions

View File

@@ -14,6 +14,8 @@ import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import space.hackenslacker.kanbn4droid.app.boards.BoardsApiResult import space.hackenslacker.kanbn4droid.app.boards.BoardsApiResult
private const val ADD_CARD_DISABLED_MESSAGE = "Create a list first to add cards."
data class BoardDetailUiState( data class BoardDetailUiState(
val isInitialLoading: Boolean = false, val isInitialLoading: Boolean = false,
val isRefreshing: Boolean = false, val isRefreshing: Boolean = false,
@@ -34,7 +36,9 @@ data class BoardDetailUiState(
val isFilterDialogOpen: Boolean = false, val isFilterDialogOpen: Boolean = false,
val isSearchDialogOpen: Boolean = false, val isSearchDialogOpen: Boolean = false,
val addListTitleDraft: String = "", val addListTitleDraft: String = "",
val addListTitleError: String? = null,
val addCardTitleDraft: String = "", val addCardTitleDraft: String = "",
val addCardTitleError: String? = null,
val addCardDescriptionDraft: String = "", val addCardDescriptionDraft: String = "",
val addCardDueDate: LocalDate? = null, val addCardDueDate: LocalDate? = null,
val addCardSelectedTagIds: Set<String> = emptySet(), val addCardSelectedTagIds: Set<String> = emptySet(),
@@ -43,7 +47,7 @@ data class BoardDetailUiState(
get() = boardDetail?.lists?.isNotEmpty() == true get() = boardDetail?.lists?.isNotEmpty() == true
val addCardDisabledMessage: String? val addCardDisabledMessage: String?
get() = if (canAddCard) null else "Add a list before creating cards." get() = if (canAddCard) null else ADD_CARD_DISABLED_MESSAGE
val filteredBoardDetail: BoardDetail? val filteredBoardDetail: BoardDetail?
get() = boardDetail?.withFilteredCards( get() = boardDetail?.withFilteredCards(
@@ -198,12 +202,18 @@ class BoardDetailViewModel(
it.copy( it.copy(
isFabChooserOpen = false, isFabChooserOpen = false,
isAddListDialogOpen = true, isAddListDialogOpen = true,
addListTitleError = null,
) )
} }
} }
fun updateAddListTitle(title: String) { fun updateAddListTitle(title: String) {
_uiState.update { it.copy(addListTitleDraft = title) } _uiState.update {
it.copy(
addListTitleDraft = title,
addListTitleError = null,
)
}
} }
fun cancelAddListDialog() { fun cancelAddListDialog() {
@@ -211,6 +221,7 @@ class BoardDetailViewModel(
it.copy( it.copy(
isAddListDialogOpen = false, isAddListDialogOpen = false,
addListTitleDraft = "", addListTitleDraft = "",
addListTitleError = null,
) )
} }
} }
@@ -223,26 +234,19 @@ class BoardDetailViewModel(
val detail = snapshot.boardDetail ?: return val detail = snapshot.boardDetail ?: return
val title = snapshot.addListTitleDraft.trim() val title = snapshot.addListTitleDraft.trim()
if (title.isBlank()) { if (title.isBlank()) {
viewModelScope.launch { setAddListLocalValidationError("List title is required")
_events.emit(BoardDetailUiEvent.ShowServerError("List title is required"))
}
return return
} }
val expectedIndex = detail.lists.size val expectedIndex = detail.lists.size
viewModelScope.launch { viewModelScope.launch {
_uiState.update { it.copy(isMutating = true) } beginCreateListMutation()
when (val result = repository.createList(detail.id, title)) { when (val result = repository.createList(detail.id, title)) {
is BoardsApiResult.Success -> { is BoardsApiResult.Success -> {
_uiState.update { closeAddListDialogAndResetDrafts()
it.copy(
isAddListDialogOpen = false,
addListTitleDraft = "",
)
}
when (val reload = reloadDetailAndReconcile()) { when (val reload = reloadDetailAndReconcile()) {
is BoardsApiResult.Success -> { is BoardsApiResult.Success -> {
_uiState.update { it.copy(isMutating = false) } endMutation()
val warning = verifyCreatedList(result.value.publicId, expectedIndex, reload.value) val warning = verifyCreatedList(result.value.publicId, expectedIndex, reload.value)
if (warning != null) { if (warning != null) {
_events.emit(BoardDetailUiEvent.ShowWarning(warning)) _events.emit(BoardDetailUiEvent.ShowWarning(warning))
@@ -250,18 +254,14 @@ class BoardDetailViewModel(
} }
is BoardsApiResult.Failure -> { is BoardsApiResult.Failure -> {
_uiState.update { it.copy(isMutating = false) } endMutation()
_events.emit( emitRefreshFailureWarning()
BoardDetailUiEvent.ShowWarning(
"Changes applied, but refresh failed. Pull to refresh.",
),
)
} }
} }
} }
is BoardsApiResult.Failure -> { is BoardsApiResult.Failure -> {
_uiState.update { it.copy(isMutating = false) } endMutation()
_events.emit(BoardDetailUiEvent.ShowServerError(result.message)) _events.emit(BoardDetailUiEvent.ShowServerError(result.message))
} }
} }
@@ -279,12 +279,18 @@ class BoardDetailViewModel(
it.copy( it.copy(
isFabChooserOpen = false, isFabChooserOpen = false,
isAddCardDialogOpen = true, isAddCardDialogOpen = true,
addCardTitleError = null,
) )
} }
} }
fun updateAddCardTitle(title: String) { fun updateAddCardTitle(title: String) {
_uiState.update { it.copy(addCardTitleDraft = title) } _uiState.update {
it.copy(
addCardTitleDraft = title,
addCardTitleError = null,
)
}
} }
fun updateAddCardDescription(description: String) { fun updateAddCardDescription(description: String) {
@@ -327,14 +333,12 @@ class BoardDetailViewModel(
val currentList = detail.lists.getOrNull(snapshot.currentPageIndex) ?: return val currentList = detail.lists.getOrNull(snapshot.currentPageIndex) ?: return
val title = snapshot.addCardTitleDraft.trim() val title = snapshot.addCardTitleDraft.trim()
if (title.isBlank()) { if (title.isBlank()) {
viewModelScope.launch { setAddCardLocalValidationError("Card title is required")
_events.emit(BoardDetailUiEvent.ShowServerError("Card title is required"))
}
return return
} }
viewModelScope.launch { viewModelScope.launch {
_uiState.update { it.copy(isMutating = true) } beginCreateCardMutation()
when ( when (
val result = repository.createCard( val result = repository.createCard(
listPublicId = currentList.id, listPublicId = currentList.id,
@@ -345,12 +349,10 @@ class BoardDetailViewModel(
) )
) { ) {
is BoardsApiResult.Success -> { is BoardsApiResult.Success -> {
_uiState.update { closeAddCardDialogAndResetDrafts()
it.resetAddCardDrafts().copy(isAddCardDialogOpen = false)
}
when (val reload = reloadDetailAndReconcile()) { when (val reload = reloadDetailAndReconcile()) {
is BoardsApiResult.Success -> { is BoardsApiResult.Success -> {
_uiState.update { it.copy(isMutating = false) } endMutation()
val warning = verifyCreatedCard( val warning = verifyCreatedCard(
createdCardId = result.value.publicId, createdCardId = result.value.publicId,
targetListId = currentList.id, targetListId = currentList.id,
@@ -362,18 +364,14 @@ class BoardDetailViewModel(
} }
is BoardsApiResult.Failure -> { is BoardsApiResult.Failure -> {
_uiState.update { it.copy(isMutating = false) } endMutation()
_events.emit( emitRefreshFailureWarning()
BoardDetailUiEvent.ShowWarning(
"Changes applied, but refresh failed. Pull to refresh.",
),
)
} }
} }
} }
is BoardsApiResult.Failure -> { is BoardsApiResult.Failure -> {
_uiState.update { it.copy(isMutating = false) } endMutation()
_events.emit(BoardDetailUiEvent.ShowServerError(result.message)) _events.emit(BoardDetailUiEvent.ShowServerError(result.message))
} }
} }
@@ -768,6 +766,52 @@ class BoardDetailViewModel(
return if (actualIndex != 0) CREATE_CARD_PLACEMENT_WARNING else null return if (actualIndex != 0) CREATE_CARD_PLACEMENT_WARNING else null
} }
private fun setAddListLocalValidationError(message: String) {
_uiState.update { it.copy(addListTitleError = message) }
}
private fun setAddCardLocalValidationError(message: String) {
_uiState.update { it.copy(addCardTitleError = message) }
}
private fun beginCreateListMutation() {
_uiState.update {
it.copy(
isMutating = true,
addListTitleError = null,
)
}
}
private fun beginCreateCardMutation() {
_uiState.update {
it.copy(
isMutating = true,
addCardTitleError = null,
)
}
}
private fun closeAddListDialogAndResetDrafts() {
_uiState.update { it.resetAddListDrafts().copy(isAddListDialogOpen = false) }
}
private fun closeAddCardDialogAndResetDrafts() {
_uiState.update { it.resetAddCardDrafts().copy(isAddCardDialogOpen = false) }
}
private fun endMutation() {
_uiState.update { it.copy(isMutating = false) }
}
private suspend fun emitRefreshFailureWarning() {
_events.emit(
BoardDetailUiEvent.ShowWarning(
"Changes applied, but refresh failed. Pull to refresh.",
),
)
}
private companion object { private companion object {
const val CREATE_LIST_PLACEMENT_WARNING = const val CREATE_LIST_PLACEMENT_WARNING =
"List created, but server ordering differed. Pull to refresh if needed." "List created, but server ordering differed. Pull to refresh if needed."
@@ -797,12 +841,20 @@ class BoardDetailViewModel(
private fun BoardDetailUiState.resetAddCardDrafts(): BoardDetailUiState { private fun BoardDetailUiState.resetAddCardDrafts(): BoardDetailUiState {
return copy( return copy(
addCardTitleDraft = "", addCardTitleDraft = "",
addCardTitleError = null,
addCardDescriptionDraft = "", addCardDescriptionDraft = "",
addCardDueDate = null, addCardDueDate = null,
addCardSelectedTagIds = emptySet(), addCardSelectedTagIds = emptySet(),
) )
} }
private fun BoardDetailUiState.resetAddListDrafts(): BoardDetailUiState {
return copy(
addListTitleDraft = "",
addListTitleError = null,
)
}
private fun BoardDetail.withFilteredCards(tagFilterIds: Set<String>, titleQuery: String): BoardDetail { private fun BoardDetail.withFilteredCards(tagFilterIds: Set<String>, titleQuery: String): BoardDetail {
val normalizedTitleQuery = titleQuery.trim() val normalizedTitleQuery = titleQuery.trim()
if (tagFilterIds.isEmpty() && normalizedTitleQuery.isEmpty()) { if (tagFilterIds.isEmpty() && normalizedTitleQuery.isEmpty()) {

View File

@@ -10,6 +10,7 @@ import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.test.setMain import kotlinx.coroutines.test.setMain
import kotlinx.coroutines.withTimeoutOrNull
import java.time.LocalDate import java.time.LocalDate
import org.junit.After import org.junit.After
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
@@ -71,7 +72,7 @@ class BoardDetailViewModelTest {
viewModel.openAddCardDialog() viewModel.openAddCardDialog()
assertFalse(viewModel.uiState.value.canAddCard) assertFalse(viewModel.uiState.value.canAddCard)
assertEquals("Add a list before creating cards.", viewModel.uiState.value.addCardDisabledMessage) assertEquals("Create a list first to add cards.", viewModel.uiState.value.addCardDisabledMessage)
assertFalse(viewModel.uiState.value.isAddCardDialogOpen) assertFalse(viewModel.uiState.value.isAddCardDialogOpen)
} }
@@ -207,12 +208,11 @@ class BoardDetailViewModelTest {
} }
@Test @Test
fun createList_blankTitle_rejectsBeforeRepositoryCall() = runTest { fun createList_blankTitle_setsLocalError_andRejectsBeforeRepositoryCall() = runTest {
val repository = FakeBoardDetailDataSource( val repository = FakeBoardDetailDataSource(
boardDetailResults = ArrayDeque(listOf(BoardsApiResult.Success(detailWithSingleList()))), boardDetailResults = ArrayDeque(listOf(BoardsApiResult.Success(detailWithSingleList()))),
) )
val viewModel = newLoadedViewModel(this, repository) val viewModel = newLoadedViewModel(this, repository)
val eventDeferred = async { viewModel.events.first() }
viewModel.openAddListDialog() viewModel.openAddListDialog()
viewModel.updateAddListTitle(" ") viewModel.updateAddListTitle(" ")
@@ -220,18 +220,21 @@ class BoardDetailViewModelTest {
advanceUntilIdle() advanceUntilIdle()
assertEquals(0, repository.createListCalls) assertEquals(0, repository.createListCalls)
val event = eventDeferred.await() assertEquals("List title is required", viewModel.uiState.value.addListTitleError)
assertTrue(event is BoardDetailUiEvent.ShowServerError) assertTrue(viewModel.uiState.value.isAddListDialogOpen)
assertEquals("List title is required", (event as BoardDetailUiEvent.ShowServerError).message) val event = withTimeoutOrNull(50) { viewModel.events.first() }
assertNull(event)
viewModel.updateAddListTitle("Valid")
assertNull(viewModel.uiState.value.addListTitleError)
} }
@Test @Test
fun createCard_blankTitle_rejectsBeforeRepositoryCall() = runTest { fun createCard_blankTitle_setsLocalError_andRejectsBeforeRepositoryCall() = runTest {
val repository = FakeBoardDetailDataSource( val repository = FakeBoardDetailDataSource(
boardDetailResults = ArrayDeque(listOf(BoardsApiResult.Success(detailWithSingleList()))), boardDetailResults = ArrayDeque(listOf(BoardsApiResult.Success(detailWithSingleList()))),
) )
val viewModel = newLoadedViewModel(this, repository) val viewModel = newLoadedViewModel(this, repository)
val eventDeferred = async { viewModel.events.first() }
viewModel.openAddCardDialog() viewModel.openAddCardDialog()
viewModel.updateAddCardTitle(" ") viewModel.updateAddCardTitle(" ")
@@ -239,9 +242,13 @@ class BoardDetailViewModelTest {
advanceUntilIdle() advanceUntilIdle()
assertEquals(0, repository.createCardCalls) assertEquals(0, repository.createCardCalls)
val event = eventDeferred.await() assertEquals("Card title is required", viewModel.uiState.value.addCardTitleError)
assertTrue(event is BoardDetailUiEvent.ShowServerError) assertTrue(viewModel.uiState.value.isAddCardDialogOpen)
assertEquals("Card title is required", (event as BoardDetailUiEvent.ShowServerError).message) val event = withTimeoutOrNull(50) { viewModel.events.first() }
assertNull(event)
viewModel.updateAddCardTitle("Valid")
assertNull(viewModel.uiState.value.addCardTitleError)
} }
@Test @Test