From 9bc7e751c46c5d20d63245386d0e451762eac12f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 7 Oct 2026 22:00:12 +0200 Subject: [PATCH 1/2] Keep the incoming share displayed when the current account is signed out The incoming share was displayed in the logged in flow of the current session, so it was destroyed when the current session was signed out, for instance from another device. When another session was signed out, the room select screen was kept and displayed an error if this session was selected. To make the behavior consistent: - The incoming share is now displayed by the RootFlowNode, on top of the logged in flow, and does not depend on a session anymore. The latest used session is passed from the RootFlowNode to the room select screen, through the share flow, and is used as its initial session. - When the root of the RootFlowNode backstack changes while an incoming share is displayed, the new root is set below the incoming share, using the new replaceRoot backstack operation, instead of removing all the elements. - The room select screen handles the invalidation of the current session like for any other session. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../io/element/android/appnav/BackstackExt.kt | 48 +++++++++++++ .../appnav/LoggedInAppScopeFlowNode.kt | 7 -- .../android/appnav/LoggedInFlowNode.kt | 49 ------------- .../io/element/android/appnav/RootFlowNode.kt | 70 +++++++++++++++---- .../android/appnav/BackstackExtTest.kt | 56 +++++++++++++++ .../android/appnav/RootFlowNodeTest.kt | 11 +++ .../features/share/api/ShareEntryPoint.kt | 18 ++++- .../share/impl/DefaultShareEntryPoint.kt | 9 ++- .../android/features/share/impl/ShareNode.kt | 18 ++--- .../features/share/impl/SharePresenter.kt | 15 ++-- .../android/features/share/impl/ShareState.kt | 13 +++- .../share/impl/ShareStatePreviewParam.kt | 18 +++-- .../android/features/share/impl/ShareView.kt | 3 +- .../share/impl/DefaultShareEntryPointTest.kt | 7 +- .../features/share/impl/SharePresenterTest.kt | 20 ++---- .../roomselect/impl/RoomSelectPresenter.kt | 50 +++++++------ .../impl/DefaultRoomSelectEntryPointTest.kt | 2 +- .../impl/RoomSelectPresenterTest.kt | 35 +++++++++- 18 files changed, 299 insertions(+), 150 deletions(-) create mode 100644 appnav/src/test/kotlin/io/element/android/appnav/BackstackExtTest.kt diff --git a/appnav/src/main/kotlin/io/element/android/appnav/BackstackExt.kt b/appnav/src/main/kotlin/io/element/android/appnav/BackstackExt.kt index 1185016c0b..1b9d1b56cb 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/BackstackExt.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/BackstackExt.kt @@ -8,9 +8,15 @@ package io.element.android.appnav +import com.bumble.appyx.core.navigation.NavKey import com.bumble.appyx.navmodel.backstack.BackStack +import com.bumble.appyx.navmodel.backstack.BackStackElement +import com.bumble.appyx.navmodel.backstack.BackStackElements +import com.bumble.appyx.navmodel.backstack.operation.BackStackOperation import com.bumble.appyx.navmodel.backstack.operation.NewRoot import com.bumble.appyx.navmodel.backstack.operation.Remove +import kotlinx.parcelize.Parcelize +import kotlinx.parcelize.RawValue /** * Don't process NewRoot if the nav target already exists in the stack. @@ -32,3 +38,45 @@ fun BackStack.removeLast(element: T) { } ?: return accept(Remove(lastExpectedNavElement.key)) } + +/** + * Replace the root element of the backstack, and keep the elements on top of it, contrary to [safeRoot] which removes all the elements. + */ +fun BackStack.replaceRoot(element: T) { + accept(ReplaceRoot(element)) +} + +/** + * Replace the root element of the backstack, keeping the elements on top of it. + * + * Operation: + * + * [A, B, C] + ReplaceRoot(D) = [D, B, C] + * [A] + ReplaceRoot(D) = [D] + */ +@Parcelize +internal data class ReplaceRoot( + private val element: @RawValue T +) : BackStackOperation { + override fun isApplicable(elements: BackStackElements): Boolean = + elements.rootElement()?.key?.navTarget != element + + override fun invoke(elements: BackStackElements): BackStackElements { + val root = elements.rootElement() ?: return elements + if (root.targetState == BackStack.State.ACTIVE) { + // Nothing on top of the root, so this is the same as NewRoot + return NewRoot(element).invoke(elements) + } + // The root is stashed below other elements: drop it, as Remove does for a non active element, + // and add the new root, stashed, in place of it. + val newRoot = BackStackElement( + key = NavKey(element), + fromState = BackStack.State.STASHED, + targetState = BackStack.State.STASHED, + operation = this, + ) + return elements.map { if (it == root) newRoot else it } + } + + private fun BackStackElements.rootElement() = firstOrNull { it.targetState != BackStack.State.DESTROYED } +} diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInAppScopeFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInAppScopeFlowNode.kt index 32e1723d99..ee8398186d 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInAppScopeFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInAppScopeFlowNode.kt @@ -33,8 +33,6 @@ import io.element.android.libraries.architecture.createNode import io.element.android.libraries.architecture.inputs import io.element.android.libraries.di.DependencyInjectionGraphOwner import io.element.android.libraries.matrix.api.MatrixClient -import io.element.android.libraries.matrix.api.core.RoomId -import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrixmedia.api.ImageLoaderHolder import kotlinx.parcelize.Parcelize @@ -61,7 +59,6 @@ class LoggedInAppScopeFlowNode( interface Callback : Plugin { fun navigateToBugReport() fun navigateToAddAccount() - fun switchAccountAndOpenRoom(sessionId: SessionId, roomId: RoomId?) } private val callback: Callback = callback() @@ -94,10 +91,6 @@ class LoggedInAppScopeFlowNode( override fun navigateToAddAccount() { callback.navigateToAddAccount() } - - override fun switchAccountAndOpenRoom(sessionId: SessionId, roomId: RoomId?) { - callback.switchAccountAndOpenRoom(sessionId, roomId) - } } return createNode(buildContext, listOf(callback)) } diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index a361576050..270ea6b052 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -67,8 +67,6 @@ import io.element.android.features.preferences.api.PreferencesEntryPoint import io.element.android.features.roomdirectory.api.RoomDescription import io.element.android.features.roomdirectory.api.RoomDirectoryEntryPoint import io.element.android.features.securebackup.api.SecureBackupEntryPoint -import io.element.android.features.share.api.ShareEntryPoint -import io.element.android.features.share.api.ShareIntentData import io.element.android.features.startchat.api.StartChatEntryPoint import io.element.android.features.userprofile.api.UserProfileEntryPoint import io.element.android.features.verifysession.api.IncomingVerificationEntryPoint @@ -88,7 +86,6 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.RoomIdOrAlias -import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.core.toRoomIdOrAlias import io.element.android.libraries.matrix.api.permalink.PermalinkData @@ -138,7 +135,6 @@ class LoggedInFlowNode( private val sessionCoroutineScope: CoroutineScope, private val ftueService: FtueService, private val roomDirectoryEntryPoint: RoomDirectoryEntryPoint, - private val shareEntryPoint: ShareEntryPoint, private val matrixClient: MatrixClient, private val sendingQueue: SendQueues, private val nativeCallHost: NativeCallHost, @@ -174,7 +170,6 @@ class LoggedInFlowNode( interface Callback : Plugin { fun navigateToBugReport() fun navigateToAddAccount() - fun switchAccountAndOpenRoom(sessionId: SessionId, roomId: RoomId?) } private val callback: Callback = callback() @@ -296,9 +291,6 @@ class LoggedInFlowNode( @Parcelize data object RoomDirectory : NavTarget - @Parcelize - data class IncomingShare(val shareIntentData: ShareIntentData) : NavTarget - @Parcelize data class IncomingVerificationRequest(val data: VerificationRequest.Incoming) : NavTarget } @@ -570,36 +562,6 @@ class LoggedInFlowNode( }, ) } - is NavTarget.IncomingShare -> { - shareEntryPoint.createNode( - parentNode = this, - buildContext = buildContext, - params = ShareEntryPoint.Params(shareIntentData = navTarget.shareIntentData), - callback = object : ShareEntryPoint.Callback { - override fun onDone(sessionId: SessionId, roomIds: List) { - // Remove the incoming share screen - backstack.pop() - - if (sessionId != matrixClient.sessionId) { - // The data has been shared using another session, switch to it - callback.switchAccountAndOpenRoom(sessionId, roomIds.singleOrNull()) - return - } - - // Navigate to the room if the text/media was shared to a single one - roomIds.singleOrNull()?.let { roomId -> - lifecycleScope.launch { - // Wait until the incoming share screen is removed - backstack.elements.first { it.lastOrNull()?.key?.navTarget !is NavTarget.IncomingShare } - - // Then attach the room - attachRoom(roomId.toRoomIdOrAlias(), clearBackstack = false) - } - } - } - }, - ) - } is NavTarget.IncomingVerificationRequest -> { incomingVerificationEntryPoint.createNode( parentNode = this, @@ -659,17 +621,6 @@ class LoggedInFlowNode( } } - internal suspend fun attachIncomingShare(shareIntentData: ShareIntentData) { - waitForNavTargetAttached { navTarget -> - navTarget is NavTarget.Home - } - attachChild { - backstack.push( - NavTarget.IncomingShare(shareIntentData) - ) - } - } - @Composable override fun View(modifier: Modifier) { val colors by remember { diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt index 934964e447..5ff887b209 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt @@ -47,6 +47,7 @@ import io.element.android.features.announcement.api.AnnouncementService import io.element.android.features.login.api.LoginParams import io.element.android.features.login.api.accesscontrol.AccountProviderAccessControl import io.element.android.features.rageshake.api.bugreport.BugReportEntryPoint +import io.element.android.features.share.api.ShareEntryPoint import io.element.android.features.share.api.ShareIntentData import io.element.android.features.signedout.api.SignedOutEntryPoint import io.element.android.libraries.accountselect.api.AccountSelectEntryPoint @@ -112,6 +113,7 @@ class RootFlowNode( private val analyticsService: AnalyticsService, private val analyticsColdStartWatcher: AnalyticsColdStartWatcher, private val incomingVerificationRequestObserver: IncomingVerificationRequestObserver, + private val shareEntryPoint: ShareEntryPoint, @AppCoroutineScope private val appCoroutineScope: CoroutineScope, ) : BaseFlowNode( backstack = BackStack( @@ -216,7 +218,7 @@ class RootFlowNode( private fun switchToLoggedInFlow(sessionId: SessionId, navId: Int) { pendingLoginParams = null - backstack.safeRoot(NavTarget.LoggedInFlow(sessionId, navId)) + setRoot(NavTarget.LoggedInFlow(sessionId, navId)) restoreAllSessions() } @@ -286,7 +288,20 @@ class RootFlowNode( private fun switchToSignedOutFlow(sessionId: SessionId) { pendingLoginParams = null - backstack.safeRoot(NavTarget.SignedOutFlow(sessionId)) + setRoot(NavTarget.SignedOutFlow(sessionId)) + } + + /** + * Set the root of the backstack. If an incoming share is displayed, keep it on top of the new root, so that the user can + * finish sharing, for instance with another account if the current one has been signed out. + */ + private fun setRoot(navTarget: NavTarget) { + val hasIncomingShare = backstack.elements.value.any { it.key.navTarget is NavTarget.IncomingShare } + if (hasIncomingShare) { + backstack.replaceRoot(navTarget) + } else { + backstack.safeRoot(navTarget) + } } private suspend fun restoreSessionIfNeeded( @@ -367,6 +382,11 @@ class RootFlowNode( @Parcelize data object BugReport : NavTarget + @Parcelize data class IncomingShare( + val sessionId: SessionId, + val shareIntentData: ShareIntentData, + ) : NavTarget + @Parcelize data class OtherAccountIncomingVerificationRequest( val sessionId: SessionId, val verificationRequest: VerificationRequest.Incoming, @@ -389,15 +409,6 @@ class RootFlowNode( override fun navigateToAddAccount() { backstack.push(NavTarget.NotLoggedInFlow(null)) } - - override fun switchAccountAndOpenRoom(sessionId: SessionId, roomId: RoomId?) { - lifecycleScope.launch { - val loggedInFlowNode = attachSession(sessionId) - roomId?.let { - loggedInFlowNode.attachRoom(it.toRoomIdOrAlias(), clearBackstack = false) - } - } - } } val savedNavState = extractSavedStateForNavTarget(navTarget, this.buildContext.savedStateMap) val buildContext = if (savedNavState != null) { @@ -451,6 +462,35 @@ class RootFlowNode( } createNode(buildContext, plugins = listOf(inputs, callback)) } + is NavTarget.IncomingShare -> { + val callback = object : ShareEntryPoint.Callback { + override fun onDone(sessionId: SessionId, roomIds: List) { + // Remove the incoming share screen + backstack.pop() + lifecycleScope.launch { + // Switch to the session which has been used to share the data, if necessary, + // and navigate to the room if the data was shared to a single one. + val loggedInFlowNode = attachSession(sessionId) + roomIds.singleOrNull()?.let { roomId -> + loggedInFlowNode.attachRoom(roomId.toRoomIdOrAlias(), clearBackstack = false) + } + } + } + + override fun onCancel() { + backstack.pop() + } + } + shareEntryPoint.createNode( + parentNode = this, + buildContext = buildContext, + params = ShareEntryPoint.Params( + sessionId = navTarget.sessionId, + shareIntentData = navTarget.shareIntentData, + ), + callback = callback, + ) + } NavTarget.BugReport -> { val callback = object : BugReportEntryPoint.Callback { override fun onDone() { @@ -542,9 +582,11 @@ class RootFlowNode( // No session, open login switchToNotLoggedInFlow(null) } else { - // wait for the current session to be restored, then attach the incoming share node. - // In case of multiple accounts, the user can switch account from the room select screen. - attachSession(latestSessionId).attachIncomingShare(shareIntentData) + // Wait for the current session to be restored, then display the incoming share on top of it. + // The incoming share does not belong to the current session: in case of multiple accounts, the user can switch account + // from the room select screen, and the share is kept if the current session is signed out in the meantime. + attachSession(latestSessionId) + backstack.push(NavTarget.IncomingShare(latestSessionId, shareIntentData)) } } diff --git a/appnav/src/test/kotlin/io/element/android/appnav/BackstackExtTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/BackstackExtTest.kt new file mode 100644 index 0000000000..87173022a1 --- /dev/null +++ b/appnav/src/test/kotlin/io/element/android/appnav/BackstackExtTest.kt @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.appnav + +import com.bumble.appyx.navmodel.backstack.BackStack +import com.bumble.appyx.navmodel.backstack.operation.pop +import com.bumble.appyx.navmodel.backstack.operation.push +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +class BackstackExtTest { + @Test + fun `replaceRoot without element on top replaces the root`() { + val backStack = BackStack(initialElement = "A", savedStateMap = null) + backStack.replaceRoot("D") + assertThat(backStack.navTargets()).containsExactly("D") + assertThat(backStack.activeTarget()).isEqualTo("D") + } + + @Test + fun `replaceRoot with elements on top replaces the root and keeps the elements on top`() { + val backStack = BackStack(initialElement = "A", savedStateMap = null) + backStack.push("B") + backStack.push("C") + backStack.replaceRoot("D") + assertThat(backStack.navTargets()).containsExactly("D", "B", "C").inOrder() + assertThat(backStack.activeTarget()).isEqualTo("C") + // When the elements on top are removed, the new root is displayed + backStack.pop() + backStack.pop() + assertThat(backStack.activeTarget()).isEqualTo("D") + } + + @Test + fun `replaceRoot with the current root does nothing`() { + val backStack = BackStack(initialElement = "A", savedStateMap = null) + backStack.push("B") + backStack.replaceRoot("A") + assertThat(backStack.navTargets()).containsExactly("A", "B").inOrder() + assertThat(backStack.activeTarget()).isEqualTo("B") + } + + private fun BackStack.navTargets() = elements.value + .filter { it.targetState != BackStack.State.DESTROYED } + .map { it.key.navTarget } + + private fun BackStack.activeTarget() = elements.value + .single { it.targetState == BackStack.State.ACTIVE } + .key + .navTarget +} diff --git a/appnav/src/test/kotlin/io/element/android/appnav/RootFlowNodeTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/RootFlowNodeTest.kt index 4a20341212..d9b48c2ade 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/RootFlowNodeTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/RootFlowNodeTest.kt @@ -14,6 +14,7 @@ import android.net.Uri import androidx.arch.core.executor.testing.InstantTaskExecutorRule import com.bumble.appyx.core.modality.AncestryInfo import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.node import com.bumble.appyx.navmodel.backstack.activeElement import com.bumble.appyx.testing.junit4.util.MainDispatcherRule @@ -34,6 +35,7 @@ import io.element.android.features.login.test.FakeLoginIntentResolver import io.element.android.features.login.test.accesscontrol.FakeAccountProviderAccessControl import io.element.android.features.preferences.test.FakeCacheService import io.element.android.features.rageshake.test.FakeBugReportEntryPoint +import io.element.android.features.share.api.ShareEntryPoint import io.element.android.features.share.test.FakeShareIntentHandler import io.element.android.features.signedout.test.FakeSignedOutEntryPoint import io.element.android.libraries.accountselect.test.FakeAccountSelectEntryPoint @@ -52,6 +54,7 @@ import io.element.android.services.analytics.test.FakeAnalyticsService import io.element.android.services.analytics.test.watchers.FakeAnalyticsColdStartWatcher import io.element.android.services.apperror.test.FakeAppErrorStateService import io.element.android.services.appnavstate.test.FakeAppForegroundStateService +import io.element.android.tests.testutils.lambda.lambdaError import io.element.android.tests.testutils.node.FakeNodeFactoriesBindings import io.element.android.tests.testutils.node.FakeParentNode import io.element.android.tests.testutils.presenter.NotUsedPresenter @@ -192,6 +195,14 @@ class RootFlowNodeTest : RobolectricTest() { appForegroundStateService = FakeAppForegroundStateService(), ), appCoroutineScope = backgroundScope, + shareEntryPoint = object : ShareEntryPoint { + override fun createNode( + parentNode: Node, + buildContext: BuildContext, + params: ShareEntryPoint.Params, + callback: ShareEntryPoint.Callback, + ): Node = lambdaError() + }, ) } } diff --git a/features/share/api/src/main/kotlin/io/element/android/features/share/api/ShareEntryPoint.kt b/features/share/api/src/main/kotlin/io/element/android/features/share/api/ShareEntryPoint.kt index cf6ae2c219..3ceafc4b0b 100644 --- a/features/share/api/src/main/kotlin/io/element/android/features/share/api/ShareEntryPoint.kt +++ b/features/share/api/src/main/kotlin/io/element/android/features/share/api/ShareEntryPoint.kt @@ -16,7 +16,14 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId interface ShareEntryPoint : FeatureEntryPoint { - data class Params(val shareIntentData: ShareIntentData) + /** + * @param sessionId the session to use initially. The user can select another session. + * @param shareIntentData the data to share. + */ + data class Params( + val sessionId: SessionId, + val shareIntentData: ShareIntentData, + ) fun createNode( parentNode: Node, @@ -27,10 +34,15 @@ interface ShareEntryPoint : FeatureEntryPoint { interface Callback : Plugin { /** - * Called when the share is done, or cancelled. + * Called when the data has been shared. * @param sessionId the session which has been used to share the data. It can be different from the current session. - * @param roomIds the rooms the data has been shared to, empty if the share has been cancelled. + * @param roomIds the rooms the data has been shared to. */ fun onDone(sessionId: SessionId, roomIds: List) + + /** + * Called when the user cancels the share. + */ + fun onCancel() } } diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/DefaultShareEntryPoint.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/DefaultShareEntryPoint.kt index 98d4acc472..3ebaf7a753 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/DefaultShareEntryPoint.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/DefaultShareEntryPoint.kt @@ -10,12 +10,12 @@ package io.element.android.features.share.impl import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.node.Node +import dev.zacsweers.metro.AppScope import dev.zacsweers.metro.ContributesBinding import io.element.android.features.share.api.ShareEntryPoint import io.element.android.libraries.architecture.createNode -import io.element.android.libraries.di.SessionScope -@ContributesBinding(SessionScope::class) +@ContributesBinding(AppScope::class) class DefaultShareEntryPoint : ShareEntryPoint { override fun createNode( parentNode: Node, @@ -26,7 +26,10 @@ class DefaultShareEntryPoint : ShareEntryPoint { return parentNode.createNode( buildContext = buildContext, plugins = listOf( - ShareNode.Inputs(shareIntentData = params.shareIntentData), + ShareNode.Inputs( + sessionId = params.sessionId, + shareIntentData = params.shareIntentData, + ), callback, ) ) diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareNode.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareNode.kt index 8b44f2bd0f..dc659391dd 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareNode.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareNode.kt @@ -18,6 +18,7 @@ import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode import com.bumble.appyx.core.plugin.Plugin +import dev.zacsweers.metro.AppScope import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedInject import io.element.android.annotations.ContributesNode @@ -26,21 +27,19 @@ import io.element.android.features.share.api.ShareIntentData import io.element.android.libraries.architecture.NodeInputs import io.element.android.libraries.architecture.callback import io.element.android.libraries.architecture.inputs -import io.element.android.libraries.di.SessionScope import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.roomselect.api.RoomSelectEntryPoint import io.element.android.libraries.roomselect.api.RoomSelectMode import kotlinx.parcelize.Parcelize -@ContributesNode(SessionScope::class) +@ContributesNode(AppScope::class) @AssistedInject class ShareNode( @Assisted buildContext: BuildContext, @Assisted plugins: List, presenterFactory: SharePresenter.Factory, private val roomSelectEntryPoint: RoomSelectEntryPoint, - private val sessionId: SessionId, ) : ParentNode( navModel = PermanentNavModel( navTargets = setOf(NavTarget), @@ -52,10 +51,13 @@ class ShareNode( @Parcelize object NavTarget : Parcelable - data class Inputs(val shareIntentData: ShareIntentData) : NodeInputs + data class Inputs( + val sessionId: SessionId, + val shareIntentData: ShareIntentData, + ) : NodeInputs private val inputs = inputs() - private val presenter = presenterFactory.create(inputs.shareIntentData, sessionId) + private val presenter = presenterFactory.create(inputs.shareIntentData) private val callback: ShareEntryPoint.Callback = callback() override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { @@ -65,7 +67,7 @@ class ShareNode( } override fun onCancel() { - callback.onDone(sessionId, emptyList()) + callback.onCancel() } } @@ -73,7 +75,7 @@ class ShareNode( parentNode = this, buildContext = buildContext, params = RoomSelectEntryPoint.Params( - sessionId = sessionId, + sessionId = inputs.sessionId, mode = RoomSelectMode.Share, maxNumberOfRooms = RoomSelectEntryPoint.DEFAULT_MAX_NUMBER_OF_ROOMS, ), @@ -92,7 +94,7 @@ class ShareNode( val state = presenter.present() ShareView( state = state, - onShareSuccess = { roomIds -> callback.onDone(state.sessionId, roomIds) }, + onShareSuccess = { result -> callback.onDone(result.sessionId, result.roomIds) }, ) } } diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt index 905840d1df..91961afb6a 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt @@ -10,9 +10,7 @@ package io.element.android.features.share.impl import androidx.compose.runtime.Composable import androidx.compose.runtime.MutableState -import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.setValue import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedFactory import dev.zacsweers.metro.AssistedInject @@ -30,7 +28,6 @@ import kotlinx.coroutines.launch @AssistedInject class SharePresenter( @Assisted private val shareIntentData: ShareIntentData, - @Assisted initialSessionId: SessionId, @AppCoroutineScope private val appCoroutineScope: CoroutineScope, private val matrixClientProvider: MatrixClientProvider, @@ -38,24 +35,21 @@ class SharePresenter( ) : Presenter { @AssistedFactory fun interface Factory { - fun create(shareIntentData: ShareIntentData, initialSessionId: SessionId): SharePresenter + fun create(shareIntentData: ShareIntentData): SharePresenter } - private val shareActionState: MutableState>> = mutableStateOf(AsyncAction.Uninitialized) + private val shareActionState: MutableState> = mutableStateOf(AsyncAction.Uninitialized) /** - * The session the data is shared with, it can be updated if the user selects rooms from another session. + * Share the data to the rooms with [roomIds], which belong to the session [sessionId]. It can be another session than the current one. */ - private var targetSessionId by mutableStateOf(initialSessionId) - fun onRoomSelected(sessionId: SessionId, roomIds: List) { if (shareActionState.value.isLoading()) return - targetSessionId = sessionId appCoroutineScope.launch { suspend { val client = matrixClientProvider.getOrRestore(sessionId).getOrThrow() shareDataSender.send(client, shareIntentData, roomIds).getOrThrow() - roomIds + ShareResult(sessionId = sessionId, roomIds = roomIds) }.runCatchingUpdatingState(shareActionState) } } @@ -69,7 +63,6 @@ class SharePresenter( } return ShareState( - sessionId = targetSessionId, shareAction = shareActionState.value, eventSink = ::handleEvent, ) diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareState.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareState.kt index 10046cf3b2..eb50410526 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareState.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareState.kt @@ -13,7 +13,16 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId data class ShareState( - val sessionId: SessionId, - val shareAction: AsyncAction>, + val shareAction: AsyncAction, val eventSink: (ShareEvent) -> Unit ) + +/** + * The result of a successful share. + * @param sessionId the session which has been used to share the data, it can be another session than the current one. + * @param roomIds the rooms the data has been shared to. + */ +data class ShareResult( + val sessionId: SessionId, + val roomIds: List, +) diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareStatePreviewParam.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareStatePreviewParam.kt index c765db9195..38ae8369df 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareStatePreviewParam.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareStatePreviewParam.kt @@ -21,9 +21,7 @@ open class ShareStatePreviewParam : PreviewParameterProvider { shareAction = AsyncAction.Loading, ), aShareState( - shareAction = AsyncAction.Success( - listOf(RoomId("!room2:domain")), - ) + shareAction = AsyncAction.Success(aShareResult()) ), aShareState( shareAction = AsyncAction.Failure(RuntimeException("error")), @@ -31,12 +29,18 @@ open class ShareStatePreviewParam : PreviewParameterProvider { ) } -fun aShareState( - sessionId: SessionId = SessionId("@alice:server.org"), - shareAction: AsyncAction> = AsyncAction.Uninitialized, +private fun aShareState( + shareAction: AsyncAction = AsyncAction.Uninitialized, eventSink: (ShareEvent) -> Unit = {} ) = ShareState( - sessionId = sessionId, shareAction = shareAction, eventSink = eventSink ) + +private fun aShareResult( + sessionId: SessionId = SessionId("@alice:server.org"), + roomIds: List = listOf(RoomId("!room2:domain")), +) = ShareResult( + sessionId = sessionId, + roomIds = roomIds, +) diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareView.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareView.kt index 2516c31a66..f2d91972cf 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareView.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareView.kt @@ -13,12 +13,11 @@ import androidx.compose.ui.tooling.preview.PreviewParameter import io.element.android.libraries.designsystem.components.async.AsyncActionView import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight -import io.element.android.libraries.matrix.api.core.RoomId @Composable fun ShareView( state: ShareState, - onShareSuccess: (List) -> Unit, + onShareSuccess: (ShareResult) -> Unit, ) { AsyncActionView( async = state.shareAction, diff --git a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/DefaultShareEntryPointTest.kt b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/DefaultShareEntryPointTest.kt index 85068f5402..e1a5709df0 100644 --- a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/DefaultShareEntryPointTest.kt +++ b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/DefaultShareEntryPointTest.kt @@ -39,15 +39,16 @@ class DefaultShareEntryPointTest { ShareNode( buildContext = buildContext, plugins = plugins, - presenterFactory = { _, _ -> createSharePresenter() }, + presenterFactory = { createSharePresenter() }, roomSelectEntryPoint = FakeRoomSelectEntryPoint(), - sessionId = A_SESSION_ID, ) } val callback = object : ShareEntryPoint.Callback { override fun onDone(sessionId: SessionId, roomIds: List) = lambdaError() + override fun onCancel() = lambdaError() } val params = ShareEntryPoint.Params( + sessionId = A_SESSION_ID, shareIntentData = ShareIntentData.PlainText(A_MESSAGE), ) val result = entryPoint.createNode( @@ -57,7 +58,7 @@ class DefaultShareEntryPointTest { callback = callback, ) assertThat(result).isInstanceOf(ShareNode::class.java) - assertThat(result.plugins).contains(ShareNode.Inputs(params.shareIntentData)) + assertThat(result.plugins).contains(ShareNode.Inputs(params.sessionId, params.shareIntentData)) assertThat(result.plugins).contains(callback) } } diff --git a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt index 8fc5109636..a2575d8186 100644 --- a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt +++ b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt @@ -44,7 +44,6 @@ class SharePresenterTest { presenter.present() }.test { val initialState = awaitItem() - assertThat(initialState.sessionId).isEqualTo(A_SESSION_ID) assertThat(initialState.shareAction.isUninitialized()).isTrue() } } @@ -86,8 +85,7 @@ class SharePresenterTest { presenter.onRoomSelected(A_SESSION_ID, listOf(A_ROOM_ID)) assertThat(awaitItem().shareAction.isLoading()).isTrue() val success = awaitItem() - assertThat(success.sessionId).isEqualTo(A_SESSION_ID) - assertThat(success.shareAction).isEqualTo(AsyncAction.Success(listOf(A_ROOM_ID))) + assertThat(success.shareAction).isEqualTo(AsyncAction.Success(ShareResult(A_SESSION_ID, listOf(A_ROOM_ID)))) sendResult.assertions().isCalledOnce().with(value(client), value(shareIntentData), value(listOf(A_ROOM_ID))) } } @@ -106,16 +104,11 @@ class SharePresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() - assertThat(initialState.sessionId).isEqualTo(A_SESSION_ID) + skipItems(1) presenter.onRoomSelected(A_SESSION_ID_2, listOf(A_ROOM_ID)) - val sessionChanged = awaitItem() - assertThat(sessionChanged.sessionId).isEqualTo(A_SESSION_ID_2) - assertThat(sessionChanged.shareAction.isUninitialized()).isTrue() assertThat(awaitItem().shareAction.isLoading()).isTrue() val success = awaitItem() - assertThat(success.sessionId).isEqualTo(A_SESSION_ID_2) - assertThat(success.shareAction).isEqualTo(AsyncAction.Success(listOf(A_ROOM_ID))) + assertThat(success.shareAction).isEqualTo(AsyncAction.Success(ShareResult(A_SESSION_ID_2, listOf(A_ROOM_ID)))) getClient.assertions().isCalledOnce().with(value(A_SESSION_ID_2)) sendResult.assertions().isCalledOnce() } @@ -133,7 +126,6 @@ class SharePresenterTest { }.test { skipItems(1) presenter.onRoomSelected(A_SESSION_ID_2, listOf(A_ROOM_ID)) - assertThat(awaitItem().sessionId).isEqualTo(A_SESSION_ID_2) assertThat(awaitItem().shareAction.isLoading()).isTrue() assertThat(awaitItem().shareAction).isEqualTo(AsyncAction.Failure(AN_EXCEPTION)) sendResult.assertions().isNeverCalled() @@ -161,8 +153,8 @@ class SharePresenterTest { presenter.onRoomSelected(A_SESSION_ID_2, listOf(A_ROOM_ID)) completable.complete(Result.success(Unit)) val success = awaitItem() - assertThat(success.sessionId).isEqualTo(A_SESSION_ID) - assertThat(success.shareAction).isEqualTo(AsyncAction.Success(listOf(A_ROOM_ID))) + // The second selection has been ignored + assertThat(success.shareAction).isEqualTo(AsyncAction.Success(ShareResult(A_SESSION_ID, listOf(A_ROOM_ID)))) sendResult.assertions().isCalledOnce() } } @@ -170,13 +162,11 @@ class SharePresenterTest { internal fun TestScope.createSharePresenter( shareIntentData: ShareIntentData = ShareIntentData.PlainText(A_MESSAGE), - initialSessionId: SessionId = A_SESSION_ID, matrixClientProvider: FakeMatrixClientProvider = FakeMatrixClientProvider(), shareDataSender: ShareDataSender = FakeShareDataSender(), ): SharePresenter { return SharePresenter( shareIntentData = shareIntentData, - initialSessionId = initialSessionId, appCoroutineScope = this, matrixClientProvider = matrixClientProvider, shareDataSender = shareDataSender, diff --git a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt index 3de9d22ba8..d8a7a10823 100644 --- a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt +++ b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt @@ -65,17 +65,19 @@ class RoomSelectPresenter( val queryState = rememberTextFieldState() var isSearchActive by remember { mutableStateOf(false) } var isAccountListExpanded by remember { mutableStateOf(false) } + // Null until the sessions are loaded + val sessions by remember { sessionStore.sessionsFlow() }.collectAsState(initial = null) 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 { - derivedStateOf { sessions.map { it.toMatrixUser() } } + derivedStateOf { sessions.orEmpty().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. + // signed out from another device. val isSelectedSessionInvalid by remember { derivedStateOf { - selectedSessionId != initialSessionId && sessions.none { it.userId == selectedSessionId.value && it.isTokenValid } + val loadedSessions = sessions ?: return@derivedStateOf false + loadedSessions.none { it.userId == selectedSessionId.value && it.isTokenValid } } } LaunchedEffect(isSelectedSessionInvalid) { @@ -99,27 +101,31 @@ 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 (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, 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 } + // Also use the validity of the session as a key, so that the data source is removed if the session is invalidated, and recreated + // if the session becomes valid again. + val (dataSource, hasClientError) = key(selectedSessionId, isSelectedSessionInvalid) { + val sessionId = selectedSessionId + if (isSelectedSessionInvalid) { + // Do not try to get the client of an invalid session: it cannot be restored, and an in-memory client would have been destroyed. + null to false + } else { + 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(sessionId)?.roomListService?.let { Result.success(it) }) { + if (value == null) { + value = matrixClientProvider.getOrRestore(sessionId) + .onFailure { Timber.e(it, "Failed to get the client for $sessionId") } + .map { it.roomListService } + } } + val roomListService = roomListServiceResult?.getOrNull() + val dataSource = remember(roomListService) { + roomListService?.let { dataSourceFactory.create(dataSourceCoroutineScope, it) } + } + dataSource to (roomListServiceResult?.isFailure == true) } - val roomListService = roomListServiceResult?.getOrNull() - val dataSource = remember(roomListService) { - roomListService?.let { dataSourceFactory.create(dataSourceCoroutineScope, it) } - } - 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() diff --git a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/DefaultRoomSelectEntryPointTest.kt b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/DefaultRoomSelectEntryPointTest.kt index c25e3303cc..7f88bb06ac 100644 --- a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/DefaultRoomSelectEntryPointTest.kt +++ b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/DefaultRoomSelectEntryPointTest.kt @@ -42,7 +42,7 @@ class DefaultRoomSelectEntryPointTest : RobolectricTest() { assertThat(initialSessionId).isEqualTo(A_SESSION_ID) assertThat(mode).isEqualTo(testMode) assertThat(maxNumberOfRooms).isEqualTo(testMaxNumberOfRooms) - createRoomSelectPresenter(mode, maxNumberOfRooms) + createRoomSelectPresenter(mode = mode, maxNumberOfRooms = maxNumberOfRooms) }, ) } diff --git a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt index 0694cc27f0..95f5fefa03 100644 --- a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt +++ b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt @@ -13,6 +13,7 @@ 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 +import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.roomlist.RoomListFilter import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.api.user.MatrixUser @@ -113,7 +114,7 @@ class RoomSelectPresenterTest { val presenter = createRoomSelectPresenter( mode = RoomSelectMode.Share, sessionStore = InMemorySessionStore( - initialList = listOf(aSessionData(sessionId = A_SESSION_ID.value)), + initialList = listOf(aSessionData(sessionId = A_SESSION_ID.value, isTokenValid = true)), ), ) presenter.test { @@ -208,6 +209,30 @@ class RoomSelectPresenterTest { } } + @Test + fun `present - the current account is invalidated while selected, an error is displayed`() = runTest { + val roomSummary = aRoomSummary() + val sessionStore = aSessionStoreWithTwoAccounts() + val presenter = createRoomSelectPresenter( + mode = RoomSelectMode.Share, + sessionStore = sessionStore, + roomListService = FakeRoomListService(createRoomListLambda = { FakeDynamicRoomList(summaries = MutableStateFlow(listOf(roomSummary))) }), + ) + presenter.test { + val state = consumeItemsUntilPredicate { it.resultState is SearchBarResultState.Results }.last() + state.eventSink(RoomSelectEvent.ToggleSelectedRoom(roomSummary.toSelectRoomInfo())) + assertThat(consumeItemsUntilPredicate { it.selectedRooms.isNotEmpty() }.last().selectedRooms).hasSize(1) + // The current session is signed out from another device while the screen is displayed + sessionStore.updateData(sessionStore.getSession(A_SESSION_ID.value)!!.copy(isTokenValid = false)) + val errorState = consumeItemsUntilPredicate { it.hasRoomListError && it.selectedRooms.isEmpty() }.last() + assertThat(errorState.selectedAccount.userId).isEqualTo(A_SESSION_ID) + assertThat(errorState.resultState).isInstanceOf(SearchBarResultState.Initial::class.java) + // The user can switch to the other account + assertThat(errorState.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - another account is invalidated, it is still proposed and there is no error`() = runTest { val sessionStore = aSessionStoreWithTwoAccounts() @@ -389,15 +414,19 @@ class RoomSelectPresenterTest { } internal fun TestScope.createRoomSelectPresenter( + initialSessionId: SessionId = A_SESSION_ID, mode: RoomSelectMode = RoomSelectMode.Forward, maxNumberOfRooms: Int = RoomSelectEntryPoint.DEFAULT_MAX_NUMBER_OF_ROOMS, roomListService: RoomListService = FakeRoomListService(), - sessionStore: SessionStore = InMemorySessionStore(), + // The current session is always in the store + sessionStore: SessionStore = InMemorySessionStore( + initialList = listOf(aSessionData(sessionId = A_SESSION_ID.value, isTokenValid = true)), + ), matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider { sessionId -> Result.success(FakeMatrixClient(sessionId = sessionId, roomListService = roomListService)) }, ) = RoomSelectPresenter( - initialSessionId = A_SESSION_ID, + initialSessionId = initialSessionId, mode = mode, maxNumberOfRooms = maxNumberOfRooms, dataSourceFactory = object : RoomSelectSearchDataSource.Factory { From 00dab131ddff344a04c419818530a0ebeee44e67 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 9 Oct 2026 16:36:29 +0200 Subject: [PATCH 2/2] Ask for session verification before sharing with an unverified account The incoming share is now displayed on top of the root, so the FTUE, and the session verification in particular, could be bypassed. - When the current account is not verified, wait for the FTUE to be complete before displaying the incoming share, as before. - When the user selects another account which is not verified in the room select screen, display the session verification flow of the FTUE for this account on top of the share. As in the FTUE, the user has to verify the session or to sign out. - Expose the FTUE session verification flow with a new SessionVerificationEntryPoint, displayed by OtherSessionVerificationNode which sets up the session graph of the account to verify. - Introduce RoomSelectNavigator so that RoomSelectPresenter can request the verification of the selected session. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../android/appnav/LoggedInFlowNode.kt | 9 ++ .../io/element/android/appnav/RootFlowNode.kt | 33 ++++++- .../OtherSessionVerificationNode.kt | 89 +++++++++++++++++++ .../forward/impl/ForwardMessagesNode.kt | 4 + .../ftue/api/SessionVerificationEntryPoint.kt | 32 +++++++ .../DefaultSessionVerificationEntryPoint.kt | 32 +++++++ ...efaultSessionVerificationEntryPointTest.kt | 63 +++++++++++++ .../features/share/api/ShareEntryPoint.kt | 7 ++ .../android/features/share/impl/ShareNode.kt | 4 + .../share/impl/DefaultShareEntryPointTest.kt | 1 + .../roomselect/api/RoomSelectEntryPoint.kt | 7 ++ .../roomselect/impl/RoomSelectNavigator.kt | 17 ++++ .../roomselect/impl/RoomSelectNode.kt | 9 +- .../roomselect/impl/RoomSelectPresenter.kt | 31 +++++-- .../impl/DefaultRoomSelectEntryPointTest.kt | 3 +- .../impl/RoomSelectPresenterTest.kt | 79 ++++++++++++++++ 16 files changed, 407 insertions(+), 13 deletions(-) create mode 100644 appnav/src/main/kotlin/io/element/android/appnav/verification/OtherSessionVerificationNode.kt create mode 100644 features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/SessionVerificationEntryPoint.kt create mode 100644 features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultSessionVerificationEntryPoint.kt create mode 100644 features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultSessionVerificationEntryPointTest.kt create mode 100644 libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectNavigator.kt diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index 270ea6b052..fed49bdeab 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -577,6 +577,15 @@ class LoggedInFlowNode( } } + /** + * Wait for the home screen to be attached, i.e. for the FTUE to be complete. + */ + internal suspend fun waitForHome() { + waitForNavTargetAttached { navTarget -> + navTarget is NavTarget.Home + } + } + suspend fun attachRoom( roomIdOrAlias: RoomIdOrAlias, serverNames: List = emptyList(), diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt index 5ff887b209..673bdc1388 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt @@ -43,6 +43,7 @@ import io.element.android.appnav.session.MatrixSessionCache import io.element.android.appnav.verification.IncomingVerificationRequestData import io.element.android.appnav.verification.IncomingVerificationRequestObserver import io.element.android.appnav.verification.OtherSessionIncomingVerificationNode +import io.element.android.appnav.verification.OtherSessionVerificationNode import io.element.android.features.announcement.api.AnnouncementService import io.element.android.features.login.api.LoginParams import io.element.android.features.login.api.accesscontrol.AccountProviderAccessControl @@ -244,6 +245,8 @@ class RootFlowNode( .withPreviousValue() .onEach { (previous, current) -> (previous.orEmpty() - current).forEach { sessionId -> + // The verification of a removed session cannot continue, for instance if the user has signed out from it + backstack.removeLast(NavTarget.OtherAccountSessionVerification(sessionId)) Timber.d("Session $sessionId removed, remove it from the cache") matrixSessionCache.remove(sessionId) } @@ -391,6 +394,10 @@ class RootFlowNode( val sessionId: SessionId, val verificationRequest: VerificationRequest.Incoming, ) : NavTarget + + @Parcelize data class OtherAccountSessionVerification( + val sessionId: SessionId, + ) : NavTarget } override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { @@ -462,6 +469,20 @@ class RootFlowNode( } createNode(buildContext, plugins = listOf(inputs, callback)) } + is NavTarget.OtherAccountSessionVerification -> { + if (matrixSessionCache.getOrNull(navTarget.sessionId) == null) { + Timber.w("Couldn't find session ${navTarget.sessionId} to verify") + lifecycleScope.launch { backstack.pop() } + return emptyNode(buildContext) + } + val inputs = OtherSessionVerificationNode.Inputs(sessionId = navTarget.sessionId) + val callback = object : OtherSessionVerificationNode.Callback { + override fun onDone() { + backstack.pop() + } + } + createNode(buildContext, plugins = listOf(inputs, callback)) + } is NavTarget.IncomingShare -> { val callback = object : ShareEntryPoint.Callback { override fun onDone(sessionId: SessionId, roomIds: List) { @@ -480,6 +501,13 @@ class RootFlowNode( override fun onCancel() { backstack.pop() } + + override fun onSessionVerificationRequired(sessionId: SessionId) { + val navTarget = NavTarget.OtherAccountSessionVerification(sessionId) + if (backstack.elements.value.lastOrNull()?.key?.navTarget != navTarget) { + backstack.push(navTarget) + } + } } shareEntryPoint.createNode( parentNode = this, @@ -582,10 +610,11 @@ class RootFlowNode( // No session, open login switchToNotLoggedInFlow(null) } else { - // Wait for the current session to be restored, then display the incoming share on top of it. + // Wait for the current session to be restored and for the FTUE to be complete, so that the session is verified, + // then display the incoming share on top of it. // The incoming share does not belong to the current session: in case of multiple accounts, the user can switch account // from the room select screen, and the share is kept if the current session is signed out in the meantime. - attachSession(latestSessionId) + attachSession(latestSessionId).waitForHome() backstack.push(NavTarget.IncomingShare(latestSessionId, shareIntentData)) } } diff --git a/appnav/src/main/kotlin/io/element/android/appnav/verification/OtherSessionVerificationNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/verification/OtherSessionVerificationNode.kt new file mode 100644 index 0000000000..1b6078888a --- /dev/null +++ b/appnav/src/main/kotlin/io/element/android/appnav/verification/OtherSessionVerificationNode.kt @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.appnav.verification + +import android.os.Parcelable +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import com.bumble.appyx.core.composable.Children +import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel +import com.bumble.appyx.core.node.Node +import com.bumble.appyx.core.node.ParentNode +import com.bumble.appyx.core.plugin.Plugin +import dev.zacsweers.metro.AppScope +import dev.zacsweers.metro.Assisted +import dev.zacsweers.metro.AssistedInject +import io.element.android.annotations.ContributesNode +import io.element.android.appnav.di.SessionGraphFactory +import io.element.android.features.ftue.api.SessionVerificationEntryPoint +import io.element.android.libraries.architecture.NodeInputs +import io.element.android.libraries.architecture.callback +import io.element.android.libraries.architecture.inputs +import io.element.android.libraries.di.DependencyInjectionGraphOwner +import io.element.android.libraries.matrix.api.MatrixClientProvider +import io.element.android.libraries.matrix.api.core.SessionId +import kotlinx.parcelize.Parcelize + +/** + * Display the verification of a session which is not the current one, without switching the current session. + * It sets up the Session graph of the session to verify. + */ +@ContributesNode(AppScope::class) +@AssistedInject +class OtherSessionVerificationNode( + @Assisted buildContext: BuildContext, + @Assisted plugins: List, + sessionGraphFactory: SessionGraphFactory, + matrixClientProvider: MatrixClientProvider, + private val sessionVerificationEntryPoint: SessionVerificationEntryPoint, +) : ParentNode( + navModel = PermanentNavModel( + navTargets = setOf(NavTarget), + savedStateMap = buildContext.savedStateMap, + ), + buildContext = buildContext, + plugins = plugins +), DependencyInjectionGraphOwner { + interface Callback : Plugin { + fun onDone() + } + + @Parcelize + object NavTarget : Parcelable + + data class Inputs( + val sessionId: SessionId, + ) : NodeInputs + + private val callback: Callback = callback() + private val inputs: Inputs = inputs() + override val graph = sessionGraphFactory.create( + requireNotNull(matrixClientProvider.getOrNull(inputs.sessionId)) { "No MatrixClient for session ${inputs.sessionId}" } + ) + + override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { + return sessionVerificationEntryPoint.createNode( + parentNode = this, + buildContext = buildContext, + callback = object : SessionVerificationEntryPoint.Callback { + override fun onDone() { + callback.onDone() + } + }, + ) + } + + @Composable + override fun View(modifier: Modifier) { + Children( + navModel = navModel, + modifier = modifier, + ) + } +} diff --git a/features/forward/impl/src/main/kotlin/io/element/android/features/forward/impl/ForwardMessagesNode.kt b/features/forward/impl/src/main/kotlin/io/element/android/features/forward/impl/ForwardMessagesNode.kt index f8a1048591..669c414045 100644 --- a/features/forward/impl/src/main/kotlin/io/element/android/features/forward/impl/ForwardMessagesNode.kt +++ b/features/forward/impl/src/main/kotlin/io/element/android/features/forward/impl/ForwardMessagesNode.kt @@ -72,6 +72,10 @@ class ForwardMessagesNode( override fun onCancel() { callback.onDone(emptyList()) } + + override fun onSessionVerificationRequired(sessionId: SessionId) { + // Only called in Share mode + } } return roomSelectEntryPoint.createNode( diff --git a/features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/SessionVerificationEntryPoint.kt b/features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/SessionVerificationEntryPoint.kt new file mode 100644 index 0000000000..95c801d009 --- /dev/null +++ b/features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/SessionVerificationEntryPoint.kt @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.ftue.api + +import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.node.Node +import com.bumble.appyx.core.plugin.Plugin +import io.element.android.libraries.architecture.FeatureEntryPoint + +/** + * Entry point to the session verification flow displayed during the FTUE, to be used outside of the FTUE. + * As in the FTUE, the user cannot go back from this flow: they have to verify the session or to sign out. + */ +interface SessionVerificationEntryPoint : FeatureEntryPoint { + fun createNode( + parentNode: Node, + buildContext: BuildContext, + callback: Callback, + ): Node + + interface Callback : Plugin { + /** + * Called when the session has been verified. + */ + fun onDone() + } +} diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultSessionVerificationEntryPoint.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultSessionVerificationEntryPoint.kt new file mode 100644 index 0000000000..7959391787 --- /dev/null +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultSessionVerificationEntryPoint.kt @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.ftue.impl + +import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.node.Node +import dev.zacsweers.metro.AppScope +import dev.zacsweers.metro.ContributesBinding +import io.element.android.features.ftue.api.SessionVerificationEntryPoint +import io.element.android.features.ftue.impl.sessionverification.FtueSessionVerificationFlowNode +import io.element.android.libraries.architecture.createNode + +@ContributesBinding(AppScope::class) +class DefaultSessionVerificationEntryPoint : SessionVerificationEntryPoint { + override fun createNode( + parentNode: Node, + buildContext: BuildContext, + callback: SessionVerificationEntryPoint.Callback, + ): Node { + val flowNodeCallback = object : FtueSessionVerificationFlowNode.Callback { + override fun onDone() { + callback.onDone() + } + } + return parentNode.createNode(buildContext, listOf(flowNodeCallback)) + } +} diff --git a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultSessionVerificationEntryPointTest.kt b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultSessionVerificationEntryPointTest.kt new file mode 100644 index 0000000000..b9248b3bbf --- /dev/null +++ b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultSessionVerificationEntryPointTest.kt @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.ftue.impl + +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.node.Node +import com.bumble.appyx.testing.junit4.util.MainDispatcherRule +import com.google.common.truth.Truth.assertThat +import io.element.android.features.ftue.api.SessionVerificationEntryPoint +import io.element.android.features.ftue.impl.sessionverification.FtueSessionVerificationFlowNode +import io.element.android.features.securebackup.api.SecureBackupEntryPoint +import io.element.android.features.verifysession.api.OutgoingVerificationEntryPoint +import io.element.android.tests.testutils.lambda.lambdaError +import io.element.android.tests.testutils.node.TestParentNode +import kotlinx.coroutines.test.runTest +import org.junit.Rule +import org.junit.Test + +class DefaultSessionVerificationEntryPointTest { + @get:Rule + val instantTaskExecutorRule = InstantTaskExecutorRule() + + @get:Rule + val mainDispatcherRule = MainDispatcherRule() + + @Test + fun `test node builder`() = runTest { + val entryPoint = DefaultSessionVerificationEntryPoint() + val parentNode = TestParentNode.create { buildContext, plugins -> + FtueSessionVerificationFlowNode( + buildContext = buildContext, + plugins = plugins, + outgoingVerificationEntryPoint = object : OutgoingVerificationEntryPoint { + override fun createNode( + parentNode: Node, + buildContext: BuildContext, + params: OutgoingVerificationEntryPoint.Params, + callback: OutgoingVerificationEntryPoint.Callback, + ) = lambdaError() + }, + secureBackupEntryPoint = object : SecureBackupEntryPoint { + override fun createNode( + parentNode: Node, + buildContext: BuildContext, + params: SecureBackupEntryPoint.Params, + callback: SecureBackupEntryPoint.Callback, + ) = lambdaError() + }, + ) + } + val callback = object : SessionVerificationEntryPoint.Callback { + override fun onDone() = lambdaError() + } + val result = entryPoint.createNode(parentNode, BuildContext.root(null), callback) + assertThat(result).isInstanceOf(FtueSessionVerificationFlowNode::class.java) + } +} diff --git a/features/share/api/src/main/kotlin/io/element/android/features/share/api/ShareEntryPoint.kt b/features/share/api/src/main/kotlin/io/element/android/features/share/api/ShareEntryPoint.kt index 3ceafc4b0b..e34deb9754 100644 --- a/features/share/api/src/main/kotlin/io/element/android/features/share/api/ShareEntryPoint.kt +++ b/features/share/api/src/main/kotlin/io/element/android/features/share/api/ShareEntryPoint.kt @@ -44,5 +44,12 @@ interface ShareEntryPoint : FeatureEntryPoint { * Called when the user cancels the share. */ fun onCancel() + + /** + * Called when the user has selected an account whose session is not verified. + * The session has to be verified before the user can share content with it. + * @param sessionId the session to verify. + */ + fun onSessionVerificationRequired(sessionId: SessionId) } } diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareNode.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareNode.kt index dc659391dd..72f331abb2 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareNode.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/ShareNode.kt @@ -69,6 +69,10 @@ class ShareNode( override fun onCancel() { callback.onCancel() } + + override fun onSessionVerificationRequired(sessionId: SessionId) { + callback.onSessionVerificationRequired(sessionId) + } } return roomSelectEntryPoint.createNode( diff --git a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/DefaultShareEntryPointTest.kt b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/DefaultShareEntryPointTest.kt index e1a5709df0..62c78d5a66 100644 --- a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/DefaultShareEntryPointTest.kt +++ b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/DefaultShareEntryPointTest.kt @@ -46,6 +46,7 @@ class DefaultShareEntryPointTest { val callback = object : ShareEntryPoint.Callback { override fun onDone(sessionId: SessionId, roomIds: List) = lambdaError() override fun onCancel() = lambdaError() + override fun onSessionVerificationRequired(sessionId: SessionId) = lambdaError() } val params = ShareEntryPoint.Params( sessionId = A_SESSION_ID, diff --git a/libraries/roomselect/api/src/main/kotlin/io/element/android/libraries/roomselect/api/RoomSelectEntryPoint.kt b/libraries/roomselect/api/src/main/kotlin/io/element/android/libraries/roomselect/api/RoomSelectEntryPoint.kt index 5fec007261..39025a95d1 100644 --- a/libraries/roomselect/api/src/main/kotlin/io/element/android/libraries/roomselect/api/RoomSelectEntryPoint.kt +++ b/libraries/roomselect/api/src/main/kotlin/io/element/android/libraries/roomselect/api/RoomSelectEntryPoint.kt @@ -41,6 +41,13 @@ interface RoomSelectEntryPoint : FeatureEntryPoint { */ fun onRoomSelected(sessionId: SessionId, roomIds: List) fun onCancel() + + /** + * Called in [RoomSelectMode.Share] mode when the user has selected an account whose session is not verified. + * The session has to be verified before the user can share content with it. + * @param sessionId the session to verify. + */ + fun onSessionVerificationRequired(sessionId: SessionId) } companion object { diff --git a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectNavigator.kt b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectNavigator.kt new file mode 100644 index 0000000000..1b7747736e --- /dev/null +++ b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectNavigator.kt @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.libraries.roomselect.impl + +import io.element.android.libraries.matrix.api.core.SessionId + +fun interface RoomSelectNavigator { + /** + * Navigate to the verification of the session [sessionId], which has to be verified before content can be shared with it. + */ + fun navigateToSessionVerification(sessionId: SessionId) +} diff --git a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectNode.kt b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectNode.kt index f137ecc848..148b8bf53d 100644 --- a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectNode.kt +++ b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectNode.kt @@ -33,7 +33,7 @@ class RoomSelectNode( @Assisted buildContext: BuildContext, @Assisted plugins: List, presenterFactory: RoomSelectPresenter.Factory, -) : Node(buildContext, plugins = plugins) { +) : Node(buildContext, plugins = plugins), RoomSelectNavigator { data class Inputs( val sessionId: SessionId, val mode: RoomSelectMode, @@ -41,13 +41,18 @@ class RoomSelectNode( ) : NodeInputs private val inputs: Inputs = inputs() + private val callback: RoomSelectEntryPoint.Callback = callback() private val presenter = presenterFactory.create( initialSessionId = inputs.sessionId, mode = inputs.mode, maxNumberOfRooms = inputs.maxNumberOfRooms, + navigator = this, ) private val stateFlow = launchMolecule { presenter.present() } - private val callback: RoomSelectEntryPoint.Callback = callback() + + override fun navigateToSessionVerification(sessionId: SessionId) { + callback.onSessionVerificationRequired(sessionId) + } @Composable override fun View(modifier: Modifier) { diff --git a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt index d8a7a10823..ff2036226d 100644 --- a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt +++ b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt @@ -31,12 +31,14 @@ import io.element.android.libraries.matrix.api.MatrixClientProvider import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.matrix.api.user.toMatrixUser +import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.matrix.ui.model.SelectRoomInfo import io.element.android.libraries.roomselect.api.RoomSelectMode import io.element.android.libraries.sessionstorage.api.SessionStore import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch import timber.log.Timber @@ -46,6 +48,7 @@ class RoomSelectPresenter( @Assisted private val initialSessionId: SessionId, @Assisted private val mode: RoomSelectMode, @Assisted private val maxNumberOfRooms: Int, + @Assisted private val navigator: RoomSelectNavigator, private val dataSourceFactory: RoomSelectSearchDataSource.Factory, private val matrixClientProvider: MatrixClientProvider, private val sessionStore: SessionStore, @@ -56,6 +59,7 @@ class RoomSelectPresenter( initialSessionId: SessionId, mode: RoomSelectMode, maxNumberOfRooms: Int, + navigator: RoomSelectNavigator, ): RoomSelectPresenter } @@ -103,30 +107,41 @@ class RoomSelectPresenter( // data source is removed from the composition and its coroutine scope is cancelled. // Also use the validity of the session as a key, so that the data source is removed if the session is invalidated, and recreated // if the session becomes valid again. - val (dataSource, hasClientError) = key(selectedSessionId, isSelectedSessionInvalid) { + val (dataSource, hasClientError, isSessionNotVerified) = key(selectedSessionId, isSelectedSessionInvalid) { val sessionId = selectedSessionId if (isSelectedSessionInvalid) { // Do not try to get the client of an invalid session: it cannot be restored, and an in-memory client would have been destroyed. - null to false + Triple(null, false, false) } else { 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(sessionId)?.roomListService?.let { Result.success(it) }) { + val clientResult by produceState(matrixClientProvider.getOrNull(sessionId)?.let { Result.success(it) }) { if (value == null) { value = matrixClientProvider.getOrRestore(sessionId) .onFailure { Timber.e(it, "Failed to get the client for $sessionId") } - .map { it.roomListService } } } - val roomListService = roomListServiceResult?.getOrNull() - val dataSource = remember(roomListService) { - roomListService?.let { dataSourceFactory.create(dataSourceCoroutineScope, it) } + val client = clientResult?.getOrNull() + val dataSource = remember(client) { + client?.let { dataSourceFactory.create(dataSourceCoroutineScope, it.roomListService) } } - dataSource to (roomListServiceResult?.isFailure == true) + // Content cannot be shared with a session which is not verified. The initial session is verified, since the FTUE is complete. + val sessionVerifiedStatus by remember(client) { + if (mode == RoomSelectMode.Share && sessionId != initialSessionId && client != null) { + client.sessionVerificationService.sessionVerifiedStatus + } else { + MutableStateFlow(SessionVerifiedStatus.Unknown) + } + }.collectAsState() + Triple(dataSource, clientResult?.isFailure == true, sessionVerifiedStatus == SessionVerifiedStatus.NotVerified) } } val hasRoomListError = hasClientError || isSelectedSessionInvalid + val sessionToVerify = selectedSessionId.takeIf { isSessionNotVerified } + LaunchedEffect(sessionToVerify) { + sessionToVerify?.let(navigator::navigateToSessionVerification) + } val searchQuery = queryState.text.toString() LaunchedEffect(dataSource, searchQuery) { diff --git a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/DefaultRoomSelectEntryPointTest.kt b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/DefaultRoomSelectEntryPointTest.kt index 7f88bb06ac..229e6bd3b0 100644 --- a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/DefaultRoomSelectEntryPointTest.kt +++ b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/DefaultRoomSelectEntryPointTest.kt @@ -38,7 +38,7 @@ class DefaultRoomSelectEntryPointTest : RobolectricTest() { RoomSelectNode( buildContext = buildContext, plugins = plugins, - presenterFactory = { initialSessionId, mode, maxNumberOfRooms -> + presenterFactory = { initialSessionId, mode, maxNumberOfRooms, _ -> assertThat(initialSessionId).isEqualTo(A_SESSION_ID) assertThat(mode).isEqualTo(testMode) assertThat(maxNumberOfRooms).isEqualTo(testMaxNumberOfRooms) @@ -49,6 +49,7 @@ class DefaultRoomSelectEntryPointTest : RobolectricTest() { val callback = object : RoomSelectEntryPoint.Callback { override fun onRoomSelected(sessionId: SessionId, roomIds: List) = lambdaError() override fun onCancel() = lambdaError() + override fun onSessionVerificationRequired(sessionId: SessionId) = lambdaError() } val params = RoomSelectEntryPoint.Params( sessionId = A_SESSION_ID, diff --git a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt index 95f5fefa03..aa02171d29 100644 --- a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt +++ b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt @@ -17,6 +17,7 @@ import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.roomlist.RoomListFilter import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.api.user.MatrixUser +import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.matrix.test.A_ROOM_ID_2 @@ -27,6 +28,7 @@ import io.element.android.libraries.matrix.test.FakeMatrixClientProvider import io.element.android.libraries.matrix.test.room.aRoomSummary import io.element.android.libraries.matrix.test.roomlist.FakeDynamicRoomList import io.element.android.libraries.matrix.test.roomlist.FakeRoomListService +import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService import io.element.android.libraries.matrix.ui.model.toSelectRoomInfo import io.element.android.libraries.roomselect.api.RoomSelectEntryPoint import io.element.android.libraries.roomselect.api.RoomSelectMode @@ -39,6 +41,7 @@ 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.lambda.value import io.element.android.tests.testutils.test import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.collections.immutable.persistentListOf @@ -289,6 +292,80 @@ class RoomSelectPresenterTest { } } + @Test + fun `present - select another account which is not verified navigates to its verification`() = runTest { + val sessionVerificationService = FakeSessionVerificationService(SessionVerifiedStatus.NotVerified) + val navigateToSessionVerificationLambda = lambdaRecorder { } + val presenter = createRoomSelectPresenter( + navigator = RoomSelectNavigator(navigateToSessionVerificationLambda), + mode = RoomSelectMode.Share, + sessionStore = aSessionStoreWithTwoAccounts(), + matrixClientProvider = FakeMatrixClientProvider { sessionId -> + Result.success( + FakeMatrixClient( + sessionId = sessionId, + sessionVerificationService = if (sessionId == A_SESSION_ID_2) sessionVerificationService else FakeSessionVerificationService(), + ) + ) + }, + ) + presenter.test { + val state = awaitLastSequentialItem() + state.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID_2)) + consumeItemsUntilPredicate { it.selectedAccount.userId == A_SESSION_ID_2 } + testScheduler.advanceUntilIdle() + navigateToSessionVerificationLambda.assertions().isCalledOnce().with(value(A_SESSION_ID_2)) + // The session is verified, no new navigation + sessionVerificationService.emitVerifiedStatus(SessionVerifiedStatus.Verified) + testScheduler.advanceUntilIdle() + navigateToSessionVerificationLambda.assertions().isCalledOnce() + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - the initial session is not verified, no navigation to its verification`() = runTest { + val presenter = createRoomSelectPresenter( + mode = RoomSelectMode.Share, + matrixClientProvider = FakeMatrixClientProvider { sessionId -> + Result.success( + FakeMatrixClient( + sessionId = sessionId, + sessionVerificationService = FakeSessionVerificationService(SessionVerifiedStatus.NotVerified), + ) + ) + }, + ) + presenter.test { + // The default navigator fails if it is invoked + awaitLastSequentialItem() + testScheduler.advanceUntilIdle() + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - forward mode, no navigation to the verification of the session`() = runTest { + val presenter = createRoomSelectPresenter( + mode = RoomSelectMode.Forward, + initialSessionId = A_SESSION_ID_2, + matrixClientProvider = FakeMatrixClientProvider { sessionId -> + Result.success( + FakeMatrixClient( + sessionId = sessionId, + sessionVerificationService = FakeSessionVerificationService(SessionVerifiedStatus.NotVerified), + ) + ) + }, + ) + presenter.test { + // The default navigator fails if it is invoked + awaitLastSequentialItem() + testScheduler.advanceUntilIdle() + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - forward mode with several accounts does not show the account switch`() = runTest { val presenter = createRoomSelectPresenter( @@ -425,10 +502,12 @@ internal fun TestScope.createRoomSelectPresenter( matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider { sessionId -> Result.success(FakeMatrixClient(sessionId = sessionId, roomListService = roomListService)) }, + navigator: RoomSelectNavigator = RoomSelectNavigator { lambdaError() }, ) = RoomSelectPresenter( initialSessionId = initialSessionId, mode = mode, maxNumberOfRooms = maxNumberOfRooms, + navigator = navigator, dataSourceFactory = object : RoomSelectSearchDataSource.Factory { override fun create(coroutineScope: CoroutineScope, roomListService: RoomListService): RoomSelectSearchDataSource { return RoomSelectSearchDataSource(