cpvalente/ontime
1
0
Fork 0
mirror of https://github.com/cpvalente/ontime.git synced 2026-09-10 16:49:41 +00:00
Code Issues Packages Projects Releases 10 Wiki Activity

fix(automation): make creating from a recipe retry-safe too

Browse Source 
Review caught the recipe dialog having the same shape of bug as the form, one
step earlier. addAutomation ran unconditionally on every attempt, so a trigger
request that failed left an automation behind and pressing create again made a
second one. With a recipe that binds more than one lifecycle, the cycles that
had succeeded would then exist on both, and the show would fire them twice.

The comment there claimed the half created automation was recoverable, which it
was, but only by abandoning the dialog: the retry the error message offered was
the thing that duplicated it.

The dialog now records what it has already put on the server. A second attempt
edits that automation rather than creating another, so parameters corrected
after the failure are still applied, and adds only the triggers still missing.

Verified by failing the first trigger request: the failed attempt leaves one
automation with no trigger, and the retry reuses it and adds the trigger, rather
than leaving two automations behind.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AfDKsy6PE3Rbyt32Fg4YKf
This commit is contained in:
Claude
2026-09-09 15:32:34 +00:00
parent 99a2747705
commit 85d89b24b8
1 changed files with 28 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx
+28 -10
View File
@@ -1,8 +1,8 @@
import type { Automation } from 'ontime-types';
import { useMemo, useState, type KeyboardEvent } from 'react';
import type { Automation, TimerLifeCycle } from 'ontime-types';
import { useMemo, useRef, useState, type KeyboardEvent } from 'react';
import { IoAdd, IoArrowBack, IoChevronForward, IoClose, IoSearch } from 'react-icons/io5';
import { addAutomation, addTrigger } from '../../../../common/api/automation';
import { addAutomation, addTrigger, editAutomation } from '../../../../common/api/automation';
import { maybeAxiosError } from '../../../../common/api/utils';
import Button from '../../../../common/components/buttons/Button';
import IconButton from '../../../../common/components/buttons/IconButton';
@@ -202,24 +202,42 @@ function RecipeSetup({ recipe, onClose, onBack, onCreated }: RecipeSetupProps) {
const setValue = (name: string, value: string) => setValues((prev) => ({ ...prev, [name]: value }));
/**
* What this dialog has already put on the server.
*
* Creating takes one request per trigger on top of the automation itself, so a failure
* part way through leaves work already done. Recording it means pressing create again
* edits that automation and adds only the triggers still missing, rather than making a
* second automation and firing the same cycles twice.
*/
const created = useRef<Automation | null>(null);
const createdCycles = useRef<Set<TimerLifeCycle>>(new Set());
const handleCreate = async () => {
setError(null);
setIsCreating(true);
try {
// the same two steps the automation form takes when it saves a new automation:
// the server generates the id, so the automation has to exist before a trigger can point at it
const created = await addAutomation(automation);
// the server generates the id, so the automation has to exist before a trigger points at it
const existing = created.current;
created.current = existing
? await editAutomation(existing.id, { id: existing.id, ...automation })
: await addAutomation(automation);
for (const cycle of recipe.triggers) {
if (createdCycles.current.has(cycle)) {
continue;
}
await addTrigger({
title: makeTriggerTitle(automation.title, cycle),
trigger: cycle,
automationId: created.id,
automationId: created.current.id,
});
createdCycles.current.add(cycle);
}
onCreated(created);
onCreated(created.current);
} catch (error) {
// a half created automation is visible in the list and flagged there, so say what
// happened and let the user finish it in the form rather than undoing their work
// what did land is a normal automation, visible in the list. Say what happened and let
// the user press create again rather than undoing work behind their back
setError(maybeAxiosError(error));
} finally {
setIsCreating(false);