From b60cfaccb688225ceb615fe7e979310ac0c16497 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 27 Jan 2026 14:26:57 +0100 Subject: [PATCH] Call spaceRoomList.reset when exiting add/remove room flow --- .../space/impl/addroom/AddRoomToSpaceEvent.kt | 1 + .../space/impl/addroom/AddRoomToSpaceNode.kt | 2 +- .../impl/addroom/AddRoomToSpacePresenter.kt | 11 +++- .../space/impl/addroom/AddRoomToSpaceView.kt | 1 + .../space/impl/root/SpacePresenter.kt | 19 +++--- .../addroom/AddRoomToSpacePresenterTest.kt | 62 +++++++++++++++++++ .../impl/addroom/AddRoomToSpaceViewTest.kt | 5 +- .../space/impl/root/SpacePresenterTest.kt | 62 ++++++++++++++++++- 8 files changed, 151 insertions(+), 12 deletions(-) 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 0a3dd07418..89e78e86a1 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 @@ -27,7 +27,6 @@ import io.element.android.libraries.designsystem.theme.components.SearchBarResul 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 @@ -50,6 +49,8 @@ class AddRoomToSpacePresenter( 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) } @@ -96,6 +97,9 @@ class AddRoomToSpacePresenter( dataSource = dataSource, saveAction = saveAction, onPartialSuccess = { successfullyAdded -> + if (successfullyAdded.isNotEmpty()) { + hasAddedRooms = true + } selectedRooms = selectedRooms.filterNot { it.roomId in successfullyAdded }.toImmutableList() }, ) @@ -103,6 +107,11 @@ class AddRoomToSpacePresenter( AddRoomToSpaceEvent.ResetSaveAction -> { saveAction.value = AsyncAction.Uninitialized } + AddRoomToSpaceEvent.Dismiss -> { + if (hasAddedRooms) { + coroutineScope.launch { spaceRoomList.reset() } + } + } } } 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 a238e10196..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 @@ -66,6 +66,7 @@ fun AddRoomToSpaceView( if (state.isSearchActive) { state.eventSink(AddRoomToSpaceEvent.OnSearchActiveChanged(false)) } else { + state.eventSink(AddRoomToSpaceEvent.Dismiss) onBackClick() } } 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 5470ad5d5d..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 @@ -49,7 +49,6 @@ 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 @@ -136,6 +135,16 @@ 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) { // SpaceRoomList is loaded automatically as backend is really slow. Event is kept for future. @@ -168,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) { @@ -202,10 +210,7 @@ class SpacePresenter( 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() + exitManageMode(shouldReset = true) } } } 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 2b6eacf17e..8d99e2c550 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 @@ -27,6 +27,7 @@ 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.assert import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.test import io.element.android.tests.testutils.testCoroutineDispatchers @@ -278,6 +279,67 @@ 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) }, 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 37870a7576..6e82a5f7d7 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 b3a302a914..b655f2f022 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 @@ -350,16 +350,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() } } @@ -467,6 +475,56 @@ class SpacePresenterTest { } } + @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() + } + } + @Test fun `present - children filtered in manage mode shows only rooms`() = runTest { val aRoom = aSpaceRoom(