ICS export option - #732
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds configurable frontend ICS export, generates ChangesICS Export
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant CalendarView
participant Ics_Export
participant Calendar
Visitor->>CalendarView: Select ICS export
CalendarView->>Ics_Export: Request export URL
Ics_Export->>Calendar: Validate setting and load events
Calendar-->>Ics_Export: Calendar event groups
Ics_Export-->>Visitor: Download .ics file
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@includes/ics-export.php`:
- Around line 275-279: Update the line-folding loop to use mb_strcut() instead
of substr() when extracting each 75-byte segment, preserving the existing
CRLF-and-space folding format and remaining-line handling while ensuring
multibyte characters are never split.
- Around line 195-200: Update the event end-date calculation in the
`$event->end_dt` branch to normalize `end_dt` with `startOfDay()->addDay()`
before formatting it as `Ymd`. Remove the brittle `addSeconds(59)` logic while
preserving the existing fallback calculation for events without an end date.
- Around line 51-54: Update the calendar validation around
get_post($calendar_id) to allow exports only when the calendar is published or
the current user is authorized to access its private or draft status. Preserve
the existing invalid-post and wrong-post-type rejection, and terminate
unauthorized requests with the established 404 response without exposing
calendar events.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 186f69c9-9df8-46e3-82e8-c50d967d3f61
📒 Files selected for processing (7)
assets/css/default-calendar-grid.cssincludes/admin/metaboxes/settings.phpincludes/calendars/views/default-calendar-grid.phpincludes/calendars/views/default-calendar-list.phpincludes/ics-export.phpincludes/main.phpreadme.txt
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
assets/js/default-calendar.js (1)
434-439: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake
Copied!translatable —assets/js/default-calendar.jshardcodes the copy feedback label; pass it through the localized script strings so non-English sites can translate it.🤖 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 `@assets/js/default-calendar.js` around lines 434 - 439, Update showCopied in the default calendar script to obtain the “Copied!” feedback text from the existing localized script strings instead of hardcoding it, while preserving the timeout that restores originalLabel. Add or reuse the appropriate localization key and ensure the localized value is displayed through $label.text.
🤖 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 `@assets/css/default-calendar-grid.css`:
- Around line 610-617: Update the stroke declaration in .simcal-calendar-actions
.simcal-calendar-action-icon to use the lowercase currentcolor keyword required
by stylelint, leaving the surrounding icon styles unchanged.
- Around line 558-608: Update the shared focus styling for
.simcal-calendar-actions links and buttons to provide a clearly visible keyboard
focus indicator, replacing the outline: none !important rule with an accessible
focus treatment while preserving the existing hover appearance and other button
styling.
In `@assets/js/default-calendar.js`:
- Around line 454-459: Update copyWithTextarea so the temporary textarea is
visually hidden/offscreen before appending it, preventing flashes and scroll
jumps, then explicitly focus the created textarea before calling select().
Preserve the existing execCommand('copy') and cleanup behavior.
---
Nitpick comments:
In `@assets/js/default-calendar.js`:
- Around line 434-439: Update showCopied in the default calendar script to
obtain the “Copied!” feedback text from the existing localized script strings
instead of hardcoding it, while preserving the timeout that restores
originalLabel. Add or reuse the appropriate localization key and ensure the
localized value is displayed through $label.text.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05ee2bed-8274-4252-b072-4b39e16757ea
📒 Files selected for processing (5)
assets/css/default-calendar-grid.cssassets/js/default-calendar.jsincludes/calendars/views/default-calendar-grid.phpincludes/calendars/views/default-calendar-list.phpincludes/ics-export.php
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/calendars/views/default-calendar-grid.php
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
includes/ics-export.php (3)
269-273: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not collapse ID-less events that share a start time.
When both
uidandical_idare empty, the deduplication key becomes-{$event->start}. Two distinct events at the same start time are therefore silently omitted, even thoughbuild_vevent()has a fallback identity based on title/start/end at Lines 300-303. Use the same fallback or another stable per-event identifier.🤖 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 `@includes/ics-export.php` around lines 269 - 273, Update the deduplication key logic near $dedupe_key so events without both uid and ical_id use the same stable fallback identity as build_vevent(), including title, start, and end, instead of relying only on the shared start time. Preserve existing uid and ical_id-based deduplication for identified events.
322-326: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not emit
DTENDequal toDTSTART.When
end_utc/endis missing, this path emits an explicit zero-lengthVEVENT, which is invalid. OmitDTENDfor an event with no valid end, or derive a positive duration from the event model.🤖 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 `@includes/ics-export.php` around lines 322 - 326, Update the event export logic around $end_utc so missing or invalid end values do not fall back to $start_utc and emit a zero-length DTEND. Omit the DTEND line when no valid end exists, or derive and emit a strictly later end time from the event model; preserve DTSTART generation.Source: MCP tools
347-348: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse URI-specific escaping for the
URLproperty.
URLtakes a URI value, and RFC 5545 does not apply the TEXT backslash escaping rules to URI values.escape_text()rewrites commas, semicolons, and backslashes inside the URL, so links likehttps://example.com/a,b?x=1are exported ashttps://example.com/a\,b?x=1. Sanitize the link without adding backslash escapes before using it inURL:.🤖 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 `@includes/ics-export.php` around lines 347 - 348, Update the URL export in the event serialization block to use URI-specific sanitization instead of escape_text(), preserving commas, semicolons, and backslashes as valid URI content. Keep the existing empty-link guard and URL: prefix unchanged.Source: MCP tools
🧹 Nitpick comments (1)
includes/ics-export.php (1)
275-275: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid repeatedly copying the accumulated ICS payload.
array_merge($lines, ...)copies the growing$linesarray for every event, which becomes quadratic for large calendars. Append each VEVENT line directly instead.Proposed refactor
- $lines = array_merge($lines, $this->build_vevent($event)); + foreach ($this->build_vevent($event) as $vevent_line) { + $lines[] = $vevent_line; + }🤖 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 `@includes/ics-export.php` at line 275, Update the event loop around build_vevent to append each generated VEVENT line directly to the existing $lines array instead of repeatedly calling array_merge, preserving the same output order and contents without copying the accumulated payload.
🤖 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.
Outside diff comments:
In `@includes/ics-export.php`:
- Around line 269-273: Update the deduplication key logic near $dedupe_key so
events without both uid and ical_id use the same stable fallback identity as
build_vevent(), including title, start, and end, instead of relying only on the
shared start time. Preserve existing uid and ical_id-based deduplication for
identified events.
- Around line 322-326: Update the event export logic around $end_utc so missing
or invalid end values do not fall back to $start_utc and emit a zero-length
DTEND. Omit the DTEND line when no valid end exists, or derive and emit a
strictly later end time from the event model; preserve DTSTART generation.
- Around line 347-348: Update the URL export in the event serialization block to
use URI-specific sanitization instead of escape_text(), preserving commas,
semicolons, and backslashes as valid URI content. Keep the existing empty-link
guard and URL: prefix unchanged.
---
Nitpick comments:
In `@includes/ics-export.php`:
- Line 275: Update the event loop around build_vevent to append each generated
VEVENT line directly to the existing $lines array instead of repeatedly calling
array_merge, preserving the same output order and contents without copying the
accumulated payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e4466c2-0be5-4887-8afb-f7d988e46e56
📒 Files selected for processing (3)
includes/functions/shared.phpincludes/ics-export.phpreadme.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- readme.txt
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
includes/ics-export.php (1)
246-295: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAnchor recurring master VEVENTs to the original series start.
When repeated with expanded recurring events, Google returns occurrences ordered by
startTimein the requested time window, and repeated queries may not return the first series occurrence.build_ics()currently emits oneRRULEper series using whichever event is encountered first, whilefetch_recurrence_map()only stores rule lines without the master’sgetStart(). For interval-based or finiteCOUNTrules, settingDTSTARTon a later occurrence can export a recurrence set that does not match the original series. Persist the master:start time in the recurrence mapping and use it to construct recurring VEVENTs.🤖 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 `@includes/ics-export.php` around lines 246 - 295, Update fetch_recurrence_map() to persist each recurring series’ original master start time alongside its rule lines, rather than storing only the RRULE. In build_ics(), resolve that stored master start for each series and use it when constructing the recurring VEVENT through build_vevent(), while preserving the existing one-master-per-series deduplication.
🧹 Nitpick comments (1)
includes/feeds/google.php (1)
633-654: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider skipping
fetch_recurrence_map()when no expanded instance actually needs it.The extra master-events request runs on every cache refresh whenever
singleEventsis set, even for calendars with zero recurring events. A quick scan of$response->getItems()for any item withgetRecurringEventId()set and emptygetRecurrence()before callingfetch_recurrence_map()would avoid the unnecessary API round-trip (and quota usage) in the common case.🤖 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 `@includes/feeds/google.php` around lines 633 - 654, Before invoking fetch_recurrence_map() in the singleEvents branch, scan $response->getItems() for at least one expanded instance with a recurring event ID and no recurrence data. Only populate recurrence_map when such an item exists; preserve the existing behavior and return structure otherwise.
🤖 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 `@includes/ics-export.php`:
- Around line 59-62: Update the guard around get_post($calendar_id) to first
verify that $post exists and is the expected calendar post type, returning the
existing not-found 404 response for invalid or missing calendars; only
dereference post_status and apply the read_post permission check after that
validation.
- Around line 274-291: Update the recurring-event deduplication block around
get_event_rrule() so events missing both ical_id and uid fall back to the same
UID strategy used by build_vevent(), rather than being skipped when series_key
is empty. Preserve duplicate suppression for valid series keys and ensure
recurring events without either identifier are still exported.
---
Outside diff comments:
In `@includes/ics-export.php`:
- Around line 246-295: Update fetch_recurrence_map() to persist each recurring
series’ original master start time alongside its rule lines, rather than storing
only the RRULE. In build_ics(), resolve that stored master start for each series
and use it when constructing the recurring VEVENT through build_vevent(), while
preserving the existing one-master-per-series deduplication.
---
Nitpick comments:
In `@includes/feeds/google.php`:
- Around line 633-654: Before invoking fetch_recurrence_map() in the
singleEvents branch, scan $response->getItems() for at least one expanded
instance with a recurring event ID and no recurrence data. Only populate
recurrence_map when such an item exists; preserve the existing behavior and
return structure otherwise.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 147ca4b3-5a99-4153-94b9-508ca593dc59
📒 Files selected for processing (6)
assets/css/admin.cssassets/css/default-calendar-grid.cssassets/js/default-calendar.jsincludes/admin/metaboxes/settings.phpincludes/feeds/google.phpincludes/ics-export.php
🚧 Files skipped from review as they are similar to previous changes (3)
- assets/js/default-calendar.js
- includes/admin/metaboxes/settings.php
- assets/css/default-calendar-grid.css
2 participants
Description: I have added ICS Export button similar to print calendar button user can check this from appearance and button will display below the calendar in grid/list view by pressing button user can download the ICS file. Also fix the print calendar issue when we have multiple calendar on same page.
Clickup: https://app.clickup.com/t/1867958/86d3e1phg
https://app.clickup.com/t/1867958/86d3uxq5y
After: https://drive.google.com/file/d/1ZNIDXfrQaRbe_DZN-E_2Lsi454A3Nisc/view?usp=drivesdk
Summary by CodeRabbit
Summary by CodeRabbit