Creators can downgrade owners. Fixes #5632

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<AsyncAction<Boolean>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
         val usersWithRole = produceState<ImmutableList<MatrixUser>>(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()

features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt

@@ -50,13 +50,7 @@ class ChangeRolesStateProvider : PreviewParameterProvider<ChangeRolesState> {
             aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Loading),
             aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Success(true)),
             aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Failure(Exception("boom"))),
-            aChangeRolesStateWithOwners(
+            aChangeRolesStateWithOwners(role = RoomMember.Role.Admin),
-                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.Owner(isCreator = false)),
             aChangeRolesStateWithOwners(role = RoomMember.Role.Owner(isCreator = false))
                 .copy(savingState = ConfirmingModifyingOwners),

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) {
+            selectedUsersList(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)
         }
         if (searchResults.owners.isNotEmpty()) {
             stickyHeader { ListSectionHeader(text = stringResource(R.string.screen_room_roles_and_permissions_owners)) }

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().also {
-            awaitItem().eventSink(ChangeRolesEvent.Save)
+                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().also {
-            awaitItem().eventSink(ChangeRolesEvent.Save)
+                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().also {
-            awaitItem().eventSink(ChangeRolesEvent.Save)
+                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)