[Refactor/#457] Setting 페이지 커스텀 훅 분리 - #458
Conversation
📚 Storybook 배포 완료
|
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (3)
📝 WalkthroughWalkthrough설정 페이지의 프로필, 비밀번호, 알림, 저장 로직을 전용 훅으로 분리했습니다. 알림 타입과 프로필 타입을 추가하고, Changes설정 상태 계약과 프로필·비밀번호 훅
알림 상태와 웹훅 처리
통합 저장 흐름
설정 페이지 훅 조합
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/setting/Setting.tsx (2)
119-135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win알림 로드 실패 UI에
role="alert"를 추가해주세요.이 블록은 로딩이 끝난 뒤 조건부로 나타납니다. 현재는 일반
<p>라서 스크린 리더가 등장을 알리지 못합니다. 시각 사용자는 즉시 인지하지만, 스크린 리더 사용자는 직접 탐색해야 문제를 발견합니다.
role="alert"를 붙이면 나타나는 순간 읽어줍니다.♿ 접근성 수정안
- <div className="flex min-h-40 flex-col items-center justify-center gap-4 rounded-lg bg-surface-100 p-8"> + <div + role="alert" + className="flex min-h-40 flex-col items-center justify-center gap-4 rounded-lg bg-surface-100 p-8" + > <p className="text-center font-body2 text-text-muted">DOM 구조와 클래스는 그대로 두었습니다.
🤖 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 119 - 135, 알림 로드 실패 UI의 메시지 요소에 role="alert"를 추가하세요. isNotificationError 조건에서 렌더링되는 기존 <p> 요소와 DOM 구조 및 클래스는 그대로 유지하세요.Source: Path instructions
211-221: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win저장 중에도 저장 버튼을 누를 수 있습니다.
Line 220은
isSaving일 때 "저장 중..."을 표시합니다. 그런데 Line 217의disabled조건에는isSaving이 없습니다. 버튼 문구는 진행 중이라고 말하는데 클릭은 계속 받습니다.
useSettingSave.tsLine 40의isSavingRef가드 덕분에 중복 요청은 발생하지 않습니다. 그래서 데이터 문제는 없습니다. 다만 사용자는 버튼이 반응하지 않는 이유를 알 수 없습니다.🐛 저장 중 비활성화
- disabled={!hasChanges || isLoading || isNotificationSectionLoading} + disabled={ + !hasChanges || isSaving || isLoading || isNotificationSectionLoading + }🤖 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 211 - 221, Update the save Button’s disabled condition in Setting.tsx to include isSaving, so it is disabled while saving as well as when there are no changes or loading is active; preserve the existing label and other disabled-state checks.
🧹 Nitpick comments (5)
src/pages/setting/Setting.tsx (2)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueimport 별칭이 단수형입니다. 원본 이름과 맞춰주세요.
useSettingNotifications.ts의 export 이름은useSettingNotifications(복수)입니다. 여기서는useSettingNotification(단수)로 받습니다. default export라 동작은 정상입니다. 다만 다른 세 훅(useSettingProfile,useSettingPassword,useSettingSave)은 원본 이름을 그대로 씁니다. 검색과 추적이 어려워집니다.♻️ 이름 일치
-import useSettingNotification from "`@/hooks/setting/useSettingNotifications`"; +import useSettingNotifications from "`@/hooks/setting/useSettingNotifications`";Line 24도 함께 바꿔주세요.
- const notifications = useSettingNotification(); + const notifications = useSettingNotifications();🤖 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` at line 4, Update the imported hook alias and its usage on line 24 from the singular useSettingNotification to the plural useSettingNotifications, matching the export filename and the naming convention of the other setting hooks.
137-140: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
resetKeys에는 필요한 원시값만 전달하세요.
react-error-boundary@6.1.2는 오류 상태에서resetKeys배열 원소를 얕게 비교합니다. 현재 토글 변경은 draft 객체의 참조를 바꾸므로ErrorBoundary가 리셋됩니다. 이 동작이 의도되지 않았다면 토글 값과 필요한 연결 상태 값을 원시값으로 전달하세요.🤖 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 137 - 140, Update the ErrorBoundary resetKeys in Setting to use only the primitive toggle values and required connection-state values from draftOrgNotif, draftChannel, and draftWorkspaceNotif, rather than the draft objects themselves. Keep reset behavior tied to meaningful value changes and avoid resets caused solely by draft object reference changes.src/hooks/setting/useSettingNotifications.ts (2)
101-199: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff웹훅 핸들러 4개의 구조가 동일합니다. 하나로 묶는 것을 검토해주세요.
handleConnectSlack,handleDisconnectSlack,handleConnectDiscord,handleDisconnectDiscord는 채널 이름과 토스트 문구만 다릅니다. 검증 로직,pendingOrgAction처리, 에러 처리는 같습니다. 지금은 슬랙 쪽 버그를 고치면 디스코드 쪽도 따로 고쳐야 합니다.채널을 파라미터로 받는 함수 2개로 줄일 수 있습니다.
♻️ 채널 파라미터화 예시
type TWebhookChannel = "slack" | "discord"; const CHANNEL_LABEL: Record<TWebhookChannel, string> = { slack: "슬랙", discord: "디스코드", }; const connectWebhook = async (channel: TWebhookChannel) => { const isSlack = channel === "slack"; const url = (isSlack ? slackWebhookUrl : discordWebhookUrl).trim(); const setError = isSlack ? setSlackWebhookError : setDiscordWebhookError; const setUrl = isSlack ? setSlackWebhookUrl : setDiscordWebhookUrl; const label = CHANNEL_LABEL[channel]; if (!url) return setError("Webhook URL을 입력해주세요"); if (!url.startsWith("https://")) return setError("올바른 URL 형식으로 입력해주세요"); setPendingOrgAction(channel); try { await updateOrg.mutateAsync( buildOrgBody( isSlack ? { isSlackEnabled: true, slackWebhookUrl: url, disconnectSlack: false } : { isDiscordEnabled: true, discordWebhookUrl: url, disconnectDiscord: false }, ), ); toast.success(`${label}이(가) 연동되었습니다`); setUrl(""); setError(""); } catch (e) { toast.error((e as IApiErrorResponse).message ?? `${label} 연동에 실패했습니다`); } finally { setPendingOrgAction(null); } };이번 PR의 목표가 "기존 동작 유지"인 점을 감안하면 후속 작업으로 미뤄도 괜찮습니다.
🤖 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/hooks/setting/useSettingNotifications.ts` around lines 101 - 199, Refactor the four webhook handlers in useSettingNotifications around handleConnectSlack, handleDisconnectSlack, handleConnectDiscord, and handleDisconnectDiscord into parameterized connect and disconnect functions accepting the channel. Reuse shared URL validation, pendingOrgAction handling, mutation/error flow, channel-specific state setters, request fields, labels, and existing Korean toast behavior; preserve all current Slack and Discord behavior.
309-359: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff반환 객체가 50개 필드입니다. 소비자별로 묶는 것을 검토해주세요.
현재 이 훅은
setSavedChannel,updateChannels,buildOrgBody같은 내부 구현까지 노출합니다. 이 값들은useSettingSave만 사용합니다.Setting.tsx는 쓰지 않습니다.반환값을
ui와save로 나누거나, 저장 로직 자체를 이 훅 안으로 옮기면 경계가 명확해집니다. 지금 구조에서는useSettingSave가useSettingNotifications의 내부 상태를 직접 조작합니다. 두 훅의 결합도가 높습니다.이번 PR의 분리 작업만으로도 가독성은 충분히 좋아졌습니다. 이 정리는 후속 작업으로 두어도 괜찮습니다.
🤖 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/hooks/setting/useSettingNotifications.ts` around lines 309 - 359, 후속 작업으로 useSettingNotifications의 반환 객체를 소비자 기준으로 재구성하세요. UI가 사용하는 상태와 핸들러는 ui 그룹에, useSettingSave만 사용하는 setSavedChannel, setSavedWorkspaceNotif, setSavedOrgNotif, updateChannels, updateAlerts, updateOrg, updateMaster, buildOrgBody 및 관련 저장 상태는 save 그룹에 배치해 내부 상태 직접 조작을 줄이고 두 훅의 경계를 명확히 하세요.src/hooks/setting/useSettingProfile.ts (1)
68-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win프로필 조회를
useCoreQuery로 전환해주세요.
getMyInfo()는MainLayout과WorkspaceSetting에서도 동일한QUERY_KEYS.auth.myInfo()를 사용합니다. 현재 직접 요청은 공통 캐시와 재시도 정책을 사용하지 않으며, 설정 페이지 진입마다 요청을 새로 보냅니다.
useCoreQuery의data.data를 프로필 초안 상태에 동기화하고, query의isLoading과 오류 상태를 사용해주세요. 조회 실패 시toast.error("회원 정보를 불러오는데 실패했습니다")동작은 유지해주세요.
setPreview는useImageUploader의 React 상태 setter이므로 해당 의존성으로 인해 매 렌더마다 effect가 재실행되지는 않습니다.🤖 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/hooks/setting/useSettingProfile.ts` around lines 68 - 92, useSettingProfile의 직접 getMyInfo 호출을 제거하고, 공통 QUERY_KEYS.auth.myInfo()를 사용하는 useCoreQuery로 전환하세요. query의 data.data가 변경될 때 setSavedProfile, setDraftProfile, setPreview를 동기화하고, query의 isLoading을 기존 로딩 상태에 반영하세요. query 오류 시 기존 toast.error("회원 정보를 불러오는데 실패했습니다") 동작을 유지하며, 안정적인 setPreview 의존성만 사용하세요.Sources: Coding guidelines, 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/hooks/setting/useSettingNotifications.ts`:
- Around line 107-110: Update the webhook validation around the URL check in the
setting notification flow to parse the URL and require a Slack webhook host,
rejecting Discord hosts and unrelated HTTPS addresses with the existing error
handling. Also verify the corresponding server-side webhook validation and
enforce the same allowed-host rule there, keeping client validation as user
feedback rather than the trust boundary.
- Around line 87-99: Update buildOrgBody to construct webhook integration
request fields from savedOrgNotif and savedWorkspaceNotif rather than draft
values, while preserving explicit overrides for the fields being changed by the
integration action. Verify the useSettingSave shouldSaveOrg flow still
explicitly overrides Slack and Discord enablement and does not regress alert
persistence ordering; remove the redundant ?? false fallback for boolean
clickAlarm if appropriate.
- Around line 253-294: Update the effect in useSettingNotifications so
notificationSettings changes always refresh the saved* state, but refresh draft*
only when the current workspace has no unsaved edits, preserving active draft
changes and hasNotificationChanges. Keep the selectedOrgId === null defaults,
and reset the new workspace’s draft only after its notificationSettings data
arrives rather than clearing it prematurely on workspace changes.
In `@src/hooks/setting/useSettingPassWord.ts`:
- Line 8: 대소문자 불일치로 인한 CI import 오류를 해결하세요.
src/hooks/setting/useSettingPassWord.ts의 파일명을 useSettingPassword.ts로 변경하고, Git이
대소문자 변경을 인식하도록 임시 이름을 거치는 git mv를 두 번 수행하세요. src/hooks/setting/useSettingSave.ts
7-7과 src/pages/setting/Setting.tsx 5-5의 import 경로는 변경 후 정상 동작하므로 추가 수정하지 마세요.
In `@src/hooks/setting/useSettingSave.ts`:
- Around line 151-162: Update the catch block in the setting-save flow to
include both the successfully saved steps and error.message in the
partial-failure toast, while preserving the existing fallback when no server
message exists. Also review the partial-failure path around the save handler and
invoke notifications.refetchNotificationSettings() to restore server state,
coordinating with the existing draft-overwrite handling in
useSettingNotifications.ts.
---
Outside diff comments:
In `@src/pages/setting/Setting.tsx`:
- Around line 119-135: 알림 로드 실패 UI의 메시지 요소에 role="alert"를 추가하세요.
isNotificationError 조건에서 렌더링되는 기존 <p> 요소와 DOM 구조 및 클래스는 그대로 유지하세요.
- Around line 211-221: Update the save Button’s disabled condition in
Setting.tsx to include isSaving, so it is disabled while saving as well as when
there are no changes or loading is active; preserve the existing label and other
disabled-state checks.
---
Nitpick comments:
In `@src/hooks/setting/useSettingNotifications.ts`:
- Around line 101-199: Refactor the four webhook handlers in
useSettingNotifications around handleConnectSlack, handleDisconnectSlack,
handleConnectDiscord, and handleDisconnectDiscord into parameterized connect and
disconnect functions accepting the channel. Reuse shared URL validation,
pendingOrgAction handling, mutation/error flow, channel-specific state setters,
request fields, labels, and existing Korean toast behavior; preserve all current
Slack and Discord behavior.
- Around line 309-359: 후속 작업으로 useSettingNotifications의 반환 객체를 소비자 기준으로 재구성하세요.
UI가 사용하는 상태와 핸들러는 ui 그룹에, useSettingSave만 사용하는 setSavedChannel,
setSavedWorkspaceNotif, setSavedOrgNotif, updateChannels, updateAlerts,
updateOrg, updateMaster, buildOrgBody 및 관련 저장 상태는 save 그룹에 배치해 내부 상태 직접 조작을 줄이고
두 훅의 경계를 명확히 하세요.
In `@src/hooks/setting/useSettingProfile.ts`:
- Around line 68-92: useSettingProfile의 직접 getMyInfo 호출을 제거하고, 공통
QUERY_KEYS.auth.myInfo()를 사용하는 useCoreQuery로 전환하세요. query의 data.data가 변경될 때
setSavedProfile, setDraftProfile, setPreview를 동기화하고, query의 isLoading을 기존 로딩 상태에
반영하세요. query 오류 시 기존 toast.error("회원 정보를 불러오는데 실패했습니다") 동작을 유지하며, 안정적인
setPreview 의존성만 사용하세요.
In `@src/pages/setting/Setting.tsx`:
- Line 4: Update the imported hook alias and its usage on line 24 from the
singular useSettingNotification to the plural useSettingNotifications, matching
the export filename and the naming convention of the other setting hooks.
- Around line 137-140: Update the ErrorBoundary resetKeys in Setting to use only
the primitive toggle values and required connection-state values from
draftOrgNotif, draftChannel, and draftWorkspaceNotif, rather than the draft
objects themselves. Keep reset behavior tied to meaningful value changes and
avoid resets caused solely by draft object reference changes.
🪄 Autofix
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: f2a7b69d-55d5-4acc-ad6a-cc03fb2c80e8
📒 Files selected for processing (6)
src/hooks/setting/useSettingNotifications.tssrc/hooks/setting/useSettingPassWord.tssrc/hooks/setting/useSettingProfile.tssrc/hooks/setting/useSettingSave.tssrc/pages/setting/Setting.tsxsrc/types/setting/settingPage.ts
2 participants
🚨 관련 이슈
Closed #457
✨ 변경사항
✏️ 작업 내용
Setting.tsx페이지에 몰려있던 프로필/비밀번호/알림/저장 로직을 커스텀 훅으로 분리하여 가독성을 높였습니다.types/setting/settingPage.ts로 분리하였습니다.Setting.tsx페이지에서는 훅 연결과 섹션 조립만 진행합니다.😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
기존 동작 변경은 없습니다
Summary by CodeRabbit
새로운 기능
버그 수정