Merge pull request #2721 from element-hq/feature/bma/roomMemberDetailsPresenterImprovement

Room member details presenter improvement

changelog.d/2721.misc

@@ -0,0 +1 @@
+RoomMember screen: fallback to userProfile data, if the member is not a user of the room.

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<ConfirmationDialog?>(null) }
         val roomMember by room.getRoomMemberAsState(roomMemberId)
+        var userProfile by remember { mutableStateOf<MatrixUser?>(null) }
         val startDmActionState: MutableState<AsyncAction<RoomId>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
-        // the room member is not really live...
+        val isBlocked: MutableState<AsyncData<Boolean>> = remember { mutableStateOf(AsyncData.Uninitialized) }
-        val isBlocked: MutableState<AsyncData<Boolean>> = remember(roomMember) {
+        LaunchedEffect(Unit) {
-            val isIgnored = roomMember?.isIgnored
+            client.ignoredUsersFlow
-            if (isIgnored == null) {
+                .map { ignoredUsers -> roomMemberId in ignoredUsers }
-                mutableStateOf(AsyncData.Uninitialized)
+                .distinctUntilChanged()
-            } else {
+                .onEach { isBlocked.value = AsyncData.Success(it) }
-                mutableStateOf(AsyncData.Success(isIgnored))
+                .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) {
+        val userName: String? by produceState(
-            room.userDisplayName(roomMemberId).onSuccess { displayName ->
+            initialValue = roomMember?.displayName ?: userProfile?.displayName,
-                if (displayName != null) value = 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) {
+        val userAvatar: String? by produceState(
-            room.userAvatarUrl(roomMemberId).onSuccess { avatarUrl ->
+            initialValue = roomMember?.avatarUrl ?: userProfile?.avatarUrl,
-                if (avatarUrl != null) value = 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<AsyncData<Boolean>>) = launch {
         isBlockedState.value = AsyncData.Loading(false)
         client.ignoreUser(userId)
-            .fold(
+            .onFailure {
-                onSuccess = {
+                isBlockedState.value = AsyncData.Failure(it, false)
-                    isBlockedState.value = AsyncData.Success(true)
+            }
-                    room.getUpdatedMember(userId)
+        // Note: on success, ignoredUserList will be updated.
-                },
-                onFailure = {
-                    isBlockedState.value = AsyncData.Failure(it, false)
-                }
-            )
     }
 
     private fun CoroutineScope.unblockUser(userId: UserId, isBlockedState: MutableState<AsyncData<Boolean>>) = launch {
         isBlockedState.value = AsyncData.Loading(true)
         client.unignoreUser(userId)
-            .fold(
+            .onFailure {
-                onSuccess = {
+                isBlockedState.value = AsyncData.Failure(it, true)
-                    isBlockedState.value = AsyncData.Success(false)
+            }
-                    room.getUpdatedMember(userId)
+        // Note: on success, ignoredUserList will be updated.
-                },
-                onFailure = {
-                    isBlockedState.value = AsyncData.Failure(it, true)
-                }
-            )
     }
 }

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 <T> ReceiveTurbine<T>.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

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<UserId>) {
+        ignoredUsersFlow.emit(users.toImmutableList())
+    }
+
     // Mocks
 
     fun givenLogoutError(failure: Throwable?) {