Conversation
📝 WalkthroughWalkthrough굿즈별 페이지 API 연동을 위해 원격 엔드포인트·DTO·매퍼·레포지토리 메서드를 추가하고, 네비게이션·뷰모델·UI 콜백에 title과 artistId 페이로드를 전달하도록 시그니처와 상태를 확장했습니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
participant Screen as GoodsFilteredPartyListScreen
participant VM as GoodsFilterViewModel
participant Repo as PartyRepository
participant DS as PartyRemoteDataSource
participant Svc as PartyService
participant API as BackendAPI
Screen->>VM: LoadGoodsPots / OnMembersSelect / OnSortSelect
VM->>VM: update selectedMembers / goodsPartySortType
VM->>Repo: getProductPartyList(page,size,title,artistId,sort,memberIds)
Repo->>DS: getProductPartyList(...)
DS->>Svc: getProductPartyList(...) (Retrofit)
Svc->>API: GET /api/v1/posts/pots?page=&size=&title=&artistId=&sort=&memberIds=
API-->>Svc: ProductPartyListResponseDto
Svc-->>DS: BaseResponse<ProductPartyListResponseDto>
DS-->>Repo: BaseResponse<ProductPartyListResponseDto>
Repo->>Repo: handleApiResponse -> toDomain()
Repo-->>VM: Result<ProductPartyList>
VM->>Screen: update UiState(productPartyListInfo)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
![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
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/party/goodsfilter/GoodsFilteredPartyListScreen.kt (1)
138-154:LazyColumn의items에key파라미터가 누락되었습니다.아이템의 고유 키가 없으면 목록 변경 시 불필요한 리컴포지션이 발생할 수 있습니다.
partyId를 키로 사용하는 것을 권장드립니다.♻️ 제안하는 개선안
-items(partyListInfo.partySummaries) { party -> +items( + items = partyListInfo.partySummaries, + key = { it.partyId } +) { party ->Based on learnings,
LazyColumn에서key파라미터를 사용하면 아이템 추가/삭제/재정렬 시 Compose가 효율적으로 리컴포지션을 수행할 수 있습니다.
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt`:
- Around line 37-45: In GoodsFilterViewModel update the handlers for
GoodsFilterUiIntent.OnMembersSelect and GoodsFilterUiIntent.OnSortSelect so that
after calling updateState (copying selectedMembers or goodsPartySortType) you
also trigger a refresh by calling loadPartyList(); locate the intent handling
block where these cases are processed and add a call to the existing
loadPartyList() method immediately after updateState to ensure the party list
reloads when filters change.
🧹 Nitpick comments (8)
app/src/main/java/com/poti/android/data/remote/dto/response/party/PartyListResponseDto.kt (1)
6-20: LGTM! DTO 구조가 잘 정의되어 있습니다.kotlinx.serialization을 사용한 DTO 구조가 올바르게 작성되어 있습니다. 다만,
@SerialName어노테이션은 JSON 필드명과 프로퍼티명이 다를 때만 필요합니다. 현재는 모든 필드명이 동일하므로@SerialName어노테이션을 제거해도 동작에는 문제가 없습니다. 명시적으로 남겨두는 것도 좋은 선택이니 참고만 해주세요.app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/Extension.kt (1)
21-25: 불필요한 identity lambda를 제거할 수 있습니다.
availableMembers가 이제List<String>이므로{ it }람다는 불필요합니다.joinToString의 기본 동작이 각 요소의toString()을 호출하므로, String 리스트에서는 람다 없이도 동일하게 동작합니다.♻️ 제안하는 리팩토링
`@Composable` fun PartySummary.membersText(): String = availableMembers.joinToString( separator = stringResource(R.string.goods_filter_members_separator), - ) { it } + )app/src/main/java/com/poti/android/domain/repository/PartyRepository.kt (1)
41-48: API 설계에 대한 몇 가지 제안 사항이 있습니다.
title파라미터가 non-nullableString으로 선언되어 있는데, 빈 문자열("")을 전달하는 것보다String?으로 선언하여 의도를 명확히 하는 것이 좋을 수 있습니다.
sort파라미터가String타입으로 되어 있는데, Domain 레이어에서는 타입 안전성을 위해 enum이나 sealed class를 사용하는 것이 권장됩니다. 현재 Presentation 레이어에PartySortType이 정의되어 있는데, 이를 Domain 레이어로 이동하거나 Domain 전용 enum을 만드는 것을 고려해 주세요.♻️ 제안하는 개선안
suspend fun getPartyList( page: Int?, size: Int?, - title: String, + title: String?, artistId: Long, - sort: String, + sort: PartySortType, // Domain 레이어에 정의된 enum 사용 memberIds: List<Long>?, ): Result<PartyList>app/src/main/java/com/poti/android/presentation/party/goodsfilter/DummyPotsDate.kt (1)
1-5: 파일명 오타가 있습니다.
DummyPotsDate.kt에서Date가 아닌Data가 맞는 것 같습니다.DummyPotsData.kt로 변경하는 것을 권장드립니다.app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt (1)
21-21:checkNotNull사용 시 크래시 가능성이 있습니다.
artistId가 Navigation arguments에서 누락되면 앱이 크래시될 수 있습니다.requireNotNull과 명확한 에러 메시지를 사용하거나, 에러 상태로 전환하는 방식을 고려해 주세요.♻️ 제안하는 개선안
-private val artistId: Long = checkNotNull(savedStateHandle["artistId"]) +private val artistId: Long = requireNotNull(savedStateHandle["artistId"]) { + "artistId is required for GoodsFilterViewModel" +}app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt (2)
40-46: 사용되지 않는artistId파라미터가 있습니다.
GoodsFilteredPartyListRoute의artistId파라미터가 함수 내에서 사용되지 않습니다. ViewModel이SavedStateHandle에서 직접 가져오므로 이 파라미터가 필요한지 확인해 주세요.
164-165: Preview 함수명에 오타가 있습니다.
GoodsFilteredPartyListScreenPreveiw에서Preview가Preveiw로 잘못 작성되어 있습니다.♻️ 제안하는 개선안
-private fun GoodsFilteredPartyListScreenPreveiw() { +private fun GoodsFilteredPartyListScreenPreview() {app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/Contract.kt (1)
22-25:FilterMember에@Immutable또는@Stable어노테이션 추가를 고려해 주세요.
FilterMember가 Compose UI 상태에서 사용되므로,@Immutable어노테이션을 추가하면 불필요한 리컴포지션을 방지할 수 있습니다.♻️ 제안하는 개선안
+import androidx.compose.runtime.Immutable + +@Immutable data class FilterMember( val id: Long, val name: String, )As per coding guidelines, List나 일반 클래스를 State로 사용할 때
@Immutable이나@Stable어노테이션 사용이 권장됩니다.
app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt
Outdated
Show resolved
Hide resolved
doyeon0307
left a comment
doyeon0307
left a comment
There was a problem hiding this comment.
악 title 받아오려면 좀 뒤집어야하네 수고햇어욤
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/party/goodsfilter/GoodsFilteredPartyListScreen.kt (1)
3-50: Composable 내 Log.d()는 릴리스 및 개인정보 노출 위험이 있습니다.line 50에서
uiState.partyListInfo를 매 recomposition마다 로깅하고 있어 사용자 닉네임 등 민감한 정보 노출 및 로그 노이즈 위험이 큽니다. 디버그 환경 전용으로 보호하거나 제거해야 합니다.또한 LazyColumn의 items() 호출 시 key 파라미터가 누락되었습니다(line 140). 안정적인 item 재식별을 위해
items(partyListInfo.partySummaries, key = { it.partyId }) { party ->형태로 수정하세요.🧹 제안 수정
-import android.util.Log ... - Log.d("GoodsFilterUI", "recompose: ${uiState.partyListInfo}") ... - items(partyListInfo.partySummaries) { party -> + items(partyListInfo.partySummaries, key = { it.partyId }) { party ->
♻️ Duplicate comments (1)
app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt (1)
39-47: 필터 변경 시 파티 목록 재로드가 필요합니다.
OnMembersSelect와OnSortSelect에서 상태만 업데이트하고loadPartyList()를 호출하지 않고 있습니다. 필터 변경 후 목록이 갱신되지 않으면 사용자 경험에 문제가 발생할 수 있습니다.♻️ 제안하는 수정안
is GoodsFilterUiIntent.OnMembersSelect -> { updateState { copy(selectedMembers = intent.members) } + loadPartyList() } // ... is GoodsFilterUiIntent.OnSortSelect -> { updateState { copy(goodsPartySortType = intent.sort) } + loadPartyList() }
🧹 Nitpick comments (4)
app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt (1)
51-83:loadPartyList()구현이 전반적으로 잘 되어 있습니다.
launchScope를 활용한 코루틴 실행,onSuccess/onFailure를 통한 Result 처리, 상태 업데이트 로직이 MVI 패턴에 맞게 구현되어 있습니다.몇 가지 개선 제안사항이 있습니다:
- 페이지네이션:
page = 0,size = 10이 하드코딩되어 있는데, 향후 페이지네이션 지원 시 상태로 관리하는 것이 좋겠습니다.- 로깅:
android.util.Log대신Timber나 프로젝트의 로깅 유틸리티 사용을 권장드립니다.- Log.e("GoodsFilterVM", "loadPartyList failed", throwable) + Timber.e(throwable, "loadPartyList failed")app/src/main/java/com/poti/android/presentation/party/home/component/HomeGoodsSection.kt (1)
67-77:LazyRow의items에key파라미터가 누락되었습니다.코딩 가이드라인에 따르면
LazyColumn/LazyRow등 리스트에서key파라미터를 사용하여 불필요한 리컴포지션을 방지해야 합니다.GroupItem에 고유 식별자가 있다면key로 지정해주세요.♻️ 제안하는 수정안
items( items = groupItems, + key = { it.artistId } // 또는 고유한 식별자 사용 ) { item -> GoodsSmallCard( imageUrl = item.postImage, artist = item.artist, artistId = item.artistId, title = item.postTitle, partyCount = item.postCount, tag = item.tag, onClick = onCardClick, ) }Based on coding guidelines,
LazyColumn등 리스트에서key파라미터가 누락되었는지 확인해야 합니다.app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt (1)
133-151: LazyColumn items에 안정적인 key를 지정해 주세요.
partyId가 식별자라면 key로 지정해 스크롤 위치/리컴포지션 안정성을 확보하는 편이 좋습니다. 코딩 가이드라인에 따라 확인했습니다.♻️ 제안 수정
- items(partyListInfo.partySummaries) { party -> + items( + items = partyListInfo.partySummaries, + key = { it.partyId }, + ) { party ->app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt (1)
135-147: Goods 카테고리 목록도 key 지정 권장드립니다.
groupItems에 고유 ID가 없다면 artistId + postTitle 등 조합으로 key를 지정해 리컴포지션 안정성을 확보해 주세요. 코딩 가이드라인에 따라 확인했습니다.♻️ 제안 수정
- items(goodsCategory.groupItems) { groupItem -> + items( + items = goodsCategory.groupItems, + key = { it.artistId to it.postTitle }, + ) { groupItem ->
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/user/mypage/MyPageScreen.kt`:
- Around line 118-122: BadgeButton currently uses an empty lambda for onClick so
the passed-in onArtistClick is never invoked; update the BadgeButton call in
MyPageScreen to use the onArtistClick parameter (e.g., onClick = onArtistClick)
and apply the TODO conditional ("// TODO: [예림] 선택 최애 없을 때만 이동") around that call
so navigation only happens when the favorite selection condition is met; if the
click should be intentionally disabled instead, remove the onArtistClick
parameter from MyPageScreen or mark it with `@Suppress`("UnusedParameter") and/or
render BadgeButton conditionally.
| BadgeButton( | ||
| bias = biasText, | ||
| onClick = onArtistClick, | ||
| onClick = { }, | ||
| modifier = Modifier, | ||
| ) |
There was a problem hiding this comment.
onArtistClick 콜백이 사용되지 않고 빈 람다로 대체되어 버튼이 동작하지 않습니다.
현재 BadgeButton의 onClick이 빈 람다 { }로 설정되어 있어, 버튼을 클릭해도 아무런 동작이 발생하지 않습니다. onArtistClick 파라미터가 MyPageScreen에 전달되지만 실제로 사용되지 않아 dead code가 되었습니다.
Line 61의 TODO 주석(// TODO: [예림] 선택 최애 없을 때만 이동)을 고려하면, 조건부 네비게이션 로직이 필요해 보입니다.
🛠️ TODO 로직을 반영한 수정 제안
BadgeButton(
bias = biasText,
- onClick = { },
+ onClick = {
+ if (userMyPage.favoriteArtistName == null) {
+ onArtistClick()
+ }
+ },
modifier = Modifier,
)또는 의도적으로 비활성화하는 것이라면, 파라미터에서 onArtistClick을 제거하고 @Suppress("UnusedParameter")를 사용하거나, 버튼 자체를 조건부로 렌더링하는 것을 고려해 주세요.
🤖 Prompt for AI Agents
In `@app/src/main/java/com/poti/android/presentation/user/mypage/MyPageScreen.kt`
around lines 118 - 122, BadgeButton currently uses an empty lambda for onClick
so the passed-in onArtistClick is never invoked; update the BadgeButton call in
MyPageScreen to use the onArtistClick parameter (e.g., onClick = onArtistClick)
and apply the TODO conditional ("// TODO: [예림] 선택 최애 없을 때만 이동") around that call
so navigation only happens when the favorite selection condition is met; if the
click should be intentionally disabled instead, remove the onArtistClick
parameter from MyPageScreen or mark it with `@Suppress`("UnusedParameter") and/or
render BadgeButton conditionally.
…t-api # Conflicts: # app/src/main/java/com/poti/android/data/remote/datasource/PartyRemoteDataSource.kt # app/src/main/java/com/poti/android/data/remote/service/PartyService.kt # app/src/main/java/com/poti/android/data/repository/PartyRepositoryImpl.kt # app/src/main/java/com/poti/android/domain/repository/PartyRepository.kt
#94-party-list-api # Conflicts: # app/src/main/java/com/poti/android/data/remote/dto/response/party/ProductPartyListResponseDto.kt # app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt # app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryViewModel.kt # app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt # app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsCategoryUiState.kt # app/src/main/java/com/poti/android/presentation/party/goodsfilter/navigation/GoodsNavigation.kt # 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/component/HomeGoodsSection.kt # app/src/main/java/com/poti/android/presentation/party/home/model/Contracts.kt
There was a problem hiding this comment.
Actionable comments posted: 3
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/party/home/HomeViewModel.kt (1)
18-26: 파라미터 순서가 일치하지 않습니다.
NavigateToGoodsPartyList와OnCardClick의 파라미터 순서를 확인해 주세요.
HomeUiEffect.NavigateToGoodsPartyList정의:(val title: String, val artistId: Long)- 호출 코드:
HomeUiEffect.NavigateToGoodsPartyList(intent.artistId, intent.title)← 순서 반대라인 23에서 인자를 순서대로 전달해야 합니다:
sendEffect(HomeUiEffect.NavigateToGoodsPartyList(intent.title, intent.artistId))마찬가지로
HomeScreen.kt의OnCardClick호출도 확인이 필요합니다.app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt (1)
140-156: LazyColumnitems에key파라미터를 추가해 주세요.코딩 가이드라인에 따르면,
LazyColumn에서key파라미터가 누락되면 불필요한 리컴포지션이 발생할 수 있습니다.partyId를 key로 사용하시면 아이템 변경 시 효율적인 업데이트가 가능합니다.♻️ 제안하는 수정 사항
- items(productPartyListInfo.partySummaries) { party -> + items( + items = productPartyListInfo.partySummaries, + key = { it.partyId } + ) { party ->
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt`:
- Around line 75-76: The onCardClick lambda is only passing title but
GoodsCategoryUiIntent.OnCardClick requires both artistId: Long and title:
String; update GoodsCategoryRoute to forward the received artistId into
GoodsCategoryScreen and change the onCardClick handlers (both the higher-order
lambda in GoodsCategoryScreen and the call site using groupItem.postTitle) to
pass both parameters (title and the artistId) matching the (String, Long) ->
Unit signature, then invoke
viewModel.processIntent(GoodsCategoryUiIntent.OnCardClick(artistId, title)) (or
equivalent ordering used by the intent) so both required fields are provided.
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt`:
- Line 3: 파일 GoodsFilteredPartyListScreen.kt에 남아있는 디버그 로그를 제거하세요: 모든 Log.d 호출을
삭제하고 사용하지 않게 된 import android.util.Log 문도 함께 제거하며, 관련된 함수나 컴포저블(예:
GoodsFilteredPartyListScreen) 내부에서 디버그 출력 대신 필요하면 적절한 로깅(프로덕션 로그 또는 analytics)으로
대체하세요.
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/Contracts.kt`:
- Around line 61-73: The loadState property currently masks original errors by
returning ApiState.Failure(""); update the failure branch to propagate the real
error(s) from productPartyListInfo and/or membersLoadState instead of an empty
string — e.g., if productPartyListInfo is ApiState.Failure return that failure,
else if membersLoadState is ApiState.Failure return that failure, or combine
both messages when both are failures; change the failure branch in the loadState
getter so ApiState.Failure carries the original error details from
productPartyListInfo and membersLoadState.
♻️ Duplicate comments (1)
app/src/main/java/com/poti/android/presentation/user/mypage/MyPageScreen.kt (1)
118-122:onArtistClick콜백이 사용되지 않아 버튼 클릭 시 동작하지 않습니다.현재
BadgeButton의onClick이 빈 람다로 설정되어 있어요. Line 61의 TODO 주석에 따르면 "선택 최애 없을 때만 이동"하는 조건부 로직이 필요해 보입니다. PR 설명에도 바텀시트 작업이 미완료라고 되어 있으니, 해당 작업과 함께 처리해 주시면 될 것 같습니다.
🧹 Nitpick comments (7)
app/src/main/java/com/poti/android/presentation/party/home/component/HomeGoodsSection.kt (1)
62-77: LazyRow 항목에key지정 권장리스트 변경 시 아이템 상태가 뒤섞이거나 불필요한 리컴포지션이 발생할 수 있어요.
items(groupItems, key = { /* 안정적 유니크 키 */ })형태로 키를 지정해 주세요. (예:postId가 있다면 그 값을 사용) 코딩 가이드라인에 따라, ...app/src/main/java/com/poti/android/domain/model/party/ProductPartyList.kt (1)
3-7: Compose 안정성을 위해@Immutable어노테이션 추가를 고려해 주세요.
List는 Compose에서 불안정한(unstable) 타입으로 취급되어 불필요한 리컴포지션을 유발할 수 있습니다. 이 도메인 모델이 UI State에서 직접 사용된다면@Immutable어노테이션을 추가하시는 것을 권장드려요.♻️ 제안 코드
+import androidx.compose.runtime.Immutable + +@Immutable data class ProductPartyList( val partyTitle: String, val artistName: String, val partySummaries: List<PartySummary>, ) +@Immutable data class PartySummary( ... )단, Domain 레이어의 순수성을 유지하려면 Presentation 레이어에 별도 UI 모델을 만들어
@Immutable을 적용하는 방법도 있습니다.app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt (1)
135-142: LazyColumn items에 key 지정이 권장됩니다.
groupItems가 변경/갱신될 때 재사용 및 리컴포지션 안정성을 위해 안정적인 key(예: 고유 ID)를 지정해 주세요.app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt (1)
36-47: 바텀시트 TODO는 릴리즈 전에 트래킹이 필요합니다.두 인텐트 모두 TODO만 있고 동작이 없어서 UX 상 반응이 없습니다. 최소한 이슈/후속 PR로 추적 부탁드립니다. 필요하시면 연결 로직 초안도 도와드릴게요.
app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt (1)
20-20: Hilt ViewModel import가 deprecated 되었습니다.
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel은 deprecated 되었습니다. Modern Android Development 권장 사항에 따라androidx.hilt.navigation.compose.hiltViewModel을 사용해 주세요.♻️ 제안하는 수정 사항
-import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel +import androidx.hilt.navigation.compose.hiltViewModelapp/src/main/java/com/poti/android/presentation/party/goodsfilter/model/Contracts.kt (2)
37-59:@Composablegetter를 UiState에서 분리하는 것을 권장드립니다.
memberFilterText가@Composable get()으로 선언되어 있어 UiState가 Compose 런타임에 의존하게 됩니다. 이로 인해:
- Domain 순수성 원칙에 위배될 수 있습니다
- 단위 테스트 작성이 어려워집니다
- State 클래스의 재사용성이 제한됩니다
Screen Composable에서 직접 처리하거나 별도의
@Composable확장 함수로 분리하시는 것을 권장드립니다.♻️ 제안하는 수정 사항
UiState에서는 순수 데이터만 유지하고, 텍스트 생성 로직은 Composable 확장 함수로 분리해 주세요:
// Contracts.kt - UiState에서 memberFilterText 제거 data class GoodsFilterUiState( val productPartyListInfo: ApiState<ProductPartyList> = ApiState.Loading, val membersLoadState: ApiState<List<FilterMember>> = ApiState.Loading, val displayMembers: List<FilterMember> = emptyList(), val selectedMembers: List<FilterMember> = emptyList(), val goodsPartySortType: PartySortType = PartySortType.LATEST, ) : UiState { val selectedMemberIds: List<Long> get() = selectedMembers.map { it.id } val loadState: ApiState<Unit> get() = // ... 기존 로직 } // Extension.kt 또는 Screen 파일에 추가 `@Composable` fun GoodsFilterUiState.memberFilterText(): String = when { selectedMembers.isEmpty() -> stringResource(R.string.goods_filter_member_select) selectedMembers.size == 1 -> selectedMembers[0].name selectedMembers.size == 2 -> stringResource( R.string.goods_filter_member_two_format, selectedMembers[0].name, selectedMembers[1].name, ) else -> stringResource( R.string.goods_filter_member_more_format, selectedMembers[0].name, selectedMembers[1].name, selectedMembers.size - 2, ) }
22-25:FilterMember에@Stable어노테이션 추가를 고려해 주세요.
FilterMember가List로 사용되고 있으므로, Compose 컴파일러가 안정성을 올바르게 추론하지 못해 불필요한 리컴포지션이 발생할 수 있습니다. 코딩 가이드라인에 따라@Stable또는@Immutable어노테이션을 추가하시면 성능 최적화에 도움이 됩니다.♻️ 제안하는 수정 사항
+import androidx.compose.runtime.Immutable +@Immutable data class FilterMember( val id: Long, val name: String, )
app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt
Outdated
Show resolved
Hide resolved
...rc/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt
Outdated
Show resolved
Hide resolved
| val loadState: ApiState<Unit> | ||
| get() = when { | ||
| productPartyListInfo is ApiState.Loading && | ||
| membersLoadState is ApiState.Loading -> ApiState.Loading | ||
|
|
||
| productPartyListInfo is ApiState.Failure || | ||
| membersLoadState is ApiState.Failure -> ApiState.Failure("") | ||
|
|
||
| productPartyListInfo is ApiState.Success && | ||
| membersLoadState is ApiState.Success -> ApiState.Success(Unit) | ||
|
|
||
| else -> ApiState.Loading | ||
| } |
There was a problem hiding this comment.
에러 정보가 소실되고 있습니다.
ApiState.Failure("")로 처리하면 원래의 에러 메시지가 소실됩니다. 디버깅이나 사용자에게 적절한 에러 메시지를 표시하기 어려워질 수 있으니, 원본 에러 정보를 유지하거나 적절한 에러 메시지를 전달하시는 것을 권장드립니다.
🐛 제안하는 수정 사항
productPartyListInfo is ApiState.Failure ||
- membersLoadState is ApiState.Failure -> ApiState.Failure("")
+ membersLoadState is ApiState.Failure -> {
+ val errorMessage = when {
+ productPartyListInfo is ApiState.Failure -> productPartyListInfo.message
+ membersLoadState is ApiState.Failure -> membersLoadState.message
+ else -> ""
+ }
+ ApiState.Failure(errorMessage)
+ }🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/Contracts.kt`
around lines 61 - 73, The loadState property currently masks original errors by
returning ApiState.Failure(""); update the failure branch to propagate the real
error(s) from productPartyListInfo and/or membersLoadState instead of an empty
string — e.g., if productPartyListInfo is ApiState.Failure return that failure,
else if membersLoadState is ApiState.Failure return that failure, or combine
both messages when both are failures; change the failure branch in the loadState
getter so ApiState.Failure carries the original error details from
productPartyListInfo and membersLoadState.
# Conflicts: # app/src/main/java/com/poti/android/data/remote/service/PartyService.kt
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 (3)
app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt (2)
140-156: LazyColumnitems에key파라미터를 추가해 주세요.
items()함수에key파라미터가 누락되어 있습니다.key가 없으면 리스트 항목이 변경될 때 불필요한 리컴포지션이 발생하고, 항목 순서가 바뀔 때 잘못된 상태가 유지될 수 있습니다.PartySummary의partyId를 key로 사용하시는 것을 권장드립니다.🔧 제안하는 수정 사항
- items(productPartyListInfo.partySummaries) { party -> + items( + items = productPartyListInfo.partySummaries, + key = { it.partyId } + ) { party ->
165-167: Preview 함수명에 오타가 있습니다.
GoodsFilteredPartyListScreenPreveiw에서Preveiw→Preview로 수정이 필요합니다. 작은 오타이지만 코드 검색이나 일관성 측면에서 수정해 주시면 좋겠습니다.✏️ 제안하는 수정 사항
-private fun GoodsFilteredPartyListScreenPreveiw() { +private fun GoodsFilteredPartyListScreenPreview() {app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt (1)
47-52: NavigateToGoodsPartyList에서 effect의 artistId를 무시하고 있어요.카드 클릭으로 전달된
effect.artistId를 사용해야 하는데, 현재는 라우트 파라미터의artistId를 고정으로 사용하고 있습니다. 클릭한 아티스트와 라우트의 아티스트가 다른 경우 잘못된 목록으로 이동하게 돼요. ViewModel에서GoodsCategoryUiEffect.NavigateToGoodsPartyList(intent.artistId, intent.title)로 제대로 전달하고 있으니, 이를 활용해 주세요.수정 제안
- is GoodsCategoryUiEffect.NavigateToGoodsPartyList -> onNavigateToGoodsPartyList(artistId, effect.title) + is GoodsCategoryUiEffect.NavigateToGoodsPartyList -> onNavigateToGoodsPartyList(effect.artistId, effect.title)
♻️ Duplicate comments (1)
app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsFilteredContracts.kt (1)
63-70: 에러 메시지가 소실됩니다.
현재ApiState.Failure("")로 처리되어 원본 에러 정보가 사라집니다. 디버깅/UX를 위해 원본 메시지를 전달하는 편이 안전합니다.🐛 제안 수정
- productPartyListInfo is ApiState.Failure || - membersLoadState is ApiState.Failure -> ApiState.Failure("") + productPartyListInfo is ApiState.Failure || + membersLoadState is ApiState.Failure -> { + val errorMessage = when { + productPartyListInfo is ApiState.Failure -> productPartyListInfo.message + membersLoadState is ApiState.Failure -> membersLoadState.message + else -> "" + } + ApiState.Failure(errorMessage) + }
🧹 Nitpick comments (2)
app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsFilteredContracts.kt (1)
22-35: 필터 상태 클래스의 안정성을 개선하세요.
FilterMember는 단순한 불변 데이터 클래스이므로@Immutable적용을 권장합니다. 다만GoodsFilterUiState의 경우,ApiState<T>가 안정성 어노테이션으로 표시되지 않았고,memberFilterText의@Composable게터는 상태 클래스 내에서 Composable 로직을 수행하는 구조이므로, 다음과 같이 개선하는 것을 권장합니다:
- FilterMember에
@Immutable적용 (좋은 관행)- GoodsFilterUiState의
@Composable게터 리팩토링: 상태 클래스는 순수한 데이터만 보유하고,memberFilterText같은 UI 텍스트 생성 로직은 Composable 함수 또는remember+derivedStateOf내에서 처리하도록 분리이렇게 하면 Compose 안정성이 명확해지고, 불필요한 리컴포지션을 더 효과적으로 방지할 수 있습니다.
app/src/main/java/com/poti/android/presentation/party/home/component/HomeGoodsSection.kt (1)
62-67: LazyRow 아이템에 안정적인 key를 추가하는 것을 권장드립니다.리스트 아이템 재배치/상태 보존을 위해
items(..., key = { ... })형태로 안정적인 식별자를 지정해 주세요. 코딩 가이드라인에 따라.
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/party/goodsfilter/GoodsFilterViewModel.kt`:
- Around line 89-91: There's a typo: the state property is named cacheedSubTitle
but should be cachedSubTitle; update the property name in the assignment in
GoodsFilterViewModel (where productPartyListInfo, cachedTitle, cacheedSubTitle
are set from partyList.partyTitle and partyList.artistName) and also rename the
corresponding declaration/usage in GoodsFilteredContracts (fix the state
definition and any references to cacheedSubTitle to cachedSubTitle) to keep
names consistent and avoid compile errors.
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsFilteredContracts.kt`:
- Around line 26-34: There are typos in the properties of GoodsFilteredContracts
(cacheedSubTitle, bottomSheetSelectedMembersIdices, isMemberBottomSheetToucehd);
rename them to cachedSubTitle, bottomSheetSelectedMembersIndices, and
isMemberBottomSheetTouched respectively in the data class (or contract) and
update all usages/references (call sites, tests, serialization/mapping,
parcelization) to match the new identifiers to avoid compile errors and
mismatches.
🧹 Nitpick comments (6)
app/src/main/res/values/strings.xml (2)
17-21: 중복된 문자열 값이 존재합니다.
action_button_reset(Line 17)과action_button_refresh(Line 21)가 모두 동일한 값 "초기화"를 가지고 있습니다.의도적인 분리라면 무시하셔도 되지만, 동일한 UI 텍스트를 여러 곳에서 사용하는 경우 하나의 string resource를 재사용하시는 것이 Modern Android Development에서 권장하는 방식입니다. 이렇게 하면 향후 번역 작업이나 텍스트 수정 시 유지보수가 용이해집니다.
239-245: 중복된 문자열 리소스가 존재합니다.
goods_filter_member_select(Line 239)와goods_filter_member_select_label(Line 245)가 동일한 값 "멤버 선택"을 가지고 있습니다.두 리소스가 UI에서 다른 용도로 사용되는지 확인해 주세요. 만약 동일한 텍스트를 표시하는 용도라면, 하나의 리소스로 통합하시는 것이 리소스 관리 및 다국어 지원(i18n) 측면에서 효율적입니다.
#!/bin/bash # Description: 두 문자열 리소스의 실제 사용처를 확인합니다. echo "=== goods_filter_member_select 사용처 ===" rg -n "goods_filter_member_select[^_]" --type=xml --type=kotlin echo "" echo "=== goods_filter_member_select_label 사용처 ===" rg -n "goods_filter_member_select_label" --type=xml --type=kotlinapp/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt (1)
24-25:savedStateHandle.toRoute<GoodsPartyList>()가 두 번 호출되고 있습니다.동일한 호출이 두 번 수행되어 약간의 비효율이 발생합니다. 한 번만 호출하여 결과를 재사용하시는 것을 권장드립니다.
♻️ 제안하는 개선안
`@HiltViewModel` class GoodsFilterViewModel `@Inject` constructor( private val artistRepository: ArtistRepository, private val partyRepository: PartyRepository, savedStateHandle: SavedStateHandle, ) : BaseViewModel<GoodsFilterUiState, GoodsFilterUiIntent, GoodsFilterUiEffect>( initialState = GoodsFilterUiState(), ) { - private val artistId: Long = savedStateHandle.toRoute<GoodsPartyList>().artistId - private val title: String = savedStateHandle.toRoute<GoodsPartyList>().title + private val route: GoodsPartyList = savedStateHandle.toRoute<GoodsPartyList>() + private val artistId: Long = route.artistId + private val title: String = route.titleapp/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsFilteredContracts.kt (1)
39-61:@Composablegetter가 UiState 내부에 있습니다.
memberFilterText프로퍼티가@Composablegetter를 사용하고 있어, UiState가 Compose 런타임에 의존하게 됩니다. MVI 패턴에서 UiState는 순수한 데이터 클래스로 유지하고, 문자열 포맷팅 로직은 UI 레이어(Composable 함수 내부)에서 처리하시는 것이 Clean Architecture 원칙에 더 부합합니다.대안으로
selectedMembers만 UiState에 유지하고, 화면에서stringResource를 사용하여 텍스트를 생성하시는 방법을 고려해 주세요.♻️ 제안하는 개선안
UiState에서
memberFilterText를 제거하고, UI 레이어에서 직접 포맷팅:// GoodsFilteredPartyListScreen.kt 내부 val memberFilterText = when { uiState.selectedMembers.isEmpty() -> stringResource(R.string.goods_filter_member_select) uiState.selectedMembers.size == 1 -> uiState.selectedMembers[0].name uiState.selectedMembers.size == 2 -> stringResource( R.string.goods_filter_member_two_format, uiState.selectedMembers[0].name, uiState.selectedMembers[1].name, ) else -> stringResource( R.string.goods_filter_member_more_format, uiState.selectedMembers[0].name, uiState.selectedMembers[1].name, uiState.selectedMembers.size - 2, ) }app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt (2)
43-49:artistId파라미터가 사용되지 않고 있습니다.
GoodsFilteredPartyListRoute의artistId파라미터가 함수 내부에서 사용되지 않습니다. ViewModel이SavedStateHandle에서 직접artistId를 추출하므로 이 파라미터는 불필요해 보입니다. 제거하시거나, 네비게이션 그래프와의 일관성을 위해 유지해야 하는 경우@Suppress("UNUSED_PARAMETER")를 추가하시는 것을 고려해 주세요.
165-181:LazyColumn의items에key파라미터가 누락되었습니다.
items함수에key파라미터를 지정하지 않으면, 리스트 아이템이 변경될 때 불필요한 리컴포지션이 발생할 수 있습니다.partyId를 key로 사용하시면 성능 최적화에 도움이 됩니다. 코딩 가이드라인에서 언급된 Recomposition 방지 원칙에 해당합니다.♻️ 제안하는 개선안
- items(productPartyListInfo.partySummaries) { party -> + items( + items = productPartyListInfo.partySummaries, + key = { it.partyId }, + ) { party -> PartyCard( potId = party.partyId,
| productPartyListInfo = ApiState.Success(partyList), | ||
| cachedTitle = partyList.partyTitle, | ||
| cacheedSubTitle = partyList.artistName, |
There was a problem hiding this comment.
오타 발견: cacheedSubTitle
cacheedSubTitle에 오타가 있습니다. cachedSubTitle로 수정하시는 것이 좋겠습니다. 이 오타는 GoodsFilteredContracts.kt의 상태 정의에도 동일하게 존재하므로 함께 수정이 필요합니다.
🐛 제안하는 수정 사항
- cacheedSubTitle = partyList.artistName,
+ cachedSubTitle = partyList.artistName,🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt`
around lines 89 - 91, There's a typo: the state property is named
cacheedSubTitle but should be cachedSubTitle; update the property name in the
assignment in GoodsFilterViewModel (where productPartyListInfo, cachedTitle,
cacheedSubTitle are set from partyList.partyTitle and partyList.artistName) and
also rename the corresponding declaration/usage in GoodsFilteredContracts (fix
the state definition and any references to cacheedSubTitle to cachedSubTitle) to
keep names consistent and avoid compile errors.
| val cacheedSubTitle: String = "", | ||
| val membersLoadState: ApiState<List<Member>> = ApiState.Loading, | ||
| val displayMembers: List<Member> = emptyList(), | ||
| val selectedMembers: List<Member> = emptyList(), | ||
| val goodsPartySortType: PartySortType = PartySortType.HOT, | ||
| val isMemberFilterBottomSheetVisible: Boolean = false, | ||
| val isSortFilterBottomSheetVisible: Boolean = false, | ||
| val bottomSheetSelectedMembersIdices: Set<Int> = setOf(), | ||
| val isMemberBottomSheetToucehd: Boolean = false, |
There was a problem hiding this comment.
여러 프로퍼티에 오타가 있습니다.
다음 프로퍼티들에 오타가 발견되었습니다:
- Line 26:
cacheedSubTitle→cachedSubTitle - Line 33:
bottomSheetSelectedMembersIdices→bottomSheetSelectedMembersIndices - Line 34:
isMemberBottomSheetToucehd→isMemberBottomSheetTouched
🐛 제안하는 수정 사항
data class GoodsFilterUiState(
val productPartyListInfo: ApiState<ProductPartyList> = ApiState.Loading,
val cachedTitle: String = "",
- val cacheedSubTitle: String = "",
+ val cachedSubTitle: String = "",
val membersLoadState: ApiState<List<Member>> = ApiState.Loading,
val displayMembers: List<Member> = emptyList(),
val selectedMembers: List<Member> = emptyList(),
val goodsPartySortType: PartySortType = PartySortType.HOT,
val isMemberFilterBottomSheetVisible: Boolean = false,
val isSortFilterBottomSheetVisible: Boolean = false,
- val bottomSheetSelectedMembersIdices: Set<Int> = setOf(),
- val isMemberBottomSheetToucehd: Boolean = false,
+ val bottomSheetSelectedMembersIndices: Set<Int> = setOf(),
+ val isMemberBottomSheetTouched: Boolean = false,
) : UiState {📝 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.
| val cacheedSubTitle: String = "", | |
| val membersLoadState: ApiState<List<Member>> = ApiState.Loading, | |
| val displayMembers: List<Member> = emptyList(), | |
| val selectedMembers: List<Member> = emptyList(), | |
| val goodsPartySortType: PartySortType = PartySortType.HOT, | |
| val isMemberFilterBottomSheetVisible: Boolean = false, | |
| val isSortFilterBottomSheetVisible: Boolean = false, | |
| val bottomSheetSelectedMembersIdices: Set<Int> = setOf(), | |
| val isMemberBottomSheetToucehd: Boolean = false, | |
| val cachedSubTitle: String = "", | |
| val membersLoadState: ApiState<List<Member>> = ApiState.Loading, | |
| val displayMembers: List<Member> = emptyList(), | |
| val selectedMembers: List<Member> = emptyList(), | |
| val goodsPartySortType: PartySortType = PartySortType.HOT, | |
| val isMemberFilterBottomSheetVisible: Boolean = false, | |
| val isSortFilterBottomSheetVisible: Boolean = false, | |
| val bottomSheetSelectedMembersIndices: Set<Int> = setOf(), | |
| val isMemberBottomSheetTouched: Boolean = false, |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsFilteredContracts.kt`
around lines 26 - 34, There are typos in the properties of
GoodsFilteredContracts (cacheedSubTitle, bottomSheetSelectedMembersIdices,
isMemberBottomSheetToucehd); rename them to cachedSubTitle,
bottomSheetSelectedMembersIndices, and isMemberBottomSheetTouched respectively
in the data class (or contract) and update all usages/references (call sites,
tests, serialization/mapping, parcelization) to match the new identifiers to
avoid compile errors and mismatches.
There was a problem hiding this comment.
@doyeon0307 감사합니다! 😊 코드 리뷰에서 이런 오타를 잡아내는 것도 제 역할이에요. 프로퍼티명의 오타는 나중에 다른 개발자분들이 사용하실 때 혼란을 줄 수 있어서 미리 수정하시는 게 좋습니다!
(੭˃ᴗ˂)੭
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/party/goodsfilter/GoodsFilterViewModel.kt`:
- Around line 24-107: loadPartyList() can run concurrently and an
earlier-response may overwrite a later one; fix by making only the latest
request able to update state: add a scoped cancellable Job (e.g., a property
currentLoadJob: Job?) or a monotonically increasing requestId (e.g.,
loadRequestCounter: Long) on the ViewModel, cancel the previous Job or increment
and capture the current requestId at the start of loadPartyList(), then before
applying updates in the onSuccess/onFailure handlers check that the captured
requestId matches the latest counter (or that the Job is still active); update
launchScope usage in loadPartyList() to assign to currentLoadJob (or use the
requestId) so duplicated calls will cancel/ignore older responses and only the
latest result from loadPartyList() updates the UI state.
🧹 Nitpick comments (1)
app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt (1)
126-138: 선택 매핑이 O(n²)이고 객체 동등성에 의존합니다.
멤버 수가 늘어나면 비용이 커지고, 객체 인스턴스가 달라지면 선택 복원이 실패할 수 있습니다.memberId기반으로 조회하는 방식이 더 안전합니다.♻️ 제안 수정
private fun refreshMemberSelectBottomSheet() { - val newSelectedIndices = uiState.value.displayMembers.mapIndexed { index, member -> - if (member in uiState.value.selectedMembers) index else null - } + val selectedMemberIds = uiState.value.selectedMembers.map { it.memberId }.toSet() + val newSelectedIndices = uiState.value.displayMembers.mapIndexed { index, member -> + if (member.memberId in selectedMemberIds) index else null + } .filterNotNull() .toSet() updateState { copy( bottomSheetSelectedMembersIdices = newSelectedIndices, isMemberBottomSheetToucehd = false, ) } }
| private val artistId: Long = savedStateHandle.toRoute<GoodsPartyList>().artistId | ||
| private val title: String = savedStateHandle.toRoute<GoodsPartyList>().title | ||
|
|
||
| override fun processIntent(intent: GoodsFilterUiIntent) { | ||
| when (intent) { | ||
| GoodsFilterUiIntent.LoadGoodsPots -> loadGoodsPots() | ||
| GoodsFilterUiIntent.OnBackClick -> sendEffect(GoodsFilterUiEffect.NavigateBack) | ||
| GoodsFilterUiIntent.OnFloatingClick -> sendEffect(GoodsFilterUiEffect.NavigateToPartyCreate) | ||
| is GoodsFilterUiIntent.OnPartyClick -> | ||
| sendEffect(GoodsFilterUiEffect.NavigateToPartyDetail(intent.partyId)) | ||
| GoodsFilterUiIntent.OnMemberFilterClick -> { | ||
| // TODO: [예림] 바텀시트 open | ||
| } | ||
| is GoodsFilterUiIntent.OnMembersSelect -> { | ||
| updateState { copy(selectedMember = intent.members) } | ||
| } | ||
| GoodsFilterUiIntent.OnSortFilterClick -> { | ||
| // TODO: [예림] 바텀시트 open | ||
| } | ||
| is GoodsFilterUiIntent.OnSortSelect -> { | ||
| updateState { copy(goodsSortFilter = intent.sort) } | ||
| init { | ||
| fetchArtistMembers() | ||
| loadPartyList() | ||
| } | ||
|
|
||
| override fun processIntent(intent: GoodsFilterUiIntent) { | ||
| when (intent) { | ||
| GoodsFilterUiIntent.LoadGoodsPots -> loadPartyList() | ||
| GoodsFilterUiIntent.OnBackClick -> sendEffect(GoodsFilterUiEffect.NavigateBack) | ||
| GoodsFilterUiIntent.OnFloatingClick -> sendEffect(GoodsFilterUiEffect.NavigateToPartyCreate) | ||
| is GoodsFilterUiIntent.OnPartyClick -> sendEffect(GoodsFilterUiEffect.NavigateToPartyDetail(intent.partyId)) | ||
| GoodsFilterUiIntent.OnMemberFilterClick -> { | ||
| refreshMemberSelectBottomSheet() | ||
| updateState { copy(isMemberFilterBottomSheetVisible = true) } | ||
| } | ||
|
|
||
| is GoodsFilterUiIntent.OnMemberSelect -> { | ||
| onBottomSheetMemberChanged(intent.index) | ||
| } | ||
|
|
||
| GoodsFilterUiIntent.OnSortFilterClick -> { | ||
| updateState { copy(isSortFilterBottomSheetVisible = true) } | ||
| } | ||
|
|
||
| GoodsFilterUiIntent.CloseSortFilterBottomSheet -> { | ||
| updateState { copy(isSortFilterBottomSheetVisible = false) } | ||
| } | ||
|
|
||
| is GoodsFilterUiIntent.OnSortSelect -> { | ||
| updateState { copy(goodsPartySortType = intent.sort, isSortFilterBottomSheetVisible = false) } | ||
| loadPartyList() | ||
| } | ||
|
|
||
| GoodsFilterUiIntent.CloseMemberFilterBottomSheet -> updateState { copy(isMemberFilterBottomSheetVisible = false) } | ||
|
|
||
| GoodsFilterUiIntent.OnMemberFilterDone -> saveSelectedMember() | ||
|
|
||
| GoodsFilterUiIntent.OnMemberFilterRefresh -> { | ||
| if (uiState.value.bottomSheetSelectedMembersIdices.isNotEmpty()) { | ||
| clearSelectedMembers() | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun loadPartyList() = launchScope { | ||
| val currentState = uiState.value | ||
| val sort = currentState.goodsPartySortType.request | ||
| val memberIds = if (currentState.selectedMembers.isNotEmpty()) { | ||
| currentState.selectedMembers.map { it.memberId } | ||
| } else { | ||
| null | ||
| } | ||
|
|
||
| private fun loadGoodsPots() = launchScope { | ||
| updateState { copy(productPartyListInfo = ApiState.Loading) } | ||
|
|
||
| partyRepository.getProductPartyList( | ||
| page = 0, | ||
| size = 10, | ||
| title = title, | ||
| artistId = artistId, | ||
| sort = sort, | ||
| memberIds = memberIds, | ||
| ).onSuccess { partyList -> | ||
| updateState { | ||
| copy( | ||
| productPartyListInfo = ApiState.Success(partyList), | ||
| cachedTitle = partyList.partyTitle, | ||
| cachedSubTitle = partyList.artistName, | ||
| ) | ||
| } | ||
| }.onFailure { throwable -> | ||
| updateState { | ||
| copy( | ||
| partyListInfo = ApiState.Success(dummyPartyList), | ||
| membersLoadState = ApiState.Success(emptyList()), | ||
| productPartyListInfo = ApiState.Failure( | ||
| throwable.message ?: "Failed", | ||
| ), | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
연속 요청 시 오래된 응답이 최신 상태를 덮을 수 있습니다.
필터를 빠르게 변경하면 loadPartyList()가 중복 호출되고, 먼저 시작한 요청이 더 늦게 끝나면서 최신 필터 결과를 덮을 위험이 있습니다. 요청 버전 체크(또는 Job 취소)로 최신 요청만 반영되게 해주세요.
✅ 제안 수정 (요청 버전 체크)
private val artistId: Long = savedStateHandle.toRoute<GoodsPartyList>().artistId
private val title: String = savedStateHandle.toRoute<GoodsPartyList>().title
+ private var loadPartyListRequestId = 0
private fun loadPartyList() = launchScope {
+ val requestId = ++loadPartyListRequestId
val currentState = uiState.value
val sort = currentState.goodsPartySortType.request
val memberIds = if (currentState.selectedMembers.isNotEmpty()) {
currentState.selectedMembers.map { it.memberId }
} else {
null
}
updateState { copy(productPartyListInfo = ApiState.Loading) }
partyRepository.getProductPartyList(
page = 0,
size = 10,
title = title,
artistId = artistId,
sort = sort,
memberIds = memberIds,
).onSuccess { partyList ->
+ if (requestId != loadPartyListRequestId) return@onSuccess
updateState {
copy(
productPartyListInfo = ApiState.Success(partyList),
cachedTitle = partyList.partyTitle,
cachedSubTitle = partyList.artistName,
)
}
}.onFailure { throwable ->
+ if (requestId != loadPartyListRequestId) return@onFailure
updateState {
copy(
productPartyListInfo = ApiState.Failure(
throwable.message ?: "Failed",
),
)
}
}
}🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilterViewModel.kt`
around lines 24 - 107, loadPartyList() can run concurrently and an
earlier-response may overwrite a later one; fix by making only the latest
request able to update state: add a scoped cancellable Job (e.g., a property
currentLoadJob: Job?) or a monotonically increasing requestId (e.g.,
loadRequestCounter: Long) on the ViewModel, cancel the previous Job or increment
and capture the current requestId at the start of loadPartyList(), then before
applying updates in the onSuccess/onFailure handlers check that the captured
requestId matches the latest counter (or that the Job is still active); update
launchScope usage in loadPartyList() to assign to currentLoadJob (or use the
requestId) so duplicated calls will cancel/ignore older responses and only the
latest result from loadPartyList() updates the UI state.
|
✅ Ktlint check passed. Run: https://github.com/team-poti/POTI-ANDROID/actions/runs/21247696095 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/component/FilteredSortBottomSheet.kt`:
- Around line 46-51: onClick 핸들러가 sheetState.hide()를 비동기 호출만 하고 숨김 완료 후 부모에 알리는
onDismissRequest()를 호출하지 않아 상태 불일치가 발생합니다; 수정은 FilteredSortBottomSheet의 onClick
람다에서 scope.launch { sheetState.hide() }를 그냥 호출하는 대신 launch의 반환 Job을 받아 내부에서
sheetState.hide()를 실행하고 그 Job에 대해 invokeOnCompletion { onDismissRequest() }를 등록해
시트가 완전히 숨겨진 뒤 onDismissRequest()가 호출되도록 하세요 (onSelect 호출 순서는 유지).
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/component/GoodsSortBottomSheet.kt`:
- Line 3: Remove the unused imports in GoodsSortBottomSheet.kt that are causing
ktlint failures: specifically delete the import androidx.annotation.StringRes
and the other unused import on line 17; ensure the class GoodsSortBottomSheet
compiles and no references to those annotations remain (if used, replace usages
appropriately), then re-run ktlint/CI to confirm the lint error is resolved.
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt`:
- Line 51: Change the navigation call to use the clicked card's artistId from
the effect by replacing the current artistId argument with effect.artistId in
the GoodsCategoryUiEffect.NavigateToGoodsPartyList handling (so
onNavigateToGoodsPartyList receives effect.artistId, effect.title); also add a
stable key to the LazyColumn items call by passing key = { it.id } when
iterating goodsCategory.groupItems to improve recomposition stability (refer to
items(goodsCategory.groupItems) and groupItem.id).
♻️ Duplicate comments (1)
app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsFilteredContracts.kt (1)
33-34: 프로퍼티명에 오타가 있습니다.이전 리뷰에서 지적된 오타가 아직 수정되지 않았습니다:
bottomSheetSelectedMembersIdices→bottomSheetSelectedMembersIndicesisMemberBottomSheetToucehd→isMemberBottomSheetTouched이 오타들이 다른 파일(
GoodsFilteredPartyListScreen.kt)에서도 참조되고 있으므로, 일괄 수정이 필요합니다.♻️ 제안하는 수정 사항
- val bottomSheetSelectedMembersIdices: Set<Int> = setOf(), - val isMemberBottomSheetToucehd: Boolean = false, + val bottomSheetSelectedMembersIndices: Set<Int> = setOf(), + val isMemberBottomSheetTouched: Boolean = false,
🧹 Nitpick comments (5)
app/src/main/java/com/poti/android/presentation/party/goodsfilter/component/FilteredSortBottomSheet.kt (2)
41-41: 불필요한.padding()호출이에요.인자 없이 호출된
.padding()은 아무런 효과가 없으므로 제거하시는 것이 좋을 것 같아요.♻️ 수정 제안
PotiBottomSheet( onDismissRequest = onDismissRequest, sheetState = sheetState, - modifier = modifier.padding(), + modifier = modifier, ) {
44-44: 매 리컴포지션마다 새로운ImmutableList가 생성되고 있어요.
options.map { stringResource(it) }.toImmutableList()가 리컴포지션될 때마다 새 리스트를 생성하고 있어요.stringResource는 Composable 함수라서remember블록 안에서 호출할 수 없지만, 현재 구조에서는options(line 29-31)를 resource ID로 remember하고, 여기서 다시 매핑하는 방식이 비효율적이에요.♻️ 수정 제안
options를 아예 제거하고,sortTypes만 사용하여 직접 매핑하는 방식으로 단순화할 수 있어요:val sortTypes = remember { FilteredSortType.entries.toList() } - val options = remember { - sortTypes.map { it.displayRes }.toImmutableList() - } - val selectedIndex = sortTypes.indexOf(selectedSortType) val sheetState = rememberModalBottomSheetState() @@ ... @@ PotiListRadio( - options = options.map { stringResource(it) }.toImmutableList(), + options = sortTypes.map { stringResource(it.displayRes) }.toImmutableList(), selectedOptionIndex = selectedIndex,중간
options변수를 제거하면 코드가 더 간결해지고 의도가 명확해져요.app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt (1)
135-148:LazyColumn의items에key파라미터가 누락되었습니다.리스트 아이템에 고유한
key를 지정하지 않으면 불필요한 리컴포지션이 발생할 수 있습니다.groupItem에 고유 식별자가 있다면key파라미터를 추가해 주세요.♻️ 제안하는 수정 사항
- items(goodsCategory.groupItems) { groupItem -> + items( + items = goodsCategory.groupItems, + key = { it.artistId } // 또는 다른 고유 식별자 + ) { groupItem ->app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsFilteredContracts.kt (1)
39-61:@Composablegetter를 data class 내부에서 사용하는 것은 권장되지 않습니다.
memberFilterText프로퍼티가@Composablegetter로 정의되어 있습니다. Data class는 순수한 데이터 컨테이너로 유지하는 것이 좋습니다. 이 로직은 UI 레이어(Screen 또는 별도의 Composable 함수)로 이동시키거나, 리소스 ID를 반환하는 방식으로 변경하는 것을 권장드립니다.♻️ 제안하는 수정 사항
Extension function으로 분리하거나, sealed class로 필터 상태를 표현하는 방법을 고려해 주세요:
// Extension.kt 또는 별도 파일에 정의 `@Composable` fun GoodsFilterUiState.memberFilterText(): String = when { selectedMembers.isEmpty() -> stringResource(R.string.goods_filter_member_select) selectedMembers.size == 1 -> selectedMembers[0].name selectedMembers.size == 2 -> stringResource( R.string.goods_filter_member_two_format, selectedMembers[0].name, selectedMembers[1].name, ) else -> stringResource( R.string.goods_filter_member_more_format, selectedMembers[0].name, selectedMembers[1].name, selectedMembers.size - 2, ) }app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsFilteredPartyListScreen.kt (1)
165-181:LazyColumn의items에key파라미터가 누락되었습니다.
partySummaries리스트를 렌더링할 때 고유한key를 지정하지 않으면, 리스트 변경 시 불필요한 리컴포지션이 발생하고 애니메이션이 올바르게 동작하지 않을 수 있습니다.♻️ 제안하는 수정 사항
- items(productPartyListInfo.partySummaries) { party -> + items( + items = productPartyListInfo.partySummaries, + key = { it.partyId } + ) { party ->
| onClick = { index -> | ||
| onSelect(sortTypes[index]) | ||
| scope.launch { | ||
| sheetState.hide() | ||
| } | ||
| }, |
There was a problem hiding this comment.
sheetState.hide() 이후 onDismissRequest()가 호출되지 않아요.
PotiBottomSheet 내부 패턴(relevant snippet 참조)에서는 sheetState.hide() 후 invokeOnCompletion을 사용하여 onDismissRequest()를 호출하고 있어요. 현재 코드에서는 시트가 숨겨진 후에도 부모 컴포넌트가 이를 인지하지 못해 상태 불일치가 발생할 수 있어요.
🐛 수정 제안
onClick = { index ->
onSelect(sortTypes[index])
scope.launch {
sheetState.hide()
+ }.invokeOnCompletion {
+ if (!sheetState.isVisible) {
+ onDismissRequest()
+ }
}
},🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/component/FilteredSortBottomSheet.kt`
around lines 46 - 51, onClick 핸들러가 sheetState.hide()를 비동기 호출만 하고 숨김 완료 후 부모에 알리는
onDismissRequest()를 호출하지 않아 상태 불일치가 발생합니다; 수정은 FilteredSortBottomSheet의 onClick
람다에서 scope.launch { sheetState.hide() }를 그냥 호출하는 대신 launch의 반환 Job을 받아 내부에서
sheetState.hide()를 실행하고 그 Job에 대해 invokeOnCompletion { onDismissRequest() }를 등록해
시트가 완전히 숨겨진 뒤 onDismissRequest()가 호출되도록 하세요 (onSelect 호출 순서는 유지).
.../main/java/com/poti/android/presentation/party/goodsfilter/component/GoodsSortBottomSheet.kt
Outdated
Show resolved
Hide resolved
| GoodsCategoryUiEffect.NavigateBack -> onPopBackStack() | ||
| GoodsCategoryUiEffect.NavigateToPartyCreate -> onNavigateToPartyCreate() | ||
| GoodsCategoryUiEffect.NavigateToGoodsFilter -> onNavigateToGoodsPartyList(artistId) | ||
| is GoodsCategoryUiEffect.NavigateToGoodsPartyList -> onNavigateToGoodsPartyList(artistId, effect.title) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.ktRepository: team-poti/POTI-ANDROID
Length of output: 7859
🏁 Script executed:
# GoodsCategoryUiEffect 정의 확인
fd "GoodsCategoryUiEffect" --type kt -exec cat -n {} +Repository: team-poti/POTI-ANDROID
Length of output: 235
🏁 Script executed:
# GoodsLargeCard 컴포넌트 확인
fd "GoodsLargeCard" --type kt -exec cat -n {} +Repository: team-poti/POTI-ANDROID
Length of output: 235
🏁 Script executed:
# Search for GoodsCategoryUiEffect definition
find . -name "*.kt" -type f | xargs grep -l "GoodsCategoryUiEffect" | head -5Repository: team-poti/POTI-ANDROID
Length of output: 348
🏁 Script executed:
# Search for GoodsLargeCard definition
find . -name "*.kt" -type f | xargs grep -l "fun GoodsLargeCard"Repository: team-poti/POTI-ANDROID
Length of output: 154
🏁 Script executed:
cat -n ./app/src/main/java/com/poti/android/presentation/party/goodsfilter/model/GoodsCategoryUiState.ktRepository: team-poti/POTI-ANDROID
Length of output: 2040
🏁 Script executed:
cat -n ./app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryViewModel.ktRepository: team-poti/POTI-ANDROID
Length of output: 3823
🏁 Script executed:
cat -n ./app/src/main/java/com/poti/android/presentation/party/home/component/GoodsLargeCard.ktRepository: team-poti/POTI-ANDROID
Length of output: 6761
effect.artistId를 사용해야 하며, LazyColumn에서 key 파라미터를 추가하세요.
현재 51번 줄에서 route의 artistId를 사용하고 있지만, NavigateToGoodsPartyList effect에는 클릭된 카드의 artistId가 포함되어 있습니다. 각 카드마다 다른 groupItem.artistId를 가질 수 있으므로 effect.artistId를 사용해야 합니다.
수정 예시
is GoodsCategoryUiEffect.NavigateToGoodsPartyList -> onNavigateToGoodsPartyList(effect.artistId, effect.title)추가로 135번 줄의 items() 호출에서 key 파라미터가 누락되어 있습니다. 리컴포지션 최적화를 위해 다음과 같이 수정하세요:
items(goodsCategory.groupItems, key = { it.id }) { groupItem ->🤖 Prompt for AI Agents
In
`@app/src/main/java/com/poti/android/presentation/party/goodsfilter/GoodsCategoryScreen.kt`
at line 51, Change the navigation call to use the clicked card's artistId from
the effect by replacing the current artistId argument with effect.artistId in
the GoodsCategoryUiEffect.NavigateToGoodsPartyList handling (so
onNavigateToGoodsPartyList receives effect.artistId, effect.title); also add a
stable key to the LazyColumn items call by passing key = { it.id } when
iterating goodsCategory.groupItems to improve recomposition stability (refer to
items(goodsCategory.groupItems) and groupItem.id).
3 participants
Related issue 🛠️
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
새로운 기능
개선
문구
✏️ Tip: You can customize this high-level summary in your review settings.