Repository navigation
feat: 알림 조회 요청 관련 enable 옵션 추가 및 레드닷 로직 추가 - #294
Conversation
shinwokkang
added
✨ feat
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a notification unread indicator in the header and updates the notification preview query to respect the user's authentication status. Review feedback highlighted a potential issue where cached data from a previous session might cause the unread indicator or dropdown to display incorrect information after logout; suggestions were made to explicitly filter the data based on the login state. Furthermore, it was noted that relying on a limited preview of five notifications to determine the unread status may lead to inaccuracies if older unread notifications exist.
| const { data: notifications } = useNotificationPreviewQuery(5, isLoggedIn); | ||
| const hasUnread = notifications?.some((notif) => !notif.read) ?? false; |
There was a problem hiding this comment.
로그아웃 시 TanStack Query 캐시에 이전 사용자의 알림 데이터가 남아있을 수 있습니다. isLoggedIn이 false가 되어도 캐시된 데이터로 인해 hasUnread가 true로 계산되어 레드닷이 잘못 표시될 수 있습니다. isLoggedIn 상태를 확인하여 데이터를 필터링하는 것이 안전합니다.
| const { data: notifications } = useNotificationPreviewQuery(5, isLoggedIn); | |
| const hasUnread = notifications?.some((notif) => !notif.read) ?? false; | |
| const { data: notificationsData } = useNotificationPreviewQuery(5, isLoggedIn); | |
| const notifications = isLoggedIn ? notificationsData : []; | |
| const hasUnread = notifications?.some((notif) => !notif.read) ?? false; |
| export default function NotificationDropdown() { | ||
| const { data: notifications, isLoading } = useNotificationPreviewQuery(5); | ||
| const { isLoggedIn } = useAuthStore(); | ||
| const { data: notifications, isLoading } = useNotificationPreviewQuery(5, isLoggedIn); |
There was a problem hiding this comment.
로그아웃 후에도 TanStack Query 캐시에 이전 사용자의 데이터가 남아있을 수 있어, 비로그인 상태에서 드롭다운을 열었을 때 이전 사용자의 알림이 노출될 위험이 있습니다. isLoggedIn 상태일 때만 데이터를 사용하도록 제한하는 것이 안전합니다.
| const { data: notifications, isLoading } = useNotificationPreviewQuery(5, isLoggedIn); | |
| const { data: notificationsData, isLoading } = useNotificationPreviewQuery(5, isLoggedIn); | |
| const notifications = isLoggedIn ? notificationsData : []; |
| const notificationRef = useRef<HTMLDivElement>(null); | ||
|
|
||
| const { data: notifications } = useNotificationPreviewQuery(5, isLoggedIn); | ||
| const hasUnread = notifications?.some((notif) => !notif.read) ?? false; |
There was a problem hiding this comment.
현재 로직은 최신 5개의 알림(size: 5)만 확인하여 읽지 않은 상태를 판단합니다. 만약 읽지 않은 알림이 6번째 이후에 있다면 레드닷이 표시되지 않는 문제가 발생할 수 있습니다. 전체 읽지 않은 알림 여부를 반환하는 별도의 API가 있다면 해당 API를 사용하는 것이 더 정확합니다.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/components/layout/Header.tsx (1)
136-151:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStatic
aria-labeldoesn't reflect unread notification stateThe button's
aria-labelis always"알림"regardless ofhasUnread. Screen-reader users get no indication that there are unread notifications. Updating it dynamically is a one-liner:♿ Proposed fix
<button onClick={() => setIsNotificationOpen(!isNotificationOpen)} - aria-label="알림" + aria-label={hasUnread ? "알림 (읽지 않은 알림 있음)" : "알림"} className="relative w-6 h-6 cursor-pointer" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/Header.tsx` around lines 136 - 151, The notification button in Header.tsx always uses a static aria-label "알림"; update the button element to compute a dynamic aria-label based on the hasUnread state (and the unread count if available) so screen readers announce when there are unread notifications (e.g., "알림, 새 알림 있음" or "알림, 새 알림 N개"). Keep the existing onClick handler (setIsNotificationOpen) and visual unread dot, but replace the hardcoded aria-label with a conditional string that reflects hasUnread (and count) so assistive tech gets the correct state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/layout/Header.tsx`:
- Around line 46-47: The computed hasUnread currently uses cached notifications
even when the query is disabled, so guard it with the login state: change the
calculation in Header.tsx to only consider notifications when isLoggedIn is true
(e.g. hasUnread = isLoggedIn && (notifications?.some(n => !n.read) ?? false));
update references to hasUnread to rely on this guarded value so the badge hides
immediately on logout; keep useNotificationPreviewQuery as-is but ensure
notifications is only used when isLoggedIn.
---
Outside diff comments:
In `@src/components/layout/Header.tsx`:
- Around line 136-151: The notification button in Header.tsx always uses a
static aria-label "알림"; update the button element to compute a dynamic
aria-label based on the hasUnread state (and the unread count if available) so
screen readers announce when there are unread notifications (e.g., "알림, 새 알림 있음"
or "알림, 새 알림 N개"). Keep the existing onClick handler (setIsNotificationOpen) and
visual unread dot, but replace the hardcoded aria-label with a conditional
string that reflects hasUnread (and count) so assistive tech gets the correct
state.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: be4acb3b-29fb-44eb-89e5-3227ff943411
📒 Files selected for processing (3)
src/components/layout/Header.tsxsrc/components/layout/NotificationDropdown.tsxsrc/hooks/queries/useNotificationQueries.ts
| const { data: notifications } = useNotificationPreviewQuery(5, isLoggedIn); | ||
| const hasUnread = notifications?.some((notif) => !notif.read) ?? false; |
There was a problem hiding this comment.
hasUnread can remain true after logout due to stale TanStack Query cache
Disabling a query only prevents it from running; it may still return cached data. When the user logs out, isLoggedIn becomes false and enabled is set to false, but the cached notification array is still returned as data. This means hasUnread can be true and the red dot can persist until the cache entry is garbage-collected (default 5 minutes).
Guard hasUnread with isLoggedIn to ensure the badge is hidden immediately on logout:
🐛 Proposed fix
- const hasUnread = notifications?.some((notif) => !notif.read) ?? false;
+ const hasUnread = isLoggedIn && (notifications?.some((notif) => !notif.read) ?? false);📝 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 { data: notifications } = useNotificationPreviewQuery(5, isLoggedIn); | |
| const hasUnread = notifications?.some((notif) => !notif.read) ?? false; | |
| const { data: notifications } = useNotificationPreviewQuery(5, isLoggedIn); | |
| const hasUnread = isLoggedIn && (notifications?.some((notif) => !notif.read) ?? false); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/layout/Header.tsx` around lines 46 - 47, The computed
hasUnread currently uses cached notifications even when the query is disabled,
so guard it with the login state: change the calculation in Header.tsx to only
consider notifications when isLoggedIn is true (e.g. hasUnread = isLoggedIn &&
(notifications?.some(n => !n.read) ?? false)); update references to hasUnread to
rely on this guarded value so the badge hides immediately on logout; keep
useNotificationPreviewQuery as-is but ensure notifications is only used when
isLoggedIn.
|
@coderabbitai TanStack Query 캐시에 이전 사용자의 데이터가 남아있을 수 있는 문제들에 대해서 모두 자세하게 알려주세요. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 18608
🔍 발견된 문제 목록1. 🚨 [가장 심각] 로그아웃 시 캐시를 초기화하지 않음
// src/store/useAuthStore.ts
logout: () => {
Cookies.remove("accessToken");
set({ user: null, isLoggedIn: false, isInitialized: true });
// ❌ queryClient.clear() 또는 removeQueries() 호출 없음
},TanStack Query의 기본 해결책: // useAuthStore.ts 수정 예시
import { getQueryClient } from "@/lib/queryClient"; // QueryClient 싱글톤 참조
logout: () => {
Cookies.remove("accessToken");
set({ user: null, isLoggedIn: false, isInitialized: true });
getQueryClient().clear(); // ✅ 모든 캐시 제거
},2. 🚨 사용자 식별자가 없는 쿼리 키현재 모든 알림 관련 쿼리 키에 사용자 ID가 포함되지 않습니다. // src/hooks/queries/useNotificationQueries.ts
export const notificationKeys = {
all: ["notifications"] as const,
settings: () => [...notificationKeys.all, "settings"] as const,
infiniteList: () => [...notificationKeys.all, "infiniteList"] as const,
preview: (size?: number) => [...notificationKeys.all, "preview", size] as const,
// ❌ 예: ["notifications", "preview", 5] → 사용자 구분 없음
};사용자 A가 로그아웃하고 사용자 B가 로그인하더라도 쿼리 키가 동일하기 때문에, TanStack Query는 기존 캐시를 그대로 재사용합니다. 위 Issue 1이 해결되어도, 이 구조가 유지되는 한 근본적으로 안전하지 않습니다. 해결책: 쿼리 키에 export const notificationKeys = {
all: (userId: string) => ["notifications", userId] as const,
preview: (userId: string, size?: number) =>
[...notificationKeys.all(userId), "preview", size] as const,
// ...
};3.
|
| 문제 | 위치 | 수정 방법 |
|---|---|---|
| 로그아웃 시 캐시 미정리 | useAuthStore.ts |
logout()에 queryClient.clear() 추가 |
| 사용자 미식별 쿼리 키 | useNotificationQueries.ts |
키에 userId 포함 |
enabled 가드 누락 |
useNotificationQueries.ts |
useNotificationSettingsQuery, useInfiniteNotificationsQuery에도 enabled 추가 |
가장 빠른 해결은 logout() 함수에서 queryClient.clear()를 호출하는 것입니다. queryClient를 Zustand store 외부에서 접근하려면 싱글톤 패턴으로 QueryClient 인스턴스를 관리하거나, 로그아웃 처리 로직을 컴포넌트 레이어로 올려서 useQueryClient() 훅을 활용하는 방법을 고려해 보세요.
주요 수정 사항
useNotificationPreviewQuery 훅 개선:
비로그인 상태에서 불필요한 알림 조회 요청이 발생하지 않도록 enabled 옵션을 추가했습니다.
Header.tsx 레드닷 로직 추가:
헤더에서 알림 데이터를 가져와 읽지 않은 알림(!notif.read)이 하나라도 존재할 경우 hasUnread 상태를 계산합니다.
hasUnread가 true일 때 알림 아이콘 우측 상단에 bg-Red 색상의 둥근 점을 렌더링하도록 UI를 추가했습니다.
데이터 동기화 및 성능 최적화:
TanStack Query의 캐시를 공유하므로, 알림 드롭다운을 열거나 알림을 읽었을 때(캐시 무효화) 헤더의 빨간 점도 즉각적으로 반응하여 사라지거나 나타납니다.
중복 네트워크 요청 없이 효율적으로 상태를 관리합니다.
Summary by CodeRabbit
New Features
Improvements