From a4e83dd9a04dd4b40dc2896d6ba7b31b05da9784 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 19:36:09 +0000 Subject: [PATCH] refactor(automation): give triggers back to the triggers list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The automation form had grown a lifecycle picker that created and deleted global triggers behind the user's back. It broke the model the panel is built on — an automation is what to send, a trigger is when — and left the global triggers section describing itself as a place to rename things made elsewhere. Triggers now belong only to the triggers list and the event editor. The automation form edits a title, filters and outputs, and saves in one request: the two-request save, its partial-failure recovery and the snapshot it reconciled against are all gone with it. An automation with no global trigger offers to make one from its own row, opening the trigger form with the automation preselected, so the connection is still one click away without the form pretending to own it. Other changes in the same pass: - the form uses the compact modal instead of the wide one. Nothing in it justified 1800px, and output fields now pair up two to a row rather than stretching across four columns - a blank automation gets its own header button beside Start from recipe, instead of being a footnote under the recipe list - the settings form seeds itself from the query when it resolves, as the other settings panels do. It was showing automations as OFF while they were on - the filter operator list goes back to the three master offered, which makes the note about not_contains unnecessary rather than explanatory - comments that narrated the change rather than explaining the code Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AfDKsy6PE3Rbyt32Fg4YKf --- .../AutomationForm.module.scss | 23 +- .../automations-panel/AutomationForm.tsx | 525 +++++++----------- .../AutomationSettingsForm.tsx | 12 +- .../automations-panel/AutomationsList.tsx | 70 ++- .../automations-panel/NewAutomationDialog.tsx | 37 +- .../panel/automations-panel/TriggerForm.tsx | 6 +- .../panel/automations-panel/TriggersList.tsx | 8 +- .../__tests__/automationUtils.test.ts | 8 +- .../automations-panel/automationUtils.ts | 19 +- .../__tests__/automation.dao.test.ts | 2 - .../src/api-data/automation/automation.dao.ts | 5 +- 11 files changed, 279 insertions(+), 436 deletions(-) diff --git a/apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.module.scss b/apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.module.scss index e2a3da972..07d31ccef 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.module.scss +++ b/apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.module.scss @@ -1,18 +1,3 @@ -/** - * The wide modal body does not scroll, so the form owns it. - * Without this the form is simply clipped: four outputs is enough to put Save out of reach. - */ -.form { - display: flex; - flex-direction: column; - flex: 1; - min-height: 0; -} - -.formScroll { - height: 100%; -} - .outerColumn { display: flex; flex-direction: column; @@ -20,8 +5,6 @@ font-size: calc(1rem - 1px); color: $ui-white; padding-block: 0.5rem; - // leaves the overlay scrollbar somewhere to sit without covering a field - padding-right: 0.5rem; h3 { font-size: 1rem; @@ -83,9 +66,13 @@ color: $secondary-text-gray; } +/** + * Two columns, so fields that belong together sit on one row: a host and its port, + * a filter field and its operator. Anything wider than half a card opts out with spanFull + */ .cardBody { display: grid; - grid-template-columns: repeat(auto-fit, minmax(11rem, 1fr)); + grid-template-columns: repeat(2, minmax(0, 1fr)); gap: 0.5rem 0.75rem; padding: 0.75rem; } diff --git a/apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.tsx b/apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.tsx index 73bb22ebd..c30e0af04 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.tsx @@ -1,24 +1,9 @@ -import { - Automation, - AutomationDTO, - AutomationFilter, - TimerLifeCycle, - Trigger, - isHTTPOutput, - isOSCOutput, - isOntimeAction, -} from 'ontime-types'; +import { Automation, AutomationDTO, AutomationFilter, isHTTPOutput, isOSCOutput, isOntimeAction } from 'ontime-types'; import { useEffect, useMemo, useRef, useState } from 'react'; import { useFieldArray, useForm } from 'react-hook-form'; import { IoAdd, IoTrash } from 'react-icons/io5'; -import { - addAutomation, - addTrigger, - deleteTrigger, - editAutomation, - testOutput, -} from '../../../../common/api/automation'; +import { addAutomation, editAutomation, testOutput } from '../../../../common/api/automation'; import { maybeAxiosError } from '../../../../common/api/utils'; import Button from '../../../../common/components/buttons/Button'; import IconButton from '../../../../common/components/buttons/IconButton'; @@ -28,13 +13,12 @@ import Input from '../../../../common/components/input/input/Input'; import ExternalLink from '../../../../common/components/link/external-link/ExternalLink'; import Modal from '../../../../common/components/modal/Modal'; import RadioGroup from '../../../../common/components/radio-group/RadioGroup'; -import ScrollArea from '../../../../common/components/scroll-area/ScrollArea'; import Select from '../../../../common/components/select/Select'; import Tag from '../../../../common/components/tag/Tag'; import useAutomationSettings from '../../../../common/hooks-query/useAutomationSettings'; import useCustomFields from '../../../../common/hooks-query/useCustomFields'; import * as Panel from '../../panel-utils/PanelUtils'; -import { cycles, isAutomation, makeFieldList, makeTriggerTitle, operators, type OutputErrors } from './automationUtils'; +import { isAutomation, makeFieldList, operators, type OutputErrors } from './automationUtils'; import HttpOutputForm from './HttpOutputForm'; import OntimeActionForm from './OntimeActionForm'; import OscOutputForm from './OscOutputForm'; @@ -48,55 +32,21 @@ const formId = 'automation-form'; /** how long a successful test keeps its confirmation on screen */ const testFeedbackDuration = 2000; -/** lifecycles that fire continuously, and are worth a warning before a user picks one */ -const continuousCycles: TimerLifeCycle[] = [TimerLifeCycle.onClock, TimerLifeCycle.onUpdate]; - interface AutomationFormProps { automation: Automation | AutomationDTO; - /** global triggers, used to resolve which lifecycles this automation is currently bound to */ - triggers: Trigger[]; onClose: () => void; } -export default function AutomationForm({ automation, triggers, onClose }: AutomationFormProps) { +/** + * Edits what an automation sends: its filters and its outputs. + * When it runs is a separate concept, owned by the triggers list and by the event editor. + */ +export default function AutomationForm({ automation, onClose }: AutomationFormProps) { const isEdit = isAutomation(automation); const { data } = useCustomFields(); const { refetch } = useAutomationSettings(); const fieldList = useMemo(() => makeFieldList(data), [data]); - /** - * The triggers the server holds for this automation, as far as this form knows. - * - * Seeded from a snapshot taken when the form opens, never from the live prop: settings are - * polled, so a trigger created elsewhere while this form is open must not be deleted by a - * save that never saw it. It then advances as each request succeeds, so a save that fails - * half way leaves only the outstanding work for the retry. - */ - const [syncedTriggers, setSyncedTriggers] = useState(() => - isAutomation(automation) ? triggers.filter((trigger) => trigger.automationId === automation.id) : [], - ); - const syncedCycles = useMemo( - () => Array.from(new Set(syncedTriggers.map((trigger) => trigger.trigger))), - [syncedTriggers], - ); - const [selectedCycles, setSelectedCycles] = useState(syncedCycles); - /** set once a create succeeds, so a retry after a failed trigger sync edits instead of creating a duplicate */ - const [createdId, setCreatedId] = useState(null); - - // both are deduped, so equal lengths and one being a subset makes them the same selection - const cyclesAreDirty = - selectedCycles.length !== syncedCycles.length || selectedCycles.some((cycle) => !syncedCycles.includes(cycle)); - - const toggleCycle = (cycle: TimerLifeCycle) => { - setSelectedCycles((prev) => (prev.includes(cycle) ? prev.filter((c) => c !== cycle) : [...prev, cycle])); - }; - - /** - * A lifecycle can carry several differently named triggers, which the chips collapse into one. - * Unchecking it removes all of them, so say which ones rather than deleting them quietly. - */ - const triggersToRemove = syncedTriggers.filter((trigger) => !selectedCycles.includes(trigger.trigger)); - /** * Test results are keyed by the field array id rather than the index: * removing an output shifts every index after it, which would leave feedback on the wrong row @@ -216,59 +166,21 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa } }; - /** - * Reconciles the lifecycle selection against the global triggers. - * Runs after the automation itself is saved: a new automation has no id until then. - * - * Every request advances the synced snapshot as it succeeds, so pressing save again after - * a failure half way through retries only what is left. Without that a retry would re-add - * a trigger it already created, and re-delete one it already deleted, which the server - * rejects outright. - */ - const syncTriggers = async (automationId: string, title: string) => { - for (const trigger of triggersToRemove) { - await deleteTrigger(trigger.id); - setSyncedTriggers((prev) => prev.filter((synced) => synced.id !== trigger.id)); - } - - const toAdd = selectedCycles.filter((cycle) => !syncedCycles.includes(cycle)); - for (const cycle of toAdd) { - const created = await addTrigger({ title: makeTriggerTitle(title, cycle), trigger: cycle, automationId }); - setSyncedTriggers((prev) => [...prev, created]); - } - }; - const onSubmit = async (values: AutomationDTO) => { // a stale failure from the previous attempt would otherwise sit under a successful retry clearErrors('root'); - // saving happens in two requests, so a retry after a partial failure must edit rather than create again - const existingId = isAutomation(automation) ? automation.id : createdId; - let automationId: string; - try { - if (existingId) { - await editAutomation(existingId, { id: existingId, ...values }); - automationId = existingId; + if (isAutomation(automation)) { + await editAutomation(automation.id, { id: automation.id, ...values }); } else { - const created = await addAutomation(values); - setCreatedId(created.id); - automationId = created.id; + await addAutomation(values); } } catch (error) { setError('root', { message: maybeAxiosError(error) }); return; } - try { - await syncTriggers(automationId, values.title); - } catch (error) { - // the automation itself is saved, only its triggers failed. Keep the form open so the user can retry - refetch(); - setError('root', { message: `Automation saved, but its triggers failed: ${maybeAxiosError(error)}` }); - return; - } - refetch(); onClose(); }; @@ -288,14 +200,10 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa return `${fieldLabel} ${operatorLabel} ${value ? `“${value}”` : 'nothing'}`; }; - /** - * A failed save reports itself as a root error, which react-hook-form counts against - * isValid. Left alone that disables the very retry the message is asking the user to make, - * so a root error on its own does not block submitting again. - */ + // a failed save lands on `root`, which react-hook-form counts against isValid. + // Only errors on actual fields should stand between the user and another attempt const invalidFields = Object.keys(errors).filter((field) => field !== 'root'); - const canSubmit = !isSubmitting && (isDirty || cyclesAreDirty) && (isValid || invalidFields.length === 0); - const hasContinuousCycle = selectedCycles.some((cycle) => continuousCycles.includes(cycle)); + const canSubmit = !isSubmitting && isDirty && (isValid || invalidFields.length === 0); return ( - -
-

Automation options

-
+
+
+
+ + {errors.title?.message} +
+
+ +
+

Filters (optional)

+ + Without filters the outputs are sent every time the automation is triggered. + +
+ {fieldFilters.length > 1 && ( - {errors.title?.message} -
- -
- - - Pick the moments in the timer lifecycle that should run this automation. You can also attach it to a - single event from the event editor. - - - {cycles.map(({ id, label, value }) => { - const cycle = value as TimerLifeCycle; - const isSelected = selectedCycles.includes(cycle); - return ( - - ); - })} - - {hasContinuousCycle && ( - - Every second and On Timer Update fire continuously while the timer runs. Add a filter unless you - mean to send on every tick. - - )} - {triggersToRemove.length > 0 && ( - - {`Saving removes ${triggersToRemove.length === 1 ? 'the trigger' : `${triggersToRemove.length} triggers`}: ${triggersToRemove - .map((trigger) => trigger.title) - .join(', ')}`} - - )} -
-
- -
-

Filters (optional)

- - Without filters the outputs are sent every time the automation is triggered. - -
- {fieldFilters.length > 1 && ( - - )} - {fieldFilters.map((field, index) => { - const description = describeFilter(index); - return ( -
-
- Filter - {description} - removeFilter(index)} - > - - -
-
- - -
-
- ); - })} -
- -
-
-
- -
-

Outputs

- - Type {'{{'} in any field to drop in Ontime runtime data, like the running event title.{' '} - read the docs - - - {fieldOutputs.length === 0 && ( - )} - - {fieldOutputs.map((output, index) => { - const rowErrors = getOutputErrors(index); - const cardProps = { - testState: testResults[output.id], - onTest: () => handleTest(index, output.id), - onDelete: () => removeOutput(index), - }; - - if (isOSCOutput(output)) { - return ( - - - - ); - } - - if (isHTTPOutput(output)) { - return ( - - - - ); - } - - if (isOntimeAction(output)) { - return ( - - - - ); - } - - return null; + {fieldFilters.map((field, index) => { + const description = describeFilter(index); + return ( +
+
+ Filter + {description} + removeFilter(index)} + > + + +
+
+ + +
+
+ ); })}
- } - items={[ - { - type: 'item', - label: 'OSC', - description: 'Send an OSC message to a device on the network', - onClick: handleAddNewOSCOutput, - }, - { - type: 'item', - label: 'HTTP', - description: 'Call a URL, for webhooks and REST APIs', - onClick: handleAddNewHTTPOutput, - }, - { - type: 'item', - label: 'Ontime action', - description: 'Change something inside Ontime, like a message or an aux timer', - onClick: handleAddnewOntimeAction, - }, - ]} - > - Add output - +
- +
+ +
+

Outputs

+ + Type {'{{'} in any field to drop in Ontime runtime data, like the running event title.{' '} + read the docs + + + {fieldOutputs.length === 0 && ( + + )} + + {fieldOutputs.map((output, index) => { + const rowErrors = getOutputErrors(index); + const cardProps = { + testState: testResults[output.id], + onTest: () => handleTest(index, output.id), + onDelete: () => removeOutput(index), + }; + + if (isOSCOutput(output)) { + return ( + + + + ); + } + + if (isHTTPOutput(output)) { + return ( + + + + ); + } + + if (isOntimeAction(output)) { + return ( + + + + ); + } + + return null; + })} +
+ } + items={[ + { + type: 'item', + label: 'OSC', + description: 'Send an OSC message to a device on the network', + onClick: handleAddNewOSCOutput, + }, + { + type: 'item', + label: 'HTTP', + description: 'Call a URL, for webhooks and REST APIs', + onClick: handleAddNewHTTPOutput, + }, + { + type: 'item', + label: 'Ontime action', + description: 'Change something inside Ontime, like a message or an aux timer', + onClick: handleAddnewOntimeAction, + }, + ]} + > + Add output + +
+
} footerElements={ diff --git a/apps/client/src/features/app-settings/panel/automations-panel/AutomationSettingsForm.tsx b/apps/client/src/features/app-settings/panel/automations-panel/AutomationSettingsForm.tsx index b0a5872fd..135b60032 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/AutomationSettingsForm.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/AutomationSettingsForm.tsx @@ -1,3 +1,4 @@ +import { useEffect } from 'react'; import { useForm } from 'react-hook-form'; import { editAutomationSettings } from '../../../../common/api/automation'; @@ -50,13 +51,18 @@ export default function AutomationSettingsForm({ }, }); + // the panel renders before the query resolves, so the form is seeded with placeholder + // settings. Take the loaded ones when they arrive, as the other settings panels do + useEffect(() => { + reset({ enabledAutomations, enabledOscIn, oscPortIn }); + }, [enabledAutomations, enabledOscIn, oscPortIn, reset]); + const onSubmit = async (formData: AutomationSettingsProps) => { try { await editAutomationSettings(formData); reset(formData); - // the rest of the panel reads these settings from the query, and the automations list - // greys itself out while they are off. Without this it keeps the stale answer until the - // slow poll comes round, so turning automations on appears to do nothing + // the rest of the panel reads these flags from the query, which is otherwise only + // refreshed on a slow poll: refetch so a toggle takes effect where it is visible await refetch(); } catch (error) { const message = maybeAxiosError(error); diff --git a/apps/client/src/features/app-settings/panel/automations-panel/AutomationsList.tsx b/apps/client/src/features/app-settings/panel/automations-panel/AutomationsList.tsx index 05817750b..e46fb4974 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/AutomationsList.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/AutomationsList.tsx @@ -1,6 +1,6 @@ import { Automation, AutomationDTO, NormalisedAutomation, Trigger } from 'ontime-types'; import { useMemo, useState } from 'react'; -import { IoAdd, IoPencil, IoTrash } from 'react-icons/io5'; +import { IoAdd, IoPencil, IoSparkles, IoTrash } from 'react-icons/io5'; import Button from '../../../../common/components/buttons/Button'; import IconButton from '../../../../common/components/buttons/IconButton'; @@ -15,6 +15,7 @@ import { groupTriggersByAutomation, isAutomation } from './automationUtils'; import DeleteAutomationDialog from './DeleteAutomationDialog'; import NewAutomationDialog from './NewAutomationDialog'; import { getLifecycleLabel } from './timerLifecycle'; +import TriggerForm from './TriggerForm'; import style from './AutomationsList.module.scss'; @@ -40,46 +41,49 @@ export default function AutomationsList({ }: AutomationsListProps) { const { refetch } = useAutomationSettings(); const [editing, setEditing] = useState(null); - const [isPickingStart, setIsPickingStart] = useState(false); + const [isPickingRecipe, setIsPickingRecipe] = useState(false); const [deleteTarget, setDeleteTarget] = useState(null); + /** the automation a new global trigger should point at, set from the row that asked for it */ + const [triggerTarget, setTriggerTarget] = useState(null); const lifecyclesByAutomation = useMemo(() => groupTriggersByAutomation(triggers), [triggers]); const automationIds = Object.keys(automations); /** a recipe creates the automation itself, so it lands in the list rather than in a form */ const handleCreated = async () => { - setIsPickingStart(false); + setIsPickingRecipe(false); await refetch(); }; - const handleStartEmpty = () => { - setIsPickingStart(false); - setEditing(emptyAutomation); - }; - const handleDeleted = async () => { setDeleteTarget(null); await refetch(); }; + const handleTriggerCreated = async () => { + setTriggerTarget(null); + await refetch(); + }; + return ( {editing !== null && ( setEditing(null)} /> )} - {isPickingStart && ( - setIsPickingStart(false)} - onStartEmpty={handleStartEmpty} - onCreated={handleCreated} + {isPickingRecipe && setIsPickingRecipe(false)} onCreated={handleCreated} />} + {triggerTarget !== null && ( + setTriggerTarget(null)} + postSubmit={handleTriggerCreated} /> )} {deleteTarget !== null && ( @@ -92,9 +96,14 @@ export default function AutomationsList({ )} Manage automations - + + + + @@ -121,11 +130,16 @@ export default function AutomationsList({ {!isLoading && automationIds.length === 0 && ( setIsPickingStart(true)}> - New automation - + + + + } /> )} @@ -139,12 +153,14 @@ export default function AutomationsList({ {automation.title} {/* - * Only global triggers are visible here: an automation can also be attached to - * single events, which live in the rundown. An empty cell is therefore not the - * same as never running, so it says nothing rather than claiming that. + * Only global triggers are listed here: an automation can also be attached to + * single events, which live in the rundown. No global trigger therefore does not + * mean it never runs, so the cell offers to add one rather than claiming anything. */} {lifecycles.length === 0 ? ( - — + ) : (
{lifecycles.map((cycle) => ( diff --git a/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx b/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx index 721944f3a..04ed06ccf 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx @@ -31,23 +31,21 @@ import style from './NewAutomationDialog.module.scss'; interface NewAutomationDialogProps { onClose: () => void; - /** hands over to the full automation form for someone who wants to start empty */ - onStartEmpty: () => void; onCreated: (automation: Automation) => void; } /** - * The single entry point for making an automation. + * Builds a working automation for a known workflow. * * Two steps in one dialog rather than two stacked ones: pick a recipe, then answer only * what that recipe cannot know — where your gear is, how long the timer runs. Everything * else the recipe already decided, which is the point of having recipes at all. */ -export default function NewAutomationDialog({ onClose, onStartEmpty, onCreated }: NewAutomationDialogProps) { +export default function NewAutomationDialog({ onClose, onCreated }: NewAutomationDialogProps) { const [selected, setSelected] = useState(null); return selected === null ? ( - + ) : ( setSelected(null)} onCreated={onCreated} /> ); @@ -66,11 +64,10 @@ function matches(recipe: AutomationRecipe, query: string): boolean { interface RecipePickerProps { onClose: () => void; - onStartEmpty: () => void; onSelect: (recipe: AutomationRecipe) => void; } -function RecipePicker({ onClose, onStartEmpty, onSelect }: RecipePickerProps) { +function RecipePicker({ onClose, onSelect }: RecipePickerProps) { const [query, setQuery] = useState(''); const available = useMemo( @@ -108,7 +105,7 @@ function RecipePicker({ onClose, onStartEmpty, onSelect }: RecipePickerProps) { showBackdrop showCloseButton size='compact' - title='New automation' + title='Start from a recipe' bodyElements={
@@ -139,7 +136,7 @@ function RecipePicker({ onClose, onStartEmpty, onSelect }: RecipePickerProps) { {results.length === 0 && ( )} @@ -173,14 +170,7 @@ function RecipePicker({ onClose, onStartEmpty, onSelect }: RecipePickerProps) {
} - footerElements={ - <> - - - - } + footerElements={} /> ); } @@ -203,12 +193,9 @@ 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. + * What this dialog has already put on the server: creating takes one request per trigger + * on top of the automation itself, so a second attempt edits what exists and adds only + * the triggers still missing, rather than making a duplicate that fires the same cycles twice. */ const created = useRef(null); const createdCycles = useRef>(new Set()); @@ -236,8 +223,8 @@ function RecipeSetup({ recipe, onClose, onBack, onCreated }: RecipeSetupProps) { } onCreated(created.current); } catch (error) { - // 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 + // whatever landed is a normal automation, visible in the list. Report the failure and + // leave the retry to the user rather than rolling back work behind their back setError(maybeAxiosError(error)); } finally { setIsCreating(false); diff --git a/apps/client/src/features/app-settings/panel/automations-panel/TriggerForm.tsx b/apps/client/src/features/app-settings/panel/automations-panel/TriggerForm.tsx index 3c8b2cd1c..542f18308 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/TriggerForm.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/TriggerForm.tsx @@ -16,11 +16,13 @@ const formId = 'trigger-form'; interface TriggerFormProps { automations: NormalisedAutomation; trigger: Trigger | null; + /** preselects the automation for a new trigger, used when creating one from an automation row */ + automationId?: string; onCancel: () => void; postSubmit: () => void; } -export default function TriggerForm({ automations, trigger, onCancel, postSubmit }: TriggerFormProps) { +export default function TriggerForm({ automations, trigger, automationId, onCancel, postSubmit }: TriggerFormProps) { const { handleSubmit, register, @@ -33,7 +35,7 @@ export default function TriggerForm({ automations, trigger, onCancel, postSubmit defaultValues: { title: trigger?.title, trigger: trigger?.trigger ?? (cycles[0].value as TimerLifeCycle | undefined), - automationId: trigger?.automationId ?? automations?.[Object.keys(automations)[0]]?.id, + automationId: trigger?.automationId ?? automationId ?? automations?.[Object.keys(automations)[0]]?.id, }, resetOptions: { keepDirtyValues: true, diff --git a/apps/client/src/features/app-settings/panel/automations-panel/TriggersList.tsx b/apps/client/src/features/app-settings/panel/automations-panel/TriggersList.tsx index 320e57060..53cc7a926 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/TriggersList.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/TriggersList.tsx @@ -78,8 +78,8 @@ export default function TriggersList({ triggers, automations, isLoading }: Trigg - Triggers are managed from the automation itself. This list is for naming them, or for pointing several - differently named triggers at the same automation. + A global trigger runs an automation at a point in the timer lifecycle, whichever event is loaded. To run an + automation on one event only, add the trigger from the event editor instead. {duplicates && ( @@ -109,8 +109,8 @@ export default function TriggersList({ triggers, automations, isLoading }: Trigg title='No triggers yet' description={ canAdd - ? 'Triggers run an automation at a given point of the timer lifecycle. The usual way to create one is to pick the lifecycles in the automation itself.' - : 'Create an automation first, then pick the lifecycles it should run on.' + ? 'Triggers run an automation at a given point of the timer lifecycle, like when an event starts or finishes.' + : 'A trigger needs an automation to run. Create the automation first, then come back and decide when it should run.' } action={ canAdd ? ( diff --git a/apps/client/src/features/app-settings/panel/automations-panel/__tests__/automationUtils.test.ts b/apps/client/src/features/app-settings/panel/automations-panel/__tests__/automationUtils.test.ts index 908d1c31b..f1303d69a 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/__tests__/automationUtils.test.ts +++ b/apps/client/src/features/app-settings/panel/automations-panel/__tests__/automationUtils.test.ts @@ -1,6 +1,6 @@ import { TimerLifeCycle, Trigger } from 'ontime-types'; -import { checkDuplicates, cycles, groupTriggersByAutomation, operators } from '../automationUtils'; +import { checkDuplicates, cycles, groupTriggersByAutomation } from '../automationUtils'; describe('checkDuplicates', () => { it('should return undefined if there are no duplicates', () => { @@ -51,12 +51,6 @@ describe('groupTriggersByAutomation', () => { }); }); -describe('operators', () => { - it('does not offer not_contains, which the server validation rejects', () => { - expect(operators.map(({ value }) => value)).not.toContain('not_contains'); - }); -}); - describe('cycles', () => { it('uses the shared user facing labels', () => { expect(cycles.find(({ value }) => value === 'onStart')?.label).toBe('On Start'); diff --git a/apps/client/src/features/app-settings/panel/automations-panel/automationUtils.ts b/apps/client/src/features/app-settings/panel/automations-panel/automationUtils.ts index c046561bd..9741db531 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/automationUtils.ts +++ b/apps/client/src/features/app-settings/panel/automations-panel/automationUtils.ts @@ -2,10 +2,7 @@ import { Automation, AutomationDTO, AutomationFilter, CustomFields, TimerLifeCyc import { getLifecycleLabel, lifecycleLabels } from './timerLifecycle'; -/** - * Names a trigger created from an automation's lifecycle picker. - * Shared so a trigger made by the form and one made by a recipe read the same in the list. - */ +/** Names the trigger a recipe creates alongside its automation, so the pair is recognisable in the triggers list */ export function makeTriggerTitle(automationTitle: string, cycle: TimerLifeCycle): string { return `${automationTitle} — ${getLifecycleLabel(cycle)}`; } @@ -34,18 +31,11 @@ export const cycles: CycleLabel[] = [ { id: 9, label: lifecycleLabels.onDanger, value: 'onDanger' }, ]; -/** - * Filter operators offered in the automation form - * NOTE: not_contains is supported by the type and by the runtime, but the server - * validation list omits it, so an automation using it cannot be saved. - * It stays out of the UI until the server accepts it. - */ +/** Filter operators offered in the automation form, phrased to read as a sentence in the filter summary */ export const operators: Array<{ value: AutomationFilter['operator']; label: string }> = [ { value: 'equals', label: 'equals' }, { value: 'not_equals', label: 'does not equal' }, { value: 'contains', label: 'contains' }, - { value: 'greater_than', label: 'is greater than' }, - { value: 'less_than', label: 'is less than' }, ]; /** @@ -114,10 +104,7 @@ export function checkDuplicates(triggers: Trigger[]) { return duplicates.length > 0 ? duplicates : undefined; } -/** - * Groups the lifecycles each automation is bound to - * Used to show when an automation runs, and to highlight the ones that never will - */ +/** Collects the lifecycles each automation is bound to, so the list can show when it runs */ export function groupTriggersByAutomation(triggers: Trigger[]): Record { const grouped: Record = {}; diff --git a/apps/server/src/api-data/automation/__tests__/automation.dao.test.ts b/apps/server/src/api-data/automation/__tests__/automation.dao.test.ts index 533354378..83f884096 100644 --- a/apps/server/src/api-data/automation/__tests__/automation.dao.test.ts +++ b/apps/server/src/api-data/automation/__tests__/automation.dao.test.ts @@ -118,8 +118,6 @@ describe('deleteTrigger()', () => { }); it('ignores a trigger that is already gone', async () => { - // a client reconciling several triggers must not be stuck because another client - // removed one of them first: the end state it asked for is the one it gets const before = getAutomationTriggers(); await expect(deleteTrigger('never-existed')).resolves.toBeUndefined(); expect(getAutomationTriggers()).toEqual(before); diff --git a/apps/server/src/api-data/automation/automation.dao.ts b/apps/server/src/api-data/automation/automation.dao.ts index dd2ca31e3..10808e855 100644 --- a/apps/server/src/api-data/automation/automation.dao.ts +++ b/apps/server/src/api-data/automation/automation.dao.ts @@ -87,9 +87,8 @@ export async function deleteTrigger(id: string): Promise { const triggers = getAutomationTriggers(); const index = triggers.findIndex((trigger) => trigger.id === id); - // ignore request if the trigger does not exist, as deleteAutomation does for the same reason: - // the caller asked for it to be gone and it is, and failing here makes a client that is - // reconciling several triggers unable to finish once another client removed one of them + // deleting is idempotent, as it is in deleteAutomation: the state the caller asked for + // already holds, and erroring would only punish a client that raced another one if (index === -1) { return; }