From 993cf838a05b80bf0ee04f1a2c1fee5d35196dec Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 9 Jan 2026 12:00:54 +0100 Subject: [PATCH] Refactor space selection to use SpaceSelectionStateHolder Move authorized space selection state to a shared StateHolder scoped to RoomScope. This simplifies communication between SecurityAndPrivacy and ManageAuthorizedSpaces nodes by replacing the complex coroutine-based parent-child coordination with a reactive state flow pattern. --- .../impl/SecurityAndPrivacyFlowNode.kt | 23 +---- .../impl/SecurityAndPrivacyNavigator.kt | 6 +- .../ManageAuthorizedSpacesEvent.kt | 2 +- .../ManageAuthorizedSpacesNode.kt | 11 --- .../ManageAuthorizedSpacesPresenter.kt | 39 ++++---- .../ManageAuthorizedSpacesState.kt | 12 +-- .../ManageAuthorizedSpacesStateProvider.kt | 25 ++--- .../ManageAuthorizedSpacesView.kt | 29 +++--- .../SpaceSelectionState.kt | 64 ++++++++++++ .../impl/root/SecurityAndPrivacyNode.kt | 16 --- .../impl/root/SecurityAndPrivacyPresenter.kt | 98 ++++++++++++++++--- .../impl/root/SecurityAndPrivacyState.kt | 15 --- .../impl/FakeSecurityAndPrivacyNavigator.kt | 6 +- .../impl/SecurityAndPrivacyFlowNodeTest.kt | 19 +--- .../ManageAuthorizedSpacesPresenterTest.kt | 85 +++++++++++----- .../ManageAuthorizedSpacesViewTest.kt | 33 +++---- .../root/SecurityAndPrivacyPresenterTest.kt | 78 ++------------- 17 files changed, 283 insertions(+), 278 deletions(-) create mode 100644 features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/SpaceSelectionState.kt diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyFlowNode.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyFlowNode.kt index 4628974295..c306264773 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyFlowNode.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyFlowNode.kt @@ -13,14 +13,12 @@ import androidx.annotation.VisibleForTesting import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.lifecycle.Lifecycle -import androidx.lifecycle.coroutineScope import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.plugin.Plugin import com.bumble.appyx.navmodel.backstack.BackStack -import com.bumble.appyx.navmodel.backstack.activeElement import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedInject import io.element.android.annotations.ContributesNode @@ -36,12 +34,10 @@ import io.element.android.libraries.architecture.createNode import io.element.android.libraries.di.RoomScope import io.element.android.libraries.matrix.api.room.JoinedRoom import io.element.android.libraries.matrix.api.room.powerlevels.use -import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import kotlinx.parcelize.Parcelize @ContributesNode(RoomScope::class) @@ -66,7 +62,7 @@ class SecurityAndPrivacyFlowNode( data object EditRoomAddress : NavTarget @Parcelize - data class ManageAuthorizedSpaces(val forKnockRestricted: Boolean = false) : NavTarget + data object ManageAuthorizedSpaces : NavTarget } private val callback: SecurityAndPrivacyEntryPoint.Callback = callback() @@ -90,21 +86,6 @@ class SecurityAndPrivacyFlowNode( callback.onDone() } } - whenChildrenAttached { - commonLifecycle: Lifecycle, - securityAndPrivacyNode: SecurityAndPrivacyNode, - manageAuthorizedSpacesNode: ManageAuthorizedSpacesNode - -> - commonLifecycle.coroutineScope.launch { - val authorizedSpacesData = securityAndPrivacyNode.getAuthorizedSpacesData() - val selectedSpaces = manageAuthorizedSpacesNode.waitForCompletion(authorizedSpacesData) - val forKnock = (backstack.activeElement as? NavTarget.ManageAuthorizedSpaces)?.forKnockRestricted ?: false - withContext(NonCancellable) { - navigator.closeManageAuthorizedSpaces() - securityAndPrivacyNode.onAuthorizedSpacesSelected(selectedSpaces, forKnock = forKnock) - } - } - } } override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { @@ -115,7 +96,7 @@ class SecurityAndPrivacyFlowNode( NavTarget.EditRoomAddress -> { createNode(buildContext, plugins = listOf(navigator)) } - is NavTarget.ManageAuthorizedSpaces -> { + NavTarget.ManageAuthorizedSpaces -> { createNode(buildContext, plugins = listOf(navigator)) } } diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyNavigator.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyNavigator.kt index 0944ae92bd..274bf0b823 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyNavigator.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyNavigator.kt @@ -18,7 +18,7 @@ interface SecurityAndPrivacyNavigator : Plugin { fun onDone() fun openEditRoomAddress() fun closeEditRoomAddress() - fun openManageAuthorizedSpaces(forKnockRestricted: Boolean) + fun openManageAuthorizedSpaces() fun closeManageAuthorizedSpaces() } @@ -38,8 +38,8 @@ class BackstackSecurityAndPrivacyNavigator( backStack.pop() } - override fun openManageAuthorizedSpaces(forKnockRestricted: Boolean) { - backStack.push(SecurityAndPrivacyFlowNode.NavTarget.ManageAuthorizedSpaces(forKnockRestricted)) + override fun openManageAuthorizedSpaces() { + backStack.push(SecurityAndPrivacyFlowNode.NavTarget.ManageAuthorizedSpaces) } override fun closeManageAuthorizedSpaces() { diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesEvent.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesEvent.kt index 47abe2fbce..37d70b65f5 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesEvent.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesEvent.kt @@ -11,7 +11,7 @@ package io.element.android.features.securityandprivacy.impl.manageauthorizedspac import io.element.android.libraries.matrix.api.core.RoomId sealed interface ManageAuthorizedSpacesEvent { - data class SetData(val data: AuthorizedSpacesSelection) : ManageAuthorizedSpacesEvent + data object Cancel : ManageAuthorizedSpacesEvent data object Done : ManageAuthorizedSpacesEvent data class ToggleSpace(val roomId: RoomId) : ManageAuthorizedSpacesEvent } diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesNode.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesNode.kt index 08c757b87d..709f2189bc 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesNode.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesNode.kt @@ -19,12 +19,8 @@ import com.bumble.appyx.core.plugin.plugins import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedInject import io.element.android.annotations.ContributesNode -import io.element.android.features.securityandprivacy.impl.SecurityAndPrivacyNavigator import io.element.android.libraries.architecture.appyx.launchMolecule import io.element.android.libraries.di.RoomScope -import io.element.android.libraries.matrix.api.core.RoomId -import kotlinx.collections.immutable.ImmutableList -import kotlinx.coroutines.flow.first @ContributesNode(RoomScope::class) @AssistedInject @@ -33,20 +29,13 @@ class ManageAuthorizedSpacesNode( @Assisted plugins: List, presenter: ManageAuthorizedSpacesPresenter, ) : Node(buildContext, plugins = plugins) { - private val navigator = plugins().first() private val stateFlow = launchMolecule { presenter.present() } - suspend fun waitForCompletion(data: AuthorizedSpacesSelection): ImmutableList { - stateFlow.value.eventSink(ManageAuthorizedSpacesEvent.SetData(data)) - return stateFlow.first { it.isSelectionComplete }.selectedIds - } - @Composable override fun View(modifier: Modifier) { val state by stateFlow.collectAsState() ManageAuthorizedSpacesView( state = state, - onBackClick = { navigator.closeManageAuthorizedSpaces() }, modifier = modifier ) } diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesPresenter.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesPresenter.kt index aa2251408f..5922e910a2 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesPresenter.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesPresenter.kt @@ -9,46 +9,43 @@ package io.element.android.features.securityandprivacy.impl.manageauthorizedspaces import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue import dev.zacsweers.metro.Inject import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.matrix.api.core.RoomId -import kotlinx.collections.immutable.ImmutableList -import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList @Inject -class ManageAuthorizedSpacesPresenter : Presenter { +class ManageAuthorizedSpacesPresenter( + private val spaceSelectionStateHolder: SpaceSelectionStateHolder, +) : Presenter { @Composable override fun present(): ManageAuthorizedSpacesState { - var selectedIds: ImmutableList by remember { mutableStateOf(persistentListOf()) } - var spacesSelection by remember { mutableStateOf(AuthorizedSpacesSelection()) } - var isSelectionComplete by remember { mutableStateOf(false) } - + val spaceSelectionState by spaceSelectionStateHolder.state.collectAsState() fun handleEvent(event: ManageAuthorizedSpacesEvent) { when (event) { - ManageAuthorizedSpacesEvent.Done -> isSelectionComplete = true is ManageAuthorizedSpacesEvent.ToggleSpace -> { - selectedIds = if (selectedIds.contains(event.roomId)) { - selectedIds.minus(event.roomId).toImmutableList() + val currentSelectedIds = spaceSelectionState.selectedSpaceIds + val newSelectedIds = if (currentSelectedIds.contains(event.roomId)) { + currentSelectedIds.minus(event.roomId).toImmutableList() } else { - selectedIds.plus(event.roomId).toImmutableList() + currentSelectedIds.plus(event.roomId).toImmutableList() } + spaceSelectionStateHolder.updateSelectedSpaceIds(newSelectedIds) } - is ManageAuthorizedSpacesEvent.SetData -> { - spacesSelection = event.data - selectedIds = event.data.initialSelectedIds + ManageAuthorizedSpacesEvent.Done -> { + spaceSelectionStateHolder.setCompletion(SpaceSelectionState.Completion.Completed) + } + ManageAuthorizedSpacesEvent.Cancel -> { + spaceSelectionStateHolder.setCompletion(SpaceSelectionState.Completion.Cancelled) } } } return ManageAuthorizedSpacesState( - selection = spacesSelection, - selectedIds = selectedIds, - isSelectionComplete = isSelectionComplete, + selectableSpaces = spaceSelectionState.selectableSpaces, + unknownSpaceIds = spaceSelectionState.unknownSpaceIds, + selectedIds = spaceSelectionState.selectedSpaceIds, eventSink = ::handleEvent, ) } diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesState.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesState.kt index 291cd7c760..b729fc12fa 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesState.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesState.kt @@ -11,19 +11,13 @@ package io.element.android.features.securityandprivacy.impl.manageauthorizedspac import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.spaces.SpaceRoom import kotlinx.collections.immutable.ImmutableList -import kotlinx.collections.immutable.persistentListOf +import kotlinx.collections.immutable.ImmutableSet data class ManageAuthorizedSpacesState( - val selection: AuthorizedSpacesSelection, + val selectableSpaces: ImmutableSet, + val unknownSpaceIds: ImmutableList, val selectedIds: ImmutableList, - val isSelectionComplete: Boolean, val eventSink: (ManageAuthorizedSpacesEvent) -> Unit ) { val isDoneButtonEnabled = selectedIds.isNotEmpty() } - -data class AuthorizedSpacesSelection( - val joinedSpaces: ImmutableList = persistentListOf(), - val unknownSpaceIds: ImmutableList = persistentListOf(), - val initialSelectedIds: ImmutableList = persistentListOf() -) diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesStateProvider.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesStateProvider.kt index d818e31317..8c55af54e5 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesStateProvider.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesStateProvider.kt @@ -14,21 +14,17 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.spaces.SpaceRoom import io.element.android.libraries.previewutils.room.aSpaceRoom import kotlinx.collections.immutable.toImmutableList +import kotlinx.collections.immutable.toImmutableSet open class ManageAuthorizedSpacesStateProvider : PreviewParameterProvider { override val values: Sequence get() = sequenceOf( aManageAuthorizedSpacesState(), aManageAuthorizedSpacesState( - authorizedSpacesSelection = anAuthorizedSpaceSelection( - unknownSpaceIds = listOf(aRoomId(99)) - ) + unknownSpaceIds = listOf(aRoomId(99)) ), aManageAuthorizedSpacesState( selectedIds = listOf(aRoomId(1), aRoomId(3)), - authorizedSpacesSelection = anAuthorizedSpaceSelection( - initialSelectedIds = listOf(aRoomId(1)), - ), ), ) } @@ -49,23 +45,14 @@ private fun aSpaceRoomList(count: Int): List { } } -fun anAuthorizedSpaceSelection( - joinedSpaces: List = aSpaceRoomList(5), +fun aManageAuthorizedSpacesState( + selectableSpaces: List = aSpaceRoomList(5), unknownSpaceIds: List = emptyList(), - initialSelectedIds: List = emptyList(), -) = AuthorizedSpacesSelection( - joinedSpaces = joinedSpaces.toImmutableList(), - unknownSpaceIds = unknownSpaceIds.toImmutableList(), - initialSelectedIds = initialSelectedIds.toImmutableList(), -) - -private fun aManageAuthorizedSpacesState( - authorizedSpacesSelection: AuthorizedSpacesSelection = anAuthorizedSpaceSelection(), selectedIds: List = emptyList(), eventSink: (ManageAuthorizedSpacesEvent) -> Unit = {}, ) = ManageAuthorizedSpacesState( - selection = authorizedSpacesSelection, + selectableSpaces = selectableSpaces.toImmutableSet(), + unknownSpaceIds = unknownSpaceIds.toImmutableList(), selectedIds = selectedIds.toImmutableList(), - isSelectionComplete = false, eventSink = eventSink, ) diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesView.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesView.kt index 022010f267..9b839b80db 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesView.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesView.kt @@ -8,6 +8,7 @@ package io.element.android.features.securityandprivacy.impl.manageauthorizedspaces +import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListScope @@ -42,17 +43,24 @@ import io.element.android.libraries.ui.strings.CommonStrings @Composable fun ManageAuthorizedSpacesView( state: ManageAuthorizedSpacesState, - onBackClick: () -> Unit, modifier: Modifier = Modifier, ) { + fun onCancel() { + state.eventSink(ManageAuthorizedSpacesEvent.Cancel) + } + + fun onDone() { + state.eventSink(ManageAuthorizedSpacesEvent.Done) + } + + BackHandler(onBack = ::onCancel) + Scaffold( modifier = modifier, topBar = { ManageAuthorizedSpacesTopBar( - onBackClick = onBackClick, - onDoneClick = { - state.eventSink(ManageAuthorizedSpacesEvent.Done) - }, + onBackClick = ::onCancel, + onDoneClick = ::onDone, isDoneButtonEnabled = state.isDoneButtonEnabled ) } @@ -67,7 +75,7 @@ fun ManageAuthorizedSpacesView( hasDivider = false, ) } - items(items = state.selection.joinedSpaces) { space -> + items(items = state.selectableSpaces.toList()) { space -> CheckableSpaceListItem( headlineText = space.displayName, supportingText = space.canonicalAlias?.value, @@ -80,14 +88,14 @@ fun ManageAuthorizedSpacesView( } ) } - if (state.selection.unknownSpaceIds.isNotEmpty()) { + if (state.unknownSpaceIds.isNotEmpty()) { item { ListSectionHeader( title = stringResource(R.string.screen_manage_authorized_spaces_unknown_spaces_section_title), hasDivider = true, ) } - items(items = state.selection.unknownSpaceIds) { + items(items = state.unknownSpaceIds) { CheckableSpaceListItem( headlineText = stringResource(R.string.screen_manage_authorized_spaces_unknown_space), supportingText = it.value, @@ -185,8 +193,5 @@ private fun ManageAuthorizedSpacesTopBar( internal fun ManageAuthorizedSpacesViewPreview( @PreviewParameter(ManageAuthorizedSpacesStateProvider::class) state: ManageAuthorizedSpacesState ) = ElementPreview { - ManageAuthorizedSpacesView( - state = state, - onBackClick = {}, - ) + ManageAuthorizedSpacesView(state = state) } diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/SpaceSelectionState.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/SpaceSelectionState.kt new file mode 100644 index 0000000000..f24d1621b0 --- /dev/null +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/SpaceSelectionState.kt @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2025 Element Creations Ltd. + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.securityandprivacy.impl.manageauthorizedspaces + +import dev.zacsweers.metro.Inject +import dev.zacsweers.metro.SingleIn +import io.element.android.libraries.di.RoomScope +import io.element.android.libraries.matrix.api.core.RoomId +import io.element.android.libraries.matrix.api.spaces.SpaceRoom +import kotlinx.collections.immutable.ImmutableList +import kotlinx.collections.immutable.ImmutableSet +import kotlinx.collections.immutable.persistentListOf +import kotlinx.collections.immutable.persistentSetOf +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update + +data class SpaceSelectionState( + val selectableSpaces: ImmutableSet, + val unknownSpaceIds: ImmutableList, + val selectedSpaceIds: ImmutableList, + val completion: Completion, +) { + enum class Completion { + Initial, + Completed, + Cancelled, + } + + companion object { + val INITIAL = SpaceSelectionState( + selectableSpaces = persistentSetOf(), + unknownSpaceIds = persistentListOf(), + selectedSpaceIds = persistentListOf(), + completion = Completion.Initial, + ) + } +} + +@Inject +@SingleIn(RoomScope::class) +class SpaceSelectionStateHolder { + private val _state = MutableStateFlow(SpaceSelectionState.INITIAL) + val state: StateFlow = _state.asStateFlow() + + fun update(transform: (SpaceSelectionState) -> SpaceSelectionState) { + _state.update(transform) + } + + fun updateSelectedSpaceIds(selectedSpaceIds: ImmutableList) { + update { it.copy(selectedSpaceIds = selectedSpaceIds) } + } + + fun setCompletion(completion: SpaceSelectionState.Completion) { + update { it.copy(completion = completion) } + } +} diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyNode.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyNode.kt index 815b7bd35e..d5fb72e72e 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyNode.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyNode.kt @@ -23,12 +23,9 @@ import dev.zacsweers.metro.AssistedInject import io.element.android.annotations.ContributesNode import io.element.android.compound.theme.ElementTheme import io.element.android.features.securityandprivacy.impl.SecurityAndPrivacyNavigator -import io.element.android.features.securityandprivacy.impl.manageauthorizedspaces.AuthorizedSpacesSelection import io.element.android.libraries.androidutils.browser.openUrlInChromeCustomTab import io.element.android.libraries.architecture.appyx.launchMolecule import io.element.android.libraries.di.RoomScope -import io.element.android.libraries.matrix.api.core.RoomId -import kotlinx.collections.immutable.ImmutableList @ContributesNode(RoomScope::class) @AssistedInject @@ -46,19 +43,6 @@ class SecurityAndPrivacyNode( activity.openUrlInChromeCustomTab(null, darkTheme, url) } - fun getAuthorizedSpacesData(): AuthorizedSpacesSelection { - return stateFlow.value.getAuthorizedSpacesSelection() - } - - fun onAuthorizedSpacesSelected(selectedSpaces: ImmutableList, forKnock: Boolean) { - val roomAccess = if (forKnock) { - SecurityAndPrivacyRoomAccess.AskToJoinWithSpaceMember(selectedSpaces) - } else { - SecurityAndPrivacyRoomAccess.SpaceMember(selectedSpaces) - } - stateFlow.value.eventSink(SecurityAndPrivacyEvent.ChangeRoomAccess(roomAccess)) - } - @Composable override fun View(modifier: Modifier) { val activity = requireNotNull(LocalActivity.current) diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyPresenter.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyPresenter.kt index f80b96ca00..09873b6b86 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyPresenter.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyPresenter.kt @@ -26,6 +26,8 @@ import io.element.android.features.securityandprivacy.api.SecurityAndPrivacyPerm import io.element.android.features.securityandprivacy.api.securityAndPrivacyPermissions import io.element.android.features.securityandprivacy.impl.SecurityAndPrivacyNavigator import io.element.android.features.securityandprivacy.impl.editroomaddress.matchesServer +import io.element.android.features.securityandprivacy.impl.manageauthorizedspaces.SpaceSelectionState +import io.element.android.features.securityandprivacy.impl.manageauthorizedspaces.SpaceSelectionStateHolder import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.Presenter @@ -51,18 +53,22 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch @AssistedInject class SecurityAndPrivacyPresenter( @Assisted private val navigator: SecurityAndPrivacyNavigator, + private val spaceSelectionStateHolder: SpaceSelectionStateHolder, private val matrixClient: MatrixClient, private val room: JoinedRoom, private val featureFlagService: FeatureFlagService, ) : Presenter { @AssistedFactory interface Factory { - fun create(navigator: SecurityAndPrivacyNavigator): SecurityAndPrivacyPresenter + fun create( + navigator: SecurityAndPrivacyNavigator, + ): SecurityAndPrivacyPresenter } @Composable @@ -136,6 +142,18 @@ class SecurityAndPrivacyPresenter( } } + LaunchedEffect(selectableJoinedSpaces, savedSettings.roomAccess) { + val unknownSpaceIds = savedSettings.roomAccess.spaceIds().filter { spaceId -> + selectableJoinedSpaces.none { it.roomId == spaceId } + }.toImmutableList() + spaceSelectionStateHolder.update { state -> + state.copy( + selectableSpaces = selectableJoinedSpaces, + unknownSpaceIds = unknownSpaceIds, + ) + } + } + var showEnableEncryptionConfirmation by remember(savedSettings.isEncrypted) { mutableStateOf(false) } val permissions by room.permissionsAsState(SecurityAndPrivacyPermissions.DEFAULT) { perms -> perms.securityAndPrivacyPermissions() @@ -191,19 +209,27 @@ class SecurityAndPrivacyPresenter( SecurityAndPrivacyEvent.DismissExitConfirmation -> { saveAction.value = AsyncAction.Uninitialized } - SecurityAndPrivacyEvent.ManageAuthorizedSpaces -> { - navigator.openManageAuthorizedSpaces( + SecurityAndPrivacyEvent.ManageAuthorizedSpaces -> coroutineScope.launch { + handleMultipleSelection( + savedAccess = savedSettings.roomAccess, + editedRoomAccess = editedRoomAccess, forKnockRestricted = editedRoomAccess.value is SecurityAndPrivacyRoomAccess.AskToJoinWithSpaceMember ) } - SecurityAndPrivacyEvent.SelectSpaceMemberAccess -> handleSpaceMemberAccessSelection( - spaceSelectionMode = spaceSelectionMode, - editedAccess = editedRoomAccess, - ) - SecurityAndPrivacyEvent.SelectAskToJoinWithSpaceMembersAccess -> handleAskToJoinWithSpaceMembersAccessSelection( - spaceSelectionMode = spaceSelectionMode, - editedAccess = editedRoomAccess, - ) + SecurityAndPrivacyEvent.SelectSpaceMemberAccess -> coroutineScope.launch { + handleSpaceMemberAccessSelection( + spaceSelectionMode = spaceSelectionMode, + savedAccess = savedSettings.roomAccess, + editedAccess = editedRoomAccess, + ) + } + SecurityAndPrivacyEvent.SelectAskToJoinWithSpaceMembersAccess -> coroutineScope.launch { + handleAskToJoinWithSpaceMembersAccessSelection( + spaceSelectionMode = spaceSelectionMode, + savedAccess = savedSettings.roomAccess, + editedAccess = editedRoomAccess, + ) + } } } @@ -248,8 +274,9 @@ class SecurityAndPrivacyPresenter( return state } - private fun handleSpaceMemberAccessSelection( + private suspend fun handleSpaceMemberAccessSelection( spaceSelectionMode: SpaceSelectionMode, + savedAccess: SecurityAndPrivacyRoomAccess, editedAccess: MutableState, ) { if (editedAccess.value is SecurityAndPrivacyRoomAccess.SpaceMember) { @@ -257,7 +284,11 @@ class SecurityAndPrivacyPresenter( } when (spaceSelectionMode) { is SpaceSelectionMode.None -> Unit - is SpaceSelectionMode.Multiple -> navigator.openManageAuthorizedSpaces(forKnockRestricted = false) + is SpaceSelectionMode.Multiple -> handleMultipleSelection( + savedAccess = savedAccess, + editedRoomAccess = editedAccess, + forKnockRestricted = false, + ) is SpaceSelectionMode.Single -> { val newRoomAccess = SecurityAndPrivacyRoomAccess.SpaceMember( spaceIds = persistentListOf(spaceSelectionMode.spaceId) @@ -267,8 +298,9 @@ class SecurityAndPrivacyPresenter( } } - private fun handleAskToJoinWithSpaceMembersAccessSelection( + private suspend fun handleAskToJoinWithSpaceMembersAccessSelection( spaceSelectionMode: SpaceSelectionMode, + savedAccess: SecurityAndPrivacyRoomAccess, editedAccess: MutableState, ) { if (editedAccess.value is SecurityAndPrivacyRoomAccess.AskToJoinWithSpaceMember) { @@ -276,7 +308,11 @@ class SecurityAndPrivacyPresenter( } when (spaceSelectionMode) { is SpaceSelectionMode.None -> Unit - is SpaceSelectionMode.Multiple -> navigator.openManageAuthorizedSpaces(forKnockRestricted = true) + is SpaceSelectionMode.Multiple -> handleMultipleSelection( + savedAccess = savedAccess, + editedRoomAccess = editedAccess, + forKnockRestricted = true, + ) is SpaceSelectionMode.Single -> { val newRoomAccess = SecurityAndPrivacyRoomAccess.AskToJoinWithSpaceMember( spaceIds = persistentListOf(spaceSelectionMode.spaceId) @@ -286,6 +322,38 @@ class SecurityAndPrivacyPresenter( } } + private suspend fun handleMultipleSelection( + savedAccess: SecurityAndPrivacyRoomAccess, + editedRoomAccess: MutableState, + forKnockRestricted: Boolean + ) { + val initialSelection = when (val currentRoomAccess = editedRoomAccess.value) { + is SecurityAndPrivacyRoomAccess.SpaceMember -> currentRoomAccess.spaceIds + is SecurityAndPrivacyRoomAccess.AskToJoinWithSpaceMember -> currentRoomAccess.spaceIds + else -> savedAccess.spaceIds() + } + spaceSelectionStateHolder.update { state -> + state.copy(selectedSpaceIds = initialSelection, completion = SpaceSelectionState.Completion.Initial) + } + navigator.openManageAuthorizedSpaces() + val newState = spaceSelectionStateHolder.state.first { it.completion != SpaceSelectionState.Completion.Initial } + when (newState.completion) { + SpaceSelectionState.Completion.Initial -> Unit + SpaceSelectionState.Completion.Cancelled -> { + navigator.closeManageAuthorizedSpaces() + } + SpaceSelectionState.Completion.Completed -> { + val selectedIds = newState.selectedSpaceIds + editedRoomAccess.value = if (forKnockRestricted) { + SecurityAndPrivacyRoomAccess.AskToJoinWithSpaceMember(spaceIds = selectedIds) + } else { + SecurityAndPrivacyRoomAccess.SpaceMember(spaceIds = selectedIds) + } + navigator.closeManageAuthorizedSpaces() + } + } + } + private fun getSpaceSelectionMode( selectableJoinedSpaces: Set, savedAccess: SecurityAndPrivacyRoomAccess, diff --git a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyState.kt b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyState.kt index ff627f6a0e..6ec47ba183 100644 --- a/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyState.kt +++ b/features/securityandprivacy/impl/src/main/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyState.kt @@ -12,7 +12,6 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.res.stringResource import io.element.android.features.securityandprivacy.api.SecurityAndPrivacyPermissions import io.element.android.features.securityandprivacy.impl.R -import io.element.android.features.securityandprivacy.impl.manageauthorizedspaces.AuthorizedSpacesSelection import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.matrix.api.core.RoomId @@ -121,20 +120,6 @@ data class SecurityAndPrivacyState( stringResource(R.string.screen_security_and_privacy_ask_to_join_option_description) } } - - fun getAuthorizedSpacesSelection(): AuthorizedSpacesSelection { - return AuthorizedSpacesSelection( - joinedSpaces = selectableJoinedSpaces.toImmutableList(), - unknownSpaceIds = savedSettings.roomAccess.spaceIds().filter { spaceId -> - selectableJoinedSpaces.none { it.roomId == spaceId } - }.toImmutableList(), - initialSelectedIds = when (editedSettings.roomAccess) { - is SecurityAndPrivacyRoomAccess.SpaceMember -> editedSettings.roomAccess.spaceIds - is SecurityAndPrivacyRoomAccess.AskToJoinWithSpaceMember -> editedSettings.roomAccess.spaceIds - else -> savedSettings.roomAccess.spaceIds() - } - ) - } } data class SecurityAndPrivacySettings( diff --git a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/FakeSecurityAndPrivacyNavigator.kt b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/FakeSecurityAndPrivacyNavigator.kt index 5c5366caba..c0a7ca8e7f 100644 --- a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/FakeSecurityAndPrivacyNavigator.kt +++ b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/FakeSecurityAndPrivacyNavigator.kt @@ -14,7 +14,7 @@ class FakeSecurityAndPrivacyNavigator( private val onDoneLambda: () -> Unit = { lambdaError() }, private val openEditRoomAddressLambda: () -> Unit = { lambdaError() }, private val closeEditRoomAddressLambda: () -> Unit = { lambdaError() }, - private val openManageAuthorizedSpacesLambda: (Boolean) -> Unit = { lambdaError() }, + private val openManageAuthorizedSpacesLambda: () -> Unit = { lambdaError() }, private val closeManageAuthorizedSpacesLambda: () -> Unit = { lambdaError() }, ) : SecurityAndPrivacyNavigator { override fun onDone() { @@ -29,8 +29,8 @@ class FakeSecurityAndPrivacyNavigator( closeEditRoomAddressLambda() } - override fun openManageAuthorizedSpaces(forKnockRestricted: Boolean) { - openManageAuthorizedSpacesLambda(forKnockRestricted) + override fun openManageAuthorizedSpaces() { + openManageAuthorizedSpacesLambda() } override fun closeManageAuthorizedSpaces() { diff --git a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyFlowNodeTest.kt b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyFlowNodeTest.kt index baae82a586..a6f21c0162 100644 --- a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyFlowNodeTest.kt +++ b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/SecurityAndPrivacyFlowNodeTest.kt @@ -49,27 +49,16 @@ class SecurityAndPrivacyFlowNodeTest { } @Test - fun `openManageAuthorizedSpaces navigates with forKnockRestricted false`() = runTest { + fun `openManageAuthorizedSpaces navigates to ManageAuthorizedSpaces`() = runTest { val flowNode = createFlowNode() - flowNode.navigator.openManageAuthorizedSpaces(forKnockRestricted = false) - assertThat(flowNode.currentNavTarget()).isEqualTo( - SecurityAndPrivacyFlowNode.NavTarget.ManageAuthorizedSpaces(forKnockRestricted = false) - ) - } - - @Test - fun `openManageAuthorizedSpaces navigates with forKnockRestricted true`() = runTest { - val flowNode = createFlowNode() - flowNode.navigator.openManageAuthorizedSpaces(forKnockRestricted = true) - assertThat(flowNode.currentNavTarget()).isEqualTo( - SecurityAndPrivacyFlowNode.NavTarget.ManageAuthorizedSpaces(forKnockRestricted = true) - ) + flowNode.navigator.openManageAuthorizedSpaces() + assertThat(flowNode.currentNavTarget()).isEqualTo(SecurityAndPrivacyFlowNode.NavTarget.ManageAuthorizedSpaces) } @Test fun `closeManageAuthorizedSpaces pops backstack`() = runTest { val flowNode = createFlowNode() - flowNode.navigator.openManageAuthorizedSpaces(forKnockRestricted = false) + flowNode.navigator.openManageAuthorizedSpaces() assertThat(flowNode.currentNavTarget()) .isInstanceOf(SecurityAndPrivacyFlowNode.NavTarget.ManageAuthorizedSpaces::class.java) flowNode.navigator.closeManageAuthorizedSpaces() diff --git a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesPresenterTest.kt b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesPresenterTest.kt index 7d678b23bb..8314aca4cd 100644 --- a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesPresenterTest.kt +++ b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesPresenterTest.kt @@ -12,38 +12,33 @@ import com.google.common.truth.Truth.assertThat import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.tests.testutils.test import kotlinx.collections.immutable.persistentListOf +import kotlinx.collections.immutable.persistentSetOf import kotlinx.coroutines.test.runTest import org.junit.Test class ManageAuthorizedSpacesPresenterTest { @Test - fun `present - initial state has empty selection`() = runTest { - val presenter = ManageAuthorizedSpacesPresenter() + fun `present - initial state reflects shared state`() = runTest { + val sharedStateHolder = SpaceSelectionStateHolder() + val presenter = ManageAuthorizedSpacesPresenter(sharedStateHolder) presenter.test { with(awaitItem()) { assertThat(selectedIds).isEmpty() - assertThat(isSelectionComplete).isFalse() assertThat(isDoneButtonEnabled).isFalse() } } } @Test - fun `present - SetData event updates selection and initial selectedIds`() = runTest { - val presenter = ManageAuthorizedSpacesPresenter() + fun `present - state reflects shared state with pre-selected spaces`() = runTest { + val sharedStateHolder = SpaceSelectionStateHolder() + val roomId = A_ROOM_ID + sharedStateHolder.update { + it.copy(selectedSpaceIds = persistentListOf(roomId)) + } + val presenter = ManageAuthorizedSpacesPresenter(sharedStateHolder) presenter.test { - val initialState = awaitItem() - val roomId = A_ROOM_ID - val data = AuthorizedSpacesSelection( - joinedSpaces = persistentListOf(), - unknownSpaceIds = persistentListOf(), - initialSelectedIds = persistentListOf(roomId) - ) - initialState.eventSink(ManageAuthorizedSpacesEvent.SetData(data)) - // SetData updates two state variables, which may emit intermediate states - skipItems(1) with(awaitItem()) { - assertThat(selection).isEqualTo(data) assertThat(selectedIds).containsExactly(roomId) assertThat(isDoneButtonEnabled).isTrue() } @@ -51,8 +46,9 @@ class ManageAuthorizedSpacesPresenterTest { } @Test - fun `present - ToggleSpace event adds space to selectedIds`() = runTest { - val presenter = ManageAuthorizedSpacesPresenter() + fun `present - ToggleSpace event adds space to selectedIds in shared state`() = runTest { + val sharedStateHolder = SpaceSelectionStateHolder() + val presenter = ManageAuthorizedSpacesPresenter(sharedStateHolder) presenter.test { val initialState = awaitItem() val roomId = A_ROOM_ID @@ -61,34 +57,69 @@ class ManageAuthorizedSpacesPresenterTest { assertThat(selectedIds).containsExactly(roomId) assertThat(isDoneButtonEnabled).isTrue() } + // Verify the shared state is also updated + assertThat(sharedStateHolder.state.value.selectedSpaceIds).containsExactly(roomId) } } @Test fun `present - ToggleSpace event removes space when already selected`() = runTest { - val presenter = ManageAuthorizedSpacesPresenter() + val sharedStateHolder = SpaceSelectionStateHolder() + sharedStateHolder.updateSelectedSpaceIds(persistentListOf(A_ROOM_ID)) + val presenter = ManageAuthorizedSpacesPresenter(sharedStateHolder) presenter.test { val initialState = awaitItem() - val roomId = A_ROOM_ID - initialState.eventSink(ManageAuthorizedSpacesEvent.ToggleSpace(roomId)) - val stateWithSelection = awaitItem() - assertThat(stateWithSelection.selectedIds).containsExactly(roomId) - stateWithSelection.eventSink(ManageAuthorizedSpacesEvent.ToggleSpace(roomId)) + assertThat(initialState.selectedIds).containsExactly(A_ROOM_ID) + initialState.eventSink(ManageAuthorizedSpacesEvent.ToggleSpace(A_ROOM_ID)) with(awaitItem()) { assertThat(selectedIds).isEmpty() assertThat(isDoneButtonEnabled).isFalse() } + // Verify the shared state is also updated + assertThat(sharedStateHolder.state.value.selectedSpaceIds).isEmpty() } } @Test - fun `present - Done event sets isSelectionComplete to true`() = runTest { - val presenter = ManageAuthorizedSpacesPresenter() + fun `present - Done event sets completion to Completed`() = runTest { + val sharedStateHolder = SpaceSelectionStateHolder() + val presenter = ManageAuthorizedSpacesPresenter(sharedStateHolder) presenter.test { val initialState = awaitItem() initialState.eventSink(ManageAuthorizedSpacesEvent.Done) + cancelAndIgnoreRemainingEvents() + assertThat(sharedStateHolder.state.value.completion) + .isEqualTo(SpaceSelectionState.Completion.Completed) + } + } + + @Test + fun `present - Cancel event sets completion to Cancelled`() = runTest { + val sharedStateHolder = SpaceSelectionStateHolder() + val presenter = ManageAuthorizedSpacesPresenter(sharedStateHolder) + presenter.test { + val initialState = awaitItem() + initialState.eventSink(ManageAuthorizedSpacesEvent.Cancel) + cancelAndIgnoreRemainingEvents() + assertThat(sharedStateHolder.state.value.completion) + .isEqualTo(SpaceSelectionState.Completion.Cancelled) + } + } + + @Test + fun `present - displays spaces from shared state`() = runTest { + val sharedStateHolder = SpaceSelectionStateHolder() + sharedStateHolder.update { + it.copy( + selectableSpaces = persistentSetOf(), + unknownSpaceIds = persistentListOf(A_ROOM_ID), + ) + } + val presenter = ManageAuthorizedSpacesPresenter(sharedStateHolder) + presenter.test { with(awaitItem()) { - assertThat(isSelectionComplete).isTrue() + assertThat(selectableSpaces).isEmpty() + assertThat(unknownSpaceIds).containsExactly(A_ROOM_ID) } } } diff --git a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesViewTest.kt b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesViewTest.kt index 515fe590a3..c732df6df0 100644 --- a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesViewTest.kt +++ b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/manageauthorizedspaces/ManageAuthorizedSpacesViewTest.kt @@ -15,15 +15,15 @@ import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.test.ext.junit.runners.AndroidJUnit4 import io.element.android.libraries.matrix.api.core.RoomId +import io.element.android.libraries.matrix.api.spaces.SpaceRoom import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.previewutils.room.aSpaceRoom import io.element.android.libraries.ui.strings.CommonStrings -import io.element.android.tests.testutils.EnsureNeverCalled 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.toImmutableSet import org.junit.Rule import org.junit.Test import org.junit.rules.TestRule @@ -34,11 +34,12 @@ class ManageAuthorizedSpacesViewTest { @get:Rule val rule = createAndroidComposeRule() @Test - fun `clicking back invokes callback`() { - ensureCalledOnce { callback -> - rule.setManageAuthorizedSpacesView(onBackClick = callback) - rule.pressBack() - } + fun `clicking back emits Cancel event`() { + val recorder = EventsRecorder() + val state = aManageAuthorizedSpacesState(eventSink = recorder) + rule.setManageAuthorizedSpacesView(state) + rule.pressBack() + recorder.assertSingle(ManageAuthorizedSpacesEvent.Cancel) } @Test @@ -47,9 +48,7 @@ class ManageAuthorizedSpacesViewTest { val space = aSpaceRoom(roomId = roomId, displayName = "Test Space") val recorder = EventsRecorder() val state = aManageAuthorizedSpacesState( - selection = anAuthorizedSpaceSelection( - joinedSpaces = listOf(space) - ), + selectableSpaces = listOf(space), eventSink = recorder ) rule.setManageAuthorizedSpacesView(state) @@ -86,24 +85,20 @@ private fun AndroidComposeTestRule.setManag state: ManageAuthorizedSpacesState = aManageAuthorizedSpacesState( eventSink = EventsRecorder(expectEvents = false) ), - onBackClick: () -> Unit = EnsureNeverCalled(), ) { setContent { - ManageAuthorizedSpacesView( - state = state, - onBackClick = onBackClick, - ) + ManageAuthorizedSpacesView(state = state) } } private fun aManageAuthorizedSpacesState( - selection: AuthorizedSpacesSelection = AuthorizedSpacesSelection(), + selectableSpaces: List = emptyList(), + unknownSpaceIds: List = emptyList(), selectedIds: List = emptyList(), - isSelectionComplete: Boolean = false, eventSink: (ManageAuthorizedSpacesEvent) -> Unit = {}, ) = ManageAuthorizedSpacesState( - selection = selection, + selectableSpaces = selectableSpaces.toImmutableSet(), + unknownSpaceIds = unknownSpaceIds.toImmutableList(), selectedIds = selectedIds.toImmutableList(), - isSelectionComplete = isSelectionComplete, eventSink = eventSink, ) diff --git a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyPresenterTest.kt b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyPresenterTest.kt index 2c1611fec8..e9cd49cc94 100644 --- a/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyPresenterTest.kt +++ b/features/securityandprivacy/impl/src/test/kotlin/io/element/android/features/securityandprivacy/impl/root/SecurityAndPrivacyPresenterTest.kt @@ -10,6 +10,7 @@ package io.element.android.features.securityandprivacy.impl.root import com.google.common.truth.Truth.assertThat import io.element.android.features.securityandprivacy.impl.FakeSecurityAndPrivacyNavigator import io.element.android.features.securityandprivacy.impl.SecurityAndPrivacyNavigator +import io.element.android.features.securityandprivacy.impl.manageauthorizedspaces.SpaceSelectionStateHolder import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.featureflag.api.FeatureFlagService @@ -436,7 +437,7 @@ class SecurityAndPrivacyPresenterTest { @Test fun `present - SelectSpaceMemberAccess with multiple spaces opens ManageAuthorizedSpaces`() = runTest { - val openManageAuthorizedSpacesLambda = lambdaRecorder { } + val openManageAuthorizedSpacesLambda = lambdaRecorder { } val navigator = FakeSecurityAndPrivacyNavigator(openManageAuthorizedSpacesLambda = openManageAuthorizedSpacesLambda) val room = FakeJoinedRoom( @@ -471,7 +472,7 @@ class SecurityAndPrivacyPresenterTest { val state = awaitItem() assertThat(state.isSpaceMemberSelectable).isTrue() state.eventSink(SecurityAndPrivacyEvent.SelectSpaceMemberAccess) - assert(openManageAuthorizedSpacesLambda).isCalledOnce().with(value(false)) + assert(openManageAuthorizedSpacesLambda).isCalledOnce() } } @@ -605,7 +606,7 @@ class SecurityAndPrivacyPresenterTest { @Test fun `present - SelectAskToJoinWithSpaceMembersAccess with multiple spaces opens ManageAuthorizedSpaces`() = runTest { - val openManageAuthorizedSpacesLambda = lambdaRecorder { } + val openManageAuthorizedSpacesLambda = lambdaRecorder { } val navigator = FakeSecurityAndPrivacyNavigator(openManageAuthorizedSpacesLambda = openManageAuthorizedSpacesLambda) val room = FakeJoinedRoom( @@ -642,7 +643,7 @@ class SecurityAndPrivacyPresenterTest { val state = awaitItem() assertThat(state.isAskToJoinWithSpaceMembersSelectable).isTrue() state.eventSink(SecurityAndPrivacyEvent.SelectAskToJoinWithSpaceMembersAccess) - assert(openManageAuthorizedSpacesLambda).isCalledOnce().with(value(true)) + assert(openManageAuthorizedSpacesLambda).isCalledOnce() } } @@ -950,73 +951,6 @@ class SecurityAndPrivacyPresenterTest { } } - @Test - fun `present - getAuthorizedSpacesSelection returns correct data for SpaceMember`() = runTest { - val room = FakeJoinedRoom( - baseRoom = FakeBaseRoom( - roomPermissions = roomPermissions(), - initialRoomInfo = aRoomInfo( - historyVisibility = RoomHistoryVisibility.Shared, - joinRule = JoinRule.Restricted( - rules = persistentListOf(AllowRule.RoomMembership(A_ROOM_ID)) - ) - ) - ) - ) - val spaceRoom = aSpaceRoom(roomId = A_ROOM_ID) - val client = FakeMatrixClient( - userIdServerNameLambda = { "matrix.org" }, - spaceService = FakeSpaceService( - joinedParentsResult = { _ -> Result.success(listOf(spaceRoom)) }, - getSpaceRoomResult = { null } - ) - ) - val presenter = createSecurityAndPrivacyPresenter( - room = room, - matrixClient = client, - featureFlagService = FakeFeatureFlagService( - initialState = mapOf(FeatureFlags.SpaceSettings.key to true) - ) - ) - presenter.test { - skipItems(1) - with(awaitItem()) { - val selection = getAuthorizedSpacesSelection() - assertThat(selection.joinedSpaces).containsExactly(spaceRoom) - assertThat(selection.initialSelectedIds).containsExactly(A_ROOM_ID) - assertThat(selection.unknownSpaceIds).isEmpty() - } - cancelAndIgnoreRemainingEvents() - } - } - - @Test - fun `present - getAuthorizedSpacesSelection identifies unknown space IDs`() = runTest { - val unknownSpaceId = RoomId("!unknown:matrix.org") - val room = FakeJoinedRoom( - baseRoom = FakeBaseRoom( - roomPermissions = roomPermissions(), - initialRoomInfo = aRoomInfo( - historyVisibility = RoomHistoryVisibility.Shared, - joinRule = JoinRule.Restricted( - rules = persistentListOf(AllowRule.RoomMembership(unknownSpaceId)) - ) - ) - ) - ) - // No spaces available (the space in the join rule is unknown) - val presenter = createSecurityAndPrivacyPresenter(room = room) - presenter.test { - skipItems(1) - with(awaitItem()) { - val selection = getAuthorizedSpacesSelection() - assertThat(selection.joinedSpaces).isEmpty() - assertThat(selection.unknownSpaceIds).containsExactly(unknownSpaceId) - } - cancelAndIgnoreRemainingEvents() - } - } - @Test fun `present - SelectAskToJoinWithSpaceMembersAccess with single space auto-selects`() = runTest { val room = FakeJoinedRoom( @@ -1169,12 +1103,14 @@ class SecurityAndPrivacyPresenterTest { getSpaceRoomResult = { null } ), ), + spaceSelectionStateHolder: SpaceSelectionStateHolder = SpaceSelectionStateHolder(), ): SecurityAndPrivacyPresenter { return SecurityAndPrivacyPresenter( room = room, matrixClient = matrixClient, navigator = navigator, featureFlagService = featureFlagService, + spaceSelectionStateHolder = spaceSelectionStateHolder, ) } }