[Fix] 홈/주간통계 화면 미기록 처리 및 알림 화면 이동 #171 #180
Conversation
|
Warning Rate limit exceeded@librawish808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Walkthrough서버 응답 구조 변경에 맞춰 홈·주간 통계 DTO와 관련 UI/네비게이션을 null-safe로 전환하고, HomeRepository API를 DTO 반환으로 단순화했으며 알림 네비게이션 콜백을 홈/통계 화면으로 전파했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Repo as HomeRepositoryImpl
participant API as Remote API / DTO
participant VM as HomeViewModel
participant UI as HomeScreen
Repo->>API: getHomeSummary(elderId)
API-->>Repo: HomeResponseDto (nullable fields)
Repo-->>VM: HomeResponseDto
VM->>VM: HomeUiState.from(dto) (null-safe mapping)
VM-->>UI: HomeUiState (includes unreadNotification)
UI->>UI: render with null-handling (isNullOrBlank, ?:)
sequenceDiagram
participant VM as StatisticsViewModel
participant API as Remote API / StatisticsResponseDto
participant UI as StatisticsScreen
VM->>API: request statistics DTO
API-->>VM: StatisticsResponseDto (nullable fields)
VM->>VM: WeeklySummaryUiState.from(dto, order)
VM-->>UI: WeeklySummaryUiState (nested Weekly*UiState)
UI->>UI: render cards, use unreadNotification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/StatisticsViewModel.kt (1)
194-194: 사용하지 않는 상수를 제거해주세요.EMPTY_HEALTH_MESSAGE 상수가 코드에서 사용되지 않고 있습니다. 정적 분석 도구에서도 감지되었네요.
이 상수를 제거하는 diff를 적용해주세요:
- companion object { - private const val EMPTY_HEALTH_MESSAGE = "아직 충분한 기록이 쌓이지 않았어요." - }app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt (1)
38-43: 미사용 함수 제거 필요
mapNextTimeToKor함수가 정의되어 있지만 사용되지 않습니다. 이 함수는 이전에 수동 매핑 로직에서 사용되었지만, 현재는HomeUiState.from(res)으로 대체되어 더 이상 필요하지 않습니다.다음 diff를 적용해서 제거하세요:
- private fun mapNextTimeToKor(nextTime: String?): String = when (nextTime) { - "MORNING" -> "아침" - "LUNCH" -> "점심" - "DINNER" -> "저녁" - else -> "-" - } -
🧹 Nitpick comments (4)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeStateMentalContainer.kt (1)
79-89: null 안전성 처리가 깔끔합니다.
isNullOrBlank()체크로 null과 빈 문자열을 동시에 처리하고, 미기록 시 "미기록" 텍스트와 gray4 색상을 일관되게 적용한 점이 좋습니다. 스마트 캐스팅도 올바르게 동작할 것으로 보입니다.선택사항으로, 미기록 상태를 시각적으로 확인할 수 있도록 Preview를 하나 더 추가하면 좋을 것 같습니다:
@Preview @Composable fun PreviewHomeStateMentalContainerEmpty() { HomeStateMentalContainer( mentalStatus = null, onClick = {}, ) }app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeGlucoseLevelContainer.kt (1)
111-127: null 케이스 프리뷰 추가를 고려해보세요.현재 프리뷰는 기록 있음(120)과 기록 없음(0) 케이스를 다루고 있는데,
glucoseLevelAverageToday = null케이스도 추가하면 테스트 커버리지가 더 완전해집니다. 물론 0과 동일하게 렌더링되지만, 명시적으로 null 처리를 확인할 수 있습니다.@Preview(showBackground = true, name = "혈당 기록 null") @Composable private fun PreviewHomeGlucoseLevelContainerNull() { HomeGlucoseLevelContainer( glucoseLevelAverageToday = null, onClick = {}, ) }app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt (1)
58-63: 복용 시간 매핑 개선 제안
nextDoseTime매핑이 "MORNING", "LUNCH", "DINNER"만 한글로 변환하고 있습니다. 만약 서버가 예상치 못한 값(예: "SNACK", 빈 문자열 등)을 보내면 그대로 표시됩니다.현재 코드는 동작하지만, 예상치 못한 값에 대한 처리를 명확히 하고 싶다면 else 분기에서 null을 반환하거나 기본 메시지를 제공하는 것을 고려해보세요.
예시 (선택사항):
nextDoseTime = when (it.nextTime) { "MORNING" -> "아침" "LUNCH" -> "점심" "DINNER" -> "저녁" - else -> it.nextTime + else -> it.nextTime ?: "-" },app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/WeeklySummaryUiState.kt (1)
102-160: 중첩 데이터 클래스 구조 개선
WeeklyMealUiState,WeeklyMedicineUiState,WeeklyMentalUiState,WeeklyGlucoseUiState를WeeklySummaryUiState내부에 중첩시킨 것은 좋은 구조입니다.
- 관련 타입들이 논리적으로 그룹화됨
- 네임스페이스 오염 감소
- EMPTY companion 객체들로 기본값 제공
다만 각 클래스가 100줄 이상의 파일에 함께 있어서 파일이 좀 길어졌네요. 향후 더 복잡해지면 별도 파일로 분리하는 것도 고려해볼 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
app/src/main/java/com/konkuk/medicarecall/data/dto/response/HomeResponseDto.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/dto/response/StatisticsResponseDto.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/common/component/NameBar.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/common/component/NotificationIconWithBadge.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/common/util/WeeklySummaryUtil.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeGlucoseLevelContainer.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeStateHealthContainer.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeStateMentalContainer.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/navigation/HomeNavigation.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt(4 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/screen/StatisticsScreen.kt(6 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/StatisticsViewModel.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/WeeklySummaryUiState.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklyGlucoseCard.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklyMealCard.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklyMedicineCard.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklyMentalCard.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklySummaryCard.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/navigation/MainNavigator.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/alarm/navigation/AlarmNavigation.kt (1)
alarmNavGraph(13-21)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeSleepContainer.kt (1)
HomeSleepContainer(33-150)
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/ui/common/component/NotificationIconWithBadge.kt
[error] 35-35: Detekt TrailingCommaOnCallSite: Missing trailing comma before ")".
[error] 46-46: Detekt TrailingCommaOnCallSite: Missing trailing comma before ")".
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
[error] 77-77: Detekt SpacingAroundCurly: Missing spacing before "***".
[error] 78-78: Detekt SpacingAroundCurly: Missing spacing before "***".
app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt
[error] 108-108: Detekt NoConsecutiveBlankLines: Needless blank line(s).
[error] 105-105: Detekt TrailingCommaOnCallSite: Missing trailing comma before ")".
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt
[error] 62-62: Detekt NoBlankLineBeforeRbrace: Unexpected blank line(s) before "***".
[error] 38-38: Detekt UnusedPrivateMember: Private function mapNextTimeToKor is unused.
app/src/main/java/com/konkuk/medicarecall/data/dto/response/StatisticsResponseDto.kt
[error] 25-25: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 31-31: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 33-33: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 35-35: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 37-37: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 53-53: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 55-55: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 61-61: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 63-63: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 69-69: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 71-71: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 73-73: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 89-89: Detekt SpacingAroundOperators: Missing spacing before "=".
[error] 91-91: Detekt SpacingAroundOperators: Missing spacing before "=".
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/WeeklySummaryUiState.kt
[error] 32-32: Detekt Indentation: Unexpected indentation (4) (should be 0).
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/StatisticsViewModel.kt
[error] 194-194: Detekt UnusedPrivateProperty: Private property EMPTY_HEALTH_MESSAGE is unused.
app/src/main/java/com/konkuk/medicarecall/ui/common/util/WeeklySummaryUtil.kt
[error] 9-9: Detekt UnusedPrivateProperty: Private property count is unused.
app/src/main/java/com/konkuk/medicarecall/data/dto/response/HomeResponseDto.kt
[error] 49-49: Detekt NoConsecutiveBlankLines: Needless blank line(s).
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt
[error] 381-381: Detekt Indentation: Unexpected indentation (36) (should be 32).
🔇 Additional comments (23)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeStateMentalContainer.kt (1)
30-34: nullable 파라미터 변경 잘 적용되었습니다.서버 응답 변경에 맞춰
mentalStatus를 nullable로 변경한 것이 PR 목표와 잘 맞습니다. 미기록 데이터를 안전하게 처리할 수 있게 되었네요.app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeGlucoseLevelContainer.kt (1)
31-31: null 처리가 안전하게 구현되었습니다.nullable 파라미터로 변경하고 Elvis 연산자로 null을 0으로 처리하는 로직이 적절합니다. null과 0 모두 "기록 없음"으로 처리되어 "--"를 표시하며, 이는 PR 목표와 일치합니다.
Line 81의 문자열 보간도 안전합니다.
isRecorded가 true일 때만 실행되므로glucoseLevelAverageToday가 null이 아닌 양수임이 보장됩니다.Also applies to: 80-81
app/src/main/java/com/konkuk/medicarecall/ui/common/component/NameBar.kt (1)
43-46: 명명된 인자 사용으로 가독성이 향상되었습니다.
Modifier.clickable을 명명된 인자로 변경하여 코드의 명확성이 개선되었습니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklyMealCard.kt (1)
26-26: 중첩된 UI 상태 타입으로 잘 마이그레이션되었습니다.
WeeklySummaryUiState.WeeklyMealUiState로 import 경로가 업데이트되어 새로운 중첩 타입 구조와 일치합니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/home/navigation/HomeNavigation.kt (1)
21-21: 알림 화면 네비게이션이 올바르게 연결되었습니다.
navigateToAlarmScreen파라미터가 추가되고HomeScreen으로 적절히 전달되었습니다. PR 목표에 따라 알림 화면 이동 기능이 구현되었습니다.Also applies to: 32-32
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (1)
77-78: null 안전성 처리가 잘 적용되었습니다.복약 카운트 필드에 Elvis 연산자(
?: 0)를 사용하여 nullable 값을 안전하게 처리하고 있습니다. 서버 응답 변경 사항과 잘 맞습니다.Also applies to: 152-152
app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt (1)
103-106: 알림 네비게이션 그래프가 올바르게 통합되었습니다.
alarmNavGraph가 추가되고 홈 및 통계 화면에서 알림 화면으로의 네비게이션이 적절히 연결되었습니다. PR 목표에 부합하는 변경사항입니다.Also applies to: 127-127, 182-182
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeStateHealthContainer.kt (1)
32-93: nullable 처리 잘 적용되었습니다!healthStatus를 String?로 변경하고 isNullOrBlank() 체크로 미기록 상태를 안전하게 처리하고 있네요. 서버 응답 변경에 맞춰 null 안전성을 확보한 깔끔한 수정입니다.
app/src/main/java/com/konkuk/medicarecall/ui/navigation/MainNavigator.kt (1)
1-240: import 정리 변경사항입니다.import 문 재정리만 이루어졌고 기능적 변경은 없습니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklyMedicineCard.kt (1)
29-148: nested UI state 타입으로 깔끔하게 리팩토링되었습니다.WeeklySummaryUiState.WeeklyMedicineUiState로 타입을 변경하여 주간통계 UI 상태를 중앙화한 점이 좋습니다. Preview도 잘 업데이트되었네요.
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklySummaryCard.kt (1)
79-117: nullable Int 처리가 잘 되어 있습니다.value를 Int?로 변경하고 null 체크를 when 표현식으로 깔끔하게 처리했네요. 미기록 시 "-" 표시도 적절합니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/StatisticsViewModel.kt (1)
164-165: DTO 변환 로직이 깔끔하게 이동되었습니다.WeeklySummaryUiState.from() 팩토리 메서드를 사용하여 DTO-to-UI 변환 로직을 UI state 쪽으로 이동시킨 점이 좋습니다. 관심사 분리가 더 명확해졌네요.
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt (2)
83-83: 알림 화면 네비게이션이 잘 연결되었습니다.navigateToAlarm 콜백을 추가하고 NameBar까지 전달하여 알림 아이콘 클릭 시 네비게이션이 작동하도록 구현했네요. PR 목표에 맞는 변경사항입니다.
Also applies to: 126-126, 164-164, 280-281
383-389: 수면 데이터 nullable 처리가 안전하게 구현되었습니다.sleep?.meanHours ?: 0와 같은 안전한 접근 방식으로 null 케이스를 처리하고 있고, isRecorded 로직도 적절합니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklyMentalCard.kt (1)
26-140: nested 타입으로 일관성 있게 리팩토링되었습니다.WeeklySummaryUiState.WeeklyMentalUiState로 타입을 변경하여 주간통계 UI 상태 구조를 통일했네요. Preview 코드도 잘 업데이트되었습니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/weeklycard/WeeklyGlucoseCard.kt (1)
26-211: 혈당 카드도 nested 타입으로 깔끔하게 전환되었습니다.WeeklySummaryUiState.WeeklyGlucoseUiState 타입을 사용하여 다른 주간통계 카드들과 일관된 구조를 유지하고 있습니다. 전체적인 리팩토링이 잘 마무리되었네요!
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt (1)
61-61: 좋습니다! 매핑 로직 간소화수동 매핑 대신
HomeUiState.from(res)팩토리 메서드를 사용하는 것은 좋은 리팩토링입니다. 코드가 더 간결해지고 매핑 로직이 UI 상태 클래스에 캡슐화되어 유지보수가 용이합니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/screen/StatisticsScreen.kt (2)
59-59: 알림 네비게이션 구현 확인됨
navigateToAlarm파라미터가StatisticsScreen→StatisticsScreenLayout→NameBar까지 올바르게 전달되고 있으며,navController.navigateToAlarm()을 통해 실제 네비게이션이 수행됩니다.알림 개수도
uiState.summary.unreadNotification ?: 0로 null 안전하게 처리되고 있습니다.Also applies to: 131-131, 140-140, 160-161
43-43: 중첩 타입으로 마이그레이션 완료
WeeklySummaryUiState내부의 중첩 타입들(WeeklyMealUiState,WeeklyMedicineUiState,WeeklyMentalUiState,WeeklyGlucoseUiState)을 사용하도록 import와 Preview 데이터가 올바르게 업데이트되었습니다. 타입 구조가 더 명확해졌네요.Also applies to: 282-309
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt (2)
14-21: Nullable 필드 처리 잘 구현됨서버의 미기록 응답 처리에 맞춰
totalTaken,totalGoal,sleep,healthStatus,mentalStatus,glucoseLevelAverageToday,unreadNotification필드들이 nullable로 변경되었고, DTO에서 안전하게 매핑되고 있습니다.이렇게 하면 데이터가 없을 때도 앱이 크래시 없이 동작할 수 있습니다.
Also applies to: 48-50, 66-70
77-79: MedicineUiState nullable 필드 처리 확인
todayTakenCount와todayRequiredCount가 nullableInt?로 변경되어 미기록 상태를 안전하게 처리할 수 있습니다.medicineName도it.type.orEmpty()로 null 안전하게 처리되고 있네요.app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/WeeklySummaryUiState.kt (2)
35-62: 복약 데이터 폴백 로직 잘 구현됨
from()팩토리 메서드의 복약 데이터 처리 로직이 세 가지 경우를 잘 처리하고 있습니다:
- 서버에서 복약 통계가 있는 경우 → 실제 데이터 매핑
- 복약 통계는 없지만 약 이름 목록이 있는 경우 → placeholder 생성 (
takenCount: null,totalCount: 0)- 둘 다 없는 경우 → 빈 리스트
이렇게 하면 미기록 상태에서도 UI에 약 이름이 표시될 수 있어서 사용자 경험이 개선됩니다.
64-95: DTO to UI 매핑 중앙화DTO를 UI 상태로 변환하는 로직을
WeeklySummaryUiState.from()팩토리 메서드로 이동한 것은 좋은 설계입니다.
- ViewModel의 책임이 줄어들고
- 매핑 로직이 한 곳에 모여서 유지보수가 쉬워집니다
- null 안전성 처리가 일관되게 적용됩니다
app/src/main/java/com/konkuk/medicarecall/data/dto/response/HomeResponseDto.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/data/dto/response/StatisticsResponseDto.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/ui/common/component/NotificationIconWithBadge.kt
Outdated
Show resolved
Hide resolved
| modifier = Modifier | ||
| .size(28.dp) | ||
| .clickable { | ||
| onClick() | ||
| } |
There was a problem hiding this comment.
파이프라인 실패 & 터치 타겟 크기 부족
두 가지 문제가 있습니다:
- Critical: Detekt이 line 46에서 trailing comma 누락을 경고합니다.
- Major: 아이콘 크기가 28.dp인데, Android 권장 최소 터치 타겟은 48.dp입니다. 접근성과 사용성에 문제가 생길 수 있습니다.
다음 diff로 두 문제를 모두 해결하세요:
modifier = Modifier
.size(28.dp)
.clickable {
onClick()
- }
+ },추가로, 터치 타겟 크기 문제를 해결하려면 아이콘 자체는 28.dp로 유지하되 clickable 영역을 넓히는 방법을 고려하세요:
modifier = Modifier
- .size(28.dp)
.clickable {
onClick()
- }
+ }
+ .size(48.dp),
+ contentDescription = "알림",
+ tint = Color.Unspecified,
)
+
+ // 아이콘을 중앙에 배치하기 위해 Box 내부에 추가 조정 필요
+ Box(
+ modifier = Modifier.size(48.dp),
+ contentAlignment = Alignment.Center,
+ ) {
+ Icon(
+ painter = painterResource(id = R.drawable.ic_notification),
+ contentDescription = "알림",
+ tint = Color.Unspecified,
+ modifier = Modifier.size(28.dp),
+ )
+ }또는 더 간단하게 minimumInteractiveComponentSize modifier를 사용할 수 있습니다 (Compose Material3):
import androidx.compose.material3.minimumInteractiveComponentSize
Icon(
painter = painterResource(id = R.drawable.ic_notification),
contentDescription = "알림",
tint = Color.Unspecified,
modifier = Modifier
.size(28.dp)
.clickable { onClick() }
.minimumInteractiveComponentSize(),
)🧰 Tools
🪛 GitHub Actions: Android CI
[error] 46-46: Detekt TrailingCommaOnCallSite: Missing trailing comma before ")".
🤖 Prompt for AI Agents
In
app/src/main/java/com/konkuk/medicarecall/ui/common/component/NotificationIconWithBadge.kt
around lines 42 to 46, add Compose Material3's minimumInteractiveComponentSize()
to the modifier chain (so the icon remains 28.dp but the clickable hit area
meets the 48.dp recommendation) and ensure there's a trailing comma after the
modifier parameter to satisfy Detekt; i.e., append
.minimumInteractiveComponentSize() to the Modifier chain and place a trailing
comma after the modifier argument in the Icon call.
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt
Outdated
Show resolved
Hide resolved
...rc/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/WeeklySummaryUiState.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/common/util/WeeklySummaryUtil.kt (2)
8-15: nullable 값으로 인한 NPE 위험이 있습니다.Line 11의
meal.eatenCount in 1 until meal.totalCount표현식은eatenCount나totalCount중 하나라도 null일 경우 range 생성 시 NPE가 발생할 수 있습니다. PR 목표에 따르면 모든 DTO 필드가 nullable로 변경되었으므로, null-safe한 처리가 필요합니다.다음 diff를 적용하여 null-safe하게 처리하세요:
fun getMealIconColor(meal: WeeklySummaryUiState.WeeklyMealUiState, colors: MediCareCallColors): Color { + val eatenCount = meal.eatenCount ?: return colors.gray3 + val totalCount = meal.totalCount ?: return colors.gray3 return when { - meal.eatenCount == meal.totalCount -> colors.positive - meal.eatenCount in 1 until meal.totalCount -> colors.warning2 - meal.eatenCount == 0 -> colors.negative + eatenCount == totalCount -> colors.positive + eatenCount in 1 until totalCount -> colors.warning2 + eatenCount == 0 -> colors.negative else -> colors.gray3 } }
17-24: nullable 값으로 인한 NPE 위험이 있습니다.Line 20의
medicine.takenCount in 1 until medicine.totalCount표현식은takenCount나totalCount중 하나라도 null일 경우 range 생성 시 NPE가 발생할 수 있습니다. 위의getMealIconColor와 동일한 이슈입니다.다음 diff를 적용하여 null-safe하게 처리하세요:
fun getMedicineIconColor(medicine: WeeklySummaryUiState.WeeklyMedicineUiState, colors: MediCareCallColors): Color { + val takenCount = medicine.takenCount ?: return colors.gray3 + val totalCount = medicine.totalCount ?: return colors.gray3 return when { - medicine.takenCount == medicine.totalCount -> colors.positive - medicine.takenCount in 1 until medicine.totalCount -> colors.warning2 - medicine.takenCount == 0 -> colors.negative + takenCount == totalCount -> colors.positive + takenCount in 1 until totalCount -> colors.warning2 + takenCount == 0 -> colors.negative else -> colors.gray3 } }
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/WeeklySummaryUiState.kt (1)
69-73: 식사 totalCount 하드코딩현재는 주간 통계라서
totalCount = 7로 고정하신 게 맞습니다. 나중에 기간을 유연하게 조정하게 되면 이 부분도 파라미터로 받는 걸 고려해볼 수 있을 것 같아요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/src/main/java/com/konkuk/medicarecall/data/dto/response/HomeResponseDto.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/dto/response/StatisticsResponseDto.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/common/component/NotificationIconWithBadge.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/common/util/WeeklySummaryUtil.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt(4 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/StatisticsViewModel.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/WeeklySummaryUiState.kt(2 hunks)app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt
- app/src/main/java/com/konkuk/medicarecall/ui/common/component/NotificationIconWithBadge.kt
- app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/StatisticsViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/alarm/navigation/AlarmNavigation.kt (1)
alarmNavGraph(13-21)
⏰ 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). (1)
- GitHub Check: ci-build
🔇 Additional comments (21)
app/src/main/java/com/konkuk/medicarecall/data/dto/response/StatisticsResponseDto.kt (8)
9-9: elderName의 빈 문자열 기본값 선택을 확인해주세요.
elderName필드는 빈 문자열("")을 기본값으로 사용하는 반면, 다른 선택적 필드들은 모두null을 기본값으로 사용합니다. 빈 문자열은 "실제로 비어있는 이름"과 "서버에서 누락된 필드"를 구분할 수 없게 만들 수 있습니다.서버가 항상 이 필드를 포함하도록 변경되었다면 현재 방식이 의도적일 수 있지만, 일관성을 위해
String? = null로 변경하는 것이 더 명확할 수 있습니다.
31-37: 변경사항 승인합니다.모든 통계 필드를 nullable로 변경한 것은 미기록 데이터를 안전하게 처리하기 위한 적절한 접근입니다. 과거 리뷰에서 지적된 spacing 이슈도 모두 수정되었습니다.
43-47: 식사 통계 nullable 처리가 적절합니다.식사 데이터가 없을 때를 대비한 nullable 처리가 잘 되어있습니다.
53-55: 복약 통계 필드 변경이 적절합니다.미기록 상태를 null로 명확히 표현할 수 있도록 변경되었습니다.
61-63: 수면 시간 필드 처리 확인.nullable 타입과 spacing이 올바르게 적용되었습니다.
69-73: 정신건강 요약 필드 변경이 적절합니다.미기록 데이터 처리를 위한 nullable 변경이 올바르게 적용되었습니다.
79-81: 혈당 데이터 nullable 처리가 적절합니다.식전/식후 혈당 데이터가 없을 수 있는 경우를 안전하게 처리할 수 있습니다.
87-91: 혈당 상세 통계 필드 변경이 적절합니다.정상/고혈당/저혈당 카운트를 nullable로 처리하여 미기록 상태를 안전하게 다룰 수 있습니다.
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/WeeklySummaryUiState.kt (5)
15-31: nullable 필드 처리 잘 하셨네요!서버에서 미기록 데이터를 null로 보낼 수 있도록 변경된 부분을 잘 반영하셨습니다. 각 통계 필드를 nullable로 선언해서 안전하게 처리하고 있어요.
34-41: DTO 변환 로직을 factory method로 분리한 점 좋습니다ViewModel에서 이 로직을 가져와서 UiState의 companion object에 두니 책임 분리가 깔끔해졌어요.
78-78: 수면 기록 여부 계산 로직 정확합니다시간 또는 분이 하나라도 0보다 크면 기록된 것으로 처리하는 로직이 맞네요. 예를 들어 "0시간 30분"도 올바르게 감지됩니다.
117-159: EMPTY 상태 값 설계 의도 확인
WeeklyMentalUiState.EMPTY는 -1을 사용하고,WeeklyGlucoseUiState.EMPTY는 0을 사용하는데, 이 차이가 의도된 건지 확인하고 싶어요.
- 심리 상태:
good = -1, normal = -1, bad = -1→ sentinel 값으로 "데이터 없음" 표시- 혈당 상태: 모두 0 → 실제 0회와 "데이터 없음"을 구분할 수 없음
혈당도 -1을 사용하거나, 혹은 nullable Int로 바꾸는 게 더 명확할 수 있습니다. 현재 방식이 의도된 거라면 주석으로 설명을 추가해주시면 좋을 것 같아요.
42-61: UI 구현이 이미 totalCount=0 케이스를 잘 처리하고 있습니다.검토 코멘트의 우려와 달리, WeeklyMedicineCard에서 미기록 상태는 "- / 0" 형태로 표시되고 있습니다 (대시가 기록되지 않은 상태를 명확하게 나타냄). 따라서 totalCount=0 설정은 의도된 디자인이며, UI에서 깔끔하게 처리되고 있으므로 변경이 필요하지 않습니다.
Likely an incorrect or invalid review comment.
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/screen/HomeScreen.kt (4)
83-83: 알림 네비게이션 파라미터 연동이 깔끔합니다.
navigateToAlarm콜백이HomeScreen→HomeScreenLayout→NameBar로 제대로 전달되고 있습니다.HomeScreenLayout의 기본값으로 빈 람다를 제공한 것도 프리뷰와 테스트에서 유용하겠네요.Also applies to: 126-126, 164-164
280-281: 읽지 않은 알림 카운트 처리가 안전합니다.하드코딩된 값에서
homeUiState.unreadNotification으로 변경하고 null일 때 0으로 기본값 처리한 것이 좋습니다. Elvis 연산자로 안전하게 처리되고 있네요.
383-389: 수면 데이터 null 처리가 안전하게 구현되었습니다.
sleepData가 null이거나 값이 0일 때를 안전하게 처리하고 있습니다.isRecorded로직도 시간이나 분 중 하나라도 0보다 크면 기록된 것으로 판단하는 것이 적절해 보입니다.
475-522: 미기록 상태 프리뷰 추가가 유용합니다.기록되지 않은 상태를 별도 프리뷰로 추가한 것이 개발과 QA에 도움이 되겠네요. 빈 데이터 처리를 시각적으로 확인할 수 있어서 좋습니다.
app/src/main/java/com/konkuk/medicarecall/data/dto/response/HomeResponseDto.kt (2)
6-18: Null 값 처리가 완벽하게 구현되었습니다.검증 결과 UI 레이어에서 nullable Boolean 값들을 안전하게 처리하고 있습니다.
getRiceBowlIcon()함수가when표현식으로true,false,null세 가지 경우를 모두 명시적으로 처리하고 있어서, 미기록 상태(null)일 때는ic_ricebowl_uncheck아이콘으로 표시됩니다.DTO → ViewModel → UI Component → Icon Selection까지 전체 흐름이 일관되게 null-safe하게 설계되어 있네요. 👍
28-41: 구조 변경 검증 완료 — 이슈 없음
nextMedicationTime필드 제거가 코드베이스 전체에서 사용되지 않고 있으므로 breaking change가 없습니다. 새로운 구조는 상태 레벨의 단순한 다음 시간 정보에서doseStatusList를 통한 시간대별 상세 추적으로 개선되었으며, 개별MedicationDto는 여전히nextTime필드를 유지하고 있어 설계가 깔끔합니다.app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt (1)
53-60: 팩토리 메서드의 null-safety 처리가 잘 구현되어 있습니다.
HomeUiState.from()메서드를 확인했는데, nullable 필드들이 모두 적절하게 처리되고 있네요:
aiSummary:.orEmpty()사용medicationList:.orEmpty()사용bloodSugar?.meanValue: 옵셔널 체이닝으로 안전 처리sleep,healthStatus,mentalStatus,unreadNotification: nullable 필드이므로 직접 할당해서 null 허용변환 로직이 깔끔하고 DTO에서 UI 상태로의 매핑이 견고합니다. 에러 처리와 함께 좋은 개선입니다.
app/src/main/java/com/konkuk/medicarecall/ui/navigation/NavGraph.kt (1)
103-106: 알림 화면 네비게이션 연동이 잘 구현되었습니다.홈 화면과 주간통계 화면 모두에서 알림 화면으로의 네비게이션이 올바르게 연결되었습니다.
alarmNavGraph를 별도 그래프로 분리한 것도 모듈화 관점에서 좋은 구조입니다.확인 결과,
MainNavigator의navigateToAlarm()메서드(224번 라인)가 올바르게 구현되어 있으며, 필요한 import도 모두 갖춰져 있습니다. 네비게이션 흐름이 일관성 있게 잘 연결되어 있습니다.
app/src/main/java/com/konkuk/medicarecall/data/dto/response/StatisticsResponseDto.kt
Show resolved
Hide resolved
# Conflicts: # app/src/main/java/com/konkuk/medicarecall/data/dto/response/HomeResponseDto.kt # app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt # app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (1)
137-147: 널 처리 로직이 잘 구현되었습니다.
doseStatusList가 null이거나 비어있을 때 빈 아이콘으로 채우고, 데이터가 있으면 실제 상태를 표시하는 로직이 명확해요. 서버 변경사항에 맞춰 안전하게 처리하고 있습니다.다만, 137번 줄의
coerceAtLeast(0)는 불필요해 보여요. 이미?: 0으로 기본값이 0이라서 추가 체크가 중복입니다.선택적으로 다음처럼 간소화할 수 있습니다:
-val requiredCount = (medicine.todayRequiredCount ?: 0).coerceAtLeast(0) +val requiredCount = medicine.todayRequiredCount ?: 0app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (1)
180-186: 약물 정렬 로직의 중복을 고려해보세요 (선택사항)약물 리스트를
correctMedicationOrder기준으로 정렬하는 로직이 세 군데(Lines 180-186, 258-261, 309-326)에서 반복됩니다. 나중에 시간이 되면 이 로직을 헬퍼 함수로 추출하는 것을 고려해볼 수 있습니다.예시:
private fun List<MedicineUiState>.sortByMedicationOrder(order: List<String>): List<MedicineUiState> { return sortedBy { med -> order.indexOf(med.medicineName).let { if (it == -1) Int.MAX_VALUE else it } } }현재 코드도 충분히 명확하니 급하게 바꿀 필요는 없습니다.
Also applies to: 258-261, 309-326
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(5 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt(3 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt(2 hunks)
🔇 Additional comments (9)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (4)
79-80: 널 처리가 적절합니다.각 약의 복용 횟수가 null일 때 0으로 안전하게 처리하고 있어요. 서버에서 기록이 없을 때 null을 반환하는 변경사항에 맞춰 잘 대응했습니다.
117-124: 조건부 렌더링이 잘 구현되었습니다.다음 복약 시간이 없을 때는 표시하지 않도록 처리했네요.
isNullOrBlank()체크로 null과 빈 문자열을 모두 안전하게 처리하고 있습니다.
166-166: 일관된 널 처리입니다.개별 약의 복용 횟수 표시에서도 동일한 방식으로 null을 0으로 처리하고 있어 일관성이 좋습니다.
220-220: 프리뷰 데이터로 null 케이스를 테스트하는 것 좋습니다.하나는
null로, 다른 하나는emptyList()로 설정해서 두 가지 미기록 시나리오를 모두 확인할 수 있네요. 이렇게 테스트 커버리지를 확보한 점이 좋습니다.app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeUiState.kt (4)
14-21: nullable 필드 추가 잘 처리되었습니다!서버에서 기록이 없을 때 null을 반환하는 것에 대응하여 모든 관련 필드를 nullable로 변경한 것이 적절합니다.
totalTaken/totalGoal필드 추가로 약물 복용 횟수 계산 로직도 서버 기준으로 변경되어 PR 목표와 정확히 일치하네요.
48-77: 매핑 로직 전반적으로 잘 구현되었습니다!서버 DTO에서 UI 상태로의 변환이 안전하게 처리되고 있네요. 한 가지 사소한 점만 확인해주세요:
- Line 62:
else분기에서it.nextTime을 그대로 반환하는데, Line 55의it.type.orEmpty()나 Line 67의dtoDose.time.orEmpty()와 패턴이 약간 다릅니다.nextDoseTime이 nullable이므로 기술적으로는 문제없지만, null 값을 의도적으로 보존하려는 것인지 확인해주시면 좋을 것 같아요.
81-87: MedicineUiState 필드 nullable 전환이 적절합니다!모든 핵심 필드가 nullable로 변경되고
doseStatusList도 기본값 null과 함께 추가되어, 서버에서 데이터가 없을 때 안전하게 처리할 수 있습니다. 기본값 덕분에 기존 코드와의 호환성도 유지되네요.
89-93: DoseStatusUiState 구조가 명확합니다!복용 시간대(
time)는 필수값으로, 복용 여부(taken)는 nullable Boolean으로 설계하여 "미기록" 상태를 잘 표현할 수 있습니다. 주석도 명확하네요.app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (1)
229-251: fallback 경로에서 doseStatusList 처리가 잘 되어 있습니다!서버 응답이 없는 경우(404 등)의 fallback 시나리오에서
doseStatusList를emptyList()로 초기화하는 것이 적절합니다. HomeUiState의 구조 변경에 맞춰 정확하게 업데이트되었네요.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (1)
137-147: 복약 아이콘 렌더링 로직이 안전하게 구현되었습니다.
coerceAtLeast(0)로 음수 값을 방어하고,isNullOrEmpty()로 널과 빈 리스트를 모두 처리하는 점이 좋습니다. 데이터가 부족하거나 많을 때도 적절히 대응하고 있네요.한 가지만 참고하자면,
coerceAtLeast(0)가 음수 값을 숨길 수 있어서 만약 서버에서 잘못된 데이터가 온다면 디버깅이 어려울 수 있습니다. 필요하다면 음수 값이 들어올 때 로그를 남기는 것도 고려해볼 수 있지만, 현재 구현도 충분히 안전합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt(5 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). (1)
- GitHub Check: ci-build
🔇 Additional comments (4)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/component/HomeMedicineContainer.kt (4)
79-80: 널 안전 처리 잘 적용되었습니다.이전 리뷰에서 지적된 중괄호 앞 공백 문제가 해결되었고,
?: 0으로 널 값을 안전하게 처리하고 있습니다. 로직도 명확합니다.
117-124: 조건부 렌더링 잘 적용되었습니다.
nextDoseTime이 널이거나 빈 값일 때 다음 복약 시간을 표시하지 않도록 처리한 부분이 적절합니다. 미기록 상태에서 불필요한 UI 요소를 숨기는 좋은 접근입니다.
166-166: 일관된 널 안전 처리입니다.복용 횟수 표시에도 동일한
?: 0패턴을 사용해서 일관성이 있습니다.
218-218: 프리뷰 데이터가 미기록 상태를 잘 표현합니다.
doseStatusList를null로 설정해서 미기록 상태를 더 명확하게 테스트할 수 있습니다. Line 225의emptyList()와 함께 널과 빈 리스트 두 가지 경우를 모두 프리뷰에서 확인할 수 있네요.
| HomeUiState.from(res) | ||
| } catch (e: HttpException) { | ||
| Log.e("HomeRepo", "HTTP error fetching home data: ${e.code()}", e) | ||
| HomeUiState.Companion.EMPTY |
There was a problem hiding this comment.
현재 Data 레이어에 있는 Repository 에서 UI 레이어에 있는 UiState를 참조하고 있습니다.
RepositoryImpl 클래스에서는 응답값을 리턴할 때 Ui 레이어에 없는 단순 Dto 를 리턴하거나, 도메인 모델(현재는 없지만)을 리턴하는 게 좋을 것 같습니다.
그 후 Ui 레이어 혹은 매퍼를 통해 변환하는 게 좋을 것 같습니다.
추후 from 대신 toUiState 함수가 더 좋을 것 같아요!
참고: https://developer.android.com/topic/architecture?hl=ko#data-layer
There was a problem hiding this comment.
아하 거꾸로 참조해서 그런거군요...! 수정해보겠습니다
리뷰 감사합니다!!
| navigateToStateMentalDetailScreen = navigateToStateMentalDetailScreen, | ||
| navigateToGlucoseDetailScreen = navigateToGlucoseDetailScreen, | ||
| mainBackStackEntry = backStackEntry, | ||
| navigateToAlarm = navigateToAlarmScreen, |
- RepositoryImpl 내부에서 UIState.from(res) 호출 제거 - 불필요한 로그/중복 코드(HomeViewModel) 정리
- getHomeUiState() 호출 제거 및 getHomeSummary() 사용으로 변경 - 서버가 날짜를 받지 않으므로 today 파라미터 제거 - softRefreshCurrentElder 과거 구조에서 필요했던 임시 refresh 로직 주석처리
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/konkuk/medicarecall/data/repository/HomeRepository.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt(1 hunks)app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
🔇 Additional comments (1)
app/src/main/java/com/konkuk/medicarecall/data/repository/HomeRepository.kt (1)
7-7: DTO 반환으로 계층 분리가 명확해졌어요.UiState 대신 HomeResponseDto를 직접 노출하도록 바꾼 덕분에 Data → UI 간 의존이 정리되어 방향성이 좋아졌습니다.
app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/HomeRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (1)
291-342: 주석 처리된 코드 블록을 제거하는 것이 좋습니다.52줄에 달하는
softRefreshCurrentElder함수 전체가 주석 처리되어 있습니다. 이 함수는 더 이상 사용되지 않으며(Line 44 참조), 필요할 경우 Git 히스토리에서 언제든 확인할 수 있으므로 제거하는 것을 추천합니다.다음 diff를 적용하여 주석 처리된 코드를 제거하세요:
-// private fun softRefreshCurrentElder(timeoutMs: Long = 1200L) { -// val id = selectedElderId.value ?: return -// viewModelScope.launch { -// val keepName = _homeUiState.value.elderName -// -// runCatching { -// withTimeout(timeoutMs) { -// val today = LocalDate.now() -// val dto = homeRepository.getHomeSummary(id, today) -// val uiFromServer = HomeUiState.from(dto) -// -// val healthInfo = runCatching { -// eldersHealthInfoRepository.refresh() -// eldersHealthInfoRepository.getEldersHealthInfo().getOrNull() -// ?.firstOrNull { it.elderId == id } -// }.getOrNull() -// -// val correctMedicationOrder = healthInfo?.medications -// ?.flatMap { it.value } -// ?.distinct().orEmpty() -// -// val mergedMedicines = if (uiFromServer.medicines.isNotEmpty()) { -// uiFromServer.medicines -// } else { -// healthInfo?.medications -// ?.flatMap { (time, meds) -> meds.map { it to time } } -// ?.groupBy { it.first } -// ?.map { (name, group) -> -// MedicineUiState( -// medicineName = name, -// todayTakenCount = 0, -// todayRequiredCount = group.size, -// nextDoseTime = "-", -// doseStatusList = emptyList(), -// ) -// }.orEmpty() -// }.sortedBy { med -> -// correctMedicationOrder.indexOf(med.medicineName) -// .let { if (it == -1) Int.MAX_VALUE else it } -// } -// -// _homeUiState.value = uiFromServer.copy( -// elderName = keepName, -// medicines = mergedMedicines, -// isLoading = false, -// ) -// } -// }.onFailure { -// _homeUiState.update { it.copy(isLoading = false) } -// } -// } -// } -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt(6 hunks)
🧰 Additional context used
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
[error] 144-144: Detekt: Missing space after // [CommentSpacing].
🔇 Additional comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (2)
146-150: DTO 기반 변환 로직이 잘 구현되었습니다.새로운
getHomeSummary(elderId)→HomeUiState.from(dto)플로우가 PR 목표에 맞게 깔끔하게 구현되었습니다. Repository에서 UI 상태 생성 로직을 제거하고 DTO를 반환하도록 변경한 것이 올바른 접근입니다.
177-177:doseStatusList필드 추가가 올바르게 처리되었습니다.
MedicineUiState의 새로운doseStatusList필드를 세 곳(fallback 약물 생성 시)에서 일관되게emptyList()로 초기화하고 있습니다. Null-safe 처리가 잘 되어 있습니다.Also applies to: 238-238, 251-251
| _homeUiState.update { it.copy(isLoading = false) } | ||
| val today = LocalDate.now() | ||
| //val today = LocalDate.now() |
There was a problem hiding this comment.
로딩 상태 초기화가 잘못되었고 주석 포맷팅 오류가 있습니다.
두 가지 문제가 있습니다:
-
치명적 문제: Line 143에서
isLoading = false로 설정하고 있는데, 이는 비동기 작업을 시작하는 시점이므로isLoading = true가 되어야 합니다. 현재 코드는 데이터를 가져오는 동안 로딩 인디케이터가 표시되지 않습니다. -
린터 오류: Line 144의 주석에
//뒤에 공백이 없습니다 (Detekt CommentSpacing 규칙 위반, 파이프라인 실패 로그에서 확인됨).
다음 diff를 적용하여 두 문제를 모두 해결하세요:
private fun fetchHomeSummaryForToday(elderId: Int) {
viewModelScope.launch {
- _homeUiState.update { it.copy(isLoading = false) }
- //val today = LocalDate.now()
+ _homeUiState.update { it.copy(isLoading = true) }
+ // val today = LocalDate.now()
try {📝 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.
| _homeUiState.update { it.copy(isLoading = false) } | |
| val today = LocalDate.now() | |
| //val today = LocalDate.now() | |
| _homeUiState.update { it.copy(isLoading = true) } | |
| // val today = LocalDate.now() |
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 144-144: Detekt: Missing space after // [CommentSpacing].
🤖 Prompt for AI Agents
In
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
around lines 143 to 144, change the loading-state update to set isLoading = true
(since this is the start of an async operation) instead of false, and fix the
comment spacing by adding a single space after the // so it reads "// val today
= LocalDate.now()".
…171' into fix/home-statistics-empty-data-#171 # Conflicts: # app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (1)
143-143: 로딩 상태 초기화가 잘못되었습니다.비동기 작업을 시작하는 시점이므로
isLoading = true로 설정해야 합니다. 현재 코드는 데이터를 가져오는 동안 로딩 인디케이터가 표시되지 않습니다.다음 diff를 적용하세요:
- _homeUiState.update { it.copy(isLoading = false) } + _homeUiState.update { it.copy(isLoading = true) }
🧹 Nitpick comments (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (1)
291-342: 주석 처리된 코드를 제거하는 것을 고려해보세요.52줄의
softRefreshCurrentElder메서드가 주석 처리되어 있습니다. 이 코드가 더 이상 필요하지 않다면 버전 관리 시스템에 이력이 남아있으니 완전히 제거하는 것이 코드베이스를 깔끔하게 유지하는 데 도움이 됩니다.과도기적으로 남겨두려는 의도라면 무시하셔도 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt(6 hunks)
🧰 Additional context used
🪛 GitHub Actions: Android CI
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
[error] 44-44: gradlew detekt failed: Detekt indentation issue. Unexpected indentation (4) vs expected (8) at HomeViewModel.kt: line 44. (Indentation)
🔇 Additional comments (2)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (2)
146-150: DTO 기반 데이터 플로우로 개선되었습니다.Repository에서 DTO를 반환하고 ViewModel이 UI 상태로 변환하는 구조로 변경되어 레이어 분리가 명확해졌습니다. null-safe 처리도 DTO 단계에서 처리할 수 있게 되었네요.
177-177: doseStatusList 필드 초기화가 일관되게 처리되었습니다.새로 추가된
doseStatusList필드가 모든MedicineUiState생성 지점에서emptyList()로 일관되게 초기화되었습니다.Also applies to: 237-238, 251-251
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
Outdated
Show resolved
Hide resolved
|
@alswlekk @ProtossManse |
| val todayTakenCount: Int?, | ||
| val todayRequiredCount: Int?, | ||
| val nextDoseTime: String?, | ||
| val doseStatusList: List<DoseStatusUiState>? = null, |
🔗 관련 이슈
📙 작업 설명
Dto 필드의 모든 타입을 널러블하게 수정했습니다.
누락된 필드를 안전하게 파싱할 수 있도록 기본값을 ? = null로 설정했습니다.
UI State에서도 null값을 그대로 받을 수 있도록 수정했습니다.
StatisticsViewModel에 있던 DTO 변환 로직을 WeeklySummaryUiState의 from() 팩토리 메서드로
분리했습니다.
NameBar의 알림아이콘 클릭시, 알림화면을 호출하고 이동하도록 변경하였습니다
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
새로운 기능
개선사항
버그 수정/안정성