mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-10-31 16:22:49 +00:00
Display an error in the room select screen if the selected account has an invalid token
All the accounts are proposed in the account switch, even the ones with an invalid token, so that the user can still see them. If the selected account has an invalid token, or is invalidated while the screen is displayed (for instance signed out from another device), the error is displayed instead of the room list, the selected rooms are cleared, and the client of this account is not used anymore. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+25
-8
@@ -38,7 +38,6 @@ import kotlinx.collections.immutable.ImmutableList
|
||||
import kotlinx.collections.immutable.persistentListOf
|
||||
import kotlinx.collections.immutable.toImmutableList
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.flow.map
|
||||
import kotlinx.coroutines.launch
|
||||
import timber.log.Timber
|
||||
|
||||
@@ -67,11 +66,24 @@ class RoomSelectPresenter(
|
||||
var isSearchActive by remember { mutableStateOf(false) }
|
||||
var isAccountListExpanded by remember { mutableStateOf(false) }
|
||||
var selectedSessionId by remember { mutableStateOf(initialSessionId) }
|
||||
val sessions by remember { sessionStore.sessionsFlow() }.collectAsState(initial = emptyList())
|
||||
// All the accounts are proposed, even the ones with an invalid token. Selecting them displays an error.
|
||||
val accounts by remember {
|
||||
sessionStore.sessionsFlow().map { list ->
|
||||
list.map { it.toMatrixUser() }
|
||||
derivedStateOf { sessions.map { it.toMatrixUser() } }
|
||||
}
|
||||
// The selected session can have an invalid token, or be invalidated while this screen is displayed, for instance if it is
|
||||
// signed out from another device. The invalidation of the initial session, which is the current one, is handled by the application.
|
||||
val isSelectedSessionInvalid by remember {
|
||||
derivedStateOf {
|
||||
selectedSessionId != initialSessionId && sessions.none { it.userId == selectedSessionId.value && it.isTokenValid }
|
||||
}
|
||||
}.collectAsState(initial = emptyList())
|
||||
}
|
||||
LaunchedEffect(isSelectedSessionInvalid) {
|
||||
if (isSelectedSessionInvalid) {
|
||||
// The rooms of an invalid session cannot be used
|
||||
selectedRooms = persistentListOf()
|
||||
}
|
||||
}
|
||||
val selectedAccount by remember {
|
||||
derivedStateOf {
|
||||
// Fallback to a MatrixUser with only the userId until the sessions are loaded
|
||||
@@ -87,12 +99,14 @@ class RoomSelectPresenter(
|
||||
val coroutineScope = rememberCoroutineScope()
|
||||
// Create one data source per session. When the selected session changes, the previous
|
||||
// data source is removed from the composition and its coroutine scope is cancelled.
|
||||
val (dataSource, hasRoomListError) = key(selectedSessionId) {
|
||||
val (selectedSessionDataSource, hasClientError) = key(selectedSessionId) {
|
||||
val dataSourceCoroutineScope = rememberCoroutineScope()
|
||||
// The sessions are restored at startup, so the client should already be in memory.
|
||||
// A null value means that the client is being restored.
|
||||
val roomListServiceResult by produceState(matrixClientProvider.getOrNull(selectedSessionId)?.roomListService?.let { Result.success(it) }) {
|
||||
if (value == null) {
|
||||
// A null value means that the client is being restored, or that the session is invalid: its client cannot be restored,
|
||||
// and an in-memory client would have been destroyed, so do not try to get it.
|
||||
val initialRoomListService = if (isSelectedSessionInvalid) null else matrixClientProvider.getOrNull(selectedSessionId)?.roomListService
|
||||
val roomListServiceResult by produceState(initialRoomListService?.let { Result.success(it) }) {
|
||||
if (value == null && !isSelectedSessionInvalid) {
|
||||
value = matrixClientProvider.getOrRestore(selectedSessionId)
|
||||
.onFailure { Timber.e(it, "Failed to get the client for $selectedSessionId") }
|
||||
.map { it.roomListService }
|
||||
@@ -104,6 +118,9 @@ class RoomSelectPresenter(
|
||||
}
|
||||
dataSource to (roomListServiceResult?.isFailure == true)
|
||||
}
|
||||
// Do not use the data source of an invalid session, its client has been destroyed
|
||||
val dataSource = selectedSessionDataSource.takeIf { !isSelectedSessionInvalid }
|
||||
val hasRoomListError = hasClientError || isSelectedSessionInvalid
|
||||
|
||||
val searchQuery = queryState.text.toString()
|
||||
LaunchedEffect(dataSource, searchQuery) {
|
||||
|
||||
+90
-2
@@ -9,6 +9,7 @@
|
||||
package io.element.android.libraries.roomselect.impl
|
||||
|
||||
import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd
|
||||
import app.cash.turbine.Event
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.libraries.designsystem.theme.components.SearchBarResultState
|
||||
import io.element.android.libraries.matrix.api.MatrixClientProvider
|
||||
@@ -35,6 +36,7 @@ import io.element.android.tests.testutils.WarmUpRule
|
||||
import io.element.android.tests.testutils.awaitLastSequentialItem
|
||||
import io.element.android.tests.testutils.consumeItemsUntilPredicate
|
||||
import io.element.android.tests.testutils.lambda.assert
|
||||
import io.element.android.tests.testutils.lambda.lambdaError
|
||||
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
||||
import io.element.android.tests.testutils.test
|
||||
import io.element.android.tests.testutils.testCoroutineDispatchers
|
||||
@@ -141,6 +143,92 @@ class RoomSelectPresenterTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - accounts with an invalid token are displayed, selecting one displays an error`() = runTest {
|
||||
val presenter = createRoomSelectPresenter(
|
||||
mode = RoomSelectMode.Share,
|
||||
sessionStore = InMemorySessionStore(
|
||||
initialList = listOf(
|
||||
aSessionData(sessionId = A_SESSION_ID.value, userDisplayName = "Alice", isTokenValid = true),
|
||||
aSessionData(sessionId = A_SESSION_ID_2.value, userDisplayName = "Bob", isTokenValid = false),
|
||||
),
|
||||
),
|
||||
// The client of the invalid session must not be requested
|
||||
matrixClientProvider = FakeMatrixClientProvider { sessionId ->
|
||||
if (sessionId == A_SESSION_ID) Result.success(FakeMatrixClient(sessionId = sessionId)) else lambdaError()
|
||||
},
|
||||
)
|
||||
presenter.test {
|
||||
val state = awaitLastSequentialItem()
|
||||
assertThat(state.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2)
|
||||
assertThat(state.hasRoomListError).isFalse()
|
||||
state.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID_2))
|
||||
val errorState = consumeItemsUntilPredicate { it.selectedAccount.userId == A_SESSION_ID_2 && it.hasRoomListError }.last()
|
||||
// The account information is still available
|
||||
assertThat(errorState.selectedAccount.displayName).isEqualTo("Bob")
|
||||
assertThat(errorState.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - the selected account is invalidated, an error is displayed and the selected rooms are cleared`() = runTest {
|
||||
val roomSummary2 = aRoomSummary(roomId = A_ROOM_ID_2)
|
||||
val roomListServices = mapOf(
|
||||
A_SESSION_ID to FakeRoomListService(createRoomListLambda = { FakeDynamicRoomList(summaries = MutableStateFlow(listOf(aRoomSummary()))) }),
|
||||
A_SESSION_ID_2 to FakeRoomListService(createRoomListLambda = { FakeDynamicRoomList(summaries = MutableStateFlow(listOf(roomSummary2))) }),
|
||||
)
|
||||
val sessionStore = aSessionStoreWithTwoAccounts()
|
||||
val presenter = createRoomSelectPresenter(
|
||||
mode = RoomSelectMode.Share,
|
||||
sessionStore = sessionStore,
|
||||
matrixClientProvider = FakeMatrixClientProvider { sessionId ->
|
||||
Result.success(FakeMatrixClient(sessionId = sessionId, roomListService = roomListServices.getValue(sessionId)))
|
||||
},
|
||||
)
|
||||
presenter.test {
|
||||
val state = awaitLastSequentialItem()
|
||||
state.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID_2))
|
||||
val otherAccountState = consumeItemsUntilPredicate {
|
||||
it.selectedAccount.userId == A_SESSION_ID_2 &&
|
||||
(it.resultState as? SearchBarResultState.Results)?.results?.map { room -> room.roomId } == listOf(A_ROOM_ID_2)
|
||||
}.last()
|
||||
otherAccountState.eventSink(RoomSelectEvent.ToggleSelectedRoom(roomSummary2.toSelectRoomInfo()))
|
||||
assertThat(consumeItemsUntilPredicate { it.selectedRooms.isNotEmpty() }.last().selectedRooms).hasSize(1)
|
||||
// The session of the selected account is invalidated, for instance signed out from another device
|
||||
sessionStore.updateData(sessionStore.getSession(A_SESSION_ID_2.value)!!.copy(isTokenValid = false))
|
||||
val errorState = consumeItemsUntilPredicate { it.hasRoomListError && it.selectedRooms.isEmpty() }.last()
|
||||
assertThat(errorState.selectedAccount.userId).isEqualTo(A_SESSION_ID_2)
|
||||
assertThat(errorState.selectedAccount.displayName).isEqualTo("Bob")
|
||||
assertThat(errorState.resultState).isInstanceOf(SearchBarResultState.Initial::class.java)
|
||||
// Go back to the current account: the invalid account is still proposed
|
||||
errorState.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID))
|
||||
val finalState = consumeItemsUntilPredicate { it.selectedAccount.userId == A_SESSION_ID && !it.hasRoomListError }.last()
|
||||
assertThat(finalState.showAccountSwitch).isTrue()
|
||||
assertThat(finalState.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - another account is invalidated, it is still proposed and there is no error`() = runTest {
|
||||
val sessionStore = aSessionStoreWithTwoAccounts()
|
||||
val presenter = createRoomSelectPresenter(
|
||||
mode = RoomSelectMode.Share,
|
||||
sessionStore = sessionStore,
|
||||
)
|
||||
presenter.test {
|
||||
val state = awaitLastSequentialItem()
|
||||
assertThat(state.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2)
|
||||
sessionStore.updateData(sessionStore.getSession(A_SESSION_ID_2.value)!!.copy(isTokenValid = false))
|
||||
testScheduler.advanceUntilIdle()
|
||||
// The state may not change at all
|
||||
val finalState = cancelAndConsumeRemainingEvents().filterIsInstance<Event.Item<RoomSelectState>>().lastOrNull()?.value ?: state
|
||||
assertThat(finalState.selectedAccount.userId).isEqualTo(A_SESSION_ID)
|
||||
assertThat(finalState.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2)
|
||||
assertThat(finalState.hasRoomListError).isFalse()
|
||||
assertThat(finalState.showAccountSwitch).isTrue()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - select another account displays the rooms of this account`() = runTest {
|
||||
val roomSummary1 = aRoomSummary(roomId = A_ROOM_ID)
|
||||
@@ -191,8 +279,8 @@ class RoomSelectPresenterTest {
|
||||
|
||||
private fun aSessionStoreWithTwoAccounts() = InMemorySessionStore(
|
||||
initialList = listOf(
|
||||
aSessionData(sessionId = A_SESSION_ID.value, userDisplayName = "Alice"),
|
||||
aSessionData(sessionId = A_SESSION_ID_2.value, userDisplayName = "Bob"),
|
||||
aSessionData(sessionId = A_SESSION_ID.value, userDisplayName = "Alice", isTokenValid = true),
|
||||
aSessionData(sessionId = A_SESSION_ID_2.value, userDisplayName = "Bob", isTokenValid = true),
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user