Address review comments.

Author: bmarty

features/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpacePresenter.kt

@@ -27,7 +27,10 @@ import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.spaces.SpaceRoom
 import kotlinx.collections.immutable.ImmutableList
+import kotlinx.collections.immutable.ImmutableSet
+import kotlinx.collections.immutable.persistentSetOf
 import kotlinx.collections.immutable.toPersistentList
+import kotlinx.collections.immutable.toPersistentSet
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import kotlin.jvm.optionals.getOrNull
@@ -52,13 +55,13 @@ class LeaveSpacePresenter(
             mutableStateOf<AsyncAction<Unit>>(AsyncAction.Uninitialized)
         }
         val selectedRoomIds = remember {
-            mutableStateOf<Set<RoomId>>(emptySet())
+            mutableStateOf<ImmutableSet<RoomId>>(persistentSetOf())
         }
         val joinedSpaceRooms by produceState(emptyList()) {
-            // TODO Get the joined room from the SDK, should also have the
+            // TODO Get the joined room from the SDK, should also have the isLastAdmin boolean
             val rooms = emptyList<SpaceRoom>()
             // By default select all rooms
-            selectedRoomIds.value = rooms.map { it.roomId }.toSet()
+            selectedRoomIds.value = rooms.map { it.roomId }.toPersistentSet()
             value = rooms
         }
         val selectableSpaceRooms by produceState<AsyncData<ImmutableList<SelectableSpaceRoom>>>(
@@ -81,14 +84,14 @@ class LeaveSpacePresenter(
         fun handleEvents(event: LeaveSpaceEvents) {
             when (event) {
                 LeaveSpaceEvents.DeselectAllRooms -> {
-                    selectedRoomIds.value = emptySet()
+                    selectedRoomIds.value = persistentSetOf()
                 }
                 LeaveSpaceEvents.SelectAllRooms -> {
                     selectedRoomIds.value = selectableSpaceRooms.dataOrNull()
                         .orEmpty()
                         .filter { it.isLastAdmin.not() }
                         .map { it.spaceRoom.roomId }
-                        .toSet()
+                        .toPersistentSet()
                 }
                 is LeaveSpaceEvents.ToggleRoomSelection -> {
                     val currentSet = selectedRoomIds.value
@@ -97,6 +100,7 @@ class LeaveSpacePresenter(
                     } else {
                         currentSet + event.roomId
                     }
+                        .toPersistentSet()
                 }
                 LeaveSpaceEvents.LeaveSpace -> coroutineScope.leaveSpace(
                     leaveSpaceAction = leaveSpaceAction,

features/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpaceState.kt

@@ -18,33 +18,27 @@ data class LeaveSpaceState(
     val eventSink: (LeaveSpaceEvents) -> Unit,
 ) {
     private val rooms = selectableSpaceRooms.dataOrNull().orEmpty()
+    private val partition = rooms.partition { it.isLastAdmin }
+    private val lastAdminRooms = partition.first
+    private val selectableRooms = partition.second
 
     /**
      * True if we should show the quick action to select/deselect all rooms.
      */
-    val showQuickAction = rooms
-        .any { !it.isLastAdmin }
+    val showQuickAction = selectableRooms.isNotEmpty()
 
     /**
-     * True if there are rooms and they are all selected.
+     * True if there all the selectable rooms are selected.
      */
-    val areAllSelected = rooms
-        .filter { !it.isLastAdmin }
-        .let { rooms ->
-            rooms.isNotEmpty() && rooms.all { it.isSelected }
-        }
+    val areAllSelected = selectableRooms.all { it.isSelected }
 
     /**
      * True if there are rooms but the user is the last admin in all of them.
      */
-    val hasOnlyLastAdminRoom = rooms
-        .let { rooms ->
-            rooms.isNotEmpty() && rooms.all { it.isLastAdmin }
-        }
+    val hasOnlyLastAdminRoom = lastAdminRooms.isNotEmpty() && selectableRooms.isEmpty()
 
     /**
      * Number of selected rooms.
      */
-    val numberOfSelectRooms = rooms
-        .count { it.isSelected }
+    val selectedRoomsCount = selectableRooms.count { it.isSelected }
 }

features/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpaceView.kt

@@ -118,7 +118,7 @@ fun LeaveSpaceView(
             }
             LeaveSpaceButtons(
                 showLeaveButton = state.selectableSpaceRooms is AsyncData.Success,
-                nbOfSelectedRooms = state.numberOfSelectRooms,
+                selectedRoomsCount = state.selectedRoomsCount,
                 onLeaveSpace = {
                     state.eventSink(LeaveSpaceEvents.LeaveSpace)
                 },
@@ -198,16 +198,16 @@ private fun LeaveSpaceHeader(
 @Composable
 private fun LeaveSpaceButtons(
     showLeaveButton: Boolean,
-    nbOfSelectedRooms: Int,
+    selectedRoomsCount: Int,
     onLeaveSpace: () -> Unit,
     onCancel: () -> Unit,
 ) {
     ButtonColumnMolecule(
         modifier = Modifier.padding(top = 16.dp)
     ) {
         if (showLeaveButton) {
-            val text = if (nbOfSelectedRooms > 0) {
-                pluralStringResource(R.plurals.screen_leave_space_submit, nbOfSelectedRooms, nbOfSelectedRooms)
+            val text = if (selectedRoomsCount > 0) {
+                pluralStringResource(R.plurals.screen_leave_space_submit, selectedRoomsCount, selectedRoomsCount)
             } else {
                 stringResource(CommonStrings.action_leave_space)
             }

features/space/impl/src/test/kotlin/io/element/android/features/space/impl/leave/LeaveSpaceStateTest.kt

@@ -20,9 +20,9 @@ class LeaveSpaceStateTest {
             selectableSpaceRooms = AsyncData.Loading()
         )
         assertThat(sut.showQuickAction).isFalse()
-        assertThat(sut.areAllSelected).isFalse()
+        assertThat(sut.areAllSelected).isTrue()
         assertThat(sut.hasOnlyLastAdminRoom).isFalse()
-        assertThat(sut.numberOfSelectRooms).isEqualTo(0)
+        assertThat(sut.selectedRoomsCount).isEqualTo(0)
     }
 
     @Test
@@ -33,9 +33,9 @@ class LeaveSpaceStateTest {
             )
         )
         assertThat(sut.showQuickAction).isFalse()
-        assertThat(sut.areAllSelected).isFalse()
+        assertThat(sut.areAllSelected).isTrue()
         assertThat(sut.hasOnlyLastAdminRoom).isFalse()
-        assertThat(sut.numberOfSelectRooms).isEqualTo(0)
+        assertThat(sut.selectedRoomsCount).isEqualTo(0)
     }
 
     @Test
@@ -51,7 +51,7 @@ class LeaveSpaceStateTest {
         assertThat(sut.showQuickAction).isTrue()
         assertThat(sut.areAllSelected).isFalse()
         assertThat(sut.hasOnlyLastAdminRoom).isFalse()
-        assertThat(sut.numberOfSelectRooms).isEqualTo(1)
+        assertThat(sut.selectedRoomsCount).isEqualTo(1)
     }
 
     @Test
@@ -67,7 +67,7 @@ class LeaveSpaceStateTest {
         assertThat(sut.showQuickAction).isTrue()
         assertThat(sut.areAllSelected).isTrue()
         assertThat(sut.hasOnlyLastAdminRoom).isFalse()
-        assertThat(sut.numberOfSelectRooms).isEqualTo(2)
+        assertThat(sut.selectedRoomsCount).isEqualTo(2)
     }
 
     @Test
@@ -84,7 +84,7 @@ class LeaveSpaceStateTest {
         assertThat(sut.showQuickAction).isTrue()
         assertThat(sut.areAllSelected).isTrue()
         assertThat(sut.hasOnlyLastAdminRoom).isFalse()
-        assertThat(sut.numberOfSelectRooms).isEqualTo(2)
+        assertThat(sut.selectedRoomsCount).isEqualTo(2)
     }
 
     @Test
@@ -98,8 +98,8 @@ class LeaveSpaceStateTest {
             )
         )
         assertThat(sut.showQuickAction).isFalse()
-        assertThat(sut.areAllSelected).isFalse()
+        assertThat(sut.areAllSelected).isTrue()
         assertThat(sut.hasOnlyLastAdminRoom).isTrue()
-        assertThat(sut.numberOfSelectRooms).isEqualTo(0)
+        assertThat(sut.selectedRoomsCount).isEqualTo(0)
     }
 }