From e963e183dbf9017a698ef965c90380334e6aa25f Mon Sep 17 00:00:00 2001 From: Carlos Valente Date: Mon, 30 Jun 2025 07:16:08 +0200 Subject: [PATCH] fix: entry actions in cuesheet fix: insert entry before fix: avoid double submit on enter --- .../input/text-input/useReactiveTextInput.tsx | 28 +++++++++-- .../rundown/__tests__/rundown.dao.test.ts | 4 +- .../rundown/__tests__/rundown.utils.test.ts | 46 ++++++++++++++----- .../src/api-data/rundown/rundown.dao.ts | 16 +++---- .../src/api-data/rundown/rundown.service.ts | 10 ++-- .../src/api-data/rundown/rundown.utils.ts | 27 +++++++---- 6 files changed, 92 insertions(+), 39 deletions(-) diff --git a/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx b/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx index 91dd879a4..305458b50 100644 --- a/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx +++ b/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx @@ -1,4 +1,4 @@ -import { ChangeEvent, KeyboardEvent, RefObject, useCallback, useEffect, useMemo, useState } from 'react'; +import { ChangeEvent, KeyboardEvent, RefObject, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { getHotkeyHandler, HotkeyItem } from '@mantine/hooks'; interface UseReactiveTextInputReturn { @@ -21,6 +21,8 @@ export default function useReactiveTextInput( }, ): UseReactiveTextInputReturn { const [text, setText] = useState(initialText); + // track whether we are submitting via a submit key (eg enter) and avoid submitting again on blur + const isKeyboardSubmitting = useRef(false); useEffect(() => { if (typeof initialText === 'undefined') { @@ -99,11 +101,25 @@ export default function useReactiveTextInput( ]; if (options?.submitOnEnter) { - hotKeys.push(['Enter', () => handleSubmit(text)]); + hotKeys.push(['Enter', () => { + isKeyboardSubmitting.current = true; + handleSubmit(text); + // clear flag after blur has been processed + setTimeout(() => { + isKeyboardSubmitting.current = false; + }, 0); + }]); } if (options?.submitOnCtrlEnter) { - hotKeys.push(['mod + Enter', () => handleSubmit(text)]); + hotKeys.push(['mod + Enter', () => { + isKeyboardSubmitting.current = true; + handleSubmit(text); + // clear flag after blur has been processed + setTimeout(() => { + isKeyboardSubmitting.current = false; + }, 0); + }]); } const hotKeyHandler = getHotkeyHandler(hotKeys); @@ -126,7 +142,11 @@ export default function useReactiveTextInput( return { value: text, onChange: (event: ChangeEvent) => handleChange((event.target as HTMLInputElement).value), - onBlur: (event: ChangeEvent) => handleSubmit((event.target as HTMLInputElement).value), + onBlur: (event: ChangeEvent) => { + if (!isKeyboardSubmitting.current) { + handleSubmit((event.target as HTMLInputElement).value); + } + }, onKeyDown: keyHandler, }; } diff --git a/apps/server/src/api-data/rundown/__tests__/rundown.dao.test.ts b/apps/server/src/api-data/rundown/__tests__/rundown.dao.test.ts index d800de298..5e4e3e87a 100644 --- a/apps/server/src/api-data/rundown/__tests__/rundown.dao.test.ts +++ b/apps/server/src/api-data/rundown/__tests__/rundown.dao.test.ts @@ -699,7 +699,7 @@ describe('rundownMutation.add()', () => { }, }); - rundownMutation.add(rundown, mockEvent, null, '1'); + rundownMutation.add(rundown, mockEvent, null, rundown.entries['1'] as OntimeBlock); expect(rundown.order).toStrictEqual(['1']); expect(rundown.flatOrder).toStrictEqual(['1', 'mock', '1a']); @@ -717,7 +717,7 @@ describe('rundownMutation.add()', () => { }, }); - rundownMutation.add(rundown, mockEvent, '1a', '1'); + rundownMutation.add(rundown, mockEvent, '1a', rundown.entries['1'] as OntimeBlock); expect(rundown.order).toStrictEqual(['1']); expect(rundown.flatOrder).toStrictEqual(['1', '1a', 'mock']); diff --git a/apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts b/apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts index c0b956f70..43adb62fd 100644 --- a/apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts +++ b/apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts @@ -1,10 +1,17 @@ -import { TimeStrategy, EndAction, TimerType, OntimeEvent } from 'ontime-types'; +import { TimeStrategy, EndAction, TimerType, OntimeEvent, OntimeBlock } from 'ontime-types'; import { MILLIS_PER_HOUR } from 'ontime-utils'; import { assertType } from 'vitest'; -import { calculateDayOffset, createEvent, deleteById, doesInvalidateMetadata, getInsertAfterId, hasChanges } from '../rundown.utils.js'; -import { makeRundown } from '../__mocks__/rundown.mocks.js'; +import { + calculateDayOffset, + createEvent, + deleteById, + doesInvalidateMetadata, + getInsertAfterId, + hasChanges, +} from '../rundown.utils.js'; +import { makeOntimeBlock, makeOntimeEvent, makeRundown } from '../__mocks__/rundown.mocks.js'; describe('test event validator', () => { it('validates a good object', () => { @@ -217,22 +224,39 @@ describe('calculateDayOffset()', () => { describe('getInsertAfterId()', () => { const rundown = makeRundown({ - flatOrder: ['a', 'b', 'c', 'd'], + entries: { + '1': makeOntimeEvent({ id: '1', parent: null }), + '2': makeOntimeEvent({ id: '2', parent: null }), + block: makeOntimeBlock({ id: 'block', entries: ['31', '32'] }), + '31': makeOntimeEvent({ id: '31', parent: 'block' }), + '32': makeOntimeEvent({ id: '32', parent: 'block' }), + '4': makeOntimeEvent({ id: '31', parent: null }), + }, + order: ['1', '2', 'block', '4'], + flatOrder: ['1', '2', 'block', '31', '32', '4'], }); it('returns afterId if provided', () => { - expect(getInsertAfterId(rundown, 'b')).toBe('b'); + expect(getInsertAfterId(rundown, null, 'b')).toBe('b'); }); - it('returns the previous id before beforeId if provided', () => { - expect(getInsertAfterId(rundown, undefined, 'c')).toBe('b'); + it('returns null if neither afterId nor beforeId is provided', () => { + expect(getInsertAfterId(rundown, null)).toBeNull(); }); - it('returns undefined if neither afterId nor beforeId is provided', () => { - expect(getInsertAfterId(rundown)).toBeNull(); + it('returns null if beforeId is not found', () => { + expect(getInsertAfterId(rundown, null, undefined, 'z')).toBeNull(); + expect(getInsertAfterId(rundown, null, undefined, '1')).toBeNull(); }); - it('returns undefined if beforeId is not found', () => { - expect(getInsertAfterId(rundown, undefined, 'z')).toBeNull(); + it('returns the previous id of an entry in the rundown', () => { + expect(getInsertAfterId(rundown, null, undefined, '2')).toBe('1'); + expect(getInsertAfterId(rundown, null, undefined, '4')).toBe('block'); + expect(getInsertAfterId(rundown, null, undefined, 'block')).toBe('2'); + }); + + it('returns the previous id of an event in a block', () => { + expect(getInsertAfterId(rundown, rundown.entries.block as OntimeBlock, undefined, '31')).toBeNull(); + expect(getInsertAfterId(rundown, rundown.entries.block as OntimeBlock, undefined, '32')).toBe('31'); }); }); diff --git a/apps/server/src/api-data/rundown/rundown.dao.ts b/apps/server/src/api-data/rundown/rundown.dao.ts index 91032718a..b51aed641 100644 --- a/apps/server/src/api-data/rundown/rundown.dao.ts +++ b/apps/server/src/api-data/rundown/rundown.dao.ts @@ -189,18 +189,17 @@ export function createTransaction(options: TransactionOptions): Transaction { * - 2a. add entry to the rundown, after a given entry * - 2b. add entry to the rundown, at the beginning */ -function add(rundown: Rundown, entry: OntimeEntry, afterId: EntryId | null, parentId: EntryId | null): OntimeEntry { - if (parentId) { +function add(rundown: Rundown, entry: OntimeEntry, afterId: EntryId | null, parent: OntimeBlock | null): OntimeEntry { + if (parent) { // 1. inserting an entry inside a block - const parentBlock = rundown.entries[parentId] as OntimeBlock; if (afterId) { - const atEventsIndex = parentBlock.entries.indexOf(afterId) + 1; + const atEventsIndex = parent.entries.indexOf(afterId) + 1; const atFlatIndex = rundown.flatOrder.indexOf(afterId) + 1; - parentBlock.entries = insertAtIndex(atEventsIndex, entry.id, parentBlock.entries); + parent.entries = insertAtIndex(atEventsIndex, entry.id, parent.entries); rundown.flatOrder = insertAtIndex(atFlatIndex, entry.id, rundown.flatOrder); } else { - parentBlock.entries = insertAtIndex(0, entry.id, parentBlock.entries); - const atFlatIndex = rundown.flatOrder.indexOf(parentId) + 1; + parent.entries = insertAtIndex(0, entry.id, parent.entries); + const atFlatIndex = rundown.flatOrder.indexOf(parent.id) + 1; rundown.flatOrder = insertAtIndex(atFlatIndex, entry.id, rundown.flatOrder); } } else { @@ -469,7 +468,8 @@ function clone(rundown: Rundown, entry: OntimeEntry): OntimeEntry { return newBlock; } else { - return add(rundown, cloneEntry(entry, getUniqueId(rundown)), entry.id, entry.parent); + const parent: OntimeBlock | null = entry.parent ? (rundown.entries[entry.parent] as OntimeBlock) : null; + return add(rundown, cloneEntry(entry, getUniqueId(rundown)), entry.id, parent); } } diff --git a/apps/server/src/api-data/rundown/rundown.service.ts b/apps/server/src/api-data/rundown/rundown.service.ts index 4a8e986a4..13d2a3f58 100644 --- a/apps/server/src/api-data/rundown/rundown.service.ts +++ b/apps/server/src/api-data/rundown/rundown.service.ts @@ -7,6 +7,7 @@ import { isOntimeBlock, isOntimeDelay, isOntimeEvent, + OntimeBlock, OntimeEntry, OntimeEvent, PatchWithId, @@ -35,23 +36,24 @@ export async function addEntry(eventData: EventPostPayload): Promise id === beforeId); - if (atIndex < 1) return null; - return rundown.flatOrder[atIndex - 1]; - } + /** + * At this point we know we want to insert before a given ID + * We need to check which list we should use to insert and find the event there + */ + const insertionList = parent ? parent.entries : rundown.order; + if (!insertionList || insertionList.length === 0) return null; - return null; + const atIndex = insertionList.findIndex((id) => id === beforeId); + if (atIndex < 1) return null; + return insertionList[atIndex - 1]; } /**