ICS export option#732
Conversation
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 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
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
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
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.
Clickup: https://app.clickup.com/t/1867958/86d3e1phg
After: https://drive.google.com/file/d/1ZNIDXfrQaRbe_DZN-E_2Lsi454A3Nisc/view?usp=drivesdk
Summary by CodeRabbit
Summary by CodeRabbit