From 58e2c93018572d497bf41612cd544136da2cdb6e Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 2 May 2023 13:04:00 +0200 Subject: [PATCH] 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 { } } } +