From d0dfe3ca2f5ccac3132bb0f36244ca1846d36933 Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 30 Jan 2024 19:55:49 -0600 Subject: [PATCH] Add dialog state to LoginWithDevice state (#883) --- .../loginwithdevice/LoginWithDeviceScreen.kt | 57 ++++++------ .../LoginWithDeviceViewModel.kt | 35 +++++-- .../LoginWithDeviceScreenTest.kt | 36 +++---- .../LoginWithDeviceViewModelTest.kt | 93 ++++++++++--------- 4 files changed, 126 insertions(+), 95 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreen.kt index 1ecf63fe6..2ac0f65ca 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreen.kt @@ -1,7 +1,6 @@ package com.x8bit.bitwarden.ui.auth.feature.loginwithdevice import android.widget.Toast -import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.defaultMinSize @@ -39,10 +38,10 @@ import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect -import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.components.BasicDialogState import com.x8bit.bitwarden.ui.platform.components.BitwardenBasicDialog import com.x8bit.bitwarden.ui.platform.components.BitwardenClickableText +import com.x8bit.bitwarden.ui.platform.components.BitwardenLoadingContent import com.x8bit.bitwarden.ui.platform.components.BitwardenScaffold import com.x8bit.bitwarden.ui.platform.components.BitwardenTopAppBar import com.x8bit.bitwarden.ui.platform.theme.LocalNonMaterialColors @@ -69,6 +68,13 @@ fun LoginWithDeviceScreen( } } + LoginWithDeviceDialogs( + state = state.dialogState, + onDismissDialog = remember(viewModel) { + { viewModel.trySendAction(LoginWithDeviceAction.DismissDialog) } + }, + ) + val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState()) BitwardenScaffold( modifier = Modifier @@ -93,9 +99,6 @@ fun LoginWithDeviceScreen( is LoginWithDeviceState.ViewState.Content -> { LoginWithDeviceScreenContent( state = viewState, - onErrorDialogDismiss = remember(viewModel) { - { viewModel.trySendAction(LoginWithDeviceAction.ErrorDialogDismiss) } - }, onResendNotificationClick = remember(viewModel) { { viewModel.trySendAction(LoginWithDeviceAction.ResendNotificationClick) } }, @@ -106,16 +109,9 @@ fun LoginWithDeviceScreen( ) } - LoginWithDeviceState.ViewState.Loading -> { - Column( - modifier = modifier, - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally, - ) { - CircularProgressIndicator() - Spacer(modifier = Modifier.navigationBarsPadding()) - } - } + LoginWithDeviceState.ViewState.Loading -> BitwardenLoadingContent( + modifier = modifier, + ) } } } @@ -125,23 +121,10 @@ fun LoginWithDeviceScreen( @Composable private fun LoginWithDeviceScreenContent( state: LoginWithDeviceState.ViewState.Content, - onErrorDialogDismiss: () -> Unit, onResendNotificationClick: () -> Unit, onViewAllLogInOptionsClick: () -> Unit, modifier: Modifier = Modifier, ) { - BitwardenBasicDialog( - visibilityState = if (state.shouldShowErrorDialog) { - BasicDialogState.Shown( - title = R.string.an_error_has_occurred.asText(), - message = R.string.generic_error_message.asText(), - ) - } else { - BasicDialogState.Hidden - }, - onDismissRequest = onErrorDialogDismiss, - ) - Column( horizontalAlignment = Alignment.CenterHorizontally, modifier = modifier @@ -260,3 +243,21 @@ private fun LoginWithDeviceScreenContent( Spacer(modifier = Modifier.navigationBarsPadding()) } } + +@Composable +private fun LoginWithDeviceDialogs( + state: LoginWithDeviceState.DialogState?, + onDismissDialog: () -> Unit, +) { + when (state) { + is LoginWithDeviceState.DialogState.Error -> BitwardenBasicDialog( + visibilityState = BasicDialogState.Shown( + title = state.title, + message = state.message, + ), + onDismissRequest = onDismissDialog, + ) + + null -> Unit + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceViewModel.kt index 9490c897b..740b128fd 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceViewModel.kt @@ -3,9 +3,12 @@ package com.x8bit.bitwarden.ui.auth.feature.loginwithdevice import android.os.Parcelable import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.AuthRequestResult 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 @@ -26,6 +29,7 @@ class LoginWithDeviceViewModel @Inject constructor( ?: LoginWithDeviceState( emailAddress = LoginWithDeviceArgs(savedStateHandle).emailAddress, viewState = LoginWithDeviceState.ViewState.Loading, + dialogState = null, ), ) { init { @@ -35,7 +39,7 @@ class LoginWithDeviceViewModel @Inject constructor( override fun handleAction(action: LoginWithDeviceAction) { when (action) { LoginWithDeviceAction.CloseButtonClick -> handleCloseButtonClicked() - LoginWithDeviceAction.ErrorDialogDismiss -> handleErrorDialogDismissed() + LoginWithDeviceAction.DismissDialog -> handleErrorDialogDismissed() LoginWithDeviceAction.ResendNotificationClick -> handleResendNotificationClicked() LoginWithDeviceAction.ViewAllLogInOptionsClick -> handleViewAllLogInOptionsClicked() @@ -50,7 +54,7 @@ class LoginWithDeviceViewModel @Inject constructor( } private fun handleErrorDialogDismissed() { - updateContent { it.copy(shouldShowErrorDialog = false) } + mutableStateFlow.update { it.copy(dialogState = null) } } private fun handleResendNotificationClicked() { @@ -71,8 +75,8 @@ class LoginWithDeviceViewModel @Inject constructor( viewState = LoginWithDeviceState.ViewState.Content( fingerprintPhrase = action.result.authRequest.fingerprint, isResendNotificationLoading = false, - shouldShowErrorDialog = false, ), + dialogState = null, ) } } @@ -83,7 +87,10 @@ class LoginWithDeviceViewModel @Inject constructor( viewState = LoginWithDeviceState.ViewState.Content( fingerprintPhrase = "", isResendNotificationLoading = false, - shouldShowErrorDialog = true, + ), + dialogState = LoginWithDeviceState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.generic_error_message.asText(), ), ) } @@ -128,6 +135,7 @@ class LoginWithDeviceViewModel @Inject constructor( data class LoginWithDeviceState( val emailAddress: String, val viewState: ViewState, + val dialogState: DialogState?, ) : Parcelable { /** * Represents the specific view states for the [LoginWithDeviceScreen]. @@ -150,9 +158,22 @@ data class LoginWithDeviceState( data class Content( val fingerprintPhrase: String, val isResendNotificationLoading: Boolean, - val shouldShowErrorDialog: Boolean, ) : ViewState() } + + /** + * Represents the current state of any dialogs on the screen. + */ + sealed class DialogState : Parcelable { + /** + * Displays an error dialog to the user. + */ + @Parcelize + data class Error( + val title: Text?, + val message: Text, + ) : DialogState() + } } /** @@ -182,9 +203,9 @@ sealed class LoginWithDeviceAction { data object CloseButtonClick : LoginWithDeviceAction() /** - * Indicates that the error dialog was dismissed. + * Indicates that the dialog should be dismissed. */ - data object ErrorDialogDismiss : LoginWithDeviceAction() + data object DismissDialog : LoginWithDeviceAction() /** * Indicates that the "Resend notification" text has been clicked. diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreenTest.kt index fbba1d2bc..b14ef3d21 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreenTest.kt @@ -8,8 +8,10 @@ import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollTo +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest +import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.util.isProgressBar import io.mockk.every import io.mockk.mockk @@ -48,13 +50,12 @@ class LoginWithDeviceScreenTest : BaseComposeTest() { } @Test - fun `dismissing error dialog should send ErrorDialogDismiss`() { + fun `dismissing dialog should send DismissDialog`() { mutableStateFlow.update { it.copy( - viewState = LoginWithDeviceState.ViewState.Content( - fingerprintPhrase = "", - isResendNotificationLoading = false, - shouldShowErrorDialog = true, + dialogState = LoginWithDeviceState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.generic_error_message.asText(), ), ) } @@ -63,7 +64,7 @@ class LoginWithDeviceScreenTest : BaseComposeTest() { .assert(hasAnyAncestor(isDialog())) .performClick() verify { - viewModel.trySendAction(LoginWithDeviceAction.ErrorDialogDismiss) + viewModel.trySendAction(LoginWithDeviceAction.DismissDialog) } } @@ -101,16 +102,15 @@ class LoginWithDeviceScreenTest : BaseComposeTest() { } composeTestRule.onNode(isProgressBar).assertDoesNotExist() } - - companion object { - private const val EMAIL = "test@gmail.com" - private val DEFAULT_STATE = LoginWithDeviceState( - emailAddress = EMAIL, - viewState = LoginWithDeviceState.ViewState.Content( - fingerprintPhrase = "alabster-drinkable-mystified-rapping-irrigate", - isResendNotificationLoading = false, - shouldShowErrorDialog = false, - ), - ) - } } + +private const val EMAIL = "test@gmail.com" + +private val DEFAULT_STATE = LoginWithDeviceState( + emailAddress = EMAIL, + viewState = LoginWithDeviceState.ViewState.Content( + fingerprintPhrase = "alabster-drinkable-mystified-rapping-irrigate", + isResendNotificationLoading = false, + ), + dialogState = null, +) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceViewModelTest.kt index 5cc53ef4c..9c917dd86 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceViewModelTest.kt @@ -2,10 +2,12 @@ package com.x8bit.bitwarden.ui.auth.feature.loginwithdevice import androidx.lifecycle.SavedStateHandle 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.AuthRequest import com.x8bit.bitwarden.data.auth.repository.model.AuthRequestResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest +import com.x8bit.bitwarden.ui.platform.base.util.asText import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every @@ -39,14 +41,7 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() { coEvery { authRepository.createAuthRequest(newEmail) } returns AuthRequestResult.Success(AUTH_REQUEST) - val state = LoginWithDeviceState( - emailAddress = newEmail, - viewState = LoginWithDeviceState.ViewState.Content( - fingerprintPhrase = FINGERPRINT, - isResendNotificationLoading = false, - shouldShowErrorDialog = false, - ), - ) + val state = DEFAULT_STATE.copy(emailAddress = newEmail) val viewModel = createViewModel(state) viewModel.stateFlow.test { assertEquals(state, awaitItem()) @@ -68,6 +63,19 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() { } } + @Test + fun `DismissDialog should clear the dialog state`() = runTest { + val initialState = DEFAULT_STATE.copy( + dialogState = LoginWithDeviceState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.generic_error_message.asText(), + ), + ) + val viewModel = createViewModel(initialState) + viewModel.actionChannel.trySend(LoginWithDeviceAction.DismissDialog) + assertEquals(initialState.copy(dialogState = null), viewModel.stateFlow.value) + } + @Test fun `ResendNotificationClick should create new auth request and update state`() = runTest { val newFingerprint = "newFingerprint" @@ -78,10 +86,8 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() { viewModel.actionChannel.trySend(LoginWithDeviceAction.ResendNotificationClick) assertEquals( DEFAULT_STATE.copy( - viewState = LoginWithDeviceState.ViewState.Content( + viewState = DEFAULT_CONTENT_VIEW_STATE.copy( fingerprintPhrase = newFingerprint, - isResendNotificationLoading = false, - shouldShowErrorDialog = false, ), ), viewModel.stateFlow.value, @@ -117,10 +123,8 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() { ) assertEquals( DEFAULT_STATE.copy( - viewState = LoginWithDeviceState.ViewState.Content( + viewState = DEFAULT_CONTENT_VIEW_STATE.copy( fingerprintPhrase = newFingerprint, - isResendNotificationLoading = false, - shouldShowErrorDialog = false, ), ), viewModel.stateFlow.value, @@ -141,7 +145,10 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() { viewState = LoginWithDeviceState.ViewState.Content( fingerprintPhrase = "", isResendNotificationLoading = false, - shouldShowErrorDialog = true, + ), + dialogState = LoginWithDeviceState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.generic_error_message.asText(), ), ), viewModel.stateFlow.value, @@ -149,36 +156,38 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() { } private fun createViewModel( - state: LoginWithDeviceState = DEFAULT_STATE, + state: LoginWithDeviceState? = DEFAULT_STATE, ): LoginWithDeviceViewModel = LoginWithDeviceViewModel( authRepository = authRepository, savedStateHandle = SavedStateHandle().apply { set("state", state) }, ) - - companion object { - private const val EMAIL = "test@gmail.com" - private const val FINGERPRINT = "fingerprint" - private val DEFAULT_STATE = LoginWithDeviceState( - emailAddress = EMAIL, - viewState = LoginWithDeviceState.ViewState.Content( - fingerprintPhrase = FINGERPRINT, - isResendNotificationLoading = false, - shouldShowErrorDialog = false, - ), - ) - private val AUTH_REQUEST = AuthRequest( - id = "1", - publicKey = "2", - platform = "Android", - ipAddress = "192.168.0.1", - key = "public", - masterPasswordHash = "verySecureHash", - creationDate = ZonedDateTime.parse("2024-09-13T00:00Z"), - responseDate = null, - requestApproved = true, - originUrl = "www.bitwarden.com", - fingerprint = FINGERPRINT, - ) - } } + +private const val EMAIL = "test@gmail.com" +private const val FINGERPRINT = "fingerprint" + +private val DEFAULT_CONTENT_VIEW_STATE = LoginWithDeviceState.ViewState.Content( + fingerprintPhrase = FINGERPRINT, + isResendNotificationLoading = false, +) + +private val DEFAULT_STATE = LoginWithDeviceState( + emailAddress = EMAIL, + viewState = DEFAULT_CONTENT_VIEW_STATE, + dialogState = null, +) + +private val AUTH_REQUEST = AuthRequest( + id = "1", + publicKey = "2", + platform = "Android", + ipAddress = "192.168.0.1", + key = "public", + masterPasswordHash = "verySecureHash", + creationDate = ZonedDateTime.parse("2024-09-13T00:00Z"), + responseDate = null, + requestApproved = true, + originUrl = "www.bitwarden.com", + fingerprint = FINGERPRINT, +)