Merge pull request #6099 from element-hq/feature/fga/space_management_iteration

Improve space management with pagination and partial failure handling
This commit is contained in:
ganfra
2026-01-29 10:57:14 +01:00
committed by GitHub
18 changed files with 339 additions and 107 deletions

View File

@@ -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 -> {

View File

@@ -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
}

View File

@@ -40,7 +40,7 @@ class AddRoomToSpaceNode(
val state by stateFlow.collectAsState()
AddRoomToSpaceView(
state = state,
onBackClick = ::navigateUp,
onBackClick = callback::onFinish,
onRoomsAdded = callback::onFinish,
modifier = modifier
)

View File

@@ -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<SelectRoomInfo> by remember { mutableStateOf(persistentListOf()) }
var searchQuery = rememberTextFieldState()
val searchQuery = rememberTextFieldState()
var isSearchActive by remember { mutableStateOf(false) }
val saveAction = remember { mutableStateOf<AsyncAction<Unit>>(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<State<SearchBarResultState<ImmutableList<SelectRoomInfo>>>> {
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<ImmutableList<SelectRoomInfo>>()
else -> SearchBarResultState.Initial<ImmutableList<SelectRoomInfo>>()
}
}
}
@@ -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<SelectRoomInfo>,
addAction: MutableState<AsyncAction<Unit>>,
dataSource: AddRoomToSpaceSearchDataSource,
saveAction: MutableState<AsyncAction<Unit>>,
onPartialSuccess: (Set<RoomId>) -> Unit,
) = launch {
addAction.runUpdatingState {
val results = selectedRooms.map { selectedRoom ->
saveAction.runUpdatingState {
val spaceId = spaceRoomList.spaceId
val successfullyAdded = mutableSetOf<RoomId>()
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)
}
}

View File

@@ -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<RoomId>) -> 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<Set<RoomId>>(emptySet())
/**
* Marks rooms as added to the space (for partial success handling).
*/
fun markAsAdded(roomIds: Set<RoomId>) {
addedRoomIdsFlow.value += roomIds
}
private val filterRoomPredicate: (RoomInfo, Set<RoomId>, Set<RoomId>) -> 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<ImmutableList<SelectRoomInfo>> = 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<ImmutableList<SelectRoomInfo>> = spaceChildrenFlow.map { childIds ->
val suggestions: Flow<ImmutableList<SelectRoomInfo>> = 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)

View File

@@ -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) })
}
}
}

View File

@@ -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(

View File

@@ -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<Set<RoomId>>(emptySet()) }
var removeRoomsAction by remember { mutableStateOf<AsyncAction<Unit>>(AsyncAction.Uninitialized) }
// Track locally removed rooms for partial failure cases
var removedRoomIds by remember { mutableStateOf<Set<RoomId>>(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<RoomId>()
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()
}
}

View File

@@ -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(

View File

@@ -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<Result<Unit>>(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<Unit>> { Result.success(Unit) }
val spaceRoomList = FakeSpaceRoomList(
paginateResult = { Result.success(Unit) },
resetResult = resetResult,
)
// Room 1 succeeds, Room 2 fails
val addChildToSpaceResult = lambdaRecorder<RoomId, RoomId, Result<Unit>> { _, 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<SelectRoomInfo> = listOf(
aSelectRoomInfo(roomId = A_ROOM_ID, name = "Room 1"),
aSelectRoomInfo(roomId = A_ROOM_ID_2, name = "Room 2"),
).toImmutableList()

View File

@@ -32,16 +32,19 @@ class AddRoomToSpaceViewTest {
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
@Test
fun `clicking back when search inactive invokes onBackClick`() {
fun `clicking back when search inactive emits Dismiss and invokes onBackClick`() {
val eventsRecorder = EventsRecorder<AddRoomToSpaceEvent>()
ensureCalledOnce {
rule.setAddRoomToSpaceView(
anAddRoomToSpaceState(
isSearchActive = false,
eventSink = eventsRecorder,
),
onBackClick = it,
)
rule.pressBack()
}
eventsRecorder.assertSingle(AddRoomToSpaceEvent.Dismiss)
}
@Test

View File

@@ -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<Unit>> {
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<Unit>> {
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<Result<Unit>>(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<RoomId, RoomId, Result<Unit>> { _, _ ->
Result.success(Unit)
}
val resetResult = lambdaRecorder<Result<Unit>> { 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<RoomId, RoomId, Result<Unit>> { _, childId ->
if (childId == A_ROOM_ID_2) {
Result.failure(AN_EXCEPTION)
} else {
Result.success(Unit)
}
}
val resetResult = lambdaRecorder<Result<Unit>> { 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<RoomId, RoomId, Result<Unit>> { _, _ ->
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<RoomId, RoomId, Result<Unit>> { _, 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,

View File

@@ -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<Optional<SpaceRoom>>
val spaceRoomsFlow: Flow<List<SpaceRoom>>
val paginationStatusFlow: StateFlow<PaginationStatus>
suspend fun paginate(): Result<Unit>
suspend fun reset(): Result<Unit>
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)
}

View File

@@ -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<Unit> {
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()

View File

@@ -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,

View File

@@ -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<SpaceRoom> = { lambdaError() },
) : SpaceRoomList(NoHandle) {
@@ -31,6 +32,10 @@ class FakeFfiSpaceRoomList(
paginateResult()
}
override suspend fun reset() = simulateLongTask {
resetResult()
}
override fun paginationState(): SpaceRoomListPaginationState {
return paginationStateResult()
}

View File

@@ -94,7 +94,7 @@ class RustSpaceRoomListTest {
spaceRoomMapper: SpaceRoomMapper = SpaceRoomMapper(),
): RustSpaceRoomList {
return RustSpaceRoomList(
roomId = roomId,
spaceId = roomId,
innerProvider = innerProvider,
coroutineScope = backgroundScope,
spaceRoomMapper = spaceRoomMapper,

View File

@@ -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<SpaceRoom> = emptyList(),
initialSpaceRoomList: SpaceRoomList.PaginationStatus = SpaceRoomList.PaginationStatus.Loading,
private val paginateResult: () -> Result<Unit> = { lambdaError() },
private val resetResult: () -> Result<Unit> = { lambdaError() },
) : SpaceRoomList {
private val currentSpaceMutableStateFlow: MutableStateFlow<Optional<SpaceRoom>> = MutableStateFlow(Optional.ofNullable(initialSpaceFlowValue))
override val currentSpaceFlow: StateFlow<Optional<SpaceRoom>> = currentSpaceMutableStateFlow.asStateFlow()
@@ -34,7 +35,8 @@ class FakeSpaceRoomList(
currentSpaceMutableStateFlow.value = Optional.ofNullable(value)
}
private val _spaceRoomsFlow: MutableStateFlow<List<SpaceRoom>> = MutableStateFlow(initialSpaceRoomsValue)
private val _spaceRoomsFlow = MutableStateFlow<List<SpaceRoom>>(initialSpaceRoomsValue)
override val spaceRoomsFlow: Flow<List<SpaceRoom>> = _spaceRoomsFlow.asStateFlow()
fun emitSpaceRooms(value: List<SpaceRoom>) {
@@ -52,6 +54,10 @@ class FakeSpaceRoomList(
paginateResult()
}
override suspend fun reset(): Result<Unit> = simulateLongTask {
resetResult()
}
override fun destroy() {
// No op
}