diff --git a/app/src/main/java/space/hackenslacker/kanbn4droid/app/boarddetail/BoardDetailViewModel.kt b/app/src/main/java/space/hackenslacker/kanbn4droid/app/boarddetail/BoardDetailViewModel.kt index fc49471..5bb8dbd 100644 --- a/app/src/main/java/space/hackenslacker/kanbn4droid/app/boarddetail/BoardDetailViewModel.kt +++ b/app/src/main/java/space/hackenslacker/kanbn4droid/app/boarddetail/BoardDetailViewModel.kt @@ -14,6 +14,8 @@ import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import space.hackenslacker.kanbn4droid.app.boards.BoardsApiResult +private const val ADD_CARD_DISABLED_MESSAGE = "Create a list first to add cards." + data class BoardDetailUiState( val isInitialLoading: Boolean = false, val isRefreshing: Boolean = false, @@ -34,7 +36,9 @@ data class BoardDetailUiState( val isFilterDialogOpen: Boolean = false, val isSearchDialogOpen: Boolean = false, val addListTitleDraft: String = "", + val addListTitleError: String? = null, val addCardTitleDraft: String = "", + val addCardTitleError: String? = null, val addCardDescriptionDraft: String = "", val addCardDueDate: LocalDate? = null, val addCardSelectedTagIds: Set = emptySet(), @@ -43,7 +47,7 @@ data class BoardDetailUiState( get() = boardDetail?.lists?.isNotEmpty() == true 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? get() = boardDetail?.withFilteredCards( @@ -198,12 +202,18 @@ class BoardDetailViewModel( it.copy( isFabChooserOpen = false, isAddListDialogOpen = true, + addListTitleError = null, ) } } fun updateAddListTitle(title: String) { - _uiState.update { it.copy(addListTitleDraft = title) } + _uiState.update { + it.copy( + addListTitleDraft = title, + addListTitleError = null, + ) + } } fun cancelAddListDialog() { @@ -211,6 +221,7 @@ class BoardDetailViewModel( it.copy( isAddListDialogOpen = false, addListTitleDraft = "", + addListTitleError = null, ) } } @@ -223,26 +234,19 @@ class BoardDetailViewModel( val detail = snapshot.boardDetail ?: return val title = snapshot.addListTitleDraft.trim() if (title.isBlank()) { - viewModelScope.launch { - _events.emit(BoardDetailUiEvent.ShowServerError("List title is required")) - } + setAddListLocalValidationError("List title is required") return } val expectedIndex = detail.lists.size viewModelScope.launch { - _uiState.update { it.copy(isMutating = true) } + beginCreateListMutation() when (val result = repository.createList(detail.id, title)) { is BoardsApiResult.Success -> { - _uiState.update { - it.copy( - isAddListDialogOpen = false, - addListTitleDraft = "", - ) - } + closeAddListDialogAndResetDrafts() when (val reload = reloadDetailAndReconcile()) { is BoardsApiResult.Success -> { - _uiState.update { it.copy(isMutating = false) } + endMutation() val warning = verifyCreatedList(result.value.publicId, expectedIndex, reload.value) if (warning != null) { _events.emit(BoardDetailUiEvent.ShowWarning(warning)) @@ -250,18 +254,14 @@ class BoardDetailViewModel( } is BoardsApiResult.Failure -> { - _uiState.update { it.copy(isMutating = false) } - _events.emit( - BoardDetailUiEvent.ShowWarning( - "Changes applied, but refresh failed. Pull to refresh.", - ), - ) + endMutation() + emitRefreshFailureWarning() } } } is BoardsApiResult.Failure -> { - _uiState.update { it.copy(isMutating = false) } + endMutation() _events.emit(BoardDetailUiEvent.ShowServerError(result.message)) } } @@ -279,12 +279,18 @@ class BoardDetailViewModel( it.copy( isFabChooserOpen = false, isAddCardDialogOpen = true, + addCardTitleError = null, ) } } fun updateAddCardTitle(title: String) { - _uiState.update { it.copy(addCardTitleDraft = title) } + _uiState.update { + it.copy( + addCardTitleDraft = title, + addCardTitleError = null, + ) + } } fun updateAddCardDescription(description: String) { @@ -327,14 +333,12 @@ class BoardDetailViewModel( val currentList = detail.lists.getOrNull(snapshot.currentPageIndex) ?: return val title = snapshot.addCardTitleDraft.trim() if (title.isBlank()) { - viewModelScope.launch { - _events.emit(BoardDetailUiEvent.ShowServerError("Card title is required")) - } + setAddCardLocalValidationError("Card title is required") return } viewModelScope.launch { - _uiState.update { it.copy(isMutating = true) } + beginCreateCardMutation() when ( val result = repository.createCard( listPublicId = currentList.id, @@ -345,12 +349,10 @@ class BoardDetailViewModel( ) ) { is BoardsApiResult.Success -> { - _uiState.update { - it.resetAddCardDrafts().copy(isAddCardDialogOpen = false) - } + closeAddCardDialogAndResetDrafts() when (val reload = reloadDetailAndReconcile()) { is BoardsApiResult.Success -> { - _uiState.update { it.copy(isMutating = false) } + endMutation() val warning = verifyCreatedCard( createdCardId = result.value.publicId, targetListId = currentList.id, @@ -362,18 +364,14 @@ class BoardDetailViewModel( } is BoardsApiResult.Failure -> { - _uiState.update { it.copy(isMutating = false) } - _events.emit( - BoardDetailUiEvent.ShowWarning( - "Changes applied, but refresh failed. Pull to refresh.", - ), - ) + endMutation() + emitRefreshFailureWarning() } } } is BoardsApiResult.Failure -> { - _uiState.update { it.copy(isMutating = false) } + endMutation() _events.emit(BoardDetailUiEvent.ShowServerError(result.message)) } } @@ -768,6 +766,52 @@ class BoardDetailViewModel( 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 { const val CREATE_LIST_PLACEMENT_WARNING = "List created, but server ordering differed. Pull to refresh if needed." @@ -797,12 +841,20 @@ class BoardDetailViewModel( private fun BoardDetailUiState.resetAddCardDrafts(): BoardDetailUiState { return copy( addCardTitleDraft = "", + addCardTitleError = null, addCardDescriptionDraft = "", addCardDueDate = null, addCardSelectedTagIds = emptySet(), ) } +private fun BoardDetailUiState.resetAddListDrafts(): BoardDetailUiState { + return copy( + addListTitleDraft = "", + addListTitleError = null, + ) +} + private fun BoardDetail.withFilteredCards(tagFilterIds: Set, titleQuery: String): BoardDetail { val normalizedTitleQuery = titleQuery.trim() if (tagFilterIds.isEmpty() && normalizedTitleQuery.isEmpty()) { diff --git a/app/src/test/java/space/hackenslacker/kanbn4droid/app/boarddetail/BoardDetailViewModelTest.kt b/app/src/test/java/space/hackenslacker/kanbn4droid/app/boarddetail/BoardDetailViewModelTest.kt index d81117b..a7f1687 100644 --- a/app/src/test/java/space/hackenslacker/kanbn4droid/app/boarddetail/BoardDetailViewModelTest.kt +++ b/app/src/test/java/space/hackenslacker/kanbn4droid/app/boarddetail/BoardDetailViewModelTest.kt @@ -10,6 +10,7 @@ import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeoutOrNull import java.time.LocalDate import org.junit.After import org.junit.Assert.assertEquals @@ -71,7 +72,7 @@ class BoardDetailViewModelTest { viewModel.openAddCardDialog() 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) } @@ -207,12 +208,11 @@ class BoardDetailViewModelTest { } @Test - fun createList_blankTitle_rejectsBeforeRepositoryCall() = runTest { + fun createList_blankTitle_setsLocalError_andRejectsBeforeRepositoryCall() = runTest { val repository = FakeBoardDetailDataSource( boardDetailResults = ArrayDeque(listOf(BoardsApiResult.Success(detailWithSingleList()))), ) val viewModel = newLoadedViewModel(this, repository) - val eventDeferred = async { viewModel.events.first() } viewModel.openAddListDialog() viewModel.updateAddListTitle(" ") @@ -220,18 +220,21 @@ class BoardDetailViewModelTest { advanceUntilIdle() assertEquals(0, repository.createListCalls) - val event = eventDeferred.await() - assertTrue(event is BoardDetailUiEvent.ShowServerError) - assertEquals("List title is required", (event as BoardDetailUiEvent.ShowServerError).message) + assertEquals("List title is required", viewModel.uiState.value.addListTitleError) + assertTrue(viewModel.uiState.value.isAddListDialogOpen) + val event = withTimeoutOrNull(50) { viewModel.events.first() } + assertNull(event) + + viewModel.updateAddListTitle("Valid") + assertNull(viewModel.uiState.value.addListTitleError) } @Test - fun createCard_blankTitle_rejectsBeforeRepositoryCall() = runTest { + fun createCard_blankTitle_setsLocalError_andRejectsBeforeRepositoryCall() = runTest { val repository = FakeBoardDetailDataSource( boardDetailResults = ArrayDeque(listOf(BoardsApiResult.Success(detailWithSingleList()))), ) val viewModel = newLoadedViewModel(this, repository) - val eventDeferred = async { viewModel.events.first() } viewModel.openAddCardDialog() viewModel.updateAddCardTitle(" ") @@ -239,9 +242,13 @@ class BoardDetailViewModelTest { advanceUntilIdle() assertEquals(0, repository.createCardCalls) - val event = eventDeferred.await() - assertTrue(event is BoardDetailUiEvent.ShowServerError) - assertEquals("Card title is required", (event as BoardDetailUiEvent.ShowServerError).message) + assertEquals("Card title is required", viewModel.uiState.value.addCardTitleError) + assertTrue(viewModel.uiState.value.isAddCardDialogOpen) + val event = withTimeoutOrNull(50) { viewModel.events.first() } + assertNull(event) + + viewModel.updateAddCardTitle("Valid") + assertNull(viewModel.uiState.value.addCardTitleError) } @Test