Skip to content

Commit cf7dfdf

Browse files
shobhitagarwal1612shobhitagarwal1612
authored
Move sign-in error handling to SignInScreen (#3473)
* Move permission denied handling to `SignInScreen` The `PermissionDeniedDialog` is now managed within the `SignInScreen` based on the sign-in state, rather than being handled globally via `MainUiState` in `MainActivity`. * Move sign-in error handling to `SignInScreen` * Remove sign-in error handling from `MainViewModel` The `onUserSignInError` method and associated logic have been removed from `MainViewModel`, as sign-in error handling is now managed within the `SignInScreen`. The fallback behavior on error remains a sign-out state with a placeholder for future error dialog improvements. * Add previews for sign-in error states in `SignInScreen` * Add tests for error handling in `SignInScreen` Tests have been added to verify: - Display of permission denied dialog on Firestore permission denied errors. - Application exit and sign-out actions within the permission denied dialog. - Suppression of error UI when sign-in is cancelled by the user. - Display of a snackbar for unknown sign-in errors. * Reformat code in `SignInScreenTest.kt` * Remove sign-in error handling test from `MainViewModelTest` The test case verifying that sign-in errors redirect to a signed-out state has been removed from `MainViewModelTest`. * Undo nullness safety check * Rename onExitClick to onCloseApp. Simplify method references. * Add empty line at end * Revert: Make onUserSignedIn return non-null * Add unit tests for SignInViewModel
1 parent 1ce8444 commit cf7dfdf

9 files changed

Lines changed: 279 additions & 78 deletions

File tree

‎app/src/main/java/org/groundplatform/android/ui/main/MainActivity.kt‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@ import android.os.Bundle
2323
import androidx.activity.OnBackPressedCallback
2424
import androidx.activity.enableEdgeToEdge
2525
import androidx.appcompat.app.AppCompatDelegate
26-
import androidx.compose.runtime.getValue
27-
import androidx.compose.runtime.mutableStateOf
28-
import androidx.compose.runtime.remember
29-
import androidx.compose.runtime.setValue
3026
import androidx.core.view.WindowInsetsCompat
3127
import androidx.lifecycle.lifecycleScope
3228
import androidx.navigation.NavDirections
@@ -35,7 +31,6 @@ import dagger.hilt.android.AndroidEntryPoint
3531
import javax.inject.Inject
3632
import kotlinx.coroutines.flow.filterNotNull
3733
import kotlinx.coroutines.launch
38-
import org.groundplatform.android.BuildConfig
3934
import org.groundplatform.android.R
4035
import org.groundplatform.android.databinding.MainActBinding
4136
import org.groundplatform.android.repository.UserRepository
@@ -44,11 +39,9 @@ import org.groundplatform.android.system.ActivityStreams
4439
import org.groundplatform.android.ui.common.AbstractActivity
4540
import org.groundplatform.android.ui.common.BackPressListener
4641
import org.groundplatform.android.ui.common.ViewModelFactory
47-
import org.groundplatform.android.ui.components.PermissionDeniedDialog
4842
import org.groundplatform.android.ui.home.HomeScreenFragmentDirections
4943
import org.groundplatform.android.ui.signin.SignInFragmentDirections
5044
import org.groundplatform.android.ui.surveyselector.SurveySelectorFragmentDirections
51-
import org.groundplatform.android.util.renderComposableDialog
5245
import timber.log.Timber
5346

5447
/**
@@ -115,9 +108,6 @@ class MainActivity : AbstractActivity() {
115108

116109
private fun updateUi(uiState: MainUiState) {
117110
when (uiState) {
118-
MainUiState.OnPermissionDenied -> {
119-
showPermissionDeniedDialog()
120-
}
121111
MainUiState.OnUserSignedOut -> {
122112
navigateTo(SignInFragmentDirections.showSignInScreen())
123113
}
@@ -138,27 +128,6 @@ class MainActivity : AbstractActivity() {
138128
}
139129
}
140130

141-
private fun showPermissionDeniedDialog() {
142-
renderComposableDialog {
143-
var showDialog by remember { mutableStateOf(true) }
144-
if (showDialog) {
145-
PermissionDeniedDialog(
146-
// TODO: Read url from Firestore config/properties/signUpUrl
147-
// Issue URL: https://github.com/google/ground-android/issues/2402
148-
BuildConfig.SIGNUP_FORM_LINK,
149-
onSignOut = {
150-
showDialog = false
151-
userRepository.signOut()
152-
},
153-
onCloseApp = {
154-
showDialog = false
155-
finish()
156-
},
157-
)
158-
}
159-
}
160-
}
161-
162131
override fun onWindowInsetChanged(insets: WindowInsetsCompat) {
163132
super.onWindowInsetChanged(insets)
164133
viewModel.windowInsets.postValue(insets)

‎app/src/main/java/org/groundplatform/android/ui/main/MainUiState.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ package org.groundplatform.android.ui.main
1818

1919
sealed class MainUiState {
2020

21-
data object OnPermissionDenied : MainUiState()
22-
2321
data object OnUserSignedOut : MainUiState()
2422

2523
data object TosNotAccepted : MainUiState()

‎app/src/main/java/org/groundplatform/android/ui/main/MainViewModel.kt‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ import android.net.Uri
1919
import androidx.core.view.WindowInsetsCompat
2020
import androidx.lifecycle.MutableLiveData
2121
import androidx.lifecycle.viewModelScope
22-
import com.google.android.gms.auth.api.signin.GoogleSignInStatusCodes.SIGN_IN_CANCELLED
23-
import com.google.android.gms.common.api.ApiException
2422
import com.google.firebase.remoteconfig.FirebaseRemoteConfig
2523
import javax.inject.Inject
2624
import kotlinx.coroutines.CoroutineDispatcher
@@ -42,7 +40,6 @@ import org.groundplatform.android.ui.common.AbstractViewModel
4240
import org.groundplatform.android.ui.common.SharedViewModel
4341
import org.groundplatform.android.usecases.session.ClearUserSessionUseCase
4442
import org.groundplatform.android.usecases.survey.ReactivateLastSurveyUseCase
45-
import org.groundplatform.android.util.isPermissionDeniedException
4643
import timber.log.Timber
4744

4845
/** Top-level view model representing state of the [MainActivity] shared by all fragments. */
@@ -85,29 +82,11 @@ constructor(
8582

8683
private suspend fun onSignInStateChange(signInState: SignInState): MainUiState? =
8784
when (signInState) {
88-
is SignInState.Error -> onUserSignInError(signInState.error)
8985
is SignInState.SignedIn -> onUserSignedIn(signInState.user)
9086
is SignInState.SignedOut -> onUserSignedOut()
9187
else -> null
9288
}
9389

94-
private fun onUserSignInError(error: Throwable): MainUiState {
95-
Timber.e(error, "Sign in failed")
96-
return if (error.isPermissionDeniedException()) {
97-
MainUiState.OnPermissionDenied
98-
} else if (error.isSignInCancelledException()) {
99-
Timber.d("User cancelled sign in")
100-
MainUiState.OnUserSignedOut
101-
} else {
102-
// TODO: Display some error dialog to the user with a helpful user-readable message.
103-
// Issue URL: https://github.com/google/ground-android/issues/1808
104-
onUserSignedOut()
105-
}
106-
}
107-
108-
private fun Throwable.isSignInCancelledException() =
109-
this is ApiException && statusCode == SIGN_IN_CANCELLED
110-
11190
private fun onUserSignedOut(): MainUiState {
11291
// Scope of subscription is until view model is cleared. Dispose it manually otherwise, firebase
11392
// attempts to maintain a connection even after user has logged out and throws an error.
@@ -142,7 +121,9 @@ constructor(
142121
MainUiState.ShowHomeScreen
143122
}
144123
} catch (e: Throwable) {
145-
onUserSignInError(e)
124+
Timber.e(e)
125+
// TODO: Display some error dialog to the user with a helpful user-readable message.
126+
onUserSignedOut()
146127
}
147128

148129
/** Returns true if the user has already accepted the Terms of Service. */

‎app/src/main/java/org/groundplatform/android/ui/signin/SignInFragment.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class SignInFragment : AbstractFragment(), BackPressListener {
3636
): View =
3737
ComposeView(requireContext()).apply {
3838
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
39-
setContent { AppTheme { SignInScreen() } }
39+
setContent { AppTheme { SignInScreen(onCloseApp = { requireActivity().finish() }) } }
4040
}
4141

4242
override fun onBack(): Boolean {

‎app/src/main/java/org/groundplatform/android/ui/signin/SignInScreen.kt‎

Lines changed: 122 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,19 @@ import androidx.compose.ui.unit.dp
5252
import androidx.compose.ui.unit.sp
5353
import androidx.hilt.navigation.compose.hiltViewModel
5454
import androidx.lifecycle.compose.collectAsStateWithLifecycle
55+
import com.google.android.gms.auth.api.signin.GoogleSignInStatusCodes.SIGN_IN_CANCELLED
56+
import com.google.android.gms.common.api.ApiException
57+
import com.google.android.gms.common.api.Status
58+
import com.google.firebase.firestore.FirebaseFirestoreException
59+
import com.google.firebase.firestore.FirebaseFirestoreException.Code
60+
import org.groundplatform.android.BuildConfig
5561
import org.groundplatform.android.R
5662
import org.groundplatform.android.system.auth.SignInState
5763
import org.groundplatform.android.ui.common.ExcludeFromJacocoGeneratedReport
5864
import org.groundplatform.android.ui.components.LoadingDialog
65+
import org.groundplatform.android.ui.components.PermissionDeniedDialog
5966
import org.groundplatform.android.ui.theme.AppTheme
67+
import org.groundplatform.android.util.isPermissionDeniedException
6068

6169
const val BUTTON_TEST_TAG = "google_sign_in_button"
6270

@@ -66,19 +74,27 @@ const val BUTTON_TEST_TAG = "google_sign_in_button"
6674
* @param viewModel the view model used to manage sign-in state and network connectivity.
6775
*/
6876
@Composable
69-
fun SignInScreen(viewModel: SignInViewModel = hiltViewModel()) {
77+
fun SignInScreen(onCloseApp: () -> Unit, viewModel: SignInViewModel = hiltViewModel()) {
7078
val connected by viewModel.networkAvailable.collectAsStateWithLifecycle()
7179
val signInState by viewModel.signInState.collectAsStateWithLifecycle()
7280

7381
SignInContent(
7482
connected = connected,
7583
signInState = signInState,
76-
onSignInClick = { viewModel.onSignInButtonClick() },
84+
onSignInClick = viewModel::onSignInButtonClick,
85+
onSignOutClick = viewModel::onSignOutButtonClick,
86+
onCloseApp = onCloseApp,
7787
)
7888
}
7989

8090
@Composable
81-
private fun SignInContent(connected: Boolean, signInState: SignInState, onSignInClick: () -> Unit) {
91+
private fun SignInContent(
92+
connected: Boolean,
93+
signInState: SignInState,
94+
onSignInClick: () -> Unit,
95+
onSignOutClick: () -> Unit,
96+
onCloseApp: () -> Unit,
97+
) {
8298
val snackbarHostState = remember { SnackbarHostState() }
8399
val networkErrorMessage = stringResource(R.string.network_error_when_signing_in)
84100

@@ -90,6 +106,8 @@ private fun SignInContent(connected: Boolean, signInState: SignInState, onSignIn
90106

91107
if (signInState is SignInState.SigningIn) {
92108
LoadingDialog(R.string.signing_in)
109+
} else if (signInState is SignInState.Error) {
110+
SignInErrorUi(signInState, onSignOutClick, onCloseApp, snackbarHostState)
93111
}
94112

95113
Box(modifier = Modifier.fillMaxSize()) {
@@ -117,6 +135,39 @@ private fun SignInContent(connected: Boolean, signInState: SignInState, onSignIn
117135
}
118136
}
119137

138+
@Composable
139+
private fun SignInErrorUi(
140+
signInState: SignInState.Error,
141+
onSignOutClick: () -> Unit,
142+
onCloseApp: () -> Unit,
143+
snackbarHostState: SnackbarHostState,
144+
) {
145+
when {
146+
signInState.error.isPermissionDeniedException() -> {
147+
PermissionDeniedDialog(
148+
// TODO: Read url from Firestore config/properties/signUpUrl
149+
// Issue URL: https://github.com/google/ground-android/issues/2402
150+
signupLink = BuildConfig.SIGNUP_FORM_LINK,
151+
onSignOut = onSignOutClick,
152+
onCloseApp = onCloseApp,
153+
)
154+
}
155+
156+
signInState.error.isSignInCancelledException() -> {
157+
/* Do nothing, as this was a user choice, not a system error. */
158+
}
159+
160+
// For any other type of error, show a generic message.
161+
else -> {
162+
val unknownErrorMessage = stringResource(R.string.something_went_wrong)
163+
LaunchedEffect(signInState.error) { snackbarHostState.showSnackbar(unknownErrorMessage) }
164+
}
165+
}
166+
}
167+
168+
private fun Throwable.isSignInCancelledException() =
169+
this is ApiException && statusCode == SIGN_IN_CANCELLED
170+
120171
@Composable
121172
private fun BackgroundOverlay(modifier: Modifier = Modifier) {
122173
Column(modifier.fillMaxSize().background(color = Color(0x66146C2E))) {
@@ -190,7 +241,13 @@ private fun GoogleSignInButton(
190241
@ExcludeFromJacocoGeneratedReport
191242
private fun SignInScreenSignedOutPreview() {
192243
AppTheme {
193-
SignInContent(connected = true, signInState = SignInState.SignedOut, onSignInClick = {})
244+
SignInContent(
245+
connected = true,
246+
signInState = SignInState.SignedOut,
247+
onSignInClick = {},
248+
onSignOutClick = {},
249+
onCloseApp = {},
250+
)
194251
}
195252
}
196253

@@ -199,7 +256,13 @@ private fun SignInScreenSignedOutPreview() {
199256
@ExcludeFromJacocoGeneratedReport
200257
private fun SignInScreenSigningInPreview() {
201258
AppTheme {
202-
SignInContent(connected = true, signInState = SignInState.SigningIn, onSignInClick = {})
259+
SignInContent(
260+
connected = true,
261+
signInState = SignInState.SigningIn,
262+
onSignInClick = {},
263+
onSignOutClick = {},
264+
onCloseApp = {},
265+
)
203266
}
204267
}
205268

@@ -208,6 +271,59 @@ private fun SignInScreenSigningInPreview() {
208271
@ExcludeFromJacocoGeneratedReport
209272
private fun SignInScreenNotConnectedPreview() {
210273
AppTheme {
211-
SignInContent(connected = false, signInState = SignInState.SignedOut, onSignInClick = {})
274+
SignInContent(
275+
connected = false,
276+
signInState = SignInState.SignedOut,
277+
onSignInClick = {},
278+
onSignOutClick = {},
279+
onCloseApp = {},
280+
)
281+
}
282+
}
283+
284+
@Preview(showBackground = true)
285+
@Composable
286+
@ExcludeFromJacocoGeneratedReport
287+
private fun SignInScreenPermissionDeniedErrorPreview() {
288+
AppTheme {
289+
val error = FirebaseFirestoreException("Permission denied", Code.PERMISSION_DENIED)
290+
SignInContent(
291+
connected = true,
292+
signInState = SignInState.Error(error),
293+
onSignInClick = {},
294+
onSignOutClick = {},
295+
onCloseApp = {},
296+
)
297+
}
298+
}
299+
300+
@Preview(showBackground = true)
301+
@Composable
302+
@ExcludeFromJacocoGeneratedReport
303+
private fun SignInScreenUserCancelledErrorPreview() {
304+
AppTheme {
305+
val error = ApiException(Status(SIGN_IN_CANCELLED))
306+
SignInContent(
307+
connected = true,
308+
signInState = SignInState.Error(error),
309+
onSignInClick = {},
310+
onSignOutClick = {},
311+
onCloseApp = {},
312+
)
313+
}
314+
}
315+
316+
@Preview(showBackground = true)
317+
@Composable
318+
@ExcludeFromJacocoGeneratedReport
319+
private fun SignInScreenUnknownErrorPreview() {
320+
AppTheme {
321+
SignInContent(
322+
connected = true,
323+
signInState = SignInState.Error(Error("Unknown error")),
324+
onSignInClick = {},
325+
onSignOutClick = {},
326+
onCloseApp = {},
327+
)
212328
}
213329
}

‎app/src/main/java/org/groundplatform/android/ui/signin/SignInViewModel.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,8 @@ internal constructor(networkManager: NetworkManager, private val userRepository:
5656
fun onSignInButtonClick() {
5757
userRepository.signIn()
5858
}
59+
60+
fun onSignOutButtonClick() {
61+
userRepository.signOut()
62+
}
5963
}

‎app/src/test/java/org/groundplatform/android/ui/main/MainViewModelTest.kt‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -171,19 +171,4 @@ class MainViewModelTest : BaseHiltTest() {
171171
assertThat(awaitItem()).isEqualTo(MainUiState.TosNotAccepted)
172172
}
173173
}
174-
175-
@Test
176-
fun `sign in error redirects to signed out state`() = runWithTestDispatcher {
177-
setupUserPreferences()
178-
179-
viewModel.navigationRequests.test {
180-
fakeAuthenticationManager.setState(SignInState.Error(Exception()))
181-
advanceUntilIdle()
182-
183-
assertThat(awaitItem()).isEqualTo(MainUiState.OnUserSignedOut)
184-
verifyUserPreferencesCleared()
185-
verifyUserNotSaved()
186-
assertThat(tosRepository.isTermsOfServiceAccepted).isFalse()
187-
}
188-
}
189174
}

0 commit comments

Comments
 (0)