From 5104fc8ac1aa260b85da86653d9d16f4247e8cea Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 20 Apr 2023 15:58:20 +0200 Subject: [PATCH 01/13] Fix bottomsheet not using right theme (BottomSheet is not part of material3) --- .../features/messages/impl/actionlist/ActionListView.kt | 9 ++++++++- .../theme/components/ModalBottomSheetLayout.kt | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListView.kt index dc71d1d834..c2becf71cf 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListView.kt @@ -102,6 +102,7 @@ private fun SheetContent( // Crashes if sheetContent size is zero Box(modifier = modifier.size(1.dp)) } + is ActionListState.Target.Success -> { val actions = target.actions LazyColumn( @@ -146,5 +147,11 @@ fun SheetContentDarkPreview(@PreviewParameter(ActionListStateProvider::class) st @Composable private fun ContentToPreview(state: ActionListState) { - SheetContent(state) + ActionListView( + state = state, + modalBottomSheetState = ModalBottomSheetState( + initialValue = ModalBottomSheetValue.Expanded + ), + onActionSelected = { _, _ -> } + ) } diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/ModalBottomSheetLayout.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/ModalBottomSheetLayout.kt index 7c498a610c..d897f6ad4e 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/ModalBottomSheetLayout.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/ModalBottomSheetLayout.kt @@ -21,12 +21,12 @@ package io.element.android.libraries.designsystem.theme.components import androidx.compose.foundation.background import androidx.compose.foundation.layout.ColumnScope import androidx.compose.material.ExperimentalMaterialApi -import androidx.compose.material.MaterialTheme import androidx.compose.material.ModalBottomSheetDefaults import androidx.compose.material.ModalBottomSheetState import androidx.compose.material.ModalBottomSheetValue -import androidx.compose.material.contentColorFor import androidx.compose.material.rememberModalBottomSheetState +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.contentColorFor import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -44,7 +44,7 @@ fun ModalBottomSheetLayout( sheetState: ModalBottomSheetState = rememberModalBottomSheetState(ModalBottomSheetValue.Hidden), sheetShape: Shape = MaterialTheme.shapes.large, sheetElevation: Dp = ModalBottomSheetDefaults.Elevation, - sheetBackgroundColor: Color = MaterialTheme.colors.surface, + sheetBackgroundColor: Color = MaterialTheme.colorScheme.surface, sheetContentColor: Color = contentColorFor(sheetBackgroundColor), scrimColor: Color = ModalBottomSheetDefaults.scrimColor, content: @Composable () -> Unit = {} From a1869a30192ef23c94ea14cf18c0622bfe8b377c Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 20 Apr 2023 18:21:47 +0200 Subject: [PATCH 02/13] Improve handling members --- .../io/element/android/appnav/RoomFlowNode.kt | 7 ++- .../android/appnav/RoomFlowNodeTest.kt | 14 ----- .../impl/root/CreateRoomRootPresenterTests.kt | 2 +- .../event/TimelineItemEventFactory.kt | 2 + .../roomdetails/impl/RoomDetailsPresenter.kt | 27 +++++----- .../roomdetails/impl/di/RoomMemberModules.kt | 9 ---- .../impl/members/RoomMemberListNode.kt | 1 + .../impl/members/RoomUserListDataSource.kt | 2 +- .../roomdetails/RoomDetailsPresenterTests.kt | 25 +++++---- .../libraries/matrix/api/room/MatrixRoom.kt | 39 ++++++++++---- .../libraries/matrix/impl/RustMatrixClient.kt | 2 +- .../matrix/impl/room/RustMatrixRoom.kt | 47 ++++------------- .../impl/timeline/RustMatrixTimeline.kt | 16 +++++- .../libraries/matrix/test/FakeMatrixClient.kt | 2 +- .../matrix/test/room/FakeMatrixRoom.kt | 51 ++++++------------- 15 files changed, 107 insertions(+), 139 deletions(-) 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 22929e3d66..4eaa5b83df 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt @@ -41,7 +41,6 @@ import io.element.android.libraries.di.SessionScope import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMembershipObserver import io.element.android.services.appnavstate.api.AppNavigationStateService -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach @@ -102,12 +101,18 @@ class RoomFlowNode @AssistedInject constructor( private fun fetchRoomMembers() = lifecycleScope.launch { val room = inputs.room + /* room.fetchMembers() + .map { + room.updateMembers() + } .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 { diff --git a/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt index ef611a2f4b..a151be665c 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt @@ -24,7 +24,6 @@ import com.bumble.appyx.core.node.node import com.bumble.appyx.core.plugin.Plugin import com.bumble.appyx.navmodel.backstack.activeElement import com.bumble.appyx.testing.junit4.util.MainDispatcherRule -import com.bumble.appyx.testing.unit.common.helper.nodeTestHelper import com.bumble.appyx.testing.unit.common.helper.parentNodeTestHelper import com.google.common.truth.Truth import io.element.android.features.messages.api.MessagesEntryPoint @@ -81,19 +80,6 @@ class RoomFlowNodeTest { roomMembershipObserver = RoomMembershipObserver() ) - @Test - fun `given a room flow node when initialized then it fetches room members`() { - // GIVEN - val room = FakeMatrixRoom() - val inputs = RoomFlowNode.Inputs(room) - val roomFlowNode = aRoomFlowNode(listOf(inputs)) - Truth.assertThat(room.areMembersFetched).isFalse() - // WHEN - roomFlowNode.nodeTestHelper() - // THEN - Truth.assertThat(room.areMembersFetched).isTrue() - } - @Test fun `given a room flow node when initialized then it loads messages entry point`() { // GIVEN diff --git a/features/createroom/impl/src/test/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenterTests.kt b/features/createroom/impl/src/test/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenterTests.kt index 64d59b221c..db7cd06170 100644 --- a/features/createroom/impl/src/test/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenterTests.kt +++ b/features/createroom/impl/src/test/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenterTests.kt @@ -102,7 +102,7 @@ class CreateRoomRootPresenterTests { }.test { val initialState = awaitItem() val matrixUser = MatrixUser(UserId("@name:domain")) - val fakeDmResult = FakeMatrixRoom(RoomId("!fakeDmResult:domain")) + val fakeDmResult = FakeMatrixRoom(roomId = RoomId("!fakeDmResult:domain")) fakeMatrixClient.givenFindDmResult(fakeDmResult) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt index c21622b50f..c62719ae51 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt @@ -25,6 +25,7 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails import kotlinx.collections.immutable.toImmutableList +import timber.log.Timber import javax.inject.Inject class TimelineItemEventFactory @Inject constructor( @@ -42,6 +43,7 @@ class TimelineItemEventFactory @Inject constructor( val senderDisplayName: String? val senderAvatarUrl: String? + Timber.v("SenderProfile($currentSender) = ${currentTimelineItem.event.senderProfile}") when (val senderProfile = currentTimelineItem.event.senderProfile) { ProfileTimelineDetails.Unavailable, ProfileTimelineDetails.Pending, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index 45e978b80a..969e2aa86b 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -18,6 +18,7 @@ package io.element.android.features.roomdetails.impl import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.MutableState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -25,17 +26,16 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import io.element.android.libraries.architecture.Async import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.matrix.api.MatrixClient -import io.element.android.libraries.matrix.api.core.SessionId +import io.element.android.libraries.architecture.executeResult import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMembershipObserver +import io.element.android.libraries.matrix.api.room.getDmMember +import io.element.android.libraries.matrix.api.room.memberCount import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import javax.inject.Inject class RoomDetailsPresenter @Inject constructor( - private val sessionId: SessionId, private val room: MatrixRoom, private val roomMembershipObserver: RoomMembershipObserver, ) : Presenter { @@ -50,15 +50,14 @@ class RoomDetailsPresenter @Inject constructor( mutableStateOf(null) } - var memberCount: Async by remember { mutableStateOf(Async.Loading()) } + val memberCount: MutableState> = remember { + mutableStateOf(Async.Uninitialized) + } LaunchedEffect(Unit) { - withContext(Dispatchers.IO) { - memberCount = runCatching { room.memberCount() } - .fold( - onSuccess = { Async.Success(it) }, - onFailure = { Async.Failure(it) } - ) - } + suspend { + room.updateMembers() + .map { room.memberCount() } + }.executeResult(memberCount) } val dmMember = room.getDmMember() @@ -72,7 +71,7 @@ class RoomDetailsPresenter @Inject constructor( when (event) { is RoomDetailsEvent.LeaveRoom -> { if (event.needsConfirmation) { - leaveRoomWarning = LeaveRoomWarning.computeLeaveRoomWarning(room.isPublic, memberCount) + leaveRoomWarning = LeaveRoomWarning.computeLeaveRoomWarning(room.isPublic, memberCount.value) } else { coroutineScope.launch(Dispatchers.IO) { room.leave() @@ -96,7 +95,7 @@ class RoomDetailsPresenter @Inject constructor( roomAlias = room.alias, roomAvatarUrl = room.avatarUrl, roomTopic = room.topic, - memberCount = memberCount, + memberCount = memberCount.value, isEncrypted = room.isEncrypted, displayLeaveRoomWarning = leaveRoomWarning, error = error, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt index 68f77b5821..aeb42893e8 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt @@ -44,15 +44,6 @@ interface RoomMemberBindsModule { @ContributesTo(RoomScope::class) object RoomMemberProvidesModule { - @Provides - fun provideRoomDetailsPresenter( - matrixClient: MatrixClient, - room: MatrixRoom, - roomMembershipObserver: RoomMembershipObserver, - ): RoomDetailsPresenter { - return RoomDetailsPresenter(matrixClient.sessionId, room, roomMembershipObserver) - } - @Provides fun provideRoomMemberDetailsPresenterFactory( room: MatrixRoom, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt index fafe0ede99..60ba07d1ee 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt @@ -28,6 +28,7 @@ import io.element.android.anvilannotations.ContributesNode import io.element.android.libraries.di.RoomScope import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMember +import io.element.android.libraries.matrix.api.room.getMember import io.element.android.libraries.matrix.ui.model.MatrixUser import timber.log.Timber diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt index dc0008d2a3..6bf1203e83 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt @@ -30,7 +30,7 @@ class RoomUserListDataSource @Inject constructor( ) : UserListDataSource { override suspend fun search(query: String): List { - return room.members().filter { member -> + return room.membersFlow.value.filter { member -> if (query.isBlank()) { true } else { diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index 66e74cd5cc..1d25a27e44 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -32,7 +32,6 @@ import io.element.android.libraries.matrix.api.room.RoomMembershipState 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.A_ROOM_NAME -import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.A_USER_ID import io.element.android.libraries.matrix.test.room.FakeMatrixRoom import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -50,7 +49,7 @@ class RoomDetailsPresenterTests { @Test fun `present - initial state is created from room info`() = runTest { val room = aMatrixRoom() - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -59,7 +58,7 @@ class RoomDetailsPresenterTests { Truth.assertThat(initialState.roomName).isEqualTo(room.name) Truth.assertThat(initialState.roomAvatarUrl).isEqualTo(room.avatarUrl) Truth.assertThat(initialState.roomTopic).isEqualTo(room.topic) - Truth.assertThat(initialState.memberCount).isEqualTo(Async.Loading(null)) + Truth.assertThat(initialState.memberCount).isEqualTo(Async.Uninitialized) Truth.assertThat(initialState.isEncrypted).isEqualTo(room.isEncrypted) cancelAndIgnoreRemainingEvents() @@ -69,12 +68,12 @@ class RoomDetailsPresenterTests { @Test fun `present - room member count is calculated asynchronously`() = runTest { val room = aMatrixRoom() - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { val initialState = awaitItem() - Truth.assertThat(initialState.memberCount).isEqualTo(Async.Loading(null)) + Truth.assertThat(initialState.memberCount).isEqualTo(Async.Uninitialized) val finalState = awaitItem() Truth.assertThat(finalState.memberCount).isEqualTo(Async.Success(0)) @@ -84,7 +83,7 @@ class RoomDetailsPresenterTests { @Test fun `present - initial state with no room name`() = runTest { val room = aMatrixRoom(name = null) - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -100,7 +99,7 @@ class RoomDetailsPresenterTests { val room = aMatrixRoom(name = null).apply { givenFetchMemberResult(Result.failure(Throwable())) } - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -114,7 +113,7 @@ class RoomDetailsPresenterTests { @Test fun `present - Leave with confirmation on private room shows a specific warning`() = runTest { val room = aMatrixRoom(isPublic = false) - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -131,7 +130,7 @@ class RoomDetailsPresenterTests { @Test fun `present - Leave with confirmation on empty room shows a specific warning`() = runTest { val room = aMatrixRoom(members = listOf(aRoomMember())) - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -148,7 +147,7 @@ class RoomDetailsPresenterTests { @Test fun `present - Leave with confirmation shows a generic warning`() = runTest { val room = aMatrixRoom() - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -165,7 +164,7 @@ class RoomDetailsPresenterTests { @Test fun `present - Leave without confirmation leaves the room`() = runTest { val room = aMatrixRoom() - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -189,7 +188,7 @@ class RoomDetailsPresenterTests { val room = aMatrixRoom().apply { givenLeaveRoomError(Throwable()) } - val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -218,10 +217,10 @@ fun aMatrixRoom( ) = FakeMatrixRoom( roomId = roomId, name = name, + initialMembers = members, displayName = displayName, topic = topic, avatarUrl = avatarUrl, - members = members, isEncrypted = isEncrypted, isPublic = isPublic, ) 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 d4338bb497..fae6550a4f 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 @@ -18,12 +18,15 @@ package io.element.android.libraries.matrix.api.room 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.SessionId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.StateFlow import java.io.Closeable -interface MatrixRoom: Closeable { +interface MatrixRoom : Closeable { + val sessionId: SessionId val roomId: RoomId val name: String? val bestName: String @@ -36,20 +39,22 @@ interface MatrixRoom: Closeable { val isDirect: Boolean val isPublic: Boolean - suspend fun members() : List + /** + * The current loaded members as a StateFlow. + * Initial value is an emptyList. + * To update them you should call [updateMembers]. + */ + val membersFlow: StateFlow> - suspend fun memberCount(): Int - - fun getMember(userId: UserId): RoomMember? - - fun getDmMember(): RoomMember? + /** + * Try to load the room members and update the membersFlow. + */ + suspend fun updateMembers(): Result fun syncUpdateFlow(): Flow fun timeline(): MatrixTimeline - suspend fun fetchMembers(): Result - suspend fun userDisplayName(userId: UserId): Result suspend fun userAvatarUrl(userId: UserId): Result @@ -64,3 +69,19 @@ interface MatrixRoom: Closeable { suspend fun leave(): Result } + +fun MatrixRoom.getMember(userId: UserId): RoomMember? { + return membersFlow.value.find { it.userId == userId } +} + +fun MatrixRoom.getDmMember(): RoomMember? { + return if (membersFlow.value.size == 2 && isDirect && isEncrypted) { + membersFlow.value.find { it.userId != this.sessionId } + } else { + null + } +} + +fun MatrixRoom.memberCount(): Int { + return membersFlow.value.size +} diff --git 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 index 63bd4e13a2..2a7e7c0826 100644 --- 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 @@ -198,7 +198,7 @@ class RustMatrixClient constructor( val slidingSyncRoom = slidingSync.getRoom(roomId.value) ?: return null val fullRoom = slidingSyncRoom.fullRoom() ?: return null return RustMatrixRoom( - currentUserId = sessionId, + sessionId = sessionId, slidingSyncUpdateFlow = slidingSyncObserverProxy.updateSummaryFlow, slidingSyncRoom = slidingSyncRoom, innerRoom = fullRoom, 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 cc0eac2e1f..1b87d271dc 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 @@ -17,21 +17,21 @@ package io.element.android.libraries.matrix.impl.room import io.element.android.libraries.core.coroutine.CoroutineDispatchers -import io.element.android.libraries.core.data.tryOrNull 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.SessionId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMember import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.impl.timeline.RustMatrixTimeline import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onStart -import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.Room import org.matrix.rustcomponents.sdk.SlidingSyncRoom @@ -40,7 +40,7 @@ import org.matrix.rustcomponents.sdk.genTransactionId import org.matrix.rustcomponents.sdk.messageEventContentFromMarkdown class RustMatrixRoom( - private val currentUserId: UserId, + override val sessionId: SessionId, private val slidingSyncUpdateFlow: Flow, private val slidingSyncRoom: SlidingSyncRoom, private val innerRoom: Room, @@ -48,39 +48,10 @@ class RustMatrixRoom( private val coroutineDispatchers: CoroutineDispatchers, ) : MatrixRoom { - private var loadMembersJob: Job? = null - private var cachedMembers: List = emptyList() + override val membersFlow: StateFlow> + get() = cachedMembers - override suspend fun members(): List { - return cachedMembers.ifEmpty { - if (loadMembersJob == null) { - loadMembersJob = coroutineScope.launch(coroutineDispatchers.io) { - cachedMembers = tryOrNull { - innerRoom.members().map(RoomMemberMapper::map) - } ?: emptyList() - } - } - loadMembersJob?.join() - loadMembersJob = null - cachedMembers - } - } - - override suspend fun memberCount(): Int { - return members().size - } - - override fun getMember(userId: UserId): RoomMember? { - return cachedMembers.find { it.userId == userId } - } - - override fun getDmMember(): RoomMember? { - return if (cachedMembers.size == 2 && isDirect && isEncrypted) { - cachedMembers.find { it.userId != currentUserId } - } else { - null - } - } + private var cachedMembers = MutableStateFlow>(emptyList()) override fun syncUpdateFlow(): Flow { return slidingSyncUpdateFlow @@ -150,9 +121,9 @@ class RustMatrixRoom( override val isDirect: Boolean get() = innerRoom.isDirect() - override suspend fun fetchMembers(): Result = withContext(coroutineDispatchers.io) { + override suspend fun updateMembers(): Result = withContext(coroutineDispatchers.io) { runCatching { - innerRoom.fetchMembers() + cachedMembers.value = innerRoom.members().map(RoomMemberMapper::map) } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt index e942f31b76..17e865f9fb 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt @@ -139,10 +139,24 @@ class RustMatrixTimeline( private suspend fun addListener(timelineListener: TimelineListener): Result> = withContext(coroutineDispatchers.io) { runCatching { - val settings = RoomSubscription(requiredState = listOf(RequiredState(key = "m.room.canonical_alias", value = "")), timelineLimit = null) + val settings = RoomSubscription( + requiredState = listOf( + RequiredState(key = "m.room.canonical_alias", value = ""), + RequiredState(key = "m.room.topic", value = ""), + RequiredState(key = "m.room.join_rule", value = ""), + ), + timelineLimit = 20.toUInt() + ) val result = slidingSyncRoom.subscribeAndAddTimelineListener(timelineListener, settings) + fetchMembers() listenerTokens += result.taskHandle result.items } } + + private suspend fun fetchMembers() = withContext(coroutineDispatchers.io) { + runCatching { + innerRoom.fetchMembers() + } + } } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt index c2a234e3de..2c08023f97 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt @@ -54,7 +54,7 @@ class FakeMatrixClient( private var logoutFailure: Throwable? = null override fun getRoom(roomId: RoomId): MatrixRoom? { - return FakeMatrixRoom(roomId) + return FakeMatrixRoom(sessionId = sessionId, roomId = roomId) } override fun findDM(userId: UserId): MatrixRoom? { 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 902f41a80c..b80b84a666 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 @@ -18,17 +18,21 @@ package io.element.android.libraries.matrix.test.room 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.SessionId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMember -import io.element.android.libraries.matrix.test.A_ROOM_ID -import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline import io.element.android.libraries.matrix.api.timeline.MatrixTimeline +import io.element.android.libraries.matrix.test.A_ROOM_ID +import io.element.android.libraries.matrix.test.A_SESSION_ID +import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.emptyFlow class FakeMatrixRoom( + override val sessionId: SessionId = A_SESSION_ID, override val roomId: RoomId = A_ROOM_ID, override val name: String? = null, override val bestName: String = "", @@ -40,20 +44,23 @@ class FakeMatrixRoom( override val alternativeAliases: List = emptyList(), override val isPublic: Boolean = true, override val isDirect: Boolean = false, - private val members: List = emptyList(), + initialMembers: List = emptyList(), private val matrixTimeline: MatrixTimeline = FakeMatrixTimeline(), ) : MatrixRoom { private var userDisplayNameResult = Result.success(null) private var userAvatarUrlResult = Result.success(null) private var dmMember: RoomMember? = null - private var fetchMemberResult: Result = Result.success(Unit) - - var areMembersFetched: Boolean = false - private set + private var updateMembersResult: Result = Result.success(Unit) private var leaveRoomError: Throwable? = null + override val membersFlow: MutableStateFlow> = MutableStateFlow(initialMembers) + + override suspend fun updateMembers(): Result { + return updateMembersResult + } + override fun syncUpdateFlow(): Flow { return emptyFlow() } @@ -62,18 +69,6 @@ class FakeMatrixRoom( return matrixTimeline } - override suspend fun fetchMembers(): Result { - return fetchMemberResult.also { result -> - if (result.isSuccess) { - areMembersFetched = true - } - } - } - - override fun getDmMember(): RoomMember? { - return dmMember - } - override suspend fun userDisplayName(userId: UserId): Result { return userDisplayNameResult } @@ -82,22 +77,6 @@ class FakeMatrixRoom( return userAvatarUrlResult } - override suspend fun members(): List { - return members - } - - override suspend fun memberCount(): Int { - if (fetchMemberResult.isSuccess) { - return members.count() - } else { - throw fetchMemberResult.exceptionOrNull()!! - } - } - - override fun getMember(userId: UserId): RoomMember? { - return members.firstOrNull { it.userId == userId } - } - override suspend fun sendMessage(message: String): Result { delay(100) return Result.success(Unit) @@ -139,7 +118,7 @@ class FakeMatrixRoom( } fun givenFetchMemberResult(result: Result) { - fetchMemberResult = result + updateMembersResult = result } fun givenDmMember(roomMember: RoomMember) { From 90bfe9725061fdf5731f3b508b0d1203a94ac250 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 21 Apr 2023 14:39:47 +0200 Subject: [PATCH 03/13] Room : continue improving members loading --- .../io/element/android/appnav/RoomFlowNode.kt | 8 +- features/messages/impl/build.gradle.kts | 1 + .../messages/impl/timeline/TimelineView.kt | 26 ++-- .../event/TimelineItemEventFactory.kt | 2 - .../messages/MessagesPresenterTest.kt | 1 - .../messages/fixtures/timelineItemsFactory.kt | 4 + features/roomdetails/impl/build.gradle.kts | 1 + .../roomdetails/impl/RoomDetailsPresenter.kt | 115 +++++++++++------- .../impl/members/RoomMemberListEvents.kt} | 15 +-- .../impl/members/RoomMemberListNode.kt | 14 +-- .../impl/members/RoomMemberListPresenter.kt | 32 ++++- .../impl/members/RoomMemberListState.kt | 4 +- .../members/RoomMemberListStateProvider.kt | 1 + .../impl/members/RoomMemberListView.kt | 17 ++- .../impl/members/RoomUserListDataSource.kt | 29 +++-- .../roomdetails/RoomDetailsPresenterTests.kt | 60 ++++----- .../members/RoomMemberListPresenterTests.kt | 15 ++- .../userlist/api/UserListDataSource.kt | 1 + .../android/libraries/architecture/Async.kt | 4 +- .../libraries/matrix/api/room/MatrixRoom.kt | 30 +++-- .../matrix/api/room/MatrixRoomMembersState.kt | 31 +++++ .../matrix/impl/room/RustMatrixRoom.kt | 17 ++- .../impl/timeline/RustMatrixTimeline.kt | 5 +- .../matrix/test/room/FakeMatrixRoom.kt | 14 +-- tests/testutils/build.gradle.kts | 2 + .../testutils/TestCoroutineDispatchers.kt | 46 +++++++ 26 files changed, 329 insertions(+), 166 deletions(-) rename features/{messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/testCoroutineDispatchers.kt => roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListEvents.kt} (56%) create mode 100644 libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomMembersState.kt create mode 100644 tests/testutils/src/main/kotlin/io/element/android/tests/testutils/TestCoroutineDispatchers.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 4eaa5b83df..7cd33c2958 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt @@ -101,18 +101,12 @@ class RoomFlowNode @AssistedInject constructor( private fun fetchRoomMembers() = lifecycleScope.launch { val room = inputs.room - /* - room.fetchMembers() - .map { - room.updateMembers() - } + room.updateMembers() .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 { diff --git a/features/messages/impl/build.gradle.kts b/features/messages/impl/build.gradle.kts index 9b43d41d4d..b695cc5512 100644 --- a/features/messages/impl/build.gradle.kts +++ b/features/messages/impl/build.gradle.kts @@ -55,6 +55,7 @@ dependencies { testImplementation(projects.libraries.matrix.test) testImplementation(projects.libraries.dateformatter.test) testImplementation(projects.features.networkmonitor.test) + testImplementation(projects.tests.testutils) androidTestImplementation(libs.test.junitext) ksp(libs.showkase.processor) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt index 7ea427a8cf..c7b071e069 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt @@ -75,6 +75,7 @@ import io.element.android.libraries.designsystem.theme.components.Text import kotlinx.collections.immutable.ImmutableList import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch +import timber.log.Timber @Composable fun TimelineView( @@ -100,11 +101,11 @@ fun TimelineView( itemsIndexed( items = state.timelineItems, contentType = { _, timelineItem -> timelineItem.contentType() }, - key = { _, timelineItem -> timelineItem.key() }, + key = { _, timelineItem -> timelineItem.identifier() }, ) { index, timelineItem -> TimelineItemRow( timelineItem = timelineItem, - isHighlighted = timelineItem.key() == state.highlightedEventId?.value, + isHighlighted = timelineItem.identifier() == state.highlightedEventId?.value, onClick = onMessageClicked, onLongClick = onMessageLongClicked ) @@ -114,27 +115,22 @@ fun TimelineView( } } + /* TimelineScrollHelper( lazyListState = lazyListState, timelineItems = state.timelineItems, onLoadMore = ::onReachedLoadMore ) + + */ } } -private fun TimelineItem.key(): String { - return when (this) { - is TimelineItem.Event -> id - is TimelineItem.Virtual -> id - } -} - -private fun TimelineItem.contentType(): Int { - // Todo optimize for each subtype - return when (this) { - is TimelineItem.Event -> 0 - is TimelineItem.Virtual -> 1 - } +private fun TimelineItem.contentType() = when (this) { + is TimelineItem.Event -> content.javaClass.simpleName + is TimelineItem.Virtual -> model.javaClass.simpleName +}.also { + Timber.v("ContentType = $it") } @Composable diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt index c62719ae51..c21622b50f 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt @@ -25,7 +25,6 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails import kotlinx.collections.immutable.toImmutableList -import timber.log.Timber import javax.inject.Inject class TimelineItemEventFactory @Inject constructor( @@ -43,7 +42,6 @@ class TimelineItemEventFactory @Inject constructor( val senderDisplayName: String? val senderAvatarUrl: String? - Timber.v("SenderProfile($currentSender) = ${currentTimelineItem.event.senderProfile}") when (val senderProfile = currentTimelineItem.event.senderProfile) { ProfileTimelineDetails.Unavailable, ProfileTimelineDetails.Pending, diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt index a7bc112174..bc898d5159 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt @@ -131,7 +131,6 @@ class MessagesPresenterTest { appCoroutineScope = this, room = matrixRoom ) - val timelinePresenter = TimelinePresenter( timelineItemsFactory = aTimelineItemsFactory(), room = matrixRoom, diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/timelineItemsFactory.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/timelineItemsFactory.kt index 404e50a3bd..2d4ee3842c 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/timelineItemsFactory.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/timelineItemsFactory.kt @@ -14,6 +14,8 @@ * limitations under the License. */ +@file:OptIn(ExperimentalCoroutinesApi::class) + package io.element.android.features.messages.fixtures import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory @@ -31,6 +33,8 @@ import io.element.android.features.messages.impl.timeline.factories.event.Timeli import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemDaySeparatorFactory import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemVirtualFactory import io.element.android.libraries.dateformatter.test.FakeDaySeparatorFormatter +import io.element.android.tests.testutils.testCoroutineDispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi internal fun aTimelineItemsFactory() = TimelineItemsFactory( dispatchers = testCoroutineDispatchers(), diff --git a/features/roomdetails/impl/build.gradle.kts b/features/roomdetails/impl/build.gradle.kts index dc840b038f..41f117d6a4 100644 --- a/features/roomdetails/impl/build.gradle.kts +++ b/features/roomdetails/impl/build.gradle.kts @@ -53,6 +53,7 @@ dependencies { testImplementation(projects.libraries.matrix.test) testImplementation(projects.features.userlist.impl) testImplementation(projects.features.userlist.test) + testImplementation(projects.tests.testutils) ksp(libs.showkase.processor) } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index 969e2aa86b..9b6a1916c7 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -17,75 +17,60 @@ package io.element.android.features.roomdetails.impl import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState +import androidx.compose.runtime.State +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope -import androidx.compose.runtime.setValue import io.element.android.libraries.architecture.Async import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.architecture.executeResult +import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState +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.getDmMember -import io.element.android.libraries.matrix.api.room.memberCount -import kotlinx.coroutines.Dispatchers +import io.element.android.libraries.matrix.api.room.getDmMemberFlow +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import javax.inject.Inject class RoomDetailsPresenter @Inject constructor( private val room: MatrixRoom, private val roomMembershipObserver: RoomMembershipObserver, + private val coroutineDispatchers: CoroutineDispatchers, ) : Presenter { @Composable override fun present(): RoomDetailsState { val coroutineScope = rememberCoroutineScope() - var leaveRoomWarning by remember { + val leaveRoomWarning = remember { mutableStateOf(null) } - var error by remember { + val error = remember { mutableStateOf(null) } + val membersState by room.membersStateFlow.collectAsState() + val memberCount by getMemberCount(membersState) + val dmMemberState by room.getDmMemberFlow() + .collectAsState(initial = null, context = coroutineDispatchers.computation) - val memberCount: MutableState> = remember { - mutableStateOf(Async.Uninitialized) - } - LaunchedEffect(Unit) { - suspend { - room.updateMembers() - .map { room.memberCount() } - }.executeResult(memberCount) - } - - val dmMember = room.getDmMember() - val roomType = if (dmMember != null) { - RoomDetailsType.Dm(dmMember) - } else { - RoomDetailsType.Room - } + val roomType = getRoomType(dmMemberState) fun handleEvents(event: RoomDetailsEvent) { when (event) { is RoomDetailsEvent.LeaveRoom -> { - if (event.needsConfirmation) { - leaveRoomWarning = LeaveRoomWarning.computeLeaveRoomWarning(room.isPublic, memberCount.value) - } else { - coroutineScope.launch(Dispatchers.IO) { - room.leave() - .onSuccess { - roomMembershipObserver.notifyUserLeftRoom(room.roomId) - }.onFailure { - error = RoomDetailsError.AlertGeneric - } - leaveRoomWarning = null - } - } + coroutineScope.leaveRoom( + needsConfirmation = event.needsConfirmation, + memberCount = memberCount, + leaveRoomWarning = leaveRoomWarning, + error = error, + ) } - is RoomDetailsEvent.ClearLeaveRoomWarning -> leaveRoomWarning = null - RoomDetailsEvent.ClearError -> error = null + is RoomDetailsEvent.ClearLeaveRoomWarning -> leaveRoomWarning.value = null + RoomDetailsEvent.ClearError -> error.value = null } } @@ -95,12 +80,56 @@ class RoomDetailsPresenter @Inject constructor( roomAlias = room.alias, roomAvatarUrl = room.avatarUrl, roomTopic = room.topic, - memberCount = memberCount.value, + memberCount = memberCount, isEncrypted = room.isEncrypted, - displayLeaveRoomWarning = leaveRoomWarning, - error = error, - roomType = roomType, + displayLeaveRoomWarning = leaveRoomWarning.value, + error = error.value, + roomType = roomType.value, eventSink = ::handleEvents, ) } + + @Composable + private fun getRoomType(dmMember: RoomMember?): State = remember(dmMember) { + derivedStateOf { + if (dmMember != null) { + RoomDetailsType.Dm(dmMember) + } else { + RoomDetailsType.Room + } + } + } + + @Composable + private fun getMemberCount(membersState: MatrixRoomMembersState): State> = remember(membersState) { + derivedStateOf { + when (membersState) { + MatrixRoomMembersState.Unknown -> Async.Uninitialized + MatrixRoomMembersState.Pending -> Async.Loading() + is MatrixRoomMembersState.Ready -> Async.Success(membersState.roomMembers.size) + is MatrixRoomMembersState.Error -> Async.Failure(membersState.failure) + } + } + } + + private fun CoroutineScope.leaveRoom( + needsConfirmation: Boolean, + memberCount: Async, + leaveRoomWarning: MutableState, + error: MutableState, + ) = launch(coroutineDispatchers.io) { + if (needsConfirmation) { + leaveRoomWarning.value = LeaveRoomWarning.computeLeaveRoomWarning(room.isPublic, memberCount) + } else { + room.leave() + .onSuccess { + roomMembershipObserver.notifyUserLeftRoom(room.roomId) + }.onFailure { + error.value = RoomDetailsError.AlertGeneric + } + leaveRoomWarning.value = null + } + } } + + diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/testCoroutineDispatchers.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListEvents.kt similarity index 56% rename from features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/testCoroutineDispatchers.kt rename to features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListEvents.kt index 60b0b7cf4b..4534842cac 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/testCoroutineDispatchers.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListEvents.kt @@ -14,15 +14,10 @@ * limitations under the License. */ -package io.element.android.features.messages.fixtures +package io.element.android.features.roomdetails.impl.members -import io.element.android.libraries.core.coroutine.CoroutineDispatchers -import kotlinx.coroutines.test.UnconfinedTestDispatcher +import io.element.android.libraries.matrix.ui.model.MatrixUser -// TODO Move to common module to reuse -internal fun testCoroutineDispatchers() = CoroutineDispatchers( - io = UnconfinedTestDispatcher(), - computation = UnconfinedTestDispatcher(), - main = UnconfinedTestDispatcher(), - diffUpdateDispatcher = UnconfinedTestDispatcher(), -) +sealed interface RoomMemberListEvents { + data class SelectUser(val user: MatrixUser) : RoomMemberListEvents +} diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt index 60ba07d1ee..ff2b1a0245 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt @@ -28,9 +28,6 @@ import io.element.android.anvilannotations.ContributesNode import io.element.android.libraries.di.RoomScope import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMember -import io.element.android.libraries.matrix.api.room.getMember -import io.element.android.libraries.matrix.ui.model.MatrixUser -import timber.log.Timber @ContributesNode(RoomScope::class) class RoomMemberListNode @AssistedInject constructor( @@ -46,12 +43,9 @@ class RoomMemberListNode @AssistedInject constructor( private val callbacks = plugins() - private fun onUserSelected(matrixUser: MatrixUser) { - val member = room.getMember(matrixUser.id) - if (member != null) { - callbacks.forEach { it.openRoomMemberDetails(member) } - } else { - Timber.e("Could find room member ${matrixUser.id} in room ${room.roomId}") + private fun openRoomMemberDetails(roomMember: RoomMember) { + callbacks.forEach { + it.openRoomMemberDetails(roomMember) } } @@ -62,7 +56,7 @@ class RoomMemberListNode @AssistedInject constructor( state = state, modifier = modifier, onBackPressed = { navigateUp() }, - onUserSelected = ::onUserSelected, + onMemberSelected = this::openRoomMemberDetails, ) } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt index 8841bd9d5e..897e56728b 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt @@ -18,8 +18,10 @@ package io.element.android.features.roomdetails.impl.members import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.MutableState import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import io.element.android.features.userlist.api.SelectionMode import io.element.android.features.userlist.api.UserListDataSource import io.element.android.features.userlist.api.UserListDataStore @@ -27,10 +29,16 @@ import io.element.android.features.userlist.api.UserListPresenter import io.element.android.features.userlist.api.UserListPresenterArgs import io.element.android.libraries.architecture.Async import io.element.android.libraries.architecture.Presenter +import io.element.android.libraries.core.coroutine.CoroutineDispatchers +import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.api.room.RoomMember +import io.element.android.libraries.matrix.api.room.getMemberFlow import io.element.android.libraries.matrix.ui.model.MatrixUser import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.toImmutableList -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.firstOrNull +import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import javax.inject.Inject import javax.inject.Named @@ -39,6 +47,8 @@ class RoomMemberListPresenter @Inject constructor( private val userListPresenterFactory: UserListPresenter.Factory, @Named("RoomMembers") private val userListDataSource: UserListDataSource, private val userListDataStore: UserListDataStore, + private val room: MatrixRoom, + private val coroutineDispatchers: CoroutineDispatchers, ) : Presenter { private val userListPresenter by lazy { @@ -51,17 +61,33 @@ class RoomMemberListPresenter @Inject constructor( @Composable override fun present(): RoomMemberListState { + val coroutineScope = rememberCoroutineScope() val userListState = userListPresenter.present() val allUsers = remember { mutableStateOf>>(Async.Loading()) } + val selectedMember: MutableState = remember { + mutableStateOf(null) + } LaunchedEffect(Unit) { - withContext(Dispatchers.IO) { + withContext(coroutineDispatchers.io) { allUsers.value = Async.Success(userListDataSource.search("").toImmutableList()) } } + + fun handleEvents(roomMemberListEvents: RoomMemberListEvents) { + when (roomMemberListEvents) { + is RoomMemberListEvents.SelectUser -> coroutineScope.loadRoomMember(roomMemberListEvents.user, selectedMember) + } + } return RoomMemberListState( allUsers = allUsers.value, - userListState = userListState + userListState = userListState, + selectedRoomMember = selectedMember.value, + eventSink = ::handleEvents ) } + + private fun CoroutineScope.loadRoomMember(user: MatrixUser, selectedMember: MutableState) = launch(coroutineDispatchers.io) { + selectedMember.value = room.getMemberFlow(user.id).firstOrNull() + } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt index f5e5bd3efb..42289b9ebe 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt @@ -18,11 +18,13 @@ package io.element.android.features.roomdetails.impl.members import io.element.android.features.userlist.api.UserListState import io.element.android.libraries.architecture.Async +import io.element.android.libraries.matrix.api.room.RoomMember import io.element.android.libraries.matrix.ui.model.MatrixUser import kotlinx.collections.immutable.ImmutableList data class RoomMemberListState( val allUsers: Async>, val userListState: UserListState, -// val eventSink: (AddPeopleEvents) -> Unit, + val selectedRoomMember: RoomMember? = null, + val eventSink: (RoomMemberListEvents) -> Unit, ) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt index fc98ae7544..57012c6d86 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt @@ -39,4 +39,5 @@ internal fun aRoomMemberListState( RoomMemberListState( userListState = aUserListState().copy(searchResults = searchResults), allUsers = allUsers, + eventSink = {} ) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt index f356e203f2..47aeb635df 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt @@ -28,6 +28,7 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.pluralStringResource @@ -51,6 +52,7 @@ import io.element.android.libraries.designsystem.theme.components.CenterAlignedT import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator import io.element.android.libraries.designsystem.theme.components.Scaffold import io.element.android.libraries.designsystem.theme.components.Text +import io.element.android.libraries.matrix.api.room.RoomMember import io.element.android.libraries.matrix.ui.model.MatrixUser @OptIn(ExperimentalMaterial3Api::class) @@ -59,8 +61,19 @@ fun RoomMemberListView( state: RoomMemberListState, modifier: Modifier = Modifier, onBackPressed: () -> Unit = {}, - onUserSelected: (MatrixUser) -> Unit = {}, + onMemberSelected: (RoomMember) -> Unit = {}, ) { + + LaunchedEffect(state.selectedRoomMember) { + if (state.selectedRoomMember != null) { + onMemberSelected(state.selectedRoomMember) + } + } + + fun onUserSelected(user: MatrixUser) { + state.eventSink(RoomMemberListEvents.SelectUser(user)) + } + Scaffold( topBar = { if (!state.userListState.isSearchActive) { @@ -76,7 +89,7 @@ fun RoomMemberListView( ) { UserListView( state = state.userListState, - onUserSelected = onUserSelected, + onUserSelected = ::onUserSelected, ) if (!state.userListState.isSearchActive) { diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt index 6bf1203e83..10d9d55c02 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt @@ -18,26 +18,40 @@ package io.element.android.features.roomdetails.impl.members import io.element.android.features.userlist.api.UserListDataSource import io.element.android.libraries.core.bool.orFalse +import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.designsystem.components.avatar.AvatarData import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.room.RoomMember +import io.element.android.libraries.matrix.api.room.roomMembers import io.element.android.libraries.matrix.ui.model.MatrixUser +import kotlinx.coroutines.flow.dropWhile +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.skip +import kotlinx.coroutines.flow.takeWhile +import kotlinx.coroutines.withContext import javax.inject.Inject class RoomUserListDataSource @Inject constructor( - private val room: MatrixRoom + private val room: MatrixRoom, + private val coroutineDispatchers: CoroutineDispatchers, ) : UserListDataSource { - override suspend fun search(query: String): List { - return room.membersFlow.value.filter { member -> - if (query.isBlank()) { - true - } else { + override suspend fun search(query: String): List = withContext(coroutineDispatchers.io) { + val roomMembers = room.membersStateFlow + .dropWhile { it !is MatrixRoomMembersState.Ready} + .first() + .roomMembers() + val filteredMembers = if (query.isBlank()) { + roomMembers + } else { + roomMembers.filter { member -> member.userId.value.contains(query, ignoreCase = true) || member.displayName?.contains(query, ignoreCase = true).orFalse() } - }.map(::mapMemberToMatrixUser) + } + filteredMembers.map(::mapMemberToMatrixUser) } override suspend fun getProfile(userId: UserId): MatrixUser? { @@ -55,5 +69,4 @@ class RoomUserListDataSource @Inject constructor( ) ) } - } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index 1d25a27e44..05617ebf1a 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -26,6 +26,7 @@ import io.element.android.features.roomdetails.impl.RoomDetailsPresenter import io.element.android.libraries.architecture.Async import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.UserId +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState 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.RoomMembershipState @@ -34,6 +35,7 @@ import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.matrix.test.A_ROOM_NAME import io.element.android.libraries.matrix.test.A_USER_ID import io.element.android.libraries.matrix.test.room.FakeMatrixRoom +import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.onEach @@ -45,11 +47,12 @@ import org.junit.Test class RoomDetailsPresenterTests { private val roomMembershipObserver = RoomMembershipObserver() + private val testCoroutineDispatchers = testCoroutineDispatchers() @Test fun `present - initial state is created from room info`() = runTest { val room = aMatrixRoom() - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -68,13 +71,13 @@ class RoomDetailsPresenterTests { @Test fun `present - room member count is calculated asynchronously`() = runTest { val room = aMatrixRoom() - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { val initialState = awaitItem() Truth.assertThat(initialState.memberCount).isEqualTo(Async.Uninitialized) - + room.givenRoomMembersState(MatrixRoomMembersState.Ready(emptyList())) val finalState = awaitItem() Truth.assertThat(finalState.memberCount).isEqualTo(Async.Success(0)) } @@ -83,7 +86,7 @@ class RoomDetailsPresenterTests { @Test fun `present - initial state with no room name`() = runTest { val room = aMatrixRoom(name = null) - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -97,30 +100,27 @@ class RoomDetailsPresenterTests { @Test fun `present - can handle error while fetching member count`() = runTest { val room = aMatrixRoom(name = null).apply { - givenFetchMemberResult(Result.failure(Throwable())) + givenRoomMembersState(MatrixRoomMembersState.Error(Throwable())) } - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) Truth.assertThat(awaitItem().memberCount).isInstanceOf(Async.Failure::class.java) - cancelAndIgnoreRemainingEvents() } } @Test fun `present - Leave with confirmation on private room shows a specific warning`() = runTest { - val room = aMatrixRoom(isPublic = false) - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val room = aMatrixRoom(isPublic = false).apply { + givenRoomMembersState(MatrixRoomMembersState.Ready(emptyList())) + } + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { val initialState = awaitItem() - // Allow room member count to load - skipItems(1) - initialState.eventSink(RoomDetailsEvent.LeaveRoom(needsConfirmation = true)) val confirmationState = awaitItem() Truth.assertThat(confirmationState.displayLeaveRoomWarning).isEqualTo(LeaveRoomWarning.PrivateRoom) @@ -129,15 +129,14 @@ class RoomDetailsPresenterTests { @Test fun `present - Leave with confirmation on empty room shows a specific warning`() = runTest { - val room = aMatrixRoom(members = listOf(aRoomMember())) - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val room = aMatrixRoom().apply { + givenRoomMembersState(MatrixRoomMembersState.Ready(listOf(aRoomMember()))) + } + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { val initialState = awaitItem() - // Allow room member count to load - skipItems(1) - initialState.eventSink(RoomDetailsEvent.LeaveRoom(needsConfirmation = true)) val confirmationState = awaitItem() Truth.assertThat(confirmationState.displayLeaveRoomWarning).isEqualTo(LeaveRoomWarning.LastUserInRoom) @@ -146,15 +145,14 @@ class RoomDetailsPresenterTests { @Test fun `present - Leave with confirmation shows a generic warning`() = runTest { - val room = aMatrixRoom() - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val room = aMatrixRoom().apply { + givenRoomMembersState(MatrixRoomMembersState.Ready(emptyList())) + } + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { val initialState = awaitItem() - // Allow room member count to load - skipItems(1) - initialState.eventSink(RoomDetailsEvent.LeaveRoom(needsConfirmation = true)) val confirmationState = awaitItem() Truth.assertThat(confirmationState.displayLeaveRoomWarning).isEqualTo(LeaveRoomWarning.Generic) @@ -163,15 +161,14 @@ class RoomDetailsPresenterTests { @Test fun `present - Leave without confirmation leaves the room`() = runTest { - val room = aMatrixRoom() - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val room = aMatrixRoom().apply { + givenRoomMembersState(MatrixRoomMembersState.Ready(emptyList())) + } + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { val initialState = awaitItem() - // Allow room member count to load - skipItems(1) - initialState.eventSink(RoomDetailsEvent.LeaveRoom(needsConfirmation = false)) cancelAndIgnoreRemainingEvents() @@ -188,14 +185,11 @@ class RoomDetailsPresenterTests { val room = aMatrixRoom().apply { givenLeaveRoomError(Throwable()) } - val presenter = RoomDetailsPresenter(room, roomMembershipObserver) + val presenter = RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { val initialState = awaitItem() - // Allow room member count to load - skipItems(1) - initialState.eventSink(RoomDetailsEvent.LeaveRoom(needsConfirmation = false)) val errorState = awaitItem() Truth.assertThat(errorState.error).isNotNull() @@ -211,13 +205,11 @@ fun aMatrixRoom( displayName: String = "A fallback display name", topic: String? = "A topic", avatarUrl: String? = "https://matrix.org/avatar.jpg", - members: List = emptyList(), isEncrypted: Boolean = true, isPublic: Boolean = true, ) = FakeMatrixRoom( roomId = roomId, name = name, - initialMembers = members, displayName = displayName, topic = topic, avatarUrl = avatarUrl, diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt index 373ebbb347..a5798381c0 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt @@ -29,8 +29,12 @@ import io.element.android.features.userlist.api.UserListPresenterArgs import io.element.android.features.userlist.impl.DefaultUserListPresenter import io.element.android.features.userlist.test.FakeUserListDataSource import io.element.android.libraries.architecture.Async +import io.element.android.libraries.core.coroutine.CoroutineDispatchers +import io.element.android.libraries.matrix.test.room.FakeMatrixRoom import io.element.android.libraries.matrix.ui.components.aMatrixUser +import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import okhttp3.internal.toImmutableList import org.junit.Test @@ -38,6 +42,8 @@ import org.junit.Test @ExperimentalCoroutinesApi class RoomMemberListPresenterTests { + private val testCoroutineDispatchers = testCoroutineDispatchers() + @Test fun `present - search is done automatically on start, but is async`() = runTest { val searchResult = listOf(aMatrixUser()) @@ -52,7 +58,14 @@ class RoomMemberListPresenterTests { userListDataStore: UserListDataStore, ) = DefaultUserListPresenter(args, userListDataSource, userListDataStore) } - val presenter = RoomMemberListPresenter(userListFactory, userListDataSource, userListDataStore) + val fakeRoom = FakeMatrixRoom() + val presenter = RoomMemberListPresenter( + userListPresenterFactory = userListFactory, + userListDataSource = userListDataSource, + userListDataStore = userListDataStore, + room = fakeRoom, + coroutineDispatchers = testCoroutineDispatchers + ) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { diff --git a/features/userlist/api/src/main/kotlin/io/element/android/features/userlist/api/UserListDataSource.kt b/features/userlist/api/src/main/kotlin/io/element/android/features/userlist/api/UserListDataSource.kt index afe2d1ab3d..2cfd23eb61 100644 --- a/features/userlist/api/src/main/kotlin/io/element/android/features/userlist/api/UserListDataSource.kt +++ b/features/userlist/api/src/main/kotlin/io/element/android/features/userlist/api/UserListDataSource.kt @@ -20,6 +20,7 @@ import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.ui.model.MatrixUser interface UserListDataSource { + //TODO should probably have a flow suspend fun search(query: String): List suspend fun getProfile(userId: UserId): MatrixUser? } diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/Async.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/Async.kt index bb74dff2a9..3be961598d 100644 --- a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/Async.kt +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/Async.kt @@ -47,7 +47,9 @@ suspend fun (suspend () -> T).execute(state: MutableState>, errorMa } suspend fun (suspend () -> Result).executeResult(state: MutableState>) { - state.value = Async.Loading() + if (state.value !is Async.Success) { + state.value = Async.Loading() + } this().fold( onSuccess = { state.value = Async.Success(it) 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 fae6550a4f..2bff8fe76c 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 @@ -23,6 +23,7 @@ import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.map import java.io.Closeable interface MatrixRoom : Closeable { @@ -41,10 +42,10 @@ interface MatrixRoom : Closeable { /** * The current loaded members as a StateFlow. - * Initial value is an emptyList. + * Initial value is [MatrixRoomMembersState.Unknown]. * To update them you should call [updateMembers]. */ - val membersFlow: StateFlow> + val membersStateFlow: StateFlow /** * Try to load the room members and update the membersFlow. @@ -70,18 +71,21 @@ interface MatrixRoom : Closeable { suspend fun leave(): Result } -fun MatrixRoom.getMember(userId: UserId): RoomMember? { - return membersFlow.value.find { it.userId == userId } -} - -fun MatrixRoom.getDmMember(): RoomMember? { - return if (membersFlow.value.size == 2 && isDirect && isEncrypted) { - membersFlow.value.find { it.userId != this.sessionId } - } else { - null +fun MatrixRoom.getMemberFlow(userId: UserId): Flow { + return membersStateFlow.map { state -> + state.roomMembers().find { + it.userId == userId + } } } -fun MatrixRoom.memberCount(): Int { - return membersFlow.value.size +fun MatrixRoom.getDmMemberFlow(): Flow { + return membersStateFlow.map { state -> + val members = state.roomMembers() + if (members.size == 2 && isDirect && isEncrypted) { + members.find { it.userId != this.sessionId } + } else { + null + } + } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomMembersState.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomMembersState.kt new file mode 100644 index 0000000000..319a5f7605 --- /dev/null +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomMembersState.kt @@ -0,0 +1,31 @@ +/* + * 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.libraries.matrix.api.room + +sealed interface MatrixRoomMembersState { + object Unknown : MatrixRoomMembersState + object Pending : MatrixRoomMembersState + data class Error(val failure: Throwable) : MatrixRoomMembersState + data class Ready(val roomMembers: List) : MatrixRoomMembersState +} + +fun MatrixRoomMembersState.roomMembers(): List { + return when (this) { + is MatrixRoomMembersState.Ready -> roomMembers + else -> emptyList() + } +} 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 1b87d271dc..9e780e57de 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 @@ -22,7 +22,7 @@ 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.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom -import io.element.android.libraries.matrix.api.room.RoomMember +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.impl.timeline.RustMatrixTimeline import kotlinx.coroutines.CoroutineScope @@ -48,10 +48,10 @@ class RustMatrixRoom( private val coroutineDispatchers: CoroutineDispatchers, ) : MatrixRoom { - override val membersFlow: StateFlow> - get() = cachedMembers + override val membersStateFlow: StateFlow + get() = _membersStateFlow - private var cachedMembers = MutableStateFlow>(emptyList()) + private var _membersStateFlow = MutableStateFlow(MatrixRoomMembersState.Unknown) override fun syncUpdateFlow(): Flow { return slidingSyncUpdateFlow @@ -122,9 +122,14 @@ class RustMatrixRoom( get() = innerRoom.isDirect() override suspend fun updateMembers(): Result = withContext(coroutineDispatchers.io) { + _membersStateFlow.value = MatrixRoomMembersState.Pending runCatching { - cachedMembers.value = innerRoom.members().map(RoomMemberMapper::map) - } + innerRoom.members().map(RoomMemberMapper::map) + }.onSuccess { + _membersStateFlow.value = MatrixRoomMembersState.Ready(it) + }.onFailure { + _membersStateFlow.value = MatrixRoomMembersState.Error(it) + }.map { } } override suspend fun userDisplayName(userId: UserId): Result = diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt index 17e865f9fb..6d19e817a4 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt @@ -143,12 +143,15 @@ class RustMatrixTimeline( requiredState = listOf( RequiredState(key = "m.room.canonical_alias", value = ""), RequiredState(key = "m.room.topic", value = ""), + RequiredState(key = "m.room.name", value = ""), RequiredState(key = "m.room.join_rule", value = ""), ), timelineLimit = 20.toUInt() ) val result = slidingSyncRoom.subscribeAndAddTimelineListener(timelineListener, settings) - fetchMembers() + launch { + fetchMembers() + } listenerTokens += result.taskHandle result.items } 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 b80b84a666..a685e369d1 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 @@ -21,7 +21,7 @@ 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.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom -import io.element.android.libraries.matrix.api.room.RoomMember +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.matrix.test.A_SESSION_ID @@ -44,18 +44,16 @@ class FakeMatrixRoom( override val alternativeAliases: List = emptyList(), override val isPublic: Boolean = true, override val isDirect: Boolean = false, - initialMembers: List = emptyList(), private val matrixTimeline: MatrixTimeline = FakeMatrixTimeline(), ) : MatrixRoom { private var userDisplayNameResult = Result.success(null) private var userAvatarUrlResult = Result.success(null) - private var dmMember: RoomMember? = null private var updateMembersResult: Result = Result.success(Unit) private var leaveRoomError: Throwable? = null - override val membersFlow: MutableStateFlow> = MutableStateFlow(initialMembers) + override val membersStateFlow: MutableStateFlow = MutableStateFlow(MatrixRoomMembersState.Unknown) override suspend fun updateMembers(): Result { return updateMembersResult @@ -117,12 +115,12 @@ class FakeMatrixRoom( this.leaveRoomError = throwable } - fun givenFetchMemberResult(result: Result) { - updateMembersResult = result + fun givenRoomMembersState(state: MatrixRoomMembersState) { + membersStateFlow.value = state } - fun givenDmMember(roomMember: RoomMember) { - this.dmMember = roomMember + fun givenUpdateMembersResult(result: Result) { + updateMembersResult = result } fun givenUserDisplayNameResult(displayName: Result) { diff --git a/tests/testutils/build.gradle.kts b/tests/testutils/build.gradle.kts index 44167f906f..fda44f46d1 100644 --- a/tests/testutils/build.gradle.kts +++ b/tests/testutils/build.gradle.kts @@ -34,4 +34,6 @@ dependencies { implementation(libs.coroutines.test) implementation(projects.libraries.matrix.test) implementation(projects.services.appnavstate.test) + implementation(projects.services.appnavstate.test) + implementation(projects.libraries.core) } diff --git a/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/TestCoroutineDispatchers.kt b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/TestCoroutineDispatchers.kt new file mode 100644 index 0000000000..1309a14cb1 --- /dev/null +++ b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/TestCoroutineDispatchers.kt @@ -0,0 +1,46 @@ +/* + * 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. + */ + +@file:OptIn(ExperimentalCoroutinesApi::class) + +package io.element.android.tests.testutils + +import io.element.android.libraries.core.coroutine.CoroutineDispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.TestDispatcher +import kotlinx.coroutines.test.UnconfinedTestDispatcher + +fun testCoroutineDispatchers( + testScheduler: TestCoroutineScheduler? = null, +) = CoroutineDispatchers( + io = UnconfinedTestDispatcher(testScheduler), + computation = UnconfinedTestDispatcher(testScheduler), + main = UnconfinedTestDispatcher(testScheduler), + diffUpdateDispatcher = UnconfinedTestDispatcher(testScheduler), +) + +fun testCoroutineDispatchers( + io: TestDispatcher = UnconfinedTestDispatcher(), + computation: TestDispatcher = UnconfinedTestDispatcher(), + main: TestDispatcher = UnconfinedTestDispatcher(), + diffUpdateDispatcher: TestDispatcher = UnconfinedTestDispatcher(), +) = CoroutineDispatchers( + io = io, + computation = computation, + main = main, + diffUpdateDispatcher = diffUpdateDispatcher, +) From e65adaecd15c0a34786e4cf0b0a69c22a7b90e69 Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 24 Apr 2023 10:42:27 +0200 Subject: [PATCH 04/13] Timeline: add ContentType instead of using introspection. --- .../features/messages/impl/timeline/TimelineView.kt | 11 ----------- .../messages/impl/timeline/diff/CacheInvalidator.kt | 8 ++++---- .../messages/impl/timeline/model/TimelineItem.kt | 5 +++++ .../timeline/model/event/TimelineItemEmoteContent.kt | 4 +++- .../model/event/TimelineItemEncryptedContent.kt | 4 +++- .../timeline/model/event/TimelineItemEventContent.kt | 4 +++- .../timeline/model/event/TimelineItemImageContent.kt | 4 +++- .../timeline/model/event/TimelineItemNoticeContent.kt | 4 +++- .../model/event/TimelineItemRedactedContent.kt | 4 +++- .../timeline/model/event/TimelineItemTextContent.kt | 4 +++- .../model/event/TimelineItemUnknownContent.kt | 4 +++- .../model/virtual/TimelineItemDaySeparatorModel.kt | 4 +++- .../model/virtual/TimelineItemLoadingModel.kt | 4 +++- .../model/virtual/TimelineItemReadMarkerModel.kt | 4 +++- .../model/virtual/TimelineItemTimelineStartModel.kt | 4 +++- .../model/virtual/TimelineItemUnknownVirtualModel.kt | 4 +++- .../model/virtual/TimelineItemVirtualModel.kt | 4 +++- 17 files changed, 51 insertions(+), 29 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt index c7b071e069..a378c508f6 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt @@ -75,7 +75,6 @@ import io.element.android.libraries.designsystem.theme.components.Text import kotlinx.collections.immutable.ImmutableList import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch -import timber.log.Timber @Composable fun TimelineView( @@ -115,24 +114,14 @@ fun TimelineView( } } - /* TimelineScrollHelper( lazyListState = lazyListState, timelineItems = state.timelineItems, onLoadMore = ::onReachedLoadMore ) - - */ } } -private fun TimelineItem.contentType() = when (this) { - is TimelineItem.Event -> content.javaClass.simpleName - is TimelineItem.Virtual -> model.javaClass.simpleName -}.also { - Timber.v("ContentType = $it") -} - @Composable fun TimelineItemRow( timelineItem: TimelineItem, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/CacheInvalidator.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/CacheInvalidator.kt index c2f3b33667..9aa3ab5e02 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/CacheInvalidator.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/diff/CacheInvalidator.kt @@ -25,7 +25,7 @@ internal class CacheInvalidator(private val itemStatesCache: MutableList id } + fun contentType(): String = when (this) { + is Event -> content.type + is Virtual -> model.type + } + @Immutable data class Virtual( val id: String, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEmoteContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEmoteContent.kt index 67fefb665e..a059c9b275 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEmoteContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEmoteContent.kt @@ -21,4 +21,6 @@ import org.jsoup.nodes.Document data class TimelineItemEmoteContent( override val body: String, override val htmlDocument: Document? -) : TimelineItemTextBasedContent +) : TimelineItemTextBasedContent { + override val type: String = "TimelineItemEmoteContent" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEncryptedContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEncryptedContent.kt index b42c2ac535..ff1bb36faf 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEncryptedContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEncryptedContent.kt @@ -20,4 +20,6 @@ import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecry data class TimelineItemEncryptedContent( val data: UnableToDecryptContent.Data -) : TimelineItemEventContent +) : TimelineItemEventContent { + override val type: String = "TimelineItemEncryptedContent" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt index 7e166dba97..233f51a5a2 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt @@ -19,4 +19,6 @@ package io.element.android.features.messages.impl.timeline.model.event import androidx.compose.runtime.Immutable @Immutable -sealed interface TimelineItemEventContent +sealed interface TimelineItemEventContent { + val type: String +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemImageContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemImageContent.kt index 8013ca8a65..1d2367d13f 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemImageContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemImageContent.kt @@ -23,4 +23,6 @@ data class TimelineItemImageContent( val imageMeta: MediaResolver.Meta, val blurhash: String?, val aspectRatio: Float -) : TimelineItemEventContent +) : TimelineItemEventContent{ + override val type: String = "TimelineItemImageContent" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemNoticeContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemNoticeContent.kt index 6c647158c4..7974f188a3 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemNoticeContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemNoticeContent.kt @@ -21,4 +21,6 @@ import org.jsoup.nodes.Document data class TimelineItemNoticeContent( override val body: String, override val htmlDocument: Document? -) : TimelineItemTextBasedContent +) : TimelineItemTextBasedContent { + override val type: String = "TimelineItemNoticeContent" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemRedactedContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemRedactedContent.kt index de98b22bbb..7a8edae953 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemRedactedContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemRedactedContent.kt @@ -16,4 +16,6 @@ package io.element.android.features.messages.impl.timeline.model.event -object TimelineItemRedactedContent : TimelineItemEventContent +object TimelineItemRedactedContent : TimelineItemEventContent{ + override val type: String = "TimelineItemRedactedContent" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemTextContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemTextContent.kt index 6cbb0ccd08..993275e627 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemTextContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemTextContent.kt @@ -21,4 +21,6 @@ import org.jsoup.nodes.Document data class TimelineItemTextContent( override val body: String, override val htmlDocument: Document? -) : TimelineItemTextBasedContent +) : TimelineItemTextBasedContent{ + override val type: String = "TimelineItemTextContent" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemUnknownContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemUnknownContent.kt index eb79b29f79..44aeb93e3b 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemUnknownContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemUnknownContent.kt @@ -16,4 +16,6 @@ package io.element.android.features.messages.impl.timeline.model.event -object TimelineItemUnknownContent : TimelineItemEventContent +object TimelineItemUnknownContent : TimelineItemEventContent { + override val type: String = "TimelineItemUnknownContent" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemDaySeparatorModel.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemDaySeparatorModel.kt index b4c1235f8f..54e95b7294 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemDaySeparatorModel.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemDaySeparatorModel.kt @@ -18,4 +18,6 @@ package io.element.android.features.messages.impl.timeline.model.virtual data class TimelineItemDaySeparatorModel( val formattedDate: String -) : TimelineItemVirtualModel +) : TimelineItemVirtualModel { + override val type: String = "TimelineItemDaySeparatorModel" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemLoadingModel.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemLoadingModel.kt index 4870177a84..9cc7280b07 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemLoadingModel.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemLoadingModel.kt @@ -16,4 +16,6 @@ package io.element.android.features.messages.impl.timeline.model.virtual -object TimelineItemLoadingModel : TimelineItemVirtualModel +object TimelineItemLoadingModel : TimelineItemVirtualModel { + override val type: String = "TimelineItemLoadingModel" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemReadMarkerModel.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemReadMarkerModel.kt index 247cd58212..0b8e3fc0e5 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemReadMarkerModel.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemReadMarkerModel.kt @@ -16,4 +16,6 @@ package io.element.android.features.messages.impl.timeline.model.virtual -object TimelineItemReadMarkerModel : TimelineItemVirtualModel +object TimelineItemReadMarkerModel : TimelineItemVirtualModel { + override val type: String = "TimelineItemReadMarkerModel" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemTimelineStartModel.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemTimelineStartModel.kt index ab0ec4fdf8..8c1afea886 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemTimelineStartModel.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemTimelineStartModel.kt @@ -16,4 +16,6 @@ package io.element.android.features.messages.impl.timeline.model.virtual -object TimelineItemTimelineStartModel : TimelineItemVirtualModel +object TimelineItemTimelineStartModel : TimelineItemVirtualModel { + override val type: String = "TimelineItemTimelineStartModel" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemUnknownVirtualModel.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemUnknownVirtualModel.kt index 6d023bf748..8b4fe44744 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemUnknownVirtualModel.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemUnknownVirtualModel.kt @@ -16,4 +16,6 @@ package io.element.android.features.messages.impl.timeline.model.virtual -object TimelineItemUnknownVirtualModel : TimelineItemVirtualModel +object TimelineItemUnknownVirtualModel : TimelineItemVirtualModel { + override val type: String = "TimelineItemUnknownVirtualModel" +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemVirtualModel.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemVirtualModel.kt index a7911867f9..d6c3529ab4 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemVirtualModel.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemVirtualModel.kt @@ -19,4 +19,6 @@ package io.element.android.features.messages.impl.timeline.model.virtual import androidx.compose.runtime.Immutable @Immutable -sealed interface TimelineItemVirtualModel +sealed interface TimelineItemVirtualModel { + val type: String +} From dfabc02bf6e4359f8be33e6b32555ae20f81ac1a Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 24 Apr 2023 10:43:37 +0200 Subject: [PATCH 05/13] Timeline : Add isInit to avoid calling rust methods when the timeline is not ready. --- .../libraries/matrix/impl/timeline/RustMatrixTimeline.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt index 6d19e817a4..3222cda704 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt @@ -42,6 +42,7 @@ import org.matrix.rustcomponents.sdk.SlidingSyncRoom import org.matrix.rustcomponents.sdk.TimelineItem import org.matrix.rustcomponents.sdk.TimelineListener import timber.log.Timber +import java.util.concurrent.atomic.AtomicBoolean class RustMatrixTimeline( private val matrixRoom: MatrixRoom, @@ -51,6 +52,8 @@ class RustMatrixTimeline( private val coroutineDispatchers: CoroutineDispatchers, ) : MatrixTimeline { + private val isInit = AtomicBoolean(false) + private val timelineItems: MutableStateFlow> = MutableStateFlow(emptyList()) @@ -95,6 +98,7 @@ class RustMatrixTimeline( withContext(coroutineDispatchers.diffUpdateDispatcher) { this@RustMatrixTimeline.timelineItems.value = matrixTimelineItems } + isInit.set(true) } .onFailure { Timber.e("Failed adding timeline listener on room with identifier: ${matrixRoom.roomId})") @@ -105,6 +109,7 @@ class RustMatrixTimeline( override fun dispose() { Timber.v("Dispose timeline for room ${matrixRoom.roomId}") listenerTokens.dispose() + isInit.set(false) } /** @@ -125,6 +130,9 @@ class RustMatrixTimeline( override suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result = withContext(coroutineDispatchers.io) { runCatching { Timber.v("Start back paginating for room ${matrixRoom.roomId} ") + if (!isInit.get()) { + throw IllegalStateException("Timeline is not init yet") + } val paginationOptions = PaginationOptions.UntilNumItems( eventLimit = requestSize.toUShort(), items = untilNumberOfItems.toUShort() From f601bb5c1b75136dae5654c8bd4afab7b27bdcad Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 24 Apr 2023 10:57:05 +0200 Subject: [PATCH 06/13] Timeline: copy getInitial method from EA to avoid showing @ as avatar. --- .../designsystem/components/avatar/Avatar.kt | 2 +- .../components/avatar/AvatarData.kt | 34 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/Avatar.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/Avatar.kt index 06aa4435a0..05257ece7f 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/Avatar.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/Avatar.kt @@ -93,7 +93,7 @@ private fun InitialsAvatar( ) { Text( modifier = Modifier.align(Alignment.Center), - text = avatarData.getInitial(), + text = avatarData.initial, fontSize = (avatarData.size.dp / 2).value.sp, color = Color.White, ) diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt index 3bf4f7d0b4..e46751f15f 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt @@ -30,8 +30,36 @@ data class AvatarData( @IgnoredOnParcel val size: AvatarSize = AvatarSize.MEDIUM ) : Parcelable { - fun getInitial(): String { - val firstChar = name?.firstOrNull() ?: id.getOrNull(1) ?: '?' - return firstChar.uppercase() + + val initial by lazy { + (name?.takeIf { it.isNotBlank() } ?: id) + .let { dn -> + var startIndex = 0 + val initial = dn[startIndex] + + if (initial in listOf('@', '#', '+') && dn.length > 1) { + startIndex++ + } + + var length = 1 + var first = dn[startIndex] + + // LEFT-TO-RIGHT MARK + if (dn.length >= 2 && 0x200e == first.code) { + startIndex++ + first = dn[startIndex] + } + + // check if it’s the start of a surrogate pair + if (first.code in 0xD800..0xDBFF && dn.length > startIndex + 1) { + val second = dn[startIndex + 1] + if (second.code in 0xDC00..0xDFFF) { + length++ + } + } + + dn.substring(startIndex, startIndex + length) + } + .uppercase() } } From 1ab96e2f34753e7c89280025026e846eff004325 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 25 Apr 2023 11:17:05 +0200 Subject: [PATCH 07/13] Some clean up --- .../main/kotlin/io/element/android/x/ElementXApplication.kt | 2 +- .../libraries/designsystem/components/avatar/AvatarData.kt | 1 + .../android/libraries/matrix/impl/room/RustMatrixRoom.kt | 4 ++-- .../element/android/libraries/matrix/ui/media/MediaKeyer.kt | 5 ++++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/src/main/kotlin/io/element/android/x/ElementXApplication.kt b/app/src/main/kotlin/io/element/android/x/ElementXApplication.kt index 2b02bf3700..31dab9ad48 100644 --- a/app/src/main/kotlin/io/element/android/x/ElementXApplication.kt +++ b/app/src/main/kotlin/io/element/android/x/ElementXApplication.kt @@ -19,8 +19,8 @@ package io.element.android.x import android.app.Application import androidx.startup.AppInitializer import io.element.android.x.di.AppComponent -import io.element.android.x.di.DaggerAppComponent import io.element.android.libraries.di.DaggerComponentOwner +import io.element.android.x.di.DaggerAppComponent import io.element.android.x.info.logApplicationInfo import io.element.android.x.initializer.CrashInitializer import io.element.android.x.initializer.MatrixInitializer diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt index e46751f15f..2d1e0558f4 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/avatar/AvatarData.kt @@ -31,6 +31,7 @@ data class AvatarData( val size: AvatarSize = AvatarSize.MEDIUM ) : Parcelable { + @IgnoredOnParcel val initial by lazy { (name?.takeIf { it.isNotBlank() } ?: id) .let { dn -> 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 11558e86b6..4efe35f6b5 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 @@ -184,13 +184,13 @@ class RustMatrixRoom( } override suspend fun acceptInvitation(): Result = withContext(coroutineDispatchers.io) { - kotlin.runCatching { + runCatching { innerRoom.acceptInvitation() } } override suspend fun rejectInvitation(): Result = withContext(coroutineDispatchers.io) { - kotlin.runCatching { + runCatching { innerRoom.rejectInvitation() } } diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaKeyer.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaKeyer.kt index 2d4ab683b1..6a1a5e8bfb 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaKeyer.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/MediaKeyer.kt @@ -33,4 +33,7 @@ internal class MediaKeyer : Keyer { } } -private fun MediaResolver.Meta.toKey() = "${url}_${kind}" +private fun MediaResolver.Meta.toKey(): String? { + if (url.isNullOrBlank()) return null + return "${url}_${kind}" +} From 10ea382e3506bbf0d820599b8f1b92fc873dacc3 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 25 Apr 2023 11:37:27 +0200 Subject: [PATCH 08/13] Generate new snapshots --- ...Group_SheetContentDarkPreview_0_null_0,NEXUS_5,1.0,en].png | 4 ++-- ...Group_SheetContentDarkPreview_0_null_1,NEXUS_5,1.0,en].png | 4 ++-- ...Group_SheetContentDarkPreview_0_null_2,NEXUS_5,1.0,en].png | 4 ++-- ...roup_SheetContentLightPreview_0_null_0,NEXUS_5,1.0,en].png | 4 ++-- ...roup_SheetContentLightPreview_0_null_1,NEXUS_5,1.0,en].png | 4 ++-- ...roup_SheetContentLightPreview_0_null_2,NEXUS_5,1.0,en].png | 4 ++-- ...dalBottomSheetLayoutDarkPreview_0_null,NEXUS_5,1.0,en].png | 4 ++-- ...alBottomSheetLayoutLightPreview_0_null,NEXUS_5,1.0,en].png | 4 ++-- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_0,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_0,NEXUS_5,1.0,en].png index 78ce1fa217..5555d6e6b3 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_0,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_0,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a5948ff8260dc73ff19232679312141dd4021a00cce2871f69870b291a237aa9 -size 4478 +oid sha256:21698ff1c1f2b30ee9c3cc0c2539b35fe7cf54aac07cb0dc376d7c1a03c8814b +size 4483 diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_1,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_1,NEXUS_5,1.0,en].png index 78ce1fa217..5555d6e6b3 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_1,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_1,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a5948ff8260dc73ff19232679312141dd4021a00cce2871f69870b291a237aa9 -size 4478 +oid sha256:21698ff1c1f2b30ee9c3cc0c2539b35fe7cf54aac07cb0dc376d7c1a03c8814b +size 4483 diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_2,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_2,NEXUS_5,1.0,en].png index 88a70ce698..c18cc0dd08 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_2,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentDarkPreview_0_null_2,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:49aafbdb693cccd3ab7ca94e61f7a1de437766a29967dd89c4aaf5134b55c004 -size 14860 +oid sha256:db69f27f60dd9d93bb4d313741b84aa4a3ed008d229590338514c7683c0e3a11 +size 14786 diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_0,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_0,NEXUS_5,1.0,en].png index 665c8811ac..eb47d1cd71 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_0,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_0,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:bb0d3bfcfd75cbd75fd9270ff1dc27090e5dbac79ca8db8a46d91a4c12bc966b -size 4457 +oid sha256:fc3bf884b0425c72cafecdd4afa4e2c28064799f695962360ae4c979a3fe542e +size 4490 diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_1,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_1,NEXUS_5,1.0,en].png index 665c8811ac..eb47d1cd71 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_1,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_1,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:bb0d3bfcfd75cbd75fd9270ff1dc27090e5dbac79ca8db8a46d91a4c12bc966b -size 4457 +oid sha256:fc3bf884b0425c72cafecdd4afa4e2c28064799f695962360ae4c979a3fe542e +size 4490 diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_2,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_2,NEXUS_5,1.0,en].png index f7378511e1..94f6bbbd99 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_2,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.actionlist_null_DefaultGroup_SheetContentLightPreview_0_null_2,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:857fedd347931600aa5613565887384579b565efb86d5eeca6e93c13f5ff442c -size 13994 +oid sha256:6c630475e03d86195a0ebcc57bd12934b799fee956c635b30df60913cd9a3f50 +size 16032 diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme.components_null_DefaultGroup_ModalBottomSheetLayoutDarkPreview_0_null,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme.components_null_DefaultGroup_ModalBottomSheetLayoutDarkPreview_0_null,NEXUS_5,1.0,en].png index a4aa94b6e4..a64e1b5a7a 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme.components_null_DefaultGroup_ModalBottomSheetLayoutDarkPreview_0_null,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme.components_null_DefaultGroup_ModalBottomSheetLayoutDarkPreview_0_null,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:ddbb6611ae83055106f7b67ec828542f8a896cafb49001ed0baef43633cc77c1 -size 8884 +oid sha256:428372bd789ff5e83ba4d837bbb0592edd8a01571728c7b284c57ddb200226ed +size 9407 diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme.components_null_DefaultGroup_ModalBottomSheetLayoutLightPreview_0_null,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme.components_null_DefaultGroup_ModalBottomSheetLayoutLightPreview_0_null,NEXUS_5,1.0,en].png index 317643c598..9aa01dc971 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme.components_null_DefaultGroup_ModalBottomSheetLayoutLightPreview_0_null,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme.components_null_DefaultGroup_ModalBottomSheetLayoutLightPreview_0_null,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:5c9a3c9f68a6627654856b03d2534ae1e4e8e600989bc3719407fbf8e17a7ab1 -size 8631 +oid sha256:718cf6e3323ceb9bbf8b4dd9752203d5137840bd3dfb538008d579511b177412 +size 9504 From b433725783d79a4e448d85acdea596deeb80b7af Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 27 Apr 2023 17:34:27 +0200 Subject: [PATCH 09/13] RoomDetailsPresenter: fix dm test --- .../roomdetails/RoomDetailsPresenterTests.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index acac5e7ccc..949cf269d4 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -113,15 +113,13 @@ class RoomDetailsPresenterTests { @Test fun `present - initial state with DM member sets custom DM roomType`() = runTest { + val myRoomMember = aRoomMember(A_SESSION_ID) + val otherRoomMember = aRoomMember(A_USER_ID_2) val room = aMatrixRoom( isEncrypted = true, - isPublic = false, - name = null + isDirect = true, ).apply { - val roomMembers = listOf( - aRoomMember(A_SESSION_ID), - aRoomMember(A_USER_ID_2), - ) + val roomMembers = listOf(myRoomMember, otherRoomMember) givenRoomMembersState(MatrixRoomMembersState.Ready(roomMembers)) } val presenter = aRoomDetailsPresenter(room) @@ -134,7 +132,7 @@ class RoomDetailsPresenterTests { // Once updated, the RoomDetailsType becomes 'Dm' val updatedState = awaitItem() - Truth.assertThat(updatedState.roomType).isEqualTo(RoomDetailsType.Dm(aRoomMember())) + Truth.assertThat(updatedState.roomType).isEqualTo(RoomDetailsType.Dm(otherRoomMember)) cancelAndIgnoreRemainingEvents() } @@ -250,6 +248,7 @@ fun aMatrixRoom( avatarUrl: String? = "https://matrix.org/avatar.jpg", isEncrypted: Boolean = true, isPublic: Boolean = true, + isDirect: Boolean = false, ) = FakeMatrixRoom( roomId = roomId, name = name, @@ -258,6 +257,7 @@ fun aMatrixRoom( avatarUrl = avatarUrl, isEncrypted = isEncrypted, isPublic = isPublic, + isDirect = isDirect, ) fun aRoomMember( From 64c50d44681d613aac7aaf6cdd631e4c7dab1f76 Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 27 Apr 2023 22:13:25 +0200 Subject: [PATCH 10/13] Ignore/Unignore: makes more sense to be at the client level than room --- .../roomdetails/impl/di/RoomMemberModules.kt | 2 +- .../details/RoomMemberDetailsPresenter.kt | 9 +++++---- .../roomdetails/RoomDetailsPresenterTests.kt | 8 +++++++- .../RoomMemberDetailsPresenterTests.kt | 16 ++++++++------- .../libraries/matrix/api/MatrixClient.kt | 2 ++ .../libraries/matrix/api/room/MatrixRoom.kt | 4 ---- .../libraries/matrix/impl/RustMatrixClient.kt | 12 +++++++++++ .../matrix/impl/room/RustMatrixRoom.kt | 20 ++++++------------- .../libraries/matrix/test/FakeMatrixClient.kt | 18 +++++++++++++++++ .../matrix/test/room/FakeMatrixRoom.kt | 4 ---- 10 files changed, 60 insertions(+), 35 deletions(-) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt index b17cb9ab6a..66fae2b50c 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt @@ -49,7 +49,7 @@ object RoomMemberProvidesModule { ): RoomMemberDetailsPresenter.Factory { return object : RoomMemberDetailsPresenter.Factory { override fun create(roomMember: RoomMember): RoomMemberDetailsPresenter { - return RoomMemberDetailsPresenter(matrixClient.sessionId, room, roomMember) + return RoomMemberDetailsPresenter(matrixClient, room, roomMember) } } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt index 8256453a08..33814c29f0 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt @@ -28,6 +28,7 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsState.ConfirmationDialog import io.element.android.libraries.architecture.Presenter +import io.element.android.libraries.matrix.api.MatrixClient 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.room.MatrixRoom @@ -36,7 +37,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch class RoomMemberDetailsPresenter @AssistedInject constructor( - private val currentUserSessionId: SessionId, + private val client: MatrixClient, private val room: MatrixRoom, @Assisted private val roomMember: RoomMember, ) : Presenter { @@ -91,16 +92,16 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( avatarUrl = userAvatar, isBlocked = isBlocked.value, displayConfirmationDialog = confirmationDialog, - isCurrentUser = roomMember.userId == currentUserSessionId, + isCurrentUser = roomMember.userId == client.sessionId, eventSink = ::handleEvents ) } private fun CoroutineScope.blockUser(userId: UserId, isBlockedState: MutableState) = launch { - room.ignoreUser(userId).onSuccess { isBlockedState.value = true } + client.ignoreUser(userId).onSuccess { isBlockedState.value = true } } private fun CoroutineScope.unblockUser(userId: UserId, isBlockedState: MutableState) = launch { - room.unignoreUser(userId).onSuccess { isBlockedState.value = false } + client.unignoreUser(userId).onSuccess { isBlockedState.value = false } } } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index 949cf269d4..2409172f04 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -27,6 +27,7 @@ import io.element.android.features.roomdetails.impl.RoomDetailsType import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsPresenter import io.element.android.libraries.architecture.Async 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.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState @@ -39,6 +40,7 @@ import io.element.android.libraries.matrix.test.A_ROOM_NAME import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.A_USER_ID import io.element.android.libraries.matrix.test.A_USER_ID_2 +import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.room.FakeMatrixRoom import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -57,7 +59,7 @@ class RoomDetailsPresenterTests { private fun aRoomDetailsPresenter(room: MatrixRoom): RoomDetailsPresenter { val roomMemberDetailsPresenterFactory = object : RoomMemberDetailsPresenter.Factory { override fun create(roomMember: RoomMember): RoomMemberDetailsPresenter { - return RoomMemberDetailsPresenter(A_SESSION_ID, room, roomMember) + return RoomMemberDetailsPresenter(aMatrixClient(), room, roomMember) } } return RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers, roomMemberDetailsPresenterFactory) @@ -240,6 +242,10 @@ class RoomDetailsPresenterTests { } } +fun aMatrixClient( + sessionId: SessionId = A_SESSION_ID, +) = FakeMatrixClient() + fun aMatrixRoom( roomId: RoomId = A_ROOM_ID, name: String? = A_ROOM_NAME, diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt index a98ec1f971..3731c910fc 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt @@ -20,12 +20,12 @@ import app.cash.molecule.RecompositionClock import app.cash.molecule.moleculeFlow import app.cash.turbine.test import com.google.common.truth.Truth +import io.element.android.features.roomdetails.aMatrixClient import io.element.android.features.roomdetails.aMatrixRoom import io.element.android.features.roomdetails.aRoomMember import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsEvents import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsPresenter import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsState -import io.element.android.libraries.matrix.test.A_SESSION_ID import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Test @@ -33,6 +33,8 @@ import org.junit.Test @ExperimentalCoroutinesApi class RoomMemberDetailsPresenterTests { + private val matrixClient = aMatrixClient() + @Test fun `present - returns the room member's data, then updates it if needed`() = runTest { val room = aMatrixRoom().apply { @@ -40,7 +42,7 @@ class RoomMemberDetailsPresenterTests { givenUserAvatarUrlResult(Result.success("A custom avatar")) } val roomMember = aRoomMember(displayName = "Alice") - val presenter = RoomMemberDetailsPresenter(A_SESSION_ID, room, roomMember) + val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -63,7 +65,7 @@ class RoomMemberDetailsPresenterTests { givenUserAvatarUrlResult(Result.failure(Throwable())) } val roomMember = aRoomMember(displayName = "Alice") - val presenter = RoomMemberDetailsPresenter(A_SESSION_ID, room, roomMember) + val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -82,7 +84,7 @@ class RoomMemberDetailsPresenterTests { givenUserAvatarUrlResult(Result.success(null)) } val roomMember = aRoomMember(displayName = "Alice") - val presenter = RoomMemberDetailsPresenter(A_SESSION_ID, room, roomMember) + val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -98,7 +100,7 @@ class RoomMemberDetailsPresenterTests { fun `present - BlockUser needing confirmation displays confirmation dialog`() = runTest { val room = aMatrixRoom() val roomMember = aRoomMember() - val presenter = RoomMemberDetailsPresenter(A_SESSION_ID, room, roomMember) + val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -119,7 +121,7 @@ class RoomMemberDetailsPresenterTests { fun `present - BlockUser and UnblockUser without confirmation change the 'blocked' state`() = runTest { val room = aMatrixRoom() val roomMember = aRoomMember() - val presenter = RoomMemberDetailsPresenter(A_SESSION_ID, room, roomMember) + val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -136,7 +138,7 @@ class RoomMemberDetailsPresenterTests { fun `present - UnblockUser needing confirmation displays confirmation dialog`() = runTest { val room = aMatrixRoom() val roomMember = aRoomMember() - val presenter = RoomMemberDetailsPresenter(A_SESSION_ID, room, roomMember) + val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt index eab4060d85..7bc9ba0797 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt @@ -35,6 +35,8 @@ interface MatrixClient : Closeable { val invitesDataSource: RoomSummaryDataSource fun getRoom(roomId: RoomId): MatrixRoom? fun findDM(userId: UserId): MatrixRoom? + suspend fun ignoreUser(userId: UserId): Result + suspend fun unignoreUser(userId: UserId): Result suspend fun createRoom(createRoomParams: CreateRoomParameters): Result suspend fun createDM(userId: UserId): Result fun startSync() 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 87afee55da..41cd874098 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 @@ -68,10 +68,6 @@ interface MatrixRoom : Closeable { suspend fun redactEvent(eventId: EventId, reason: String? = null): Result - suspend fun ignoreUser(userId: UserId): Result - - suspend fun unignoreUser(userId: UserId): Result - suspend fun leave(): Result suspend fun acceptInvitation(): Result diff --git 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 index 2a7e7c0826..b710a5cf6d 100644 --- 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 @@ -212,6 +212,18 @@ class RustMatrixClient constructor( return roomId?.let { getRoom(it) } } + override suspend fun ignoreUser(userId: UserId): Result = withContext(dispatchers.io) { + runCatching { + client.ignoreUser(userId.value) + } + } + + override suspend fun unignoreUser(userId: UserId): Result = withContext(dispatchers.io) { + runCatching { + client.unignoreUser(userId.value) + } + } + override suspend fun createRoom(createRoomParams: CreateRoomParameters): Result = withContext(dispatchers.io) { runCatching { val rustParams = RustCreateRoomParameters( 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 95094cfdc6..1e75867215 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 @@ -38,6 +38,7 @@ import org.matrix.rustcomponents.sdk.SlidingSyncRoom import org.matrix.rustcomponents.sdk.UpdateSummary import org.matrix.rustcomponents.sdk.genTransactionId import org.matrix.rustcomponents.sdk.messageEventContentFromMarkdown +import org.matrix.rustcomponents.sdk.use import org.matrix.rustcomponents.sdk.RoomMember as RustRoomMember class RustMatrixRoom( @@ -200,19 +201,10 @@ class RustMatrixRoom( } } - override suspend fun ignoreUser(userId: UserId): Result { - return runCatching { - getRustMember(userId)?.ignore() ?: error("No member with userId $userId exists in room $roomId") - } - } - - override suspend fun unignoreUser(userId: UserId): Result { - return runCatching { - getRustMember(userId)?.unignore() ?: error("No member with userId $userId exists in room $roomId") - } - } - - private fun getRustMember(userId: UserId): RustRoomMember? { - return innerRoom.members().find { it.userId() == userId.value } + private fun findRoomMember(userId: UserId, action: (RustRoomMember).() -> Unit) { + return innerRoom.members() + .find { it.userId() == userId.value } + ?.use(action) + ?: error("No member with userId $userId exists in room $roomId") } } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt index d33bd5c939..c547ac8237 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt @@ -47,6 +47,8 @@ class FakeMatrixClient( private val notificationService: FakeNotificationService = FakeNotificationService(), ) : MatrixClient { + private var ignoreUserResult: Result = Result.success(Unit) + private var unignoreUserResult: Result = Result.success(Unit) private var createRoomResult: Result = Result.success(A_ROOM_ID) private var createDmResult: Result = Result.success(A_ROOM_ID) private var createDmFailure: Throwable? = null @@ -62,6 +64,14 @@ class FakeMatrixClient( return findDmResult } + override suspend fun ignoreUser(userId: UserId): Result { + return ignoreUserResult + } + + override suspend fun unignoreUser(userId: UserId): Result { + return unignoreUserResult + } + override suspend fun createRoom(createRoomParams: CreateRoomParameters): Result { delay(100) return createRoomResult @@ -130,6 +140,14 @@ class FakeMatrixClient( createDmResult = result } + fun givenIgnoreUserResult(result: Result) { + ignoreUserResult = result + } + + fun givenUnignoreUserResult(result: Result) { + unignoreUserResult = result + } + fun givenCreateDmError(failure: Throwable?) { createDmFailure = failure } 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 c0f4a7ee20..28d6525739 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,10 +117,6 @@ class FakeMatrixRoom( return Result.success(Unit) } - override suspend fun ignoreUser(userId: UserId): Result = ignoreResult - - override suspend fun unignoreUser(userId: UserId): Result = unignoreResult - override suspend fun leave(): Result = leaveRoomError?.let { Result.failure(it) } ?: Result.success(Unit) override suspend fun acceptInvitation(): Result { isInviteAccepted = true From 4b9f2fdae0a8a877d583b9b14fc6a5b66f5108ba Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 27 Apr 2023 22:38:52 +0200 Subject: [PATCH 11/13] Don't pass RoomMember to Node but a UserId instead --- .../roomdetails/impl/RoomDetailsFlowNode.kt | 8 ++-- .../roomdetails/impl/RoomDetailsPresenter.kt | 2 +- .../roomdetails/impl/di/RoomMemberModules.kt | 5 ++- .../impl/members/RoomMemberListNode.kt | 7 +-- .../impl/members/RoomMemberListPresenter.kt | 16 +------ .../impl/members/RoomMemberListState.kt | 2 - .../members/RoomMemberListStateProvider.kt | 1 - .../impl/members/RoomMemberListView.kt | 13 ++---- .../members/details/RoomMemberDetailsNode.kt | 8 ++-- .../details/RoomMemberDetailsPresenter.kt | 44 ++++++++++++------- .../roomdetails/RoomDetailsPresenterTests.kt | 4 +- .../libraries/matrix/api/room/RoomMember.kt | 5 +-- 12 files changed, 52 insertions(+), 63 deletions(-) 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 f86b9952a8..f3a0ef9001 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 @@ -59,7 +59,7 @@ class RoomDetailsFlowNode @AssistedInject constructor( object RoomMemberList : NavTarget @Parcelize - data class RoomMemberDetails(val roomMember: RoomMember) : NavTarget + data class RoomMemberDetails(val roomMemberId: UserId) : NavTarget } override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { @@ -74,14 +74,14 @@ class RoomDetailsFlowNode @AssistedInject constructor( } NavTarget.RoomMemberList -> { val roomMemberListCallback = object : RoomMemberListNode.Callback { - override fun openRoomMemberDetails(roomMember: RoomMember) { - backstack.push(NavTarget.RoomMemberDetails(roomMember)) + override fun openRoomMemberDetails(roomMemberId: UserId) { + backstack.push(NavTarget.RoomMemberDetails(roomMemberId)) } } createNode(buildContext, listOf(roomMemberListCallback)) } is NavTarget.RoomMemberDetails -> { - createNode(buildContext, listOf(RoomMemberDetailsNode.Inputs(navTarget.roomMember))) + createNode(buildContext, listOf(RoomMemberDetailsNode.Inputs(navTarget.roomMemberId))) } } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index bb4cae8423..3479adbb81 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -99,7 +99,7 @@ class RoomDetailsPresenter @Inject constructor( @Composable private fun roomMemberDetailsPresenter(dmMemberState: RoomMember?) = remember(dmMemberState) { dmMemberState?.let { roomMember -> - roomMembersDetailsPresenterFactory.create(roomMember) + roomMembersDetailsPresenterFactory.create(roomMember.userId) } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt index 66fae2b50c..2cd3b7eb08 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/di/RoomMemberModules.kt @@ -25,6 +25,7 @@ import io.element.android.features.roomdetails.impl.members.details.RoomMemberDe import io.element.android.features.userlist.api.UserListDataSource import io.element.android.libraries.di.RoomScope import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMember import javax.inject.Named @@ -48,8 +49,8 @@ object RoomMemberProvidesModule { room: MatrixRoom, ): RoomMemberDetailsPresenter.Factory { return object : RoomMemberDetailsPresenter.Factory { - override fun create(roomMember: RoomMember): RoomMemberDetailsPresenter { - return RoomMemberDetailsPresenter(matrixClient, room, roomMember) + override fun create(roomMemberId: UserId): RoomMemberDetailsPresenter { + return RoomMemberDetailsPresenter(matrixClient, room, roomMemberId) } } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt index 48743b66fa..5b1e7a72e0 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListNode.kt @@ -26,6 +26,7 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode import io.element.android.libraries.di.RoomScope +import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.RoomMember @ContributesNode(RoomScope::class) @@ -36,14 +37,14 @@ class RoomMemberListNode @AssistedInject constructor( ) : Node(buildContext, plugins = plugins) { interface Callback : Plugin { - fun openRoomMemberDetails(roomMember: RoomMember) + fun openRoomMemberDetails(roomMemberId: UserId) } private val callbacks = plugins() - private fun openRoomMemberDetails(roomMember: RoomMember) { + private fun openRoomMemberDetails(roomMemberId: UserId) { callbacks.forEach { - it.openRoomMemberDetails(roomMember) + it.openRoomMemberDetails(roomMemberId) } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt index 897e56728b..4d87c0e158 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt @@ -21,7 +21,6 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import io.element.android.features.userlist.api.SelectionMode import io.element.android.features.userlist.api.UserListDataSource import io.element.android.features.userlist.api.UserListDataStore @@ -61,33 +60,20 @@ class RoomMemberListPresenter @Inject constructor( @Composable override fun present(): RoomMemberListState { - val coroutineScope = rememberCoroutineScope() val userListState = userListPresenter.present() val allUsers = remember { mutableStateOf>>(Async.Loading()) } - val selectedMember: MutableState = remember { - mutableStateOf(null) - } + LaunchedEffect(Unit) { withContext(coroutineDispatchers.io) { allUsers.value = Async.Success(userListDataSource.search("").toImmutableList()) } } - fun handleEvents(roomMemberListEvents: RoomMemberListEvents) { - when (roomMemberListEvents) { - is RoomMemberListEvents.SelectUser -> coroutineScope.loadRoomMember(roomMemberListEvents.user, selectedMember) - } - } return RoomMemberListState( allUsers = allUsers.value, userListState = userListState, - selectedRoomMember = selectedMember.value, - eventSink = ::handleEvents ) } - private fun CoroutineScope.loadRoomMember(user: MatrixUser, selectedMember: MutableState) = launch(coroutineDispatchers.io) { - selectedMember.value = room.getMemberFlow(user.id).firstOrNull() - } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt index 42289b9ebe..28885006b1 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt @@ -25,6 +25,4 @@ import kotlinx.collections.immutable.ImmutableList data class RoomMemberListState( val allUsers: Async>, val userListState: UserListState, - val selectedRoomMember: RoomMember? = null, - val eventSink: (RoomMemberListEvents) -> Unit, ) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt index 57012c6d86..fc98ae7544 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt @@ -39,5 +39,4 @@ internal fun aRoomMemberListState( RoomMemberListState( userListState = aUserListState().copy(searchResults = searchResults), allUsers = allUsers, - eventSink = {} ) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt index 47aeb635df..99c6a9299a 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt @@ -28,7 +28,6 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.pluralStringResource @@ -52,7 +51,7 @@ import io.element.android.libraries.designsystem.theme.components.CenterAlignedT import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator import io.element.android.libraries.designsystem.theme.components.Scaffold import io.element.android.libraries.designsystem.theme.components.Text -import io.element.android.libraries.matrix.api.room.RoomMember +import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.ui.model.MatrixUser @OptIn(ExperimentalMaterial3Api::class) @@ -61,17 +60,11 @@ fun RoomMemberListView( state: RoomMemberListState, modifier: Modifier = Modifier, onBackPressed: () -> Unit = {}, - onMemberSelected: (RoomMember) -> Unit = {}, + onMemberSelected: (UserId) -> Unit = {}, ) { - LaunchedEffect(state.selectedRoomMember) { - if (state.selectedRoomMember != null) { - onMemberSelected(state.selectedRoomMember) - } - } - fun onUserSelected(user: MatrixUser) { - state.eventSink(RoomMemberListEvents.SelectUser(user)) + onMemberSelected(user.id) } Scaffold( diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsNode.kt index 72e335c1d2..7fd4dd3876 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsNode.kt @@ -30,8 +30,8 @@ import io.element.android.libraries.androidutils.system.startSharePlainTextInten import io.element.android.libraries.architecture.NodeInputs import io.element.android.libraries.architecture.inputs import io.element.android.libraries.di.RoomScope +import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.permalink.PermalinkBuilder -import io.element.android.libraries.matrix.api.room.RoomMember import timber.log.Timber import io.element.android.libraries.androidutils.R as AndroidUtilsR @@ -43,18 +43,18 @@ class RoomMemberDetailsNode @AssistedInject constructor( ) : Node(buildContext, plugins = plugins) { data class Inputs( - val member: RoomMember, + val roomMemberId: UserId, ) : NodeInputs private val inputs = inputs() - private val presenter = presenterFactory.create(inputs.member) + private val presenter = presenterFactory.create(inputs.roomMemberId) @Composable override fun View(modifier: Modifier) { val context = LocalContext.current fun onShareUser() { - val permalinkResult = PermalinkBuilder.permalinkForUser(inputs.member.userId) + val permalinkResult = PermalinkBuilder.permalinkForUser(inputs.roomMemberId) permalinkResult.onSuccess { permalink -> startSharePlainTextIntent( context = context, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt index 33814c29f0..e51205728e 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt @@ -18,6 +18,7 @@ package io.element.android.features.roomdetails.impl.members.details import androidx.compose.runtime.Composable import androidx.compose.runtime.MutableState +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState @@ -28,29 +29,33 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsState.ConfirmationDialog import io.element.android.libraries.architecture.Presenter +import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.matrix.api.MatrixClient -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.room.MatrixRoom -import io.element.android.libraries.matrix.api.room.RoomMember +import io.element.android.libraries.matrix.api.room.getMemberFlow import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch class RoomMemberDetailsPresenter @AssistedInject constructor( private val client: MatrixClient, private val room: MatrixRoom, - @Assisted private val roomMember: RoomMember, + @Assisted private val roomMemberId: UserId, ) : Presenter { interface Factory { - fun create(roomMember: RoomMember): RoomMemberDetailsPresenter + fun create(roomMemberId: UserId): RoomMemberDetailsPresenter } @Composable override fun present(): RoomMemberDetailsState { val coroutineScope = rememberCoroutineScope() var confirmationDialog by remember { mutableStateOf(null) } - val isBlocked = remember { mutableStateOf(roomMember.isIgnored) } + val roomMember by room.getMemberFlow(roomMemberId).collectAsState(initial = null) + + val isBlocked = remember(roomMember?.isIgnored) { + mutableStateOf(roomMember?.isIgnored.orFalse()) + } fun handleEvents(event: RoomMemberDetailsEvents) { when (event) { @@ -59,7 +64,7 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( confirmationDialog = ConfirmationDialog.Block } else { confirmationDialog = null - coroutineScope.blockUser(roomMember.userId, isBlocked) + coroutineScope.blockUser(roomMemberId, isBlocked) } } is RoomMemberDetailsEvents.UnblockUser -> { @@ -67,41 +72,50 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( confirmationDialog = ConfirmationDialog.Unblock } else { confirmationDialog = null - coroutineScope.unblockUser(roomMember.userId, isBlocked) + coroutineScope.unblockUser(roomMemberId, isBlocked) } } RoomMemberDetailsEvents.ClearConfirmationDialog -> confirmationDialog = null } } - val userName by produceState(initialValue = roomMember.displayName) { - room.userDisplayName(roomMember.userId).onSuccess { displayName -> + val userName by produceState(initialValue = roomMember?.displayName) { + room.userDisplayName(roomMemberId).onSuccess { displayName -> if (displayName != null) value = displayName } } - val userAvatar by produceState(initialValue = roomMember.avatarUrl) { - room.userAvatarUrl(roomMember.userId).onSuccess { avatarUrl -> + val userAvatar by produceState(initialValue = roomMember?.avatarUrl) { + room.userAvatarUrl(roomMemberId).onSuccess { avatarUrl -> if (avatarUrl != null) value = avatarUrl } } return RoomMemberDetailsState( - userId = roomMember.userId.value, + userId = roomMemberId.value, userName = userName, avatarUrl = userAvatar, isBlocked = isBlocked.value, displayConfirmationDialog = confirmationDialog, - isCurrentUser = roomMember.userId == client.sessionId, + isCurrentUser = roomMember?.userId == client.sessionId, eventSink = ::handleEvents ) } private fun CoroutineScope.blockUser(userId: UserId, isBlockedState: MutableState) = launch { - client.ignoreUser(userId).onSuccess { isBlockedState.value = true } + client.ignoreUser(userId) + .map { + isBlockedState.value = true + room.updateMembers() + } + } private fun CoroutineScope.unblockUser(userId: UserId, isBlockedState: MutableState) = launch { - client.unignoreUser(userId).onSuccess { isBlockedState.value = false } + client.unignoreUser(userId) + .map { + isBlockedState.value = false + room.updateMembers() + } } } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index 2409172f04..bd0b5906cb 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -58,8 +58,8 @@ class RoomDetailsPresenterTests { private fun aRoomDetailsPresenter(room: MatrixRoom): RoomDetailsPresenter { val roomMemberDetailsPresenterFactory = object : RoomMemberDetailsPresenter.Factory { - override fun create(roomMember: RoomMember): RoomMemberDetailsPresenter { - return RoomMemberDetailsPresenter(aMatrixClient(), room, roomMember) + override fun create(roomMemberId: UserId): RoomMemberDetailsPresenter { + return RoomMemberDetailsPresenter(aMatrixClient(), room, roomMemberId) } } return RoomDetailsPresenter(room, roomMembershipObserver, testCoroutineDispatchers, roomMemberDetailsPresenterFactory) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt index 25bb3e8a7e..3c9bd030b0 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt @@ -16,11 +16,8 @@ package io.element.android.libraries.matrix.api.room -import android.os.Parcelable import io.element.android.libraries.matrix.api.core.UserId -import kotlinx.parcelize.Parcelize -@Parcelize data class RoomMember( val userId: UserId, val displayName: String?, @@ -30,7 +27,7 @@ data class RoomMember( val powerLevel: Long, val normalizedPowerLevel: Long, val isIgnored: Boolean, -) : Parcelable +) enum class RoomMembershipState { BAN, INVITE, JOIN, KNOCK, LEAVE From 23a7b871f73dc919f289b18a3e9bfedfe20cd3f4 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 28 Apr 2023 15:04:33 +0200 Subject: [PATCH 12/13] RoomMembers: change the API again.. --- .../roomdetails/impl/RoomDetailsPresenter.kt | 19 +++--- .../roomdetails/impl/RoomDetailsView.kt | 2 +- .../impl/members/RoomMemberListPresenter.kt | 7 --- .../impl/members/RoomUserListDataSource.kt | 1 + .../details/RoomMemberDetailsPresenter.kt | 14 +++-- .../roomdetails/RoomDetailsPresenterTests.kt | 8 +-- .../RoomMemberDetailsPresenterTests.kt | 22 ++++--- .../libraries/matrix/api/room/MatrixRoom.kt | 20 ------- .../matrix/api/room/MatrixRoomMembersState.kt | 12 ++-- .../matrix/impl/room/RustMatrixRoom.kt | 16 ++--- .../impl/timeline/RustMatrixTimeline.kt | 3 +- .../matrix/ui/room/MatrixRoomMembers.kt | 58 +++++++++++++++++++ 12 files changed, 109 insertions(+), 73 deletions(-) create mode 100644 libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomMembers.kt diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index 3479adbb81..aa46dacd6d 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -17,6 +17,7 @@ package io.element.android.features.roomdetails.impl import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState import androidx.compose.runtime.State import androidx.compose.runtime.collectAsState @@ -33,7 +34,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState 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.getDmMemberFlow +import io.element.android.libraries.matrix.ui.room.directRoomMember import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import javax.inject.Inject @@ -54,14 +55,14 @@ class RoomDetailsPresenter @Inject constructor( val error = remember { mutableStateOf(null) } + LaunchedEffect(Unit) { + room.updateMembers() + } val membersState by room.membersStateFlow.collectAsState() val memberCount by getMemberCount(membersState) - val dmMemberState by room.getDmMemberFlow() - .collectAsState(initial = null, context = coroutineDispatchers.computation) - - val roomMemberDetailsPresenter = roomMemberDetailsPresenter(dmMemberState) - - val roomType = getRoomType(dmMemberState) + val dmMember by room.directRoomMember() + val roomMemberDetailsPresenter = roomMemberDetailsPresenter(dmMember) + val roomType = getRoomType(dmMember) fun handleEvents(event: RoomDetailsEvent) { when (event) { @@ -119,9 +120,9 @@ class RoomDetailsPresenter @Inject constructor( derivedStateOf { when (membersState) { MatrixRoomMembersState.Unknown -> Async.Uninitialized - MatrixRoomMembersState.Pending -> Async.Loading() + is MatrixRoomMembersState.Pending -> Async.Loading(prevState = membersState.prevRoomMembers?.size) + is MatrixRoomMembersState.Error -> Async.Failure(membersState.failure, prevState = membersState.prevRoomMembers?.size) is MatrixRoomMembersState.Ready -> Async.Success(membersState.roomMembers.size) - is MatrixRoomMembersState.Error -> Async.Failure(membersState.failure) } } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsView.kt index 9b3f83d456..dbee610dba 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsView.kt @@ -117,7 +117,7 @@ fun RoomDetailsView( } if (state.roomType is RoomDetailsType.Room) { - val memberCount = (state.memberCount as? Async.Success)?.state + val memberCount = state.memberCount.dataOrNull() MembersSection( memberCount = memberCount, isLoading = state.memberCount.isLoading(), diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt index 4d87c0e158..8d66d60bc2 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt @@ -18,7 +18,6 @@ package io.element.android.features.roomdetails.impl.members import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.MutableState import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import io.element.android.features.userlist.api.SelectionMode @@ -30,14 +29,9 @@ import io.element.android.libraries.architecture.Async import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.matrix.api.room.MatrixRoom -import io.element.android.libraries.matrix.api.room.RoomMember -import io.element.android.libraries.matrix.api.room.getMemberFlow import io.element.android.libraries.matrix.ui.model.MatrixUser import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.toImmutableList -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.firstOrNull -import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import javax.inject.Inject import javax.inject.Named @@ -74,6 +68,5 @@ class RoomMemberListPresenter @Inject constructor( userListState = userListState, ) } - } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt index 9133db7688..e0559f3caf 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomUserListDataSource.kt @@ -41,6 +41,7 @@ class RoomUserListDataSource @Inject constructor( .dropWhile { it !is MatrixRoomMembersState.Ready } .first() .roomMembers() + .orEmpty() val filteredMembers = if (query.isBlank()) { roomMembers } else { diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt index e51205728e..904d4f183b 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt @@ -17,8 +17,8 @@ package io.element.android.features.roomdetails.impl.members.details import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState -import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState @@ -33,9 +33,10 @@ import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom -import io.element.android.libraries.matrix.api.room.getMemberFlow +import io.element.android.libraries.matrix.ui.room.roomMember import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch +import timber.log.Timber class RoomMemberDetailsPresenter @AssistedInject constructor( private val client: MatrixClient, @@ -51,11 +52,14 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( override fun present(): RoomMemberDetailsState { val coroutineScope = rememberCoroutineScope() var confirmationDialog by remember { mutableStateOf(null) } - val roomMember by room.getMemberFlow(roomMemberId).collectAsState(initial = null) - - val isBlocked = remember(roomMember?.isIgnored) { + val roomMember by room.roomMember(roomMemberId) + // the room member is not really live... + val isBlocked = remember { mutableStateOf(roomMember?.isIgnored.orFalse()) } + LaunchedEffect(Unit) { + room.updateMembers() + } fun handleEvents(event: RoomMemberDetailsEvents) { when (event) { diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index bd0b5906cb..585a61f05d 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -96,6 +96,7 @@ class RoomDetailsPresenterTests { room.givenRoomMembersState(MatrixRoomMembersState.Ready(emptyList())) val finalState = awaitItem() Truth.assertThat(finalState.memberCount).isEqualTo(Async.Success(0)) + cancelAndIgnoreRemainingEvents() } } @@ -129,12 +130,7 @@ class RoomDetailsPresenterTests { presenter.present() }.test { val initialState = awaitItem() - // It's not configured yet in the first iteration - Truth.assertThat(initialState.roomType).isEqualTo(RoomDetailsType.Room) - - // Once updated, the RoomDetailsType becomes 'Dm' - val updatedState = awaitItem() - Truth.assertThat(updatedState.roomType).isEqualTo(RoomDetailsType.Dm(otherRoomMember)) + Truth.assertThat(initialState.roomType).isEqualTo(RoomDetailsType.Dm(otherRoomMember)) cancelAndIgnoreRemainingEvents() } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt index 3731c910fc..13eb28ca85 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/details/RoomMemberDetailsPresenterTests.kt @@ -26,6 +26,7 @@ import io.element.android.features.roomdetails.aRoomMember import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsEvents import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsPresenter import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsState +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Test @@ -37,12 +38,13 @@ class RoomMemberDetailsPresenterTests { @Test fun `present - returns the room member's data, then updates it if needed`() = runTest { + val roomMember = aRoomMember(displayName = "Alice") val room = aMatrixRoom().apply { givenUserDisplayNameResult(Result.success("A custom name")) givenUserAvatarUrlResult(Result.success("A custom avatar")) + givenRoomMembersState(MatrixRoomMembersState.Ready(listOf(roomMember))) } - val roomMember = aRoomMember(displayName = "Alice") - val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) + val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember.userId) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -60,12 +62,13 @@ class RoomMemberDetailsPresenterTests { @Test fun `present - will recover when retrieving room member details fails`() = runTest { + val roomMember = aRoomMember(displayName = "Alice") val room = aMatrixRoom().apply { givenUserDisplayNameResult(Result.failure(Throwable())) givenUserAvatarUrlResult(Result.failure(Throwable())) + givenRoomMembersState(MatrixRoomMembersState.Ready(listOf(roomMember))) } - val roomMember = aRoomMember(displayName = "Alice") - val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) + val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember.userId) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -79,12 +82,13 @@ class RoomMemberDetailsPresenterTests { @Test fun `present - will fallback to original data if the updated data is null`() = runTest { + val roomMember = aRoomMember(displayName = "Alice") val room = aMatrixRoom().apply { givenUserDisplayNameResult(Result.success(null)) givenUserAvatarUrlResult(Result.success(null)) + givenRoomMembersState(MatrixRoomMembersState.Ready(listOf(roomMember))) } - val roomMember = aRoomMember(displayName = "Alice") - val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) + val presenter =RoomMemberDetailsPresenter(matrixClient, room, roomMember.userId) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -100,7 +104,7 @@ class RoomMemberDetailsPresenterTests { fun `present - BlockUser needing confirmation displays confirmation dialog`() = runTest { val room = aMatrixRoom() val roomMember = aRoomMember() - val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) + val presenter =RoomMemberDetailsPresenter(matrixClient, room, roomMember.userId) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -121,7 +125,7 @@ class RoomMemberDetailsPresenterTests { fun `present - BlockUser and UnblockUser without confirmation change the 'blocked' state`() = runTest { val room = aMatrixRoom() val roomMember = aRoomMember() - val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) + val presenter =RoomMemberDetailsPresenter(matrixClient, room, roomMember.userId) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { @@ -138,7 +142,7 @@ class RoomMemberDetailsPresenterTests { fun `present - UnblockUser needing confirmation displays confirmation dialog`() = runTest { val room = aMatrixRoom() val roomMember = aRoomMember() - val presenter = RoomMemberDetailsPresenter(matrixClient, room, roomMember) + val presenter =RoomMemberDetailsPresenter(matrixClient, room, roomMember.userId) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { 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 41cd874098..35451874aa 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 @@ -23,7 +23,6 @@ import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.map import java.io.Closeable interface MatrixRoom : Closeable { @@ -74,22 +73,3 @@ interface MatrixRoom : Closeable { suspend fun rejectInvitation(): Result } - -fun MatrixRoom.getMemberFlow(userId: UserId): Flow { - return membersStateFlow.map { state -> - state.roomMembers().find { - it.userId == userId - } - } -} - -fun MatrixRoom.getDmMemberFlow(): Flow { - return membersStateFlow.map { state -> - val members = state.roomMembers() - if (members.size == 2 && isDirect && isEncrypted) { - members.find { it.userId != this.sessionId } - } else { - null - } - } -} diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomMembersState.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomMembersState.kt index 319a5f7605..4e41fd43ba 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomMembersState.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomMembersState.kt @@ -18,14 +18,18 @@ package io.element.android.libraries.matrix.api.room sealed interface MatrixRoomMembersState { object Unknown : MatrixRoomMembersState - object Pending : MatrixRoomMembersState - data class Error(val failure: Throwable) : MatrixRoomMembersState + data class Pending(val prevRoomMembers: List? = null) : MatrixRoomMembersState + data class Error(val failure: Throwable, val prevRoomMembers: List? = null) : MatrixRoomMembersState data class Ready(val roomMembers: List) : MatrixRoomMembersState } -fun MatrixRoomMembersState.roomMembers(): List { +fun MatrixRoomMembersState.roomMembers(): List? { return when (this) { is MatrixRoomMembersState.Ready -> roomMembers - else -> emptyList() + is MatrixRoomMembersState.Pending -> prevRoomMembers + is MatrixRoomMembersState.Error -> prevRoomMembers + else -> null } } + + 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 1e75867215..9caba227ce 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 @@ -23,6 +23,7 @@ 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.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState +import io.element.android.libraries.matrix.api.room.roomMembers import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.impl.timeline.RustMatrixTimeline import kotlinx.coroutines.CoroutineScope @@ -32,14 +33,13 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onStart +import kotlinx.coroutines.flow.onSubscription import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.Room import org.matrix.rustcomponents.sdk.SlidingSyncRoom import org.matrix.rustcomponents.sdk.UpdateSummary import org.matrix.rustcomponents.sdk.genTransactionId import org.matrix.rustcomponents.sdk.messageEventContentFromMarkdown -import org.matrix.rustcomponents.sdk.use -import org.matrix.rustcomponents.sdk.RoomMember as RustRoomMember class RustMatrixRoom( override val sessionId: SessionId, @@ -128,13 +128,15 @@ class RustMatrixRoom( get() = innerRoom.isDirect() override suspend fun updateMembers(): Result = withContext(coroutineDispatchers.io) { - _membersStateFlow.value = MatrixRoomMembersState.Pending + val currentState = _membersStateFlow.value + val currentMembers = currentState.roomMembers() + _membersStateFlow.value = MatrixRoomMembersState.Pending(prevRoomMembers = currentMembers) runCatching { innerRoom.members().map(RoomMemberMapper::map) }.map { _membersStateFlow.value = MatrixRoomMembersState.Ready(it) }.onFailure { - _membersStateFlow.value = MatrixRoomMembersState.Error(it) + _membersStateFlow.value = MatrixRoomMembersState.Error(prevRoomMembers = currentMembers, failure = it) } } @@ -201,10 +203,4 @@ class RustMatrixRoom( } } - private fun findRoomMember(userId: UserId, action: (RustRoomMember).() -> Unit) { - return innerRoom.members() - .find { it.userId() == userId.value } - ?.use(action) - ?: error("No member with userId $userId exists in room $roomId") - } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt index bffaa30886..6f64db76ef 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt @@ -153,8 +153,7 @@ class RustMatrixTimeline( RequiredState(key = "m.room.topic", value = ""), RequiredState(key = "m.room.join_rules", value = ""), ), - //TODO allow configuration - timelineLimit = 20.toUInt() + timelineLimit = null ) val result = slidingSyncRoom.subscribeAndAddTimelineListener(timelineListener, settings) launch { diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomMembers.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomMembers.kt new file mode 100644 index 0000000000..2df7c1a152 --- /dev/null +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomMembers.kt @@ -0,0 +1,58 @@ +/* + * 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.libraries.matrix.ui.room + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.State +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.derivedStateOf +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember +import io.element.android.libraries.matrix.api.core.UserId +import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.api.room.RoomMember +import io.element.android.libraries.matrix.api.room.roomMembers + +@Composable +fun MatrixRoom.roomMember(userId: UserId): State { + val roomMembersState by membersStateFlow.collectAsState() + val roomMembers = roomMembersState.roomMembers() + return remember(roomMembers) { + derivedStateOf { + roomMembers?.find { + it.userId == userId + } + } + } +} + +@Composable +fun MatrixRoom.directRoomMember(): State { + val roomMembersState by membersStateFlow.collectAsState() + val roomMembers = roomMembersState.roomMembers() + return remember(roomMembers) { + derivedStateOf { + if (roomMembers == null) { + null + } else if (roomMembers.size == 2 && isDirect && isEncrypted) { + roomMembers.find { it.userId != this.sessionId } + } else { + null + } + } + } +} From 58e2c93018572d497bf41612cd544136da2cdb6e Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 2 May 2023 13:04:00 +0200 Subject: [PATCH 13/13] Update tests and avoid useless recomposition --- .../roomdetails/impl/RoomDetailsPresenter.kt | 23 ++++++----- .../details/RoomMemberDetailsPresenter.kt | 5 +-- .../roomdetails/RoomDetailsPresenterTests.kt | 38 ++++++++++--------- .../matrix/ui/room/MatrixRoomMembers.kt | 16 +++++++- 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index aa46dacd6d..8f96487583 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -34,7 +34,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.room.RoomMember import io.element.android.libraries.matrix.api.room.RoomMembershipObserver -import io.element.android.libraries.matrix.ui.room.directRoomMember +import io.element.android.libraries.matrix.ui.room.getDirectRoomMember import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import javax.inject.Inject @@ -58,9 +58,10 @@ class RoomDetailsPresenter @Inject constructor( LaunchedEffect(Unit) { room.updateMembers() } + val membersState by room.membersStateFlow.collectAsState() val memberCount by getMemberCount(membersState) - val dmMember by room.directRoomMember() + val dmMember by room.getDirectRoomMember(membersState) val roomMemberDetailsPresenter = roomMemberDetailsPresenter(dmMember) val roomType = getRoomType(dmMember) @@ -116,13 +117,15 @@ class RoomDetailsPresenter @Inject constructor( } @Composable - private fun getMemberCount(membersState: MatrixRoomMembersState): State> = remember(membersState) { - derivedStateOf { - when (membersState) { - MatrixRoomMembersState.Unknown -> Async.Uninitialized - is MatrixRoomMembersState.Pending -> Async.Loading(prevState = membersState.prevRoomMembers?.size) - is MatrixRoomMembersState.Error -> Async.Failure(membersState.failure, prevState = membersState.prevRoomMembers?.size) - is MatrixRoomMembersState.Ready -> Async.Success(membersState.roomMembers.size) + private fun getMemberCount(membersState: MatrixRoomMembersState): State> { + return remember(membersState) { + derivedStateOf { + when (membersState) { + MatrixRoomMembersState.Unknown -> Async.Uninitialized + is MatrixRoomMembersState.Pending -> Async.Loading(prevState = membersState.prevRoomMembers?.size) + is MatrixRoomMembersState.Error -> Async.Failure(membersState.failure, prevState = membersState.prevRoomMembers?.size) + is MatrixRoomMembersState.Ready -> Async.Success(membersState.roomMembers.size) + } } } } @@ -148,3 +151,5 @@ class RoomDetailsPresenter @Inject constructor( } + + diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt index 904d4f183b..594152e241 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt @@ -33,10 +33,9 @@ import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom -import io.element.android.libraries.matrix.ui.room.roomMember +import io.element.android.libraries.matrix.ui.room.getRoomMember import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch -import timber.log.Timber class RoomMemberDetailsPresenter @AssistedInject constructor( private val client: MatrixClient, @@ -52,7 +51,7 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( override fun present(): RoomMemberDetailsState { val coroutineScope = rememberCoroutineScope() var confirmationDialog by remember { mutableStateOf(null) } - val roomMember by room.roomMember(roomMemberId) + val roomMember by room.getRoomMember(roomMemberId) // the room member is not really live... val isBlocked = remember { mutableStateOf(roomMember?.isIgnored.orFalse()) diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index 585a61f05d..2cc6d0ec24 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -86,16 +86,34 @@ class RoomDetailsPresenterTests { @Test fun `present - room member count is calculated asynchronously`() = runTest { + val error = RuntimeException() val room = aMatrixRoom() + val roomMembers = listOf( + aRoomMember(A_USER_ID), + aRoomMember(A_USER_ID_2), + ) val presenter = aRoomDetailsPresenter(room) moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { + room.givenRoomMembersState(MatrixRoomMembersState.Unknown) val initialState = awaitItem() Truth.assertThat(initialState.memberCount).isEqualTo(Async.Uninitialized) - room.givenRoomMembersState(MatrixRoomMembersState.Ready(emptyList())) - val finalState = awaitItem() - Truth.assertThat(finalState.memberCount).isEqualTo(Async.Success(0)) + + room.givenRoomMembersState(MatrixRoomMembersState.Pending(null)) + val loadingState = awaitItem() + Truth.assertThat(loadingState.memberCount).isEqualTo(Async.Loading(null)) + + room.givenRoomMembersState(MatrixRoomMembersState.Error(error)) + //skipItems(1) + val failureState = awaitItem() + Truth.assertThat(failureState.memberCount).isEqualTo(Async.Failure(error, null)) + + room.givenRoomMembersState(MatrixRoomMembersState.Ready(roomMembers)) + //skipItems(1) + val successState = awaitItem() + Truth.assertThat(successState.memberCount).isEqualTo(Async.Success(roomMembers.size)) + cancelAndIgnoreRemainingEvents() } } @@ -136,20 +154,6 @@ class RoomDetailsPresenterTests { } } - @Test - fun `present - can handle error while fetching member count`() = runTest { - val room = aMatrixRoom(name = null).apply { - givenRoomMembersState(MatrixRoomMembersState.Error(Throwable())) - } - val presenter = aRoomDetailsPresenter(room) - moleculeFlow(RecompositionClock.Immediate) { - presenter.present() - }.test { - Truth.assertThat(awaitItem().memberCount).isInstanceOf(Async.Failure::class.java) - cancelAndIgnoreRemainingEvents() - } - } - @Test fun `present - Leave with confirmation on private room shows a specific warning`() = runTest { val room = aMatrixRoom(isPublic = false).apply { diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomMembers.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomMembers.kt index 2df7c1a152..061ce365eb 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomMembers.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/room/MatrixRoomMembers.kt @@ -24,12 +24,18 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.room.RoomMember import io.element.android.libraries.matrix.api.room.roomMembers @Composable -fun MatrixRoom.roomMember(userId: UserId): State { +fun MatrixRoom.getRoomMember(userId: UserId): State { val roomMembersState by membersStateFlow.collectAsState() + return getRoomMember(roomMembersState = roomMembersState, userId = userId) +} + +@Composable +fun getRoomMember(roomMembersState: MatrixRoomMembersState, userId: UserId): State { val roomMembers = roomMembersState.roomMembers() return remember(roomMembers) { derivedStateOf { @@ -41,8 +47,13 @@ fun MatrixRoom.roomMember(userId: UserId): State { } @Composable -fun MatrixRoom.directRoomMember(): State { +fun MatrixRoom.getDirectRoomMember(): State { val roomMembersState by membersStateFlow.collectAsState() + return getDirectRoomMember(roomMembersState = roomMembersState) +} + +@Composable +fun MatrixRoom.getDirectRoomMember(roomMembersState: MatrixRoomMembersState): State { val roomMembers = roomMembersState.roomMembers() return remember(roomMembers) { derivedStateOf { @@ -56,3 +67,4 @@ fun MatrixRoom.directRoomMember(): State { } } } +