Skip to content

[feat] 알림 페이지 구현#41

Merged
limtjdghks merged 25 commits into
devfrom
feat/ALT-232
Jun 5, 2026
Merged

[feat] 알림 페이지 구현#41
limtjdghks merged 25 commits into
devfrom
feat/ALT-232

Conversation

@limtjdghks

@limtjdghks limtjdghks commented May 20, 2026

Copy link
Copy Markdown
Member

ID

  • ALT-232

변경 내용

  • 알림 페이지, 설정 페이지 UI 구현
  • 알림 조회 / 알림 수신 설정 API 연동

구현 사항

알림 아이템 컴포넌트 (NotificationItem)

  • 읽음/미읽음 상태에 따른 배경색 구분
  • 알림 메시지 내 특정 단어 하이라이트 지원
  • 좌측 스와이프로 삭제 — 임계값(30px) 초과 시 자동 삭제, 별도 버튼 클릭 불필요

알림 페이지 (/notifications)

  • 대타 / 평판 탭 전환, 미읽음 알림 탭에 빨간 점 표시
  • 스티키 헤더 (스크롤 시 탭 고정)
  • 스크롤 기반 무한 스크롤 (IntersectionObserver)
  • 로딩 스피너 / 에러 / 빈 상태 처리
  • 헤더 우측 설정 아이콘 → 알림 설정 페이지 이동
  • Navbar 벨 아이콘 → 알림 페이지 이동 연결

알림 설정 페이지 (/notifications/settings)

  • 전체 알림 켜기 (GENERAL) — API 연동
  • 대타 / 평판 알림 켜기 — 로컬 상태, 전체 알림 off 시 비활성화
  • Toggle 공용 컴포넌트 추가

API 연동

  • 알림 목록 조회: 알바생 GET /app/users/me/notifications / 사장님 GET /manager/notifications/me
    • scope 기반 자동 엔드포인트 선택
    • 커서 기반 페이지네이션 (useInfiniteQuery)
  • 알림 수신 설정 조회: GET /app/users/me/notification-consent / GET /manager/me/notification-consent
    • staleTime 1시간으로 불필요한 재요청 방지
  • 알림 수신 설정 변경: PUT /app/users/me/notification-consent / PUT /manager/me/notification-consent
  • query / mutation 훅 분리 (useNotificationConsent, useUpdateNotificationConsent)
  • queryKeys.notification.list / queryKeys.notification.consent 추가

구현 시연 (필요 시)

2026-05-20.10.08.43.mov

참고 사항 (필요 시)

필요한 API

  • 알림 삭제
  • 평판 알림 API
  • 알림 수신 설정
    • 현재 전체 알림, 야간 알림만 있음
    • UI상으로는 대타 알림과 평판 알림 on/off 기능이 추가되어야함

디자인 수정되어야하는거

  • 대타, 평판 알림만 있음
  • 그 이외의 알림은 어떻게 확인해야하나요

Summary by CodeRabbit

새로운 기능

  • 알림 페이지 추가 - 타입별 필터링 및 무한 스크롤로 알림 목록 조회 가능
  • 알림 설정 페이지 추가 - 일반/야간/대타/평판 알림 동의 관리
  • 알림 배지 표시 - 상단 내비게이션에 미읽음 알림 여부 표시
  • 전체 읽음 기능 - 현재 페이지 알림을 한 번에 읽음 처리 가능

@limtjdghks limtjdghks requested review from dohy-eon and kim3360 May 20, 2026 12:26
@limtjdghks limtjdghks self-assigned this May 20, 2026
@vercel

vercel Bot commented May 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
alter-client Ready Ready Preview, Comment Jun 5, 2026 4:50am

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

알림 기능 전체를 추가했습니다. 타입·API 정의부터 React Query 훅, 뷰모델, 페이지 구현, UI 컴포넌트, Storybook 스토리, 그리고 라우팅 통합까지 아우릅니다. 사용자/매니저 scope 분기, 커서 기반 무한 페이징, swipe-to-delete 상호작용, 동의 설정 조건부 갱신 로직이 포함됩니다.

Changes

알림 기능 전체 추가

Layer / File(s) Summary
알림 타입 정의
src/features/notification/types/notificationType.ts, src/features/notification/types/consent.ts, src/features/notification/types/index.ts
NOTIFICATION_TYPE, CONSENT_TYPE 상수와 NotificationDto, NotificationPage, NotificationListResponse, NotificationConsentResponse 등 응답/요청 스키마 정의
알림 API 래퍼
src/features/notification/api/notifications.ts, src/features/notification/api/notificationConsent.ts
사용자/매니저 알림 목록·읽음·미읽음 카운트 조회, 동의 조회/수정 API 함수 (axiosInstance 기반, cursor 페이징 지원)
쿼리키 및 React Query 훅
src/shared/lib/queryKeys.ts, src/features/notification/hooks/useNotifications.ts, src/features/notification/hooks/useNotificationConsent.ts, src/features/notification/hooks/useUpdateNotificationConsent.ts, src/features/notification/hooks/useMarkNotificationRead.ts, src/features/notification/hooks/useNotificationUnreadCount.ts
알림 목록 무한쿼리, 동의 조회·수정 뮤테이션, 읽음 처리 뮤테이션, 미읽음 카운트 쿼리 (scope 기반 분기, 캐시 무효화)
뷰모델 계층
src/features/notification/useNotificationViewModel.ts, src/features/notification/useNotificationSettingsViewModel.ts
알림 목록 상태: 타입 필터, 삭제/읽음 ID 세트, 파생 항목 / 설정 상태: 동의 조회, enabled 파생, 조건부 갱신 (SUBSTITUTE/REPUTATION 함께 비활성화 시 GENERAL도 off)
UI 컴포넌트
src/shared/ui/common/Toggle.tsx, src/shared/ui/notification/NotificationItem.tsx, src/pages/notification/settings/components/NotificationToggleRow.tsx
Toggle: aria-switch 접근성 / NotificationItem: swipe-to-delete, 하이라이트 메시지, read 상태 스타일 / NotificationToggleRow: 라벨 변형, description, disabled 바인딩
알림 목록 페이지
src/pages/notification/index.tsx
드롭다운 타입 필터, IntersectionObserver 무한 스크롤, 전체 읽음 버튼, 로딩/에러/빈 상태 분기
알림 설정 페이지
src/pages/notification/settings/index.tsx
전체/대타/평판/야간 알림 토글, 로딩 상태 처리, disabled 조건부 적용 (allEnabled에 따라)
Storybook 및 설정
storybook/stories/NotificationItem.stories.tsx, tailwind.config.js
NotificationItem 스토리 4가지 (읽음/미읽음/삭제/긴 메시지), Tailwind storybook 경로 추가
라우팅 및 Navbar 통합
src/app/App.tsx, src/shared/constants/routes.ts, src/shared/ui/common/Navbar.tsx
모바일 라우트에 /notifications, /notifications/settings 추가 / ROUTES 상수 확장 / Navbar: 알림 버튼 클릭 시 네비게이션, useNotificationUnreadCount로 미읽음 배지 표시

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes


Key Issues & Observations

🚨 로직 위험: useNotificationSettingsViewModel의 조건부 갱신 로직

이슈: handleSubstituteChange, handleReputationChange에서 "둘 다 비활성 → GENERAL도 false로 갱신"하는 조건문이 있지만, 이 로직이 UI에서 실제로 원하는 동작인지 명확하지 않습니다.

// 예: substituteEnabled를 끔 + reputationEnabled도 이미 false
// → GENERAL을 추가로 false로 갱신
if (!reputationEnabled && !substituteEnabled) {
  handleAllChange(false);  // 이게 의도된 동작?
}

제안:

  • 이 "cascading disable" 동작이 기획 의도인지 확인하세요.
  • 만약 의도라면, UI에서 토글 클릭 시 이 side-effect를 사용자에게 명확히 알리는 feedback이 필요합니다.
  • 의도가 아니라면 조건부 로직을 제거하세요.

⚠️ API 에러 처리 누락

이슈: notifications.ts, notificationConsent.ts의 모든 API 함수가 try-catch 없이 원시 axios 호출을 노출합니다. 네트워크 에러 시 unhandled rejection이 발생할 수 있습니다.

// notifications.ts
export const fetchUserNotifications = async (params = {}) => {
  const response = await axiosInstance.get('/app/users/me/notifications', { params: buildParams(params) });
  return response.data; // ← 에러 처리 없음
};

제안:

  • 훅(useNotifications)의 useInfiniteQuery가 자동으로 캐치하긴 하지만, API 함수 레벨에서도 에러를 정의하고 로깅하세요.
  • 또는 Navbar에서 useNotificationUnreadCount 호출이 에러 시 silent fail하므로, enabled: false 외에 retry: 0 또는 staleTime: Infinity 같은 보조 옵션을 고려하세요.

🔍 NotificationItem의 swipe 상호작용: 터치/마우스 일관성

이슈: NotificationItem.tsx에서 touch와 mouse 이벤트를 모두 처리하지만, 일부 엣지 케이스에서 상태 불일치가 가능합니다.

// mousemove를 document에 attach → 컴포넌트 언마운트 중 listener 정리 실패 가능
onMouseDown={() => {
  document.addEventListener('mousemove', handleMouseMove);
  document.addEventListener('mouseup', handleMouseUp);
}};

제안:

  • cleanup useEffect에서 반드시 이벤트 리스너를 제거하도록 구성했으나, 드래그 중 언마운트 시 누수가 발생할 수 있으니 useRef로 listener 핸들을 캐싱하고 확실히 제거하세요.
  • 또는 swipe 라이브러리(예: react-use-gesture)를 도입하는 것도 방법입니다.

좋은 점

  • scope 기반 분기: 사용자/매니저 API를 깔끔하게 분리했습니다.
  • 쿼리키 네임스페이싱: queryKeys.notification.list(scope, type) 형태로 명확합니다.
  • 무한 페이징: cursor 기반 pagination이 제대로 구현되었습니다.
  • UI 접근성: Toggle에 aria-checked, aria-label 적용되었습니다.

Suggested reviewers

  • kim3360
  • dohy-eon
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 제목 '[feat] 알림 페이지 구현'은 이번 PR의 주요 변경사항(알림 페이지 및 설정 페이지 UI 구현)을 명확하고 간결하게 요약하고 있습니다.
Description check ✅ Passed PR 설명이 ID(ALT-232), 변경 내용, 상세한 구현 사항을 포함하고 있으며, 데모 영상과 참고 사항도 포함되어 있어 템플릿 요구사항을 충족합니다.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ALT-232

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (2)
src/app/App.tsx (1)

29-30: ⚡ Quick win

신규 알림 페이지 라우트도 lazy/Suspense로 분리해주세요.

NotificationPage, NotificationSettingsPage가 동기 import라 초기 번들에 포함됩니다. 기존 SignupPage처럼 lazy route로 맞추는 게 일관성과 초기 로딩에 유리합니다.

As per coding guidelines "src/app/**: 라우팅 구조가 lazy loading을 활용하는지 확인" 및 "src/pages/**: React.lazy / Suspense를 통한 코드 스플리팅 적용 여부".

Also applies to: 103-107

🤖 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/app/App.tsx` around lines 29 - 30, NotificationPage and
NotificationSettingsPage are currently imported synchronously and should be
converted to lazy-loaded routes to avoid including them in the initial bundle;
replace their direct imports with React.lazy wrappers (e.g., const
NotificationPage = React.lazy(() => import('...NotificationPage')) and const
NotificationSettingsPage = React.lazy(() =>
import('...NotificationSettingsPage'))), then ensure the routes that render
NotificationPage and NotificationSettingsPage are wrapped with a Suspense
fallback (same pattern used for SignupPage) so code-splitting is applied
consistently.
src/pages/notification/index.tsx (1)

60-75: 🏗️ Heavy lift

페이지에서 무한스크롤 관찰 로직을 분리해주세요.

IntersectionObserver 생성/해제 로직이 페이지에 들어와 있어 페이지 조합 책임을 넘습니다. useNotificationViewModel 또는 별도 hook으로 옮겨 페이지는 렌더링 조합만 담당하게 유지하는 편이 좋습니다.

As per coding guidelines "src/pages/**: 페이지 컴포넌트가 비즈니스 로직 없이 조합(Composition)만 하는지".

🤖 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/pages/notification/index.tsx` around lines 60 - 75, Move the
IntersectionObserver creation/teardown out of the page component and into the
notification view model or a new custom hook (e.g. useNotificationViewModel or
useInfiniteScrollObserver) so the page only composes UI; specifically, take the
useEffect block that references sentinelRef, creates new IntersectionObserver,
observes el, and disconnects on cleanup, and implement it inside the view
model/hook so it accepts sentinelRef (or a ref setter) and the control flags
hasNextPage, isFetchingNextPage and action fetchNextPage; ensure the observer
callback uses those flags and that the hook returns any necessary wiring for the
page (e.g. sentinelRef or ref callback) and that cleanup is performed on
unmount.
🤖 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/features/notification/hooks/useNotifications.ts`:
- Line 19: NotificationPage's cursor is currently typed as string but must allow
null per API behavior; open the NotificationPage type in the notification types
(NotificationPage) and change the cursor property to string | null (matching
other page DTOs like ChatRoomPageDto/ApplicationPageDto/PostingPageDto), then
run a quick typecheck and adjust any callsites that assume a non-null cursor
(e.g., pagination helpers such as getNextPageParam) to handle null/undefined
accordingly.

In `@src/features/notification/hooks/useUpdateNotificationConsent.ts`:
- Around line 8-14: useUpdateNotificationConsent currently chooses updater based
on scope but lets scope=null implicitly fall back to
updateUserNotificationConsent; change the mutation to explicitly handle null by
preventing execution when scope is null: inside useUpdateNotificationConsent set
the useMutation's mutationFn to undefined (or an early-returning function) when
scope === null, or add a guard that throws/returns early so callers cannot call
mutate with scope null; update references to updater, useMutation,
updateManagerNotificationConsent and updateUserNotificationConsent accordingly
and ensure callers either never call mutate when scope is null or check
mutation.isIdle before invoking.

In `@src/features/notification/types/consent.ts`:
- Around line 18-21: Update the UpdateNotificationConsentRequest interface so
the type field is constrained to the CONSENT_TYPE union instead of plain string:
replace type: string with type: CONSENT_TYPE (or the appropriate union/enum
exported as CONSENT_TYPE) and ensure you import or reference CONSENT_TYPE in
this module; update any usages that construct UpdateNotificationConsentRequest
to use a valid CONSENT_TYPE value.

In `@src/features/notification/useNotificationSettingsViewModel.ts`:
- Around line 23-24: The substituteEnabled and reputationEnabled toggles in
useNotificationSettingsViewModel are only local state and reset to true on
refresh; persist them to avoid user confusion by reading initial values from
localStorage and writing updates back whenever setSubstituteEnabled or
setReputationEnabled change (use useEffect to sync), using distinct localStorage
keys (e.g., "notification.substituteEnabled" and
"notification.reputationEnabled") and fall back to true if absent; if you prefer
not to persist because the API is missing, update the UI behavior in the same
hook to expose a "coming soon"/"not saved" flag instead of implying the toggles
are saved.

In `@src/features/notification/useNotificationViewModel.ts`:
- Around line 56-63: The currentItems value is not being filtered by activeTab
so tabs don't change displayed items; update the logic that computes
currentItems (and the derived hasUnreadSubstitute and hasUnreadReputation) to
filter the full notifications list by activeTab (use the item's title or
category to distinguish '대타' vs '평판'), return only the items matching the
activeTab for currentItems, and compute hasUnreadSubstitute/hasUnreadReputation
from the filtered lists (keep using setActiveTab as-is). Ensure you reference
and update the existing symbols currentItems, activeTab, hasUnreadSubstitute,
hasUnreadReputation when adding the filter logic.

In `@src/pages/notification/index.tsx`:
- Around line 130-131: 현재 currentItems.map(...)에서 <li key={item.id ?? idx}>처럼
index를 fallback으로 사용하고 있어 삭제/페이지네이션 시 DOM 재사용 문제가 발생할 수 있습니다; 이 문제를 해결하려면 렌더링할 때
반드시 안정적인 고유 key만 사용하도록 item.id를 필수화하거나 렌더 전에 id가 없는 항목을 필터링하세요 (예: 처리 로직에서
currentItems를 생성하는 곳 또는 컴포넌트 내부에서 map 호출 전에 id가 없는 항목을 제거하거나 데이터 유효성 검사 추가).
currentItems, item.id, 그리고 해당 map 渲染 블록을 찾아 수정하세요.

In `@src/shared/ui/notification/NotificationItem.tsx`:
- Around line 84-133: The onClick can fire after a swipe because touchend is
followed by a click; track a didDrag flag and ignore clicks when a drag
happened. Modify startDrag/moveDrag/endDrag (and their callers
handleTouchStart/handleTouchMove/handleTouchEnd and handleMouseDown's
onMove/onUp) to set didDrag = false at start, set didDrag = true when moveDrag
detects sufficient movement, and reset didDrag to false after endDrag completes;
then in the button onClick handler check didDrag and return early if true
(instead of only checking offset). Use the existing functions startDrag,
moveDrag, endDrag and state setters like setOffset to locate where to add and
clear the didDrag flag.
- Around line 90-102: handleMouseDown currently attaches document 'mousemove'
and 'mouseup' handlers that are only removed in the 'mouseup' path, which leaks
listeners if the component unmounts mid-drag; fix by storing the onMove and onUp
listener references in refs (e.g. moveListenerRef, upListenerRef) when you
create them in handleMouseDown, and add a useEffect cleanup that checks those
refs and calls document.removeEventListener('mousemove',
moveListenerRef.current) and document.removeEventListener('mouseup',
upListenerRef.current) and clears the refs; ensure you still call endDrag() in
cleanup and keep using startDrag, moveDrag and endDrag as before so the drag
state is correctly finalized.

---

Nitpick comments:
In `@src/app/App.tsx`:
- Around line 29-30: NotificationPage and NotificationSettingsPage are currently
imported synchronously and should be converted to lazy-loaded routes to avoid
including them in the initial bundle; replace their direct imports with
React.lazy wrappers (e.g., const NotificationPage = React.lazy(() =>
import('...NotificationPage')) and const NotificationSettingsPage =
React.lazy(() => import('...NotificationSettingsPage'))), then ensure the routes
that render NotificationPage and NotificationSettingsPage are wrapped with a
Suspense fallback (same pattern used for SignupPage) so code-splitting is
applied consistently.

In `@src/pages/notification/index.tsx`:
- Around line 60-75: Move the IntersectionObserver creation/teardown out of the
page component and into the notification view model or a new custom hook (e.g.
useNotificationViewModel or useInfiniteScrollObserver) so the page only composes
UI; specifically, take the useEffect block that references sentinelRef, creates
new IntersectionObserver, observes el, and disconnects on cleanup, and implement
it inside the view model/hook so it accepts sentinelRef (or a ref setter) and
the control flags hasNextPage, isFetchingNextPage and action fetchNextPage;
ensure the observer callback uses those flags and that the hook returns any
necessary wiring for the page (e.g. sentinelRef or ref callback) and that
cleanup is performed on unmount.
🪄 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

Run ID: 52985ef4-a7d0-44e7-ae4c-1ede75e2abf3

📥 Commits

Reviewing files that changed from the base of the PR and between 15f63bc and 441436b.

⛔ Files ignored due to path filters (2)
  • src/assets/alter-logo-vector.svg is excluded by !**/*.svg
  • src/assets/icons/settings.svg is excluded by !**/*.svg
📒 Files selected for processing (19)
  • src/app/App.tsx
  • src/features/notification/api/notificationConsent.ts
  • src/features/notification/api/notifications.ts
  • src/features/notification/hooks/useNotificationConsent.ts
  • src/features/notification/hooks/useNotifications.ts
  • src/features/notification/hooks/useUpdateNotificationConsent.ts
  • src/features/notification/types/consent.ts
  • src/features/notification/types/index.ts
  • src/features/notification/useNotificationSettingsViewModel.ts
  • src/features/notification/useNotificationViewModel.ts
  • src/pages/notification/index.tsx
  • src/pages/notification/settings/index.tsx
  • src/shared/constants/routes.ts
  • src/shared/lib/queryKeys.ts
  • src/shared/ui/common/Navbar.tsx
  • src/shared/ui/common/Toggle.tsx
  • src/shared/ui/notification/NotificationItem.tsx
  • storybook/stories/NotificationItem.stories.tsx
  • tailwind.config.js

Comment thread src/features/notification/hooks/useNotifications.ts
Comment thread src/features/notification/hooks/useUpdateNotificationConsent.ts Outdated
Comment thread src/features/notification/types/consent.ts
Comment thread src/features/notification/useNotificationSettingsViewModel.ts Outdated
Comment thread src/features/notification/useNotificationViewModel.ts Outdated
Comment thread src/pages/notification/index.tsx
Comment thread src/shared/ui/notification/NotificationItem.tsx
Comment thread src/shared/ui/notification/NotificationItem.tsx
Comment thread src/shared/ui/notification/NotificationItem.tsx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/features/notification/useNotificationSettingsViewModel.ts (1)

24-25: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

대타/평판 토글 상태가 새로고침 시 유실됩니다.

substituteEnabled, reputationEnableduseState(true)로 고정 초기화되어 사용자 변경값이 유지되지 않습니다. API 미지원 상태라면 localStorage 동기화 또는 “저장되지 않음/준비 중” 상태를 명시해 오해를 막아주세요.

수정 예시
- const [substituteEnabled, setSubstituteEnabled] = useState(true)
- const [reputationEnabled, setReputationEnabled] = useState(true)
+ const [substituteEnabled, setSubstituteEnabled] = useState(
+   () => localStorage.getItem('notification.substituteEnabled') !== 'false'
+ )
+ const [reputationEnabled, setReputationEnabled] = useState(
+   () => localStorage.getItem('notification.reputationEnabled') !== 'false'
+ )
+
+ const handleSubstituteEnabledChange = (checked: boolean) => {
+   setSubstituteEnabled(checked)
+   localStorage.setItem('notification.substituteEnabled', String(checked))
+ }
+
+ const handleReputationEnabledChange = (checked: boolean) => {
+   setReputationEnabled(checked)
+   localStorage.setItem('notification.reputationEnabled', String(checked))
+ }
🤖 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/features/notification/useNotificationSettingsViewModel.ts` around lines
24 - 25, substituteEnabled and reputationEnabled are hard-coded to true and lose
user changes on refresh; initialize them from localStorage (e.g. keys like
"notification.substituteEnabled" and "notification.reputationEnabled") instead
of useState(true), and add an effect to persist updates via
setSubstituteEnabled/setReputationEnabled to localStorage whenever they change;
if the API is not available, surface an explicit "unsaved/preview" state (e.g. a
boolean flag) so users know changes are local-only instead of silently
reverting.
🤖 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.

Duplicate comments:
In `@src/features/notification/useNotificationSettingsViewModel.ts`:
- Around line 24-25: substituteEnabled and reputationEnabled are hard-coded to
true and lose user changes on refresh; initialize them from localStorage (e.g.
keys like "notification.substituteEnabled" and "notification.reputationEnabled")
instead of useState(true), and add an effect to persist updates via
setSubstituteEnabled/setReputationEnabled to localStorage whenever they change;
if the API is not available, surface an explicit "unsaved/preview" state (e.g. a
boolean flag) so users know changes are local-only instead of silently
reverting.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 22e59437-784c-435f-b24c-6eb2f1d9fba1

📥 Commits

Reviewing files that changed from the base of the PR and between 441436b and b654be8.

📒 Files selected for processing (6)
  • src/features/notification/hooks/useUpdateNotificationConsent.ts
  • src/features/notification/types/consent.ts
  • src/features/notification/types/index.ts
  • src/features/notification/useNotificationSettingsViewModel.ts
  • src/pages/notification/settings/index.tsx
  • src/shared/ui/notification/NotificationItem.tsx

@dohy-eon dohy-eon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

탭이랑 필터링 로직 추가 + 코멘트 읽어보시구 처리해주심 감사드리겠습니다.

Comment thread src/features/notification/useNotificationViewModel.ts Outdated
Comment thread src/shared/ui/common/Navbar.tsx
@limtjdghks

Copy link
Copy Markdown
Member Author
image
image

@dohy-eon 다 햇으요
전체 읽음도 됨

@limtjdghks limtjdghks requested a review from dohy-eon June 3, 2026 10:58

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/shared/ui/common/Navbar.tsx`:
- Around line 2-3: Navbar currently imports and calls the feature hook
useNotificationUnreadCount, breaking layer boundaries; change Navbar (component
name Navbar) to accept unread props from its parent (e.g. unreadCount and
optionally isUnreadLoading) and remove the import/use of
useNotificationUnreadCount from the file, then update any internal references to
unread state (render logic around the badge/count at lines referenced in the
review) to use the new props; update callers (pages/widgets/app or parent
container) to call useNotificationUnreadCount and pass the resulting unread
values into Navbar as props.
🪄 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

Run ID: 0c79ff65-2134-4833-8cfd-4556937420a3

📥 Commits

Reviewing files that changed from the base of the PR and between 74ae6d0 and da2e5cd.

📒 Files selected for processing (8)
  • src/features/notification/api/notifications.ts
  • src/features/notification/hooks/useMarkNotificationRead.ts
  • src/features/notification/hooks/useNotificationUnreadCount.ts
  • src/features/notification/types/index.ts
  • src/features/notification/useNotificationViewModel.ts
  • src/pages/notification/index.tsx
  • src/shared/lib/queryKeys.ts
  • src/shared/ui/common/Navbar.tsx
✅ Files skipped from review due to trivial changes (1)
  • src/features/notification/hooks/useNotificationUnreadCount.ts

Comment on lines +2 to +3
import { useAuthStore } from '@/shared/stores/useAuthStore'
import { useNotificationUnreadCount } from '@/features/notification/hooks/useNotificationUnreadCount'

@coderabbitai coderabbitai Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

shared 컴포넌트가 features 훅에 직접 의존해 레이어 경계를 깨고 있습니다.

Navbar는 공용 UI인데 unread 조회 로직을 내장하면서 feature에 결합됐습니다. unread 상태/카운트는 상위 레이어(pages/widgets/app)에서 조회 후 props로 주입하고, Navbar는 표시만 담당하도록 분리해주세요.

♻️ 최소 분리 방향 예시
-import { useNotificationUnreadCount } from '`@/features/notification/hooks/useNotificationUnreadCount`'
@@
 interface NavbarProps {
@@
+  hasUnreadNotification?: boolean
 }
@@
 export function Navbar({
@@
+  hasUnreadNotification = false,
 }: NavbarProps) {
@@
-  const scope = useAuthStore(s => s.scope)
-  const { data: unreadData } = useNotificationUnreadCount(isMain ? scope : null)
@@
-                {unreadData?.hasUnread && (
+                {hasUnreadNotification && (
                   <span className="absolute right-0 top-0 h-2 w-2 rounded-full bg-error" />
                 )}

As per coding guidelines src/shared/**: "다른 레이어를 import하지 않는지 (완전한 독립성)".

Also applies to: 35-37, 90-92

🤖 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/shared/ui/common/Navbar.tsx` around lines 2 - 3, Navbar currently imports
and calls the feature hook useNotificationUnreadCount, breaking layer
boundaries; change Navbar (component name Navbar) to accept unread props from
its parent (e.g. unreadCount and optionally isUnreadLoading) and remove the
import/use of useNotificationUnreadCount from the file, then update any internal
references to unread state (render logic around the badge/count at lines
referenced in the review) to use the new props; update callers
(pages/widgets/app or parent container) to call useNotificationUnreadCount and
pass the resulting unread values into Navbar as props.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아쉽게 됐다 그건 어려울거같은데

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@limtjdghks 이해합니다! 현재 구조가 기능적으로는 문제없이 동작하니까요. 다만 나중에 Navbar를 재사용하거나 테스트할 때 features 의존성이 걸림돌이 될 수 있어서, 추후 여유가 생길 때 리팩터링 이슈로 트래킹해두시는 걸 추천드립니다. 혹시 GitHub 이슈로 남겨드릴까요?

(´・ω・`) 🐇


🧠 Learnings used
Learnt from: dohy-eon
Repo: alter-app/alter-client PR: 7
File: src/pages/manager/home/index.tsx:96-101
Timestamp: 2026-03-11T19:27:41.867Z
Learning: In the alter-client repository, avoid hardcoding colors in TSX files. Use the Tailwind tokens already registered in tailwind.config.js: use text-sub and border-sub for the color '`#3a9982`' (sub.DEFAULT), and use bg-bg-dark for '`#efefef`'. Replace literals like text-[`#3A9982`] and bg-[`#EFEFEF`] with the appropriate tokens in src/pages/manager/home/index.tsx and src/shared/ui/manager/**/*.tsx.

@dohy-eon dohy-eon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

알림 설정 뭔가뭔가임.. 나머지 알림 다 꺼지면 전체 알림도 꺼지게 바껴야 할 듯

방해금지 시간 부분은 아예 빠지면 될거 같고

image

해당부분 disabled로 회색 값 통일하는게 좋아보입니다.

그 외 전체 읽음 / 읽지않음 뱃지까지 확인 완료했고 이견 없습니다~

추가로 궁금하신점 있으시면 알려주시고 위에 두 내용 수정 이후에 머지해주세요!

@limtjdghks

Copy link
Copy Markdown
Member Author

@dohy-eon 와 레전드버그!
근데 방해금지 시간은 명시하는게 낫지 않을까요 사용자가 방해금지시간이 언제인지를 알아야 끌지 말지를 정할 것 같은데
글씨크기를 줄이고 색을 흐리게하면 괜찮을 것 같은데 어떻게 생각하시나여

@dohy-eon

dohy-eon commented Jun 4, 2026

Copy link
Copy Markdown
Member

@dohy-eon 와 레전드버그!

근데 방해금지 시간은 명시하는게 낫지 않을까요 사용자가 방해금지시간이 언제인지를 알아야 끌지 말지를 정할 것 같은데

글씨크기를 줄이고 색을 흐리게하면 괜찮을 것 같은데 어떻게 생각하시나여

좋아유 작업 완료되면 스샷 같이 첨부 부탁드림다

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/features/notification/useNotificationSettingsViewModel.ts`:
- Around line 33-45: handleSubstituteChange and handleReputationChange currently
read stale render-time flags (reputationEnabled/substituteEnabled) to decide
whether to set CONSENT_TYPE.GENERAL, which can miss updates if toggles happen
quickly; change the flow to base the GENERAL decision on the latest
server/returned state after the mutateAsync call (e.g., use the value returned
by mutateAsync or refetch the consent state) instead of
reputationEnabled/substituteEnabled so that after calling mutateAsync for
CONSENT_TYPE.SUBSTITUTE or CONSENT_TYPE.REPUTATION you then check the fresh
consent values and only call mutateAsync({ type: CONSENT_TYPE.GENERAL, consent:
false }) when both current substitute and reputation are false; keep the
existing function names handleSubstituteChange, handleReputationChange and
COMMIT using mutateAsync and CONSENT_TYPE constants.

In `@src/pages/notification/settings/index.tsx`:
- Line 4: The page is importing the internal symbol
useNotificationSettingsViewModel directly from the feature implementation;
change the page import to consume the feature's public API by importing
useNotificationSettingsViewModel from the feature root (the feature's index.ts
re-export). If the feature's index.ts does not yet export
useNotificationSettingsViewModel, add a re-export there (export {
useNotificationSettingsViewModel } from './useNotificationSettingsViewModel')
and then update the page import to import { useNotificationSettingsViewModel }
from '`@/features/notification`'.
🪄 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

Run ID: 1db3ffd3-16fe-468d-9e8d-4b3b4452a25b

📥 Commits

Reviewing files that changed from the base of the PR and between da2e5cd and 7368854.

📒 Files selected for processing (8)
  • src/app/App.tsx
  • src/features/notification/types/consent.ts
  • src/features/notification/useNotificationSettingsViewModel.ts
  • src/pages/notification/settings/components/NotificationToggleRow.tsx
  • src/pages/notification/settings/index.tsx
  • src/shared/constants/routes.ts
  • src/shared/lib/queryKeys.ts
  • src/shared/ui/common/Navbar.tsx
✅ Files skipped from review due to trivial changes (2)
  • src/features/notification/types/consent.ts
  • src/shared/constants/routes.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/app/App.tsx
  • src/shared/ui/common/Navbar.tsx
  • src/shared/lib/queryKeys.ts

Comment thread src/features/notification/useNotificationSettingsViewModel.ts
Comment thread src/pages/notification/settings/index.tsx Outdated
@limtjdghks

Copy link
Copy Markdown
Member Author

@dohy-eon 작업 완료했슴니다

2026-06-05.1.57.45.mov

@limtjdghks limtjdghks requested a review from dohy-eon June 5, 2026 04:58

@dohy-eon dohy-eon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@limtjdghks limtjdghks merged commit 10b7d66 into dev Jun 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants