[Code health] Move remaining cases to MainUiState (#2521)

* Add remaining ui states to sealed class

* Update unit tests

* Add braces to when for readability

ground/src/main/java/com/google/android/ground/MainActivity.kt

@@ -42,6 +42,9 @@ 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
+import com.google.android.ground.ui.signin.SignInFragmentDirections
+import com.google.android.ground.ui.surveyselector.SurveySelectorFragmentDirections
 import com.google.android.ground.ui.theme.AppTheme
 import dagger.hilt.android.AndroidEntryPoint
 import javax.inject.Inject
@@ -89,9 +92,6 @@ class MainActivity : AbstractActivity() {
       supportFragmentManager.findFragmentById(R.id.nav_host_fragment) as NavHostFragment
 
     viewModel = viewModelFactory[this, MainViewModel::class.java]
-    viewModel.signInProgressDialogVisibility.observe(this) { visible: Boolean ->
-      onSignInProgress(visible)
-    }
 
     lifecycleScope.launch {
       viewModel.uiState.filterNotNull().collect { updateUi(binding.root, it) }
@@ -100,7 +100,28 @@ class MainActivity : AbstractActivity() {
 
   private fun updateUi(viewGroup: ViewGroup, uiState: MainUiState) {
     when (uiState) {
-      MainUiState.onPermissionDenied -> showPermissionDeniedDialog(viewGroup)
+      MainUiState.OnPermissionDenied -> {
+        showPermissionDeniedDialog(viewGroup)
+      }
+      MainUiState.OnUserSignedOut -> {
+        navigator.navigate(SignInFragmentDirections.showSignInScreen())
+      }
+      MainUiState.TosNotAccepted -> {
+        navigator.navigate(SignInFragmentDirections.showTermsOfService(false))
+      }
+      MainUiState.NoActiveSurvey -> {
+        navigator.navigate(SurveySelectorFragmentDirections.showSurveySelectorScreen(true))
+      }
+      MainUiState.ShowHomeScreen -> {
+        navigator.navigate(HomeScreenFragmentDirections.showHomeScreen())
+      }
+      MainUiState.OnUserSigningIn -> {
+        onSignInProgress(true)
+      }
+    }
+
+    if (uiState != MainUiState.OnUserSigningIn) {
+      onSignInProgress(false)
     }
   }
 

ground/src/main/java/com/google/android/ground/MainUiState.kt

@@ -18,7 +18,15 @@ package com.google.android.ground
 
 sealed class MainUiState {
 
-  // TODO(#2402): Move remaining ui states from view model
+  data object OnPermissionDenied : MainUiState()
 
-  data object onPermissionDenied : MainUiState()
+  data object OnUserSignedOut : MainUiState()
+
+  data object OnUserSigningIn : MainUiState()
+
+  data object TosNotAccepted : MainUiState()
+
+  data object NoActiveSurvey : MainUiState()
+
+  data object ShowHomeScreen : MainUiState()
 }

ground/src/main/java/com/google/android/ground/MainViewModel.kt

@@ -18,7 +18,6 @@ package com.google.android.ground
 import androidx.core.view.WindowInsetsCompat
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.viewModelScope
-import androidx.navigation.NavDirections
 import com.google.android.ground.coroutines.IoDispatcher
 import com.google.android.ground.domain.usecases.survey.ReactivateLastSurveyUseCase
 import com.google.android.ground.model.User
@@ -29,11 +28,7 @@ import com.google.android.ground.repository.UserRepository
 import com.google.android.ground.system.auth.AuthenticationManager
 import com.google.android.ground.system.auth.SignInState
 import com.google.android.ground.ui.common.AbstractViewModel
-import com.google.android.ground.ui.common.Navigator
 import com.google.android.ground.ui.common.SharedViewModel
-import com.google.android.ground.ui.home.HomeScreenFragmentDirections
-import com.google.android.ground.ui.signin.SignInFragmentDirections
-import com.google.android.ground.ui.surveyselector.SurveySelectorFragmentDirections
 import com.google.android.ground.util.isPermissionDeniedException
 import javax.inject.Inject
 import kotlinx.coroutines.CoroutineDispatcher
@@ -55,7 +50,6 @@ constructor(
   private val termsOfServiceRepository: TermsOfServiceRepository,
   private val reactivateLastSurvey: ReactivateLastSurveyUseCase,
   @IoDispatcher private val ioDispatcher: CoroutineDispatcher,
-  navigator: Navigator,
   authenticationManager: AuthenticationManager,
 ) : AbstractViewModel() {
 
@@ -65,43 +59,32 @@ constructor(
   /** The window insets determined by the activity. */
   val windowInsets: MutableLiveData<WindowInsetsCompat> = MutableLiveData()
 
-  /** The state of sign in progress dialog visibility. */
-  val signInProgressDialogVisibility: MutableLiveData<Boolean> = MutableLiveData()
-
   init {
     viewModelScope.launch {
       // TODO: Check auth status whenever fragments resumes
-      authenticationManager.signInState.collect {
-        val nextState = onSignInStateChange(it)
-        nextState?.let { navigator.navigate(nextState) }
-      }
+      authenticationManager.signInState.collect { _uiState.emit(onSignInStateChange(it)) }
     }
   }
 
-  private suspend fun onSignInStateChange(signInState: SignInState): NavDirections? {
-    // Display progress only when signing in.
-    signInProgressDialogVisibility.postValue(signInState == SignInState.SigningIn)
-
-    return when (signInState) {
+  private suspend fun onSignInStateChange(signInState: SignInState): MainUiState =
+    when (signInState) {
       is SignInState.Error -> onUserSignInError(signInState.error)
       is SignInState.SignedIn -> onUserSignedIn(signInState.user)
       is SignInState.SignedOut -> onUserSignedOut()
-      is SignInState.SigningIn -> null
+      is SignInState.SigningIn -> MainUiState.OnUserSigningIn
     }
-  }
 
-  private suspend fun onUserSignInError(error: Throwable): NavDirections? {
+  private fun onUserSignInError(error: Throwable): MainUiState {
     Timber.e(error, "Sign in failed")
     return if (error.isPermissionDeniedException()) {
-      _uiState.emit(MainUiState.onPermissionDenied)
-      null
+      MainUiState.OnPermissionDenied
     } else {
       // TODO(#1808): Display some error dialog to the user with a helpful user-readable messagez.
       onUserSignedOut()
     }
   }
 
-  private fun onUserSignedOut(): NavDirections {
+  private fun onUserSignedOut(): MainUiState {
     // Scope of subscription is until view model is cleared. Dispose it manually otherwise, firebase
     // attempts to maintain a connection even after user has logged out and throws an error.
     surveyRepository.clearActiveSurvey()
@@ -111,28 +94,36 @@ constructor(
     //  currently being done to prevent one user's data to be submitted as another user after
     //  re-login.
     viewModelScope.launch { withContext(ioDispatcher) { localDatabase.clearAllTables() } }
-
-    return SignInFragmentDirections.showSignInScreen()
+    return MainUiState.OnUserSignedOut
   }
 
-  private suspend fun onUserSignedIn(user: User): NavDirections? =
+  private suspend fun onUserSignedIn(user: User): MainUiState =
     try {
       userRepository.saveUserDetails(user)
-      val tos = termsOfServiceRepository.getTermsOfService()
-      if (tos == null || termsOfServiceRepository.isTermsOfServiceAccepted) {
-        reactivateLastSurvey()
-        getDirectionAfterSignIn()
+      if (!isTosMissingOrAccepted()) {
+        MainUiState.TosNotAccepted
+      } else if (!attemptToReactiveLastActiveSurvey()) {
+        MainUiState.NoActiveSurvey
       } else {
-        SignInFragmentDirections.showTermsOfService(false)
+        // Everything is fine, show the home screen
+        MainUiState.ShowHomeScreen
       }
     } catch (e: Throwable) {
       onUserSignInError(e)
     }
 
-  private fun getDirectionAfterSignIn(): NavDirections =
-    if (surveyRepository.selectedSurveyId != null) {
-      HomeScreenFragmentDirections.showHomeScreen()
-    } else {
-      SurveySelectorFragmentDirections.showSurveySelectorScreen(true)
-    }
+  /**
+   * Returns true if the terms of service are missing from firestore config or if the user has not
+   * accepted it yet.
+   */
+  private suspend fun isTosMissingOrAccepted(): Boolean {
+    val tos = termsOfServiceRepository.getTermsOfService()
+    return tos == null || termsOfServiceRepository.isTermsOfServiceAccepted
+  }
+
+  /** Returns true if the last survey was successfully reactivated, if any. */
+  private suspend fun attemptToReactiveLastActiveSurvey(): Boolean {
+    reactivateLastSurvey()
+    return surveyRepository.selectedSurveyId != null
+  }
 }

ground/src/test/java/com/google/android/ground/MainViewModelTest.kt

@@ -21,30 +21,29 @@ import com.google.android.ground.persistence.local.room.LocalDataStoreException
 import com.google.android.ground.repository.TermsOfServiceRepository
 import com.google.android.ground.repository.UserRepository
 import com.google.android.ground.system.auth.SignInState
-import com.google.android.ground.ui.common.Navigator
-import com.google.android.ground.ui.signin.SignInFragmentDirections
 import com.google.common.truth.Truth.assertThat
 import com.google.firebase.firestore.FirebaseFirestoreException
 import com.sharedtest.FakeData
-import com.sharedtest.TestObservers.observeUntilFirstChange
 import com.sharedtest.persistence.remote.FakeRemoteDataStore
 import com.sharedtest.system.auth.FakeAuthenticationManager
 import dagger.hilt.android.testing.HiltAndroidTest
 import javax.inject.Inject
 import kotlin.test.assertFailsWith
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.advanceUntilIdle
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 
+@OptIn(ExperimentalCoroutinesApi::class)
 @HiltAndroidTest
 @RunWith(RobolectricTestRunner::class)
 class MainViewModelTest : BaseHiltTest() {
 
   @Inject lateinit var fakeAuthenticationManager: FakeAuthenticationManager
   @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore
   @Inject lateinit var viewModel: MainViewModel
-  @Inject lateinit var navigator: Navigator
   @Inject lateinit var sharedPreferences: SharedPreferences
   @Inject lateinit var tosRepository: TermsOfServiceRepository
   @Inject lateinit var userRepository: UserRepository
@@ -72,32 +71,29 @@ class MainViewModelTest : BaseHiltTest() {
     assertFailsWith<LocalDataStoreException> { userRepository.getUser(FakeData.USER.id) }
   }
 
-  private fun verifyProgressDialogVisible(visible: Boolean) {
-    observeUntilFirstChange(viewModel.signInProgressDialogVisibility)
-    assertThat(viewModel.signInProgressDialogVisibility.value).isEqualTo(visible)
+  private fun verifyUiState(uiState: MainUiState) = runWithTestDispatcher {
+    viewModel.uiState.test { assertThat(expectMostRecentItem()).isEqualTo(uiState) }
   }
 
   @Test
   fun testSignInStateChanged_onSignedOut() = runWithTestDispatcher {
     setupUserPreferences()
 
-    testNavigateTo(navigator.getNavigateRequests(), SignInFragmentDirections.showSignInScreen()) {
-      fakeAuthenticationManager.signOut()
-    }
+    fakeAuthenticationManager.signOut()
+    advanceUntilIdle()
 
-    verifyProgressDialogVisible(false)
+    verifyUiState(MainUiState.OnUserSignedOut)
     verifyUserPreferencesCleared()
     verifyUserNotSaved()
     assertThat(tosRepository.isTermsOfServiceAccepted).isFalse()
   }
 
   @Test
   fun testSignInStateChanged_onSigningIn() = runWithTestDispatcher {
-    testNoNavigation(navigator.getNavigateRequests()) {
-      fakeAuthenticationManager.setState(SignInState.SigningIn)
-    }
+    fakeAuthenticationManager.setState(SignInState.SigningIn)
+    advanceUntilIdle()
 
-    verifyProgressDialogVisible(true)
+    verifyUiState(MainUiState.OnUserSigningIn)
     verifyUserNotSaved()
     assertThat(tosRepository.isTermsOfServiceAccepted).isFalse()
   }
@@ -110,14 +106,10 @@ class MainViewModelTest : BaseHiltTest() {
     tosRepository.isTermsOfServiceAccepted = false
     fakeRemoteDataStore.termsOfService = Result.success(FakeData.TERMS_OF_SERVICE)
 
-    testNavigateTo(
-      navigator.getNavigateRequests(),
-      SignInFragmentDirections.showTermsOfService(false),
-    ) {
-      fakeAuthenticationManager.signIn()
-    }
+    fakeAuthenticationManager.signIn()
+    advanceUntilIdle()
 
-    verifyProgressDialogVisible(false)
+    verifyUiState(MainUiState.TosNotAccepted)
     verifyUserSaved()
     assertThat(tosRepository.isTermsOfServiceAccepted).isFalse()
   }
@@ -127,14 +119,10 @@ class MainViewModelTest : BaseHiltTest() {
     tosRepository.isTermsOfServiceAccepted = false
     fakeRemoteDataStore.termsOfService = null
 
-    testNavigateTo(
-      navigator.getNavigateRequests(),
-      SignInFragmentDirections.showSurveySelectorScreen(true),
-    ) {
-      fakeAuthenticationManager.signIn()
-    }
+    fakeAuthenticationManager.signIn()
+    advanceUntilIdle()
 
-    verifyProgressDialogVisible(false)
+    verifyUiState(MainUiState.NoActiveSurvey)
     verifyUserSaved()
     assertThat(tosRepository.isTermsOfServiceAccepted).isFalse()
   }
@@ -150,14 +138,11 @@ class MainViewModelTest : BaseHiltTest() {
         )
       )
 
-    testNoNavigation(navigator.getNavigateRequests()) { fakeAuthenticationManager.signIn() }
+    fakeAuthenticationManager.signIn()
+    advanceUntilIdle()
 
-    verifyProgressDialogVisible(false)
     assertThat(tosRepository.isTermsOfServiceAccepted).isFalse()
-
-    viewModel.uiState.test {
-      assertThat(expectMostRecentItem()).isEqualTo(MainUiState.onPermissionDenied)
-    }
+    verifyUiState(MainUiState.OnPermissionDenied)
   }
 
   @Test
@@ -166,23 +151,21 @@ class MainViewModelTest : BaseHiltTest() {
       tosRepository.isTermsOfServiceAccepted = false
       fakeRemoteDataStore.termsOfService = Result.failure(Error("user error"))
 
-      testNavigateTo(navigator.getNavigateRequests(), SignInFragmentDirections.showSignInScreen()) {
-        fakeAuthenticationManager.signIn()
-      }
+      fakeAuthenticationManager.signIn()
+      advanceUntilIdle()
 
-      verifyProgressDialogVisible(false)
       assertThat(tosRepository.isTermsOfServiceAccepted).isFalse()
+      verifyUiState(MainUiState.OnUserSignedOut)
     }
 
   @Test
   fun testSignInStateChanged_onSignInError() = runWithTestDispatcher {
     setupUserPreferences()
 
-    testNavigateTo(navigator.getNavigateRequests(), SignInFragmentDirections.showSignInScreen()) {
-      fakeAuthenticationManager.setState(SignInState.Error(Exception()))
-    }
+    fakeAuthenticationManager.setState(SignInState.Error(Exception()))
+    advanceUntilIdle()
 
-    verifyProgressDialogVisible(false)
+    verifyUiState(MainUiState.OnUserSignedOut)
     verifyUserPreferencesCleared()
     verifyUserNotSaved()
     assertThat(tosRepository.isTermsOfServiceAccepted).isFalse()