Skip to content

Commit 604f787

Browse files
os-zhuangclaude
andcommitted
feat(reports): report schedules honor cron_expression + timezone (#1983)
sys_report_schedule has long carried `cron_expression` and `timezone` fields, but ReportService only ever advanced next_run_at by `interval_minutes` — both fields were stored and ignored. Scheduling now computes next_run_at from the cron expression (when present) in the schedule's timezone via croner — the same library the job scheduler uses — so "every weekday 09:00 local" is expressible. interval_minutes remains the fallback. - cron_expression wins over interval_minutes when set (the field's documented contract); evaluated in `timezone` (default UTC). - scheduleReport validates the cron eagerly → VALIDATION_FAILED on bad input, rather than silently falling back at sweep time. A cron that becomes unschedulable later is logged and falls back to interval; it never throws into the dispatch sweep. - Shared nextRunAt() helper drives both initial schedule and advance. DB-polling dispatch model unchanged; only the next-run computation is cron-aware. Self-contained slice of ADR-0053 Phase 2 — report schedules run under a system context, so the tz source is the schedule's own field, independent of the reference-timezone resolver. Tests: cron drives/overrides interval; honors timezone (09:00 ET = 14:00Z); invalid cron rejected; dispatchDue advances to next cron occurrence. Full plugin-reports suite green (29). Closes #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 67c29ee commit 604f787

6 files changed

Lines changed: 111 additions & 9 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/plugin-reports": minor
3+
---
4+
5+
feat(reports): report schedules honor `cron_expression` + `timezone`
6+
7+
`sys_report_schedule` has long carried `cron_expression` and `timezone` fields, but `ReportService` only ever advanced `next_run_at` by `interval_minutes` — both fields were stored and ignored. Scheduling now computes `next_run_at` from the cron expression (when present) in the schedule's timezone via `croner` — the same library the job scheduler uses — so "every weekday at 09:00 local" is expressible. `interval_minutes` remains the fallback.
8+
9+
- `cron_expression` wins over `interval_minutes` when set (the field's documented contract).
10+
- Evaluated in `timezone` (default `UTC`).
11+
- `scheduleReport` validates the cron expression eagerly and rejects an invalid one with `VALIDATION_FAILED`, rather than silently falling back at sweep time. A cron that becomes unschedulable later is logged and falls back to the interval — it never throws into the dispatch sweep.
12+
13+
The DB-polling dispatch model is unchanged; only the next-run computation is cron-aware. Part of ADR-0053 Phase 2 (#1983) but self-contained — report schedules run under a system context, so the timezone source is the schedule's own field, independent of the reference-timezone resolver.

packages/platform-objects/src/audit/sys-report-schedule.object.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ import { ObjectSchema, Field } from '@objectstack/spec/data';
99
* the reports plugin can deliver "daily pipeline digest" / "weekly
1010
* lead summary" without a separate workflow.
1111
*
12-
* Scheduling: MVP supports `interval_minutes` only (1440 = daily,
13-
* 10080 = weekly). The `cron_expression` field is reserved for the
14-
* follow-up that wires `CronJobAdapter` — when present and the
15-
* adapter is available, it wins over `interval_minutes`.
12+
* Scheduling: `interval_minutes` (1440 = daily, 10080 = weekly) or a
13+
* `cron_expression`. When a `cron_expression` is set it wins over
14+
* `interval_minutes` and is evaluated in `timezone` (default UTC) via
15+
* croner — so "every weekday 09:00 local" is expressible. `next_run_at`
16+
* is computed from whichever applies.
1617
*
1718
* Delivery: when the master dispatch job ticks (every minute by
1819
* default), every schedule with `next_run_at <= now` is loaded,

packages/plugins/plugin-reports/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919
"dependencies": {
2020
"@objectstack/core": "workspace:*",
2121
"@objectstack/platform-objects": "workspace:*",
22-
"@objectstack/spec": "workspace:*"
22+
"@objectstack/spec": "workspace:*",
23+
"croner": "^10.0.1"
2324
},
2425
"devDependencies": {
2526
"@types/node": "^25.9.3",

packages/plugins/plugin-reports/src/report-service.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,49 @@ describe('ReportService', () => {
237237
expect(s.recipients).toBe('x@t');
238238
});
239239

240+
it('scheduleReport: cron_expression drives next_run_at and overrides interval (#1983)', async () => {
241+
const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
242+
// now = 2026-01-15T10:00:00Z (past 09:00); daily-9am cron → tomorrow 09:00Z.
243+
const s = await svc.scheduleReport({
244+
reportId: r.id, recipients: ['x@t'], intervalMinutes: 60, cronExpression: '0 9 * * *',
245+
}, CTX);
246+
expect(s.cron_expression).toBe('0 9 * * *');
247+
expect(s.next_run_at).toBe('2026-01-16T09:00:00.000Z');
248+
// …not the interval's now + 60m.
249+
expect(s.next_run_at).not.toBe(new Date(now.getTime() + 60 * 60_000).toISOString());
250+
});
251+
252+
it('scheduleReport: cron honors the schedule timezone (#1983)', async () => {
253+
const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
254+
// 09:00 America/New_York (UTC-5 in January) = 14:00Z; now=05:00 ET → same day.
255+
const s = await svc.scheduleReport({
256+
reportId: r.id, recipients: ['x@t'], cronExpression: '0 9 * * *', timezone: 'America/New_York',
257+
}, CTX);
258+
expect(s.next_run_at).toBe('2026-01-15T14:00:00.000Z');
259+
});
260+
261+
it('scheduleReport: rejects an invalid cron_expression (#1983)', async () => {
262+
const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
263+
await expect(svc.scheduleReport({
264+
reportId: r.id, recipients: ['x@t'], cronExpression: 'not a cron',
265+
}, CTX)).rejects.toThrow(/VALIDATION_FAILED/);
266+
});
267+
268+
it('dispatchDue: advances a cron schedule to the next cron occurrence (#1983)', async () => {
269+
const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
270+
await svc.scheduleReport({
271+
reportId: r.id, recipients: ['x@t'], cronExpression: '0 9 * * *', format: 'csv',
272+
}, CTX);
273+
// Force due, then dispatch at `now`.
274+
engine._tables['sys_report_schedule'][0].next_run_at = new Date(now.getTime() - 1000).toISOString();
275+
const result = await svc.dispatchDue();
276+
expect(result.fired).toBe(1);
277+
const advanced = engine._tables['sys_report_schedule'][0];
278+
expect(advanced.last_status).toBe('ok');
279+
// Advanced via cron (tomorrow 09:00Z), not now + interval.
280+
expect(advanced.next_run_at).toBe('2026-01-16T09:00:00.000Z');
281+
});
282+
240283
it('listSchedules + unscheduleReport', async () => {
241284
const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
242285
const s = await svc.scheduleReport({ reportId: r.id, recipients: ['x@t'] }, CTX);

packages/plugins/plugin-reports/src/report-service.ts

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type {
1111
ScheduleReportInput,
1212
SharingExecutionContext,
1313
} from '@objectstack/spec/contracts';
14+
import { Cron } from 'croner';
1415

1516
/**
1617
* Narrow engine surface — keeps the service testable without booting
@@ -342,14 +343,27 @@ export class ReportService implements IReportService {
342343

343344
const now = this.clock.now();
344345
const interval = input.intervalMinutes ?? DEFAULT_INTERVAL_MIN;
345-
const nextRun = new Date(now.getTime() + interval * 60_000).toISOString();
346+
const cron = input.cronExpression?.trim() || null;
347+
if (cron) {
348+
// Validate eagerly so an author gets a clear error at schedule time
349+
// instead of a schedule that silently falls back to interval on sweep.
350+
try {
351+
new Cron(cron, { timezone: input.timezone || 'UTC' });
352+
} catch (err) {
353+
throw new Error(`VALIDATION_FAILED: invalid cron_expression '${cron}': ${(err as Error).message}`);
354+
}
355+
}
356+
const nextRun = this.nextRunAt(
357+
{ cron_expression: cron, interval_minutes: interval, timezone: input.timezone ?? 'UTC' },
358+
now,
359+
).toISOString();
346360
const id = uid('rsch');
347361
const row: any = {
348362
id,
349363
report_id: input.reportId,
350364
name: input.name ?? null,
351365
interval_minutes: interval,
352-
cron_expression: input.cronExpression ?? null,
366+
cron_expression: cron,
353367
timezone: input.timezone ?? 'UTC',
354368
active: input.active !== false,
355369
recipients: input.recipients.join(','),
@@ -459,9 +473,36 @@ export class ReportService implements IReportService {
459473
return { fired, failed, skipped };
460474
}
461475

462-
private async advanceSchedule(schedule: ReportSchedule, ranAt: string): Promise<void> {
476+
/**
477+
* Compute the next fire time for a schedule. A `cron_expression` wins over
478+
* `interval_minutes` (the documented `sys_report_schedule` contract) and is
479+
* evaluated in the schedule's `timezone` (default UTC) via croner — the same
480+
* library the job scheduler uses. Falls back to `from + interval_minutes` for
481+
* interval schedules, and also if a cron expression is invalid or has no
482+
* future occurrence (logged; never throws into the sweep). `from` is the
483+
* reference instant (the injected clock), so `today()`-style boundaries honor
484+
* the test clock.
485+
*/
486+
private nextRunAt(
487+
schedule: { cron_expression?: string | null; interval_minutes?: number | null; timezone?: string | null },
488+
from: Date,
489+
): Date {
490+
const cron = (schedule.cron_expression ?? '').trim();
491+
if (cron) {
492+
try {
493+
const next = new Cron(cron, { timezone: schedule.timezone || 'UTC' }).nextRun(from);
494+
if (next) return next;
495+
this.logger.warn?.(`ReportService: cron '${cron}' has no next occurrence; falling back to interval`);
496+
} catch (err) {
497+
this.logger.warn?.(`ReportService: invalid cron '${cron}'; falling back to interval`, err);
498+
}
499+
}
463500
const interval = schedule.interval_minutes ?? DEFAULT_INTERVAL_MIN;
464-
const nextRun = new Date(this.clock.now().getTime() + interval * 60_000).toISOString();
501+
return new Date(from.getTime() + interval * 60_000);
502+
}
503+
504+
private async advanceSchedule(schedule: ReportSchedule, ranAt: string): Promise<void> {
505+
const nextRun = this.nextRunAt(schedule, this.clock.now()).toISOString();
465506
await this.engine.update('sys_report_schedule', {
466507
id: schedule.id,
467508
next_run_at: nextRun,

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)