fix : use RoomMembershipObserver to close room screen when leaving/declining invite/canceling knock request

2024-11-18 16:14:08 +01:00
parent 1b33e1ac31
commit 62f5dc3bf0
8 changed files with 44 additions and 33 deletions
--- a/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt
+++ b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt
@@ -48,9 +48,11 @@ import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
 import io.element.android.libraries.matrix.api.core.toRoomIdOrAlias
 import io.element.android.libraries.matrix.api.getRoomInfoFlow
 import io.element.android.libraries.matrix.api.room.CurrentUserMembership
+import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
 import io.element.android.libraries.matrix.api.room.alias.ResolvedRoomAlias
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.launch
@@ -67,6 +69,7 @@ class RoomFlowNode @AssistedInject constructor(
    private val joinRoomEntryPoint: JoinRoomEntryPoint,
    private val roomAliasResolverEntryPoint: RoomAliasResolverEntryPoint,
    private val networkMonitor: NetworkMonitor,
+    private val membershipObserver: RoomMembershipObserver,
 ) : BaseFlowNode<RoomFlowNode.NavTarget>(
    backstack = BackStack(
        initialElement = NavTarget.Loading,
@@ -145,10 +148,6 @@ class RoomFlowNode @AssistedInject constructor(
                        backstack.newRoot(NavTarget.JoinedRoom(roomId))
                    }
                }
-                CurrentUserMembership.LEFT -> {
-                    // Left the room, navigate out of this flow
-                    navigateUp()
-                }
                else -> {
                    // Was invited or the room is not known, display the join room screen
                    backstack.newRoot(
@@ -161,6 +160,15 @@ class RoomFlowNode @AssistedInject constructor(
                }
            }
        }.launchIn(lifecycleScope)
+
+        // If the user leaves the room from this client, close the room flow.
+        lifecycleScope.launch {
+            membershipObserver.updates
+                .first { it.roomId == roomId && !it.isUserInRoom }
+                .run {
+                    navigateUp()
+                }
+        }
    }

    override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
--- a/features/joinroom/impl/src/main/kotlin/io/element/android/features/joinroom/impl/JoinRoomNode.kt
+++ b/features/joinroom/impl/src/main/kotlin/io/element/android/features/joinroom/impl/JoinRoomNode.kt
@@ -43,14 +43,14 @@ class JoinRoomNode @AssistedInject constructor(
            state = state,
            onBackClick = ::navigateUp,
            onJoinSuccess = ::navigateUp,
-            onCancelKnockSuccess = ::navigateUp,
-            onKnockSuccess = { },
+            onCancelKnockSuccess = {},
+            onKnockSuccess = {},
            modifier = modifier
        )
        acceptDeclineInviteView.Render(
            state = state.acceptDeclineInviteState,
            onAcceptInvite = {},
-            onDeclineInvite = { navigateUp() },
+            onDeclineInvite = {},
            modifier = Modifier
        )
    }
--- a/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenter.kt
+++ b/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenter.kt
@@ -22,7 +22,6 @@ import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.RoomId
-import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
 import io.element.android.libraries.matrix.api.room.isDm
 import kotlinx.coroutines.launch
 import timber.log.Timber
@@ -30,7 +29,6 @@ import javax.inject.Inject

 class LeaveRoomPresenter @Inject constructor(
    private val client: MatrixClient,
-    private val roomMembershipObserver: RoomMembershipObserver,
    private val dispatchers: CoroutineDispatchers,
 ) : Presenter<LeaveRoomState> {
    @Composable
@@ -58,7 +56,6 @@ class LeaveRoomPresenter @Inject constructor(
                is LeaveRoomEvent.LeaveRoom -> scope.launch(dispatchers.io) {
                    client.leaveRoom(
                        roomId = event.roomId,
-                        roomMembershipObserver = roomMembershipObserver,
                        confirmation = confirmation,
                        progress = progress,
                        error = error,
@@ -88,7 +85,6 @@ private suspend fun showLeaveRoomAlert(

 private suspend fun MatrixClient.leaveRoom(
    roomId: RoomId,
-    roomMembershipObserver: RoomMembershipObserver,
    confirmation: MutableState<LeaveRoomState.Confirmation>,
    progress: MutableState<LeaveRoomState.Progress>,
    error: MutableState<LeaveRoomState.Error>,
@@ -96,12 +92,11 @@ private suspend fun MatrixClient.leaveRoom(
    confirmation.value = LeaveRoomState.Confirmation.Hidden
    progress.value = LeaveRoomState.Progress.Shown
    getRoom(roomId)?.use { room ->
-        room.leave().onSuccess {
-            roomMembershipObserver.notifyUserLeftRoom(room.roomId)
-        }.onFailure {
-            Timber.e(it, "Error while leaving room ${room.displayName} - ${room.roomId}")
-            error.value = LeaveRoomState.Error.Shown
-        }
+        room.leave()
+            .onFailure {
+                Timber.e(it, "Error while leaving room ${room.roomId}")
+                error.value = LeaveRoomState.Error.Shown
+            }
    }
    progress.value = LeaveRoomState.Progress.Hidden
 }
--- a/features/leaveroom/impl/src/test/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterTest.kt
+++ b/features/leaveroom/impl/src/test/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterTest.kt
@@ -14,15 +14,15 @@ import com.google.common.truth.Truth.assertThat
 import io.element.android.features.leaveroom.api.LeaveRoomEvent
 import io.element.android.features.leaveroom.api.LeaveRoomState
 import io.element.android.libraries.matrix.api.MatrixClient
-import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
-import io.element.android.libraries.matrix.api.timeline.item.event.MembershipChange
 import io.element.android.libraries.matrix.test.A_ROOM_ID
 import io.element.android.libraries.matrix.test.FakeMatrixClient
 import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
 import io.element.android.tests.testutils.WarmUpRule
+import io.element.android.tests.testutils.lambda.assert
+import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.testCoroutineDispatchers
-import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.advanceUntilIdle
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
 import org.junit.Test
@@ -126,26 +126,27 @@ class LeaveRoomPresenterTest {

    @Test
    fun `present - leaving a room leaves the room`() = runTest {
-        val roomMembershipObserver = RoomMembershipObserver()
+        val leaveRoomLambda = lambdaRecorder<Result<Unit>> { Result.success(Unit) }
        val presenter = createLeaveRoomPresenter(
            client = FakeMatrixClient().apply {
                givenGetRoomResult(
                    roomId = A_ROOM_ID,
                    result = FakeMatrixRoom(
-                        leaveRoomLambda = { Result.success(Unit) }
+                        leaveRoomLambda = leaveRoomLambda
                    ),
                )
            },
-            roomMembershipObserver = roomMembershipObserver
        )
        moleculeFlow(RecompositionMode.Immediate) {
            presenter.present()
        }.test {
            val initialState = awaitItem()
            initialState.eventSink(LeaveRoomEvent.LeaveRoom(A_ROOM_ID))
-            // Membership observer should receive a 'left room' change
-            assertThat(roomMembershipObserver.updates.first().change).isEqualTo(MembershipChange.LEFT)
+            advanceUntilIdle()
            cancelAndIgnoreRemainingEvents()
+            assert(leaveRoomLambda)
+                .isCalledOnce()
+                .withNoParameter()
        }
    }

@@ -227,9 +228,7 @@ class LeaveRoomPresenterTest {

 private fun TestScope.createLeaveRoomPresenter(
    client: MatrixClient = FakeMatrixClient(),
-    roomMembershipObserver: RoomMembershipObserver = RoomMembershipObserver(),
 ): LeaveRoomPresenter = LeaveRoomPresenter(
    client = client,
-    roomMembershipObserver = roomMembershipObserver,
    dispatchers = testCoroutineDispatchers(false),
 )
--- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt
+++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt
@@ -178,6 +178,8 @@ class RustMatrixClient(
        sessionCoroutineScope = sessionCoroutineScope,
    )

+    private val roomMembershipObserver = RoomMembershipObserver()
+
    private val roomFactory = RustRoomFactory(
        roomListService = roomListService,
        innerRoomListService = innerRoomListService,
@@ -191,6 +193,7 @@ class RustMatrixClient(
        roomSyncSubscriber = roomSyncSubscriber,
        timelineEventTypeFilterFactory = timelineEventTypeFilterFactory,
        featureFlagService = featureFlagService,
+        roomMembershipObserver = roomMembershipObserver,
    )

    override val mediaLoader: MatrixMediaLoader = RustMediaLoader(
@@ -199,8 +202,6 @@ class RustMatrixClient(
        innerClient = client,
    )

-    private val roomMembershipObserver = RoomMembershipObserver()
-
    private var clientDelegateTaskHandle: TaskHandle? = client.setDelegate(sessionDelegate)

    private val _userProfile: MutableStateFlow<MatrixUser> = MutableStateFlow(
--- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt
+++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt
@@ -34,6 +34,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState
 import io.element.android.libraries.matrix.api.room.MatrixRoomNotificationSettingsState
 import io.element.android.libraries.matrix.api.room.MessageEventType
 import io.element.android.libraries.matrix.api.room.RoomMember
+import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
 import io.element.android.libraries.matrix.api.room.StateEventType
 import io.element.android.libraries.matrix.api.room.draft.ComposerDraft
 import io.element.android.libraries.matrix.api.room.location.AssetType
@@ -58,7 +59,6 @@ import io.element.android.libraries.matrix.impl.widget.RustWidgetDriver
 import io.element.android.libraries.matrix.impl.widget.generateWidgetWebViewUrl
 import io.element.android.services.toolbox.api.systemclock.SystemClock
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -90,7 +90,6 @@ import org.matrix.rustcomponents.sdk.IdentityStatusChange as RustIdentityStateCh
 import org.matrix.rustcomponents.sdk.Room as InnerRoom
 import org.matrix.rustcomponents.sdk.Timeline as InnerTimeline

-@OptIn(ExperimentalCoroutinesApi::class)
 class RustMatrixRoom(
    override val sessionId: SessionId,
    private val deviceId: DeviceId,
@@ -105,6 +104,7 @@ class RustMatrixRoom(
    private val roomSyncSubscriber: RoomSyncSubscriber,
    private val matrixRoomInfoMapper: MatrixRoomInfoMapper,
    private val featureFlagService: FeatureFlagService,
+    private val roomMembershipObserver: RoomMembershipObserver,
 ) : MatrixRoom {
    override val roomId = RoomId(innerRoom.id())

@@ -374,6 +374,8 @@ class RustMatrixRoom(
    override suspend fun leave(): Result<Unit> = withContext(roomDispatcher) {
        runCatching {
            innerRoom.leave()
+        }.onSuccess {
+            roomMembershipObserver.notifyUserLeftRoom(roomId)
        }
    }

--- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustPendingRoom.kt
+++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustPendingRoom.kt
@@ -10,15 +10,19 @@ package io.element.android.libraries.matrix.impl.room
 import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.api.room.PendingRoom
+import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
 import org.matrix.rustcomponents.sdk.RoomPreview

 class RustPendingRoom(
    override val sessionId: SessionId,
    override val roomId: RoomId,
    private val inner: RoomPreview,
+    private val roomMembershipObserver: RoomMembershipObserver,
 ) : PendingRoom {
    override suspend fun leave(): Result<Unit> = runCatching {
        inner.leave()
+    }.onSuccess {
+        roomMembershipObserver.notifyUserLeftRoom(roomId)
    }

    override fun close() {
--- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt
+++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt
@@ -17,13 +17,13 @@ import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService
 import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.room.PendingRoom
+import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
 import io.element.android.libraries.matrix.api.roomlist.RoomListService
 import io.element.android.libraries.matrix.api.roomlist.awaitLoaded
 import io.element.android.libraries.matrix.impl.roomlist.fullRoomWithTimeline
 import io.element.android.libraries.matrix.impl.roomlist.roomOrNull
 import io.element.android.services.toolbox.api.systemclock.SystemClock
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
@@ -51,8 +51,8 @@ class RustRoomFactory(
    private val roomSyncSubscriber: RoomSyncSubscriber,
    private val timelineEventTypeFilterFactory: TimelineEventTypeFilterFactory,
    private val featureFlagService: FeatureFlagService,
+    private val roomMembershipObserver: RoomMembershipObserver,
 ) {
-    @OptIn(ExperimentalCoroutinesApi::class)
    private val dispatcher = dispatchers.io.limitedParallelism(1)
    private val mutex = Mutex()
    private var isDestroyed: Boolean = false
@@ -120,6 +120,7 @@ class RustRoomFactory(
                roomSyncSubscriber = roomSyncSubscriber,
                matrixRoomInfoMapper = matrixRoomInfoMapper,
                featureFlagService = featureFlagService,
+                roomMembershipObserver = roomMembershipObserver,
            )
        }
    }
@@ -148,6 +149,7 @@ class RustRoomFactory(
            sessionId = sessionId,
            roomId = roomId,
            inner = innerRoom,
+            roomMembershipObserver = roomMembershipObserver,
        )
    }