[FIX/#294] SSE API 응답 구조 수정 - #295
Merged
Merged
Conversation
- GameResultStatusType에 CANCELED enum 추가 - MatchingViewModel 및 HomeViewModel에서 게임 취소 시 UI 상태 업데이트 로직 반영 - HomeViewModel 내 미사용 UserRepository import 제거
- GameResultStatusType에 CANCELED enum 추가 - MatchingViewModel 및 HomeViewModel에서 게임 취소 시 UI 상태 업데이트 로직 반영 - HomeViewModel 내 미사용 UserRepository import 제거
- NetworkErrorResponse DTO 및 Throwable 확장 함수 추가 - SubmitViewModel에서 에러 발생 시 throwable 메시지를 전달하도록 수정 - SubmitReviewScreen에서 실패 시 다이얼로그에 서버 에러 메시지 노출
- Requester의 userId를 profileId(requesterProfileId)로 변경 - MatchingReceivedDto의 sportId(Long)를 sportCode(String)로 변경 - 필드 변경에 따른 Mapper 및 ViewModel 로직 수정
- MatchingRequest, MatchingAccept, GameResultSubmitted, GameResultRejected, ReviewReceived 등 알림 관련 SSE 이벤트 타입 제거 - 관련 Dto(GameResultRejectedNotificationDto 등 5종) 및 Mapper 삭제 - `SseEventType` 및 `SseEvent` 모델에서 알림 관련 정의 제거 - `EventRepositoryImpl` 내 알림 이벤트 역직렬화 로직 제거
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughSSE 응답 구조를 재정의하여 여러 알림 DTO 및 이벤트 타입을 제거하고, 요청자/스포츠 식별자 필드명을 변경하며, 게임 결과 상태에 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
🧹 Nitpick comments (1)
app/src/main/java/com/smashing/app/presentation/splash/SplashViewModel.kt (1)
45-46: SSE 연결 완료 보장 없이 홈 이동이 발생합니다.현재는
onUserLoggedIn()이 fire-and-forget으로 실행되어, 홈 진입 직후 초기 SSE 이벤트를 놓칠 여지가 있습니다. 임시 로직 정리 시점에 연결 준비 상태를 확인한 뒤 네비게이션을 발생시키는 구조로 합치는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/smashing/app/presentation/splash/SplashViewModel.kt` around lines 45 - 46, The current flow fires sseManager.onUserLoggedIn() and immediately emits _sideEffect.emit(NavigateToHome), risking missed initial SSE events; change the logic so navigation happens only after SSE reports a ready/connected state. Concretely, update or overload sseManager.onUserLoggedIn() to provide a readiness signal (e.g., a suspend return value, callback, or expose a ConnectionState/Flow) and in SplashViewModel wait for that ready signal before calling _sideEffect.emit(NavigateToHome); alternatively observe sseManager.connectionState (or add one) and emit NavigateToHome only when it reports CONNECTED/READY. Ensure the modification references sseManager.onUserLoggedIn() and _sideEffect.emit(NavigateToHome).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/smashing/app/presentation/write/submit/SubmitViewModel.kt`:
- Around line 193-198: The failure branch in SubmitViewModel currently only
falls back on a default message when throwable.message is null, so empty or
whitespace messages slip through; update the onFailure handling (the
_uiState.update call that sets SubmitContract.SubmitUiState.Failure) to treat
empty/blank strings as missing by using a blank-aware check (e.g., use
throwable.message.takeIf { !it.isNullOrBlank() } ?: "매칭 결과를 확인해주세요." or
equivalent) so the default message is applied when message is null, empty, or
only whitespace.
---
Nitpick comments:
In `@app/src/main/java/com/smashing/app/presentation/splash/SplashViewModel.kt`:
- Around line 45-46: The current flow fires sseManager.onUserLoggedIn() and
immediately emits _sideEffect.emit(NavigateToHome), risking missed initial SSE
events; change the logic so navigation happens only after SSE reports a
ready/connected state. Concretely, update or overload
sseManager.onUserLoggedIn() to provide a readiness signal (e.g., a suspend
return value, callback, or expose a ConnectionState/Flow) and in SplashViewModel
wait for that ready signal before calling _sideEffect.emit(NavigateToHome);
alternatively observe sseManager.connectionState (or add one) and emit
NavigateToHome only when it reports CONNECTED/READY. Ensure the modification
references sseManager.onUserLoggedIn() and _sideEffect.emit(NavigateToHome).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2145cae5-599d-406a-8ee7-bd408cea76ea
📒 Files selected for processing (23)
app/src/main/java/com/smashing/app/core/designsystem/style/ButtonStyle.ktapp/src/main/java/com/smashing/app/data/mapper/event/EventMapper.ktapp/src/main/java/com/smashing/app/data/model/event/Requester.ktapp/src/main/java/com/smashing/app/data/model/event/SseEvent.ktapp/src/main/java/com/smashing/app/data/remote/dto/BaseResponse.ktapp/src/main/java/com/smashing/app/data/remote/dto/event/GameResultRejectedNotificationDto.ktapp/src/main/java/com/smashing/app/data/remote/dto/event/GameResultSubmittedNotificationDto.ktapp/src/main/java/com/smashing/app/data/remote/dto/event/MatchingAcceptNotificationDto.ktapp/src/main/java/com/smashing/app/data/remote/dto/event/MatchingReceivedDto.ktapp/src/main/java/com/smashing/app/data/remote/dto/event/MatchingRequestNotificationDto.ktapp/src/main/java/com/smashing/app/data/remote/dto/event/ReviewReceivedNotificationDto.ktapp/src/main/java/com/smashing/app/data/remote/dto/event/common/RequesterDto.ktapp/src/main/java/com/smashing/app/data/repository/impl/EventRepositoryImpl.ktapp/src/main/java/com/smashing/app/data/repository/impl/GameRepositoryImpl.ktapp/src/main/java/com/smashing/app/data/type/GameResultStatusType.ktapp/src/main/java/com/smashing/app/data/type/SseEventType.ktapp/src/main/java/com/smashing/app/presentation/home/HomeViewModel.ktapp/src/main/java/com/smashing/app/presentation/matching/MatchingViewModel.ktapp/src/main/java/com/smashing/app/presentation/mypage/MyPageViewModel.ktapp/src/main/java/com/smashing/app/presentation/splash/SplashViewModel.ktapp/src/main/java/com/smashing/app/presentation/withdraw/WithdrawViewModel.ktapp/src/main/java/com/smashing/app/presentation/write/submit/SubmitReviewScreen.ktapp/src/main/java/com/smashing/app/presentation/write/submit/SubmitViewModel.kt
💤 Files with no reviewable changes (7)
- app/src/main/java/com/smashing/app/data/remote/dto/event/MatchingRequestNotificationDto.kt
- app/src/main/java/com/smashing/app/data/remote/dto/event/MatchingAcceptNotificationDto.kt
- app/src/main/java/com/smashing/app/data/remote/dto/event/GameResultRejectedNotificationDto.kt
- app/src/main/java/com/smashing/app/data/repository/impl/EventRepositoryImpl.kt
- app/src/main/java/com/smashing/app/data/remote/dto/event/GameResultSubmittedNotificationDto.kt
- app/src/main/java/com/smashing/app/data/remote/dto/event/ReviewReceivedNotificationDto.kt
- app/src/main/java/com/smashing/app/data/model/event/SseEvent.kt
Comment on lines
+193
to
+198
| }.onFailure { throwable -> | ||
| _uiState.update { | ||
| it.copy( | ||
| submitUiState = SubmitContract.SubmitUiState.Failure("경기 결과 제출 실패"), | ||
| submitUiState = SubmitContract.SubmitUiState.Failure( | ||
| throwable.message ?: "매칭 결과를 확인해주세요." | ||
| ), |
There was a problem hiding this comment.
빈/공백 에러 메시지 처리 누락으로 안내 문구가 비어질 수 있습니다.
Line 197은 null만 fallback 처리합니다. "" 또는 " "인 경우 기본 문구가 적용되지 않습니다.
수정 제안
}.onFailure { throwable ->
_uiState.update {
it.copy(
submitUiState = SubmitContract.SubmitUiState.Failure(
- throwable.message ?: "매칭 결과를 확인해주세요."
+ throwable.message?.takeUnless { message -> message.isBlank() }
+ ?: "매칭 결과를 확인해주세요."
),
isResubmitDialogVisible = false,
isAlertDialogOpen = false,
isConfirmDialogOpen = true,
)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| }.onFailure { throwable -> | |
| _uiState.update { | |
| it.copy( | |
| submitUiState = SubmitContract.SubmitUiState.Failure("경기 결과 제출 실패"), | |
| submitUiState = SubmitContract.SubmitUiState.Failure( | |
| throwable.message ?: "매칭 결과를 확인해주세요." | |
| ), | |
| }.onFailure { throwable -> | |
| _uiState.update { | |
| it.copy( | |
| submitUiState = SubmitContract.SubmitUiState.Failure( | |
| throwable.message?.takeUnless { message -> message.isBlank() } | |
| ?: "매칭 결과를 확인해주세요." | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/smashing/app/presentation/write/submit/SubmitViewModel.kt`
around lines 193 - 198, The failure branch in SubmitViewModel currently only
falls back on a default message when throwable.message is null, so empty or
whitespace messages slip through; update the onFailure handling (the
_uiState.update call that sets SubmitContract.SubmitUiState.Failure) to treat
empty/blank strings as missing by using a blank-aware check (e.g., use
throwable.message.takeIf { !it.isNullOrBlank() } ?: "매칭 결과를 확인해주세요." or
equivalent) so the default message is applied when message is null, empty, or
only whitespace.
ShinHyeongcheol
approved these changes
Mar 30, 2026
ShinHyeongcheol
left a comment
ShinHyeongcheol
left a comment
Collaborator
There was a problem hiding this comment.
응답 구조 수정을 봤을 때는 문제 없는 것 같아요!!
BaseResponse에 NetworkErrorResponse처럼 만들까 말까 고민했었는데, 기존 에러 관련 Response 활용하는 부분 수정해봐도 좋겠네요
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue 🛠
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
SSE 관련 DTO 를 수정했습니다.
경기 결과 제출시에 정책(24시간 lock, 10분 lock, host lock)등 여러 락이 걸리는 경우가 있어, 임시로 다이얼로그로 error message 를 보여주고자 NetworkErrorResponse 로 처리해두었습니다.
아직 SSE 연결 로직 관련 리팩이 완전히 완료된건 아니여서 1차 스프린트까진 해당 상태로 임시로 두고 추후에 위에 적힌 남은 작업 pr 올릴게요 !
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
리팩토링