Skip to content

Commit ab3108f

Browse files
KristjanESPERANTOKristjanESPERANTO
authored
[calendar] refactor: delegate event expansion to node-ical's expandRecurringEvent (#4047)
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.js` loses ~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 extraction - `isFullDayEvent()` — heuristic with multiple fallback checks - `isFacebookBirthday` workaround — patched years < 1900 and special-cased `@facebook.com` UIDs - The `if (event.rrule) / else` split — all events now go through a single code path ## Bugs 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 with `unwrapParameterValue()`. - **`excludedEvents` with `until` never worked** — `shouldEventBeExcluded()` returned `{ excluded, until }` but the caller destructured it as `{ excluded, eventFilterUntil }`, so the until date was always `undefined` and 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 - Replaced `this.shouldEventBeExcluded` with `CalendarFetcherUtils.shouldEventBeExcluded` — avoids context-loss bugs when the method is destructured or called indirectly. - Replaced deprecated `substr()` with `slice()`. - Replaced `now < filterUntil` (operator overloading) with `now.isBefore(filterUntil)` — idiomatic Moment.js comparison. - Fixed `@returns` JSDoc: `string[]` → `object[]`. - Moved verbose `Log.debug("Processing entry...")` after the `VEVENT` type guard to reduce log noise from non-event entries. - Replaced `JSON.stringify(event)` debug log with a lightweight summary to avoid unnecessary serialization cost. - Added comment explaining the 0-duration → end-of-day fallback for events without DTEND. ## Tests 24 unit tests, all passing (`npx vitest run tests/unit/modules/default/calendar/`). New coverage: `excludedEvents` with/without `until`, Facebook birthday year expansion, output object shape, no-DTEND fallback, error isolation, `unwrapParameterValue`, `getTitleFromEvent`, ParameterValue properties, RECURRENCE-ID overrides, DURATION (single and recurring).
1 parent 06b1361 commit ab3108f

File tree

6 files changed

+476
-243
lines changed

6 files changed

+476
-243
lines changed

‎defaultmodules/calendar/calendarfetcherutils.js‎

Lines changed: 79 additions & 204 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @external Moment
33
*/
44
const moment = require("moment-timezone");
5+
const ical = require("node-ical");
56

67
const Log = require("logger");
78

@@ -40,58 +41,15 @@ const CalendarFetcherUtils = {
4041
return moment.tz.guess();
4142
},
4243

43-
/**
44-
* This function returns a list of moments for a recurring event.
45-
* @param {object} event the current event which is a recurring event
46-
* @param {moment.Moment} pastLocalMoment The past date to search for recurring events
47-
* @param {moment.Moment} futureLocalMoment The future date to search for recurring events
48-
* @param {number} durationInMs the duration of the event, this is used to take into account currently running events
49-
* @returns {moment.Moment[]} All moments for the recurring event
50-
*/
51-
getMomentsFromRecurringEvent (event, pastLocalMoment, futureLocalMoment, durationInMs) {
52-
const rule = event.rrule;
53-
const isFullDayEvent = CalendarFetcherUtils.isFullDayEvent(event);
54-
const eventTimezone = event.start.tz || CalendarFetcherUtils.getLocalTimezone();
55-
56-
// rrule.js interprets years < 1900 as offsets from 1900, causing issues with some birthday calendars
57-
if (rule.origOptions?.dtstart?.getFullYear() < 1900) {
58-
rule.origOptions.dtstart.setFullYear(1900);
59-
}
60-
if (rule.options?.dtstart?.getFullYear() < 1900) {
61-
rule.options.dtstart.setFullYear(1900);
62-
}
63-
64-
// Expand search window to include ongoing events
65-
const oneDayInMs = 24 * 60 * 60 * 1000;
66-
const searchFromDate = pastLocalMoment.clone().subtract(Math.max(durationInMs, oneDayInMs), "milliseconds").toDate();
67-
const searchToDate = futureLocalMoment.clone().add(1, "days").toDate();
68-
69-
const dates = rule.between(searchFromDate, searchToDate, true) || [];
70-
71-
// Convert dates to moments in the event's timezone.
72-
// Full-day events need UTC component extraction to avoid date shifts across timezone boundaries.
73-
return dates.map((date) => {
74-
if (isFullDayEvent) {
75-
return moment.tz([date.getUTCFullYear(), date.getUTCMonth(), date.getUTCDate()], eventTimezone);
76-
}
77-
return moment.tz(date, eventTimezone);
78-
});
79-
},
80-
8144
/**
8245
* Filter the events from ical according to the given config
8346
* @param {object} data the calendar data from ical
8447
* @param {object} config The configuration object
85-
* @returns {string[]} the filtered events
48+
* @returns {object[]} the filtered events
8649
*/
8750
filterEvents (data, config) {
8851
const newEvents = [];
8952

90-
const eventDate = function (event, time) {
91-
const startMoment = event[time].tz ? moment.tz(event[time], event[time].tz) : moment.tz(event[time], CalendarFetcherUtils.getLocalTimezone());
92-
return CalendarFetcherUtils.isFullDayEvent(event) ? startMoment.startOf("day") : startMoment;
93-
};
94-
9553
Log.debug(`There are ${Object.entries(data).length} calendar entries.`);
9654

9755
const now = moment();
@@ -105,102 +63,60 @@ const CalendarFetcherUtils = {
10563
.subtract(1, "seconds");
10664

10765
Object.entries(data).forEach(([key, event]) => {
108-
Log.debug("Processing entry...");
66+
if (event.type !== "VEVENT") {
67+
return;
68+
}
10969

11070
const title = CalendarFetcherUtils.getTitleFromEvent(event);
11171
Log.debug(`title: ${title}`);
11272

11373
// Return quickly if event should be excluded.
114-
let { excluded, eventFilterUntil } = this.shouldEventBeExcluded(config, title);
74+
const { excluded, until: eventFilterUntil } = CalendarFetcherUtils.shouldEventBeExcluded(config, title);
11575
if (excluded) {
11676
return;
11777
}
11878

119-
// FIXME: Ugly fix to solve the facebook birthday issue.
120-
// Otherwise, the recurring events only show the birthday for next year.
121-
let isFacebookBirthday = false;
122-
if (typeof event.uid !== "undefined") {
123-
if (event.uid.indexOf("@facebook.com") !== -1) {
124-
isFacebookBirthday = true;
125-
}
126-
}
79+
Log.debug(`Event: ${title} | start: ${event.start} | end: ${event.end} | recurring: ${!!event.rrule}`);
12780

128-
if (event.type === "VEVENT") {
129-
Log.debug(`Event:\n${JSON.stringify(event, null, 2)}`);
130-
let eventStartMoment = eventDate(event, "start");
131-
let eventEndMoment;
81+
const location = CalendarFetcherUtils.unwrapParameterValue(event.location) || false;
82+
const geo = event.geo || false;
83+
const description = CalendarFetcherUtils.unwrapParameterValue(event.description) || false;
13284

133-
if (typeof event.end !== "undefined") {
134-
eventEndMoment = eventDate(event, "end");
135-
} else if (typeof event.duration !== "undefined") {
136-
eventEndMoment = eventStartMoment.clone().add(moment.duration(event.duration));
137-
} else {
138-
if (!isFacebookBirthday) {
139-
// make copy of start date, separate storage area
140-
eventEndMoment = eventStartMoment.clone();
141-
} else {
142-
eventEndMoment = eventStartMoment.clone().add(1, "days");
143-
}
144-
}
145-
146-
Log.debug(`start: ${eventStartMoment.toDate()}`);
147-
Log.debug(`end: ${eventEndMoment.toDate()}`);
148-
149-
// Calculate the duration of the event for use with recurring events.
150-
const durationMs = eventEndMoment.valueOf() - eventStartMoment.valueOf();
151-
Log.debug(`duration: ${durationMs}`);
85+
let instances;
86+
try {
87+
instances = CalendarFetcherUtils.expandRecurringEvent(event, pastLocalMoment, futureLocalMoment);
88+
} catch (error) {
89+
Log.error(`Could not expand event "${title}": ${error.message}`);
90+
return;
91+
}
15292

153-
const location = event.location || false;
154-
const geo = event.geo || false;
155-
const description = event.description || false;
93+
for (const instance of instances) {
94+
const { event: instanceEvent, startMoment, endMoment, isRecurring, isFullDay } = instance;
15695

157-
let instances = [];
158-
if (event.rrule && typeof event.rrule !== "undefined" && !isFacebookBirthday) {
159-
instances = CalendarFetcherUtils.expandRecurringEvent(event, pastLocalMoment, futureLocalMoment, durationMs);
160-
} else {
161-
const fullDayEvent = isFacebookBirthday ? true : CalendarFetcherUtils.isFullDayEvent(event);
162-
let end = eventEndMoment;
163-
if (fullDayEvent && eventStartMoment.valueOf() === end.valueOf()) {
164-
end = end.endOf("day");
165-
}
166-
167-
instances.push({
168-
event: event,
169-
startMoment: eventStartMoment,
170-
endMoment: end,
171-
isRecurring: false
172-
});
96+
// Filter logic
97+
if (endMoment.isBefore(pastLocalMoment) || startMoment.isAfter(futureLocalMoment)) {
98+
continue;
17399
}
174100

175-
for (const instance of instances) {
176-
const { event: instanceEvent, startMoment, endMoment, isRecurring } = instance;
177-
178-
// Filter logic
179-
if (endMoment.isBefore(pastLocalMoment) || startMoment.isAfter(futureLocalMoment)) {
180-
continue;
181-
}
182-
183-
if (CalendarFetcherUtils.timeFilterApplies(now, endMoment, eventFilterUntil)) {
184-
continue;
185-
}
186-
187-
const title = CalendarFetcherUtils.getTitleFromEvent(instanceEvent);
188-
const fullDay = isFacebookBirthday ? true : CalendarFetcherUtils.isFullDayEvent(event);
189-
190-
Log.debug(`saving event: ${title}`);
191-
newEvents.push({
192-
title: title,
193-
startDate: startMoment.format("x"),
194-
endDate: endMoment.format("x"),
195-
fullDayEvent: fullDay,
196-
recurringEvent: isRecurring,
197-
class: event.class,
198-
firstYear: event.start.getFullYear(),
199-
location: instanceEvent.location || location,
200-
geo: instanceEvent.geo || geo,
201-
description: instanceEvent.description || description
202-
});
101+
if (CalendarFetcherUtils.timeFilterApplies(now, endMoment, eventFilterUntil)) {
102+
continue;
203103
}
104+
105+
const instanceTitle = CalendarFetcherUtils.getTitleFromEvent(instanceEvent);
106+
107+
Log.debug(`saving event: ${instanceTitle}, start: ${startMoment.toDate()}, end: ${endMoment.toDate()}`);
108+
newEvents.push({
109+
title: instanceTitle,
110+
startDate: startMoment.format("x"),
111+
endDate: endMoment.format("x"),
112+
fullDayEvent: isFullDay,
113+
recurringEvent: isRecurring,
114+
class: event.class,
115+
firstYear: event.start.getFullYear(),
116+
location: CalendarFetcherUtils.unwrapParameterValue(instanceEvent.location) || location,
117+
geo: instanceEvent.geo || geo,
118+
description: CalendarFetcherUtils.unwrapParameterValue(instanceEvent.description) || description
119+
});
204120
}
205121
});
206122

@@ -217,35 +133,21 @@ const CalendarFetcherUtils = {
217133
* @returns {string} The title of the event, or "Event" if no title is found.
218134
*/
219135
getTitleFromEvent (event) {
220-
let title = "Event";
221-
if (event.summary) {
222-
title = typeof event.summary.val !== "undefined" ? event.summary.val : event.summary;
223-
} else if (event.description) {
224-
title = event.description;
225-
}
226-
227-
return title;
136+
return CalendarFetcherUtils.unwrapParameterValue(event.summary || event.description) || "Event";
228137
},
229138

230139
/**
231-
* Checks if an event is a fullday event.
232-
* @param {object} event The event object to check.
233-
* @returns {boolean} True if the event is a fullday event, false otherwise
140+
* Extracts the string value from a node-ical ParameterValue object ({val, params})
141+
* or returns the value as-is if it is already a plain string.
142+
* This handles ICS properties with parameters, e.g. DESCRIPTION;LANGUAGE=de:Text.
143+
* @param {string|object} value The raw value from node-ical
144+
* @returns {string|object} The unwrapped string value, or the original value if not a ParameterValue
234145
*/
235-
isFullDayEvent (event) {
236-
if (event.start.length === 8 || event.start.dateOnly || event.datetype === "date") {
237-
return true;
238-
}
239-
240-
const start = event.start || 0;
241-
const startDate = new Date(start);
242-
const end = event.end || 0;
243-
if ((end - start) % (24 * 60 * 60 * 1000) === 0 && startDate.getHours() === 0 && startDate.getMinutes() === 0) {
244-
// Is 24 hours, and starts on the middle of the night.
245-
return true;
146+
unwrapParameterValue (value) {
147+
if (value && typeof value === "object" && typeof value.val !== "undefined") {
148+
return value.val;
246149
}
247-
248-
return false;
150+
return value;
249151
},
250152

251153
/**
@@ -262,7 +164,7 @@ const CalendarFetcherUtils = {
262164
increment = until[1].slice(-1) === "s" ? until[1] : `${until[1]}s`, // Massage the data for moment js
263165
filterUntil = moment(endDate.format()).subtract(value, increment);
264166

265-
return now < filterUntil;
167+
return now.isBefore(filterUntil);
266168
}
267169

268170
return false;
@@ -282,7 +184,7 @@ const CalendarFetcherUtils = {
282184
// Assume if leading slash, there is also trailing slash
283185
if (filter[0] === "/") {
284186
// Strip leading and trailing slashes
285-
regexFilter = filter.substr(1).slice(0, -1);
187+
regexFilter = filter.slice(1, -1);
286188
}
287189
return new RegExp(regexFilter, regexFlags).test(title);
288190
} else {
@@ -291,65 +193,38 @@ const CalendarFetcherUtils = {
291193
},
292194

293195
/**
294-
* Expands a recurring event into individual event instances.
196+
* Expands a recurring event into individual event instances using node-ical.
197+
* Handles RRULE expansion, EXDATE filtering, RECURRENCE-ID overrides, and ongoing events.
295198
* @param {object} event The recurring event object
296199
* @param {moment.Moment} pastLocalMoment The past date limit
297200
* @param {moment.Moment} futureLocalMoment The future date limit
298-
* @param {number} durationMs The duration of the event in milliseconds
299-
* @returns {object[]} Array of event instances
201+
* @returns {object[]} Array of event instances with startMoment/endMoment in the local timezone
300202
*/
301-
expandRecurringEvent (event, pastLocalMoment, futureLocalMoment, durationMs) {
302-
const moments = CalendarFetcherUtils.getMomentsFromRecurringEvent(event, pastLocalMoment, futureLocalMoment, durationMs);
303-
const instances = [];
304-
305-
for (const startMoment of moments) {
306-
let curEvent = event;
307-
let showRecurrence = true;
308-
let recurringEventStartMoment = startMoment.clone().tz(CalendarFetcherUtils.getLocalTimezone());
309-
let recurringEventEndMoment = recurringEventStartMoment.clone().add(durationMs, "ms");
310-
311-
// For full-day events, use local date components to match node-ical's getDateKey behavior
312-
// For timed events, use UTC to match ISO string slice
313-
const isFullDay = CalendarFetcherUtils.isFullDayEvent(event);
314-
const dateKey = isFullDay
315-
? recurringEventStartMoment.format("YYYY-MM-DD")
316-
: recurringEventStartMoment.tz("UTC").format("YYYY-MM-DD");
317-
318-
// Check for overrides
319-
if (curEvent.recurrences !== undefined) {
320-
if (curEvent.recurrences[dateKey] !== undefined) {
321-
curEvent = curEvent.recurrences[dateKey];
322-
// Re-calculate start/end based on override
323-
const start = curEvent.start;
324-
const end = curEvent.end;
325-
const localTimezone = CalendarFetcherUtils.getLocalTimezone();
326-
327-
recurringEventStartMoment = (start.tz ? moment(start).tz(start.tz) : moment(start)).tz(localTimezone);
328-
recurringEventEndMoment = (end.tz ? moment(end).tz(end.tz) : moment(end)).tz(localTimezone);
329-
}
330-
}
331-
332-
// Check for exceptions
333-
if (curEvent.exdate !== undefined) {
334-
if (curEvent.exdate[dateKey] !== undefined) {
335-
showRecurrence = false;
203+
expandRecurringEvent (event, pastLocalMoment, futureLocalMoment) {
204+
const localTimezone = CalendarFetcherUtils.getLocalTimezone();
205+
206+
return ical
207+
.expandRecurringEvent(event, {
208+
from: pastLocalMoment.toDate(),
209+
to: futureLocalMoment.toDate(),
210+
includeOverrides: true,
211+
excludeExdates: true,
212+
expandOngoing: true
213+
})
214+
.map((inst) => {
215+
let startMoment, endMoment;
216+
if (inst.isFullDay) {
217+
startMoment = moment.tz([inst.start.getFullYear(), inst.start.getMonth(), inst.start.getDate()], localTimezone);
218+
endMoment = moment.tz([inst.end.getFullYear(), inst.end.getMonth(), inst.end.getDate()], localTimezone);
219+
} else {
220+
startMoment = moment(inst.start).tz(localTimezone);
221+
endMoment = moment(inst.end).tz(localTimezone);
336222
}
337-
}
338-
339-
if (recurringEventStartMoment.valueOf() === recurringEventEndMoment.valueOf()) {
340-
recurringEventEndMoment = recurringEventEndMoment.endOf("day");
341-
}
342-
343-
if (showRecurrence) {
344-
instances.push({
345-
event: curEvent,
346-
startMoment: recurringEventStartMoment,
347-
endMoment: recurringEventEndMoment,
348-
isRecurring: true
349-
});
350-
}
351-
}
352-
return instances;
223+
// Events without DTEND (e.g. reminders) get start === end from node-ical;
224+
// extend to end-of-day so they remain visible on the calendar.
225+
if (startMoment.valueOf() === endMoment.valueOf()) endMoment = endMoment.endOf("day");
226+
return { event: inst.event, startMoment, endMoment, isRecurring: inst.isRecurring, isFullDay: inst.isFullDay };
227+
});
353228
},
354229

355230
/**

‎eslint.config.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ export default defineConfig([
144144
],
145145
"vitest/max-nested-describe": ["error", { max: 3 }],
146146
"vitest/prefer-to-be": "error",
147-
"vitest/prefer-to-have-length": "error"
147+
"vitest/prefer-to-have-length": "error",
148+
"max-lines-per-function": "off"
148149
}
149150
},
150151
{

0 commit comments

Comments
 (0)