[FEAT/#103] 챌린지 루틴 목록 조회 API 연결 완료 #119
Conversation
- Int가 아닌 Long으로 변경 - 가존 Model에 isSelected를 추가하고 이 부분을 State에서 관리할 수 있도록 변경
- UiState(Loading/Failure/Success) 수집 후 상태에 따라 화면 분기 처리 - Success 시 ChallengeMissionSelectedScreen에 missions/hasSelected 등 UI 상태 전달 - 뒤로가기/닫기/미션 클릭/추가하기 이벤트를 ViewModel 메서드로 연결(onBackClick, onCloseClick, onTodoMissionClick, onAddTodoClick) - Preview에서 미션 클릭 시 isSelected 토글 및 버튼 enabled 동작까지 확인 가능하도록 구성
- ChallengeMissionSelected 화면에 ViewModel을 적용하고 Route에서 UiState를 수집하도록 구조 정리 - UI에서 직접 상태를 관리하던 로직을 제거하고, Screen은 전달받은 상태와 이벤트만 처리하도록 역할 분리 - 미션 클릭, 뒤로가기, 닫기, 추가하기 액션을 ViewModel 이벤트로 위임하여 단방향 데이터 흐름 적용 - Preview에서는 로컬 상태를 사용해 선택 토글 및 버튼 활성화 동작을 확인 가능하도록 유지
- 컨벤션에 맞춰 줄바꿈 정리 - ktLint 실행
- ChallengeRoutine 모델 기본값 부여 - UI에 Model과 겹치는 부분 삭제 및 변경
- 챌린지 뷰모델 선ㅇ너
- 챌린지 뷰모델을 스크린에 적용
# Conflicts: # app/src/main/java/com/cherrish/android/data/di/DataSourceModule.kt # app/src/main/java/com/cherrish/android/data/di/RepositoryModule.kt # app/src/main/java/com/cherrish/android/data/di/ServiceModule.kt
Walkthrough홈케어 루틴 조회 API 연동을 위해 서비스/데이터소스/레포지토리와 DI 바인딩을 추가하고, 프레젠테이션에서 루트 및 모델을 ChallengeStart/ChallengeRoutine/ChallengeMission 구조로 재배치하며 side-effect 기반 네비게이션을 도입했습니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant Screen as ChallengeRoutineScreen
participant VM as ChallengeRoutineViewModel
participant Repo as ChallengeRepository
participant DS as ChallengeDataSource
participant Service as ChallengeService
participant API as RemoteAPI
User->>Screen: 진입
Screen->>VM: 초기화
VM->>Repo: getChallengeRoutineData()
Repo->>DS: getHomecareRoutineData()
DS->>Service: getHomecareRoutineData()
Service->>API: GET /api/challenges/homecare-routines
API-->>Service: DTO 리스트
Service-->>DS: BaseResponse<DTO[]>
DS-->>Repo: BaseResponse<DTO[]>
Repo->>Repo: DTO -> Model (toModel/ toUiModel)
Repo-->>VM: Result<Model 리스트>
VM-->>Screen: uiState 업데이트
User->>Screen: 루틴 선택 / 다음 클릭
Screen->>VM: onRoutineClick / onNextClick
VM->>VM: _sideEffect.emit(navigateToMission)
VM-->>Screen: ChallengeSideEffect.navigateToMission
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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
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 |
머지 병합 # Conflicts: # app/src/main/java/com/cherrish/android/core/designsystem/component/type/CherrishGaugeType.kt # Conflicts: # app/src/main/java/com/cherrish/android/core/designsystem/component/type/CherrishGaugeType.kt
![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 (3)
app/src/main/java/com/cherrish/android/presentation/challenge/start/ChallengeStartScreen.kt (1)
45-49: paddingValues 적용 순서가 올바르지 않습니다.Based on learnings,
paddingValues는 시스템 바(status/navigation bar)를 위한 것이므로 root container의 modifier에 먼저 적용해야 합니다. 현재는horizontal패딩이 먼저 적용되고 있습니다.♻️ 제안된 수정
Column( modifier = modifier .fillMaxSize() - .padding(horizontal = 17.dp) .padding(paddingValues) + .padding(horizontal = 17.dp) - ) {app/src/main/java/com/cherrish/android/presentation/challenge/mission/ChallengeMissionViewModel.kt (1)
55-60: onAddTodoClick이 선택된 상태에서도 기능 구현이 없습니다.
if (!state.isSelected) return@updateSuccess state조건은 통과하지만, 이후state.copy()만 호출하여 상태 변화가 없는 no-op 상태입니다. 선택된 미션을 처리하는 로직(API 호출, 데이터 저장, 네비게이션 등)을 구현하거나, 프로젝트의 다른 ViewModel들처럼 SideEffect 패턴을 통해 네비게이션을 처리해야 합니다.app/src/main/java/com/cherrish/android/presentation/home/HomeScreen.kt (1)
52-54: 로딩 화면에서 Scaffold 인셋이 누락되고 있습니다로딩 상태에서
paddingValues가 전달되지 않아 시스템 바/Scaffold 인셋이 무시되고 있습니다. 또한 로컬LoadingScreen이 디자인시스템LoadingScreen컴포넌트를 섀도잉하고 있습니다. 아래처럼paddingValues를 전달하고 루트 컨테이너에 적용해 주세요.🛠️ 수정 제안
is UiState.Loading -> { - LoadingScreen() + LoadingScreen(paddingValues = paddingValues) }`@Composable` private fun LoadingScreen( + paddingValues: PaddingValues, ) { Column( modifier = Modifier .fillMaxSize() + .padding(paddingValues) .background(color = CherrishTheme.colors.gray0), horizontalAlignment = Alignment.CenterHorizontally, verticalArrangement = Arrangement.Center ) { CircularProgressIndicator() } }
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/cherrish/android/data/remote/dto/response/ChallengesResponseDto.kt`:
- Around line 28-59: The DTOs have a type mismatch:
ChallengeMissionProgressDataDto.progressPercentage is Int while
AdvanceDayDataDto.progressPercentage is Double; since the advance-day endpoint
and ChallengeMissionProgressResponseDto use Int, either remove the unused
AdvanceDayDataDto or change its progressPercentage to Int to match; locate
AdvanceDayDataDto and either delete that class (if unused) or update its
progressPercentage property to Int and run build/tests to ensure consistency
across usages (also update any mapping/parsing that references
AdvanceDayDataDto).
In
`@app/src/main/java/com/cherrish/android/data/repositoryimpl/ChallengeRepositoryImpl.kt`:
- Around line 13-19: Replace the unsafe use of data!! in getChallengeRoutineData
to handle null explicitly: call challengeDataSource.getHomecareRoutineData(),
check its .data for null, and either return a failed Result (e.g.,
Result.failure(IllegalStateException("homecare data missing"))) or return
Result.success(emptyList()) per project policy; update suspendRunCatching usage
so mapping (it.toModel()) only runs when .data is non-null and include a clear
error message referencing getChallengeRoutineData and getHomecareRoutineData
when returning failure.
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/mission/ChallengeMissionViewModel.kt`:
- Around line 18-20: The ViewModel currently seeds MutableStateFlow with
UiState.Success(ChallengeMissionUiState.Fake) (and later overwrites with a fixed
list in init), which can mask real API results; change the initial state to a
neutral state (e.g., UiState.Loading or UiState.Empty) in
ChallengeMissionViewModel and remove the hardcoded overwrite in init so the
state is updated only from the Repository/UseCase result (the fetch/update
method used in init). If you need fake data for previews or debug, gate
ChallengeMissionUiState.Fake behind a debug/preview flag or a separate factory
method so production paths always consume the real useCase/repository responses.
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/missionprogress/ChallengeMissionprogressScreen.kt`:
- Around line 34-56: UiState.Failure currently renders nothing; update the
Failure branch inside ChallengeMissionProgressRoute to show a simple error UI
(e.g., an ErrorScreen composable or Column with Text and a Button) that displays
the failure message and offers a retry action. Wire the retry Button to a
ViewModel retry/refresh method (call viewModel::retry or add a retry/fetch
function to ChallengeMissionProgressViewModel if missing) and pass any error
detail from the UiState.Failure payload so the user can see what went wrong and
retry. Ensure the new Error UI replaces the empty block under the
UiState.Failure case in ChallengeMissionProgressRoute.
- Around line 101-132: Replace the hardcoded texts in
ChallengeMissionSelectedTitle with string resources: add two entries in
strings.xml (e.g., <string name="challenge_title">%s 챌린지</string> and <string
name="seven_day_plan">7일 플랜</string>), then in ChallengeMissionSelectedTitle use
stringResource(R.string.challenge_title, challengeName) for the dynamic title
and stringResource(R.string.seven_day_plan) for the badge; also ensure you
import androidx.compose.ui.res.stringResource and remove the literal strings
from Text composables.
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/navigation/ChallengeNavigator.kt`:
- Around line 50-55: Rename the inconsistent parameter
navigationChallengeMission in the function challengeNavGraph to
navigateToChallengeMission to match the navigateToChallengeRoutine naming
pattern; update all references/usages of navigationChallengeMission (e.g., the
call site inside challengeNavGraph where it's invoked) to the new name so the
parameter and its invocations (within challengeNavGraph and any lambdas or
callers) remain consistent.
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/routine/ChallengeRoutineViewModel.kt`:
- Around line 54-63: Remove the dead commented-out hardcoded initialization
block for _uiState.updateSuccess and its
ChallengeRoutineUiState/ChallengeRoutineUiModel entries; locate the commented
lines referencing _uiState.updateSuccess, ChallengeRoutineUiState, and
ChallengeRoutineUiModel in ChallengeRoutineViewModel and delete the entire
commented section so only active code remains (no behavior change required).
- Around line 51-53: The onLogFailure block in ChallengeRoutineViewModel is
empty, leaving _uiState stuck at UiState.Loading on API failure; update
onLogFailure to set the ViewModel's state to an error (e.g., _uiState.value =
UiState.Error(...)) or a non-loading state and pass an appropriate error
message/exception so the UI can show feedback, and ensure you reference and use
the existing _uiState and UiState types inside the onLogFailure handler in
ChallengeRoutineViewModel.
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/start/ChallengeStartScreen.kt`:
- Around line 27-37: ChallengeStartRoute currently injects a
ChallengeStartViewModel that is never used; either remove the unused parameter
from ChallengeStartRoute (and its default hiltViewModel() injection) and update
callers to stop passing a viewModel, or if the ViewModel is needed for side
effects, subscribe to its sideEffect Flow inside ChallengeStartRoute (e.g., use
LaunchedEffect and collect/collectLatest on viewModel.sideEffect) and handle
emitted events (or forward state/handlers into ChallengeStartScreen). Update
imports and any callers accordingly and keep the function signature consistent
with ChallengeStartScreen’s needs.
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/start/ChallengeStartViewModel.kt`:
- Around line 16-25: The _sideEffect MutableSharedFlow, the exposed sideEffect
SharedFlow, and the onNextClick() method in ChallengeStartViewModel are unused
because ChallengeStartScreen receives an onNavigateRoutine callback directly;
remove the unused fields and method (symbols: _sideEffect, sideEffect,
onNextClick, ChallengeStartViewModel) to avoid dead code, or alternatively
refactor ChallengeStartScreen to collect viewModel.sideEffect and invoke
onNavigateRoutine from the collected ChallengeSideEffect if you prefer the
shared-effect pattern (symbols to touch: ChallengeStartScreen,
ChallengeStartViewModel, ChallengeSideEffect).
In `@app/src/main/java/com/cherrish/android/presentation/home/HomeViewModel.kt`:
- Line 8: HomeViewModel currently depends on the concrete HomeRepositoryImpl
instead of the HomeRepository interface; update the class to depend on
HomeRepository (not HomeRepositoryImpl) so DI/RepositoryModule bindings and
tests/mocks work correctly: change any imports/constructor parameters/properties
referencing HomeRepositoryImpl to HomeRepository and ensure the DI-injected type
used in HomeViewModel is the interface (HomeRepository) everywhere.
🧹 Nitpick comments (7)
app/src/main/java/com/cherrish/android/data/remote/dto/response/ChallengesResponseDto.kt (1)
7-14:TodayRoutineDto와ChallengeRoutineDto가 동일한 필드를 가지고 있습니다.두 DTO 클래스가 완전히 동일한 필드(
routineId,name,scheduledDate,isComplete)를 가지고 있어 DRY 원칙을 위반합니다. 하나의 공통 DTO로 통합하는 것을 고려해 주세요.또한
ChallengeHomecareRoutinesResponseDto의id는Int타입인 반면, 다른 DTO들의routineId와challengeId는Long타입입니다. API 응답에 따라 일관된 타입 사용이 필요한지 확인해 주세요.Also applies to: 61-74, 100-113
app/src/main/java/com/cherrish/android/presentation/challenge/ChallengeSideEffect.kt (1)
3-5: Kotlin 네이밍 컨벤션: data object는 PascalCase 사용 권장
navigateToMission은 소문자로 시작하고 있으나, Kotlin의data object는 타입이므로 PascalCase 네이밍 컨벤션을 따르는 것이 좋습니다.♻️ 제안된 수정
sealed interface ChallengeSideEffect { - data object navigateToMission : ChallengeSideEffect + data object NavigateToMission : ChallengeSideEffect }app/src/main/java/com/cherrish/android/presentation/challenge/routine/ChallengeRoutineScreen.kt (1)
48-67: Loading/Failure 상태에 대한 UI 피드백 고려현재
UiState.Loading과UiState.Failure상태에서 빈 블록으로 처리되어 사용자에게 피드백이 없습니다. 로딩 인디케이터나 에러 메시지 표시를 고려해 보세요.♻️ 제안된 개선 예시
when (val state = uiState) { is UiState.Loading -> { // 로딩 인디케이터 표시 CherrishLoadingScreen() } is UiState.Failure -> { // 에러 메시지 또는 재시도 버튼 표시 CherrishErrorScreen(onRetry = viewModel::fetchRoutines) } is UiState.Success -> { /* ... */ } else -> {} }app/src/main/java/com/cherrish/android/presentation/main/MainAppState.kt (1)
117-123:navigateToChallengeRoutine에NavOptions파라미터 추가를 고려해주세요.
navigateToChallengeMission은NavOptions파라미터를 받지만,navigateToChallengeRoutine은 받지 않습니다. 다른 네비게이션 함수들(navigateToOnboarding,navigateToHome등)과의 일관성을 위해 동일한 시그니처를 사용하는 것이 좋습니다.또한, 두 함수 사이에 빈 줄을 추가하면 가독성이 향상됩니다.
♻️ 제안하는 수정사항
- fun navigateToChallengeRoutine() { - navController.navigateToChallengeRoutine() + fun navigateToChallengeRoutine(navOptions: NavOptions? = null) { + navController.navigateToChallengeRoutine(navOptions) } + fun navigateToChallengeMission(navOptions: NavOptions? = clearStackNavOptions) { navController.navigateToChallengeMission(navOptions) }app/src/main/java/com/cherrish/android/presentation/challenge/navigation/ChallengeNavigator.kt (2)
21-23:ChallengeMission선언과 함수 사이에 빈 줄을 추가해주세요.data object 선언과 extension function 사이에 빈 줄이 없어 가독성이 떨어집니다.
♻️ 제안하는 수정사항
`@Serializable` data object ChallengeMission : Route + fun NavController.navigateToChallengeStart(
72-77: 주석 처리된 TODO 코드를 정리해주세요.
// navigateToProgress = { }가 주석으로 남아 있습니다. 향후 구현이 필요한 경우 TODO 주석으로 명시하거나, 불필요하면 제거해주세요.♻️ 제안하는 수정사항
composable<ChallengeMission> { ChallengeMissionSelectedRoute( paddingValues = paddingValues - // navigateToProgress = { } ) }app/src/main/java/com/cherrish/android/presentation/challenge/mission/ChallengeMissionUiState.kt (1)
6-6: Fake 데이터는 테스트/프리뷰 전용으로 한정하는 게 안전합니다.companion object의 public
Fake는 실사용 경로에서 의존될 위험이 있어,internal/@VisibleForTesting/프리뷰 전용 Provider로 분리하는 것을 권장합니다.Also applies to: 19-48
| @Serializable | ||
| data class ChallengeMissionProgressDataDto( | ||
| @SerialName("challengeId") | ||
| val challengeId: Long, | ||
|
|
||
| @SerialName("title") | ||
| val title: String, | ||
|
|
||
| @SerialName("currentDay") | ||
| val currentDay: Int, | ||
|
|
||
| @SerialName("progressPercentage") | ||
| val progressPercentage: Int, | ||
|
|
||
| @SerialName("cherryLevel") | ||
| val cherryLevel: Int, | ||
|
|
||
| @SerialName("cherryLevelName") | ||
| val cherryLevelName: String, | ||
|
|
||
| @SerialName("progressToNextLevel") | ||
| val progressToNextLevel: Double, | ||
|
|
||
| @SerialName("remainingRoutinesToNextLevel") | ||
| val remainingRoutinesToNextLevel: Int, | ||
|
|
||
| @SerialName("todayRoutines") | ||
| val todayRoutines: List<TodayRoutineDto>, | ||
|
|
||
| @SerialName("cheeringMessage") | ||
| val cheeringMessage: String | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "progressPercentage" --type=kt -C2Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 100
🏁 Script executed:
fd "ChallengesResponseDto.kt" && cat -n app/src/main/java/com/cherrish/android/data/remote/dto/response/ChallengesResponseDto.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 4228
🏁 Script executed:
rg "ChallengeMissionProgressDataDto|AdvanceDayDataDto" -C 3 | head -50Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 1548
🏁 Script executed:
rg "progressPercentage" -B 2 -A 2Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 9103
🏁 Script executed:
rg "AdvanceDayDataDto" -B 3 -A 3Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 802
🏁 Script executed:
rg "AdvanceDayDataDto" --type-list | grep -i kotlinRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 463
🏁 Script executed:
rg "AdvanceDayDataDto" -C 5Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 1264
🏁 Script executed:
rg "AdvanceDayDataDto" --no-headingRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 192
🏁 Script executed:
rg "AdvanceDay" -C 2Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 4750
🏁 Script executed:
fd "ChallengeMissionProgressResponseDto.kt" && cat -n app/src/main/java/com/cherrish/android/data/remote/dto/response/ChallengeMissionProgressResponseDto.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 1619
progressPercentage 필드 타입 불일치 확인
ChallengeMissionProgressDataDto에서는 progressPercentage가 Int 타입(Line 40)이고, AdvanceDayDataDto에서는 Double 타입(Line 127)입니다. 두 DTO는 거의 동일한 구조인데 이 필드만 타입이 다르며, AdvanceDayDataDto는 현재 코드베이스에서 사용되지 않고 있습니다.
실제 advance-day 엔드포인트는 ChallengeMissionProgressResponseDto를 반환하며 여기서도 progressPercentage는 Int 타입입니다. 불필요한 AdvanceDayDataDto를 제거하거나, 사용 계획이 있다면 두 DTO의 타입을 일치시켜야 합니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/data/remote/dto/response/ChallengesResponseDto.kt`
around lines 28 - 59, The DTOs have a type mismatch:
ChallengeMissionProgressDataDto.progressPercentage is Int while
AdvanceDayDataDto.progressPercentage is Double; since the advance-day endpoint
and ChallengeMissionProgressResponseDto use Int, either remove the unused
AdvanceDayDataDto or change its progressPercentage to Int to match; locate
AdvanceDayDataDto and either delete that class (if unused) or update its
progressPercentage property to Int and run build/tests to ensure consistency
across usages (also update any mapping/parsing that references
AdvanceDayDataDto).
| override suspend fun getChallengeRoutineData(): | ||
| Result<List<ChallengeHomecareRoutinesResponseModel>> = | ||
| suspendRunCatching { | ||
| challengeDataSource | ||
| .getHomecareRoutineData() | ||
| .data!! | ||
| .map { it.toModel() } |
There was a problem hiding this comment.
data!! NPE 위험 — 명시적 널 처리 필요.
서버 응답에서 data가 null이면 즉시 NPE가 발생합니다. 실패로 돌릴지/빈 리스트로 처리할지 정책을 명시해 주세요.
🛠️ 수정 제안(명시적 실패 처리)
- suspendRunCatching {
- challengeDataSource
- .getHomecareRoutineData()
- .data!!
- .map { it.toModel() }
- }
+ suspendRunCatching {
+ val data = challengeDataSource
+ .getHomecareRoutineData()
+ .data
+ ?: throw IllegalStateException("Homecare routine data is null")
+ data.map { it.toModel() }
+ }📝 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.
| override suspend fun getChallengeRoutineData(): | |
| Result<List<ChallengeHomecareRoutinesResponseModel>> = | |
| suspendRunCatching { | |
| challengeDataSource | |
| .getHomecareRoutineData() | |
| .data!! | |
| .map { it.toModel() } | |
| override suspend fun getChallengeRoutineData(): | |
| Result<List<ChallengeHomecareRoutinesResponseModel>> = | |
| suspendRunCatching { | |
| val data = challengeDataSource | |
| .getHomecareRoutineData() | |
| .data | |
| ?: throw IllegalStateException("Homecare routine data is null") | |
| data.map { it.toModel() } | |
| } |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/data/repositoryimpl/ChallengeRepositoryImpl.kt`
around lines 13 - 19, Replace the unsafe use of data!! in
getChallengeRoutineData to handle null explicitly: call
challengeDataSource.getHomecareRoutineData(), check its .data for null, and
either return a failed Result (e.g.,
Result.failure(IllegalStateException("homecare data missing"))) or return
Result.success(emptyList()) per project policy; update suspendRunCatching usage
so mapping (it.toModel()) only runs when .data is non-null and include a clear
error message referencing getChallengeRoutineData and getHomecareRoutineData
when returning failure.
| MutableStateFlow<UiState<ChallengeMissionUiState>>( | ||
| UiState.Success(ChallengeMissionUiState.Fake) | ||
| ) |
There was a problem hiding this comment.
하드코딩된 초기/로드 데이터가 실제 API 결과를 덮어쓸 위험이 큽니다.
초기 상태를 Success(Fake)로 두고 init에서 다시 고정 리스트로 덮어써서, API 연동이 되어 있더라도 UI가 스텁 데이터로 고정될 수 있습니다. 실제 Repository/UseCase 결과로 갱신하도록 전환하거나, Fake는 디버그/프리뷰 분기에서만 쓰는 구조로 분리해 주세요.
Also applies to: 27-38
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/mission/ChallengeMissionViewModel.kt`
around lines 18 - 20, The ViewModel currently seeds MutableStateFlow with
UiState.Success(ChallengeMissionUiState.Fake) (and later overwrites with a fixed
list in init), which can mask real API results; change the initial state to a
neutral state (e.g., UiState.Loading or UiState.Empty) in
ChallengeMissionViewModel and remove the hardcoded overwrite in init so the
state is updated only from the Repository/UseCase result (the fetch/update
method used in init). If you need fake data for previews or debug, gate
ChallengeMissionUiState.Fake behind a debug/preview flag or a separate factory
method so production paths always consume the real useCase/repository responses.
There was a problem hiding this comment.
저 이것도 반영할까요 ..?
There was a problem hiding this comment.
@hyeminililo 좋은 질문입니다! 제 의견은 다음과 같습니다:
현재 PR 범위를 고려한 추천:
이 PR은 "챌린지 루틴 목록 조회 API 연결"에 집중하고 있고, ChallengeMissionViewModel의 미션 데이터는 아직 API가 연결되지 않은 상태로 보입니다.
두 가지 옵션이 있습니다:
-
지금 반영하지 않기 (추천):
- 현재 PR의 범위를 루틴 API 연결로 한정
- 미션 API 연결할 때 함께 리팩토링
- 이슈를 생성하여 추후 작업으로 관리
-
지금 일부 반영하기:
- 초기 상태만
UiState.Loading으로 변경 loadMissions()는 그대로 두되, 추후 API 연결 시 교체 예정임을 주석으로 표시
private val _uiState = MutableStateFlow<UiState<ChallengeMissionUiState>>(UiState.Loading) init { loadMissions() // TODO: Replace with API call when mission endpoint is ready }
- 초기 상태만
추천: 현재 PR의 범위를 지키고, 미션 데이터 API 연결은 별도 이슈로 관리하는 것이 좋을 것 같습니다. 원하신다면 follow-up 이슈를 생성해 드릴까요?
| @Composable | ||
| fun ChallengeMissionProgressRoute( | ||
| paddingValues: PaddingValues, | ||
| viewModel: ChallengeMissionProgressViewModel = hiltViewModel() | ||
| ) { | ||
| val uiState by viewModel.uiState.collectAsStateWithLifecycle() | ||
|
|
||
| when (val state = uiState) { | ||
| is UiState.Loading -> { | ||
| LoadingScreen() | ||
| } | ||
| is UiState.Failure -> {} | ||
| is UiState.Success -> { | ||
| ChallengeMissionprogressScreen( | ||
| paddingValues = paddingValues, | ||
| uiState = state.data, | ||
| onTodoClick = viewModel::onTodoClick, | ||
| onCompleteTodayClick = viewModel::onCompletedTodayClick | ||
| ) | ||
| } | ||
|
|
||
| else -> {} | ||
| } |
There was a problem hiding this comment.
Failure 상태가 빈 화면으로 남습니다
UiState.Failure에서 아무 UI도 그리지 않아 사용자에게 멈춘 화면처럼 보일 수 있습니다. 최소한 오류 메시지/재시도 버튼을 제공해 주세요.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/missionprogress/ChallengeMissionprogressScreen.kt`
around lines 34 - 56, UiState.Failure currently renders nothing; update the
Failure branch inside ChallengeMissionProgressRoute to show a simple error UI
(e.g., an ErrorScreen composable or Column with Text and a Button) that displays
the failure message and offers a retry action. Wire the retry Button to a
ViewModel retry/refresh method (call viewModel::retry or add a retry/fetch
function to ChallengeMissionProgressViewModel if missing) and pass any error
detail from the UiState.Failure payload so the user can see what went wrong and
retry. Ensure the new Error UI replaces the empty block under the
UiState.Failure case in ChallengeMissionProgressRoute.
| @Composable | ||
| private fun ChallengeMissionSelectedTitle( | ||
| challengeName: String, | ||
| modifier: Modifier = Modifier | ||
| ) { | ||
| Row( | ||
| modifier = modifier.fillMaxWidth(), | ||
| horizontalArrangement = Arrangement.spacedBy(12.dp), | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| Text( | ||
| text = "$challengeName 챌린지", | ||
| style = CherrishTheme.typography.title1SB18, | ||
| color = CherrishTheme.colors.gray1000 | ||
| ) | ||
|
|
||
| Text( | ||
| text = "7일 플랜", | ||
| color = CherrishTheme.colors.gray700, | ||
| style = CherrishTheme.typography.body3M12, | ||
| modifier = Modifier | ||
| .background( | ||
| color = CherrishTheme.colors.gray100, | ||
| shape = RoundedCornerShape(4.dp) | ||
| ) | ||
| .border( | ||
| width = 1.dp, | ||
| color = CherrishTheme.colors.gray700, | ||
| shape = RoundedCornerShape(4.dp) | ||
| ) | ||
| .padding(horizontal = 8.dp, vertical = 3.dp) | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "strings.xml" | head -20Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 111
🏁 Script executed:
git ls-files | grep -E "(strings|ChallengeMissionprogress)" | head -20Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 221
🏁 Script executed:
head -150 app/src/main/java/com/cherrish/android/presentation/challenge/missionprogress/ChallengeMissionprogressScreen.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 5247
🏁 Script executed:
cat app/src/main/res/values/strings.xml | head -100Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 144
🏁 Script executed:
rg "stringResource" app/src/main/java/com/cherrish/android/presentation/challenge/ -A 1 | head -30Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 56
🏁 Script executed:
rg "import.*stringResource" app/src/main/java/com/cherrish/android/ | head -10Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 56
🏁 Script executed:
rg "stringResource" app/src/main/java/ --type kotlin | head -20Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 56
🏁 Script executed:
fd "strings" app/src/main/res/ --type fRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 109
🏁 Script executed:
rg "chapellenges|challenge" app/src/main/res/values/strings.xmlRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 56
하드코딩된 문자열을 stringResource로 추출 권장
"7일 플랜"과 "${challengeName} 챌린지"는 다국어 지원을 위해 strings.xml에 정의하고 stringResource()를 사용하는 것이 좋습니다. 특히 "7일 플랜"은 직접 추출 가능하며, 동적 값이 포함된 문자열은 strings.xml에서 "%s 챌린지" 형태로 포매팅 인자를 활용할 수 있습니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/missionprogress/ChallengeMissionprogressScreen.kt`
around lines 101 - 132, Replace the hardcoded texts in
ChallengeMissionSelectedTitle with string resources: add two entries in
strings.xml (e.g., <string name="challenge_title">%s 챌린지</string> and <string
name="seven_day_plan">7일 플랜</string>), then in ChallengeMissionSelectedTitle use
stringResource(R.string.challenge_title, challengeName) for the dynamic title
and stringResource(R.string.seven_day_plan) for the badge; also ensure you
import androidx.compose.ui.res.stringResource and remove the literal strings
from Text composables.
| .onLogFailure { | ||
| } | ||
| } |
There was a problem hiding this comment.
API 실패 시 에러 처리가 누락되었습니다.
onLogFailure 블록이 비어 있어서 API 호출 실패 시 UI 상태가 UiState.Loading에 머물러 사용자에게 아무런 피드백이 제공되지 않습니다. 실패 상태로 업데이트하거나 적절한 에러 처리를 추가해주세요.
🐛 제안하는 수정사항
.onLogFailure {
+ _uiState.value = UiState.Failure(it)
}📝 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.
| .onLogFailure { | |
| } | |
| } | |
| .onLogFailure { | |
| _uiState.value = UiState.Failure(it) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/routine/ChallengeRoutineViewModel.kt`
around lines 51 - 53, The onLogFailure block in ChallengeRoutineViewModel is
empty, leaving _uiState stuck at UiState.Loading on API failure; update
onLogFailure to set the ViewModel's state to an error (e.g., _uiState.value =
UiState.Error(...)) or a non-loading state and pass an appropriate error
message/exception so the UI can show feedback, and ensure you reference and use
the existing _uiState and UiState types inside the onLogFailure handler in
ChallengeRoutineViewModel.
| @Composable | ||
| fun ChallengeStartRoute( | ||
| paddingValues: PaddingValues, | ||
| onNavigateRoutine: () -> Unit, | ||
| viewModel: ChallengeStartViewModel = hiltViewModel() | ||
|
|
||
| ) { | ||
| ChallengeStartScreen( | ||
| paddingValues = paddingValues, | ||
| onNextClick = viewModel::onNextClick | ||
| onNextClick = onNavigateRoutine | ||
| ) | ||
| } |
There was a problem hiding this comment.
사용되지 않는 viewModel 파라미터
viewModel: ChallengeStartViewModel이 주입되지만 Route 내에서 전혀 사용되지 않습니다. 사용하지 않는 의존성은 제거하거나, ChallengeStartViewModel의 sideEffect를 활용하려면 해당 flow를 구독해야 합니다.
♻️ 제안: 사용하지 않는 viewModel 제거
`@Composable`
fun ChallengeStartRoute(
paddingValues: PaddingValues,
- onNavigateRoutine: () -> Unit,
- viewModel: ChallengeStartViewModel = hiltViewModel()
+ onNavigateRoutine: () -> Unit
) {
ChallengeStartScreen(
paddingValues = paddingValues,
onNextClick = onNavigateRoutine
)
}📝 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.
| @Composable | |
| fun ChallengeStartRoute( | |
| paddingValues: PaddingValues, | |
| onNavigateRoutine: () -> Unit, | |
| viewModel: ChallengeStartViewModel = hiltViewModel() | |
| ) { | |
| ChallengeStartScreen( | |
| paddingValues = paddingValues, | |
| onNextClick = viewModel::onNextClick | |
| onNextClick = onNavigateRoutine | |
| ) | |
| } | |
| `@Composable` | |
| fun ChallengeStartRoute( | |
| paddingValues: PaddingValues, | |
| onNavigateRoutine: () -> Unit | |
| ) { | |
| ChallengeStartScreen( | |
| paddingValues = paddingValues, | |
| onNextClick = onNavigateRoutine | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/start/ChallengeStartScreen.kt`
around lines 27 - 37, ChallengeStartRoute currently injects a
ChallengeStartViewModel that is never used; either remove the unused parameter
from ChallengeStartRoute (and its default hiltViewModel() injection) and update
callers to stop passing a viewModel, or if the ViewModel is needed for side
effects, subscribe to its sideEffect Flow inside ChallengeStartRoute (e.g., use
LaunchedEffect and collect/collectLatest on viewModel.sideEffect) and handle
emitted events (or forward state/handlers into ChallengeStartScreen). Update
imports and any callers accordingly and keep the function signature consistent
with ChallengeStartScreen’s needs.
| private val _sideEffect = MutableSharedFlow<ChallengeSideEffect>() | ||
| val sideEffect: SharedFlow<ChallengeSideEffect> = _sideEffect.asSharedFlow() | ||
|
|
||
| fun onNextClick() { | ||
| viewModelScope.launch { | ||
| _sideEffect.emit( | ||
| ChallengeSideEffect.navigateToMission | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "ChallengeStartScreen.kt" --type fRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 165
🏁 Script executed:
rg "ChallengeStartScreen" -t kt --max-count=20Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 100
🏁 Script executed:
rg "sideEffect" -t kt -B 3 -A 3 --max-count=50Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 100
🏁 Script executed:
rg "onNextClick" -t kt -B 2 -A 2Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 100
🏁 Script executed:
cat app/src/main/java/com/cherrish/android/presentation/challenge/start/ChallengeStartScreen.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 3849
🏁 Script executed:
rg "sideEffect" --max-count=50Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 4847
🏁 Script executed:
rg "onNextClick" -B 2 -A 2Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 15857
🏁 Script executed:
rg "navigateToMission" -B 3 -A 3Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 5135
sideEffect flow와 onNextClick() 메서드는 사용되지 않는 코드입니다
ChallengeStartViewModel에서 정의한 sideEffect flow와 onNextClick() 메서드는 실제로 호출되지 않습니다. ChallengeStartScreen은 viewModel을 주입받지만 사용하지 않으며, 대신 onNavigateRoutine 콜백을 직접 전달받아 처리합니다.
다른 화면들(SplashScreen, HomeScreen, CalendarScreen, ChallengeRoutineScreen 등)은 viewModel.sideEffect.collectLatestSideEffect를 통해 side effect를 처리하는 패턴을 따르고 있으므로, 두 가지 방식 중 하나를 선택해야 합니다:
ChallengeStartScreen에서도viewModel.sideEffect를 수집하여 side effect 패턴으로 통일- 또는 사용하지 않는
_sideEffect,sideEffect,onNextClick()제거
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/cherrish/android/presentation/challenge/start/ChallengeStartViewModel.kt`
around lines 16 - 25, The _sideEffect MutableSharedFlow, the exposed sideEffect
SharedFlow, and the onNextClick() method in ChallengeStartViewModel are unused
because ChallengeStartScreen receives an onNavigateRoutine callback directly;
remove the unused fields and method (symbols: _sideEffect, sideEffect,
onNextClick, ChallengeStartViewModel) to avoid dead code, or alternatively
refactor ChallengeStartScreen to collect viewModel.sideEffect and invoke
onNavigateRoutine from the collected ChallengeSideEffect if you prefer the
shared-effect pattern (symbols to touch: ChallengeStartScreen,
ChallengeStartViewModel, ChallengeSideEffect).
| import com.cherrish.android.core.common.state.UiState | ||
| import com.cherrish.android.data.model.toTodayDateString | ||
| import com.cherrish.android.data.repository.HomeRepository | ||
| import com.cherrish.android.data.repositoryimpl.HomeRepositoryImpl |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the HomeViewModel file and check its content
find . -name "HomeViewModel.kt" -type f | head -20Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 204
🏁 Script executed:
# Also check for HomeRepository interface and HomeRepositoryImpl
fd -t f "HomeRepository" --extension ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 227
🏁 Script executed:
# Look for DI/Hilt module files
fd -t f -e kt | grep -i "module\|di" | head -20Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 1788
🏁 Script executed:
cat -n app/src/main/java/com/cherrish/android/presentation/home/HomeViewModel.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 3260
🏁 Script executed:
cat -n app/src/main/java/com/cherrish/android/data/repository/HomeRepository.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 317
🏁 Script executed:
cat -n app/src/main/java/com/cherrish/android/data/di/RepositoryModule.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 2573
🏁 Script executed:
head -40 app/src/main/java/com/cherrish/android/data/repositoryimpl/HomeRepositoryImpl.ktRepository: TEAM-Cherrish/Cherrish-Android
Length of output: 728
🏁 Script executed:
# Search for other ViewModel `@Inject` constructors to see the pattern
rg -A2 "@HiltViewModel" --type=kt -A3 | grep -E "class |@Inject|Repository" | head -30Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 100
🏁 Script executed:
# Search for other ViewModel `@Inject` constructors to see the pattern
rg -A3 "@HiltViewModel" --type kotlin | grep -E "class |@Inject|Repository" | head -40Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 2612
🏁 Script executed:
# Alternative: search for all `@Inject` constructor patterns in ViewModels
rg "@Inject constructor" app/src/main/java/com/cherrish/android/presentation --type-list | head -5Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 463
🏁 Script executed:
# Search for other ViewModels to see if they use interface or impl
fd "ViewModel.kt" app/src/main/java/com/cherrish/android/presentation -x grep -l "Repository" {}Repository: TEAM-Cherrish/Cherrish-Android
Length of output: 630
구현체 직접 주입은 확립된 DI 패턴을 벗어납니다.
이 파일은 다른 모든 ViewModel과 달리 HomeRepository 인터페이스 대신 HomeRepositoryImpl 구현체에 직접 의존합니다. 이는 테스트/모킹을 어렵게 하고 코드베이스의 일관된 DI 패턴을 해칩니다. HomeRepository 인터페이스에 의존하도록 수정하세요 (RepositoryModule에 이미 바인딩되어 있음).
♻️ 권장 수정안
-import com.cherrish.android.data.repositoryimpl.HomeRepositoryImpl
+import com.cherrish.android.data.repository.HomeRepository
...
-class HomeViewModel `@Inject` constructor(
- private val homeRepositoryImpl: HomeRepositoryImpl
+class HomeViewModel `@Inject` constructor(
+ private val homeRepository: HomeRepository
...
- homeRepositoryImpl.getMainDashboard().onSuccess { response ->
+ homeRepository.getMainDashboard().onSuccess { response ->📝 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.
| import com.cherrish.android.data.repositoryimpl.HomeRepositoryImpl | |
| import com.cherrish.android.data.repository.HomeRepository |
🤖 Prompt for AI Agents
In `@app/src/main/java/com/cherrish/android/presentation/home/HomeViewModel.kt` at
line 8, HomeViewModel currently depends on the concrete HomeRepositoryImpl
instead of the HomeRepository interface; update the class to depend on
HomeRepository (not HomeRepositoryImpl) so DI/RepositoryModule bindings and
tests/mocks work correctly: change any imports/constructor parameters/properties
referencing HomeRepositoryImpl to HomeRepository and ensure the DI-injected type
used in HomeViewModel is the interface (HomeRepository) everywhere.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/cherrish/android/presentation/challenge/routine/ChallengeRoutineViewModel.kt (1)
56-63:onRoutineClick함수 시그니처가 호출부 기대 타입과 불일치합니다.
ChallengeRoutineViewModel.onRoutineClick(id: Int)는(Int) -> Unit이지만,ChallengeMissionTodoSection의 콜백 파라미터는onRoutineClick: (Long) -> Unit을 기대합니다. 이로 인해 ViewModel 함수를 직접 전달할 수 없으며, 타입 변환 시 데이터 손실(Long → Int) 위험이 있습니다. 호출부의 콜백 타입(Long)과 ViewModel 함수 매개변수(Int) 타입을 일치시키거나, 적절한 변환 로직을 추가하세요.
♻️ Duplicate comments (2)
app/src/main/java/com/cherrish/android/presentation/challenge/navigation/ChallengeNavigator.kt (1)
50-55: 파라미터 네이밍 일관성 맞춰 주세요.
navigationChallengeMission만 동사형 규칙에서 벗어나 있어 가독성이 떨어집니다.navigateToChallengeMission으로 맞추는 게 좋습니다.✏️ 제안 수정
fun NavGraphBuilder.challengeNavGraph( paddingValues: PaddingValues, navigateToChallengeRoutine: () -> Unit, - navigationChallengeMission: () -> Unit, + navigateToChallengeMission: () -> Unit, navigateUp: () -> Unit ) { ... - navigateToMission = navigationChallengeMission + navigateToMission = navigateToChallengeMissionAlso applies to: 68-68
app/src/main/java/com/cherrish/android/presentation/challenge/routine/ChallengeRoutineViewModel.kt (1)
38-52: API 실패 시 상태 전환 누락(중복 지적).
실패 시_uiState가Loading에 머무를 수 있어 사용자 피드백이 없습니다. 이전 리뷰에서 동일 이슈가 이미 지적되었습니다.
nhyeonii
left a comment
nhyeonii
left a comment
There was a problem hiding this comment.
오호오호 ~~ 리뷰 한번만 확인해주세요 !!
| val currentDay: Int, | ||
|
|
||
| @SerialName("progressPercentage") | ||
| val progressPercentage: Double, // 42.8 |
There was a problem hiding this comment.
| val progressPercentage: Double, // 42.8 | |
| val progressPercentage: Double, |
주석 다 지워주시오 ~
| challengeDataSource | ||
| .getHomecareRoutineData() | ||
| .data!! | ||
| .map { it.toModel() } |
There was a problem hiding this comment.
| challengeDataSource | |
| .getHomecareRoutineData() | |
| .data!! | |
| .map { it.toModel() } | |
| challengeDataSource.getHomecareRoutineData().data!!.map { it.toModel() } |
요정도는 한줄에 붙여서 작성해주면 좋을 거 가트 ~~
There was a problem hiding this comment.
고마워요 !!! 고쳤습니당 ~!!!
|
|
||
| @Serializable | ||
| data object ChallengeMission : Route | ||
| fun NavController.navigateToChallengeStart( |
There was a problem hiding this comment.
악 아직도 요런 실수를 하다니 !!!!!!!!!!!!
| challengeRepository | ||
| .getChallengeRoutineData() | ||
| .onSuccess { responseModels -> | ||
| _uiState.value = UiState.Success( |
There was a problem hiding this comment.
value 직접 접근 말고 update 사용해주세용
There was a problem hiding this comment.
넵 !!!! 요거 ,, 저 이거 완전 기억해요 ,,
| id = id, | ||
| routine = description |
There was a problem hiding this comment.
| id = id, | |
| routine = description | |
| id = this.id, | |
| routine = this.description |
this 붙여주깅
There was a problem hiding this comment.
넵 !! 항상 친절한 설명감사합니다아 ~!!
| @HiltViewModel | ||
| class HomeViewModel @Inject constructor( | ||
| private val homeRepository: HomeRepository | ||
| private val homeRepositoryImpl: HomeRepositoryImpl |
There was a problem hiding this comment.
요것도 일단 수정해둘게용 !!!
| _uiState.update { UiState.Loading } | ||
|
|
||
| homeRepository.getMainDashboard().onSuccess { response -> | ||
| homeRepositoryImpl.getMainDashboard().onSuccess { response -> |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/cherrish/android/presentation/challenge/start/ChallengeStartScreen.kt (1)
45-50: Scaffold paddingValues를 먼저 적용하세요현재는 수평 패딩이 먼저 적용돼 시스템 인셋이 의도대로 반영되지 않을 수 있습니다. 루트에
paddingValues를 우선 적용한 뒤 추가 패딩을 주는 패턴으로 정리해주세요.♻️ 제안 수정
Column( modifier = modifier .fillMaxSize() - .padding(horizontal = 17.dp) - .padding(paddingValues) + .padding(paddingValues) + .padding(horizontal = 17.dp)Based on learnings, apply
paddingValuesbefore any additional padding.
♻️ Duplicate comments (2)
app/src/main/java/com/cherrish/android/presentation/challenge/routine/ChallengeRoutineViewModel.kt (1)
38-51: API 실패 시 에러 처리가 누락되었습니다.
getChallengeRoutineData()호출에서onSuccess만 처리하고onFailure가 없습니다. API 호출 실패 시_uiState가UiState.Loading상태에 머물러 사용자에게 피드백이 제공되지 않습니다.🐛 제안하는 수정사항
private fun loadRoutines() { viewModelScope.launch { challengeRepository.getChallengeRoutineData() .onSuccess { responseModels -> _uiState.update { UiState.Success( ChallengeRoutineUiState( routines = responseModels.map { it.toUiModel() }.toPersistentList() ) ) } } + .onFailure { throwable -> + _uiState.update { UiState.Failure(throwable) } + } } }app/src/main/java/com/cherrish/android/presentation/challenge/start/ChallengeStartScreen.kt (1)
27-36: 사용하지 않는 ViewModel 주입 정리 여부 확인
ChallengeStartRoute에서viewModel이 사용되지 않아 불필요한 인스턴스 생성이 발생할 수 있습니다. 필요 없다면 파라미터/주입 제거 또는 side-effect 수집 로직 추가를 검토해주세요.
🧹 Nitpick comments (3)
app/src/main/java/com/cherrish/android/presentation/challenge/routine/ChallengeRoutineScreen.kt (2)
84-90:paddingValues파라미터가 사용되지 않고 있습니다.
ChallengeRoutineScreen이paddingValues를 파라미터로 받지만 실제로 root container에 적용하지 않습니다. 학습된 패턴에 따르면, Scaffold에서 전달받은paddingValues는 root container의 modifier에 먼저 적용해야 시스템 바와의 레이아웃 겹침을 방지할 수 있습니다.현재
systemBarsPadding()과navigationBarsPadding()을 함께 사용하면 중복 패딩이 발생할 수 있습니다.♻️ 제안하는 수정사항
Column( modifier = modifier .fillMaxSize() .background(CherrishTheme.colors.gray0) - .navigationBarsPadding() - .systemBarsPadding() + .padding(paddingValues) ) {또는 Scaffold 없이 사용하는 경우라면, 사용하지 않는
paddingValues파라미터를 제거하는 것을 고려해주세요.Based on learnings,
paddingValues는 root container에 먼저 적용되어야 합니다.
50-56: Loading 및 Failure 상태에 대한 UI 처리가 누락되었습니다.현재
UiState.Loading과UiState.Failure상태에서 빈 블록으로 처리되어 사용자에게 아무런 피드백이 제공되지 않습니다. 로딩 인디케이터나 에러 메시지를 표시하는 것을 권장합니다.♻️ 제안 예시
is UiState.Loading -> { // 로딩 인디케이터 표시 Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { CircularProgressIndicator() } } is UiState.Failure -> { // 에러 메시지 또는 재시도 버튼 표시 }app/src/main/java/com/cherrish/android/presentation/challenge/routine/ChallengeRoutineViewModel.kt (1)
71-74: 사용되지 않는 빈 함수들을 제거해주세요.
onBackClick()과onCloseClick()은 빈 구현이며,ChallengeRoutineRoute에서 직접 파라미터로 전달받은 콜백을 사용하고 있어 ViewModel의 이 함수들은 호출되지 않습니다. 불필요한 코드는 제거하는 것이 좋습니다.♻️ 제안하는 수정사항
- fun onBackClick() { - } - - fun onCloseClick() {}
4 participants
Related issue 🛠
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
N/A
To Reviewers 📢
항상 고맙고 감사합니다
Summary by CodeRabbit
새로운 기능
개선 사항
제거
✏️ Tip: You can customize this high-level summary in your review settings.