feat: Add ability to edit existing shifts#125
Conversation
📝 WalkthroughWalkthroughAdds end-to-end shift editing: a new PUT API for updating shifts with permission and external-sync checks, an update mutation with optimistic cache updates, and UI wiring across page and components to initiate and submit edit flows from dialogs, cards, and lists. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (UI)
participant Page as Home Page
participant Dialog as Dialog Manager / ShiftSheet
participant Hook as useShifts (mutation + cache)
participant API as API Endpoint
participant DB as Database
User->>Page: Click edit on shift card / overview
Page->>Page: set editingShift
Page->>Dialog: open dialog with editingShift
User->>Dialog: Submit edited shift
Dialog->>Page: handleShiftSubmit(formData)
Page->>Hook: updateShift(shiftId, formData) (optimistic)
Hook->>Hook: onMutate - snapshot & update cache
Hook->>API: PUT /api/shifts/{id}
API->>API: verify canEditCalendar & externalSync
API->>DB: update shift record
DB-->>API: updated shift
API-->>Hook: 200 + updated shift
Hook->>Hook: onSuccess/onSettled - toast & invalidate/refetch
Hook-->>Page: mutation settled
Page->>Dialog: close dialog, clear editingShift
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/page.tsx (1)
697-763: Add shift edit functionality to compare mode dialogs.The
DialogManagerin compare mode (lines 697-763) is missing theeditingShiftandonEditShiftFromDayDialogprops, while normal mode provides both. This prevents shifts from being edited when opened from the day shifts dialog in compare mode. All other dialog infrastructure is present, and the comment indicates dialogs should be fully functional in this mode—pass these props through to maintain feature parity.
🤖 Fix all issues with AI agents
In `@app/api/shifts/`[id]/route.ts:
- Around line 111-125: The PUT handler currently checks write permission via
canEditCalendar but does not prevent edits to externally managed shifts; after
fetching existingShift from shifts (existingShift variable) add a guard that
reads existingShift.syncedFromExternal and, if true, return a 403/appropriate
error JSON indicating the shift is read-only to external syncs; keep this check
before any mutation or permission-dependent logic so externally synced shifts
cannot be edited even by authorized users.
In `@components/shifts-overview-dialog.tsx`:
- Around line 74-77: The styling conditional currently uses
!shift.externalSyncId but the click handler was changed to use
shift.syncedFromExternal; update both the className condition and the edit
button rendering to use the same check (onEditShift &&
!shift.syncedFromExternal) so hover/cursor styles and the edit-button visibility
match the click-handler logic; specifically modify the className template string
and the edit button render block (the code that references onEditShift and
shift.externalSyncId) to reference shift.syncedFromExternal instead.
- Around line 45-50: The click handler handleShiftClick currently only blocks
edits when shift.externalSyncId exists; update it to also check
shift.syncedFromExternal so it matches the guard used in CalendarShiftCard and
prevents editing externally synced shifts—i.e., change the conditional in
handleShiftClick to require both !shift.externalSyncId &&
!shift.syncedFromExternal before calling onOpenChange/onEditShift.
🧹 Nitpick comments (1)
components/shift-card.tsx (1)
77-89: Consider consistent read-only check for delete button.The delete button only checks
!shift.externalSyncIdbut the edit button checks bothexternalSyncIdandsyncedFromExternal. For consistency and to align with the learnings about enforcing read-only access for synced shifts, the delete condition should also checksyncedFromExternal.♻️ Suggested fix
- {!shift.externalSyncId && onDelete && ( + {!shift.externalSyncId && !shift.syncedFromExternal && onDelete && (
|
Great feature! Will be available in the next dev docker image within the next 30 mins! |
2 participants
feat: Add shift edit functionality
Add the ability to edit existing shifts by clicking on them throughout the application.
Features
Backend
Frontend
Files Changed
Testing
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.