[REFACTOR/#374] 물품 상세 화면 뷰모델 리팩토링 #375
Conversation
- 초기 상태를 뷰모델의 init 블록으로 초기화 - 좋아요를 이벤트 형식으로 수집하고, 중복 서버 통신을 방지 - 좋아요 상태 및 좋아요 수의 갱신 로직을 합침 - 좋아요 유즈케이스를 더욱 직관적으로 수정
- 상태 업데이트 로직 안정화 - 불필요한 예외처리 제거 - 함수 책임 분리 강화
auto-assign
bot
requested review from
chrin05,
jangsjw,
jm991014 and
yeonjeen
WalkthroughProductDetail 기능 리팩토링으로 새로운 SetInterestUseCase 도입 및 관심 상품 처리 흐름 개선. ProductDetailScreen에서 라이프사이클 기반 상태 관리 제거 및 콜백 기반 위임으로 단순화. ProductDetailViewModel에서 관심 상품 처리를 흐름 기반으로 재구성하여 디바운싱 및 중복 제거 로직 적용. MainScreen 네비게이션 그래프 패딩 조정. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
feature/detail/src/main/java/com/napzak/market/detail/ProductDetailScreen.kt (1)
143-151: 콜백 파라미터가 무시되고 있습니다.
ProductDetailBottomBar의LikeButton은 이미 클릭 시onLikeButtonClick(!isLiked)를 호출하여 토글된 값을 전달합니다. 하지만 여기서 전달하는 람다는 해당 파라미터를 무시하고!isInterested를 다시 계산합니다.
isLiked == isInterested이므로 동작은 정확하지만, 코드 가독성을 위해 파라미터를 직접 전달하는 것이 좋습니다.ProductDetailBottomBar( isLiked = isInterested, onChatButtonClick = { onChatButtonClick(productId) }, - onLikeButtonClick = { onLikeButtonClick(!isInterested) }, + onLikeButtonClick = onLikeButtonClick, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
domain/interest/src/main/java/com/napzak/market/interest/usecase/SetInterestUseCase.kt(1 hunks)feature/detail/src/main/java/com/napzak/market/detail/ProductDetailScreen.kt(7 hunks)feature/detail/src/main/java/com/napzak/market/detail/ProductDetailViewModel.kt(3 hunks)feature/main/src/main/java/com/napzak/market/main/MainScreen.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-02T08:42:53.237Z
Learnt from: boiledeggg
Repo: napzakmarket/Napzak-Android PR: 367
File: feature/chat/src/main/java/com/napzak/market/chat/chatroom/ChatRoomScreen.kt:87-88
Timestamp: 2025-11-02T08:42:53.237Z
Learning: In the Napzak-Android project, when ChatRoomViewModel exposes chatMessageFlow as StateFlow<Flow<PagingData<ReceiveMessage<*>>>>, the correct pattern in ChatRoomScreen is to first collect it with collectAsStateWithLifecycle() to get the inner Flow<PagingData<...>>, then call collectAsLazyPagingItems() on that result. This nested StateFlow structure is used because the paging flow depends on dynamic state (e.g., current room ID).
Applied to files:
feature/detail/src/main/java/com/napzak/market/detail/ProductDetailViewModel.ktfeature/detail/src/main/java/com/napzak/market/detail/ProductDetailScreen.kt
📚 Learning: 2025-08-12T07:18:00.700Z
Learnt from: jm991014
Repo: napzakmarket/Napzak-Android PR: 319
File: feature/detail/src/main/java/com/napzak/market/detail/component/bottomsheet/ProductBottomSheet.kt:32-34
Timestamp: 2025-08-12T07:18:00.700Z
Learning: `ProductBottomSheet`과 같은 `DragHandleBottomSheet` 기반 컴포넌트에서는 `navigationBarsPadding()`을 사용하지 않고 고정 bottom padding (28.dp)을 사용한다. ModalBottomSheet는 시스템 내비게이션 바 위에 플로팅되므로 navigationBarsPadding이 불필요하다.
Applied to files:
feature/detail/src/main/java/com/napzak/market/detail/ProductDetailScreen.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
feature/main/src/main/java/com/napzak/market/main/MainScreen.kt (1)
307-307: LGTM!Product detail 화면의 패딩 책임을 네비게이션 그래프 레벨에서 상세 화면 자체로 이동한 것은 적절합니다.
ProductDetailScreen.kt에서systemBarsPadding()을 직접 적용하도록 변경되어 일관성 있게 동작합니다.feature/detail/src/main/java/com/napzak/market/detail/ProductDetailScreen.kt (1)
154-156: LGTM!네비게이션 그래프에서 패딩을 제거하고
systemBarsPadding()을 Scaffold에 직접 적용한 것은 적절한 변경입니다. 상세 화면이 자체적으로 시스템 바 인셋을 처리합니다.feature/detail/src/main/java/com/napzak/market/detail/ProductDetailViewModel.kt (4)
93-100: LGTM! debounce와 distinctUntilChanged를 활용한 좋은 패턴입니다.
_interestFlow에 debounce와 distinctUntilChanged를 적용하여 불필요한 API 호출을 방지하고, 낙관적 UI 업데이트와 실제 서버 동기화를 분리한 설계가 잘 되어 있습니다.
161-168: 삭제 후 사이드 이펙트 순서 확인 필요
NavigateUp이ShowToast보다 먼저 전송됩니다. 화면이 먼저 닫히면 토스트가 표시되지 않거나, 이전 화면에서 표시될 수 있습니다. 의도한 동작인지 확인해 주세요.순서를 바꾸거나 토스트 표시 후 딜레이를 추가하는 것을 고려해 보세요.
102-105: LGTM!
updateIsInterested가 UI 상태를 즉시 업데이트하고 사이드 이펙트를 트리거하는 명확한 책임 분리가 잘 되어 있습니다.
65-71: LGTM!init 블록에서 초기 데이터 로드를 처리하여 UI 측의
LifecycleResumeEffect기반 초기화 로직을 제거한 것은 ViewModel의 책임에 맞는 적절한 변경입니다.productId가 null인 경우의 에러 처리도 잘 되어 있습니다.domain/interest/src/main/java/com/napzak/market/interest/usecase/SetInterestUseCase.kt (1)
6-13: I'll verify the original review comment by checking whether the oldSetInterestProductUseCasehas been properly removed or is no longer used in the codebase.
<function_calls>
#!/bin/bashSearch for all references to SetInterestProductUseCase
echo "=== Searching for SetInterestProductUseCase references ==="
rg -n "SetInterestProductUseCase" --type kotlin -A 2 -B 2echo -e "\n=== Checking if SetInterestProductUseCase file exists ==="
fd -t f "SetInterestProductUseCase" --type kotlinecho -e "\n=== Searching in all file types ==="
rg -n "SetInterestProductUseCase" -A 2 -B 2 2>/dev/null | head -100
</function_calls>
Related issue 🛠
Work Description ✏️
컴포지션 단계에서 상태 초기화하는 책임을 뷰모델로 이전.
LifecycleResumeEffect를 활용해 UI에서 물품 정보를 초기화하던 로직을 제거init블럭 내부에서 초기 상태를 불러오도록 수정 (테스트 코드와의 tradeoff)좋아요를 상태 관리에서 이벤트 관리 방식으로 수정
좋아요 로직 수정
기타 코드 리팩터링
Uncompleted Tasks 😅
To Reviewers 📢