From c7ab805f91b94d3c8a4845bdfaffae5e1a097205 Mon Sep 17 00:00:00 2001 From: Ramsey Smith <142836716+ramsey-livefront@users.noreply.github.com> Date: Tue, 10 Oct 2023 08:04:59 -0600 Subject: [PATCH] BIT-320: Loading and error dialogs (#101) --- .../network/model/GetTokenResponseJson.kt | 19 ++++ .../datasource/network/model/LoginResult.kt | 4 +- .../network/service/IdentityServiceImpl.kt | 22 ++-- .../auth/repository/AuthRepositoryImpl.kt | 10 +- .../network/model/BitwardenError.kt | 52 +++++++++ .../network/util/ExceptionExtensions.kt | 25 ++-- .../ui/auth/feature/login/LoginScreen.kt | 16 ++- .../ui/auth/feature/login/LoginViewModel.kt | 107 +++++++++++++----- .../components/BitwardenBasicDialog.kt | 18 +++ .../components/BitwardenLoadingDialog.kt | 105 +++++++++++++++++ .../network/service/IdentityServiceTest.kt | 25 ++++ .../auth/repository/AuthRepositoryTest.kt | 42 ++++++- .../ui/auth/feature/login/LoginScreenTest.kt | 16 +++ .../auth/feature/login/LoginViewModelTest.kt | 60 +++++++--- 14 files changed, 434 insertions(+), 87 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/model/BitwardenError.kt create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenLoadingDialog.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/GetTokenResponseJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/GetTokenResponseJson.kt index 7adb741a9..50929eb85 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/GetTokenResponseJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/GetTokenResponseJson.kt @@ -26,4 +26,23 @@ sealed class GetTokenResponseJson { @SerialName("HCaptcha_SiteKey") val captchaKey: String, ) : GetTokenResponseJson() + + /** + * Models json body of an invalid request. + */ + @Serializable + data class Invalid( + @SerialName("ErrorModel") + val errorModel: ErrorModel, + ) : GetTokenResponseJson() { + + /** + * The error body of an invalid request containing a message. + */ + @Serializable + data class ErrorModel( + @SerialName("Message") + val errorMessage: String, + ) + } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/LoginResult.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/LoginResult.kt index 174b63673..3179108f1 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/LoginResult.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/LoginResult.kt @@ -2,8 +2,6 @@ package com.x8bit.bitwarden.data.auth.datasource.network.model /** * Models result of logging in. - * - * TODO: Add more detail to these cases to expose server error messages (BIT-320) */ sealed class LoginResult { @@ -20,5 +18,5 @@ sealed class LoginResult { /** * There was an error logging in. */ - data object Error : LoginResult() + data class Error(val errorMessage: String?) : LoginResult() } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceImpl.kt index a35b79e85..b84495e69 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceImpl.kt @@ -2,8 +2,9 @@ package com.x8bit.bitwarden.data.auth.datasource.network.service import com.x8bit.bitwarden.data.auth.datasource.network.api.IdentityApi import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson +import com.x8bit.bitwarden.data.platform.datasource.network.model.toBitwardenError import com.x8bit.bitwarden.data.platform.datasource.network.util.base64UrlEncode -import com.x8bit.bitwarden.data.platform.datasource.network.util.parseErrorBodyAsResult +import com.x8bit.bitwarden.data.platform.datasource.network.util.parseErrorBodyOrNull import kotlinx.serialization.json.Json import java.net.HttpURLConnection.HTTP_BAD_REQUEST import java.util.UUID @@ -36,13 +37,14 @@ class IdentityServiceImpl constructor( email = email, captchaResponse = captchaToken, ) - .fold( - onSuccess = { Result.success(it) }, - onFailure = { - it.parseErrorBodyAsResult( - code = HTTP_BAD_REQUEST, - json = json, - ) - }, - ) + .recoverCatching { throwable -> + val bitwardenError = throwable.toBitwardenError() + bitwardenError.parseErrorBodyOrNull( + code = HTTP_BAD_REQUEST, + json = json, + ) ?: bitwardenError.parseErrorBodyOrNull( + code = HTTP_BAD_REQUEST, + json = json, + ) ?: throw throwable + } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 48a7fa515..d3392cf81 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -3,6 +3,7 @@ package com.x8bit.bitwarden.data.auth.repository import com.bitwarden.core.Kdf import com.bitwarden.sdk.Client import com.x8bit.bitwarden.data.auth.datasource.network.model.AuthState +import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson.CaptchaRequired import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson.Success import com.x8bit.bitwarden.data.auth.datasource.network.model.LoginResult @@ -61,10 +62,7 @@ class AuthRepositoryImpl @Inject constructor( ) } .fold( - onFailure = { - // TODO: Add more detail to error case to expose server error messages (BIT-320) - LoginResult.Error - }, + onFailure = { LoginResult.Error(errorMessage = null) }, onSuccess = { when (it) { is CaptchaRequired -> LoginResult.CaptchaRequired(it.captchaKey) @@ -75,6 +73,10 @@ class AuthRepositoryImpl @Inject constructor( mutableAuthStateFlow.value = AuthState.Authenticated(it.accessToken) LoginResult.Success } + + is GetTokenResponseJson.Invalid -> { + LoginResult.Error(errorMessage = it.errorModel.errorMessage) + } } }, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/model/BitwardenError.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/model/BitwardenError.kt new file mode 100644 index 000000000..2b93a9234 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/model/BitwardenError.kt @@ -0,0 +1,52 @@ +package com.x8bit.bitwarden.data.platform.datasource.network.model + +import retrofit2.HttpException +import java.io.IOException + +/** + * Represents different types of errors that can occur in the Bitwarden application. + */ +sealed class BitwardenError { + /** + * An abstract property that holds the underlying throwable that caused the error. + */ + abstract val throwable: Throwable + + /** + * Errors related to HTTP requests and responses. + */ + data class Http(override val throwable: HttpException) : BitwardenError() { + /** + * The error code of the HTTP response. + */ + val code: Int get() = throwable.code() + + /** + * The response body of the HTTP response. + */ + val responseBodyString: String? by lazy { + throwable.response()?.errorBody()?.string() + } + } + + /** + * Errors related to network. + */ + data class Network(override val throwable: IOException) : BitwardenError() + + /** + * Other types of errors not covered by any special cases. + */ + data class Other(override val throwable: Throwable) : BitwardenError() +} + +/** + * Convert a [Throwable] into a [BitwardenError]. + */ +fun Throwable.toBitwardenError(): BitwardenError { + return when (this) { + is IOException -> BitwardenError.Network(this) + is HttpException -> BitwardenError.Http(this) + else -> BitwardenError.Other(this) + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/util/ExceptionExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/util/ExceptionExtensions.kt index 34124e437..80a9ca213 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/util/ExceptionExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/util/ExceptionExtensions.kt @@ -1,8 +1,7 @@ package com.x8bit.bitwarden.data.platform.datasource.network.util -import kotlinx.serialization.ExperimentalSerializationApi +import com.x8bit.bitwarden.data.platform.datasource.network.model.BitwardenError import kotlinx.serialization.json.Json -import kotlinx.serialization.json.decodeFromStream import retrofit2.HttpException /** @@ -10,23 +9,21 @@ import retrofit2.HttpException * * Useful in service layer for parsing non-200 response bodies. * - * If the receiver is not an [HttpException] or the error body cannot be parsed, the original - * Throwable will be returned as a Result.failure. + * If the receiver is not an [HttpException] or the error body cannot be parsed, null will be + * returned. * * @param code HTTP code associated with the error. Only responses with this code will be attempted * to be parsed. * @param json [Json] serializer to use. */ -@OptIn(ExperimentalSerializationApi::class) -inline fun Throwable.parseErrorBodyAsResult(code: Int, json: Json): Result = - (this as? HttpException) - ?.response() - ?.takeIf { it.code() == code } - ?.errorBody() - ?.let { errorBody -> +inline fun BitwardenError.parseErrorBodyOrNull(code: Int, json: Json): T? = + (this as? BitwardenError.Http) + ?.takeIf { it.code == code } + ?.responseBodyString + ?.let { responseBody -> try { - Result.success(json.decodeFromStream(errorBody.byteStream())) + json.decodeFromString(responseBody) } catch (_: Exception) { - Result.failure(this) + null } - } ?: Result.failure(this) + } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreen.kt index d5e44fe8d..57f19a1a6 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreen.kt @@ -27,9 +27,11 @@ 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.IntentHandler -import com.x8bit.bitwarden.ui.platform.components.BitwardenOverflowTopAppBar +import com.x8bit.bitwarden.ui.platform.components.BitwardenBasicDialog import com.x8bit.bitwarden.ui.platform.components.BitwardenFilledButton +import com.x8bit.bitwarden.ui.platform.components.BitwardenLoadingDialog import com.x8bit.bitwarden.ui.platform.components.BitwardenOutlinedButtonWithIcon +import com.x8bit.bitwarden.ui.platform.components.BitwardenOverflowTopAppBar import com.x8bit.bitwarden.ui.platform.components.BitwardenPasswordField /** @@ -48,11 +50,6 @@ fun LoginScreen( when (event) { LoginEvent.NavigateBack -> onNavigateBack() is LoginEvent.NavigateToCaptcha -> intentHandler.startActivity(intent = event.intent) - is LoginEvent.ShowErrorDialog -> { - // TODO Show proper error Dialog - Toast.makeText(context, event.messageRes, Toast.LENGTH_SHORT).show() - } - is LoginEvent.ShowToast -> { Toast.makeText(context, event.message, Toast.LENGTH_SHORT).show() } @@ -67,6 +64,13 @@ fun LoginScreen( .background(MaterialTheme.colorScheme.surface) .verticalScroll(scrollState), ) { + BitwardenLoadingDialog( + visibilityState = state.loadingDialogState, + ) + BitwardenBasicDialog( + visibilityState = state.errorDialogState, + onDismissRequest = { viewModel.trySendAction(LoginAction.ErrorDialogDismiss) }, + ) BitwardenOverflowTopAppBar( title = stringResource(id = R.string.app_name), navigationIcon = painterResource(id = R.drawable.ic_close), diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModel.kt index 15526b993..c04eda532 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModel.kt @@ -1,8 +1,9 @@ +@file:Suppress("TooManyFunctions") + package com.x8bit.bitwarden.ui.auth.feature.login import android.content.Intent import android.os.Parcelable -import androidx.annotation.StringRes import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.R @@ -11,6 +12,9 @@ import com.x8bit.bitwarden.data.auth.datasource.network.util.CaptchaCallbackToke import com.x8bit.bitwarden.data.auth.datasource.network.util.generateIntentForCaptcha import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.ui.platform.base.BaseViewModel +import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.components.BasicDialogState +import com.x8bit.bitwarden.ui.platform.components.LoadingDialogState import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach @@ -35,6 +39,8 @@ class LoginViewModel @Inject constructor( isLoginButtonEnabled = true, passwordInput = "", region = LoginArgs(savedStateHandle).regionLabel, + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ), ) { @@ -62,16 +68,59 @@ class LoginViewModel @Inject constructor( LoginAction.NotYouButtonClick -> handleNotYouButtonClicked() LoginAction.SingleSignOnClick -> handleSingleSignOnClicked() is LoginAction.PasswordInputChanged -> handlePasswordInputChanged(action) + is LoginAction.ErrorDialogDismiss -> handleErrorDialogDismiss() is LoginAction.Internal.ReceiveCaptchaToken -> { handleCaptchaTokenReceived(action.tokenResult) } + is LoginAction.Internal.ReceiveLoginResult -> { + handleReceiveLoginResult(action = action) + } } } + private fun handleReceiveLoginResult(action: LoginAction.Internal.ReceiveLoginResult) { + when (val loginResult = action.loginResult) { + is LoginResult.CaptchaRequired -> { + mutableStateFlow.update { it.copy(loadingDialogState = LoadingDialogState.Hidden) } + sendEvent( + event = LoginEvent.NavigateToCaptcha( + intent = loginResult.generateIntentForCaptcha(), + ), + ) + } + is LoginResult.Error -> { + mutableStateFlow.update { + it.copy( + errorDialogState = BasicDialogState.Shown( + title = R.string.an_error_has_occurred.asText(), + message = (loginResult.errorMessage)?.asText() + ?: R.string.generic_error_message.asText(), + ), + loadingDialogState = LoadingDialogState.Hidden, + ) + } + } + is LoginResult.Success -> { + mutableStateFlow.update { it.copy(loadingDialogState = LoadingDialogState.Hidden) } + } + } + } + + private fun handleErrorDialogDismiss() { + mutableStateFlow.update { it.copy(errorDialogState = BasicDialogState.Hidden) } + } + private fun handleCaptchaTokenReceived(tokenResult: CaptchaCallbackTokenResult) { when (tokenResult) { CaptchaCallbackTokenResult.MissingToken -> { - sendEvent(LoginEvent.ShowErrorDialog(messageRes = R.string.captcha_failed)) + mutableStateFlow.update { + it.copy( + errorDialogState = BasicDialogState.Shown( + title = R.string.log_in_denied.asText(), + message = R.string.captcha_failed.asText(), + ), + ) + } } is CaptchaCallbackTokenResult.Success -> attemptLogin(captchaToken = tokenResult.token) @@ -87,37 +136,24 @@ class LoginViewModel @Inject constructor( } private fun attemptLogin(captchaToken: String?) { - viewModelScope.launch { - // TODO: show progress here BIT-320 - sendEvent( - event = LoginEvent.ShowToast( - message = "Loading...", + mutableStateFlow.update { + it.copy( + loadingDialogState = LoadingDialogState.Shown( + text = R.string.logging_in.asText(), ), ) + } + viewModelScope.launch { val result = authRepository.login( email = mutableStateFlow.value.emailAddress, password = mutableStateFlow.value.passwordInput, captchaToken = captchaToken, ) - when (result) { - // TODO: show an error here BIT-320 - LoginResult.Error -> { - sendEvent( - event = LoginEvent.ShowToast( - message = "Error when logging in", - ), - ) - } - // No action required on success, root nav will navigate to logged in state - LoginResult.Success -> Unit - is LoginResult.CaptchaRequired -> { - sendEvent( - event = LoginEvent.NavigateToCaptcha( - intent = result.generateIntentForCaptcha(), - ), - ) - } - } + sendAction( + LoginAction.Internal.ReceiveLoginResult( + loginResult = result, + ), + ) } } @@ -149,6 +185,8 @@ data class LoginState( val emailAddress: String, val region: String, val isLoginButtonEnabled: Boolean, + val loadingDialogState: LoadingDialogState, + val errorDialogState: BasicDialogState, ) : Parcelable /** @@ -165,11 +203,6 @@ sealed class LoginEvent { */ data class NavigateToCaptcha(val intent: Intent) : LoginEvent() - /** - * Shows an error pop up with a given message - */ - data class ShowErrorDialog(@StringRes val messageRes: Int) : LoginEvent() - /** * Shows a toast with the given [message]. */ @@ -205,6 +238,11 @@ sealed class LoginAction { */ data object SingleSignOnClick : LoginAction() + /** + * Indicates that the error dialog has been dismissed. + */ + data object ErrorDialogDismiss : LoginAction() + /** * Indicates that the password input has changed. */ @@ -220,5 +258,12 @@ sealed class LoginAction { data class ReceiveCaptchaToken( val tokenResult: CaptchaCallbackTokenResult, ) : Internal() + + /** + * Indicates a login result has been received. + */ + data class ReceiveLoginResult( + val loginResult: LoginResult, + ) : Internal() } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenBasicDialog.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenBasicDialog.kt index c6637a703..318b1097c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenBasicDialog.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenBasicDialog.kt @@ -6,8 +6,11 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.res.stringResource +import androidx.compose.ui.tooling.preview.Preview import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.Text +import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme import kotlinx.parcelize.Parcelize /** @@ -46,6 +49,21 @@ fun BitwardenBasicDialog( style = MaterialTheme.typography.bodyMedium, ) }, + containerColor = MaterialTheme.colorScheme.surfaceContainerHigh, + ) + } +} + +@Preview +@Composable +private fun BitwardenBasicDialog_preview() { + BitwardenTheme { + BitwardenBasicDialog( + visibilityState = BasicDialogState.Shown( + title = "An error has occurred.".asText(), + message = "Username or password is incorrect. Try again.".asText(), + ), + onDismissRequest = {}, ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenLoadingDialog.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenLoadingDialog.kt new file mode 100644 index 000000000..c2ceb7be9 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenLoadingDialog.kt @@ -0,0 +1,105 @@ +package com.x8bit.bitwarden.ui.platform.components + +import android.os.Parcelable +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.wrapContentHeight +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Card +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.ExperimentalMaterial3Api +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.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import com.x8bit.bitwarden.ui.platform.base.util.Text +import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme +import kotlinx.parcelize.Parcelize + +/** + * Represents a Bitwarden-styled loading dialog that shows text and a circular progress indicator. + * + * @param visibilityState the [LoadingDialogState] used to populate the dialog. + * @param onDismissRequest called when the user has requested to dismiss the dialog, whether by + * * tapping "OK", tapping outside the dialog, or pressing the back button + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun BitwardenLoadingDialog( + visibilityState: LoadingDialogState, +) { + when (visibilityState) { + is LoadingDialogState.Hidden -> Unit + is LoadingDialogState.Shown -> { + AlertDialog( + onDismissRequest = {}, + ) { + Card( + shape = RoundedCornerShape(28.dp), + modifier = Modifier + .fillMaxWidth() + .wrapContentHeight(), + ) { + Column( + modifier = Modifier + .fillMaxWidth() + .background(MaterialTheme.colorScheme.surfaceContainerHigh), + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Text( + text = visibilityState.text(), + modifier = Modifier.padding( + top = 24.dp, + bottom = 8.dp, + ), + ) + CircularProgressIndicator( + modifier = Modifier.padding( + top = 8.dp, + bottom = 24.dp, + ), + ) + } + } + } + } + } +} + +@Preview +@Composable +private fun BitwardenLoadingDialog_preview() { + BitwardenTheme { + BitwardenLoadingDialog( + visibilityState = LoadingDialogState.Shown( + text = "Loading...".asText(), + ), + ) + } +} + +/** + * Models display of a [BitwardenLoadingDialog]. + */ +sealed class LoadingDialogState : Parcelable { + /** + * Hide the dialog. + */ + @Parcelize + data object Hidden : LoadingDialogState() + + /** + * Show the dialog with the given values. + */ + @Parcelize + data class Shown(val text: Text) : LoadingDialogState() +} diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt index 46ef09db6..89ce9fd45 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt @@ -54,6 +54,17 @@ class IdentityServiceTest : BaseServiceTest() { assertEquals(Result.success(CAPTCHA_BODY), result) } + @Test + fun `getToken when response is a 400 with an error body should return Invalid`() = runTest { + server.enqueue(MockResponse().setResponseCode(400).setBody(INVALID_LOGIN_JSON)) + val result = identityService.getToken( + email = EMAIL, + passwordHash = PASSWORD_HASH, + captchaToken = null, + ) + assertEquals(Result.success(INVALID_LOGIN), result) + } + companion object { private const val EMAIL = "email" private const val PASSWORD_HASH = "passwordHash" @@ -73,3 +84,17 @@ private const val LOGIN_SUCCESS_JSON = """ } """ private val LOGIN_SUCCESS = GetTokenResponseJson.Success("123") + +private const val INVALID_LOGIN_JSON = """ +{ + "ErrorModel": { + "Message": "123" + } +} +""" + +private val INVALID_LOGIN = GetTokenResponseJson.Invalid( + errorModel = GetTokenResponseJson.Invalid.ErrorModel( + errorMessage = "123", + ), +) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 860bc01d5..129c1b04b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -50,18 +50,18 @@ class AuthRepositoryTest { } @Test - fun `login when pre login fails should return Error`() = runTest { + fun `login when pre login fails should return Error with no message`() = runTest { coEvery { accountsService.preLogin(email = EMAIL) } returns (Result.failure(RuntimeException())) val result = repository.login(email = EMAIL, password = PASSWORD, captchaToken = null) - assertEquals(LoginResult.Error, result) + assertEquals(LoginResult.Error(errorMessage = null), result) assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value) coVerify { accountsService.preLogin(email = EMAIL) } } @Test - fun `login get token fails should return Error`() = runTest { + fun `login get token fails should return Error with no message`() = runTest { coEvery { accountsService.preLogin(email = EMAIL) } returns Result.success(PRE_LOGIN_SUCCESS) @@ -74,7 +74,41 @@ class AuthRepositoryTest { } .returns(Result.failure(RuntimeException())) val result = repository.login(email = EMAIL, password = PASSWORD, captchaToken = null) - assertEquals(LoginResult.Error, result) + assertEquals(LoginResult.Error(errorMessage = null), result) + assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value) + coVerify { accountsService.preLogin(email = EMAIL) } + coVerify { + identityService.getToken( + email = EMAIL, + passwordHash = PASSWORD_HASH, + captchaToken = null, + ) + } + } + + @Test + fun `login get token returns Invalid should return Error with correct message`() = runTest { + coEvery { + accountsService.preLogin(email = EMAIL) + } returns Result.success(PRE_LOGIN_SUCCESS) + coEvery { + identityService.getToken( + email = EMAIL, + passwordHash = PASSWORD_HASH, + captchaToken = null, + ) + } + .returns( + Result.success( + GetTokenResponseJson.Invalid( + errorModel = GetTokenResponseJson.Invalid.ErrorModel( + errorMessage = "mock_error_message", + ), + ), + ), + ) + val result = repository.login(email = EMAIL, password = PASSWORD, captchaToken = null) + assertEquals(LoginResult.Error(errorMessage = "mock_error_message"), result) assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value) coVerify { accountsService.preLogin(email = EMAIL) } coVerify { diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreenTest.kt index 2a84682bf..2e0745527 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreenTest.kt @@ -14,6 +14,8 @@ import androidx.compose.ui.test.performScrollTo import androidx.compose.ui.test.performTextInput import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest import com.x8bit.bitwarden.ui.platform.base.util.IntentHandler +import com.x8bit.bitwarden.ui.platform.components.BasicDialogState +import com.x8bit.bitwarden.ui.platform.components.LoadingDialogState import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -35,6 +37,8 @@ class LoginScreenTest : BaseComposeTest() { isLoginButtonEnabled = false, passwordInput = "", region = "", + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ), ) } @@ -60,6 +64,8 @@ class LoginScreenTest : BaseComposeTest() { isLoginButtonEnabled = false, passwordInput = "", region = "", + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ), ) } @@ -85,6 +91,8 @@ class LoginScreenTest : BaseComposeTest() { isLoginButtonEnabled = false, passwordInput = "", region = "", + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ), ) } @@ -110,6 +118,8 @@ class LoginScreenTest : BaseComposeTest() { isLoginButtonEnabled = false, passwordInput = "", region = "", + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ), ) } @@ -147,6 +157,8 @@ class LoginScreenTest : BaseComposeTest() { isLoginButtonEnabled = false, passwordInput = "", region = "", + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ), ) } @@ -173,6 +185,8 @@ class LoginScreenTest : BaseComposeTest() { isLoginButtonEnabled = false, passwordInput = "", region = "", + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ), ) } @@ -199,6 +213,8 @@ class LoginScreenTest : BaseComposeTest() { isLoginButtonEnabled = false, passwordInput = "", region = "", + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ), ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt index 2009b2e6b..9af0e7f41 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt @@ -3,11 +3,15 @@ package com.x8bit.bitwarden.ui.auth.feature.login import android.content.Intent import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.datasource.network.model.LoginResult import com.x8bit.bitwarden.data.auth.datasource.network.util.CaptchaCallbackTokenResult import com.x8bit.bitwarden.data.auth.datasource.network.util.generateIntentForCaptcha import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest +import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.components.BasicDialogState +import com.x8bit.bitwarden.ui.platform.components.LoadingDialogState import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every @@ -92,8 +96,7 @@ class LoginViewModelTest : BaseViewModelTest() { } @Test - fun `LoginButtonClick login returns error should emit error ShowToast`() = runTest { - // TODO: handle and display errors (BIT-320) + fun `LoginButtonClick login returns error should update errorDialogState`() = runTest { val authRepository = mockk { coEvery { login( @@ -101,19 +104,32 @@ class LoginViewModelTest : BaseViewModelTest() { password = "", captchaToken = null, ) - } returns LoginResult.Error + } returns LoginResult.Error(errorMessage = "mock_error") every { captchaTokenResultFlow } returns flowOf() } val viewModel = LoginViewModel( authRepository = authRepository, savedStateHandle = savedStateHandle, ) - viewModel.eventFlow.test { - viewModel.actionChannel.trySend(LoginAction.LoginButtonClick) - assertEquals(DEFAULT_STATE, viewModel.stateFlow.value) - assertEquals(LoginEvent.ShowToast("Loading..."), awaitItem()) + viewModel.stateFlow.test { + assertEquals(DEFAULT_STATE, awaitItem()) + viewModel.trySendAction(LoginAction.LoginButtonClick) assertEquals( - LoginEvent.ShowToast("Error when logging in"), + DEFAULT_STATE.copy( + loadingDialogState = LoadingDialogState.Shown( + text = R.string.logging_in.asText(), + ), + ), + awaitItem(), + ) + assertEquals( + DEFAULT_STATE.copy( + errorDialogState = BasicDialogState.Shown( + title = R.string.an_error_has_occurred.asText(), + message = "mock_error".asText(), + ), + loadingDialogState = LoadingDialogState.Hidden, + ), awaitItem(), ) } @@ -123,19 +139,32 @@ class LoginViewModelTest : BaseViewModelTest() { } @Test - fun `LoginButtonClick login returns success should emit loading ShowToast`() = runTest { + fun `LoginButtonClick login returns success should update loadingDialogState`() = runTest { val authRepository = mockk { - coEvery { login("test@gmail.com", "", captchaToken = null) } returns LoginResult.Success + coEvery { + login("test@gmail.com", "", captchaToken = null) + } returns LoginResult.Success every { captchaTokenResultFlow } returns flowOf() } val viewModel = LoginViewModel( authRepository = authRepository, savedStateHandle = savedStateHandle, ) - viewModel.eventFlow.test { - viewModel.actionChannel.trySend(LoginAction.LoginButtonClick) - assertEquals(DEFAULT_STATE, viewModel.stateFlow.value) - assertEquals(LoginEvent.ShowToast("Loading..."), awaitItem()) + viewModel.stateFlow.test { + assertEquals(DEFAULT_STATE, awaitItem()) + viewModel.trySendAction(LoginAction.LoginButtonClick) + assertEquals( + DEFAULT_STATE.copy( + loadingDialogState = LoadingDialogState.Shown( + text = R.string.logging_in.asText(), + ), + ), + awaitItem(), + ) + assertEquals( + DEFAULT_STATE.copy(loadingDialogState = LoadingDialogState.Hidden), + awaitItem(), + ) } coVerify { authRepository.login(email = "test@gmail.com", password = "", captchaToken = null) @@ -163,7 +192,6 @@ class LoginViewModelTest : BaseViewModelTest() { viewModel.eventFlow.test { viewModel.actionChannel.trySend(LoginAction.LoginButtonClick) assertEquals(DEFAULT_STATE, viewModel.stateFlow.value) - assertEquals(LoginEvent.ShowToast("Loading..."), awaitItem()) assertEquals(LoginEvent.NavigateToCaptcha(intent = mockkIntent), awaitItem()) } coVerify { @@ -271,6 +299,8 @@ class LoginViewModelTest : BaseViewModelTest() { passwordInput = "", isLoginButtonEnabled = true, region = "", + loadingDialogState = LoadingDialogState.Hidden, + errorDialogState = BasicDialogState.Hidden, ) private const val LOGIN_RESULT_PATH =