[fix] 대타 요청 비즈니스 로직 API 수정#58
Open
kim3360 wants to merge 8 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough대타 요청 목록에 상태 필터를 추가하고, 유저/매니저 목록을 sections 기반 렌더링으로 전환했다. 상태값과 쿼리 파라미터 정규화도 함께 변경됐고, 스케줄 선택 UI는 읽기전용 표시로 바뀌었다. React Query devtools는 개발 환경에서만 렌더링된다. Changes대타 요청 상태 필터 및 섹션 리팩터
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/features/user/substitute/hooks/useUserSubstituteRequestsViewModel.ts (1)
77-101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win무한 쿼리의 다음 페이지 로딩 경로를 다시 노출해주세요.
useInfiniteQuery를 유지하면서fetchNextPage/hasNextPage/isFetchingNextPage반환을 제거해, 소비자가PAGE_LIMIT이후 데이터를 불러올 수 없습니다. 기존 무한 스크롤/더보기 동작이 있던 목록이면 첫 20개만 보이는 회귀가 됩니다.🐛 제안 수정
- const { data, isPending, isError, refetch } = useInfiniteQuery< + const { + data, + fetchNextPage, + hasNextPage, + isFetchingNextPage, + isPending, + isError, + refetch, + } = useInfiniteQuery< @@ isLoading: isPending, isError, + fetchNextPage, + hasNextPage: !!hasNextPage, + isFetchingNextPage, refetch, }Also applies to: 121-128
🤖 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/user/substitute/hooks/useUserSubstituteRequestsViewModel.ts` around lines 77 - 101, The `useInfiniteQuery` in `useUserSubstituteRequestsViewModel` is still set up for pagination, but the view model no longer exposes the next-page controls needed by consumers. Re-introduce the infinite-query paging outputs from `useInfiniteQuery`—specifically `fetchNextPage`, `hasNextPage`, and `isFetchingNextPage`—alongside the existing `data`, `isPending`, `isError`, and `refetch` return values so list components can keep loading beyond `PAGE_LIMIT`. Make the same update in the corresponding sent/received view model path referenced by the related lines, and keep the `getNextPageParam`/`queryFn` logic intact.src/features/user/home/workspace/hooks/useSubstituteRequestFlow.ts (1)
51-64: 🎯 Functional Correctness | 🟡 Minor같은 날짜의 근무가 여러 건이면 선택이 모호합니다.
pickSelectedScheduleEvent가 같은dateKey의 항목을 시작시간순으로 정렬해 첫 번째만substituteScheduleId로 사용합니다. 지금 플로우는 날짜만 선택할 수 있어서, 동일 날짜에 여러 근무가 있으면 사용자가 다른 근무를 구분하지 못한 채 가장 이른 근무로 요청이 생성됩니다. 날짜당 1건 보장이 없다면 시간/근무 선택 UI를 추가하거나 API를 단건으로 맞춰야 합니다.🤖 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/user/home/workspace/hooks/useSubstituteRequestFlow.ts` around lines 51 - 64, The substitute request flow currently resolves a selected day to the earliest event in pickSelectedScheduleEvent, which makes same-day multiple shifts ambiguous. Update useSubstituteRequestFlow so the user cannot accidentally submit the wrong substituteScheduleId: either add UI/state to select a specific shift time from the matched CalendarEvent list, or enforce a single-event-per-date contract before using pickSelectedScheduleEvent. Make sure the logic that consumes selectedScheduleEvent and substituteScheduleId uses an unambiguous event chosen by the user, not just the first event returned for a dateKey.
🤖 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 `@package.json`:
- Line 48: `@tanstack/react-query-devtools` and `@tanstack/react-query` are on
mismatched release lines, causing the peer dependency conflict. Update the
dependency entry for `@tanstack/react-query` in package.json to match the
devtools version used by `@tanstack/react-query-devtools`, or downgrade the
devtools package so both stay on the same compatible 5.x version.
In `@src/features/manager/home/hooks/useSubstituteRequestsViewModel.ts`:
- Around line 5-6: `useSubstituteRequestsViewModel`가 `manager/substitute`와
`user/substitute`의 내부 lib에 직접 의존하고 있어 feature 간 결합이 생깁니다.
`resolveManagerApiStatuses`와 `SubstituteListStatusFilter`를 `shared` 또는
`entities` 레이어로 옮겨 공용 상태 필터 타입/매핑으로 재배치한 뒤, 이 훅에서는 그 공용 모듈만 import하도록 수정하세요.
In
`@src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.ts`:
- Around line 10-11: The hook is importing a filter type from another feature,
creating an unwanted cross-feature dependency. Update
useManagerSubstituteRequestViewModel to stop importing
SubstituteListStatusFilter from features/user/substitute and instead use a
shared or manager-owned contract; if needed, move the filter type to a
shared/entities location and adjust ManagerSubstituteListFilters and related
usages to reference that local/shared type.
In `@src/features/manager/substitute/lib/managerSubstituteListFilters.ts`:
- Line 1: The manager substitute filter module is importing a type from the user
feature, which breaks the feature-layer dependency rule. Update
managerSubstituteListFilters to stop referencing SubstituteListStatusFilter from
the user feature by either moving that type into a shared location under
src/shared/** or defining an equivalent manager-local type within the manager
feature, and then adjust the import so manager code only depends on shared or
internal symbols.
In `@src/features/user/substitute/lib/adaptUserSubstituteRequest.ts`:
- Line 200: The status mapping in adaptUserSubstituteRequest is trusting
unwrapSubstituteEnum(dto.status) too directly, which can lead to runtime crashes
when the wrapped value is null or an object. Update unwrapSubstituteEnum and the
status conversion path used around rawStatus and the later mapping logic to
safely normalize value into a string before any toUpperCase or enum lookup
occurs, with a guard for invalid/empty inputs.
In `@src/pages/manager/substitute-request/index.tsx`:
- Around line 8-15: The manager substitute-request page is depending directly on
user page-slice components and user-specific filter utilities, which breaks the
layer boundary. Move SubstituteStatusFilterDropdown and the substitute list
status filter type/label out of the user page slice into a shared or
widget-level module, or create manager-owned equivalents, then update
src/pages/manager/substitute-request/index.tsx to import only from the new
location. Also clean up the same cross-slice type usage in
useManagerSubstituteRequestViewModel so the manager feature no longer references
`@/features/user/substitute/lib` directly.
---
Outside diff comments:
In `@src/features/user/home/workspace/hooks/useSubstituteRequestFlow.ts`:
- Around line 51-64: The substitute request flow currently resolves a selected
day to the earliest event in pickSelectedScheduleEvent, which makes same-day
multiple shifts ambiguous. Update useSubstituteRequestFlow so the user cannot
accidentally submit the wrong substituteScheduleId: either add UI/state to
select a specific shift time from the matched CalendarEvent list, or enforce a
single-event-per-date contract before using pickSelectedScheduleEvent. Make sure
the logic that consumes selectedScheduleEvent and substituteScheduleId uses an
unambiguous event chosen by the user, not just the first event returned for a
dateKey.
In `@src/features/user/substitute/hooks/useUserSubstituteRequestsViewModel.ts`:
- Around line 77-101: The `useInfiniteQuery` in
`useUserSubstituteRequestsViewModel` is still set up for pagination, but the
view model no longer exposes the next-page controls needed by consumers.
Re-introduce the infinite-query paging outputs from
`useInfiniteQuery`—specifically `fetchNextPage`, `hasNextPage`, and
`isFetchingNextPage`—alongside the existing `data`, `isPending`, `isError`, and
`refetch` return values so list components can keep loading beyond `PAGE_LIMIT`.
Make the same update in the corresponding sent/received view model path
referenced by the related lines, and keep the `getNextPageParam`/`queryFn` logic
intact.
🪄 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: 4f89543a-004c-4d8a-b2da-af4c9d3a9082
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
package.jsonsrc/app/providers/index.tsxsrc/features/manager/api/substitute.tssrc/features/manager/home/hooks/useSubstituteRequestsViewModel.tssrc/features/manager/home/types/substitute.tssrc/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.tssrc/features/manager/substitute/lib/managerSubstituteListFilters.tssrc/features/user/home/workspace/hooks/useSubstituteRequestFlow.tssrc/features/user/substitute/api/userSubstituteRequests.tssrc/features/user/substitute/hooks/useUserSubstituteRequestsViewModel.tssrc/features/user/substitute/lib/adaptUserSubstituteRequest.test.tssrc/features/user/substitute/lib/adaptUserSubstituteRequest.tssrc/features/user/substitute/lib/substituteListFilters.tssrc/features/user/substitute/types.tssrc/pages/manager/substitute-request/components/ManagerSubstituteActionModal.tsxsrc/pages/manager/substitute-request/index.tsxsrc/pages/user/substitute-request/components/SubstituteRequestListSections.tsxsrc/pages/user/substitute-request/components/SubstituteStatusFilterDropdown.tsxsrc/pages/user/substitute-request/index.tsxsrc/pages/user/workspace-detail/components/SubstituteRequestModalFlow.tsxsrc/shared/lib/queryKeys.ts
| "@storybook/addon-docs": "10.3.6", | ||
| "@storybook/addon-vitest": "10.3.6", | ||
| "@storybook/react-vite": "10.3.6", | ||
| "@tanstack/react-query-devtools": "^5.101.1", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "repo react-query version:"
jq -r '.dependencies["`@tanstack/react-query`"]' package.json
echo "devtools@5.101.1 peer range for react-query:"
curl -s https://registry.npmjs.org/@tanstack%2Freact-query-devtools/5.101.1 \
| jq -r '.peerDependencies["`@tanstack/react-query`"]'Repository: alter-app/alter-client
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "package.json relevant lines:"
sed -n '1,140p' package.json | nl -ba | sed -n '1,140p'
echo
echo "tanstack-related dependencies:"
jq -r '.dependencies, .devDependencies | to_entries[] | select(.key|test("^`@tanstack/`")) | "\(.key)=\(.value)"' package.jsonRepository: alter-app/alter-client
Length of output: 229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "package.json relevant lines:"
sed -n '1,140p' package.json | nl -ba | sed -n '1,140p'
echo
echo "tanstack-related dependencies:"
jq -r '.dependencies, .devDependencies | to_entries[] | select(.key|test("^`@tanstack/`")) | "\(.key)=\(.value)"' package.jsonRepository: alter-app/alter-client
Length of output: 229
@tanstack/react-query와 Devtools 버전을 맞춰주세요. @tanstack/react-query-devtools@5.101.1은 @tanstack/react-query@^5.101.1을 요구하므로, 현재 ^5.90.21과 peer 조건이 맞지 않습니다. react-query를 ^5.101.1로 올리거나 Devtools를 같은 릴리스 라인으로 내리세요.
🤖 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 `@package.json` at line 48, `@tanstack/react-query-devtools` and
`@tanstack/react-query` are on mismatched release lines, causing the peer
dependency conflict. Update the dependency entry for `@tanstack/react-query` in
package.json to match the devtools version used by
`@tanstack/react-query-devtools`, or downgrade the devtools package so both stay
on the same compatible 5.x version.
Comment on lines
+5
to
+6
| import { resolveManagerApiStatuses } from '@/features/manager/substitute/lib/managerSubstituteListFilters' | ||
| import type { SubstituteListStatusFilter } from '@/features/user/substitute/lib/substituteListFilters' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
상태 필터 매핑을 feature 밖으로 올려주세요.
manager/home 훅이 manager/substitute와 user/substitute 내부 lib에 직접 의존합니다. 공용 상태 필터 타입/매핑을 shared 또는 entities로 이동하면 feature 간 의존성을 끊을 수 있습니다.
As per path instructions, “src/features/** … entities, shared 레이어만 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/features/manager/home/hooks/useSubstituteRequestsViewModel.ts` around
lines 5 - 6, `useSubstituteRequestsViewModel`가 `manager/substitute`와
`user/substitute`의 내부 lib에 직접 의존하고 있어 feature 간 결합이 생깁니다.
`resolveManagerApiStatuses`와 `SubstituteListStatusFilter`를 `shared` 또는
`entities` 레이어로 옮겨 공용 상태 필터 타입/매핑으로 재배치한 뒤, 이 훅에서는 그 공용 모듈만 import하도록 수정하세요.
Source: Path instructions
Comment on lines
+10
to
+11
| import type { ManagerSubstituteListFilters } from '@/features/manager/substitute/lib/managerSubstituteListFilters' | ||
| import type { SubstituteListStatusFilter } from '@/features/user/substitute/lib/substituteListFilters' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
필터 타입을 user feature에서 직접 가져오지 마세요.
manager/substitute가 features/user/substitute 타입에 의존해서 feature 간 결합이 생깁니다. SubstituteListStatusFilter 같은 공용 필터 계약은 shared/entities로 옮기거나 manager 전용 타입으로 분리해주세요.
As per path instructions, “src/features/** … entities, shared 레이어만 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/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.ts`
around lines 10 - 11, The hook is importing a filter type from another feature,
creating an unwanted cross-feature dependency. Update
useManagerSubstituteRequestViewModel to stop importing
SubstituteListStatusFilter from features/user/substitute and instead use a
shared or manager-owned contract; if needed, move the filter type to a
shared/entities location and adjust ManagerSubstituteListFilters and related
usages to reference that local/shared type.
Source: Path instructions
| @@ -0,0 +1,23 @@ | |||
| import type { SubstituteListStatusFilter } from '@/features/user/substitute/lib/substituteListFilters' | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
매니저 feature가 user feature 타입에 직접 의존하고 있습니다.
@/features/user/... 타입 import는 feature 레이어 경계를 깨뜨립니다. SubstituteListStatusFilter를 src/shared/**로 이동해 공용화하거나, 매니저 feature 내부 타입으로 분리해 의존 방향을 정리해주세요.
수정 예시(단기 완화안)
-import type { SubstituteListStatusFilter } from '`@/features/user/substitute/lib/substituteListFilters`'
import { SubstituteApiStatus } from '`@/shared/types/substituteStatus`'
+type SubstituteListStatusFilter = 'all' | 'pending' | 'accepted' | 'cancelled'
+
export type ManagerSubstituteListFilters = {
statusFilter: SubstituteListStatusFilter
}As per path instructions, "src/features/**: entities, shared 레이어만 import하는지" 규칙을 기준으로 확인했습니다.
📝 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.
Suggested change
| import type { SubstituteListStatusFilter } from '@/features/user/substitute/lib/substituteListFilters' | |
| type SubstituteListStatusFilter = 'all' | 'pending' | 'accepted' | 'cancelled' |
🤖 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/substitute/lib/managerSubstituteListFilters.ts` at line
1, The manager substitute filter module is importing a type from the user
feature, which breaks the feature-layer dependency rule. Update
managerSubstituteListFilters to stop referencing SubstituteListStatusFilter from
the user feature by either moving that type into a shared location under
src/shared/** or defining an equivalent manager-local type within the manager
feature, and then adjust the import so manager code only depends on shared or
internal symbols.
Source: Path instructions
| const storeName = dto.workspace.workspaceName?.trim() ?? '매장' | ||
| const personName = dto.requester.workerName?.trim() ?? '이름' | ||
| const rawStatus = String(dto.status) | ||
| const rawStatus = unwrapSubstituteEnum(dto.status) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
enum 래퍼 value를 그대로 신뢰하면 런타임 크래시가 날 수 있습니다.
Line 200, Line 263에서 unwrapSubstituteEnum 결과를 상태 매핑에 바로 사용합니다. 현재 unwrapSubstituteEnum은 객체면 value를 그대로 반환해, 비정상 응답(null/객체 등) 시 toUpperCase() 경로에서 크래시할 수 있습니다. value를 문자열로 강제 변환하는 가드를 넣어 주세요.
🔧 제안 수정
export function unwrapSubstituteEnum<T extends string>(
field: SubstituteEnumValueDto<T> | T | string | null | undefined
): string {
if (field == null) return ''
- if (typeof field === 'object' && 'value' in field) return field.value
+ if (typeof field === 'object' && 'value' in field) {
+ const value = (field as { value: unknown }).value
+ return typeof value === 'string' ? value : String(value ?? '')
+ }
return String(field)
}As per path instructions, **/*.ts에서는 “잘못된 단언으로 인한 런타임 위험”을 우선 지적해야 합니다.
Also applies to: 255-263
🤖 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/user/substitute/lib/adaptUserSubstituteRequest.ts` at line 200,
The status mapping in adaptUserSubstituteRequest is trusting
unwrapSubstituteEnum(dto.status) too directly, which can lead to runtime crashes
when the wrapped value is null or an object. Update unwrapSubstituteEnum and the
status conversion path used around rawStatus and the later mapping logic to
safely normalize value into a string before any toUpperCase or enum lookup
occurs, with a guard for invalid/empty inputs.
Source: Path instructions
Comment on lines
+8
to
+15
| import { SubstituteStatusFilterDropdown } from '@/pages/user/substitute-request/components/SubstituteStatusFilterDropdown' | ||
| import { useNavbarNotificationProps } from '@/features/notification' | ||
| import { useManagerSubstituteRequestViewModel } from '@/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel' | ||
| import { | ||
| useManagerSubstituteRequestViewModel, | ||
| type ManagerSubstituteSection, | ||
| } from '@/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel' | ||
| import type { SubstituteListStatusFilter } from '@/features/user/substitute/lib/substituteListFilters' | ||
| import { statusFilterLabel } from '@/features/user/substitute/lib/substituteListFilters' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP "from '`@/pages/user/substitute-request/components/SubstituteStatusFilterDropdown`'" src
rg -nP "from '`@/features/user/substitute/lib/substituteListFilters`'" src/pages/manager src/features/managerRepository: alter-app/alter-client
Length of output: 1356
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/pages/manager/substitute-request/index.tsx ---'
sed -n '1,240p' src/pages/manager/substitute-request/index.tsx
echo
echo '--- src/pages/user/substitute-request/components/SubstituteStatusFilterDropdown.tsx ---'
sed -n '1,220p' src/pages/user/substitute-request/components/SubstituteStatusFilterDropdown.tsx
echo
echo '--- src/features/manager/substitute/lib/managerSubstituteListFilters.ts ---'
sed -n '1,220p' src/features/manager/substitute/lib/managerSubstituteListFilters.ts
echo
echo '--- src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.ts ---'
sed -n '1,220p' src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.tsRepository: alter-app/alter-client
Length of output: 16151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/pages/manager/substitute-request/index.tsx ---'
sed -n '1,240p' src/pages/manager/substitute-request/index.tsx
echo
echo '--- src/pages/user/substitute-request/components/SubstituteStatusFilterDropdown.tsx ---'
sed -n '1,220p' src/pages/user/substitute-request/components/SubstituteStatusFilterDropdown.tsx
echo
echo '--- src/features/manager/substitute/lib/managerSubstituteListFilters.ts ---'
sed -n '1,220p' src/features/manager/substitute/lib/managerSubstituteListFilters.ts
echo
echo '--- src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.ts ---'
sed -n '1,220p' src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.tsRepository: alter-app/alter-client
Length of output: 16151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/pages/manager/substitute-request/index.tsx ---'
sed -n '1,240p' src/pages/manager/substitute-request/index.tsx
echo
echo '--- src/pages/user/substitute-request/components/SubstituteStatusFilterDropdown.tsx ---'
sed -n '1,220p' src/pages/user/substitute-request/components/SubstituteStatusFilterDropdown.tsx
echo
echo '--- src/features/manager/substitute/lib/managerSubstituteListFilters.ts ---'
sed -n '1,220p' src/features/manager/substitute/lib/managerSubstituteListFilters.ts
echo
echo '--- src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.ts ---'
sed -n '1,220p' src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.tsRepository: alter-app/alter-client
Length of output: 16151
레이어 경계 위반
src/pages/manager/substitute-request/index.tsx가 @/pages/user/... 컴포넌트와 @/features/user/substitute/lib/...를 직접 참조하고 있습니다. manager 쪽은 user page slice에 의존하지 말고, 필터 드롭다운/타입·라벨을 shared(또는 widgets)로 올리거나 manager 전용으로 분리하세요. 같은 타입 의존이 src/features/manager/substitute/hooks/useManagerSubstituteRequestViewModel.ts에도 있으니 함께 정리해야 합니다.
🤖 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 8 - 15, The
manager substitute-request page is depending directly on user page-slice
components and user-specific filter utilities, which breaks the layer boundary.
Move SubstituteStatusFilterDropdown and the substitute list status filter
type/label out of the user page slice into a shared or widget-level module, or
create manager-owned equivalents, then update
src/pages/manager/substitute-request/index.tsx to import only from the new
location. Also clean up the same cross-slice type usage in
useManagerSubstituteRequestViewModel so the manager feature no longer references
`@/features/user/substitute/lib` directly.
Source: Path instructions
Member
|
hey man 언제해 |
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/pages/user/substitute-request/components/SubstituteRequestCard.tsx (1)
23-27: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win액션 버튼 클릭 시 카드 onClick도 같이 트리거됨
showResponseActions가 true인 경우(수신 탭 + pending), 카드 wrapper(라인 32-49)의onClick과 내부SubstituteRequestResponseActions(라인 70-74)의 수락/거절 버튼이 중첩됩니다.SubstituteRequestResponseActions구현(관련 코드 스니펫 참고)에는stopPropagation호출이 없어, 수락/거절 버튼을 클릭하면 이벤트가 버블링되어 카드의onClick(상세 진입 등)도 함께 실행됩니다. 사용자가 거절 버튼을 눌렀는데 상세 모달이 열리는 등 의도치 않은 동작이 발생할 수 있습니다.🐛 제안하는 수정 (SubstituteRequestResponseActions.tsx)
<ActionIconButton label="수락" className="bg-main" - onClick={onAccept} + onClick={e => { + e.stopPropagation() + onAccept() + }} disabled={disabled} >동일하게 거절 버튼에도 적용.
Also applies to: 32-49, 69-81
🤖 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/user/substitute-request/components/SubstituteRequestCard.tsx` around lines 23 - 27, `SubstituteRequestCard`에서 수신/대기 상태일 때 카드 wrapper의 onClick과 `SubstituteRequestResponseActions`의 수락/거절 버튼 클릭이 함께 발생하는 중첩 이벤트를 막아야 합니다. `showResponseActions`가 true인 경우 버튼 클릭이 카드 클릭으로 버블링되지 않도록 `SubstituteRequestResponseActions`의 액션 핸들러들에서 이벤트 전파를 차단하고, 수락/거절 모두 동일하게 적용되도록 수정하세요. 카드 진입 로직이 있는 wrapper와 `SubstituteRequestResponseActions` 구현을 함께 확인해 버튼 클릭 시 카드 onClick이 실행되지 않게 정리하세요.
♻️ Duplicate comments (1)
src/features/user/substitute/lib/adaptUserSubstituteRequest.ts (1)
21-27: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
unwrapSubstituteEnum의 타입 가드 누락 — 이전 리뷰와 동일한 이슈가 아직 남아있습니다.
typeof field === 'object' && 'value' in field분기에서field.value를 문자열 검증 없이 바로 반환합니다. 이번 PR에서status/requestType유니온이SubstituteEnumValueDto까지 확장되면서 이 경로를 타는 빈도가 늘었는데, 백엔드가value에 문자열이 아닌 값(또는null)을 내려주면 이후normalizeSubstituteStatus의toUpperCase()에서 런타임 크래시가 발생합니다.🔧 제안 수정
export function unwrapSubstituteEnum<T extends string>( field: SubstituteEnumValueDto<T> | T | string | null | undefined ): string { if (field == null) return '' - if (typeof field === 'object' && 'value' in field) return field.value + if (typeof field === 'object' && 'value' in field) { + const value = (field as { value: unknown }).value + return typeof value === 'string' ? value : String(value ?? '') + } return String(field) }As per path instructions,
**/*.ts에서는 "잘못된 단언으로 인한 런타임 위험"을 지적해야 합니다.🤖 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/user/substitute/lib/adaptUserSubstituteRequest.ts` around lines 21 - 27, `unwrapSubstituteEnum`에서 잘못된 단언으로 인한 런타임 위험이 있습니다. `SubstituteEnumValueDto`를 처리하는 `typeof field === 'object' && 'value' in field` 분기에서 `field.value`를 그대로 반환하지 말고, 문자열인지 먼저 검증한 뒤에만 반환하도록 수정하세요. `adaptUserSubstituteRequest`의 `unwrapSubstituteEnum`과 이를 사용하는 `normalizeSubstituteStatus`/`status`/`requestType` 경로를 기준으로, 문자열이 아니거나 `null`이면 안전한 기본값으로 처리해 이후 `toUpperCase()`가 크래시 나지 않게 하세요.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/features/user/substitute/lib/adaptExchangeableSchedules.ts`:
- Around line 12-16: normalizeScheduleItems currently turns any non-array
payload into an empty array silently, which can mask wrapped responses like {
schedules: [...] }. Update normalizeScheduleItems in
adaptExchangeableSchedules.ts to explicitly handle the expected shapes from
ExchangeableSchedulesApiResponse and, when the payload is not an array, emit a
development-time warning or log with the unexpected payload shape instead of
failing silently. Keep the fallback to an empty array, but make the behavior
observable so callers can detect backend shape mismatches.
---
Outside diff comments:
In `@src/pages/user/substitute-request/components/SubstituteRequestCard.tsx`:
- Around line 23-27: `SubstituteRequestCard`에서 수신/대기 상태일 때 카드 wrapper의 onClick과
`SubstituteRequestResponseActions`의 수락/거절 버튼 클릭이 함께 발생하는 중첩 이벤트를 막아야 합니다.
`showResponseActions`가 true인 경우 버튼 클릭이 카드 클릭으로 버블링되지 않도록
`SubstituteRequestResponseActions`의 액션 핸들러들에서 이벤트 전파를 차단하고, 수락/거절 모두 동일하게 적용되도록
수정하세요. 카드 진입 로직이 있는 wrapper와 `SubstituteRequestResponseActions` 구현을 함께 확인해 버튼 클릭
시 카드 onClick이 실행되지 않게 정리하세요.
---
Duplicate comments:
In `@src/features/user/substitute/lib/adaptUserSubstituteRequest.ts`:
- Around line 21-27: `unwrapSubstituteEnum`에서 잘못된 단언으로 인한 런타임 위험이 있습니다.
`SubstituteEnumValueDto`를 처리하는 `typeof field === 'object' && 'value' in field`
분기에서 `field.value`를 그대로 반환하지 말고, 문자열인지 먼저 검증한 뒤에만 반환하도록 수정하세요.
`adaptUserSubstituteRequest`의 `unwrapSubstituteEnum`과 이를 사용하는
`normalizeSubstituteStatus`/`status`/`requestType` 경로를 기준으로, 문자열이 아니거나 `null`이면
안전한 기본값으로 처리해 이후 `toUpperCase()`가 크래시 나지 않게 하세요.
🪄 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: b94c783d-57fe-44d9-8c7e-216af5ac2472
📒 Files selected for processing (5)
src/features/user/substitute/lib/adaptExchangeableSchedules.tssrc/features/user/substitute/lib/adaptUserSubstituteRequest.tssrc/features/user/substitute/types.tssrc/pages/user/substitute-request/components/SubstituteRequestCard.tsxsrc/pages/user/substitute-request/components/SubstituteRequestStatusBadge.tsx
Comment on lines
12
to
16
| function normalizeScheduleItems( | ||
| payload: ExchangeableSchedulesApiResponse['data'] | ||
| ): MyScheduleItemDto[] { | ||
| if (payload == null) return [] | ||
| if (Array.isArray(payload)) return payload | ||
| if (typeof payload === 'object' && Array.isArray(payload.schedules)) { | ||
| return payload.schedules | ||
| } | ||
| return [] | ||
| return Array.isArray(payload) ? payload : [] | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
배열이 아닌 payload는 무조건 빈 배열로 침묵 처리됩니다.
payload가 배열이 아니면 ({ schedules: [...] } 포함) 에러나 로그 없이 빈 배열을 반환합니다. types.ts의 ExchangeableSchedulesApiResponse 축소와 짝을 이루는 변경인데, 실제 백엔드가 아직 래핑된 형태를 내려줄 가능성이 있다면 사용자에게 "일정 없음"으로 잘못 보일 수 있습니다. 최소한 개발 환경에서 경고 로그라도 남기는 것을 고려해주세요.
🛡️ 제안 수정
function normalizeScheduleItems(
payload: ExchangeableSchedulesApiResponse['data']
): MyScheduleItemDto[] {
- return Array.isArray(payload) ? payload : []
+ if (Array.isArray(payload)) return payload
+ if (process.env.NODE_ENV !== 'production') {
+ console.warn('Unexpected exchangeable-schedules payload shape', payload)
+ }
+ return []
}📝 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.
Suggested change
| function normalizeScheduleItems( | |
| payload: ExchangeableSchedulesApiResponse['data'] | |
| ): MyScheduleItemDto[] { | |
| if (payload == null) return [] | |
| if (Array.isArray(payload)) return payload | |
| if (typeof payload === 'object' && Array.isArray(payload.schedules)) { | |
| return payload.schedules | |
| } | |
| return [] | |
| return Array.isArray(payload) ? payload : [] | |
| } | |
| function normalizeScheduleItems( | |
| payload: ExchangeableSchedulesApiResponse['data'] | |
| ): MyScheduleItemDto[] { | |
| if (Array.isArray(payload)) return payload | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.warn('Unexpected exchangeable-schedules payload shape', payload) | |
| } | |
| 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/features/user/substitute/lib/adaptExchangeableSchedules.ts` around lines
12 - 16, normalizeScheduleItems currently turns any non-array payload into an
empty array silently, which can mask wrapped responses like { schedules: [...]
}. Update normalizeScheduleItems in adaptExchangeableSchedules.ts to explicitly
handle the expected shapes from ExchangeableSchedulesApiResponse and, when the
payload is not an array, emit a development-time warning or log with the
unexpected payload shape instead of failing silently. Keep the fallback to an
empty array, but make the behavior observable so callers can detect backend
shape mismatches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ID
변경 내용
구현 사항
G3 — 근무시간 UI / 스케줄 연동
G4 — 유저 대타 목록 필터
G4 — 매니저 대타 목록 필터
G6 — 매니저 승인 코멘트
G7 — 받은 대타 enum 래퍼
TanStack Query DevTools
구현 시연 (필요 시)
참고 사항 (필요 시)
미포함 (백엔드 파라미터 수정중 & UI확장 가능성으로 인한)
포함
Summary by CodeRabbit