From 281f90e1489f2ca5ec533318dde42feb8f6c6731 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 12 Apr 2023 12:22:13 +0200 Subject: [PATCH] Some clean up on room details --- .../io/element/android/appnav/RoomFlowNode.kt | 26 +++++++---- .../android/appnav/RoomFlowPresenter.kt | 46 ------------------- .../roomdetails/impl/RoomDetailsFlowNode.kt | 19 ++++---- .../roomdetails/impl/RoomDetailsNode.kt | 6 ++- .../libraries/matrix/api/room/MatrixRoom.kt | 2 +- .../matrix/impl/room/RustMatrixRoom.kt | 7 ++- .../matrix/test/room/FakeMatrixRoom.kt | 2 +- 7 files changed, 37 insertions(+), 71 deletions(-) delete mode 100644 appnav/src/main/kotlin/io/element/android/appnav/RoomFlowPresenter.kt diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt index 69c02d500e..be3b88a18c 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt @@ -18,8 +18,8 @@ package io.element.android.appnav import android.os.Parcelable import androidx.compose.runtime.Composable -import androidx.compose.runtime.DisposableEffect import androidx.compose.ui.Modifier +import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.composable.Children import com.bumble.appyx.core.lifecycle.subscribe import com.bumble.appyx.core.modality.BuildContext @@ -45,6 +45,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize import timber.log.Timber @@ -56,7 +57,6 @@ class RoomFlowNode @AssistedInject constructor( private val roomDetailsEntryPoint: RoomDetailsEntryPoint, private val appNavigationStateService: AppNavigationStateService, roomMembershipObserver: RoomMembershipObserver, - coroutineScope: CoroutineScope, ) : BackstackNode( backstack = BackStack( initialElement = NavTarget.Messages, @@ -76,9 +76,6 @@ class RoomFlowNode @AssistedInject constructor( ) : NodeInputs private val inputs: Inputs = inputs() - private val timeline = inputs.room.timeline() - - private val roomFlowPresenter = RoomFlowPresenter(inputs.room) init { lifecycle.subscribe( @@ -95,22 +92,34 @@ class RoomFlowNode @AssistedInject constructor( } ) + lifecycleScope.fetchRoomMembers() roomMembershipObserver.updates .filter { update -> update.roomId == inputs.room.roomId && !update.isUserInRoom } .onEach { navigateUp() } - .launchIn(coroutineScope) + .launchIn(lifecycleScope) + } + + private fun CoroutineScope.fetchRoomMembers() = launch { + val room = inputs.room + room.fetchMembers() + .onFailure { + Timber.e(it, "Fail to fetch members for room ${room.roomId}") + }.onSuccess { + Timber.v("Success fetching members for room ${room.roomId}") + } } override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { return when (navTarget) { NavTarget.Messages -> { - messagesEntryPoint.createNode(this, buildContext, object : MessagesEntryPoint.Callback { + val callback = object : MessagesEntryPoint.Callback { override fun onRoomDetailsClicked() { backstack.push(NavTarget.RoomDetails) } - }) + } + messagesEntryPoint.createNode(this, buildContext, callback) } NavTarget.RoomDetails -> { roomDetailsEntryPoint.createNode(this, buildContext, emptyList()) @@ -128,7 +137,6 @@ class RoomFlowNode @AssistedInject constructor( @Composable override fun View(modifier: Modifier) { - roomFlowPresenter.present() Children( navModel = backstack, modifier = modifier, diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowPresenter.kt b/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowPresenter.kt deleted file mode 100644 index 0a2b066da4..0000000000 --- a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowPresenter.kt +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright (c) 2023 New Vector Ltd - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.element.android.appnav - -import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect -import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.matrix.api.room.MatrixRoom -import timber.log.Timber - -class RoomFlowPresenter( - private val room: MatrixRoom, -) : Presenter { - - @Composable - override fun present(): RoomFlowState { - // Preload room members so we can quickly detect if the room is a DM room - LaunchedEffect(Unit) { - room.fetchMembers() - .onFailure { - Timber.e(it, "Fail to fetch members for room ${room.roomId}") - }.onSuccess { - Timber.v("Success fetching members for room ${room.roomId}") - } - } - - return RoomFlowState - } -} - -// At first the return type was Unit, but detekt complained about it -object RoomFlowState diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsFlowNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsFlowNode.kt index 4a46f3370c..a65449546e 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsFlowNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsFlowNode.kt @@ -56,19 +56,16 @@ class RoomDetailsFlowNode @AssistedInject constructor( object RoomMemberList : NavTarget } - interface Callback : Plugin { - fun openRoomMemberList() - } - - val callback = object : Callback { - override fun openRoomMemberList() { - backstack.push(NavTarget.RoomMemberList) - } - } - override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { return when (navTarget) { - NavTarget.RoomDetails -> createNode(buildContext, listOf(callback)) + NavTarget.RoomDetails -> { + val callback = object : RoomDetailsNode.Callback { + override fun openRoomMemberList() { + backstack.push(NavTarget.RoomMemberList) + } + } + createNode(buildContext, listOf(callback)) + } NavTarget.RoomMemberList -> createNode(buildContext) } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsNode.kt index 937d755df5..f234f835ca 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsNode.kt @@ -41,7 +41,11 @@ class RoomDetailsNode @AssistedInject constructor( private val room: MatrixRoom, ) : Node(buildContext, plugins = plugins) { - private val callback = plugins().firstOrNull() + interface Callback : Plugin { + fun openRoomMemberList() + } + + private val callback = plugins().firstOrNull() private fun openRoomMemberList() { callback?.openRoomMemberList() diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index 60ff09a15b..331dc70de0 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -56,5 +56,5 @@ interface MatrixRoom: Closeable { suspend fun redactEvent(eventId: EventId, reason: String? = null): Result - fun leave(): Result + suspend fun leave(): Result } diff --git 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 index fd5a63cb3b..948c4e9bd1 100644 --- 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 @@ -88,6 +88,7 @@ class RustMatrixRoom( coroutineDispatchers = coroutineDispatchers ) } + override fun close() { innerRoom.destroy() slidingSyncRoom.destroy() @@ -183,7 +184,9 @@ class RustMatrixRoom( } } - override fun leave(): Result { - return runCatching { innerRoom.leave() } + override suspend fun leave(): Result = withContext(coroutineDispatchers.io) { + runCatching { + innerRoom.leave() + } } } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index 1c6d580a3c..ffc7a60363 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -117,7 +117,7 @@ class FakeMatrixRoom( return Result.success(Unit) } - override fun leave(): Result = leaveRoomError?.let { Result.failure(it) } ?: Result.success(Unit) + override suspend fun leave(): Result = leaveRoomError?.let { Result.failure(it) } ?: Result.success(Unit) override fun close() = Unit