From fdc133ea7735dc5c634cd5e0cce94f2779202519 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 25 Nov 2025 16:30:16 +0100 Subject: [PATCH] Creators can downgrade owners. Fixes #5632 --- .../impl/roles/ChangeRolesPresenter.kt | 20 +++++++- .../impl/roles/ChangeRolesStateProvider.kt | 8 +--- .../impl/roles/ChangeRolesView.kt | 15 +----- .../impl/roles/ChangeRolesPresenterTest.kt | 47 ++++++++++--------- 4 files changed, 46 insertions(+), 44 deletions(-) diff --git a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenter.kt b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenter.kt index fdb17d6e04..f7736cb77c 100644 --- a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenter.kt +++ b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenter.kt @@ -44,6 +44,8 @@ import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach @@ -77,7 +79,23 @@ class ChangeRolesPresenter( } val saveState: MutableState> = remember { mutableStateOf(AsyncAction.Uninitialized) } val usersWithRole = produceState>(initialValue = persistentListOf()) { - room.usersWithRole(role).map { members -> members.map { it.toMatrixUser() } } + // 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 + } + } else { + emptyFlow() + } + combine( + owners, + room.usersWithRole(role), + ) { owners, users -> + owners + users + }.map { members -> members.map { it.toMatrixUser() } } .onEach { users -> val previous = value value = users.toImmutableList() diff --git a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt index 2594681850..654259c02a 100644 --- a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt +++ b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt @@ -50,13 +50,7 @@ class ChangeRolesStateProvider : PreviewParameterProvider { aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Loading), aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Success(true)), aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Failure(Exception("boom"))), - aChangeRolesStateWithOwners( - role = RoomMember.Role.Admin, - // Do not include the owners in the selectedUsers (the presenter will not do it), the View will add them - selectedUsers = listOf( - aMatrixUser(id = "@carol:server.org", displayName = "Carol"), - ) - ), + aChangeRolesStateWithOwners(role = RoomMember.Role.Admin), aChangeRolesStateWithOwners(role = RoomMember.Role.Owner(isCreator = false)), aChangeRolesStateWithOwners(role = RoomMember.Role.Owner(isCreator = false)) .copy(savingState = ConfirmingModifyingOwners), diff --git a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt index bd9969dec4..bab24d3f42 100644 --- a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt +++ b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt @@ -30,7 +30,6 @@ import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.runtime.Composable -import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource @@ -72,7 +71,6 @@ import io.element.android.libraries.matrix.ui.components.SelectedUsersRowList import io.element.android.libraries.matrix.ui.model.getAvatarData import io.element.android.libraries.ui.strings.CommonStrings import kotlinx.collections.immutable.ImmutableList -import kotlinx.collections.immutable.toImmutableList @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -222,18 +220,7 @@ private fun SearchResultsList( state = lazyListState, ) { item { - val usersInHorizontalRow = remember(searchResults.owners, selectedUsers) { - if (currentRole == RoomMember.Role.Admin) { - // Also include the owners in the horizontal list - val owners = searchResults.owners.map { - it.toMatrixUser() - } - (owners + selectedUsers).toImmutableList() - } else { - selectedUsers - } - } - selectedUsersList(usersInHorizontalRow) + selectedUsersList(selectedUsers) } if (searchResults.owners.isNotEmpty()) { stickyHeader { ListSectionHeader(text = stringResource(R.string.screen_room_roles_and_permissions_owners)) } diff --git a/features/rolesandpermissions/impl/src/test/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenterTest.kt b/features/rolesandpermissions/impl/src/test/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenterTest.kt index 11b9a66319..4bc39fd971 100644 --- a/features/rolesandpermissions/impl/src/test/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenterTest.kt +++ b/features/rolesandpermissions/impl/src/test/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenterTest.kt @@ -106,6 +106,7 @@ class ChangeRolesPresenterTest { awaitItem().run { assertThat(canChangeMemberRole(A_USER_ID_2)).isTrue() // Admin assertThat(canChangeMemberRole(A_USER_ID_3)).isTrue() // Moderator + assertThat(canChangeMemberRole(superAdminUserId)).isTrue() // Super admin assertThat(canChangeMemberRole(creatorUserId)).isFalse() // Owner } } @@ -170,7 +171,7 @@ class ChangeRolesPresenterTest { } val presenter = createChangeRolesPresenter(room = room) presenter.test { - skipItems(1) + skipItems(2) awaitItem().searchResults.run { assertThat(this).isInstanceOf(SearchBarResultState.Results::class.java) val results = (this as SearchBarResultState.Results).results @@ -424,11 +425,14 @@ class ChangeRolesPresenterTest { analyticsService = analyticsService ) presenter.test { - skipItems(2) + skipItems(1) val initialState = awaitItem() - assertThat(initialState.selectedUsers).hasSize(1) + assertThat(initialState.selectedUsers).isEmpty() initialState.eventSink(ChangeRolesEvent.UserSelectionToggled(MatrixUser(A_USER_ID_2))) - awaitItem().eventSink(ChangeRolesEvent.Save) + awaitItem().also { + assertThat(it.selectedUsers).hasSize(1) + it.eventSink(ChangeRolesEvent.Save) + } assertThat(awaitItem().savingState).isInstanceOf(AsyncAction.Loading::class.java) assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(true)) assertThat(analyticsService.capturedEvents.last()).isEqualTo(RoomModeration(RoomModeration.Action.ChangeMemberRole, RoomModeration.Role.Moderator)) @@ -459,14 +463,13 @@ class ChangeRolesPresenterTest { analyticsService = analyticsService ) presenter.test { - skipItems(1) val initialState = awaitItem() - assertThat(initialState.selectedUsers).hasSize(1) - + assertThat(initialState.selectedUsers).isEmpty() initialState.eventSink(ChangeRolesEvent.UserSelectionToggled(MatrixUser(A_USER_ID_2))) - - awaitItem().eventSink(ChangeRolesEvent.Save) - + awaitItem().also { + assertThat(it.selectedUsers).hasSize(1) + it.eventSink(ChangeRolesEvent.Save) + } assertThat(awaitItem().savingState.isConfirming()).isTrue() } } @@ -494,12 +497,12 @@ class ChangeRolesPresenterTest { presenter.test { skipItems(2) val initialState = awaitItem() - assertThat(initialState.selectedUsers).hasSize(1) - + assertThat(initialState.selectedUsers).hasSize(2) initialState.eventSink(ChangeRolesEvent.UserSelectionToggled(MatrixUser(A_USER_ID_2))) - - awaitItem().eventSink(ChangeRolesEvent.Save) - + awaitItem().also { + assertThat(it.selectedUsers).hasSize(1) + it.eventSink(ChangeRolesEvent.Save) + } val loadingState = awaitItem() assertThat(loadingState.savingState).isInstanceOf(AsyncAction.Loading::class.java) assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(true)) @@ -517,25 +520,25 @@ class ChangeRolesPresenterTest { } val presenter = createChangeRolesPresenter(role = RoomMember.Role.Moderator, room = room) presenter.test { - skipItems(2) + skipItems(1) val initialState = awaitItem() - assertThat(initialState.selectedUsers).hasSize(1) - + assertThat(initialState.selectedUsers).isEmpty() initialState.eventSink(ChangeRolesEvent.UserSelectionToggled(MatrixUser(A_USER_ID_2))) - - awaitItem().eventSink(ChangeRolesEvent.Save) + awaitItem().also { + assertThat(it.selectedUsers).hasSize(1) + it.eventSink(ChangeRolesEvent.Save) + } val loadingState = awaitItem() assertThat(loadingState.savingState).isInstanceOf(AsyncAction.Loading::class.java) val failedState = awaitItem() assertThat(failedState.savingState).isInstanceOf(AsyncAction.Failure::class.java) - failedState.eventSink(ChangeRolesEvent.CloseDialog) assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Uninitialized) } } @Test - fun `test analytics mapping`()= runTest { + fun `test analytics mapping`() = runTest { val presenter = createChangeRolesPresenter() with(presenter) { assertThat(RoomMember.Role.User.toAnalyticsMemberRole()).isEqualTo(RoomModeration.Role.User)