[design] 사장님 대타 요청 관리 UI 구현#40
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthrough매니저를 위한 대체 요청 승인/거절 기능을 구현했습니다. 공유 상태 타입과 API 함수, DTO 매핑, 상태 관리 훅, 모달 및 페이지 컴포넌트, 라우팅을 추가하고 기존 카드 컴포넌트를 확장했습니다. ChangesManager Substitute Request Approval Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
래빗이가 먼저해라잉 |
|
래비시가 안해서 내가 햇다잉 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/app/App.tsx (1)
26-27: ⚡ Quick win신규 매니저 페이지 라우트도 lazy 로딩으로 분리하는 편이 좋습니다.
현재는 정적 import라 초기 번들에 바로 포함됩니다. 기존
SignupPage패턴처럼 lazy + Suspense를 적용하면 초기 로딩 비용을 줄일 수 있습니다.⚡ 제안 수정안
-import { ManagerSubstituteRequestPage } from '`@/pages/manager/substitute-request`' +const ManagerSubstituteRequestPage = lazy(async () => { + const m = await import('`@/pages/manager/substitute-request`') + return { default: m.ManagerSubstituteRequestPage } +})<Route path={ROUTES.MANAGER.SUBSTITUTE_REQUEST} element={ - <HomeRouteGuard expected="MANAGER"> - <ManagerSubstituteRequestPage /> - </HomeRouteGuard> + <Suspense fallback={null}> + <HomeRouteGuard expected="MANAGER"> + <ManagerSubstituteRequestPage /> + </HomeRouteGuard> + </Suspense> } />As per coding guidelines
src/app/**: "라우팅 구조가 lazy loading을 활용하는지 확인".Also applies to: 149-156
🤖 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 26 - 27, The two statically imported manager route components ManagerSubstituteRequestPage and StoreRegisterPage should be converted to lazy-loaded components (same pattern used for SignupPage) to avoid bundling them in the initial bundle; replace the direct imports with React.lazy wrappers (e.g., const ManagerSubstituteRequestPage = lazy(() => import('...'))) and ensure the routes that render them are wrapped with Suspense/fallback where routes are defined (also update the other manager routes around the area referenced by the existing SignupPage pattern, including the routes around the 149-156 region) so these pages are code-split and only loaded on navigation.
🤖 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/manager/home/types/substitute.ts`:
- Line 2: The DTO adapter in substitute.ts currently imports the UI type
SubstituteRequestItem from '`@/shared/ui/manager/SubstituteApprovalCard`',
coupling business mapping to a component; extract a shared domain type (e.g.,
create a SubstituteRequestItem definition under src/shared/types or similar) and
update the adapter to import that domain type instead of the UI file, then
update the UI component SubstituteApprovalCard to import the same shared type;
ensure the new domain type matches the existing fields used by the adapter and
run/adjust any type references in functions or exports that mention
SubstituteRequestItem.
- Around line 65-70: The mapping function mapApiStatusToUiStatus currently
defaults all non-APPROVED/REJECTED_BY_APPROVER values to 'pending', causing
statuses like CANCELLED, EXPIRED, and REJECTED_BY_TARGET to be misclassified;
update mapApiStatusToUiStatus to explicitly switch/map every SubstituteApiStatus
enum member to the correct SubstituteRequestItem['status'] value (e.g., map
APPROVED -> 'accepted', REJECTED_BY_APPROVER -> 'cancelled', REJECTED_BY_TARGET
-> 'rejected' or appropriate label, EXPIRED -> 'expired', CANCELLED ->
'cancelled', etc.) so no API status falls through to the 'pending' default and
all enum cases are handled explicitly.
In
`@src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.ts`:
- Line 11: The feature layer is importing a page type (SubstituteActionType)
which breaks layer boundaries; move the type definition for SubstituteActionType
into a shared location (either src/shared/types or
src/features/manager/substitute/types) and update imports in both
useManagerSubstituteRequestViewModel.ts and the page component
ManagerSubstituteActionModal to import from the new shared/types module; ensure
no imports from src/pages remain in src/features and adjust any exported name if
needed so both files reference the same exported SubstituteActionType.
In
`@src/pages/manager/substitute-request/components/ManagerSubstituteActionModal.tsx`:
- Around line 30-79: The ManagerSubstituteActionModal currently contains
validation, error state and submit orchestration (comment, setComment, error,
handleSubmit, onSubmit) which should be moved to a features-level ViewModel
hook; create a hook (e.g. useManagerSubstituteActionViewModel) that owns comment
state, setComment, validation logic (trim/empty/length checks) and a submit()
that calls the domain/service and returns/sets submitError, then update
ManagerSubstituteActionModal to remove local comment/error/handleSubmit and
instead receive those values and callbacks from the hook (or via props) so the
component becomes purely input/display-focused and delegates all business logic
to the ViewModel.
In `@src/pages/manager/substitute-request/index.tsx`:
- Around line 4-8: The page file is importing components from another page
layer; move the shared UI components SubstituteProfileAvatar,
SubstituteRequestResponseActions, and SubstituteRequestStatusBadge out of
src/pages/user/... into a proper shared or feature layer (e.g., shared/ui or
features/substitute/ui), export them from the new module, and update the imports
in useManagerSubstituteRequestViewModel and this page (index.tsx) to import from
the new shared/feature paths; ensure you keep ManagerSubstituteActionModal and
useManagerSubstituteRequestViewModel where they are, update any barrel exports
if needed, and verify there are no circular dependency changes after relocating
the components.
- Around line 146-147: The code is coercing item.id with Number(item.id) which
can produce NaN and break onApproveClick/onRejectClick; update the call sites to
validate and convert safely: check SubstituteRequestItem.id (or item.id) is
numeric (e.g., using Number.isFinite(parseInt(...)) or /^\d+$/ test) and only
call onApproveClick/onRejectClick with a parsed integer, or pass the original
string through and adjust onApproveClick/onRejectClick to accept string IDs;
prefer harmonizing the type end-to-end to number by ensuring the source
populates id as a number and updating the props/types accordingly.
In `@src/shared/ui/manager/SubstituteApprovalCard.tsx`:
- Around line 36-40: resolveStatusLabel currently falls back to returning '대기중'
for any item.status when rawStatus is null, which incorrectly renders
'cancelled' as '대기중'; update resolveStatusLabel (and its use of
SUBSTITUTE_STATUS_LABEL) to explicitly handle item.status === 'cancelled' and
return the correct cancelled label (e.g., '취소') before the accepted/other
fallback so cancelled requests display properly.
---
Nitpick comments:
In `@src/app/App.tsx`:
- Around line 26-27: The two statically imported manager route components
ManagerSubstituteRequestPage and StoreRegisterPage should be converted to
lazy-loaded components (same pattern used for SignupPage) to avoid bundling them
in the initial bundle; replace the direct imports with React.lazy wrappers
(e.g., const ManagerSubstituteRequestPage = lazy(() => import('...'))) and
ensure the routes that render them are wrapped with Suspense/fallback where
routes are defined (also update the other manager routes around the area
referenced by the existing SignupPage pattern, including the routes around the
149-156 region) so these pages are code-split and only loaded on navigation.
🪄 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: 21532815-cd10-46a2-b51b-ef38f039dc3a
📒 Files selected for processing (12)
src/app/App.tsxsrc/features/manager/api/substitute.tssrc/features/manager/home/types/substitute.tssrc/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.tssrc/pages/manager/substitute-request/components/ManagerSubstituteActionModal.tsxsrc/pages/manager/substitute-request/index.tsxsrc/shared/constants/routes.tssrc/shared/types/substituteStatus.tssrc/shared/ui/common/Docbar.tsxsrc/shared/ui/manager/SubstituteApprovalCard.tsxstorybook/stories/Albabox.stories.tsxstorybook/stories/ManagerSubstituteActionModal.stories.tsx
| function mapApiStatusToUiStatus( | ||
| apiStatus: string | ||
| apiStatus: SubstituteApiStatus | ||
| ): SubstituteRequestItem['status'] { | ||
| if (apiStatus === 'ACCEPTED' || apiStatus === 'APPROVED') return 'accepted' | ||
| if (apiStatus === SubstituteApiStatus.APPROVED) return 'accepted' | ||
| if (apiStatus === SubstituteApiStatus.REJECTED_BY_APPROVER) return 'cancelled' | ||
| return 'pending' |
There was a problem hiding this comment.
상태 매핑의 기본값 pending은 오분류를 유발합니다.
현재 APPROVED, REJECTED_BY_APPROVER 외 상태가 전부 pending으로 내려가서 CANCELLED, EXPIRED, REJECTED_BY_TARGET도 요청중으로 표시될 수 있습니다. API 상태를 명시적으로 전부 매핑해 주세요.
수정 예시
function mapApiStatusToUiStatus(
apiStatus: SubstituteApiStatus
): SubstituteRequestItem['status'] {
- if (apiStatus === SubstituteApiStatus.APPROVED) return 'accepted'
- if (apiStatus === SubstituteApiStatus.REJECTED_BY_APPROVER) return 'cancelled'
- return 'pending'
+ if (apiStatus === SubstituteApiStatus.ACCEPTED) return 'pending'
+ if (apiStatus === SubstituteApiStatus.APPROVED) return 'accepted'
+ if (
+ apiStatus === SubstituteApiStatus.REJECTED_BY_APPROVER ||
+ apiStatus === SubstituteApiStatus.REJECTED_BY_TARGET ||
+ apiStatus === SubstituteApiStatus.CANCELLED ||
+ apiStatus === SubstituteApiStatus.EXPIRED
+ ) {
+ return 'cancelled'
+ }
+ return 'pending'
}📝 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.
| function mapApiStatusToUiStatus( | |
| apiStatus: string | |
| apiStatus: SubstituteApiStatus | |
| ): SubstituteRequestItem['status'] { | |
| if (apiStatus === 'ACCEPTED' || apiStatus === 'APPROVED') return 'accepted' | |
| if (apiStatus === SubstituteApiStatus.APPROVED) return 'accepted' | |
| if (apiStatus === SubstituteApiStatus.REJECTED_BY_APPROVER) return 'cancelled' | |
| return 'pending' | |
| function mapApiStatusToUiStatus( | |
| apiStatus: SubstituteApiStatus | |
| ): SubstituteRequestItem['status'] { | |
| if (apiStatus === SubstituteApiStatus.ACCEPTED) return 'pending' | |
| if (apiStatus === SubstituteApiStatus.APPROVED) return 'accepted' | |
| if ( | |
| apiStatus === SubstituteApiStatus.REJECTED_BY_APPROVER || | |
| apiStatus === SubstituteApiStatus.REJECTED_BY_TARGET || | |
| apiStatus === SubstituteApiStatus.CANCELLED || | |
| apiStatus === SubstituteApiStatus.EXPIRED | |
| ) { | |
| return 'cancelled' | |
| } | |
| return 'pending' | |
| } |
🤖 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/manager/home/types/substitute.ts` around lines 65 - 70, The
mapping function mapApiStatusToUiStatus currently defaults all
non-APPROVED/REJECTED_BY_APPROVER values to 'pending', causing statuses like
CANCELLED, EXPIRED, and REJECTED_BY_TARGET to be misclassified; update
mapApiStatusToUiStatus to explicitly switch/map every SubstituteApiStatus enum
member to the correct SubstituteRequestItem['status'] value (e.g., map APPROVED
-> 'accepted', REJECTED_BY_APPROVER -> 'cancelled', REJECTED_BY_TARGET ->
'rejected' or appropriate label, EXPIRED -> 'expired', CANCELLED -> 'cancelled',
etc.) so no API status falls through to the 'pending' default and all enum cases
are handled explicitly.
There was a problem hiding this comment.
@limtjdghks 알겠습니다! 서버 계약이 명확하다면 현재 코드로도 문제없습니다.
다만 SubstituteApiStatus에 다른 값들이 있는 이상, 미래 유지보수 시 혼란을 줄이기 위해 짧은 주석 하나만 달아두면 좋을 것 같습니다:
// 서버가 반환하는 상태: ACCEPTED(요청중) | APPROVED(승인) | REJECTED_BY_APPROVER(거절)
function mapApiStatusToUiStatus(
apiStatus: SubstituteApiStatus
): SubstituteRequestItem['status'] {
if (apiStatus === SubstituteApiStatus.APPROVED) return 'accepted'
if (apiStatus === SubstituteApiStatus.REJECTED_BY_APPROVER) return 'cancelled'
return 'pending'
}이 정도면 나중에 다른 개발자가 보더라도 의도를 바로 파악할 수 있습니다. 꼭 수정 안 하셔도 됩니다 👍
🐇 (´・ω・`)
| import { SubstituteProfileAvatar } from '@/pages/user/substitute-request/components/SubstituteProfileAvatar' | ||
| import { SubstituteRequestResponseActions } from '@/pages/user/substitute-request/components/SubstituteRequestResponseActions' | ||
| import { SubstituteRequestStatusBadge } from '@/pages/user/substitute-request/components/SubstituteRequestStatusBadge' | ||
| import { useManagerSubstituteRequestViewModel } from '@/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel' | ||
| import { ManagerSubstituteActionModal } from '@/pages/manager/substitute-request/components/ManagerSubstituteActionModal' |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
페이지가 다른 페이지(@/pages/user/...)를 직접 import하고 있습니다.
src/pages/**는 하위 레이어만 참조해야 하므로, 공용 컴포넌트(SubstituteProfileAvatar, SubstituteRequestResponseActions, SubstituteRequestStatusBadge)는 shared/ui 또는 features로 올려서 의존 방향을 정리해 주세요.
As per coding guidelines src/pages/**: "하위 레이어(widgets, features, entities, shared)만 import하는지 (상위 레이어 import 금지)".
🤖 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/manager/substitute-request/index.tsx` around lines 4 - 8, The page
file is importing components from another page layer; move the shared UI
components SubstituteProfileAvatar, SubstituteRequestResponseActions, and
SubstituteRequestStatusBadge out of src/pages/user/... into a proper shared or
feature layer (e.g., shared/ui or features/substitute/ui), export them from the
new module, and update the imports in useManagerSubstituteRequestViewModel and
this page (index.tsx) to import from the new shared/feature paths; ensure you
keep ManagerSubstituteActionModal and useManagerSubstituteRequestViewModel where
they are, update any barrel exports if needed, and verify there are no circular
dependency changes after relocating the components.
| function resolveStatusLabel(item: SubstituteRequestItem): string { | ||
| if (item.rawStatus != null) { | ||
| return SUBSTITUTE_STATUS_LABEL[item.rawStatus] | ||
| } | ||
| return item.status === 'accepted' ? '승인' : '대기중' |
There was a problem hiding this comment.
cancelled 상태 라벨이 fallback에서 잘못 표시됩니다.
rawStatus가 없는 경우 cancelled도 '대기중'으로 내려가서 상태 텍스트가 틀어집니다. cancelled 분기를 명시해 주세요.
🔧 제안 수정안
function resolveStatusLabel(item: SubstituteRequestItem): string {
if (item.rawStatus != null) {
return SUBSTITUTE_STATUS_LABEL[item.rawStatus]
}
- return item.status === 'accepted' ? '승인' : '대기중'
+ if (item.status === 'accepted') return '승인'
+ if (item.status === 'cancelled') return '취소됨'
+ return '대기중'
}🤖 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/manager/SubstituteApprovalCard.tsx` around lines 36 - 40,
resolveStatusLabel currently falls back to returning '대기중' for any item.status
when rawStatus is null, which incorrectly renders 'cancelled' as '대기중'; update
resolveStatusLabel (and its use of SUBSTITUTE_STATUS_LABEL) to explicitly handle
item.status === 'cancelled' and return the correct cancelled label (e.g., '취소')
before the accepted/other fallback so cancelled requests display properly.

ID
변경 내용
구현 사항
신규 파일
src/pages/manager/substitute-request/index.tsx— 대타 요청 목록 페이지 (요청됨 / 수락됨 / 취소됨 섹션)src/pages/manager/substitute-request/components/ManagerSubstituteActionModal.tsx— 승인·거절 시 코멘트 입력 모달src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.ts— 페이지 로직 분리 (상태 그룹핑, mutation, 모달 제어)src/shared/types/substituteStatus.ts— 대타 상태 enum 및 한국어 레이블 중앙화기존 파일 수정
src/features/manager/api/substitute.ts— approveSubstituteRequest, rejectSubstituteRequest API 함수 추가 (comment body 포함)src/features/manager/home/types/substitute.ts— workerRole, scheduledDate, rawStatus 필드 추가 및 상태 매핑 수정 (ACCEPTED → 요청됨,APPROVED → 수락됨)
src/shared/ui/manager/SubstituteApprovalCard.tsx— SubstituteRequestItem 타입에 workerRole, scheduledDate, rawStatus 추가src/shared/ui/common/Docbar.tsx— 매니저 탭 목록에 대타 탭 추가, scope별 경로 분기src/shared/constants/routes.ts— ROUTES.MANAGER.SUBSTITUTE_REQUEST 추가src/app/App.tsx— 매니저 대타 요청 페이지 라우트 등록구현 시연 (필요 시)
2026-05-20.4.18.49.mov
Summary by CodeRabbit
릴리스 노트
New Features
Refactor