feat: implement iCloud sync functionality with CRUD operations#7
feat: implement iCloud sync functionality with CRUD operations#7
Conversation
- Added API routes for managing iCloud syncs, including GET, POST, PATCH, and DELETE methods. - Implemented synchronization logic to fetch and update shifts from iCloud calendars. - Created UI components for managing iCloud syncs and displaying shifts in a dialog. - Updated database schema to include new fields for iCloud syncs and associated shifts. - Integrated error handling and user feedback for sync operations.
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds multi-calendar iCloud sync: DB migration and types, API endpoints for listing/creating/reading/updating/deleting and on-demand syncing (fetch/parse ICS with ical.js and transactional shift sync), new UI dialogs/components and integrations, i18n strings, shared preset constants, and utility helpers. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Browser / UI
participant API as Next.js API
participant DB as Database
participant iCloud as iCloud Calendar
User->>UI: Click "Sync Now" for a sync entry
UI->>API: POST /api/icloud-syncs/{id}/sync
API->>DB: Load icloud_syncs by id
DB-->>API: icloud_sync record
API->>API: Normalize URL (webcal→https) & validate
API->>iCloud: Fetch ICS
alt fetch failed
API-->>UI: 500 error
else fetch success
API->>API: Parse ICS (ical.js)
alt parse failed
API-->>UI: 500 error
else parse success
API->>DB: Load existing shifts for sync
DB-->>API: existing shifts
API->>API: Expand recurring events & split multi-day events
loop each occurrence
alt match existing icloud_event_id
API->>DB: update shift
else
API->>DB: insert shift
end
end
API->>DB: delete shifts not in ICS
API->>DB: update icloud_syncs.last_synced_at
DB-->>API: transaction commit
API-->>UI: { created, updated, deleted, total }
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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
🧹 Nitpick comments (8)
README.md (1)
39-39: Use Apple’s standard “iCloud” casing in feature bulletFor consistency with Apple branding and the rest of the codebase, consider changing “ICloud Sync” to “iCloud Sync”.
lib/db/schema.ts (1)
19-36: Schema design for iCloud syncs is consistent with existing patterns
icloud_syncsand the new shift fields use cascading FKs and timestamp columns in line with the rest of the schema, and the exportedICloudSync/NewICloudSynctypes follow the project’s type conventions. This should integrate smoothly with the new API routes and UI.If you expect large numbers of synced events and frequent lookups by
icloudEventIdoricloudSyncId, consider adding an index (or composite index) on those columns in a follow-up migration for better query performance. Based on learnings, schema changes are coupled with migrations.Also applies to: 58-64, 116-117
app/api/icloud-syncs/[id]/route.ts (1)
3-3: Remove unused import.The
shiftsimport is not used in this file and should be removed to maintain code cleanliness.Apply this diff:
-import { icloudSyncs, shifts } from "@/lib/db/schema"; +import { icloudSyncs } from "@/lib/db/schema";app/api/icloud-syncs/[id]/sync/route.ts (2)
34-38: Add timeout to external fetch to prevent hanging requests.The fetch call to the iCloud URL has no timeout, which could cause the request to hang indefinitely if the iCloud server is slow or unresponsive.
- const response = await fetch(fetchUrl); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); // 30s timeout + const response = await fetch(fetchUrl, { signal: controller.signal }); + clearTimeout(timeoutId);
140-169: Consider batching database operations for better performance.Individual
insert,update, anddeletecalls within loops create N+1 database round-trips. For calendars with many events, consider batching these operations.For example, batch inserts:
// Collect all new shifts, then insert in one call if (newShiftsData.length > 0) { await db.insert(shifts).values(newShiftsData); }components/icloud-sync-manage-dialog.tsx (1)
197-198: Consider using a custom confirmation dialog.Using native
confirm()works but is inconsistent with the app's styled dialog patterns. Consider using a custom confirmation dialog for better UX consistency.drizzle/0004_modern_swarm.sql (2)
1-11: Consider adding a unique constraint on (calendar_id, icloud_url).The UI prevents duplicate URLs per calendar client-side, but a database-level unique constraint would ensure data integrity and prevent race conditions.
CREATE TABLE `icloud_syncs` ( `id` text PRIMARY KEY NOT NULL, `calendar_id` text NOT NULL, `name` text NOT NULL, `icloud_url` text NOT NULL, `color` text DEFAULT '#3b82f6' NOT NULL, `last_synced_at` integer, `created_at` integer DEFAULT CURRENT_TIMESTAMP NOT NULL, `updated_at` integer DEFAULT CURRENT_TIMESTAMP NOT NULL, - FOREIGN KEY (`calendar_id`) REFERENCES `calendars`(`id`) ON UPDATE no action ON DELETE cascade + FOREIGN KEY (`calendar_id`) REFERENCES `calendars`(`id`) ON UPDATE no action ON DELETE cascade, + UNIQUE(`calendar_id`, `icloud_url`) );
13-15: Consider adding an index onicloud_sync_idfor query performance.The sync route queries shifts by
icloud_sync_id(line 66 in sync/route.ts). Adding an index would improve performance for calendars with many shifts.CREATE INDEX idx_shifts_icloud_sync_id ON shifts(icloud_sync_id);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
README.md(1 hunks)app/api/icloud-syncs/[id]/route.ts(1 hunks)app/api/icloud-syncs/[id]/sync/route.ts(1 hunks)app/api/icloud-syncs/route.ts(1 hunks)app/api/shifts/route.ts(1 hunks)app/page.tsx(8 hunks)components/app-header.tsx(4 hunks)components/calendar-grid.tsx(3 hunks)components/calendar-selector.tsx(4 hunks)components/day-shifts-dialog.tsx(1 hunks)components/icloud-sync-manage-dialog.tsx(1 hunks)components/shift-card.tsx(1 hunks)drizzle/0004_modern_swarm.sql(1 hunks)drizzle/meta/0004_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)lib/db/schema.ts(3 hunks)lib/types.ts(2 hunks)messages/de.json(1 hunks)messages/en.json(1 hunks)package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
app/api/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/api/**/*.ts: Always await dynamic route params in API routes and server components (Next.js 16 breaking change)
Create API route handlers for new database tables at app/api/tablename/route.ts and app/api/tablename/[id]/route.ts
Log all API errors with console.error() for debugging
Files:
app/api/shifts/route.tsapp/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/sync/route.tsapp/api/icloud-syncs/[id]/route.ts
app/api/**/route.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
API GET routes must validate required query parameters and return 400 status for missing params
Files:
app/api/shifts/route.tsapp/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/sync/route.tsapp/api/icloud-syncs/[id]/route.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Files:
app/api/shifts/route.tsapp/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/sync/route.tsapp/api/icloud-syncs/[id]/route.tsapp/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Files:
app/api/shifts/route.tscomponents/shift-card.tsxapp/api/icloud-syncs/route.tscomponents/calendar-grid.tsxcomponents/day-shifts-dialog.tsxlib/db/schema.tsapp/api/icloud-syncs/[id]/sync/route.tsapp/api/icloud-syncs/[id]/route.tscomponents/icloud-sync-manage-dialog.tsxapp/page.tsxcomponents/calendar-selector.tsxlib/types.tscomponents/app-header.tsx
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PRESET_COLORS constant array for all color picker options
Files:
components/shift-card.tsxcomponents/calendar-grid.tsxcomponents/day-shifts-dialog.tsxcomponents/icloud-sync-manage-dialog.tsxcomponents/calendar-selector.tsxcomponents/app-header.tsx
components/**/calendar*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Files:
components/calendar-grid.tsxcomponents/calendar-selector.tsx
components/**/*-dialog.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*-dialog.tsx: Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Files:
components/day-shifts-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
lib/db/schema.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
lib/db/schema.ts: Database schema changes in lib/db/schema.ts require running npm run db:generate followed by npm run db:migrate
Use cascade delete relationships in database schema (onDelete: "cascade") for dependent tables
Drizzle ORM timestamp fields must use { mode: "timestamp" } and are stored as integers, auto-converted to Date objects
Export type definitions from database schema as TableName = typeof tableName.$inferSelect for use in components and API routes
Files:
lib/db/schema.ts
app/page.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/page.tsx: Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Refresh data after mutations by incrementing statsRefreshTrigger counter
Files:
app/page.tsx
app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/**/*.tsx: Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Shift toggle logic must check if matching shift exists: delete if present, create if not
Use locale-specific date formatters in components: locale === "de" ? de : enUS
Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Files:
app/page.tsx
🧠 Learnings (14)
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Applied to files:
components/shift-card.tsxcomponents/calendar-grid.tsxcomponents/day-shifts-dialog.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Applied to files:
components/shift-card.tsxcomponents/calendar-grid.tsxcomponents/day-shifts-dialog.tsxlib/db/schema.tscomponents/icloud-sync-manage-dialog.tsxapp/page.tsxcomponents/calendar-selector.tsxcomponents/app-header.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/api/**/*.ts : Create API route handlers for new database tables at app/api/tablename/route.ts and app/api/tablename/[id]/route.ts
Applied to files:
app/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Applied to files:
app/api/icloud-syncs/route.tsapp/page.tsxcomponents/calendar-selector.tsxcomponents/app-header.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Applied to files:
components/day-shifts-dialog.tsxcomponents/icloud-sync-manage-dialog.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use locale-specific date formatters in components: locale === "de" ? de : enUS
Applied to files:
components/day-shifts-dialog.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Applied to files:
components/day-shifts-dialog.tsxcomponents/icloud-sync-manage-dialog.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Applied to files:
components/day-shifts-dialog.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Migrations are committed to git; schema changes require both updating lib/db/schema.ts AND running migrations
Applied to files:
lib/db/schema.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to lib/db/schema.ts : Database schema changes in lib/db/schema.ts require running npm run db:generate followed by npm run db:migrate
Applied to files:
lib/db/schema.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to lib/db/schema.ts : Export type definitions from database schema as TableName = typeof tableName.$inferSelect for use in components and API routes
Applied to files:
lib/db/schema.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to lib/db/schema.ts : Drizzle ORM timestamp fields must use { mode: "timestamp" } and are stored as integers, auto-converted to Date objects
Applied to files:
lib/db/schema.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Applied to files:
package.json
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Applied to files:
app/page.tsxcomponents/calendar-selector.tsxcomponents/app-header.tsx
🧬 Code graph analysis (7)
app/api/shifts/route.ts (1)
lib/db/schema.ts (1)
shifts(38-71)
components/shift-card.tsx (1)
components/ui/button.tsx (1)
Button(60-60)
components/calendar-grid.tsx (2)
lib/db/schema.ts (1)
shifts(38-71)lib/types.ts (1)
ShiftWithCalendar(14-32)
components/day-shifts-dialog.tsx (1)
lib/types.ts (1)
ShiftWithCalendar(14-32)
lib/db/schema.ts (1)
lib/types.ts (1)
ICloudSync(2-2)
app/api/icloud-syncs/[id]/route.ts (3)
app/api/icloud-syncs/route.ts (1)
GET(7-33)lib/db/index.ts (1)
db(19-19)lib/db/schema.ts (1)
icloudSyncs(19-36)
components/icloud-sync-manage-dialog.tsx (7)
lib/db/schema.ts (1)
ICloudSync(116-116)lib/types.ts (1)
ICloudSync(2-2)components/ui/dialog.tsx (5)
Dialog(139-139)DialogContent(141-141)DialogHeader(144-144)DialogTitle(147-147)DialogDescription(142-142)components/ui/button.tsx (1)
Button(60-60)components/ui/label.tsx (1)
Label(24-24)components/ui/input.tsx (1)
Input(21-21)components/ui/color-picker.tsx (1)
ColorPicker(24-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: build-and-push
🔇 Additional comments (26)
components/app-header.tsx (1)
17-35: NewonICloudSyncwiring in header looks consistent across desktop and mobileThe new
onICloudSyncprop is threaded cleanly throughAppHeaderinto both desktop and mobileCalendarSelectorusages, and the mobile dialog correctly closes before triggering the callback. No issues from a typing or behavioral perspective.Also applies to: 37-55, 120-128, 242-261
messages/de.json (1)
158-193: Germanicloudtranslations are structurally sound and aligned with English keysThe new
icloudblock mirrors the English keys and fits existing i18n structure; strings look clear and appropriate for the described flows.messages/en.json (1)
158-193: Englishicloudtranslations cleanly cover the new sync UXThe added
icloudblock provides all needed labels, help text, and status messages, and aligns with the German translations and new UI flows.drizzle/meta/_journal.json (1)
32-39: Migration journal correctly tracks the new iCloud schema changeThe additional journal entry (
idx: 4, tag0004_modern_swarm) and version bump ensure Drizzle’s migration history is consistent with the new schema.app/api/shifts/route.ts (1)
21-35: IncludingicloudSyncIdin GET shifts response is consistent with the new schemaExposing
icloudSyncIdalongside the existing shift fields lines up with the schema and enables the UI to associate shifts with specific iCloud syncs. Just ensure any consumer types (e.g.,ShiftWithCalendar) are updated to include this optional field, which looks already planned in related types.components/shift-card.tsx (1)
52-61: LGTM! Proper protection for synced shifts.The conditional rendering prevents deletion of iCloud-synced shifts, which is the correct behavior to avoid conflicts with the sync source.
lib/types.ts (2)
2-2: LGTM! Type export extends public API surface.Adding ICloudSync to the exported types properly supports the new iCloud sync functionality.
29-29: LGTM! Optional field correctly typed.The icloudSyncId field is appropriately optional and nullable, allowing shifts to exist both with and without iCloud sync associations.
components/calendar-selector.tsx (1)
13-13: LGTM! Cloud sync button follows established patterns.The new iCloud sync button implementation is consistent with existing action buttons (password management, deletion) and properly uses conditional rendering and internationalized titles.
Also applies to: 22-22, 32-32, 67-77
components/calendar-grid.tsx (1)
206-214: LGTM! Clickable overflow indicator properly implemented.The "+N" indicator correctly uses
stopPropagationto prevent the day click handler from firing, and the optional callback pattern allows for graceful degradation when the handler isn't provided.app/api/icloud-syncs/route.ts (2)
7-33: LGTM! GET handler follows API guidelines.The handler correctly validates the required
calendarIdquery parameter and returns a 400 status for missing params, as specified in the coding guidelines.
36-69: LGTM! POST handler properly validates required fields.The handler validates all required fields (calendarId, name, icloudUrl) and includes appropriate error handling with console logging as per guidelines.
components/day-shifts-dialog.tsx (2)
26-42: LGTM! Dialog follows component guidelines.The dialog properly controls open state via props and uses locale-specific date formatting with date-fns, consistent with coding guidelines.
86-95: LGTM! Consistent deletion logic for synced shifts.The conditional rendering of the delete button matches the pattern in shift-card.tsx, correctly preventing deletion of iCloud-synced shifts.
drizzle/meta/0004_snapshot.json (1)
1-498: Database snapshot looks structurally correct.This is a generated Drizzle migration snapshot. The schema includes the new icloud_syncs table with proper foreign key relationships and cascade behavior. No manual review needed for generated files.
app/api/icloud-syncs/[id]/route.ts (1)
7-35: LGTM! Dynamic params properly awaited.All route handlers correctly await the dynamic
paramsas required by Next.js 15, and include proper error handling with console logging per the coding guidelines.Also applies to: 38-76, 79-96
app/api/icloud-syncs/[id]/sync/route.ts (2)
92-110: Potential timezone issue with time extraction.Using
toJSDate()followed bygetHours()/getMinutes()extracts time in the server's local timezone, which may differ from the event's original timezone. For timed events, consider using theICAL.Timehour/minute properties directly to preserve the intended time.if (!isAllDay) { - const startHours = startJsDate.getHours().toString().padStart(2, "0"); - const startMinutes = startJsDate - .getMinutes() - .toString() - .padStart(2, "0"); + const startHours = startDate.hour.toString().padStart(2, "0"); + const startMinutes = startDate.minute.toString().padStart(2, "0"); startTime = `${startHours}:${startMinutes}`; - const endHours = endJsDate.getHours().toString().padStart(2, "0"); - const endMinutes = endJsDate.getMinutes().toString().padStart(2, "0"); + const endHours = endDate.hour.toString().padStart(2, "0"); + const endMinutes = endDate.minute.toString().padStart(2, "0"); endTime = `${endHours}:${endMinutes}`; }
7-12: LGTM!Correctly awaits the dynamic route params per Next.js 16 breaking change requirements. The async params pattern is properly implemented.
app/page.tsx (4)
103-108: LGTM!New state variables for iCloud sync dialog and day shifts dialog are properly initialized.
163-211: LGTM!The
handleICloudSyncClickcorrectly implements the pendingAction state pattern for password-protected calendars, verifying stored passwords via/api/calendars/[id]/verify-passwordbefore opening the iCloud sync dialog. Based on learnings, this follows the established pattern.
446-457: LGTM!The
handleShowAllShiftsandhandleDeleteShiftFromDayDialogfunctions properly manage dialog state and refresh data after mutations by callingrefetchShifts().
853-874: LGTM!Dialog components are properly wired with controlled
openstate via props. TheonSyncCompletecallback correctly refreshes shifts, calendars, and incrementsstatsRefreshTriggerto update the UI after sync operations.components/icloud-sync-manage-dialog.tsx (3)
79-126: LGTM!The
handleAddSyncfunction properly validates inputs, checks for duplicate URLs, and auto-triggers a sync after creation. Error handling uses toast notifications appropriately.
128-160: LGTM!The update logic correctly handles the case where URL editing is disabled. The
formUrl.trim() || undefinedensures the original URL is preserved when editing.
245-256: LGTM!Dialog component correctly implements controlled open state via props. The gradient styling is consistent with other dialogs in the codebase.
drizzle/0004_modern_swarm.sql (1)
14-14: LGTM!The CASCADE delete on
icloud_sync_idensures that deleting an iCloud sync configuration automatically removes all associated shifts, maintaining referential integrity.
There was a problem hiding this comment.
Pull request overview
This pull request implements comprehensive iCloud calendar synchronization functionality, enabling users to import and manage shifts from multiple iCloud calendars. The feature includes full CRUD operations for sync configurations, automated event synchronization with conflict resolution, and seamless UI integration with the existing calendar management system.
Key Changes
- Database Schema Extension: Added
icloud_syncstable with cascade delete relationships and three new fields to theshiftstable (icloud_event_id,icloud_sync_id,synced_from_icloud) to track synchronized events - API Routes: Implemented complete REST API for iCloud sync management with endpoints for listing, creating, updating, deleting sync configurations, and triggering synchronization operations
- UI Components: Created
ICloudSyncManageDialogfor managing multiple sync configurations andDayShiftsDialogfor viewing all shifts on days with more than 2 events, both with full i18n support
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
package.json & package-lock.json |
Added ical.js for parsing iCalendar data and node-fetch (unused) as new dependencies |
messages/en.json & messages/de.json |
Added 33 new translation keys for iCloud sync UI elements in both English and German |
lib/types.ts |
Exported ICloudSync type and added icloudSyncId field to ShiftWithCalendar interface |
lib/db/schema.ts |
Added icloudSyncs table schema and extended shifts table with sync tracking fields |
drizzle/meta/_journal.json & drizzle/meta/0004_snapshot.json |
Migration metadata for database schema changes |
drizzle/0004_modern_swarm.sql |
SQL migration creating icloud_syncs table and adding iCloud-related fields to shifts table |
components/shift-card.tsx |
Conditionally hides delete button for synced shifts to prevent manual deletion |
components/icloud-sync-manage-dialog.tsx |
New component for managing iCloud sync configurations with add/edit/delete/sync operations |
components/day-shifts-dialog.tsx |
New component displaying all shifts for a selected day with conditional delete functionality |
components/calendar-selector.tsx |
Added iCloud sync button to calendar selector UI |
components/calendar-grid.tsx |
Made "+N more" indicator clickable to show all shifts in a dialog |
components/app-header.tsx |
Integrated iCloud sync button in both desktop and mobile header layouts |
app/page.tsx |
Added iCloud sync dialog integration with password protection flow and shift dialog management |
app/api/shifts/route.ts |
Added icloudSyncId to the shift query response |
app/api/icloud-syncs/route.ts |
New API route handling GET (list syncs) and POST (create sync) operations |
app/api/icloud-syncs/[id]/sync/route.ts |
New API route implementing the core synchronization logic with event parsing and CRUD operations |
app/api/icloud-syncs/[id]/route.ts |
New API route handling GET (fetch sync), PATCH (update sync), and DELETE (remove sync) operations |
README.md |
Updated features list to include iCloud sync functionality |
package.json
Outdated
| "motion": "^12.23.24", | ||
| "next": "16.0.3", | ||
| "next-intl": "^4.5.5", | ||
| "node-fetch": "^3.3.2", |
There was a problem hiding this comment.
The node-fetch package is added as a dependency but is never imported or used anywhere in the codebase. Next.js provides a built-in fetch API that works in both server and client contexts, making this dependency unnecessary. Consider removing it from the dependencies.
| "node-fetch": "^3.3.2", |
| import { NextResponse } from "next/server"; | ||
| import { db } from "@/lib/db"; | ||
| import { icloudSyncs, shifts } from "@/lib/db/schema"; | ||
| import { eq, and } from "drizzle-orm"; |
There was a problem hiding this comment.
The and import from drizzle-orm is imported but never used in this file. Remove unused imports to keep the code clean.
| import { eq, and } from "drizzle-orm"; | |
| import { eq } from "drizzle-orm"; |
| useEffect(() => { | ||
| if (open && calendarId) { | ||
| fetchSyncs(); | ||
| } | ||
| }, [open, calendarId]); |
There was a problem hiding this comment.
The fetchSyncs function is called in the useEffect but is not included in the dependency array. This violates the React hooks exhaustive-deps rule and could lead to stale closures. Either include fetchSyncs in the dependency array or wrap it with useCallback.
| for (const vevent of vevents) { | ||
| const event = new ICAL.Event(vevent); | ||
| const eventId = event.uid; | ||
| processedEventIds.add(eventId); | ||
|
|
||
| const startDate = event.startDate; | ||
| const endDate = event.endDate; | ||
|
|
||
| if (!startDate || !endDate) { | ||
| continue; // Skip events without dates | ||
| } | ||
|
|
||
| const isAllDay = event.startDate.isDate; | ||
|
|
||
| // Convert ICAL.Time to JavaScript Date | ||
| const startJsDate = startDate.toJSDate(); | ||
| const endJsDate = endDate.toJSDate(); | ||
|
|
||
| let startTime = "00:00"; | ||
| let endTime = "23:59"; | ||
|
|
||
| if (!isAllDay) { | ||
| const startHours = startJsDate.getHours().toString().padStart(2, "0"); | ||
| const startMinutes = startJsDate | ||
| .getMinutes() | ||
| .toString() | ||
| .padStart(2, "0"); | ||
| startTime = `${startHours}:${startMinutes}`; | ||
|
|
||
| const endHours = endJsDate.getHours().toString().padStart(2, "0"); | ||
| const endMinutes = endJsDate.getMinutes().toString().padStart(2, "0"); | ||
| endTime = `${endHours}:${endMinutes}`; | ||
| } | ||
|
|
||
| // Normalize date to midnight for storage | ||
| const normalizedDate = new Date( | ||
| startJsDate.getFullYear(), | ||
| startJsDate.getMonth(), | ||
| startJsDate.getDate() | ||
| ); | ||
|
|
||
| const shiftData = { | ||
| calendarId: icloudSync.calendarId, | ||
| date: normalizedDate, | ||
| startTime, | ||
| endTime, | ||
| title: event.summary || "Untitled Event", | ||
| color: icloudSync.color, | ||
| notes: event.description || null, | ||
| isAllDay, | ||
| isSecondary: false, | ||
| icloudEventId: eventId, | ||
| icloudSyncId: syncId, | ||
| syncedFromIcloud: true, | ||
| presetId: null, | ||
| }; | ||
|
|
||
| // Check if this event already exists | ||
| const existingShift = existingShifts.find( | ||
| (s) => s.icloudEventId === eventId | ||
| ); | ||
|
|
||
| if (existingShift) { | ||
| // Update existing shift | ||
| await db | ||
| .update(shifts) | ||
| .set({ | ||
| ...shiftData, | ||
| updatedAt: new Date(), | ||
| }) | ||
| .where(eq(shifts.id, existingShift.id)); | ||
| updatedShifts.push(eventId); | ||
| } else { | ||
| // Create new shift | ||
| await db.insert(shifts).values({ | ||
| id: crypto.randomUUID(), | ||
| ...shiftData, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| }); | ||
| newShifts.push(eventId); | ||
| } | ||
| } | ||
|
|
||
| // Delete shifts that are no longer in the iCloud calendar | ||
| const shiftsToDelete = existingShifts.filter( | ||
| (s) => s.icloudEventId && !processedEventIds.has(s.icloudEventId) | ||
| ); | ||
|
|
||
| for (const shift of shiftsToDelete) { | ||
| await db.delete(shifts).where(eq(shifts.id, shift.id)); | ||
| } |
There was a problem hiding this comment.
The sync logic performs individual database operations (INSERT/UPDATE/DELETE) in a loop, which can result in poor performance when syncing calendars with many events. Consider using batch operations or a transaction to improve performance. For example, collect all inserts/updates/deletes and execute them in batches using db.insert().values([...]) and db.delete().where(inArray(shifts.id, [...])).
| import { | ||
| CloudUpload, | ||
| Loader2, | ||
| Trash2, | ||
| RefreshCw, | ||
| Plus, | ||
| Edit2, | ||
| } from "lucide-react"; |
There was a problem hiding this comment.
Unused import CloudUpload.
Adds strict iCloud URL validation to prevent SSRF attacks. Optimizes sync logic using batch database operations for inserts, updates, and deletes. Ensures associated shift colors update with sync changes and emits events for client updates. Cleans up unused dependencies.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/api/icloud-syncs/[id]/sync/route.ts (1)
100-112: Timezone handling may cause incorrect times on servers in different timezones.The time extraction uses
getHours()andgetMinutes()on JavaScript Date objects, which returns hours/minutes in the server's local timezone. If an event is stored as "9:00 AM Eastern" and the server runs in UTC, the stored time will be "14:00" instead of "09:00".Consider using UTC methods or the ical.js native time properties for consistent behavior across different server environments.
🧹 Nitpick comments (5)
app/api/icloud-syncs/[id]/sync/route.ts (1)
31-45: Consider adding a fetch timeout.The external fetch to the iCloud calendar has no timeout configured. If the iCloud server is slow or unresponsive, this could cause the request to hang indefinitely, potentially exhausting server resources.
Apply this diff to add a timeout:
let icsData: string; try { - const response = await fetch(fetchUrl); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); + + const response = await fetch(fetchUrl, { + signal: controller.signal, + }); + clearTimeout(timeoutId); + if (!response.ok) { throw new Error(`Failed to fetch calendar: ${response.statusText}`); } icsData = await response.text(); } catch (error) { console.error("Error fetching iCloud calendar:", error); return NextResponse.json( - { error: "Failed to fetch iCloud calendar. Please check the URL." }, + { error: error instanceof Error && error.name === 'AbortError' + ? "Request timed out. The calendar server took too long to respond." + : "Failed to fetch iCloud calendar. Please check the URL." }, { status: 500 } ); }app/api/icloud-syncs/[id]/route.ts (2)
7-32: Consider extractingisValidICloudUrlto a shared module.This validation function is duplicated from
app/api/icloud-syncs/route.ts. Extracting it to a shared utility (e.g.,lib/validators.ts) would improve maintainability and ensure consistent validation logic.
136-154: Consider emitting a calendar-change event on DELETE.When an iCloud sync is deleted, the cascade delete removes associated shifts, but no event is emitted to notify connected clients. This could cause stale data to remain visible until the page is manually refreshed.
Apply this diff to emit an event:
export async function DELETE( request: Request, { params }: { params: Promise<{ id: string }> } ) { try { const { id } = await params; + // Get the sync first to access calendarId for event emission + const [syncToDelete] = await db + .select() + .from(icloudSyncs) + .where(eq(icloudSyncs.id, id)) + .limit(1); + await db.delete(icloudSyncs).where(eq(icloudSyncs.id, id)); + if (syncToDelete) { + eventEmitter.emit("calendar-change", { + type: "shift", + action: "delete", + calendarId: syncToDelete.calendarId, + data: { icloudSyncId: id, deleted: true }, + }); + } + return NextResponse.json({ success: true });components/icloud-sync-manage-dialog.tsx (2)
20-29: Consider importingPRESET_COLORSfrom a shared location.Per coding guidelines, "Use PRESET_COLORS constant array for all color picker options." Defining colors locally may lead to inconsistency if other components use different color sets. Consider moving this to
@/lib/constants.tsand importing it here.+import { PRESET_COLORS } from "@/lib/constants"; -const PRESET_COLORS = [ - { name: "Blue", value: "#3b82f6" }, - { name: "Red", value: "#ef4444" }, - { name: "Green", value: "#10b981" }, - { name: "Amber", value: "#f59e0b" }, - { name: "Violet", value: "#8b5cf6" }, - { name: "Pink", value: "#ec4899" }, - { name: "Cyan", value: "#06b6d4" }, - { name: "Orange", value: "#f97316" }, -];And in
lib/constants.ts:export const PRESET_COLORS = [ { name: "Blue", value: "#3b82f6" }, { name: "Red", value: "#ef4444" }, { name: "Green", value: "#10b981" }, { name: "Amber", value: "#f59e0b" }, { name: "Violet", value: "#8b5cf6" }, { name: "Pink", value: "#ec4899" }, { name: "Cyan", value: "#06b6d4" }, { name: "Orange", value: "#f97316" }, ];
213-215: Consider replacing nativeconfirm()with an accessible dialog.The native
window.confirm()has accessibility limitations (no keyboard focus management, inconsistent styling, blocks the main thread). Consider using a custom confirmation dialog component for better UX and accessibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
app/api/icloud-syncs/[id]/route.ts(1 hunks)app/api/icloud-syncs/[id]/sync/route.ts(1 hunks)app/api/icloud-syncs/route.ts(1 hunks)components/icloud-sync-manage-dialog.tsx(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- app/api/icloud-syncs/route.ts
🧰 Additional context used
📓 Path-based instructions (6)
app/api/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/api/**/*.ts: Always await dynamic route params in API routes and server components (Next.js 16 breaking change)
Create API route handlers for new database tables at app/api/tablename/route.ts and app/api/tablename/[id]/route.ts
Log all API errors with console.error() for debugging
Files:
app/api/icloud-syncs/[id]/sync/route.tsapp/api/icloud-syncs/[id]/route.ts
app/api/**/route.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
API GET routes must validate required query parameters and return 400 status for missing params
Files:
app/api/icloud-syncs/[id]/sync/route.tsapp/api/icloud-syncs/[id]/route.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Files:
app/api/icloud-syncs/[id]/sync/route.tsapp/api/icloud-syncs/[id]/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Files:
app/api/icloud-syncs/[id]/sync/route.tsapp/api/icloud-syncs/[id]/route.tscomponents/icloud-sync-manage-dialog.tsx
components/**/*-dialog.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*-dialog.tsx: Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Files:
components/icloud-sync-manage-dialog.tsx
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PRESET_COLORS constant array for all color picker options
Files:
components/icloud-sync-manage-dialog.tsx
🧠 Learnings (10)
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to lib/db/schema.ts : Drizzle ORM timestamp fields must use { mode: "timestamp" } and are stored as integers, auto-converted to Date objects
Applied to files:
app/api/icloud-syncs/[id]/sync/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Applied to files:
app/api/icloud-syncs/[id]/sync/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to lib/db/schema.ts : Use cascade delete relationships in database schema (onDelete: "cascade") for dependent tables
Applied to files:
app/api/icloud-syncs/[id]/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Applied to files:
app/api/icloud-syncs/[id]/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Applied to files:
components/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Applied to files:
components/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Applied to files:
components/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Applied to files:
components/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*.tsx : Use PRESET_COLORS constant array for all color picker options
Applied to files:
components/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to **/*.{ts,tsx} : Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Applied to files:
components/icloud-sync-manage-dialog.tsx
🧬 Code graph analysis (2)
app/api/icloud-syncs/[id]/sync/route.ts (4)
app/api/icloud-syncs/route.ts (1)
POST(63-107)app/api/shifts/route.ts (1)
POST(72-133)lib/db/index.ts (1)
db(19-19)lib/db/schema.ts (2)
icloudSyncs(19-36)shifts(38-71)
components/icloud-sync-manage-dialog.tsx (7)
lib/types.ts (1)
ICloudSync(2-2)lib/db/schema.ts (1)
ICloudSync(116-116)components/ui/dialog.tsx (5)
Dialog(139-139)DialogContent(141-141)DialogHeader(144-144)DialogTitle(147-147)DialogDescription(142-142)components/ui/button.tsx (1)
Button(60-60)components/ui/label.tsx (1)
Label(24-24)components/ui/input.tsx (1)
Input(21-21)components/ui/color-picker.tsx (1)
ColorPicker(24-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-push
🔇 Additional comments (11)
app/api/icloud-syncs/[id]/sync/route.ts (4)
1-5: Imports are correctly used.The
eqandinArrayimports are both utilized in the code (eqat lines 18, 66, 177, 192 andinArrayat line 183). The past review comment about unusedandimport appears to have been addressed.
7-12: Params correctly awaited.Properly awaits the dynamic route params as required by Next.js 16.
166-194: Transaction pattern is appropriate for synchronous driver.The code correctly uses the synchronous transaction pattern with
.run()calls, which is the expected approach for better-sqlite3 with Drizzle ORM. The transaction ensures atomicity for batch inserts, updates, and deletes.
196-211: Response and error handling are well-implemented.The endpoint returns useful statistics (created, updated, deleted counts) and properly logs errors with
console.error()per coding guidelines.app/api/icloud-syncs/[id]/route.ts (3)
1-5: Imports are correctly used.The
shiftsimport is used at lines 109-115 for updating shift colors when the sync color changes. The past review comment about unused imports is no longer applicable.
35-63: GET handler is well-implemented.Correctly awaits params, handles the 404 case, and logs errors per guidelines.
107-124: Color propagation to shifts is properly handled.The PATCH handler correctly updates all associated shifts when the sync color changes and emits a calendar-change event to notify clients. This addresses the previous review comment.
components/icloud-sync-manage-dialog.tsx (4)
57-74:fetchSyncscorrectly wrapped inuseCallback.The function is properly memoized with
calendarIdas a dependency, addressing the previous review comment about missing dependencies in the useEffect.
76-92: State reset on dialog close is properly implemented.The useEffect now resets all internal state when the dialog closes, addressing the previous review comment. The dependency array correctly includes
fetchSyncs.
376-386: ColorPicker correctly receivespresetColorsprop.The
presetColors={PRESET_COLORS}prop is now passed to the ColorPicker component, addressing the previous review comment about following coding guidelines.
359-374: URL field disabled during edit is a reasonable security measure.Disabling the URL field when editing prevents users from changing the iCloud URL to a potentially malicious endpoint after initial validation. If URL changes are needed, users can delete and recreate the sync.
Moves iCloud URL validation and recurring event handling into shared utilities for improved maintainability and accuracy. Centralizes color preset definitions to prevent duplication and ensure consistency across dialogs and forms. Enhances sync to handle recurring and multi-day events more robustly, tracks stats for inserted, updated, and deleted shifts.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
app/api/icloud-syncs/[id]/sync/route.ts (2)
109-114: Timezone handling may cause incorrect times.The
toJSDate()conversion followed bygetHours()/getMinutes()insplitMultiDayEventoperates in the server's local timezone. This could cause incorrect times if iCloud events contain timezone information different from the server's timezone.
35-46: Add timeout and response size limits to protect against resource exhaustion.The
fetchcall lacks safeguards against malicious or slow-responding URLs. WhileisValidICloudUrlvalidates the domain, an attacker with access could still cause resource exhaustion via large responses or slow connections.+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); // 30s timeout + try { - const response = await fetch(fetchUrl); + const response = await fetch(fetchUrl, { signal: controller.signal }); + clearTimeout(timeoutId); + if (!response.ok) { throw new Error(`Failed to fetch calendar: ${response.statusText}`); } + + const contentLength = response.headers.get("content-length"); + if (contentLength && parseInt(contentLength) > 10 * 1024 * 1024) { + throw new Error("Calendar file too large"); + } + icsData = await response.text(); } catch (error) { + clearTimeout(timeoutId); console.error("Error fetching iCloud calendar:", error);lib/icloud-utils.ts (1)
164-171: Timezone-sensitive time extraction.Using
getHours()andgetMinutes()on JavaScriptDateobjects operates in the server's local timezone. If iCloud events contain timezone information different from the server, extracted times may be incorrect. Consider using iCal.js timezone-aware properties or normalizing to UTC.
🧹 Nitpick comments (3)
components/preset-edit-dialog.tsx (1)
55-77: Consider resetting state when dialog closes.Per coding guidelines, dialog components should reset internal state when
openchanges tofalse. Currently, theuseEffectonly handles state initialization whenopenistrue. While this works because state is reset on next open, explicitly clearing state on close is more defensive.useEffect(() => { if (open && preset && !isCreating) { setFormData({ title: preset.title, startTime: preset.startTime, endTime: preset.endTime, color: preset.color, notes: preset.notes || "", isSecondary: preset.isSecondary || false, isAllDay: preset.isAllDay || false, }); } else if (open && isCreating) { setFormData({ title: "", startTime: "09:00", endTime: "17:00", color: PRESET_COLORS[0].value, notes: "", isSecondary: false, isAllDay: false, }); + } else if (!open) { + // Reset state when dialog closes + setFormData({ + title: "", + startTime: "09:00", + endTime: "17:00", + color: PRESET_COLORS[0].value, + notes: "", + isSecondary: false, + isAllDay: false, + }); } }, [open, preset, isCreating]);app/api/icloud-syncs/route.ts (1)
60-71: Consider adding duplicate URL check on the backend.The POST endpoint doesn't prevent creating duplicate iCloud syncs with the same URL for the same calendar. While client-side validation may exist, the backend should enforce uniqueness to prevent race conditions or direct API calls bypassing the UI.
+ // Check for duplicate URL within the same calendar + const existingSync = await db + .select() + .from(icloudSyncs) + .where(eq(icloudSyncs.calendarId, calendarId)) + .where(eq(icloudSyncs.icloudUrl, icloudUrl)) + .limit(1); + + if (existingSync.length > 0) { + return NextResponse.json( + { error: "An iCloud sync with this URL already exists for this calendar" }, + { status: 409 } + ); + } + const [icloudSync] = await db .insert(icloudSyncs) .values({Alternatively, add a unique constraint on
(calendar_id, icloud_url)in the database schema.components/icloud-sync-manage-dialog.tsx (1)
378-394: Simplify cancel button logic.The cancel button's else branch (lines 385-388) manually resets form state, but this logic is already encapsulated in the
cancelEdit()function. Consider consolidating this for better maintainability.Apply this diff to simplify:
- <Button - variant="outline" - onClick={() => { - if (editingSync) { - cancelEdit(); - } else { - setShowAddForm(false); - setFormName(""); - setFormUrl(""); - setFormColor("#3b82f6"); - } - }} - disabled={isLoading} - > + <Button + variant="outline" + onClick={() => { + if (editingSync) { + cancelEdit(); + } else { + setShowAddForm(false); + cancelEdit(); // Reuse existing reset logic + } + }} + disabled={isLoading} + >Or create a dedicated
cancelAdd()helper function similar tocancelEdit()for symmetry.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/api/icloud-syncs/[id]/route.ts(1 hunks)app/api/icloud-syncs/[id]/sync/route.ts(1 hunks)app/api/icloud-syncs/route.ts(1 hunks)components/calendar-dialog.tsx(3 hunks)components/icloud-sync-manage-dialog.tsx(1 hunks)components/preset-edit-dialog.tsx(1 hunks)components/shift-form-fields.tsx(1 hunks)lib/constants.ts(1 hunks)lib/icloud-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/icloud-syncs/[id]/route.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Files:
lib/constants.tslib/icloud-utils.tsapp/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/sync/route.tscomponents/shift-form-fields.tsxcomponents/preset-edit-dialog.tsxcomponents/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
app/api/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/api/**/*.ts: Always await dynamic route params in API routes and server components (Next.js 16 breaking change)
Create API route handlers for new database tables at app/api/tablename/route.ts and app/api/tablename/[id]/route.ts
Log all API errors with console.error() for debugging
Files:
app/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/sync/route.ts
app/api/**/route.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
API GET routes must validate required query parameters and return 400 status for missing params
Files:
app/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/sync/route.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Files:
app/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/sync/route.ts
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PRESET_COLORS constant array for all color picker options
Files:
components/shift-form-fields.tsxcomponents/preset-edit-dialog.tsxcomponents/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
components/**/*-dialog.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*-dialog.tsx: Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Files:
components/preset-edit-dialog.tsxcomponents/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
components/**/calendar*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Files:
components/calendar-dialog.tsx
🧠 Learnings (11)
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*.tsx : Use PRESET_COLORS constant array for all color picker options
Applied to files:
lib/constants.tscomponents/shift-form-fields.tsxcomponents/preset-edit-dialog.tsxcomponents/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to **/*.{ts,tsx} : Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Applied to files:
lib/constants.tscomponents/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Applied to files:
lib/constants.tscomponents/shift-form-fields.tsxcomponents/preset-edit-dialog.tsxcomponents/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Applied to files:
app/api/icloud-syncs/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/api/**/*.ts : Create API route handlers for new database tables at app/api/tablename/route.ts and app/api/tablename/[id]/route.ts
Applied to files:
app/api/icloud-syncs/route.tsapp/api/icloud-syncs/[id]/sync/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Applied to files:
app/api/icloud-syncs/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to lib/db/schema.ts : Drizzle ORM timestamp fields must use { mode: "timestamp" } and are stored as integers, auto-converted to Date objects
Applied to files:
app/api/icloud-syncs/[id]/sync/route.ts
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Applied to files:
app/api/icloud-syncs/[id]/sync/route.tscomponents/preset-edit-dialog.tsxcomponents/calendar-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Applied to files:
components/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Applied to files:
components/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Applied to files:
components/calendar-dialog.tsxcomponents/icloud-sync-manage-dialog.tsx
🧬 Code graph analysis (2)
app/api/icloud-syncs/[id]/sync/route.ts (4)
app/api/icloud-syncs/route.ts (1)
POST(37-81)lib/db/index.ts (1)
db(19-19)lib/db/schema.ts (2)
icloudSyncs(19-36)shifts(38-71)lib/icloud-utils.ts (2)
expandRecurringEvents(41-116)splitMultiDayEvent(125-219)
components/calendar-dialog.tsx (1)
lib/constants.ts (1)
PRESET_COLORS(8-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-push
🔇 Additional comments (21)
components/preset-edit-dialog.tsx (1)
17-17: Good: Centralized PRESET_COLORS import.The import from
@/lib/constantsaligns with the coding guidelines to use a shared PRESET_COLORS constant array for all color picker options.components/calendar-dialog.tsx (1)
17-17: LGTM! PRESET_COLORS refactored correctly.The changes properly:
- Import
PRESET_COLORSfrom the centralized constants module- Initialize and reset
selectedColorusingPRESET_COLORS[0].value- Iterate with
colorObjaccessing.valueand.nameproperties- Apply hex colors with opacity suffixes (e.g.,
${colorObj.value}40) per coding guidelinesAlso applies to: 32-32, 45-45, 89-124
app/api/icloud-syncs/[id]/sync/route.ts (1)
172-209: Transaction handling looks correct.The transaction result is properly captured and returned. The synchronous transaction with better-sqlite3 is correctly structured with batch inserts, individual updates (SQLite limitation), batch deletes, and metadata update.
lib/icloud-utils.ts (2)
12-32: Good SSRF mitigation for iCloud URLs.The validation correctly restricts URLs to
webcal://orhttps://protocols and ensures the hostname is from the iCloud domain usingendsWith(".icloud.com")which prevents subdomain spoofing attacks.
41-116: Well-implemented recurring event expansion.The function correctly:
- Uses iCal.js iterator API to expand recurring events
- Filters occurrences within the specified time window
- Calculates end times using event duration
- Returns
recurrenceIdfor unique identification of each occurrenceapp/api/icloud-syncs/route.ts (2)
5-5: Good: Centralized URL validation utility.Using
isValidICloudUrlfrom@/lib/icloud-utilsaddresses the previous DRY concern about duplicated validation logic.
8-34: GET handler follows API route guidelines.Properly validates the required
calendarIdquery parameter and returns 400 status for missing params. Errors are logged withconsole.error.lib/constants.ts (1)
1-17: LGTM! Good centralization of color presets.This new constants module properly exports
PRESET_COLORSwith hex values, enabling consistent color options across all color picker components. Aligns with coding guidelines to use a shared constant array.components/shift-form-fields.tsx (2)
9-9: Good: Uses centralized PRESET_COLORS.Correctly imports the color presets from
@/lib/constantsper coding guidelines.
145-150: ColorPicker integration looks correct.The
PRESET_COLORSis properly passed to theColorPickercomponent, and the default color fallback (#3b82f6) matches the first preset color.components/icloud-sync-manage-dialog.tsx (11)
1-26: LGTM! Imports and interface are well-structured.All necessary dependencies are imported, including
PRESET_COLORSfrom the shared constants file. The interface properly defines the dialog props following the component guidelines.
28-46: LGTM! State management is well-organized.The component uses separate loading states for different operations, which enables proper per-item loading indicators and prevents race conditions.
47-64: LGTM! Proper use of useCallback.The function is correctly memoized with
calendarIdin the dependency array, which was properly addressed from previous feedback.
67-82: LGTM! Dialog state management follows guidelines.The effect properly loads syncs when the dialog opens and comprehensively resets all internal state when it closes, which addresses the previous review feedback. The
fetchSyncscallback is correctly included in the dependency array.
133-166: LGTM! Update logic is correct.The function properly updates the sync and calls
onSyncCompleteto refresh shifts when the color is updated, with a helpful comment explaining the reason.
168-201: LGTM! Sync operation with good user feedback.The manual sync function provides excellent user feedback by displaying detailed statistics (created, updated, deleted) in the success toast, which helps users understand what changed.
203-226: LGTM! Delete operation works correctly.The function properly confirms deletion and updates the UI. The native browser
confirm()is functional, though a custom dialog component would provide a more polished UX if you want to enhance this in the future.
228-249: LGTM! Helper functions for form state management.These functions cleanly encapsulate the logic for transitioning between different form modes (add, edit, view).
251-328: LGTM! Dialog structure and sync list UI are well-implemented.The dialog properly controls open state via props and displays syncs with color-coded indicators. The per-item action buttons have appropriate loading states and disabled conditions.
366-376: LGTM! ColorPicker correctly uses PRESET_COLORS.The ColorPicker component properly receives the
PRESET_COLORSconstant via thepresetColorsprop, which addresses previous review feedback. As per coding guidelines.
411-436: LGTM! Add button and instructional guidance are user-friendly.The conditional add button and instruction panel provide clear guidance for users on how to set up iCloud syncs.
Prevents invalid iCloud URLs from being added by validating the format before submission. Improves user feedback with localized error messages for incorrect URL input.
1 participant
Summary by CodeRabbit
New Features
UI Updates
Localization
✏️ Tip: You can customize this high-level summary in your review settings.