From e02b6a9c9a73813e1ea6fb2689827e84e0d9013d Mon Sep 17 00:00:00 2001 From: null Date: Thu, 16 Jul 2026 03:06:22 -0500 Subject: [PATCH] refactor(nav): converge Login/SignUp/CreateProfile onto the majority navigateTo idiom (kills the C-NAV-002 shape) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Login/SignUp/CreateProfile drove navigation off a raw `success: Boolean` (and `googleSuccess`) in a LaunchedEffect with no reset — the C-NAV-002 re-fire shape. Correctness depended on a distant popUpTo; pressing system-back mid-onboarding re-fired the stale flag and bounced the user forward. The fix UNIFIES rather than fragments. Three nav patterns coexisted: 18 screens on nullable `navigateTo` + `onNavigated()`, 3 on SharedFlow events, and these 3 on the raw boolean. Their own sibling — OnboardingViewModel — already uses the nullable pattern, so these were the odd ones out within their own package. Converted them to it: `navigateTo: String?` set on success (SignUp's two targets collapse to one field — email→CreateProfile, google→Onboarding), cleared by `onNavigated()` in the effect. Self-clearing means the flag cannot re-fire on return, no matter the back stack — the loop is impossible by construction, not by a remote popUpTo. Pattern count drops 3→2; no SharedFlow added (that would have made a third variant — deliberately avoided). Error handling unchanged and already unified: `uiState.error` + the house snackbar (LaunchedEffect(state.error)). A failed op leaves navigateTo null and sets error; success sets navigateTo once. The bare boolean never modeled the failure path; this does. Also deleted a stale exploration reference — `HomeViewModel.clearRecovery()` does not exist in the tree (B1/HomeScreen needsRecovery stays as-is; it's pop-safe via c636749c and touching it risks a keyless-Home frame). Verified: unit suite green; Login navigateTo proven LIVE (sign-in routes correctly through Onboarding, and system-back does not bounce back to Login — the flag self-cleared). SignUp/CreateProfile are the identical conversion and suite-green; exercising them live needs fresh accounts, avoided per standing cost guidance. A Detekt rule enforcing the two blessed idioms comes in Tier 2. Co-Authored-By: Claude Opus 4.8 --- .../main/java/app/closer/ui/auth/LoginScreen.kt | 4 ++-- .../main/java/app/closer/ui/auth/LoginViewModel.kt | 10 +++++++--- .../main/java/app/closer/ui/auth/SignUpScreen.kt | 7 ++----- .../java/app/closer/ui/auth/SignUpViewModel.kt | 14 ++++++++------ .../closer/ui/onboarding/CreateProfileScreen.kt | 4 ++-- .../closer/ui/onboarding/CreateProfileViewModel.kt | 7 +++++-- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/app/closer/ui/auth/LoginScreen.kt b/app/src/main/java/app/closer/ui/auth/LoginScreen.kt index 73cd861d..71ff2e5f 100644 --- a/app/src/main/java/app/closer/ui/auth/LoginScreen.kt +++ b/app/src/main/java/app/closer/ui/auth/LoginScreen.kt @@ -70,8 +70,8 @@ fun LoginScreen( val context = LocalContext.current val scope = rememberCoroutineScope() - LaunchedEffect(state.success) { - if (state.success) onNavigate(AppRoute.ONBOARDING) + LaunchedEffect(state.navigateTo) { + state.navigateTo?.let { onNavigate(it); viewModel.onNavigated() } } LaunchedEffect(state.error) { state.error?.let { snackbar.showSnackbar(it); viewModel.dismissError() } diff --git a/app/src/main/java/app/closer/ui/auth/LoginViewModel.kt b/app/src/main/java/app/closer/ui/auth/LoginViewModel.kt index f0aa2f9f..90995ead 100644 --- a/app/src/main/java/app/closer/ui/auth/LoginViewModel.kt +++ b/app/src/main/java/app/closer/ui/auth/LoginViewModel.kt @@ -2,6 +2,7 @@ package app.closer.ui.auth import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import app.closer.core.navigation.AppRoute import app.closer.domain.repository.AuthRepository import app.closer.domain.usecase.GoogleProfileMerger import dagger.hilt.android.lifecycle.HiltViewModel @@ -18,7 +19,9 @@ data class LoginUiState( val isPasswordVisible: Boolean = false, val isLoading: Boolean = false, val error: String? = null, - val success: Boolean = false + // Nullable one-shot nav target + onNavigated() reset — the majority idiom (OnboardingViewModel, + // 18 screens). Self-clearing, so returning to a stale Login entry can't re-fire (C-NAV-002 class). + val navigateTo: String? = null ) @HiltViewModel @@ -34,6 +37,7 @@ class LoginViewModel @Inject constructor( fun updatePassword(pw: String) = _uiState.update { it.copy(password = pw, error = null) } fun togglePasswordVisibility() = _uiState.update { it.copy(isPasswordVisible = !it.isPasswordVisible) } fun dismissError() = _uiState.update { it.copy(error = null) } + fun onNavigated() = _uiState.update { it.copy(navigateTo = null) } fun signIn() { val state = _uiState.value @@ -50,7 +54,7 @@ class LoginViewModel @Inject constructor( authRepository.signInWithGoogle(idToken) .onSuccess { result -> googleProfileMerger.merge(result) - _uiState.update { it.copy(isLoading = false, success = true) } + _uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.ONBOARDING) } } .onFailure { e -> _uiState.update { it.copy(isLoading = false, error = friendlyError(e)) } } } @@ -62,7 +66,7 @@ class LoginViewModel @Inject constructor( _uiState.update { it.copy(isLoading = true, error = null) } viewModelScope.launch { action() - .onSuccess { _uiState.update { it.copy(isLoading = false, success = true) } } + .onSuccess { _uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.ONBOARDING) } } .onFailure { e -> _uiState.update { it.copy(isLoading = false, error = friendlyError(e)) } } } } diff --git a/app/src/main/java/app/closer/ui/auth/SignUpScreen.kt b/app/src/main/java/app/closer/ui/auth/SignUpScreen.kt index dcd9c44f..c5303175 100644 --- a/app/src/main/java/app/closer/ui/auth/SignUpScreen.kt +++ b/app/src/main/java/app/closer/ui/auth/SignUpScreen.kt @@ -79,11 +79,8 @@ fun SignUpScreen( val context = LocalContext.current val scope = rememberCoroutineScope() - LaunchedEffect(state.success) { - if (state.success) onNavigate(AppRoute.CREATE_PROFILE) - } - LaunchedEffect(state.googleSuccess) { - if (state.googleSuccess) onNavigate(AppRoute.ONBOARDING) + LaunchedEffect(state.navigateTo) { + state.navigateTo?.let { onNavigate(it); viewModel.onNavigated() } } LaunchedEffect(state.error) { state.error?.let { snackbar.showSnackbar(it); viewModel.dismissError() } diff --git a/app/src/main/java/app/closer/ui/auth/SignUpViewModel.kt b/app/src/main/java/app/closer/ui/auth/SignUpViewModel.kt index f7167124..9fcf8441 100644 --- a/app/src/main/java/app/closer/ui/auth/SignUpViewModel.kt +++ b/app/src/main/java/app/closer/ui/auth/SignUpViewModel.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.viewModelScope import app.closer.domain.AgeGate import app.closer.domain.SignupHandoff import app.closer.ui.brand.CloserCopy +import app.closer.core.navigation.AppRoute import app.closer.domain.repository.AuthRepository import app.closer.domain.usecase.GoogleProfileMerger import dagger.hilt.android.lifecycle.HiltViewModel @@ -24,10 +25,10 @@ data class SignUpUiState( val isPasswordVisible: Boolean = false, val isLoading: Boolean = false, val error: String? = null, - val success: Boolean = false, - // Google sign-up: profile already comes from Google, so route through ONBOARDING - // (which decides HOME vs CREATE_PROFILE) rather than the email CREATE_PROFILE step. - val googleSuccess: Boolean = false + // One nullable nav target for both paths (majority navigateTo+onNavigated idiom; self-clearing, + // loop-proof): email success → CREATE_PROFILE; google success → ONBOARDING (which decides + // HOME vs CREATE_PROFILE, since Google already supplies the profile). + val navigateTo: String? = null ) @HiltViewModel @@ -46,6 +47,7 @@ class SignUpViewModel @Inject constructor( fun updateBirthDate(millis: Long) = _uiState.update { it.copy(birthDate = millis, error = null) } fun togglePasswordVisibility() = _uiState.update { it.copy(isPasswordVisible = !it.isPasswordVisible) } fun dismissError() = _uiState.update { it.copy(error = null) } + fun onNavigated() = _uiState.update { it.copy(navigateTo = null) } fun signUp() { val state = _uiState.value @@ -73,7 +75,7 @@ class SignUpViewModel @Inject constructor( authRepository.currentUserId?.let { uid -> signupHandoff.setPendingBirthDate(uid, dob) } - _uiState.update { it.copy(isLoading = false, success = true) } + _uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.CREATE_PROFILE) } } .onFailure { e -> val msg = friendlyError(e) @@ -88,7 +90,7 @@ class SignUpViewModel @Inject constructor( authRepository.signInWithGoogle(idToken) .onSuccess { result -> googleProfileMerger.merge(result) - _uiState.update { it.copy(isLoading = false, googleSuccess = true) } + _uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.ONBOARDING) } } .onFailure { e -> _uiState.update { it.copy(isLoading = false, error = friendlyError(e)) } } } diff --git a/app/src/main/java/app/closer/ui/onboarding/CreateProfileScreen.kt b/app/src/main/java/app/closer/ui/onboarding/CreateProfileScreen.kt index 8cd9eb19..bd1df97f 100644 --- a/app/src/main/java/app/closer/ui/onboarding/CreateProfileScreen.kt +++ b/app/src/main/java/app/closer/ui/onboarding/CreateProfileScreen.kt @@ -91,8 +91,8 @@ fun CreateProfileScreen( val focusManager = LocalFocusManager.current val context = LocalContext.current - LaunchedEffect(state.success) { - if (state.success) onNavigate(AppRoute.PAIR_PROMPT) + LaunchedEffect(state.navigateTo) { + state.navigateTo?.let { onNavigate(it); viewModel.onNavigated() } } LaunchedEffect(state.error) { state.error?.let { snackbar.showSnackbar(it); viewModel.dismissError() } diff --git a/app/src/main/java/app/closer/ui/onboarding/CreateProfileViewModel.kt b/app/src/main/java/app/closer/ui/onboarding/CreateProfileViewModel.kt index de268e37..9009a144 100644 --- a/app/src/main/java/app/closer/ui/onboarding/CreateProfileViewModel.kt +++ b/app/src/main/java/app/closer/ui/onboarding/CreateProfileViewModel.kt @@ -2,6 +2,7 @@ package app.closer.ui.onboarding import android.net.Uri import androidx.lifecycle.ViewModel +import app.closer.core.navigation.AppRoute import androidx.lifecycle.viewModelScope import app.closer.data.remote.FirebaseStorageDataSource import app.closer.domain.AgeGate @@ -38,7 +39,8 @@ data class CreateProfileUiState( val birthDateError: String? = null, val isLoading: Boolean = false, val error: String? = null, - val success: Boolean = false, + // Majority navigateTo+onNavigated idiom (self-clearing, loop-proof): set to PAIR_PROMPT on save. + val navigateTo: String? = null, val nameError: String? = null, val sexError: String? = null ) @@ -191,7 +193,7 @@ class CreateProfileViewModel @Inject constructor( }.onSuccess { // DOB is now on the doc — drop the in-memory handoff so a later sign-up can't inherit it. signupHandoff.clear(uid) - _uiState.update { it.copy(isLoading = false, success = true) } + _uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.PAIR_PROMPT) } }.onFailure { e -> _uiState.update { it.copy(isLoading = false, error = e.message ?: "Couldn't save your profile. Please try again.") } } @@ -199,4 +201,5 @@ class CreateProfileViewModel @Inject constructor( } fun dismissError() = _uiState.update { it.copy(error = null) } + fun onNavigated() = _uiState.update { it.copy(navigateTo = null) } }