fix: rely only on RoomMember Role values instead of using the powerLevel.

This commit is contained in:
ganfra
2025-12-01 16:08:11 +01:00
parent 7a1dd24dbd
commit a91c78b56f
10 changed files with 84 additions and 122 deletions

View File

@@ -96,10 +96,7 @@ class LeaveRoomPresenter(
} else {
val hasPrivilegedCreatorRole = roomInfoFlow.value.privilegedCreatorRole
if (!hasPrivilegedCreatorRole) return false
val creators = usersWithRole(RoomMember.Role.Owner(isCreator = true)).first()
val superAdmins = usersWithRole(RoomMember.Role.Owner(isCreator = false)).first()
val owners = creators + superAdmins
val owners = usersWithRole { role -> role is RoomMember.Role.Owner }.first()
return owners.size == 1 && owners.first().userId == sessionId
}
}

View File

@@ -79,18 +79,13 @@ class ChangeRolesPresenter(
val usersWithRole = produceState<ImmutableList<MatrixUser>>(initialValue = persistentListOf()) {
// If the role is admin, we need to include the owners as well since they implicitly have admin role
val owners = if (role == RoomMember.Role.Admin) {
combine(
room.usersWithRole(RoomMember.Role.Owner(isCreator = true)),
room.usersWithRole(RoomMember.Role.Owner(isCreator = false)),
) { creators, superAdmins ->
creators + superAdmins
}
room.usersWithRole { role -> role is RoomMember.Role.Owner }
} else {
emptyFlow()
}
combine(
owners,
room.usersWithRole(role),
room.usersWithRole { it == role },
) { owners, users ->
owners + users
}.map { members -> members.map { it.toMatrixUser() } }

View File

@@ -22,12 +22,10 @@ import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.architecture.runUpdatingState
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.room.JoinedRoom
import io.element.android.libraries.matrix.api.room.RoomInfo
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.activeRoomMembers
import io.element.android.libraries.matrix.api.room.powerlevels.UserRoleChange
import io.element.android.libraries.matrix.api.room.powerlevels.userCountWithRole
import io.element.android.libraries.matrix.ui.model.roleOf
import io.element.android.services.analytics.api.AnalyticsService
import kotlinx.coroutines.CoroutineScope
@@ -43,32 +41,14 @@ class RolesAndPermissionsPresenter(
override fun present(): RolesAndPermissionsState {
val coroutineScope = rememberCoroutineScope()
val roomInfo by room.roomInfoFlow.collectAsState()
val roomMembers by room.membersStateFlow.collectAsState()
// Get the list of active room members (joined or invited), in order to filter members present in the power
// level state Event.
val activeRoomMemberIds by remember {
derivedStateOf {
roomMembers.activeRoomMembers().map { it.userId }
}
}
val moderatorCount by remember {
derivedStateOf {
roomInfo.userCountWithRole(activeRoomMemberIds, RoomMember.Role.Moderator)
}
}
room.userCountWithRole { role -> role is RoomMember.Role.Moderator }
}.collectAsState(null)
val adminCount by remember {
derivedStateOf {
val admins = roomInfo.userCountWithRole(activeRoomMemberIds, RoomMember.Role.Admin)
val ownersCount = if (roomInfo.privilegedCreatorRole) {
val superAdmins = roomInfo.userCountWithRole(activeRoomMemberIds, RoomMember.Role.Owner(isCreator = false))
val creators = roomInfo.userCountWithRole(activeRoomMemberIds, RoomMember.Role.Owner(isCreator = true))
superAdmins + creators
} else {
0
}
admins + ownersCount
}
}
room.userCountWithRole { role -> role is RoomMember.Role.Admin || role is RoomMember.Role.Owner }
}.collectAsState(null)
val canDemoteSelf = remember { derivedStateOf { roomInfo.roleOf(room.sessionId) !is RoomMember.Role.Owner } }
val changeOwnRoleAction = remember { mutableStateOf<AsyncAction<Unit>>(AsyncAction.Uninitialized) }
val resetPermissionsAction = remember { mutableStateOf<AsyncAction<Unit>>(AsyncAction.Uninitialized) }
@@ -122,8 +102,4 @@ class RolesAndPermissionsPresenter(
room.resetPowerLevels()
}
}
private fun RoomInfo.userCountWithRole(userIds: List<UserId>, role: RoomMember.Role): Int {
return usersWithRole(role).filter { it in userIds }.size
}
}

View File

@@ -12,8 +12,8 @@ import io.element.android.libraries.architecture.AsyncAction
data class RolesAndPermissionsState(
val roomSupportsOwnerRole: Boolean,
val adminCount: Int,
val moderatorCount: Int,
val adminCount: Int?,
val moderatorCount: Int?,
val canDemoteSelf: Boolean,
val changeOwnRoleAction: AsyncAction<Unit>,
val resetPermissionsAction: AsyncAction<Unit>,

View File

@@ -63,13 +63,17 @@ fun RolesAndPermissionsView(
ListItem(
headlineContent = { Text(adminsTitle) },
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.Admin())),
trailingContent = ListItemContent.Text("${state.adminCount}"),
trailingContent = state.adminCount?.let { adminCount ->
ListItemContent.Text("$adminCount")
},
onClick = { rolesAndPermissionsNavigator.openAdminList() },
)
ListItem(
headlineContent = { Text(stringResource(R.string.screen_room_roles_and_permissions_moderators)) },
leadingContent = ListItemContent.Icon(IconSource.Vector(CompoundIcons.ChatProblem())),
trailingContent = ListItemContent.Text("${state.moderatorCount}"),
trailingContent = state.moderatorCount?.let { moderationCount ->
ListItemContent.Text("$moderationCount")
},
onClick = { rolesAndPermissionsNavigator.openModeratorList() },
)
if (state.canDemoteSelf) {

View File

@@ -18,7 +18,9 @@ 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.RoomMembersState
import io.element.android.libraries.matrix.api.room.powerlevels.RoomPowerLevels
import io.element.android.libraries.matrix.api.room.toMatrixUser
import io.element.android.libraries.matrix.api.user.MatrixUser
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.A_USER_ID_3
@@ -26,11 +28,13 @@ import io.element.android.libraries.matrix.test.room.FakeBaseRoom
import io.element.android.libraries.matrix.test.room.FakeJoinedRoom
import io.element.android.libraries.matrix.test.room.aRoomInfo
import io.element.android.libraries.matrix.test.room.aRoomMember
import io.element.android.libraries.matrix.test.room.anAlice
import io.element.android.libraries.matrix.test.room.defaultRoomPowerLevelValues
import io.element.android.libraries.previewutils.room.aRoomMemberList
import io.element.android.services.analytics.test.FakeAnalyticsService
import io.element.android.tests.testutils.test
import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.persistentMapOf
import kotlinx.collections.immutable.toImmutableList
import kotlinx.collections.immutable.toImmutableMap
@@ -63,7 +67,7 @@ class ChangeRolesPresenterTest {
}
val presenter = createChangeRolesPresenter(room = room)
presenter.test {
skipItems(1)
skipItems(2)
assertThat(awaitItem().searchResults).isInstanceOf(SearchBarResultState.Results::class.java)
}
}
@@ -161,13 +165,13 @@ class ChangeRolesPresenterTest {
}
@Test
fun `present - when modifying admins, creators are displayed too`() = runTest {
fun `present - when modifying admins, creators are displayed too - privilegedCreatorRole is true`() = runTest {
val room = FakeJoinedRoom().apply {
val creatorUserId = UserId("@creator:matrix.org")
val memberList = aRoomMemberList()
.plus(aRoomMember(displayName = "CREATOR", role = RoomMember.Role.Owner(isCreator = true), userId = creatorUserId))
.toImmutableList()
givenRoomInfo(aRoomInfo(roomCreators = listOf(creatorUserId)))
givenRoomInfo(aRoomInfo(roomCreators = listOf(creatorUserId), privilegedCreatorRole = true))
givenRoomMembersState(RoomMembersState.Ready(memberList))
}
val presenter = createChangeRolesPresenter(room = room)
@@ -190,6 +194,7 @@ class ChangeRolesPresenterTest {
}
val presenter = createChangeRolesPresenter(room = room)
presenter.test {
skipItems(1)
val initialState = awaitItem()
initialState.eventSink(ChangeRolesEvent.ToggleSearchActive)
@@ -207,6 +212,7 @@ class ChangeRolesPresenterTest {
}
val presenter = createChangeRolesPresenter(room = room)
presenter.test {
skipItems(1)
val initialState = awaitItem()
val initialResults = (awaitItem().searchResults as? SearchBarResultState.Results)?.results
assertThat(initialResults?.members).hasSize(8)
@@ -231,7 +237,7 @@ class ChangeRolesPresenterTest {
}
val presenter = createChangeRolesPresenter(room = room)
presenter.test {
skipItems(1)
skipItems(2)
val initialResults = (awaitItem().searchResults as? SearchBarResultState.Results)?.results
assertThat(initialResults?.members).hasSize(8)
assertThat(initialResults?.moderators).hasSize(1)
@@ -478,17 +484,14 @@ class ChangeRolesPresenterTest {
@Test
fun `present - Save will just save the changes if the current user is a room creator and the selected users are not`() = runTest {
val analyticsService = FakeAnalyticsService()
val alice = anAlice()
val me = aRoomMember(displayName = "CREATOR", role = RoomMember.Role.Owner(isCreator = true), userId = A_SESSION_ID)
val room = FakeJoinedRoom(
updateUserRoleResult = { Result.success(Unit) },
baseRoom = FakeBaseRoom(updateMembersResult = { Result.success(Unit) }),
).apply {
givenRoomMembersState(RoomMembersState.Ready(aRoomMemberList()))
givenRoomInfo(
aRoomInfo(
roomCreators = listOf(sessionId),
roomPowerLevels = roomPowerLevelsWithRole(role = RoomMember.Role.Admin, userId = A_USER_ID_2)
)
)
val roomMemberList = persistentListOf(alice, me)
givenRoomMembersState(RoomMembersState.Ready(roomMemberList))
}
val presenter = createChangeRolesPresenter(
role = RoomMember.Role.Admin,
@@ -499,7 +502,7 @@ class ChangeRolesPresenterTest {
skipItems(2)
val initialState = awaitItem()
assertThat(initialState.selectedUsers).hasSize(2)
initialState.eventSink(ChangeRolesEvent.UserSelectionToggled(MatrixUser(A_USER_ID_2)))
initialState.eventSink(ChangeRolesEvent.UserSelectionToggled(alice.toMatrixUser()))
awaitItem().also {
assertThat(it.selectedUsers).hasSize(1)
it.eventSink(ChangeRolesEvent.Save)

View File

@@ -8,16 +8,17 @@
package io.element.android.features.rolesandpermissions.impl.root
import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import im.vector.app.features.analytics.plan.RoomModeration
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.RoomMembersState
import io.element.android.libraries.matrix.test.room.FakeBaseRoom
import io.element.android.libraries.matrix.test.room.FakeJoinedRoom
import io.element.android.libraries.matrix.test.room.aRoomMemberList
import io.element.android.services.analytics.test.FakeAnalyticsService
import io.element.android.tests.testutils.test
import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.StandardTestDispatcher
@@ -30,12 +31,10 @@ class RolesAndPermissionPresenterTest {
@Test
fun `present - initial state`() = runTest {
val presenter = createRolesAndPermissionsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
with(awaitItem()) {
assertThat(adminCount).isEqualTo(0)
assertThat(moderatorCount).isEqualTo(0)
assertThat(adminCount).isNull()
assertThat(moderatorCount).isNull()
assertThat(changeOwnRoleAction).isEqualTo(AsyncAction.Uninitialized)
}
}
@@ -44,12 +43,9 @@ class RolesAndPermissionPresenterTest {
@Test
fun `present - ChangeOwnRole presents a confirmation dialog`() = runTest {
val presenter = createRolesAndPermissionsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(RolesAndPermissionsEvents.ChangeOwnRole)
assertThat(awaitItem().changeOwnRoleAction).isEqualTo(AsyncAction.ConfirmingNoParams)
}
}
@@ -60,12 +56,11 @@ class RolesAndPermissionPresenterTest {
val presenter = createRolesAndPermissionsPresenter(
dispatchers = testCoroutineDispatchers(),
room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(updateMembersResult = {}),
updateUserRoleResult = { Result.success(Unit) }
),
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(RolesAndPermissionsEvents.DemoteSelfTo(RoomMember.Role.Moderator))
@@ -81,12 +76,11 @@ class RolesAndPermissionPresenterTest {
@Test
fun `present - DemoteSelfTo can handle failures and clean them`() = runTest(StandardTestDispatcher()) {
val room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(updateMembersResult = {}),
updateUserRoleResult = { Result.failure(Exception("Failed to update role")) }
)
val presenter = createRolesAndPermissionsPresenter(room = room, dispatchers = testCoroutineDispatchers())
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(RolesAndPermissionsEvents.DemoteSelfTo(RoomMember.Role.Moderator))
@@ -104,9 +98,7 @@ class RolesAndPermissionPresenterTest {
@Test
fun `present - CancelPendingAction dismisses confirmation dialog too`() = runTest {
val presenter = createRolesAndPermissionsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(RolesAndPermissionsEvents.ChangeOwnRole)
awaitItem().eventSink(RolesAndPermissionsEvents.CancelPendingAction)
@@ -121,12 +113,11 @@ class RolesAndPermissionPresenterTest {
val presenter = createRolesAndPermissionsPresenter(
analyticsService = analyticsService,
room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(updateMembersResult = {}),
resetPowerLevelsResult = { Result.success(Unit) }
)
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(RolesAndPermissionsEvents.ResetPermissions)
// Confirmation
@@ -141,9 +132,7 @@ class RolesAndPermissionPresenterTest {
@Test
fun `present - ResetPermissions confirmation can be cancelled`() = runTest {
val presenter = createRolesAndPermissionsPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(RolesAndPermissionsEvents.ResetPermissions)
awaitItem().eventSink(RolesAndPermissionsEvents.CancelPendingAction)
@@ -152,8 +141,26 @@ class RolesAndPermissionPresenterTest {
}
}
@Test
fun `present - admins and moderator counts are updated when members changes`() = runTest {
val room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(updateMembersResult = {}),
)
val presenter = createRolesAndPermissionsPresenter(room = room)
presenter.test {
val initialState = awaitItem()
assertThat(initialState.adminCount).isNull()
assertThat(initialState.moderatorCount).isNull()
room.givenRoomMembersState(state = RoomMembersState.Ready(aRoomMemberList()))
skipItems(1)
val finalState = awaitItem()
assertThat(finalState.adminCount).isEqualTo(1)
assertThat(finalState.moderatorCount).isEqualTo(1)
}
}
private fun TestScope.createRolesAndPermissionsPresenter(
room: FakeJoinedRoom = FakeJoinedRoom(),
room: FakeJoinedRoom = FakeJoinedRoom(baseRoom = FakeBaseRoom(updateMembersResult = {})),
dispatchers: CoroutineDispatchers = testCoroutineDispatchers(),
analyticsService: FakeAnalyticsService = FakeAnalyticsService()
): RolesAndPermissionsPresenter {

View File

@@ -79,15 +79,4 @@ data class RoomInfo(
) {
val aliases: List<RoomAlias>
get() = listOfNotNull(canonicalAlias) + alternativeAliases
/**
* Returns the list of users with the given [role] in this room.
*/
fun usersWithRole(role: RoomMember.Role): List<UserId> {
return if (role is RoomMember.Role.Owner && role.isCreator) {
this.creators
} else {
this.roomPowerLevels?.usersWithRole(role).orEmpty().toList()
}
}
}

View File

@@ -15,17 +15,16 @@ import io.element.android.libraries.matrix.api.room.activeRoomMembers
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onStart
/**
* Return a flow of the list of active room members who have the given role.
* Return a flow of the list of active room members who match the predicate.
*/
fun BaseRoom.usersWithRole(role: RoomMember.Role): Flow<ImmutableList<RoomMember>> {
// Ensure the room members flow is ready
fun BaseRoom.usersWithRole(predicate: (RoomMember.Role) -> Boolean): Flow<ImmutableList<RoomMember>> {
// Wait until members are ready to avoid returning empty lists initially
val readyMembersFlow = membersStateFlow
.onStart {
if (membersStateFlow.value is RoomMembersState.Unknown) {
@@ -34,12 +33,17 @@ fun BaseRoom.usersWithRole(role: RoomMember.Role): Flow<ImmutableList<RoomMember
}
.filter { it is RoomMembersState.Ready }
return roomInfoFlow
.map { roomInfo -> roomInfo.usersWithRole(role) }
.combine(readyMembersFlow) { powerLevels, membersState ->
membersState.activeRoomMembers()
.filter { powerLevels.contains(it.userId) }
.toImmutableList()
}
.distinctUntilChanged()
return readyMembersFlow.map { membersState ->
membersState
.activeRoomMembers()
.filter({ predicate(it.role) })
.toImmutableList()
}.distinctUntilChanged()
}
/**
* Return the number of active room members who match the predicate.
*/
fun BaseRoom.userCountWithRole(predicate: (RoomMember.Role) -> Boolean): Flow<Int> {
return usersWithRole(predicate).map { it.size }
}

View File

@@ -35,19 +35,6 @@ data class RoomPowerLevels(
return users[userId] ?: 0L
}
/**
* Returns the set of [UserId]s that have the given role in the room.
*
* **WARNING**: This method must not be used with a creator role. It'll result in a runtime error.
*/
fun usersWithRole(role: RoomMember.Role): Set<UserId> {
return if (role is RoomMember.Role.Owner && role.isCreator) {
error("RoomPowerLevels.usersWithRole should not be used with a creator role, use roomInfo.creators instead")
} else {
users.filterValues { RoomMember.Role.forPowerLevel(it) == role }.keys
}
}
/**
* Returns the role of the user in the room based on their power level.
* If the user is not found, returns null.