diff --git a/changelog.d/2721.misc b/changelog.d/2721.misc new file mode 100644 index 0000000000..2c38dc4ac7 --- /dev/null +++ b/changelog.d/2721.misc @@ -0,0 +1 @@ +RoomMember screen: fallback to userProfile data, if the member is not a user of the room. 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 cb749d2c80..dcb953e978 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 @@ -37,8 +37,13 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.matrix.ui.room.getRoomMemberAsState import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch class RoomMemberDetailsPresenter @AssistedInject constructor( @@ -56,20 +61,24 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( val coroutineScope = rememberCoroutineScope() var confirmationDialog by remember { mutableStateOf(null) } val roomMember by room.getRoomMemberAsState(roomMemberId) + var userProfile by remember { mutableStateOf(null) } val startDmActionState: MutableState> = remember { mutableStateOf(AsyncAction.Uninitialized) } - // the room member is not really live... - val isBlocked: MutableState> = remember(roomMember) { - val isIgnored = roomMember?.isIgnored - if (isIgnored == null) { - mutableStateOf(AsyncData.Uninitialized) - } else { - mutableStateOf(AsyncData.Success(isIgnored)) - } + val isBlocked: MutableState> = remember { mutableStateOf(AsyncData.Uninitialized) } + LaunchedEffect(Unit) { + client.ignoredUsersFlow + .map { ignoredUsers -> roomMemberId in ignoredUsers } + .distinctUntilChanged() + .onEach { isBlocked.value = AsyncData.Success(it) } + .launchIn(this) } LaunchedEffect(Unit) { // Update room member info when opening this screen // We don't need to assign the result as it will be automatically propagated by `room.getRoomMemberAsState` room.getUpdatedMember(roomMemberId) + .onFailure { + // Not a member of the room, try to get the user profile + userProfile = client.getProfile(roomMemberId).getOrNull() + } } fun handleEvents(event: RoomMemberDetailsEvents) { @@ -105,16 +114,34 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( } } - val userName by produceState(initialValue = roomMember?.displayName) { - room.userDisplayName(roomMemberId).onSuccess { displayName -> - if (displayName != null) value = displayName - } + val userName: String? by produceState( + initialValue = roomMember?.displayName ?: userProfile?.displayName, + key1 = roomMember, + key2 = userProfile, + ) { + value = room.userDisplayName(roomMemberId) + .fold( + onSuccess = { it }, + onFailure = { + // Fallback to user profile + userProfile?.displayName + } + ) } - val userAvatar by produceState(initialValue = roomMember?.avatarUrl) { - room.userAvatarUrl(roomMemberId).onSuccess { avatarUrl -> - if (avatarUrl != null) value = avatarUrl - } + val userAvatar: String? by produceState( + initialValue = roomMember?.avatarUrl ?: userProfile?.avatarUrl, + key1 = roomMember, + key2 = userProfile, + ) { + value = room.userAvatarUrl(roomMemberId) + .fold( + onSuccess = { it }, + onFailure = { + // Fallback to user profile + userProfile?.avatarUrl + } + ) } return RoomMemberDetailsState( @@ -124,7 +151,7 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( isBlocked = isBlocked.value, startDmActionState = startDmActionState.value, displayConfirmationDialog = confirmationDialog, - isCurrentUser = client.isMe(roomMember?.userId), + isCurrentUser = client.isMe(roomMemberId), eventSink = ::handleEvents ) } @@ -132,28 +159,18 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( private fun CoroutineScope.blockUser(userId: UserId, isBlockedState: MutableState>) = launch { isBlockedState.value = AsyncData.Loading(false) client.ignoreUser(userId) - .fold( - onSuccess = { - isBlockedState.value = AsyncData.Success(true) - room.getUpdatedMember(userId) - }, - onFailure = { - isBlockedState.value = AsyncData.Failure(it, false) - } - ) + .onFailure { + isBlockedState.value = AsyncData.Failure(it, false) + } + // Note: on success, ignoredUserList will be updated. } private fun CoroutineScope.unblockUser(userId: UserId, isBlockedState: MutableState>) = launch { isBlockedState.value = AsyncData.Loading(true) client.unignoreUser(userId) - .fold( - onSuccess = { - isBlockedState.value = AsyncData.Success(false) - room.getUpdatedMember(userId) - }, - onFailure = { - isBlockedState.value = AsyncData.Failure(it, true) - } - ) + .onFailure { + isBlockedState.value = AsyncData.Failure(it, true) + } + // Note: on success, ignoredUserList will be updated. } } 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 e1f0d76ec0..4c8b944c8a 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 @@ -18,6 +18,7 @@ package io.element.android.features.roomdetails.members.details import app.cash.molecule.RecompositionMode import app.cash.molecule.moleculeFlow +import app.cash.turbine.ReceiveTurbine import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.features.createroom.api.StartDMAction @@ -30,12 +31,13 @@ import io.element.android.features.roomdetails.impl.members.details.RoomMemberDe import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData 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.MatrixRoomMembersState -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.A_THROWABLE import io.element.android.libraries.matrix.test.FakeMatrixClient +import io.element.android.libraries.matrix.ui.components.aMatrixUser import io.element.android.tests.testutils.WarmUpRule import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -58,12 +60,12 @@ class RoomMemberDetailsPresenterTests { } val presenter = createRoomMemberDetailsPresenter( room = room, - roomMember = roomMember + roomMemberId = roomMember.userId ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() + val initialState = awaitFirstItem() assertThat(initialState.userId).isEqualTo(roomMember.userId.value) assertThat(initialState.userName).isEqualTo(roomMember.displayName) assertThat(initialState.avatarUrl).isEqualTo(roomMember.avatarUrl) @@ -85,12 +87,12 @@ class RoomMemberDetailsPresenterTests { } val presenter = createRoomMemberDetailsPresenter( room = room, - roomMember = roomMember + roomMemberId = roomMember.userId ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() + val initialState = awaitFirstItem() assertThat(initialState.userName).isEqualTo(roomMember.displayName) assertThat(initialState.avatarUrl).isEqualTo(roomMember.avatarUrl) @@ -108,12 +110,12 @@ class RoomMemberDetailsPresenterTests { } val presenter = createRoomMemberDetailsPresenter( room = room, - roomMember = roomMember + roomMemberId = roomMember.userId ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() + val initialState = awaitFirstItem() assertThat(initialState.userName).isEqualTo(roomMember.displayName) assertThat(initialState.avatarUrl).isEqualTo(roomMember.avatarUrl) @@ -121,13 +123,40 @@ class RoomMemberDetailsPresenterTests { } } + @Test + fun `present - will fallback to user profile if user is not a member of the room`() = runTest { + val bobProfile = aMatrixUser("@bob:server.org", "Bob", avatarUrl = "anAvatarUrl") + val room = aMatrixRoom().apply { + givenUserDisplayNameResult(Result.failure(Exception("Not a member!"))) + givenUserAvatarUrlResult(Result.failure(Exception("Not a member!"))) + } + val client = FakeMatrixClient().apply { + givenGetProfileResult(bobProfile.userId, Result.success(bobProfile)) + } + val presenter = createRoomMemberDetailsPresenter( + client = client, + room = room, + roomMemberId = UserId("@bob:server.org") + ) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + skipItems(2) + val initialState = awaitFirstItem() + assertThat(initialState.userName).isEqualTo("Bob") + assertThat(initialState.avatarUrl).isEqualTo("anAvatarUrl") + + ensureAllEventsConsumed() + } + } + @Test fun `present - BlockUser needing confirmation displays confirmation dialog`() = runTest { val presenter = createRoomMemberDetailsPresenter() moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() + val initialState = awaitFirstItem() initialState.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = true)) val dialogState = awaitItem() @@ -142,17 +171,24 @@ class RoomMemberDetailsPresenterTests { @Test fun `present - BlockUser and UnblockUser without confirmation change the 'blocked' state`() = runTest { - val presenter = createRoomMemberDetailsPresenter() + val client = FakeMatrixClient() + val roomMember = aRoomMember() + val presenter = createRoomMemberDetailsPresenter( + client = client, + roomMemberId = roomMember.userId + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() + val initialState = awaitFirstItem() initialState.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = false)) assertThat(awaitItem().isBlocked.isLoading()).isTrue() + client.emitIgnoreUserList(listOf(roomMember.userId)) assertThat(awaitItem().isBlocked.dataOrNull()).isTrue() initialState.eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = false)) assertThat(awaitItem().isBlocked.isLoading()).isTrue() + client.emitIgnoreUserList(listOf()) assertThat(awaitItem().isBlocked.dataOrNull()).isFalse() } } @@ -165,7 +201,7 @@ class RoomMemberDetailsPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() + val initialState = awaitFirstItem() initialState.eventSink(RoomMemberDetailsEvents.BlockUser(needsConfirmation = false)) assertThat(awaitItem().isBlocked.isLoading()).isTrue() val errorState = awaitItem() @@ -176,13 +212,32 @@ class RoomMemberDetailsPresenterTests { } } + @Test + fun `present - UnblockUser with error`() = runTest { + val matrixClient = FakeMatrixClient() + matrixClient.givenUnignoreUserResult(Result.failure(A_THROWABLE)) + val presenter = createRoomMemberDetailsPresenter(client = matrixClient) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitFirstItem() + initialState.eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = false)) + assertThat(awaitItem().isBlocked.isLoading()).isTrue() + val errorState = awaitItem() + assertThat(errorState.isBlocked.errorOrNull()).isEqualTo(A_THROWABLE) + // Clear error + initialState.eventSink(RoomMemberDetailsEvents.ClearBlockUserError) + assertThat(awaitItem().isBlocked).isEqualTo(AsyncData.Success(true)) + } + } + @Test fun `present - UnblockUser needing confirmation displays confirmation dialog`() = runTest { val presenter = createRoomMemberDetailsPresenter() moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() + val initialState = awaitFirstItem() initialState.eventSink(RoomMemberDetailsEvents.UnblockUser(needsConfirmation = true)) val dialogState = awaitItem() @@ -202,7 +257,7 @@ class RoomMemberDetailsPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val initialState = awaitItem() + val initialState = awaitFirstItem() assertThat(initialState.startDmActionState).isInstanceOf(AsyncAction.Uninitialized::class.java) val startDMSuccessResult = AsyncAction.Success(A_ROOM_ID) val startDMFailureResult = AsyncAction.Failure(A_THROWABLE) @@ -229,14 +284,19 @@ class RoomMemberDetailsPresenterTests { } } + private suspend fun ReceiveTurbine.awaitFirstItem(): T { + skipItems(1) + return awaitItem() + } + private fun createRoomMemberDetailsPresenter( client: MatrixClient = FakeMatrixClient(), room: MatrixRoom = aMatrixRoom(), - roomMember: RoomMember = aRoomMember(), + roomMemberId: UserId = UserId("@alice:server.org"), startDMAction: StartDMAction = FakeStartDMAction() ): RoomMemberDetailsPresenter { return RoomMemberDetailsPresenter( - roomMemberId = roomMember.userId, + roomMemberId = roomMemberId, client = client, room = room, startDMAction = startDMAction 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 7e86f29cb7..3feb161cb3 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 @@ -48,6 +48,7 @@ import io.element.android.libraries.matrix.test.verification.FakeSessionVerifica import io.element.android.tests.testutils.simulateLongTask import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf +import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow @@ -205,6 +206,10 @@ class FakeMatrixClient( return RoomMembershipObserver() } + suspend fun emitIgnoreUserList(users: List) { + ignoredUsersFlow.emit(users.toImmutableList()) + } + // Mocks fun givenLogoutError(failure: Throwable?) {