Skip to content

Commit 8f8302f

Browse files
fix: RECURRENCE-ID override must not bleed into other same-day timed instances
Pre-existing bug: storeRecurrenceOverride (ical.js) stores every DATE-TIME RECURRENCE-ID under both an ISO key and a date-only key. The original code resolved overrides using only the date-only key; the previous commit improved this to ISO-key-first with a date-only fallback. The fallback is still wrong when two instances share a calendar date (e.g. BYHOUR=9,15): a miss on the ISO key unambiguously means "no override for this instance", because every properly stored DATE-TIME RECURRENCE-ID carries both keys. Falling back to dateKey would incorrectly apply one occurrence's override to a different occurrence on the same day. Fix: for timed events, use the ISO key only. Full-day events have no ISO key and continue to use dateKey. Also add regression tests for both bugs fixed across commits 3–4: - EXDATE: only the matching timed instance must be excluded when two share a date - RECURRENCE-ID: only the matching timed instance must receive the override
1 parent 501981e commit 8f8302f

2 files changed

Lines changed: 124 additions & 9 deletions

File tree

node-ical.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -498,14 +498,15 @@ function buildRecurringInstance(date, event, isFullDay, baseDurationMs, options)
498498
return null;
499499
}
500500

501-
// For timed events, try the precise ISO key first so that a RECURRENCE-ID
502-
// override for one instance of an event that recurs multiple times per day
503-
// is not mistakenly applied to the other instances on the same date.
504-
// Falls back to the date-only key, matching the dual-key strategy in
505-
// storeRecurrenceOverride (ical.js).
501+
// For timed events use only the precise ISO key: storeRecurrenceOverride (ical.js)
502+
// stores every DATE-TIME RECURRENCE-ID under both the ISO key and the date-only
503+
// key, so a miss on the ISO key unambiguously means "no override for this
504+
// specific instance". Falling back to the date-only key would incorrectly apply
505+
// a different occurrence's override when two instances share the same calendar
506+
// date (e.g. BYHOUR=9,15). Full-day events have no ISO key and use dateKey only.
506507
const isoKey = isFullDay ? null : date.toISOString();
507508
const overrideEvent = includeOverrides
508-
&& ((isoKey && event.recurrences?.[isoKey]) || event.recurrences?.[dateKey]);
509+
&& (isoKey ? event.recurrences?.[isoKey] : event.recurrences?.[dateKey]);
509510
const isOverride = Boolean(overrideEvent);
510511
const instanceEvent = isOverride ? overrideEvent : event;
511512

test/expand-recurring-event.test.js

Lines changed: 117 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,50 @@ describe('expandRecurringEvent', () => {
291291
const hasNov15 = instances.some(instance => instance.start.toISOString() === '2023-11-16T00:00:00.000Z');
292292
assert.ok(hasNov15, 'Nov 15 16:00 PST instance (2023-11-16T00:00:00Z) should be present');
293293
});
294+
295+
it('should only exclude the exact timed instance when two instances share a calendar date', () => {
296+
// Regression test: when an event generates two instances on the same calendar date
297+
// (e.g. BYHOUR=9,15), only the instance whose timestamp matches the EXDATE should be
298+
// excluded. The old code looked up exdate by date-only key and wrongly excluded both.
299+
const event = {
300+
type: 'VEVENT',
301+
uid: 'exdate-two-per-day@test',
302+
summary: 'Twice Daily',
303+
start: new Date('2025-01-13T09:00:00.000Z'),
304+
end: new Date('2025-01-13T10:00:00.000Z'),
305+
rrule: {
306+
freq: 'DAILY',
307+
between(_start, _end) {
308+
return [
309+
new Date('2025-01-13T09:00:00.000Z'),
310+
new Date('2025-01-13T15:00:00.000Z'),
311+
new Date('2025-01-14T09:00:00.000Z'),
312+
new Date('2025-01-14T15:00:00.000Z'),
313+
];
314+
},
315+
},
316+
// EXDATE targets only the 09:00 instance on Jan 13.
317+
// ical.js stores timed EXDATEs under both the ISO key and the date-only key;
318+
// only the ISO key should be used for timed-event lookup.
319+
exdate: {
320+
'2025-01-13T09:00:00.000Z': new Date('2025-01-13T09:00:00.000Z'),
321+
'2025-01-13': new Date('2025-01-13T09:00:00.000Z'),
322+
},
323+
};
324+
325+
const instances = ical.expandRecurringEvent(event, {
326+
from: new Date('2025-01-13T00:00:00Z'),
327+
to: new Date('2025-01-14T23:59:59Z'),
328+
excludeExdates: true,
329+
});
330+
331+
const isos = new Set(instances.map(i => i.start.toISOString()));
332+
333+
assert.ok(!isos.has('2025-01-13T09:00:00.000Z'), 'Jan 13 09:00 should be excluded (matches EXDATE)');
334+
assert.ok(isos.has('2025-01-13T15:00:00.000Z'), 'Jan 13 15:00 should NOT be excluded (different time)');
335+
assert.ok(isos.has('2025-01-14T09:00:00.000Z'), 'Jan 14 09:00 should be present (different day)');
336+
assert.ok(isos.has('2025-01-14T15:00:00.000Z'), 'Jan 14 15:00 should be present');
337+
});
294338
});
295339

296340
describe('RECURRENCE-ID overrides', () => {
@@ -374,15 +418,24 @@ describe('expandRecurringEvent', () => {
374418
},
375419
},
376420
recurrences: {
377-
// Override for Jan 8 - moved to 14:00
378-
// The key must match the date-only format that ical.js uses as primary key
379-
'2025-01-08': {
421+
// Override for Jan 8 - moved to 14:00.
422+
// ical.js stores RECURRENCE-ID overrides under both the ISO key (the precise
423+
// occurrence timestamp) and the date-only key (dual-key strategy).
424+
'2025-01-08T10:00:00.000Z': {
380425
type: 'VEVENT',
381426
uid: 'test-override-dtstart@test',
382427
summary: 'Daily Meeting (Moved)',
383428
start: new Date('2025-01-08T14:00:00.000Z'), // Different time!
384429
end: new Date('2025-01-08T15:00:00.000Z'),
385430
},
431+
// Date-only key alias (mirrors what ical.js stores)
432+
'2025-01-08': {
433+
type: 'VEVENT',
434+
uid: 'test-override-dtstart@test',
435+
summary: 'Daily Meeting (Moved)',
436+
start: new Date('2025-01-08T14:00:00.000Z'),
437+
end: new Date('2025-01-08T15:00:00.000Z'),
438+
},
386439
},
387440
};
388441

@@ -401,6 +454,67 @@ describe('expandRecurringEvent', () => {
401454
assert.strictEqual(jan8Instance.end.getUTCHours(), 15, 'Should use override end time (15:00)');
402455
assert.strictEqual(jan8Instance.summary, 'Daily Meeting (Moved)');
403456
});
457+
458+
it('should apply RECURRENCE-ID override only to the matching timed instance when two share a date', () => {
459+
// Regression test: when two instances fall on the same calendar date, a RECURRENCE-ID
460+
// override keyed to the 09:00 instance must not bleed into the 15:00 instance.
461+
// The old code resolved overrides by date-only key, returning the same override for both.
462+
const event = {
463+
type: 'VEVENT',
464+
uid: 'recurrence-id-two-per-day@test',
465+
summary: 'Base Event',
466+
start: new Date('2025-01-08T09:00:00.000Z'),
467+
end: new Date('2025-01-08T10:00:00.000Z'),
468+
rrule: {
469+
freq: 'DAILY',
470+
between(_start, _end) {
471+
return [
472+
new Date('2025-01-08T09:00:00.000Z'),
473+
new Date('2025-01-08T15:00:00.000Z'),
474+
];
475+
},
476+
},
477+
// Override only the 09:00 instance.
478+
// ical.js stores RECURRENCE-ID overrides under both the full ISO key
479+
// and the date-only key; the ISO key must take precedence in lookup.
480+
recurrences: {
481+
'2025-01-08T09:00:00.000Z': {
482+
type: 'VEVENT',
483+
uid: 'recurrence-id-two-per-day@test',
484+
summary: 'Morning Override',
485+
start: new Date('2025-01-08T09:00:00.000Z'),
486+
end: new Date('2025-01-08T10:00:00.000Z'),
487+
},
488+
'2025-01-08': {
489+
type: 'VEVENT',
490+
uid: 'recurrence-id-two-per-day@test',
491+
summary: 'Morning Override',
492+
start: new Date('2025-01-08T09:00:00.000Z'),
493+
end: new Date('2025-01-08T10:00:00.000Z'),
494+
},
495+
},
496+
};
497+
498+
const instances = ical.expandRecurringEvent(event, {
499+
from: new Date('2025-01-08T00:00:00Z'),
500+
to: new Date('2025-01-08T23:59:59Z'),
501+
includeOverrides: true,
502+
});
503+
504+
assert.strictEqual(instances.length, 2, 'Both instances should be present');
505+
506+
const morning = instances.find(i => i.start.getUTCHours() === 9);
507+
const afternoon = instances.find(i => i.start.getUTCHours() === 15);
508+
509+
assert.ok(morning, 'Morning instance should exist');
510+
assert.ok(afternoon, 'Afternoon instance should exist');
511+
512+
assert.strictEqual(morning.isOverride, true, 'Morning should use override');
513+
assert.strictEqual(morning.event.summary, 'Morning Override', 'Morning should have override summary');
514+
515+
assert.strictEqual(afternoon.isOverride, false, 'Afternoon should NOT use override');
516+
assert.strictEqual(afternoon.event.summary, 'Base Event', 'Afternoon should have base event summary');
517+
});
404518
});
405519

406520
describe('DST transitions', () => {

0 commit comments

Comments
 (0)