refactor(automation): trim recipe scaffolding

2026-09-12 01:29:35 +00:00 · 2026-09-11 20:06:20 +02:00
parent 397b54a472
commit 1a7bc2da25
5 changed files with 26 additions and 200 deletions
@@ -80,7 +80,6 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa
    [syncedTriggers],
  );
  const [selectedCycles, setSelectedCycles] = useState<TimerLifeCycle[]>(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) {
@@ -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;
 }
@@ -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<AutomationRecipe | null>(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) {
          )}

          <Panel.Description>
-            {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.
          </Panel.Description>
        </div>
      }
@@ -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, string>): 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<string, string>) {
-    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');
  });
 });
@@ -10,20 +10,15 @@ export const recipeCategoryLabels: Record<RecipeCategory, string> = {
  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<string, string>;

 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/<page>/<row>/<column>/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');
-}