From 04735a379699deb99424cae959906d3082e71a60 Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 30 Jun 2026 10:00:25 -0500 Subject: [PATCH] PM-39758: feat: Improve sync failure dialog (#7114) --- .../ui/vault/feature/vault/VaultScreen.kt | 15 ++- .../ui/vault/feature/vault/VaultViewModel.kt | 115 ++++++++---------- .../ui/vault/feature/vault/VaultScreenTest.kt | 65 ++++++++++ .../vault/feature/vault/VaultViewModelTest.kt | 37 +++--- ui/src/main/res/values/strings.xml | 2 + 5 files changed, 149 insertions(+), 85 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt index 618889bce1..da50c62d4b 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt @@ -76,6 +76,7 @@ import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import kotlin.time.Duration.Companion.milliseconds private const val APP_REVIEW_DELAY = 3000L @@ -121,7 +122,7 @@ fun VaultScreen( val launchPrompt = remember { { scope.launch { - delay(APP_REVIEW_DELAY) + delay(APP_REVIEW_DELAY.milliseconds) appReviewManager.promptForReview() } } @@ -493,6 +494,18 @@ private fun VaultDialogs( ) } + is VaultState.DialogState.SyncError -> { + BitwardenTwoButtonDialog( + title = dialogState.title(), + message = dialogState.message(), + confirmButtonText = stringResource(id = BitwardenString.try_again), + dismissButtonText = stringResource(id = BitwardenString.not_now), + onConfirmClick = vaultHandlers.tryAgainClick, + onDismissClick = vaultHandlers.dialogDismiss, + onDismissRequest = vaultHandlers.dialogDismiss, + ) + } + null -> Unit } } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt index 5b5132d95e..6ab599184d 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt @@ -84,7 +84,6 @@ import kotlinx.collections.immutable.persistentSetOf import kotlinx.collections.immutable.toImmutableList import kotlinx.collections.immutable.toImmutableSet import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge @@ -95,6 +94,7 @@ import kotlinx.parcelize.Parcelize import timber.log.Timber import java.time.Clock import javax.inject.Inject +import kotlin.time.Duration.Companion.milliseconds private const val VAULT_DATA_RECEIVED_DELAY: Long = 550L private const val LOGIN_SUCCESS_SNACKBAR_DELAY: Long = 550L @@ -219,7 +219,7 @@ class VaultViewModel @Inject constructor( .onEach { // When the vault data is received, the current activity is about to // be recreated. Adding this delay prevents the dialogs from disappearing. - delay(VAULT_DATA_RECEIVED_DELAY) + delay(VAULT_DATA_RECEIVED_DELAY.milliseconds) trySendAction(it) } .launchIn(viewModelScope) @@ -741,6 +741,9 @@ class VaultViewModel @Inject constructor( } private fun handleTryAgainClick() { + mutableStateFlow.update { + it.copy(dialog = VaultState.DialogState.Syncing) + } vaultRepository.sync(forced = true) } @@ -754,7 +757,7 @@ class VaultViewModel @Inject constructor( private fun handleRefreshPull() { mutableStateFlow.update { it.copy(isRefreshing = true) } viewModelScope.launch { - delay(250) + delay(250.milliseconds) if (networkConnectionManager.isNetworkConnected) { vaultRepository.sync(forced = false) } else { @@ -1453,21 +1456,40 @@ class VaultViewModel @Inject constructor( vaultData: DataState.Error, validTotpIds: Set, ) { - mutableStateFlow.updateToErrorStateOrDialog( - baseIconUrl = state.baseIconUrl, - vaultData = vaultData.data, - vaultFilterType = vaultFilterTypeOrDefault, - isIconLoadingDisabled = state.isIconLoadingDisabled, - isPremium = state.isPremium, - hasMasterPassword = state.hasMasterPassword, - errorTitle = BitwardenString.an_error_has_occurred.asText(), - errorMessage = vaultData.error.userFriendlyMessage?.asText() - ?: BitwardenString.generic_error_message.asText(), - isRefreshing = false, - restrictItemTypesPolicyOrgIds = state.restrictItemTypesPolicyOrgIds, - validTotpIds = validTotpIds, - isNewItemTypesEnabled = state.isNewItemTypesEnabled, - ) + val errorMessage = vaultData.error.userFriendlyMessage?.asText() + ?: BitwardenString.vault_sync_failed_description.asText() + mutableStateFlow.update { currentVaultState -> + vaultData + .data + ?.let { + currentVaultState.copy( + viewState = it.toViewState( + baseIconUrl = state.baseIconUrl, + isPremium = state.isPremium, + hasMasterPassword = state.hasMasterPassword, + vaultFilterType = vaultFilterTypeOrDefault, + isIconLoadingDisabled = state.isIconLoadingDisabled, + restrictItemTypesPolicyOrgIds = state.restrictItemTypesPolicyOrgIds, + validTotpIds = validTotpIds, + isNewItemTypesEnabled = state.isNewItemTypesEnabled, + ), + dialog = VaultState.DialogState.SyncError( + title = BitwardenString.vault_sync_unsuccessful.asText(), + message = errorMessage, + ), + isRefreshing = false, + validTotpIds = validTotpIds.toImmutableSet(), + ) + } + ?: currentVaultState.copy( + viewState = VaultState.ViewState.Error( + message = errorMessage, + ), + dialog = null, + isRefreshing = false, + validTotpIds = validTotpIds.toImmutableSet(), + ) + } } private fun vaultLoadedReceive( @@ -2267,6 +2289,15 @@ data class VaultState( val message: Text, val error: Throwable? = null, ) : DialogState() + + /** + * Represents an error dialog with the given [title] and [message]. + */ + @Parcelize + data class SyncError( + val title: Text, + val message: Text, + ) : DialogState() } } @@ -2777,51 +2808,3 @@ sealed class VaultAction { ) : Internal() } } - -@Suppress("LongParameterList") -private fun MutableStateFlow.updateToErrorStateOrDialog( - baseIconUrl: String, - vaultData: VaultData?, - vaultFilterType: VaultFilterType, - isIconLoadingDisabled: Boolean, - isPremium: Boolean, - hasMasterPassword: Boolean, - errorTitle: Text, - errorMessage: Text, - isRefreshing: Boolean, - restrictItemTypesPolicyOrgIds: List, - validTotpIds: Set, - isNewItemTypesEnabled: Boolean, -) { - this.update { - if (vaultData != null) { - it.copy( - viewState = vaultData.toViewState( - baseIconUrl = baseIconUrl, - isPremium = isPremium, - hasMasterPassword = hasMasterPassword, - vaultFilterType = vaultFilterType, - isIconLoadingDisabled = isIconLoadingDisabled, - restrictItemTypesPolicyOrgIds = restrictItemTypesPolicyOrgIds, - validTotpIds = validTotpIds, - isNewItemTypesEnabled = isNewItemTypesEnabled, - ), - dialog = VaultState.DialogState.Error( - title = errorTitle, - message = errorMessage, - ), - isRefreshing = isRefreshing, - validTotpIds = validTotpIds.toImmutableSet(), - ) - } else { - it.copy( - viewState = VaultState.ViewState.Error( - message = errorMessage, - ), - dialog = null, - isRefreshing = isRefreshing, - validTotpIds = validTotpIds.toImmutableSet(), - ) - } - } -} diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt index 94e97b7921..ddf1ddbc96 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt @@ -528,6 +528,71 @@ class VaultScreenTest : BitwardenComposeTest() { verify { viewModel.trySendAction(VaultAction.DialogDismiss) } } + @Test + fun `sync error dialog should be shown or hidden according to the state`() { + val errorTitle = "Error title" + val errorMessage = "Error message" + composeTestRule.assertNoDialogExists() + composeTestRule.onNodeWithText(text = errorTitle).assertDoesNotExist() + composeTestRule.onNodeWithText(text = errorMessage).assertDoesNotExist() + + mutableStateFlow.update { + it.copy( + dialog = VaultState.DialogState.SyncError( + title = errorTitle.asText(), + message = errorMessage.asText(), + ), + ) + } + + composeTestRule + .onAllNodesWithText(text = errorTitle) + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + composeTestRule + .onAllNodesWithText(text = errorMessage) + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + } + + @Test + fun `Try again button click in sync error dialog should send SyncClick`() { + mutableStateFlow.update { + it.copy( + dialog = VaultState.DialogState.SyncError( + title = "Error title".asText(), + message = "Error message".asText(), + ), + ) + } + + composeTestRule + .onAllNodesWithText(text = "Try again") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + verify(exactly = 1) { viewModel.trySendAction(VaultAction.TryAgainClick) } + } + + @Test + fun `Not now button click in sync error dialog should send DialogDismiss`() { + mutableStateFlow.update { + it.copy( + dialog = VaultState.DialogState.SyncError( + title = "Error title".asText(), + message = "Error message".asText(), + ), + ) + } + + composeTestRule + .onAllNodesWithText(text = "Not now") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + verify(exactly = 1) { viewModel.trySendAction(VaultAction.DialogDismiss) } + } + @Test fun `ThirdPartyBrowserAutofill should be displayed according to state`() { composeTestRule.assertNoDialogExists() diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt index 0ac61a2ede..924a288e35 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt @@ -108,6 +108,7 @@ import org.junit.jupiter.api.Test import java.time.Clock import java.time.Instant import java.time.ZoneOffset +import kotlin.time.Duration.Companion.milliseconds @Suppress("LargeClass") class VaultViewModelTest : BaseViewModelTest() { @@ -910,7 +911,7 @@ class VaultViewModelTest : BaseViewModelTest() { data = vaultData, ), ) - advanceTimeBy(1500) + advanceTimeBy(1500.milliseconds) viewModel.trySendAction( action = VaultAction.ShareAllCipherDecryptionErrorsClick, ) @@ -1615,7 +1616,7 @@ class VaultViewModelTest : BaseViewModelTest() { ), ) // Allow time for state to update - advanceTimeBy(1500) + advanceTimeBy(1500.milliseconds) assertEquals(expectedState, viewModel.stateFlow.value) assertEquals( VaultEvent.ShowSnackbar(BitwardenString.syncing_complete.asText()), @@ -1670,7 +1671,7 @@ class VaultViewModelTest : BaseViewModelTest() { ), ) // Allow time for state to update - advanceTimeBy(1500) + advanceTimeBy(1500.milliseconds) assertEquals(expectedState, viewModel.stateFlow.value) assertEquals( VaultEvent.ShowSnackbar(BitwardenString.syncing_complete.asText()), @@ -1790,7 +1791,7 @@ class VaultViewModelTest : BaseViewModelTest() { assertEquals( createMockVaultState( viewState = VaultState.ViewState.Error( - message = BitwardenString.generic_error_message.asText(), + message = BitwardenString.vault_sync_failed_description.asText(), ), ), viewModel.stateFlow.value, @@ -1885,9 +1886,9 @@ class VaultViewModelTest : BaseViewModelTest() { showLicenseGroup = false, showPassportGroup = false, ), - dialog = VaultState.DialogState.Error( - title = BitwardenString.an_error_has_occurred.asText(), - message = BitwardenString.generic_error_message.asText(), + dialog = VaultState.DialogState.SyncError( + title = BitwardenString.vault_sync_unsuccessful.asText(), + message = BitwardenString.vault_sync_failed_description.asText(), ), ), viewModel.stateFlow.value, @@ -1960,8 +1961,8 @@ class VaultViewModelTest : BaseViewModelTest() { showLicenseGroup = false, showPassportGroup = false, ), - dialog = VaultState.DialogState.Error( - title = BitwardenString.an_error_has_occurred.asText(), + dialog = VaultState.DialogState.SyncError( + title = BitwardenString.vault_sync_unsuccessful.asText(), message = ( "Your request was interrupted because the app needed to " + "re-authenticate. Please try again." @@ -1994,9 +1995,9 @@ class VaultViewModelTest : BaseViewModelTest() { assertEquals( createMockVaultState( viewState = VaultState.ViewState.NoItems, - dialog = VaultState.DialogState.Error( - title = BitwardenString.an_error_has_occurred.asText(), - message = BitwardenString.generic_error_message.asText(), + dialog = VaultState.DialogState.SyncError( + title = BitwardenString.vault_sync_unsuccessful.asText(), + message = BitwardenString.vault_sync_failed_description.asText(), ), ), viewModel.stateFlow.value, @@ -2713,7 +2714,7 @@ class VaultViewModelTest : BaseViewModelTest() { mutableVaultDataStateFlow.tryEmit(value = dataState) // Advance time to allow state updates - advanceTimeBy(1500) + advanceTimeBy(1500.milliseconds) assertEquals( createMockVaultState( @@ -2780,9 +2781,9 @@ class VaultViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() val initialState = DEFAULT_STATE.copy( viewState = VaultState.ViewState.NoItems, - dialog = VaultState.DialogState.Error( - title = BitwardenString.an_error_has_occurred.asText(), - message = BitwardenString.generic_error_message.asText(), + dialog = VaultState.DialogState.SyncError( + title = BitwardenString.vault_sync_unsuccessful.asText(), + message = BitwardenString.vault_sync_failed_description.asText(), ), ) assertEquals( @@ -2803,7 +2804,7 @@ class VaultViewModelTest : BaseViewModelTest() { fun `RefreshPull should call vault repository sync`() = runTest { val viewModel = createViewModel() viewModel.trySendAction(VaultAction.RefreshPull) - advanceTimeBy(300) + advanceTimeBy(300.milliseconds) verify(exactly = 1) { vaultRepository.sync(forced = false) } @@ -2818,7 +2819,7 @@ class VaultViewModelTest : BaseViewModelTest() { } returns false viewModel.trySendAction(VaultAction.RefreshPull) - advanceTimeBy(300) + advanceTimeBy(300.milliseconds) assertEquals( DEFAULT_STATE.copy( isRefreshing = false, diff --git a/ui/src/main/res/values/strings.xml b/ui/src/main/res/values/strings.xml index 028f754732..7a8f7baca6 100644 --- a/ui/src/main/res/values/strings.xml +++ b/ui/src/main/res/values/strings.xml @@ -1153,6 +1153,8 @@ Do you want to switch to this account? No items imported No items received from the selected app. Vault sync failed + Sync unsuccessful + We couldn’t sync your vault with the server. Your vault may not reflect your latest changes. You can try again now or it will sync automatically later. Your items have been successfully imported, but they will not be visible in your vault until sync is performed. There was a problem importing your items. Please try again. If the problem persists, contact support. Saving items…