Repository navigation
[URECA-72] Fix: refresh시 userId 세팅, summary 및 bookmark 수정 - #70
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthrough여러 컴포넌트와 훅에서 하드코딩된 사용자 ID(1)를 인증 상태의 동적 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 요약변경은 주로 인증 기반 데이터 분리와 UI의 날짜 포맷 통일에 초점이 있습니다. 훅 시그니처 변경으로 쿼리키에 예상 코드 리뷰 난이도🎯 3 (보통) | ⏱️ ~25분 주요 검토 포인트
필요하면 변경된 파일별 구체적 라인 리뷰와 제안 패치를 제공하겠습니다. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/services/api/interceptors.ts (1)
113-143: refresh 응답의 userId 검증 강화 필요현재 타입 가드가
accessToken만 검증하므로, 서버에서userId를 누락하거나 유효하지 않은 값(NaN, undefined, 음수)을 반환해도 통과합니다. 이후store.dispatch(authActions.setUserid(userId))로 잘못된 사용자 ID가 저장되어 요청 실패나 보안 문제가 발생할 수 있습니다.
isValidRefreshResponse에서userId필드와 타입을 검증하세요. 사용자 ID는 양의 정수여야 하므로Number.isInteger()또는Number.isFinite()와 함께 범위 체크를 권장합니다.✅ 제안 수정
function isValidRefreshResponse(data: unknown): data is RefreshResponse { return ( typeof data === 'object' && data !== null && 'accessToken' in data && typeof (data as RefreshResponse).accessToken === 'string' && + 'userId' in data && + Number.isInteger((data as RefreshResponse).userId) && + (data as RefreshResponse).userId > 0 ); }또는
Number.isFinite()사용 시:+ 'userId' in data && + Number.isFinite((data as RefreshResponse).userId)src/app/(private)/summary/_components/SummarySuccessPage.tsx (1)
29-36: 북마크 토글에 GET 메서드 사용은 HTTP 스펙 위반입니다상태를 변경하는 요청에는 PATCH/POST가 표준입니다. GET은 멱등성(idempotency)과 캐싱 안전성 때문에 부작용 없는 조회 전용이어야 하는데, 현재 북마크 상태를 변경하고 있습니다. 이는 프록시/브라우저 캐싱 무시, 서버 스펙 불일치로 실패할 가능성이 높습니다.
더 나아가 기존의
toggleSummaryBookmark함수는 이미 PATCH를 올바르게 사용하고 있고,useToggleBookmarkhook은 React Query의 useMutation으로 캐시 동기화도 자동 처리합니다. 이미 구현된 hook을 재사용하는 것이 권장됩니다.추가로 convertDate 함수가 SummarySuccessPage.tsx와 SummaryNavigateCard.tsx에 중복 정의되어 있으니, 공용 유틸리티로 분리하여 관리하세요.
✅ 제안 수정
-import { apiClient } from '@/services/api'; +import { useToggleBookmark } from '@/hooks/summary/useToggleBookmark'; - const handleToggleBookmark = async () => { - if (loading) return; - - try { - setLoading(true); - await apiClient.get(`/api/summaries/${summaryId}/bookmark`); - setBookmarked((prev) => !prev); - } finally { - setLoading(false); - } - }; + const { mutate: toggleBookmark, isPending } = useToggleBookmark(summaryId, userId); + + const handleToggleBookmark = () => { + toggleBookmark(undefined, { + onSuccess: () => setBookmarked((prev) => !prev), + }); + };
🤖 Fix all issues with AI agents
In `@src/app/`(private)/bookmark/page.tsx:
- Around line 12-13: Remove the debugging console.log and change the selector to
return only the userId to avoid unnecessary re-renders: stop selecting the
entire auth object and instead select userId directly via useAppSelector (i.e.,
ensure the selector passed to useAppSelector extracts only s.auth.userId),
keeping references to the existing symbol names useAppSelector and userId in
page.tsx.
- Line 15: The hook call useBookmarkedSummaries(userId) can trigger unnecessary
errors when userId is null; update the hook to guard the query with React
Query's enabled option (mirror useSummaryDetail's pattern) so the queryFn only
runs when userId is a valid number, i.e., set enabled: userId !== null (or
Number.isFinite(userId)) inside useBookmarkedSummaries while still using
getBookmarkedSummaries(userId) in the queryFn, or alternatively prevent calling
useBookmarkedSummaries from the component by early-returning (e.g., show auth
UI) when userId is falsy before invoking useBookmarkedSummaries.
In `@src/hooks/summary/useBookmarkedSummaries.ts`:
- Around line 6-12: The hook useBookmarkedSummaries currently throws inside
queryFn when userId is null, causing error states during auth load; instead add
TanStack Query's enabled option to the useQuery call (using the existing
queryKey ['summaries','bookmarks', userId]) so the query only runs when userId
!== null, remove the manual throw in queryFn, and keep queryFn to call
getBookmarkedSummaries(userId) and return res.data; this ensures the query is
disabled while userId is null and re-enables automatically when userId changes.
In `@src/hooks/summary/useSummaryList.ts`:
- Around line 16-22: useSummaryList currently uses a static queryKey
['summaries'] and will call the API even when userId is null, causing cache
pollution and wrong results; update useSummaryList so the queryKey includes
userId (e.g. ['summaries', userId]) and prevent queries when userId is null by
either adding enabled: !!userId and guarding the queryFn inside useSummaryList
(option 1) or by switching to TanStack Query's skipToken pattern for the
queryKey to avoid querying when userId is null (option 2); locate and change the
useQuery call in useSummaryList to implement one of these fixes.
🧹 Nitpick comments (2)
src/app/(private)/summary/_components/SummarySuccessPage.tsx (1)
41-51: 날짜 포맷 헬퍼 중복 가능성 → 공용 유틸로 분리 권장다른 컴포넌트에서도 동일한 로직이 추가되고 있어, utils/date 같은 공용 유틸로 분리하면 유지보수/일관성이 좋아집니다.
src/app/(private)/bookmark/page.tsx (1)
47-64: badges 파싱 로직을 유틸리티 함수로 분리하는 것을 권장합니다.현재 IIFE 패턴으로 작성된
keywords → badges변환 로직이 복잡하고, 다른 summary 관련 컴포넌트에서도 동일한 처리가 필요할 수 있습니다.분리의 장점:
- 테스트 용이성: 순수 함수로 분리하면 단위 테스트 작성이 쉬워집니다.
- 재사용성: 다른 컴포넌트에서 동일 로직 사용 가능
- 가독성: 컴포넌트 내 렌더링 로직이 간결해집니다.
♻️ 유틸리티 함수 분리 예시
// src/utils/parseKeywordsToBadges.ts export function parseKeywordsToBadges( keywords: string | string[] | undefined | null ): string[] | undefined { if (!keywords) return undefined; if (Array.isArray(keywords)) { return keywords; } if (typeof keywords === 'string') { try { const parsed = JSON.parse(keywords); return Array.isArray(parsed) ? parsed : [keywords]; } catch { return [keywords]; } } return undefined; }컴포넌트에서 사용:
+import { parseKeywordsToBadges } from '@/utils/parseKeywordsToBadges'; {data?.map((summary) => { - const badges = (() => { - // ... 기존 로직 - })(); + const badges = parseKeywordsToBadges(summary.keywords); return ( // ... ); })}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/hooks/summary/useSummaryList.ts`:
- Around line 32-35: The current inline JSON.parse in useSummaryList.ts can
throw on malformed strings; create a defensive helper (e.g., safeParseJSON) in
src/utils/parse that accepts the string and a fallback (default []), tries
JSON.parse inside try/catch and returns fallback on error, then replace the
ternary in useSummaryList (the typeof item.keywords === 'string' ?
JSON.parse(item.keywords) : (item.keywords ?? [])) to call
safeParseJSON(item.keywords, []) so malformed JSON becomes a controlled fallback
and prevents runtime exceptions.
🧹 Nitpick comments (2)
src/hooks/summary/useSummaryList.ts (1)
22-26: API 응답 타입 명시로 타입 안정성 개선 권장현재
res.data또는res.data.data분기 처리는 동작하지만, API 응답 타입을 명시하면 타입 안정성과 유지보수성이 향상됩니다.📝 API 응답 타입 정의 예시
// src/types/summary/summary.ts에 추가 interface ApiSummaryResponse { data?: ApiSummaryItem[]; } // 훅에서 사용 const res = await apiClient.get<ApiSummaryItem[] | ApiSummaryResponse>('/api/summaries', { params: { userId }, }); const list: ApiSummaryItem[] = Array.isArray(res.data) ? res.data : (res.data?.data ?? []);백엔드 API 스펙이 확정되면 응답 구조를 단일화하는 것도 좋은 방향입니다.
src/app/(private)/bookmark/page.tsx (1)
12-42: userId가 null일 때 빈 상태로 보이는 UX 확인 필요
userId가 아직 없거나 인증이 완료되지 않은 순간에는 쿼리가 비활성화되더라도 화면이 “북마크한 요약이 없습니다”로 보일 수 있습니다. 이 페이지가 미들웨어로 보호되는지 확인하고, 아니라면 별도의 로그인/로딩 분기를 추가하는 편이 안전합니다.✅ 예시 분기
+ if (userId == null) { + return ( + <> + <Header /> + <div className="py-12 text-center text-sm text-gray-400">로그인이 필요합니다</div> + <BottomNav /> + </> + ); + }
Key Changes
작업 내역
📸 스크린샷
💬 공유사항 to 리뷰어
비고
PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.