diff --git a/apps/client/src/common/hooks/useEntryAction.ts b/apps/client/src/common/hooks/useEntryAction.ts index 22a222276..26274449e 100644 --- a/apps/client/src/common/hooks/useEntryAction.ts +++ b/apps/client/src/common/hooks/useEntryAction.ts @@ -483,7 +483,11 @@ function useEntryActionsForRundown(scopedRundownId: string | undefined) { // Snapshot the previous value const previousRundown = queryClient.getQueryData(queryKey); - if (previousRundown) { + // changing the duration cascades through the rundown, we cannot resolve the new schedule here + // we skip the optimistic update and wait for the recalculated rundown from the server + const canPredictResult = !('duration' in data.data); + + if (previousRundown && canPredictResult) { const { data: patch, ids } = data; const eventIds = new Set(ids); const newRundown = { ...previousRundown.entries }; diff --git a/apps/client/src/features/rundown/entry-editor/EventEditor.tsx b/apps/client/src/features/rundown/entry-editor/EventEditor.tsx index 024b28e6c..af2242d11 100644 --- a/apps/client/src/features/rundown/entry-editor/EventEditor.tsx +++ b/apps/client/src/features/rundown/entry-editor/EventEditor.tsx @@ -6,13 +6,14 @@ import Info from '../../../common/components/info/Info'; import AppLink from '../../../common/components/link/app-link/AppLink'; import { useEntryActionsContext } from '../../../common/context/EntryActionsContext'; import useCustomFields from '../../../common/hooks-query/useCustomFields'; +import EventEditorBatchSchedule from './composite/EventEditorBatchSchedule'; import EntryEditorCustomFields from './composite/EventEditorCustomFields'; import EventEditorSchedule from './composite/EventEditorSchedule'; import EventEditorTimes from './composite/EventEditorTimes'; import EventEditorTitles from './composite/EventEditorTitles'; import EventEditorTriggers from './composite/EventEditorTriggers'; import { mixedPlaceholder } from './entryEditor.utils'; -import { mergeEvents } from './mergeEvents'; +import { mergeEvents, resolveConflict } from './mergeEvents'; import style from './EntryEditor.module.scss'; @@ -67,7 +68,7 @@ export default function EventEditor({ events }: EventEditorProps) { return (
- {singleEvent && ( + {singleEvent ? ( + ) : ( + )}
diff --git a/apps/client/src/features/rundown/entry-editor/__tests__/mergeEvents.test.ts b/apps/client/src/features/rundown/entry-editor/__tests__/mergeEvents.test.ts index eaf74132d..c0807fca8 100644 --- a/apps/client/src/features/rundown/entry-editor/__tests__/mergeEvents.test.ts +++ b/apps/client/src/features/rundown/entry-editor/__tests__/mergeEvents.test.ts @@ -1,6 +1,6 @@ import { EndAction, OntimeEvent, SupportedEntry, TimerType } from 'ontime-types'; -import { mergeEvents } from '../mergeEvents'; +import { conflict, isConflict, mergeEvents, resolveConflict } from '../mergeEvents'; function makeEvent(patch: Partial): OntimeEvent { return { @@ -10,6 +10,7 @@ function makeEvent(patch: Partial): OntimeEvent { note: 'note', colour: '#FFCC78', flag: false, + duration: 600000, endAction: EndAction.None, countToEnd: false, timerType: TimerType.CountDown, @@ -30,6 +31,7 @@ describe('mergeEvents()', () => { note: 'note', colour: '#FFCC78', flag: false, + duration: 600000, endAction: EndAction.None, countToEnd: false, timerType: TimerType.CountDown, @@ -46,12 +48,13 @@ describe('mergeEvents()', () => { expect(merged.title).toBe('title'); expect(merged.colour).toBe('#FFCC78'); expect(merged.timerType).toBe(TimerType.CountDown); + expect(merged.duration).toBe(600000); }); - it('marks only the fields which differ as undefined', () => { + it('marks only the fields which differ as conflicting', () => { const merged = mergeEvents([makeEvent({ id: '1', title: 'first' }), makeEvent({ id: '2', title: 'second' })]); - expect(merged.title).toBeUndefined(); + expect(merged.title).toBe(conflict); expect(merged.note).toBe('note'); expect(merged.colour).toBe('#FFCC78'); expect(merged.flag).toBe(false); @@ -59,12 +62,13 @@ describe('mergeEvents()', () => { it('handles boolean and numeric fields', () => { const merged = mergeEvents([ - makeEvent({ id: '1', flag: true, timeWarning: 1000 }), - makeEvent({ id: '2', flag: false, timeWarning: 1000 }), + makeEvent({ id: '1', flag: true, timeWarning: 1000, duration: 1000 }), + makeEvent({ id: '2', flag: false, timeWarning: 1000, duration: 2000 }), ]); - expect(merged.flag).toBeUndefined(); + expect(merged.flag).toBe(conflict); expect(merged.timeWarning).toBe(1000); + expect(merged.duration).toBe(conflict); }); describe('custom fields', () => { @@ -74,13 +78,13 @@ describe('mergeEvents()', () => { makeEvent({ id: '2', custom: { lx: 'same', sound: 'b' } }), ]); - expect(merged.custom).toStrictEqual({ lx: 'same', sound: undefined }); + expect(merged.custom).toStrictEqual({ lx: 'same', sound: conflict }); }); it('treats a missing key as empty', () => { const merged = mergeEvents([makeEvent({ id: '1', custom: { lx: 'value' } }), makeEvent({ id: '2', custom: {} })]); - expect(merged.custom).toStrictEqual({ lx: undefined }); + expect(merged.custom).toStrictEqual({ lx: conflict }); }); it('collects keys from all events', () => { @@ -90,7 +94,29 @@ describe('mergeEvents()', () => { ]); // lx is empty in both events, sound is only filled in one of them - expect(merged.custom).toStrictEqual({ lx: '', sound: undefined }); + expect(merged.custom).toStrictEqual({ lx: '', sound: conflict }); + }); + + it('does not report a key which is absent from every event', () => { + const merged = mergeEvents([makeEvent({ id: '1', custom: {} }), makeEvent({ id: '2', custom: {} })]); + + expect(merged.custom).toStrictEqual({}); + // an absent key is distinguishable from a conflicting one + expect(merged.custom.lx).toBeUndefined(); + expect(isConflict(merged.custom.lx)).toBe(false); }); }); }); + +describe('resolveConflict()', () => { + it('passes known values through', () => { + expect(resolveConflict('value')).toBe('value'); + expect(resolveConflict(0)).toBe(0); + expect(resolveConflict(false)).toBe(false); + expect(resolveConflict('')).toBe(''); + }); + + it('resolves a conflict to undefined', () => { + expect(resolveConflict(conflict)).toBeUndefined(); + }); +}); diff --git a/apps/client/src/features/rundown/entry-editor/composite/EventEditorBatchSchedule.tsx b/apps/client/src/features/rundown/entry-editor/composite/EventEditorBatchSchedule.tsx new file mode 100644 index 000000000..90007a8f4 --- /dev/null +++ b/apps/client/src/features/rundown/entry-editor/composite/EventEditorBatchSchedule.tsx @@ -0,0 +1,48 @@ +import { OntimeEvent } from 'ontime-types'; +import { MILLIS_PER_SECOND, dayInMs, parseUserTime } from 'ontime-utils'; +import { memo } from 'react'; + +import * as Editor from '../../../../common/components/editor-utils/EditorUtils'; +import TimeInput from '../../../../common/components/input/time-input/TimeInput'; +import { mixedPlaceholder } from '../entryEditor.utils'; + +import style from '../EntryEditor.module.scss'; + +interface EventEditorBatchScheduleProps { + /** undefined when the events do not share a duration */ + duration: number | undefined; + submit: (patch: Partial) => void; +} + +/** + * Schedule fields which can be applied to several events at once + * Start and end times are absolute points in time: giving several events the same + * value would collapse their durations, so only the duration is offered here + * The server infers the duration lock and recalculates the rundown once for the whole batch + */ +export default memo(EventEditorBatchSchedule); +function EventEditorBatchSchedule({ duration, submit }: EventEditorBatchScheduleProps) { + const handleSubmit = (_field: 'duration', value: string) => { + // durations cannot exceed a day + submit({ duration: Math.min(parseUserTime(value), dayInMs - MILLIS_PER_SECOND) }); + }; + + return ( +
+ Event schedule +
+
+ Duration + +
+
+
+ ); +} diff --git a/apps/client/src/features/rundown/entry-editor/composite/EventEditorCustomFields.tsx b/apps/client/src/features/rundown/entry-editor/composite/EventEditorCustomFields.tsx index 90efc2abc..48d21fbde 100644 --- a/apps/client/src/features/rundown/entry-editor/composite/EventEditorCustomFields.tsx +++ b/apps/client/src/features/rundown/entry-editor/composite/EventEditorCustomFields.tsx @@ -3,7 +3,7 @@ import { CSSProperties, Fragment } from 'react'; import { getAccessibleColour } from '../../../../common/utils/styleUtils'; import { EventEditorUpdateFields } from '../EventEditor'; -import { MergedCustomFields } from '../mergeEvents'; +import { MergedCustomFields, resolveConflict } from '../mergeEvents'; import EventEditorImage from './EventEditorImage'; import EventTextArea from './EventTextArea'; import EntryEditorTextInput from './EventTextInput'; @@ -32,8 +32,8 @@ export default function EntryEditorCustomFields({ {Object.keys(customFields).map((fieldKey) => { const key = `${idKey}-${fieldKey}`; const fieldName = `custom-${fieldKey}`; - const value = custom[fieldKey]; - const initialValue = fieldKey in custom ? value : ''; + // a key which is absent from the merged view is not present in any of the entries + const initialValue = resolveConflict(custom[fieldKey] ?? ''); const placeholder = initialValue === undefined ? mixedPlaceholder : undefined; const { backgroundColor, color } = getAccessibleColour(customFields[fieldKey].colour); const labelText = customFields[fieldKey].label; diff --git a/apps/client/src/features/rundown/entry-editor/mergeEvents.ts b/apps/client/src/features/rundown/entry-editor/mergeEvents.ts index 9a74bbae3..18d8fbcf4 100644 --- a/apps/client/src/features/rundown/entry-editor/mergeEvents.ts +++ b/apps/client/src/features/rundown/entry-editor/mergeEvents.ts @@ -1,15 +1,36 @@ import { CustomFieldKey, OntimeEvent } from 'ontime-types'; +/** + * Marks a value which is not the same across the entries being edited + * This is a distinct value from undefined, which means that the field is absent + * Being a symbol, it also cannot be assigned to a patch by accident + */ +export const conflict: unique symbol = Symbol('conflict'); +export type Conflict = typeof conflict; + +/** A value which may not be the same across the entries being edited */ +export type MergedValue = T | Conflict; + +export function isConflict(value: MergedValue): value is Conflict { + return value === conflict; +} + +/** Resolves a merged value for the UI, where an unknown value is represented by undefined */ +export function resolveConflict(value: MergedValue): T | undefined { + return isConflict(value) ? undefined : value; +} + /** * Fields which can be edited across a selection of events - * Schedule fields, cue and triggers are deliberately excluded: - * they are either unique to an event or would cascade through the rundown + * timeStart and timeEnd are absolute points in time and are excluded: + * giving several events the same start or end collapses their durations */ export const batchEditableFields = [ 'title', 'note', 'colour', 'flag', + 'duration', 'endAction', 'countToEnd', 'timerType', @@ -20,21 +41,25 @@ export const batchEditableFields = [ type BatchEditableField = (typeof batchEditableFields)[number]; -export type MergedCustomFields = Record; +/** + * Custom fields of the entries being edited + * A missing key means that none of the entries have a value for that field + */ +export type MergedCustomFields = Record>; /** * A merged view over a set of events - * A field is undefined when the events do not agree on its value + * A field holds the conflict symbol when the events do not agree on its value */ export type MergedEvent = { - [K in BatchEditableField]: OntimeEvent[K] | undefined; + [K in BatchEditableField]: MergedValue; } & { custom: MergedCustomFields; }; /** * Merges a list of events into a single view - * For a single event, every field is defined and matches the event + * For a single event, no field is in conflict and every value matches the event * @param events - events to merge, must contain at least one element */ export function mergeEvents(events: OntimeEvent[]): MergedEvent { @@ -43,6 +68,7 @@ export function mergeEvents(events: OntimeEvent[]): MergedEvent { note: mergeField(events, 'note'), colour: mergeField(events, 'colour'), flag: mergeField(events, 'flag'), + duration: mergeField(events, 'duration'), endAction: mergeField(events, 'endAction'), countToEnd: mergeField(events, 'countToEnd'), timerType: mergeField(events, 'timerType'), @@ -54,11 +80,11 @@ export function mergeEvents(events: OntimeEvent[]): MergedEvent { } /** - * Returns the shared value of a field, or undefined if the events disagree + * Returns the shared value of a field, or the conflict symbol if the events disagree */ -function mergeField(events: OntimeEvent[], field: K): OntimeEvent[K] | undefined { +function mergeField(events: OntimeEvent[], field: K): MergedValue { const value = events[0][field]; - return events.some((event) => event[field] !== value) ? undefined : value; + return events.some((event) => event[field] !== value) ? conflict : value; } /** @@ -74,7 +100,7 @@ function mergeCustomFields(events: OntimeEvent[]): MergedCustomFields { continue; } const value = event.custom[key] ?? ''; - merged[key] = events.some((other) => (other.custom[key] ?? '') !== value) ? undefined : value; + merged[key] = events.some((other) => (other.custom[key] ?? '') !== value) ? conflict : value; } } diff --git a/e2e/tests/features/215-multi-edit.spec.ts b/e2e/tests/features/215-multi-edit.spec.ts index c8c0c80e3..dc1559cdf 100644 --- a/e2e/tests/features/215-multi-edit.spec.ts +++ b/e2e/tests/features/215-multi-edit.spec.ts @@ -21,9 +21,9 @@ test('Editing multiple events', async ({ page }) => { const editor = page.getByTestId('editor-container'); - // a single selection shows the schedule and the event id + // a single selection shows the full schedule and the event id await page.getByTestId('entry-1').getByTestId('rundown-event').click(); - await expect(editor.getByText('Event schedule')).toBeVisible(); + await expect(editor.getByTestId('time-input-timeStart')).toBeVisible(); await expect(editor.getByLabel('Title', { exact: true })).toHaveValue('first'); // selecting both events shows a merged view @@ -32,9 +32,13 @@ test('Editing multiple events', async ({ page }) => { .getByTestId('rundown-event') .click({ modifiers: ['Shift'] }); await expect(editor.locator('#eventId')).toHaveValue('2 events selected'); - await expect(editor.getByText('Event schedule')).toBeHidden(); await expect(editor.getByText('Automations are not available when editing multiple events')).toBeVisible(); + // start and end times are unique to an event, only the duration can be batched + await expect(editor.getByTestId('time-input-timeStart')).toBeHidden(); + await expect(editor.getByTestId('time-input-timeEnd')).toBeHidden(); + await expect(editor.getByTestId('time-input-duration')).toBeVisible(); + // fields which do not agree are shown as mixed const title = editor.getByLabel('Title', { exact: true }); await expect(title).toHaveValue(''); @@ -55,8 +59,19 @@ test('Editing multiple events', async ({ page }) => { // the value is no longer mixed await expect(editor.getByLabel('Title', { exact: true })).toHaveValue('shared title'); + // a batched duration is applied to every event and the rundown is recalculated once + const duration = editor.getByTestId('time-input-duration'); + await duration.click(); + await duration.fill('5m'); + await duration.press('Enter'); + await expect(page.getByTestId('entry-1').getByTestId('time-input-duration')).toHaveValue('00:05:00'); + await expect(page.getByTestId('entry-2').getByTestId('time-input-duration')).toHaveValue('00:05:00'); + // the second event is linked, so its start follows the new end of the first + await expect(page.getByTestId('entry-2').getByTestId('time-input-timeStart')).toHaveValue('00:05:00'); + await expect(page.getByTestId('entry-2').getByTestId('time-input-timeEnd')).toHaveValue('00:10:00'); + // going back to a single selection restores the full editor await page.getByTestId('entry-1').getByTestId('rundown-event').click(); - await expect(editor.getByText('Event schedule')).toBeVisible(); + await expect(editor.getByTestId('time-input-timeStart')).toBeVisible(); await expect(editor.locator('#eventId')).not.toHaveValue('2 events selected'); });