change(roles and permissions): improve the flow

features/home/impl/src/main/kotlin/io/element/android/features/home/impl/HomeFlowNode.kt

@@ -40,7 +40,6 @@ import io.element.android.features.logout.api.direct.DirectLogoutView
 import io.element.android.features.reportroom.api.ReportRoomEntryPoint
 import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRolesEntryPoint
 import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRolesListType
-import io.element.android.features.rolesandpermissions.api.RolesAndPermissionsEntryPoint
 import io.element.android.libraries.architecture.BackstackView
 import io.element.android.libraries.architecture.BaseFlowNode
 import io.element.android.libraries.architecture.appyx.launchMolecule
@@ -94,10 +93,12 @@ class HomeFlowNode(
             changeRoomMemberRolesNode: ChangeRoomMemberRolesEntryPoint.NodeProxy,
             ->
             commonLifecycle.coroutineScope.launch {
-                changeRoomMemberRolesNode.waitForRoleChanged()
+                val isNewOwnerSelected = changeRoomMemberRolesNode.waitForCompletion()
                 withContext(NonCancellable) {
                     backstack.pop()
-                    onNewOwnersSelected(changeRoomMemberRolesNode.roomId)
+                    if(isNewOwnerSelected) {
+                        onNewOwnersSelected(changeRoomMemberRolesNode.roomId)
+                    }
                 }
             }
         }

features/rolesandpermissions/api/src/main/kotlin/io/element/android/features/rolesandpermissions/api/ChangeRoomMemberRolesEntryPoint.kt

@@ -24,11 +24,11 @@ fun interface ChangeRoomMemberRolesEntryPoint : FeatureEntryPoint {
 
     interface NodeProxy {
         val roomId: RoomId
-        suspend fun waitForRoleChanged()
+        suspend fun waitForCompletion(): Boolean
     }
 }
 
-enum class ChangeRoomMemberRolesListType : NodeInputs {
+enum class ChangeRoomMemberRolesListType {
     SelectNewOwnersWhenLeaving,
     Admins,
     Moderators

features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/RolesAndPermissionsFlowNode.kt

@@ -8,8 +8,11 @@
 package io.element.android.features.rolesandpermissions.impl
 
 import android.os.Parcelable
+import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.statusBarsPadding
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.res.stringResource
 import androidx.lifecycle.coroutineScope
 import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.node.Node
@@ -20,15 +23,18 @@ import com.bumble.appyx.navmodel.backstack.operation.push
 import dev.zacsweers.metro.Assisted
 import dev.zacsweers.metro.AssistedInject
 import io.element.android.annotations.ContributesNode
-import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRolesEntryPoint
 import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRolesListType
 import io.element.android.features.rolesandpermissions.impl.permissions.ChangeRoomPermissionsNode
+import io.element.android.features.rolesandpermissions.impl.roles.ChangeRolesNode
 import io.element.android.features.rolesandpermissions.impl.root.RolesAndPermissionsNode
 import io.element.android.libraries.architecture.BackstackView
 import io.element.android.libraries.architecture.BaseFlowNode
 import io.element.android.libraries.architecture.createNode
+import io.element.android.libraries.designsystem.components.async.AsyncIndicator
+import io.element.android.libraries.designsystem.components.async.AsyncIndicatorHost
+import io.element.android.libraries.designsystem.components.async.AsyncIndicatorState
 import io.element.android.libraries.di.RoomScope
-import io.element.android.libraries.matrix.api.room.JoinedRoom
+import io.element.android.libraries.ui.strings.CommonStrings
 import kotlinx.coroutines.launch
 import kotlinx.parcelize.Parcelize
 
@@ -37,11 +43,9 @@ import kotlinx.parcelize.Parcelize
 class RolesAndPermissionsFlowNode(
     @Assisted buildContext: BuildContext,
     @Assisted plugins: List<Plugin>,
-    private val joinedRoom: JoinedRoom,
-    private val changeRoomMemberRolesEntryPoint: ChangeRoomMemberRolesEntryPoint,
 ) : BaseFlowNode<RolesAndPermissionsFlowNode.NavTarget>(
     backstack = BackStack(
-        initialElement = NavTarget.AdminSettings,
+        initialElement = NavTarget.Root,
         savedStateMap = buildContext.savedStateMap,
     ),
     buildContext = buildContext,
@@ -49,38 +53,49 @@ class RolesAndPermissionsFlowNode(
 ) {
     sealed interface NavTarget : Parcelable {
         @Parcelize
-        data object AdminSettings : NavTarget
+        data object Root : NavTarget
 
         @Parcelize
-        data object AdminList : NavTarget
+        data object ChangeAdmins : NavTarget
 
         @Parcelize
-        data object ModeratorList : NavTarget
+        data object ChangeModerators : NavTarget
 
         @Parcelize
         data object ChangeRoomPermissions: NavTarget
     }
 
+    private val asyncIndicatorState = AsyncIndicatorState()
+
     override fun onBuilt() {
         super.onBuilt()
-        whenChildAttached { lifecycle, node: ChangeRoomMemberRolesEntryPoint.NodeProxy ->
+        whenChildAttached { lifecycle, node: ChangeRolesNode ->
             lifecycle.coroutineScope.launch {
-                node.waitForRoleChanged()
-                backstack.pop()
+                val changesSaved = node.waitForCompletion()
+                onChangeComplete(changesSaved)
+            }
+        }
+    }
+
+    private fun onChangeComplete(changesSaved: Boolean) {
+        backstack.pop()
+        if (changesSaved) {
+            asyncIndicatorState.enqueue(durationMs = AsyncIndicator.DURATION_SHORT) {
+                AsyncIndicator.Custom(text = stringResource(CommonStrings.common_saved_changes))
             }
         }
     }
 
     override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
         return when (navTarget) {
-            is NavTarget.AdminSettings -> {
+            is NavTarget.Root -> {
                 val callback = object : RolesAndPermissionsNode.Callback {
                     override fun openAdminList() {
-                        backstack.push(NavTarget.AdminList)
+                        backstack.push(NavTarget.ChangeAdmins)
                     }
 
                     override fun openModeratorList() {
-                        backstack.push(NavTarget.ModeratorList)
+                        backstack.push(NavTarget.ChangeModerators)
                     }
 
                     override fun openEditPermissions() {
@@ -93,30 +108,32 @@ class RolesAndPermissionsFlowNode(
                     plugins = listOf(callback),
                 )
             }
-            is NavTarget.AdminList -> {
-                changeRoomMemberRolesEntryPoint.createNode(
-                    parentNode = this,
-                    buildContext = buildContext,
-                    room = joinedRoom,
-                    listType = ChangeRoomMemberRolesListType.Admins,
-                )
+            is NavTarget.ChangeAdmins -> {
+                val inputs = ChangeRolesNode.Inputs(ChangeRoomMemberRolesListType.Admins)
+                createNode<ChangeRolesNode>(buildContext = buildContext, plugins = listOf(inputs))
             }
-            is NavTarget.ModeratorList -> {
-                changeRoomMemberRolesEntryPoint.createNode(
-                    parentNode = this,
-                    buildContext = buildContext,
-                    room = joinedRoom,
-                    listType = ChangeRoomMemberRolesListType.Moderators,
-                )
+            is NavTarget.ChangeModerators -> {
+                val inputs = ChangeRolesNode.Inputs(ChangeRoomMemberRolesListType.Moderators)
+                createNode<ChangeRolesNode>(buildContext = buildContext, plugins = listOf(inputs))
             }
             is NavTarget.ChangeRoomPermissions -> {
-                createNode<ChangeRoomPermissionsNode>(buildContext = buildContext)
+                val callback = object : ChangeRoomPermissionsNode.Callback {
+                    override fun onComplete(changesSaved: Boolean) {
+                        onChangeComplete(changesSaved)
+                    }
+                }
+                createNode<ChangeRoomPermissionsNode>(buildContext = buildContext, plugins = listOf(callback))
             }
         }
     }
 
+
+
     @Composable
     override fun View(modifier: Modifier) {
-        BackstackView()
+        Box(modifier = modifier) {
+            BackstackView()
+            AsyncIndicatorHost(modifier = Modifier.statusBarsPadding(), asyncIndicatorState)
+        }
     }
 }

features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/permissions/ChangeRoomPermissionsNode.kt

@@ -15,6 +15,7 @@ import com.bumble.appyx.core.plugin.Plugin
 import dev.zacsweers.metro.Assisted
 import dev.zacsweers.metro.AssistedInject
 import io.element.android.annotations.ContributesNode
+import io.element.android.libraries.architecture.callback
 import io.element.android.libraries.di.RoomScope
 
 @ContributesNode(RoomScope::class)
@@ -25,13 +26,19 @@ class ChangeRoomPermissionsNode(
     private val presenter: ChangeRoomPermissionsPresenter,
 ) : Node(buildContext, plugins = plugins) {
 
+    interface Callback : Plugin {
+        fun onComplete(changesSaved: Boolean)
+    }
+
+    private val callback: Callback = callback()
+
     @Composable
     override fun View(modifier: Modifier) {
         val state = presenter.present()
         ChangeRoomPermissionsView(
             modifier = modifier,
             state = state,
-            onBackClick = this::navigateUp,
+            onComplete = callback::onComplete,
         )
     }
 }

features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/permissions/ChangeRoomPermissionsView.kt

@@ -16,26 +16,17 @@ import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.PreviewParameter
-import io.element.android.compound.tokens.generated.CompoundIcons
 import io.element.android.features.rolesandpermissions.impl.R
-import io.element.android.libraries.core.bool.orFalse
 import io.element.android.libraries.designsystem.components.async.AsyncActionView
 import io.element.android.libraries.designsystem.components.button.BackButton
 import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog
-import io.element.android.libraries.designsystem.components.list.ListItemContent
 import io.element.android.libraries.designsystem.components.preferences.PreferenceDropdown
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
-import io.element.android.libraries.designsystem.theme.components.IconSource
-import io.element.android.libraries.designsystem.theme.components.ListItem
-import io.element.android.libraries.designsystem.theme.components.ListItemStyle
 import io.element.android.libraries.designsystem.theme.components.ListSectionHeader
 import io.element.android.libraries.designsystem.theme.components.Scaffold
-import io.element.android.libraries.designsystem.theme.components.Text
 import io.element.android.libraries.designsystem.theme.components.TextButton
 import io.element.android.libraries.designsystem.theme.components.TopAppBar
-import io.element.android.libraries.matrix.api.room.RoomMember
-import io.element.android.libraries.matrix.api.room.powerlevels.RoomPowerLevelsValues
 import io.element.android.libraries.ui.strings.CommonStrings
 import kotlinx.collections.immutable.toImmutableList
 
@@ -43,7 +34,7 @@ import kotlinx.collections.immutable.toImmutableList
 @Composable
 fun ChangeRoomPermissionsView(
     state: ChangeRoomPermissionsState,
-    onBackClick: () -> Unit,
+    onComplete: (Boolean) -> Unit,
     modifier: Modifier = Modifier,
 ) {
     BackHandler {
@@ -74,7 +65,7 @@ fun ChangeRoomPermissionsView(
         ) {
             state.itemsBySection.onEachIndexed { index, (section, items) ->
                 item {
-                    ListSectionHeader(titleForSection(section), hasDivider = index>0)
+                    ListSectionHeader(titleForSection(section), hasDivider = index > 0)
                 }
                 for (permissionType in items) {
                     item {
@@ -99,13 +90,13 @@ fun ChangeRoomPermissionsView(
 
     AsyncActionView(
         async = state.saveAction,
-        onSuccess = { onBackClick() },
+        onSuccess = { onComplete(true) },
         onErrorDismiss = { state.eventSink(ChangeRoomPermissionsEvent.ResetPendingActions) }
     )
 
     AsyncActionView(
         async = state.confirmExitAction,
-        onSuccess = { onBackClick() },
+        onSuccess = { onComplete(false) },
         confirmationDialog = {
             ConfirmationDialog(
                 title = stringResource(R.string.screen_room_change_role_unsaved_changes_title),
@@ -119,6 +110,7 @@ fun ChangeRoomPermissionsView(
         onErrorDismiss = {},
     )
 }
+
 @Composable
 private fun titleForSection(section: RoomPermissionsSection): String = when (section) {
     RoomPermissionsSection.RoomDetails -> stringResource(R.string.screen_room_change_permissions_room_details)
@@ -144,7 +136,7 @@ internal fun ChangeRoomPermissionsViewPreview(@PreviewParameter(ChangeRoomPermis
     ElementPreview {
         ChangeRoomPermissionsView(
             state = state,
-            onBackClick = {},
+            onComplete = {},
         )
     }
 }

features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesNode.kt

@@ -21,6 +21,7 @@ import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRoles
 import io.element.android.libraries.architecture.NodeInputs
 import io.element.android.libraries.architecture.appyx.launchMolecule
 import io.element.android.libraries.architecture.inputs
+import io.element.android.libraries.core.bool.orFalse
 import io.element.android.libraries.di.RoomScope
 import io.element.android.libraries.matrix.api.room.RoomMember
 import kotlinx.coroutines.flow.first
@@ -40,17 +41,17 @@ class ChangeRolesNode(
     private val presenter = presenterFactory.create(inputs.listType.toRoomMemberRole())
     private val stateFlow = launchMolecule { presenter.present() }
 
-    suspend fun waitForRoleChanged() {
-        stateFlow.first { it.savingState.isSuccess() }
+    suspend fun waitForCompletion(): Boolean {
+        val successState = stateFlow.first { it.savingState.isSuccess() }
+        return successState.savingState.dataOrNull().orFalse()
     }
 
     @Composable
     override fun View(modifier: Modifier) {
         val state by stateFlow.collectAsState()
         ChangeRolesView(
-            modifier = modifier,
             state = state,
-            navigateUp = this::navigateUp,
+            modifier = modifier,
         )
     }
 }

features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesPresenter.kt

@@ -27,6 +27,7 @@ import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.designsystem.theme.components.SearchBarResultState
+import io.element.android.libraries.di.annotations.SessionCoroutineScope
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.room.JoinedRoom
 import io.element.android.libraries.matrix.api.room.RoomMember
@@ -41,10 +42,12 @@ import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toImmutableList
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 
 @AssistedInject
 class ChangeRolesPresenter(
@@ -214,9 +217,13 @@ class ChangeRolesPresenter(
                 saveState.value = AsyncAction.Failure(it)
             }
             .onSuccess {
-                saveState.value = AsyncAction.Success(true)
                 // Asynchronously reload the room members
-                launch { room.updateMembers() }
+                launch {
+                    withContext(NonCancellable) {
+                        room.updateMembers()
+                    }
+                }
+                saveState.value = AsyncAction.Success(true)
             }
     }
 }

features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt

@@ -29,8 +29,6 @@ import androidx.compose.foundation.lazy.items
 import androidx.compose.foundation.lazy.rememberLazyListState
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
@@ -42,7 +40,6 @@ import io.element.android.compound.theme.ElementTheme
 import io.element.android.features.rolesandpermissions.impl.R
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.designsystem.components.async.AsyncActionView
-import io.element.android.libraries.designsystem.components.async.AsyncIndicator
 import io.element.android.libraries.designsystem.components.async.AsyncIndicatorHost
 import io.element.android.libraries.designsystem.components.async.rememberAsyncIndicatorState
 import io.element.android.libraries.designsystem.components.avatar.Avatar
@@ -77,10 +74,8 @@ import kotlinx.collections.immutable.ImmutableList
 @Composable
 fun ChangeRolesView(
     state: ChangeRolesState,
-    navigateUp: () -> Unit,
     modifier: Modifier = Modifier,
 ) {
-    val latestNavigateUp by rememberUpdatedState(newValue = navigateUp)
     BackHandler(enabled = !state.isSearchActive) {
         state.eventSink(ChangeRolesEvent.Exit)
     }
@@ -168,18 +163,9 @@ fun ChangeRolesView(
 
         val asyncIndicatorState = rememberAsyncIndicatorState()
         AsyncIndicatorHost(modifier = Modifier.statusBarsPadding(), asyncIndicatorState)
-
         AsyncActionView(
             async = state.savingState,
-            onSuccess = { changeSaved ->
-                if (changeSaved) {
-                    asyncIndicatorState.enqueue(durationMs = AsyncIndicator.DURATION_SHORT) {
-                        AsyncIndicator.Custom(text = stringResource(CommonStrings.common_saved_changes))
-                    }
-                } else {
-                    latestNavigateUp()
-                }
-            },
+            onSuccess = {},
             confirmationDialog = { confirming ->
                 when (confirming) {
                     is AsyncAction.ConfirmingCancellation -> {
@@ -416,8 +402,7 @@ private fun MemberRow(
 internal fun ChangeRolesViewPreview(@PreviewParameter(ChangeRolesStateProvider::class) state: ChangeRolesState) {
     ElementPreview {
         ChangeRolesView(
-            state = state,
-            navigateUp = {},
+            state = state
         )
     }
 }

features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRoomMemberRolesRootNode.kt

@@ -21,7 +21,6 @@ import dev.zacsweers.metro.AssistedInject
 import io.element.android.annotations.ContributesNode
 import io.element.android.appnav.di.RoomGraphFactory
 import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRolesEntryPoint
-import io.element.android.features.rolesandpermissions.api.RolesAndPermissionsEntryPoint
 import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRolesListType
 import io.element.android.libraries.architecture.NodeInputs
 import io.element.android.libraries.architecture.createNode
@@ -71,7 +70,7 @@ class ChangeRoomMemberRolesRootNode(
 
     override val roomId: RoomId = inputs.joinedRoom.roomId
 
-    override suspend fun waitForRoleChanged() {
-        waitForChildAttached<ChangeRolesNode>().waitForRoleChanged()
+    override suspend fun waitForCompletion(): Boolean {
+        return waitForChildAttached<ChangeRolesNode>().waitForCompletion()
     }
 }

features/rolesandpermissions/impl/src/test/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesViewTest.kt

@@ -23,7 +23,6 @@ import io.element.android.libraries.matrix.api.room.RoomMember
 import io.element.android.libraries.matrix.api.room.toMatrixUser
 import io.element.android.libraries.matrix.ui.components.aMatrixUserList
 import io.element.android.libraries.ui.strings.CommonStrings
-import io.element.android.tests.testutils.EnsureNeverCalled
 import io.element.android.tests.testutils.EnsureNeverCalledWithParam
 import io.element.android.tests.testutils.EventsRecorder
 import io.element.android.tests.testutils.clickOn
@@ -306,12 +305,10 @@ class ChangeRolesViewTest {
 
     private fun <R : TestRule> AndroidComposeTestRule<R, ComponentActivity>.setChangeRolesContent(
         state: ChangeRolesState,
-        onBackClick: () -> Unit = EnsureNeverCalled(),
     ) {
         setContent {
             ChangeRolesView(
                 state = state,
-                navigateUp = onBackClick,
             )
         }
     }

features/rolesandpermissions/test/src/main/kotlin/io/element/android/features/changeroommemberroles/test/FakeChangeRoomMemberRolesEntryPoint.kt

@@ -5,16 +5,16 @@
  * Please see LICENSE files in the repository root for full details.
  */
 
-package io.element.android.features.rolesandpermissions.test
+package io.element.android.features.changeroommemberroles.test
 
 import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.node.Node
-import io.element.android.features.rolesandpermissions.api.RolesAndPermissionsEntryPoint
+import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRolesEntryPoint
 import io.element.android.features.rolesandpermissions.api.ChangeRoomMemberRolesListType
 import io.element.android.libraries.matrix.api.room.JoinedRoom
 import io.element.android.tests.testutils.lambda.lambdaError
 
-class FakeChangeRoomMemberRolesEntryPoint : RolesAndPermissionsEntryPoint {
+class FakeChangeRoomMemberRolesEntryPoint : ChangeRoomMemberRolesEntryPoint {
     override fun createNode(
         parentNode: Node,
         buildContext: BuildContext,

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsFlowNode.kt

@@ -157,10 +157,12 @@ class RoomDetailsFlowNode(
             changeRoomMemberRolesNode: ChangeRoomMemberRolesEntryPoint.NodeProxy,
             ->
             commonLifecycle.coroutineScope.launch {
-                changeRoomMemberRolesNode.waitForRoleChanged()
+                val isNewOwnerSelected = changeRoomMemberRolesNode.waitForCompletion()
                 withContext(NonCancellable) {
                     backstack.pop()
-                    roomDetailsNode.onNewOwnersSelected()
+                    if (isNewOwnerSelected) {
+                        roomDetailsNode.onNewOwnersSelected()
+                    }
                 }
             }
         }

features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/impl/DefaultRoomDetailsEntryPointTest.kt

@@ -12,7 +12,7 @@ import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.testing.junit4.util.MainDispatcherRule
 import com.google.common.truth.Truth.assertThat
 import io.element.android.features.call.test.FakeElementCallEntryPoint
-import io.element.android.features.rolesandpermissions.test.FakeChangeRoomMemberRolesEntryPoint
+import io.element.android.features.changeroommemberroles.test.FakeChangeRoomMemberRolesEntryPoint
 import io.element.android.features.knockrequests.test.FakeKnockRequestsListEntryPoint
 import io.element.android.features.messages.test.FakeMessagesEntryPoint
 import io.element.android.features.poll.test.history.FakePollHistoryEntryPoint