Avoid creating many PowerLevelRoomMemberComparator instance and also avoid doing the sorting twice.

This commit is contained in:
Benoit Marty 2025-11-18 17:37:15 +01:00
parent b3969fdb8e
commit 1dbf2319ff
5 changed files with 33 additions and 25 deletions

View file

@ -61,6 +61,8 @@ class ChangeRolesPresenter(
fun create(role: RoomMember.Role): ChangeRolesPresenter fun create(role: RoomMember.Role): ChangeRolesPresenter
} }
private val powerLevelRoomMemberComparator = PowerLevelRoomMemberComparator()
@Composable @Composable
override fun present(): ChangeRolesState { override fun present(): ChangeRolesState {
val dataSource = remember { RoomMemberListDataSource(room, dispatchers) } val dataSource = remember { RoomMemberListDataSource(room, dispatchers) }
@ -176,17 +178,7 @@ class ChangeRolesPresenter(
} }
private fun List<RoomMember>.groupedByRole(): MembersByRole { private fun List<RoomMember>.groupedByRole(): MembersByRole {
val groupedMembers = MembersByRole(this) return MembersByRole(this, powerLevelRoomMemberComparator)
return MembersByRole(
owners = groupedMembers.owners.sorted(),
admins = groupedMembers.admins.sorted(),
moderators = groupedMembers.moderators.sorted(),
members = groupedMembers.members.sorted(),
)
}
private fun Iterable<RoomMember>.sorted(): ImmutableList<RoomMember> {
return sortedWith(PowerLevelRoomMemberComparator()).toImmutableList()
} }
private fun CoroutineScope.save( private fun CoroutineScope.save(

View file

@ -13,8 +13,8 @@ import io.element.android.libraries.designsystem.theme.components.SearchBarResul
import io.element.android.libraries.matrix.api.core.UserId 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.RoomMember
import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.matrix.ui.room.PowerLevelRoomMemberComparator
import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toImmutableList import kotlinx.collections.immutable.toImmutableList
data class ChangeRolesState( data class ChangeRolesState(
@ -35,16 +35,23 @@ data class MembersByRole(
val moderators: ImmutableList<RoomMember>, val moderators: ImmutableList<RoomMember>,
val members: ImmutableList<RoomMember>, val members: ImmutableList<RoomMember>,
) { ) {
constructor(members: List<RoomMember>) : this( constructor() : this(
owners = members.filter { it.role is RoomMember.Role.Owner }.sorted(), owners = persistentListOf(),
admins = members.filter { it.role == RoomMember.Role.Admin }.sorted(), admins = persistentListOf(),
moderators = members.filter { it.role == RoomMember.Role.Moderator }.sorted(), moderators = persistentListOf(),
members = members.filter { it.role == RoomMember.Role.User }.sorted(), members = persistentListOf(),
)
constructor(members: List<RoomMember>, comparator: Comparator<RoomMember>) : this(
owners = members.filter { it.role is RoomMember.Role.Owner }.sorted(comparator),
admins = members.filter { it.role == RoomMember.Role.Admin }.sorted(comparator),
moderators = members.filter { it.role == RoomMember.Role.Moderator }.sorted(comparator),
members = members.filter { it.role == RoomMember.Role.User }.sorted(comparator),
) )
fun isEmpty() = owners.isEmpty() && admins.isEmpty() && moderators.isEmpty() && members.isEmpty() fun isEmpty() = owners.isEmpty() && admins.isEmpty() && moderators.isEmpty() && members.isEmpty()
} }
private fun Iterable<RoomMember>.sorted(): ImmutableList<RoomMember> { private fun Iterable<RoomMember>.sorted(comparator: Comparator<RoomMember>): ImmutableList<RoomMember> {
return sortedWith(PowerLevelRoomMemberComparator()).toImmutableList() return sortedWith(comparator).toImmutableList()
} }

View file

@ -17,6 +17,7 @@ import io.element.android.libraries.matrix.api.room.RoomMembershipState
import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.matrix.ui.components.aMatrixUser import io.element.android.libraries.matrix.ui.components.aMatrixUser
import io.element.android.libraries.matrix.ui.components.aMatrixUserList import io.element.android.libraries.matrix.ui.components.aMatrixUserList
import io.element.android.libraries.matrix.ui.room.PowerLevelRoomMemberComparator
import io.element.android.libraries.previewutils.room.aRoomMember import io.element.android.libraries.previewutils.room.aRoomMember
import io.element.android.libraries.previewutils.room.aRoomMemberList import io.element.android.libraries.previewutils.room.aRoomMemberList
import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.ImmutableList
@ -36,7 +37,12 @@ class ChangeRolesStateProvider : PreviewParameterProvider<ChangeRolesState> {
aChangeRolesStateWithSelectedUsers().copy( aChangeRolesStateWithSelectedUsers().copy(
query = "Alice", query = "Alice",
isSearchActive = true, isSearchActive = true,
searchResults = SearchBarResultState.Results(MembersByRole(aRoomMemberList().take(1).toImmutableList())), searchResults = SearchBarResultState.Results(
MembersByRole(
members = aRoomMemberList().take(1),
comparator = PowerLevelRoomMemberComparator(),
)
),
selectedUsers = aMatrixUserList().take(1).toImmutableList(), selectedUsers = aMatrixUserList().take(1).toImmutableList(),
), ),
aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.ConfirmingCancellation), aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.ConfirmingCancellation),
@ -87,7 +93,8 @@ internal fun aChangeRolesStateWithSelectedUsers() = aChangeRolesState(
} else { } else {
roomMember roomMember
} }
} },
comparator = PowerLevelRoomMemberComparator(),
) )
), ),
hasPendingChanges = true, hasPendingChanges = true,
@ -126,7 +133,8 @@ internal fun aChangeRolesStateWithOwners(
displayName = "David", displayName = "David",
role = RoomMember.Role.User, role = RoomMember.Role.User,
), ),
) ),
comparator = PowerLevelRoomMemberComparator(),
), ),
), ),
canRemoveMember = { userId -> canRemoveMember = { userId ->

View file

@ -145,7 +145,7 @@ fun ChangeRolesView(
SearchResultsList( SearchResultsList(
currentRole = state.role, currentRole = state.role,
lazyListState = lazyListState, lazyListState = lazyListState,
searchResults = (state.searchResults as? SearchBarResultState.Results)?.results ?: MembersByRole(emptyList()), searchResults = (state.searchResults as? SearchBarResultState.Results)?.results ?: MembersByRole(),
selectedUsers = state.selectedUsers, selectedUsers = state.selectedUsers,
canRemoveMember = state.canChangeMemberRole, canRemoveMember = state.canChangeMemberRole,
onToggleSelection = { state.eventSink(ChangeRolesEvent.UserSelectionToggled(it.toMatrixUser())) }, onToggleSelection = { state.eventSink(ChangeRolesEvent.UserSelectionToggled(it.toMatrixUser())) },

View file

@ -18,6 +18,7 @@ import io.element.android.libraries.matrix.test.A_USER_ID_5
import io.element.android.libraries.matrix.test.A_USER_ID_6 import io.element.android.libraries.matrix.test.A_USER_ID_6
import io.element.android.libraries.matrix.test.A_USER_ID_7 import io.element.android.libraries.matrix.test.A_USER_ID_7
import io.element.android.libraries.matrix.test.room.aRoomMember import io.element.android.libraries.matrix.test.room.aRoomMember
import io.element.android.libraries.matrix.ui.room.PowerLevelRoomMemberComparator
import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.persistentListOf
import org.junit.Test import org.junit.Test
@ -33,7 +34,7 @@ class MembersByRoleTest {
aRoomMember(A_USER_ID_6, displayName = "Justin", role = RoomMember.Role.Owner(isCreator = true)), aRoomMember(A_USER_ID_6, displayName = "Justin", role = RoomMember.Role.Owner(isCreator = true)),
aRoomMember(A_USER_ID_7, displayName = "Mallory", role = RoomMember.Role.Owner(isCreator = false)), aRoomMember(A_USER_ID_7, displayName = "Mallory", role = RoomMember.Role.Owner(isCreator = false)),
) )
val membersByRole = MembersByRole(members = members) val membersByRole = MembersByRole(members = members, comparator = PowerLevelRoomMemberComparator())
assertThat(membersByRole.owners).containsExactly( assertThat(membersByRole.owners).containsExactly(
aRoomMember(A_USER_ID_6, displayName = "Justin", role = RoomMember.Role.Owner(isCreator = true)), aRoomMember(A_USER_ID_6, displayName = "Justin", role = RoomMember.Role.Owner(isCreator = true)),
aRoomMember(A_USER_ID_7, displayName = "Mallory", role = RoomMember.Role.Owner(isCreator = false)), aRoomMember(A_USER_ID_7, displayName = "Mallory", role = RoomMember.Role.Owner(isCreator = false)),
@ -52,7 +53,7 @@ class MembersByRoleTest {
@Test @Test
fun `isEmpty - only returns true with no members of any role`() { fun `isEmpty - only returns true with no members of any role`() {
val emptyMembersByRole = MembersByRole(emptyList()) val emptyMembersByRole = MembersByRole()
assertThat(emptyMembersByRole.isEmpty()).isTrue() assertThat(emptyMembersByRole.isEmpty()).isTrue()
val membersByRoleWithOwners = MembersByRole( val membersByRoleWithOwners = MembersByRole(