From 1e25938ef7509ad53d366b7092c5d3c5ef4ee68c Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 26 Jan 2026 21:39:00 +0100 Subject: [PATCH] Iterate on manage space rooms, but not happy with the reset method. --- .../features/space/impl/SpaceFlowNode.kt | 5 ++ .../impl/addroom/AddRoomToSpacePresenter.kt | 39 +++++++---- .../addroom/AddRoomToSpaceSearchDataSource.kt | 43 +++++++++---- .../space/impl/addroom/AddRoomToSpaceView.kt | 18 +++--- .../features/space/impl/root/SpaceNode.kt | 2 +- .../space/impl/root/SpacePresenter.kt | 38 +++++------ .../features/space/impl/root/SpaceView.kt | 22 ++++--- .../addroom/AddRoomToSpacePresenterTest.kt | 11 ++++ .../impl/addroom/AddRoomToSpaceViewTest.kt | 4 +- .../space/impl/root/SpacePresenterTest.kt | 64 +++++++++++-------- .../matrix/api/spaces/SpaceRoomList.kt | 27 +++----- .../matrix/impl/spaces/RustSpaceRoomList.kt | 5 +- .../matrix/impl/spaces/RustSpaceService.kt | 2 +- .../impl/spaces/RustSpaceRoomListTest.kt | 2 +- .../matrix/test/spaces/FakeSpaceRoomList.kt | 5 +- 15 files changed, 170 insertions(+), 117 deletions(-) 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..0123beb515 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() } 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..0a3dd07418 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,8 +24,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.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.api.spaces.resetAndWaitForFullReload import io.element.android.libraries.matrix.ui.model.SelectRoomInfo import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf @@ -35,6 +36,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.launch +import kotlin.time.Duration.Companion.seconds @Inject class AddRoomToSpacePresenter( @@ -45,7 +47,7 @@ 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) } @@ -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,7 +93,11 @@ class AddRoomToSpacePresenter( AddRoomToSpaceEvent.Save -> { coroutineScope.addRoomsToSpace( selectedRooms = selectedRooms, - addAction = saveAction, + dataSource = dataSource, + saveAction = saveAction, + onPartialSuccess = { successfullyAdded -> + selectedRooms = selectedRooms.filterNot { it.roomId in successfullyAdded }.toImmutableList() + }, ) } AddRoomToSpaceEvent.ResetSaveAction -> { @@ -113,21 +119,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..b582e07fdf 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 @@ -16,19 +16,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.api.room.RoomInfo import io.element.android.libraries.matrix.api.room.isDm +import io.element.android.libraries.matrix.ui.model.SelectRoomInfo +import io.element.android.libraries.matrix.ui.model.toSelectRoomInfo import io.element.android.libraries.matrix.api.room.recent.getRecentlyVisitedRoomInfoFlow import io.element.android.libraries.matrix.api.roomlist.RoomList import io.element.android.libraries.matrix.api.roomlist.RoomListFilter import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.api.roomlist.loadAllIncrementally import io.element.android.libraries.matrix.api.spaces.SpaceRoomList -import io.element.android.libraries.matrix.ui.model.SelectRoomInfo -import io.element.android.libraries.matrix.ui.model.toSelectRoomInfo import kotlinx.collections.immutable.ImmutableList 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 failure cases + private val addedRoomIds = MutableStateFlow>(emptySet()) + + /** + * Marks rooms as added to the space (for partial failure handling). + * These rooms will be filtered out from search results and suggestions. + */ + fun markAsAdded(roomIds: Set) { + addedRoomIds.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 -> + addedRoomIds, + ) { 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, + addedRoomIds, + ) { 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..a238e10196 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,8 +58,8 @@ 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() { @@ -114,18 +114,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 +142,7 @@ fun AddRoomToSpaceView( if (state.selectedRooms.isNotEmpty()) { SelectedRoomsRow( selectedRooms = state.selectedRooms, - onRemoveRoom = ::onRoomRemoved, + onRemoveRoom = ::onRoomToggled, modifier = Modifier.padding(vertical = 16.dp) ) } @@ -159,7 +159,7 @@ fun AddRoomToSpaceView( RoomListItem( roomInfo = roomInfo, isSelected = state.selectedRooms.any { it.roomId == roomInfo.roomId }, - onToggle = { state.eventSink(AddRoomToSpaceEvent.ToggleRoom(it)) } + onToggle = ::onRoomToggled ) } } @@ -205,8 +205,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..5470ad5d5d 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 @@ -49,6 +49,7 @@ import kotlinx.collections.immutable.toImmutableSet import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch @@ -69,7 +70,6 @@ class SpacePresenter( @Composable override fun present(): SpaceState { LaunchedEffect(Unit) { - paginate() spaceRoomList.spaceRoomsFlow.collect { children = it.toImmutableList() } } @@ -111,21 +111,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() + } } } @@ -141,7 +138,8 @@ class SpacePresenter( 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) } @@ -186,7 +184,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 +194,18 @@ 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() + // Reset the space room list to see the updates. + spaceRoomList.reset() } } } @@ -246,8 +246,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..2b6eacf17e 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,11 +17,16 @@ 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 kotlinx.collections.immutable.ImmutableList +import kotlinx.collections.immutable.toImmutableList import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.test import io.element.android.tests.testutils.testCoroutineDispatchers @@ -276,6 +281,7 @@ class AddRoomToSpacePresenterTest { private fun TestScope.createAddRoomToSpacePresenter( spaceRoomList: FakeSpaceRoomList = FakeSpaceRoomList( paginateResult = { Result.success(Unit) }, + resetResult = { Result.success(Unit) }, ), spaceService: FakeSpaceService = FakeSpaceService( addChildToSpaceResult = { _, _ -> Result.success(Unit) }, @@ -301,3 +307,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..37870a7576 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 @@ -20,7 +20,7 @@ import io.element.android.tests.testutils.EventsRecorder import io.element.android.tests.testutils.clickOn import io.element.android.tests.testutils.ensureCalledOnce import io.element.android.tests.testutils.pressBack -import kotlinx.collections.immutable.toImmutableList +import kotlinx.collections.immutable.toPersistentList import org.junit.Rule import org.junit.Test import org.junit.rules.TestRule @@ -62,7 +62,7 @@ class AddRoomToSpaceViewTest { val eventsRecorder = EventsRecorder() rule.setAddRoomToSpaceView( anAddRoomToSpaceState( - selectedRooms = aSelectRoomInfoList().take(1).toImmutableList(), + selectedRooms = aSelectRoomInfoList().take(1).toPersistentList(), eventSink = eventsRecorder, ), ) 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..b3a302a914 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 @@ -30,6 +30,7 @@ import io.element.android.libraries.matrix.api.room.CurrentUserMembership import io.element.android.libraries.matrix.api.room.RoomType import io.element.android.libraries.matrix.api.room.join.JoinRoom import io.element.android.libraries.matrix.api.spaces.SpaceRoomList +import io.element.android.libraries.matrix.api.spaces.loadAllIncrementally 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 @@ -42,6 +43,7 @@ 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 @@ -56,10 +58,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 +73,6 @@ class SpacePresenterTest { assertThat(state.acceptDeclineInviteState).isEqualTo(anAcceptDeclineInviteState()) assertThat(state.topicViewerState).isEqualTo(TopicViewerState.Hidden) assertThat(state.canAccessSpaceSettings).isFalse() - advanceUntilIdle() - paginateResult.assertions().isCalledOnce() } } @@ -105,19 +104,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 +193,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 +207,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), @@ -389,6 +385,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 +393,7 @@ class SpacePresenterTest { val fakeSpaceRoomList = FakeSpaceRoomList( initialSpaceRoomsValue = listOf(aRoom), paginateResult = { Result.success(Unit) }, + resetResult = resetResult, ) val presenter = createSpacePresenter( spaceRoomList = fakeSpaceRoomList, @@ -416,8 +414,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 +463,7 @@ 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) } } @@ -501,10 +499,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 +513,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 +536,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 +581,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 4723ff0f93..528a75f563 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 @@ -25,38 +25,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 -> - when (paginationStatus) { - is SpaceRoomList.PaginationStatus.Idle -> { - if (paginationStatus.hasMoreToLoad) { - paginate() - } - } - SpaceRoomList.PaginationStatus.Loading -> Unit + if (paginationStatus is SpaceRoomList.PaginationStatus.Idle && paginationStatus.hasMoreToLoad) { + paginate() } } .launchIn(coroutineScope) } - -suspend fun SpaceRoomList.resetAndWaitForFullReload(timeout: Duration) { - reset() - withTimeoutOrNull(timeout) { - paginationStatusFlow.first { status -> - status is SpaceRoomList.PaginationStatus.Idle && !status.hasMoreToLoad - } - } -} 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 31f1a0b0d1..549fde2d5d 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 @@ -17,6 +17,7 @@ import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.launchIn @@ -29,7 +30,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, @@ -89,7 +90,7 @@ class RustSpaceRoomList( @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/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 02020de0de..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,7 +21,7 @@ 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, @@ -35,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) {