Skip to content

Commit

Permalink
Fixed bug
Browse files Browse the repository at this point in the history
  • Loading branch information
isaac-udy committed Aug 19, 2024
1 parent d7a6eae commit d6a034f
Show file tree
Hide file tree
Showing 6 changed files with 275 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## 2.8.0
* Added support for NavigationKey.WithExtras to `NavigationResultChannel` and `NavigationFlowScope`
* Updated `enro-test` methods to provide more descriptive error messages when assert/expect methods fail, and added kdoc comments to many of the functions
* Fixed a bug where managed flows (`registerForFlowResult`) that launch embedded flows (`deliverResultFromPush/Present`) were not correctly handling the result of the embedded flow

## 2.7.0
* ⚠️ Updated to androidx.lifecycle 2.8.1 ⚠️
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import dev.enro.core.controller.interceptor.NavigationInstructionInterceptor
import dev.enro.core.controller.usecase.AddPendingResult
import dev.enro.core.navigationContext
import dev.enro.core.readOpenInstruction
import dev.enro.core.result.flows.FlowStep
import dev.enro.core.rootContext

internal object ForwardingResultInterceptor : NavigationInstructionInterceptor {
Expand All @@ -37,7 +38,7 @@ internal object ForwardingResultInterceptor : NavigationInstructionInterceptor
val next = containers.removeAt(0)
val filteredBackstack = next.backstack
.filterNot {
it.instructionId == forwardingResultId ||
(it.instructionId == forwardingResultId && it.internal.resultKey !is FlowStep<*>) ||
AdvancedResultExtensions.getForwardingInstructionId(it) == forwardingResultId
}
.toBackstack()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import dev.enro.test.application.compose.SyntheticViewModelAccessRobot
import dev.enro.test.application.compose.results.ComposeAsyncManagedResultFlowRobot
import dev.enro.test.application.compose.results.ComposeEmbeddedResultFlowRobot
import dev.enro.test.application.compose.results.ComposeManagedResultFlowRobot
import dev.enro.test.application.compose.results.ComposeManagedResultsWithNestedFlowAndEmptyRootRobot
import dev.enro.test.application.compose.results.ComposeNestedResultsRobot
import dev.enro.test.application.compose.results.ResultsWithExtraRobot
import dev.enro.test.application.fragment.UnboundBottomSheetRobot
Expand Down Expand Up @@ -161,4 +162,14 @@ class SelectDestinationRobot(

return ResultsWithExtraRobot(composeRule)
}

fun openComposeManagedResultsWithNestedFlowAndEmptyRoot() : ComposeManagedResultsWithNestedFlowAndEmptyRootRobot {
composeRule.onNode(hasText("Compose Managed Results With Nested Flow And Empty Root"))
.performScrollTo()
.onSiblings()
.filterToOne(hasText("Push"))
.performClick()

return ComposeManagedResultsWithNestedFlowAndEmptyRootRobot(composeRule)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package dev.enro.test.application.compose.results

import androidx.compose.ui.test.ExperimentalTestApi
import androidx.compose.ui.test.hasText
import androidx.compose.ui.test.junit4.ComposeTestRule
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick

@OptIn(ExperimentalTestApi::class)
class ComposeManagedResultsWithNestedFlowAndEmptyRootRobot(
private val composeRule: ComposeTestRule
) {

fun assertNestedFlowIsDisplayed() : ComposeManagedResultsWithNestedFlowAndEmptyRootRobot {
composeRule.waitUntilAtLeastOneExists(hasText("Nested Flow"))
composeRule.onNodeWithText("Nested Flow")
.assertExists()
return this
}

fun assertOuterStepTwoIsDisplayed() : ComposeManagedResultsWithNestedFlowAndEmptyRootRobot {
composeRule.waitUntilAtLeastOneExists(hasText("Step Two"))
composeRule.onNodeWithText("Step Two")
.assertExists()
return this
}

fun goToNestedStepOne(): ComposeManagedResultsWithNestedFlowAndEmptyRootRobot {
composeRule.onNodeWithText("Next (to nested step one)")
.performClick()
return this
}

fun goToNestedStepTwo(): ComposeManagedResultsWithNestedFlowAndEmptyRootRobot {
composeRule.onNodeWithText("Next (to nested step two)")
.performClick()
return this
}

fun setResultOnNestedStepTwo(): ComposeManagedResultsWithNestedFlowAndEmptyRootRobot {
composeRule.onNodeWithText("Sheep")
.performClick()
return this
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package dev.enro.test.application.compose.results

import androidx.compose.ui.test.junit4.createAndroidComposeRule
import dev.enro.test.application.SelectDestinationRobot
import dev.enro.tests.application.TestActivity
import org.junit.Rule
import org.junit.Test

class ComposeManagedResultsWithNestedFlowAndEmptyRootTest {

@get:Rule
val composeRule = createAndroidComposeRule<TestActivity>()

@Test
fun test() {
SelectDestinationRobot(composeRule)
.openComposeManagedResultsWithNestedFlowAndEmptyRoot()
.assertNestedFlowIsDisplayed()
.goToNestedStepOne()
.goToNestedStepTwo()
.setResultOnNestedStepTwo()
.assertOuterStepTwoIsDisplayed()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
package dev.enro.tests.application.compose.results

import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.material.Button
import androidx.compose.material.CircularProgressIndicator
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewmodel.compose.viewModel
import dev.enro.annotations.AdvancedEnroApi
import dev.enro.annotations.ExperimentalEnroApi
import dev.enro.annotations.NavigationDestination
import dev.enro.core.NavigationKey
import dev.enro.core.closeWithResult
import dev.enro.core.compose.navigationHandle
import dev.enro.core.compose.rememberNavigationContainer
import dev.enro.core.container.EmptyBehavior
import dev.enro.core.result.deliverResultFromPush
import dev.enro.core.result.flows.registerForFlowResult
import dev.enro.tests.application.compose.common.TitledColumn
import dev.enro.viewmodel.navigationHandle
import kotlinx.coroutines.delay
import kotlinx.parcelize.Parcelize

/**
* A bug in Enro was identified, where a managed flow that has no root would not correctly work when navigating to a nested flow
* that uses deliverResultForPush/Present.
*
* The issue is described as follows:
* 1. Create a managed flow with a "CloseParent" empty behaviour
* 2. Before the first navigation event, perform an async action to get some information
* 3. The first step should navigate to an embedded flow
* 4. When the final screen of the embedded flow returns a result, the parent flow will close (but it shouldn't)
*
* This bug was fixed by ensuring that the ForwardingResultInterceptor does not automatically close the original destination
* that called deliverResultForPush/Present if that destination was launched in a managed flow.
*/
@Parcelize
object ComposeManagedResultsWithNestedFlowAndEmptyRoot : NavigationKey.SupportsPush.WithResult<String> {

@Parcelize
internal class NestedFlow : NavigationKey.SupportsPush.WithResult<String> {
@Parcelize
internal class StepOne : NavigationKey.SupportsPush.WithResult<String>

@Parcelize
internal class StepTwo : NavigationKey.SupportsPush.WithResult<String>
}

@Parcelize
internal class StepTwo : NavigationKey.SupportsPush.WithResult<String>

@Parcelize
internal class FinalScreen : NavigationKey.SupportsPush.WithResult<String>

@OptIn(ExperimentalEnroApi::class, AdvancedEnroApi::class)
internal class FlowViewModel(
private val savedStateHandle: SavedStateHandle,
) : ViewModel() {
private val navigation by navigationHandle<ComposeManagedResultsWithNestedFlowAndEmptyRoot>()
private val flow by registerForFlowResult(
savedStateHandle = savedStateHandle,
flow = {
val started = async { getAsyncStarter() }
val nestedResult = push { NestedFlow() }
val secondResult = push { StepTwo() }
val finalResult = push { FinalScreen() }
return@registerForFlowResult "$started\n$nestedResult\n$secondResult\n$finalResult"
},
onCompleted = { result ->
navigation.closeWithResult(result)
}
)

private suspend fun getAsyncStarter(): String {
delay(350)
return "Flow Started:"
}
}
}

@NavigationDestination(ComposeManagedResultsWithNestedFlowAndEmptyRoot::class)
@Composable
internal fun ComposeManagedResultsWithNestedFlowAndEmptyRootDestination(
viewModel: ComposeManagedResultsWithNestedFlowAndEmptyRoot.FlowViewModel = viewModel()
) {
val container = rememberNavigationContainer(
emptyBehavior = EmptyBehavior.CloseParent,
)
TitledColumn(
title = "Results with Nested Flow and Empty Root"
) {
Box(Modifier.fillMaxSize()) {
container.Render()
if (container.backstack.isEmpty()) {
CircularProgressIndicator(modifier = Modifier.align(Alignment.Center))
}
}
}
}

@Composable
@NavigationDestination(ComposeManagedResultsWithNestedFlowAndEmptyRoot.NestedFlow::class)
internal fun CmrwnfNestedFlowDestination() {
val navigation = navigationHandle<ComposeManagedResultsWithNestedFlowAndEmptyRoot.NestedFlow>()
TitledColumn(
title = "Nested Flow"
) {
Button(onClick = {
navigation.deliverResultFromPush(ComposeManagedResultsWithNestedFlowAndEmptyRoot.NestedFlow.StepOne())
}) {
Text(text = "Next (to nested step one)")
}
}
}

@Composable
@NavigationDestination(ComposeManagedResultsWithNestedFlowAndEmptyRoot.NestedFlow.StepOne::class)
internal fun CmrwnfNestedStepOneDestination() {
val navigation = navigationHandle<ComposeManagedResultsWithNestedFlowAndEmptyRoot.NestedFlow.StepOne>()
TitledColumn(
title = "Nested Step One"
) {
Button(onClick = {
navigation.deliverResultFromPush(ComposeManagedResultsWithNestedFlowAndEmptyRoot.NestedFlow.StepTwo())
}) {
Text(text = "Next (to nested step two)")
}
}
}

@Composable
@NavigationDestination(ComposeManagedResultsWithNestedFlowAndEmptyRoot.NestedFlow.StepTwo::class)
internal fun CmrwnfNestedStepTwoDestination() {
val navigation = navigationHandle<ComposeManagedResultsWithNestedFlowAndEmptyRoot.NestedFlow.StepTwo>()
TitledColumn(
title = "Nested Step Two"
) {
Button(onClick = {
navigation.closeWithResult("Cow")
}) {
Text(text = "Cow")
}
Button(onClick = {
navigation.closeWithResult("Sheep")
}) {
Text(text = "Sheep")
}
}
}

@Composable
@NavigationDestination(ComposeManagedResultsWithNestedFlowAndEmptyRoot.StepTwo::class)
internal fun CmrwnfFlowStepTwoDestination() {
val navigation = navigationHandle<ComposeManagedResultsWithNestedFlowAndEmptyRoot.StepTwo>()
TitledColumn(
title = "Step Two"
) {
Button(onClick = {
navigation.closeWithResult("House")
}) {
Text(text = "House")
}
Button(onClick = {
navigation.closeWithResult("Farm")
}) {
Text(text = "Farm")
}
}
}

@Composable
@NavigationDestination(ComposeManagedResultsWithNestedFlowAndEmptyRoot.FinalScreen::class)
internal fun CmrwnfFlowFinalScreenDestination() {
val navigation = navigationHandle<ComposeManagedResultsWithNestedFlowAndEmptyRoot.FinalScreen>()
TitledColumn(
title = "Final Screen"
) {
Button(onClick = {
navigation.closeWithResult("End")
}) {
Text(text = "End")
}
}
}

0 comments on commit d6a034f

Please sign in to comment.