Conversation
Walkthrough사전 의견(pre-opinions) 기능을 새로 추가: 타입·API·엔드포인트·모크·쿼리 훅·컴포넌트·페이지·라우트 및 관련 공유 UI 확장으로 의견 조회 및 본인 삭제 흐름을 구현합니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as PreOpinionListPage
participant Hook as usePreOpinionAnswers
participant Query as QueryClient
participant API as API_Layer
User->>Page: 진입(gatheringId, meetingId)
Page->>Hook: usePreOpinionAnswers 호출
Hook->>Query: queryKey 확인
alt 캐시 없음
Query->>API: getPreOpinionAnswers 요청
API-->>Query: PreOpinionAnswersData 반환
else 캐시 있음
Query-->>Hook: 캐시 데이터 반환
end
Hook-->>Page: data, isLoading 반환
Page->>Page: activeMember 결정 및 렌더링(PreOpinionMemberList, PreOpinionDetail)
sequenceDiagram
participant User
participant Detail as PreOpinionDetail
participant Modal as GlobalModalStore
participant Mutate as useDeleteMyPreOpinionAnswer
participant API as API_Layer
participant Query as QueryClient
User->>Detail: 삭제 버튼 클릭
Detail->>Modal: openConfirm 호출
Modal-->>User: 확인 모달 표시
User->>Modal: 확인 선택
Modal-->>Detail: onConfirm 콜백
Detail->>Mutate: mutate(delete params)
Mutate->>API: deleteMyPreOpinionAnswer 요청
alt 성공
API-->>Mutate: 200 OK
Mutate->>Query: invalidateQueries(preOpinionQueryKeys.answers())
Query-->>Page: refetch 트리거
else 실패
API-->>Mutate: Error
Mutate-->>Detail: error 반환
Detail->>Modal: openError 호출(에러 표시)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pages/PreOpinions/PreOpinionListPage.tsx`:
- Around line 26-30: The useEffect is being retriggered because you destructure
the whole modal store causing openError's reference to change; change the
subscription to only the openError function by selecting it directly from
useGlobalModalStore (e.g., useGlobalModalStore(state => state.openError)) so
openError's identity is stable and the effect depending on openError does not
rerun unnecessarily; update the existing destructuring at the top of
PreOpinionListPage (replace const { openError } = useGlobalModalStore() or
similar) and keep the useEffect as-is to rely on the stable openError reference.
🧹 Nitpick comments (10)
src/shared/ui/UserChip.tsx (1)
10-10:variant타입을AvatarVariant에서 가져오는 것을 고려해보세요.
'leader' | 'host' | 'member'를 직접 정의하면Avatar의AvatarVariant타입이 변경될 때 동기화가 깨질 수 있습니다.Avatar.tsx에서AvatarVariant를 export하고 재사용하면 유지보수가 편해집니다.Also applies to: 34-34, 70-70
src/features/preOpinions/preOpinions.mock.ts (1)
100-102: 목 데이터 참조 공유 주의
getMockPreOpinionAnswers가 동일 객체 참조를 반환합니다. 테스트나 mock 환경에서 반환값을 변형(mutate)하면 이후 호출에도 영향을 줍니다. 현재 사용처가 읽기 전용이면 괜찮지만, 안전하게 하려면structuredClone을 고려해보세요.src/features/preOpinions/preOpinions.types.ts (1)
63-73:GetPreOpinionAnswersParams와DeleteMyPreOpinionAnswerParams의 구조가 동일합니다.현재는
{ gatheringId: number; meetingId: number }로 동일하지만, 의미적 분리를 위해 별도 유지하는 것도 합리적입니다. 향후 파라미터가 분기될 가능성이 없다면 공통 타입 추출도 고려해보세요.src/features/preOpinions/components/PreOpinionMemberList.tsx (1)
43-47:onClick내부의isSubmitted체크가 중복입니다.
UserChip은disabled상태일 때 내부적으로onClick을 호출하지 않습니다(Line 49 of UserChip.tsx).disabled={!member.isSubmitted}를 이미 전달하고 있으므로 여기서 다시 체크할 필요는 없습니다.제안
- onClick={() => { - if (member.isSubmitted) { - onSelectMember(member.memberInfo.userId) - } - }} + onClick={() => onSelectMember(member.memberInfo.userId)}src/routes/index.tsx (1)
104-107: 라우트 정의 방식이 기존과 다릅니다.기존 라우트들은
`${ROUTES.GATHERINGS}/:gatheringId/meetings/:meetingId`형태의 템플릿 리터럴을 사용하는데(Line 101), 여기서는ROUTES.PRE_OPINIONS(':gatheringId', ':meetingId')헬퍼 함수를 사용합니다. 기능적으로 동일하지만, 코드베이스 내 일관성 측면에서 한 가지 방식으로 통일하면 좋겠습니다.src/features/preOpinions/index.ts (1)
10-10: mock 모듈을 배럴에서 re-export하는 것은 불필요한 코드 구조입니다.현재 코드에서는
preOpinions.api.ts가 mock을 직접 import하고, 프로덕션 코드는 배럴에서 mock을 import하지 않습니다. 다만 배럴에서 export하면 번들 분석이 복잡해지고, 의도하지 않은 import 경로가 열립니다. Vite의 tree-shaking이 처리하더라도, 명시적으로 배럴에서 제외하는 것이 명확합니다.// API export * from './preOpinions.api' export * from './preOpinions.endpoints' -export * from './preOpinions.mock' // Types export * from './preOpinions.types'Mock이 필요한 내부 로직에서는 이미 직접 import 중이므로, 배럴 export는 제거해도 문제없습니다.
src/pages/PreOpinions/PreOpinionListPage.tsx (2)
21-24:Number()변환이 중복 수행됩니다.
Number(gatheringId),Number(meetingId)가 라인 22-23과 78-79에서 반복됩니다. → params가undefined일 때 매번NaN을 생성하며, 값 불일치 가능성도 생깁니다.컴포넌트 상단에서 한 번만 변환하세요.
♻️ 제안
export default function PreOpinionListPage() { const { gatheringId, meetingId } = useParams<{ gatheringId: string; meetingId: string }>() + const numericGatheringId = Number(gatheringId) + const numericMeetingId = Number(meetingId) const navigate = useNavigate() const { openError } = useGlobalModalStore() ... const { data, isLoading, error } = usePreOpinionAnswers({ - gatheringId: Number(gatheringId), - meetingId: Number(meetingId), + gatheringId: numericGatheringId, + meetingId: numericMeetingId, })그리고 라인 78-79에서도 동일하게 사용:
<PreOpinionDetail member={selectedMember} topics={data.topics} - gatheringId={Number(gatheringId)} - meetingId={Number(meetingId)} + gatheringId={numericGatheringId} + meetingId={numericMeetingId} />Also applies to: 78-79
32-45: IntersectionObserver의[isLoading]의존성 — 의도는 맞지만 주석이 있으면 좋겠습니다.
isLoading이true일 때 라인 56에서 early return하므로 sentinel이 DOM에 없고,false가 된 후에야 observer를 연결하는 구조입니다. 의존성 배열에isLoading을 넣은 이유가 자명하지 않으니, 간단한 주석을 추가하면 이후 유지보수에 도움이 됩니다.src/features/preOpinions/preOpinions.api.ts (1)
54-58:deleteMyPreOpinionAnswer에 mock 분기가 없습니다.
getPreOpinionAnswers에는USE_MOCK분기가 있지만, 삭제 함수에는 없습니다. → mock 모드 개발 시 삭제 호출이 실제 API로 나가거나 실패합니다.의도적이라면 무시해도 되지만, mock 모드에서의 개발 편의를 위해 동일하게 mock 분기를 추가하는 것을 권장합니다.
src/features/preOpinions/components/PreOpinionDetail.tsx (1)
77-106: 키워드 필터링이 각 타입별로 두 번씩 수행됩니다.
filter(k => k.type === 'BOOK')이 조건 확인(라인 77)과 렌더링(라인 82)에서 중복 호출됩니다.'IMPRESSION'도 동일(라인 93, 98).→ 리스트가 작아 성능 문제는 아니지만, 변수로 추출하면 가독성과 유지보수성이 좋아집니다.
♻️ 제안
+ const bookKeywords = bookReview.keywords.filter((k) => k.type === 'BOOK') + const impressionKeywords = bookReview.keywords.filter((k) => k.type === 'IMPRESSION')이후 조건과 렌더링에서
bookKeywords,impressionKeywords를 재사용하세요.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pages/PreOpinions/PreOpinionListPage.tsx`:
- Around line 26-30: The useEffect only handles the specific
ErrorCode.BOOK_REVIEW_ACCESS_DENIED_NOT_WRITTEN case so other failures leave
isLoading/data undefined and the UI shows "멤버를 선택해주세요"; update the error
handling in PreOpinionListPage by adding a branch in the existing useEffect (or
a separate effect) that catches all other errors (error != null &&
!error.is(ErrorCode.BOOK_REVIEW_ACCESS_DENIED_NOT_WRITTEN)) and calls openError
with a generic title and error.userMessage (or a fallback message), ensures any
loading flags are cleared, and/or sets a local error state to render a fallback
error UI instead of the empty member prompt; reference the existing useEffect,
error, ErrorCode.BOOK_REVIEW_ACCESS_DENIED_NOT_WRITTEN, openError, navigate, and
the component render path that uses isLoading/data to implement the fix.
🧹 Nitpick comments (3)
src/pages/PreOpinions/PreOpinionListPage.tsx (3)
21-24:Number(gatheringId)중복 파싱 → 변수로 추출 권장
Number(gatheringId)/Number(meetingId)가 Line 22-23과 Line 78-79에서 반복됩니다. 상단에서 한 번만 파싱하면 중복을 제거하고,NaN전파 위험도 한 곳에서 관리할 수 있습니다.♻️ 제안
+ const numGatheringId = Number(gatheringId) + const numMeetingId = Number(meetingId) + const { data, isLoading, error } = usePreOpinionAnswers({ - gatheringId: Number(gatheringId), - meetingId: Number(meetingId), + gatheringId: numGatheringId, + meetingId: numMeetingId, })Line 78-79도 동일하게 교체:
- gatheringId={Number(gatheringId)} - meetingId={Number(meetingId)} + gatheringId={numGatheringId} + meetingId={numMeetingId}
32-45: IntersectionObserver 의존성이 간접적
isLoading이 의존성으로 들어가 있지만, 실제 의도는 sentinel DOM 노드의 마운트 여부입니다. 현재 동작은 정상이지만, 향후 로딩 UI 변경(Suspense 전환 등) 시 observer가 의도대로 붙지 않을 수 있습니다.
sentinelRef를 callback ref로 전환하면 DOM 존재 여부에 직접 반응할 수 있어 더 견고합니다. 다만 현재 구조에서는 큰 문제가 아니므로 참고 수준입니다.
48-54: 삭제 후selectedMemberId상태 불일치 가능성본인 의견 삭제 → 캐시 무효화 →
data.members에서 해당 멤버가 사라질 경우,selectedMemberId는 여전히 이전 값을 보유합니다.selectedMember가undefined가 되어 placeholder가 표시되므로 크래시는 없지만, 삭제 성공 시setSelectedMemberId(null)로 리셋하면 자연스럽게 첫 번째 제출 멤버로 전환되어 UX가 개선됩니다.
PreOpinionDetail내부의 삭제 콜백에서 처리하거나,data변경 시 유효성을 검사하는 effect를 추가하는 방법이 있습니다.
🚀 풀 리퀘스트 제안
📋 작업 내용
사전의견(Pre-Opinions) 조회 및 삭제 기능 구현
🔧 변경 사항
📸 스크린샷 (선택 사항)
📄 기타
Summary by CodeRabbit
신규 기능
UI 개선
라우팅