Merge pull request #2780 from element-hq/feature/bma/fixNotificationNavigation

Fix notification navigation

Author: bmarty

appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt

@@ -35,7 +35,6 @@ import com.bumble.appyx.core.plugin.plugins
 import com.bumble.appyx.navmodel.backstack.BackStack
 import com.bumble.appyx.navmodel.backstack.operation.push
 import com.bumble.appyx.navmodel.backstack.operation.replace
-import com.bumble.appyx.navmodel.backstack.operation.singleTop
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.anvilannotations.ContributesNode
@@ -60,6 +59,7 @@ import io.element.android.features.securebackup.api.SecureBackupEntryPoint
 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.architecture.waitForNavTargetAttached
 import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
 import io.element.android.libraries.di.AppScope
 import io.element.android.libraries.di.SessionScope
@@ -345,11 +345,6 @@ class LoggedInFlowNode @AssistedInject constructor(
             }
             NavTarget.Ftue -> {
                 ftueEntryPoint.nodeBuilder(this, buildContext)
-                    .callback(object : FtueEntryPoint.Callback {
-                        override fun onFtueFlowFinished() {
-                            lifecycleScope.launch { attachRoomList() }
-                        }
-                    })
                     .build()
             }
             NavTarget.RoomDirectorySearch -> {
@@ -368,32 +363,22 @@ class LoggedInFlowNode @AssistedInject constructor(
         }
     }
 
-    suspend fun attachRoomList() {
-        if (!canShowRoomList()) return
-        attachChild<Node> {
-            backstack.singleTop(NavTarget.RoomList)
-        }
-    }
-
     suspend fun attachRoom(roomId: RoomId) {
-        if (!canShowRoomList()) return
+        waitForNavTargetAttached { navTarget ->
+            navTarget is NavTarget.RoomList
+        }
         attachChild<RoomFlowNode> {
-            backstack.singleTop(NavTarget.RoomList)
             backstack.push(NavTarget.Room(roomId.toRoomIdOrAlias()))
         }
     }
 
-    private fun canShowRoomList(): Boolean {
-        return ftueService.state.value is FtueState.Complete
-    }
-
     @Composable
     override fun View(modifier: Modifier) {
         Box(modifier = modifier) {
             val lockScreenState by lockScreenStateService.lockState.collectAsState()
-            val isFtueDisplayed by ftueService.state.collectAsState()
+            val ftueState by ftueService.state.collectAsState()
             BackstackView()
-            if (isFtueDisplayed is FtueState.Complete) {
+            if (ftueState is FtueState.Complete) {
                 PermanentChild(permanentNavModel = permanentNavModel, navTarget = NavTarget.LoggedInPermanent)
             }
             if (lockScreenState == LockScreenLockState.Locked) {

appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt

@@ -288,7 +288,7 @@ class RootFlowNode @AssistedInject constructor(
             .attachSession()
             .apply {
                 when (deeplinkData) {
-                    is DeeplinkData.Root -> attachRoomList()
+                    is DeeplinkData.Root -> Unit // The room list will always be shown, observing FtueState
                     is DeeplinkData.Room -> attachRoom(deeplinkData.roomId)
                 }
             }

changelog.d/2778.bugfix

@@ -0,0 +1 @@
+Ensure the application open the room when a notification is clicked.

features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/FtueEntryPoint.kt

@@ -18,18 +18,12 @@ package io.element.android.features.ftue.api
 
 import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.node.Node
-import com.bumble.appyx.core.plugin.Plugin
 import io.element.android.libraries.architecture.FeatureEntryPoint
 
 interface FtueEntryPoint : FeatureEntryPoint {
     fun nodeBuilder(parentNode: Node, buildContext: BuildContext): NodeBuilder
 
     interface NodeBuilder {
-        fun callback(callback: Callback): NodeBuilder
         fun build(): Node
     }
-
-    interface Callback : Plugin {
-        fun onFtueFlowFinished()
-    }
 }

features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPoint.kt

@@ -31,11 +31,6 @@ class DefaultFtueEntryPoint @Inject constructor() : FtueEntryPoint {
         val plugins = ArrayList<Plugin>()
 
         return object : FtueEntryPoint.NodeBuilder {
-            override fun callback(callback: FtueEntryPoint.Callback): FtueEntryPoint.NodeBuilder {
-                plugins += callback
-                return this
-            }
-
             override fun build(): Node {
                 return parentNode.createNode<FtueFlowNode>(buildContext, plugins)
             }

features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt

@@ -32,7 +32,6 @@ import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.anvilannotations.ContributesNode
 import io.element.android.features.analytics.api.AnalyticsEntryPoint
-import io.element.android.features.ftue.api.FtueEntryPoint
 import io.element.android.features.ftue.impl.notifications.NotificationsOptInNode
 import io.element.android.features.ftue.impl.sessionverification.FtueSessionVerificationFlowNode
 import io.element.android.features.ftue.impl.state.DefaultFtueService
@@ -86,8 +85,6 @@ class FtueFlowNode @AssistedInject constructor(
         data object LockScreenSetup : NavTarget
     }
 
-    private val callback = plugins.filterIsInstance<FtueEntryPoint.Callback>().firstOrNull()
-
     override fun onBuilt() {
         super.onBuilt()
 
@@ -157,7 +154,7 @@ class FtueFlowNode @AssistedInject constructor(
             FtueStep.LockscreenSetup -> {
                 backstack.newRoot(NavTarget.LockScreenSetup)
             }
-            null -> callback?.onFtueFlowFinished()
+            null -> Unit
         }
     }
 

libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt

@@ -20,6 +20,7 @@ import androidx.lifecycle.lifecycleScope
 import com.bumble.appyx.core.children.nodeOrNull
 import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.node.ParentNode
+import kotlinx.coroutines.cancel
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlin.coroutines.resume
@@ -48,3 +49,23 @@ suspend inline fun <reified N : Node, NavTarget : Any> ParentNode<NavTarget>.wai
             continuation.cancel()
         }
     }
+
+/**
+ * Wait for a child to be attached to the parent node, only using the NavTarget.
+ */
+suspend inline fun <NavTarget : Any> ParentNode<NavTarget>.waitForNavTargetAttached(crossinline predicate: (NavTarget) -> Boolean) =
+    suspendCancellableCoroutine { continuation ->
+        lifecycleScope.launch {
+            children.collect { childMap ->
+                val node = childMap.entries
+                    .map { it.key.navTarget }
+                    .lastOrNull(predicate)
+                if (node != null && !continuation.isCompleted) {
+                    continuation.resume(Unit)
+                    cancel()
+                }
+            }
+        }.invokeOnCompletion {
+            continuation.cancel()
+        }
+    }