[Fix] 알림페이지 디자인 반영 / 버그 일부 수정 #269
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 개요여러 알림 UI 개선사항(무한 스크롤, 읽음 표시), API 오류 처리 시 로그아웃 추가, 쿼리 키 관리 단순화, 마이너 스타일 조정이 포함된 변경사항입니다. 변경사항
예상 코드 리뷰 노력🎯 3 (중간) | ⏱️ ~25분 관련 가능성 있는 PR
제안 레이블
제안 검토자
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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 |
|
@coderabbitai review |
🎨 Storybook Report✅ 변경 사항이 없습니다 모든 Story가 이전 빌드와 동일합니다.
|
🎭 Playwright Report✨ E2E Test가 성공적으로 완료되었습니다. Test 요약 내용을 확인해주세요.
📊 Test Summary
📜 Test Details✅ Passed Tests (3)
|
🚀 PR Preview Report✨ Build가 성공적으로 완료되었습니다. Preview에서 변경사항을 확인하세요.
|
📊 Coverage Report
📉 #269을 main에 병합하면 coverage가 Coverage 요약@@ Coverage Diff @@
## main #269 +/- ##
===========================================
- Coverage 42.06% 41.89% -0.17%
===========================================
Files 194 194 0
Lines 8498 8533 +35
Branches 387 387 0
===========================================
Hits 3575 3575 0
+ Misses 4923 4958 +35 영향받은 파일
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/lib/formatDateTime.ts (1)
13-14: 서버/클라이언트 시간 차이 처리가 적절합니다.미래 또는 현재 시간에 대해 '0초 전'을 반환하여 시간 차이 문제를 방어적으로 처리합니다.
선택적 개선: 디버깅을 위한 로깅 추가
시간 불일치가 발생할 때 디버깅을 쉽게 하기 위해 개발 환경에서 로깅을 추가하는 것을 고려해보세요:
if (dateInput.getTime() >= dateNow.getTime()) { + if (process.env.NODE_ENV === 'development') { + console.warn('formatTimeAgo: Future timestamp detected', { dateInput, dateNow }); + } return '0초 전'; }src/components/pages/notification/notification-header/index.tsx (1)
38-48: "모두 읽음" 버튼 구현이 적절합니다.읽지 않은 알림이 없을 때 버튼을 비활성화하고 적절한 스타일을 적용합니다.
선택적 개선: 에러 처리 개선
현재
alert()를 사용하는 대신, 토스트 메시지나 더 나은 UX 패턴을 고려해보세요:// 예시: 토스트 사용 (프로젝트에 토스트 라이브러리가 있는 경우) const handleReadAllClick = async () => { if (unReadCount === 0) return; try { await mutateAsync(); // toast.success('모든 알림을 읽음 처리했습니다.'); } catch { // toast.error('요청 처리에 실패했습니다.'); alert('요청 처리에 실패했습니다.'); } };src/app/notification/page.tsx (2)
30-30: 센티널 요소의 사용자 경험을 개선하세요.현재 "다음" 텍스트가 표시되는 센티널 요소가 있습니다. 일반적으로 Intersection Observer 센티널은 다음 중 하나로 구현합니다:
- 투명한 요소 (높이만 확보)
- 로딩 스피너 또는 인디케이터
의도적으로 "다음"을 보여주려는 것이 아니라면, 더 나은 UX를 위해 로딩 인디케이터를 추가하거나 보이지 않게 처리하는 것을 권장합니다.
🔎 센티널 요소 개선 제안
옵션 1: 로딩 인디케이터 표시
- {hasNextPage && <div ref={fetchObserverRef}>다음</div>} + {hasNextPage && ( + <div ref={fetchObserverRef} className="flex justify-center py-4"> + {isFetchingNextPage ? <LoadingSpinner /> : <div className="h-4" />} + </div> + )}옵션 2: 투명한 센티널 (권장)
- {hasNextPage && <div ref={fetchObserverRef}>다음</div>} + {hasNextPage && <div ref={fetchObserverRef} className="h-4" aria-hidden="true" />}
8-33: 로딩 및 에러 상태 처리를 고려하세요.현재 컴포넌트는 초기 로딩 상태와 에러 상태를 처리하지 않습니다.
useGetNotificationsInfinite훅에서 반환되는isLoading,isError,error등을 활용하여 사용자에게 피드백을 제공하는 것을 권장합니다.🔎 로딩 및 에러 상태 처리 예시
const { data: list, fetchNextPage, hasNextPage, isFetchingNextPage, + isLoading, + isError, + error, } = useGetNotificationsInfinite({ size: 20 }); const fetchObserverRef = useIntersectionObserver({ onIntersect: () => { if (hasNextPage && !isFetchingNextPage) { fetchNextPage(); } }, }); - if (!list) return; + if (isLoading) return <LoadingState />; + if (isError) return <ErrorState error={error} />; + if (!list) return null; return ( <section> <NotificationHeader /> {list.length > 0 && list.map((item) => <NotificationCard key={item.id} item={item} />)} {hasNextPage && <div ref={fetchObserverRef}>다음</div>} {list.length === 0 && <EmptyState>아직 받은 알림이 없어요.</EmptyState>} </section> );
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/api/core/index.tssrc/app/notification/page.tsxsrc/components/layout/header/index.tsxsrc/components/pages/notification/notification-card/index.tsxsrc/components/pages/notification/notification-header/index.tsxsrc/hooks/use-notification/use-notification-connect-sse/index.tssrc/hooks/use-notification/use-notification-get-list/index.tssrc/lib/formatDateTime.tssrc/lib/query-key/query-key-notification/index.tssrc/styles/base.css
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T09:40:37.733Z
Learnt from: HopeFullee
Repo: WeGo-Together/WeGo_FrontEnd PR: 205
File: src/components/pages/post-meetup/fields/cap-field/index.tsx:47-47
Timestamp: 2025-12-17T09:40:37.733Z
Learning: In TanStack Form with Zod validation, when field.state.meta.isValid is false, field.state.meta.errors is guaranteed to have at least one entry. Therefore, in an isInvalid check defined as field.state.meta.isTouched && !field.state.meta.isValid, safely access field.state.meta.errors[0] without extra null/empty checks. This guideline applies to TSX form fields under src/components/pages (and similar forms using the same pattern).
Applied to files:
src/components/pages/notification/notification-card/index.tsxsrc/components/pages/notification/notification-header/index.tsx
🧬 Code graph analysis (5)
src/hooks/use-notification/use-notification-get-list/index.ts (1)
src/lib/query-key/query-key-notification/index.ts (1)
notificationKeys(1-5)
src/hooks/use-notification/use-notification-connect-sse/index.ts (1)
src/lib/query-key/query-key-notification/index.ts (1)
notificationKeys(1-5)
src/components/layout/header/index.tsx (1)
src/lib/utils.ts (1)
cn(74-76)
src/api/core/index.ts (1)
src/api/index.ts (1)
API(25-25)
src/components/pages/notification/notification-header/index.tsx (2)
src/components/pages/notification/index.ts (1)
NotificationHeader(2-2)src/lib/utils.ts (1)
cn(74-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: coverage
- GitHub Check: chromatic
🔇 Additional comments (8)
src/components/pages/notification/notification-card/index.tsx (1)
34-34: 읽지 않은 알림의 배경색 변경이 적절합니다.
bg-mint-100에서bg-mint-50으로 변경하여 더 부드러운 시각적 강조를 제공합니다. 디자인 의도와 잘 맞습니다.src/styles/base.css (1)
9-11: 비활성화된 버튼의 커서 스타일 추가가 좋습니다.전역 스타일로
button:disabled에cursor: not-allowed를 추가하여 일관된 UX를 제공합니다.src/components/layout/header/index.tsx (1)
26-38: 읽지 않은 알림이 없을 때 배지를 숨기는 처리가 좋습니다.
unReadCount > 0조건으로 배지 요소들을 렌더링하여 읽지 않은 알림이 없을 때 깔끔한 UI를 제공합니다. PR 목표에 명시된 디자인 반영과 일치합니다.src/hooks/use-notification/use-notification-connect-sse/index.ts (2)
55-55: SSE 수신 시 알림 목록 즉시 갱신 로직이 추가되어 버그가 수정되었습니다.
notificationKeys.list()에 대한 무효화를 추가하여 SSE 알림 수신 시 알림 목록이 즉시 업데이트됩니다. PR에서 언급된 버그 수정과 일치합니다.
66-66: SSE 오류 시 토큰 클리어 처리가 적절합니다.SSE 연결 오류 발생 시
accessToken.value를null로 설정하여 재인증 플로우를 활성화합니다. API 인터셉터의 로그아웃 처리와 일관성 있게 동작합니다.src/app/notification/page.tsx (2)
9-14: 무한 스크롤 구현이 적절합니다.
size: 20으로 변경하고fetchNextPage,hasNextPage,isFetchingNextPage를 적절히 구조 분해하여 무한 스크롤에 필요한 모든 요소를 갖추었습니다.
16-22: Intersection Observer 로직이 정확합니다.
hasNextPage와!isFetchingNextPage를 모두 확인하여 중복 페칭을 방지하는 적절한 가드가 구현되어 있습니다.src/lib/query-key/query-key-notification/index.ts (1)
3-3: 쿼리 키 정적화가 올바르게 구현되었습니다.파라미터 기반 쿼리 키에서 정적 키
() => ['notifications', 'list']로 변경한 것은 무한 스크롤, SSE 즉시 업데이트, 모두 읽음 처리 등 PR 목적과 완벽하게 일치합니다. 모든 호출 사이트(useGetNotificationsInfinite,useUpdateNotificationRead,useUpdateNotificationReadAll, SSE 연결)에서 파라미터 없이 정확히 사용하고 있으며, 캐시 무효화가 단순하고 일관되게 처리됩니다.
| await API.authService.refresh(); | ||
| return baseAPI(originalRequest); | ||
| } catch (refreshError) { | ||
| await API.authService.logout(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the authService implementation
find . -type f -name "*.ts" -o -name "*.js" | head -20Repository: WeGo-Together/WeGo_FrontEnd
Length of output: 656
🏁 Script executed:
# Look at the context around line 59 in src/api/core/index.ts
cat -n src/api/core/index.ts | sed -n '45,75p'Repository: WeGo-Together/WeGo_FrontEnd
Length of output: 1367
🏁 Script executed:
# Find authService logout implementation
rg "logout\s*\(" --type ts -A 10 | head -50Repository: WeGo-Together/WeGo_FrontEnd
Length of output: 2186
🏁 Script executed:
cat -n src/api/service/auth-service/index.tsRepository: WeGo-Together/WeGo_FrontEnd
Length of output: 1533
logout() 호출이 에러 처리 없이 사용자 리다이렉션을 차단할 수 있습니다.
logout()은 API 요청을 수행하며 예외를 던질 수 있습니다. 현재 코드에서 이 호출이 try-catch로 보호되지 않으면, 예외 발생 시 60-69번 줄의 리다이렉션 로직이 실행되지 않아 사용자가 로그인 페이지로 이동하지 못합니다.
try-catch로 감싸서 안전하게 처리하세요:
try {
await API.authService.logout();
} catch (error) {
console.error('[LOGOUT ERROR]', error);
}
// 예외 발생 여부와 관계없이 리다이렉션 진행🤖 Prompt for AI Agents
In src/api/core/index.ts around line 59, the call to await
API.authService.logout() can throw and prevent the subsequent redirect logic
from running; wrap the logout call in a try-catch, log the caught error (e.g.
with console.error or the project logger) and allow execution to continue so the
redirect logic on lines ~60-69 runs regardless of whether logout failed.
| const handleReadAllClick = () => { | ||
| if (unReadCount === 0) return; | ||
| try { | ||
| mutateAsync(); | ||
| } catch { | ||
| alert('요청 처리에 실패했습니다.'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
비동기 mutation을 await 하지 않아 에러 처리가 작동하지 않습니다.
mutateAsync()는 Promise를 반환하지만 await 없이 호출되고 있습니다. 이로 인해 mutation이 실패해도 catch 블록이 실행되지 않습니다.
🔎 수정 제안
- const handleReadAllClick = () => {
+ const handleReadAllClick = async () => {
if (unReadCount === 0) return;
try {
- mutateAsync();
+ await mutateAsync();
} catch {
alert('요청 처리에 실패했습니다.');
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleReadAllClick = () => { | |
| if (unReadCount === 0) return; | |
| try { | |
| mutateAsync(); | |
| } catch { | |
| alert('요청 처리에 실패했습니다.'); | |
| } | |
| }; | |
| const handleReadAllClick = async () => { | |
| if (unReadCount === 0) return; | |
| try { | |
| await mutateAsync(); | |
| } catch { | |
| alert('요청 처리에 실패했습니다.'); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/components/pages/notification/notification-header/index.tsx around lines
19 to 26, the handler calls mutateAsync() without awaiting it so the try/catch
won't catch rejected promises; make the handler async and await mutateAsync()
(or return the promise and handle .catch) so errors are properly caught, and
keep the existing alert inside the catch block to show failure.
| eventSourceRef.current = null; | ||
| }; | ||
| }, [accessToken.value, queryClient]); | ||
| }, [accessToken, queryClient]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find useAuth hook implementation
echo "=== Finding useAuth hook ==="
fd -n "useAuth" src/ -t f
echo -e "\n=== Checking auth provider/hook implementations ==="
find src/ -type f -name "*auth*" | head -20Repository: WeGo-Together/WeGo_FrontEnd
Length of output: 559
🏁 Script executed:
#!/bin/bash
echo "=== Searching for useAuth hook definition ==="
rg -n "export.*useAuth|const useAuth" src/ -t ts -t tsx
echo -e "\n=== Checking use-notification file for accessToken usage ==="
cat src/hooks/use-notification/use-notification-connect-sse/index.ts | head -100
echo -e "\n=== Searching for accessToken in hooks/auth or providers ==="
find src/hooks -type f -name "*auth*"
find src/providers -type f -name "*auth*"Repository: WeGo-Together/WeGo_FrontEnd
Length of output: 2455
🏁 Script executed:
#!/bin/bash
echo "=== Finding provider-auth implementation ==="
find src/providers -type f | grep -i auth
echo -e "\n=== Checking provider-auth files ==="
ls -la src/providers/
echo -e "\n=== Reading provider-auth implementation ==="
cat src/providers/provider-auth.ts 2>/dev/null || cat src/providers/provider-auth.tsx 2>/dev/null || echo "File not found"Repository: WeGo-Together/WeGo_FrontEnd
Length of output: 751
🏁 Script executed:
#!/bin/bash
echo "=== Reading provider-auth/index.tsx ==="
cat src/providers/provider-auth/index.tsxRepository: WeGo-Together/WeGo_FrontEnd
Length of output: 1258
의존성 배열을 수정하세요: [accessToken.value, queryClient]로 변경이 필요합니다.
accessToken 객체는 AuthProvider가 렌더링될 때마다 새로 생성되므로 참조가 매번 변경됩니다. 현재 [accessToken, queryClient] 의존성은 토큰 값이 변경되지 않아도 매 렌더링마다 effect를 재실행하게 되어, SSE 연결이 불필요하게 계속 재연결됩니다. 대신 [accessToken.value, queryClient]를 사용하면 실제 토큰 값이 변경될 때만 effect가 재실행됩니다.
🤖 Prompt for AI Agents
In src/hooks/use-notification/use-notification-connect-sse/index.ts around line
75, the effect dependency currently uses the accessToken object which is
recreated on every render; change the dependency to accessToken.value (i.e., use
[accessToken.value, queryClient]) so the effect only re-runs when the actual
token string changes; update the dependency array accordingly and ensure any
linting references to accessToken are adjusted to use its value.
| export const useGetNotificationsInfinite = (params?: GetNotificationListQueryParams) => { | ||
| return useInfiniteQuery({ | ||
| queryKey: notificationKeys.list(params), | ||
| queryKey: notificationKeys.list(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find GetNotificationListQueryParams definition
echo "=== GetNotificationListQueryParams type definition ==="
rg -n "GetNotificationListQueryParams" --type=ts --type=tsx -B 2 -A 8
echo -e "\n=== useGetNotificationsInfinite usage ==="
rg -n "useGetNotificationsInfinite" --type=ts --type=tsx -B 2 -A 5
echo -e "\n=== notificationKeys.list usage ==="
rg -n "notificationKeys\.list" --type=ts --type=tsx -B 1 -A 1Repository: WeGo-Together/WeGo_FrontEnd
Length of output: 306
🏁 Script executed:
#!/bin/bash
# Find GetNotificationListQueryParams definition
echo "=== GetNotificationListQueryParams type definition ==="
rg -n "GetNotificationListQueryParams" -B 2 -A 8
echo -e "\n=== useGetNotificationsInfinite usage ==="
rg -n "useGetNotificationsInfinite" -B 2 -A 5
echo -e "\n=== notificationKeys structure ==="
rg -n "notificationKeys" -B 1 -A 1 | head -50
echo -e "\n=== Checking the hook file context ==="
cat -n src/hooks/use-notification/use-notification-get-list/index.tsRepository: WeGo-Together/WeGo_FrontEnd
Length of output: 11371
queryKey에 params의 size 파라미터를 포함하세요.
현재 useGetNotificationsInfinite는 size 파라미터를 받지만 queryKey에 포함하지 않습니다.
useGetNotificationsInfinite({ size: 20 })과useGetNotificationsInfinite({ size: 50 })을 순차적으로 호출하면 둘 다 동일한 캐시 키['notifications', 'list']를 사용합니다.- 결과적으로
size: 50으로 호출했을 때size: 20의 캐시된 데이터가 반환되어 의도하지 않은 데이터가 표시됩니다.
notificationKeys.list()를 notificationKeys.list(params?.size)로 변경하거나, 더 포괄적으로는 params를 queryKey에 포함하는 것을 권장합니다.
🤖 Prompt for AI Agents
In src/hooks/use-notification/use-notification-get-list/index.ts around line 9,
the queryKey uses notificationKeys.list() but omits the params.size, causing
different size requests to share the same cache key; update the queryKey to
include the size (e.g., notificationKeys.list(params?.size)) or include the
whole params object in the key so calls with different size values produce
distinct cache entries.
📝 변경 사항
💡디자인 반영
💡모두 읽음 처리
모두 읽음 버튼 클릭 시 모든 알림을 읽음 처리 합니다.
💡알림 목록 실시간 업데이트
SSE 수신 시 알림 목록이 업데이트 되지 않는 버그를 수정하였습니다.
💡알림 페이지 무한 스크롤 적용
Todo
웹페이지 이용 중 token이 만료될 경우 AuthProvider의 accessToken 상태 동기화 작업 필요
🔗 관련 이슈
Closes #
🧪 테스트 방법
📸 스크린샷 (선택)
📋 체크리스트
💬 추가 코멘트
CodeRabbit Review는 자동으로 실행되지 않습니다.
Review를 실행하려면 comment에 아래와 같이 작성해주세요
Summary by CodeRabbit
릴리스 노트
새로운 기능
스타일
✏️ Tip: You can customize this high-level summary in your review settings.