Skip to content

fix(calendar): RECURRENCE-ID exception handling + sync failure notifications #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
icciaaron wants to merge 1 commit into
base: release/17.0
Choose a base branch
from
Open

fix(calendar): RECURRENCE-ID exception handling + sync failure notifications #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions Calendar.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,11 +648,33 @@ public function sync($output, $force = false)
$next = !empty($calendar['next']) ? $calendar['next'] : 300;
if ($force || ($last + $next) < time()) {
$calendar['id'] = $id;
$c = $cal->getDriverById($id);
$c->processCalendar($calendar);
$cal->setConfig($id, time(), 'calendar-sync');
$output->writeln("Done");
$this->FreePBX->Hooks->processHooks($calendar['id']);
try {
$c = $cal->getDriverById($id);
$c->processCalendar($calendar);
$cal->setConfig($id, time(), 'calendar-sync');
$output->writeln("Done");
$this->FreePBX->Hooks->processHooks($calendar['id']);
// Clear any previous failure notification for this calendar
\FreePBX::Notifications()->delete('calendar', 'SYNC_FAIL_' . $id);
} catch (\Throwable $e) {
$output->writeln("<error>FAILED: " . $e->getMessage() . "</error>");
$calName = htmlspecialchars($calendar['name'], ENT_QUOTES, 'UTF-8');
$errMsg = htmlspecialchars($e->getMessage(), ENT_QUOTES, 'UTF-8');
\FreePBX::Notifications()->add_critical(
'calendar',
'SYNC_FAIL_' . $id,
sprintf(_('Calendar sync failed: %s'), $calName),
sprintf(
_('The calendar "%s" failed to sync at %s. Time conditions and call routing that depend on this calendar are using stale data and may route calls incorrectly. Error: %s'),
$calName,
date('Y-m-d H:i:s T'),
$errMsg
),
'',
false,
true
);
}
} else
$output->writeln("Skipping");
}
Expand Down
107 changes: 40 additions & 67 deletions IcalParser/IcalRangedParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,21 @@ public function parseRecurrences($event): array
break; //retrieval failed, probably we are out of period or the recurrence is invalid
}

// Filter occurrences that have been replaced by RECURRENCE-ID
// exception events (moved/cancelled instances). The non-fast
// path does this at the end of parseRecurrences; the fast path
// must do it here because it returns early.
if (!empty($this->data['_RECURRENCE_IDS'])) {
$tz = $event['DTSTART']->getTimezone();
$recurrences = array_values(array_filter($recurrences, function ($ts) use ($tz) {
$local = (new \DateTime('@' . $ts))->setTimezone($tz);
$utc = new \DateTime('@' . $ts, new \DateTimeZone('UTC'));
return empty($this->data['_RECURRENCE_IDS'][$local->format('Ymd')])
&& empty($this->data['_RECURRENCE_IDS'][$local->format('Ymd\THis')])
&& empty($this->data['_RECURRENCE_IDS'][$utc->format('Ymd\THis\Z')]);
}));
}

return $recurrences;
} else {
//setting end to the one defined above and generating a new Frequency object
Expand Down Expand Up @@ -286,69 +301,23 @@ public function getEvents(): EventsList
$event = $this->data['VEVENT'][$i];

if (empty($event['RECURRENCES'])) {
if (!empty($event['RECURRENCE-ID']) && !empty($event['UID']) && isset($event['SEQUENCE'])) {
/**
* You may ask why I abandoned all this code, I will try to explain as clearly as possible:
*
* First problem -> the recurring rule could have many formats, but as it is now only a specific one can be interpreted. This is not a big problem, we could generate a warning if we were unable to decode it. Here are some exmaples:
* RECURRENCE-ID;RANGE=THISANDFUTURE:19960120T120000Z -> Becomes 19960120T120000Z (not matched becasue of the final "Z" + RANGE ignored)
* RECURRENCE-ID;VALUE=DATE:19960401 -> Becomes 19960401 (not matched beacuse it is only a date)
* RECURRENCE-ID;RANGE=THISANDFUTURE:TZID=UTC:20230531T120000 -> Becomes TZID=UTC:20230531T120000 (not matched because of the extra TZID)
* RECURRENCE-ID;TZID=UTC:20230531T120000 -> Becomes 20230531T120000 (matched)
* RECURRENCE-ID;VALUE=DATE-TIME:20210115T100000 -> Becomes 20230531T120000 (matched)
*
* Second problem -> If we were able to overcome the first point and we get a match with the exact same DTSTART and an increased sequence (look at the first "if" below) the event is replaced entirely. Fine you'll say but there is a catch, recurrences are calculated on the original event and not updated causing all sort of problems (beside that, why don't you update the original event directly instead of using this quirks?)
*
* Third problem -> If we instead fall into the second "if" we will search for recurrences to replace inside the original event. Perfect! But there is a catch! You see as soon as we find a recurrence that match we delete it and... nothing more! So the user is expecting this particular recurrence to be replaced by the new event but instead will find that recurrence completely deleted!
*
* So all in all fixing all this is issues is too complicated, if someone wants to do it free to do so, but be careful of the speed too...
*/

/*
$modifiedEventUID = $event['UID'];
$modifiedEventRecurID = $event['RECURRENCE-ID'];
$modifiedEventSeq = intval($event['SEQUENCE'], 10);

if (isset($this->data["_RECURRENCE_COUNTERS_BY_UID"][$modifiedEventUID])) {
$counter = $this->data["_RECURRENCE_COUNTERS_BY_UID"][$modifiedEventUID];

$originalEvent = $this->data["VEVENT"][$counter];
if (isset($originalEvent['SEQUENCE'])) {
$originalEventSeq = intval($originalEvent['SEQUENCE'], 10);
$originalEventFormattedStartDate = $originalEvent['DTSTART']->format('Ymd\THis');
if ($modifiedEventRecurID === $originalEventFormattedStartDate && $modifiedEventSeq > $originalEventSeq) {
// this modifies the original event
$modifiedEvent = array_replace_recursive($originalEvent, $event);
$this->data["VEVENT"][$counter] = $modifiedEvent;
foreach ($events as $z => $event) {
if ($events[$z]['UID'] === $originalEvent['UID'] &&
$events[$z]['SEQUENCE'] === $originalEvent['SEQUENCE']) {
// replace the original event with the modified event
$events[$z] = $modifiedEvent;
break;
}
}
$event = null; // don't add this to the $events[] array again
} else if (!empty($originalEvent['RECURRENCES'])) {
for ($j = 0; $j < count($originalEvent['RECURRENCES']); $j++) {
$recurDate = $originalEvent['RECURRENCES'][$j];
$formattedStartDate = $recurDate->format('Ymd\THis');
if ($formattedStartDate === $modifiedEventRecurID) {
unset($this->data["VEVENT"][$counter]['RECURRENCES'][$j]);
$this->data["VEVENT"][$counter]['RECURRENCES'] = array_values($this->data["VEVENT"][$counter]['RECURRENCES']);
break;
}
}
}
}
}
*/
if (!empty($event['RECURRENCE-ID']) && !empty($event['UID'])) {
// RECURRENCE-ID exception: this VEVENT replaces or cancels
// a single occurrence of a recurring series. The original
// occurrence was already removed during parseRecurrences()
// (which filters timestamps matching _RECURRENCE_IDS).
// We include the replacement event if it is not cancelled
// and falls within the calendar range — no need to touch
// the parent series' recurrence list.
if (strtoupper($event['STATUS'] ?? '') === 'CANCELLED') {
$event = null;
}
}

\FreePBX::Notifications()->add_warning('calendar', 'RECURRENCEID', _('Calendar using RECURRENCE-ID'), str_replace('%event', $event['SUMMARY'], _('A calendar you have added has an event called "%event" that has a RECURRENCE-ID rule. Because there is no full support yet they are ignored, please consider changing this to Exceptions/Additions.')), "", true, true);
$event = null; //this is an event that modifies another one. In every case (even if we weren't able to overwrite the original one for whatever reason) it should not end up in the output
} else {
//neither start nor end is within range so skip it
if(isset($event['DTSTART']) && isset($event['DTEND'])) {
// Range check for all non-recurring events (regular +
// RECURRENCE-ID replacements that survived the cancel check).
if (!empty($event)) {
if (isset($event['DTSTART']) && isset($event['DTEND'])) {
if (!$this->eventRangeInCalendarRange($event['DTSTART'], $event['DTEND'])) {
$event = null;
}
Expand Down Expand Up @@ -426,11 +395,15 @@ public function getEventsNow($now = 0)
$event = $this->data['VEVENT'][$i];

if (empty($event['RECURRENCES'])) {
if (!empty($event['RECURRENCE-ID']) && !empty($event['UID']) && isset($event['SEQUENCE'])) {
\FreePBX::Notifications()->add_warning('calendar', 'RECURRENCEID', _('Calendar using RECURRENCE-ID'), str_replace('%event', $event['SUMMARY'], _('A calendar you have added has an event called "%event" that has a RECURRENCE-ID rule. Because there is no full support yet they are ignored, please consider changing this to Exceptions/Additions.')), "", true, true);
} else {
if ($event['DTSTART']->getTimestamp() < $now && $now < $event['DTEND']->getTimestamp())
array_push($events, $event); //event is now, keep it
// Skip cancelled RECURRENCE-ID exceptions (deleted instances)
if (!empty($event['RECURRENCE-ID']) && strtoupper($event['STATUS'] ?? '') === 'CANCELLED') {
continue;
}

// Include single events and RECURRENCE-ID replacements if happening now
if (isset($event['DTSTART']) && isset($event['DTEND'])
&& $event['DTSTART']->getTimestamp() < $now && $now < $event['DTEND']->getTimestamp()) {
array_push($events, $event);
}
} else {
$recurrences = $event['RECURRENCES'];
Expand Down
38 changes: 30 additions & 8 deletions drivers/Base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,36 @@ public function buildCache()
$end->add(new \DateInterval(self::ADD_END));
$icalData = $this->getIcal();
if ($icalData) {
$cal = new IcalRangedParser(true);
$cal->setStartRange($start);
$cal->setEndRange($end);
$raw = $cal->parseString($icalData);
$this->calendarClass->setConfig($this->calendar['id'], serialize($raw), 'calendar-cache');
$this->calendarClass->setConfig($this->calendar['id'], $start->getTimestamp(), 'calendar-cache_valid_notbefore');
$this->calendarClass->setConfig($this->calendar['id'], $end->getTimestamp(), 'calendar-cache_valid_notafter');
return true;
try {
$cal = new IcalRangedParser(true);
$cal->setStartRange($start);
$cal->setEndRange($end);
$raw = $cal->parseString($icalData);
$this->calendarClass->setConfig($this->calendar['id'], serialize($raw), 'calendar-cache');
$this->calendarClass->setConfig($this->calendar['id'], $start->getTimestamp(), 'calendar-cache_valid_notbefore');
$this->calendarClass->setConfig($this->calendar['id'], $end->getTimestamp(), 'calendar-cache_valid_notafter');
return true;
} catch (\Throwable $e) {
if (class_exists('FreePBX')) {
$calName = htmlspecialchars($this->calendar['name'] ?? $this->calendar['id'], ENT_QUOTES, 'UTF-8');
$errMsg = htmlspecialchars($e->getMessage(), ENT_QUOTES, 'UTF-8');
\FreePBX::Notifications()->add_critical(
'calendar',
'PARSE_FAIL_' . $this->calendar['id'],
sprintf(_('Calendar parse failed: %s'), $calName),
sprintf(
_('The calendar "%s" could not be parsed at %s. This calendar\'s events will not be evaluated for time conditions or call routing until the problem is resolved. Error: %s'),
$calName,
date('Y-m-d H:i:s T'),
$errMsg
),
'',
false,
true
);
}
return false;
}
} else {
return false;
}
Expand Down
136 changes: 136 additions & 0 deletions utests/IcalRangedParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,142 @@ function testFREEPBX18919() {
$this->assertEvents($assertEvents, $events);
}

/**
* Test RECURRENCE-ID exception handling:
* - Jan 26 occurrence moved to 10:00-18:00 (should appear at new time)
* - Feb 2 occurrence cancelled (should not appear)
* - Other Mondays should appear at the original 09:00-17:00
*/
function testRecurrenceIdExceptions() {
$raw = file_get_contents(__DIR__.'/icals/recurrence-id-exceptions.ics');
$cal = new IcalRangedParser();
$cal->setStartRange(Carbon::parse("2026-01-12"));
$cal->setEndRange(Carbon::parse("2026-02-09"));
$cal->parseString($raw);
$events = $cal->getSortedEvents();

$this->assertNotEmpty($events, 'No events could be found');

// Collect event summaries and start dates for easier assertion
$parsed = [];
foreach ($events as $event) {
$parsed[] = [
'date' => Carbon::instance($event['DTSTART'])->format('Y-m-d'),
'time' => Carbon::instance($event['DTSTART'])->format('H:i'),
'summary' => $event['SUMMARY'],
'cancelled' => (strtoupper($event['STATUS'] ?? '') === 'CANCELLED'),
];
}

// Build a lookup by date
$byDate = [];
foreach ($parsed as $p) {
$byDate[$p['date']] = $p;
}

// Jan 12 — original occurrence, should exist at 09:00
$this->assertArrayHasKey('2026-01-12', $byDate, 'Jan 12 occurrence missing');
$this->assertEquals('09:00', $byDate['2026-01-12']['time']);

// Jan 19 — original occurrence, should exist at 09:00
$this->assertArrayHasKey('2026-01-19', $byDate, 'Jan 19 occurrence missing');
$this->assertEquals('09:00', $byDate['2026-01-19']['time']);

// Jan 26 — moved to 10:00, should appear at 10:00 with modified summary
$this->assertArrayHasKey('2026-01-26', $byDate, 'Jan 26 modified occurrence missing');
$this->assertEquals('10:00', $byDate['2026-01-26']['time']);
$this->assertStringContainsString('moved to 10am', $byDate['2026-01-26']['summary']);

// Feb 2 — cancelled, should NOT appear
$this->assertArrayNotHasKey('2026-02-02', $byDate, 'Feb 2 cancelled occurrence should not appear');

// Feb 9 — original occurrence, should exist at 09:00
$this->assertArrayHasKey('2026-02-09', $byDate, 'Feb 9 occurrence missing');
$this->assertEquals('09:00', $byDate['2026-02-09']['time']);
}

/**
* Test RECURRENCE-ID with all-day events (VALUE=DATE format).
*/
function testRecurrenceIdAllDay() {
$raw = file_get_contents(__DIR__.'/icals/recurrence-id-allday.ics');
$cal = new IcalRangedParser();
$cal->setStartRange(Carbon::parse("2026-01-05"));
$cal->setEndRange(Carbon::parse("2026-02-02"));
$cal->parseString($raw);
$events = $cal->getSortedEvents();

$this->assertNotEmpty($events, 'No events could be found');

$dates = [];
$summaries = [];
foreach ($events as $event) {
$d = Carbon::instance($event['DTSTART'])->format('Y-m-d');
$dates[] = $d;
$summaries[$d] = $event['SUMMARY'];
}

// Jan 5 — original
$this->assertContains('2026-01-05', $dates, 'Jan 5 occurrence missing');

// Jan 12 — original
$this->assertContains('2026-01-12', $dates, 'Jan 12 occurrence missing');

// Jan 19 — modified title (should appear)
$this->assertContains('2026-01-19', $dates, 'Jan 19 modified occurrence missing');
$this->assertStringContainsString('modified title', $summaries['2026-01-19']);

// Jan 26 — cancelled (should NOT appear)
$this->assertNotContains('2026-01-26', $dates, 'Jan 26 cancelled occurrence should not appear');
}

/**
* Test RECURRENCE-ID handling in fast mode (getEventsNow).
* Verifies that:
* - A modified occurrence is matched at its new time
* - A cancelled occurrence is not matched
* - Original (non-excepted) occurrences still match at original time
*/
function testRecurrenceIdFastMode() {
$raw = file_get_contents(__DIR__.'/icals/recurrence-id-exceptions.ics');

// Check that Jan 26 at 14:00 EST matches (moved occurrence: 10:00-18:00)
$cal = new IcalRangedParser(true);
$jan26_2pm = Carbon::parse("2026-01-26 14:00:00", "America/New_York");
$cal->setStartRange($jan26_2pm->copy()->subDay());
$cal->setEndRange($jan26_2pm->copy()->addDay());
$cal->parseString($raw);
$events = $cal->getEventsNow($jan26_2pm->getTimestamp());
$this->assertNotEmpty($events, 'Jan 26 2pm should match the moved occurrence');

// Check that Jan 26 at 08:30 EST does NOT match (old time, occurrence was moved)
$cal2 = new IcalRangedParser(true);
$jan26_830 = Carbon::parse("2026-01-26 08:30:00", "America/New_York");
$cal2->setStartRange($jan26_830->copy()->subDay());
$cal2->setEndRange($jan26_830->copy()->addDay());
$cal2->parseString($raw);
$events2 = $cal2->getEventsNow($jan26_830->getTimestamp());
$this->assertFalse($events2, 'Jan 26 8:30am should NOT match (occurrence was moved to 10am)');

// Check that Feb 2 at 12:00 EST does NOT match (cancelled)
$cal3 = new IcalRangedParser(true);
$feb2_noon = Carbon::parse("2026-02-02 12:00:00", "America/New_York");
$cal3->setStartRange($feb2_noon->copy()->subDay());
$cal3->setEndRange($feb2_noon->copy()->addDay());
$cal3->parseString($raw);
$events3 = $cal3->getEventsNow($feb2_noon->getTimestamp());
$this->assertFalse($events3, 'Feb 2 noon should NOT match (occurrence was cancelled)');

// Check that Jan 19 at 12:00 EST still matches (unmodified occurrence)
$cal4 = new IcalRangedParser(true);
$jan19_noon = Carbon::parse("2026-01-19 12:00:00", "America/New_York");
$cal4->setStartRange($jan19_noon->copy()->subDay());
$cal4->setEndRange($jan19_noon->copy()->addDay());
$cal4->parseString($raw);
$events4 = $cal4->getEventsNow($jan19_noon->getTimestamp());
$this->assertNotEmpty($events4, 'Jan 19 noon should match (unmodified occurrence)');
}

function assertEvents($assertEvents, $events) {
$this->assertEquals(is_countable($events) ? count($events) : 0, is_countable($assertEvents) ? count($assertEvents) : 0);
foreach($assertEvents as $k => $args) {
Expand Down
30 changes: 30 additions & 0 deletions utests/icals/recurrence-id-allday.ics
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//ICCI LLC//FreePBX Calendar Test//EN
X-WR-TIMEZONE:America/New_York
BEGIN:VEVENT
DTSTART;VALUE=DATE:20260105
DTEND;VALUE=DATE:20260106
RRULE:FREQ=WEEKLY;BYDAY=MO
SUMMARY:All-Day Monday
UID:test-recurrence-id-allday@icci.com
SEQUENCE:0
END:VEVENT
BEGIN:VEVENT
DTSTART;VALUE=DATE:20260119
DTEND;VALUE=DATE:20260120
RECURRENCE-ID;VALUE=DATE:20260119
SUMMARY:All-Day Monday (modified title)
UID:test-recurrence-id-allday@icci.com
SEQUENCE:1
END:VEVENT
BEGIN:VEVENT
DTSTART;VALUE=DATE:20260126
DTEND;VALUE=DATE:20260127
RECURRENCE-ID;VALUE=DATE:20260126
SUMMARY:All-Day Monday
UID:test-recurrence-id-allday@icci.com
SEQUENCE:1
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR
Loading