Conversation
Walkthrough로그인 후 백스택 정리 대상 변경, 홈 화면 레이아웃을 LazyColumn 내로 재구성해 새로고침/낙관적 투표 흐름을 도입(롤백 지원), 스낵바 하단 패딩 적용, 회원 탈퇴 다이얼로그 콜백 노출, 업로드 중복 제출 방지 로직 추가. Changes
Sequence DiagramsequenceDiagram
participant User as 사용자
participant UI as HomeScreen UI
participant VM as HomeViewModel
participant Cache as LocalState
participant Server as Backend(API)
rect rgba(200,200,255,0.5)
Note over UI,VM: 사용자가 투표를 시도
end
User->>UI: 투표 클릭
UI->>VM: handleVote(itemId, choice)
VM->>VM: 소유자/종료/중복 검사
alt 검사 실패
VM->>UI: 에러 이벤트 (스낵바)
else 검사 통과
VM->>Cache: optimisticVoteUpdate (UI 즉시 변경)
VM->>Server: 투표 요청 전송
alt 서버 성공
Server-->>VM: 최종 결과
VM->>Cache: 서버 결과로 확정
VM->>UI: 성공 이벤트
else 서버 실패
Server-->>VM: 에러 응답
VM->>Cache: 이전 상태로 롤백
VM->>UI: 실패 이벤트 (스낵바)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/SnackBar.kt (1)
151-156:⚠️ Potential issue | 🟡 MinorKDoc 문서가 실제 구현과 일치하지 않습니다.
Line 155에서 "Mutex를 사용하여 순차적으로 처리합니다"라고 설명하고 있지만, Mutex가 제거되었습니다. 현재 구현은 이전 스낵바를 즉시 dismiss하고 새 메시지로 교체하는 방식입니다.
📝 문서 수정 제안
* 이 함수는 suspend 함수로, 스낵바가 사라질 때까지 대기합니다. - * 동시에 여러 스낵바가 표시되지 않도록 Mutex를 사용하여 순차적으로 처리합니다. + * 새로운 스낵바 표시 요청 시 기존 스낵바를 즉시 dismiss하여 항상 최신 메시지만 표시합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/SnackBar.kt` around lines 151 - 156, The KDoc for the SnackBar implementation incorrectly says a Mutex is used to serialize snackbars; update the comment for the SnackBar function (SnackBar in SnackBar.kt) to reflect the current behavior: remove any mention of Mutex and state sequential queuing, and instead document that the implementation immediately dismisses the previous snackbar and replaces it with the new message, and that the function is suspend and waits until the displayed snackbar disappears.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/SnackBar.kt`:
- Around line 151-156: The KDoc for the SnackBar implementation incorrectly says
a Mutex is used to serialize snackbars; update the comment for the SnackBar
function (SnackBar in SnackBar.kt) to reflect the current behavior: remove any
mention of Mutex and state sequential queuing, and instead document that the
implementation immediately dismisses the previous snackbar and replaces it with
the new message, and that the function is suspend and waits until the displayed
snackbar disappears.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b7baf244-383d-4530-a4e4-061762049e06
📒 Files selected for processing (7)
app/src/main/java/com/sseotdabwa/buyornot/navigation/BuyOrNotNavHost.ktcore/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/SnackBar.ktfeature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.ktfeature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.ktfeature/mypage/src/main/java/com/sseotdabwa/buyornot/feature/mypage/ui/WithdrawalScreen.ktfeature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadScreen.ktfeature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadViewModel.kt
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt (1)
147-165:⚠️ Potential issue | 🟠 Major이전 요청의 늦은 응답이 현재 탭/필터를 덮어쓸 수 있습니다.
Line 147에서
requestedFilter를 잡아두지만 Line 163에서는 탭만 비교하고 있고,loadFeeds()/handleRefresh()는 요청 시점의 탭/필터를 아예 검증하지 않습니다. 탭/필터를 빠르게 바꾸면 늦게 끝난 이전 요청이 현재 목록을 덮어써 잘못된 피드가 보일 수 있습니다. 요청 파라미터나 requestId를 캡처해서 불일치 응답은 버려주세요.Also applies to: 337-377, 392-415
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt` around lines 147 - 165, The issue is that late responses can overwrite the current tab/filter because only requestedTab is validated; modify loadFeeds()/handleRefresh() flows to capture an immutable request identifier (e.g., requestId or the exact request params: requestedTab + requestedFilter + currentState.nextCursor) before calling runCatchingCancellable and, onSuccess, verify the captured params still match currentState (compare both selectedTab and selectedFilter or the requestId) and only then call updateState/apply the result (otherwise drop the response); apply the same fix to the other similar blocks referenced (around the code handling pagination and refresh: the blocks using runCatchingCancellable and feedRepository.getFeedList/getMyFeeds).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.kt`:
- Around line 18-25: The file is missing Lazy DSL imports used by the
composables: add the missing imports for stickyHeader, items, and animateItem
from the androidx.compose.foundation.lazy package so the LazyColumn/LazyRow DSL
references resolve (i.e., import the functions referenced as stickyHeader,
items, and animateItem to match their usages in HomeScreen.kt).
In
`@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt`:
- Around line 211-213: The optimistic update currently sets userVotedOptionIndex
(via optimisticVoteUpdate and updateState on feeds) which prevents hiding
results until server confirmation; change the flow so optimisticVoteUpdate
updates only a separate pending field (e.g., add pendingUserVoteIndex or
pendingVoteState on the Feed model) to block duplicate taps, and do NOT set
userVotedOptionIndex there; instead set userVotedOptionIndex only in the success
path (the code that handles server confirmation) and clear the pending field on
failure; update references in optimisticVoteUpdate, the Feed data class, and the
success/failure handlers to use these distinct fields so pending vs confirmed
state are separated.
- Around line 209-244: The rollback currently replaces the entire feeds list
with previousFeeds, wiping other concurrent updates; instead, when handling
failure in the vote flow (in HomeViewModel inside the viewModelScope.launch
failure handler), updateState should only restore the single failed feed:
compute the originalFeed by finding previousFeeds.first { it.id == feedId } and
then map the current state feeds (it.feeds) to replace the feed with matching id
with originalFeed while leaving others untouched; update the rollback call that
now uses updateState { it.copy(feeds = previousFeeds) } to perform this
single-feed replacement using feedId and previousFeeds.
- Around line 132-135: handleFilterSelection currently updates selectedFilter
but leaves pagination state (hasNextPage/nextCursor) from the previous filter,
causing subsequent loadFeeds calls to pass stale cursors; fix by resetting
pagination when changing filter: in handleFilterSelection (and any other
filter-change entrypoints) call updateState to set selectedFilter = filter AND
clear nextCursor (e.g., null) and set hasNextPage = false (and optionally clear
current feeds if needed), so that loadFeeds will request the new filter's first
page instead of using the old nextCursor; also audit loadFeeds and any code
paths that append pages (references: handleFilterSelection, loadFeeds,
selectedFilter, nextCursor, hasNextPage) to ensure they rely on the reset values
when clearFeeds is false/true.
---
Outside diff comments:
In
`@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt`:
- Around line 147-165: The issue is that late responses can overwrite the
current tab/filter because only requestedTab is validated; modify
loadFeeds()/handleRefresh() flows to capture an immutable request identifier
(e.g., requestId or the exact request params: requestedTab + requestedFilter +
currentState.nextCursor) before calling runCatchingCancellable and, onSuccess,
verify the captured params still match currentState (compare both selectedTab
and selectedFilter or the requestId) and only then call updateState/apply the
result (otherwise drop the response); apply the same fix to the other similar
blocks referenced (around the code handling pagination and refresh: the blocks
using runCatchingCancellable and feedRepository.getFeedList/getMyFeeds).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: dd5b0076-c258-4917-8877-bbc00f845e25
📒 Files selected for processing (4)
core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/SnackBar.ktfeature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeContract.ktfeature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.ktfeature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/SnackBar.kt
feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.kt
Show resolved
Hide resolved
feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt
Show resolved
Hide resolved
feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt
Outdated
Show resolved
Hide resolved
feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt (1)
169-188:⚠️ Potential issue | 🟡 Minor페이지네이션 응답 시 필터 변경 검증 누락
Line 170에서 탭 변경은 검증하지만, 필터 변경은 검증하지 않습니다. 요청 중 필터가 변경되면 이전 필터의 데이터가 새 필터 목록에 추가될 수 있습니다.
🔧 제안하는 수정
}.onSuccess { feedList -> - if (currentState.selectedTab != requestedTab) { + if (currentState.selectedTab != requestedTab || + currentState.selectedFilter != requestedFilter + ) { updateState { it.copy(isNextPageLoading = false) } return@launch }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt` around lines 169 - 188, When applying paginated results in HomeViewModel (the onSuccess block that checks currentState.selectedTab vs requestedTab), also verify the active filter hasn't changed by comparing currentState.selectedFilter (or selectedFilters) to the requestedFilter (or requestedFilters) used for the request; if they differ, call updateState to set isNextPageLoading = false and return without appending feedList.feeds. Ensure this additional guard is placed before mapping/appending newItems so stale results from a previous filter aren't merged into the current feeds.
🧹 Nitpick comments (1)
feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt (1)
34-73: 초기화 시 중복 로드 가능성
init에서observeUserPreferences()와loadInitialData()가 동시에 실행됩니다.UserType.SOCIAL인 경우 두 경로 모두loadCurrentUserIdSuspend()및loadFeeds()를 호출할 수 있어 초기화 시 중복 API 호출이 발생할 수 있습니다.
isUserIdLoaded플래그가 ID 중복 로드는 방지하지만, 피드 로드는 여전히 중복될 수 있습니다. 앱 시작 시 한 번만 발생하는 경미한 이슈이므로 우선순위가 낮습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt` around lines 34 - 73, The init path can trigger loadFeeds twice because observeUserPreferences() and loadInitialData() run concurrently; add a simple guard (e.g., a new Boolean like isFeedLoaded) and use it in observeUserPreferences(), loadInitialData(), loadUserIdAndRefreshFeeds(), and any direct loadFeeds() callers to prevent duplicate feed calls: initialize isFeedLoaded = false, set it true right before/after calling loadFeeds() (or when feeds successfully start loading), and in observeUserPreferences() only call loadUserIdAndRefreshFeeds() or loadFeeds() when isFeedLoaded is false (or check existing isUserIdLoaded && isFeedLoaded to decide); update references in functions loadCurrentUserIdSuspend(), loadUserIdAndRefreshFeeds(), loadFeeds(), and the observeUserPreferences() branch that handles UserType.SOCIAL to use this guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt`:
- Around line 169-188: When applying paginated results in HomeViewModel (the
onSuccess block that checks currentState.selectedTab vs requestedTab), also
verify the active filter hasn't changed by comparing currentState.selectedFilter
(or selectedFilters) to the requestedFilter (or requestedFilters) used for the
request; if they differ, call updateState to set isNextPageLoading = false and
return without appending feedList.feeds. Ensure this additional guard is placed
before mapping/appending newItems so stale results from a previous filter aren't
merged into the current feeds.
---
Nitpick comments:
In
`@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt`:
- Around line 34-73: The init path can trigger loadFeeds twice because
observeUserPreferences() and loadInitialData() run concurrently; add a simple
guard (e.g., a new Boolean like isFeedLoaded) and use it in
observeUserPreferences(), loadInitialData(), loadUserIdAndRefreshFeeds(), and
any direct loadFeeds() callers to prevent duplicate feed calls: initialize
isFeedLoaded = false, set it true right before/after calling loadFeeds() (or
when feeds successfully start loading), and in observeUserPreferences() only
call loadUserIdAndRefreshFeeds() or loadFeeds() when isFeedLoaded is false (or
check existing isUserIdLoaded && isFeedLoaded to decide); update references in
functions loadCurrentUserIdSuspend(), loadUserIdAndRefreshFeeds(), loadFeeds(),
and the observeUserPreferences() branch that handles UserType.SOCIAL to use this
guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: f40f2cdc-a6c0-41c5-b677-68599f311c61
📒 Files selected for processing (1)
feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt
Imagine-Choi
left a comment
There was a problem hiding this comment.
수고하셨습니다~~~람쥐 !!
내가 또 QA 잔뜩 가져올게....
🛠 Related issue
closed #65
어떤 변경사항이 있었나요?
✅ CheckPoint
PR이 다음 요구 사항을 충족하는지 확인하세요.
✏️ Work Description
🐞 BugFix
로그인 성공 시 백스택 미제거
onLoginSuccess에서popUpTo(SPLASH_ROUTE)를 사용했으나,navigateToLogin()호출 시점에 이미 SPLASH_ROUTE가 백스택에서 제거된 상태라 AUTH_ROUTE가 잔류popUpTo(AUTH_ROUTE) { inclusive = true }로 변경 (BuyOrNotNavHost.kt)투표 피드 중복 업로드
isLoading체크 누락uiState.isLoading중 업로드 버튼 비활성화 및 ViewModel에서 이중 호출 방어 (UploadScreen.kt,UploadViewModel.kt)회원탈퇴 다이얼로그 미노출
WithdrawalScreen에서onShowWithdrawalDialog/onDismissWithdrawalDialog콜백이 전달되지 않음WithdrawalScreen.kt)투표 낙관적 업데이트 시 득표율 노출
HomeViewModel.kt)✨ Feature
Pull To Refresh 구현
HomeUiState에isRefreshing상태 추가HomeIntent.Refresh추가PullToRefreshBox로LazyColumn감싸기isLoading(전체 화면 스피너) 대신isRefreshing을 사용해 기존 피드를 유지한 채로 갱신필터 변경 시 깜빡임 방지
clearFeeds = false)🔨 Refactor
HomeScreen 헤더 구현 단순화
SubcomposeLayout으로 TopBar·Tab 높이를 별도 측정 →NestedScrollConnection+offset으로 수동 제어item으로, Tab을stickyHeader로 배치SubcomposeLayout,HomeHeadercomposable,NestedScrollConnection,topBarHeightPx/tabHeightPxstate (HomeScreen.kt)HomeViewModel 리팩토링
cachedFeeds제거 → 탭/필터 변경 시 API를 직접 호출하도록 변경FilterChip.toFeedStatus()추가: 필터 칩 선택 값을 APIfeedStatus파라미터("OPEN"/"CLOSED"/null)로 변환하여 서버 사이드 필터링applyFiltering()제거 →loadFeeds()내에서 결과를 바로 state에 반영😅 Uncompleted Tasks
N/A
📢 To Reviewers
클로드는 신이예요
📃 RCA 룰
Summary by CodeRabbit
버그 수정
개선 사항