[Feature/#306] Setting 페이지 내 클릭수/주간리포트 알림 UI - #322
Conversation
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough채널 알림과 워크스페이스 알림을 별도 타입·상태로 분리하고, 선택된 워크스페이스 기준의 클릭수 알람과 주간 리포트 토글을 설정 화면에 추가했습니다. 변경 감지, 저장, 조직 변경 시 초기화 흐름도 함께 갱신되었습니다. Changes알림 설정 확장
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant Setting
participant WorkspaceQuery
participant NotificationSection
User->>Setting: 설정 페이지 진입
Setting->>WorkspaceQuery: 워크스페이스 목록 조회
WorkspaceQuery-->>Setting: 워크스페이스 목록 반환
Setting->>NotificationSection: 선택 워크스페이스와 알림 상태 전달
User->>NotificationSection: 알림 토글 변경
NotificationSection->>Setting: draft 상태 변경
User->>Setting: 저장 실행
Setting-->>User: 저장 완료 토스트 표시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
📚 Storybook 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/setting/NotificationSection.tsx (1)
12-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winProps 타입 네이밍 컨벤션 확인해주세요.
TNotificationSectionProps는 컴포넌트 props 타입인데T*접두사를 쓰고 있어요. 가이드라인상 컴포넌트 props는I*Props네이밍(예:INotificationSectionProps)을 사용해야 합니다. 코드 자체는 문제없지만, 팀 컨벤션과 어긋나서 나중에 타입 검색/일관성에서 혼란을 줄 수 있어요.As per coding guidelines: "Type naming conventions: API/request-response use I* (e.g., ILoginRequest), unions/options use T*, component props use I*Props."
♻️ 제안
-type TNotificationSectionProps = { +interface INotificationSectionProps { email: string; //channel browserPush: boolean; emailNotif: boolean; onBrowserPushChange: (value: boolean) => void; onEmailNotifChange: (value: boolean) => void; //workspace workspaceName: string | null; // null이면 미선택/없음 clickAlarm: boolean; weeklyReport: boolean; onClickAlarmChange: (value: boolean) => void; onWeeklyReportChange: (value: boolean) => void; workspaceNotifiDisabled: boolean; //selectedOrgId === null 등 -}; +}그리고
}: TNotificationSectionProps)→}: INotificationSectionProps)로 변경해주세요.🤖 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/components/setting/NotificationSection.tsx` around lines 12 - 26, Rename the component props type TNotificationSectionProps to INotificationSectionProps and update the NotificationSection component’s props annotation to use the new name, preserving all fields and behavior unchanged.Source: Coding guidelines
src/pages/setting/Setting.tsx (1)
59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win알림 상태 로직을 커스텀 훅으로 분리하는 것도 고려해보세요.
프로필/비밀번호/채널 알림/워크스페이스 알림까지
Setting컴포넌트 하나가 상태·변경감지·저장·초기화를 모두 담당하면서 점점 커지고 있어요. 채널/워크스페이스 알림 관련 state(59-66), 변경 감지(108-125), org 변경 시 초기화(242-246) 부분을useNotificationSettings같은 커스텀 훅으로 묶으면 페이지 컴포넌트의 책임이 가벼워지고 테스트도 쉬워집니다.As per path instructions: "구조와 책임 분리: 페이지에 비즈니스 로직이 과도하지 않은지 확인. 커스텀 훅으로의 분리 여부 검토."
Also applies to: 108-125, 242-246
🤖 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/setting/Setting.tsx` around lines 59 - 66, Extract the channel and workspace notification state management from Setting into a useNotificationSettings custom hook. Move the related saved/draft state, change detection logic, and organization-change initialization currently handled by Setting, and expose the values and actions the page needs for rendering and saving. Keep profile and password responsibilities in Setting while preserving the existing notification behavior.Source: Path instructions
🤖 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/pages/setting/Setting.tsx`:
- Around line 194-208: Update the toast.success message selection in Setting so
hasWorkspaceNotifChanges is included when determining the successful save
message. Ensure workspace notification-only changes do not fall through to the
account-update message, while preserving the existing account and channel
messaging behavior.
- Around line 79-91: Distinguish workspace loading from the unselected state in
the settings flow: destructure the query’s loading flag from useCoreQuery,
derive workspaceNotifiDisabled from selectedOrgId rather than the temporarily
missing currentWorkspaceName, and pass the loading state to NotificationSection
so it can show loading UI instead of “select a workspace” while data is pending.
---
Nitpick comments:
In `@src/components/setting/NotificationSection.tsx`:
- Around line 12-26: Rename the component props type TNotificationSectionProps
to INotificationSectionProps and update the NotificationSection component’s
props annotation to use the new name, preserving all fields and behavior
unchanged.
In `@src/pages/setting/Setting.tsx`:
- Around line 59-66: Extract the channel and workspace notification state
management from Setting into a useNotificationSettings custom hook. Move the
related saved/draft state, change detection logic, and organization-change
initialization currently handled by Setting, and expose the values and actions
the page needs for rendering and saving. Keep profile and password
responsibilities in Setting while preserving the existing notification behavior.
🪄 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 Plus
Run ID: 653f1b74-dc84-4321-bd72-2c7d0753c66e
📒 Files selected for processing (3)
src/components/setting/NotificationSection.tsxsrc/pages/setting/Setting.tsxsrc/types/setting/notification.ts
2 participants
🚨 관련 이슈
Closed #306
✨ 변경사항
✏️ 작업 내용
selectedOrgId기준으로 워크스페이스명 표시하고, 미선택하였으면, empty/disabled🖥️ 작업 화면
😅 미완성 작업
📢 논의 사항 및 참고 사항
Summary by CodeRabbit