diff --git a/server/routes/schedules.js b/server/routes/schedules.js index 5c4cb55..3954223 100644 --- a/server/routes/schedules.js +++ b/server/routes/schedules.js @@ -358,33 +358,58 @@ function expandSchedule(schedule, rangeStart, rangeEnd) { } const recEnd = schedule.recurrence_end ? new Date(schedule.recurrence_end) : rangeEnd; - let current = new Date(start); - let count = 0; - const maxIterations = 366; - while (current <= rangeEnd && current <= recEnd && count < maxIterations) { - const instanceEnd = new Date(current.getTime() + durationMs); + // Walk DAY BY DAY across the visible range and draw every day the rule actually fires. + // + // The old loop stepped by the recurrence unit from the schedule's original start, which got both + // of the common presets wrong: + // - WEEKLY advanced a whole week at a time, so dayOfWeek never changed and a + // FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR rule matched only its start day — one event a week, or + // none at all if it had been created on a weekend. + // - Starting from the original start with a 366-iteration cap meant a schedule begun more than + // a year ago never reached the current week, so it drew nothing whatsoever. + // The engine meanwhile evaluates day-of-week directly, so it ran Mon-Fri regardless. The calendar + // is the operator's only view of what is scheduled, and it disagreed with reality in both + // directions. Iterating the range instead means the drawing follows the same rule the engine + // applies, and the cost is bounded by the window being displayed rather than by history. + const dayMs = 24 * 60 * 60 * 1000; + const startTimeOfDay = { h: start.getHours(), m: start.getMinutes(), s: start.getSeconds() }; - if (current >= rangeStart || instanceEnd >= rangeStart) { - const dayOfWeek = current.getDay(); - const matchesDay = !rule.byDay || rule.byDay.includes(dayOfWeek); + // First candidate day: the later of the schedule's start and the window's start. + let cursor = new Date(Math.max(start.getTime(), rangeStart.getTime())); + cursor.setHours(startTimeOfDay.h, startTimeOfDay.m, startTimeOfDay.s, 0); + if (cursor.getTime() + durationMs < rangeStart.getTime()) cursor = new Date(cursor.getTime() + dayMs); - if (matchesDay) { - events.push({ - ...schedule, - instance_start: current.toISOString(), - instance_end: instanceEnd.toISOString() - }); - } - } + const lastDay = new Date(Math.min(rangeEnd.getTime(), recEnd.getTime())); + const interval = Math.max(1, rule.interval || 1); + while (cursor <= lastDay) { + const instanceEnd = new Date(cursor.getTime() + durationMs); + let fires = false; switch (rule.freq) { - case 'DAILY': current.setDate(current.getDate() + (rule.interval || 1)); break; - case 'WEEKLY': current.setDate(current.getDate() + 7 * (rule.interval || 1)); break; - case 'MONTHLY': current.setMonth(current.getMonth() + (rule.interval || 1)); break; - default: current.setDate(current.getDate() + 1); + case 'DAILY': + // Honour the interval by counting whole days from the original start. + fires = Math.floor((cursor - start) / dayMs) % interval === 0; + break; + case 'WEEKLY': + // byDay is what makes Mon-Fri work. Without it, weekly means "the start's weekday". + fires = rule.byDay ? rule.byDay.includes(cursor.getDay()) : cursor.getDay() === start.getDay(); + break; + case 'MONTHLY': + fires = cursor.getDate() === start.getDate(); + break; + default: + fires = true; } - count++; + if (fires && (cursor >= rangeStart || instanceEnd >= rangeStart)) { + events.push({ + ...schedule, + instance_start: cursor.toISOString(), + instance_end: instanceEnd.toISOString(), + }); + } + cursor = new Date(cursor.getTime() + dayMs); + cursor.setHours(startTimeOfDay.h, startTimeOfDay.m, startTimeOfDay.s, 0); // DST-safe re-anchor } return events; @@ -410,3 +435,6 @@ function parseRRule(rrule) { } module.exports = router; +// Exported for testing, the same way playlists.js exports publishPlaylist. The calendar's +// correctness is arithmetic and deserves to be checked without standing up a server. +module.exports.expandSchedule = expandSchedule; diff --git a/server/test/schedule-calendar-expansion.test.js b/server/test/schedule-calendar-expansion.test.js new file mode 100644 index 0000000..4ff6cff --- /dev/null +++ b/server/test/schedule-calendar-expansion.test.js @@ -0,0 +1,91 @@ +'use strict'; + +// The calendar is the operator's only view of what is scheduled, and it disagreed with the engine +// in both directions for the two most-used repeat presets. +// +// The old expansion stepped by the recurrence unit from the schedule's original start: +// - WEEKLY advanced a whole week at a time, so dayOfWeek never changed and a +// FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR rule matched only its start day. Created on a Monday it drew +// one event a week; created on a Saturday it drew nothing at all. +// - The walk began at the original start under a 366-iteration cap, so a schedule begun more than +// a year ago never reached the current week and drew nothing. +// Meanwhile the engine evaluates day-of-week directly, so those schedules ran Mon-Fri the whole +// time. Screens were switching content that the calendar said was not scheduled. +// +// The invariant: the calendar draws an event on every day the schedule actually fires. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'st-cal-')); +process.env.DATA_DIR = tmp; +process.env.JWT_SECRET = 'test-secret-calendar'; + +const { expandSchedule } = require('../routes/schedules'); + +// A Monday-to-Sunday window well clear of the schedules' start dates. +const WEEK_START = new Date('2026-08-03T00:00:00'); // Monday +const WEEK_END = new Date('2026-08-09T23:59:59'); // Sunday + +const mk = (recurrence, startISO, recurrenceEnd = null) => ({ + id: 's', recurrence, recurrence_end: recurrenceEnd, + start_time: startISO, + end_time: new Date(new Date(startISO).getTime() + 60 * 60 * 1000).toISOString(), +}); +const weekdaysOf = (events) => events.map(e => new Date(e.instance_start).getDay()).sort(); + +test('THE BUG: a Mon-Fri rule draws five events, not one', () => { + const ev = expandSchedule(mk('FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR', '2026-07-27T09:00:00'), WEEK_START, WEEK_END); + assert.equal(ev.length, 5); + assert.deepEqual(weekdaysOf(ev), [1, 2, 3, 4, 5], 'Mon..Fri'); +}); + +test('...and it does not matter which day the rule was created on', () => { + // Created on a Saturday, the old code drew nothing whatsoever. + const ev = expandSchedule(mk('FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR', '2026-08-01T09:00:00'), WEEK_START, WEEK_END); + assert.equal(ev.length, 5); + assert.deepEqual(weekdaysOf(ev), [1, 2, 3, 4, 5]); +}); + +test('a DAILY schedule older than a year still draws', () => { + // The 366-iteration cap meant the walk never reached the visible window. + const ev = expandSchedule(mk('FREQ=DAILY', '2024-05-01T09:00:00'), WEEK_START, WEEK_END); + assert.equal(ev.length, 7, 'every day of the week'); +}); + +test('WEEKLY without byDay still means "the same weekday as the start"', () => { + const ev = expandSchedule(mk('FREQ=WEEKLY', '2026-07-27T09:00:00'), WEEK_START, WEEK_END); // a Monday + assert.equal(ev.length, 1); + assert.deepEqual(weekdaysOf(ev), [1]); +}); + +test('a DAILY interval is honoured rather than drawn every day', () => { + const ev = expandSchedule(mk('FREQ=DAILY;INTERVAL=2', '2026-08-03T09:00:00'), WEEK_START, WEEK_END); + assert.equal(ev.length, 4, 'Mon, Wed, Fri, Sun'); +}); + +test('recurrence_end stops the drawing', () => { + const ev = expandSchedule( + mk('FREQ=DAILY', '2026-07-27T09:00:00', '2026-08-05T23:59:59'), WEEK_START, WEEK_END); + assert.equal(ev.length, 3, 'Mon, Tue, Wed then it ends'); +}); + +test('a one-off schedule is unaffected', () => { + const ev = expandSchedule( + { id: 's', recurrence: null, start_time: '2026-08-05T09:00:00', end_time: '2026-08-05T10:00:00' }, + WEEK_START, WEEK_END); + assert.equal(ev.length, 1); +}); + +test('each drawn event keeps the schedule duration', () => { + const ev = expandSchedule(mk('FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR', '2026-07-27T09:00:00'), WEEK_START, WEEK_END); + for (const e of ev) { + const mins = (new Date(e.instance_end) - new Date(e.instance_start)) / 60000; + assert.equal(mins, 60); + } +}); + +test.after(() => { try { fs.rmSync(tmp, { recursive: true, force: true }); } catch (_) {} });