Handle 'invalid server' error in server selection screen properly. (#214)
* Handle 'invalid server' error in server selection screen properly. * Use `action_learn_more` for composing the server location footer action.
This commit is contained in:
Jorge Martin Espinosa
GitHub
parent
93a77d94c1
commit
2906168baa
24 changed files with 132 additions and 127 deletions
|
|
@ -0,0 +1,38 @@
|
|||
/*
|
||||
* 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.impl.changeserver
|
||||
|
||||
import androidx.annotation.StringRes
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import io.element.android.libraries.matrix.api.auth.AuthenticationException
|
||||
import io.element.android.libraries.ui.strings.R
|
||||
|
||||
sealed class ChangeServerError : Throwable() {
|
||||
data class InlineErrorMessage(@StringRes val messageId: Int) : ChangeServerError() {
|
||||
@Composable
|
||||
fun message(): String = stringResource(messageId)
|
||||
}
|
||||
object SlidingSyncAlert : ChangeServerError()
|
||||
|
||||
companion object {
|
||||
fun from(error: Throwable): ChangeServerError = when (error) {
|
||||
is AuthenticationException.SlidingSyncNotAvailable -> SlidingSyncAlert
|
||||
else -> InlineErrorMessage(R.string.server_selection_invalid_homeserver_error)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -48,7 +48,10 @@ class ChangeServerPresenter @Inject constructor(private val authenticationServic
|
|||
|
||||
fun handleEvents(event: ChangeServerEvents) {
|
||||
when (event) {
|
||||
is ChangeServerEvents.SetServer -> homeserver.value = event.server
|
||||
is ChangeServerEvents.SetServer -> {
|
||||
homeserver.value = event.server
|
||||
handleEvents(ChangeServerEvents.ClearError)
|
||||
}
|
||||
ChangeServerEvents.Submit -> {
|
||||
localCoroutineScope.submit(homeserver, changeServerAction)
|
||||
}
|
||||
|
|
@ -68,6 +71,6 @@ class ChangeServerPresenter @Inject constructor(private val authenticationServic
|
|||
val domain = tryOrNull { URL(homeserverUrl.value) }?.host ?: homeserverUrl.value
|
||||
authenticationService.setHomeserver(domain)
|
||||
homeserverUrl.value = domain
|
||||
}.execute(changeServerAction)
|
||||
}.execute(changeServerAction, errorMapping = ChangeServerError::from)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,5 +23,5 @@ data class ChangeServerState(
|
|||
val changeServerAction: Async<Unit>,
|
||||
val eventSink: (ChangeServerEvents) -> Unit,
|
||||
) {
|
||||
val submitEnabled = homeserver.isNotEmpty() && changeServerAction !is Async.Loading
|
||||
val submitEnabled = homeserver.isNotEmpty() && changeServerAction is Async.Uninitialized
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@
|
|||
package io.element.android.features.login.impl.changeserver
|
||||
|
||||
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
|
||||
import io.element.android.libraries.ui.strings.R
|
||||
import io.element.android.libraries.architecture.Async
|
||||
|
||||
open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerState> {
|
||||
|
|
@ -25,7 +26,11 @@ open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerStat
|
|||
aChangeServerState(),
|
||||
aChangeServerState().copy(homeserver = "matrix.org"),
|
||||
aChangeServerState().copy(homeserver = "matrix.org", changeServerAction = Async.Loading()),
|
||||
aChangeServerState().copy(homeserver = "invalid.org", changeServerAction = Async.Failure(Throwable("An error"))),
|
||||
aChangeServerState().copy(
|
||||
homeserver = "invalid.org",
|
||||
changeServerAction = Async.Failure(ChangeServerError.InlineErrorMessage(R.string.server_selection_invalid_homeserver_error))
|
||||
),
|
||||
aChangeServerState().copy(homeserver = "invalid.org", changeServerAction = Async.Failure(ChangeServerError.SlidingSyncAlert)),
|
||||
aChangeServerState().copy(homeserver = "matrix.org", changeServerAction = Async.Success(Unit)),
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -58,7 +58,6 @@ 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.impl.R
|
||||
import io.element.android.features.login.impl.error.changeServerError
|
||||
import io.element.android.features.login.impl.util.LoginConstants
|
||||
import io.element.android.libraries.architecture.Async
|
||||
import io.element.android.libraries.designsystem.ElementTextStyles
|
||||
|
|
@ -80,7 +79,6 @@ import io.element.android.libraries.designsystem.theme.components.Text
|
|||
import io.element.android.libraries.designsystem.theme.components.TextField
|
||||
import io.element.android.libraries.designsystem.theme.components.TopAppBar
|
||||
import io.element.android.libraries.designsystem.theme.components.onTabOrEnterKeyFocusNext
|
||||
import io.element.android.libraries.matrix.api.auth.AuthenticationException
|
||||
import io.element.android.libraries.testtags.TestTags
|
||||
import io.element.android.libraries.testtags.testTag
|
||||
import io.element.android.libraries.ui.strings.R as StringR
|
||||
|
|
@ -101,6 +99,8 @@ fun ChangeServerView(
|
|||
state.changeServerAction !is Async.Loading
|
||||
}
|
||||
}
|
||||
val invalidHomeserverError = (state.changeServerAction as? Async.Failure)?.error as? ChangeServerError.InlineErrorMessage
|
||||
val slidingSyncNotSupportedError = (state.changeServerAction as? Async.Failure)?.error as? ChangeServerError.SlidingSyncAlert
|
||||
val focusManager = LocalFocusManager.current
|
||||
|
||||
fun submit() {
|
||||
|
|
@ -200,53 +200,50 @@ fun ChangeServerView(
|
|||
trailingIcon = if (homeserverFieldState.isNotEmpty()) {
|
||||
{
|
||||
IconButton(onClick = {
|
||||
homeserverFieldState = ""
|
||||
eventSink(ChangeServerEvents.SetServer(""))
|
||||
}, enabled = interactionEnabled) {
|
||||
Icon(imageVector = Icons.Filled.Close, contentDescription = stringResource(StringR.string.a11y_clear))
|
||||
}
|
||||
}
|
||||
} else null,
|
||||
)
|
||||
if (state.changeServerAction is Async.Failure) {
|
||||
if (state.changeServerAction.error is AuthenticationException.SlidingSyncNotAvailable) {
|
||||
SlidingSyncNotSupportedDialog(onLearnMoreClicked = {
|
||||
onLearnMoreClicked()
|
||||
eventSink(ChangeServerEvents.ClearError)
|
||||
}, onDismiss = {
|
||||
eventSink(ChangeServerEvents.ClearError)
|
||||
})
|
||||
} else {
|
||||
ChangeServerErrorDialog(
|
||||
error = state.changeServerAction.error,
|
||||
onDismiss = {
|
||||
eventSink(ChangeServerEvents.ClearError)
|
||||
isError = invalidHomeserverError != null,
|
||||
supportingText = {
|
||||
if (invalidHomeserverError != null) {
|
||||
Text(invalidHomeserverError.message(), color = MaterialTheme.colorScheme.error)
|
||||
} else {
|
||||
val footerMessage = stringResource(StringR.string.server_selection_server_footer)
|
||||
val footerAction = stringResource(StringR.string.action_learn_more)
|
||||
val footerText = buildAnnotatedString {
|
||||
val defaultColor = MaterialTheme.colorScheme.tertiary
|
||||
withStyle(ParagraphStyle(textAlign = TextAlign.Start)) {
|
||||
withStyle(SpanStyle(color = defaultColor)) {
|
||||
append(footerMessage)
|
||||
append(" ")
|
||||
}
|
||||
val start = length
|
||||
withStyle(SpanStyle(color = LinkColor)) {
|
||||
append(footerAction)
|
||||
}
|
||||
addUrlAnnotation(UrlAnnotation(LoginConstants.SLIDING_SYNC_READ_MORE_URL), start, length)
|
||||
}
|
||||
}
|
||||
)
|
||||
}
|
||||
}
|
||||
Spacer(Modifier.height(8.dp))
|
||||
val footerMessage = stringResource(StringR.string.server_selection_server_footer)
|
||||
val footerAction = stringResource(StringR.string.server_selection_server_footer_action)
|
||||
val footerText = buildAnnotatedString {
|
||||
val defaultColor = MaterialTheme.colorScheme.tertiary
|
||||
withStyle(ParagraphStyle(textAlign = TextAlign.Start)) {
|
||||
withStyle(SpanStyle(color = defaultColor)) {
|
||||
append(footerMessage)
|
||||
append(" ")
|
||||
ClickableLinkText(
|
||||
text = footerText,
|
||||
interactionSource = MutableInteractionSource(),
|
||||
style = ElementTextStyles.Regular.caption1,
|
||||
)
|
||||
}
|
||||
val start = length
|
||||
withStyle(SpanStyle(color = LinkColor)) {
|
||||
append(footerAction)
|
||||
}
|
||||
addUrlAnnotation(UrlAnnotation(LoginConstants.SLIDING_SYNC_READ_MORE_URL), start, length)
|
||||
}
|
||||
}
|
||||
ClickableLinkText(
|
||||
text = footerText,
|
||||
interactionSource = MutableInteractionSource(),
|
||||
modifier = Modifier.padding(horizontal = 16.dp),
|
||||
style = ElementTextStyles.Regular.caption1,
|
||||
|
||||
)
|
||||
if (slidingSyncNotSupportedError != null) {
|
||||
SlidingSyncNotSupportedDialog(onLearnMoreClicked = {
|
||||
onLearnMoreClicked()
|
||||
eventSink(ChangeServerEvents.ClearError)
|
||||
}, onDismiss = {
|
||||
eventSink(ChangeServerEvents.ClearError)
|
||||
})
|
||||
}
|
||||
Spacer(Modifier.height(32.dp))
|
||||
Button(
|
||||
onClick = ::submit,
|
||||
|
|
@ -271,9 +268,10 @@ fun ChangeServerView(
|
|||
}
|
||||
|
||||
@Composable
|
||||
internal fun ChangeServerErrorDialog(error: Throwable, onDismiss: () -> Unit) {
|
||||
internal fun ChangeServerErrorDialog(title: String, message: String, onDismiss: () -> Unit) {
|
||||
ErrorDialog(
|
||||
content = stringResource(changeServerError(error)),
|
||||
title = title,
|
||||
content = message,
|
||||
onDismiss = onDismiss
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,13 +31,3 @@ fun loginError(
|
|||
AuthErrorCode.UNKNOWN -> StringR.unknown_error
|
||||
}
|
||||
}
|
||||
|
||||
fun changeServerError(
|
||||
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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -95,7 +95,7 @@ class ChangeServerPresenterTest {
|
|||
assertThat(loadingState.submitEnabled).isFalse()
|
||||
assertThat(loadingState.changeServerAction).isInstanceOf(Async.Loading::class.java)
|
||||
val successState = awaitItem()
|
||||
assertThat(successState.submitEnabled).isTrue()
|
||||
assertThat(successState.submitEnabled).isFalse()
|
||||
assertThat(successState.changeServerAction).isInstanceOf(Async.Success::class.java)
|
||||
}
|
||||
}
|
||||
|
|
@ -118,7 +118,7 @@ class ChangeServerPresenterTest {
|
|||
assertThat(loadingState.changeServerAction).isInstanceOf(Async.Loading::class.java)
|
||||
awaitItem() // Skip changing the url to the parsed domain
|
||||
val successState = awaitItem()
|
||||
assertThat(successState.submitEnabled).isTrue()
|
||||
assertThat(successState.submitEnabled).isFalse()
|
||||
assertThat(successState.changeServerAction).isInstanceOf(Async.Success::class.java)
|
||||
assertThat(successState.homeserver).isEqualTo("matrix.org")
|
||||
}
|
||||
|
|
@ -135,7 +135,7 @@ class ChangeServerPresenterTest {
|
|||
authServer.givenChangeServerError(Throwable())
|
||||
initialState.eventSink.invoke(ChangeServerEvents.Submit)
|
||||
val failureState = awaitItem()
|
||||
assertThat(failureState.submitEnabled).isTrue()
|
||||
assertThat(failureState.submitEnabled).isFalse()
|
||||
assertThat(failureState.changeServerAction).isInstanceOf(Async.Failure::class.java)
|
||||
}
|
||||
}
|
||||
|
|
@ -157,7 +157,7 @@ class ChangeServerPresenterTest {
|
|||
|
||||
// Check an error was returned
|
||||
val submittedState = awaitItem()
|
||||
assertThat(submittedState.changeServerAction).isEqualTo(Async.Failure<Unit>(A_THROWABLE))
|
||||
assertThat(submittedState.changeServerAction).isInstanceOf(Async.Failure::class.java)
|
||||
|
||||
// Assert the error is then cleared
|
||||
submittedState.eventSink(ChangeServerEvents.ClearError)
|
||||
|
|
|
|||
|
|
@ -55,44 +55,4 @@ class ErrorFormatterTests {
|
|||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue