Use user friendly error messages in login following iOS logic (#174)

* Use user friendly error messages in login following iOS logic, fix dialog colors.

* Use `AlertDialogDefaults` for the default properties of dialogs

* Improve Maestro tests with wrong password dialog

* Add tests for error messages
This commit is contained in:
Jorge Martin Espinosa
2023-03-07 17:55:48 +01:00
committed by GitHub
parent eabf99cefb
commit ec66ed0a68
46 changed files with 300 additions and 113 deletions

View File

@@ -9,9 +9,15 @@ appId: ${APP_ID}
id: "login-email_username"
- inputText: ${USERNAME}
- pressKey: Enter
- tapOn: "Password"
- tapOn:
id: "login-password"
- inputText: "wrong-password"
- pressKey: Enter
- tapOn: "Continue"
- tapOn: "OK"
- tapOn:
id: "login-password"
- eraseText: 20
- inputText: ${PASSWORD}
- pressKey: Enter
- tapOn: "Continue"

1
changelog.d/132.bugfix Normal file
View File

@@ -0,0 +1 @@
Use user friendly error messages in login following iOS logic, fix dialog colors.

View File

@@ -25,6 +25,12 @@ plugins {
android {
namespace = "io.element.android.features.login"
testOptions {
unitTests {
isIncludeAndroidResources = true
}
}
}
anvil {

View File

@@ -51,6 +51,7 @@ import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.unit.dp
import io.element.android.features.login.R
import io.element.android.features.login.error.changeServerError
import io.element.android.libraries.architecture.Async
import io.element.android.libraries.designsystem.ElementTextStyles
import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog
@@ -239,7 +240,7 @@ fun ChangeServerView(
@Composable
internal fun ChangeServerErrorDialog(error: Throwable, onDismiss: () -> Unit) {
ErrorDialog(
content = error.localizedMessage ?: stringResource(id = StringR.string.unknown_error),
content = stringResource(changeServerError(error)),
onDismiss = onDismiss
)
}

View File

@@ -16,34 +16,28 @@
package io.element.android.features.login.error
import androidx.compose.runtime.Composable
import androidx.compose.ui.res.stringResource
import io.element.android.features.login.root.LoginFormState
import io.element.android.libraries.core.uri.isValidUrl
import io.element.android.libraries.ui.strings.R as StringR
import io.element.android.libraries.matrix.api.auth.AuthErrorCode
import io.element.android.libraries.matrix.api.auth.errorCode
import org.matrix.rustcomponents.sdk.AuthenticationException
import io.element.android.libraries.ui.strings.R.string as StringR
@Composable
fun loginError(
data: LoginFormState,
throwable: Throwable?
): String {
return when {
data.login.isEmpty() -> "Please enter a login"
data.password.isEmpty() -> "Please enter a password"
throwable != null -> stringResource(id = StringR.string.auth_invalid_login_param)
else -> "No error provided"
throwable: Throwable
): Int {
val authException = throwable as? AuthenticationException ?: return StringR.unknown_error
return when (authException.errorCode) {
AuthErrorCode.FORBIDDEN -> StringR.auth_invalid_login_param
AuthErrorCode.USER_DEACTIVATED -> StringR.auth_invalid_login_deactivated_account
AuthErrorCode.UNKNOWN -> StringR.unknown_error
}
}
@Composable
fun changeServerError(
data: String,
throwable: Throwable?
): String {
return when {
data.isEmpty() -> "Please enter a server URL"
!data.isValidUrl() -> stringResource(id = StringR.string.login_error_invalid_home_server)
throwable != null -> "That server doesnt seem right. Please check the address."
else -> "No error provided"
throwable: Throwable
): Int {
val authException = throwable as? AuthenticationException ?: return StringR.unknown_error
return when (authException) {
is AuthenticationException.InvalidServerName -> StringR.login_error_homeserver_not_found
else -> StringR.unknown_error
}
}

View File

@@ -62,6 +62,7 @@ import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.unit.dp
import io.element.android.features.login.error.loginError
import io.element.android.libraries.designsystem.ElementTextStyles
import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
import io.element.android.libraries.designsystem.components.form.textFieldState
@@ -153,6 +154,12 @@ fun LoginRootScreen(
}
}
}
if (state.loggedInState is LoggedInState.ErrorLoggingIn) {
LoginErrorDialog(error = state.loggedInState.failure, onDismiss = {
state.eventSink(LoginRootEvents.ClearError)
})
}
}
@Composable
@@ -299,12 +306,6 @@ internal fun LoginForm(
singleLine = true,
maxLines = 1,
)
if (state.loggedInState is LoggedInState.ErrorLoggingIn) {
LoginErrorDialog(error = state.loggedInState.failure, onDismiss = {
eventSink(LoginRootEvents.ClearError)
})
}
Spacer(Modifier.height(28.dp))
// Submit
@@ -323,7 +324,7 @@ internal fun LoginForm(
@Composable
internal fun LoginErrorDialog(error: Throwable, onDismiss: () -> Unit) {
ErrorDialog(
content = error.localizedMessage ?: stringResource(id = StringR.string.unknown_error),
content = stringResource(loginError(error)),
onDismiss = onDismiss
)
}

View File

@@ -0,0 +1,98 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.features.login.error
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import io.element.android.libraries.ui.strings.R
import org.matrix.rustcomponents.sdk.AuthenticationException
class ErrorFormatterTests {
// region loginError
@Test
fun `loginError - invalid unknown error returns unknown error message`() {
val error = Throwable("Some unknown error")
assertThat(loginError(error)).isEqualTo(R.string.unknown_error)
}
@Test
fun `loginError - invalid auth error returns unknown error message`() {
val error = AuthenticationException.SlidingSyncNotAvailable("Some message. Also contains M_FORBIDDEN, but won't be parsed")
assertThat(loginError(error)).isEqualTo(R.string.unknown_error)
}
@Test
fun `loginError - unknown error returns unknown error message`() {
val error = AuthenticationException.Generic("M_UNKNOWN")
assertThat(loginError(error)).isEqualTo(R.string.unknown_error)
}
@Test
fun `loginError - forbidden error returns incorrect credentials message`() {
val error = AuthenticationException.Generic("M_FORBIDDEN")
assertThat(loginError(error)).isEqualTo(R.string.auth_invalid_login_param)
}
@Test
fun `loginError - user_deactivated error returns deactivated account message`() {
val error = AuthenticationException.Generic("M_USER_DEACTIVATED")
assertThat(loginError(error)).isEqualTo(R.string.auth_invalid_login_deactivated_account)
}
// endregion loginError
// region changeServerError
@Test
fun `changeServerError - invalid unknown error returns unknown error message`() {
val error = Throwable("Some unknown error")
assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
}
@Test
fun `changeServerError - invalid auth error returns unknown error message`() {
val error = AuthenticationException.SlidingSyncNotAvailable("Some message. Also contains M_FORBIDDEN, but won't be parsed")
assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
}
@Test
fun `changeServerError - unknown error returns unknown error message`() {
val error = AuthenticationException.Generic("M_UNKNOWN")
assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
}
@Test
fun `changeServerError - forbidden error returns unknown error message`() {
val error = AuthenticationException.Generic("M_FORBIDDEN")
assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
}
@Test
fun `changeServerError - user_deactivated error returns unknown error message`() {
val error = AuthenticationException.Generic("M_USER_DEACTIVATED")
assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
}
@Test
fun `changeServerError - invalid server name error returns invalid server name error message`() {
val error = AuthenticationException.InvalidServerName("Server is not valid")
assertThat(changeServerError(error)).isEqualTo(R.string.login_error_homeserver_not_found)
}
// endregion changeServerError
}

View File

@@ -48,10 +48,10 @@ fun ConfirmationDialog(
onThirdButtonClicked: () -> Unit = {},
onDismiss: () -> Unit = {},
shape: Shape = AlertDialogDefaults.shape,
containerColor: Color = MaterialTheme.colorScheme.surfaceVariant,
iconContentColor: Color = MaterialTheme.colorScheme.onSurfaceVariant,
titleContentColor: Color = MaterialTheme.colorScheme.onSurfaceVariant,
textContentColor: Color = MaterialTheme.colorScheme.onSurfaceVariant,
containerColor: Color = AlertDialogDefaults.containerColor,
iconContentColor: Color = AlertDialogDefaults.iconContentColor,
titleContentColor: Color = AlertDialogDefaults.titleContentColor,
textContentColor: Color = AlertDialogDefaults.textContentColor,
tonalElevation: Dp = AlertDialogDefaults.TonalElevation,
) {
AlertDialog(

View File

@@ -16,10 +16,6 @@
package io.element.android.libraries.designsystem.components.dialogs
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.AlertDialog
import androidx.compose.material3.AlertDialogDefaults
import androidx.compose.material3.MaterialTheme
@@ -31,10 +27,8 @@ import androidx.compose.ui.graphics.Shape
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import io.element.android.libraries.designsystem.preview.ElementPreviewDark
import io.element.android.libraries.designsystem.preview.ElementPreviewLight
import io.element.android.libraries.designsystem.theme.components.Button
import io.element.android.libraries.designsystem.theme.components.Text
import io.element.android.libraries.ui.strings.R as StringR
@@ -46,10 +40,10 @@ fun ErrorDialog(
submitText: String = stringResource(id = StringR.string.ok),
onDismiss: () -> Unit = {},
shape: Shape = AlertDialogDefaults.shape,
containerColor: Color = MaterialTheme.colorScheme.surfaceVariant,
iconContentColor: Color = MaterialTheme.colorScheme.onSurfaceVariant,
titleContentColor: Color = MaterialTheme.colorScheme.onSurfaceVariant,
textContentColor: Color = MaterialTheme.colorScheme.onSurfaceVariant,
containerColor: Color = AlertDialogDefaults.containerColor,
iconContentColor: Color = AlertDialogDefaults.iconContentColor,
titleContentColor: Color = AlertDialogDefaults.titleContentColor,
textContentColor: Color = AlertDialogDefaults.textContentColor,
tonalElevation: Dp = AlertDialogDefaults.TonalElevation,
) {
AlertDialog(

View File

@@ -60,7 +60,7 @@ val materialColorSchemeLight = lightColorScheme(
surface = Color.White,
onSurface = Color.Black,
surfaceVariant = Gray_25,
onSurfaceVariant = Gray_150,
onSurfaceVariant = Gray_200,
// TODO surfaceTint = primary,
// TODO inverseSurface = ColorLightTokens.InverseSurface,
// TODO inverseOnSurface = ColorLightTokens.InverseOnSurface,

View File

@@ -41,4 +41,7 @@ dependencies {
implementation(libs.serialization.json)
api(projects.libraries.sessionStorage.api)
implementation(libs.coroutines.core)
testImplementation(libs.test.junit)
testImplementation(libs.test.truth)
}

View File

@@ -0,0 +1,34 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.libraries.matrix.api.auth
import org.matrix.rustcomponents.sdk.AuthenticationException
enum class AuthErrorCode(val value: String) {
UNKNOWN("M_UNKNOWN"),
USER_DEACTIVATED("M_USER_DEACTIVATED"),
FORBIDDEN("M_FORBIDDEN")
}
// This is taken from the iOS version. It seems like currently there's no better way to extract error codes
val AuthenticationException.errorCode: AuthErrorCode
get() {
val message = (this as? AuthenticationException.Generic)?.message ?: return AuthErrorCode.UNKNOWN
return enumValues<AuthErrorCode>()
.firstOrNull { message.contains(it.value) }
?: AuthErrorCode.UNKNOWN
}

View File

@@ -0,0 +1,49 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.libraries.matrix.api.auth
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.matrix.rustcomponents.sdk.AuthenticationException
class AuthErrorCodeTests {
@Test
fun `errorCode finds UNKNOWN code`() {
val error = AuthenticationException.Generic("M_UNKNOWN")
assertThat(error.errorCode).isEqualTo(AuthErrorCode.UNKNOWN)
}
@Test
fun `errorCode finds USER_DEACTIVATED code`() {
val error = AuthenticationException.Generic("M_USER_DEACTIVATED")
assertThat(error.errorCode).isEqualTo(AuthErrorCode.USER_DEACTIVATED)
}
@Test
fun `errorCode finds FORBIDDEN code`() {
val error = AuthenticationException.Generic("M_FORBIDDEN")
assertThat(error.errorCode).isEqualTo(AuthErrorCode.FORBIDDEN)
}
@Test
fun `errorCode cannot find code so it returns UNKNOWN`() {
val error = AuthenticationException.Generic("Some other error")
assertThat(error.errorCode).isEqualTo(AuthErrorCode.UNKNOWN)
}
}