diff --git a/ghost/core/core/server/services/automations/fake-database-automations-repository.ts b/ghost/core/core/server/services/automations/fake-database-automations-repository.ts index 9f2d9a42114..4612590fd84 100644 --- a/ghost/core/core/server/services/automations/fake-database-automations-repository.ts +++ b/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['data']; +type SendEmailActionData = Extract['data']; +type RevisionDataFor = { + [FieldT in keyof ActionDataT]: ActionRevisionRow[FieldT & keyof ActionRevisionRow]; +}; +type WaitRevisionData = RevisionDataFor; +type SendEmailRevisionData = RevisionDataFor; + 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 { + 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; diff --git a/ghost/core/package.json b/ghost/core/package.json index 28dd12e548a..a7dd6ae3580 100644 --- a/ghost/core/package.json +++ b/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", diff --git a/ghost/core/test/unit/server/services/automations/automations-repository.test.ts b/ghost/core/test/unit/server/services/automations/automations-repository.test.ts index 1c2e5312398..1501224f371 100644 --- a/ghost/core/test/unit/server/services/automations/automations-repository.test.ts +++ b/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); + }); + }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5b5b058f606..247f35fddf5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2531,6 +2531,9 @@ importers: date-fns: specifier: 2.30.0 version: 2.30.0 + dequal: + specifier: 'catalog:' + version: 2.0.3 dompurify: specifier: 'catalog:' version: 3.4.5