From 759d80642071039ed004f34f26d7faf0fb50e6c8 Mon Sep 17 00:00:00 2001 From: Akshay Nandwana Date: Sat, 7 Dec 2024 00:26:41 +0530 Subject: [PATCH] Removed remaining usages of navigator (#2898) --- .../com/google/android/ground/MainActivity.kt | 31 ++---- .../ground/ui/common/NavigationRequest.kt | 26 ----- .../android/ground/ui/common/Navigator.kt | 66 ----------- .../ui/offlineareas/OfflineAreasFragment.kt | 9 +- .../ui/offlineareas/OfflineAreasViewModel.kt | 2 +- .../android/ground/NavigationTestHelper.kt | 105 ------------------ .../android/ground/ui/common/NavigatorTest.kt | 55 --------- .../SurveySelectorFragmentTest.kt | 13 ++- .../ui/tos/TermsOfServiceFragmentTest.kt | 2 - 9 files changed, 26 insertions(+), 283 deletions(-) delete mode 100644 ground/src/main/java/com/google/android/ground/ui/common/NavigationRequest.kt delete mode 100644 ground/src/main/java/com/google/android/ground/ui/common/Navigator.kt delete mode 100644 ground/src/test/java/com/google/android/ground/NavigationTestHelper.kt delete mode 100644 ground/src/test/java/com/google/android/ground/ui/common/NavigatorTest.kt diff --git a/ground/src/main/java/com/google/android/ground/MainActivity.kt b/ground/src/main/java/com/google/android/ground/MainActivity.kt index 7c600794cf..d085ad4460 100644 --- a/ground/src/main/java/com/google/android/ground/MainActivity.kt +++ b/ground/src/main/java/com/google/android/ground/MainActivity.kt @@ -27,17 +27,13 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.platform.ComposeView import androidx.core.view.WindowInsetsCompat import androidx.lifecycle.lifecycleScope +import androidx.navigation.NavDirections import androidx.navigation.fragment.NavHostFragment import com.google.android.ground.databinding.MainActBinding import com.google.android.ground.repository.UserRepository import com.google.android.ground.system.ActivityCallback import com.google.android.ground.system.ActivityStreams import com.google.android.ground.ui.common.BackPressListener -import com.google.android.ground.ui.common.FinishApp -import com.google.android.ground.ui.common.NavigateTo -import com.google.android.ground.ui.common.NavigateUp -import com.google.android.ground.ui.common.NavigationRequest -import com.google.android.ground.ui.common.Navigator import com.google.android.ground.ui.common.ViewModelFactory import com.google.android.ground.ui.common.modalSpinner import com.google.android.ground.ui.home.HomeScreenFragmentDirections @@ -57,7 +53,6 @@ import timber.log.Timber class MainActivity : AbstractActivity() { @Inject lateinit var activityStreams: ActivityStreams @Inject lateinit var viewModelFactory: ViewModelFactory - @Inject lateinit var navigator: Navigator @Inject lateinit var userRepository: UserRepository private lateinit var viewModel: MainViewModel @@ -79,8 +74,6 @@ class MainActivity : AbstractActivity() { } } - lifecycleScope.launch { navigator.getNavigateRequests().collect { onNavigate(it) } } - val binding = MainActBinding.inflate(layoutInflater) setContentView(binding.root) @@ -100,16 +93,16 @@ class MainActivity : AbstractActivity() { showPermissionDeniedDialog(viewGroup) } MainUiState.OnUserSignedOut -> { - navigator.navigate(SignInFragmentDirections.showSignInScreen()) + navigateTo(SignInFragmentDirections.showSignInScreen()) } MainUiState.TosNotAccepted -> { - navigator.navigate(SignInFragmentDirections.showTermsOfService(false)) + navigateTo(SignInFragmentDirections.showTermsOfService(false)) } MainUiState.NoActiveSurvey -> { - navigator.navigate(SurveySelectorFragmentDirections.showSurveySelectorScreen(true)) + navigateTo(SurveySelectorFragmentDirections.showSurveySelectorScreen(true)) } MainUiState.ShowHomeScreen -> { - navigator.navigate(HomeScreenFragmentDirections.showHomeScreen()) + navigateTo(HomeScreenFragmentDirections.showHomeScreen()) } MainUiState.OnUserSigningIn -> { onSignInProgress(true) @@ -137,7 +130,7 @@ class MainActivity : AbstractActivity() { }, onCloseApp = { showDialog = false - navigator.finishApp() + finish() }, ) } @@ -152,14 +145,6 @@ class MainActivity : AbstractActivity() { viewModel.windowInsets.postValue(insets) } - private fun onNavigate(navRequest: NavigationRequest) { - when (navRequest) { - is NavigateTo -> navHostFragment.navController.navigate(navRequest.directions) - is NavigateUp -> navigateUp() - is FinishApp -> finish() - } - } - /** * The Android permissions API requires this callback to live in an Activity; here we dispatch the * result back to the PermissionManager for handling. @@ -225,4 +210,8 @@ class MainActivity : AbstractActivity() { signInProgressDialog = null } } + + private fun navigateTo(directions: NavDirections) { + navHostFragment.navController.navigate(directions) + } } diff --git a/ground/src/main/java/com/google/android/ground/ui/common/NavigationRequest.kt b/ground/src/main/java/com/google/android/ground/ui/common/NavigationRequest.kt deleted file mode 100644 index 86ca870c59..0000000000 --- a/ground/src/main/java/com/google/android/ground/ui/common/NavigationRequest.kt +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * 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 - * - * https://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 com.google.android.ground.ui.common - -import androidx.navigation.NavDirections - -sealed interface NavigationRequest - -data class NavigateTo(val directions: NavDirections) : NavigationRequest - -data object NavigateUp : NavigationRequest - -data object FinishApp : NavigationRequest diff --git a/ground/src/main/java/com/google/android/ground/ui/common/Navigator.kt b/ground/src/main/java/com/google/android/ground/ui/common/Navigator.kt deleted file mode 100644 index 87375956ba..0000000000 --- a/ground/src/main/java/com/google/android/ground/ui/common/Navigator.kt +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright 2018 Google LLC - * - * 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 - * - * https://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 com.google.android.ground.ui.common - -import androidx.navigation.NavDirections -import com.google.android.ground.coroutines.MainDispatcher -import com.google.android.ground.coroutines.MainScope -import javax.inject.Inject -import javax.inject.Singleton -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.SharedFlow -import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.shareIn -import kotlinx.coroutines.launch - -/** - * Responsible for abstracting navigation from fragment to fragment. Exposes various actions to - * ViewModels that cause a NavDirections to be emitted to the observer (in this case, the - * [com.google.android.ground.MainActivity], which is expected to pass it to the current - * [androidx.navigation.NavController]. - */ -@Singleton -class Navigator -@Inject -constructor( - @MainDispatcher private val dispatcher: CoroutineDispatcher, - @MainScope private val coroutineScope: CoroutineScope, -) { - private val _navigateRequests = MutableSharedFlow() - - /** Stream of navigation requests for fulfillment by the view layer. */ - fun getNavigateRequests(): SharedFlow = - _navigateRequests.shareIn(coroutineScope, SharingStarted.Lazily, replay = 0) - - /** Navigates up one level on the back stack. */ - fun navigateUp() { - requestNavigation(NavigateUp) - } - - fun navigate(directions: NavDirections) { - requestNavigation(NavigateTo(directions)) - } - - fun finishApp() { - requestNavigation(FinishApp) - } - - private fun requestNavigation(request: NavigationRequest) { - CoroutineScope(dispatcher).launch { _navigateRequests.emit(request) } - } -} diff --git a/ground/src/main/java/com/google/android/ground/ui/offlineareas/OfflineAreasFragment.kt b/ground/src/main/java/com/google/android/ground/ui/offlineareas/OfflineAreasFragment.kt index 711c2366f5..da7adbf769 100644 --- a/ground/src/main/java/com/google/android/ground/ui/offlineareas/OfflineAreasFragment.kt +++ b/ground/src/main/java/com/google/android/ground/ui/offlineareas/OfflineAreasFragment.kt @@ -29,10 +29,12 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.platform.testTag import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController +import com.google.android.ground.R import com.google.android.ground.databinding.OfflineAreasFragBinding import com.google.android.ground.ui.common.AbstractFragment import com.google.android.ground.ui.theme.AppTheme import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch /** @@ -69,8 +71,11 @@ class OfflineAreasFragment : AbstractFragment() { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) lifecycleScope.launch { - viewModel.navigateToOfflineAreaSelector.collect { - findNavController().navigate(OfflineAreasFragmentDirections.showOfflineAreaSelector()) + viewModel.navigateToOfflineAreaSelector.collectLatest { + val navController = findNavController() + if (navController.currentDestination?.id == R.id.offline_areas_fragment) { + navController.navigate(OfflineAreasFragmentDirections.showOfflineAreaSelector()) + } } } } diff --git a/ground/src/main/java/com/google/android/ground/ui/offlineareas/OfflineAreasViewModel.kt b/ground/src/main/java/com/google/android/ground/ui/offlineareas/OfflineAreasViewModel.kt index 5254bbbea5..e7fe017073 100644 --- a/ground/src/main/java/com/google/android/ground/ui/offlineareas/OfflineAreasViewModel.kt +++ b/ground/src/main/java/com/google/android/ground/ui/offlineareas/OfflineAreasViewModel.kt @@ -49,7 +49,7 @@ internal constructor(private val offlineAreaRepository: OfflineAreaRepository) : val showNoAreasMessage: LiveData val showProgressSpinner: LiveData - private val _navigateToOfflineAreaSelector = MutableSharedFlow(replay = 0) + private val _navigateToOfflineAreaSelector = MutableSharedFlow(replay = 1) val navigateToOfflineAreaSelector = _navigateToOfflineAreaSelector.asSharedFlow() init { diff --git a/ground/src/test/java/com/google/android/ground/NavigationTestHelper.kt b/ground/src/test/java/com/google/android/ground/NavigationTestHelper.kt deleted file mode 100644 index 8691057257..0000000000 --- a/ground/src/test/java/com/google/android/ground/NavigationTestHelper.kt +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * 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 - * - * https://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 com.google.android.ground - -import androidx.navigation.NavDirections -import app.cash.turbine.test -import com.google.android.ground.ui.common.NavigateTo -import com.google.android.ground.ui.common.NavigationRequest -import com.google.common.truth.Truth.assertThat -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.SharedFlow -import kotlinx.coroutines.flow.onSubscription -import kotlinx.coroutines.test.TestScope -import kotlinx.coroutines.test.advanceUntilIdle - -/** - * Asserts that the given [expectedNavDirections] nav direction is emitted by [testSharedFlow] when - * the given block [runOnSubscription] is invoked. - * - * @param testSharedFlow SharedFlow under test - * @param expectedNavDirections NavDirections to validate after the flow has been subscribed - * @param runOnSubscription Runs the block when the given flow is subscribed - */ -suspend fun TestScope.testNavigateTo( - testSharedFlow: SharedFlow, - expectedNavDirections: NavDirections, - runOnSubscription: () -> Unit, -) { - testNavigateTo( - testSharedFlow, - { navDirections -> assertThat(navDirections).isEqualTo(expectedNavDirections) }, - runOnSubscription, - ) -} - -suspend fun TestScope.testNavigateTo( - testSharedFlow: SharedFlow, - validate: (NavDirections) -> Unit, - runOnSubscription: () -> Unit, -) { - testNavigate( - testSharedFlow, - { navigationRequest -> - assertThat(navigationRequest).isInstanceOf(NavigateTo::class.java) - validate((navigationRequest as NavigateTo).directions) - }, - runOnSubscription, - ) -} - -@OptIn(ExperimentalCoroutinesApi::class) -suspend fun TestScope.testNavigate( - testSharedFlow: SharedFlow, - validate: (NavigationRequest) -> Unit, - runOnSubscription: () -> Unit, -) { - testSharedFlow - .onSubscription { - runOnSubscription() - advanceUntilIdle() - } - .test { validate(expectMostRecentItem()) } -} - -@OptIn(ExperimentalCoroutinesApi::class) -suspend fun TestScope.testNoNavigation( - testSharedFlow: SharedFlow, - runOnSubscription: () -> Unit, -) { - testSharedFlow - .onSubscription { - runOnSubscription() - advanceUntilIdle() - } - .test { expectNoEvents() } -} - -/** - * Verifies that the given [NavDirections] is emitted if [expectedNavDirections] is provided, - * otherwise asserts that [NavDirections] should be emitted. - */ -suspend fun TestScope.testMaybeNavigateTo( - testSharedFlow: SharedFlow, - expectedNavDirections: NavDirections?, - runOnSubscription: () -> Unit, -) { - if (expectedNavDirections == null) { - testNoNavigation(testSharedFlow, runOnSubscription) - } else { - testNavigateTo(testSharedFlow, expectedNavDirections, runOnSubscription) - } -} diff --git a/ground/src/test/java/com/google/android/ground/ui/common/NavigatorTest.kt b/ground/src/test/java/com/google/android/ground/ui/common/NavigatorTest.kt deleted file mode 100644 index 190f6bcb53..0000000000 --- a/ground/src/test/java/com/google/android/ground/ui/common/NavigatorTest.kt +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * 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 - * - * https://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 com.google.android.ground.ui.common - -import com.google.android.ground.BaseHiltTest -import com.google.android.ground.testNavigate -import com.google.android.ground.testNavigateTo -import com.google.android.ground.ui.signin.SignInFragmentDirections -import com.google.common.truth.Truth.assertThat -import dagger.hilt.android.testing.HiltAndroidTest -import javax.inject.Inject -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner - -@HiltAndroidTest -@RunWith(RobolectricTestRunner::class) -class NavigatorTest : BaseHiltTest() { - - @Inject lateinit var navigator: Navigator - - @Test - fun `navigate up`() = runWithTestDispatcher { - testNavigate(navigator.getNavigateRequests(), { assertThat(it).isEqualTo(NavigateUp) }) { - navigator.navigateUp() - } - } - - @Test - fun `finish app`() = runWithTestDispatcher { - testNavigate(navigator.getNavigateRequests(), { assertThat(it).isEqualTo(FinishApp) }) { - navigator.finishApp() - } - } - - @Test - fun `navigate to`() = runWithTestDispatcher { - testNavigateTo(navigator.getNavigateRequests(), SignInFragmentDirections.showSignInScreen()) { - navigator.navigate(SignInFragmentDirections.showSignInScreen()) - } - } -} diff --git a/ground/src/test/java/com/google/android/ground/ui/surveyselector/SurveySelectorFragmentTest.kt b/ground/src/test/java/com/google/android/ground/ui/surveyselector/SurveySelectorFragmentTest.kt index 36e3695911..5f54f54d3a 100644 --- a/ground/src/test/java/com/google/android/ground/ui/surveyselector/SurveySelectorFragmentTest.kt +++ b/ground/src/test/java/com/google/android/ground/ui/surveyselector/SurveySelectorFragmentTest.kt @@ -34,8 +34,6 @@ import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase import com.google.android.ground.model.SurveyListItem import com.google.android.ground.repository.SurveyRepository import com.google.android.ground.repository.UserRepository -import com.google.android.ground.ui.common.Navigator -import com.google.android.ground.ui.home.HomeScreenFragmentDirections import com.google.common.truth.Truth.assertThat import com.sharedtest.FakeData import com.sharedtest.system.auth.FakeAuthenticationManager @@ -63,7 +61,6 @@ import org.robolectric.shadows.ShadowToast @RunWith(RobolectricTestRunner::class) class SurveySelectorFragmentTest : BaseHiltTest() { - @BindValue @Mock lateinit var navigator: Navigator @BindValue @Mock lateinit var surveyRepository: SurveyRepository @BindValue @Mock lateinit var userRepository: UserRepository @BindValue @Mock lateinit var activateSurvey: ActivateSurveyUseCase @@ -157,7 +154,13 @@ class SurveySelectorFragmentTest : BaseHiltTest() { setSurveyList(listOf(TEST_SURVEY_1, TEST_SURVEY_2)) setLocalSurveys(listOf()) - setUpFragment() + + launchFragmentWithNavController( + fragmentArgs = bundleOf(Pair("shouldExitApp", false)), + destId = R.id.surveySelectorFragment, + navControllerCallback = { navController = it }, + ) + advanceUntilIdle() // Click second item onView(withId(R.id.recycler_view)) @@ -167,7 +170,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() { // Assert survey is activated. verify(activateSurvey).invoke(TEST_SURVEY_2.id) // Assert that navigation to home screen was not requested - verify(navigator, times(0)).navigate(HomeScreenFragmentDirections.showHomeScreen()) + assertThat(navController.currentDestination?.id).isEqualTo(R.id.surveySelectorFragment) // Error toast message assertThat(ShadowToast.shownToastCount()).isEqualTo(1) } diff --git a/ground/src/test/java/com/google/android/ground/ui/tos/TermsOfServiceFragmentTest.kt b/ground/src/test/java/com/google/android/ground/ui/tos/TermsOfServiceFragmentTest.kt index e4710fbd1a..82e04c7dac 100644 --- a/ground/src/test/java/com/google/android/ground/ui/tos/TermsOfServiceFragmentTest.kt +++ b/ground/src/test/java/com/google/android/ground/ui/tos/TermsOfServiceFragmentTest.kt @@ -31,7 +31,6 @@ import com.google.android.ground.launchFragmentInHiltContainer import com.google.android.ground.launchFragmentWithNavController import com.google.android.ground.model.TermsOfService import com.google.android.ground.repository.TermsOfServiceRepository -import com.google.android.ground.ui.common.Navigator import com.google.common.truth.Truth.assertThat import com.sharedtest.persistence.remote.FakeRemoteDataStore import dagger.hilt.android.testing.HiltAndroidTest @@ -50,7 +49,6 @@ import org.robolectric.RobolectricTestRunner class TermsOfServiceFragmentTest : BaseHiltTest() { @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore - @Inject lateinit var navigator: Navigator @Inject lateinit var termsOfServiceRepository: TermsOfServiceRepository @Inject lateinit var viewModel: TermsOfServiceViewModel private lateinit var navController: NavController