Add [if-event] conditional, [link url], grouped-calendar priority + sortable admin UI#690
Add [if-event] conditional, [link url], grouped-calendar priority + sortable admin UI#690rosinghal wants to merge 4 commits into
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 50 minutes and 21 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR implements calendar priority ordering for grouped calendars. It adds a ChangesCalendar Priority Ordering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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/calendars/views/default-calendar-grid.php`:
- Around line 310-329: The primary sort key in local_key() currently formats
$event->start_dt in the event's timezone which can yield different string keys
for the same instant across timezones; change local_key() to return a
timezone-normalized epoch value (e.g. convert $event->start_dt to UTC and return
its UNIX timestamp or microsecond epoch) so comparisons in cmp() use the same
instant for all events; keep the existing fallback for non-Carbon $event->start
but convert it to a normalized epoch too, and ensure the returned value is
comparable (int or numeric string) so the tie-breaker logic (calendar_priority
and title) can run correctly.
In `@includes/calendars/views/default-calendar-list.php`:
- Around line 498-517: The primary sort key function local_key currently formats
start_dt in local time which can split identical instants across timezones and
bypass calendar_priority; change local_key to return a timezone-canonical
instant (e.g. UTC timestamp or Unix epoch) for start_dt instead of a
local-formatted string, and ensure the fallback for $event->start returns the
same canonical representation (numeric/string) so cmp's comparisons use the true
instant; update local_key and keep cmp's tie handling (calendar_priority and
title) unchanged.
In `@includes/events/event-builder.php`:
- Around line 584-603: The code block handling the 'if-event' shortcode (using
matches_event_id, $event->uid and calendar->get_event_html) has formatting drift
flagged by CI; run your project's Prettier formatter (e.g., prettier --write)
over this file to normalize spacing and line breaks, then re-run lint/CI and
commit the formatted changes; also apply the same formatting pass to the other
affected region around the logic that spans the 1251-1275 lines to keep the file
consistent.
🪄 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: 02e3b130-9e34-4f39-8575-c2a56e9be847
📒 Files selected for processing (7)
assets/js/admin.jsincludes/calendars/views/default-calendar-grid.phpincludes/calendars/views/default-calendar-list.phpincludes/events/event-builder.phpincludes/events/event.phpincludes/feeds/admin/grouped-calendars-admin.phpincludes/feeds/grouped-calendars.php
| private static function local_key($event) | ||
| { | ||
| return $event->start_dt instanceof Carbon ? $event->start_dt->format('YmdHis') : (string) $event->start; | ||
| } | ||
|
|
||
| private static function cmp($a, $b) | ||
| { | ||
| if ($a->start == $b->start) { | ||
| $a_local = self::local_key($a); | ||
| $b_local = self::local_key($b); | ||
| if ($a_local === $b_local) { | ||
| if ($a->calendar_priority != $b->calendar_priority) { | ||
| return $a->calendar_priority < $b->calendar_priority ? -1 : 1; | ||
| } | ||
| if ($a->title == $b->title) { | ||
| return 0; | ||
| } | ||
| return $a->title < $b->title ? -1 : 1; | ||
| } else { | ||
| return $a->start < $b->start ? -1 : 1; | ||
| } | ||
| return $a_local < $b_local ? -1 : 1; | ||
| } |
There was a problem hiding this comment.
Use a timezone-normalized timestamp key for primary sort.
On Line [312], formatting start_dt in the event’s own timezone can produce different keys for the same instant, so Line [320] tie-breaking by calendar_priority may never run for cross-timezone events. Use a normalized epoch key instead.
Suggested fix
private static function local_key($event)
{
- return $event->start_dt instanceof Carbon ? $event->start_dt->format('YmdHis') : (string) $event->start;
+ return $event->start_dt instanceof Carbon
+ ? $event->start_dt->copy()->setTimezone('UTC')->getTimestamp()
+ : intval($event->start);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static function local_key($event) | |
| { | |
| return $event->start_dt instanceof Carbon ? $event->start_dt->format('YmdHis') : (string) $event->start; | |
| } | |
| private static function cmp($a, $b) | |
| { | |
| if ($a->start == $b->start) { | |
| $a_local = self::local_key($a); | |
| $b_local = self::local_key($b); | |
| if ($a_local === $b_local) { | |
| if ($a->calendar_priority != $b->calendar_priority) { | |
| return $a->calendar_priority < $b->calendar_priority ? -1 : 1; | |
| } | |
| if ($a->title == $b->title) { | |
| return 0; | |
| } | |
| return $a->title < $b->title ? -1 : 1; | |
| } else { | |
| return $a->start < $b->start ? -1 : 1; | |
| } | |
| return $a_local < $b_local ? -1 : 1; | |
| } | |
| private static function local_key($event) | |
| { | |
| return $event->start_dt instanceof Carbon | |
| ? $event->start_dt->copy()->setTimezone('UTC')->getTimestamp() | |
| : intval($event->start); | |
| } | |
| private static function cmp($a, $b) | |
| { | |
| $a_local = self::local_key($a); | |
| $b_local = self::local_key($b); | |
| if ($a_local === $b_local) { | |
| if ($a->calendar_priority != $b->calendar_priority) { | |
| return $a->calendar_priority < $b->calendar_priority ? -1 : 1; | |
| } | |
| if ($a->title == $b->title) { | |
| return 0; | |
| } | |
| return $a->title < $b->title ? -1 : 1; | |
| } | |
| return $a_local < $b_local ? -1 : 1; | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 315-329: Avoid unused private methods such as 'cmp'. (undefined)
(UnusedPrivateMethod)
🤖 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/calendars/views/default-calendar-grid.php` around lines 310 - 329,
The primary sort key in local_key() currently formats $event->start_dt in the
event's timezone which can yield different string keys for the same instant
across timezones; change local_key() to return a timezone-normalized epoch value
(e.g. convert $event->start_dt to UTC and return its UNIX timestamp or
microsecond epoch) so comparisons in cmp() use the same instant for all events;
keep the existing fallback for non-Carbon $event->start but convert it to a
normalized epoch too, and ensure the returned value is comparable (int or
numeric string) so the tie-breaker logic (calendar_priority and title) can run
correctly.
| private static function local_key($event) | ||
| { | ||
| return $event->start_dt instanceof Carbon ? $event->start_dt->format('YmdHis') : (string) $event->start; | ||
| } | ||
|
|
||
| private static function cmp($a, $b) | ||
| { | ||
| if ($a->start == $b->start) { | ||
| $a_local = self::local_key($a); | ||
| $b_local = self::local_key($b); | ||
| if ($a_local === $b_local) { | ||
| if ($a->calendar_priority != $b->calendar_priority) { | ||
| return $a->calendar_priority < $b->calendar_priority ? -1 : 1; | ||
| } | ||
| if ($a->title == $b->title) { | ||
| return 0; | ||
| } | ||
| return $a->title < $b->title ? -1 : 1; | ||
| } else { | ||
| return $a->start < $b->start ? -1 : 1; | ||
| } | ||
| return $a_local < $b_local ? -1 : 1; | ||
| } |
There was a problem hiding this comment.
Primary sort key has the same cross-timezone tie bug as grid view.
On Line [500], using local formatted time from start_dt can split equal instants into different buckets, so priority ordering on Lines [508-510] is bypassed.
Suggested fix
private static function local_key($event)
{
- return $event->start_dt instanceof Carbon ? $event->start_dt->format('YmdHis') : (string) $event->start;
+ return $event->start_dt instanceof Carbon
+ ? $event->start_dt->copy()->setTimezone('UTC')->getTimestamp()
+ : intval($event->start);
}🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 503-517: Avoid unused private methods such as 'cmp'. (undefined)
(UnusedPrivateMethod)
🤖 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/calendars/views/default-calendar-list.php` around lines 498 - 517,
The primary sort key function local_key currently formats start_dt in local time
which can split identical instants across timezones and bypass
calendar_priority; change local_key to return a timezone-canonical instant (e.g.
UTC timestamp or Unix epoch) for start_dt instead of a local-formatted string,
and ensure the fallback for $event->start returns the same canonical
representation (numeric/string) so cmp's comparisons use the true instant;
update local_key and keep cmp's tie handling (calendar_priority and title)
unchanged.
1 participant
Summary
[if-event id--in="…" id--not-in="…"]event-builder tag for conditional content; accepts full UIDs, event-IDs alone, and Google Calendar URL base64 IDsurlattribute on[link]to override the event's own URLEvent::$calendar_priority)Test plan
[if-event id--in="<uid>"]X[/if-event]to the event template and confirm X only renders for matching events; repeat withid--not-inxxx@google.com, barexxx, base64 URL ID) and confirm each matches[link url="https://example.com"]click[/link]renders an anchor toexample.com; omittingurlstill links to\$event->linkSummary by CodeRabbit
Release Notes
New Features
if-eventtemplate tag for conditional event rendering based on event IDImprovements