diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/SpaceFlowNode.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/SpaceFlowNode.kt index 666a538a29..2b29a895a6 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/SpaceFlowNode.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/SpaceFlowNode.kt @@ -14,6 +14,7 @@ import android.os.Parcelable import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier +import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.lifecycle.subscribe import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.node.Node @@ -40,6 +41,7 @@ import io.element.android.libraries.di.RoomScope import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.room.JoinedRoom import io.element.android.libraries.matrix.api.spaces.SpaceService +import io.element.android.libraries.matrix.api.spaces.loadAllIncrementally import kotlinx.parcelize.Parcelize @ContributesNode(RoomScope::class) @@ -83,6 +85,9 @@ class SpaceFlowNode( override fun onBuilt() { super.onBuilt() lifecycle.subscribe( + onCreate = { + spaceRoomList.loadAllIncrementally(lifecycleScope) + }, onDestroy = { spaceRoomList.destroy() } @@ -161,7 +166,7 @@ class SpaceFlowNode( buildContext = buildContext, callback = callback, ) - .setParentSpace(spaceRoomList.roomId) + .setParentSpace(spaceRoomList.spaceId) .build() } NavTarget.AddRoom -> { diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceEvent.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceEvent.kt index ee4dc12ba4..b052bbc37e 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceEvent.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceEvent.kt @@ -14,4 +14,5 @@ sealed interface AddRoomToSpaceEvent { data class OnSearchActiveChanged(val active: Boolean) : AddRoomToSpaceEvent data object Save : AddRoomToSpaceEvent data object ResetSaveAction : AddRoomToSpaceEvent + data object Dismiss : AddRoomToSpaceEvent } diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceNode.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceNode.kt index 71e7665844..dadd54cf6f 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceNode.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceNode.kt @@ -40,7 +40,7 @@ class AddRoomToSpaceNode( val state by stateFlow.collectAsState() AddRoomToSpaceView( state = state, - onBackClick = ::navigateUp, + onBackClick = callback::onFinish, onRoomsAdded = callback::onFinish, modifier = modifier ) diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpacePresenter.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpacePresenter.kt index 657d0702f8..9393018cdd 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpacePresenter.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpacePresenter.kt @@ -12,7 +12,6 @@ import androidx.compose.foundation.text.input.rememberTextFieldState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState -import androidx.compose.runtime.State import androidx.compose.runtime.collectAsState import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue @@ -25,6 +24,7 @@ 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.designsystem.theme.components.SearchBarResultState +import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.spaces.SpaceRoomList import io.element.android.libraries.matrix.api.spaces.SpaceService import io.element.android.libraries.matrix.ui.model.SelectRoomInfo @@ -45,9 +45,11 @@ class AddRoomToSpacePresenter( @Composable override fun present(): AddRoomToSpaceState { var selectedRooms: ImmutableList by remember { mutableStateOf(persistentListOf()) } - var searchQuery = rememberTextFieldState() + val searchQuery = rememberTextFieldState() var isSearchActive by remember { mutableStateOf(false) } val saveAction = remember { mutableStateOf>(AsyncAction.Uninitialized) } + // Track whether any rooms were added (for conditional reset on Dismiss) + var hasAddedRooms by remember { mutableStateOf(false) } val coroutineScope = rememberCoroutineScope() val dataSource = remember { dataSourceFactory.create(coroutineScope) } @@ -63,12 +65,12 @@ class AddRoomToSpacePresenter( val suggestions by dataSource.suggestions.collectAsState(initial = persistentListOf()) val filteredRooms by dataSource.roomInfoList.collectAsState(initial = persistentListOf()) - val searchResults by remember>>> { + val searchResults by remember { derivedStateOf { when { filteredRooms.isNotEmpty() -> SearchBarResultState.Results(filteredRooms) - isSearchActive && searchQuery.text.isNotEmpty() -> SearchBarResultState.NoResultsFound() - else -> SearchBarResultState.Initial() + isSearchActive && searchQuery.text.isNotEmpty() -> SearchBarResultState.NoResultsFound>() + else -> SearchBarResultState.Initial>() } } } @@ -91,12 +93,24 @@ class AddRoomToSpacePresenter( AddRoomToSpaceEvent.Save -> { coroutineScope.addRoomsToSpace( selectedRooms = selectedRooms, - addAction = saveAction, + dataSource = dataSource, + saveAction = saveAction, + onPartialSuccess = { successfullyAdded -> + if (successfullyAdded.isNotEmpty()) { + hasAddedRooms = true + } + selectedRooms = selectedRooms.filterNot { it.roomId in successfullyAdded }.toImmutableList() + }, ) } AddRoomToSpaceEvent.ResetSaveAction -> { saveAction.value = AsyncAction.Uninitialized } + AddRoomToSpaceEvent.Dismiss -> { + if (hasAddedRooms) { + coroutineScope.launch { spaceRoomList.reset() } + } + } } } @@ -113,21 +127,30 @@ class AddRoomToSpacePresenter( private fun CoroutineScope.addRoomsToSpace( selectedRooms: ImmutableList, - addAction: MutableState>, + dataSource: AddRoomToSpaceSearchDataSource, + saveAction: MutableState>, + onPartialSuccess: (Set) -> Unit, ) = launch { - addAction.runUpdatingState { - val results = selectedRooms.map { selectedRoom -> + saveAction.runUpdatingState { + val spaceId = spaceRoomList.spaceId + val successfullyAdded = mutableSetOf() + val results = selectedRooms.map { room -> async { spaceService.addChildToSpace( - spaceId = spaceRoomList.roomId, - childId = selectedRoom.roomId, - ) + spaceId = spaceId, + childId = room.roomId, + ).onSuccess { successfullyAdded.add(room.roomId) } } }.awaitAll() val anyFailure = results.any { it.isFailure } if (anyFailure) { + // On partial success, mark added rooms in data source and update selection + dataSource.markAsAdded(successfullyAdded) + onPartialSuccess(successfullyAdded) Result.failure(Exception("Failed to add some rooms")) } else { + // On full success, refresh the space room list + spaceRoomList.reset() Result.success(Unit) } } diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceSearchDataSource.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceSearchDataSource.kt index 61f08d53ae..24c2095ff9 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceSearchDataSource.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceSearchDataSource.kt @@ -29,6 +29,7 @@ import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map @@ -48,7 +49,7 @@ class AddRoomToSpaceSearchDataSource( roomListService: RoomListService, spaceRoomList: SpaceRoomList, private val matrixClient: MatrixClient, - private val coroutineDispatchers: CoroutineDispatchers, + coroutineDispatchers: CoroutineDispatchers, ) { @AssistedFactory interface Factory { @@ -62,33 +63,49 @@ class AddRoomToSpaceSearchDataSource( coroutineScope = coroutineScope, ) - private val spaceChildrenFlow = spaceRoomList.spaceRoomsFlow.map { spaceChildren -> - spaceChildren.map { it.roomId }.toSet() + private val spaceChildrenFlow = spaceRoomList.spaceRoomsFlow.map { rooms -> + rooms.map { it.roomId }.toSet() } - private val filterRoomPredicate: (RoomInfo, Set) -> Boolean = { info, childIds -> + // Track locally added rooms for partial success handling. + // These rooms will be filtered out from search results and suggestions. + private val addedRoomIdsFlow = MutableStateFlow>(emptySet()) + + /** + * Marks rooms as added to the space (for partial success handling). + */ + fun markAsAdded(roomIds: Set) { + addedRoomIdsFlow.value += roomIds + } + + private val filterRoomPredicate: (RoomInfo, Set, Set) -> Boolean = { info, childIds, addedIds -> !info.isSpace && !info.isDm && info.currentUserMembership == CurrentUserMembership.JOINED && - info.id !in childIds + info.id !in childIds && + info.id !in addedIds } val roomInfoList: Flow> = combine( roomList.filteredSummaries, spaceChildrenFlow, - ) { roomSummaries, childIds -> + addedRoomIdsFlow, + ) { roomSummaries, childIds, addedIds -> roomSummaries - .filter { filterRoomPredicate(it.info, childIds) } - .map { it.toSelectRoomInfo() } + .filter { filterRoomPredicate(it.info, childIds, addedIds) } + .map { it.info.toSelectRoomInfo() } .toImmutableList() }.flowOn(coroutineDispatchers.computation) - val suggestions: Flow> = spaceChildrenFlow.map { childIds -> + val suggestions: Flow> = combine( + spaceChildrenFlow, + addedRoomIdsFlow, + ) { childIds, addedIds -> matrixClient - .getRecentlyVisitedRoomInfoFlow { filterRoomPredicate(it, childIds) } + .getRecentlyVisitedRoomInfoFlow { filterRoomPredicate(it, childIds, addedIds) } .take(MAX_SUGGESTIONS_COUNT) - .map { it.toSelectRoomInfo() } .toList() + .map { it.toSelectRoomInfo() } .toImmutableList() }.flowOn(coroutineDispatchers.computation) diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceView.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceView.kt index 854924041b..df40a0145f 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceView.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceView.kt @@ -58,14 +58,15 @@ fun AddRoomToSpaceView( onRoomsAdded: () -> Unit, modifier: Modifier = Modifier, ) { - fun onRoomRemoved(roomInfo: SelectRoomInfo) { - state.eventSink(AddRoomToSpaceEvent.ToggleRoom(roomInfo)) + fun onRoomToggled(room: SelectRoomInfo) { + state.eventSink(AddRoomToSpaceEvent.ToggleRoom(room)) } fun onBack() { if (state.isSearchActive) { state.eventSink(AddRoomToSpaceEvent.OnSearchActiveChanged(false)) } else { + state.eventSink(AddRoomToSpaceEvent.Dismiss) onBackClick() } } @@ -114,18 +115,18 @@ fun AddRoomToSpaceView( if (state.selectedRooms.isNotEmpty()) { SelectedRoomsRow( selectedRooms = state.selectedRooms, - onRemoveRoom = ::onRoomRemoved, + onRemoveRoom = ::onRoomToggled, modifier = Modifier.padding(vertical = 16.dp) ) } }, ) { rooms -> LazyColumn { - items(rooms, key = { it.roomId.value }) { roomInfo -> + items(rooms, key = { it.roomId }) { roomInfo -> RoomListItem( roomInfo = roomInfo, isSelected = state.selectedRooms.any { it.roomId == roomInfo.roomId }, - onToggle = { state.eventSink(AddRoomToSpaceEvent.ToggleRoom(it)) } + onToggle = ::onRoomToggled ) } } @@ -142,7 +143,7 @@ fun AddRoomToSpaceView( if (state.selectedRooms.isNotEmpty()) { SelectedRoomsRow( selectedRooms = state.selectedRooms, - onRemoveRoom = ::onRoomRemoved, + onRemoveRoom = ::onRoomToggled, modifier = Modifier.padding(vertical = 16.dp) ) } @@ -159,7 +160,7 @@ fun AddRoomToSpaceView( RoomListItem( roomInfo = roomInfo, isSelected = state.selectedRooms.any { it.roomId == roomInfo.roomId }, - onToggle = { state.eventSink(AddRoomToSpaceEvent.ToggleRoom(it)) } + onToggle = ::onRoomToggled ) } } @@ -205,8 +206,8 @@ private fun SelectedRoomsRow( contentPadding = PaddingValues(horizontal = 16.dp), horizontalArrangement = Arrangement.spacedBy(32.dp) ) { - items(selectedRooms, key = { it.roomId.value }) { roomInfo -> - SelectedRoom(roomInfo = roomInfo, onRemoveRoom = onRemoveRoom) + items(selectedRooms, key = { it.roomId }) { roomInfo -> + SelectedRoom(roomInfo = roomInfo, onRemoveRoom = { onRemoveRoom(roomInfo) }) } } } diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceNode.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceNode.kt index 5a5b10671c..9cb5138933 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceNode.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceNode.kt @@ -54,7 +54,7 @@ class SpaceNode( private val callback: Callback = callback() private fun onShareRoom(context: Context) = lifecycleScope.launch { - matrixClient.getRoom(spaceRoomList.roomId)?.use { room -> + matrixClient.getRoom(spaceRoomList.spaceId)?.use { room -> room.getPermalink() .onSuccess { permalink -> context.startSharePlainTextIntent( diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpacePresenter.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpacePresenter.kt index a5bc2ec52e..055982cefa 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpacePresenter.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpacePresenter.kt @@ -69,7 +69,6 @@ class SpacePresenter( @Composable override fun present(): SpaceState { LaunchedEffect(Unit) { - paginate() spaceRoomList.spaceRoomsFlow.collect { children = it.toImmutableList() } } @@ -111,21 +110,18 @@ class SpacePresenter( var isManageMode by remember { mutableStateOf(false) } var selectedRoomIds by remember { mutableStateOf>(emptySet()) } var removeRoomsAction by remember { mutableStateOf>(AsyncAction.Uninitialized) } + // Track locally removed rooms for partial failure cases var removedRoomIds by remember { mutableStateOf>(emptySet()) } val filteredChildren by remember { derivedStateOf { - children - .filterNot { it.roomId in removedRoomIds } - .let { list -> - if (isManageMode) { - // In manage mode, only show rooms (not spaces) - list.filter { !it.isSpace } - } else { - list - } - } - .toImmutableList() + val notRemoved = children.filterNot { it.roomId in removedRoomIds } + if (isManageMode) { + // In manage mode, only show rooms (not spaces) + notRemoved.filter { !it.isSpace }.toImmutableList() + } else { + notRemoved.toImmutableList() + } } } @@ -139,9 +135,20 @@ class SpacePresenter( val acceptDeclineInviteState = acceptDeclineInvitePresenter.present() + suspend fun exitManageMode(shouldReset: Boolean) { + isManageMode = false + selectedRoomIds = emptySet() + removedRoomIds = emptySet() + if (shouldReset) { + // Reset the space room list to see the updates. + spaceRoomList.reset() + } + } + fun handleEvent(event: SpaceEvents) { when (event) { - SpaceEvents.LoadMore -> localCoroutineScope.paginate() + // SpaceRoomList is loaded automatically as backend is really slow. Event is kept for future. + SpaceEvents.LoadMore -> Unit is SpaceEvents.Join -> { sessionCoroutineScope.joinRoom(event.spaceRoom, joinActions, setJoinActions) } @@ -170,8 +177,7 @@ class SpacePresenter( selectedRoomIds = emptySet() } SpaceEvents.ExitManageMode -> { - isManageMode = false - selectedRoomIds = emptySet() + localCoroutineScope.launch { exitManageMode(shouldReset = removedRoomIds.isNotEmpty()) } } is SpaceEvents.ToggleRoomSelection -> { selectedRoomIds = if (event.roomId in selectedRoomIds) { @@ -186,7 +192,7 @@ class SpacePresenter( SpaceEvents.ConfirmRoomRemoval -> { localCoroutineScope.launch { removeRoomsAction = AsyncAction.Loading - val spaceId = spaceRoomList.roomId + val spaceId = spaceRoomList.spaceId val roomsToRemove = selectedRoomIds.toSet() val successfullyRemoved = mutableSetOf() val results = roomsToRemove.map { roomId -> @@ -196,16 +202,15 @@ class SpacePresenter( } } results.awaitAll() - if (successfullyRemoved.isNotEmpty()) { - removedRoomIds = removedRoomIds + successfullyRemoved - } val hasError = successfullyRemoved.size < roomsToRemove.size if (hasError) { + // On partial success, update selection to only keep failed rooms + selectedRoomIds = selectedRoomIds - successfullyRemoved + removedRoomIds = removedRoomIds + successfullyRemoved removeRoomsAction = AsyncAction.Failure(Exception("Failed to remove some rooms")) } else { removeRoomsAction = AsyncAction.Success(Unit) - isManageMode = false - selectedRoomIds = emptySet() + exitManageMode(shouldReset = true) } } } @@ -246,8 +251,4 @@ class SpacePresenter( setJoinActions(joinActions + mapOf(spaceRoom.roomId to AsyncAction.Failure(it))) } } - - private fun CoroutineScope.paginate() = launch { - spaceRoomList.paginate() - } } diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceView.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceView.kt index c54bf0009e..69fca57cb5 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceView.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceView.kt @@ -139,6 +139,7 @@ fun SpaceView( SpaceViewTopBar( spaceInfo = state.spaceInfo, canAccessSpaceSettings = state.canAccessSpaceSettings, + canEditSpaceGraph = state.canEditSpaceGraph, showManageRoomsAction = state.showManageRoomsAction, onBackClick = onBackClick, onLeaveSpaceClick = onLeaveSpaceClick, @@ -376,6 +377,7 @@ private fun LoadingMoreIndicator( private fun SpaceViewTopBar( spaceInfo: RoomInfo, canAccessSpaceSettings: Boolean, + canEditSpaceGraph: Boolean, showManageRoomsAction: Boolean, onBackClick: () -> Unit, onLeaveSpaceClick: () -> Unit, @@ -416,7 +418,7 @@ private fun SpaceViewTopBar( expanded = showMenu, onDismissRequest = { showMenu = false } ) { - if (showManageRoomsAction) { + if (canEditSpaceGraph) { SpaceMenuItem( titleRes = CommonStrings.action_create_room, icon = CompoundIcons.Plus(), @@ -433,14 +435,16 @@ private fun SpaceViewTopBar( onAddRoomClick() } ) - SpaceMenuItem( - titleRes = CommonStrings.action_manage_rooms, - icon = CompoundIcons.Edit(), - onClick = { - showMenu = false - onManageRoomsClick() - } - ) + if (showManageRoomsAction) { + SpaceMenuItem( + titleRes = CommonStrings.action_manage_rooms, + icon = CompoundIcons.Edit(), + onClick = { + showMenu = false + onManageRoomsClick() + } + ) + } HorizontalDivider() } SpaceMenuItem( diff --git a/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpacePresenterTest.kt b/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpacePresenterTest.kt index b9a67704c9..381206c3ec 100644 --- a/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpacePresenterTest.kt +++ b/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpacePresenterTest.kt @@ -17,14 +17,20 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.room.CurrentUserMembership import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.A_ROOM_ID +import io.element.android.libraries.matrix.test.A_ROOM_ID_2 import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.room.aRoomSummary import io.element.android.libraries.matrix.test.roomlist.FakeRoomListService import io.element.android.libraries.matrix.test.spaces.FakeSpaceRoomList import io.element.android.libraries.matrix.test.spaces.FakeSpaceService +import io.element.android.libraries.matrix.ui.components.aSelectRoomInfo +import io.element.android.libraries.matrix.ui.model.SelectRoomInfo +import io.element.android.tests.testutils.lambda.assert import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.test import io.element.android.tests.testutils.testCoroutineDispatchers +import kotlinx.collections.immutable.ImmutableList +import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestScope @@ -273,9 +279,71 @@ class AddRoomToSpacePresenterTest { } } + @Test + fun `present - Dismiss without additions does not call reset`() = runTest { + val resetResult = lambdaRecorder>(ensureNeverCalled = true) { Result.success(Unit) } + val spaceRoomList = FakeSpaceRoomList( + paginateResult = { Result.success(Unit) }, + resetResult = resetResult, + ) + val presenter = createAddRoomToSpacePresenter(spaceRoomList = spaceRoomList) + presenter.test { + val state = awaitItem() + state.eventSink(AddRoomToSpaceEvent.Dismiss) + advanceUntilIdle() + // reset should NOT be called since no rooms were added + assert(resetResult).isNeverCalled() + } + } + + @Test + fun `present - Dismiss after partial success calls reset`() = runTest { + val resetResult = lambdaRecorder> { Result.success(Unit) } + val spaceRoomList = FakeSpaceRoomList( + paginateResult = { Result.success(Unit) }, + resetResult = resetResult, + ) + // Room 1 succeeds, Room 2 fails + val addChildToSpaceResult = lambdaRecorder> { _, childId -> + if (childId == A_ROOM_ID_2) { + Result.failure(AN_EXCEPTION) + } else { + Result.success(Unit) + } + } + val spaceService = FakeSpaceService( + addChildToSpaceResult = addChildToSpaceResult, + ) + val presenter = createAddRoomToSpacePresenter( + spaceRoomList = spaceRoomList, + spaceService = spaceService, + ) + presenter.test { + val state = awaitItem() + // Select two rooms + val room1 = aSelectRoomInfoList()[0] + val room2 = aSelectRoomInfoList()[1] + state.eventSink(AddRoomToSpaceEvent.ToggleRoom(room1)) + awaitItem() + state.eventSink(AddRoomToSpaceEvent.ToggleRoom(room2)) + awaitItem() + // Save - partial success (one room added, one failed) + state.eventSink(AddRoomToSpaceEvent.Save) + skipItems(1) // Loading + advanceUntilIdle() + val failureState = expectMostRecentItem() + assertThat(failureState.saveAction).isInstanceOf(AsyncAction.Failure::class.java) + // Dismiss after partial success - reset should be called + failureState.eventSink(AddRoomToSpaceEvent.Dismiss) + advanceUntilIdle() + assert(resetResult).isCalledOnce() + } + } + private fun TestScope.createAddRoomToSpacePresenter( spaceRoomList: FakeSpaceRoomList = FakeSpaceRoomList( paginateResult = { Result.success(Unit) }, + resetResult = { Result.success(Unit) }, ), spaceService: FakeSpaceService = FakeSpaceService( addChildToSpaceResult = { _, _ -> Result.success(Unit) }, @@ -301,3 +369,8 @@ class AddRoomToSpacePresenterTest { ) } } + +private fun aSelectRoomInfoList(): ImmutableList = listOf( + aSelectRoomInfo(roomId = A_ROOM_ID, name = "Room 1"), + aSelectRoomInfo(roomId = A_ROOM_ID_2, name = "Room 2"), +).toImmutableList() diff --git a/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceViewTest.kt b/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceViewTest.kt index d6a9ff770a..16710ae0e4 100644 --- a/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceViewTest.kt +++ b/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/addroom/AddRoomToSpaceViewTest.kt @@ -32,16 +32,19 @@ class AddRoomToSpaceViewTest { @get:Rule val rule = createAndroidComposeRule() @Test - fun `clicking back when search inactive invokes onBackClick`() { + fun `clicking back when search inactive emits Dismiss and invokes onBackClick`() { + val eventsRecorder = EventsRecorder() ensureCalledOnce { rule.setAddRoomToSpaceView( anAddRoomToSpaceState( isSearchActive = false, + eventSink = eventsRecorder, ), onBackClick = it, ) rule.pressBack() } + eventsRecorder.assertSingle(AddRoomToSpaceEvent.Dismiss) } @Test diff --git a/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/root/SpacePresenterTest.kt b/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/root/SpacePresenterTest.kt index 811e9158b9..1d38e2e0f7 100644 --- a/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/root/SpacePresenterTest.kt +++ b/features/space/impl/src/test/kotlin/io/element/android/features/space/impl/root/SpacePresenterTest.kt @@ -42,13 +42,13 @@ import io.element.android.libraries.matrix.test.spaces.FakeSpaceRoomList import io.element.android.libraries.matrix.test.spaces.FakeSpaceService import io.element.android.libraries.previewutils.room.aSpaceRoom import io.element.android.tests.testutils.EventsRecorder +import io.element.android.tests.testutils.lambda.assert import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import io.element.android.tests.testutils.test import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle -import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Test import im.vector.app.features.analytics.plan.JoinedRoom as AnalyticsJoinedRoom @@ -56,10 +56,9 @@ import im.vector.app.features.analytics.plan.JoinedRoom as AnalyticsJoinedRoom class SpacePresenterTest { @Test fun `present - initial state`() = runTest { - val paginateResult = lambdaRecorder> { - Result.success(Unit) - } - val spaceRoomList = FakeSpaceRoomList(paginateResult = paginateResult) + val spaceRoomList = FakeSpaceRoomList( + paginateResult = { Result.success(Unit) } + ) val presenter = createSpacePresenter(spaceRoomList = spaceRoomList) presenter.test { val state = awaitItem() @@ -72,8 +71,6 @@ class SpacePresenterTest { assertThat(state.acceptDeclineInviteState).isEqualTo(anAcceptDeclineInviteState()) assertThat(state.topicViewerState).isEqualTo(TopicViewerState.Hidden) assertThat(state.canAccessSpaceSettings).isFalse() - advanceUntilIdle() - paginateResult.assertions().isCalledOnce() } } @@ -105,19 +102,17 @@ class SpacePresenterTest { } @Test - fun `present - load more`() = runTest { - val paginateResult = lambdaRecorder> { - Result.success(Unit) - } - val spaceRoomList = FakeSpaceRoomList(paginateResult = paginateResult) + fun `present - load more does nothing`() = runTest { + // LoadMore event is a no-op as pagination is handled automatically for now as backend is slow. + val spaceRoomList = FakeSpaceRoomList( + paginateResult = { Result.success(Unit) } + ) val presenter = createSpacePresenter(spaceRoomList = spaceRoomList) presenter.test { val state = awaitItem() - advanceUntilIdle() - paginateResult.assertions().isCalledOnce() + // LoadMore event should not cause any state change state.eventSink(SpaceEvents.LoadMore) - advanceUntilIdle() - paginateResult.assertions().isCalledExactly(2) + expectNoEvents() } } @@ -196,7 +191,6 @@ class SpacePresenterTest { assertThat(joiningState.joinActions[A_ROOM_ID_2]).isEqualTo(AsyncAction.Loading) // Let the joinRoom call complete advanceUntilIdle() - runCurrent() // The room is joined fakeSpaceRoomList.emitSpaceRooms( listOf( @@ -211,7 +205,7 @@ class SpacePresenterTest { val joinedState = awaitItem() // Joined room is removed from the join actions assertThat(joinedState.joinActions).doesNotContainKey(A_ROOM_ID_2) - joinRoom.assertions().isCalledOnce().with( + assert(joinRoom).isCalledOnce().with( value(A_ROOM_ID_2.toRoomIdOrAlias()), value(serverNames), value(AnalyticsJoinedRoom.Trigger.SpaceHierarchy), @@ -354,16 +348,24 @@ class SpacePresenterTest { } @Test - fun `present - exit manage mode clears selection`() = runTest { - val presenter = createSpacePresenter() + fun `present - exit manage mode without removals does not call reset`() = runTest { + val resetResult = lambdaRecorder>(ensureNeverCalled = true) { Result.success(Unit) } + val fakeSpaceRoomList = FakeSpaceRoomList( + paginateResult = { Result.success(Unit) }, + resetResult = resetResult, + ) + val presenter = createSpacePresenter(spaceRoomList = fakeSpaceRoomList) presenter.test { val initialState = awaitItem() initialState.eventSink(SpaceEvents.EnterManageMode) initialState.eventSink(SpaceEvents.ToggleRoomSelection(A_ROOM_ID)) initialState.eventSink(SpaceEvents.ExitManageMode) + advanceUntilIdle() val finalState = expectMostRecentItem() assertThat(finalState.isManageMode).isFalse() assertThat(finalState.selectedRoomIds).isEmpty() + // reset should NOT be called since no rooms were actually removed + assert(resetResult).isNeverCalled() } } @@ -389,6 +391,7 @@ class SpacePresenterTest { val removeChildFromSpaceResult = lambdaRecorder> { _, _ -> Result.success(Unit) } + val resetResult = lambdaRecorder> { Result.success(Unit) } val aRoom = aSpaceRoom( roomId = A_ROOM_ID, roomType = RoomType.Room, @@ -396,6 +399,7 @@ class SpacePresenterTest { val fakeSpaceRoomList = FakeSpaceRoomList( initialSpaceRoomsValue = listOf(aRoom), paginateResult = { Result.success(Unit) }, + resetResult = resetResult, ) val presenter = createSpacePresenter( spaceRoomList = fakeSpaceRoomList, @@ -416,8 +420,8 @@ class SpacePresenterTest { val successState = expectMostRecentItem() assertThat(successState.removeRoomsAction).isEqualTo(AsyncAction.Success(Unit)) assertThat(successState.isManageMode).isFalse() - assertThat(successState.children).isEmpty() - removeChildFromSpaceResult.assertions().isCalledOnce() + assert(removeChildFromSpaceResult).isCalledOnce() + assert(resetResult).isCalledOnce() } } @@ -465,7 +469,57 @@ class SpacePresenterTest { assertThat(failureState.children.map { it.roomId }).doesNotContain(A_ROOM_ID) // Failed room should still be present assertThat(failureState.children.map { it.roomId }).contains(A_ROOM_ID_2) - removeChildFromSpaceResult.assertions().isCalledExactly(2) + assert(removeChildFromSpaceResult).isCalledExactly(2) + } + } + + @Test + fun `present - exit manage mode after partial failure calls reset`() = runTest { + val aRoom1 = aSpaceRoom( + roomId = A_ROOM_ID, + roomType = RoomType.Room, + ) + val aRoom2 = aSpaceRoom( + roomId = A_ROOM_ID_2, + roomType = RoomType.Room, + ) + // Room 1 succeeds, Room 2 fails + val removeChildFromSpaceResult = lambdaRecorder> { _, childId -> + if (childId == A_ROOM_ID_2) { + Result.failure(AN_EXCEPTION) + } else { + Result.success(Unit) + } + } + val resetResult = lambdaRecorder> { Result.success(Unit) } + val fakeSpaceRoomList = FakeSpaceRoomList( + initialSpaceRoomsValue = listOf(aRoom1, aRoom2), + paginateResult = { Result.success(Unit) }, + resetResult = resetResult, + ) + val presenter = createSpacePresenter( + spaceRoomList = fakeSpaceRoomList, + spaceService = FakeSpaceService( + removeChildFromSpaceResult = removeChildFromSpaceResult, + ), + ) + presenter.test { + awaitItem() // Initial empty state + advanceUntilIdle() + val stateWithChildren = awaitItem() + stateWithChildren.eventSink(SpaceEvents.EnterManageMode) + stateWithChildren.eventSink(SpaceEvents.ToggleRoomSelection(A_ROOM_ID)) + stateWithChildren.eventSink(SpaceEvents.ToggleRoomSelection(A_ROOM_ID_2)) + stateWithChildren.eventSink(SpaceEvents.RemoveSelectedRooms) + stateWithChildren.eventSink(SpaceEvents.ConfirmRoomRemoval) + advanceUntilIdle() + val failureState = expectMostRecentItem() + assertThat(failureState.removeRoomsAction.isFailure()).isTrue() + // Exit manage mode after partial failure - reset should be called + failureState.eventSink(SpaceEvents.ExitManageMode) + advanceUntilIdle() + expectMostRecentItem() + assert(resetResult).isCalledOnce() } } @@ -501,10 +555,8 @@ class SpacePresenterTest { } @Test - fun `present - removed rooms persist after flow update`() = runTest { - val removeChildFromSpaceResult = lambdaRecorder> { _, _ -> - Result.success(Unit) - } + fun `present - removed rooms persist after flow update on partial failure`() = runTest { + // On partial failure, successfully removed rooms should stay filtered even after flow updates val aRoom1 = aSpaceRoom( roomId = A_ROOM_ID, roomType = RoomType.Room, @@ -517,6 +569,14 @@ class SpacePresenterTest { roomId = A_ROOM_ID_3, roomType = RoomType.Room, ) + // Room 1 succeeds, Room 2 fails + val removeChildFromSpaceResult = lambdaRecorder> { _, childId -> + if (childId == A_ROOM_ID_2) { + Result.failure(AN_EXCEPTION) + } else { + Result.success(Unit) + } + } val spaceRoomList = FakeSpaceRoomList( initialSpaceRoomsValue = listOf(aRoom1, aRoom2), paginateResult = { Result.success(Unit) }, @@ -532,12 +592,18 @@ class SpacePresenterTest { advanceUntilIdle() val stateWithChildren = awaitItem() stateWithChildren.eventSink(SpaceEvents.EnterManageMode) + // Select both rooms for removal stateWithChildren.eventSink(SpaceEvents.ToggleRoomSelection(A_ROOM_ID)) + stateWithChildren.eventSink(SpaceEvents.ToggleRoomSelection(A_ROOM_ID_2)) stateWithChildren.eventSink(SpaceEvents.RemoveSelectedRooms) stateWithChildren.eventSink(SpaceEvents.ConfirmRoomRemoval) advanceUntilIdle() - val successState = expectMostRecentItem() - assertThat(successState.children.map { it.roomId }).doesNotContain(A_ROOM_ID) + val failureState = expectMostRecentItem() + assertThat(failureState.removeRoomsAction.isFailure()).isTrue() + // Successfully removed room should be filtered out + assertThat(failureState.children.map { it.roomId }).doesNotContain(A_ROOM_ID) + // Failed room should still be present + assertThat(failureState.children.map { it.roomId }).contains(A_ROOM_ID_2) // Emit new flow update with a new room added (simulating server refresh) spaceRoomList.emitSpaceRooms(listOf(aRoom1, aRoom2, aRoom3)) advanceUntilIdle() @@ -571,7 +637,7 @@ class SpacePresenterTest { seenInvitesStore = seenInvitesStore, joinRoom = joinRoom, acceptDeclineInvitePresenter = acceptDeclineInvitePresenter, - sessionCoroutineScope = backgroundScope, + sessionCoroutineScope = this, featureFlagService = FakeFeatureFlagService( initialState = mapOf( FeatureFlags.SpaceSettings.key to spaceSettingsEnabled, diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/spaces/SpaceRoomList.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/spaces/SpaceRoomList.kt index e2528bf117..2ccee8d1ee 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/spaces/SpaceRoomList.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/spaces/SpaceRoomList.kt @@ -9,8 +9,11 @@ package io.element.android.libraries.matrix.api.spaces import io.element.android.libraries.matrix.api.core.RoomId +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import java.util.Optional interface SpaceRoomList { @@ -19,13 +22,31 @@ interface SpaceRoomList { data class Idle(val hasMoreToLoad: Boolean) : PaginationStatus } - val roomId: RoomId + val spaceId: RoomId val currentSpaceFlow: StateFlow> val spaceRoomsFlow: Flow> val paginationStatusFlow: StateFlow + suspend fun paginate(): Result + suspend fun reset(): Result fun destroy() } + +/** + * Loads all space rooms incrementally by automatically paginating whenever more data is available. + * This function observes the pagination status and triggers [paginate] calls until the entire list is loaded. + * + * @param coroutineScope The scope in which the pagination flow will be collected. + */ +fun SpaceRoomList.loadAllIncrementally(coroutineScope: CoroutineScope) { + paginationStatusFlow + .onEach { paginationStatus -> + if (paginationStatus is SpaceRoomList.PaginationStatus.Idle && paginationStatus.hasMoreToLoad) { + paginate() + } + } + .launchIn(coroutineScope) +} diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomList.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomList.kt index 80652bee71..867c14d18f 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomList.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomList.kt @@ -29,7 +29,7 @@ import java.util.Optional import org.matrix.rustcomponents.sdk.SpaceRoomList as InnerSpaceRoomList class RustSpaceRoomList( - override val roomId: RoomId, + override val spaceId: RoomId, private val innerProvider: suspend () -> InnerSpaceRoomList, private val coroutineScope: CoroutineScope, spaceRoomMapper: SpaceRoomMapper, @@ -81,9 +81,15 @@ class RustSpaceRoomList( } } + override suspend fun reset(): Result { + return runCatchingExceptions { + innerCompletable.await().reset() + } + } + @OptIn(ExperimentalCoroutinesApi::class) override fun destroy() { - Timber.d("Destroying SpaceRoomList $roomId") + Timber.d("Destroying SpaceRoomList $spaceId") coroutineScope.cancel() try { innerCompletable.getCompleted().destroy() diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceService.kt index 4d9bc2fe61..0bf677e09d 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceService.kt @@ -79,7 +79,7 @@ class RustSpaceService( override fun spaceRoomList(id: RoomId): SpaceRoomList { val childCoroutineScope = sessionCoroutineScope.childScope(sessionDispatcher, "SpaceRoomListScope-$this") return RustSpaceRoomList( - roomId = id, + spaceId = id, innerProvider = { innerSpaceService.spaceRoomList(id.value) }, coroutineScope = childCoroutineScope, spaceRoomMapper = spaceRoomMapper, diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiSpaceRoomList.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiSpaceRoomList.kt index 40c36271ea..c0ecc53d4f 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiSpaceRoomList.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiSpaceRoomList.kt @@ -21,6 +21,7 @@ import uniffi.matrix_sdk_ui.SpaceRoomListPaginationState class FakeFfiSpaceRoomList( private val paginateResult: () -> Unit = { lambdaError() }, + private val resetResult: () -> Unit = { lambdaError() }, private val paginationStateResult: () -> SpaceRoomListPaginationState = { lambdaError() }, private val roomsResult: () -> List = { lambdaError() }, ) : SpaceRoomList(NoHandle) { @@ -31,6 +32,10 @@ class FakeFfiSpaceRoomList( paginateResult() } + override suspend fun reset() = simulateLongTask { + resetResult() + } + override fun paginationState(): SpaceRoomListPaginationState { return paginationStateResult() } diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomListTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomListTest.kt index 08f9407b93..2b7dab4fb6 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomListTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomListTest.kt @@ -94,7 +94,7 @@ class RustSpaceRoomListTest { spaceRoomMapper: SpaceRoomMapper = SpaceRoomMapper(), ): RustSpaceRoomList { return RustSpaceRoomList( - roomId = roomId, + spaceId = roomId, innerProvider = innerProvider, coroutineScope = backgroundScope, spaceRoomMapper = spaceRoomMapper, diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/spaces/FakeSpaceRoomList.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/spaces/FakeSpaceRoomList.kt index 70d919dc21..f723e770d2 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/spaces/FakeSpaceRoomList.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/spaces/FakeSpaceRoomList.kt @@ -21,11 +21,12 @@ import kotlinx.coroutines.flow.asStateFlow import java.util.Optional class FakeSpaceRoomList( - override val roomId: RoomId = A_ROOM_ID, + override val spaceId: RoomId = A_ROOM_ID, initialSpaceFlowValue: SpaceRoom? = null, initialSpaceRoomsValue: List = emptyList(), initialSpaceRoomList: SpaceRoomList.PaginationStatus = SpaceRoomList.PaginationStatus.Loading, private val paginateResult: () -> Result = { lambdaError() }, + private val resetResult: () -> Result = { lambdaError() }, ) : SpaceRoomList { private val currentSpaceMutableStateFlow: MutableStateFlow> = MutableStateFlow(Optional.ofNullable(initialSpaceFlowValue)) override val currentSpaceFlow: StateFlow> = currentSpaceMutableStateFlow.asStateFlow() @@ -34,7 +35,8 @@ class FakeSpaceRoomList( currentSpaceMutableStateFlow.value = Optional.ofNullable(value) } - private val _spaceRoomsFlow: MutableStateFlow> = MutableStateFlow(initialSpaceRoomsValue) + private val _spaceRoomsFlow = MutableStateFlow>(initialSpaceRoomsValue) + override val spaceRoomsFlow: Flow> = _spaceRoomsFlow.asStateFlow() fun emitSpaceRooms(value: List) { @@ -52,6 +54,10 @@ class FakeSpaceRoomList( paginateResult() } + override suspend fun reset(): Result = simulateLongTask { + resetResult() + } + override fun destroy() { // No op }