refactor(calendar): improve recurring event handling clarity and robustness

CHANGELOG.md

@@ -41,6 +41,7 @@ Thanks to: @dathbe.
   - Improve test reliability and maintainability
 - [tests] add alert module tests for different welcome_message configurations (#3867)
 - [lint-staged] use `prettier --write --ignore-unknown` in `lint-staged` to avoid errors on unsupported files (#3888)
+- [calendar] refactor: improve recurring event handling clarity and robustness (#3900)
 
 ### Updated
 

modules/default/calendar/calendar.js

@@ -584,10 +584,18 @@ Module.register("calendar", {
 	/**
 	 * converts the given timestamp to a moment with a timezone
 	 * @param {number} timestamp timestamp from an event
+	 * @param {boolean} isFullDayEvent flag indicating whether the timestamp represents an all-day event
+	 * @param {?string} floatingDate canonical YYYY-MM-DD date for floating events
 	 * @returns {moment.Moment} moment with a timezone
 	 */
-	timestampToMoment (timestamp) {
-		return moment(timestamp, "x").tz(moment.tz.guess());
+	timestampToMoment (timestamp, isFullDayEvent = false, floatingDate = null) {
+		const viewerTimezone = moment.tz.guess();
+		if (isFullDayEvent && floatingDate) {
+			return moment.tz(floatingDate, "YYYY-MM-DD", viewerTimezone).startOf("day");
+		}
+		const timestampMs = parseInt(timestamp, 10);
+		const baseMoment = moment.utc(timestampMs);
+		return baseMoment.clone().tz(viewerTimezone, isFullDayEvent);
 	},
 
 	/**
@@ -702,7 +710,8 @@ Module.register("calendar", {
 		 */
 		if (this.config.limitDays > 0 && events.length > 0) { // watch out for initial display before events arrive from helper
 			// Group all events by date, events on the same date will be in a list with the key being the date.
-			const eventsByDate = Object.groupBy(events, (ev) => this.timestampToMoment(ev.startDate).format("YYYY-MM-DD"));
+			const eventsByDate
+				= Object.groupBy(events, (ev) => this.timestampToMoment(ev.startDate).format("YYYY-MM-DD"));
 			const newEvents = [];
 			let currentDate = moment();
 			let daysCollected = 0;

modules/default/calendar/calendarfetcherutils.js

@@ -84,47 +84,85 @@ const CalendarFetcherUtils = {
 	 */
 	getMomentsFromRecurringEvent (event, pastLocalMoment, futureLocalMoment, durationInMs) {
 		const rule = event.rrule;
+		const isFullDay = CalendarFetcherUtils.isFullDayEvent(event);
+		const localTimezone = CalendarFetcherUtils.getLocalTimezone();
 
-		// can cause problems with e.g. birthdays before 1900
-		if ((rule.options && rule.origOptions && rule.origOptions.dtstart && rule.origOptions.dtstart.getFullYear() < 1900) || (rule.options && rule.options.dtstart && rule.options.dtstart.getFullYear() < 1900)) {
+		// rrule.js interprets years < 1900 as offsets from 1900 which breaks parsing for
+		// some imported calendars (notably Google birthday calendars). Normalise those
+		// values before we expand the recurrence window.
+		if (rule.origOptions?.dtstart instanceof Date && rule.origOptions.dtstart.getFullYear() < 1900) {
 			rule.origOptions.dtstart.setYear(1900);
+		}
+		if (rule.options?.dtstart instanceof Date && rule.options.dtstart.getFullYear() < 1900) {
 			rule.options.dtstart.setYear(1900);
 		}
 
-		// subtract the max of the duration of this event or 1 day to find events in the past that are currently still running and should therefor be displayed.
-		const oneDayInMs = 24 * 60 * 60000;
-		let searchFromDate = pastLocalMoment.clone().subtract(Math.max(durationInMs, oneDayInMs), "milliseconds").toDate();
-		let searchToDate = futureLocalMoment.clone().add(1, "days").toDate();
+		// Expand the search window by the event duration (or a full day) so ongoing recurrences are included.
+		// Without this buffer a long-running recurrence that already started before "pastLocalMoment"
+		// would be skipped even though it is still active.
+		const oneDayInMs = 24 * 60 * 60 * 1000;
+		const searchWindowMs = Math.max(durationInMs, oneDayInMs);
+		const searchFromDate = pastLocalMoment.clone().subtract(searchWindowMs, "milliseconds").toDate();
+		const searchToDate = futureLocalMoment.clone().add(1, "days").toDate();
 		Log.debug(`Search for recurring events between: ${searchFromDate} and ${searchToDate}`);
 
-		// if until is set, and its a full day event, force the time to midnight. rrule gets confused with non-00 offset
-		// looks like MS Outlook sets the until time incorrectly for fullday events
-		if ((rule.options.until !== undefined) && CalendarFetcherUtils.isFullDayEvent(event)) {
+		if (isFullDay && rule.options && rule.options.until) {
+			// node-ical supplies "until" in UTC for all-day events; push the date to the end of
+			// that day so the last occurrence is part of the set we request from rrule.js.
 			Log.debug("fixup rrule until");
 			rule.options.until = moment(rule.options.until).clone().startOf("day").add(1, "day")
 				.toDate();
 		}
 
-		Log.debug("fix rrule start=", rule.options.dtstart);
+		Log.debug("fix rrule start=", rule.options?.dtstart);
 		Log.debug("event before rrule.between=", JSON.stringify(event, null, 2), "exdates=", event.exdate);
-
 		Log.debug(`RRule: ${rule.toString()}`);
-		rule.options.tzid = null; // RRule gets *very* confused with timezones
 
-		let dates = rule.between(searchFromDate, searchToDate, true, () => {
-			return true;
-		});
-
-		Log.debug(`Title: ${event.summary}, with dates: \n\n${JSON.stringify(dates)}\n`);
+		if (rule.options) {
+			// Let moment.js handle the timezone conversion afterwards. Keeping tzid here lets
+			// rrule.js double adjust the times which causes one-day drifts.
+			rule.options.tzid = null;
+		}
 
-		// shouldn't need this  anymore, as RRULE not passed junk
-		dates = dates.filter((d) => {
-			return JSON.stringify(d) !== "null";
+		const rawDates = rule.between(searchFromDate, searchToDate, true, () => true) || [];
+		Log.debug(`Title: ${event.summary}, with dates: \n\n${JSON.stringify(rawDates)}\n`);
+
+		const validDates = rawDates.filter(Boolean);
+		return validDates.map((date) => {
+			let occurrenceMoment;
+			let floatingStartDate = null;
+			if (isFullDay) {
+				// Treat DATE-based recurrences as floating dates in their original timezone so they
+				// stay anchored to the same calendar day regardless of where the viewer is located.
+				const floatingZone = event.start?.tz || rule.origOptions?.tzid;
+				if (floatingZone) {
+					const canonicalDate = moment(date).format("YYYY-MM-DD");
+					occurrenceMoment = moment.tz(canonicalDate, "YYYY-MM-DD", floatingZone);
+				} else {
+					occurrenceMoment = moment(date).startOf("day");
+				}
+				floatingStartDate = occurrenceMoment.clone().format("YYYY-MM-DD");
+				if (!event._debugLogged) {
+					event._debugLogged = true;
+					Log.debug("[Calendar] Floating recurrence", {
+						title: CalendarFetcherUtils.getTitleFromEvent(event),
+						rawDate: date,
+						floatingZone: floatingZone,
+						floatingStartDate
+					});
+				}
+			} else {
+				const baseUtcMoment = moment.tz(date, "UTC");
+				if (event.start && event.start.tz) {
+					// Preserve the original start timezone when the ICS explicitly defines one.
+					occurrenceMoment = baseUtcMoment.clone().tz(event.start.tz, true);
+				} else {
+					// Fallback: render in the viewer's local timezone while keeping the absolute instant.
+					occurrenceMoment = baseUtcMoment.clone().tz(localTimezone, true);
+				}
+			}
+			return { occurrence: occurrenceMoment, floatingStartDate: floatingStartDate };
 		});
-
-		// Dates are returned in UTC timezone but with localdatetime because tzid is null.
-		// So we map the date to a moment using the original timezone of the event.
-		return dates.map((d) => (event.start.tz ? moment.tz(d, "UTC").tz(event.start.tz, true) : moment.tz(d, "UTC").tz(CalendarFetcherUtils.getLocalTimezone(), true)));
 	},
 
 	/**
@@ -206,17 +244,29 @@ const CalendarFetcherUtils = {
 				// TODO This should be a seperate function.
 				if (event.rrule && typeof event.rrule !== "undefined" && !isFacebookBirthday) {
 					// Recurring event.
-					let moments = CalendarFetcherUtils.getMomentsFromRecurringEvent(event, pastLocalMoment, futureLocalMoment, durationMs);
+					const occurrences = CalendarFetcherUtils.getMomentsFromRecurringEvent(event, pastLocalMoment, futureLocalMoment, durationMs);
 
 					// Loop through the set of moment entries to see which recurrences should be added to our event list.
 					// TODO This should create an event per moment so we can change anything we want.
-					for (let m in moments) {
+					for (const occurrenceData of occurrences) {
 						let curEvent = event;
 						let showRecurrence = true;
-						let recurringEventStartMoment = moments[m].tz(CalendarFetcherUtils.getLocalTimezone()).clone();
+						let recurringEventStartMoment = occurrenceData.occurrence
+							.clone()
+							.tz(CalendarFetcherUtils.getLocalTimezone(), CalendarFetcherUtils.isFullDayEvent(event));
 						let recurringEventEndMoment = recurringEventStartMoment.clone().add(durationMs, "ms");
 
-						let dateKey = recurringEventStartMoment.tz("UTC").format("YYYY-MM-DD");
+						let floatingStartDate = occurrenceData.floatingStartDate;
+						let floatingEndDate = null;
+						if (floatingStartDate) {
+							let floatingEndMoment = occurrenceData.occurrence.clone().add(durationMs, "ms");
+							if (durationMs === 0) {
+								floatingEndMoment = occurrenceData.occurrence.clone().endOf("day");
+							}
+							floatingEndDate = floatingEndMoment.format("YYYY-MM-DD");
+						}
+
+						let dateKey = recurringEventStartMoment.clone().tz("UTC").format("YYYY-MM-DD");
 
 						Log.debug("event date dateKey=", dateKey);
 						// For each date that we're checking, it's possible that there is a recurrence override for that one day.
@@ -226,16 +276,30 @@ const CalendarFetcherUtils = {
 								Log.debug("have a recurrence match for dateKey=", dateKey);
 								// We found an override, so for this recurrence, use a potentially different title, start date, and duration.
 								curEvent = curEvent.recurrences[dateKey];
+								const recurrenceIsFullDay = CalendarFetcherUtils.isFullDayEvent(curEvent);
 								// Some event start/end dates don't have timezones
 								if (curEvent.start.tz) {
-									recurringEventStartMoment = moment(curEvent.start).tz(curEvent.start.tz).tz(CalendarFetcherUtils.getLocalTimezone());
+									recurringEventStartMoment = moment(curEvent.start).tz(curEvent.start.tz).tz(CalendarFetcherUtils.getLocalTimezone(), recurrenceIsFullDay);
 								} else {
-									recurringEventStartMoment = moment(curEvent.start).tz(CalendarFetcherUtils.getLocalTimezone());
+									recurringEventStartMoment = moment(curEvent.start).tz(CalendarFetcherUtils.getLocalTimezone(), recurrenceIsFullDay);
 								}
 								if (curEvent.end.tz) {
-									recurringEventEndMoment = moment(curEvent.end).tz(curEvent.end.tz).tz(CalendarFetcherUtils.getLocalTimezone());
+									recurringEventEndMoment = moment(curEvent.end).tz(curEvent.end.tz).tz(CalendarFetcherUtils.getLocalTimezone(), recurrenceIsFullDay);
 								} else {
-									recurringEventEndMoment = moment(curEvent.end).tz(CalendarFetcherUtils.getLocalTimezone());
+									recurringEventEndMoment = moment(curEvent.end).tz(CalendarFetcherUtils.getLocalTimezone(), recurrenceIsFullDay);
+								}
+
+								if (recurrenceIsFullDay) {
+									const overrideStart = curEvent.start.tz ? moment(curEvent.start).tz(curEvent.start.tz, true).startOf("day") : moment(curEvent.start).startOf("day");
+									floatingStartDate = overrideStart.format("YYYY-MM-DD");
+									let overrideEnd = curEvent.end ? (curEvent.end.tz ? moment(curEvent.end).tz(curEvent.end.tz, true) : moment(curEvent.end)) : overrideStart.clone();
+									if (overrideStart.valueOf() === overrideEnd.valueOf()) {
+										overrideEnd = overrideEnd.endOf("day");
+									}
+									floatingEndDate = overrideEnd.format("YYYY-MM-DD");
+								} else {
+									floatingStartDate = null;
+									floatingEndDate = null;
 								}
 							} else {
 								Log.debug("recurrence key ", dateKey, " doesn't match");
@@ -252,11 +316,26 @@ const CalendarFetcherUtils = {
 
 						if (recurringEventStartMoment.valueOf() === recurringEventEndMoment.valueOf()) {
 							recurringEventEndMoment = recurringEventEndMoment.endOf("day");
+							if (floatingStartDate && !floatingEndDate) {
+								floatingEndDate = floatingStartDate;
+							}
 						}
 
 						const recurrenceTitle = CalendarFetcherUtils.getTitleFromEvent(curEvent);
+						const fullDayRecurringEvent = CalendarFetcherUtils.isFullDayEvent(curEvent);
+						if (fullDayRecurringEvent) {
+							if (!floatingStartDate) {
+								floatingStartDate = recurringEventStartMoment.clone().format("YYYY-MM-DD");
+							}
+							if (!floatingEndDate) {
+								floatingEndDate = recurringEventEndMoment.clone().format("YYYY-MM-DD");
+							}
+						} else {
+							floatingStartDate = null;
+							floatingEndDate = null;
+						}
 
-						// If this recurrence ends before the start of the date range, or starts after the end of the date range, don"t add
+						// If this recurrence ends before the start of the date range, or starts after the end of the date range, don't add
 						// it to the event list.
 						if (recurringEventEndMoment.isBefore(pastLocalMoment) || recurringEventStartMoment.isAfter(futureLocalMoment)) {
 							showRecurrence = false;
@@ -272,13 +351,15 @@ const CalendarFetcherUtils = {
 								title: recurrenceTitle,
 								startDate: recurringEventStartMoment.format("x"),
 								endDate: recurringEventEndMoment.format("x"),
-								fullDayEvent: CalendarFetcherUtils.isFullDayEvent(event),
+								fullDayEvent: fullDayRecurringEvent,
 								recurringEvent: true,
 								class: event.class,
 								firstYear: event.start.getFullYear(),
 								location: location,
 								geo: geo,
-								description: description
+								description: description,
+								floatingStartDate: floatingStartDate,
+								floatingEndDate: floatingEndDate
 							});
 						} else {
 							Log.debug("not saving event ", recurrenceTitle, eventStartMoment);
@@ -323,6 +404,8 @@ const CalendarFetcherUtils = {
 					}
 
 					// Every thing is good. Add it to the list.
+					const floatingStartDate = fullDayEvent ? eventStartMoment.clone().format("YYYY-MM-DD") : null;
+					const floatingEndDate = fullDayEvent ? eventEndMoment.clone().format("YYYY-MM-DD") : null;
 					newEvents.push({
 						title: title,
 						startDate: eventStartMoment.format("x"),
@@ -333,7 +416,9 @@ const CalendarFetcherUtils = {
 						firstYear: event.start.getFullYear(),
 						location: location,
 						geo: geo,
-						description: description
+						description: description,
+						floatingStartDate: floatingStartDate,
+						floatingEndDate: floatingEndDate
 					});
 				}
 			}

tests/unit/modules/default/calendar/calendar_fetcher_utils_spec.js

@@ -104,13 +104,13 @@ SUMMARY:Test
 TRANSP:OPAQUE
 END:VEVENT`);
 
-			const moments = CalendarFetcherUtils.getMomentsFromRecurringEvent(data["67e65a1d-b889-4451-8cab-5518cecb9c66"], moment(), moment().add(365, "days"));
+			const occurrences = CalendarFetcherUtils.getMomentsFromRecurringEvent(data["67e65a1d-b889-4451-8cab-5518cecb9c66"], moment(), moment().add(365, "days"));
 
-			const januaryFirst = moments.filter((m) => m.format("MM-DD") === "01-01");
-			const julyFirst = moments.filter((m) => m.format("MM-DD") === "07-01");
+			const januaryFirst = occurrences.filter((entry) => entry.occurrence.format("MM-DD") === "01-01");
+			const julyFirst = occurrences.filter((entry) => entry.occurrence.format("MM-DD") === "07-01");
 
-			expect(januaryFirst[0].toISOString(true)).toContain("09:00:00.000+01:00");
-			expect(julyFirst[0].toISOString(true)).toContain("09:00:00.000+02:00");
+			expect(januaryFirst[0].occurrence.toISOString(true)).toContain("09:00:00.000+01:00");
+			expect(julyFirst[0].occurrence.toISOString(true)).toContain("09:00:00.000+02:00");
 		});
 	});
 });