Conversation
# Conflicts: # app/src/main/java/com/poti/android/presentation/party/home/HomeScreen.kt # app/src/main/java/com/poti/android/presentation/party/home/HomeViewModel.kt # app/src/main/java/com/poti/android/presentation/party/home/model/Contracts.kt
# Conflicts: # app/src/main/java/com/poti/android/presentation/party/create/PartyArtistSelectScreen.kt
📝 WalkthroughWalkthrough상품(Goods) → Product 명칭 전환 및 관련 화면/네비게이션/매퍼 대규모 리팩터, Product 카테고리·파티 목록 신규 화면·뷰모델 추가, DTO/매퍼 통합·nullability 조정, UI 스타일·인셋·가격 포맷·라이프사이클 처리 일부 수정이 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ProductCategoryRoute (UI)
participant VM as ProductCategoryViewModel
participant Repo as HomeRepository
participant Remote as HomeRemoteDataSource
UI->>VM: collect uiState / send intents (OnSortSelected / OnCardClick / OnBackClick)
VM->>Repo: getGoodsCategoryList(page=0,size=100,sort=selected.request,artistId)
Repo->>Remote: remote.getGoodsCategoryList(params)
Remote-->>Repo: ApiResponse (success / failure)
Repo-->>VM: Result<ProductCategory> (toDomain())
VM-->>UI: uiState update (Loading → Success/Failure)
VM-->>UI: emit effects (NavigateToProductPartyList / NavigateToPartyCreate / NavigateBack)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 분 Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 2
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/poti/android/presentation/onboarding/OnboardingViewModel.kt (1)
134-144: API 호출 결과와 관계없이 상태 업데이트 및 네비게이션이 실행됩니다.현재 구현에서
updateState와sendEffect가.onSuccess블록 외부에 있어서, API 호출의 성공/실패 여부와 관계없이 항상 실행됩니다. 이는handleStartClick()(Line 119-132)의 패턴과 일관성이 없으며, 서버 상태와 클라이언트 상태 간 불일치가 발생할 수 있습니다.🐛 API 응답 처리 후 상태 업데이트하도록 수정
private fun handleSkipClick() = launchScope { userRepository.patchOnboarding(uiState.value.nickname, null) - .onSuccess { } - updateState { - copy( - selectedArtistId = null, - isButtonVisible = false, - ) - } - sendEffect(OnboardingUiEffect.NavigateToHome) + .onSuccess { + updateState { + copy( + selectedArtistId = null, + isButtonVisible = false, + ) + } + sendEffect(OnboardingUiEffect.NavigateToHome) + } + .onFailure { error -> + Timber.e(error, "온보딩 스킵 저장 실패") + } }app/src/main/java/com/poti/android/presentation/party/product/partylist/model/Contracts.kt (1)
23-24: 필드명에 오타가 있습니다.
bottomSheetSelectedMembersIdices→bottomSheetSelectedMembersIndices,isMemberBottomSheetToucehd→isMemberBottomSheetTouched로 수정하시면 좋겠습니다. 코드 가독성과 일관성을 위해 오타를 수정해 주세요.✏️ 제안하는 수정
- val bottomSheetSelectedMembersIdices: Set<Int> = setOf(), - val isMemberBottomSheetToucehd: Boolean = false, + val bottomSheetSelectedMembersIndices: Set<Int> = setOf(), + val isMemberBottomSheetTouched: Boolean = false,app/src/main/java/com/poti/android/presentation/party/product/partylist/ProductPartyListViewModel.kt (1)
140-143: Contracts.kt의 오타 필드명이 여기서도 사용되고 있습니다.
bottomSheetSelectedMembersIdices와isMemberBottomSheetToucehd필드명의 오타가 이 ViewModel에서도 사용되고 있습니다. Contracts.kt의 오타를 수정하시면 이 파일도 함께 수정이 필요합니다.Also applies to: 147-147, 163-163
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/poti/android/presentation/party/product/productcategory/ProductCategoryViewModel.kt`:
- Around line 56-60: ProductCategoryViewModel에서
homeRepository.getGoodsCategoryList 호출 시 sort 파라미터로 enum의 이름(sortType.name)을 넘기고
있어 enum 이름과 API 요청 값이 불일치하면 깨질 위험이 있으니, ProductSortType의 실제 요청값 필드(request)를
사용하도록 수정하세요 — 즉 getGoodsCategoryList(..., sort = sortType.request, ...)로 변경해
안전하게 전달되도록 업데이트하십시오.
- Around line 69-72: The hardcoded message passed into ApiState.Failure should
be replaced with a resource-backed string; update ProductCategoryViewModel to
inject a string provider (or android Context wrapper) and use it when
constructing ApiState.Failure (i.e., replace throwable.message ?: "Failed to
load goods category" with throwable.message ?:
stringProvider.getString(R.string.error_load_goods_category")); keep
ApiState.Failure signature the same and avoid changing its type—add the
dependency to the ViewModel constructor (e.g., StringProvider or
ResourceProvider) and call its getString method where the failure state is
created.
🧹 Nitpick comments (13)
app/src/main/java/com/poti/android/core/designsystem/component/navigation/PotiHeaderPage.kt (1)
64-66:remember를 사용하여 불필요한 객체 생성을 방지하는 것을 권장드립니다.
.copy()는 리컴포지션마다 새로운TextStyle객체를 생성합니다. 헤더 컴포넌트의 특성상 리컴포지션 빈도가 낮아 성능에 큰 영향은 없지만, Modern Android Development 권장 사항에 따라remember로 감싸시면 더 효율적입니다.♻️ 제안하는 수정 사항
subTitle?.let { + val subTitleStyle = remember { + PotiTheme.typography.caption12m.copy( + lineHeight = 1.35.em, + ) + } Text( text = subTitle, - style = PotiTheme.typography.caption12m.copy( - lineHeight = 1.35.em, - ), + style = subTitleStyle, color = PotiTheme.colors.gray700, ) }app/src/main/java/com/poti/android/presentation/party/product/component/PartyCard.kt (1)
52-53:isClosed조건의 가독성 개선을 고려해 주세요.현재
totalCount - currentCount == 0표현은 의도를 파악하는 데 한 단계 더 생각이 필요합니다. 조건의 의미를 더 명확하게 표현하면 코드 리뷰나 유지보수 시 이해하기 쉬워집니다.♻️ 제안하는 개선안
- val isClosed = totalCount - currentCount == 0 + val isClosed = currentCount >= totalCountapp/src/main/java/com/poti/android/presentation/party/product/Extension.kt (1)
21-25: 불필요한 람다 표현식이 있습니다.
availableMembers가List<String>타입이라면,{ it }람다는joinToString의 기본 동작과 동일하므로 제거하셔도 됩니다. 코드가 더 간결해집니다.♻️ 간결한 코드 제안
`@Composable` fun PartySummary.membersText(): String = availableMembers.joinToString( separator = stringResource(R.string.goods_filter_members_separator), - ) { it } + )app/src/main/java/com/poti/android/presentation/history/list/HistoryListViewModel.kt (1)
111-121:distinctBy를 통한 중복 제거가 추가되었네요.데이터 중복을 UI 레이어에서 방어적으로 처리하는 것은 좋은 접근이에요. 다만 몇 가지 확인이 필요할 수 있어요:
- 근본 원인 파악: 중복이 발생하는 원인이 백엔드인지, 매핑 로직인지 확인해 보시면 좋겠어요.
distinctBy는 증상을 처리하지만 근본 원인을 숨길 수 있어요.- 순서 보장:
distinctBy는 첫 번째 항목을 유지하는데, 이것이 의도한 동작인지 확인해 주세요.의도된 방어 코드라면 현재 구현으로 충분해요!
app/src/main/java/com/poti/android/presentation/user/mypage/MyPageScreen.kt (1)
70-74: 사용되지 않는onArtistClick파라미터 제거를 권장드려요
onArtistClick파라미터가 선언되어 있지만 함수 본문에서 전혀 사용되지 않고 있습니다. Line 117의BadgeButton에서도onClick = { }로 하드코딩되어 있어요.PR 목적이 아티스트 네비게이션 제거이므로, 해당 파라미터를 완전히 제거하시면 코드가 더 깔끔해질 것 같습니다.
♻️ 제안하는 수정 사항
`@Composable` private fun MyPageScreen( userMyPage: UserMyPage, - onArtistClick: () -> Unit, onHistoryClick: (HistoryMode, HistorySummaryType) -> Unit, modifier: Modifier = Modifier, ) {그리고 호출부(Line 56-65)도 함께 수정해주세요:
MyPageScreen( userMyPage = userMyPage, - onArtistClick = { }, onHistoryClick = { mode, type -> viewModel.processIntent( MyPageUiIntent.OnHistoryClick(mode, type), ) }, modifier = modifier, )Preview 함수들(Line 170, 202)에서도
onArtistClick = {}를 제거해주세요.app/src/main/java/com/poti/android/presentation/party/home/DummyHomeData.kt (1)
5-29: 하드코딩 문자열은 리소스화 또는 debug/preview 한정 사용을 권장드립니다.
프로덕션 경로에서 노출될 수 있다면strings.xml로 분리해 i18n 대비가 안전합니다.app/src/main/java/com/poti/android/presentation/party/product/component/GoodsSortBottomSheet.kt (1)
42-46: 불필요한padding()호출이 있습니다.
modifier.padding()에 인자가 없어서 실제로 패딩이 적용되지 않습니다. 의도적인 것이 아니라면 제거하시는 것이 좋겠습니다.♻️ 제안하는 수정
PotiBottomSheet( onDismissRequest = onDismissRequest, sheetState = sheetState, - modifier = modifier.padding(), + modifier = modifier, ) {app/src/main/java/com/poti/android/presentation/party/product/productcategory/ProductCategoryScreen.kt (2)
122-135:LazyColumn의items에key파라미터가 누락되었습니다.코딩 가이드라인에 따르면,
LazyColumn에서key파라미터가 누락되면 불필요한 리컴포지션이 발생할 수 있습니다. 각 아이템의 고유 식별자를key로 지정해 주시면 성능이 개선됩니다.♻️ 제안하는 수정
- items(productCategory.groupItems) { groupItem -> + items( + items = productCategory.groupItems, + key = { it.artistId } + ) { groupItem ->
129-129:onClick람다의 파라미터가 사용되지 않고 있습니다.
onClick = { id, title -> onCardClick(groupItem.artistId, groupItem.postTitle) }에서 람다 파라미터id와title이 무시되고groupItem의 값이 직접 사용되고 있습니다. 이는 혼란을 줄 수 있으며, 불필요한 파라미터를 제거하거나_로 명시하는 것이 좋겠습니다.♻️ 제안하는 수정
- onClick = { id, title -> onCardClick(groupItem.artistId, groupItem.postTitle) }, + onClick = { _, _ -> onCardClick(groupItem.artistId, groupItem.postTitle) },app/src/main/java/com/poti/android/presentation/party/product/partylist/model/Contracts.kt (1)
29-51:UiState내부에@Composablegetter가 있습니다.
memberFilterText프로퍼티에@Composable어노테이션이 있어 UiState가 Compose 프레임워크에 의존하게 됩니다. 이는 Clean Architecture 관점에서 상태 계층이 UI 프레임워크에 결합되는 문제가 있으며, 단위 테스트 시에도 어려움이 발생할 수 있습니다.가능하다면 이 로직을 Screen Composable 내부로 이동하거나, 순수 Kotlin 함수로 분리하여 stringResource 호출은 UI 계층에서 처리하는 것을 권장드립니다.
app/src/main/java/com/poti/android/presentation/party/product/partylist/ProductPartyListViewModel.kt (1)
24-25:toRoute가 중복 호출되고 있습니다.
savedStateHandle.toRoute<ProductRoute.ProductPartyList>()가 두 번 호출되어 불필요한 파싱이 발생합니다. 한 번만 호출하여 변수에 저장한 후 사용하시면 좋겠습니다.♻️ 제안하는 수정
) : BaseViewModel<ProductPartyListUiState, ProductPartyListUiIntent, ProductPartyListUiEffect>( initialState = ProductPartyListUiState(), ) { - private val artistId: Long = savedStateHandle.toRoute<ProductRoute.ProductPartyList>().artistId - private val title: String = savedStateHandle.toRoute<ProductRoute.ProductPartyList>().title + private val route = savedStateHandle.toRoute<ProductRoute.ProductPartyList>() + private val artistId: Long = route.artistId + private val title: String = route.titleapp/src/main/java/com/poti/android/presentation/party/product/partylist/ProductPartyListScreen.kt (1)
124-165: LazyColumnitems에key를 지정해 주세요.
리스트 갱신/정렬 시 항목 상태가 섞일 수 있어 안정성에 불리합니다.partyId를 키로 지정해 두면 안전합니다. 코딩 가이드라인에 따라 말씀드립니다.♻️ 제안 변경
- items(productPartyListInfo.partySummaries) { party -> + items( + items = productPartyListInfo.partySummaries, + key = { it.partyId }, + ) { party -> PartyCard( potId = party.partyId, profileImageUrl = party.profileImageUrl ?: "", nickname = party.nickname, rating = party.ratingText(), imageUrl = party.productImageUrl, members = party.membersText(), price = party.priceText(), currentCount = party.currentCount, totalCount = party.totalCount, onClick = onCardClick, modifier = Modifier .fillMaxWidth() .padding(bottom = 16.dp), ) }app/src/main/java/com/poti/android/presentation/party/product/DummyPotsDate.kt (1)
1-1: 파일명 오타 확인이 필요해요.
DummyPotsDate.kt가DummyPotsData.kt의 오타로 보여요. "Date"가 아닌 "Data"가 의도한 이름인지 확인 부탁드려요.
| homeRepository.getGoodsCategoryList( | ||
| page = 0, | ||
| size = 100, | ||
| sort = sortType.name, | ||
| artistId = artistId, |
There was a problem hiding this comment.
정렬 파라미터는 request 값을 사용해주세요.
sortType.name는 enum 이름과 API 요청 값이 달라지면 바로 깨집니다. 의도된 ProductSortType.request로 전달하는 편이 안전합니다.
🐛 제안 수정
- sort = sortType.name,
+ sort = sortType.request,📝 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.
| homeRepository.getGoodsCategoryList( | |
| page = 0, | |
| size = 100, | |
| sort = sortType.name, | |
| artistId = artistId, | |
| homeRepository.getGoodsCategoryList( | |
| page = 0, | |
| size = 100, | |
| sort = sortType.request, | |
| artistId = artistId, |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/party/product/productcategory/ProductCategoryViewModel.kt`
around lines 56 - 60, ProductCategoryViewModel에서
homeRepository.getGoodsCategoryList 호출 시 sort 파라미터로 enum의 이름(sortType.name)을 넘기고
있어 enum 이름과 API 요청 값이 불일치하면 깨질 위험이 있으니, ProductSortType의 실제 요청값 필드(request)를
사용하도록 수정하세요 — 즉 getGoodsCategoryList(..., sort = sortType.request, ...)로 변경해
안전하게 전달되도록 업데이트하십시오.
| copy( | ||
| productCategoryLoadState = ApiState.Failure( | ||
| throwable.message ?: "Failed to load goods category", | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. 프로젝트 내 메시지/리소스 래퍼 패턴 탐색
echo "=== Message/Resource patterns in project ==="
rg -n "UiText|StringRes|StringResource|errorMessage|ErrorMessage" --type kt | head -30
# 2. 해당 ViewModel 파일 확인
echo -e "\n=== ProductCategoryViewModel.kt content (lines 60-85) ==="
if [ -f "app/src/main/java/com/poti/android/presentation/party/product/productcategory/ProductCategoryViewModel.kt" ]; then
sed -n '60,85p' "app/src/main/java/com/poti/android/presentation/party/product/productcategory/ProductCategoryViewModel.kt"
fi
# 3. ViewModel 내 다른 하드코딩 메시지 찾기
echo -e "\n=== All hardcoded messages in ProductCategoryViewModel ==="
if [ -f "app/src/main/java/com/poti/android/presentation/party/product/productcategory/ProductCategoryViewModel.kt" ]; then
rg -n "\"[A-Za-z]" "app/src/main/java/com/poti/android/presentation/party/product/productcategory/ProductCategoryViewModel.kt"
fiRepository: team-poti/POTI-ANDROID
Length of output: 1072
🏁 Script executed:
#!/bin/bash
# 1. 다양한 메시지 패턴 탐색 (타입 필터 제거)
echo "=== Searching for message/resource patterns (no type filter) ==="
rg -n "UiText|StringRes|errorMessage|ResourceString|@StringRes" | head -20
# 2. ApiState 클래스 정의 확인
echo -e "\n=== ApiState class definition ==="
rg -A 5 "class ApiState|sealed class ApiState|data class Failure" | head -30
# 3. strings.xml 리소스 파일 확인
echo -e "\n=== Looking for resource files ==="
find . -name "strings*.xml" -o -name "*.xml" | grep -i string | head -10
# 4. 다른 ViewModel에서 에러 메시지 처리 패턴
echo -e "\n=== Error message handling in other ViewModels ==="
rg -n "Failure\(|ApiState\.Failure" -A 1 | head -30
# 5. ViewModel에서 메시지 전달 방식 탐색
echo -e "\n=== Message parameter patterns in ViewModel ==="
rg -n "throwable\.message|\.message \?:" | head -20Repository: team-poti/POTI-ANDROID
Length of output: 10678
🏁 Script executed:
#!/bin/bash
# ErrorText 클래스 전체 정의 확인
echo "=== ErrorText class definition ==="
cat "app/src/main/java/com/poti/android/presentation/onboarding/model/ErrorText.kt"
# ApiState.Failure 완전한 정의 확인
echo -e "\n=== ApiState complete definition ==="
cat "app/src/main/java/com/poti/android/core/common/state/ApiState.kt"Repository: team-poti/POTI-ANDROID
Length of output: 1043
리소스 파일에서 메시지를 읽어서 전달해주세요.
현재 "Failed to load goods category"는 하드코딩되어 있고, 프로젝트의 ApiState 구조상 리소스 ID를 직접 전달할 수 없습니다. 가능한 개선 방법:
-
리소스 기반 문자열 읽기 (가장 현실적):
throwable.message ?: context.getString(R.string.error_load_goods_category)
이를 위해 context 또는 StringProvider 주입이 필요할 수 있습니다.
-
ApiState 구조 개선 (장기 솔루션):
ApiState.Failure를 리팩토링하여 ErrorType enum이나@StringResresId를 받도록 변경하면, UI에서 유연하게 처리 가능합니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/party/product/productcategory/ProductCategoryViewModel.kt`
around lines 69 - 72, The hardcoded message passed into ApiState.Failure should
be replaced with a resource-backed string; update ProductCategoryViewModel to
inject a string provider (or android Context wrapper) and use it when
constructing ApiState.Failure (i.e., replace throwable.message ?: "Failed to
load goods category" with throwable.message ?:
stringProvider.getString(R.string.error_load_goods_category")); keep
ApiState.Failure signature the same and avoid changing its type—add the
dependency to the ViewModel constructor (e.g., StringProvider or
ResourceProvider) and call its getString method where the failure state is
created.
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/presentation/history/manage/ParticipantManageScreen.kt`:
- Around line 15-16: 현재 사용 중인 LifecycleEventEffect(Lifecycle.Event.ON_RESUME)은
컴포저블이 이미 RESUMED 상태일 때 최초 진입 시 콜백이 실행되지 않으니 초기 데이터 로드를 보장하려면
LifecycleEventEffect(Lifecycle.Event.ON_RESUME)만 의존하지 말고 ViewModel의 init 블록에서 초기
로드를 수행하거나, 라이프사이클 상태 기반으로 감지하도록 리팩터링(예: Lifecycle.currentStateAsState()를 구독해
Lifecycle.State.RESUMED 진입을 감지)하여 ParticipantManageScreen의 초기 진입 시에도 데이터가 로드되도록
수정하세요.
In
`@app/src/main/java/com/poti/android/presentation/history/recruiter/RecruiterViewModel.kt`:
- Line 39: RecruiterDetailUiIntent.OnResume 핸들러에서 recruitId 유효성 검사(recruitId !=
-1L)가 빠져 있어 resume 시 불필요한 API 호출이 발생할 수 있습니다; RecruiterDetailUiIntent.OnResume
케이스에서 getRecruiterDetail()을 바로 호출하지 말고 init 블록에서 사용한 것처럼 recruitId != -1L 조건을
확인한 뒤에만 getRecruiterDetail()을 호출하도록 수정하세요 (참조: RecruiterDetailUiIntent.OnResume,
getRecruiterDetail(), init 블록의 recruitId != -1L 검사).
🧹 Nitpick comments (7)
app/src/main/java/com/poti/android/presentation/history/recruiter/model/Contracts.kt (1)
22-28:UiEffect도data object로 통일하는 것을 고려해주세요.
RecruiterDetailUiIntent는data object로 변환하셨는데,RecruiterDetailUiEffect의NavigateBack은 여전히object로 남아있습니다. 일관성을 위해 파라미터가 없는 Effect도data object로 변경하시는 것을 권장드립니다.♻️ 제안하는 수정
sealed interface RecruiterDetailUiEffect : UiEffect { - object NavigateBack : RecruiterDetailUiEffect + data object NavigateBack : RecruiterDetailUiEffect data class NavigateToPartyDetail(val recruitId: Long) : RecruiterDetailUiEffect data class NavigateToParticipantList(val recruitId: Long) : RecruiterDetailUiEffect }app/src/main/java/com/poti/android/presentation/history/recruiter/RecruiterViewModel.kt (2)
10-10: Wildcard import 대신 명시적 import를 고려해주세요.Wildcard import(
*)는 코드의 출처를 파악하기 어렵게 만들 수 있습니다. 어떤 Effect가 사용되는지 명확하게 보이도록 개별 import를 사용하시는 것을 권장드립니다.♻️ 제안하는 수정
import com.poti.android.presentation.history.recruiter.model.RecruiterDetailUiEffect -import com.poti.android.presentation.history.recruiter.model.RecruiterDetailUiEffect.* +import com.poti.android.presentation.history.recruiter.model.RecruiterDetailUiEffect.NavigateBack +import com.poti.android.presentation.history.recruiter.model.RecruiterDetailUiEffect.NavigateToPartyDetail +import com.poti.android.presentation.history.recruiter.model.RecruiterDetailUiEffect.NavigateToParticipantList
43-58: API 호출 전 Loading 상태 처리를 고려해주세요.현재
getRecruiterDetail()은 API 호출 전에 Loading 상태를 설정하지 않아서 새로고침 중에도 이전 데이터가 표시됩니다. 이것이 의도된 UX라면 괜찮지만, 사용자에게 새로고침 중임을 알리고 싶으시다면 Loading 상태를 먼저 설정하시는 것도 고려해볼 수 있습니다.♻️ 선택적 개선 제안
private fun getRecruiterDetail() = launchScope { + updateState { copy(recruiterDetailState = ApiState.Loading) } partyRepository.getRecruitDetail(recruitId) .onSuccess { updateState { copy(recruiterDetailState = ApiState.Success(it.toUiModel())) } }참고: 화면 깜빡임(flickering)을 방지하기 위해 의도적으로 Loading 상태를 생략하셨을 수 있습니다. UX 요구사항에 따라 결정해주세요.
app/src/main/java/com/poti/android/presentation/history/list/HistoryListViewModel.kt (3)
15-15: 와일드카드 import 사용에 대한 고려사항입니다.
HistoryListUiEffect.*와일드카드 import는 코드를 간결하게 만들지만, 어떤 Effect가 사용되는지 명시적으로 파악하기 어려울 수 있습니다. Sealed class의 하위 타입들을 개별적으로 import하면 가독성이 향상될 수 있습니다.♻️ 명시적 import 제안
import com.poti.android.presentation.history.list.model.HistoryListUiEffect -import com.poti.android.presentation.history.list.model.HistoryListUiEffect.* +import com.poti.android.presentation.history.list.model.HistoryListUiEffect.NavigateBack +import com.poti.android.presentation.history.list.model.HistoryListUiEffect.NavigateToRecruiterDetail +import com.poti.android.presentation.history.list.model.HistoryListUiEffect.NavigateToParticipantDetail
50-50: OnResume과 init 블록의 중복 호출 가능성을 확인해 주세요.
init블록(Line 66)과OnResume인텐트(Line 50) 모두loadUserHistoryList()를 호출합니다. 화면 최초 진입 시init→ON_RESUME순서로 두 번 호출될 수 있습니다.
fetchJob?.cancel()로 이전 작업이 취소되어 중복 요청은 방지되지만, 의도적인 설계라면init에서 호출을 제거하고OnResume에서만 로딩하는 것이 더 명확할 수 있습니다.♻️ init 블록에서 loadUserHistoryList() 제거 제안
init { updateState { copy( mode = initialMode ?: this.mode, selectedTab = when (initialType) { null -> this.selectedTab HistorySummaryType.COMPLETED -> PotiHeaderTabType.ENDED else -> PotiHeaderTabType.ONGOING }, ) } - - loadUserHistoryList() }Also applies to: 66-66
96-133: Coroutine 예외 처리 보강을 권장드립니다.현재
result.onSuccess/onFailure로Result타입을 처리하고 있지만, Repository 호출 자체에서 예외가 발생할 경우(네트워크 오류 등) catch되지 않을 수 있습니다. 코딩 가이드라인에 따라runCatching또는 예외 처리 추가를 권장드립니다.♻️ 예외 처리 보강 제안
private fun loadUserHistoryList() { fetchJob?.cancel() fetchJob = viewModelScope.launch { + runCatching { val requestStatus = when (uiState.value.selectedTab) { PotiHeaderTabType.ONGOING -> HistoryListType.IN_PROGRESS PotiHeaderTabType.ENDED -> HistoryListType.COMPLETED } val result = if (uiState.value.mode == HistoryMode.RECRUIT) { partyRepository.getMyRecruitList(requestStatus.name) } else { participationRepository.getMyParticipationList(requestStatus.name) } result.onSuccess { myPartyList -> // ... existing success handling }.onFailure { error -> // ... existing failure handling } + }.onFailure { throwable -> + updateState { + copy(historyListLoadState = ApiState.Failure(throwable.message ?: "Unknown error")) + } + } } }app/src/main/java/com/poti/android/presentation/history/manage/ParticipantManageViewModel.kt (1)
38-51: 초기 진입 중복 로딩 가능성 있습니다.
init { loadParticipantManageDetail() }가 이미 실행되는데OnResume에서도 동일 호출을 하므로, 첫 진입 시 중복 요청이 발생할 수 있습니다. 초기 로딩을OnResume로 단일화하거나, 최초 1회 가드/Job 취소로 중복 호출을 막는 방향을 권장드립니다.
| import androidx.lifecycle.Lifecycle | ||
| import androidx.lifecycle.compose.LifecycleEventEffect |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ParticipantManageScreen.kt" -type fRepository: team-poti/POTI-ANDROID
Length of output: 157
🏁 Script executed:
cat -n app/src/main/java/com/poti/android/presentation/history/manage/ParticipantManageScreen.kt | head -60Repository: team-poti/POTI-ANDROID
Length of output: 3489
🌐 Web query:
androidx.lifecycle.compose LifecycleEventEffect ON_RESUME first composition behavior documentation
💡 Result:
LifecycleEventEffect(Lifecycle.Event.ON_RESUME) does not “fire on first composition” just because the owner is already in RESUMED.
Per the docs, when LifecycleEventEffect enters the composition it installs a LifecycleEventObserver, and your onEvent runs only when the lifecycle subsequently receives that specific Lifecycle.Event (i.e., it’s event-driven, not “replay current state”). [1]
So, if the composable is first composed while the LifecycleOwner is already resumed, the ON_RESUME callback typically won’t run until the next actual resume transition (after a pause/stop, etc.).
If you need “run immediately if already resumed, and also on future resumes”, base it on lifecycle state (e.g., currentStateAsState() / currentStateFlow) rather than only events. [1]
Sources: Android “Integrate Lifecycle with Compose” (Lifecycle effects + currentStateAsState) [1]; Android “Side-effects in Compose” (registering a LifecycleEventObserver via DisposableEffect, which similarly only reacts to events as they occur) [2].
초기 진입 시 ON_RESUME 이벤트 발생 여부 확인 필요합니다.
LifecycleEventEffect(Lifecycle.Event.ON_RESUME)는 이벤트 기반으로 동작하기 때문에, 컴포저블이 이미 RESUMED 상태인 라이프사이클에서 처음 구성될 때는 콜백이 실행되지 않습니다. 다음 resume 전환(pause/stop 후 재개)이 발생할 때만 작동합니다.
따라서 이 화면에 최초 진입했을 때 데이터 로딩이 필요하다면:
- ViewModel의
init블록이나 다른 메커니즘에서 초기 로드가 발생하는지 확인하거나 Lifecycle.State.RESUMED상태 기반으로 감지하는 방식으로 리팩터링을 고려해 주세요.
필요시 currentStateAsState()를 활용해 라이프사이클 상태를 구독하는 방식도 고려할 수 있습니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/history/manage/ParticipantManageScreen.kt`
around lines 15 - 16, 현재 사용 중인 LifecycleEventEffect(Lifecycle.Event.ON_RESUME)은
컴포저블이 이미 RESUMED 상태일 때 최초 진입 시 콜백이 실행되지 않으니 초기 데이터 로드를 보장하려면
LifecycleEventEffect(Lifecycle.Event.ON_RESUME)만 의존하지 말고 ViewModel의 init 블록에서 초기
로드를 수행하거나, 라이프사이클 상태 기반으로 감지하도록 리팩터링(예: Lifecycle.currentStateAsState()를 구독해
Lifecycle.State.RESUMED 진입을 감지)하여 ParticipantManageScreen의 초기 진입 시에도 데이터가 로드되도록
수정하세요.
| is RecruiterDetailUiIntent.ParticipantSectionClicked -> sendEffect(RecruiterDetailUiEffect.NavigateToParticipantList(recruitId)) | ||
| is RecruiterDetailUiIntent.PartyCardClicked -> sendEffect(NavigateToPartyDetail(recruitId)) | ||
| is RecruiterDetailUiIntent.ParticipantSectionClicked -> sendEffect(NavigateToParticipantList(recruitId)) | ||
| RecruiterDetailUiIntent.OnResume -> getRecruiterDetail() |
There was a problem hiding this comment.
OnResume에서 recruitId 유효성 검사가 누락되었습니다.
init 블록(Line 27)에서는 recruitId != -1L 검사를 수행하지만, OnResume 처리에서는 이 검사 없이 바로 getRecruiterDetail()을 호출하고 있습니다. recruitId가 -1L인 상태에서 화면이 resume되면 불필요한 API 호출이나 예상치 못한 오류가 발생할 수 있습니다.
🐛 제안하는 수정
- RecruiterDetailUiIntent.OnResume -> getRecruiterDetail()
+ RecruiterDetailUiIntent.OnResume -> {
+ if (recruitId != -1L) {
+ getRecruiterDetail()
+ }
+ }📝 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.
| RecruiterDetailUiIntent.OnResume -> getRecruiterDetail() | |
| RecruiterDetailUiIntent.OnResume -> { | |
| if (recruitId != -1L) { | |
| getRecruiterDetail() | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/history/recruiter/RecruiterViewModel.kt`
at line 39, RecruiterDetailUiIntent.OnResume 핸들러에서 recruitId 유효성 검사(recruitId !=
-1L)가 빠져 있어 resume 시 불필요한 API 호출이 발생할 수 있습니다; RecruiterDetailUiIntent.OnResume
케이스에서 getRecruiterDetail()을 바로 호출하지 말고 init 블록에서 사용한 것처럼 recruitId != -1L 조건을
확인한 뒤에만 getRecruiterDetail()을 호출하도록 수정하세요 (참조: RecruiterDetailUiIntent.OnResume,
getRecruiterDetail(), init 블록의 recruitId != -1L 검사).
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 (2)
app/src/main/java/com/poti/android/presentation/history/participant/ParticipantViewModel.kt (2)
69-87: 결제 실패 시에도 오버레이가 닫히는 문제가 있습니다.
postPayment호출이 실패하더라도 85-86번 라인이 무조건 실행되어 오버레이가 닫히고 데이터가 새로고침됩니다. 사용자는 결제 정보 제출이 실패했는지 알 수 없게 됩니다.
onFailure블록에서 적절한 에러 처리 후 early return 하시거나, 성공 시에만 후속 로직이 실행되도록 수정하시는 것이 좋겠습니다.🐛 수정 제안
private fun patchSubmitDeposit( depositor: String, depositTime: String, ) = launchScope { updateState { copy(participantDetailState = ApiState.Loading) } paymentRepository.postPayment( orderId = participantId, depositorName = depositor, depositAt = depositTime, ).onSuccess { Timber.d("success: $depositor, $depositTime") + getParticipantDetail() + updateState { copy(overlayState = ParticipantDetailOverlayState.None) } }.onFailure { error -> Timber.d("fail: ${error.message}") + // 실패 시 Loading 상태 해제 및 사용자에게 에러 피드백 제공 + updateState { copy(participantDetailState = ApiState.Failure(error.message ?: "결제 정보 제출 실패")) } } - - getParticipantDetail() - updateState { copy(overlayState = ParticipantDetailOverlayState.None) } }
118-130: 리뷰 제출 실패 시 사용자 피드백이 누락되어 있습니다.
submitReview의onFailure블록(127-129번 라인)에서 에러 로깅만 하고 있어서, 리뷰 제출이 실패해도 사용자에게 아무런 피드백이 없습니다. 오버레이가 열린 채로 유지되어 사용자가 혼란스러울 수 있습니다.실패 시 토스트 메시지를 표시하거나, Side Effect를 통해 에러 상태를 알려주시는 것이 좋겠습니다.
♻️ 수정 제안
private fun submitReview( transactionId: Long, star: Int, ) = launchScope { reviewRepository.postReview(transactionId, star) .onSuccess { result -> Timber.d("success: $result") updateState { copy(overlayState = ParticipantDetailOverlayState.None) } getParticipantDetail() }.onFailure { error -> Timber.e(error, "submit review failed") + // 사용자에게 에러 피드백 제공 + sendEffect(ParticipantDetailUiEffect.ShowError("리뷰 제출에 실패했습니다")) } }또는 UiEffect에 에러 타입이 없다면 오버레이를 닫고 재시도를 유도하실 수도 있습니다.
♻️ Duplicate comments (1)
app/src/main/java/com/poti/android/presentation/history/recruiter/RecruiterViewModel.kt (1)
26-28:init블록과OnResume에서recruitId유효성 검사가 누락되었습니다.이전에
init블록에서 수행하던recruitId != -1L검사가 제거되었고,OnResume핸들러에도 동일한 검사가 없습니다.recruitId가-1L인 상태에서 화면이 초기화되거나 resume되면 불필요한 API 호출이 발생할 수 있습니다.🐛 제안하는 수정
init { - getRecruiterDetail() + if (recruitId != -1L) { + getRecruiterDetail() + } }-RecruiterDetailUiIntent.OnResume -> getRecruiterDetail() +RecruiterDetailUiIntent.OnResume -> { + if (recruitId != -1L) { + getRecruiterDetail() + } +}Also applies to: 35-35
🧹 Nitpick comments (1)
app/src/main/java/com/poti/android/presentation/history/recruiter/RecruiterViewModel.kt (1)
10-10: 와일드카드 import 대신 명시적 import를 권장드립니다.Sealed class의 하위 타입들을 와일드카드(
*)로 import하면 코드 가독성이 떨어질 수 있습니다. 어떤 Effect가 사용되는지 파일 상단에서 명확히 파악하기 어려워지기 때문입니다.♻️ 제안하는 수정
-import com.poti.android.presentation.history.recruiter.model.RecruiterDetailUiEffect.* +import com.poti.android.presentation.history.recruiter.model.RecruiterDetailUiEffect.NavigateToPartyDetail +import com.poti.android.presentation.history.recruiter.model.RecruiterDetailUiEffect.NavigateToParticipantList
2 participants
Related issue 🛠️
Work Description ✏️
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
새로운 기능
개선사항
리팩토링
✏️ Tip: You can customize this high-level summary in your review settings.