PM-39758: feat: Improve sync failure dialog (#7114)

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
     }
 }

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<VaultData>,
         validTotpIds: Set<String>,
     ) {
-        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<VaultState>.updateToErrorStateOrDialog(
-    baseIconUrl: String,
-    vaultData: VaultData?,
-    vaultFilterType: VaultFilterType,
-    isIconLoadingDisabled: Boolean,
-    isPremium: Boolean,
-    hasMasterPassword: Boolean,
-    errorTitle: Text,
-    errorMessage: Text,
-    isRefreshing: Boolean,
-    restrictItemTypesPolicyOrgIds: List<String>,
-    validTotpIds: Set<String>,
-    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(),
-            )
-        }
-    }
-}

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()

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,

ui/src/main/res/values/strings.xml

@@ -1153,6 +1153,8 @@ Do you want to switch to this account?</string>
     <string name="no_items_imported">No items imported</string>
     <string name="no_items_received_from_the_selected_app">No items received from the selected app.</string>
     <string name="vault_sync_failed">Vault sync failed</string>
+    <string name="vault_sync_unsuccessful">Sync unsuccessful</string>
+    <string name="vault_sync_failed_description">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.</string>
     <string name="your_items_have_been_successfully_imported_but_could_not_sync_vault">Your items have been successfully imported, but they will not be visible in your vault until sync is performed.</string>
     <string name="there_was_a_problem_importing_your_items">There was a problem importing your items. Please try again. If the problem persists, contact support.</string>
     <string name="saving_items">Saving items…</string>