mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-08 09:54:26 +00:00
fix: persist a one-time schedule's run time so edit and disable keep its year (#1499)
PR #1497 pinned an enabled one-shot's next_run_at on creation, but the chosen instant did not survive two paths: editing reconstructed the date from the yearless cron (current year), and a disabled one-shot nulled next_run_at with no other store, so enabling it later recomputed from the cron. Both moved a future-year one-shot to a different annual occurrence than the date displayed. Persist the one-shot's absolute fire time in a dedicated run_at column (additive, nullable; recurring schedules leave it null). Create and update store run_at independently of the enabled state; next_run_at is derived from it when enabled and null while disabled, so a disabled one-shot keeps its run_at and the enable toggle restores the exact instant from the column rather than the cron. The editor reconstructs a one-shot's date from the persisted run_at, so opening and re-saving without changes preserves the originally chosen year. No behavior change for recurring schedules or fresh installs; the column is added by an additive migration safe for upgrades from v0.92.0.
This commit is contained in:
@@ -233,6 +233,8 @@ describe('POST /api/scheduled-tasks', () => {
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.next_run_at).toBe(runAt);
|
||||
// run_at is persisted in its own column so it survives disable/enable and edit.
|
||||
expect(res.body.run_at).toBe(runAt);
|
||||
});
|
||||
|
||||
it('falls back to the cron-derived next run when no run_at is supplied', async () => {
|
||||
@@ -269,13 +271,21 @@ describe('POST /api/scheduled-tasks', () => {
|
||||
expect(res.body.error).toMatch(/run_at must be an epoch-millisecond timestamp/);
|
||||
});
|
||||
|
||||
it('ignores run_at for a disabled task (next_run_at stays null)', async () => {
|
||||
it('persists a disabled one-shot run_at (next_run_at null) so enabling restores the chosen instant', async () => {
|
||||
const runAt = new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime();
|
||||
const res = await request(app).post('/api/scheduled-tasks').set('Cookie', adminCookie).send({
|
||||
...basePayload, enabled: false, cron_expression: '0 23 1 7 *', delete_after_run: true,
|
||||
run_at: new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime(),
|
||||
...basePayload, enabled: false, cron_expression: '0 23 1 7 *', delete_after_run: true, run_at: runAt,
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
// A disabled task does not schedule, but the pinned instant is retained.
|
||||
expect(res.body.next_run_at).toBeNull();
|
||||
expect(res.body.run_at).toBe(runAt);
|
||||
|
||||
// Enabling it must restore the exact chosen instant, not the cron's annual occurrence.
|
||||
const on = await request(app).patch(`/api/scheduled-tasks/${res.body.id}/toggle`).set('Cookie', adminCookie);
|
||||
expect(on.status).toBe(200);
|
||||
expect(on.body.enabled).toBe(1);
|
||||
expect(on.body.next_run_at).toBe(runAt);
|
||||
});
|
||||
|
||||
it('rejects unsupported actions', async () => {
|
||||
@@ -472,7 +482,7 @@ describe('PATCH /api/scheduled-tasks/:id/toggle', () => {
|
||||
expect(typeof on.body.next_run_at).toBe('number');
|
||||
});
|
||||
|
||||
it('preserves a one-shot pinned next_run_at across a disable/enable cycle', async () => {
|
||||
it('preserves a one-shot pinned run_at across a disable/enable cycle', async () => {
|
||||
const now = Date.now();
|
||||
// A one-shot pinned to next year (its yearless cron would resolve to this year).
|
||||
const pinned = new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime();
|
||||
@@ -480,14 +490,15 @@ describe('PATCH /api/scheduled-tasks/:id/toggle', () => {
|
||||
name: 'once', target_type: 'stack', target_id: 's', node_id: 1, action: 'auto_backup',
|
||||
cron_expression: '0 23 1 7 *', enabled: 1, created_by: 'admin', created_at: now, updated_at: now,
|
||||
last_run_at: null, next_run_at: pinned, last_status: null, last_error: null,
|
||||
prune_targets: null, target_services: null, prune_label_filter: null, delete_after_run: 1,
|
||||
prune_targets: null, target_services: null, prune_label_filter: null, delete_after_run: 1, run_at: pinned,
|
||||
});
|
||||
|
||||
const off = await request(app).patch(`/api/scheduled-tasks/${id}/toggle`).set('Cookie', adminCookie);
|
||||
expect(off.status).toBe(200);
|
||||
expect(off.body.enabled).toBe(0);
|
||||
// The pinned instant survives the disable so it can be restored on re-enable.
|
||||
expect(off.body.next_run_at).toBe(pinned);
|
||||
// Disabling clears next_run_at, but the run_at column retains the instant.
|
||||
expect(off.body.next_run_at).toBeNull();
|
||||
expect(off.body.run_at).toBe(pinned);
|
||||
|
||||
const on = await request(app).patch(`/api/scheduled-tasks/${id}/toggle`).set('Cookie', adminCookie);
|
||||
expect(on.status).toBe(200);
|
||||
@@ -677,6 +688,52 @@ describe('PUT /api/scheduled-tasks/:id - delete_after_run', () => {
|
||||
.send({ cron_expression: '0 23 1 7 *', run_at: runAt });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.next_run_at).toBe(runAt);
|
||||
// The column is persisted too, so a later disable/enable restores it.
|
||||
expect(res.body.run_at).toBe(runAt);
|
||||
});
|
||||
|
||||
it('clears run_at when an update switches a one-shot to a recurring schedule', async () => {
|
||||
const now = Date.now();
|
||||
const pinned = new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime();
|
||||
const id = DatabaseService.getInstance().createScheduledTask({
|
||||
name: 'once', target_type: 'stack', target_id: 's', node_id: 1, action: 'auto_backup',
|
||||
cron_expression: '0 23 1 7 *', enabled: 1, created_by: 'admin', created_at: now, updated_at: now,
|
||||
last_run_at: null, next_run_at: pinned, last_status: null, last_error: null,
|
||||
prune_targets: null, target_services: null, prune_label_filter: null, delete_after_run: 1, run_at: pinned,
|
||||
});
|
||||
|
||||
// Editing to a recurring daily schedule sends run_at: null (the frontend
|
||||
// sends null for every non-once shape). The pin must be cleared and
|
||||
// next_run_at recomputed from the cron, not left on the stale instant.
|
||||
const res = await request(app)
|
||||
.put(`/api/scheduled-tasks/${id}`)
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ cron_expression: '0 3 * * *', delete_after_run: false, run_at: null });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.run_at).toBeNull();
|
||||
expect(res.body.next_run_at).not.toBe(pinned);
|
||||
expect(res.body.next_run_at).toBeGreaterThan(Date.now());
|
||||
});
|
||||
|
||||
it('keeps a one-shot run_at persisted when an update disables it', async () => {
|
||||
const now = Date.now();
|
||||
const pinned = new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime();
|
||||
const id = DatabaseService.getInstance().createScheduledTask({
|
||||
name: 'once', target_type: 'stack', target_id: 's', node_id: 1, action: 'auto_backup',
|
||||
cron_expression: '0 23 1 7 *', enabled: 1, created_by: 'admin', created_at: now, updated_at: now,
|
||||
last_run_at: null, next_run_at: pinned, last_status: null, last_error: null,
|
||||
prune_targets: null, target_services: null, prune_label_filter: null, delete_after_run: 1, run_at: pinned,
|
||||
});
|
||||
|
||||
// Disabling via Save: next_run_at clears, but the pinned instant is retained
|
||||
// so re-enabling later restores the exact date.
|
||||
const res = await request(app)
|
||||
.put(`/api/scheduled-tasks/${id}`)
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ enabled: false, run_at: pinned });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.next_run_at).toBeNull();
|
||||
expect(res.body.run_at).toBe(pinned);
|
||||
});
|
||||
|
||||
it('rejects an update with a past run_at', async () => {
|
||||
|
||||
@@ -251,11 +251,15 @@ scheduledTasksRouter.post('/', (req: Request, res: Response): void => {
|
||||
|
||||
const scheduler = SchedulerService.getInstance();
|
||||
const now = Date.now();
|
||||
// A one-shot's run_at pins the exact instant (including year); fall back to
|
||||
// the cron-derived next run when no explicit timestamp is supplied.
|
||||
// Persist the one-shot's pinned instant in its own column so it survives a
|
||||
// disabled state and edit (the yearless cron cannot reconstruct the year).
|
||||
// next_run_at is the cron-derived run unless a run_at pins it, and is null
|
||||
// while disabled; the pinned run_at is retained regardless so enabling later
|
||||
// restores the exact instant.
|
||||
const pinnedRunAt = typeof run_at === 'number' ? run_at : null;
|
||||
const nextRun = (enabled === false)
|
||||
? null
|
||||
: (typeof run_at === 'number' ? run_at : scheduler.calculateNextRun(cron_expression));
|
||||
: (pinnedRunAt ?? scheduler.calculateNextRun(cron_expression));
|
||||
const normalizedTargetId = target_type === 'stack' ? target_id : null;
|
||||
const normalizedNodeId = actionRequiresNode(action) ? parsePositiveNodeId(node_id) : null;
|
||||
|
||||
@@ -278,6 +282,7 @@ scheduledTasksRouter.post('/', (req: Request, res: Response): void => {
|
||||
target_services: action === 'restart' && target_type === 'stack' && target_services ? JSON.stringify(target_services) : null,
|
||||
prune_label_filter: action === 'prune' && prune_label_filter ? prune_label_filter.trim() : null,
|
||||
delete_after_run: delete_after_run ? 1 : 0,
|
||||
run_at: pinnedRunAt,
|
||||
});
|
||||
|
||||
console.log(`[ScheduledTasks] Created task id=${id} action=${sanitizeForLog(action)} target=${sanitizeForLog(target_id || 'none')}`);
|
||||
@@ -385,14 +390,19 @@ scheduledTasksRouter.put('/:id', (req: Request, res: Response): void => {
|
||||
}
|
||||
if (delete_after_run !== undefined) updates.delete_after_run = delete_after_run ? 1 : 0;
|
||||
|
||||
// Persist a re-supplied run_at to its column (a number pins a one-shot; null
|
||||
// clears it when an edit switches the schedule to a recurring shape). When
|
||||
// run_at is omitted, the existing pinned value carries forward.
|
||||
const runAtProvided = run_at !== undefined;
|
||||
if (runAtProvided) updates.run_at = typeof run_at === 'number' ? run_at : null;
|
||||
const effectiveRunAt = (runAtProvided ? updates.run_at : existing.run_at) ?? null;
|
||||
|
||||
const finalCron = cron_expression || existing.cron_expression;
|
||||
const finalEnabled = enabled !== undefined ? enabled : existing.enabled;
|
||||
if (finalEnabled) {
|
||||
// A re-supplied one-shot run_at pins the exact instant; otherwise recompute
|
||||
// from the (possibly updated) cron.
|
||||
updates.next_run_at = typeof run_at === 'number'
|
||||
? run_at
|
||||
: SchedulerService.getInstance().calculateNextRun(finalCron);
|
||||
// The pinned one-shot instant wins; otherwise recompute from the (possibly
|
||||
// updated) cron.
|
||||
updates.next_run_at = effectiveRunAt ?? SchedulerService.getInstance().calculateNextRun(finalCron);
|
||||
} else {
|
||||
updates.next_run_at = null;
|
||||
}
|
||||
@@ -439,19 +449,13 @@ scheduledTasksRouter.patch('/:id/toggle', (req: Request, res: Response): void =>
|
||||
if (!existing) { res.status(404).json({ error: 'Scheduled task not found' }); return; }
|
||||
|
||||
const newEnabled = existing.enabled ? 0 : 1;
|
||||
// A one-shot's exact instant cannot be reconstructed from its yearless cron,
|
||||
// so preserve its pinned next_run_at across a disable/enable cycle (the
|
||||
// enabled=0 gate stops a disabled task from firing). Recurring tasks clear
|
||||
// next_run_at while disabled and recompute it from the cron on enable.
|
||||
const isOneShot = existing.delete_after_run === 1 && existing.next_run_at != null;
|
||||
let nextRun: number | null;
|
||||
if (isOneShot) {
|
||||
// The pinned instant is kept in both directions: disabling leaves it set
|
||||
// (the enabled=0 gate stops it firing) so re-enabling restores it.
|
||||
nextRun = existing.next_run_at;
|
||||
} else {
|
||||
nextRun = newEnabled ? SchedulerService.getInstance().calculateNextRun(existing.cron_expression) : null;
|
||||
}
|
||||
// On enable, a one-shot's persisted run_at restores the exact pinned instant
|
||||
// (its yearless cron cannot reconstruct the chosen year); a recurring task
|
||||
// recomputes from the cron. Disabling clears next_run_at for both; the
|
||||
// run_at column is untouched, so re-enabling later still restores the instant.
|
||||
const nextRun: number | null = newEnabled
|
||||
? (existing.run_at ?? SchedulerService.getInstance().calculateNextRun(existing.cron_expression))
|
||||
: null;
|
||||
|
||||
db.updateScheduledTask(id, {
|
||||
enabled: newEnabled,
|
||||
|
||||
@@ -546,6 +546,11 @@ export interface ScheduledTask {
|
||||
target_services: string | null;
|
||||
prune_label_filter: string | null;
|
||||
delete_after_run?: number;
|
||||
// Absolute epoch-ms fire time for a one-time ('once') schedule. A 5-field
|
||||
// cron has no year field, so the chosen instant (including year) is persisted
|
||||
// here and survives disable/enable and edit, where the cron alone would
|
||||
// collapse to the next annual occurrence. Null for recurring schedules.
|
||||
run_at?: number | null;
|
||||
}
|
||||
|
||||
export interface ScheduledTaskRun {
|
||||
@@ -1558,6 +1563,7 @@ export class DatabaseService {
|
||||
maybeAddCol('scheduled_tasks', 'target_services', 'TEXT DEFAULT NULL');
|
||||
maybeAddCol('scheduled_tasks', 'prune_label_filter', 'TEXT DEFAULT NULL');
|
||||
maybeAddCol('scheduled_tasks', 'delete_after_run', 'INTEGER DEFAULT 0');
|
||||
maybeAddCol('scheduled_tasks', 'run_at', 'INTEGER DEFAULT NULL');
|
||||
|
||||
// Recreate stack_update_status with composite PK (node_id, stack_name).
|
||||
// Original table had stack_name as sole PK which breaks when multiple nodes share stack names.
|
||||
@@ -4295,13 +4301,13 @@ export class DatabaseService {
|
||||
|
||||
public createScheduledTask(task: Omit<ScheduledTask, 'id'>): number {
|
||||
const result = this.db.prepare(
|
||||
'INSERT INTO scheduled_tasks (name, target_type, target_id, node_id, action, cron_expression, enabled, created_by, created_at, updated_at, last_run_at, next_run_at, last_status, last_error, prune_targets, target_services, prune_label_filter, delete_after_run) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'
|
||||
'INSERT INTO scheduled_tasks (name, target_type, target_id, node_id, action, cron_expression, enabled, created_by, created_at, updated_at, last_run_at, next_run_at, last_status, last_error, prune_targets, target_services, prune_label_filter, delete_after_run, run_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'
|
||||
).run(
|
||||
task.name, task.target_type, task.target_id, task.node_id,
|
||||
task.action, task.cron_expression, task.enabled, task.created_by,
|
||||
task.created_at, task.updated_at, task.last_run_at, task.next_run_at,
|
||||
task.last_status, task.last_error, task.prune_targets, task.target_services,
|
||||
task.prune_label_filter, task.delete_after_run ?? 0
|
||||
task.prune_label_filter, task.delete_after_run ?? 0, task.run_at ?? null
|
||||
);
|
||||
return result.lastInsertRowid as number;
|
||||
}
|
||||
@@ -4319,8 +4325,13 @@ export class DatabaseService {
|
||||
prune_targets: updates.prune_targets, target_services: updates.target_services,
|
||||
prune_label_filter: updates.prune_label_filter,
|
||||
delete_after_run: updates.delete_after_run,
|
||||
run_at: updates.run_at,
|
||||
};
|
||||
|
||||
// `undefined` means "leave this column unchanged"; an explicit `null`
|
||||
// writes SQL NULL. Callers rely on this distinction (e.g. run_at: null
|
||||
// clears a one-shot's pin while an omitted run_at preserves it), so do
|
||||
// not relax this guard to a truthy or `!= null` check.
|
||||
for (const [col, val] of Object.entries(map)) {
|
||||
if (val !== undefined) {
|
||||
fields.push(`${col} = ?`);
|
||||
|
||||
@@ -241,7 +241,15 @@ export default function ScheduledOperationsView({ filterNodeId, onClearFilter, p
|
||||
setFormTargetId(task.target_id || '');
|
||||
setFormNodeId(task.node_id != null ? String(task.node_id) : '');
|
||||
setFormCron(task.cron_expression);
|
||||
const parsed = parseCron(task.cron_expression, (task.delete_after_run ?? 0) === 1);
|
||||
let parsed = parseCron(task.cron_expression, (task.delete_after_run ?? 0) === 1);
|
||||
// The cron has no year field, so parseCron reconstructs a one-shot's date in
|
||||
// the current year. Rebuild it from the persisted run_at instead, so editing
|
||||
// (and re-saving) preserves the originally chosen instant rather than moving
|
||||
// it to this year's occurrence.
|
||||
if (parsed && parsed.frequency === 'once' && task.run_at != null) {
|
||||
const pinned = new Date(task.run_at);
|
||||
parsed = { ...parsed, date: pinned, hour: pinned.getHours(), minute: pinned.getMinutes() };
|
||||
}
|
||||
setScheduleMode(parsed ? 'simple' : 'advanced');
|
||||
setSimpleSchedule(parsed ?? DEFAULT_SIMPLE_SCHEDULE);
|
||||
setSimpleReplacedCron(false);
|
||||
|
||||
@@ -647,6 +647,29 @@ describe('ScheduledOperationsView', () => {
|
||||
expect(screen.queryByText(/one-time schedule fires on the chosen date/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('preserves the chosen year when editing a future-year one-shot without changing it', async () => {
|
||||
// The cron (0 23 1 7 *) is yearless; the persisted run_at carries the real
|
||||
// year. Editing and re-saving must send that year, not this year's occurrence.
|
||||
const runAt = new Date(new Date().getFullYear() + 1, 6, 1, 23, 0, 0, 0).getTime();
|
||||
tasksFixture = [makeTask({
|
||||
id: 7, name: 'once-next-year', cron_expression: '0 23 1 7 *',
|
||||
delete_after_run: 1, run_at: runAt, next_run_at: runAt,
|
||||
})];
|
||||
render(<ScheduledOperationsView />);
|
||||
|
||||
await userEvent.click(await screen.findByRole('button', { name: /All tasks/ }));
|
||||
await userEvent.click(await screen.findByTitle('Edit'));
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Update' }));
|
||||
|
||||
await waitFor(() => {
|
||||
const putCall = mockedFetch.mock.calls.find(
|
||||
([url, opts]) => url === '/scheduled-tasks/7' && opts?.method === 'PUT',
|
||||
);
|
||||
expect(putCall).toBeTruthy();
|
||||
expect(JSON.parse(putCall![1].body).run_at).toBe(runAt);
|
||||
});
|
||||
});
|
||||
|
||||
it('opens a non-simple cron in Advanced mode', async () => {
|
||||
tasksFixture = [makeTask({ id: 6, name: 'every-15', cron_expression: '*/15 * * * *' })];
|
||||
render(<ScheduledOperationsView />);
|
||||
|
||||
@@ -18,6 +18,11 @@ export interface ScheduledTask {
|
||||
target_services: string | null;
|
||||
prune_label_filter: string | null;
|
||||
delete_after_run?: number;
|
||||
// Absolute epoch-ms fire time for a one-time ('once') schedule; null/absent for
|
||||
// recurring shapes. Persisted so the chosen instant (including year) survives
|
||||
// disable/enable and edit, where the yearless cron would otherwise collapse to
|
||||
// the next annual occurrence.
|
||||
run_at?: number | null;
|
||||
next_runs?: number[];
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user