Conversation
📝 WalkthroughWalkthrough마이페이지 API(/api/v1/users/mypage) 연동을 추가하고 DTO·매퍼·저장소·뷰모델을 연결했으며, 히스토리 목록 네비게이션 타입 및 화면 연결(HistoryMode/HistorySummaryType/HistoryList)이 변경·연동되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as MyPageScreen
participant VM as MyPageViewModel
participant Repo as UserRepository
participant Remote as UserRemoteDataSource
participant Service as UserService
UI->>VM: loadUserMyPage()
VM->>Repo: getUserMyPage()
Repo->>Remote: getUserMyPage()
Remote->>Service: GET /api/v1/users/mypage
Service-->>Remote: BaseResponse<MyPageResponseDto>
Remote-->>Repo: BaseResponse<MyPageResponseDto>
Repo->>Repo: dto.toDomain() (MyPageMapper)
Repo-->>VM: Result<UserMyPage>
VM-->>UI: ApiState.Success / ApiState.Failure
UI->>VM: OnHistoryClick(mode,type)
VM-->>UI: NavigateToHistoryList(mode,type)
UI->>UI: navController.navigateToHistoryList(mode,type)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
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)
app/src/main/java/com/poti/android/presentation/history/navigation/HistoryNavigation.kt (1)
67-81: 라우트 인자가 ViewModel에 전달되지 않음
HistoryRoute.HistoryList에mode와type인자가 담기지만,HistoryListViewModel에는SavedStateHandle이 없어서 이들 인자를 전혀 사용하지 않고 있어요.mode는 항상 기본값인HistoryMode.RECRUIT으로 초기화되므로,navigateToHistoryList(HistoryMode.PARTICIPATION, type)로 네비게이션해도 UI에 반영되지 않습니다.ViewModel 생성자에
SavedStateHandle을 주입받고,init블록에서 route 인자를 읽어 상태를 초기화하거나, 대안으로 composable에서 route 인자를 추출해HistoryListRoute컴포저블에 명시적으로 전달해야 합니다.
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/poti/android/presentation/history/navigation/HistoryNavigation.kt`:
- Around line 23-26: The HistoryRoute.HistoryList data class is `@Serializable`
but its enum properties are not; add `@Serializable` to the enum declarations
HistoryMode and HistorySummaryType and import kotlinx.serialization.Serializable
so both enums become serializable and the navigation argument serialization will
succeed. Locate the enum classes named HistoryMode and HistorySummaryType and
annotate each with `@Serializable` (and ensure the kotlinx.serialization import is
present).
In
`@app/src/main/java/com/poti/android/presentation/user/mypage/MyPageViewModel.kt`:
- Around line 37-48: The loadUserMyPage function can leave the UI in a loading
state if an exception escapes the Result because launchScope's default onError
is empty; update the launchScope call for loadUserMyPage to provide an onError
handler that calls updateState and sets userMyPageLoadState to
ApiState.Failure(with the throwable.message or a fallback)—mirror the logic used
in the existing onFailure block so both Result failures (onFailure) and uncaught
exceptions (onError) consistently update state; reference loadUserMyPage,
launchScope, userRepository.getUserMyPage, updateState, and userMyPageLoadState
when applying the change.
🧹 Nitpick comments (1)
app/src/main/java/com/poti/android/data/mapper/user/MyPageMapper.kt (1)
11-23: ratingAvg 포맷팅 책임을 프레젠테이션으로 이동하는 걸 고려해주세요.데이터 레이어에서 문자열 포맷을 수행하면 UI 요구사항(로케일/표현)이 도메인으로 전파될 수 있어요. 원시 값 유지 후 UI에서 표현하는 구조가 더 유연합니다.
As per coding guidelines, 레이어 책임 분리를 고려했습니다.
| data class HistoryList( | ||
| val mode: HistoryMode, | ||
| val type: HistorySummaryType, | ||
| ) : HistoryRoute |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.kt" | xargs grep -l "enum class HistoryMode\|sealed class HistoryMode\|data class HistoryMode" | head -5Repository: team-poti/POTI-ANDROID
Length of output: 149
🏁 Script executed:
find . -type f -name "*.kt" | xargs grep -l "enum class HistorySummaryType\|sealed class HistorySummaryType\|data class HistorySummaryType" | head -5Repository: team-poti/POTI-ANDROID
Length of output: 152
🏁 Script executed:
rg -nA5 "^(sealed class|enum class|data class)\s+HistoryMode" --type=ktRepository: team-poti/POTI-ANDROID
Length of output: 92
🏁 Script executed:
rg -nA5 "^(sealed class|enum class|data class)\s+HistorySummaryType" --type=ktRepository: team-poti/POTI-ANDROID
Length of output: 92
🏁 Script executed:
rg "@Serializable|enum class HistoryMode|sealed class HistoryMode" -B2 -A5 ./app/src/main/java/com/poti/android/presentation/history/Repository: team-poti/POTI-ANDROID
Length of output: 3454
🏁 Script executed:
rg "@Serializable|enum class HistorySummaryType|sealed class HistorySummaryType" -B2 -A5 ./app/src/main/java/com/poti/android/Repository: team-poti/POTI-ANDROID
Length of output: 26047
🏁 Script executed:
cat ./app/src/main/java/com/poti/android/presentation/history/navigation/HistoryNavigation.ktRepository: team-poti/POTI-ANDROID
Length of output: 3104
HistoryMode과 HistorySummaryType enum에 @Serializable 추가 필수
HistoryRoute.HistoryList가 @Serializable로 표시되어 있으므로, 포함된 모든 프로퍼티 타입도 직렬화 가능해야 합니다. 현재 HistoryMode와 HistorySummaryType enum 모두 @Serializable 어노테이션이 없어서 네비게이션 인자 전달 시 직렬화 실패가 발생합니다. 두 enum 정의에 @Serializable을 추가하세요.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/history/navigation/HistoryNavigation.kt`
around lines 23 - 26, The HistoryRoute.HistoryList data class is `@Serializable`
but its enum properties are not; add `@Serializable` to the enum declarations
HistoryMode and HistorySummaryType and import kotlinx.serialization.Serializable
so both enums become serializable and the navigation argument serialization will
succeed. Locate the enum classes named HistoryMode and HistorySummaryType and
annotate each with `@Serializable` (and ensure the kotlinx.serialization import is
present).
| private fun loadUserMyPage() = launchScope { | ||
| userRepository.getUserMyPage() | ||
| .onSuccess { userMyPage -> | ||
| updateState { | ||
| copy(userMyPageLoadState = ApiState.Success(userMyPage)) | ||
| } | ||
| } | ||
| .onFailure { throwable -> | ||
| updateState { | ||
| copy( | ||
| userMyPageLoadState = ApiState.Failure(throwable.message ?: "Failed"), | ||
| ) |
There was a problem hiding this comment.
예외가 Result 밖으로 발생할 때 실패 상태가 누락될 수 있습니다.
launchScope의 기본 onError가 비어 있어, Result로 감싸지 못한 예외는 로딩 상태로 남을 수 있어요. 실패 상태 업데이트를 onError에 연결하는 편이 안전합니다.
✅ 제안 수정안
-private fun loadUserMyPage() = launchScope {
+private fun loadUserMyPage() = launchScope(
+ onError = { throwable ->
+ updateState {
+ copy(userMyPageLoadState = ApiState.Failure(throwable.message ?: "Failed"))
+ }
+ },
+) {
userRepository.getUserMyPage()
.onSuccess { userMyPage ->
updateState {
copy(userMyPageLoadState = ApiState.Success(userMyPage))
}
}
.onFailure { throwable ->
updateState {
copy(
userMyPageLoadState = ApiState.Failure(throwable.message ?: "Failed"),
)
}
}
}🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/user/mypage/MyPageViewModel.kt`
around lines 37 - 48, The loadUserMyPage function can leave the UI in a loading
state if an exception escapes the Result because launchScope's default onError
is empty; update the launchScope call for loadUserMyPage to provide an onError
handler that calls updateState and sets userMyPageLoadState to
ApiState.Failure(with the throwable.message or a fallback)—mirror the logic used
in the existing onFailure block so both Result failures (onFailure) and uncaught
exceptions (onError) consistently update state; reference loadUserMyPage,
launchScope, userRepository.getUserMyPage, updateState, and userMyPageLoadState
when applying the change.
| data class HistoryList( | ||
| val mode: HistoryMode, | ||
| val type: HistorySummaryType, | ||
| ) : HistoryRoute |
There was a problem hiding this comment.
p1: 코드레빗 말처럼 serializable 필요해보여용
doyeon0307
left a comment
There was a problem hiding this comment.
파라미터를 nullable로 해서 기본값 null로 하면 냅바용 따로 안 파도 될 거 같은디!! 고생띠니
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/poti/android/presentation/history/navigation/HistoryNavigation.kt (1)
67-82:HistoryRoute.HistoryList파라미터가 네비게이션에서 전달되지만 사용되지 않고 있습니다.
navigateToHistoryList()함수로mode와type파라미터를 전달하고 있으나,HistoryListRoute컴포저블이 이 파라미터를 받지 않고 있으며HistoryListViewModel도SavedStateHandle을 통해 파라미터를 읽어오지 않고 있습니다. 결과적으로 네비게이션 파라미터가 완전히 무시되고 ViewModel은 항상 기본값(RECRUIT, ALL)으로 초기화됩니다.네비게이션 파라미터를 실제로 사용하려면 다음 중 하나를 선택해야 합니다:
옵션 1: HistoryListRoute가 파라미터를 받아 ViewModel 초기화 시 전달
composable<HistoryRoute.HistoryList> { backStackEntry -> val route = backStackEntry.toRoute<HistoryRoute.HistoryList>() HistoryListRoute( initialMode = route.mode ?: HistoryMode.RECRUIT, initialType = route.type ?: HistorySummaryType.ALL, // ... 다른 파라미터 ) }옵션 2: HistoryListViewModel에서
SavedStateHandle로 파라미터 읽기class HistoryListViewModel `@Inject` constructor( private val savedStateHandle: SavedStateHandle, ) : BaseViewModel<...>() { init { val route = savedStateHandle.toRoute<HistoryRoute.HistoryList>() // mode, type 초기화 } }불필요한 파라미터라면
HistoryRoute.HistoryList에서 제거하세요.
♻️ Duplicate comments (1)
app/src/main/java/com/poti/android/presentation/history/navigation/HistoryNavigation.kt (1)
22-26: 이전 리뷰 피드백 반영 확인이 필요합니다.이전 리뷰에서
HistoryMode와HistorySummaryTypeenum에@Serializable어노테이션이 필요하다고 지적되었습니다. AI 요약에 따르면 두 enum 모두 이제@Serializable이 추가된 것으로 보이나,HistorySummaryType의 경우 이 파일에서 import하여 사용하고 있으므로 해당 파일에서 어노테이션이 정상적으로 추가되었는지 확인해 주세요.#!/bin/bash # Description: HistorySummaryType에 `@Serializable` 어노테이션이 있는지 확인 rg -nB2 -A5 "enum class HistorySummaryType" --type=kt
# Conflicts: # app/src/main/java/com/poti/android/presentation/history/list/HistoryListScreen.kt # app/src/main/java/com/poti/android/presentation/history/navigation/HistoryNavigation.kt
Related issue 🛠️
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.