mirror of
https://github.com/cpvalente/ontime.git
synced 2026-07-26 10:38:55 +00:00
feat(rundown): batch edit event duration, model conflicts with a symbol
Two follow ups on multi event editing. Duration is now editable across a selection. The batch endpoint already applies every mutation to a cloned rundown and calls processRundown once on commit, so a batched duration cascades correctly through linked events with a single recalculation, and the server infers the duration lock for entries which were locked to their end. Start and end times remain excluded: they are absolute points in time, so giving several events the same value collapses every linked event to a zero duration. Only the duration is offered in a multi selection. The optimistic update is skipped when the patch carries a duration, since the resulting schedule cannot be resolved on the client. Conflicting values are now marked with a dedicated symbol rather than undefined, which was doing double duty for "the entries disagree" and "the field is absent". The distinction was already leaking into the custom field merge, which needed a key lookup to tell the two apart. The symbol also makes it a type error to assign a merged value into an entry patch, where it would previously have been dropped silently on serialisation. Components keep taking plain optional values: the conflict is resolved once at the editor boundary, so no indeterminate handling spreads into the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDETGBrhwgAqgTHjmrmJGJ
This commit is contained in:
@@ -483,7 +483,11 @@ function useEntryActionsForRundown(scopedRundownId: string | undefined) {
|
||||
// Snapshot the previous value
|
||||
const previousRundown = queryClient.getQueryData<Rundown>(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 };
|
||||
|
||||
@@ -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 (
|
||||
<div className={style.content}>
|
||||
{singleEvent && (
|
||||
{singleEvent ? (
|
||||
<EventEditorSchedule
|
||||
key={`${editorKey}-schedule`}
|
||||
eventId={singleEvent.id}
|
||||
@@ -78,14 +79,20 @@ export default function EventEditor({ events }: EventEditorProps) {
|
||||
linkStart={singleEvent.linkStart}
|
||||
delay={singleEvent.delay}
|
||||
/>
|
||||
) : (
|
||||
<EventEditorBatchSchedule
|
||||
key={`${editorKey}-schedule`}
|
||||
duration={resolveConflict(merged.duration)}
|
||||
submit={submit}
|
||||
/>
|
||||
)}
|
||||
<EventEditorTimes
|
||||
key={`${editorKey}-times`}
|
||||
countToEnd={merged.countToEnd}
|
||||
endAction={merged.endAction}
|
||||
timerType={merged.timerType}
|
||||
timeWarning={merged.timeWarning}
|
||||
timeDanger={merged.timeDanger}
|
||||
countToEnd={resolveConflict(merged.countToEnd)}
|
||||
endAction={resolveConflict(merged.endAction)}
|
||||
timerType={resolveConflict(merged.timerType)}
|
||||
timeWarning={resolveConflict(merged.timeWarning)}
|
||||
timeDanger={resolveConflict(merged.timeDanger)}
|
||||
submit={submit}
|
||||
/>
|
||||
<EventEditorTitles
|
||||
@@ -93,10 +100,10 @@ export default function EventEditor({ events }: EventEditorProps) {
|
||||
eventId={singleEvent?.id ?? null}
|
||||
eventCount={events.length}
|
||||
cue={singleEvent?.cue ?? ''}
|
||||
flag={merged.flag}
|
||||
title={merged.title}
|
||||
note={merged.note}
|
||||
colour={merged.colour}
|
||||
flag={resolveConflict(merged.flag)}
|
||||
title={resolveConflict(merged.title)}
|
||||
note={resolveConflict(merged.note)}
|
||||
colour={resolveConflict(merged.colour)}
|
||||
submit={submit}
|
||||
/>
|
||||
<div className={style.column}>
|
||||
|
||||
@@ -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>): OntimeEvent {
|
||||
return {
|
||||
@@ -10,6 +10,7 @@ function makeEvent(patch: Partial<OntimeEvent>): 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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<OntimeEvent>) => 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 (
|
||||
<div className={style.column}>
|
||||
<Editor.Title>Event schedule</Editor.Title>
|
||||
<div className={style.splitTwo}>
|
||||
<div>
|
||||
<Editor.Label htmlFor='duration'>Duration</Editor.Label>
|
||||
<TimeInput
|
||||
id='duration'
|
||||
name='duration'
|
||||
submitHandler={handleSubmit}
|
||||
time={duration}
|
||||
placeholder={duration === undefined ? mixedPlaceholder : 'Duration'}
|
||||
align='left'
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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> = T | Conflict;
|
||||
|
||||
export function isConflict<T>(value: MergedValue<T>): value is Conflict {
|
||||
return value === conflict;
|
||||
}
|
||||
|
||||
/** Resolves a merged value for the UI, where an unknown value is represented by undefined */
|
||||
export function resolveConflict<T>(value: MergedValue<T>): 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<CustomFieldKey, string | undefined>;
|
||||
/**
|
||||
* 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<CustomFieldKey, MergedValue<string>>;
|
||||
|
||||
/**
|
||||
* 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<OntimeEvent[K]>;
|
||||
} & {
|
||||
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<K extends BatchEditableField>(events: OntimeEvent[], field: K): OntimeEvent[K] | undefined {
|
||||
function mergeField<K extends BatchEditableField>(events: OntimeEvent[], field: K): MergedValue<OntimeEvent[K]> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user