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 d9a815273..ecd69ce0d 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 @@ -80,7 +80,6 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa [syncedTriggers], ); const [selectedCycles, setSelectedCycles] = useState(syncedCycles); - // 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)); @@ -88,10 +87,6 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa 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)); /** @@ -269,7 +264,6 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa onClose(); }; - /** describes a filter in plain language so the user does not have to read the form back to themselves */ const describeFilter = (index: number): string | null => { const field = watch(`filters.${index}.field`); if (!field) { diff --git a/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.module.scss b/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.module.scss index 3d0198f09..83e08dd37 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.module.scss +++ b/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.module.scss @@ -1,5 +1,3 @@ -/* ---------- step one: pick a recipe ---------- */ - .picker { display: flex; flex-direction: column; @@ -7,7 +5,6 @@ color: $ui-white; } -/** outside the scrolling list, so it stays put however many recipes there are */ .search { position: relative; display: flex; @@ -15,10 +12,6 @@ padding-block: 0.5rem; } -/** - * Caps the list rather than fixing its height, so the dialog still shrinks to two results - * when a search narrows it down. - */ .listViewport { height: auto; max-height: min(52vh, 30rem); @@ -27,7 +20,6 @@ .list { display: flex; flex-direction: column; - // room for the overlay scrollbar beside the chevrons padding-right: 0.5rem; } @@ -121,8 +113,6 @@ color: $gray-400; } -/* ---------- step two: answer what the recipe cannot know ---------- */ - .setup { display: flex; flex-direction: column; @@ -136,7 +126,6 @@ color: $secondary-text-gray; } -/** the two facts a recipe decides for you, stated before the fields you can change */ .summary { display: grid; grid-template-columns: 5rem 1fr; @@ -161,7 +150,6 @@ } } -/* three columns, so a recipe's small numeric fields fill a row instead of leaving a hole */ .fields { display: grid; grid-template-columns: repeat(3, 1fr); @@ -173,7 +161,6 @@ } } -/* an address or a sentence, which a third of a row cannot hold */ .wide { grid-column: 1 / -1; } @@ -190,7 +177,6 @@ color: $secondary-text-gray; } -/* pushes the leading action away from the confirming ones */ .apart { margin-right: auto; } 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 f0713fb68..1577df755 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 @@ -19,7 +19,6 @@ import * as Panel from '../../panel-utils/PanelUtils'; import { defaultValues, getAvailableRecipes, - needsTarget, recipeCategoryLabels, recipeCategoryOrder, type AutomationRecipe, @@ -34,18 +33,10 @@ const availableRecipes = getAvailableRecipes(Boolean(isOntimeCloud)); 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. - * - * 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) { const [selected, setSelected] = useState(null); @@ -56,7 +47,6 @@ export default function NewAutomationDialog({ onClose, onStartEmpty, onCreated } ); } -/** matches on everything the user might type: the software, its protocol, the job it does */ function matches(recipe: AutomationRecipe, query: string): boolean { const haystack = [recipe.title, recipe.description, recipeCategoryLabels[recipe.category], ...(recipe.keywords ?? [])] .join(' ') @@ -274,9 +264,7 @@ function RecipeSetup({ recipe, onClose, onBack, onCreated }: RecipeSetupProps) { )} - {needsTarget(recipe) - ? 'Created as a normal automation. Nothing is sent until an event triggers it.' - : 'Created as a normal automation, which you can edit or delete like any other.'} + Created as a normal automation, which you can edit or delete like any other. } diff --git a/apps/client/src/features/app-settings/panel/automations-panel/__tests__/automationRecipes.test.ts b/apps/client/src/features/app-settings/panel/automations-panel/__tests__/automationRecipes.test.ts index f7c2c780e..726560e4b 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/__tests__/automationRecipes.test.ts +++ b/apps/client/src/features/app-settings/panel/automations-panel/__tests__/automationRecipes.test.ts @@ -1,106 +1,24 @@ -import { isHTTPOutput, isOSCOutput, isOntimeAction, timerLifecycleValues } from 'ontime-types'; +import { automationRecipes, defaultValues, getAvailableRecipes, validateRecipeValues } from '../automationRecipes'; -import { - automationRecipes, - defaultValues, - getAvailableRecipes, - needsTarget, - recipeCategoryOrder, - validateRecipeValues, -} from '../automationRecipes'; -import { operators } from '../automationUtils'; +function buildOutputs(id: string, values: Record): string { + const recipe = automationRecipes.find((candidate) => candidate.id === id); + return JSON.stringify(recipe?.build(values).outputs); +} -/** - * Recipes are shipped as constants but created through the same endpoint as a hand written - * automation. These assertions stand in for the server side validation, so a recipe cannot - * silently rot into something that 400s when the user presses create. - */ -describe('automationRecipes', () => { - const built = automationRecipes.map((recipe) => ({ recipe, automation: recipe.build(defaultValues(recipe)) })); - - it('has unique ids', () => { - const ids = automationRecipes.map(({ id }) => id); - expect(new Set(ids).size).toBe(ids.length); - }); - - it('only uses categories the picker knows how to render', () => { - for (const { recipe } of built) { - expect(recipeCategoryOrder).toContain(recipe.category); +describe('automation recipes', () => { + it('builds a valid default automation for every recipe', () => { + for (const recipe of automationRecipes) { + expect(recipe.triggers).not.toEqual([]); + expect(recipe.build(defaultValues(recipe))).toMatchObject({ + title: expect.any(String), + filterRule: 'all', + filters: [], + }); + expect(validateRecipeValues(recipe, defaultValues(recipe))).toEqual({}); } }); - it('binds every recipe to at least one valid lifecycle', () => { - for (const { recipe } of built) { - expect(recipe.triggers.length).toBeGreaterThan(0); - for (const cycle of recipe.triggers) { - expect(timerLifecycleValues).toContain(cycle); - } - } - }); - - it('builds a titled automation with something to send, from its own defaults', () => { - for (const { automation } of built) { - expect(automation.title).not.toBe(''); - expect(automation.outputs.length).toBeGreaterThan(0); - - for (const output of automation.outputs) { - expect(isOSCOutput(output) || isHTTPOutput(output) || isOntimeAction(output)).toBe(true); - } - } - }); - - it('gives every choice parameter options, and a default that is one of them', () => { - const choices = automationRecipes.flatMap(({ params }) => params.filter(({ type }) => type === 'choice')); - expect(choices.filter(({ options }) => !options?.length)).toEqual([]); - expect(choices.filter(({ options, defaultValue }) => !options?.some((o) => o.value === defaultValue))).toEqual([]); - }); - - it('reads every parameter it declares', () => { - // a param the builder ignores is a field the user fills in for nothing, and a typo in - // either half would put the literal 'undefined' inside a URL - for (const { recipe } of built) { - for (const param of recipe.params) { - // a choice can only take one of its own options, so probe with the last one - if (param.type === 'choice') { - const last = param.options?.at(-1)?.value ?? ''; - expect(JSON.stringify(recipe.build({ ...defaultValues(recipe), [param.name]: last }))).toContain(last); - continue; - } - const marker = param.type === 'number' ? '4242' : 'ontime-probe'; - const probed = { ...defaultValues(recipe), [param.name]: marker }; - expect(JSON.stringify(recipe.build(probed))).toContain(marker); - } - } - }); - - it('only uses filter operators the server accepts', () => { - const allowed = operators.map(({ value }) => value); - for (const { automation } of built) { - for (const filter of automation.filters) { - expect(allowed).toContain(filter.operator); - } - } - }); - - it('defaults every external target to this machine', () => { - const outputs = built.flatMap(({ automation }) => automation.outputs); - const osc = outputs.filter(isOSCOutput); - const http = outputs.filter(isHTTPOutput); - - // filtering rather than asserting in a branch, so a failure names the offending recipe - expect(osc.filter(({ targetIP }) => targetIP !== '127.0.0.1')).toEqual([]); - expect(osc.filter(({ targetPort }) => !Number.isFinite(targetPort))).toEqual([]); - expect(http.filter(({ url }) => !url.startsWith('http://127.0.0.1'))).toEqual([]); - }); - - it('flags the recipes that reach outside Ontime', () => { - for (const { recipe, automation } of built) { - const reachesOut = automation.outputs.some((output) => isOSCOutput(output) || isHTTPOutput(output)); - expect(needsTarget(recipe)).toBe(reachesOut); - } - }); - - it('hides local-network recipes in Ontime Cloud', () => { + it('hides recipes that require the local network in Ontime Cloud', () => { expect(getAvailableRecipes(true).map(({ id }) => id)).toEqual([ 'ontime-aux-timer', 'ontime-aux-stop', @@ -111,58 +29,21 @@ describe('automationRecipes', () => { ]); }); - it('validates recipe addresses and numeric bounds', () => { + it('accepts a single-label hostname for OSC', () => { const qlab = automationRecipes.find(({ id }) => id === 'qlab-go'); - const companion = automationRecipes.find(({ id }) => id === 'companion-press'); - const vmix = automationRecipes.find(({ id }) => id === 'vmix-overlay-warning'); - const webhook = automationRecipes.find(({ id }) => id === 'webhook-event-title'); - expect(qlab && validateRecipeValues(qlab, { ip: 'not a host', port: '70000' })).toEqual({ - ip: 'Enter an IP address or hostname', - port: 'Enter a whole number from 1 to 65535', - }); - expect( - companion && validateRecipeValues(companion, { host: 'localhost:8888', page: '1.5', row: '-1', column: '0' }), - ).toEqual({ - host: 'Enter a URL starting with http:// or https://', - page: 'Enter a whole number of 1 or more', - row: 'Enter a whole number of 0 or more', - }); - expect(vmix && validateRecipeValues(vmix, { host: 'http://127.0.0.1:8088', overlay: '5' })).toEqual({ - overlay: 'Enter a whole number from 1 to 4', - }); - expect(webhook && validateRecipeValues(webhook, { url: 'ftp://example.com' })).toEqual({ - url: 'Enter a URL starting with http:// or https://', - }); + expect(qlab && validateRecipeValues(qlab, { ip: 'qlab', port: '53000' })).toEqual({}); }); - it('accepts every recipe default', () => { - for (const recipe of automationRecipes) { - expect(validateRecipeValues(recipe, defaultValues(recipe))).toEqual({}); - } - }); - - /** the outputs a recipe builds from the given answers, as plain JSON to assert against */ - function buildWith(id: string, values: Record) { - const recipe = automationRecipes.find((candidate) => candidate.id === id); - return JSON.stringify(recipe?.build(values).outputs); - } - - it('tolerates a URL that already carries a query', () => { - expect(buildWith('webhook-event-title', { url: 'http://127.0.0.1:3000/now?source=ontime' })).toContain( - '/now?source=ontime&title=', + it('preserves an existing webhook query and URL-encodes its event title', () => { + expect(buildOutputs('webhook-event-title', { url: 'http://127.0.0.1:3000/now?source=ontime' })).toContain( + '/now?source=ontime&title={{url:eventNow.title}}', ); }); - it('marks the event title for URL-safe substitution', () => { - expect(buildWith('webhook-event-title', { url: 'http://127.0.0.1:3000/now' })).toContain( - 'title={{url:eventNow.title}}', - ); - }); - - it('tolerates an address pasted with a trailing slash', () => { + it('removes a pasted trailing slash before adding a Companion path', () => { expect( - buildWith('companion-press', { host: 'http://127.0.0.1:8888/', page: '1', row: '0', column: '0' }), + buildOutputs('companion-press', { host: 'http://127.0.0.1:8888/', page: '1', row: '0', column: '0' }), ).toContain('http://127.0.0.1:8888/api/location/1/0/0/press'); }); }); diff --git a/apps/client/src/features/app-settings/panel/automations-panel/automationRecipes.ts b/apps/client/src/features/app-settings/panel/automations-panel/automationRecipes.ts index be499beee..64a94279b 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/automationRecipes.ts +++ b/apps/client/src/features/app-settings/panel/automations-panel/automationRecipes.ts @@ -10,20 +10,15 @@ export const recipeCategoryLabels: Record = { messaging: 'Webhooks and messaging', }; -/** presentation order, empty categories are not rendered */ export const recipeCategoryOrder: RecipeCategory[] = ['ontime', 'playback', 'video', 'messaging']; export type RecipeParam = { name: string; label: string; - /** one line under the field, for anything the label cannot say */ hint?: string; type?: 'text' | 'number' | 'choice'; - /** required by 'choice', which renders a select rather than a free field */ options?: { value: string; label: string }[]; - /** takes a whole row: addresses and free text read badly in a narrow column */ wide?: boolean; - /** every default points at this machine, so a recipe cannot reach a venue network unasked */ defaultValue: string; validation?: { kind: 'host' } | { kind: 'url' } | { kind: 'integer'; min: number; max?: number }; }; @@ -31,20 +26,14 @@ export type RecipeParam = { export type RecipeValues = Record; export type AutomationRecipe = { - /** stable, client only. Never persisted */ id: string; title: string; - /** one line, plain language: what this does for the user */ description: string; category: RecipeCategory; - /** requires access to software on the same network as the Ontime server */ localOnly?: boolean; - /** extra search terms: other names for the software, its protocol, the job it does */ keywords?: string[]; - /** what the dialog asks for. Empty when the recipe needs nothing */ params: RecipeParam[]; triggers: TimerLifeCycle[]; - /** typed, so the compiler catches a recipe drifting from the automation schema */ build: (values: RecipeValues) => AutomationDTO; }; @@ -70,12 +59,10 @@ const auxStart = { 1: 'aux1-start', 2: 'aux2-start', 3: 'aux3-start' } as const; const auxStop = { 1: 'aux1-stop', 2: 'aux2-stop', 3: 'aux3-stop' } as const; const auxSource = { 1: 'aux1', 2: 'aux2', 3: 'aux3' } as const; -/** a user pasting an address is as likely to include the trailing slash as not */ function origin(value: string): string { return value.trim().replace(/\/+$/, ''); } -/** the recipe cannot know whether the user's URL already carries a query */ function withQuery(url: string, query: string): string { const trimmed = url.trim(); return trimmed.includes('?') ? `${trimmed}&${query}` : `${trimmed}?${query}`; @@ -215,7 +202,6 @@ export const automationRecipes: AutomationRecipe[] = [ triggers: [Cycle.onStart], build: ({ host, page, row, column }) => buildUnfilteredAutomation('Companion button press', [ - // Companion HTTP API: /api/location////press { type: 'http', url: `${origin(host)}/api/location/${page}/${row}/${column}/press` }, ]), }, @@ -273,7 +259,6 @@ export const automationRecipes: AutomationRecipe[] = [ }, ]; -/** the values the dialog starts with, so a recipe can be created without touching a field */ export function defaultValues(recipe: AutomationRecipe): RecipeValues { return Object.fromEntries(recipe.params.map(({ name, defaultValue }) => [name, defaultValue])); } @@ -324,8 +309,8 @@ function isHost(value: string): boolean { } const ipv4 = /^(?:(?:25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)$/; - const fqdn = /^(?=.{1,253}$)(?:[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?\.)+[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?$/i; - if (ipv4.test(value) || fqdn.test(value)) { + const hostname = /^(?=.{1,253}$)[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?(?:\.[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?)*$/i; + if (ipv4.test(value) || hostname.test(value)) { return true; } @@ -335,11 +320,3 @@ function isHost(value: string): boolean { return false; } } - -/** - * A recipe that only sends Ontime actions works the moment it is created. - * Anything else points at software we cannot locate for the user. - */ -export function needsTarget(recipe: AutomationRecipe): boolean { - return !recipe.build(defaultValues(recipe)).outputs.every((output) => output.type === 'ontime'); -}