From 604f78762b1c43687693337f0d731dc260cd29c8 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Wed, 17 Jun 2026 06:55:55 +0800 Subject: [PATCH] feat(reports): report schedules honor cron_expression + timezone (#1983) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .changeset/report-schedule-cron-tz.md | 13 +++++ .../src/audit/sys-report-schedule.object.ts | 9 ++-- packages/plugins/plugin-reports/package.json | 3 +- .../plugin-reports/src/report-service.test.ts | 43 ++++++++++++++++ .../plugin-reports/src/report-service.ts | 49 +++++++++++++++++-- pnpm-lock.yaml | 3 ++ 6 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 .changeset/report-schedule-cron-tz.md diff --git a/.changeset/report-schedule-cron-tz.md b/.changeset/report-schedule-cron-tz.md new file mode 100644 index 0000000000..224aab2d7e --- /dev/null +++ b/.changeset/report-schedule-cron-tz.md @@ -0,0 +1,13 @@ +--- +"@objectstack/plugin-reports": minor +--- + +feat(reports): report schedules honor `cron_expression` + `timezone` + +`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. + +- `cron_expression` wins over `interval_minutes` when set (the field's documented contract). +- Evaluated in `timezone` (default `UTC`). +- `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. + +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. diff --git a/packages/platform-objects/src/audit/sys-report-schedule.object.ts b/packages/platform-objects/src/audit/sys-report-schedule.object.ts index 419687f2cd..24e1ec7cf3 100644 --- a/packages/platform-objects/src/audit/sys-report-schedule.object.ts +++ b/packages/platform-objects/src/audit/sys-report-schedule.object.ts @@ -9,10 +9,11 @@ import { ObjectSchema, Field } from '@objectstack/spec/data'; * the reports plugin can deliver "daily pipeline digest" / "weekly * lead summary" without a separate workflow. * - * Scheduling: MVP supports `interval_minutes` only (1440 = daily, - * 10080 = weekly). The `cron_expression` field is reserved for the - * follow-up that wires `CronJobAdapter` — when present and the - * adapter is available, it wins over `interval_minutes`. + * Scheduling: `interval_minutes` (1440 = daily, 10080 = weekly) or a + * `cron_expression`. When a `cron_expression` is set it wins over + * `interval_minutes` and is evaluated in `timezone` (default UTC) via + * croner — so "every weekday 09:00 local" is expressible. `next_run_at` + * is computed from whichever applies. * * Delivery: when the master dispatch job ticks (every minute by * default), every schedule with `next_run_at <= now` is loaded, diff --git a/packages/plugins/plugin-reports/package.json b/packages/plugins/plugin-reports/package.json index 8d9b0beb8f..471b0f7f6e 100644 --- a/packages/plugins/plugin-reports/package.json +++ b/packages/plugins/plugin-reports/package.json @@ -19,7 +19,8 @@ "dependencies": { "@objectstack/core": "workspace:*", "@objectstack/platform-objects": "workspace:*", - "@objectstack/spec": "workspace:*" + "@objectstack/spec": "workspace:*", + "croner": "^10.0.1" }, "devDependencies": { "@types/node": "^25.9.3", diff --git a/packages/plugins/plugin-reports/src/report-service.test.ts b/packages/plugins/plugin-reports/src/report-service.test.ts index 5f28299eb2..e9a4d8aee4 100644 --- a/packages/plugins/plugin-reports/src/report-service.test.ts +++ b/packages/plugins/plugin-reports/src/report-service.test.ts @@ -237,6 +237,49 @@ describe('ReportService', () => { expect(s.recipients).toBe('x@t'); }); + it('scheduleReport: cron_expression drives next_run_at and overrides interval (#1983)', async () => { + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + // now = 2026-01-15T10:00:00Z (past 09:00); daily-9am cron → tomorrow 09:00Z. + const s = await svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], intervalMinutes: 60, cronExpression: '0 9 * * *', + }, CTX); + expect(s.cron_expression).toBe('0 9 * * *'); + expect(s.next_run_at).toBe('2026-01-16T09:00:00.000Z'); + // …not the interval's now + 60m. + expect(s.next_run_at).not.toBe(new Date(now.getTime() + 60 * 60_000).toISOString()); + }); + + it('scheduleReport: cron honors the schedule timezone (#1983)', async () => { + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + // 09:00 America/New_York (UTC-5 in January) = 14:00Z; now=05:00 ET → same day. + const s = await svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], cronExpression: '0 9 * * *', timezone: 'America/New_York', + }, CTX); + expect(s.next_run_at).toBe('2026-01-15T14:00:00.000Z'); + }); + + it('scheduleReport: rejects an invalid cron_expression (#1983)', async () => { + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + await expect(svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], cronExpression: 'not a cron', + }, CTX)).rejects.toThrow(/VALIDATION_FAILED/); + }); + + it('dispatchDue: advances a cron schedule to the next cron occurrence (#1983)', async () => { + const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); + await svc.scheduleReport({ + reportId: r.id, recipients: ['x@t'], cronExpression: '0 9 * * *', format: 'csv', + }, CTX); + // Force due, then dispatch at `now`. + engine._tables['sys_report_schedule'][0].next_run_at = new Date(now.getTime() - 1000).toISOString(); + const result = await svc.dispatchDue(); + expect(result.fired).toBe(1); + const advanced = engine._tables['sys_report_schedule'][0]; + expect(advanced.last_status).toBe('ok'); + // Advanced via cron (tomorrow 09:00Z), not now + interval. + expect(advanced.next_run_at).toBe('2026-01-16T09:00:00.000Z'); + }); + it('listSchedules + unscheduleReport', async () => { const r = await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); const s = await svc.scheduleReport({ reportId: r.id, recipients: ['x@t'] }, CTX); diff --git a/packages/plugins/plugin-reports/src/report-service.ts b/packages/plugins/plugin-reports/src/report-service.ts index a9aa24eb9c..44de47750c 100644 --- a/packages/plugins/plugin-reports/src/report-service.ts +++ b/packages/plugins/plugin-reports/src/report-service.ts @@ -11,6 +11,7 @@ import type { ScheduleReportInput, SharingExecutionContext, } from '@objectstack/spec/contracts'; +import { Cron } from 'croner'; /** * Narrow engine surface — keeps the service testable without booting @@ -342,14 +343,27 @@ export class ReportService implements IReportService { const now = this.clock.now(); const interval = input.intervalMinutes ?? DEFAULT_INTERVAL_MIN; - const nextRun = new Date(now.getTime() + interval * 60_000).toISOString(); + const cron = input.cronExpression?.trim() || null; + if (cron) { + // Validate eagerly so an author gets a clear error at schedule time + // instead of a schedule that silently falls back to interval on sweep. + try { + new Cron(cron, { timezone: input.timezone || 'UTC' }); + } catch (err) { + throw new Error(`VALIDATION_FAILED: invalid cron_expression '${cron}': ${(err as Error).message}`); + } + } + const nextRun = this.nextRunAt( + { cron_expression: cron, interval_minutes: interval, timezone: input.timezone ?? 'UTC' }, + now, + ).toISOString(); const id = uid('rsch'); const row: any = { id, report_id: input.reportId, name: input.name ?? null, interval_minutes: interval, - cron_expression: input.cronExpression ?? null, + cron_expression: cron, timezone: input.timezone ?? 'UTC', active: input.active !== false, recipients: input.recipients.join(','), @@ -459,9 +473,36 @@ export class ReportService implements IReportService { return { fired, failed, skipped }; } - private async advanceSchedule(schedule: ReportSchedule, ranAt: string): Promise { + /** + * Compute the next fire time for a schedule. A `cron_expression` wins over + * `interval_minutes` (the documented `sys_report_schedule` contract) and is + * evaluated in the schedule's `timezone` (default UTC) via croner — the same + * library the job scheduler uses. Falls back to `from + interval_minutes` for + * interval schedules, and also if a cron expression is invalid or has no + * future occurrence (logged; never throws into the sweep). `from` is the + * reference instant (the injected clock), so `today()`-style boundaries honor + * the test clock. + */ + private nextRunAt( + schedule: { cron_expression?: string | null; interval_minutes?: number | null; timezone?: string | null }, + from: Date, + ): Date { + const cron = (schedule.cron_expression ?? '').trim(); + if (cron) { + try { + const next = new Cron(cron, { timezone: schedule.timezone || 'UTC' }).nextRun(from); + if (next) return next; + this.logger.warn?.(`ReportService: cron '${cron}' has no next occurrence; falling back to interval`); + } catch (err) { + this.logger.warn?.(`ReportService: invalid cron '${cron}'; falling back to interval`, err); + } + } const interval = schedule.interval_minutes ?? DEFAULT_INTERVAL_MIN; - const nextRun = new Date(this.clock.now().getTime() + interval * 60_000).toISOString(); + return new Date(from.getTime() + interval * 60_000); + } + + private async advanceSchedule(schedule: ReportSchedule, ranAt: string): Promise { + const nextRun = this.nextRunAt(schedule, this.clock.now()).toISOString(); await this.engine.update('sys_report_schedule', { id: schedule.id, next_run_at: nextRun, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3dd162c6d5..a2f2c18682 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1462,6 +1462,9 @@ importers: '@objectstack/spec': specifier: workspace:* version: link:../../spec + croner: + specifier: ^10.0.1 + version: 10.0.1 devDependencies: '@types/node': specifier: ^25.9.3