diff --git a/CHANGELOG.md b/CHANGELOG.md index a3fe44968c..4c520ded64 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/modules/default/calendar/calendar.js b/modules/default/calendar/calendar.js index e5b3621ffa..5fa85010dd 100644 --- a/modules/default/calendar/calendar.js +++ b/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; diff --git a/modules/default/calendar/calendarfetcherutils.js b/modules/default/calendar/calendarfetcherutils.js index 729f121ce4..b8f05288e0 100644 --- a/modules/default/calendar/calendarfetcherutils.js +++ b/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 }); } } diff --git a/tests/unit/modules/default/calendar/calendar_fetcher_utils_spec.js b/tests/unit/modules/default/calendar/calendar_fetcher_utils_spec.js index fe3e1261be..de6e5afdd1 100644 --- a/tests/unit/modules/default/calendar/calendar_fetcher_utils_spec.js +++ b/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"); }); }); });