Skip to content

Commit 77bc9b8

Browse files
bmartybmarty
authored
Merge pull request #5692 from element-hq/feature/bma/loginFlow
Improve account provider selection during the login flow
2 parents 80196c2 + fe2a317 commit 77bc9b8

File tree

36 files changed

+366
-254
lines changed

36 files changed

+366
-254
lines changed

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,12 @@ class ChangeServerPresenter(
6060
title = data.title,
6161
accountProviderUrl = data.url,
6262
)
63-
authenticationService.setHomeserver(data.url).map {
64-
authenticationService.getHomeserverDetails().value!!
65-
// Valid, remember user choice
66-
accountProviderDataSource.userSelection(data)
67-
}.getOrThrow()
63+
val details = authenticationService.setHomeserver(data.url).getOrThrow()
64+
if (!details.isSupported) {
65+
throw ChangeServerError.UnsupportedServer
66+
}
67+
// Homeserver is valid, remember user choice
68+
accountProviderDataSource.userSelection(data)
6869
}.runCatchingUpdatingState(changeServerAction, errorTransform = ChangeServerError::from)
6970
}
7071
}

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,12 @@ package io.element.android.features.login.impl.changeserver
1010
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
1111
import io.element.android.features.login.impl.error.ChangeServerError
1212
import io.element.android.libraries.architecture.AsyncData
13-
import io.element.android.libraries.ui.strings.CommonStrings
1413

1514
open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerState> {
1615
override val values: Sequence<ChangeServerState>
1716
get() = sequenceOf(
1817
aChangeServerState(),
19-
aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.Error(CommonStrings.error_unknown))),
18+
aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.Error(null))),
2019
aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.SlidingSyncAlert)),
2120
aChangeServerState(
2221
changeServerAction = AsyncData.Failure(
@@ -34,6 +33,11 @@ open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerStat
3433
),
3534
)
3635
),
36+
aChangeServerState(
37+
changeServerAction = AsyncData.Failure(
38+
ChangeServerError.UnsupportedServer
39+
)
40+
),
3741
)
3842
}
3943

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
2626
import io.element.android.libraries.designsystem.preview.ElementPreview
2727
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
2828
import io.element.android.libraries.designsystem.theme.LocalBuildMeta
29+
import io.element.android.libraries.ui.strings.CommonStrings
2930

3031
@Composable
3132
fun ChangeServerView(
@@ -39,10 +40,26 @@ fun ChangeServerView(
3940
when (state.changeServerAction) {
4041
is AsyncData.Failure -> {
4142
when (val error = state.changeServerAction.error as? ChangeServerError) {
43+
ChangeServerError.InvalidServer ->
44+
ErrorDialog(
45+
modifier = modifier,
46+
content = stringResource(R.string.screen_change_server_error_invalid_homeserver),
47+
onSubmit = {
48+
eventSink.invoke(ChangeServerEvents.ClearError)
49+
}
50+
)
51+
ChangeServerError.UnsupportedServer ->
52+
ErrorDialog(
53+
modifier = modifier,
54+
content = stringResource(R.string.screen_login_error_unsupported_authentication),
55+
onSubmit = {
56+
eventSink.invoke(ChangeServerEvents.ClearError)
57+
}
58+
)
4259
is ChangeServerError.Error -> {
4360
ErrorDialog(
4461
modifier = modifier,
45-
content = error.message(),
62+
content = error.messageStr ?: stringResource(CommonStrings.error_unknown),
4663
onSubmit = {
4764
eventSink.invoke(ChangeServerEvents.ClearError)
4865
}

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,13 @@
77

88
package io.element.android.features.login.impl.error
99

10-
import androidx.annotation.StringRes
11-
import androidx.compose.runtime.Composable
12-
import androidx.compose.runtime.ReadOnlyComposable
13-
import androidx.compose.ui.res.stringResource
14-
import io.element.android.features.login.impl.R
1510
import io.element.android.features.login.impl.changeserver.AccountProviderAccessException
1611
import io.element.android.libraries.matrix.api.auth.AuthenticationException
17-
import io.element.android.libraries.ui.strings.CommonStrings
1812

1913
sealed class ChangeServerError : Exception() {
2014
data class Error(
21-
@StringRes val messageId: Int? = null,
2215
val messageStr: String? = null,
23-
) : ChangeServerError() {
24-
@Composable
25-
@ReadOnlyComposable
26-
fun message(): String = messageStr ?: stringResource(messageId ?: CommonStrings.error_unknown)
27-
}
16+
) : ChangeServerError()
2817

2918
data class NeedElementPro(
3019
val unauthorisedAccountProviderTitle: String,
@@ -37,11 +26,23 @@ sealed class ChangeServerError : Exception() {
3726
) : ChangeServerError()
3827

3928
data object SlidingSyncAlert : ChangeServerError()
29+
data object InvalidServer : ChangeServerError()
30+
data object UnsupportedServer : ChangeServerError()
4031

4132
companion object {
4233
fun from(error: Throwable): ChangeServerError = when (error) {
43-
is AuthenticationException.SlidingSyncVersion -> SlidingSyncAlert
44-
is AuthenticationException.Oidc -> Error(messageStr = error.message)
34+
is ChangeServerError -> error
35+
is AuthenticationException -> {
36+
when (error) {
37+
is AuthenticationException.SlidingSyncVersion -> SlidingSyncAlert
38+
is AuthenticationException.InvalidServerName,
39+
is AuthenticationException.ServerUnreachable -> InvalidServer
40+
// AccountAlreadyLoggedIn error should not happen at this point
41+
is AuthenticationException.AccountAlreadyLoggedIn -> Error(messageStr = error.message)
42+
is AuthenticationException.Generic -> Error(messageStr = error.message)
43+
is AuthenticationException.Oidc -> Error(messageStr = error.message)
44+
}
45+
}
4546
is AccountProviderAccessException.NeedElementProException -> NeedElementPro(
4647
unauthorisedAccountProviderTitle = error.unauthorisedAccountProviderTitle,
4748
applicationId = error.applicationId,
@@ -50,7 +51,7 @@ sealed class ChangeServerError : Exception() {
5051
unauthorisedAccountProviderTitle = error.unauthorisedAccountProviderTitle,
5152
authorisedAccountProviderTitles = error.authorisedAccountProviderTitles,
5253
)
53-
else -> Error(messageId = R.string.screen_change_server_error_invalid_homeserver)
54+
else -> Error(messageStr = error.message)
5455
}
5556
}
5657
}

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerErrorProvider.kt‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,11 @@
88
package io.element.android.features.login.impl.error
99

1010
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
11-
import io.element.android.features.login.impl.R
1211

1312
class ChangeServerErrorProvider : PreviewParameterProvider<ChangeServerError> {
1413
override val values: Sequence<ChangeServerError>
1514
get() = sequenceOf(
16-
ChangeServerError.Error(
17-
messageId = R.string.screen_change_server_error_invalid_homeserver,
18-
),
15+
ChangeServerError.InvalidServer,
1916
ChangeServerError.Error(
2017
messageStr = "An error description",
2118
),
@@ -28,5 +25,6 @@ class ChangeServerErrorProvider : PreviewParameterProvider<ChangeServerError> {
2825
authorisedAccountProviderTitles = listOf("provider.org", "provider.io"),
2926
),
3027
ChangeServerError.SlidingSyncAlert,
28+
ChangeServerError.UnsupportedServer,
3129
)
3230
}

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginHelper.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ class LoginHelper(
6565
loginHint: String?,
6666
) = coroutineScope.launch {
6767
suspend {
68-
authenticationService.setHomeserver(homeserverUrl).map {
69-
val matrixHomeServerDetails = authenticationService.getHomeserverDetails().value!!
68+
authenticationService.setHomeserver(homeserverUrl).map { matrixHomeServerDetails ->
7069
if (matrixHomeServerDetails.supportsOidcLogin) {
7170
// Retrieve the details right now
7271
val oidcPrompt = if (isAccountCreation) OidcPrompt.Create else OidcPrompt.Login

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,20 @@ fun LoginModeView(
4141
when (val error = loginMode.error) {
4242
is ChangeServerError -> {
4343
when (error) {
44+
ChangeServerError.InvalidServer ->
45+
ErrorDialog(
46+
content = stringResource(R.string.screen_change_server_error_invalid_homeserver),
47+
onSubmit = onClearError,
48+
)
49+
is ChangeServerError.UnsupportedServer -> {
50+
ErrorDialog(
51+
content = stringResource(R.string.screen_login_error_unsupported_authentication),
52+
onSubmit = onClearError,
53+
)
54+
}
4455
is ChangeServerError.Error -> {
4556
ErrorDialog(
46-
content = error.message(),
57+
content = error.messageStr ?: stringResource(CommonStrings.error_unknown),
4758
onSubmit = onClearError,
4859
)
4960
}
@@ -91,7 +102,7 @@ fun LoginModeView(
91102
}
92103
is AuthenticationException.AccountAlreadyLoggedIn -> {
93104
ErrorDialog(
94-
content = stringResource(CommonStrings.error_account_already_logged_in, error.message.orEmpty()),
105+
content = stringResource(CommonStrings.error_account_already_logged_in, error.userId),
95106
onSubmit = onClearError,
96107
)
97108
}

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/resolver/HomeserverResolver.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,10 @@ class HomeserverResolver(
5252
}
5353
}
5454
}
55-
// If list is empty, and the user has entered an URL, do not block the user.
56-
if (currentList.isEmpty() && trimmedUserInput.isValidUrl()) {
57-
emit(listOf(HomeserverData(homeserverUrl = trimmedUserInput)))
55+
// If list is empty, and candidateBase is a valid an URL, do not block the user.
56+
// A unsupported homeserver / homeserver not found error will be displayed if the user continues
57+
if (currentList.isEmpty() && candidateBase.isValidUrl()) {
58+
emit(listOf(HomeserverData(homeserverUrl = candidateBase)))
5859
}
5960
}
6061

‎features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderStateProvider.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,5 @@ fun aHomeserverDataList(): List<HomeserverData> {
4343
fun aHomeserverData(
4444
homeserverUrl: String = AuthenticationConfig.MATRIX_ORG_URL,
4545
): HomeserverData {
46-
return HomeserverData(homeserverUrl = homeserverUrl,)
46+
return HomeserverData(homeserverUrl = homeserverUrl)
4747
}

‎features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ import io.element.android.features.wellknown.test.FakeWellknownRetriever
1818
import io.element.android.features.wellknown.test.anElementWellKnown
1919
import io.element.android.libraries.architecture.AsyncData
2020
import io.element.android.libraries.core.uri.ensureProtocol
21-
import io.element.android.libraries.matrix.test.A_HOMESERVER
21+
import io.element.android.libraries.matrix.test.AN_EXCEPTION
2222
import io.element.android.libraries.matrix.test.A_HOMESERVER_URL
2323
import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService
24+
import io.element.android.libraries.matrix.test.auth.aMatrixHomeServerDetails
2425
import io.element.android.libraries.wellknown.api.ElementWellKnown
2526
import io.element.android.libraries.wellknown.api.WellknownRetriever
2627
import io.element.android.libraries.wellknown.api.WellknownRetrieverResult
@@ -46,7 +47,11 @@ class ChangeServerPresenterTest {
4647

4748
@Test
4849
fun `present - change server ok`() = runTest {
49-
val authenticationService = FakeMatrixAuthenticationService()
50+
val authenticationService = FakeMatrixAuthenticationService(
51+
setHomeserverResult = {
52+
Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true))
53+
},
54+
)
5055
createPresenter(
5156
authenticationService = authenticationService,
5257
enterpriseService = FakeEnterpriseService(
@@ -55,7 +60,6 @@ class ChangeServerPresenterTest {
5560
).test {
5661
val initialState = awaitItem()
5762
assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized)
58-
authenticationService.givenHomeserver(A_HOMESERVER)
5963
initialState.eventSink.invoke(ChangeServerEvents.ChangeServer(AccountProvider(url = A_HOMESERVER_URL)))
6064
val loadingState = awaitItem()
6165
assertThat(loadingState.changeServerAction).isInstanceOf(AsyncData.Loading::class.java)
@@ -66,10 +70,16 @@ class ChangeServerPresenterTest {
6670

6771
@Test
6872
fun `present - change server error`() = runTest {
73+
val authenticationService = FakeMatrixAuthenticationService(
74+
setHomeserverResult = {
75+
Result.failure(AN_EXCEPTION)
76+
},
77+
)
6978
createPresenter(
7079
enterpriseService = FakeEnterpriseService(
7180
isAllowedToConnectToHomeserverResult = { true },
7281
),
82+
authenticationService = authenticationService,
7383
).test {
7484
val initialState = awaitItem()
7585
assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized)
@@ -85,6 +95,32 @@ class ChangeServerPresenterTest {
8595
}
8696
}
8797

98+
@Test
99+
fun `present - change server unsupported server`() = runTest {
100+
val authenticationService = FakeMatrixAuthenticationService(
101+
setHomeserverResult = {
102+
Result.success(aMatrixHomeServerDetails())
103+
},
104+
)
105+
createPresenter(
106+
enterpriseService = FakeEnterpriseService(
107+
isAllowedToConnectToHomeserverResult = { true },
108+
),
109+
authenticationService = authenticationService,
110+
).test {
111+
val initialState = awaitItem()
112+
assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized)
113+
initialState.eventSink.invoke(ChangeServerEvents.ChangeServer(AccountProvider(url = A_HOMESERVER_URL)))
114+
val loadingState = awaitItem()
115+
assertThat(loadingState.changeServerAction).isInstanceOf(AsyncData.Loading::class.java)
116+
val failureState = awaitItem()
117+
assertThat(failureState.changeServerAction).isInstanceOf(AsyncData.Failure::class.java)
118+
assertThat(failureState.changeServerAction.errorOrNull()).isEqualTo(
119+
ChangeServerError.UnsupportedServer
120+
)
121+
}
122+
}
123+
88124
@Test
89125
fun `present - change server not allowed error`() = runTest {
90126
val isAllowedToConnectToHomeserverResult = lambdaRecorder<String, Boolean> { false }

0 commit comments

Comments
 (0)