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; }