Conversation
📝 WalkthroughWalkthrough이 PR은 홈 화면의 목표(Goal) 조회 기능을 추가합니다. 네트워크 계층에 GoalService 및 DTO(GoalResponse, GoalListResponse, VerificationResponse)와 매퍼가 도입되고 AuthService의 엔드포인트 경로가 조정되었습니다. 도메인 계층에는 Goal, GoalList, GoalVerification 클래스와 RepeatCycle/GoalIconType 변환 유틸이 추가되며 기존 RepeatType은 제거되었습니다. 데이터 계층에는 DefaultGoalRepository와 DI 바인딩, 관련 build 설정 변경이 포함됩니다. UI 쪽에서는 HomeViewModel과 상태가 GoalList를 사용하도록 변경되고 디자인 시스템에 아이콘 리소스와 문자열, 벡터 드로어블이 추가되었습니다. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/main/src/main/java/com/twix/home/HomeViewModel.kt (1)
62-100:⚠️ Potential issue | 🟠 Major빠른 날짜 변경 시 이전 응답이 최신 상태를 덮어쓸 위험이 있습니다.
왜: 여러 날짜를 연속으로 선택하면 먼저 보낸 요청이 늦게 도착해, 최신 선택 날짜와 다른 goalList로 상태가 덮일 수 있습니다.
어떻게: 요청 시점의 날짜를 보존하고, 성공 콜백에서 현재 상태와 일치할 때만 상태를 갱신하도록 가드하는 게 안전합니다. 이 방향으로 처리해 보는 건 어떨까요?🔧 제안 수정
private fun fetchGoalList() { val date = currentState.selectedDate.toString() launchResult( block = { goalRepository.fetchGoalList(date = date) }, - onSuccess = { goalList -> reduce { copy(goalList = goalList) } }, + onSuccess = { goalList -> + if (currentState.selectedDate.toString() != date) return@onSuccess + reduce { copy(goalList = goalList) } + }, onError = { emitSideEffect(HomeSideEffect.ShowToast(R.string.toast_goal_fetch_failed, ToastType.ERROR)) }, ) }
🤖 Fix all issues with AI agents
In
`@core/network/src/main/java/com/twix/network/model/response/goal/model/VerificationResponse.kt`:
- Line 12: The date format comment for the uploadedAt property in
VerificationResponse (field: uploadedAt) is ambiguous using "yyyy-mm-dd"; update
the comment to use uppercase months ("yyyy-MM-dd") to avoid confusion with
minutes, e.g., change the inline comment next to uploadedAt in
VerificationResponse.kt to "yyyy-MM-dd".
In `@core/network/src/main/java/com/twix/network/service/AuthService.kt`:
- Around line 11-16: AuthService and GoalService include the "/api/v1" prefix in
their Retrofit endpoints (e.g., googleLogin in AuthService) while
OnboardingService endpoints omit it, causing potential duplicate
"/api/v1/api/v1" when BASE_URL contains the version; decide which single-source
strategy to use and make endpoints consistent: either remove "/api/v1" from all
service interface paths (AuthService, GoalService, OnboardingService) if
BASE_URL will include "/api/v1", or remove the version from BASE_URL and add
"/api/v1" to every service endpoint (including OnboardingService) so all
interfaces use the same versioned prefix. Ensure you update the service methods
(e.g., AuthService.googleLogin and corresponding GoalService/OnboardingService
methods) accordingly and update any docs/config to reflect the chosen strategy.
🧹 Nitpick comments (7)
core/design-system/src/main/res/drawable/ic_unchecked_you.xml (1)
1-18: 하드코딩된 색상값 사용에 대해 검토해 주세요.현재
#ffffff,#C6C6C6등 색상값이 직접 하드코딩되어 있습니다. 디자인 시스템의 일관성을 위해 가능하다면 color resource를 참조하는 것이 좋습니다.다만, Vector Drawable에서 theme attribute(
?attr/)나 color resource 참조는 API 레벨 제약이 있을 수 있으므로, 프로젝트의 minSdk와 디자인 시스템 정책에 따라 판단해 주세요.참고로, "unchecked" 상태를 표현하기 위해 체크마크를 흰색 배경에 흰색으로 그려 보이지 않게 하는 방식은 이해했습니다. 다른
ic_checked_*,ic_unchecked_*아이콘들과 일관된 스타일인지도 함께 확인해 보시면 좋겠습니다.core/design-system/src/main/res/drawable/ic_checked_me.xml (1)
9-10: 하드코딩된 색상이 다크 모드를 지원하지 않습니다.
#171717과#ffffff가 직접 지정되어 있어 다크 모드에서 아이콘이 제대로 표시되지 않을 수 있습니다. 예를 들어, 다크 모드 배경에서#171717원이 거의 보이지 않을 수 있습니다.디자인 시스템의 테마 속성이나 색상 리소스를 사용하는 것이 좋습니다:
?attr/colorOnSurface또는@color/...형태의 참조 사용- 필요시 night 리소스 디렉토리에 별도 버전 추가
다크 모드 지원이 현재 스코프에 포함되지 않는다면, 추후 대응을 위해 이슈로 트래킹해주세요.
Also applies to: 15-16
core/network/src/main/java/com/twix/network/service/GoalService.kt (1)
8-11:date포맷 계약을 KDoc로 명시하면 좋습니다왜 문제인지: 현재
String만으로는 호출 측이yyyy-MM-dd인지, 타임존 포함인지 헷갈릴 수 있어 잘못된 요청이 발생할 수 있습니다.
어떻게 개선: 간단한 KDoc로 기대 포맷을 명시해 주세요.📝 예시
interface GoalService { `@GET`("api/v1/goals") suspend fun fetchGoals( + /** + * `@param` date 서버가 기대하는 날짜 형식: yyyy-MM-dd (ISO-8601) + */ `@Query`("date") date: String, ): GoalListResponse }domain/src/main/java/com/twix/domain/repository/GoalRepository.kt (1)
6-8: date를 String으로 노출하면 포맷 불일치 위험이 있습니다.도메인 레이어가 문자열 포맷에 의존하면 호출부마다 포맷이 달라져 API 실패나 캐시 불일치가 생길 수 있습니다. 날짜를 표현하는 타입(도메인 전용 타입 등)으로 받고, 데이터 레이어에서 API 포맷으로 변환하는 방식이 더 안전하지 않을까요?
🔧 제안 변경 예시
interface GoalRepository { - suspend fun fetchGoalList(date: String): AppResult<GoalList> + suspend fun fetchGoalList(date: LocalDate): AppResult<GoalList> }As per coding guidelines, 비즈니스 로직이 명확히 표현되어 있는가?
feature/goal-editor/src/main/java/com/twix/goal_editor/GoalEditorIntent.kt (1)
17-19: Intent 명칭이 RepeatCycle과 불일치합니다.Intent 이름이 실제 payload 용어와 달라 검색/이해가 어려워질 수 있습니다.
SetRepeatCycle로 정합성을 맞추는 게 어떨까요?🔧 제안 변경 예시
- data class SetRepeatType( + data class SetRepeatCycle( val repeatCycle: RepeatCycle, ) : GoalEditorIntentAs per coding guidelines, Intent가 사용자 액션 단위로 명확히 정의되어 있는가?
feature/goal-editor/src/main/java/com/twix/goal_editor/GoalEditorScreen.kt (1)
296-336: 내부 변수명이 RepeatType으로 남아 가독성이 떨어집니다.왜: 타입은 RepeatCycle인데 변수명은 RepeatType이라 의미가 엇갈립니다.
어떻게: 변수명만 Cycle로 맞추면 읽기 쉬워집니다. 변경해 보시는 건 어떨까요?✏️ 제안 리네이밍
- var internalSelectedRepeatType by remember { mutableStateOf(selectedRepeatCycle) } - val maxCount = if (internalSelectedRepeatType == RepeatCycle.WEEKLY) 6 else 25 + var internalSelectedRepeatCycle by remember { mutableStateOf(selectedRepeatCycle) } + val maxCount = if (internalSelectedRepeatCycle == RepeatCycle.WEEKLY) 6 else 25 @@ - color = if (internalSelectedRepeatType == RepeatCycle.WEEKLY) CommonColor.White else GrayColor.C500, + color = if (internalSelectedRepeatCycle == RepeatCycle.WEEKLY) CommonColor.White else GrayColor.C500, @@ - .background(if (internalSelectedRepeatType == RepeatCycle.WEEKLY) GrayColor.C500 else CommonColor.White) + .background(if (internalSelectedRepeatCycle == RepeatCycle.WEEKLY) GrayColor.C500 else CommonColor.White) @@ - internalSelectedRepeatType = RepeatCycle.WEEKLY + internalSelectedRepeatCycle = RepeatCycle.WEEKLY @@ - color = if (internalSelectedRepeatType == RepeatCycle.MONTHLY) CommonColor.White else GrayColor.C500, + color = if (internalSelectedRepeatCycle == RepeatCycle.MONTHLY) CommonColor.White else GrayColor.C500, @@ - .background(if (internalSelectedRepeatType == RepeatCycle.MONTHLY) GrayColor.C500 else CommonColor.White) + .background(if (internalSelectedRepeatCycle == RepeatCycle.MONTHLY) GrayColor.C500 else CommonColor.White) @@ - internalSelectedRepeatType = RepeatCycle.MONTHLY + internalSelectedRepeatCycle = RepeatCycle.MONTHLY @@ - onClick = { onCommit(internalSelectedRepeatType, internalRepeatCount) }, + onClick = { onCommit(internalSelectedRepeatCycle, internalRepeatCount) },feature/goal-editor/src/main/java/com/twix/goal_editor/component/GoalInfoCard.kt (1)
59-105: 함수명이 RepeatType으로 남아 혼란을 줄 수 있습니다.왜: 실제로는 RepeatCycle을 다루는데 함수명이 RepeatType이라 의미가 불일치합니다.
어떻게: 함수명만 RepeatCycle로 맞추면 가독성이 좋아집니다. 변경을 고려해 보실까요?✏️ 제안 리네이밍
- RepeatTypeSettings( + RepeatCycleSettings( selectedRepeatCycle = selectedRepeatCycle, repeatCount = repeatCount, onSelectedRepeatType = onSelectedRepeatType, onShowRepeatCountBottomSheet = onShowRepeatCountBottomSheet, ) @@ -private fun RepeatTypeSettings( +private fun RepeatCycleSettings( selectedRepeatCycle: RepeatCycle, repeatCount: Int, onSelectedRepeatType: (RepeatCycle) -> Unit, onShowRepeatCountBottomSheet: () -> Unit, ) {
core/network/src/main/java/com/twix/network/model/response/goal/model/VerificationResponse.kt
Show resolved
Hide resolved
2 participants
이슈 번호
작업내용
결과물
리뷰어에게 추가로 요구하는 사항 (선택)
지금 일단 커플 연결이 안되어 있어서 조회되는 건 없지만 통신이 되는 것까지 확인했어요! 그리고 저는 local.properties에 /api/v1을 추가하지 않았는데 구글 로그인 코드 보니까 형은 추가한 거 같더라구요..? 그래서 이걸 일단 맞춰야 할 거 같아요
제가 local.properties에 /api/v1 추가하니까 LoginViewModel에서 Koin 때문에 자꾸 앱이 죽어버려서 지금 어떻게 설정해놓으셨는지 공유해주세요!