Reload member list after moderation actions (#5268)

* Reload member list after moderation actions

The previous `runActionAndWaitForMembershipChange` logic wasn't really doing anything, as the modified flow was never used.

* Make sure we always set the value in the member list state flow, even if the underlying coroutine scope is no longer there.

With `emit`, the `Ready` state was not emitted if the member list was loaded way too fast.

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt

@@ -38,6 +38,7 @@ import kotlinx.collections.immutable.ImmutableMap
 import kotlinx.collections.immutable.persistentMapOf
 import kotlinx.collections.immutable.toImmutableList
 import kotlinx.collections.immutable.toPersistentMap
+import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.launchIn
@@ -66,11 +67,6 @@ class RoomMemberListPresenter @Inject constructor(
         val syncUpdateFlow = room.syncUpdateFlow.collectAsState()
         val canInvite by room.canInviteAsState(syncUpdateFlow.value)
         val roomModerationState = roomMembersModerationPresenter.present()
-        val activeRoomMemberCount by produceState(0L) {
-            room.roomInfoFlow.map { it.activeMembersCount }
-                .distinctUntilChanged()
-                .collect { value = it }
-        }
 
         val roomMemberIdentityStates by produceState(persistentMapOf<UserId, IdentityState>()) {
             room.roomMemberIdentityStateChange(waitForEncryption = true)
@@ -81,8 +77,12 @@ class RoomMemberListPresenter @Inject constructor(
         }
 
         // Update the room members when the screen is loaded or the active member count changes
-        LaunchedEffect(activeRoomMemberCount) {
+        LaunchedEffect(Unit) {
-            room.updateMembers()
+            room.roomInfoFlow.map { it.activeMembersCount }
+                .distinctUntilChanged()
+                .collectLatest {
+                    room.updateMembers()
+                }
         }
 
         LaunchedEffect(membersState, roomMemberIdentityStates) {

features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenterTest.kt

@@ -116,9 +116,6 @@ class RoomMemberListPresenterTest {
                 }
             }
 
-            // Wait for the update to be processed
-            skipItems(1)
-
             // Update the room members state as `Room.updateMembers()` would have done with the actual implementation
             room.givenRoomMembersState(RoomMembersState.Ready(persistentListOf()))
             // Wait for another update

features/roommembermoderation/impl/src/main/kotlin/io/element/android/features/roommembermoderation/impl/RoomMemberModerationPresenter.kt

@@ -38,10 +38,12 @@ import kotlinx.collections.immutable.PersistentList
 import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.drop
-import kotlinx.coroutines.flow.take
+import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.launch
 import javax.inject.Inject
+import kotlin.time.Duration.Companion.milliseconds
 
 class RoomMemberModerationPresenter @Inject constructor(
     private val room: JoinedRoom,
@@ -82,6 +84,9 @@ class RoomMemberModerationPresenter @Inject constructor(
                     )
                 }
                 is RoomMemberModerationEvents.ProcessAction -> {
+                    // First, hide any list of existing actions that could be displayed
+                    moderationActions.value = persistentListOf()
+
                     when (event.action) {
                         is ModerationAction.DisplayProfile -> Unit
                         is ModerationAction.KickUser -> {
@@ -118,6 +123,7 @@ class RoomMemberModerationPresenter @Inject constructor(
                 }
                 is InternalRoomMemberModerationEvents.Reset -> {
                     selectedUser = null
+                    moderationActions.value = persistentListOf()
                     kickUserAsyncAction.value = AsyncAction.Uninitialized
                     banUserAsyncAction.value = AsyncAction.Uninitialized
                     unbanUserAsyncAction.value = AsyncAction.Uninitialized
@@ -204,7 +210,15 @@ class RoomMemberModerationPresenter @Inject constructor(
             action.runUpdatingState {
                 val result = block()
                 if (result.isSuccess) {
-                    room.membersStateFlow.drop(1).take(1)
+                    // We wait a bit to ensure the server has processed the membership change
+                    delay(50.milliseconds)
+
+                    // Update the members to ensure we have the latest state
+                    launch { room.updateMembers() }
+
+                    // Wait for the membership change to be processed and returned
+                    // We drop the first emission as it's the current state
+                    room.membersStateFlow.drop(1).first()
                 }
                 result
             }

features/roommembermoderation/impl/src/test/kotlin/io/element/android/features/roommembermoderation/impl/RoomMemberModerationPresenterTest.kt

@@ -29,6 +29,7 @@ import io.element.android.services.analytics.test.FakeAnalyticsService
 import io.element.android.tests.testutils.WarmUpRule
 import io.element.android.tests.testutils.test
 import io.element.android.tests.testutils.testCoroutineDispatchers
+import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runTest
@@ -217,7 +218,14 @@ class RoomMemberModerationPresenterTest {
 
     @Test
     fun `present - do kick user with success`() = runTest {
-        createRoomMemberModerationPresenter(room = aJoinedRoom()).test {
+        val room = aJoinedRoom()
+        room.baseRoom.givenUpdateMembersResult {
+            // Simulate the member list being updated
+            room.givenRoomMembersState(RoomMembersState.Ready(
+                persistentListOf(aRoomMember())
+            ))
+        }
+        createRoomMemberModerationPresenter(room = room).test {
             val initialState = awaitState()
             initialState.eventSink(
                 RoomMemberModerationEvents.ProcessAction(
@@ -238,7 +246,14 @@ class RoomMemberModerationPresenterTest {
 
     @Test
     fun `present - do ban user with success`() = runTest {
-        createRoomMemberModerationPresenter(room = aJoinedRoom()).test {
+        val room = aJoinedRoom()
+        room.baseRoom.givenUpdateMembersResult {
+            // Simulate the member list being updated
+            room.givenRoomMembersState(RoomMembersState.Ready(
+                persistentListOf(aRoomMember())
+            ))
+        }
+        createRoomMemberModerationPresenter(room = room).test {
             val initialState = awaitState()
             initialState.eventSink(
                 RoomMemberModerationEvents.ProcessAction(
@@ -259,7 +274,14 @@ class RoomMemberModerationPresenterTest {
 
     @Test
     fun `present - do unban user with success`() = runTest {
-        createRoomMemberModerationPresenter(room = aJoinedRoom()).test {
+        val room = aJoinedRoom()
+        room.baseRoom.givenUpdateMembersResult {
+            // Simulate the member list being updated
+            room.givenRoomMembersState(RoomMembersState.Ready(
+                persistentListOf(aRoomMember())
+            ))
+        }
+        createRoomMemberModerationPresenter(room = room).test {
             val initialState = awaitState()
             initialState.eventSink(
                 RoomMemberModerationEvents.ProcessAction(
@@ -326,7 +348,7 @@ class RoomMemberModerationPresenterTest {
         banUserResult: Result<Unit> = Result.success(Unit),
         unBanUserResult: Result<Unit> = Result.success(Unit),
         targetRoomMember: RoomMember? = null,
-    ): JoinedRoom {
+    ): FakeJoinedRoom {
         return FakeJoinedRoom(
             kickUserResult = { _, _ -> kickUserResult },
             banUserResult = { _, _ -> banUserResult },
@@ -335,6 +357,7 @@ class RoomMemberModerationPresenterTest {
                 canBanResult = { _ -> Result.success(canBan) },
                 canKickResult = { _ -> Result.success(canKick) },
                 userRoleResult = { Result.success(myUserRole) },
+                updateMembersResult = { Result.success(Unit) }
             ),
         ).apply {
             val roomMembers = listOfNotNull(targetRoomMember).toPersistentList()

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberListFetcher.kt

@@ -79,35 +79,35 @@ internal class RoomMemberListFetcher(
         Timber.i("Loading cached members for room $roomId")
         try {
             // Send current member list with pending state to notify the UI that we are loading new members
-            emit(pendingWithCurrentMembers())
+            value = pendingWithCurrentMembers()
             val members = parseAndEmitMembers(room.membersNoSync())
             val newState = if (asPendingState) {
                 RoomMembersState.Pending(prevRoomMembers = members)
             } else {
                 RoomMembersState.Ready(members)
             }
-            emit(newState)
+            value = newState
         } catch (exception: CancellationException) {
             Timber.d("Cancelled loading cached members for room $roomId")
             throw exception
         } catch (exception: Exception) {
             Timber.e(exception, "Failed to load cached members for room $roomId")
-            emit(RoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList()))
+            value = RoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList())
         }
     }
 
     private suspend fun MutableStateFlow<RoomMembersState>.fetchRemoteRoomMembers() {
         try {
             // Send current member list with pending state to notify the UI that we are loading new members
-            emit(pendingWithCurrentMembers())
+            value = pendingWithCurrentMembers()
             // Start loading new members
-            emit(RoomMembersState.Ready(parseAndEmitMembers(room.members())))
+            value = RoomMembersState.Ready(parseAndEmitMembers(room.members()))
         } catch (exception: CancellationException) {
             Timber.d("Cancelled loading updated members for room $roomId")
             throw exception
         } catch (exception: Exception) {
             Timber.e(exception, "Failed to load updated members for room $roomId")
-            emit(RoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList()))
+            value = RoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList())
         }
     }
 

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeBaseRoom.kt

@@ -59,7 +59,7 @@ class FakeBaseRoom(
     private val markAsReadResult: (ReceiptType) -> Result<Unit> = { Result.success(Unit) },
     private val powerLevelsResult: () -> Result<RoomPowerLevelsValues> = { lambdaError() },
     private val leaveRoomLambda: () -> Result<Unit> = { lambdaError() },
-    private val updateMembersResult: () -> Unit = { lambdaError() },
+    private var updateMembersResult: () -> Unit = { lambdaError() },
     private val getMembersResult: (Int) -> Result<List<RoomMember>> = { lambdaError() },
     private val saveComposerDraftLambda: (ComposerDraft) -> Result<Unit> = { _: ComposerDraft -> Result.success(Unit) },
     private val loadComposerDraftLambda: () -> Result<ComposerDraft?> = { Result.success<ComposerDraft?>(null) },
@@ -223,6 +223,10 @@ class FakeBaseRoom(
     override suspend fun reportRoom(reason: String?) = reportRoomResult(reason)
 
     override fun predecessorRoom(): PredecessorRoom? = predecessorRoomResult()
+
+    fun givenUpdateMembersResult(result: () -> Unit) {
+        updateMembersResult = result
+    }
 }
 
 fun defaultRoomPowerLevelValues() = RoomPowerLevelsValues(