From 38e3ab979d3edb5cc47b15b6b127fb7444b413e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 17:31:27 +0000 Subject: [PATCH] fix: resync aux timer names on project load, consolidate name handling Addresses the review findings on the aux timer naming feature. - Aux timer names are project data, but they were only applied at bootstrap and on a settings POST. Loading another project left the previous project's names in the runtime store, so the controls and views disagreed with the settings panel until a restart. The names are now applied when a project is loaded, and patching the current project settings applies them and sends a settings refetch to the clients (loading a project already triggers a full refetch via the rundown). - Consolidate the name handling into a single normaliser in ontime-utils, used by the project file parser, the API validation and to build the default value. This creates the property when importing project files saved before the feature existed, and enforces the name length limit server side rather than only in the form. - loadNames normalises its input, so the runtime state is consistent regardless of the shape of the stored settings. - Clarify that auxTimerNames is ordered (index 0 is aux timer 1), and derive the settings form from the shared aux timer count. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WCZejVTzuAY3tHTE6nB3JH --- .../src/common/models/OntimeSettings.ts | 3 +- .../panel/settings-panel/AuxTimerSettings.tsx | 12 +++++-- .../api-data/db/__tests__/db.parser.test.ts | 10 ++++++ .../api-data/db/migration/db.migration.v3.ts | 11 ++++++- .../api-data/db/migration/db.migration.v4.ts | 3 +- .../__tests__/settings.parser.test.ts | 17 ++++++++++ .../src/api-data/settings/settings.parser.ts | 16 ++-------- .../api-data/settings/settings.validation.ts | 8 ++--- apps/server/src/app.ts | 3 +- apps/server/src/models/dataModel.ts | 4 +-- apps/server/src/models/demoProject.ts | 3 +- .../aux-timer-service/AuxTimerService.ts | 15 +++++---- .../__tests__/AuxTimerService.test.ts | 12 +++++++ .../project-service/ProjectService.ts | 14 ++++++++- .../src/definitions/core/Settings.type.ts | 5 ++- packages/utils/index.ts | 7 +++++ .../src/aux-timer-utils/auxTimerUtils.test.ts | 31 +++++++++++++++++++ .../src/aux-timer-utils/auxTimerUtils.ts | 22 +++++++++++++ .../src/rundown-utils/rundownUtils.mock.ts | 4 ++- 19 files changed, 163 insertions(+), 37 deletions(-) create mode 100644 packages/utils/src/aux-timer-utils/auxTimerUtils.test.ts create mode 100644 packages/utils/src/aux-timer-utils/auxTimerUtils.ts diff --git a/apps/client/src/common/models/OntimeSettings.ts b/apps/client/src/common/models/OntimeSettings.ts index a12319f64..2ef016c05 100644 --- a/apps/client/src/common/models/OntimeSettings.ts +++ b/apps/client/src/common/models/OntimeSettings.ts @@ -1,4 +1,5 @@ import { Settings } from 'ontime-types'; +import { normaliseAuxTimerNames } from 'ontime-utils'; export const ontimePlaceholderSettings: Settings = { version: '4.0.0', @@ -6,5 +7,5 @@ export const ontimePlaceholderSettings: Settings = { operatorKey: null, timeFormat: '24', language: 'en', - auxTimerNames: ['', '', ''], + auxTimerNames: normaliseAuxTimerNames(), }; diff --git a/apps/client/src/features/app-settings/panel/settings-panel/AuxTimerSettings.tsx b/apps/client/src/features/app-settings/panel/settings-panel/AuxTimerSettings.tsx index 0128c38ed..cff483035 100644 --- a/apps/client/src/features/app-settings/panel/settings-panel/AuxTimerSettings.tsx +++ b/apps/client/src/features/app-settings/panel/settings-panel/AuxTimerSettings.tsx @@ -1,4 +1,5 @@ import { Settings } from 'ontime-types'; +import { auxTimerNameMaxLength, numberOfAuxTimers } from 'ontime-utils'; import { useEffect } from 'react'; import { useForm } from 'react-hook-form'; @@ -11,7 +12,8 @@ import useSettings from '../../../../common/hooks-query/useSettings'; import { preventEscape } from '../../../../common/utils/keyEvent'; import * as Panel from '../../panel-utils/PanelUtils'; -const auxTimerIndexes = [1, 2, 3]; +/** zero based index of each aux timer, used to address the auxTimerNames array */ +const auxTimerIndexes = Array.from({ length: numberOfAuxTimers }, (_, index) => index); export default function AuxTimerSettings() { const { data, status, refetch } = useSettings(); @@ -79,8 +81,12 @@ export default function AuxTimerSettings() { {auxTimerIndexes.map((index) => ( - - + + ))} diff --git a/apps/server/src/api-data/db/__tests__/db.parser.test.ts b/apps/server/src/api-data/db/__tests__/db.parser.test.ts index 608445935..c2b0e8f48 100644 --- a/apps/server/src/api-data/db/__tests__/db.parser.test.ts +++ b/apps/server/src/api-data/db/__tests__/db.parser.test.ts @@ -90,4 +90,14 @@ describe('test parseDatabaseModel() edge cases', () => { // @ts-expect-error -- we know this is wrong, testing imports outside domain expect(() => parseDatabaseModel('some random dataset')).toThrow(); }); + + it('creates the aux timer names when importing a project file which predates the feature', () => { + const oldProject = structuredClone(demoDb); + // @ts-expect-error -- simulating a project file saved before aux timer naming existed + delete oldProject.settings.auxTimerNames; + + const { data } = parseDatabaseModel(oldProject); + + expect(data.settings.auxTimerNames).toStrictEqual(['', '', '']); + }); }); diff --git a/apps/server/src/api-data/db/migration/db.migration.v3.ts b/apps/server/src/api-data/db/migration/db.migration.v3.ts index 8713a0b79..930fa0452 100644 --- a/apps/server/src/api-data/db/migration/db.migration.v3.ts +++ b/apps/server/src/api-data/db/migration/db.migration.v3.ts @@ -23,6 +23,7 @@ import { customFieldLabelToKey, eventDef as eventModel, isKnownTimerType, + normaliseAuxTimerNames, validateEndAction, } from 'ontime-utils'; @@ -74,7 +75,15 @@ export function migrateSettings(jsonData: object): (Settings & { serverPort: num const { serverPort, editorKey, operatorKey, timeFormat, language } = structuredClone( jsonData.settings, ) as old_Settings; - return { version: '4.0.0', serverPort, editorKey, operatorKey, timeFormat, language, auxTimerNames: ['', '', ''] }; + return { + version: '4.0.0', + serverPort, + editorKey, + operatorKey, + timeFormat, + language, + auxTimerNames: normaliseAuxTimerNames(), + }; } } diff --git a/apps/server/src/api-data/db/migration/db.migration.v4.ts b/apps/server/src/api-data/db/migration/db.migration.v4.ts index 90e75a948..ea7e1208a 100644 --- a/apps/server/src/api-data/db/migration/db.migration.v4.ts +++ b/apps/server/src/api-data/db/migration/db.migration.v4.ts @@ -1,4 +1,5 @@ import { DatabaseModel, Settings } from 'ontime-types'; +import { normaliseAuxTimerNames } from 'ontime-utils'; import { is } from '../../../utils/is.js'; @@ -23,7 +24,7 @@ export function migrateServerPort(jsonData: Partial): { const operatorKey = settings?.operatorKey; const timeFormat = settings?.timeFormat; const language = settings?.language; - const auxTimerNames = settings?.auxTimerNames ?? ['', '', '']; + const auxTimerNames = normaliseAuxTimerNames(settings?.auxTimerNames); const version = '4.5.0'; db.settings = { version, diff --git a/apps/server/src/api-data/settings/__tests__/settings.parser.test.ts b/apps/server/src/api-data/settings/__tests__/settings.parser.test.ts index ed5e909f7..48906eb2e 100644 --- a/apps/server/src/api-data/settings/__tests__/settings.parser.test.ts +++ b/apps/server/src/api-data/settings/__tests__/settings.parser.test.ts @@ -33,4 +33,21 @@ describe('parseSettings()', () => { }); expect(result.auxTimerNames).toStrictEqual(['', '', '']); }); + + it('creates the aux timer names for project files made before the feature existed', () => { + // a settings object as found in a project file which predates aux timer naming + const oldSettings = { + version: '4.5.0', + editorKey: null, + operatorKey: null, + timeFormat: '24', + language: 'en', + }; + + const result = parseSettings({ settings: oldSettings as Settings }); + + expect(result.auxTimerNames).toStrictEqual(['', '', '']); + // the rest of the settings are untouched + expect(result).toMatchObject({ timeFormat: '24', language: 'en' }); + }); }); diff --git a/apps/server/src/api-data/settings/settings.parser.ts b/apps/server/src/api-data/settings/settings.parser.ts index 378476138..3e19847f0 100644 --- a/apps/server/src/api-data/settings/settings.parser.ts +++ b/apps/server/src/api-data/settings/settings.parser.ts @@ -1,4 +1,5 @@ import { DatabaseModel, Settings } from 'ontime-types'; +import { normaliseAuxTimerNames } from 'ontime-utils'; import { getPartialProject } from '../../models/dataModel.js'; @@ -22,18 +23,7 @@ export function parseSettings(data: Partial): Settings { operatorKey: data.settings.operatorKey ?? defaultSettings.operatorKey, timeFormat: data.settings.timeFormat ?? defaultSettings.timeFormat, language: data.settings.language ?? defaultSettings.language, - auxTimerNames: sanitiseAuxTimerNames(data.settings.auxTimerNames, defaultSettings.auxTimerNames), + // property added in v4.6.0, older project files will not contain it + auxTimerNames: normaliseAuxTimerNames(data.settings.auxTimerNames), }; } - -/** - * Ensures the aux timer names are a fixed-length array of strings - * regardless of what is found in the file - */ -function sanitiseAuxTimerNames(maybeNames: unknown, fallback: string[]): string[] { - const source = Array.isArray(maybeNames) ? maybeNames : []; - return fallback.map((defaultName, index) => { - const value = source[index]; - return typeof value === 'string' ? value : defaultName; - }); -} diff --git a/apps/server/src/api-data/settings/settings.validation.ts b/apps/server/src/api-data/settings/settings.validation.ts index 031c2a473..eebac488e 100644 --- a/apps/server/src/api-data/settings/settings.validation.ts +++ b/apps/server/src/api-data/settings/settings.validation.ts @@ -1,4 +1,5 @@ import { body } from 'express-validator'; +import { normaliseAuxTimerNames } from 'ontime-utils'; import { requestValidationFunction } from '../validation-utils/validationFunction.js'; @@ -30,11 +31,8 @@ export const validateSettings = [ body('auxTimerNames') .isArray() .withMessage('auxTimerNames must be an array') - .customSanitizer((value: unknown) => { - // normalise to a fixed-length array of trimmed strings - const source = Array.isArray(value) ? value : []; - return [0, 1, 2].map((index) => (typeof source[index] === 'string' ? source[index].trim() : '')); - }), + // normalise to a fixed length array of trimmed, length capped strings + .customSanitizer(normaliseAuxTimerNames), requestValidationFunction, ]; diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index f56d646ca..395a35cc0 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -5,6 +5,7 @@ import cookieParser from 'cookie-parser'; import cors from 'cors'; import express from 'express'; import { LogOrigin, SimpleDirection, SimplePlayback, runtimeStorePlaceholder } from 'ontime-types'; +import { normaliseAuxTimerNames } from 'ontime-utils'; import serverTiming from 'server-timing'; import { oscServer } from './adapters/OscAdapter.js'; @@ -205,7 +206,7 @@ export const startServer = async (): Promise<{ message: string; serverPort: numb * Module initialises the services and provides initial payload for the store */ const state = getState(); - const { auxTimerNames } = getDataProvider().getSettings(); + const auxTimerNames = normaliseAuxTimerNames(getDataProvider().getSettings().auxTimerNames); eventStore.init({ clock: state.clock, timer: state.timer, diff --git a/apps/server/src/models/dataModel.ts b/apps/server/src/models/dataModel.ts index e77c557d0..5bd188db8 100644 --- a/apps/server/src/models/dataModel.ts +++ b/apps/server/src/models/dataModel.ts @@ -1,5 +1,5 @@ import { DatabaseModel, Rundown } from 'ontime-types'; -import { generateId } from 'ontime-utils'; +import { generateId, normaliseAuxTimerNames } from 'ontime-utils'; import { ONTIME_VERSION } from '../ONTIME_VERSION.js'; @@ -30,7 +30,7 @@ const dbModel: DatabaseModel = { operatorKey: null, timeFormat: '24', language: 'en', - auxTimerNames: ['', '', ''], + auxTimerNames: normaliseAuxTimerNames(), }, viewSettings: { overrideStyles: false, diff --git a/apps/server/src/models/demoProject.ts b/apps/server/src/models/demoProject.ts index 0bc1ad97c..5dd5755d6 100644 --- a/apps/server/src/models/demoProject.ts +++ b/apps/server/src/models/demoProject.ts @@ -1,4 +1,5 @@ import { DatabaseModel, OntimeView } from 'ontime-types'; +import { normaliseAuxTimerNames } from 'ontime-utils'; import { backstageRundown, broadcastRundown, stageRundown } from './demoRundowns.js'; @@ -29,7 +30,7 @@ export const demoDb: DatabaseModel = { operatorKey: null, timeFormat: '24', language: 'en', - auxTimerNames: ['', '', ''], + auxTimerNames: normaliseAuxTimerNames(), }, viewSettings: { dangerColor: '#ff7300', diff --git a/apps/server/src/services/aux-timer-service/AuxTimerService.ts b/apps/server/src/services/aux-timer-service/AuxTimerService.ts index 96f3b1930..affd60664 100644 --- a/apps/server/src/services/aux-timer-service/AuxTimerService.ts +++ b/apps/server/src/services/aux-timer-service/AuxTimerService.ts @@ -1,4 +1,5 @@ import { RuntimeStore, SimpleDirection, SimplePlayback } from 'ontime-types'; +import { normaliseAuxTimerNames } from 'ontime-utils'; import { SimpleTimer } from '../../classes/simple-timer/SimpleTimer.js'; import { timerConfig } from '../../setup/config.js'; @@ -26,14 +27,16 @@ export class AuxTimerService { /** * Applies custom names to the aux timers and broadcasts the change. - * Names are indexed by aux timer position (0 -> aux1, 1 -> aux2, 2 -> aux3). - * Used to seed the names at bootstrap and to keep them in sync with the settings. + * Names are given in aux timer order (index 0 is aux timer 1). + * Used to seed the names at bootstrap and to keep them in sync + * with the settings of the loaded project. */ - loadNames(names: string[]) { + loadNames(names?: string[]) { + const [name1, name2, name3] = normaliseAuxTimerNames(names); const patch: AuxTimerStateUpdate = { - auxtimer1: this.aux1.setName(names[0] ?? ''), - auxtimer2: this.aux2.setName(names[1] ?? ''), - auxtimer3: this.aux3.setName(names[2] ?? ''), + auxtimer1: this.aux1.setName(name1), + auxtimer2: this.aux2.setName(name2), + auxtimer3: this.aux3.setName(name3), }; this.emit(patch); } diff --git a/apps/server/src/services/aux-timer-service/__tests__/AuxTimerService.test.ts b/apps/server/src/services/aux-timer-service/__tests__/AuxTimerService.test.ts index e66417698..ac38396bb 100644 --- a/apps/server/src/services/aux-timer-service/__tests__/AuxTimerService.test.ts +++ b/apps/server/src/services/aux-timer-service/__tests__/AuxTimerService.test.ts @@ -28,6 +28,18 @@ describe('AuxTimerService', () => { expect(patch.auxtimer3?.name).toBe(''); }); + it('handles names missing from a project file', () => { + const emit = vi.fn(); + const service = new AuxTimerService(emit, () => 0); + + expect(() => service.loadNames(undefined)).not.toThrow(); + + const patch = emit.mock.calls.at(-1)?.[0] as Partial; + expect(patch.auxtimer1?.name).toBe(''); + expect(patch.auxtimer2?.name).toBe(''); + expect(patch.auxtimer3?.name).toBe(''); + }); + it('keeps the name on the timer through subsequent commands', () => { const emit = vi.fn(); const service = new AuxTimerService(emit, () => 0); diff --git a/apps/server/src/services/project-service/ProjectService.ts b/apps/server/src/services/project-service/ProjectService.ts index 3cad948c3..a2c01ae98 100644 --- a/apps/server/src/services/project-service/ProjectService.ts +++ b/apps/server/src/services/project-service/ProjectService.ts @@ -1,9 +1,10 @@ import { copyFile } from 'fs/promises'; import { join } from 'path'; -import { DatabaseModel, LogOrigin, ProjectFileListResponse } from 'ontime-types'; +import { DatabaseModel, LogOrigin, ProjectFileListResponse, RefetchKey } from 'ontime-types'; import { getErrorMessage, getFirstRundown } from 'ontime-utils'; +import { sendRefetch } from '../../adapters/WebsocketAdapter.js'; import { parseCustomFields } from '../../api-data/custom-fields/customFields.parser.js'; import { parseDatabaseModel } from '../../api-data/db/db.parser.js'; import { getCurrentRundown } from '../../api-data/rundown/rundown.dao.js'; @@ -28,6 +29,7 @@ import { removeFileExtension, } from '../../utils/fileManagement.js'; import { getLastLoaded, isLastLoadedProject, setLastLoaded } from '../app-state-service/AppStateService.js'; +import { auxTimerService } from '../aux-timer-service/AuxTimerService.js'; import { runtimeService } from '../runtime-service/runtime.service.js'; import { doesProjectExist, @@ -88,12 +90,16 @@ async function loadProject(projectData: DatabaseModel, fileName: string, rundown // stop the runtime service runtimeService.stop(); + // the aux timer names belong to the project, apply the ones from the newly loaded project + auxTimerService.loadNames(projectData.settings.auxTimerNames); + // load the rundown given by key otherwise load the first in the project const rundown = rundownId && rundownId in projectData.rundowns ? projectData.rundowns[rundownId] : getFirstRundown(projectData.rundowns); + // initialising the rundown with reload sends a refetch to the clients await initRundown(rundown, projectData.customFields, true); // persist the project selection @@ -346,6 +352,12 @@ export async function patchCurrentProject(data: Partial) { // we can pass some stuff straight to the data provider await getDataProvider().mergeIntoData(rest); + // the settings may contain new aux timer names, apply them and notify the clients + if (rest.settings) { + auxTimerService.loadNames(getDataProvider().getSettings().auxTimerNames); + sendRefetch(RefetchKey.Settings); + } + // the rundown depends on custom fields // so custom fields needs to be checked first if (customFields) { diff --git a/packages/types/src/definitions/core/Settings.type.ts b/packages/types/src/definitions/core/Settings.type.ts index 8ffbd6cdf..2c1758756 100644 --- a/packages/types/src/definitions/core/Settings.type.ts +++ b/packages/types/src/definitions/core/Settings.type.ts @@ -6,6 +6,9 @@ export type Settings = { operatorKey: null | string; timeFormat: TimeFormat; language: string; - /** Custom names for the aux timers, indexed by aux timer (1, 2, 3). Empty string falls back to the default label */ + /** + * Custom names for the aux timers, one entry per aux timer in order (index 0 is aux timer 1). + * An empty string means the timer is unnamed and consumers show the default label + */ auxTimerNames: string[]; }; diff --git a/packages/utils/index.ts b/packages/utils/index.ts index 6ba939175..7f6b201b3 100644 --- a/packages/utils/index.ts +++ b/packages/utils/index.ts @@ -99,6 +99,13 @@ export { export { isPlaybackActive } from './src/playback-utils/playbackstate.js'; +// aux timers +export { + auxTimerNameMaxLength, + normaliseAuxTimerNames, + numberOfAuxTimers, +} from './src/aux-timer-utils/auxTimerUtils.js'; + //Colour export { colourToHex, diff --git a/packages/utils/src/aux-timer-utils/auxTimerUtils.test.ts b/packages/utils/src/aux-timer-utils/auxTimerUtils.test.ts new file mode 100644 index 000000000..e2e3d3947 --- /dev/null +++ b/packages/utils/src/aux-timer-utils/auxTimerUtils.test.ts @@ -0,0 +1,31 @@ +import { auxTimerNameMaxLength, normaliseAuxTimerNames, numberOfAuxTimers } from './auxTimerUtils.js'; + +describe('normaliseAuxTimerNames()', () => { + it('generates the default value when given nothing', () => { + expect(normaliseAuxTimerNames()).toStrictEqual(['', '', '']); + }); + + it('always returns an entry per aux timer', () => { + expect(normaliseAuxTimerNames(['Speaker'])).toHaveLength(numberOfAuxTimers); + expect(normaliseAuxTimerNames(['a', 'b', 'c', 'extra'])).toStrictEqual(['a', 'b', 'c']); + }); + + it('pads missing entries with an empty string', () => { + expect(normaliseAuxTimerNames(['Speaker'])).toStrictEqual(['Speaker', '', '']); + }); + + it('trims whitespace', () => { + expect(normaliseAuxTimerNames([' Speaker ', '', ''])).toStrictEqual(['Speaker', '', '']); + }); + + it('caps the name length', () => { + const tooLong = 'a'.repeat(auxTimerNameMaxLength + 10); + expect(normaliseAuxTimerNames([tooLong])[0]).toHaveLength(auxTimerNameMaxLength); + }); + + it('falls back to defaults for malformed data', () => { + expect(normaliseAuxTimerNames('not-an-array')).toStrictEqual(['', '', '']); + expect(normaliseAuxTimerNames(null)).toStrictEqual(['', '', '']); + expect(normaliseAuxTimerNames([42, {}, undefined])).toStrictEqual(['', '', '']); + }); +}); diff --git a/packages/utils/src/aux-timer-utils/auxTimerUtils.ts b/packages/utils/src/aux-timer-utils/auxTimerUtils.ts new file mode 100644 index 000000000..19260634a --- /dev/null +++ b/packages/utils/src/aux-timer-utils/auxTimerUtils.ts @@ -0,0 +1,22 @@ +/** Number of aux timers available in ontime */ +export const numberOfAuxTimers = 3; + +/** Maximum length of a user given aux timer name */ +export const auxTimerNameMaxLength = 30; + +/** + * Normalises user or file provided aux timer names into a + * fixed length array of trimmed, length capped strings. + * Missing or malformed entries fallback to an empty string, + * which consumers render as the default label. + * Used when parsing project files, when validating API payloads + * and to generate the default value. + */ +export function normaliseAuxTimerNames(maybeNames?: unknown): string[] { + const source = Array.isArray(maybeNames) ? maybeNames : []; + + return Array.from({ length: numberOfAuxTimers }, (_, index) => { + const value = source[index]; + return typeof value === 'string' ? value.trim().slice(0, auxTimerNameMaxLength) : ''; + }); +} diff --git a/packages/utils/src/rundown-utils/rundownUtils.mock.ts b/packages/utils/src/rundown-utils/rundownUtils.mock.ts index cab85e3f7..84e9b54cb 100644 --- a/packages/utils/src/rundown-utils/rundownUtils.mock.ts +++ b/packages/utils/src/rundown-utils/rundownUtils.mock.ts @@ -1,6 +1,8 @@ import type { DatabaseModel } from 'ontime-types'; import { EndAction, OntimeView, SupportedEntry, TimeStrategy, TimerType } from 'ontime-types'; +import { normaliseAuxTimerNames } from '../aux-timer-utils/auxTimerUtils.js'; + export const demoDb: DatabaseModel = { rundowns: { default: { @@ -342,7 +344,7 @@ export const demoDb: DatabaseModel = { operatorKey: null, timeFormat: '24', language: 'en', - auxTimerNames: ['', '', ''], + auxTimerNames: normaliseAuxTimerNames(), }, viewSettings: { dangerColor: '#ff7300',