refactor(rundown): new entries are appended to rundown

apps/server/src/api-data/rundown/__tests__/rundown.dao.test.ts

@@ -714,17 +714,17 @@ describe('processRundown()', () => {
 });
 
 describe('rundownMutation.add()', () => {
-  test('adds an event an empty rundown', () => {
+  test('adds an event to an empty rundown', () => {
     const mockEvent = makeOntimeEvent({ id: 'mock', cue: 'mock' });
     const rundown = makeRundown({});
 
-    rundownMutation.add(rundown, mockEvent, null, null);
+    rundownMutation.add(rundown, mockEvent, null, null, null);
 
     expect(rundown.order.length).toBe(1);
     expect(rundown.entries['mock']).toMatchObject(mockEvent);
   });
 
-  test('adds an event at the top if no afterId is given', () => {
+  test('adds an event at the end if no afterId is given', () => {
     const mockEvent = makeOntimeEvent({ id: 'mock', cue: 'mock' });
     const rundown = makeRundown({
       flatOrder: ['1'],
@@ -734,28 +734,28 @@ describe('rundownMutation.add()', () => {
       },
     });
 
-    rundownMutation.add(rundown, mockEvent, null, null);
+    rundownMutation.add(rundown, mockEvent, null, null, null);
 
-    expect(rundown.order).toStrictEqual(['mock', '1']);
-    expect(rundown.flatOrder).toStrictEqual(['mock', '1']);
+    expect(rundown.order).toStrictEqual(['1', 'mock']);
+    expect(rundown.flatOrder).toStrictEqual(['1', 'mock']);
     expect(rundown.entries['mock']).toMatchObject(mockEvent);
   });
 
-  test('adds an event at the top of the group if no after is given', () => {
+  test('adds an event at the end of the group if parent is provided but no after', () => {
     const mockEvent = makeOntimeEvent({ id: 'mock', cue: 'mock' });
     const rundown = makeRundown({
       flatOrder: ['1', '1a'],
       order: ['1'],
       entries: {
-        '1': makeOntimeGroup({ id: '1' }),
+        '1': makeOntimeGroup({ id: '1', entries: ['1a'] }),
         '1a': makeOntimeEvent({ id: '1a', parent: '1' }),
       },
     });
 
-    rundownMutation.add(rundown, mockEvent, null, rundown.entries['1'] as OntimeGroup);
+    rundownMutation.add(rundown, mockEvent, rundown.entries['1'] as OntimeGroup, null, null);
 
     expect(rundown.order).toStrictEqual(['1']);
-    expect(rundown.flatOrder).toStrictEqual(['1', 'mock', '1a']);
+    expect(rundown.flatOrder).toStrictEqual(['1', '1a', 'mock']);
     expect(rundown.entries['mock']).toMatchObject(mockEvent);
   });
 
@@ -765,12 +765,12 @@ describe('rundownMutation.add()', () => {
       flatOrder: ['1', '1a'],
       order: ['1'],
       entries: {
-        '1': makeOntimeGroup({ id: '1' }),
+        '1': makeOntimeGroup({ id: '1', entries: ['1a'] }),
         '1a': makeOntimeEvent({ id: '1a', parent: '1' }),
       },
     });
 
-    rundownMutation.add(rundown, mockEvent, '1a', rundown.entries['1'] as OntimeGroup);
+    rundownMutation.add(rundown, mockEvent, rundown.entries['1'] as OntimeGroup, '1a', null);
 
     expect(rundown.order).toStrictEqual(['1']);
     expect(rundown.flatOrder).toStrictEqual(['1', '1a', 'mock']);
@@ -1839,6 +1839,49 @@ describe('rundownMutation.clone()', () => {
       parent: null,
     });
   });
+
+  it('clones an event and appends it with after true', () => {
+    const testRundown = makeRundown({
+      order: ['1', '2'],
+      entries: {
+        '1': makeOntimeEvent({ id: '1', cue: 'event1', parent: null }),
+        '2': makeOntimeEvent({ id: '2', cue: 'event2', parent: null }),
+      },
+    });
+
+    const newEntry = rundownMutation.clone(testRundown, testRundown.entries['1'], { after: true });
+
+    expect(testRundown.order).toStrictEqual(['1', '2', newEntry.id]);
+  });
+
+  it('clones an event and prepends it with before true', () => {
+    const testRundown = makeRundown({
+      order: ['1', '2'],
+      entries: {
+        '1': makeOntimeEvent({ id: '1', cue: 'event1', parent: null }),
+        '2': makeOntimeEvent({ id: '2', cue: 'event2', parent: null }),
+      },
+    });
+
+    const newEntry = rundownMutation.clone(testRundown, testRundown.entries['2'], { before: true });
+
+    expect(testRundown.order).toStrictEqual([newEntry.id, '1', '2']);
+  });
+
+  it('clones a group and appends it with after true', () => {
+    const testRundown = makeRundown({
+      order: ['1', '2'],
+      entries: {
+        '1': makeOntimeGroup({ id: '1', title: 'top', entries: ['1a'] }),
+        '1a': makeOntimeEvent({ id: '1a', cue: 'nested', parent: '1' }),
+        '2': makeOntimeEvent({ id: '2', cue: 'event2', parent: null }),
+      },
+    });
+
+    const newEntry = rundownMutation.clone(testRundown, testRundown.entries['1'], { after: true });
+
+    expect(testRundown.order).toStrictEqual(['1', '2', newEntry.id]);
+  });
 });
 
 describe('rundownMutation.group()', () => {

apps/server/src/api-data/rundown/__tests__/rundown.validation.test.ts

@@ -0,0 +1,40 @@
+import type { OntimeGroup } from 'ontime-types';
+import { describe, expect, test } from 'vitest';
+
+import { makeOntimeEvent, makeOntimeGroup, makeRundown } from '../__mocks__/rundown.mocks.js';
+import { assertInsertAnchorExists, assertInsertAnchorInOrder } from '../rundown.validation.js';
+
+const rundown = makeRundown({
+  order: ['top', 'group'],
+  entries: {
+    top: makeOntimeEvent({ id: 'top', parent: null }),
+    group: makeOntimeGroup({ id: 'group', entries: ['nested'] }),
+    nested: makeOntimeEvent({ id: 'nested', parent: 'group' }),
+  },
+});
+
+describe('insertion anchor validation', () => {
+  test('rejects a missing anchor', () => {
+    expect(() => assertInsertAnchorInOrder(rundown, null, { before: 'missing' })).toThrow(
+      'Insertion anchor with ID missing does not exist',
+    );
+  });
+
+  test('rejects an anchor from a different order', () => {
+    expect(() => assertInsertAnchorInOrder(rundown, null, { after: 'nested' })).toThrow(
+      'Insertion anchor with ID nested is not in the target order',
+    );
+  });
+
+  test('accepts a group-local anchor', () => {
+    const group = rundown.entries.group as OntimeGroup;
+
+    expect(() => assertInsertAnchorInOrder(rundown, group, { before: 'nested' })).not.toThrow();
+  });
+
+  test('checks clone anchors exist', () => {
+    expect(() => assertInsertAnchorExists(rundown, { after: 'missing' })).toThrow(
+      'Insertion anchor with ID missing does not exist',
+    );
+  });
+});

apps/server/src/api-data/rundown/rundown.dao.ts

@@ -439,21 +439,27 @@ function clone(rundown: Rundown, entry: OntimeEntry, options?: InsertOptions): O
       rundown.entries[nestedEntry.id] = nestedEntry;
     }
 
-    // by default we insert after the cloned element
-    let atIndex = rundown.order.indexOf(entry.id) + 1;
-
-    const referenceId = options?.after ?? options?.before;
-    if (referenceId) {
-      // trying to insert relatively to another entry
-      const referenceEntry = rundown.entries[referenceId];
-      if (referenceEntry) {
-        if (options?.after) {
-          atIndex = rundown.order.indexOf(referenceId) + 1;
-        } else if (options?.before) {
-          atIndex = rundown.order.indexOf(referenceId);
-        }
+    const position: { after: EntryId | null; before: EntryId | null } = (() => {
+      if (options?.after === undefined && options?.before === undefined) {
+        return { after: entry.id, before: null };
       }
-    }
+
+      const after = getInsertAfterId(rundown, null, options.after, options.before);
+      const before = options.before === true ? (after ?? null) : (options.before ?? null);
+      return { after, before };
+    })();
+
+    const atIndex = (() => {
+      if (position.before !== null) {
+        return rundown.order.indexOf(position.before);
+      }
+
+      if (position.after !== null) {
+        return rundown.order.indexOf(position.after) + 1;
+      }
+
+      return rundown.order.length;
+    })();
 
     // we only need to insert the group, the nested entries will be resolved by the rundown engine
     rundown.order = insertAtIndex(atIndex, newGroup.id, rundown.order);
@@ -461,41 +467,33 @@ function clone(rundown: Rundown, entry: OntimeEntry, options?: InsertOptions): O
     return newGroup;
   } else {
     const clonedEntry = cloneSimpleRundownEntry(entry, getUniqueId(rundown));
+    const parent = (() => {
+      const referenceId = (() => {
+        if (typeof options?.after === 'string') return options.after;
+        if (typeof options?.before === 'string') return options.before;
+        return undefined;
+      })();
 
-    let parent: OntimeGroup | null = null;
+      const referenceEntry = referenceId ? rundown.entries[referenceId] : entry;
+      const parentId = referenceEntry && !isOntimeGroup(referenceEntry) ? referenceEntry.parent : null;
+      if (parentId === null) return null;
 
-    // trying to insert relatively to another entry, check that entries parent
-    const referenceId = options?.after ?? options?.before;
+      const maybeParent = rundown.entries[parentId];
+      return isOntimeGroup(maybeParent) ? maybeParent : null;
+    })();
 
-    /**
-     * if we have a positioning reference, and that reference has a parent
-     * we need to maintain the same parent for the cloned entry
-     */
-    if (referenceId) {
-      const referenceEntry = rundown.entries[referenceId];
-
-      if (referenceEntry && !isOntimeGroup(referenceEntry)) {
-        if (referenceEntry.parent) {
-          const maybeParent = rundown.entries[referenceEntry.parent];
-          if (maybeParent && isOntimeGroup(maybeParent)) {
-            parent = maybeParent;
-          }
-        }
+    const position = (() => {
+      // if no position is given, we add immediately after the element
+      if (options?.after === undefined && options?.before === undefined) {
+        return { after: entry.id, before: null };
       }
-    } else if (entry.parent) {
-      const maybeParent = rundown.entries[entry.parent];
-      if (maybeParent && isOntimeGroup(maybeParent)) {
-        parent = maybeParent;
-      }
-    }
 
-    // if we have resolved a parent, we add it to the cloned entry
-    let after = getInsertAfterId(rundown, parent, options?.after, options?.before);
-    if (!after) {
-      after = entry.id;
-    }
+      const after = getInsertAfterId(rundown, parent, options.after, options.before);
+      const before = options?.before === true ? after : (options.before ?? null);
+      return { after, before };
+    })();
 
-    return addToRundown(rundown, clonedEntry, after, parent);
+    return addToRundown(rundown, clonedEntry, parent, position.after, position.before);
   }
 }
 

apps/server/src/api-data/rundown/rundown.service.ts

@@ -40,11 +40,15 @@ import { parseRundown } from './rundown.parser.js';
 import type { RundownMetadata } from './rundown.types.js';
 import {
   generateEvent,
+  getFirstInsertId,
   getIntegerAndFraction,
+  getLastInsertId,
+  getPreviousInsertId,
   hasChanges,
   mergeRundownPreservingFields,
   isLoadedPlayable,
 } from './rundown.utils.js';
+import { assertInsertAnchorExists, assertInsertAnchorInOrder, assertSingleInsertAnchor } from './rundown.validation.js';
 
 /**
  * creates a new entry with given data
@@ -52,6 +56,8 @@ import {
 export async function addEntry(rundownId: string, eventData: EventPostPayload): Promise<OntimeEntry> {
   const { rundown, commit } = createTransaction({ rundownId, mutableRundown: true });
 
+  assertSingleInsertAnchor(eventData);
+
   // we allow the user to provide an ID, but make sure it is unique
   if (eventData?.id && Object.hasOwn(rundown.entries, eventData.id)) {
     throw new Error(`Event with ID ${eventData.id} already exists`);
@@ -68,14 +74,29 @@ export async function addEntry(rundownId: string, eventData: EventPostPayload):
     parent = maybeParent;
   }
 
+  assertInsertAnchorInOrder(rundown, parent, eventData);
+
   // normalise the position of the event in the rundown order
-  const afterId = getInsertAfterId(rundown, parent, eventData?.after, eventData?.before);
+  const insertPosition = (() => {
+    if (eventData.before !== undefined) {
+      const beforeId = getFirstInsertId(rundown, parent, eventData.before);
+      return { afterId: null, beforeId, cueAfterId: getPreviousInsertId(rundown, parent, beforeId) };
+    }
+
+    // if after is an ID, we will try and find the entry
+    if (eventData.after !== undefined && eventData.after !== true) {
+      const afterId = getInsertAfterId(rundown, parent, eventData.after);
+      return { afterId, beforeId: null, cueAfterId: afterId };
+    }
+
+    return { afterId: null, beforeId: null, cueAfterId: getLastInsertId(rundown, parent) };
+  })();
 
   // generate a fully formed entry from the patch
-  const newEntry = generateEvent(rundown, eventData, afterId, parent?.id);
+  const newEntry = generateEvent(rundown, eventData, insertPosition.cueAfterId, parent?.id);
 
   // make mutations to rundown
-  rundownMutation.add(rundown, newEntry, afterId, parent);
+  rundownMutation.add(rundown, newEntry, parent, insertPosition.afterId, insertPosition.beforeId);
 
   const { rundown: responseRundown, rundownMetadata, revision } = await commit();
 
@@ -395,7 +416,10 @@ export async function swapEvents(rundownId: string, fromId: EntryId, toId: Entry
  * @throws if the entry to clone does not exist
  */
 export async function cloneEntry(rundownId: string, entryId: EntryId, options: InsertOptions): Promise<Rundown> {
+  assertSingleInsertAnchor(options);
+
   const { rundown, commit } = createTransaction({ rundownId, mutableRundown: true });
+  assertInsertAnchorExists(rundown, options);
   const originalEntry = rundown.entries[entryId];
 
   if (!originalEntry) {

apps/server/src/api-data/rundown/rundown.utils.ts

@@ -70,6 +70,50 @@ export function generateEvent(
   throw new Error('Invalid event type');
 }
 
+/**
+ * Gets the last valid insertion reference for a top-level rundown or group order.
+ * Used when appending entries and when generating cues from the preceding entry.
+ */
+export function getLastInsertId(rundown: Rundown, parent: OntimeGroup | null): EntryId | null {
+  const insertionList = parent ? parent.entries : rundown.order;
+  return insertionList[insertionList.length - 1] ?? null;
+}
+
+/**
+ * Resolves a `before` insertion option to the first entry in the relevant order when `true` is provided.
+ * String values are already explicit anchors and are returned unchanged.
+ */
+export function getFirstInsertId(rundown: Rundown, parent: OntimeGroup | null, before: EntryId | true): EntryId | null {
+  if (before !== true) {
+    return before;
+  }
+
+  const insertionList = parent ? parent.entries : rundown.order;
+  return insertionList[0] ?? null;
+}
+
+/**
+ * Gets the sibling before a `before` insertion anchor in the top-level or group order.
+ * Returns `null` when the new entry will be inserted at the start.
+ */
+export function getPreviousInsertId(
+  rundown: Rundown,
+  parent: OntimeGroup | null,
+  beforeId: EntryId | null,
+): EntryId | null {
+  if (beforeId === null) {
+    return null;
+  }
+
+  const insertionList = parent ? parent.entries : rundown.order;
+  const beforeIndex = insertionList.indexOf(beforeId);
+  if (beforeIndex < 1) {
+    return null;
+  }
+
+  return insertionList[beforeIndex - 1] ?? null;
+}
+
 export function createEventPatch(originalEvent: OntimeEvent, patchEvent: Partial<OntimeEvent>): OntimeEvent {
   if (Object.keys(patchEvent).length === 0) {
     return originalEvent;

apps/server/src/api-data/rundown/rundown.validation.ts

@@ -1,4 +1,5 @@
 import { body, param } from 'express-validator';
+import type { EntryId, InsertOptions, OntimeGroup, Rundown } from 'ontime-types';
 
 import { requestValidationFunction } from '../validation-utils/validationFunction.js';
 
@@ -37,15 +38,27 @@ export const rundownImportValidator = [
 
 export const entryPostValidator = [
   body('type').isString().isIn(['event', 'delay', 'group', 'milestone']),
-  body('after').optional().isString(),
-  body('before').optional().isString(),
+  body('after')
+    .optional()
+    .custom((value) => value === true || typeof value === 'string')
+    .withMessage('Allowed values for after are an ID or true.'),
+  body('before')
+    .optional()
+    .custom((value) => value === true || typeof value === 'string')
+    .withMessage('Allowed values for before are an ID or true.'),
 
   requestValidationFunction,
 ];
 
 export const clonePostValidator = [
-  body('after').optional().isString(),
-  body('before').optional().isString(),
+  body('after')
+    .optional()
+    .custom((value) => value === true || typeof value === 'string')
+    .withMessage('Allowed values for after are an ID or true.'),
+  body('before')
+    .optional()
+    .custom((value) => value === true || typeof value === 'string')
+    .withMessage('Allowed values for before are an ID or true.'),
 
   requestValidationFunction,
 ];
@@ -93,4 +106,44 @@ export const entryRenumberValidator = [
   requestValidationFunction,
 ];
 
+/**
+ * Ensures callers use a single insertion anchor so placement is unambiguous.
+ */
+export function assertSingleInsertAnchor(options: InsertOptions) {
+  if (options.after !== undefined && options.before !== undefined) {
+    throw new Error('Use only one insertion anchor: after or before');
+  }
+}
+
+/**
+ * Ensures an ID anchor exists and belongs to the order it is intended to position within.
+ */
+export function assertInsertAnchorInOrder(rundown: Rundown, parent: OntimeGroup | null, options: InsertOptions) {
+  const anchor = getStringInsertAnchor(options);
+  if (anchor === undefined) return;
+
+  if (!Object.hasOwn(rundown.entries, anchor)) {
+    throw new Error(`Insertion anchor with ID ${anchor} does not exist`);
+  }
+
+  const insertionList = parent ? parent.entries : rundown.order;
+  if (!insertionList.includes(anchor)) {
+    throw new Error(`Insertion anchor with ID ${anchor} is not in the target order`);
+  }
+}
+
+/** Ensures an ID anchor refers to an entry in the rundown. */
+export function assertInsertAnchorExists(rundown: Rundown, options: InsertOptions) {
+  const anchor = getStringInsertAnchor(options);
+  if (anchor !== undefined && !Object.hasOwn(rundown.entries, anchor)) {
+    throw new Error(`Insertion anchor with ID ${anchor} does not exist`);
+  }
+}
+
+function getStringInsertAnchor(options: InsertOptions): EntryId | undefined {
+  if (typeof options.after === 'string') return options.after;
+  if (typeof options.before === 'string') return options.before;
+  return undefined;
+}
+
 // #endregion operations on rundown entries =======================