diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenErrorContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenErrorContent.kt new file mode 100644 index 000000000..02bceeed6 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenErrorContent.kt @@ -0,0 +1,49 @@ +package com.x8bit.bitwarden.ui.platform.components + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.unit.dp +import com.x8bit.bitwarden.R + +/** + * A Bitwarden-themed, re-usable error state. + * + * Note that when [onTryAgainClick] is absent, there will be no "Try again" button displayed. + */ +@Composable +fun BitwardenErrorContent( + message: String, + modifier: Modifier = Modifier, + onTryAgainClick: (() -> Unit)? = null, +) { + Column( + modifier = modifier, + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Text( + text = message, + color = MaterialTheme.colorScheme.onSurface, + style = MaterialTheme.typography.bodyMedium, + textAlign = TextAlign.Center, + modifier = Modifier.fillMaxWidth(), + ) + onTryAgainClick?.let { + Spacer(modifier = Modifier.height(16.dp)) + BitwardenTextButton( + label = stringResource(id = R.string.try_again), + onClick = it, + ) + } + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt index bea063797..6c34a72c9 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt @@ -25,11 +25,15 @@ import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect +import com.x8bit.bitwarden.ui.platform.components.BasicDialogState import com.x8bit.bitwarden.ui.platform.components.BitwardenAccountActionItem import com.x8bit.bitwarden.ui.platform.components.BitwardenAccountSwitcher +import com.x8bit.bitwarden.ui.platform.components.BitwardenBasicDialog +import com.x8bit.bitwarden.ui.platform.components.BitwardenErrorContent import com.x8bit.bitwarden.ui.platform.components.BitwardenMediumTopAppBar import com.x8bit.bitwarden.ui.platform.components.BitwardenOverflowActionItem import com.x8bit.bitwarden.ui.platform.components.BitwardenScaffold @@ -125,6 +129,12 @@ fun VaultScreen( trashClick = remember(viewModel) { { viewModel.trySendAction(VaultAction.TrashClick) } }, + tryAgainClick = remember(viewModel) { + { viewModel.trySendAction(VaultAction.TryAgainClick) } + }, + dialogDismiss = remember(viewModel) { + { viewModel.trySendAction(VaultAction.DialogDismiss) } + }, ) } @@ -151,6 +161,8 @@ private fun VaultScreenScaffold( identityGroupClick: () -> Unit, secureNoteGroupClick: () -> Unit, trashClick: () -> Unit, + tryAgainClick: () -> Unit, + dialogDismiss: () -> Unit, ) { var accountMenuVisible by rememberSaveable { mutableStateOf(false) @@ -165,6 +177,20 @@ private fun VaultScreenScaffold( canScroll = { !accountMenuVisible }, ) + when (val dialog = state.dialog) { + is VaultState.DialogState.Error -> { + BitwardenBasicDialog( + visibilityState = BasicDialogState.Shown( + title = dialog.title, + message = dialog.message, + ), + onDismissRequest = dialogDismiss, + ) + } + + null -> Unit + } + BitwardenScaffold( topBar = { BitwardenMediumTopAppBar( @@ -229,9 +255,15 @@ private fun VaultScreenScaffold( modifier = modifier, addItemClickAction = addItemClickAction, ) + + is VaultState.ViewState.Error -> BitwardenErrorContent( + message = viewState.message(), + onTryAgainClick = tryAgainClick, + modifier = modifier + .padding(horizontal = 16.dp), + ) } - val context = LocalContext.current BitwardenAccountSwitcher( isVisible = accountMenuVisible, accountSummaries = state.accountSummaries.toImmutableList(), diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt index c7bdc7186..22a342c17 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt @@ -95,6 +95,8 @@ class VaultViewModel @Inject constructor( is VaultAction.SecureNoteGroupClick -> handleSecureNoteClick() is VaultAction.TrashClick -> handleTrashClick() is VaultAction.VaultItemClick -> handleVaultItemClick(action) + is VaultAction.TryAgainClick -> handleTryAgainClick() + is VaultAction.DialogDismiss -> handleDialogDismiss() is VaultAction.Internal.UserStateUpdateReceive -> handleUserStateUpdateReceive(action) is VaultAction.Internal.VaultDataReceive -> handleVaultDataReceive(action) } @@ -176,6 +178,16 @@ class VaultViewModel @Inject constructor( sendEvent(VaultEvent.NavigateToVaultItem(action.vaultItem.id)) } + private fun handleTryAgainClick() { + vaultRepository.sync() + } + + private fun handleDialogDismiss() { + mutableStateFlow.update { + it.copy(dialog = null) + } + } + private fun handleUserStateUpdateReceive(action: VaultAction.Internal.UserStateUpdateReceive) { // Leave the current data alone if there is no UserState; we are in the process of logging // out. @@ -225,9 +237,27 @@ class VaultViewModel @Inject constructor( } private fun vaultNoNetworkReceive(vaultData: DataState.NoNetwork) { - // TODO update state to no network state BIT-1158 - mutableStateFlow.update { it.copy(viewState = VaultState.ViewState.NoItems) } - sendEvent(VaultEvent.ShowToast(message = "Vault no network state not yet implemented")) + val title = R.string.internet_connection_required_title.asText() + val message = R.string.internet_connection_required_message.asText() + if (vaultData.data != null) { + mutableStateFlow.update { + it.copy( + viewState = vaultData.data.toViewState(), + dialog = VaultState.DialogState.Error( + title = title, + message = message, + ), + ) + } + } else { + mutableStateFlow.update { + it.copy( + viewState = VaultState.ViewState.Error( + message = message, + ), + ) + } + } } private fun vaultPendingReceive(vaultData: DataState.Pending) { @@ -245,6 +275,7 @@ class VaultViewModel @Inject constructor( * @property initials The initials to be displayed on the avatar. * @property accountSummaries List of all the current accounts. * @property viewState The specific view state representing loading, no items, or content state. + * @property dialog Information about any dialogs that may need to be displayed. * @property isSwitchingAccounts Whether or not we are actively switching accounts. */ @Parcelize @@ -253,6 +284,7 @@ data class VaultState( val initials: String, val accountSummaries: List, val viewState: ViewState, + val dialog: DialogState? = null, // Internal-use properties val isSwitchingAccounts: Boolean = false, ) : Parcelable { @@ -280,6 +312,15 @@ data class VaultState( @Parcelize data object NoItems : ViewState() + /** + * Represents a state where the [VaultScreen] is unable to display data due to an error + * retrieving it. The given [message] should be displayed. + */ + @Parcelize + data class Error( + val message: Text, + ) : ViewState() + /** * Content state for the [VaultScreen] showing the actual content or items. * @@ -433,6 +474,21 @@ data class VaultState( } } + /** + * Information about a dialog to display. + */ + sealed class DialogState : Parcelable { + + /** + * Represents an error dialog with the given [title] and [message]. + */ + @Parcelize + data class Error( + val title: Text, + val message: Text, + ) : DialogState() + } + companion object { /** * The maximum number of no folder items that can be displayed before the UI creates a @@ -572,6 +628,16 @@ sealed class VaultAction { */ data object TrashClick : VaultAction() + /** + * The user has requested that any visible dialogs are dismissed. + */ + data object DialogDismiss : VaultAction() + + /** + * User clicked the Try Again button when there is an error displayed. + */ + data object TryAgainClick : VaultAction() + /** * Models actions that the [VaultViewModel] itself might send. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt index e06ed8284..10d10abaf 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt @@ -3,9 +3,12 @@ package com.x8bit.bitwarden.ui.vault.feature.vault import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertTextEquals import androidx.compose.ui.test.filterToOne +import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasClickAction import androidx.compose.ui.test.hasScrollToNodeAction import androidx.compose.ui.test.hasText +import androidx.compose.ui.test.isDialog +import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick @@ -176,6 +179,104 @@ class VaultScreenTest : BaseComposeTest() { composeTestRule.assertNoDialogExists() } + @Test + fun `error dialog should be shown or hidden according to the state`() { + val errorTitle = "Error title" + val errorMessage = "Error message" + composeTestRule.assertNoDialogExists() + composeTestRule + .onNodeWithText(errorTitle) + .assertDoesNotExist() + composeTestRule + .onNodeWithText(errorMessage) + .assertDoesNotExist() + + mutableStateFlow.update { + it.copy( + dialog = VaultState.DialogState.Error( + title = errorTitle.asText(), + message = errorMessage.asText(), + ), + ) + } + + composeTestRule + .onAllNodesWithText(errorTitle) + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + composeTestRule + .onAllNodesWithText(errorMessage) + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + } + + @Test + fun `OK button click in error dialog should send DialogDismiss`() { + val errorTitle = "Error title" + val errorMessage = "Error message" + mutableStateFlow.update { + it.copy( + dialog = VaultState.DialogState.Error( + title = errorTitle.asText(), + message = errorMessage.asText(), + ), + ) + } + + composeTestRule + .onAllNodesWithText("Ok") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + verify { viewModel.trySendAction(VaultAction.DialogDismiss) } + } + + @Test + fun `Error screen should be shown according to the state`() { + val errorMessage = "Error message" + val tryAgainButtonText = "Try again" + composeTestRule + .onNodeWithText(errorMessage) + .assertDoesNotExist() + composeTestRule + .onNodeWithText(tryAgainButtonText) + .assertDoesNotExist() + + mutableStateFlow.update { + it.copy( + viewState = VaultState.ViewState.Error( + message = errorMessage.asText(), + ), + ) + } + + composeTestRule + .onNodeWithText(errorMessage) + .assertIsDisplayed() + composeTestRule + .onNodeWithText(tryAgainButtonText) + .assertIsDisplayed() + } + + @Test + fun `try again button click on the Error screen should send TryAgainClick`() { + val errorMessage = "Error message" + val tryAgainButtonText = "Try again" + mutableStateFlow.update { + it.copy( + viewState = VaultState.ViewState.Error( + message = errorMessage.asText(), + ), + ) + } + + composeTestRule + .onNodeWithText(tryAgainButtonText) + .performClick() + + verify { viewModel.trySendAction(VaultAction.TryAgainClick) } + } + @Test fun `search icon click should send SearchIconClick action`() { mutableStateFlow.update { it.copy(viewState = VaultState.ViewState.NoItems) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt index b45c9f199..efb5464dc 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.ui.vault.feature.vault import app.cash.turbine.test +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult import com.x8bit.bitwarden.data.auth.repository.model.UserState @@ -352,25 +353,99 @@ class VaultViewModelTest : BaseViewModelTest() { } @Test - fun `vaultDataStateFlow NoNetwork should show toast and update state to NoItems`() = runTest { + fun `vaultDataStateFlow NoNetwork without data should update state to Error`() = runTest { mutableVaultDataStateFlow.tryEmit( value = DataState.NoNetwork(), ) val viewModel = createViewModel() - viewModel.eventFlow.test { - assertEquals( - VaultEvent.ShowToast("Vault no network state not yet implemented"), - awaitItem(), - ) - } assertEquals( - createMockVaultState(viewState = VaultState.ViewState.NoItems), + createMockVaultState( + viewState = VaultState.ViewState.Error( + message = R.string.internet_connection_required_message.asText(), + ), + ), viewModel.stateFlow.value, ) } + @Suppress("MaxLineLength") + @Test + fun `vaultDataStateFlow NoNetwork with items should update state to Content and show an error dialog`() = + runTest { + mutableVaultDataStateFlow.tryEmit( + value = DataState.NoNetwork( + data = VaultData( + cipherViewList = listOf(createMockCipherView(number = 1)), + collectionViewList = listOf(createMockCollectionView(number = 1)), + folderViewList = listOf(createMockFolderView(number = 1)), + ), + ), + ) + + val viewModel = createViewModel() + + assertEquals( + createMockVaultState( + viewState = VaultState.ViewState.Content( + loginItemsCount = 1, + cardItemsCount = 0, + identityItemsCount = 0, + secureNoteItemsCount = 0, + favoriteItems = listOf(), + folderItems = listOf( + VaultState.ViewState.FolderItem( + id = "mockId-1", + name = "mockName-1".asText(), + itemCount = 1, + ), + ), + collectionItems = listOf( + VaultState.ViewState.CollectionItem( + id = "mockId-1", + name = "mockName-1", + itemCount = 1, + ), + ), + noFolderItems = listOf(), + trashItemsCount = 0, + ), + dialog = VaultState.DialogState.Error( + title = R.string.internet_connection_required_title.asText(), + message = R.string.internet_connection_required_message.asText(), + ), + ), + viewModel.stateFlow.value, + ) + } + + @Suppress("MaxLineLength") + @Test + fun `vaultDataStateFlow NoNetwork with empty items should update state to NoItems and show an error dialog`() = + runTest { + mutableVaultDataStateFlow.tryEmit( + value = DataState.NoNetwork( + data = VaultData( + cipherViewList = emptyList(), + collectionViewList = emptyList(), + folderViewList = emptyList(), + ), + ), + ) + val viewModel = createViewModel() + assertEquals( + createMockVaultState( + viewState = VaultState.ViewState.NoItems, + dialog = VaultState.DialogState.Error( + title = R.string.internet_connection_required_title.asText(), + message = R.string.internet_connection_required_message.asText(), + ), + ), + viewModel.stateFlow.value, + ) + } + @Test fun `vaultDataStateFlow updates should do nothing when switching accounts`() { val viewModel = createViewModel() @@ -532,6 +607,46 @@ class VaultViewModelTest : BaseViewModelTest() { } } + @Test + fun `TryAgainClick should sync the vault data`() { + val viewModel = createViewModel() + + viewModel.trySendAction(VaultAction.TryAgainClick) + + verify { vaultRepository.sync() } + } + + @Test + fun `DialogDismiss should clear the active dialog`() { + // Show the No Network error dialog + val viewModel = createViewModel() + mutableVaultDataStateFlow.value = DataState.NoNetwork( + data = VaultData( + cipherViewList = emptyList(), + collectionViewList = emptyList(), + folderViewList = emptyList(), + ), + ) + val initialState = DEFAULT_STATE.copy( + viewState = VaultState.ViewState.NoItems, + dialog = VaultState.DialogState.Error( + title = R.string.internet_connection_required_title.asText(), + message = R.string.internet_connection_required_message.asText(), + ), + ) + assertEquals( + initialState, + viewModel.stateFlow.value, + ) + + viewModel.trySendAction(VaultAction.DialogDismiss) + + assertEquals( + initialState.copy(dialog = null), + viewModel.stateFlow.value, + ) + } + private fun createViewModel(): VaultViewModel = VaultViewModel( authRepository = authRepository, @@ -566,7 +681,10 @@ private val DEFAULT_USER_STATE = UserState( ), ) -private fun createMockVaultState(viewState: VaultState.ViewState): VaultState = +private fun createMockVaultState( + viewState: VaultState.ViewState, + dialog: VaultState.DialogState? = null, +): VaultState = VaultState( avatarColorString = "#aa00aa", initials = "AU", @@ -591,5 +709,6 @@ private fun createMockVaultState(viewState: VaultState.ViewState): VaultState = ), ), viewState = viewState, + dialog = dialog, isSwitchingAccounts = false, )