From ee2281fe8145e80206191df64c8a93c581547136 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Fri, 5 Jun 2026 14:33:00 -0400 Subject: [PATCH] fix(02): correct DURATION-only event end in expand.ts (BUG 1) - Replace dtend ?? dtstart with event.endDate which handles DURATION-only VEVENTs - Add positive-duration guard (PT30M / P1D) to both non-recurring and recurring branches - Add single-duration.ics fixture and regression test asserting end > start for DURATION-only events --- apps/api/src/broker/expand.ts | 28 ++++++++++++++-- apps/api/tests/broker/expand.test.ts | 36 +++++++++++++++++++++ apps/api/tests/fixtures/single-duration.ics | 28 ++++++++++++++++ 3 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 apps/api/tests/fixtures/single-duration.ics diff --git a/apps/api/src/broker/expand.ts b/apps/api/src/broker/expand.ts index 3a7f016..a38f58b 100644 --- a/apps/api/src/broker/expand.ts +++ b/apps/api/src/broker/expand.ts @@ -207,9 +207,22 @@ export function expandOccurrences( // --- 4. Non-recurring event: single occurrence check --- if (!event.isRecurring()) { if (dtstart.compare(rangeStart) >= 0 && dtstart.compare(rangeEnd) < 0) { - const dtend = (vevent.getFirstPropertyValue('dtend') as ICAL.Time | null) ?? dtstart + // Use ICAL.Event.endDate which derives end from DTEND, or DTSTART+DURATION, or sensible default. + // Do NOT use getFirstPropertyValue('dtend') directly — events with only DURATION set return null, + // producing zero-duration occurrences (BUG 1). + let occEnd: ICAL.Time = (event.endDate ?? dtstart) as ICAL.Time + + // Positive-duration guard: ensure timed events have non-zero height in Schedule-X. + if (!allDay && occEnd.compare(dtstart) <= 0) { + occEnd = dtstart.clone() + occEnd.addDuration(ICAL.Duration.fromString('PT30M')) + } else if (allDay && occEnd.compare(dtstart) <= 0) { + occEnd = dtstart.clone() + occEnd.addDuration(ICAL.Duration.fromString('P1D')) + } + const start = serializeTime(dtstart, allDay) - const end = serializeTime(dtend, allDay) + const end = serializeTime(occEnd, allDay) occurrences.push({ id: makeOccurrenceId(uid, dtstart), uid, @@ -240,9 +253,18 @@ export function expandOccurrences( // Compute occurrence end from event duration const duration = event.duration - const occEnd = next.clone() + let occEnd = next.clone() occEnd.addDuration(duration) + // Positive-duration guard: same logic as non-recurring branch above. + if (!allDay && occEnd.compare(next) <= 0) { + occEnd = next.clone() + occEnd.addDuration(ICAL.Duration.fromString('PT30M')) + } else if (allDay && occEnd.compare(next) <= 0) { + occEnd = next.clone() + occEnd.addDuration(ICAL.Duration.fromString('P1D')) + } + const start = serializeTime(next, allDay) const end = serializeTime(occEnd, allDay) diff --git a/apps/api/tests/broker/expand.test.ts b/apps/api/tests/broker/expand.test.ts index a80d333..0267e6e 100644 --- a/apps/api/tests/broker/expand.test.ts +++ b/apps/api/tests/broker/expand.test.ts @@ -140,6 +140,42 @@ describe('expandOccurrences', () => { }) }) + describe('Non-recurring DURATION-only event — single-duration.ics', () => { + it('BUG-1 regression: non-recurring event with DURATION but no DTEND has end strictly after start', () => { + // Real Fastmail events use DURATION (not DTEND). Before the fix, the NON-RECURRING branch + // used getFirstPropertyValue('dtend') ?? dtstart, which returned dtstart when DTEND was absent, + // producing zero-duration occurrences (invisible in week/day views). + const rawVevent = loadFixture('single-duration.ics') + const windowStart = new Date('2026-06-01T00:00:00Z') + const windowEnd = new Date('2026-07-01T00:00:00Z') + + const occurrences = expandOccurrences( + rawVevent, + windowStart, + windowEnd, + 1, + 'My Calendar', + 1, + '#4A90D9', + false, + ) + + expect(occurrences.length).toBe(1) + + const occ = occurrences[0] + expect(occ.allDay).toBe(false) + + // Parse both via Temporal.ZonedDateTime and assert end > start + const startZdt = Temporal.ZonedDateTime.from(occ.start) + const endZdt = Temporal.ZonedDateTime.from(occ.end) + expect(Temporal.ZonedDateTime.compare(endZdt, startZdt)).toBeGreaterThan(0) + + // Verify the actual duration is correct: DURATION:PT1H → end is 1 hour after start + expect(endZdt.hour - startZdt.hour).toBe(1) + expect(endZdt.minute).toBe(startZdt.minute) + }) + }) + describe('Cross-contract: expand output → Temporal.ZonedDateTime.from (regression guard)', () => { it('timed event start/end strings from weekly-dst.ics parse via Temporal.ZonedDateTime.from without throwing', () => { // This is the integration test that was missing. It takes the actual serializeTime output diff --git a/apps/api/tests/fixtures/single-duration.ics b/apps/api/tests/fixtures/single-duration.ics new file mode 100644 index 0000000..46bd923 --- /dev/null +++ b/apps/api/tests/fixtures/single-duration.ics @@ -0,0 +1,28 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//FamilySync//Test//EN +BEGIN:VTIMEZONE +TZID:America/Toronto +BEGIN:STANDARD +DTSTART:19701101T020000 +RRULE:FREQ=YEARLY;BYDAY=1SU;BYMONTH=11 +TZOFFSETFROM:-0400 +TZOFFSETTO:-0500 +TZNAME:EST +END:STANDARD +BEGIN:DAYLIGHT +DTSTART:19700308T020000 +RRULE:FREQ=YEARLY;BYDAY=2SU;BYMONTH=3 +TZOFFSETFROM:-0500 +TZOFFSETTO:-0400 +TZNAME:EDT +END:DAYLIGHT +END:VTIMEZONE +BEGIN:VEVENT +UID:single-duration-only@familysync.test +DTSTART;TZID=America/Toronto:20260618T080000 +DURATION:PT1H +SUMMARY:Morning Standup +DESCRIPTION:Non-recurring timed event with DURATION only, no DTEND +END:VEVENT +END:VCALENDAR