feat: 찜한 기사님 목록 페이지 및 Header 프로필 메뉴 연동 - #41
Conversation
- 단건 찜과 일괄 해제·캐시 유틸을 분리 - 찜 목록 페이지는 Toolbar/Content/Selection 훅으로 분리
📝 WalkthroughWalkthrough찜한 기사님 전용 경로와 목록 화면을 추가했습니다. 무한 조회, 선택, 전체 선택, 일괄 해제, 낙관적 캐시 갱신을 지원합니다. 프로필 메뉴를 분리하고 관련 경로, 인증 로딩 화면, 공통 UI 스타일을 변경했습니다. Changes찜한 기사님 관리
헤더 및 공통 UI
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Customer
participant FavoriteMoversPageClient
participant CustomerAuthGate
participant FavoriteMoversContent
participant FavoriteMoversAPI
Customer->>FavoriteMoversPageClient: 찜한 기사님 페이지 요청
FavoriteMoversPageClient->>CustomerAuthGate: 인증 상태 확인
CustomerAuthGate->>FavoriteMoversContent: 인증 완료 후 콘텐츠 렌더링
FavoriteMoversContent->>FavoriteMoversAPI: 페이지별 찜 목록 조회
FavoriteMoversAPI-->>FavoriteMoversContent: 기사님 목록과 페이지 정보 반환
FavoriteMoversContent-->>Customer: 목록 및 선택 UI 표시
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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: 6
🧹 Nitpick comments (4)
src/hooks/useBulkRemoveFavoriteMovers.ts (1)
19-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win로그인 필요 처리 로직을 공용 훅으로 분리하세요.
두 훅이
LOGIN_REQUIRED_MESSAGE,isUnauthorizedError,requireLogin을 동일하게 구현합니다. 공용useRequireLogin훅으로 분리해 재사용하세요.🤖 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/useBulkRemoveFavoriteMovers.ts` around lines 19 - 54, Extract the duplicated LOGIN_REQUIRED_MESSAGE, isUnauthorizedError, and requireLogin logic from useBulkRemoveFavoriteMovers into a shared useRequireLogin hook, then update this hook to consume it while preserving the existing modal-or-redirect behavior and unauthorized handling.Source: Coding guidelines
src/components/common/Checkbox/Checkbox.tsx (1)
52-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win체크 아이콘을
src/icons의 공통 아이콘으로 관리해 주세요.
src/icons/index.ts에는 동일한 체크 아이콘이 없습니다.ConfirmedCheckIcon은 확인 배지 전체 모양이므로 대체용으로 사용할 수 없습니다.public/icons/checkbox-check.svg를src/icons에 등록하고@/icons에서 import해 사용해 주세요.🤖 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/components/common/Checkbox/Checkbox.tsx` around lines 52 - 62, Update the Checkbox component’s checked-state icon to use a shared icon registered in src/icons/index.ts: add public/icons/checkbox-check.svg to the icon exports, then import and render that icon from `@/icons` instead of referencing the public path directly. Do not substitute ConfirmedCheckIcon.Source: Path instructions
src/components/mover/FavoriteMoversPageClient.tsx (1)
10-25: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win정적
PageHeader까지 클라이언트 경계 안에 있습니다.이 파일은
"use client"이며PageHeader,CustomerAuthGate,FavoriteMoversContent,FavoriteMoversLoadingSkeleton을 모두 감쌉니다.PageHeader가 제목만 표시하고 상태나 이벤트 핸들러가 필요 없다면,page.tsx(Server Component)에서PageHeader를 직접 렌더링하고 이 클라이언트 컴포넌트에는 실제로 클라이언트 동작이 필요한 인증 게이트와 목록만 남기세요. 이렇게 하면 정적 헤더는 서버에서 렌더링되고 클라이언트 번들 크기도 줄어듭니다.Based on coding guidelines: "컴포넌트의 책임을 명확히 분리하고, 불필요한 상태·useEffect·Client Component를 추가하지 않는다."
🤖 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/components/mover/FavoriteMoversPageClient.tsx` around lines 10 - 25, Move the static PageHeader rendering from FavoriteMoversPageClient into the surrounding page.tsx Server Component, and remove its import and usage from FavoriteMoversPageClient. Keep CustomerAuthGate, FavoriteMoversContent, and FavoriteMoversLoadingSkeleton in the client component with the existing layout and loading behavior unchanged.Source: Coding guidelines
src/components/mover/FavoriteMoversContent.tsx (1)
100-113: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win체크박스 하나를 토글해도 로드된 카드 전체가 리렌더링됩니다.
selection={{ checked: ..., onCheckedChange: (checked) => ... }}는 매 렌더마다 새 객체와 새 함수를 생성해 각MoverCard에 전달합니다.useFavoriteMoversSelection의 상태는FavoriteMoversContent안에서 관리되므로, 체크박스 하나만 토글해도 이 컴포넌트가 리렌더되고 로드된 모든MoverCard가 새로운 props를 받아 다시 렌더링됩니다. 목록에 항목이 많이 로드될수록(더보기를 여러 번 누른 뒤) 체감 지연이 커질 수 있습니다.
mover.id와selection.handleToggleMover,selection.isMoverSelected처럼 안정적인 참조를 직접 전달하고,MoverCard쪽에서mover.id를 바인딩하도록 구조를 바꾸면 매 렌더 새 클로저 생성을 줄일 수 있습니다.MoverCard를React.memo로 감싸는 작업과 함께 적용해야 실제 효과가 있습니다.Based on path instructions: "불필요한 리렌더링을 유발하는 구조인지 확인해 주세요. (렌더링마다 새로 만들어지는 객체·배열·함수를 자식 props로 전달 등)"
🤖 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/components/mover/FavoriteMoversContent.tsx` around lines 100 - 113, Update the FavoriteMoversContent mapping and MoverCard props so mover.id, selection.handleToggleMover, and selection.isMoverSelected are passed as stable references instead of creating a new selection object and callback per item on each render. Bind mover.id inside MoverCard, wrap MoverCard with React.memo, and preserve the existing checked-state and toggle behavior so unchanged cards can skip re-rendering.Source: Path instructions
🤖 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/common/Header/ProfileMenuTrigger.tsx`:
- Around line 62-83: Update the menu focus handling around onKeyDown and the
menu container to close the open menu when focus leaves it via Tab. Add blur
handling that checks the next focused element, calls closeQuiet when it is
outside the menu container, and preserves the menu when focus moves between its
own items.
- Around line 38-40: Update the menu state in ProfileMenuTrigger to use a
boolean rather than storing openMenuPath, and reset it to closed whenever
pathname changes. Ensure browser back/forward navigation cannot make isOpen
become true automatically or move focus to the first menu item without a new
user action.
In `@src/components/mover/FavoriteMoversLoadingSkeleton.tsx`:
- Line 6: Update LIST_SKELETON_COUNT in FavoriteMoversLoadingSkeleton to match
the actual five-item page size used by the load-more flow, so the loading
skeleton count remains consistent with the loaded cards.
In `@src/hooks/useBulkRemoveFavoriteMovers.ts`:
- Around line 56-92: Update the bulk removal flow in mutationFn and onError to
distinguish partial failures instead of treating Promise.all rejection as a
complete rollback: use Promise.allSettled to track successful and failed mover
IDs, keep successful removals reflected in the cache, and restore or retain only
the failed IDs as appropriate. Ensure the error callback communicates partial
failure accurately while preserving unauthorized handling and final query
invalidation.
In `@src/hooks/useFavoriteMoversSelection.ts`:
- Around line 79-97: Update handleBulkDelete so removeFavorites failures in its
catch are not passed to setToastMessage, allowing useBulkRemoveFavoriteMovers
and its onError/authentication handling to remain the sole toast path. Keep
fetchAllFavoriteMoverIds errors on a separate path with their own toast
handling, and preserve rejection handling for removeFavorites without
duplicating user-facing errors.
In `@src/lib/api/favorites.ts`:
- Around line 44-69: Update fetchAllFavoriteMoverIds and handleConfirmDeleteAll
so the deletion target comes from a consistent favorites-list snapshot rather
than concurrently fetched offset pages based on totalPages; use the available
backend bulk-delete or cursor/snapshot-based API, preserving the full captured
ID set for the subsequent deletion.
---
Nitpick comments:
In `@src/components/common/Checkbox/Checkbox.tsx`:
- Around line 52-62: Update the Checkbox component’s checked-state icon to use a
shared icon registered in src/icons/index.ts: add
public/icons/checkbox-check.svg to the icon exports, then import and render that
icon from `@/icons` instead of referencing the public path directly. Do not
substitute ConfirmedCheckIcon.
In `@src/components/mover/FavoriteMoversContent.tsx`:
- Around line 100-113: Update the FavoriteMoversContent mapping and MoverCard
props so mover.id, selection.handleToggleMover, and selection.isMoverSelected
are passed as stable references instead of creating a new selection object and
callback per item on each render. Bind mover.id inside MoverCard, wrap MoverCard
with React.memo, and preserve the existing checked-state and toggle behavior so
unchanged cards can skip re-rendering.
In `@src/components/mover/FavoriteMoversPageClient.tsx`:
- Around line 10-25: Move the static PageHeader rendering from
FavoriteMoversPageClient into the surrounding page.tsx Server Component, and
remove its import and usage from FavoriteMoversPageClient. Keep
CustomerAuthGate, FavoriteMoversContent, and FavoriteMoversLoadingSkeleton in
the client component with the existing layout and loading behavior unchanged.
In `@src/hooks/useBulkRemoveFavoriteMovers.ts`:
- Around line 19-54: Extract the duplicated LOGIN_REQUIRED_MESSAGE,
isUnauthorizedError, and requireLogin logic from useBulkRemoveFavoriteMovers
into a shared useRequireLogin hook, then update this hook to consume it while
preserving the existing modal-or-redirect behavior and unauthorized handling.
🪄 Autofix (Beta)
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: 4f98cbe3-a1bb-4ba6-ad2d-83c213dac171
📒 Files selected for processing (24)
src/app/favorites/movers/page.tsxsrc/app/movers/favorites/page.tsxsrc/components/auth/CustomerAuthGate.tsxsrc/components/common/Checkbox/Checkbox.tsxsrc/components/common/Header/Header.tsxsrc/components/common/Header/NotificationTrigger.tsxsrc/components/common/Header/ProfileMenuTrigger.tsxsrc/components/common/Pagination/Pagination.tsxsrc/components/mover/FavoriteMoversContent.tsxsrc/components/mover/FavoriteMoversDeleteConfirmModal.tsxsrc/components/mover/FavoriteMoversLoadingSkeleton.tsxsrc/components/mover/FavoriteMoversPageClient.tsxsrc/components/mover/FavoriteMoversSidebar.tsxsrc/components/mover/FavoriteMoversToolbar.tsxsrc/components/mover/MoverCard.tsxsrc/components/mover/MoverCardSkeleton.tsxsrc/hooks/useBulkRemoveFavoriteMovers.tssrc/hooks/useFavoriteMover.tssrc/hooks/useFavoriteMovers.tssrc/hooks/useFavoriteMoversSelection.tssrc/lib/api/favorites.tssrc/lib/constants/appRoutes.tssrc/lib/utils/favoriteMoverCache.tssrc/styles/tokens.theme.css
💤 Files with no reviewable changes (1)
- src/app/favorites/movers/page.tsx
juengseulki
left a comment
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 찜한 기사님 전체 목록과 기사 찾기 사이드바용 쿼리를 각각 infinite query와 일반 query로 분리해 사용 목적을 명확하게 나눈 점이 좋았습니다.
- 찜 목록 관련 캐시 갱신 로직을
favoriteMoverCache로 분리해 단건 삭제와 일괄 삭제에서 공통으로 활용할 수 있도록 구성했습니다. - 찜 해제 시 받은 견적, 대기 견적, 견적 상세, 기사 목록·상세와 찜 목록을 함께 무효화해 화면마다 찜 상태가 달라지는 문제를 방지했습니다.
- 유한 목록과 infinite 목록 모두
FAVORITES.MOVERSprefix 아래에 구성되어 사이드바와 전체 목록 캐시를 함께 관리할 수 있습니다. - 전체선택 시 아직 불러오지 않은 데이터까지 포함하고, 실제 삭제 직전에 전체 ID를 다시 조회해 누락 없이 처리하려는 흐름이 좋았습니다.
- 부분 삭제는 즉시 실행하고 전체 삭제에는 확인 모달을 적용해 작업의 영향도에 따라 UX를 구분했습니다.
- 로딩 스켈레톤, 최초 조회 실패, 추가 조회 실패, 빈 상태, 액션 실패 Toast를 각각 분리해 상태별 대응을 꼼꼼하게 구성했습니다.
- 프로필 메뉴에 키보드 방향키, Home·End, Escape, 포커스 복귀 처리를 적용해 접근성까지 고려한 점이 좋았습니다.
- 공통
Checkbox를 추가하고MoverCard에는 선택 기능을 선택적 prop으로 확장해 기존 사용처에 영향을 최소화했습니다.
🚨 수정이 필요한 부분
인라인 코멘트로 아래 내용을 남겼습니다.
- 전체 찜 해제 시 전체 ID 개수만큼
DELETE요청을Promise.all()로 동시에 실행하고 있습니다. - 찜 개수가 많으면 서버에 요청이 한꺼번에 몰릴 수 있고, 일부 요청이 성공한 뒤 하나가 실패하면 UI에서는 전체 실패로 보이지만 실제 서버에는 일부 삭제가 반영될 수 있습니다.
- 가능하면 백엔드 일괄 삭제 API를 사용하는 것이 좋고, 현재 API만 사용해야 한다면 요청 수 제한 또는 부분 실패 처리 보완이 필요해 보입니다.
🔍 확인 및 제안
- 전체선택 후 현재 보이는 기사 한 명을 해제하면 전체선택이 풀리면서 미로드 항목도 모두 선택에서 제외됩니다. 예를 들어 30명 전체선택 상태가 현재 로드된 4명 선택 상태로 바뀔 수 있는데, 의도한 UX인지 확인 부탁드립니다.
- 전체선택이 미로드 항목까지 포함한다는 사실을 툴바나 안내 문구로 조금 더 명확하게 전달하면 사용자가 선택 범위를 이해하기 쉬울 것 같습니다.
- 현재 쿼리 키 구조와 관련 쿼리 무효화 범위를 확인했을 때, 기사 찾기 사이드바·견적 목록·상세의 찜 캐시는 정상적으로 함께 갱신될 것으로 보입니다.
- 프로필 수정 메뉴가 아직 존재하지 않는 경로로 연결되어 있으므로, 페이지 구현 전까지 비활성화하거나 준비 중 안내를 제공하는 방식도 고려하면 좋겠습니다.
전체적으로 데이터 로딩 방식부터 선택 상태, 캐시 동기화, 오류 UI까지 기능 범위를 세밀하게 나누어 구현한 PR이었습니다. 특히 기존 찜 로직을 역할별 훅과 캐시 유틸로 분리해 이후 유지보수하기 좋은 구조로 정리한 점이 좋았습니다. 다만 전체 삭제 시 다수의 DELETE 요청을 무제한으로 병렬 실행하는 부분은 실제 데이터가 늘어날수록 실패 가능성과 부분 성공 문제가 커질 수 있어 병합 전에 보완하는 것이 안전해 보입니다. 수고하셨습니다! 😊
- Promise.allSettled로 성공/실패 id를 나누고, 일부만 실패한 경우 캐시와 토스트·선택 상태를 실제 결과와 맞춤
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)
src/hooks/useFavoriteMoversSelection.ts (1)
79-94: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift전체선택 상태에서 부분 실패가 발생하면 선택 개수가 실제보다 적게 표시될 수 있습니다.
removeFavorites는 부분 실패 시isSelectAll을false로 바꾸고selectedIds를result.failedIds로 설정합니다.handleConfirmDeleteAll에서isSelectAll이true일 때 삭제 대상은fetchAllFavoriteMoverIds()로 가져온 전체 id 목록입니다. 따라서result.failedIds에는 화면에 아직 로드되지 않은 기사님 id도 포함될 수 있습니다.
isSelectAll이false가 되면selectedCount는selectedOnLoadedCount, 즉loadedIds.filter((id) => selectedIds.includes(id)).length만 사용합니다. 로드되지 않은 실패 id는 이 계산에서 빠집니다. 예를 들어 총 30명을 전체선택했고 10명만 로드된 상태에서 5명이 실패하면, 그중 로드되지 않은 실패 id는 "전체선택(N/M)" 표시 개수에서 누락됩니다. 화면 개수와selectedIds에 실제로 담긴 대상이 어긋납니다.로드되지 않은 실패 id를 별도로 추적하거나, 실패 후에도 남은 대상을
totalCount기반으로 표시하는 방식을 검토해 주세요.
원하시면 로드되지 않은 실패 id를 다루는 수정안을 제안해 드릴까요?🤖 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/useFavoriteMoversSelection.ts` around lines 79 - 94, The removeFavorites function discards unloaded failed ids when handling partial failures. When result.failedIds contains ids that haven't been loaded on screen yet, switching isSelectAll to false and setting selectedIds to result.failedIds causes those unloaded ids to drop out of the selectedCount calculation (which only counts ids in loadedIds). Instead of switching to false isSelectAll state, preserve the select-all state and update excludedIds to exclude the successfully removed items, keeping all failed ids (loaded and unloaded) counted in the selection total. This maintains consistency between the displayed selection count and the actual removal targets.
🧹 Nitpick comments (1)
src/hooks/useBulkRemoveFavoriteMovers.ts (1)
157-159: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win전체 실패 시에도 관련 쿼리를 모두 무효화합니다.
onSettled는 결과와 무관하게 항상invalidateFavoriteRelatedQueries를 호출합니다. 전체 실패 시에는 서버 상태가 변하지 않았고,onError가 이미 캐시를 이전 상태로 복원했습니다. 이 경우 무효화와 그에 따른 리페치는 실질적인 이득이 없습니다.
FAVORITES.MOVERS는 무한 조회 쿼리입니다. TanStack Query 문서에 따르면 "queryClient.invalidateQueries also works for infinite queries. Per default, it will refetch all pages". 사용자가 로드한 페이지 수가 많을수록 전체 실패 시의 불필요한 무효화 비용이 커집니다.onSettled는 "onSettled: (data: TData | undefined, error: TError | null, variables: TVariables, onMutateResult: TOnMutateResult | undefined, context: MutationFunctionContext) => void" 형태로error인자를 받을 수 있으므로, 전체 실패일 때는 무효화를 건너뛰는 편이 좋습니다.♻️ 전체 실패 시 무효화를 건너뛰는 수정안
- onSettled: async () => { - await invalidateFavoriteRelatedQueries(queryClient); - }, + onSettled: async (_data, error) => { + if (error) { + return; + } + await invalidateFavoriteRelatedQueries(queryClient); + },🤖 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/useBulkRemoveFavoriteMovers.ts` around lines 157 - 159, The onSettled callback unconditionally calls invalidateFavoriteRelatedQueries, but on error the cache has already been restored by onError and no server state change occurred, making invalidation and refetch of the infinite FAVORITES.MOVERS query unnecessary. Update the onSettled callback signature to accept the error parameter and only invoke invalidateFavoriteRelatedQueries when the error is null, skipping the invalidation entirely when a mutation failure has already been handled.
🤖 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/useBulkRemoveFavoriteMovers.ts`:
- Around line 67-107: Move the partial-success authentication side effect out of
mutationFn: remove its direct requireLogin() call and include an unauthorized
indicator in the returned BulkRemoveFavoriteResult. Update onSuccess to invoke
requireLogin() based on that indicator while preserving the existing
partial-failure message handling, so authentication expiry does not trigger
overlapping login and error-toast effects.
---
Outside diff comments:
In `@src/hooks/useFavoriteMoversSelection.ts`:
- Around line 79-94: The removeFavorites function discards unloaded failed ids
when handling partial failures. When result.failedIds contains ids that haven't
been loaded on screen yet, switching isSelectAll to false and setting
selectedIds to result.failedIds causes those unloaded ids to drop out of the
selectedCount calculation (which only counts ids in loadedIds). Instead of
switching to false isSelectAll state, preserve the select-all state and update
excludedIds to exclude the successfully removed items, keeping all failed ids
(loaded and unloaded) counted in the selection total. This maintains consistency
between the displayed selection count and the actual removal targets.
---
Nitpick comments:
In `@src/hooks/useBulkRemoveFavoriteMovers.ts`:
- Around line 157-159: The onSettled callback unconditionally calls
invalidateFavoriteRelatedQueries, but on error the cache has already been
restored by onError and no server state change occurred, making invalidation and
refetch of the infinite FAVORITES.MOVERS query unnecessary. Update the onSettled
callback signature to accept the error parameter and only invoke
invalidateFavoriteRelatedQueries when the error is null, skipping the
invalidation entirely when a mutation failure has already been handled.
🪄 Autofix (Beta)
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: 0cf0feff-3777-4245-b3b1-ba4ef63317df
📒 Files selected for processing (2)
src/hooks/useBulkRemoveFavoriteMovers.tssrc/hooks/useFavoriteMoversSelection.ts
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)
src/hooks/useFavoriteMoversSelection.ts (1)
136-156: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
selectedCount === totalCount케이스에서 새로 조회한allIds를 그대로 삭제하지 마세요.
handleBulkDelete(Line 113)는isSelectAll이 아니어도selectedCount === totalCount이면 확인 모달을 연다. 이 경우는 "사용자가 로드된 항목을 모두 수동으로 선택했다"는 뜻이지, "전체선택 토글을 눌렀다"는 뜻이 아니다.그런데
handleConfirmDeleteAll은 이 두 경우를 구분하지 않는다.isSelectAll이 아니면idsToRemove = allIds를 그대로 사용한다.allIds는 확인 시점에 서버에서 새로 조회한 값이다.확인 모달이 열려 있는 동안 다른 세션에서 새 기사님을 찜하면,
allIds에 그 새 항목이 포함된다. 사용자가 "모두 해제"를 누르면 한 번도 선택하지 않은 항목까지 삭제된다.
isSelectAll이 아닌 경우에는 클릭 시점에 이미 확정된selectedIds를 삭제 대상으로 사용하세요. 이 경로에서는fetchAllFavoriteMoverIds()호출도 불필요합니다.🛠️ 제안하는 수정
const handleConfirmDeleteAll = useCallback(() => { void (async () => { setIsResolvingAllIds(true); try { - const allIds = await fetchAllFavoriteMoverIds(); - const excludedSet = new Set(excludedIds); - const idsToRemove = isSelectAll ? allIds.filter((id) => !excludedSet.has(id)) : allIds; + let idsToRemove: string[]; + if (isSelectAll) { + const allIds = await fetchAllFavoriteMoverIds(); + const excludedSet = new Set(excludedIds); + idsToRemove = allIds.filter((id) => !excludedSet.has(id)); + } else { + // 로드된 전체를 수동으로 선택한 경우: 확인 시점에 새로 조회하지 않고 + // 클릭 시점에 확정된 selectedIds만 삭제 대상으로 사용한다. + idsToRemove = selectedIds; + } try { await removeFavorites(idsToRemove); setIsDeleteConfirmOpen(false); } catch { // 전부 실패 시 mutation onError에서 토스트 처리. 모달은 재시도 가능하도록 유지 } } catch (error) { setToastMessage(getApiErrorMessage(error, DELETE_ERROR_MESSAGE)); } finally { setIsResolvingAllIds(false); } })(); - }, [excludedIds, isSelectAll, removeFavorites]); + }, [excludedIds, isSelectAll, removeFavorites, selectedIds]);🤖 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/useFavoriteMoversSelection.ts` around lines 136 - 156, Update handleConfirmDeleteAll so the non-isSelectAll path deletes the click-time selectedIds rather than newly fetched allIds, preserving the distinction between manually selecting every loaded item and using the select-all toggle. Avoid calling fetchAllFavoriteMoverIds in that path; retain the existing fetch-and-exclude behavior only for isSelectAll.
🤖 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.
Outside diff comments:
In `@src/hooks/useFavoriteMoversSelection.ts`:
- Around line 136-156: Update handleConfirmDeleteAll so the non-isSelectAll path
deletes the click-time selectedIds rather than newly fetched allIds, preserving
the distinction between manually selecting every loaded item and using the
select-all toggle. Avoid calling fetchAllFavoriteMoverIds in that path; retain
the existing fetch-and-exclude behavior only for isSelectAll.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 36b9a4e2-6eaa-454a-b683-61a67cc2404f
📒 Files selected for processing (6)
src/components/common/Header/Header.tsxsrc/components/common/Header/ProfileMenuTrigger.tsxsrc/components/mover/FavoriteMoversContent.tsxsrc/components/mover/MoverCard.tsxsrc/hooks/useBulkRemoveFavoriteMovers.tssrc/hooks/useFavoriteMoversSelection.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/mover/FavoriteMoversContent.tsx
- src/components/common/Header/ProfileMenuTrigger.tsx
- src/hooks/useBulkRemoveFavoriteMovers.ts
- src/components/common/Header/Header.tsx
| if (result.failedIds.length > 0) { | ||
| setIsSelectAll(false); | ||
| setExcludedIds([]); | ||
| setSelectedIds(result.failedIds); |
There was a problem hiding this comment.
부분 실패 시 isSelectAll을 false로 바꾸고 있어서, 화면에 아직 뜨지 않은 실패 항목이 개수 계산에서 빠질 수 있을 것 같습니다.
isSelectAll을 유지한 채 성공한 id만 excludedIds로 넘기는 방식을 사용하면 개수를 totalCount 기준으로 계산해서 로드되지 않은 실패 항목까지 선택 개수에 남기 때문에, 실제 selectedIds와 화면에 보이는 개수가 어긋나지 않을 것 같아요!
|
MoverCard를 memo로 감싸 불필요한 리렌더를 막고, 벌크 삭제를 allSettled로 처리해 성공한 항목만 반영하는 방식으로 잘 구현된 것 같습니다. To Reviewer에 적어주신 내용 관련해서는 캐시는 invalidateFavoriteRelatedQueries를 공통으로 빼서 단건 해제와 일괄 삭제 양쪽이 같은 함수를 onSettled에서 호출하고 있어서 어느 경로로 해제해도 무효화 범위가 같으니 사이드바나 견적 쪽이 어긋나지는 않을 것 같아요! 구현하느라 고생 많으셨습니다! |
3 participants
📋 작업 내용
찜한 기사님 전체 목록 페이지(
/movers/favorites)를 구현했습니다. Header 프로필 메뉴에서 진입할 수 있으며, 더보기 버튼을 통한 찜한 기사님 목록 조회 및 전체선택·부분선택·삭제 기능을 제공합니다.🔥 변경 사항
페이지 경로 변경
/favorites/movers→/movers/favorites(APP_ROUTES.MOVERS.FAVORITES)Header / GNB
ProfileMenuTrigger로 분리하고, 고객·기사 role별 메뉴·디자인을 반영했습니다./movers/favorites에서는 "기사님 찾기" 메뉴가 활성으로 잡히지 않도록 수정했습니다.찜한 기사님 목록 페이지
고객 인증 게이트, 로딩 스켈레톤, 에러 / Empty 상태
더보기(infinite) 버튼 도입: 한 번에 5명씩 로드
전체선택 + 선택 항목 삭제
공통
Checkbox를 추가하고,MoverCard선택 UI 확장-
Pagination: 기본 흰 배경 제거, disabled 시 hover 제거로딩 UI(스켈레톤 UI)
에러 UI(목록 로드 실패 시 에러 안내 및 다시 시도 버튼, 찜 해제 등 액션이 실패할 경우 토스트)
찜 일괄 해제 도중 부분 실패할 경우도 처리할 수 있도록 수정 (리뷰 반영)
Promise.allSettled로 각 DELETE 결과를 모은 뒤 일부만 성공한 경우 캐시를 성공한 id만 반영하고, 토스트도 그에 맞게 띄움더보기 방식을 선택한 이유
refactor (파일 역할)
기존에 한 파일에 몰려 있던 찜 로직을 역할별로 분리했습니다.
favoriteMoverCacheuseFavoriteMoveruseBulkRemoveFavoriteMoversuseFavoriteMoversuseFavoriteMoversInfiniteuseFavoriteMoversSelection체크박스 토글 리렌더링 개선 (리뷰 반영)
MoverCard가 함께 리렌더링되는 문제를 발견해, 변경된 카드만 리렌더링되도록 개선했습니다.MoverCard중심으로 줄였습니다.Before
MoverCard리렌더링 / Render 56.8msAfter
MoverCard리렌더링 / Render 16.6ms✅ 체크리스트
📷 스크린샷 (선택)
Header의 프로필 메뉴
기본 (더보기 버튼)
더보기 버튼 클릭 중
모두 로드됨
기사 선택
모두 삭제 (확인 모달)
빈 상태
💬 To Reviewer
현재 구현의 한계 및 후속 작업
nextCursor기반 조회로 변경하고, 일괄 해제 API를 추가할 예정입니다!Summary by CodeRabbit
새 기능
개선