Feature/#227 세션 상세 남은 작업 및 기타 작업 #228
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough세션 상세 정보 API를 v1에서 v2로 업데이트하고, 날짜/시간 필드를 개별 문자열에서 LocalDateTime으로 통합했습니다. 네비게이션 애니메이션을 추가하고, SessionScreen UI를 조정하며, 예외 처리를 개선했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/data/src/main/java/com/yapp/core/data/remote/model/response/SessionDetailResponse.kt (1)
24-37: Repository 레이어에서 날짜 파싱 예외 처리 필요검증 결과, 지적 사항이 정당합니다.
ScheduleRepositoryImpl.kt74번 줄에서toSessionDetailInfo()호출 시 try-catch 없음SessionDetailResponse.kt29-30번 줄의LocalDateTime.parse()호출에서DateTimeParseException발생 가능- 예외가 ViewModel의
runCatchingIgnoreCancelled에서는 처리되지만, Repository 레이어 자체에 명시적 예외 처리가 없음데이터 변환 오류는 발생 지점인 Repository 레이어에서 처리해야 합니다. ViewModel이 예외를 처리하기 전에 Repository에서 명확한 예외 처리를 추가하는 것을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
app/src/main/java/com/yapp/app/official/navigation/NavigationAnimations.kt(1 hunks)app/src/main/java/com/yapp/app/official/navigation/YappNavHost.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/remote/api/ScheduleApi.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/remote/model/response/SessionDetailResponse.kt(2 hunks)core/model/src/main/java/com/yapp/model/SessionDetailInfo.kt(1 hunks)core/model/src/main/java/com/yapp/model/exceptions/YappExceptions.kt(2 hunks)core/ui/src/main/java/com/yapp/core/ui/util/DateTime.kt(1 hunks)feature/home/src/main/java/com/yapp/feature/home/HomeViewModel.kt(3 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/component/DateGroupedScheduleItem.kt(1 hunks)feature/session/src/main/java/com/yapp/feature/session/SessionContract.kt(1 hunks)feature/session/src/main/java/com/yapp/feature/session/SessionScreen.kt(3 hunks)feature/session/src/main/java/com/yapp/feature/session/SessionViewModel.kt(1 hunks)
⏰ 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). (2)
- GitHub Check: ci_dev (test, ./gradlew clean testDebugUnitTest) / build_check
- GitHub Check: ci_dev (build, ./gradlew app:assembleDebug) / build_check
🔇 Additional comments (13)
feature/schedule/src/main/java/com/yapp/feature/schedule/component/DateGroupedScheduleItem.kt (1)
57-57: 날짜 영역 너비 조정을 확인해 주세요.
showMonth가 false일 때 날짜 영역 너비를 58.dp에서 60.dp로 2dp 증가시켰습니다. v2 API 마이그레이션 및 날짜 포맷 변경에 따른 조정으로 보입니다.다양한 날짜 형식과 로케일에서 날짜 텍스트가 잘 정렬되고 잘리지 않는지 시각적으로 확인해 주세요.
core/model/src/main/java/com/yapp/model/exceptions/YappExceptions.kt (1)
30-30: 새로운 예외 타입 추가가 적절합니다.
BRD_1001매핑과UndefineNoticeWriterInfo예외 클래스가 올바르게 추가되었으며, 다른 예외들과 일관된 패턴을 따르고 있습니다.Also applies to: 59-59
feature/home/src/main/java/com/yapp/feature/home/HomeViewModel.kt (1)
21-21: 예외 처리가 적절합니다.
UndefineNoticeWriterInfo를 no-op으로 처리하는 것은 다른 비중요 예외들(NoScheduledSessionException,NotFoundException)과 일관된 패턴을 따르고 있습니다.Also applies to: 153-153, 177-177
feature/session/src/main/java/com/yapp/feature/session/SessionScreen.kt (1)
179-192: UI 개선이 적절합니다.날짜/시간 표시 영역의 변경사항들이 올바릅니다:
Alignment.Top으로 변경하여 여러 줄 텍스트의 정렬 개선- 시계 아이콘에서 달력 아이콘으로 변경하여 날짜 중심의 정보 표시에 더 적합
app/src/main/java/com/yapp/app/official/navigation/YappNavHost.kt (1)
31-34: 네비게이션 애니메이션 추가가 적절합니다.기존의
None트랜지션을 커스텀 애니메이션으로 교체하여 일관된 화면 전환 효과를 제공합니다. Forward 및 Pop 네비게이션 모두에 대한 트랜지션이 정의되어 있어 완전한 구현입니다.core/data/src/main/java/com/yapp/core/data/remote/api/ScheduleApi.kt (1)
31-34: API 버전 업그레이드가 올바릅니다.세션 상세 엔드포인트를 v2로 업그레이드하여 새로운 날짜/시간 형식(
startDateTime,endDateTime)을 지원합니다. 메서드 시그니처는 변경되지 않아 호출 코드에 영향이 없습니다.core/model/src/main/java/com/yapp/model/SessionDetailInfo.kt (1)
5-16: 리뷰 댓글이 부정확합니다.검색 결과에 따르면,
SessionDetailInfo의 필드 변경은 국한된 모델 변경이며 광범위한 공개 API 변경이 아닙니다.이전 필드명(
startDate,startTime,endDate,endTime,startDayOfWeek,endDayOfWeek)의 사용은ScheduleInfo,UpcomingSessionInfo,Sessions등 다른 모델에만 남아 있습니다.SessionDetailInfo는SessionContract.kt,SessionDetailResponse.kt,ScheduleRepositoryImpl.kt,ScheduleRepository.kt총 4개 파일에서만 참조되며, 모두 새로운LocalDateTime필드와 호환되게 처리되어 있습니다.따라서 변경사항은 이미 적절히 격리되어 있으며, 추가적인 전체 업데이트 확인이 필요하지 않습니다.
Likely an incorrect or invalid review comment.
app/src/main/java/com/yapp/app/official/navigation/NavigationAnimations.kt (3)
14-22: 진입 트랜지션 정의 적절 — 기본 방향/페이드 조합 LGTM표준화된 200ms tween과 페이드인이 명확합니다.
35-44: Pop 진입 initialOffset 타입도 동일 이슈 가능위와 같은 이유로 Compose 버전에 따라 IntOffset이 필요할 수 있습니다.
대안 패치:
+import androidx.compose.ui.unit.IntOffset @@ - initialOffset = { (it * 0.3f).toInt() } + initialOffset = { fullSize -> IntOffset((fullSize.width * 0.3f).toInt(), 0) }
24-33: 리뷰 코멘트 무효화: slideOutOfContainer API 시그니처는 Int 파라미터 사용현재 코드는 정확하며 문제가 없습니다. 리뷰의 우려사항이 부정확합니다.
Compose 2024.12.01에서
slideOutOfContainer()의targetOffset파라미터는(offsetForFullSlide: Int) -> Int시그니처를 사용합니다. 제안된IntOffset사용은 다른 함수(slideOut,slideIn)와 혼동한 것입니다:
- 현재 코드:
targetOffset = { (it * 0.3f).toInt() }— 정확함 ✓- 리뷰 제안:
IntOffset(...)을 사용하는 수정안 — 잘못됨 (컴파일 실패)
it은 정수형 오프셋 값이며, 람다는 정수를 반환해야 합니다. 코드는 컴파일되고 정상 작동합니다.Likely an incorrect or invalid review comment.
core/ui/src/main/java/com/yapp/core/ui/util/DateTime.kt (1)
124-126: API 브레이킹 변경 마이그레이션 완료 확인됨호출처 검색 결과,
formatSessionDateTime은 단 하나의 호출처(feature/session/.../SessionContract.kt:12)에서만 사용되고 있으며, 이미 새로운 서명(LocalDateTime 파라미터)으로 마이그레이션되어 있습니다. 미마이그레이션 호출처는 없습니다.feature/session/src/main/java/com/yapp/feature/session/SessionContract.kt (2)
19-29: SessionIntent.Refresh 제거 확인 완료 - 추가 조치 불필요코드베이스 전체를 검증한 결과, SessionIntent.Refresh 제거가 완벽하게 이루어졌습니다. SessionViewModel의 onIntent() 메서드에서 모든 현재 인텐트를 처리하고 있으며, SessionScreen에는 pull-to-refresh나 재시도 버튼이 없습니다. 잔존 참조가 없으므로 추가 조치가 필요하지 않습니다.
7-7: Based on my analysis of the script results and web search findings, I can now generate the rewritten review comment.초기 로딩 상태 기본값 검증 완료 — 중복 호출 방지 로직 정상
초기 진입 시
isLoading = true로 설정된 상태에서,SessionIntent.EnterSessionScreen핸들러의 가드 조건if (state.isLoading.not()) return은 올바르게 작동합니다. 첫 호출 시true.not()은 false이므로 조건이 통과되어loadSessionDetail을 실행하고, 로딩 완료 후isLoading이 false로 전환되면 재호출 시 조건이 true가 되어 조기 반환되므로 중복 호출이 자동으로 방지됩니다. 현재의 로직은 의도대로 작동하고 있습니다.
app/src/main/java/com/yapp/app/official/navigation/NavigationAnimations.kt
Show resolved
Hide resolved
feature/session/src/main/java/com/yapp/feature/session/SessionContract.kt
Outdated
Show resolved
Hide resolved
feature/session/src/main/java/com/yapp/feature/session/SessionScreen.kt
Outdated
Show resolved
Hide resolved
feature/session/src/main/java/com/yapp/feature/session/SessionViewModel.kt
Show resolved
Hide resolved
DongChyeon
left a comment
DongChyeon
left a comment
There was a problem hiding this comment.
애니메이션 확인했고 디자인 하나만 수정해주세요!
지금 봤는데 세션 화면 스켈레톤 UI 예쁘네요...
| // 애니메이션이 완료된 후 API 호출 시작 (300ms 애니메이션 + 약간의 여유) | ||
| kotlinx.coroutines.delay(350) |
There was a problem hiding this comment.
C: 요거는 SessionScreen 뿐만 아니라 나머지 화면에서도 처리해야되는 건가요?
There was a problem hiding this comment.
아뇽! SessionScreen 에서만 Navigation 이동 시에 프레임 드랍이 심하게 발생해서 넣었어요
나머지 화면에서는 그대로 둬도 괜찮을거같아용
There was a problem hiding this comment.
R: 세션의 날짜와 시간이 분리되어야 할 것 같아요
There was a problem hiding this comment.
이게 최종 디자인이라, 제 디자인이 맞는거같아용!
날짜와 시간을 별도 줄로 분리하여 가독성 향상 - formatSessionDateTime 함수를 Pair<String, String> 반환으로 수정 - SessionState에서 sessionDateTimePair 필드로 변경 - UI에서 날짜와 시간 사이 4.dp 간격 추가 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@DongChyeon 커밋 하나 더했는데 요것만 다시 한번 리뷰 부탁드립니닷 ... !! |
확인했습니다! |
💡 Issue
🌱 Key changes
✅ To Reviewers
📸 스크린샷
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항