fix: 찜 연속 클릭 시 상태 불일치 수정 및 찜 버튼 공통화 - #66
Conversation
📝 WalkthroughWalkthroughChanges찜 UI를 공통 찜 기능 통합
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant 사용자
participant FavoriteButton
participant useFavoriteMover
participant favoriteMutation
participant 관련 쿼리
사용자->>FavoriteButton: 찜 상태 변경
FavoriteButton->>useFavoriteMover: 다음 찜 상태 전달
useFavoriteMover->>favoriteMutation: 기사별 순차 요청 실행
favoriteMutation-->>useFavoriteMover: 성공 또는 오류 반환
useFavoriteMover->>관련 쿼리: 최신 요청일 때 무효화
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
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 (2)
src/hooks/useFavoriteMover.ts (2)
264-266: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win이전 요청의 401 오류로 로그인 흐름을 시작하지 마세요.
이 분기는
isLatestRequest를 확인하지 않습니다. 이전 요청이 401로 실패한 뒤 최신 요청이 대기 또는 성공하는 경우에도requireLogin()이 실행됩니다. 이 동작은 최신 상태만 오류를 처리한다는 요청 큐 계약을 깨고 사용자의 현재 작업을 중단합니다.
isUnauthorizedError(error)처리도isLatestRequest일 때만 실행하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useFavoriteMover.ts` around lines 264 - 266, Update the unauthorized-error branch in useFavoriteMover so isUnauthorizedError(error) triggers requireLogin() and returns only when isLatestRequest is true. Preserve the existing behavior for latest requests while ignoring 401 errors from superseded requests.
121-132: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift기사별 전체 mutation lifecycle을 클릭 순서로 직렬화하세요.
onMutate는mutationFn보다 먼저 실행되며, 현재 큐는mutationFn이후에만 적용됩니다.cancelQueries대기 순서가 뒤집히면 낙관적 갱신과enqueueFavoriteRequest등록 순서가 모두 뒤집힐 수 있습니다. 초기값이false일 때true → false를 빠르게 클릭하면 마지막 요청이false여도 화면과 서버가true가 될 수 있습니다.onMutate부터 요청 완료까지 기사별 큐 또는 scope로 직렬화하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useFavoriteMover.ts` around lines 121 - 132, Serialize the entire favorite-mutation lifecycle per mover, starting before onMutate and continuing through enqueueFavoriteRequest completion, so rapid clicks preserve click order. Update the mutation flow around onMutate and mutationFn to use a mover-scoped queue or serialization mechanism, ensuring cancelQueries, optimistic updates, request registration, and completion execute sequentially for each mover while allowing different movers to proceed independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/mover/FavoriteButton.tsx`:
- Around line 46-55: Update FavoriteButton’s accessible-name logic to include
favoriteCount in the button aria-label whenever showCount is true, while
preserving the count Text element’s aria-hidden="true" behavior.
In `@src/hooks/useFavoriteMover.ts`:
- Around line 70-92: Update enqueueFavoriteRequest and its queue state to
include an authScope or session generation alongside moverId, so requests from
different accounts never share a queue. Before executing addFavoriteMover or
removeFavoriteMover, compare the request’s captured scope with the current scope
and discard mismatched queued requests; preserve ordering for requests within
the same session.
---
Outside diff comments:
In `@src/hooks/useFavoriteMover.ts`:
- Around line 264-266: Update the unauthorized-error branch in useFavoriteMover
so isUnauthorizedError(error) triggers requireLogin() and returns only when
isLatestRequest is true. Preserve the existing behavior for latest requests
while ignoring 401 errors from superseded requests.
- Around line 121-132: Serialize the entire favorite-mutation lifecycle per
mover, starting before onMutate and continuing through enqueueFavoriteRequest
completion, so rapid clicks preserve click order. Update the mutation flow
around onMutate and mutationFn to use a mover-scoped queue or serialization
mechanism, ensuring cancelQueries, optimistic updates, request registration, and
completion execute sequentially for each mover while allowing different movers
to proceed independently.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 97b9ce3f-8567-4fff-8af6-8a1f7d82be83
📒 Files selected for processing (9)
src/components/estimate/detail/EstimateDetailDriverSummary.tsxsrc/components/estimate/pending/PendingEstimateCard.tsxsrc/components/estimate/received/EstimateOfferCard.tsxsrc/components/mover/FavoriteButton.tsxsrc/components/mover/MoverCard.tsxsrc/components/mover/detail/MoverDetailProfile.tsxsrc/components/mover/detail/MoverDetailView.tsxsrc/hooks/useBulkRemoveFavoriteMovers.tssrc/hooks/useFavoriteMover.ts
💤 Files with no reviewable changes (1)
- src/components/mover/detail/MoverDetailView.tsx
- onMutate를 authScope:moverId 큐로 직렬화해 빠른 연속 클릭 시 마지막 클릭 상태로 캐시가 반영되도록 수정 - 이전 요청의 401로 requireLogin()이 호출되던 문제 수정 (isLatestRequest일 때만 처리)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/hooks/useFavoriteMover.ts (1)
301-303: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
latestFavoriteRequestIds만moverId키를 사용합니다.큐는
authScope:moverId키로 세션을 분리합니다(L88~90). 그러나 최신 요청 판별 맵은moverId만 사용합니다. 계정 전환 시 서로 다른 세션의 요청이 같은 항목을 덮어씁니다.favoriteRequestId가 전역 증가하므로 현재 동작은 안전한 방향으로 기울지만, 두 자료구조의 키 규칙이 다르면 이후 변경에서 오판이 생기기 쉽습니다.getFavoriteQueueKey(variables.authScope, variables.moverId)로 통일하는 것을 권장합니다.Also applies to: 341-346
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useFavoriteMover.ts` around lines 301 - 303, Update the latest-request tracking in the error and success handling around latestFavoriteRequestIds so it uses the same session-scoped key as the favorite queue: derive the key with getFavoriteQueueKey(variables.authScope, variables.moverId) for both lookups and writes. Preserve the existing request-id comparison behavior while applying this key consistently across the affected handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/useFavoriteMover.ts`:
- Around line 173-189: Update the mutation cache-key calculations throughout the
favorite mutation callbacks to consistently use variables.authScope captured as
requestAuthScope, not the latest rendered authScope. Apply this to the list,
favorite-list, and mover-detail keys in onMutate and the corresponding rollback
and invalidation logic in onError and onSettled, preserving each callback’s
existing behavior.
In `@src/stories/FavoriteButton.stories.tsx`:
- Around line 108-125: Update the button lookup in the story’s play function to
query its accessibility name with a regular expression matching the “김무빙 기사님 찜”
prefix, so it also matches the dynamic count suffix. Keep the existing
aria-pressed and count assertions unchanged.
---
Nitpick comments:
In `@src/hooks/useFavoriteMover.ts`:
- Around line 301-303: Update the latest-request tracking in the error and
success handling around latestFavoriteRequestIds so it uses the same
session-scoped key as the favorite queue: derive the key with
getFavoriteQueueKey(variables.authScope, variables.moverId) for both lookups and
writes. Preserve the existing request-id comparison behavior while applying this
key consistently across the affected handlers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2cda3478-8dd9-45f0-84d0-9aac50446832
📒 Files selected for processing (3)
src/components/mover/FavoriteButton.tsxsrc/hooks/useFavoriteMover.tssrc/stories/FavoriteButton.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/mover/FavoriteButton.tsx
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 동일 기사에 대한 찜 요청을 Promise 큐로 직렬화해 서버 요청 순서를 클릭 순서와 일치시켰습니다.
- 큐 키에
authScope를 포함해 다른 계정의 찜 요청이 같은 큐를 공유하지 않도록 분리했습니다. - 큐 실행 직전 현재 세션을 재확인해 이전 계정의 대기 요청이 새 계정 인증으로 실행되는 문제를 방지했습니다.
- 세션 변경으로 폐기된 요청을 별도 Sentinel Error로 구분하고 일반 실패 처리와 사용자 알림에서 제외했습니다.
- API 요청뿐 아니라 낙관적 업데이트 구간도 직렬화해 빠른 연속 클릭 시 캐시 반영 순서를 보장했습니다.
- 요청별 ID와 최신 요청 ID를 비교해 이전 요청의 실패 롤백이 마지막 사용자 상태를 덮어쓰지 않도록 처리했습니다.
- 이전 요청의 401이 뒤늦게 로그인 모달을 열지 않도록 최신 요청 여부를 함께 확인했습니다.
- 가장 마지막 요청이 종료됐을 때만 관련 Query를 무효화해 중간 refetch가 최신 낙관적 상태를 덮어쓰는 문제를 줄였습니다.
FavoriteButton으로 하트, 찜 개수, 토글 이벤트, 접근성 속성을 공통화해 사용 화면 간 동작을 일관되게 정리했습니다.- 실제
button,aria-pressed, 기사명 기반aria-label, 키보드 포커스 스타일을 적용해 접근성을 보완했습니다. - 카드 내부에서 버튼 클릭 시 상위 링크 이동이 발생하지 않도록 이벤트 처리를 공통 컴포넌트에 포함했습니다.
- Storybook에서 주요 Props를 문서화하고 Play Function으로 토글 상태와 찜 개수 변경을 검증했습니다.
- 기사 카드, 기사 상세, 견적 카드 및 견적 상세 등 여러 화면에 적용하면서 화면별 크기와 배치는 확장 Class Props로 유지했습니다.
- PR 본문 기준 로컬 동작, 기존 기능 영향, lint, API 명세 확인까지 완료되었습니다.
🔍 확인 및 제안
필수 수정사항은 보이지 않았습니다.
가벼운 제안으로, latestFavoriteRequestIds는 현재 moverId만 키로 사용하고 있습니다. 요청 큐는 authScope:moverId로 분리되어 있지만 최신 요청 ID는 세션 간 공유되므로, 계정 전환 직후 동일 기사에 새 요청이 발생하면 이전 세션 요청이 최신 요청이 아닌 것으로 처리됩니다. 현재 의도상 이전 세션의 에러·롤백·무효화를 억제하는 결과이므로 동작에는 오히려 안전해 보입니다. 다만 큐와 동일한 기준으로 의미를 통일하고 싶다면 최신 요청 ID도 queueKey 기준으로 관리할 수 있습니다.
또한 FavoriteButton의 interactive={false} 상태에서는 onToggle이 사용되지 않지만 Props에서 필수입니다. 현재 모든 사용처가 동일한 API를 유지하기에는 단순한 구조이고 문제는 없지만, 읽기 전용 사용 사례가 늘어난다면 Discriminated Union으로 interactive: false일 때 onToggle을 선택적으로 만드는 방향도 고려할 수 있습니다.
전체적으로 연속 클릭, 이전 요청 실패, 세션 전환이라는 까다로운 케이스를 잘 분리했고, 공통 컴포넌트 추출도 기존 화면 디자인을 유지하는 선에서 적절하게 이뤄졌습니다. 수고하셨습니다! 😊
To Reviewer 내용 기준으로 연속 요청 처리와 공통 컴포넌트 구조를 중점적으로 확인했습니다!
연속된 찜 추가·해제 요청은 동일한 authScope + moverId 큐에 등록되고,
앞선 요청이 성공하거나 실패한 뒤 다음 요청이 실행되므로
서버에는 사용자의 클릭 순서대로 전달되는 구조입니다.
중간 요청이 실패하더라도 큐 Tail은 성공 상태로 정리되어
이후 요청이 막히지 않는 부분도 적절합니다.
낙관적 업데이트 역시 별도의 기사별 큐로 직렬화되어
cancelQueries, 이전 캐시 Snapshot 저장, 캐시 수정이 클릭 순서대로 실행됩니다.
따라서 빠르게 연속 클릭하더라도 마지막 클릭의 상태가 최종 UI에 반영될 수 있습니다.
이전 요청의 에러가 최신 상태를 덮어쓰는 문제도
요청별 requestId와 latestFavoriteRequestIds 비교로 방지하고 있습니다.
- 최신 요청 실패: 해당 요청의 Snapshot으로 롤백
- 이전 요청 실패: 롤백 및 사용자 오류 안내 생략
- 이전 요청 401: 로그인 모달 미노출
- 중간 요청 종료: Query 무효화 생략
- 마지막 요청 종료: 관련 Query 무효화 실행
흐름으로 구성되어 있어 이전 응답이 최종 낙관적 상태를 덮어쓰지 않도록 잘 처리된 것으로 보입니다.
계정 전환 대응도 큐 키에 authScope를 포함하고,
실제 요청 실행 직전에 ref를 통해 현재 세션을 다시 검사하므로
이전 계정의 대기 요청이 새 계정의 인증으로 전송되는 문제를 막을 수 있습니다.
FavoriteButton 분리 방향도 적절합니다.
공통화 대상이 된 책임은 다음처럼 UI 공통 책임에 집중되어 있습니다.
- 하트 아이콘
- 찜 개수 표시
- 개수 위치
- 토글 이벤트
- 카드 링크 이벤트 전파 차단
aria-labelaria-pressed- 키보드 포커스 스타일
화면별 차이는 className, iconClassName, countClassName, countVariant,
countPosition으로만 확장하고 있어 기존 디자인을 유지하면서 중복을 줄인 구조로 보입니다.
현재 Props 중 즉시 제거해야 할 정도로 불필요한 항목은 보이지 않았으며,
읽기 전용 사용 사례가 늘어나면 interactive: false일 때
onToggle을 선택값으로 만드는 타입 개선 정도를 추후 고려할 수 있습니다.
📋 작업 내용
기사님 찾기, 찜한 기사님 페이지의 카드와 기사 상세 패이지에서 찜 취소 후 바로 다시 등록하려고 클릭하면 반응하지 않고 한 번 더 클릭해야 동작하는 버그를 수정했습니다.
또한 여러 페이지에 중복 구현되어 있던 하트·찜 개수 UI를 공통 컴포넌트로 통합했습니다.
🔥 변경 사항
찜 토글 안정성 개선
authScope를 큐 키에 포함하고 실행 직전 세션을 재확인해, 계정 전환 시 이전 계정의 대기 요청이 새 계정의 인증으로 잘못 전송되지 않도록 수정onMutate(낙관적 업데이트)를 기사별 큐로 직렬화해 빠른 연속 클릭 시 캐시 상태가 항상 마지막 클릭 값으로 수렴하도록 수정onMutate,onError,onSettled에서 모두 요청 시점의authScope를 기준으로 캐시 조회·롤백·무효화를 수행하도록 수정해 계정 전환 시 다른 세션의 캐시를 잘못 갱신하는 문제 해결latestRequestIds)도authScope를 포함하도록 변경해 계정 간 요청이 서로 영향을 주지 않도록 수정FavoriteButton공통 컴포넌트 분리FavoriteButton공통 컴포넌트로 분리FavoriteButton스토리 추가✅ 체크리스트
📷 스크린샷 (선택)
찜 취소 후 즉시 다시 등록
💬 To Reviewer
FavoriteButton컴포넌트로 분리한 방향이 적절한지 불필요한 props가 있지는 않은지 등 의견 주시면 감사하겠습니다!Summary by CodeRabbit
새로운 기능
버그 수정