[FIX] 1차 QA 수정사항 반영 (온보딩/프로필) - #219
Hidden character warning
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough온보딩 흐름을 약관 → 콘텐츠 → 프로필 → 완료 순서로 변경했습니다. 닉네임 검증 결과는 일회성 토스트 이벤트로 전달합니다. 프로필 이스터에그 탈퇴 기능과 애플리케이션 재시작 처리를 제거했습니다. Changes온보딩 흐름 변경
프로필 이스터에그 탈퇴 제거
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Terms
participant Content
participant Profile
participant ViewModel
participant Done
Terms->>Content: 약관 동의 완료
Content->>Profile: 콘텐츠 선택 완료
Profile->>ViewModel: 닉네임 검증 요청
ViewModel-->>Profile: ShowNicknameToast 이벤트
Profile->>Done: 프로필 완료
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.kt`:
- Around line 149-162: In the result-handling flow around _profileEvent.emit,
compare the nickname associated with the availability response to the current
input before updating state or emitting ShowNicknameToast. Ignore stale
responses when they differ, and apply the existing success/failure behavior only
when the response matches the current nickname.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a9604dc1-98ff-405e-acf4-e643ae4d9e27
📒 Files selected for processing (10)
app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingProfileScreen.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingTermsScreen.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.ktapp/src/main/java/com/flint/presentation/onboarding/event/OnboardingProfileEvent.ktapp/src/main/java/com/flint/presentation/onboarding/navigation/OnboardingNavigation.ktapp/src/main/java/com/flint/presentation/profile/ProfileScreen.ktapp/src/main/java/com/flint/presentation/profile/ProfileViewModel.ktapp/src/main/java/com/flint/presentation/profile/component/ProfileTopSection.ktapp/src/main/java/com/flint/presentation/profile/sideeffect/ProfileSideEffect.kt
💤 Files with no reviewable changes (4)
- app/src/main/java/com/flint/presentation/profile/sideeffect/ProfileSideEffect.kt
- app/src/main/java/com/flint/presentation/profile/ProfileViewModel.kt
- app/src/main/java/com/flint/presentation/profile/component/ProfileTopSection.kt
- app/src/main/java/com/flint/presentation/profile/ProfileScreen.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt`:
- Around line 139-142: Update the onboarding title Text in
OnboardingContentScreen to use the complete phrase without a forced newline,
allowing it to remain on one line when it fits within the 16dp horizontal
padding and wrap naturally only when space is insufficient.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ce059229-a33f-416d-8a2a-c12106ae0c31
📒 Files selected for processing (2)
app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.kt
kimjw2003
left a comment
kimjw2003
left a comment
There was a problem hiding this comment.
온보딩 순서 변경과 토스트 이벤트화 잘 봤습니다. 머지 방향에 동의합니다.
좋았던 부분
토스트를 SharedFlow 1회성 이벤트로 옮긴 게 정확한 해법입니다. 상태 기반이면 화면 재진입 시 LaunchedEffect(key)가 다시 실행되면서 토스트가 되살아나는데, replay = 0인 SharedFlow는 구독 이후 발행분만 전달하므로 이 문제가 구조적으로 사라집니다. HomeSideEffect·ProfileSideEffect와 같은 관례라 일관성도 좋습니다.
stale 응답 가드는 QA에 안 잡혔을 실제 버그까지 잡았습니다.
토스트 상태를 Route로 호이스팅한 것도 방향이 맞습니다. OnboardingProfileScreen이 stateless해지면서 프리뷰에서 상태별 표현이 가능해졌고, 실제로 프리뷰 3종을 그에 맞게 갱신하신 점도 좋았습니다.
형식 오류 토스트 중복 발행 방지(previousErrorType != INVALID_FORMAT)로 타이핑 중 매 글자마다 토스트가 뜨는 것도 막으셨네요.
이스터에그 제거는 ProfileSideEffect.WithdrawSuccess와 authRepository 의존성까지 함께 정리해 깔끔합니다.
머지 전 봐주시면 좋을 것
인라인으로 2건 남겼습니다.
- 들여쓰기 (
OnboardingNavigation.kt) — 블록 교체하면서 어긋났습니다 checkNicknameDuplication—onFailure부재로 네트워크 실패가 무반응, 연타 시 토스트 중복
후속으로 봐도 될 것
토스트 문구가 두 곳에 이중 정의됐습니다.
OnboardingProfileUiState.hasError(OnboardingUiState.kt:43)와 errorMessage(:46)가 호출처 0건이 됐는데, 단순 데드코드가 아니라 errorMessage의 when 절과 _profileEvent.emit(...)이 같은 문자열을 각각 들고 있는 상태입니다.
// OnboardingUiState.kt (미사용)
NicknameErrorType.DUPLICATE -> "이미 사용 중인 닉네임입니다"
// OnboardingViewModel.kt (실제 사용)
message = "이미 사용 중인 닉네임입니다"한쪽만 고치면 어긋나니, 미사용 프로퍼티를 지우거나 반대로 이벤트에서 재사용하는 쪽으로 하나만 남기면 좋겠습니다.
findActivity 확장 함수가 미사용이 됩니다. ProfileScreen에서만 쓰이던 거라 이번 제거로 호출처가 0이 됩니다. (restartApplication은 MainActivity 내부에서 계속 쓰이므로 괜찮습니다.)
확인 부탁드려요
1. 형식 오류 토스트가 타이핑 중에 뜨는 게 의도인가요?
QA TC 1-40은 시트상 "확인 버튼" 기능 하위 항목이라 버튼 클릭 시 노출이 기대 동작으로 읽힙니다. 그런데 canCheckNickname이 isFormatValid를 요구해서 형식 오류일 땐 버튼이 비활성이고, 결과적으로 서버 검사 경로로는 이 토스트가 뜰 수 없습니다. 지금처럼 타이핑 중 발행이 유일한 경로인데 기획 의도와 맞는지만 확인 부탁드립니다.
2. 같은 패턴이 프로필 수정 화면에 그대로 남아 있습니다.
EditProfileScreen.kt:106의 LaunchedEffect(uiState.hasError, uiState.errorMessage)가 이 PR이 온보딩에서 고치는 바로 그 상태 기반 토스트입니다. 설정 > 프로필 수정에서도 재진입 시 토스트가 되살아날 가능성이 있어 보입니다. 이번 범위 밖이면 별도 이슈로 잡아두면 좋겠습니다.
3. PR 설명과 구현이 조금 다릅니다.
"좌우 16dp 패딩 안에서 한 줄로 안 들어갈 때만 수동 줄바꿈되도록 개선" 이라고 쓰셨는데, 실제로는 onTextLayout 기반 적응 로직을 제거하고 "내 취향에 가까운 작품\n7개를 골라주세요"로 고정하셨습니다. 닉네임이 빠져 문자열이 정적이 됐으니 고정 줄바꿈이 더 단순하고 맞는 선택이라, 설명만 정정하면 될 것 같습니다.
다만 display2M28(28sp)에 고정 \n이라 작은 화면이나 글꼴 배율을 키운 환경에서 첫 줄이 넘칠 수 있습니다. 최소 폭 기기에서 한 번만 확인해주세요.
| } | ||
|
|
||
| composable<Route.OnboardingProfile> { backStackEntry -> | ||
| composable<Route.OnboardingContent> { backStackEntry -> |
There was a problem hiding this comment.
블록을 서로 바꾸시면서 들여쓰기가 어긋났습니다.
- 66행
composable<Route.OnboardingContent>→ 12칸 (다른composable블록들과 맞추려면 8칸) - 74행 닫는 괄호
)→ 16칸 (12칸)
OnboardingProfile 블록은 오히려 정렬이 맞춰졌는데, 원래 그쪽에 있던 어긋난 들여쓰기가 이쪽으로 옮겨온 모양새입니다. ktlint indent 대상이라 ./gradlew ktlintFormat 한 번 돌려주시면 될 것 같습니다.
|
|
||
| viewModelScope.launch { | ||
| userRepository.checkNickname(currentNickname).onSuccess { result -> | ||
| // 응답이 오는 사이 닉네임이 바뀌었다면 이미 stale한 결과이므로 반영하지 않음 |
There was a problem hiding this comment.
stale 응답 가드 추가 좋습니다. 빠르게 타이핑하다 확인을 누르면 이전 요청 응답이 최신 입력을 덮어쓰던 문제가 이걸로 막힙니다.
이 블록을 손보시는 김에 두 가지만 같이 봐주시면 좋겠습니다.
1. onFailure가 없어 네트워크 실패가 완전히 묻힙니다
checkNickname에 onSuccess만 있어서, 실패 시 토스트도 로그도 없이 무반응입니다. 사용자 입장에선 확인 버튼을 눌렀는데 아무 일도 일어나지 않습니다.
기존부터 있던 문제라 이 PR이 만든 건 아니지만, 최소한 Timber.e 정도는 필요해 보입니다. QA TC 1-45(네트워크 끊김 시 전체오류 모달 노출)와도 연결되는 지점이라 토스트까지 띄우면 더 좋겠습니다.
2. 확인 버튼 연타 시 요청과 토스트가 중복됩니다
이 코루틴이 Job으로 관리되지 않아 연타하면 요청이 여러 개 나갑니다. 추가하신 stale 가드는 닉네임이 바뀐 경우만 걸러내기 때문에, 같은 닉네임으로 연타하면 그대로 통과해서 _profileEvent가 여러 번 발행되고 토스트도 그만큼 뜹니다.
같은 ViewModel의 searchJob 패턴처럼 이전 Job을 취소하거나, in-flight 플래그로 막으면 해결됩니다.
private var nicknameCheckJob: Job? = null
fun checkNicknameDuplication() {
...
nicknameCheckJob?.cancel()
nicknameCheckJob = viewModelScope.launch { ... }
}
|
기획 의도상 강제 줄바꿈이 맞아서 반영하지 않았고, 들여쓰기, 토스트 문제를 해결했습니다. |
2 participants
📮 관련 이슈
📌 작업 내용
Summary by CodeRabbit
새 기능
개선 사항