[Refactor/#74] 로그인 성공/실패 로직을 개선합니다. #82
Conversation
📋 설명이 PR은 로그인/회원가입 흐름에서 로딩 상태 관리를 추가하고 에러 처리를 개선하며, UI 텍스트 및 버튼 동작을 조정합니다. 또한 디자인 시스템의 버튼 컴포넌트를 리플 효과 없이 변경하고 네비게이션 백스택 처리를 최적화합니다. 📊 변경사항
⏱️ 코드 리뷰 예상 난이도🎯 2 (Simple) | ⏱️ ~12 분 🔗 관련 PR
🐰 축하 시
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/SignInViewModel.kt (1)
72-90:signIn()에서 로딩 상태 관리가 누락되었습니다.
requestAuthCode()와 달리signIn()에서는 API 호출 전isLoading = true설정과 완료 후isLoading = false리셋이 누락되어 있습니다. 이로 인해:
- 로그인 중 로딩 UI가 표시되지 않습니다.
- 성공/실패 시
isLoading이 리셋되지 않아 이전 상태가 유지됩니다.🔧 제안하는 수정 사항
fun signIn() { viewModelScope.launch { if (_uiState.value.isLoading) return@launch + _uiState.update { it.copy(isLoading = true) } val phoneNumber = _uiState.value.phoneNumber val authCode = _uiState.value.authCode signInUseCase(phoneNumber, authCode).fold( - onSuccess = { userRole -> handleNavigationForRole(userRole) }, + onSuccess = { userRole -> + _uiState.update { it.copy(isLoading = false) } + handleNavigationForRole(userRole) + }, onFailure = { error -> Timber.e("signIn: $error") _uiState.update { it.copy( + isLoading = false, isAuthCodeError = true, authCodeErrorMessage = "인증번호가 일치하지 않아요.\n다시 확인해주세요.", ) } }, ) } }feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/splash/SplashViewModel.kt (1)
56-64:clearBackStack = true를 사용하면popUpTo옵션은 무시되므로 제거하세요.Kdoc에 따르면
clearBackStack = true가popUpTo옵션보다 우선하며 전체 스택을 초기화합니다. 실제 구현(ObserveNavigationEvents.kt)도clearBackStack이 활성화되면 독립적으로popUpTo(navController.graph.id)를 실행하여, 지정된popUpTo = AppRoute.Splash와inclusive = true는 무시됩니다. 이 조합은 혼동을 초래할 수 있으므로 다음과 같이 정리하는 것을 권장합니다:options = NavigationOptions( clearBackStack = true )전체 백스택 초기화가 의도라면
popUpTo와inclusive는 불필요합니다.
🧹 Nitpick comments (4)
feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/SignInScreen.kt (1)
169-171:isLoading상태를 버튼 비활성화에 활용하는 것을 고려해 주세요.
SignInUiState에isLoading프로퍼티가 추가되었지만, 인증 확인 버튼과 인증 요청 버튼에서 로딩 중 비활성화 처리가 되어 있지 않습니다. API 호출 중 중복 요청을 방지하려면 버튼의enabled조건에!uiState.isLoading을 추가하는 것이 좋습니다.♻️ 제안하는 수정 사항
MaButton( onClick = onAuthConfirmClick, - enabled = uiState.authCode.isNotEmpty(), + enabled = uiState.authCode.isNotEmpty() && !uiState.isLoading, modifier = Modifier인증 요청 버튼(Line 120-131)에도 동일하게 적용:
MaButton( onClick = onAuthCodeRequestClick, enabled = !uiState.isLoading, // 추가 colors = MaButtonDefaults.maBlackButtonColors(), ... )feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signup/SignUpPhoneAuthScreen.kt (1)
171-173:isLoading상태를 버튼 비활성화에 활용하는 것을 고려해 주세요.
SignInScreen과 동일하게,SignUpPhoneAuthUiState에 추가된isLoading프로퍼티가 버튼 비활성화에 사용되지 않고 있습니다. ViewModel에서 중복 요청 방지를 하고 있지만, UI 레벨에서도 로딩 중 버튼을 비활성화하면 사용자에게 더 명확한 피드백을 제공할 수 있습니다.♻️ 제안하는 수정 사항
MaButton( onClick = onAuthConfirmClick, - enabled = uiState.authCode.isNotEmpty(), + enabled = uiState.authCode.isNotEmpty() && !uiState.isLoading, modifier = Modifierfeature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signup/SignUpSharedViewModel.kt (2)
112-113:signUp성공 시isLoading상태 리셋 누락 확인
signUp성공 시isLoading = false를 설정하지 않고 바로 네비게이션됩니다. 현재navigateToComplete가popUpTo로 화면을 제거하므로 실질적인 문제는 없지만, 네비게이션 지연이나 실패 시 로딩 상태가 유지될 수 있습니다. 명시적으로 리셋하는 것이 더 안전합니다.♻️ 제안하는 수정 사항
).fold( - onSuccess = { navigateToComplete() }, + onSuccess = { + _signUpPhoneAuthUiState.update { it.copy(isLoading = false) } + navigateToComplete() + }, onFailure = { error ->
91-91: 하드코딩된 에러 메시지에 대해 문자열 리소스 사용을 고려해 주세요.에러 메시지가 한국어로 하드코딩되어 있습니다. 다국어 지원이 필요한 경우
strings.xml리소스를 사용하는 것이 좋습니다. 현재 한국어만 지원한다면 문제없지만, 향후 확장성을 위해 참고하시기 바랍니다.Also applies to: 119-119
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
core/designsystem/src/main/kotlin/com/moa/app/designsystem/component/core/button/MaButton.ktfeature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/SignInScreen.ktfeature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/SignInViewModel.ktfeature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/model/SignInUiState.ktfeature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signup/SignUpPhoneAuthScreen.ktfeature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signup/SignUpSharedViewModel.ktfeature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signup/model/SignUpPhoneAuthUiState.ktfeature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/splash/SplashViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/SignInViewModel.kt (1)
feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/splash/SplashViewModel.kt (2)
handleNavigationForRole(47-54)navigateToRoute(56-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Unit Tests
🔇 Additional comments (9)
core/designsystem/src/main/kotlin/com/moa/app/designsystem/component/core/button/MaButton.kt (1)
28-28:clickableWithoutRipple확장 함수 구현 확인됨
clickableWithoutRipple확장 함수는core/ui/src/main/java/com/moa/app/ui/extension/Modifier.kt에서 올바르게 구현되어 있으며,indication = null로 설정되어 리플 효과를 제거합니다.interactionSource는isPressed상태를 통한 커스텀 프레스 피드백 유지를 위해 전달됩니다. 변경사항은 적절합니다.feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/model/SignInUiState.kt (1)
4-5: LGTM!
isLoading프로퍼티 추가와 초기화가 적절합니다. 로딩 상태를 UI 상태 클래스에서 명시적으로 관리하는 좋은 패턴입니다.Also applies to: 14-15
feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signup/model/SignUpPhoneAuthUiState.kt (1)
4-5: LGTM!
SignInUiState와 일관된 패턴으로isLoading프로퍼티가 추가되었습니다. 온보딩 플로우 전반에 걸쳐 로딩 상태 관리가 통일되어 있습니다.Also applies to: 14-15
feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/SignInScreen.kt (1)
114-114: LGTM!플레이스홀더 텍스트가 하이픈 없는 전화번호 형식으로 변경되었습니다.
feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signup/SignUpPhoneAuthScreen.kt (1)
116-116: LGTM!
SignInScreen과 일관되게 플레이스홀더 텍스트가 업데이트되었습니다.feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signup/SignUpSharedViewModel.kt (1)
76-97: LGTM!
isLoading가드와 상태 전환이 적절하게 구현되어 있습니다. 중복 API 호출 방지와 에러 처리가 잘 되어 있습니다.feature/onboarding/src/main/kotlin/com/moa/app/feature/onboarding/signin/SignInViewModel.kt (3)
32-32: LGTM!타입 추론을 사용한 리팩토링으로 코드가 더 간결해졌습니다.
46-70: LGTM!로딩 상태 관리와 에러 처리가 적절하게 구현되었습니다. 동시 요청 방지 가드, API 호출 전/후 로딩 상태 업데이트, 그리고 성공/실패 시 적절한 UI 상태 업데이트가 잘 되어 있습니다.
92-99: LGTM!
UserRole.CHILD에 대한 네비게이션이SplashViewModel의 동작과 일치하도록GuardianHome으로 수정되었습니다.
Related issue 🛠
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
스타일
✏️ Tip: You can customize this high-level summary in your review settings.