fix(calendar): RECURRENCE-ID exception handling + sync failure notifications#32
Open
icciaaron wants to merge 1 commit intoFreePBX:release/17.0from
Open
Conversation
…ilure notifications RECURRENCE-ID events (RFC 5545 §3.8.4.4) — used when a human moves or cancels a single occurrence of a recurring calendar event — were previously dropped silently. The original developer commented out the handling code and replaced it with a warning notification, causing: - Modified occurrences (e.g. moved meeting) to disappear entirely - Cancelled occurrences to remain active (original time still matched) - A DB write per exception per sync cycle (notification spam) - Cascading failures in time conditions and call routing This commit fixes three code paths in IcalRangedParser: 1. parseRecurrences() fast path: filter out RECURRENCE-ID replaced timestamps (matching what the non-fast path already did) 2. getEvents(): include non-cancelled replacement VEVENTs as regular single-occurrence events with standard range checking 3. getEventsNow(): same treatment for fast-mode "is it now?" matching The fix leverages the existing _RECURRENCE_IDS lookup table populated by the upstream om/icalparser during parseString(). The original occurrence is already removed from the parent series' recurrence list by the RECURRENCE-ID filter in parseRecurrences() (lines 263-281). The replacement VEVENT simply needs to not be dropped. Additionally adds sync failure notifications: - Calendar.class.php: try/catch around processCalendar() in sync() so one broken calendar no longer kills sync for all calendars. Failures produce a critical admin notification; success clears it. - Base.php: try/catch around iCal parsing in buildCache() with a critical notification on parse failure. Known limitation: RECURRENCE-ID with RANGE=THISANDFUTURE is not supported by the upstream om/icalparser library and remains unhandled. This covers <1% of real-world calendar usage (Google Calendar, Outlook, and iCloud use series-splitting instead of THISANDFUTURE). Includes unit tests for timed events, all-day (VALUE=DATE) events, and fast-mode matching with moved/cancelled/unmodified occurrences. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
|
Hi @Akarsh04 @prasanthcode4 — could one of you take a look when you have a moment? This fixes a real-world sync issue we hit in production with Google Calendar exceptions (modified instances of recurring events being dropped). CLA is signed and the change is scoped. Happy to split or rework anything that doesn't fit your conventions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RECURRENCE-ID exception events (RFC 5545 §3.8.4.4) — used when a human moves or cancels a single occurrence of a recurring calendar event — were previously dropped silently. The original developer commented out the handling code and replaced it with a warning notification, causing:
RECURRENCE-ID fix (IcalRangedParser.php)
Three code paths fixed:
parseRecurrences()fast path: Filter RECURRENCE-ID replaced timestamps from the recurrence array (matching what the non-fast path already does)getEvents(): Include non-cancelled replacement VEVENTs as regular events with standard range checking. DropSTATUS:CANCELLEDexceptionsgetEventsNow(): Same fix for fast-mode AGI call routingThe fix leverages the existing
_RECURRENCE_IDSlookup table thatom/icalparseralready populates duringparseString(). The original occurrence is already removed from the parent series' recurrence list by the RECURRENCE-ID filter inparseRecurrences(). The replacement VEVENT simply needs to not be dropped.Sync failure notifications (Calendar.class.php + Base.php)
sync()now wrapsprocessCalendar()in try/catch — one broken calendar no longer kills sync for all calendarsbuildCache()parse failures also produce a critical notificationKnown limitation
RECURRENCE-ID;RANGE=THISANDFUTUREis not supported by the upstreamom/icalparserlibrary and remains unhandled. This affects <1% of real-world usage — Google Calendar, Outlook, and iCloud all use series-splitting instead.Test plan
getEventsNow()matching (4 scenarios)