Conversation
Adds support for importing VTODO (tasks/todos) from external iCal feeds as shifts. Updates ICS validation to accept VTODOs, introduces a VTODO processing routine to convert task properties into shift data, and integrates tasks into the existing sync flow with sync-window filtering, fingerprint-based deduplication, and conditional update/insert logic. Also includes task counts in sync metrics to surface imported todo activity.
|
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. 📝 WalkthroughWalkthroughAdds VTODO (task) support to external calendar sync: parses VTODOs, normalizes them via Changes
Sequence DiagramsequenceDiagram
participant API as Sync API Route
participant ICS as ICS Parser
participant Utils as processTodoToShift
participant DB as Database
API->>ICS: Parse ICS -> extract VTODOs
ICS-->>API: List of VTODO components
loop For each VTODO
API->>Utils: processTodoToShift(vtodo)
Utils-->>API: shiftData or null
alt shiftData returned
API->>DB: Query shift by fingerprint
DB-->>API: existingShift or none
alt existing & needsUpdate
API->>DB: Update shift
else not existing
API->>DB: Insert new shift (batch)
end
else invalid/null
API-->>API: Skip VTODO
end
end
API->>DB: Delete obsolete shifts (exclude processed fingerprints)
DB-->>API: Commit/ack
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds support for importing VTODO (tasks/todos) from external iCal feeds, converting them to shifts alongside existing VEVENT support. VTODOs are processed with due/start date handling, integrated into the sync flow with fingerprint-based deduplication, and included in sync metrics.
Changes:
- Updates ICS validation to accept calendars containing only VTODOs
- Adds
processTodoToShift()function to convert VTODO components to shift data - Integrates VTODO processing into the sync route with window filtering and deduplication
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/external-calendar-utils.ts | Adds VTODO validation in isValidICSContent() and new processTodoToShift() function to convert tasks to shifts |
| app/api/external-syncs/[id]/sync/route.ts | Retrieves VTODOs from calendar, processes them through sync window filtering and fingerprint deduplication, adds totalTodos to metrics |
| if (!isAllDay && dueDate) { | ||
| // If there's a specific due time, show it as ending at that time | ||
| const hours = jsDate.getHours().toString().padStart(2, "0"); | ||
| const minutes = jsDate.getMinutes().toString().padStart(2, "0"); | ||
| endTime = `${hours}:${minutes}`; | ||
| } else if (!isAllDay && startDate) { | ||
| // If there's a start time but no due time, show it starting at that time | ||
| const hours = jsDate.getHours().toString().padStart(2, "0"); | ||
| const minutes = jsDate.getMinutes().toString().padStart(2, "0"); | ||
| startTime = `${hours}:${minutes}`; | ||
| endTime = "23:59"; |
There was a problem hiding this comment.
The logic doesn't handle VTODOs that have both DTSTART and DUE dates. When both are present, taskDate is set to dueDate (line 437), so jsDate contains the due date. In the condition at line 452, if a due time exists and it's not all-day, the code extracts hours/minutes from jsDate (the due date) for the endTime, but startTime remains '00:00'. However, if the VTODO has both start and due dates, you should use the start date for startTime and the due date for endTime, not default startTime to '00:00'. This creates shifts that always start at midnight even when a specific start time is provided.
| if (!isAllDay && dueDate) { | |
| // If there's a specific due time, show it as ending at that time | |
| const hours = jsDate.getHours().toString().padStart(2, "0"); | |
| const minutes = jsDate.getMinutes().toString().padStart(2, "0"); | |
| endTime = `${hours}:${minutes}`; | |
| } else if (!isAllDay && startDate) { | |
| // If there's a start time but no due time, show it starting at that time | |
| const hours = jsDate.getHours().toString().padStart(2, "0"); | |
| const minutes = jsDate.getMinutes().toString().padStart(2, "0"); | |
| startTime = `${hours}:${minutes}`; | |
| endTime = "23:59"; | |
| if (!isAllDay) { | |
| if (startDate && dueDate) { | |
| // When both start and due are present, use start for startTime and due for endTime | |
| const startJsDate = startDate.toJSDate(); | |
| const startHours = startJsDate.getHours().toString().padStart(2, "0"); | |
| const startMinutes = startJsDate.getMinutes().toString().padStart(2, "0"); | |
| startTime = `${startHours}:${startMinutes}`; | |
| const dueJsDate = dueDate.toJSDate(); | |
| const endHours = dueJsDate.getHours().toString().padStart(2, "0"); | |
| const endMinutes = dueJsDate.getMinutes().toString().padStart(2, "0"); | |
| endTime = `${endHours}:${endMinutes}`; | |
| } else if (dueDate) { | |
| // If there's a specific due time but no start time, show it as ending at that time | |
| const dueJsDate = dueDate.toJSDate(); | |
| const hours = dueJsDate.getHours().toString().padStart(2, "0"); | |
| const minutes = dueJsDate.getMinutes().toString().padStart(2, "0"); | |
| endTime = `${hours}:${minutes}`; | |
| } else if (startDate) { | |
| // If there's a start time but no due time, show it starting at that time | |
| const startJsDate = startDate.toJSDate(); | |
| const hours = startJsDate.getHours().toString().padStart(2, "0"); | |
| const minutes = startJsDate.getMinutes().toString().padStart(2, "0"); | |
| startTime = `${hours}:${minutes}`; | |
| endTime = "23:59"; | |
| } |
| * @param vtodo - The VTODO component from iCal.js | ||
| * @returns Shift data object or null if invalid | ||
| */ | ||
| export function processTodoToShift(vtodo: ICAL.Component): { |
There was a problem hiding this comment.
VTODOs can have recurrence rules (RRULE) just like VEVENTs, but processTodoToShift() doesn't handle recurring tasks. The existing expandRecurringEvents() function works with VEVENT components specifically. Consider whether recurring VTODOs should be supported, and if so, either extend expandRecurringEvents() to work with VTODOs or create similar expansion logic for tasks. Without this, recurring tasks will only appear once instead of generating multiple shift occurrences.
1 participant
Adds support for importing VTODO (tasks/todos) from external iCal feeds as shifts. Updates ICS validation to accept VTODOs, introduces a VTODO processing routine to convert task properties into shift data, and integrates tasks into the existing sync flow with sync-window filtering, fingerprint-based deduplication, and conditional update/insert logic. Also includes task counts in sync metrics to surface imported todo activity.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.