[calendar] fix: prevent excessive fetching on client reload and refactor calendarfetcherutils.js

[KristjanESPERANTO]

The bottom line of this PR is, that it fixes an issue and simplifies the code by dealing with the TODOs in the code.

For review, I suggest looking at each commit individually. If there are too many changes for a PR, let me know and I'll split it up 🙂

1. fix(calendar): prevent excessive fetching with smart refresh strategy

• Add lastFetch timestamp tracking to CalendarFetcher
• Add shouldRefetch() method with configurable minimum interval (default: 3 minutes)
• When reusing existing fetcher: fetch if data is stale (>3 min), otherwise broadcast cached events
• Prevents double broadcasts to consuming modules while maintaining fresh data
• Balances rate limit prevention (Issue [Bug] CALENDAR_EVENTS are being resent all the time with no updates #3971) with data freshness on user reload
• Prevents excessive fetching during rapid reloads (e.g., Fully Kiosk screensaver use case)
• Allows fresh calendar data when enough time has passed since last fetch

2. refactor(calendar): simplify event exclusion logic

• Extract filtering logic from shouldEventBeExcluded into new helper checkEventAgainstFilter
• Simplify the main loop in `shouldEventBeExcluded

It resolves a TODO from the comments in the code:

• This seems like an overly complicated way to exclude events based on the title.

3. refactor(calendar): extract recurring event expansion logic

This change separates the expansion of recurring events from the main filtering loop into a new helper function 'expandRecurringEvent'.

It resolves two TODOs from the comments in the code:

• This should be a separate function
• This should create an event per moment so we can change anything we want

This improves code readability, reduces complexity in 'filterEvents', and allows for cleaner handling of individual recurrence instances.

4. refactor(calendar): simplify recurring event handling

• Simplify 'getMomentsFromRecurringEvent' using modern syntax
• Improve handling of full-day events across different timezones

5. test(calendar): fix UNTIL date in fullday_until.ics fixture

The issue was with the UNTIL date being May 4th while DTSTART was May 5th. This created an invalid recurrence rule where the end date came before the start date.

The fix only adjusts the UNTIL date from May 4th to May 5th, so it matches the start date.

[sdetweil]

But 1 is how we get calendar updates. Resending the cached events doesn’t keep the calendar up to date

[KristjanESPERANTO]

But 1 is how we get calendar updates. Resending the cached events doesn’t keep the calendar up to date

Very good catch! Many thanks! In the meantime I've updated the implementation in commit 182d3dd to address exactly that concern.

Now when the calendar is reloaded, it always broadcasts the cached events immediately (for fast UI response), and then checks if the data is older than 3 minutes - if so, it fetches fresh data in the background. This way you get instant display with cached data, plus automatic updates when needed, while still preventing rate limit issues from rapid reloads. I couldn't think of a simpler approach.

---

I'm thinking about that I should split up this PR since it touches several points. Then we could look at each individual point more easily and decide whether we merge them or not. What do you think?

[sdetweil]

Do you know what the impact of the multiple broadcasts does to the using modules? Ext3, calendarMonthly

ext3 has a 10 min (default) refresh cycle

[KristjanESPERANTO]

Do you know what the impact of the multiple broadcasts does to the using modules? Ext3, calendarMonthly

Again a good point! I've just made a small adjustment to optimize this. As it stands now, it shouldn't have any noticeable effect on ext3 and similar modules.

Calendar Fetch Strategy Comparison

Key Differences

Aspect	Before	After (< 3 min)	After (> 3 min)
Fetch on reload	✅ Always	❌ No	✅ Yes
Broadcasts per reload	1	1	1
Rate limit risk	❌ High	✅ None	✅ Very low
Data freshness	✅ Always fresh	⚠️ Cached	✅ Fresh
Ext3 processing	1x	1x	1x
Network requests	Every reload	None	When needed

Impact on Ext3 and Other Modules

Before:

• Every page reload → 1 fetch → 1 broadcast → 1x processing (but too many fetches)

After:

• Every page reload → 0 or 1 fetch → 1 broadcast → 1x processing
• Ext3's 10-minute refresh cycle remains independent and unaffected

[KristjanESPERANTO]

Since calendars don't change that often, we could even set the default cache limit time higher. Maybe 5 or 10 minutes. And perhaps even offer it as an option that users can adjust.

[sdetweil]

On this
if (isFullDayEvent && rule.options?.until) {
rule.options.until = moment(rule.options.until).endOf("day").toDate();
}
I used start of day vs end of, cause end of could appear as tomorrow in some timezones. Start of day will never be wrong.

[KristjanESPERANTO]

Good point about timezones. But I think endOf("day") shouldn't cause issues in this case because it operates on the same calendar day - it just moves the time to 23:59:59 of that same day, never into the next day.

So even if someone is in a timezone like UTC+12, endOf("day") on May 5th stays May 5th 23:59:59 - it can't "appear as tomorrow" because we're not adding time, just extending to the end of the current day.

That said, this is admittedly a workaround for a deeper issue in rrule.js itself. The library has known problems with UNTIL and timezone handling (see rrule#596, open since Aug 2023), and unfortunately rrule.js appears to be unmaintained at this point (~180 open issues).

I'm working on replacing rrule.js with rrule-temporal in node-ical to solve these problems at the root. rrule-temporal is built on the Temporal API which handles timezones correctly by design. Since this is a major change, it will take a while to implement it, so unfortunately I think we will need to use workarounds like this for the time being.

[sdetweil]

Remember full day doesn’t have time, I ran into that here and in the front end

[KristjanESPERANTO]

Yes, it's still necessary. Here's a test script that demonstrates the bug:

const { RRule } = require('rrule');
const moment = require('moment-timezone');

// Full-day event: daily at 00:00 UTC, starting March 10
const dtstartUTC = new Date(Date.UTC(2024, 2, 10, 0, 0, 0));

// UNTIL=20240315 parsed in Europe/Berlin (UTC+1)
// "March 15 00:00 Berlin" = "March 14 23:00 UTC"
const untilRaw = moment.tz('2024-03-15', 'Europe/Berlin').toDate();
const untilFixed = moment.tz('2024-03-15', 'Europe/Berlin').endOf('day').toDate();

const rule1 = new RRule({ freq: RRule.DAILY, dtstart: dtstartUTC, until: untilRaw });
const rule2 = new RRule({ freq: RRule.DAILY, dtstart: dtstartUTC, until: untilFixed });

console.log('WITHOUT endOf("day"):', rule1.all().length, 'days, last:', rule1.all().pop()?.toISOString().slice(0,10));
console.log('WITH endOf("day"):   ', rule2.all().length, 'days, last:', rule2.all().pop()?.toISOString().slice(0,10));

Output:

WITHOUT endOf("day"): 5 days, last: 2024-03-14
WITH endOf("day"):    6 days, last: 2024-03-15

The issue: When rrule.js checks if March 15 should be included, it compares:

• Event time: 00:00:00 UTC on March 15
• UNTIL: 23:00:00 UTC on March 14 (because of timezone conversion)

Since 23:00 < 00:00, March 15th is excluded. The endOf("day") fix ensures UNTIL is always after the event time on that day.

[sdetweil]

Seems ok.

[KristjanESPERANTO]

Seems ok.

Great. It would be awesome if you'd merge it 🙂