Repository navigation
Feat/qa 59 60 홈 화면 티켓 안내 토스트 & 대화 이어하기 버튼 작업 완료 - #275
Conversation
Walkthrough홈 화면의 채팅 재개/삭제 흐름과 티켓 UI를 리팩터링했습니다. HomeState에 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Screen as HomeScreen
participant ViewModel as HomeViewModel
participant Repo as Repository
User->>Screen: "대화 시작" 또는 "대화 이어하기" 클릭
Screen->>ViewModel: handleEnterChat()
ViewModel->>ViewModel: state.isChatTempSaved 확인
alt 임시저장된 채팅 있음
ViewModel->>Screen: emit EnterChatRoom(entry=Resume)
else 티켓 부족
ViewModel->>Screen: emit TicketNotEnough
else 정상 시작
ViewModel->>Screen: emit EnterChatRoom(entry=New)
end
Screen->>Screen: 네비게이션(사이드이펙트 처리)
sequenceDiagram
participant User as 사용자
participant Screen as HomeScreen
participant ViewModel as HomeViewModel
participant Repo as Repository
User->>Screen: 채팅 삭제 버튼 클릭
Screen->>ViewModel: handleDeletePendingChat()
ViewModel->>ViewModel: loading = true
ViewModel->>Repo: deleteChatRoom(roomId)
Repo-->>ViewModel: 성공/실패 응답
alt 삭제 성공
ViewModel->>ViewModel: initChatState()로 상태 갱신 (roomId/isChatTempSaved 반영)
ViewModel->>ViewModel: loading = false
else 삭제 실패
ViewModel->>ViewModel: loading = false
ViewModel->>ViewModel: 예외 전달
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feat/home/src/main/java/com/emotionstorage/home/presentation/HomeViewModel.kt (1)
198-225:⚠️ Potential issue | 🟠 Major
handleDeletePendingChat()에서 에러 발생 시isLoading이true로 남아있는 버그가 있어요.
onError콜백에서BaseException을 throw하면, Line 222-224의reduce { state.copy(isLoading = false) }가 실행되지 않아서 로딩 상태가 영원히 유지될 수 있어요.initChatState()가 실패하는 경우에도 마찬가지입니다.
try-finally패턴이나, 에러 핸들러 내에서isLoading = false로 설정해주시는 게 안전해요.🛠️ 수정 제안
private fun handleDeletePendingChat() = baseIntent { if (state.roomId == null) { throw IllegalStateException("chatRoomId is null") } reduce { state.copy(isLoading = true) } - deleteChatRoom(state.roomId!!).handle( - onSuccess = { - Logger.d("HomeViewModel: exitChatRoom success") - // refresh chat state - initChatState() - }, - onError = { throwable, code, data -> - Logger.e("HomeViewModel: exitChatRoom failed $throwable") - throw BaseException( - cause = throwable, - code = code, - message = throwable.message ?: "Home exit chat error", - ) - }, - ) - reduce { - state.copy(isLoading = false) + try { + deleteChatRoom(state.roomId!!).handle( + onSuccess = { + Logger.d("HomeViewModel: exitChatRoom success") + // refresh chat state + initChatState() + }, + onError = { throwable, code, data -> + Logger.e("HomeViewModel: exitChatRoom failed $throwable") + throw BaseException( + cause = throwable, + code = code, + message = throwable.message ?: "Home exit chat error", + ) + }, + ) + } finally { + reduce { + state.copy(isLoading = false) + } } }참고로, 이전 학습에서
AIChatViewModel의handleTempSave()에서도 비슷한 에러 핸들링 이슈가 있었고, 별도 이슈로 개선 예정이라고 알고 있어요. 이 부분도 함께 개선하시면 좋을 것 같습니다. Based on learnings: "The team plans to improve network error handling in a separate issue."
🤖 Fix all issues with AI agents
In
`@feat/home/src/main/java/com/emotionstorage/home/presentation/HomeViewModel.kt`:
- Around line 93-100: The launched chatStateJob (created by
baseViewModelScope.launch { initChatState() }) is not awaited, causing a race
where reduce { state.copy(isLoading = false) } may run before initChatState()
completes and before roomId is set (leading to handleEnterChat() throwing). Fix
by including chatStateJob in the joinAll call (i.e., joinAll(nicknameJob,
homeStateJob, chatStateJob)) so all three jobs complete before setting isLoading
to false; alternatively, ensure the UI disables the enter chat button until
chatStateJob finishes, but prefer adding chatStateJob to joinAll for this
change.
- Around line 182-196: There is a race where handleEnterChat() can be called
before initChatState() completes leaving state.roomId null; update
handleInitiate() to await chatStateJob as well (add chatStateJob to the
joinAll(nicknameJob, homeStateJob) call) so initChatState()/getChatRoomSession()
finishes before UI becomes interactive, or alternatively ensure the enter-chat
UI/button is disabled until initChatState() completes; reference
handleEnterChat(), handleInitiate(), chatStateJob, initChatState(),
getChatRoomSession(), and state.roomId when making the change.
In `@feat/home/src/main/java/com/emotionstorage/home/ui/HomeScreen.kt`:
- Around line 378-396: The StartChatButton call is using its default ticketLimit
(10) instead of the dynamic value stored in HomeState, so update the call to
pass state.ticketLimit into StartChatButton (ensure the prop name ticketLimit is
used) so the UI reflects changes from initHomeState()/HomeState; if
StartChatButton's signature doesn't accept ticketLimit yet, add that parameter
to StartChatButton and thread it through the component so the button reads the
runtime ticketLimit from the state instead of the hardcoded default.
🧹 Nitpick comments (4)
feat/home/src/main/java/com/emotionstorage/home/presentation/HomeViewModel.kt (1)
200-207:state.roomId!!비-null 단언 대신 로컬 변수를 사용하시면 더 안전해요.Line 200에서 null 체크를 하고 Line 207에서
!!를 사용하고 있는데요, 로컬 변수로 캡처하면 smart cast도 되고,!!사용도 피할 수 있어요.♻️ 리팩토링 제안
private fun handleDeletePendingChat() = baseIntent { - if (state.roomId == null) { - throw IllegalStateException("chatRoomId is null") - } + val roomId = state.roomId + ?: throw IllegalStateException("chatRoomId is null") reduce { state.copy(isLoading = true) } - deleteChatRoom(state.roomId!!).handle( + deleteChatRoom(roomId).handle(feat/home/src/main/java/com/emotionstorage/home/ui/HomeScreen.kt (3)
388-395: 토스트 메시지에 "10번"이 하드코딩되어 있어요.
"하루에 최대 10번까지..."메시지에서10이 하드코딩되어 있는데,state.ticketLimit값을 사용하면 서버 설정 변경 시에도 일관성 있게 표시할 수 있어요.- snackbarState.showSnackbar( - "하루에 최대 10번까지 감정대화를 나눌 수 있어요.\n" + - "자정 이후에는 횟수가 다시 충전돼요.", - ) + snackbarState.showSnackbar( + "하루에 최대 ${state.ticketLimit}번까지 감정대화를 나눌 수 있어요.\n" + + "자정 이후에는 횟수가 다시 충전돼요.", + )위 수정을 적용하시려면
onTicketInfoClick람다가state에 접근할 수 있어야 하니,StartChatButton외부에서 메시지를 구성하거나,ticketLimit을 콜백에 전달하는 방식이 필요합니다. 현재StatelessHomeScreen스코프에서는state에 접근 가능하니 이 방식이 가장 간단해요.
119-123:state.value.roomId가null일 때 네비게이션이 무시돼요.
EnterChatRoomside effect가 발생했는데roomId가null이면navToChat이 호출되지 않고 조용히 무시되는데요, 이 경우 사용자는 아무 반응 없이 멈춘 것처럼 보일 수 있어요. ViewModel에서roomId유효성 검사를 먼저 하고 있지만, 방어적으로 여기서도 에러 처리(로그나 토스트)를 추가하시면 디버깅에 도움이 될 거예요.
543-569:HomeStateProvider에서isLoading = false를 명시적으로 설정하면 Preview가 더 정확해져요.현재
sampleState의isLoading기본값이true(HomeState 기본값)라서, 모든 Preview에 로딩 오버레이가 표시될 수 있어요. Preview 목적에 맞게isLoading = false를 추가해주시면 좋겠습니다.♻️ 수정 제안
class HomeStateProvider( sampleState: HomeState = HomeState( + isLoading = false, nickname = "찡찡이", roomId = 1, keyCount = 10,
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feat/home/src/main/java/com/emotionstorage/home/presentation/HomeViewModel.kt (1)
198-225:⚠️ Potential issue | 🟠 Major
handleDeletePendingChat에서 에러 발생 시isLoading이true로 남을 수 있어요.안녕하세요! 한 가지 살펴볼 부분이 있는데요 😊
Line 222-224의
reduce { state.copy(isLoading = false) }는deleteChatRoom().handle()과initChatState()모두 성공한 경우에만 도달해요.onError에서BaseException을 throw하면 Line 222에 도달하지 못해서 로딩 상태가 해제되지 않습니다.
initHomeState()나initNickname()에서는 에러 시isLoading = false를 설정한 후 throw하는 패턴을 사용하고 있는데, 여기서는 그 처리가 빠져있어요.try-finally패턴이나 에러 핸들러 내에서 로딩 해제를 추가해주시면 좋겠어요.♻️ 수정 제안
private fun handleDeletePendingChat() = baseIntent { if (state.roomId == null) { throw IllegalStateException("chatRoomId is null") } reduce { state.copy(isLoading = true) } - deleteChatRoom(state.roomId!!).handle( - onSuccess = { - Logger.d("HomeViewModel: exitChatRoom success") - // refresh chat state - initChatState() - }, - onError = { throwable, code, data -> - Logger.e("HomeViewModel: exitChatRoom failed $throwable") - throw BaseException( - cause = throwable, - code = code, - message = throwable.message ?: "Home exit chat error", - ) - }, - ) - reduce { - state.copy(isLoading = false) - } + try { + deleteChatRoom(state.roomId!!).handle( + onSuccess = { + Logger.d("HomeViewModel: exitChatRoom success") + // refresh chat state + initChatState() + }, + onError = { throwable, code, data -> + Logger.e("HomeViewModel: exitChatRoom failed $throwable") + throw BaseException( + cause = throwable, + code = code, + message = throwable.message ?: "Home exit chat error", + ) + }, + ) + } finally { + reduce { + state.copy(isLoading = false) + } + } }
🧹 Nitpick comments (3)
feat/home/src/main/java/com/emotionstorage/home/ui/HomeScreen.kt (1)
119-123:roomId가null일 때 네비게이션이 조용히 무시돼요.
state.value.roomId?.let으로 처리하고 있어서, 만약roomId가null인 상태에서EnterChatRoomside effect가 발생하면 아무런 동작 없이 넘어가요. ViewModel 쪽에서는roomId == null이면IllegalStateException을 던지도록 되어 있지만, 혹시라도 side effect가 도달했는데roomId가 null이면 사용자 입장에서 버튼을 눌러도 아무 반응이 없는 상황이 될 수 있어요.로깅이나 에러 토스트를 추가해두면 디버깅할 때 도움이 될 것 같아요.
♻️ 수정 제안
is HomeSideEffect.EnterChatRoom -> { - state.value.roomId?.let { - navToChat(it, sideEffect.entry) - } + val roomId = state.value.roomId + if (roomId != null) { + navToChat(roomId, sideEffect.entry) + } else { + Logger.e("EnterChatRoom side effect received but roomId is null") + } }feat/home/src/main/java/com/emotionstorage/home/presentation/HomeViewModel.kt (2)
204-207:state.roomId!!대신 로컬 변수를 활용해주세요.Line 200에서 null 체크를 했지만, Line 207에서
state.roomId!!로 강제 언래핑하고 있어요.state는 mutable이라 이론적으로 reduce 호출 사이에 값이 바뀔 수 있고,!!는 코드 리뷰 시 항상 주의가 필요한 패턴이에요. 체크 시점에 로컬 변수로 캡처하면 더 안전합니다.♻️ 수정 제안
private fun handleDeletePendingChat() = baseIntent { - if (state.roomId == null) { - throw IllegalStateException("chatRoomId is null") - } + val roomId = state.roomId + ?: throw IllegalStateException("chatRoomId is null") reduce { state.copy(isLoading = true) } - deleteChatRoom(state.roomId!!).handle( + deleteChatRoom(roomId).handle(
182-196:handleEnterChat에서도 동일하게 로컬 변수 패턴 적용을 권장드려요.
handleDeletePendingChat과 마찬가지로,state.roomId를 로컬 변수로 캡처하면 일관성 있고 안전한 코드가 돼요. 현재는 null 체크 후state.roomId를 직접 사용하진 않지만, side effect 쪽(EnterChatRoom)에서roomId가 필요할 때를 대비하면 좋습니다.♻️ 수정 제안
private fun handleEnterChat() = baseIntent { - if (state.roomId == null) { - throw IllegalStateException("chatRoomId is null") - } + val roomId = state.roomId + ?: throw IllegalStateException("chatRoomId is null") if (state.isChatTempSaved) {
There was a problem hiding this comment.
room id 값을 상태로 갖는 방향도 좋은 것 같네요!!
Related Issue
작업 상세 내용
Summary by CodeRabbit
릴리즈 노트
새로운 기능
버그 수정