Use member count instead of counting members (#530)

Use member count instead of counting members

For the room details screen, use the member count as supplied by
matrix instead of waiting for the entire member list to be
retrieved and then manually adding up all the relevant users.

This removes the loading state of the member count, relying on
a spinner on the member list itself if the user actually wants
to see the members. (The performance of that will be improved
separately on the rust side in the future)

Closes #505
This commit is contained in:
Chris Smith 2023-06-06 11:40:17 +01:00 committed by GitHub
parent 044a3c991e
commit 7308428596
20 changed files with 29 additions and 127 deletions

View file

@ -221,6 +221,7 @@ koverMerged {
excludes += "io.element.android.libraries.matrix.api.timeline.item.event.OtherState$*"
excludes += "io.element.android.libraries.matrix.api.timeline.item.event.EventSendState$*"
excludes += "io.element.android.libraries.matrix.api.room.RoomMembershipState*"
excludes += "io.element.android.libraries.matrix.api.room.MatrixRoomMembersState*"
excludes += "io.element.android.libraries.push.impl.notifications.NotificationState*"
excludes += "io.element.android.features.messages.impl.media.local.pdf.PdfViewerState"
}

View file

@ -27,15 +27,10 @@ import io.element.android.features.leaveroom.api.LeaveRoomState
import io.element.android.features.leaveroom.api.LeaveRoomState.Confirmation.Generic
import io.element.android.features.leaveroom.api.LeaveRoomState.Confirmation.LastUserInRoom
import io.element.android.features.leaveroom.api.LeaveRoomState.Confirmation.PrivateRoom
import io.element.android.libraries.architecture.Async
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId
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.RoomMembershipObserver
import io.element.android.libraries.matrix.api.room.RoomMembershipState
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject
@ -83,7 +78,7 @@ class LeaveRoomPresenterImpl @Inject constructor(
}
}
private suspend fun showLeaveRoomAlert(
private fun showLeaveRoomAlert(
matrixClient: MatrixClient,
roomId: RoomId,
confirmation: MutableState<LeaveRoomState.Confirmation>,
@ -91,7 +86,7 @@ private suspend fun showLeaveRoomAlert(
matrixClient.getRoom(roomId)?.use { room ->
confirmation.value = when {
!room.isPublic -> PrivateRoom(roomId)
(room.memberCount() as? Async.Success<Int>)?.state == 1 -> LastUserInRoom(roomId)
room.joinedMemberCount == 1L -> LastUserInRoom(roomId)
else -> Generic(roomId)
}
}
@ -116,12 +111,3 @@ private suspend fun MatrixClient.leaveRoom(
}
progress.value = LeaveRoomState.Progress.Hidden
}
private suspend fun MatrixRoom.memberCount(): Async<Int> = membersStateFlow.first().let { membersState ->
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.count { it.membership == RoomMembershipState.JOIN })
}
}

View file

@ -24,11 +24,7 @@ import io.element.android.features.leaveroom.api.LeaveRoomEvent
import io.element.android.features.leaveroom.api.LeaveRoomPresenter
import io.element.android.features.leaveroom.api.LeaveRoomState
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.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.api.room.RoomMembershipState
import io.element.android.libraries.matrix.api.timeline.item.event.MembershipChange
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.FakeMatrixClient
@ -100,24 +96,7 @@ class LeaveRoomPresenterImplTest {
client = FakeMatrixClient().apply {
givenGetRoomResult(
roomId = A_ROOM_ID,
result = FakeMatrixRoom().apply {
givenRoomMembersState(
MatrixRoomMembersState.Ready(
listOf(
RoomMember(
userId = UserId(value = "@aUserId:aDomain"),
displayName = null,
avatarUrl = null,
membership = RoomMembershipState.JOIN,
isNameAmbiguous = false,
powerLevel = 0,
normalizedPowerLevel = 0,
isIgnored = false
)
)
)
)
},
result = FakeMatrixRoom(joinedMemberCount = 1),
)
}
)

View file

@ -27,12 +27,10 @@ import androidx.compose.runtime.remember
import io.element.android.features.leaveroom.api.LeaveRoomEvent
import io.element.android.features.leaveroom.api.LeaveRoomPresenter
import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsPresenter
import io.element.android.libraries.architecture.Async
import io.element.android.libraries.architecture.Presenter
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.RoomMembershipState
import io.element.android.libraries.matrix.api.room.StateEventType
import io.element.android.libraries.matrix.ui.room.getDirectRoomMember
import javax.inject.Inject
@ -51,7 +49,6 @@ class RoomDetailsPresenter @Inject constructor(
}
val membersState by room.membersStateFlow.collectAsState()
val memberCount by getMemberCount(membersState)
val canInvite by getCanInvite(membersState)
val canEditName by getCanSendStateEvent(membersState, StateEventType.ROOM_NAME)
val canEditAvatar by getCanSendStateEvent(membersState, StateEventType.ROOM_AVATAR)
@ -85,7 +82,7 @@ class RoomDetailsPresenter @Inject constructor(
roomAlias = room.alias,
roomAvatarUrl = room.avatarUrl,
roomTopic = topicState,
memberCount = memberCount,
memberCount = room.joinedMemberCount,
isEncrypted = room.isEncrypted,
canInvite = canInvite,
canEdit = canEditAvatar || canEditName || canEditTopic,
@ -131,18 +128,4 @@ class RoomDetailsPresenter @Inject constructor(
}
return canSendEvent
}
@Composable
private fun getMemberCount(membersState: MatrixRoomMembersState): State<Async<Int>> {
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.count { it.membership == RoomMembershipState.JOIN })
}
}
}
}
}

View file

@ -18,7 +18,6 @@ package io.element.android.features.roomdetails.impl
import io.element.android.features.leaveroom.api.LeaveRoomState
import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsState
import io.element.android.libraries.architecture.Async
import io.element.android.libraries.matrix.api.room.RoomMember
data class RoomDetailsState(
@ -27,7 +26,7 @@ data class RoomDetailsState(
val roomAlias: String?,
val roomAvatarUrl: String?,
val roomTopic: RoomTopicState,
val memberCount: Async<Int>,
val memberCount: Long,
val isEncrypted: Boolean,
val roomType: RoomDetailsType,
val roomMemberDetailsState: RoomMemberDetailsState?,

View file

@ -19,7 +19,6 @@ package io.element.android.features.roomdetails.impl
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.features.leaveroom.api.LeaveRoomState
import io.element.android.features.roomdetails.impl.members.details.aRoomMemberDetailsState
import io.element.android.libraries.architecture.Async
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.RoomMembershipState
@ -32,7 +31,6 @@ open class RoomDetailsStateProvider : PreviewParameterProvider<RoomDetailsState>
aRoomDetailsState().copy(roomTopic = RoomTopicState.CanAddTopic),
aRoomDetailsState().copy(isEncrypted = false),
aRoomDetailsState().copy(roomAlias = null),
aRoomDetailsState().copy(memberCount = Async.Failure(Throwable())),
aDmRoomDetailsState().copy(roomName = "Daniel"),
aDmRoomDetailsState(isDmMemberIgnored = true).copy(roomName = "Daniel"),
aRoomDetailsState().copy(canInvite = true),
@ -73,7 +71,7 @@ fun aRoomDetailsState() = RoomDetailsState(
"|| MAI iki/Marketing " +
"|| MAI iki/Marketing..."
),
memberCount = Async.Success(32),
memberCount = 32,
isEncrypted = true,
canInvite = false,
canEdit = false,

View file

@ -59,7 +59,6 @@ import io.element.android.features.roomdetails.impl.blockuser.BlockUserDialogs
import io.element.android.features.roomdetails.impl.blockuser.BlockUserSection
import io.element.android.features.roomdetails.impl.members.details.RoomMemberHeaderSection
import io.element.android.features.roomdetails.impl.members.details.RoomMemberMainActionsSection
import io.element.android.libraries.architecture.isLoading
import io.element.android.libraries.designsystem.ElementTextStyles
import io.element.android.libraries.designsystem.components.avatar.Avatar
import io.element.android.libraries.designsystem.components.avatar.AvatarData
@ -145,10 +144,8 @@ fun RoomDetailsView(
}
if (state.roomType is RoomDetailsType.Room) {
val memberCount = state.memberCount.dataOrNull()
MembersSection(
memberCount = memberCount,
isLoading = state.memberCount.isLoading(),
memberCount = state.memberCount,
openRoomMemberList = openRoomMemberList,
)
@ -273,8 +270,7 @@ internal fun TopicSection(
@Composable
internal fun MembersSection(
memberCount: Int?,
isLoading: Boolean,
memberCount: Long,
openRoomMemberList: () -> Unit,
modifier: Modifier = Modifier,
) {
@ -282,9 +278,8 @@ internal fun MembersSection(
PreferenceText(
title = stringResource(R.string.screen_room_details_people_title),
icon = Icons.Outlined.Person,
currentValue = memberCount?.toString(),
currentValue = memberCount.toString(),
onClick = openRoomMemberList,
loadingCurrentValue = isLoading,
)
}
}

View file

@ -26,18 +26,15 @@ import io.element.android.features.roomdetails.impl.RoomDetailsType
import io.element.android.features.roomdetails.impl.RoomTopicState
import io.element.android.features.roomdetails.impl.members.aRoomMember
import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsPresenter
import io.element.android.libraries.architecture.Async
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
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.RoomMembershipState
import io.element.android.libraries.matrix.api.room.StateEventType
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.A_ROOM_NAME
import io.element.android.libraries.matrix.test.A_SESSION_ID
import io.element.android.libraries.matrix.test.A_USER_ID
import io.element.android.libraries.matrix.test.A_USER_ID_2
import io.element.android.libraries.matrix.test.FakeMatrixClient
import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
@ -69,48 +66,13 @@ class RoomDetailsPresenterTests {
assertThat(initialState.roomName).isEqualTo(room.name)
assertThat(initialState.roomAvatarUrl).isEqualTo(room.avatarUrl)
assertThat(initialState.roomTopic).isEqualTo(RoomTopicState.ExistingTopic(room.topic!!))
assertThat(initialState.memberCount).isEqualTo(Async.Uninitialized)
assertThat(initialState.memberCount).isEqualTo(room.joinedMemberCount)
assertThat(initialState.isEncrypted).isEqualTo(room.isEncrypted)
cancelAndIgnoreRemainingEvents()
}
}
@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, membership = RoomMembershipState.INVITE),
)
val presenter = aRoomDetailsPresenter(room)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
room.givenRoomMembersState(MatrixRoomMembersState.Unknown)
val initialState = awaitItem()
assertThat(initialState.memberCount).isEqualTo(Async.Uninitialized)
skipItems(1)
room.givenRoomMembersState(MatrixRoomMembersState.Pending(null))
val loadingState = awaitItem()
assertThat(loadingState.memberCount).isEqualTo(Async.Loading(null))
room.givenRoomMembersState(MatrixRoomMembersState.Error(error))
skipItems(1)
val failureState = awaitItem()
assertThat(failureState.memberCount).isEqualTo(Async.Failure(error, null))
room.givenRoomMembersState(MatrixRoomMembersState.Ready(roomMembers))
skipItems(1)
val successState = awaitItem()
assertThat(successState.memberCount).isEqualTo(Async.Success(1))
cancelAndIgnoreRemainingEvents()
}
}
@Test
fun `present - initial state with no room name`() = runTest {
val room = aMatrixRoom(name = null)

View file

@ -43,6 +43,7 @@ interface MatrixRoom : Closeable {
val isEncrypted: Boolean
val isDirect: Boolean
val isPublic: Boolean
val joinedMemberCount: Long
/**
* The current loaded members as a StateFlow.

View file

@ -137,6 +137,9 @@ class RustMatrixRoom(
override val isDirect: Boolean
get() = innerRoom.isDirect()
override val joinedMemberCount: Long
get() = innerRoom.joinedMembersCount().toLong()
override suspend fun updateMembers(): Result<Unit> = withContext(coroutineDispatchers.io) {
val currentState = _membersStateFlow.value
val currentMembers = currentState.roomMembers()

View file

@ -51,6 +51,7 @@ class FakeMatrixRoom(
override val alternativeAliases: List<String> = emptyList(),
override val isPublic: Boolean = true,
override val isDirect: Boolean = false,
override val joinedMemberCount: Long = 123L,
private val matrixTimeline: MatrixTimeline = FakeMatrixTimeline(),
) : MatrixRoom {

View file

@ -55,7 +55,7 @@ fun MatrixRoom.getDirectRoomMember(): State<RoomMember?> {
@Composable
fun MatrixRoom.getDirectRoomMember(roomMembersState: MatrixRoomMembersState): State<RoomMember?> {
val roomMembers = roomMembersState.roomMembers()
return remember(roomMembers) {
return remember(roomMembersState) {
derivedStateOf {
if (roomMembers == null) {
null

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:c6cc616b5345f797eee8b7c050ae7e825910928213a97169809a8d3b3c12c9bf
size 67133
oid sha256:66004f5ab345f1db47959538195381adebb9b553455d5a0c1cfb054c486e8010
size 64693

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:66004f5ab345f1db47959538195381adebb9b553455d5a0c1cfb054c486e8010
size 64693
oid sha256:81fe924e17f8c23a11dd9b2cb5380ded663eec3cbe5dd7aaee43fc24fec33374
size 59868

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:81fe924e17f8c23a11dd9b2cb5380ded663eec3cbe5dd7aaee43fc24fec33374
size 59868
oid sha256:799e499f285882e941581f40e238609ce08985b52f928df74856d4a4188caece
size 68021

View file

@ -1,3 +0,0 @@
version https://git-lfs.github.com/spec/v1
oid sha256:799e499f285882e941581f40e238609ce08985b52f928df74856d4a4188caece
size 68021

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:998819b0aeb15712bb53e982e3756cf2e4a4069a2a13f7955e10b873e3e736bd
size 63698
oid sha256:cb8dc0ca3fa34ffa40ea43934ee00d0c48c9b7ce721274763c093eee83d8cff9
size 62187

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:cb8dc0ca3fa34ffa40ea43934ee00d0c48c9b7ce721274763c093eee83d8cff9
size 62187
oid sha256:3553eac43e61daeb753762dfa53f93a4ec0d8365abffa5971ec23ec8a6ca6704
size 57130

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:3553eac43e61daeb753762dfa53f93a4ec0d8365abffa5971ec23ec8a6ca6704
size 57130
oid sha256:1797f9c90a79fc9bcbb1aa8d8ae32452c76d17b12e8ef7f441e1c93e9ac95c3f
size 64593

View file

@ -1,3 +0,0 @@
version https://git-lfs.github.com/spec/v1
oid sha256:1797f9c90a79fc9bcbb1aa8d8ae32452c76d17b12e8ef7f441e1c93e9ac95c3f
size 64593