Deduplicated revisions when editing automations

ghost/core/core/server/services/automations/fake-database-automations-repository.ts

@@ -1,6 +1,7 @@
 import errors from '@tryghost/errors';
 import tpl from '@tryghost/tpl';
 import ObjectId from 'bson-objectid';
+import {dequal} from 'dequal';
 import type {DatabaseSync} from 'node:sqlite';
 import {MEMBER_WELCOME_EMAIL_SLUGS} from '../member-welcome-emails/constants';
 import type {
@@ -12,6 +13,7 @@ import type {
     EditAutomationData,
     Page
 } from './automations-repository';
+import type {ExclusifyUnion} from 'type-fest';
 
 const HOUR_MS = 60 * 60 * 1000;
 
@@ -42,6 +44,18 @@ interface ActionRow {
     email_design_setting_id: string | null;
 }
 
+type ActionRevisionRow = {
+    action_id: string;
+    created_at: string;
+    wait_hours: number | null;
+    email_subject: string | null;
+    email_lexical: string | null;
+    email_sender_name: string | null;
+    email_sender_email: string | null;
+    email_sender_reply_to: string | null;
+    email_design_setting_id: string | null;
+};
+
 interface EdgeRow {
     source_action_id: string;
     target_action_id: string;
@@ -55,6 +69,14 @@ type NextActionRevisionRow = {
     wait_hours: number | null;
 };
 
+type WaitActionData = Extract<AutomationAction, {type: 'wait'}>['data'];
+type SendEmailActionData = Extract<AutomationAction, {type: 'send_email'}>['data'];
+type RevisionDataFor<ActionDataT> = {
+    [FieldT in keyof ActionDataT]: ActionRevisionRow[FieldT & keyof ActionRevisionRow];
+};
+type WaitRevisionData = RevisionDataFor<WaitActionData>;
+type SendEmailRevisionData = RevisionDataFor<SendEmailActionData>;
+
 export function createFakeDatabaseAutomationsRepository({
     getDatabase
 }: {
@@ -312,8 +334,10 @@ function replaceAutomationGraph(database: DatabaseSync, automationId: string, ac
             });
         }
 
-        // TODO (NY-1283): Deduplicate revisions before inserting them.
-        insertActionRevision(database, action.id, action, now);
+        const latestRevision = loadLatestActionRevision(database, action.id);
+        if (shouldInsertActionRevision(action, latestRevision)) {
+            insertActionRevision(database, action.id, action, now, latestRevision);
+        }
     }
 
     for (const existingAction of existingActions) {
@@ -362,6 +386,62 @@ function insertAction(database: DatabaseSync, action: {
     `).run(action);
 }
 
+function shouldInsertActionRevision(action: AutomationAction, latestRevision: ActionRevisionRow | null): boolean {
+    if (!latestRevision) {
+        return true;
+    }
+
+    return !dequal(buildRevisionActionData(action, latestRevision), action.data);
+}
+
+function buildRevisionActionData(action: AutomationAction, revision: ActionRevisionRow): ExclusifyUnion<WaitRevisionData | SendEmailRevisionData> {
+    switch (action.type) {
+    case 'wait':
+        return {
+            wait_hours: revision.wait_hours
+        };
+    case 'send_email':
+        return {
+            email_subject: revision.email_subject,
+            email_lexical: revision.email_lexical,
+            email_sender_name: revision.email_sender_name,
+            email_sender_email: revision.email_sender_email,
+            email_sender_reply_to: revision.email_sender_reply_to,
+            email_design_setting_id: revision.email_design_setting_id
+        };
+    default: {
+        const _exhaustive: never = action;
+        throw new errors.InternalServerError({
+            message: `Unhandled action type: ${_exhaustive}`
+        });
+    }
+    }
+}
+
+function loadLatestActionRevision(database: DatabaseSync, actionId: string): ActionRevisionRow | null {
+    const row = database.prepare(`
+        SELECT
+            action_id,
+            created_at,
+            wait_hours,
+            email_subject,
+            email_lexical,
+            email_sender_name,
+            email_sender_email,
+            email_sender_reply_to,
+            email_design_setting_id
+        FROM automation_action_revisions
+        WHERE action_id = ?
+            AND created_at = (
+                SELECT MAX(created_at)
+                FROM automation_action_revisions
+                WHERE action_id = ?
+            )
+    `).get(actionId, actionId) as ActionRevisionRow | undefined;
+
+    return row ?? null;
+}
+
 function softDeleteAction(database: DatabaseSync, actionId: string, deletedAt: string) {
     database.prepare(`
         UPDATE automation_actions
@@ -375,8 +455,8 @@ function softDeleteAction(database: DatabaseSync, actionId: string, deletedAt: s
     });
 }
 
-function insertActionRevision(database: DatabaseSync, actionId: string, action: AutomationAction, createdAt: string) {
-    const revision = buildActionRevision(actionId, action, getNextRevisionCreatedAt(database, actionId, createdAt));
+function insertActionRevision(database: DatabaseSync, actionId: string, action: AutomationAction, createdAt: string, latestRevision: ActionRevisionRow | null) {
+    const revision = buildActionRevision(actionId, action, getNextRevisionCreatedAt(latestRevision?.created_at ?? null, createdAt));
 
     database.prepare(`
         INSERT INTO automation_action_revisions
@@ -406,19 +486,13 @@ function insertActionRevision(database: DatabaseSync, actionId: string, action:
     `).run(revision);
 }
 
-function getNextRevisionCreatedAt(database: DatabaseSync, actionId: string, requestedCreatedAt: string) {
-    const row = database.prepare(`
-        SELECT MAX(created_at) AS created_at
-        FROM automation_action_revisions
-        WHERE action_id = ?
-    `).get(actionId) as {created_at: string | null} | undefined;
-
-    if (!row?.created_at) {
+function getNextRevisionCreatedAt(latestCreatedAt: string | null, requestedCreatedAt: string) {
+    if (!latestCreatedAt) {
         return requestedCreatedAt;
     }
 
     const requestedTime = new Date(requestedCreatedAt).getTime();
-    const latestTime = new Date(row.created_at).getTime();
+    const latestTime = new Date(latestCreatedAt).getTime();
 
     if (requestedTime > latestTime) {
         return requestedCreatedAt;

ghost/core/package.json

@@ -160,6 +160,7 @@
     "csso": "5.0.5",
     "csv-writer": "1.6.0",
     "date-fns": "2.30.0",
+    "dequal": "catalog:",
     "dompurify": "catalog:",
     "downsize": "0.0.8",
     "entities": "4.5.0",

ghost/core/test/unit/server/services/automations/automations-repository.test.ts

@@ -1,7 +1,8 @@
 import assert from 'node:assert/strict';
-import {AutomationsRepository} from '../../../../../core/server/services/automations/automations-repository';
+import ObjectId from 'bson-objectid';
 import {createTemporaryFakeAutomationsDatabase} from '../../../../../core/server/services/automations/temporary-fake-database';
 import {createFakeDatabaseAutomationsRepository} from '../../../../../core/server/services/automations/fake-database-automations-repository';
+import type {AutomationAction, AutomationsRepository} from '../../../../../core/server/services/automations/automations-repository';
 import type {DatabaseSync, SQLInputValue} from 'node:sqlite';
 
 const addHours = (dateCol: unknown, hours: number): Date => {
@@ -61,6 +62,24 @@ describe('automations repository', function () {
         return result?.count;
     };
 
+    const getRevisionCount = (actionId?: string) => {
+        const row = actionId
+            ? database.prepare('SELECT COUNT(*) AS count FROM automation_action_revisions WHERE action_id = ?').get(actionId)
+            : database.prepare('SELECT COUNT(*) AS count FROM automation_action_revisions').get();
+
+        return Number((row as {count: number}).count);
+    };
+
+    const changeWaitHours = (action: AutomationAction, waitHours: number): AutomationAction => {
+        assert.equal(action.type, 'wait');
+        return {
+            ...action,
+            data: {
+                wait_hours: waitHours
+            }
+        };
+    };
+
     beforeEach(function () {
         database = createTemporaryFakeAutomationsDatabase();
         repo = createFakeDatabaseAutomationsRepository({
@@ -202,4 +221,72 @@ describe('automations repository', function () {
             assert.equal(getRunCountByAutomationId(freeAutomation.id), 0);
         });
     });
+
+    describe('edit', function () {
+        it('only inserts action revisions when action data changes', async function () {
+            const initialAutomation = await getAutomationBySlug('member-welcome-email-free');
+            const initialRevisionCount = getRevisionCount();
+            const waitAction = initialAutomation.actions.find(action => action.type === 'wait');
+            const unchangedEmailAction = initialAutomation.actions.find(action => action.type === 'send_email');
+
+            assert(waitAction);
+            assert(unchangedEmailAction);
+            assert.equal(getRevisionCount(waitAction.id), 1);
+            assert.equal(getRevisionCount(unchangedEmailAction.id), 1);
+
+            await repo.edit(initialAutomation.id, {
+                status: 'inactive',
+                actions: initialAutomation.actions,
+                edges: initialAutomation.edges
+            });
+
+            assert.equal(getRevisionCount(), initialRevisionCount);
+            assert.equal(getRevisionCount(waitAction.id), 1);
+            assert.equal(getRevisionCount(unchangedEmailAction.id), 1);
+
+            const changedWaitAction = changeWaitHours(waitAction, waitAction.data.wait_hours + 24);
+
+            await repo.edit(initialAutomation.id, {
+                status: 'inactive',
+                actions: [changedWaitAction, unchangedEmailAction],
+                edges: [{
+                    source_action_id: changedWaitAction.id,
+                    target_action_id: unchangedEmailAction.id
+                }]
+            });
+
+            assert.equal(getRevisionCount(), initialRevisionCount + 1);
+            assert.equal(getRevisionCount(waitAction.id), 2);
+            assert.equal(getRevisionCount(unchangedEmailAction.id), 1);
+
+            const addedActionId = ObjectId().toString();
+            const addedAction: AutomationAction = {
+                id: addedActionId,
+                type: 'wait',
+                data: {
+                    wait_hours: 72
+                }
+            };
+
+            await repo.edit(initialAutomation.id, {
+                status: 'inactive',
+                actions: [changedWaitAction, unchangedEmailAction, addedAction],
+                edges: [
+                    {
+                        source_action_id: changedWaitAction.id,
+                        target_action_id: unchangedEmailAction.id
+                    },
+                    {
+                        source_action_id: unchangedEmailAction.id,
+                        target_action_id: addedActionId
+                    }
+                ]
+            });
+
+            assert.equal(getRevisionCount(), initialRevisionCount + 2);
+            assert.equal(getRevisionCount(waitAction.id), 2);
+            assert.equal(getRevisionCount(unchangedEmailAction.id), 1);
+            assert.equal(getRevisionCount(addedActionId), 1);
+        });
+    });
 });