diff --git a/apps/client/src/common/api/db.ts b/apps/client/src/common/api/db.ts index 1cab1e4db..733f82454 100644 --- a/apps/client/src/common/api/db.ts +++ b/apps/client/src/common/api/db.ts @@ -132,7 +132,7 @@ export async function partialDuplicateProject( filename: string, newFilename: string, sections: TemplateSection[], -): Promise { +): Promise<{ filename: string }> { const url = `${dbPath}/${filename}/partial-duplicate`; const decodedUrl = decodeURIComponent(url); const res = await axios.post(decodedUrl, { diff --git a/apps/client/src/common/utils/socket.ts b/apps/client/src/common/utils/socket.ts index 9df49bfd3..23a5c45cb 100644 --- a/apps/client/src/common/utils/socket.ts +++ b/apps/client/src/common/utils/socket.ts @@ -28,6 +28,7 @@ import { } from '../api/constants'; import { invalidateAllCaches } from '../api/utils'; import { ontimeQueryClient } from '../queryClient'; +import { addAutomationFired } from '../stores/automationFired'; import { getClientId, getClientName, @@ -37,7 +38,6 @@ import { setClients, } from '../stores/clientStore'; import { addDialog } from '../stores/dialogStore'; -import { addAutomationFired } from '../stores/automationFired'; import { addLog } from '../stores/logger'; import { patchRuntime, patchRuntimeProperty } from '../stores/runtime'; 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 7f9021ed6..77d8312ed 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 @@ -15,7 +15,13 @@ import { ReactNode, useEffect, useMemo, useRef, useState } from 'react'; import { useFieldArray, useForm } from 'react-hook-form'; import { IoAdd, IoCheckmark, IoTrash } from 'react-icons/io5'; -import { addAutomation, addTrigger, deleteTrigger, editAutomation, testOutput } from '../../../../common/api/automation'; +import { + addAutomation, + addTrigger, + deleteTrigger, + 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'; @@ -30,7 +36,6 @@ import Tag from '../../../../common/components/tag/Tag'; import useAutomationSettings from '../../../../common/hooks-query/useAutomationSettings'; import useCustomFields from '../../../../common/hooks-query/useCustomFields'; import { startsWithHttp } from '../../../../common/utils/regex'; -import { cx } from '../../../../common/utils/styleUtils'; import * as Panel from '../../panel-utils/PanelUtils'; import { cycles, isAutomation, makeFieldList, operators } from './automationUtils'; import OntimeActionForm from './OntimeActionForm'; @@ -64,12 +69,17 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa /** * Triggers are a separate entity, so they live outside the form state. - * We resolve the current selection once and reconcile it against the server on save. + * + * We snapshot the automation's triggers when the form opens and reconcile against that + * snapshot, never against 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. */ - const [initialCycles] = useState(() => - isAutomation(automation) - ? triggers.filter((trigger) => trigger.automationId === automation.id).map((trigger) => trigger.trigger) - : [], + const [initialTriggers] = useState(() => + isAutomation(automation) ? triggers.filter((trigger) => trigger.automationId === automation.id) : [], + ); + const initialCycles = useMemo( + () => Array.from(new Set(initialTriggers.map((trigger) => trigger.trigger))), + [initialTriggers], ); const [selectedCycles, setSelectedCycles] = useState(initialCycles); /** set once a create succeeds, so a retry after a failed trigger sync edits instead of creating a duplicate */ @@ -84,6 +94,12 @@ 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 = initialTriggers.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 @@ -229,17 +245,16 @@ 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. + * + * Both sides are diffed against the mount-time snapshot, so this only ever removes + * triggers the user could actually see when they made the change. */ const syncTriggers = async (automationId: string, title: string) => { - const existing = triggers.filter((trigger) => trigger.automationId === automationId); - - const toAdd = selectedCycles.filter((cycle) => !existing.some((trigger) => trigger.trigger === cycle)); - const toRemove = existing.filter((trigger) => !selectedCycles.includes(trigger.trigger)); - - for (const trigger of toRemove) { + for (const trigger of triggersToRemove) { await deleteTrigger(trigger.id); } + const toAdd = selectedCycles.filter((cycle) => !initialCycles.includes(cycle)); for (const cycle of toAdd) { const label = cycles.find(({ value }) => value === cycle)?.label ?? cycle; await addTrigger({ title: `${title} — ${label}`, trigger: cycle, automationId }); @@ -349,6 +364,13 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa to send on every tick. )} + {triggersToRemove.length > 0 && ( + + {`Saving removes ${triggersToRemove.length === 1 ? 'the trigger' : `${triggersToRemove.length} triggers`}: ${triggersToRemove + .map((trigger) => trigger.title) + .join(', ')}`} + + )} @@ -513,12 +535,7 @@ export default function AutomationForm({ automation, triggers, onClose }: Automa @@ -670,7 +687,7 @@ function OutputCard({ label, kindClass, summary, testState, onTest, onDelete, ch {testState?.status === 'error' && {testState.message}} -
{children}
+
{children}
); } 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 09f5019b3..acc1cc2fd 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 @@ -100,10 +100,12 @@ export default function AutomationSettingsForm({ - A trigger is when to send it. Triggers for a single event live in the event editor. - OSC Input tells Ontime to listen to messages on the specific port. - See the docs - + + See the docs + + 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 f156ed73e..8d9db0bbf 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 @@ -2,6 +2,7 @@ import { Automation, AutomationDTO, NormalisedAutomation, Trigger } from 'ontime import { Fragment, useEffect, useMemo, useState } from 'react'; import { IoAdd, IoPencil, IoShareOutline, IoSparklesOutline, IoTrash } from 'react-icons/io5'; +import { PROJECT_LIST } from '../../../../common/api/constants'; import Button from '../../../../common/components/buttons/Button'; import IconButton from '../../../../common/components/buttons/IconButton'; import { DropdownMenu } from '../../../../common/components/dropdown-menu/DropdownMenu'; @@ -10,13 +11,14 @@ import Tag from '../../../../common/components/tag/Tag'; import { getLifecycleLabel } from '../../../../common/constants/timerLifecycle'; import useAutomationSettings from '../../../../common/hooks-query/useAutomationSettings'; import { useOrderedProjectList } from '../../../../common/hooks-query/useProjectList'; +import { ontimeQueryClient } from '../../../../common/queryClient'; import { useAutomationFired } from '../../../../common/stores/automationFired'; import { summariseOutputs } from '../../../../common/utils/automationOutputs'; import * as Panel from '../../panel-utils/PanelUtils'; import useAppSettingsNavigation from '../../useAppSettingsNavigation'; import ProjectPartialCloneForm from '../project-panel/ProjectPartialCloneForm'; import AutomationForm from './AutomationForm'; -import { groupTriggersByAutomation } from './automationUtils'; +import { groupTriggersByAutomation, isAutomation } from './automationUtils'; import DeleteAutomationDialog from './DeleteAutomationDialog'; import RecipeLibraryModal from './recipes/RecipeLibraryModal'; @@ -36,7 +38,12 @@ interface AutomationsListProps { isLoading: boolean; } -export default function AutomationsList({ automations, triggers, enabledAutomations, isLoading }: AutomationsListProps) { +export default function AutomationsList({ + automations, + triggers, + enabledAutomations, + isLoading, +}: AutomationsListProps) { const { refetch } = useAutomationSettings(); const [automationFormData, setAutomationFormData] = useState(null); const [showRecipes, setShowRecipes] = useState(false); @@ -73,6 +80,9 @@ export default function AutomationsList({ automations, triggers, enabledAutomati {automationFormData !== null && ( setAutomationFormData(null)} @@ -90,6 +100,7 @@ export default function AutomationsList({ automations, triggers, enabledAutomati blockingTriggers={triggers.filter((trigger) => trigger.automationId === deleteTarget.id)} onCancel={() => setDeleteTarget(null)} onDeleted={handleDeleted} + onRefetch={refetch} /> )} {showTemplateForm && ( @@ -97,7 +108,11 @@ export default function AutomationsList({ automations, triggers, enabledAutomati fileName={lastLoadedProject} preselected={['automation']} onClose={() => setShowTemplateForm(false)} - onCreated={async () => setLocation('project__list')} + onCreated={async () => { + // do not rely on the project panel refetching when it mounts + await ontimeQueryClient.invalidateQueries({ queryKey: PROJECT_LIST }); + setLocation('project__list'); + }} /> )} @@ -244,13 +259,17 @@ export default function AutomationsList({ automations, triggers, enabledAutomati function LastFired({ at }: { at?: number }) { const [, setTick] = useState(0); + // once the label counts whole minutes it only changes once a minute, so drop to that + // cadence instead of holding a 1Hz timer per automation for the life of the panel + const isRecent = at !== undefined && Date.now() - at < millisPerMinute; + useEffect(() => { if (at === undefined) { return; } - const interval = setInterval(() => setTick((value) => value + 1), 1000); + const interval = setInterval(() => setTick((value) => value + 1), isRecent ? 1000 : millisPerMinute); return () => clearInterval(interval); - }, [at]); + }, [at, isRecent]); if (at === undefined) { return ; @@ -259,6 +278,8 @@ function LastFired({ at }: { at?: number }) { return {formatElapsed(Date.now() - at)}; } +const millisPerMinute = 60 * 1000; + function formatElapsed(elapsed: number): string { const seconds = Math.max(0, Math.floor(elapsed / 1000)); if (seconds < 5) { diff --git a/apps/client/src/features/app-settings/panel/automations-panel/DeleteAutomationDialog.tsx b/apps/client/src/features/app-settings/panel/automations-panel/DeleteAutomationDialog.tsx index 5db34ff68..9a1d8bb50 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/DeleteAutomationDialog.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/DeleteAutomationDialog.tsx @@ -1,7 +1,7 @@ import type { Automation, Trigger } from 'ontime-types'; import { useState } from 'react'; -import { deleteAutomation, deleteTrigger } from '../../../../common/api/automation'; +import { addTrigger, deleteAutomation, deleteTrigger } from '../../../../common/api/automation'; import { maybeAxiosError } from '../../../../common/api/utils'; import Button from '../../../../common/components/buttons/Button'; import Dialog from '../../../../common/components/dialog/Dialog'; @@ -15,6 +15,8 @@ interface DeleteAutomationDialogProps { blockingTriggers: Trigger[]; onCancel: () => void; onDeleted: () => void; + /** pulls fresh settings after a rollback, so the restored triggers carry their new ids */ + onRefetch: () => Promise; } /** @@ -29,22 +31,49 @@ export default function DeleteAutomationDialog({ blockingTriggers, onCancel, onDeleted, + onRefetch, }: DeleteAutomationDialogProps) { const [error, setError] = useState(null); const [isDeleting, setIsDeleting] = useState(false); + /** set only in the rare case where we could not undo our own trigger deletions */ + const [removeFailed, setRemoveFailed] = useState(false); + + /** + * Puts back triggers we deleted before the automation turned out to be undeletable. + * The new triggers get fresh ids, which nothing outside this panel holds on to + */ + const restoreTriggers = async (removed: Trigger[]) => { + for (const trigger of removed) { + try { + await addTrigger({ title: trigger.title, trigger: trigger.trigger, automationId: trigger.automationId }); + } catch (_error) { + setRemoveFailed(true); + } + } + }; const handleDelete = async () => { setError(null); setIsDeleting(true); + // the server reports the trigger references first, so an event reference only surfaces + // once the triggers are gone. Track them so a refusal does not cost the user their triggers + const removed: Trigger[] = []; + try { for (const trigger of blockingTriggers) { await deleteTrigger(trigger.id); + removed.push(trigger); } await deleteAutomation(automation.id); onDeleted(); } catch (error) { setError(maybeAxiosError(error)); + if (removed.length > 0) { + await restoreTriggers(removed); + // the restored triggers have new ids, the dialog needs them before a second attempt + await onRefetch(); + } } finally { setIsDeleting(false); } @@ -71,7 +100,9 @@ export default function DeleteAutomationDialog({ : `${blockingTriggers.length} triggers will be deleted with it`} - {blockingTriggers.map((trigger) => `${trigger.title} (${getLifecycleLabel(trigger.trigger)})`).join(', ')} + {blockingTriggers + .map((trigger) => `${trigger.title} (${getLifecycleLabel(trigger.trigger)})`) + .join(', ')} )} @@ -82,6 +113,9 @@ export default function DeleteAutomationDialog({ {error} Automations attached to a single event have to be removed from that event first, in the event editor. + {removeFailed + ? ' Your triggers could not be put back, you will need to recreate them.' + : ' Nothing was deleted.'} )} diff --git a/apps/client/src/features/app-settings/panel/automations-panel/recipes/RecipeLibraryModal.tsx b/apps/client/src/features/app-settings/panel/automations-panel/recipes/RecipeLibraryModal.tsx index 54e7ab4cc..737f9bbff 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/recipes/RecipeLibraryModal.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/recipes/RecipeLibraryModal.tsx @@ -11,7 +11,12 @@ import { getLifecycleLabel } from '../../../../../common/constants/timerLifecycl import { summariseOutputs } from '../../../../../common/utils/automationOutputs'; import { isOntimeCloud } from '../../../../../externals'; import * as Panel from '../../../panel-utils/PanelUtils'; -import { automationRecipes, recipeCategoryLabels, recipeCategoryOrder, type AutomationRecipe } from './automationRecipes'; +import { + automationRecipes, + recipeCategoryLabels, + recipeCategoryOrder, + type AutomationRecipe, +} from './automationRecipes'; import { installRecipe } from './recipeUtils'; import style from './RecipeLibraryModal.module.scss'; diff --git a/apps/client/src/features/app-settings/panel/automations-panel/recipes/automationRecipes.ts b/apps/client/src/features/app-settings/panel/automations-panel/recipes/automationRecipes.ts index 85af8fa78..f188d4243 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/recipes/automationRecipes.ts +++ b/apps/client/src/features/app-settings/panel/automations-panel/recipes/automationRecipes.ts @@ -76,7 +76,6 @@ export const automationRecipes: AutomationRecipe[] = [ title: 'Hide the stage message on finish', filterRule: 'all', filters: [], - // an empty text means "leave the text alone", so this only changes visibility outputs: [{ type: 'ontime', action: 'message-set', text: '', visible: false }], }, triggers: [Cycle.onFinish], @@ -84,14 +83,17 @@ export const automationRecipes: AutomationRecipe[] = [ { id: 'obs-record', title: 'OBS — start recording when the show starts', - description: 'Calls the OBS websocket HTTP bridge when the first event is loaded.', + description: + 'Presses a Companion button bound to OBS. obs-websocket speaks WebSocket rather than HTTP, so Ontime reaches OBS through Companion or a similar bridge.', category: 'video', + docsUrl: 'https://docs.getontime.no/api/automation/', needsSetup: true, automation: { title: 'OBS start recording', filterRule: 'all', filters: [], - outputs: [{ type: 'http', url: 'http://127.0.0.1:4455/api/StartRecord' }], + // Companion HTTP API: /api/location////press + outputs: [{ type: 'http', url: 'http://127.0.0.1:8888/api/location/1/0/1/press' }], }, triggers: [Cycle.onLoad], }, diff --git a/apps/client/src/features/app-settings/panel/project-panel/ProjectListItem.tsx b/apps/client/src/features/app-settings/panel/project-panel/ProjectListItem.tsx index 270428313..d0f397876 100644 --- a/apps/client/src/features/app-settings/panel/project-panel/ProjectListItem.tsx +++ b/apps/client/src/features/app-settings/panel/project-panel/ProjectListItem.tsx @@ -163,9 +163,7 @@ export default function ProjectListItem({ )} {showMergeForm && } - {showTemplateForm && ( - - )} + {showTemplateForm && } setDeleteOpen(false)} diff --git a/apps/client/src/features/app-settings/panel/project-panel/ProjectPartialCloneForm.tsx b/apps/client/src/features/app-settings/panel/project-panel/ProjectPartialCloneForm.tsx index f89f9c15c..025c421ea 100644 --- a/apps/client/src/features/app-settings/panel/project-panel/ProjectPartialCloneForm.tsx +++ b/apps/client/src/features/app-settings/panel/project-panel/ProjectPartialCloneForm.tsx @@ -29,7 +29,8 @@ const sectionCopy: Array<{ key: TemplateSection; title: string; body: string }> interface ProjectPartialCloneFormProps { onClose: () => void; - onCreated: () => Promise; + /** receives the name the file was actually given, which can differ if the name was taken */ + onCreated: (createdFilename: string) => Promise; fileName: string; /** sections switched on when the form opens */ preselected?: TemplateSection[]; @@ -76,8 +77,8 @@ export default function ProjectPartialCloneForm({ try { setError(null); - await partialDuplicateProject(fileName, values.filename, sections); - await onCreated(); + const { filename } = await partialDuplicateProject(fileName, values.filename, sections); + await onCreated(filename); onClose(); } catch (error) { setError(maybeAxiosError(error)); @@ -137,8 +138,9 @@ export default function ProjectPartialCloneForm({ - Automations attached to individual events live in the rundown. A template without rundowns carries the - automations themselves, but not the events that point at them. + Automations attached to individual events live in the rundown, so the two halves travel separately. A + template with only automations leaves behind the events that point at them, and a template with only + rundowns carries events pointing at automations it does not include. diff --git a/apps/server/src/api-data/automation/__tests__/automation.service.test.ts b/apps/server/src/api-data/automation/__tests__/automation.service.test.ts index 3eb3d9221..67114f452 100644 --- a/apps/server/src/api-data/automation/__tests__/automation.service.test.ts +++ b/apps/server/src/api-data/automation/__tests__/automation.service.test.ts @@ -702,18 +702,44 @@ describe('automation reporting', () => { expect(logSpy.mock.calls[0][1]).toContain('suppressed'); }); - it('shows the suppression notice again after a reload', async () => { + it('does not repeat the suppression notice on every load', async () => { await bind('reporting-clock', TimerLifeCycle.onClock); triggerAutomations(TimerLifeCycle.onClock); logSpy.mockClear(); - // onLoad bookends a run and resets the reporting state - triggerAutomations(TimerLifeCycle.onLoad); + // roll mode loads at every event boundary, and an operator steps through cues by hand. + // Re-notifying on each one would be the very flooding the notice exists to prevent + for (let i = 0; i < 5; i++) { + triggerAutomations(TimerLifeCycle.onLoad); + triggerAutomations(TimerLifeCycle.onClock); + } + + expect(logSpy.mock.calls.filter(([, message]) => String(message).includes('suppressed'))).toHaveLength(0); + }); + + it('shows the suppression notice again after a stop', async () => { + await bind('reporting-clock', TimerLifeCycle.onClock); + triggerAutomations(TimerLifeCycle.onClock); + logSpy.mockClear(); + + // a stop ends the run, the next one reports from scratch + triggerAutomations(TimerLifeCycle.onStop); triggerAutomations(TimerLifeCycle.onClock); expect(logSpy.mock.calls.some(([, message]) => String(message).includes('suppressed'))).toBe(true); }); + it('throttles repeated loads, which the reset used to defeat', async () => { + vi.useFakeTimers(); + await bind('reporting-load', TimerLifeCycle.onLoad); + logSpy.mockClear(); + + triggerAutomations(TimerLifeCycle.onLoad); + triggerAutomations(TimerLifeCycle.onLoad); + + expect(logSpy).toHaveBeenCalledTimes(1); + }); + it('collapses repeats of the same automation and cycle inside the throttle window', async () => { vi.useFakeTimers(); await bind('reporting-danger', TimerLifeCycle.onDanger); diff --git a/apps/server/src/api-data/automation/automation.service.ts b/apps/server/src/api-data/automation/automation.service.ts index 24bccb06b..4b3fbd7dc 100644 --- a/apps/server/src/api-data/automation/automation.service.ts +++ b/apps/server/src/api-data/automation/automation.service.ts @@ -41,9 +41,9 @@ const lastLoggedAt = new Map(); const lastReportedAt = new Map(); /** - * Clears the per-load logging state. - * Called when the runtime loads or stops so the suppression notice is shown again - * for the next show rather than once per server lifetime + * Clears the reporting state. + * Called when the runtime stops, so the next run reports from scratch rather than + * inheriting throttles from the last one */ export function resetAutomationLogState() { suppressionNotices.clear(); @@ -55,15 +55,24 @@ export function resetAutomationLogState() { * Exposes a method for triggering actions based on a TimerLifeCycle event */ export function triggerAutomations(cycle: TimerLifeCycle) { - // a load or a stop bookends a run: start reporting from scratch so the next show - // gets its own suppression notice rather than inheriting one from the last - if (cycle === TimerLifeCycle.onLoad || cycle === TimerLifeCycle.onStop) { - resetAutomationLogState(); - } - if (!getAutomationsEnabled()) { return; } + + fireForCycle(cycle); + + // A stop ends a run, so the next one reports from scratch. This deliberately does not + // happen on load: loading is not rare, roll mode loads at every event boundary, and + // resetting there would re-emit the suppression notice once per cue, which is the + // flooding the notice exists to prevent. + // It sits out here because fireForCycle returns early when nothing is bound to onStop, + // which is the common case + if (cycle === TimerLifeCycle.onStop) { + resetAutomationLogState(); + } +} + +function fireForCycle(cycle: TimerLifeCycle) { const store = eventStore.poll(); let triggers = getAutomationTriggers(); diff --git a/apps/server/src/api-data/db/db.controller.ts b/apps/server/src/api-data/db/db.controller.ts index ee4e2fba1..61fe23eb3 100644 --- a/apps/server/src/api-data/db/db.controller.ts +++ b/apps/server/src/api-data/db/db.controller.ts @@ -235,16 +235,15 @@ export async function loadDemo(_req: Request, res: Response) { +export async function partialDuplicateProjectFile(req: Request, res: Response<{ filename: string } | ErrorResponse>) { const { filename } = req.params; const { newFilename, sections } = req.body; try { + // the created name can differ from what was asked for, generateUniqueFileName resolves collisions const created = await projectService.createProjectFromSections(filename, newFilename, sections); - res.status(201).send({ - message: `Created template ${created} from ${filename}`, - }); + res.status(201).send({ filename: created }); } catch (error) { const message = getErrorMessage(error); if (message.startsWith('Project file')) { diff --git a/apps/server/src/api-data/db/db.validation.ts b/apps/server/src/api-data/db/db.validation.ts index ba735a22c..cdb58d3f0 100644 --- a/apps/server/src/api-data/db/db.validation.ts +++ b/apps/server/src/api-data/db/db.validation.ts @@ -75,7 +75,9 @@ export const validateSectionsBody = [ body('sections') .isArray({ min: 1 }) .withMessage(`Select at least one of: ${templateSections.join(', ')}`) - .custom((sections: unknown[]) => sections.every((section) => typeof section === 'string' && isTemplateSection(section))) + .custom((sections: unknown[]) => + sections.every((section) => typeof section === 'string' && isTemplateSection(section)), + ) .withMessage(`Sections must be any of: ${templateSections.join(', ')}`), requestValidationFunction, diff --git a/apps/server/src/models/demoProject.ts b/apps/server/src/models/demoProject.ts index 7e68fc583..47cd28464 100644 --- a/apps/server/src/models/demoProject.ts +++ b/apps/server/src/models/demoProject.ts @@ -99,6 +99,12 @@ export const demoDb: DatabaseModel = { trigger: TimerLifeCycle.onStart, automationId: 'demo-aux-timer', }, + { + id: 'demo-trigger-clear', + title: 'Demo: clear the wrap up warning', + trigger: TimerLifeCycle.onFinish, + automationId: 'demo-clear-message', + }, ], automations: { 'demo-aux-timer': { @@ -119,6 +125,15 @@ export const demoDb: DatabaseModel = { // self labelled, so nobody mistakes it for something Ontime does on its own outputs: [{ type: 'ontime', action: 'message-set', text: 'Demo automation: please wrap up', visible: true }], }, + 'demo-clear-message': { + id: 'demo-clear-message', + title: 'Demo: clear the wrap up warning', + filterRule: 'all', + filters: [], + // the pair to the warning above. Without it the message would stay on the stage + // timer for the rest of the session, blanking the countdown on every later event + outputs: [{ type: 'ontime', action: 'message-set', text: '', visible: false }], + }, 'demo-osc-example': { id: 'demo-osc-example', title: 'Demo: OSC to a lighting console (example, not wired up)', diff --git a/apps/server/src/services/project-service/ProjectService.ts b/apps/server/src/services/project-service/ProjectService.ts index e52e0f95b..33e7a3289 100644 --- a/apps/server/src/services/project-service/ProjectService.ts +++ b/apps/server/src/services/project-service/ProjectService.ts @@ -9,7 +9,7 @@ import { parseDatabaseModel } from '../../api-data/db/db.parser.js'; import { getCurrentRundown } from '../../api-data/rundown/rundown.dao.js'; import { parseRundowns } from '../../api-data/rundown/rundown.parser.js'; import { initRundown } from '../../api-data/rundown/rundown.service.js'; -import { getDataProvider, initPersistence } from '../../classes/data-provider/DataProvider.js'; +import { flushPendingWrites, getDataProvider, initPersistence } from '../../classes/data-provider/DataProvider.js'; import { safeMerge } from '../../classes/data-provider/DataProvider.utils.js'; import { logger } from '../../classes/Logger.js'; import { makeNewProject } from '../../models/dataModel.js'; @@ -342,6 +342,10 @@ export async function createProjectFromSections( throw new Error('At least one section must be selected'); } + // writes are debounced, and the natural flow here is "make some automations, then save them + // as a template". Without this the template silently misses anything from the last few seconds + await flushPendingWrites(); + const fileData = await parseJsonFile(projectFilePath); const { data } = parseDatabaseModel(fileData); @@ -355,6 +359,13 @@ export async function createProjectFromSections( } const template = safeMerge(makeNewProject(), patch); + + // makeNewProject seeds an empty rundown and safeMerge merges rundowns by key, so without + // this the template would ship a phantom "Default" rundown alongside the real ones + if (patch.rundowns !== undefined) { + template.rundowns = patch.rundowns; + } + const fileNameWithExtension = generateUniqueFileName(publicDir.projectsDir, ensureJsonExtension(newFilename)); await writeFile(getPathToProject(fileNameWithExtension), JSON.stringify(template, null, 2), 'utf-8'); diff --git a/apps/server/src/services/project-service/__tests__/ProjectService.test.ts b/apps/server/src/services/project-service/__tests__/ProjectService.test.ts index 6c02eff7e..86f1dc7a9 100644 --- a/apps/server/src/services/project-service/__tests__/ProjectService.test.ts +++ b/apps/server/src/services/project-service/__tests__/ProjectService.test.ts @@ -3,6 +3,7 @@ import { writeFile } from 'fs/promises'; import { OntimeView, TimerLifeCycle } from 'ontime-types'; import { Mock } from 'vitest'; +import { makeNewProject } from '../../../models/dataModel.js'; import { isLastLoadedProject } from '../../app-state-service/AppStateService.js'; import { createProjectFromSections, @@ -11,7 +12,6 @@ import { renameProjectFile, } from '../ProjectService.js'; import { doesProjectExist, parseJsonFile } from '../projectServiceUtils.js'; -import { makeNewProject } from '../../../models/dataModel.js'; // stop the database loading from initiating vi.mock('../../../setup/loadDb.js', () => { @@ -100,16 +100,20 @@ describe('createProjectFromSections', () => { (doesProjectExist as Mock).mockReturnValue('/projects/source.json'); (parseJsonFile as Mock).mockResolvedValue({ ...makeNewProject(), - urlPresets: [ - { target: OntimeView.Timer, enabled: true, alias: 'from-source', search: '', displayInNav: false }, - ], + urlPresets: [{ target: OntimeView.Timer, enabled: true, alias: 'from-source', search: '', displayInNav: false }], automation: { enabledAutomations: true, enabledOscIn: false, oscPortIn: 8888, triggers: [{ id: 't1', title: 'on start', trigger: TimerLifeCycle.onStart, automationId: 'a1' }], automations: { - a1: { id: 'a1', title: 'from source', filterRule: 'all', filters: [], outputs: [{ type: 'http', url: 'http://127.0.0.1/go' }] }, + a1: { + id: 'a1', + title: 'from source', + filterRule: 'all', + filters: [], + outputs: [{ type: 'http', url: 'http://127.0.0.1/go' }], + }, }, }, }); diff --git a/e2e/tests/features/215-automations.spec.ts b/e2e/tests/features/215-automations.spec.ts index b356f61ce..0b02b96d2 100644 --- a/e2e/tests/features/215-automations.spec.ts +++ b/e2e/tests/features/215-automations.spec.ts @@ -12,11 +12,34 @@ const templateName = 'e2e-automations-template'; * the template carries the automation and its trigger while leaving the rundown behind. */ test.describe('automations', () => { + /** + * Everything created is torn down in afterEach rather than at the end of the test body: + * a mid-test failure would otherwise leak an automation into the project, and CI retries twice. + * + * The template is tracked by the name the server actually used, since it resolves collisions. + */ + let createdTemplate: string | null = null; + let createdTriggers: string[] = []; + let createdAutomations: string[] = []; + test.afterEach(async ({ request }) => { try { - await request.delete(`${dbURL}/${templateName}.json`); + // triggers first, the server refuses to delete an automation that is still referenced + for (const id of createdTriggers) { + await request.delete(`${automationsURL}/trigger/${id}`); + } + for (const id of createdAutomations) { + await request.delete(`${automationsURL}/automation/${id}`); + } + if (createdTemplate !== null) { + await request.delete(`${dbURL}/${createdTemplate}`); + } } catch { - /** nothing to do here */ + // cleanup is best effort, it must not turn a passing test red + } finally { + createdTriggers = []; + createdAutomations = []; + createdTemplate = null; } }); @@ -32,13 +55,14 @@ test.describe('automations', () => { }); expect(createAutomation.status()).toBe(201); const automation = await createAutomation.json(); + createdAutomations.push(automation.id); // 2. bind it to a lifecycle const createTrigger = await request.post(`${automationsURL}/trigger`, { data: { title: 'e2e trigger', trigger: 'onStart', automationId: automation.id }, }); expect(createTrigger.status()).toBe(201); - const trigger = await createTrigger.json(); + createdTriggers.push((await createTrigger.json()).id); // 3. save the automations as a template, without touching the loaded project const projectList = await (await request.get(`${dbURL}/all`)).json(); @@ -48,22 +72,20 @@ test.describe('automations', () => { data: { newFilename: templateName, sections: ['automation'] }, }); expect(makeTemplate.status()).toBe(201); + createdTemplate = (await makeTemplate.json()).filename; + expect(createdTemplate).toBeTruthy(); // the running project is untouched const afterTemplate = await (await request.get(`${dbURL}/all`)).json(); expect(afterTemplate.lastLoadedProject).toBe(currentProject); // 4. the template carries the automation and its trigger, and nothing else - const template = await (await request.post(`${dbURL}/download`, { data: { filename: templateName } })).json(); + const template = await (await request.post(`${dbURL}/download`, { data: { filename: createdTemplate } })).json(); expect(Object.values(template.automation.automations)).toContainEqual( expect.objectContaining({ title: 'e2e automation' }), ); expect(template.automation.triggers).toContainEqual(expect.objectContaining({ title: 'e2e trigger' })); expect(template.urlPresets).toEqual([]); - - // 5. clean up: the trigger has to go first, the server refuses to delete a referenced automation - expect((await request.delete(`${automationsURL}/trigger/${trigger.id}`)).status()).toBe(204); - expect((await request.delete(`${automationsURL}/automation/${automation.id}`)).status()).toBe(204); }); test('refuses to delete an automation that a trigger still points at', async ({ request }) => { @@ -77,18 +99,21 @@ test.describe('automations', () => { }, }) ).json(); + createdAutomations.push(automation.id); const trigger = await ( await request.post(`${automationsURL}/trigger`, { data: { title: 'e2e blocking trigger', trigger: 'onFinish', automationId: automation.id }, }) ).json(); + createdTriggers.push(trigger.id); const refused = await request.delete(`${automationsURL}/automation/${automation.id}`); expect(refused.status()).toBe(400); expect((await refused.json()).message).toContain('e2e blocking trigger'); - await request.delete(`${automationsURL}/trigger/${trigger.id}`); + // and it goes through once the reference is removed + expect((await request.delete(`${automationsURL}/trigger/${trigger.id}`)).status()).toBe(204); expect((await request.delete(`${automationsURL}/automation/${automation.id}`)).status()).toBe(204); }); });