From a5daa76616f42bea415c571e0ea74d4d3c0d9279 Mon Sep 17 00:00:00 2001
From: Carlos Valente <carlosvalente@pm.me>
Date: Sat, 20 Dec 2025 15:07:47 +0100
Subject: [PATCH] refactor: improve merge logic

---
 .../data-provider/DataProvider.utils.ts       |  17 +--
 .../__tests__/DataProvider.utils.test.ts      | 114 +++++++++++++++---
 2 files changed, 107 insertions(+), 24 deletions(-)

diff --git a/apps/server/src/classes/data-provider/DataProvider.utils.ts b/apps/server/src/classes/data-provider/DataProvider.utils.ts
index 1cb717f3d..6a2fbb756 100644
--- a/apps/server/src/classes/data-provider/DataProvider.utils.ts
+++ b/apps/server/src/classes/data-provider/DataProvider.utils.ts
@@ -7,24 +7,27 @@ export function safeMerge(existing: DatabaseModel, newData: Partial<DatabaseMode
   const deepExisting = structuredClone(existing);
   const deepNewData = structuredClone(newData);
 
+  // destructure each property to simplify merging not provided ie: ...{} has no effect
   const {
     rundowns = {},
     project = {},
     settings = {},
     viewSettings = {},
-    urlPresets = deepExisting.urlPresets,
-    customFields = deepExisting.customFields,
-    automation = deepExisting.automation,
+    urlPresets = [],
+    customFields = {},
+    automation,
   } = deepNewData;
 
   return {
-    ...deepExisting,
     rundowns: { ...existing.rundowns, ...rundowns },
     project: { ...deepExisting.project, ...project },
     settings: { ...deepExisting.settings, ...settings },
     viewSettings: { ...deepExisting.viewSettings, ...viewSettings },
-    urlPresets: urlPresets ?? deepExisting.urlPresets,
-    customFields: customFields ?? deepExisting.customFields,
-    automation: { ...deepExisting.automation, ...automation },
+    // URL presets are independent and we can append them together
+    urlPresets: [...deepExisting.urlPresets, ...urlPresets],
+    // custom fields can be merged
+    customFields: { ...deepExisting.customFields, ...customFields },
+    // trigger and automation are coupled and cannot be changed individually so we replace the whole automation
+    automation: automation ?? deepExisting.automation,
   };
 }
diff --git a/apps/server/src/classes/data-provider/__tests__/DataProvider.utils.test.ts b/apps/server/src/classes/data-provider/__tests__/DataProvider.utils.test.ts
index 301acda7c..705cc9d40 100644
--- a/apps/server/src/classes/data-provider/__tests__/DataProvider.utils.test.ts
+++ b/apps/server/src/classes/data-provider/__tests__/DataProvider.utils.test.ts
@@ -1,14 +1,16 @@
-import { DatabaseModel, Settings, URLPreset } from 'ontime-types';
+import { AutomationSettings, CustomField, DatabaseModel, Settings, TimerLifeCycle, URLPreset } from 'ontime-types';
 
-import { demoDb } from '../../../models/demoProject.js';
+import { makeNewProject } from '../../../models/dataModel.js';
 import { makeOntimeEvent, makeRundown } from '../../../api-data/rundown/__mocks__/rundown.mocks.js';
 
 import { safeMerge } from '../DataProvider.utils.js';
 
+const baseDb = makeNewProject('demo');
+
 describe('safeMerge', () => {
   it('returns existing data if new data is not provided', () => {
-    const mergedData = safeMerge(demoDb, {});
-    expect(mergedData).toEqual(demoDb);
+    const mergedData = safeMerge(baseDb, {});
+    expect(mergedData).toEqual(baseDb);
   });
 
   it('overrides a rundown with the same key', () => {
@@ -20,7 +22,7 @@ describe('safeMerge', () => {
       },
       order: ['1', '2'],
     });
-    const mergedData = safeMerge(demoDb, { rundowns: { demo: newData } });
+    const mergedData = safeMerge(baseDb, { rundowns: { demo: newData } });
     expect(mergedData.rundowns.demo).toStrictEqual(newData);
   });
 
@@ -33,13 +35,13 @@ describe('safeMerge', () => {
       },
       order: ['1', '2'],
     });
-    const mergedData = safeMerge(demoDb, { rundowns: { rundown: newData } });
-    expect(mergedData.rundowns.demo).toStrictEqual(demoDb.rundowns.demo);
+    const mergedData = safeMerge(baseDb, { rundowns: { rundown: newData } });
+    expect(mergedData.rundowns.demo).toStrictEqual(baseDb.rundowns.demo);
     expect(mergedData.rundowns.rundown).toStrictEqual(newData);
   });
 
   it('merges the project key', () => {
-    const mergedData = safeMerge(demoDb, {
+    const mergedData = safeMerge(baseDb, {
       project: {
         title: 'new title',
         info: 'new backstage info',
@@ -54,10 +56,10 @@ describe('safeMerge', () => {
 
     expect(mergedData.project).toStrictEqual({
       title: 'new title',
-      description: demoDb.project.description,
-      url: demoDb.project.url,
+      description: baseDb.project.description,
+      url: baseDb.project.url,
       info: 'new backstage info',
-      logo: demoDb.project.logo,
+      logo: baseDb.project.logo,
       custom: [
         {
           title: 'new custom title',
@@ -68,7 +70,7 @@ describe('safeMerge', () => {
   });
 
   it('merges the settings key', () => {
-    const mergedData = safeMerge(demoDb, {
+    const mergedData = safeMerge(baseDb, {
       settings: {
         serverPort: 3000,
         language: 'pt',
@@ -80,7 +82,7 @@ describe('safeMerge', () => {
       serverPort: 3000,
       operatorKey: null,
       editorKey: null,
-      timeFormat: '24',
+      timeFormat: baseDb.settings.timeFormat,
       language: 'pt',
     });
   });
@@ -93,16 +95,17 @@ describe('safeMerge', () => {
       ] as URLPreset[],
     };
 
-    const mergedData = safeMerge(demoDb, newData);
+    const mergedData = safeMerge(baseDb, newData);
 
-    expect(mergedData.urlPresets).toStrictEqual(newData.urlPresets);
+    expect(mergedData.urlPresets).toStrictEqual([...baseDb.urlPresets, ...newData.urlPresets]);
   });
 
   it('merges customFields into existing object', () => {
     const existing = {
+      ...baseDb,
       customFields: {
-        lighting: { type: 'text', label: 'lighting' },
-        sound: { type: 'text', label: 'sound' },
+        lighting: { type: 'text', colour: 'red', label: 'lighting' },
+        sound: { type: 'text', colour: 'red', label: 'sound' },
       },
     };
 
@@ -114,6 +117,8 @@ describe('safeMerge', () => {
     };
 
     const expected = {
+      lighting: { type: 'text', colour: 'red', label: 'lighting' },
+      sound: { type: 'text', colour: 'red', label: 'sound' },
       switcher: { type: 'text', label: 'switcher' },
       vfx: { type: 'text', label: 'vfx' },
     };
@@ -122,4 +127,79 @@ describe('safeMerge', () => {
     const result = safeMerge(existing, newData);
     expect(result.customFields).toEqual(expected);
   });
+
+  it('overwrites matching customFields keys when merging', () => {
+    const existing = {
+      ...baseDb,
+      customFields: {
+        lighting: { type: 'text', colour: 'red', label: 'lighting' } satisfies CustomField,
+      },
+    };
+
+    const newData = {
+      customFields: {
+        lighting: { type: 'text', colour: 'red', label: 'updated lighting' } satisfies CustomField,
+      },
+    };
+
+    const result = safeMerge(existing, newData);
+    expect(result.customFields).toEqual({
+      lighting: { type: 'text', colour: 'red', label: 'updated lighting' },
+    });
+  });
+
+  it('replaces automation settings when provided', () => {
+    const existing = {
+      ...baseDb,
+      automation: {
+        enabledAutomations: true,
+        enabledOscIn: true,
+        oscPortIn: 9000,
+        triggers: [
+          {
+            id: 'trigger-1',
+            title: 'Trigger 1',
+            trigger: TimerLifeCycle.onStart,
+            automationId: 'automation-1',
+          },
+        ],
+        automations: {
+          'automation-1': {
+            id: 'automation-1',
+            title: 'Automation 1',
+            filterRule: 'all',
+            filters: [],
+            outputs: [],
+          },
+        } satisfies AutomationSettings['automations'],
+      },
+    };
+
+    const newAutomation: AutomationSettings = {
+      enabledAutomations: false,
+      enabledOscIn: false,
+      oscPortIn: 7777,
+      triggers: [
+        {
+          id: 'trigger-2',
+          title: 'Trigger 2',
+          trigger: TimerLifeCycle.onStop,
+          automationId: 'automation-2',
+        },
+      ],
+      automations: {
+        'automation-2': {
+          id: 'automation-2',
+          title: 'Automation 2',
+          filterRule: 'any',
+          filters: [],
+          outputs: [],
+        },
+      },
+    };
+
+    const result = safeMerge(existing, { automation: newAutomation });
+
+    expect(result.automation).toStrictEqual(newAutomation);
+  });
 });