Skip to content

Commit

Permalink
Fix app crash when navigation start destination is set to map (#3331)
Browse files Browse the repository at this point in the history
* Fix missing ID for launcher type

Signed-off-by: Elly Kitoto <[email protected]>

* Fix setting start destination on nav graph

Signed-off-by: Elly Kitoto <[email protected]>

* Fix failing tests

Signed-off-by: Elly Kitoto <[email protected]>

* Fix failing ui test

Signed-off-by: Elly Kitoto <[email protected]>

---------

Signed-off-by: Elly Kitoto <[email protected]>
Co-authored-by: Benjamin Mwalimu <[email protected]>
  • Loading branch information
ellykits and dubdabasoduba authored Jun 20, 2024
1 parent 8463098 commit 742fdeb
Show file tree
Hide file tree
Showing 14 changed files with 147 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ data class ApplicationConfiguration(
),
val logGpsLocation: List<LocationLogOptions> = emptyList(),
val usePractitionerAssignedLocationOnSync: Boolean = true,
val navigationStartDestination: LauncherType = LauncherType.REGISTER,
val navigationStartDestination: NavigationStartDestinationConfig =
NavigationStartDestinationConfig(
launcherType = LauncherType.REGISTER,
id = null,
),
val codingSystems: List<CodingSystemConfig> = emptyList(),
) : Configuration()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 2021-2024 Ona Systems, Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.smartregister.fhircore.engine.configuration.app

import android.os.Parcelable
import kotlinx.parcelize.Parcelize
import kotlinx.serialization.Serializable
import org.smartregister.fhircore.engine.domain.model.LauncherType

/**
* This class configures the initial screen the application will be directed to upon launch. The
* application currently supports [LauncherType.REGISTER] and [LauncherType.MAP] screen as the entry
* point. This config defaults to launching a register with the rest of the properties obtained from
* the first NavigationItemConfig of the NavigationConfiguration
*/
@Serializable
@Parcelize
data class NavigationStartDestinationConfig(
val id: String? = null,
val screenTitle: String? = null,
val launcherType: LauncherType = LauncherType.REGISTER,
) : Parcelable
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ data class GeoWidgetConfiguration(
override var appId: String,
override var configType: String = ConfigType.GeoWidget.name,
val id: String,
val profileId: String,
val topScreenSection: TopScreenSectionConfig? = null,
val registrationQuestionnaire: QuestionnaireConfig,
val mapLayers: List<MapLayerConfig> = listOf(MapLayerConfig(MapLayer.STREET, true)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.createAndroidComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.core.os.bundleOf
import androidx.navigation.findNavController
import androidx.test.core.app.ApplicationProvider.getApplicationContext
import androidx.test.platform.app.InstrumentationRegistry
import androidx.test.rule.GrantPermissionRule
Expand Down Expand Up @@ -94,7 +95,7 @@ class AppMainActivityTest {
grantPermission()
Assert.assertEquals(
R.id.registerFragment,
it.navHostFragment.navController.currentDestination?.id,
it.findNavController(R.id.nav_host).currentDestination?.id,
)
}
composeTestRule.onNodeWithTag(REGISTER_SCREEN_BOX_TAG).assertIsDisplayed()
Expand All @@ -105,7 +106,7 @@ class AppMainActivityTest {
fun navigationToUserSettingFragmentShouldShowUserSettingsScreen() {
composeTestRule.activityRule.scenario.onActivity {
grantPermission()
it.navHostFragment.navController.navigate(R.id.userSettingFragment)
it.findNavController(R.id.nav_host).navigate(R.id.userSettingFragment)
}

composeTestRule.onNodeWithTag(USER_SETTING_ROW_LOGOUT).assertExists()
Expand All @@ -119,21 +120,23 @@ class AppMainActivityTest {

composeTestRule.activityRule.scenario.onActivity {
grantPermission()
it.navHostFragment.navController.navigate(
R.id.profileFragment,
bundleOf(
NavigationArg.PROFILE_ID to "defaultProfile",
NavigationArg.RESOURCE_CONFIG to resourceConfig,
NavigationArg.PARAMS to
arrayOf(
ActionParameter(
key = "anyId",
paramType = ActionParameterType.PARAMDATA,
value = "anyValue",
it
.findNavController(R.id.nav_host)
.navigate(
R.id.profileFragment,
bundleOf(
NavigationArg.PROFILE_ID to "defaultProfile",
NavigationArg.RESOURCE_CONFIG to resourceConfig,
NavigationArg.PARAMS to
arrayOf(
ActionParameter(
key = "anyId",
paramType = ActionParameterType.PARAMDATA,
value = "anyValue",
),
),
),
),
)
),
)
}

composeTestRule.onNodeWithTag(PROFILE_TOP_BAR_TEST_TAG).assertIsDisplayed()
Expand All @@ -144,13 +147,15 @@ class AppMainActivityTest {
fun navigationToMeasureReportFragmentShouldShowMeasureReportScreen() {
composeTestRule.activityRule.scenario.onActivity {
grantPermission()
it.navHostFragment.navController.navigate(
R.id.measureReportFragment,
bundleOf(
NavigationArg.REPORT_ID to "serviceDeliveryMeasureReport",
NavigationArg.RESOURCE_ID to "",
),
)
it
.findNavController(R.id.nav_host)
.navigate(
R.id.measureReportFragment,
bundleOf(
NavigationArg.REPORT_ID to "serviceDeliveryMeasureReport",
NavigationArg.RESOURCE_ID to "",
),
)
}

composeTestRule.onNodeWithTag(SCREEN_TITLE).assertIsDisplayed()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,8 @@ object NavigationArg {
const val RESOURCE_ID = "resourceId"
const val RESOURCE_CONFIG = "resourceConfig"
const val MULTI_SELECT_VIEW_CONFIG = "multiSelectViewConfig"
const val CONFIG_ID = "configId"
const val GEO_WIDGET_ID = "geoWidgetId"
const val DETAILS_BOTTOM_SHEET_CONFIG = "detailsBottomSheetConfig"
const val REPORT_ID = "reportId"
const val PARAMS = "params"
const val TOOL_BAR_HOME_NAVIGATION = "toolBarHomeNavigation"
const val LAUNCHER_TYPE = "launcherType"

/** Create route paths */
fun routePathsOf(vararg navArg: String): String =
"?" + navArg.toList().joinToString("&") { "$it={$it}" }

/** Bind nav arguments values */
fun bindArgumentsOf(vararg navArg: Pair<String, String?>): String =
"?" + navArg.joinToString("&") { "${it.first}=${it.second}" }
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,32 +74,21 @@ import timber.log.Timber

@AndroidEntryPoint
class GeoWidgetLauncherFragment : Fragment() {

@Inject lateinit var eventBus: EventBus

@Inject lateinit var configurationRegistry: ConfigurationRegistry
private lateinit var geoWidgetFragment: GeoWidgetFragment
private lateinit var geoWidgetConfiguration: GeoWidgetConfiguration
private val geoWidgetLauncherViewModel by viewModels<GeoWidgetLauncherViewModel>()
private val args by navArgs<GeoWidgetLauncherFragmentArgs>()
private val geoWidgetConfiguration: GeoWidgetConfiguration by lazy {
configurationRegistry.retrieveConfiguration(
ConfigType.GeoWidget,
args.geoWidgetId,
emptyMap(),
)
}
private val navArgs by navArgs<GeoWidgetLauncherFragmentArgs>()
private val appMainViewModel by activityViewModels<AppMainViewModel>()

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
Timber.i("GeoWidgetLauncherFragment onCreate")
}

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?,
): View {
Timber.i("GeoWidgetLauncherFragment onCreateView")
buildGeoWidgetFragment()

return ComposeView(requireContext()).apply {
Expand Down Expand Up @@ -158,7 +147,7 @@ class GeoWidgetLauncherFragment : Fragment() {
openDrawer = openDrawer,
onEvent = geoWidgetLauncherViewModel::onEvent,
navController = findNavController(),
toolBarHomeNavigation = args.toolBarHomeNavigation,
toolBarHomeNavigation = navArgs.toolBarHomeNavigation,
modifier = Modifier.fillMaxSize(), // Adjust the modifier as needed
fragmentManager = childFragmentManager,
fragment = fragment,
Expand All @@ -181,6 +170,11 @@ class GeoWidgetLauncherFragment : Fragment() {
}

private fun buildGeoWidgetFragment() {
geoWidgetConfiguration =
configurationRegistry.retrieveConfiguration<GeoWidgetConfiguration>(
configType = ConfigType.GeoWidget,
configId = navArgs.geoWidgetId,
)
geoWidgetFragment =
GeoWidgetFragment.builder()
.setUseGpsOnAddingLocation(false)
Expand Down Expand Up @@ -249,8 +243,4 @@ class GeoWidgetLauncherFragment : Fragment() {
}
}
}

companion object {
const val GEO_WIDGET_FRAGMENT_TAG = "geo-widget-fragment-tag"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ import androidx.activity.result.contract.ActivityResultContracts
import androidx.activity.viewModels
import androidx.compose.material.ExperimentalMaterialApi
import androidx.core.os.bundleOf
import androidx.fragment.app.FragmentContainerView
import androidx.lifecycle.lifecycleScope
import androidx.navigation.findNavController
import androidx.navigation.fragment.NavHostFragment
import com.google.android.fhir.FhirEngine
import com.google.android.fhir.sync.CurrentSyncJobStatus
import com.google.android.gms.location.FusedLocationProviderClient
import com.google.android.gms.location.LocationServices
Expand All @@ -46,7 +45,6 @@ import kotlinx.coroutines.runBlocking
import org.hl7.fhir.r4.model.IdType
import org.hl7.fhir.r4.model.QuestionnaireResponse
import org.smartregister.fhircore.engine.configuration.QuestionnaireConfig
import org.smartregister.fhircore.engine.configuration.app.ConfigService
import org.smartregister.fhircore.engine.configuration.app.LocationLogOptions
import org.smartregister.fhircore.engine.configuration.app.SyncStrategy
import org.smartregister.fhircore.engine.configuration.workflow.ActionTrigger
Expand All @@ -55,10 +53,8 @@ import org.smartregister.fhircore.engine.datastore.syncLocationIdsProtoStore
import org.smartregister.fhircore.engine.domain.model.LauncherType
import org.smartregister.fhircore.engine.rulesengine.services.LocationCoordinate
import org.smartregister.fhircore.engine.sync.OnSyncListener
import org.smartregister.fhircore.engine.sync.SyncBroadcaster
import org.smartregister.fhircore.engine.sync.SyncListenerManager
import org.smartregister.fhircore.engine.ui.base.BaseMultiLanguageActivity
import org.smartregister.fhircore.engine.util.DefaultDispatcherProvider
import org.smartregister.fhircore.engine.util.extension.isDeviceOnline
import org.smartregister.fhircore.engine.util.extension.parcelable
import org.smartregister.fhircore.engine.util.extension.serializable
Expand All @@ -78,20 +74,11 @@ import timber.log.Timber
@ExperimentalMaterialApi
open class AppMainActivity : BaseMultiLanguageActivity(), QuestionnaireHandler, OnSyncListener {

@Inject lateinit var dispatcherProvider: DefaultDispatcherProvider

@Inject lateinit var configService: ConfigService

@Inject lateinit var syncListenerManager: SyncListenerManager

@Inject lateinit var syncBroadcaster: SyncBroadcaster

@Inject lateinit var fhirEngine: FhirEngine

@Inject lateinit var protoDataStore: ProtoDataStore

@Inject lateinit var eventBus: EventBus
lateinit var navHostFragment: NavHostFragment
val appMainViewModel by viewModels<AppMainViewModel>()
private val sentryNavListener =
SentryNavigationListener(enableNavigationBreadcrumbs = true, enableNavigationTracing = true)
Expand All @@ -106,27 +93,56 @@ open class AppMainActivity : BaseMultiLanguageActivity(), QuestionnaireHandler,
}
}

/**
* When the NavHostFragment is inflated using FragmentContainerView, if you attempt to use
* findNavController in the onCreate() of the Activity, the nav controller cannot be found. This
* is because when the fragment is inflated in the constructor of FragmentContainerView, the
* fragmentManager is in the INITIALIZING state, and therefore the added fragment only goes up to
* initializing. For the nav controller to be properly set, the fragment view needs to be created
* and onViewCreated() needs to be dispatched, which does not happen until the ACTIVITY_CREATED
* state. As a workaround retrieve the navController from the [NavHostFragment]
*/
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setupLocationServices()
setContentView(FragmentContainerView(this).apply { id = R.id.nav_host })
val topMenuConfig = appMainViewModel.navigationConfiguration.clientRegisters.first()
val clickAction = topMenuConfig.actions?.find { it.trigger == ActionTrigger.ON_CLICK }
val topMenuConfigId = clickAction?.id ?: topMenuConfig.id
navHostFragment =
NavHostFragment.create(
R.navigation.application_nav_graph,
bundleOf(
NavigationArg.SCREEN_TITLE to topMenuConfig.display,
NavigationArg.REGISTER_ID to topMenuConfigId,
),
)
setContentView(R.layout.activity_main)

supportFragmentManager
.beginTransaction()
.replace(R.id.nav_host, navHostFragment)
.setPrimaryNavigationFragment(navHostFragment)
.commit()
val startDestinationConfig =
appMainViewModel.applicationConfiguration.navigationStartDestination
val startDestinationArgs =
when (startDestinationConfig.launcherType) {
LauncherType.REGISTER -> {
val topMenuConfig = appMainViewModel.navigationConfiguration.clientRegisters.first()
val clickAction = topMenuConfig.actions?.find { it.trigger == ActionTrigger.ON_CLICK }
bundleOf(
NavigationArg.SCREEN_TITLE to
if (startDestinationConfig.screenTitle.isNullOrEmpty()) {
topMenuConfig.display
} else startDestinationConfig.screenTitle,
NavigationArg.REGISTER_ID to
if (startDestinationConfig.id.isNullOrEmpty()) {
clickAction?.id ?: topMenuConfig.id
} else startDestinationConfig.id,
)
}
LauncherType.MAP -> bundleOf(NavigationArg.GEO_WIDGET_ID to startDestinationConfig.id)
}

// Retrieve the navController directly from the NavHostFragment
val navController =
(supportFragmentManager.findFragmentById(R.id.nav_host) as NavHostFragment).navController

val graph =
navController.navInflater.inflate(R.navigation.application_nav_graph).apply {
val startDestination =
when (appMainViewModel.applicationConfiguration.navigationStartDestination.launcherType) {
LauncherType.MAP -> R.id.geoWidgetLauncherFragment
LauncherType.REGISTER -> R.id.registerFragment
}
setStartDestination(startDestination)
}

navController.setGraph(graph, startDestinationArgs)

// Register sync listener then run sync in that order
syncListenerManager.registerSyncListener(this, lifecycle)
Expand Down Expand Up @@ -160,25 +176,12 @@ open class AppMainActivity : BaseMultiLanguageActivity(), QuestionnaireHandler,

override fun onResume() {
super.onResume()
// Create NavController after fragment has been attached
navHostFragment.apply {
val graph =
navController.navInflater.inflate(R.navigation.application_nav_graph).apply {
val startDestination =
when (appMainViewModel.applicationConfiguration.navigationStartDestination) {
LauncherType.MAP -> R.id.geoWidgetLauncherFragment
else -> R.id.registerFragment
}
setStartDestination(startDestination)
}
navController.addOnDestinationChangedListener(sentryNavListener)
navController.graph = graph
}
findNavController(R.id.nav_host).addOnDestinationChangedListener(sentryNavListener)
}

override fun onPause() {
super.onPause()
navHostFragment.navController.removeOnDestinationChangedListener(sentryNavListener)
findNavController(R.id.nav_host).removeOnDestinationChangedListener(sentryNavListener)
}

override suspend fun onSubmitQuestionnaire(activityResult: ActivityResult) {
Expand Down
Loading

0 comments on commit 742fdeb

Please sign in to comment.