feat:매장 운영 업무 스케줄 위젯 추가(#28) - #29
Conversation
📝 WalkthroughWalkthrough대시보드 위젯 렌더에 Changes대시보드 위젯 컨텍스트와 위젯 구성
레이아웃 저장 시점 변경
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DashboardView
participant DashboardGrid
participant WorkScheduleSummary
participant RecentResources
DashboardView->>DashboardGrid: workspaceId 전달
DashboardGrid->>WorkScheduleSummary: render(size, { workspaceId })
WorkScheduleSummary-->>DashboardGrid: 요약 UI 반환
DashboardGrid->>RecentResources: render(size, { workspaceId })
RecentResources-->>DashboardGrid: 필터링된 자료 UI 반환
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/dashboard/edit-layout/model/useDashboardLayout.ts`:
- Line 31: The `useDashboardLayout` save flow currently implies persisting on
every `onLayoutChange`, which will hammer the server during drag/resize. Update
the layout persistence logic in `useDashboardLayout` to debounce saves or defer
them until drag/resize ավարտ/completion, and keep the change localized to the
`onLayoutChange`/DB-sync path so frequent intermediate events do not trigger
server actions.
- Around line 29-39: The current save trigger in useDashboardLayout relies only
on didMountRef, which can still fire on StrictMode remounts or duplicate initial
onLayoutChange events. Update the useEffect in useDashboardLayout to compare the
current layout against the previous layout before calling saveDashboardLayout,
and only persist when the layout has a real change. Keep the guard logic near
didMountRef/saveDashboardLayout so the initial or duplicate layout emissions do
not cause an unnecessary save.
In
`@src/widgets/store-operation/dashboard-work-schedule/ui/WorkScheduleSummary.tsx`:
- Around line 47-53: `getNextWeekdays` is including today in the “next 4 days”
range, which can duplicate the current day in the lg summary grid. Update the
logic in `WorkScheduleSummary` so the sequence starts from the day after `today`
(not `todayIndex` itself), then returns the next 4 weekdays; check the callers
that render the top “오늘 근무 인원” and the lower grid to keep their day sets
non-overlapping.
- Around line 82-86: `WorkScheduleSummary`의 `primaryShift` 선택 로직이 배열에서 처음으로 근무
인원이 0보다 큰 시프트를 고르는 문제를 수정하세요. `workingShifts.find(...)` 대신 `todayCounts`를 기준으로
가장 많은 인원이 배정된 시프트를 계산해 대표 시프트로 사용하고, 동률일 때의 우선순위도 명확히 유지되도록 하세요.
`totalWorkingMembers` 계산은 그대로 두고, `primaryShift`를 만드는 부분만 `workingShifts`와
`todayCounts`를 함께 사용해 최다 근무 인원을 가진 시프트를 선택하도록 변경하세요.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 26043281-8ed6-4835-8213-ad3a1bb774c2
📒 Files selected for processing (9)
src/features/dashboard/edit-layout/model/useDashboardLayout.tssrc/shared/dashboard/model/widget.types.tssrc/views/dashboard/config/template-widgets.tssrc/views/dashboard/config/widget-catalog.tsxsrc/views/dashboard/ui/DashboardGrid.tsxsrc/views/dashboard/ui/DashboardView.tsxsrc/widgets/store-operation/dashboard-recent-resources/ui/RecentResources.tsxsrc/widgets/store-operation/dashboard-work-schedule/index.tssrc/widgets/store-operation/dashboard-work-schedule/ui/WorkScheduleSummary.tsx
| const didMountRef = useRef(false); | ||
|
|
||
| // TODO: DB 연동 — 변경 저장 (드래그 중 잦은 호출은 debounce 예정) | ||
| const commit = useCallback( | ||
| (next: Layout) => { | ||
| void saveDashboardLayout(workspaceId, pageType, { layout: next }); | ||
| }, | ||
| [workspaceId, pageType], | ||
| ); | ||
| useEffect(() => { | ||
| if (!didMountRef.current) { | ||
| didMountRef.current = true; | ||
| return; | ||
| } | ||
|
|
||
| void saveDashboardLayout(workspaceId, pageType, { layout }); | ||
| }, [layout, workspaceId, pageType]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
react-grid-layout onLayoutChange fires without position change v2
💡 Result:
The issue of onLayoutChange firing without a tangible position change is a known behavioral characteristic in react-grid-layout, often stemming from how the component handles internal state updates, mount events, or interaction events [1][2]. While version 2 (v2) includes significant architectural improvements, such as immutable layout updates, to address previous mutation-related bugs [3][4], users may still encounter unexpected triggers due to the following common patterns: 1. Lifecycle and Initialization: The onLayoutChange callback can fire during the component's initial mount or when the layout prop is first initialized, as the grid computes its internal layout state [1]. 2. Interaction Events: Features like droppable regions (dragenter/dragleave events) can trigger onLayoutChange even if no item is dropped, as the library may recalculate layout state in response to external drag interactions [2]. 3. Prop and Data Handling: Applying certain props (like data-grid) or managing layout state in external stores (e.g., Redux) without careful memoization can lead to redundant triggers or double-firing of callbacks [5][6]. 4. External Factors: In some environments, layout recalculations may occur due to external events like page scrolling or parent container resizing if the grid's dimensions are bound to these factors [7]. To mitigate these issues: - Implement a comparison check: Inside your onLayoutChange handler, verify if the new layout actually differs from your current state before triggering side effects (e.g., API calls or state updates) [1][6]. A deep comparison or a stringified check can be effective [6]. - Use specific event handlers: For drag and resize operations, prioritize using specific callbacks like onDragStop or onResizeStop to update your state, rather than relying solely on onLayoutChange if it is firing too frequently for your use case [2]. - Upgrade to v2: Ensure you are using the latest version (v2.2.0+), as it features a complete rewrite that utilizes immutable layout updates, significantly improving the accuracy of reference comparisons and reducing bugs related to layout mutation [3][4].
Citations:
- 1: V-0.14.3: onLayoutChange called when layout didn't change react-grid-layout/react-grid-layout#504
- 2: onLayoutChange called when something is dragged in/out layout (but not yet dropped) react-grid-layout/react-grid-layout#1862
- 3:
onLayoutChangeis not triggered, butonDragStopdoes. react-grid-layout/react-grid-layout#1775 - 4: onLayoutChange event not emit while dragging conditon react-grid-layout/react-grid-layout#1792
- 5: onLayoutChange is always called twice react-grid-layout/react-grid-layout#1984
- 6: onLayoutChange triggers on every click react-grid-layout/react-grid-layout#866
- 7: onLayoutChange called on page scroll; jittery redraw even though layout hasn't changed react-grid-layout/react-grid-layout#1059
초기 저장은 실제 레이아웃 변경 시에만 수행하도록 좁혀주세요.
didMountRef만으로는 StrictMode의 재마운트나 react-grid-layout의 초기/중복 onLayoutChange 호출을 막지 못합니다. 저장 전에 이전 레이아웃과 실질적 diff를 비교해, 변경이 있을 때만 saveDashboardLayout을 호출하는 쪽이 안전합니다.
🤖 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/dashboard/edit-layout/model/useDashboardLayout.ts` around lines
29 - 39, The current save trigger in useDashboardLayout relies only on
didMountRef, which can still fire on StrictMode remounts or duplicate initial
onLayoutChange events. Update the useEffect in useDashboardLayout to compare the
current layout against the previous layout before calling saveDashboardLayout,
and only persist when the layout has a real change. Keep the guard logic near
didMountRef/saveDashboardLayout so the initial or duplicate layout emissions do
not cause an unnecessary save.
| const [editMode, setEditMode] = useState(false); | ||
| const didMountRef = useRef(false); | ||
|
|
||
| // TODO: DB 연동 — 변경 저장 (드래그 중 잦은 호출은 debounce 예정) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
TODO: 저장 debounce. 드래그/리사이즈 중 onLayoutChange가 매우 자주 발생하므로, DB 연동 시 각 이벤트마다 서버 액션이 호출됩니다. debounce(또는 드래그 종료 시점 저장) 도입이 필요합니다.
원하시면 debounce 적용 구현을 작성하거나 추적용 이슈를 생성해 드릴까요?
🤖 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/dashboard/edit-layout/model/useDashboardLayout.ts` at line 31,
The `useDashboardLayout` save flow currently implies persisting on every
`onLayoutChange`, which will hammer the server during drag/resize. Update the
layout persistence logic in `useDashboardLayout` to debounce saves or defer them
until drag/resize ավարտ/completion, and keep the change localized to the
`onLayoutChange`/DB-sync path so frequent intermediate events do not trigger
server actions.
| function getNextWeekdays(today: WeekdayKey, count: number) { | ||
| const todayIndex = weekdays.findIndex((weekday) => weekday.key === today); | ||
| return Array.from({ length: count }, (_, index) => { | ||
| const weekdayIndex = (todayIndex + index) % weekdays.length; | ||
| return weekdays[weekdayIndex]; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
getNextWeekdays가 "다음 4일"에 오늘을 포함해, lg 뷰 하단 그리드 첫 칸이 상단 "오늘 근무 인원"과 중복 표시될 수 있습니다.
(todayIndex + index) % weekdays.length는 index === 0일 때 오늘 자신을 반환합니다. 주석/UI 상 "다음 4일 근무 인원"이라는 의도라면 오늘을 제외하고 다음날부터 4일을 보여줘야 할 것으로 보입니다.
🐛 오늘을 제외한 다음 4일을 반환하도록 하는 수정안
const todayIndex = weekdays.findIndex((weekday) => weekday.key === today);
return Array.from({ length: count }, (_, index) => {
- const weekdayIndex = (todayIndex + index) % weekdays.length;
+ const weekdayIndex = (todayIndex + index + 1) % weekdays.length;
return weekdays[weekdayIndex];
});Also applies to: 122-122, 158-182
🤖 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/widgets/store-operation/dashboard-work-schedule/ui/WorkScheduleSummary.tsx`
around lines 47 - 53, `getNextWeekdays` is including today in the “next 4 days”
range, which can duplicate the current day in the lg summary grid. Update the
logic in `WorkScheduleSummary` so the sequence starts from the day after `today`
(not `todayIndex` itself), then returns the next 4 weekdays; check the callers
that render the top “오늘 근무 인원” and the lower grid to keep their day sets
non-overlapping.
| const totalWorkingMembers = workingShifts.reduce( | ||
| (total, shift) => total + (todayCounts[shift.id] ?? 0), | ||
| 0, | ||
| ); | ||
| const primaryShift = workingShifts.find((shift) => (todayCounts[shift.id] ?? 0) > 0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
primaryShift가 최다 근무 인원 시프트가 아닌 배열 순서상 첫 매칭 시프트를 선택합니다.
workingShifts.find(...)는 mockWorkScheduleConfig.shifts 배열 순서상 인원이 0보다 큰 첫 시프트를 반환합니다. 실제로 가장 많은 인원이 근무하는 시프트가 아니라도 "대표" 시프트로 표시될 수 있습니다(예: 첫 시프트에 1명, 다른 시프트에 8명이면 1명짜리가 대표로 노출).
🐛 최다 인원 시프트를 대표로 선택하는 수정안
- const primaryShift = workingShifts.find((shift) => (todayCounts[shift.id] ?? 0) > 0);
+ const primaryShift = workingShifts.reduce<(typeof workingShifts)[number] | undefined>(
+ (best, shift) => {
+ const count = todayCounts[shift.id] ?? 0;
+ const bestCount = best ? todayCounts[best.id] ?? 0 : 0;
+ return count > bestCount ? shift : best;
+ },
+ undefined,
+ );📝 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.
| const totalWorkingMembers = workingShifts.reduce( | |
| (total, shift) => total + (todayCounts[shift.id] ?? 0), | |
| 0, | |
| ); | |
| const primaryShift = workingShifts.find((shift) => (todayCounts[shift.id] ?? 0) > 0); | |
| const totalWorkingMembers = workingShifts.reduce( | |
| (total, shift) => total + (todayCounts[shift.id] ?? 0), | |
| 0, | |
| ); | |
| const primaryShift = workingShifts.reduce<(typeof workingShifts)[number] | undefined>( | |
| (best, shift) => { | |
| const count = todayCounts[shift.id] ?? 0; | |
| const bestCount = best ? todayCounts[best.id] ?? 0 : 0; | |
| return count > bestCount ? shift : best; | |
| }, | |
| undefined, | |
| ); |
🤖 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/widgets/store-operation/dashboard-work-schedule/ui/WorkScheduleSummary.tsx`
around lines 82 - 86, `WorkScheduleSummary`의 `primaryShift` 선택 로직이 배열에서 처음으로 근무
인원이 0보다 큰 시프트를 고르는 문제를 수정하세요. `workingShifts.find(...)` 대신 `todayCounts`를 기준으로
가장 많은 인원이 배정된 시프트를 계산해 대표 시프트로 사용하고, 동률일 때의 우선순위도 명확히 유지되도록 하세요.
`totalWorkingMembers` 계산은 그대로 두고, `primaryShift`를 만드는 부분만 `workingShifts`와
`todayCounts`를 함께 사용해 최다 근무 인원을 가진 시프트를 선택하도록 변경하세요.
Kwon812
left a comment
There was a problem hiding this comment.
고생하셨습니다~ 대시보드 위젯에 workspaceId 같이 내려주는게 확장성 면에서도 좋을 것 같습니다~
지금 위젯에 추가해두신 형태로 추후 pr파서 다른 위젯들도 수정해두겠습니다~
다만 위젯별 데이터를 각각 위젯 내부에서 워크스페이스 아이디로 페칭할지 아니면 대시보드에서 일괄 페칭해서 내려줄지는 추후 고민해봤으면 좋겠습니다!
Pull Request
작업 내용
workspaceId컨텍스트를 전달해 위젯별 데이터 스코프를 맞췄습니다.작업 결과
store-operation대시보드 편집 모드의 위젯 추가 목록에업무 스케줄이 표시됩니다.workspaceId기준으로 필터링되어 다른 워크스페이스 자료가 섞이지 않습니다.변경 사항
Added
dashboard-work-schedule위젯 추가workspaceId추가Changed
store-operation템플릿의 추가 가능 위젯 목록에 업무 스케줄 추가Fixed
useEffect로 이동해 state updater 내부 부수효과 제거실행화면
테스트
검증 명령:
npm run lintnpm run typechecknpm run buildgit diff --check리뷰 체크리스트
feature/*->develop, 배포 시develop또는release/*->main)Type/#issue-number/description형식을 따릅니다.console.log, 주석, 임시 코드를 제거했습니다.리뷰 요청사항
workspaceId로 확장한 방향이 이후 공지/자료/스케줄 DB 연동에도 적절한지 봐주세요.관련 이슈
Closes #28
ref #2