[calendar] refactor: delegate event expansion to node-ical's expandRecurringEvent#4047
Merged
rejas merged 19 commits intoMagicMirrorOrg:developfrom Mar 2, 2026
Merged
Conversation
…event expansion Replace custom getMomentsFromRecurringEvent + expandRecurringEvent with node-ical's built-in expandRecurringEvent which handles RRULE expansion, EXDATE filtering, RECURRENCE-ID overrides, and ongoing events. - Remove getMomentsFromRecurringEvent (no longer needed) - Simplify expandRecurringEvent to a thin wrapper (Date→Moment conversion) - Remove year < 1900 workaround (not needed with rrule-temporal) - Remove unused durationMs variable - Update tests to use new API
…tBeExcluded The return value uses 'until', not 'eventFilterUntil', so the destructuring always yielded undefined, effectively disabling the time-based event filter (excludedEvents with 'until').
Cover the scenario where an event matches an excludedEvents filter
that has an 'until' property (e.g. {filterBy: 'Payment', until: '3 days'}).
The event should be hidden when far away and shown when close to its end.
Also add a basic test for simple string-based excludedEvents.
…al handles it correctly Previously, Facebook birthday calendars (DTSTART with the birth year, e.g. 1990) triggered a special-case bypass that skipped expandRecurringEvent entirely. This was needed because rrule.js couldn't handle very old dates correctly. rrule-temporal handles pre-1900/old dates via the Temporal API, so the workaround is no longer needed. Facebook birthday events now go through expandRecurringEvent like any other yearly recurring event. Adds a regression test to verify the birthday shows in the current/upcoming year, not in the birth year.
event.start is always a Date object, so event.start.length is always undefined and the length === 8 check never matched.
Remove the manual if/else branch that handled recurring vs. non-recurring events separately. ical.expandRecurringEvent() handles both cases, so the duplicated eventDate() helper, eventStartMoment/eventEndMoment setup, and the manual duration/end fallback are no longer needed. - Remove eventDate() helper function - Remove manual eventStartMoment/eventEndMoment calculation - Replace if (event.rrule) / else branch with a single expandRecurringEvent() call - Merge start/end into the "saving event" debug log (one line per saved instance) Test: fix full-day event fixture to use ICS-compliant DTEND (exclusive next day) and set dateOnly = true so the same code path as real ICS data is exercised.
Each instance returned by expandRecurringEvent already carries isFullDay as set by node-ical. Using it directly is more accurate than calling isFullDayEvent() on the base event, which would return the wrong value for RECURRENCE-ID overrides that change a full-day instance to a timed one (or vice versa). isFullDayEvent() is now unused and has been removed.
…tion
node-ical returns {val, params} objects for ICS properties that carry
parameters (e.g. DESCRIPTION;LANGUAGE=de:Text, LOCATION;LANGUAGE=de:Ort).
Without unwrapping, these were passed as raw objects to the frontend,
causing "[object Object]" to be displayed.
Add unwrapParameterValue() helper and apply it consistently to summary,
description and location in filterEvents() and getTitleFromEvent().
Simplify getTitleFromEvent() to a single expression.
Add 9 unit tests covering unwrapParameterValue, getTitleFromEvent, and
an integration test for ParameterValue properties in filterEvents.
…vents The only use of `this` in the file — consistent with all other method calls and safe when used as a callback.
- Replace filter.substr(1).slice(0,-1) with filter.slice(1,-1) in titleFilterApplies (substr is deprecated) - Replace now < filterUntil with now.isBefore(filterUntil) in timeFilterApplies (idiomatic Moment.js)
Skip getTitleFromEvent and shouldEventBeExcluded for non-VEVENT entries (VTIMEZONE, VCALENDAR, etc.) by returning early. Removes one level of nesting and eliminates the shadowed title variable inside the loop (renamed to instanceTitle).
If expandRecurringEvent throws for a single event (e.g. due to corrupted ICS data), catch it in filterEvents, log the error, and continue with the remaining events. expandRecurringEvent itself remains clean and throws through.
…DTEND fallback
- filterEvents output shape: verifies all 9 fields (title, startDate,
endDate, fullDayEvent, recurringEvent, class, firstYear, location,
geo, description) are correctly populated
- expandRecurringEvent no-DTEND: verifies MagicMirror's endOf("day")
fallback when node-ical returns end === start
khassel
approved these changes
Mar 2, 2026
rejas
approved these changes
Mar 2, 2026
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
3 participants
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.
node-ical 0.25.x added
expandRecurringEvent()— a proper API for expanding both recurring and non-recurring events, including EXDATE filtering and RECURRENCE-ID overrides. This PR replaces our hand-rolled equivalent with it.calendarfetcherutils.jsloses ~125 lines of code. What's left only deals with MagicMirror-specific concerns: timezone conversion, config-based filtering, and output formatting. The extra lines in the diff come from new tests.What was removed
getMomentsFromRecurringEvent()— manual rrule.js wrapping with custom date extractionisFullDayEvent()— heuristic with multiple fallback checksisFacebookBirthdayworkaround — patched years < 1900 and special-cased@facebook.comUIDsif (event.rrule) / elsesplit — all events now go through a single code pathBugs fixed along the way
Both were subtle enough to go unnoticed before:
[object Object]in event titles/description/location — node-ical represents ICS properties with parameters (e.g.DESCRIPTION;LANGUAGE=de:Text) as{val, params}objects. The old code passed them straight through. Mainly affected multilingual Exchange/O365 setups. Fixed withunwrapParameterValue().excludedEventswithuntilnever worked —shouldEventBeExcluded()returned{ excluded, until }but the caller destructured it as{ excluded, eventFilterUntil }, so the until date was alwaysundefinedand events were never hidden. Fixed by correcting the destructuring key.The expansion loop also gets error isolation: a single broken event is logged and skipped instead of aborting the whole feed.
Other clean-ups
this.shouldEventBeExcludedwithCalendarFetcherUtils.shouldEventBeExcluded— avoids context-loss bugs when the method is destructured or called indirectly.substr()withslice().now < filterUntil(operator overloading) withnow.isBefore(filterUntil)— idiomatic Moment.js comparison.@returnsJSDoc:string[]→object[].Log.debug("Processing entry...")after theVEVENTtype guard to reduce log noise from non-event entries.JSON.stringify(event)debug log with a lightweight summary to avoid unnecessary serialization cost.Tests
24 unit tests, all passing (
npx vitest run tests/unit/modules/default/calendar/).New coverage:
excludedEventswith/withoutuntil, Facebook birthday year expansion, output object shape, no-DTEND fallback, error isolation,unwrapParameterValue,getTitleFromEvent, ParameterValue properties, RECURRENCE-ID overrides, DURATION (single and recurring).