Skip to content

Commit d4d57b1

Browse files
jmartinespjmartinesp
authored
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.
1 parent b2d4985 commit d4d57b1

File tree

6 files changed

+61
-23
lines changed

6 files changed

+61
-23
lines changed

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import kotlinx.collections.immutable.ImmutableMap
3838
import kotlinx.collections.immutable.persistentMapOf
3939
import kotlinx.collections.immutable.toImmutableList
4040
import kotlinx.collections.immutable.toPersistentMap
41+
import kotlinx.coroutines.flow.collectLatest
4142
import kotlinx.coroutines.flow.distinctUntilChanged
4243
import kotlinx.coroutines.flow.first
4344
import kotlinx.coroutines.flow.launchIn
@@ -66,11 +67,6 @@ class RoomMemberListPresenter @Inject constructor(
6667
val syncUpdateFlow = room.syncUpdateFlow.collectAsState()
6768
val canInvite by room.canInviteAsState(syncUpdateFlow.value)
6869
val roomModerationState = roomMembersModerationPresenter.present()
69-
val activeRoomMemberCount by produceState(0L) {
70-
room.roomInfoFlow.map { it.activeMembersCount }
71-
.distinctUntilChanged()
72-
.collect { value = it }
73-
}
7470

7571
val roomMemberIdentityStates by produceState(persistentMapOf<UserId, IdentityState>()) {
7672
room.roomMemberIdentityStateChange(waitForEncryption = true)
@@ -81,8 +77,12 @@ class RoomMemberListPresenter @Inject constructor(
8177
}
8278

8379
// Update the room members when the screen is loaded or the active member count changes
84-
LaunchedEffect(activeRoomMemberCount) {
85-
room.updateMembers()
80+
LaunchedEffect(Unit) {
81+
room.roomInfoFlow.map { it.activeMembersCount }
82+
.distinctUntilChanged()
83+
.collectLatest {
84+
room.updateMembers()
85+
}
8686
}
8787

8888
LaunchedEffect(membersState, roomMemberIdentityStates) {

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,6 @@ class RoomMemberListPresenterTest {
116116
}
117117
}
118118

119-
// Wait for the update to be processed
120-
skipItems(1)
121-
122119
// Update the room members state as `Room.updateMembers()` would have done with the actual implementation
123120
room.givenRoomMembersState(RoomMembersState.Ready(persistentListOf()))
124121
// Wait for another update

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,12 @@ import kotlinx.collections.immutable.PersistentList
3838
import kotlinx.collections.immutable.persistentListOf
3939
import kotlinx.collections.immutable.toPersistentList
4040
import kotlinx.coroutines.CoroutineScope
41+
import kotlinx.coroutines.delay
4142
import kotlinx.coroutines.flow.drop
42-
import kotlinx.coroutines.flow.take
43+
import kotlinx.coroutines.flow.first
4344
import kotlinx.coroutines.launch
4445
import javax.inject.Inject
46+
import kotlin.time.Duration.Companion.milliseconds
4547

4648
class RoomMemberModerationPresenter @Inject constructor(
4749
private val room: JoinedRoom,
@@ -82,6 +84,9 @@ class RoomMemberModerationPresenter @Inject constructor(
8284
)
8385
}
8486
is RoomMemberModerationEvents.ProcessAction -> {
87+
// First, hide any list of existing actions that could be displayed
88+
moderationActions.value = persistentListOf()
89+
8590
when (event.action) {
8691
is ModerationAction.DisplayProfile -> Unit
8792
is ModerationAction.KickUser -> {
@@ -118,6 +123,7 @@ class RoomMemberModerationPresenter @Inject constructor(
118123
}
119124
is InternalRoomMemberModerationEvents.Reset -> {
120125
selectedUser = null
126+
moderationActions.value = persistentListOf()
121127
kickUserAsyncAction.value = AsyncAction.Uninitialized
122128
banUserAsyncAction.value = AsyncAction.Uninitialized
123129
unbanUserAsyncAction.value = AsyncAction.Uninitialized
@@ -204,7 +210,15 @@ class RoomMemberModerationPresenter @Inject constructor(
204210
action.runUpdatingState {
205211
val result = block()
206212
if (result.isSuccess) {
207-
room.membersStateFlow.drop(1).take(1)
213+
// We wait a bit to ensure the server has processed the membership change
214+
delay(50.milliseconds)
215+
216+
// Update the members to ensure we have the latest state
217+
launch { room.updateMembers() }
218+
219+
// Wait for the membership change to be processed and returned
220+
// We drop the first emission as it's the current state
221+
room.membersStateFlow.drop(1).first()
208222
}
209223
result
210224
}

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import io.element.android.services.analytics.test.FakeAnalyticsService
2929
import io.element.android.tests.testutils.WarmUpRule
3030
import io.element.android.tests.testutils.test
3131
import io.element.android.tests.testutils.testCoroutineDispatchers
32+
import kotlinx.collections.immutable.persistentListOf
3233
import kotlinx.collections.immutable.toPersistentList
3334
import kotlinx.coroutines.test.TestScope
3435
import kotlinx.coroutines.test.runTest
@@ -217,7 +218,14 @@ class RoomMemberModerationPresenterTest {
217218

218219
@Test
219220
fun `present - do kick user with success`() = runTest {
220-
createRoomMemberModerationPresenter(room = aJoinedRoom()).test {
221+
val room = aJoinedRoom()
222+
room.baseRoom.givenUpdateMembersResult {
223+
// Simulate the member list being updated
224+
room.givenRoomMembersState(RoomMembersState.Ready(
225+
persistentListOf(aRoomMember())
226+
))
227+
}
228+
createRoomMemberModerationPresenter(room = room).test {
221229
val initialState = awaitState()
222230
initialState.eventSink(
223231
RoomMemberModerationEvents.ProcessAction(
@@ -238,7 +246,14 @@ class RoomMemberModerationPresenterTest {
238246

239247
@Test
240248
fun `present - do ban user with success`() = runTest {
241-
createRoomMemberModerationPresenter(room = aJoinedRoom()).test {
249+
val room = aJoinedRoom()
250+
room.baseRoom.givenUpdateMembersResult {
251+
// Simulate the member list being updated
252+
room.givenRoomMembersState(RoomMembersState.Ready(
253+
persistentListOf(aRoomMember())
254+
))
255+
}
256+
createRoomMemberModerationPresenter(room = room).test {
242257
val initialState = awaitState()
243258
initialState.eventSink(
244259
RoomMemberModerationEvents.ProcessAction(
@@ -259,7 +274,14 @@ class RoomMemberModerationPresenterTest {
259274

260275
@Test
261276
fun `present - do unban user with success`() = runTest {
262-
createRoomMemberModerationPresenter(room = aJoinedRoom()).test {
277+
val room = aJoinedRoom()
278+
room.baseRoom.givenUpdateMembersResult {
279+
// Simulate the member list being updated
280+
room.givenRoomMembersState(RoomMembersState.Ready(
281+
persistentListOf(aRoomMember())
282+
))
283+
}
284+
createRoomMemberModerationPresenter(room = room).test {
263285
val initialState = awaitState()
264286
initialState.eventSink(
265287
RoomMemberModerationEvents.ProcessAction(
@@ -326,7 +348,7 @@ class RoomMemberModerationPresenterTest {
326348
banUserResult: Result<Unit> = Result.success(Unit),
327349
unBanUserResult: Result<Unit> = Result.success(Unit),
328350
targetRoomMember: RoomMember? = null,
329-
): JoinedRoom {
351+
): FakeJoinedRoom {
330352
return FakeJoinedRoom(
331353
kickUserResult = { _, _ -> kickUserResult },
332354
banUserResult = { _, _ -> banUserResult },
@@ -335,6 +357,7 @@ class RoomMemberModerationPresenterTest {
335357
canBanResult = { _ -> Result.success(canBan) },
336358
canKickResult = { _ -> Result.success(canKick) },
337359
userRoleResult = { Result.success(myUserRole) },
360+
updateMembersResult = { Result.success(Unit) }
338361
),
339362
).apply {
340363
val roomMembers = listOfNotNull(targetRoomMember).toPersistentList()

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,35 +79,35 @@ internal class RoomMemberListFetcher(
7979
Timber.i("Loading cached members for room $roomId")
8080
try {
8181
// Send current member list with pending state to notify the UI that we are loading new members
82-
emit(pendingWithCurrentMembers())
82+
value = pendingWithCurrentMembers()
8383
val members = parseAndEmitMembers(room.membersNoSync())
8484
val newState = if (asPendingState) {
8585
RoomMembersState.Pending(prevRoomMembers = members)
8686
} else {
8787
RoomMembersState.Ready(members)
8888
}
89-
emit(newState)
89+
value = newState
9090
} catch (exception: CancellationException) {
9191
Timber.d("Cancelled loading cached members for room $roomId")
9292
throw exception
9393
} catch (exception: Exception) {
9494
Timber.e(exception, "Failed to load cached members for room $roomId")
95-
emit(RoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList()))
95+
value = RoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList())
9696
}
9797
}
9898

9999
private suspend fun MutableStateFlow<RoomMembersState>.fetchRemoteRoomMembers() {
100100
try {
101101
// Send current member list with pending state to notify the UI that we are loading new members
102-
emit(pendingWithCurrentMembers())
102+
value = pendingWithCurrentMembers()
103103
// Start loading new members
104-
emit(RoomMembersState.Ready(parseAndEmitMembers(room.members())))
104+
value = RoomMembersState.Ready(parseAndEmitMembers(room.members()))
105105
} catch (exception: CancellationException) {
106106
Timber.d("Cancelled loading updated members for room $roomId")
107107
throw exception
108108
} catch (exception: Exception) {
109109
Timber.e(exception, "Failed to load updated members for room $roomId")
110-
emit(RoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList()))
110+
value = RoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList())
111111
}
112112
}
113113

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class FakeBaseRoom(
5959
private val markAsReadResult: (ReceiptType) -> Result<Unit> = { Result.success(Unit) },
6060
private val powerLevelsResult: () -> Result<RoomPowerLevelsValues> = { lambdaError() },
6161
private val leaveRoomLambda: () -> Result<Unit> = { lambdaError() },
62-
private val updateMembersResult: () -> Unit = { lambdaError() },
62+
private var updateMembersResult: () -> Unit = { lambdaError() },
6363
private val getMembersResult: (Int) -> Result<List<RoomMember>> = { lambdaError() },
6464
private val saveComposerDraftLambda: (ComposerDraft) -> Result<Unit> = { _: ComposerDraft -> Result.success(Unit) },
6565
private val loadComposerDraftLambda: () -> Result<ComposerDraft?> = { Result.success<ComposerDraft?>(null) },
@@ -223,6 +223,10 @@ class FakeBaseRoom(
223223
override suspend fun reportRoom(reason: String?) = reportRoomResult(reason)
224224

225225
override fun predecessorRoom(): PredecessorRoom? = predecessorRoomResult()
226+
227+
fun givenUpdateMembersResult(result: () -> Unit) {
228+
updateMembersResult = result
229+
}
226230
}
227231

228232
fun defaultRoomPowerLevelValues() = RoomPowerLevelsValues(

0 commit comments

Comments
 (0)