From 4fc01c77d112cb6b6782ed63d5d8ebd0d09c54ab Mon Sep 17 00:00:00 2001 From: Dave Severns <149429124+dseverns-livefront@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:37:06 -0400 Subject: [PATCH] PM-11186 Sync in progress for import logins and full screen loading. (#4117) --- .../sdk/model/Fido2CredentialStoreImpl.kt | 4 +- .../data/vault/repository/VaultRepository.kt | 6 +- .../vault/repository/VaultRepositoryImpl.kt | 2 +- .../BitwardenFullScreenLoadingContent.kt | 43 +++++ .../content/BitwardenLoadingContent.kt | 14 ++ .../components/content/ObscuredContent.kt | 156 ++++++++++++++++++ .../vaultunlocked/VaultUnlockedNavigation.kt | 4 + .../importlogins/ImportLoginsNavigation.kt | 2 + .../importlogins/ImportLoginsScreen.kt | 130 +++++++++------ .../importlogins/ImportLoginsViewModel.kt | 106 ++++++++++-- .../handlers/ImportLoginHandler.kt | 6 + app/src/main/res/values/strings.xml | 1 + .../vault/repository/VaultRepositoryTest.kt | 12 +- .../importlogins/ImportLoginsScreenTest.kt | 57 ++++++- .../importlogins/ImportLoginsViewModelTest.kt | 116 ++++++++++++- 15 files changed, 580 insertions(+), 79 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenFullScreenLoadingContent.kt create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/ObscuredContent.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt index a119f58bb..18dc10648 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt @@ -25,7 +25,7 @@ class Fido2CredentialStoreImpl( * Return all active ciphers that contain FIDO 2 credentials. */ override suspend fun allCredentials(): List { - val syncResult = vaultRepository.syncFido2Credentials() + val syncResult = vaultRepository.syncForResult() if (syncResult is SyncVaultDataResult.Error) { syncResult.throwable ?.let { throw it } @@ -46,7 +46,7 @@ class Fido2CredentialStoreImpl( override suspend fun findCredentials(ids: List?, ripId: String): List { val userId = getActiveUserIdOrThrow() - val syncResult = vaultRepository.syncFido2Credentials() + val syncResult = vaultRepository.syncForResult() if (syncResult is SyncVaultDataResult.Error) { syncResult.throwable ?.let { throw it } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt index 16f02bd2c..1e40afe8c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt @@ -118,10 +118,10 @@ interface VaultRepository : CipherManager, VaultLockManager { fun syncIfNecessary() /** - * Syncs the user's FIDO 2 credentials. This is an explicit request to sync and is not dependent - * on whether the last sync time was sufficiently in the past. + * Syncs the vault data for the current user. This is an explicit request to sync and will + * return the result of the sync as a [SyncVaultDataResult]. */ - suspend fun syncFido2Credentials(): SyncVaultDataResult + suspend fun syncForResult(): SyncVaultDataResult /** * Flow that represents the data for a specific vault item as found by ID. This may emit `null` diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index c4ebeccbb..00ea536b8 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -340,7 +340,7 @@ class VaultRepositoryImpl( } } - override suspend fun syncFido2Credentials(): SyncVaultDataResult { + override suspend fun syncForResult(): SyncVaultDataResult { val userId = activeUserId ?: return SyncVaultDataResult.Error(throwable = null) syncJob = ioScope diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenFullScreenLoadingContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenFullScreenLoadingContent.kt new file mode 100644 index 000000000..7103c330b --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenFullScreenLoadingContent.kt @@ -0,0 +1,43 @@ +package com.x8bit.bitwarden.ui.platform.components.content + +import androidx.activity.compose.BackHandler +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier + +/** + * A full screen loading effect which covers the entire screen and overlays either + * a blur of the content or a translucent scrim. The content is not interactable and + * the system back function is disabled while the loading content is displayed. + * + * @param showLoadingState whether or not to show the loading content overlay. + * @param modifier the [Modifier] to be applied to the loading content. + * @param message optional message to display above the loading spinner + * @param content to be encased by the full screen loading content. If you want part of + * your [Composable] covered by the effect it needs to be in this content block. + */ +@Composable +fun BitwardenFullScreenLoadingContent( + showLoadingState: Boolean, + modifier: Modifier = Modifier, + message: String? = null, + content: @Composable () -> Unit, +) { + BackHandler(enabled = showLoadingState) { + // No-op + } + ObscuredContent( + enabled = showLoadingState, + modifier = Modifier + .fillMaxSize() + .then(modifier), + content = content, + overlayContent = { + BitwardenLoadingContent( + message = message, + modifier = Modifier.align(Alignment.Center), + ) + }, + ) +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenLoadingContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenLoadingContent.kt index efff17013..05dd6348a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenLoadingContent.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenLoadingContent.kt @@ -3,11 +3,15 @@ package com.x8bit.bitwarden.ui.platform.components.content import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.navigationBarsPadding +import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp import com.x8bit.bitwarden.ui.platform.components.indicator.BitwardenCircularProgressIndicator +import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme /** * A Bitwarden-themed, re-usable loading state. @@ -15,12 +19,22 @@ import com.x8bit.bitwarden.ui.platform.components.indicator.BitwardenCircularPro @Composable fun BitwardenLoadingContent( modifier: Modifier = Modifier, + message: String? = null, ) { Column( modifier = modifier, verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally, ) { + message?.let { + Text( + text = it, + style = BitwardenTheme.typography.titleMedium, + // setting color explicitly here as we can't assume what the surface will be. + color = BitwardenTheme.colorScheme.text.primary, + ) + Spacer(Modifier.height(16.dp)) + } BitwardenCircularProgressIndicator() Spacer(modifier = Modifier.navigationBarsPadding()) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/ObscuredContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/ObscuredContent.kt new file mode 100644 index 000000000..b19be2948 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/ObscuredContent.kt @@ -0,0 +1,156 @@ +package com.x8bit.bitwarden.ui.platform.components.content + +import android.os.Build +import androidx.activity.compose.BackHandler +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxScope +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.BlurredEdgeTreatment +import androidx.compose.ui.draw.blur +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import com.x8bit.bitwarden.data.platform.util.isBuildVersionBelow +import com.x8bit.bitwarden.ui.platform.components.button.BitwardenFilledButton +import com.x8bit.bitwarden.ui.platform.components.indicator.BitwardenCircularProgressIndicator +import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme + +/** + * A way to obscure content with either a blurred effect or a semi-transparent background. + * Dependent on the device's Android version. Blur is supported on Android 12 and above. + * Only the exact components passed in via the [content] will have the effect applied. + * + * @param enabled Whether the content should be obscured. + * @param modifier The modifier to be applied to the outer container. By nature any padding would be + * applied to passed in content as well. + * @param overlayContent Optional content to overlay on top of the obscured content. + * (e.g. a loading indicator) + * @param content The content to obscure. + */ +@Composable +fun ObscuredContent( + enabled: Boolean, + modifier: Modifier = Modifier, + overlayContent: (@Composable BoxScope.() -> Unit)? = null, + content: @Composable () -> Unit, +) { + val isBlurSupported = !isBuildVersionBelow(Build.VERSION_CODES.S) + val shouldApplyBlur = enabled && isBlurSupported + // if blur is not available we use a semi-transparent scrim instead. + val shouldApplyLegacyObscuring = enabled && !isBlurSupported + Box( + modifier = modifier, + ) { + val customModifier = if (shouldApplyBlur) { + Modifier + .matchParentSize() + .blur(45.dp, edgeTreatment = BlurredEdgeTreatment.Unbounded) + } else { + Modifier.matchParentSize() + } + Box(modifier = customModifier) { + content() + if (enabled) { + // Disables interaction with the obscured content + Box( + modifier = Modifier + .matchParentSize() + .background( + if (shouldApplyLegacyObscuring) { + BitwardenTheme.colorScheme.background.primary.copy(alpha = 0.95f) + } else { + Color.Transparent + }, + ), + ) + } + } + if (overlayContent != null && enabled) { + overlayContent() + } + } +} + +@Preview +@Composable +private fun ObscuredContent_preview() { + BitwardenTheme { + ObscuredContent( + enabled = true, + modifier = Modifier.fillMaxSize(), + content = { + Column( + modifier = Modifier + .fillMaxSize() + .background(Color.White), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + Spacer(Modifier.size(100.dp)) + BitwardenFilledButton( + label = "Obscure Content", + onClick = {}, + modifier = Modifier.fillMaxWidth() + .padding(horizontal = 16.dp), + ) + } + }, + overlayContent = { + BitwardenCircularProgressIndicator( + modifier = Modifier.align(Alignment.Center), + ) + }, + ) + } +} + +@Preview +@Composable +private fun InteractiveObscuredContent_preview() { + var obscureContent by remember { mutableStateOf(false) } + BackHandler(enabled = obscureContent) { + obscureContent = false + } + BitwardenTheme { + ObscuredContent( + enabled = obscureContent, + modifier = Modifier.fillMaxSize(), + content = { + Column( + modifier = Modifier + .fillMaxSize() + .background(Color.White), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + Spacer(Modifier.size(100.dp)) + BitwardenFilledButton( + label = "Obscure Content", + onClick = {}, + modifier = Modifier.fillMaxWidth() + .padding(horizontal = 16.dp), + ) + } + }, + overlayContent = { + BitwardenCircularProgressIndicator( + modifier = Modifier.align(Alignment.Center), + ) + }, + ) + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlocked/VaultUnlockedNavigation.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlocked/VaultUnlockedNavigation.kt index 04b58e5dd..78100da15 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlocked/VaultUnlockedNavigation.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlocked/VaultUnlockedNavigation.kt @@ -221,6 +221,10 @@ fun NavGraphBuilder.vaultUnlockedGraph( ) importLoginsScreenDestination( onNavigateBack = { navController.popBackStack() }, + onNavigateToImportSuccessScreen = { + // TODO: PM-11187 navigate to success screen with popping this screen from stack + navController.popBackStack() + }, ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsNavigation.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsNavigation.kt index ba6ed19a5..c91e785b2 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsNavigation.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsNavigation.kt @@ -19,12 +19,14 @@ fun NavController.navigateToImportLoginsScreen(navOptions: NavOptions? = null) { */ fun NavGraphBuilder.importLoginsScreenDestination( onNavigateBack: () -> Unit, + onNavigateToImportSuccessScreen: () -> Unit, ) { composableWithSlideTransitions( route = IMPORT_LOGINS_ROUTE, ) { ImportLoginsScreen( onNavigateBack = onNavigateBack, + onNavigateToImportSuccessScreen = onNavigateToImportSuccessScreen, ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreen.kt index fad3d71e1..23b24e053 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreen.kt @@ -45,6 +45,7 @@ import com.x8bit.bitwarden.ui.platform.components.appbar.BitwardenTopAppBar import com.x8bit.bitwarden.ui.platform.components.appbar.NavigationIcon import com.x8bit.bitwarden.ui.platform.components.button.BitwardenFilledButton import com.x8bit.bitwarden.ui.platform.components.button.BitwardenOutlinedButton +import com.x8bit.bitwarden.ui.platform.components.content.BitwardenFullScreenLoadingContent import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold import com.x8bit.bitwarden.ui.platform.components.util.rememberVectorPainter @@ -67,6 +68,7 @@ private const val IMPORT_HELP_URL = "https://bitwarden.com/help/import-data/" @Composable fun ImportLoginsScreen( onNavigateBack: () -> Unit, + onNavigateToImportSuccessScreen: () -> Unit, viewModel: ImportLoginsViewModel = hiltViewModel(), intentManager: IntentManager = LocalIntentManager.current, ) { @@ -79,6 +81,8 @@ fun ImportLoginsScreen( ImportLoginsEvent.OpenHelpLink -> { intentManager.startCustomTabsActivity(IMPORT_HELP_URL.toUri()) } + + ImportLoginsEvent.NavigateToImportSuccess -> onNavigateToImportSuccessScreen() } } @@ -91,63 +95,65 @@ fun ImportLoginsScreen( } val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState()) - BitwardenScaffold( - modifier = Modifier - .fillMaxSize() - .nestedScroll(scrollBehavior.nestedScrollConnection), - topBar = { - BitwardenTopAppBar( - title = stringResource(R.string.import_logins), - navigationIcon = NavigationIcon( - navigationIcon = rememberVectorPainter(R.drawable.ic_close), - onNavigationIconClick = handler.onCloseClick, - navigationIconContentDescription = stringResource(R.string.close), - ), - scrollBehavior = scrollBehavior, - ) - }, - ) { innerPadding -> - Crossfade( - targetState = state.viewState, - label = "CrossfadeBetweenViewStates", + BitwardenFullScreenLoadingContent( + modifier = Modifier.fillMaxSize(), + showLoadingState = state.isVaultSyncing, + message = stringResource(R.string.syncing_logins_loading_message), + ) { + BitwardenScaffold( modifier = Modifier .fillMaxSize() - .padding(paddingValues = innerPadding), - ) { viewState -> - when (viewState) { - ImportLoginsState.ViewState.InitialContent -> { - InitialImportLoginsContent( - onGetStartedClick = handler.onGetStartedClick, - onImportLaterClick = handler.onImportLaterClick, - ) - } + .nestedScroll(scrollBehavior.nestedScrollConnection), + topBar = { + BitwardenTopAppBar( + title = stringResource(R.string.import_logins), + navigationIcon = NavigationIcon( + navigationIcon = rememberVectorPainter(R.drawable.ic_close), + onNavigationIconClick = handler.onCloseClick, + navigationIconContentDescription = stringResource(R.string.close), + ), + scrollBehavior = scrollBehavior, + ) + }, + ) { innerPadding -> + Crossfade( + targetState = state.viewState, + label = "CrossfadeBetweenViewStates", + modifier = Modifier + .fillMaxSize() + .padding(paddingValues = innerPadding), + ) { viewState -> + when (viewState) { + ImportLoginsState.ViewState.InitialContent -> { + InitialImportLoginsContent( + onGetStartedClick = handler.onGetStartedClick, + onImportLaterClick = handler.onImportLaterClick, + ) + } - ImportLoginsState.ViewState.ImportStepOne -> { - ImportLoginsStepOneContent( - onBackClick = handler.onMoveToInitialContent, - onContinueClick = handler.onMoveToStepTwo, - onHelpClick = handler.onHelpClick, - ) - } + ImportLoginsState.ViewState.ImportStepOne -> { + ImportLoginsStepOneContent( + onBackClick = handler.onMoveToInitialContent, + onContinueClick = handler.onMoveToStepTwo, + onHelpClick = handler.onHelpClick, + ) + } - ImportLoginsState.ViewState.ImportStepTwo -> { - ImportLoginsStepTwoContent( - onBackClick = handler.onMoveToStepOne, - onContinueClick = handler.onMoveToStepThree, - onHelpClick = handler.onHelpClick, - ) - } + ImportLoginsState.ViewState.ImportStepTwo -> { + ImportLoginsStepTwoContent( + onBackClick = handler.onMoveToStepOne, + onContinueClick = handler.onMoveToStepThree, + onHelpClick = handler.onHelpClick, + ) + } - ImportLoginsState.ViewState.ImportStepThree -> { - ImportLoginsStepThreeContent( - onBackClick = handler.onMoveToStepTwo, - onContinueClick = handler.onMoveToSyncInProgress, - onHelpClick = handler.onHelpClick, - ) - } - - ImportLoginsState.ViewState.SyncInProgress -> { - // TODO PM-11186: Implement sync in progress + ImportLoginsState.ViewState.ImportStepThree -> { + ImportLoginsStepThreeContent( + onBackClick = handler.onMoveToStepTwo, + onContinueClick = handler.onMoveToSyncInProgress, + onHelpClick = handler.onHelpClick, + ) + } } } } @@ -164,7 +170,7 @@ private fun ImportLoginsDialogContent( when (val dialogState = state.dialogState) { ImportLoginsState.DialogState.GetStarted -> { BitwardenTwoButtonDialog( - title = dialogState.title(), + title = dialogState.title?.invoke(), message = dialogState.message(), onDismissRequest = handler.onDismissDialog, confirmButtonText = confirmButtonText, @@ -176,7 +182,7 @@ private fun ImportLoginsDialogContent( ImportLoginsState.DialogState.ImportLater -> { BitwardenTwoButtonDialog( - title = dialogState.title(), + title = dialogState.title?.invoke(), message = dialogState.message(), onDismissRequest = handler.onDismissDialog, confirmButtonText = confirmButtonText, @@ -186,6 +192,18 @@ private fun ImportLoginsDialogContent( ) } + ImportLoginsState.DialogState.Error -> { + BitwardenTwoButtonDialog( + title = dialogState.title?.invoke(), + message = dialogState.message(), + onDismissRequest = handler.onDismissDialog, + confirmButtonText = stringResource(R.string.try_again), + dismissButtonText = stringResource(R.string.ok), + onConfirmClick = handler.onRetrySync, + onDismissClick = handler.onFailedSyncAcknowledged, + ) + } + null -> Unit } } @@ -468,6 +486,8 @@ private fun ImportLoginsScreenDialog_preview( onMoveToStepTwo = {}, onMoveToStepThree = {}, onMoveToSyncInProgress = {}, + onRetrySync = {}, + onFailedSyncAcknowledged = {}, ), ) InitialImportLoginsContent( @@ -486,10 +506,12 @@ private class ImportLoginsDialogContentPreviewProvider : ImportLoginsState( dialogState = ImportLoginsState.DialogState.GetStarted, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), ImportLoginsState( dialogState = ImportLoginsState.DialogState.ImportLater, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt index 0d184ad36..8a54c0222 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt @@ -1,11 +1,15 @@ package com.x8bit.bitwarden.ui.vault.feature.importlogins +import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.data.vault.repository.VaultRepository +import com.x8bit.bitwarden.data.vault.repository.model.SyncVaultDataResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModel import com.x8bit.bitwarden.ui.platform.base.util.Text import com.x8bit.bitwarden.ui.platform.base.util.asText import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import javax.inject.Inject /** @@ -13,11 +17,14 @@ import javax.inject.Inject */ @Suppress("TooManyFunctions") @HiltViewModel -class ImportLoginsViewModel @Inject constructor() : +class ImportLoginsViewModel @Inject constructor( + private val vaultRepository: VaultRepository, +) : BaseViewModel( initialState = ImportLoginsState( null, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), ) { override fun handleAction(action: ImportLoginsAction) { @@ -34,11 +41,52 @@ class ImportLoginsViewModel @Inject constructor() : ImportLoginsAction.MoveToStepThree -> handleMoveToStepThree() ImportLoginsAction.MoveToSyncInProgress -> handleMoveToSyncInProgress() ImportLoginsAction.HelpClick -> handleHelpClick() + is ImportLoginsAction.Internal.VaultSyncResultReceived -> { + handleVaultSyncResultReceived(action) + } + + ImportLoginsAction.RetryVaultSync -> handleRetryVaultSync() + ImportLoginsAction.FailSyncAcknowledged -> handleFailedSyncAcknowledged() + } + } + + private fun handleFailedSyncAcknowledged() { + mutableStateFlow.update { + it.copy(dialogState = null) + } + sendEvent(ImportLoginsEvent.NavigateBack) + } + + private fun handleRetryVaultSync() { + mutableStateFlow.update { + it.copy( + isVaultSyncing = true, + dialogState = null, + ) + } + syncVault() + } + + private fun handleVaultSyncResultReceived( + action: ImportLoginsAction.Internal.VaultSyncResultReceived, + ) { + when (action.result) { + is SyncVaultDataResult.Error -> { + mutableStateFlow.update { + it.copy( + isVaultSyncing = false, + dialogState = ImportLoginsState.DialogState.Error, + ) + } + } + + SyncVaultDataResult.Success -> sendEvent(ImportLoginsEvent.NavigateToImportSuccess) } } private fun handleMoveToSyncInProgress() { - // TODO PM-11186: Implement sync in progress + mutableStateFlow.update { it.copy(isVaultSyncing = true) } + syncVault() } private fun handleHelpClick() { @@ -102,6 +150,13 @@ class ImportLoginsViewModel @Inject constructor() : it.copy(dialogState = dialogState) } } + + private fun syncVault() { + viewModelScope.launch { + val result = vaultRepository.syncForResult() + sendAction(ImportLoginsAction.Internal.VaultSyncResultReceived(result)) + } + } } /** @@ -110,13 +165,14 @@ class ImportLoginsViewModel @Inject constructor() : data class ImportLoginsState( val dialogState: DialogState?, val viewState: ViewState, + val isVaultSyncing: Boolean, ) { /** * Dialog states for the [ImportLoginsViewModel]. */ sealed class DialogState { abstract val message: Text - abstract val title: Text + abstract val title: Text? /** * Import logins later dialog state. @@ -135,6 +191,14 @@ data class ImportLoginsState( R.string.the_following_instructions_will_guide_you_through_importing_logins.asText() override val title: Text = R.string.do_you_have_a_computer_available.asText() } + + /** + * Show a dialog with an error message. + */ + data object Error : DialogState() { + override val title: Text? = null + override val message: Text = R.string.generic_error_message.asText() + } } /** @@ -173,13 +237,6 @@ data class ImportLoginsState( data object ImportStepThree : ViewState() { override val backAction: ImportLoginsAction = ImportLoginsAction.MoveToStepTwo } - - /** - * Sync in progress view state. - */ - data object SyncInProgress : ViewState() { - override val backAction: ImportLoginsAction? = null - } } } @@ -192,6 +249,11 @@ sealed class ImportLoginsEvent { */ data object NavigateBack : ImportLoginsEvent() + /** + * Navigate to the import success screen + */ + data object NavigateToImportSuccess : ImportLoginsEvent() + /** * Open the help link in a browser. */ @@ -259,7 +321,29 @@ sealed class ImportLoginsAction { data object MoveToStepThree : ImportLoginsAction() /** - * User has performed action which should move to the sync in progress view state. + * User has performed action which should begin the sync in progress and update the + * state accordingly. */ data object MoveToSyncInProgress : ImportLoginsAction() + + /** + * User has chosen to retry vault sync after failure. + */ + data object RetryVaultSync : ImportLoginsAction() + + /** + * User has acknowledge failed sync and chose not to retry now. + */ + data object FailSyncAcknowledged : ImportLoginsAction() + + /** + * Internal actions to be handled, not triggered by user actions. + */ + sealed class Internal : ImportLoginsAction() { + + /** + * Vault sync result received. Process in a synchronous manner. + */ + data class VaultSyncResultReceived(val result: SyncVaultDataResult) : Internal() + } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/handlers/ImportLoginHandler.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/handlers/ImportLoginHandler.kt index 50a96abec..24d59b0d4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/handlers/ImportLoginHandler.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/handlers/ImportLoginHandler.kt @@ -21,6 +21,8 @@ data class ImportLoginHandler( val onMoveToStepTwo: () -> Unit, val onMoveToStepThree: () -> Unit, val onMoveToSyncInProgress: () -> Unit, + val onRetrySync: () -> Unit, + val onFailedSyncAcknowledged: () -> Unit, ) { @Suppress("UndocumentedPublicClass") companion object { @@ -46,6 +48,10 @@ data class ImportLoginHandler( onMoveToSyncInProgress = { viewModel.trySendAction(ImportLoginsAction.MoveToSyncInProgress) }, + onRetrySync = { viewModel.trySendAction(ImportLoginsAction.RetryVaultSync) }, + onFailedSyncAcknowledged = { + viewModel.trySendAction(ImportLoginsAction.FailSyncAcknowledged) + }, ) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 0766e24d1..1afbb68af 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1059,4 +1059,5 @@ Do you want to switch to this account? Save the exported file somewhere on your computer you can find easily. Save the exported file This is not a recognized Bitwarden server. You may need to check with your provider or update your server. + Syncing logins... diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt index fce5988df..7f4b8a789 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt @@ -4335,7 +4335,7 @@ class VaultRepositoryTest { } @Test - fun `syncFido2Credentials should return result`() = runTest { + fun `syncForResult should return result`() = runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE val userId = "mockId-1" val mockSyncResponse = createMockSyncResponse(number = 1) @@ -4364,19 +4364,19 @@ class VaultRepositoryTest { ) } just runs - val syncResult = vaultRepository.syncFido2Credentials() + val syncResult = vaultRepository.syncForResult() assertEquals(SyncVaultDataResult.Success, syncResult) } @Test - fun `syncFido2Credentials should return error when getAccountRevisionDateMillis fails`() = + fun `syncForResult should return error when getAccountRevisionDateMillis fails`() = runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE val throwable = Throwable() coEvery { syncService.getAccountRevisionDateMillis() } returns throwable.asFailure() - val syncResult = vaultRepository.syncFido2Credentials() + val syncResult = vaultRepository.syncForResult() assertEquals( SyncVaultDataResult.Error(throwable = throwable), syncResult, @@ -4384,13 +4384,13 @@ class VaultRepositoryTest { } @Test - fun `syncFido2Credentials should return error when sync fails`() = runTest { + fun `syncForResult should return error when sync fails`() = runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE val throwable = Throwable() coEvery { syncService.sync() } returns throwable.asFailure() - val syncResult = vaultRepository.syncFido2Credentials() + val syncResult = vaultRepository.syncForResult() assertEquals( SyncVaultDataResult.Error(throwable = throwable), syncResult, diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreenTest.kt index ba3b39554..7491810c6 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreenTest.kt @@ -7,8 +7,10 @@ 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.onRoot import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollTo +import androidx.compose.ui.test.printToLog import androidx.core.net.toUri import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest @@ -20,12 +22,13 @@ import io.mockk.mockk import io.mockk.runs import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test class ImportLoginsScreenTest : BaseComposeTest() { - + private var navigateToImportLoginSuccessCalled = false private var navigateBackCalled = false private val mutableImportLoginsStateFlow = MutableStateFlow(DEFAULT_STATE) @@ -43,6 +46,7 @@ class ImportLoginsScreenTest : BaseComposeTest() { setContentWithBackDispatcher { ImportLoginsScreen( onNavigateBack = { navigateBackCalled = true }, + onNavigateToImportSuccessScreen = { navigateToImportLoginSuccessCalled = true }, viewModel = viewModel, intentManager = intentManager, ) @@ -318,6 +322,56 @@ class ImportLoginsScreenTest : BaseComposeTest() { verifyActionSent(ImportLoginsAction.MoveToStepTwo) } + @Test + fun `NavigateToImportSuccess event causes correct lambda to invoke`() { + mutableImportLoginsEventFlow.tryEmit(ImportLoginsEvent.NavigateToImportSuccess) + assertTrue(navigateToImportLoginSuccessCalled) + } + + @Test + fun `Loading content is displayed when isVaultSyncing is true`() { + mutableImportLoginsStateFlow.update { + it.copy(isVaultSyncing = true) + } + composeTestRule.onRoot().printToLog("woo") + composeTestRule + .onNodeWithText(text = "Syncing logins...") + .assertIsDisplayed() + } + + @Test + fun `Error dialog is displayed when dialog state is Error`() { + mutableImportLoginsStateFlow.tryEmit( + DEFAULT_STATE.copy( + dialogState = ImportLoginsState.DialogState.Error, + ), + ) + composeTestRule + .onNode(isDialog()) + .assertIsDisplayed() + composeTestRule + .onAllNodesWithText( + text = "We were unable to process your request. Please try again or contact us.", + useUnmergedTree = true, + ) + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + + composeTestRule + .onAllNodesWithText("Try again") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + .performClick() + verifyActionSent(ImportLoginsAction.RetryVaultSync) + + composeTestRule + .onAllNodesWithText("Ok") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + .performClick() + verifyActionSent(ImportLoginsAction.FailSyncAcknowledged) + } + //region Helper methods private fun verifyActionSent(action: ImportLoginsAction) { @@ -330,4 +384,5 @@ class ImportLoginsScreenTest : BaseComposeTest() { private val DEFAULT_STATE = ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModelTest.kt index 740584cfe..6d7743851 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModelTest.kt @@ -2,14 +2,24 @@ package com.x8bit.bitwarden.ui.vault.feature.importlogins import app.cash.turbine.test import app.cash.turbine.turbineScope +import com.x8bit.bitwarden.data.vault.repository.VaultRepository +import com.x8bit.bitwarden.data.vault.repository.model.SyncVaultDataResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test class ImportLoginsViewModelTest : BaseViewModelTest() { + private val vaultRepository: VaultRepository = mockk() { + coEvery { syncForResult() } returns SyncVaultDataResult.Success + } + @Test fun `initial state is correct`() { val viewModel = createViewModel() @@ -27,6 +37,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = ImportLoginsState.DialogState.GetStarted, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), viewModel.stateFlow.value, ) @@ -40,6 +51,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = ImportLoginsState.DialogState.ImportLater, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), viewModel.stateFlow.value, ) @@ -58,6 +70,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = ImportLoginsState.DialogState.GetStarted, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), awaitItem(), ) @@ -66,6 +79,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), awaitItem(), ) @@ -87,6 +101,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = ImportLoginsState.DialogState.ImportLater, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), stateFlow.awaitItem(), ) @@ -95,6 +110,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), stateFlow.awaitItem(), ) @@ -118,6 +134,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = ImportLoginsState.DialogState.GetStarted, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), awaitItem(), ) @@ -126,6 +143,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.ImportStepOne, + isVaultSyncing = false, ), awaitItem(), ) @@ -164,6 +182,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.ImportStepOne, + isVaultSyncing = false, ), viewModel.stateFlow.value, ) @@ -177,6 +196,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.ImportStepTwo, + isVaultSyncing = false, ), viewModel.stateFlow.value, ) @@ -190,6 +210,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.ImportStepThree, + isVaultSyncing = false, ), viewModel.stateFlow.value, ) @@ -207,15 +228,108 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, ), viewModel.stateFlow.value, ) } - private fun createViewModel(): ImportLoginsViewModel = ImportLoginsViewModel() + @Test + fun `MoveToSyncInProgress sets isVaultSyncing to true and calls syncForResult`() { + val viewModel = createViewModel() + viewModel.trySendAction(ImportLoginsAction.MoveToSyncInProgress) + assertEquals( + ImportLoginsState( + dialogState = null, + viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = true, + ), + viewModel.stateFlow.value, + ) + coVerify { vaultRepository.syncForResult() } + } + + @Suppress("MaxLineLength") + @Test + fun `RetryVaultSync sets isVaultSyncing to true and clears dialog state and calls syncForResult`() = + runTest { + coEvery { vaultRepository.syncForResult() } returns SyncVaultDataResult.Error(Exception()) + val viewModel = createViewModel() + viewModel.trySendAction(ImportLoginsAction.MoveToSyncInProgress) + viewModel.stateFlow.test { + assertNotNull(awaitItem().dialogState) + coEvery { vaultRepository.syncForResult() } returns SyncVaultDataResult.Success + viewModel.trySendAction(ImportLoginsAction.RetryVaultSync) + assertEquals( + ImportLoginsState( + dialogState = null, + viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = true, + ), + awaitItem(), + ) + } + coVerify { vaultRepository.syncForResult() } + } + + @Test + fun `MoveToSyncInProgress should send NavigateToImportSuccess event when sync succeeds`() = + runTest { + val viewModel = createViewModel() + viewModel.eventFlow.test { + viewModel.trySendAction(ImportLoginsAction.MoveToSyncInProgress) + assertEquals(ImportLoginsEvent.NavigateToImportSuccess, awaitItem()) + } + } + + @Test + fun `SyncVaultDataResult Error should remove loading state and show error dialog`() = runTest { + coEvery { vaultRepository.syncForResult() } returns SyncVaultDataResult.Error(Exception()) + val viewModel = createViewModel() + viewModel.trySendAction(ImportLoginsAction.MoveToSyncInProgress) + assertEquals( + ImportLoginsState( + dialogState = ImportLoginsState.DialogState.Error, + viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, + ), + viewModel.stateFlow.value, + ) + } + + @Test + fun `on FailSyncAcknowledged should remove dialog state and send NavigateBack event`() = + runTest { + coEvery { + vaultRepository.syncForResult() + } returns SyncVaultDataResult.Error(Exception()) + val viewModel = createViewModel() + viewModel.eventFlow.test { + viewModel.trySendAction(ImportLoginsAction.MoveToSyncInProgress) + assertNotNull(viewModel.stateFlow.value.dialogState) + viewModel.trySendAction(ImportLoginsAction.FailSyncAcknowledged) + assertEquals( + ImportLoginsState( + dialogState = null, + viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, + ), + viewModel.stateFlow.value, + ) + assertEquals( + ImportLoginsEvent.NavigateBack, + awaitItem(), + ) + } + } + + private fun createViewModel(): ImportLoginsViewModel = ImportLoginsViewModel( + vaultRepository = vaultRepository, + ) } private val DEFAULT_STATE = ImportLoginsState( dialogState = null, viewState = ImportLoginsState.ViewState.InitialContent, + isVaultSyncing = false, )