Conversation
📝 WalkthroughWalkthroughHistory 목록 화면을 새 UI 모델/뷰모델 기반 아키텍처로 재구성했습니다. 공개 진입점이 Changes
Sequence Diagram(s)sequenceDiagram
participant UI as HistoryListRoute
participant VM as HistoryListViewModel
participant State as HistoryListUiState
participant Nav as NavController
UI->>VM: ViewModel 획득(hiltViewModel)
VM->>VM: init -> loadUserHistoryList()
VM->>State: historyListLoadState 변경 (Loading -> Success)
State-->>UI: uiState 수집(collectAsState)
UI->>VM: processIntent(OnTabSelected / OnSwitchModeClick / OnCardClick / OnBackClick)
VM->>VM: 상태 변경(selectedTab/mode) 및 데이터 재로드
VM->>UI: 사이드이펙트 emit(NavigateToDetail(id) / NavigateBack)
UI->>Nav: onNavigateToRecruiterDetail() / onNavigateToParticipantDetail() / onPopBackStack()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/poti/android/presentation/history/list/HistoryListScreen.kt`:
- Around line 56-62: The NavigateToDetail effect handler is not passing the
selected item's id so detail screens may mismatch; update the
HistoryListUiEffect.NavigateToDetail handling to forward effect.id to the
navigation callbacks (or adjust the callbacks to accept an id) — specifically
modify the branch that checks uiState.mode to call
onNavigateToRecruiterDetail(effect.id) when mode == HistoryMode.RECRUIT and
onNavigateToParticipantDetail(effect.id) otherwise, ensuring the
NavigateToDetail effect's id is included in the route or callback signature used
by the detail screens.
🧹 Nitpick comments (2)
app/src/main/java/com/poti/android/presentation/history/list/HistoryListViewModel.kt (1)
54-94: 빠른 탭/모드 전환 시 이전 로드 결과가 최신 상태를 덮을 수 있습니다.
Line 54-94:loadHistory()가 중복 실행돼 결과 순서가 뒤바뀔 여지가 있습니다. API 연동 시 특히 문제가 됩니다.🔧 권장 개선안 (이전 작업 취소)
+import kotlinx.coroutines.Job import kotlinx.coroutines.launch @@ class HistoryListViewModel `@Inject` constructor() : BaseViewModel<HistoryListUiState, HistoryListUiIntent, HistoryListUiEffect>( @@ ) { + private var loadJob: Job? = null @@ private fun loadHistory() { - viewModelScope.launch { + loadJob?.cancel() + loadJob = viewModelScope.launch { updateState { copy(isLoading = true) } @@ - val dummyItems = if (uiState.value.selectedTab == PotiHeaderTabType.ONGOING) { + val selectedTabSnapshot = uiState.value.selectedTab + val dummyItems = if (selectedTabSnapshot == PotiHeaderTabType.ONGOING) { listOf( HistoryItem(app/src/main/java/com/poti/android/presentation/history/list/model/Contracts.kt (1)
11-27: Compose 안정성 향상을 위해@Immutable적용을 권장드립니다.
Line 11-27:HistoryListUiState,HistoryItem이 Compose 상태로 사용되므로 불변성을 명시하면 불필요한 리컴포지션을 줄일 수 있습니다. 리스트는 항상 새 인스턴스로 교체되는 전제가 필요합니다.♻️ 제안 변경
+import androidx.compose.runtime.Immutable @@ -data class HistoryListUiState( +@Immutable +data class HistoryListUiState( @@ -data class HistoryItem( +@Immutable +data class HistoryItem(코딩 가이드라인에 따라 제안드립니다.
Comment on lines
+56
to
+62
| is HistoryListUiEffect.NavigateToDetail -> { | ||
| // TODO: [예림] effect.id 전달 | ||
| if (uiState.mode == HistoryMode.RECRUIT) { | ||
| onNavigateToRecruiterDetail() | ||
| } else { | ||
| onNavigateToParticipantDetail() | ||
| } |
There was a problem hiding this comment.
상세 이동에 id가 전달되지 않아 항목 매칭이 깨질 수 있습니다.
Line 56-62: NavigateToDetail의 id가 네비게이션으로 사용되지 않습니다. 클릭한 카드와 상세 화면이 불일치할 수 있으니, 라우트에 id 파라미터를 추가해 전달하거나 id 자체를 제거해 일관성을 맞춰주세요.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/history/list/HistoryListScreen.kt`
around lines 56 - 62, The NavigateToDetail effect handler is not passing the
selected item's id so detail screens may mismatch; update the
HistoryListUiEffect.NavigateToDetail handling to forward effect.id to the
navigation callbacks (or adjust the callbacks to accept an id) — specifically
modify the branch that checks uiState.mode to call
onNavigateToRecruiterDetail(effect.id) when mode == HistoryMode.RECRUIT and
onNavigateToParticipantDetail(effect.id) otherwise, ensuring the
NavigateToDetail effect's id is included in the route or callback signature used
by the detail screens.
doyeon0307
reviewed
Jan 18, 2026
Comment on lines
+89
to
+94
| val titleRes = when (uiState.mode) { | ||
| HistoryMode.RECRUIT -> R.string.user_history_recruit | ||
| HistoryMode.PARTICIPATION -> R.string.user_history_participate | ||
| } | ||
|
|
||
| val emptyTextRes = when (uiState.mode) { |
Contributor
There was a problem hiding this comment.
p2: titleRes, emptyTextRes 변수는 Contracts.kt 내부에서 정의해주심 좋습니다!
이유는 스크린은 처리 완료된 텍스트를 보여주기만하고
모델/뷰모델에서 데이터 가공을 완료해 뷰에 넘겨주는 편이 역할 분리가 잘 되어서 구조적으로 좋아보입니다
data class HistoryListUiState(
val isLoading: Boolean = false,
val mode: HistoryMode = HistoryMode.RECRUIT,
val selectedTab: PotiHeaderTabType = PotiHeaderTabType.ONGOING,
val ongoingCount: Int = 0,
val endedCount: Int = 0,
val items: List<HistoryItem> = emptyList(),
) : UiState {
val titleRes = when (mode) {
HistoryMode.RECRUIT -> R.string.user_history_recruit
HistoryMode.PARTICIPATION -> R.string.user_history_participate
}
val emptyTextRes = when (mode) {
HistoryMode.RECRUIT -> {
when (uiState.selectedTab) {
PotiHeaderTabType.ONGOING -> R.string.history_empty_recruit_ongoing
PotiHeaderTabType.ENDED -> R.string.history_empty_recruit_ended
}
}
HistoryMode.PARTICIPATION -> {
when (uiState.selectedTab) {
PotiHeaderTabType.ONGOING -> R.string.history_empty_participation_ongoing
PotiHeaderTabType.ENDED -> R.string.history_empty_participation_ended
}
}
}
| sealed interface HistoryListUiEffect : UiEffect { | ||
| data object NavigateBack : HistoryListUiEffect | ||
|
|
||
| data class SwitchMode(val isRecruitMode: Boolean) : HistoryListUiEffect |
Contributor
There was a problem hiding this comment.
p1: SwitchMode는 UiEffect로 만들지 않아도 될 거 같아요
- OnSwitchModeClick으로 uiState의 mode를 변경
-> 스크린 타이틀에 반영 됨
-> historyApi 호출 시에는 뷰모델 내에서 mode값 활용해 적절한 history get
현재 UiEffect로는 네비게이션 정도만 들어간다고 보심됩니다!
Comment on lines
+24
to
+38
| val newMode = if (uiState.value.mode == HistoryMode.RECRUIT) { | ||
| HistoryMode.PARTICIPATION | ||
| } else { | ||
| HistoryMode.RECRUIT | ||
| } | ||
| updateState { | ||
| copy( | ||
| mode = newMode, | ||
| selectedTab = PotiHeaderTabType.ONGOING, | ||
| ) | ||
| } | ||
| sendEffect( | ||
| HistoryListUiEffect.SwitchMode(newMode == HistoryMode.RECRUIT), | ||
| ) | ||
| loadHistory() |
Contributor
There was a problem hiding this comment.
p1: processIntent가 좀 길어져서
이 내용 hadleSwitchMode private 메소드 만들어 분리하면
코드 간결해지고 가독성 좋아질 것 같아용
cmj7271
approved these changes
Jan 19, 2026
|
|
||
| @Preview(showBackground = true) | ||
| @Composable | ||
| private fun HistoryListScreenPreview_Ongoing() { |
jyvnee
reviewed
Jan 19, 2026
Comment on lines
+51
to
+52
| private fun loadHistory() { | ||
| viewModelScope.launch { |
Contributor
There was a problem hiding this comment.
P2: 탭 빠르게 바꿀 경우 이전 작업이 있다면 취소하는게 낫지 않을까욤
Suggested change
| private fun loadHistory() { | |
| viewModelScope.launch { | |
| fetchJob?.cancel() | |
| fetchJob = viewModelScope.launch { | |
| // 리스트 로드 | |
| } |
근데 이거 머.. 나중에 서버 연결할 때 해도 됨
| val selectedTab: PotiHeaderTabType = PotiHeaderTabType.ONGOING, | ||
| val ongoingCount: Int = 0, | ||
| val endedCount: Int = 0, | ||
| val items: List<HistoryItem> = emptyList(), |
Contributor
There was a problem hiding this comment.
P1: ImmutableList 사용해주면 조을 듯?? 그리고 ApiState로 감싸주면 isLoading 필요 없을 것 같아요
# Conflicts: # app/src/main/res/values/strings.xml
|
✅ Build passed. Run: https://github.com/team-poti/POTI-ANDROID/actions/runs/21163678206 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/poti/android/domain/model/history/HistoryList.kt`:
- Around line 3-18: Domain models HistoryListContent and HistoryItem currently
depend on presentation types ParticipantStateLabelStage and
ParticipantStateLabelStatus; introduce domain enums (e.g., ParticipantStage,
ParticipantStatus) in the domain layer and change HistoryListContent/HistoryItem
to use those domain enums instead of the presentation types, remove the
presentation imports, and update any callers to map domain enums to
Presentation's ParticipantStateLabelStage/ParticipantStateLabelStatus in the
Presentation layer (add mapping functions or extension functions that convert
ParticipantStage -> ParticipantStateLabelStage and ParticipantStatus ->
ParticipantStateLabelStatus) so the Domain no longer depends on Presentation.
In `@app/src/main/res/values/strings.xml`:
- Around line 123-126: The new empty-state strings use "진행 중" but the app
currently uses "진행중" elsewhere (e.g., user_history_ongoing); make the wording
consistent by updating the resources history_empty_recruit_ongoing and
history_empty_participation_ongoing to match the established form ("진행중" or "진행
중") used across the app, and ensure history_empty_recruit_ended and
history_empty_participation_ended follow the same spacing convention; update the
string values for the resource names mentioned to the chosen consistent form.
♻️ Duplicate comments (1)
app/src/main/java/com/poti/android/presentation/history/list/HistoryListScreen.kt (1)
43-61: 상세 이동에id가 전달되지 않아 항목 매칭이 깨질 수 있어요.
NavigateToDetail의effect.id가 네비게이션 콜백에 반영되지 않아 상세 화면이 잘못 열릴 수 있습니다. 콜백 시그니처를Long파라미터를 받도록 변경하고effect.id를 전달해주세요.🐛 제안 수정
-fun HistoryListRoute( - onPopBackStack: () -> Unit, - onNavigateToRecruiterDetail: () -> Unit, - onNavigateToParticipantDetail: () -> Unit, +fun HistoryListRoute( + onPopBackStack: () -> Unit, + onNavigateToRecruiterDetail: (Long) -> Unit, + onNavigateToParticipantDetail: (Long) -> Unit, modifier: Modifier = Modifier, viewModel: HistoryListViewModel = hiltViewModel(), ) { @@ - is HistoryListUiEffect.NavigateToDetail -> { - // TODO: [예림] effect.id 전달 - if (uiState.mode == HistoryMode.RECRUIT) { - onNavigateToRecruiterDetail() - } else { - onNavigateToParticipantDetail() - } - } + is HistoryListUiEffect.NavigateToDetail -> { + if (uiState.mode == HistoryMode.RECRUIT) { + onNavigateToRecruiterDetail(effect.id) + } else { + onNavigateToParticipantDetail(effect.id) + } + } } }
🧹 Nitpick comments (3)
app/src/main/java/com/poti/android/presentation/history/list/HistoryListViewModel.kt (2)
70-109:createDummyContent는 외부에 노출할 필요가 없어 보입니다.테스트나 외부 호출이 없다면
private로 좁혀 API surface를 줄이는 편이 안전합니다.♻️ 제안 수정
- fun createDummyContent(): HistoryListContent { + private fun createDummyContent(): HistoryListContent {
111-114: TODO(API 분기) 항목은 추적 이슈로 분리하는 것을 권장합니다.나중에 잊히지 않도록 이슈로 옮기거나 TODO에 목적/기한을 보강해 주세요. 필요하시면 이 작업을 위한 이슈 템플릿/초안을 만들어 드릴게요.
app/src/main/java/com/poti/android/presentation/history/list/HistoryListScreen.kt (1)
117-124: 간격 값이 하드코딩되어 있어 디자인 시스템 토큰 사용을 권장합니다.
16.dp,12.dp,10.dp같은 값은Dimens/Theme의 spacing 토큰으로 치환하면 일관성과 유지보수성이 좋아집니다. 코딩 가이드라인에 따라, ...
Comment on lines
+3
to
+18
| import com.poti.android.presentation.history.component.ParticipantStateLabelStage | ||
| import com.poti.android.presentation.history.component.ParticipantStateLabelStatus | ||
|
|
||
| data class HistoryListContent( | ||
| val ongoingCount: Int, | ||
| val endedCount: Int, | ||
| val items: List<HistoryItem>, | ||
| ) | ||
|
|
||
| data class HistoryItem( | ||
| val id: Long, | ||
| val imageUrl: String?, | ||
| val artist: String, | ||
| val title: String, | ||
| val stage: ParticipantStateLabelStage, | ||
| val status: ParticipantStateLabelStatus, |
There was a problem hiding this comment.
도메인 레이어가 프레젠테이션 타입에 의존하고 있습니다.
HistoryListContent/HistoryItem이 com.poti.android.presentation... 타입을 직접 참조하고 있어 Domain → Presentation 의존이 생깁니다. 도메인에는 순수한 상태/enum을 두고, UI 타입으로의 매핑은 Presentation 레이어에서 처리하는 방향으로 분리해주세요. 코딩 가이드라인에 따라, ...
🤖 Prompt for AI Agents
In `@app/src/main/java/com/poti/android/domain/model/history/HistoryList.kt`
around lines 3 - 18, Domain models HistoryListContent and HistoryItem currently
depend on presentation types ParticipantStateLabelStage and
ParticipantStateLabelStatus; introduce domain enums (e.g., ParticipantStage,
ParticipantStatus) in the domain layer and change HistoryListContent/HistoryItem
to use those domain enums instead of the presentation types, remove the
presentation imports, and update any callers to map domain enums to
Presentation's ParticipantStateLabelStage/ParticipantStateLabelStatus in the
Presentation layer (add mapping functions or extension functions that convert
ParticipantStage -> ParticipantStateLabelStage and ParticipantStatus ->
ParticipantStateLabelStatus) so the Domain no longer depends on Presentation.
Comment on lines
+123
to
+126
| <string name="history_empty_recruit_ongoing">진행 중인 모집 내역이 없어요</string> | ||
| <string name="history_empty_recruit_ended">지난 모집 내역이 없어요</string> | ||
| <string name="history_empty_participation_ongoing">진행 중인 참여 내역이 없어요</string> | ||
| <string name="history_empty_participation_ended">지난 참여 내역이 없어요</string> |
There was a problem hiding this comment.
진행 상태 표기 띄어쓰기 일관성 확인해주세요.
기존 문자열은 "진행중"을 사용하고 있는데(예: user_history_ongoing), 신규 빈 상태 문구는 "진행 중"으로 표기가 달라요. UI에서 같은 상태가 서로 다르게 보여 혼란을 줄 수 있으니 하나로 통일하는 편이 좋겠습니다.
🤖 Prompt for AI Agents
In `@app/src/main/res/values/strings.xml` around lines 123 - 126, The new
empty-state strings use "진행 중" but the app currently uses "진행중" elsewhere (e.g.,
user_history_ongoing); make the wording consistent by updating the resources
history_empty_recruit_ongoing and history_empty_participation_ongoing to match
the established form ("진행중" or "진행 중") used across the app, and ensure
history_empty_recruit_ended and history_empty_participation_ended follow the
same spacing convention; update the string values for the resource names
mentioned to the chosen consistent form.
jyvnee
reviewed
Jan 20, 2026
Comment on lines
+17
to
+18
| val stage: ParticipantStateLabelStage, | ||
| val status: ParticipantStateLabelStatus, |
Contributor
There was a problem hiding this comment.
P1: 도메인 레이어에서는 프레젠테이션 레이어를 의존하면 안됩니다!!!! 타입 설정이 필요하면 도메인 레이어에 있는 타입 패키지 안에 넣어주세용!!
Comment on lines
+27
to
+39
| val newMode = if (uiState.value.mode == HistoryMode.RECRUIT) { | ||
| HistoryMode.PARTICIPATION | ||
| } else { | ||
| HistoryMode.RECRUIT | ||
| } | ||
| updateState { | ||
| copy( | ||
| mode = newMode, | ||
| selectedTab = PotiHeaderTabType.ONGOING, | ||
| ) | ||
| } | ||
| loadUserHistoryList() | ||
| } |
Contributor
There was a problem hiding this comment.
P3: 처리해야할 내용이 많으면 함수로 따로 빼는게 낫지 않을까용
This was referenced Jan 20, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue 🛠️
Work Description ✏️
Screenshot 📸
Screen_recording_20260118_153932.mp4
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
새로운 기능
개선 사항
추가
✏️ Tip: You can customize this high-level summary in your review settings.