diff --git a/apps/admin-x-framework/src/api/automations.ts b/apps/admin-x-framework/src/api/automations.ts index d64bffee0a5..82d3e2b3db5 100644 --- a/apps/admin-x-framework/src/api/automations.ts +++ b/apps/admin-x-framework/src/api/automations.ts @@ -26,9 +26,6 @@ export type AutomationSendEmailAction = { data: { email_subject: string; email_lexical: string; - email_sender_name: string | null; - email_sender_email: string | null; - email_sender_reply_to: string | null; email_design_setting_id: string; }; } @@ -115,9 +112,6 @@ const buildSendEmailAction = (): AutomationSendEmailAction => ({ data: { email_subject: '', email_lexical: EMPTY_EMAIL_LEXICAL, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, // TODO NY-1252: replace this placeholder when email design settings are available. email_design_setting_id: 'placeholder' } diff --git a/apps/admin-x-framework/test/unit/api/automations.test.ts b/apps/admin-x-framework/test/unit/api/automations.test.ts index cbbeb8ddd3c..24d369aec44 100644 --- a/apps/admin-x-framework/test/unit/api/automations.test.ts +++ b/apps/admin-x-framework/test/unit/api/automations.test.ts @@ -318,9 +318,6 @@ describe('automations api helpers', () => { data: { email_subject: 'Hi', email_lexical: '{}', - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: 'placeholder' } } @@ -374,9 +371,6 @@ describe('automations api helpers', () => { data: { email_subject: 'Original subject', email_lexical: '{"root":{"children":[]}}', - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: 'placeholder', ...overrides } @@ -399,7 +393,7 @@ describe('automations api helpers', () => { }); it('updates subject and lexical, preserving the rest of data', () => { - const detail = baseDetail([sendEmailAction('a', {email_sender_name: 'Jane'})], []); + const detail = baseDetail([sendEmailAction('a', {email_design_setting_id: 'design-setting-id'})], []); const next = updateSendEmailAction({ detail, @@ -412,8 +406,7 @@ describe('automations api helpers', () => { expectSendEmailAction(updated); expect(updated.data.email_subject).toBe('Just the subject'); expect(updated.data.email_lexical).toBe('{"root":{"children":[{"type":"paragraph"}]}}'); - expect(updated.data.email_sender_name).toBe('Jane'); - expect(updated.data.email_design_setting_id).toBe('placeholder'); + expect(updated.data.email_design_setting_id).toBe('design-setting-id'); }); it('does not mutate the original detail or action', () => { diff --git a/apps/posts/test/unit/views/automations/automation-editor.test.tsx b/apps/posts/test/unit/views/automations/automation-editor.test.tsx index b6ba9ee9432..2728df30152 100644 --- a/apps/posts/test/unit/views/automations/automation-editor.test.tsx +++ b/apps/posts/test/unit/views/automations/automation-editor.test.tsx @@ -169,9 +169,6 @@ const automationDetail: AutomationDetail = { data: { email_subject: 'Welcome to The Blueprint', email_lexical: NON_EMPTY_EMAIL_LEXICAL, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: 'design-1' } } diff --git a/ghost/core/core/server/services/automations/automations-api.ts b/ghost/core/core/server/services/automations/automations-api.ts index 1b48f5a0f57..55761974f1f 100644 --- a/ghost/core/core/server/services/automations/automations-api.ts +++ b/ghost/core/core/server/services/automations/automations-api.ts @@ -53,9 +53,6 @@ const sendEmailActionSchema = z.object({ return false; } }), - email_sender_name: z.string().nullable(), - email_sender_email: z.string().nullable(), - email_sender_reply_to: z.string().nullable(), email_design_setting_id: z.string().min(1) }).strict() }).strict(); diff --git a/ghost/core/core/server/services/automations/automations-repository.ts b/ghost/core/core/server/services/automations/automations-repository.ts index 4bdf01d4dfd..369b0a9af39 100644 --- a/ghost/core/core/server/services/automations/automations-repository.ts +++ b/ghost/core/core/server/services/automations/automations-repository.ts @@ -30,9 +30,6 @@ export interface SendEmailAction { data: { email_subject: string; email_lexical: string; - email_sender_name: string | null; - email_sender_email: string | null; - email_sender_reply_to: string | null; email_design_setting_id: string; }; } @@ -88,9 +85,6 @@ export type AutomationStepToRun = ReadonlyDeep; 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 a83b8825cdb..7ea1acb6cfa 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 @@ -44,9 +44,6 @@ interface ActionRow { 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; } @@ -56,9 +53,6 @@ type ActionRevisionRow = { 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; }; @@ -92,9 +86,6 @@ type StepToRunRow = { 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; }; @@ -380,9 +371,6 @@ function fetchAndLockSteps(database: DatabaseSync, limit: number): { 'revision.wait_hours as wait_hours', 'revision.email_subject as email_subject', 'revision.email_lexical as email_lexical', - 'revision.email_sender_name as email_sender_name', - 'revision.email_sender_email as email_sender_email', - 'revision.email_sender_reply_to as email_sender_reply_to', 'revision.email_design_setting_id as email_design_setting_id' ) .innerJoin('automation_runs as run', 'run.id', 'step.automation_run_id') @@ -443,9 +431,6 @@ function buildStepToRun(row: ReadonlyDeep): AutomationStepToRun { type: 'send_email', email_subject: requireValue(row, 'email_subject'), email_lexical: requireValue(row, 'email_lexical'), - email_sender_name: row.email_sender_name, - email_sender_email: row.email_sender_email, - email_sender_reply_to: row.email_sender_reply_to, email_design_setting_id: row.email_design_setting_id }; default: @@ -759,9 +744,6 @@ function buildRevisionActionData(action: AutomationAction, revision: ActionRevis 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: { @@ -781,9 +763,6 @@ function loadLatestActionRevision(database: DatabaseSync, actionId: string): Act 'wait_hours', 'email_subject', 'email_lexical', - 'email_sender_name', - 'email_sender_email', - 'email_sender_reply_to', 'email_design_setting_id' ) .where('action_id', actionId) @@ -833,9 +812,6 @@ function buildActionRevision(actionId: string, action: AutomationAction, created wait_hours: action.data.wait_hours, email_subject: null, email_lexical: null, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: null }; } @@ -847,9 +823,6 @@ function buildActionRevision(actionId: string, action: AutomationAction, created wait_hours: null, email_subject: action.data.email_subject, email_lexical: action.data.email_lexical, - email_sender_name: action.data.email_sender_name, - email_sender_email: action.data.email_sender_email, - email_sender_reply_to: action.data.email_sender_reply_to, email_design_setting_id: action.data.email_design_setting_id }; } @@ -912,9 +885,6 @@ function loadActionRows(database: DatabaseSync, automationId: string): ActionRow 'r.wait_hours as wait_hours', 'r.email_subject as email_subject', 'r.email_lexical as email_lexical', - 'r.email_sender_name as email_sender_name', - 'r.email_sender_email as email_sender_email', - 'r.email_sender_reply_to as email_sender_reply_to', 'r.email_design_setting_id as email_design_setting_id' ) .innerJoin('automation_action_revisions as r', 'r.action_id', 'a.id') @@ -967,9 +937,6 @@ function buildActionPayload(row: ActionRow): AutomationAction { data: { email_subject: requireValue(row, 'email_subject'), email_lexical: requireValue(row, 'email_lexical'), - email_sender_name: row.email_sender_name, - email_sender_email: row.email_sender_email, - email_sender_reply_to: row.email_sender_reply_to, email_design_setting_id: requireValue(row, 'email_design_setting_id') } }; diff --git a/ghost/core/core/server/services/automations/poll.ts b/ghost/core/core/server/services/automations/poll.ts index 80ebdcc446f..0a1d0489f1c 100644 --- a/ghost/core/core/server/services/automations/poll.ts +++ b/ghost/core/core/server/services/automations/poll.ts @@ -13,9 +13,6 @@ type MemberWelcomeEmailService = { email: { designSettingId: string | null; lexical: string; - senderEmail: string | null; - senderName: string | null; - senderReplyTo: string | null; subject: string; }; member: { @@ -160,9 +157,6 @@ const processStep = async ({ email: { designSettingId: step.email_design_setting_id, lexical: step.email_lexical, - senderEmail: step.email_sender_email, - senderName: step.email_sender_name, - senderReplyTo: step.email_sender_reply_to, subject: step.email_subject }, member: { diff --git a/ghost/core/core/server/services/automations/temporary-fake-database.ts b/ghost/core/core/server/services/automations/temporary-fake-database.ts index 4a1129b26e6..af3a8a3b491 100644 --- a/ghost/core/core/server/services/automations/temporary-fake-database.ts +++ b/ghost/core/core/server/services/automations/temporary-fake-database.ts @@ -41,9 +41,6 @@ export function createTemporaryFakeAutomationsDatabase(): DatabaseSync { version: 1 } }); - const fakeEmailSenderName = 'Ghost'; - const fakeEmailSenderEmail = 'hello@example.com'; - const fakeEmailSenderReplyTo = 'support@example.com'; const fakeEmailDesignSettingId = id(); database.exec(` @@ -72,9 +69,6 @@ CREATE TABLE automation_action_revisions ( wait_hours INTEGER, email_subject TEXT, email_lexical TEXT, - email_sender_name TEXT, - email_sender_email TEXT, - email_sender_reply_to TEXT, email_design_setting_id TEXT, -- not a real foreign key here UNIQUE (created_at, action_id) ) STRICT; @@ -206,8 +200,8 @@ CREATE TABLE automation_run_steps ( const insertActionRevision = database.prepare(` INSERT INTO automation_action_revisions - (id, created_at, action_id, wait_hours, email_subject, email_lexical, email_sender_name, email_sender_email, email_sender_reply_to, email_design_setting_id) VALUES - (:id, :created_at, :action_id, :wait_hours, :email_subject, :email_lexical, :email_sender_name, :email_sender_email, :email_sender_reply_to, :email_design_setting_id) + (id, created_at, action_id, wait_hours, email_subject, email_lexical, email_design_setting_id) VALUES + (:id, :created_at, :action_id, :wait_hours, :email_subject, :email_lexical, :email_design_setting_id) `); insertActionRevision.run({ id: id(), @@ -216,9 +210,6 @@ CREATE TABLE automation_run_steps ( wait_hours: 48, email_subject: null, email_lexical: null, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: null }); insertActionRevision.run({ @@ -228,9 +219,6 @@ CREATE TABLE automation_run_steps ( wait_hours: null, email_subject: 'Welcome!', email_lexical: fakeLexical, - email_sender_name: fakeEmailSenderName, - email_sender_email: fakeEmailSenderEmail, - email_sender_reply_to: fakeEmailSenderReplyTo, email_design_setting_id: fakeEmailDesignSettingId }); insertActionRevision.run({ @@ -240,9 +228,6 @@ CREATE TABLE automation_run_steps ( wait_hours: 72, email_subject: null, email_lexical: null, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: null }); insertActionRevision.run({ @@ -252,9 +237,6 @@ CREATE TABLE automation_run_steps ( wait_hours: null, email_subject: 'Follow up', email_lexical: fakeLexical, - email_sender_name: fakeEmailSenderName, - email_sender_email: fakeEmailSenderEmail, - email_sender_reply_to: fakeEmailSenderReplyTo, email_design_setting_id: fakeEmailDesignSettingId }); insertActionRevision.run({ @@ -264,9 +246,6 @@ CREATE TABLE automation_run_steps ( wait_hours: 48, email_subject: null, email_lexical: null, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: null }); insertActionRevision.run({ @@ -276,9 +255,6 @@ CREATE TABLE automation_run_steps ( wait_hours: null, email_subject: 'Welcome to Paid!', email_lexical: fakeLexical, - email_sender_name: fakeEmailSenderName, - email_sender_email: fakeEmailSenderEmail, - email_sender_reply_to: fakeEmailSenderReplyTo, email_design_setting_id: fakeEmailDesignSettingId }); insertActionRevision.run({ @@ -288,9 +264,6 @@ CREATE TABLE automation_run_steps ( wait_hours: 72, email_subject: null, email_lexical: null, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: null }); insertActionRevision.run({ @@ -300,9 +273,6 @@ CREATE TABLE automation_run_steps ( wait_hours: null, email_subject: 'Exclusive Insights', email_lexical: fakeLexical, - email_sender_name: fakeEmailSenderName, - email_sender_email: fakeEmailSenderEmail, - email_sender_reply_to: fakeEmailSenderReplyTo, email_design_setting_id: fakeEmailDesignSettingId }); diff --git a/ghost/core/core/server/services/member-welcome-emails/service.js b/ghost/core/core/server/services/member-welcome-emails/service.js index ec2b07f91cb..11eec0b3643 100644 --- a/ghost/core/core/server/services/member-welcome-emails/service.js +++ b/ghost/core/core/server/services/member-welcome-emails/service.js @@ -27,6 +27,14 @@ const EMAIL_VALIDATION_TYPE_BY_FIELD = { */ const trimValue = value => value?.trim() || ''; +const getSenderDetailsFromDesignSettings = (designSettingsJson) => { + return { + senderName: designSettingsJson?.sender_name, + senderEmail: designSettingsJson?.sender_email, + senderReplyTo: designSettingsJson?.sender_reply_to + }; +}; + class MemberWelcomeEmailService { #mailer; #renderer; @@ -363,9 +371,11 @@ class MemberWelcomeEmailService { subject: email.get('subject'), status: row.get('status'), designSettings: designSettings?.id ? designSettings.toJSON() : null, - senderName: email.get('sender_name'), - senderEmail: email.get('sender_email'), - senderReplyTo: email.get('sender_reply_to') + senderDetails: { + senderName: email.get('sender_name'), + senderEmail: email.get('sender_email'), + senderReplyTo: email.get('sender_reply_to') + } }; } } @@ -381,9 +391,10 @@ class MemberWelcomeEmailService { * @param {string} options.email.lexical * @param {string} options.email.subject * @param {null | object} options.email.designSettings - * @param {undefined | null | string} options.email.senderName - * @param {undefined | null | string} options.email.senderEmail - * @param {undefined | null | string} options.email.senderReplyTo + * @param {object} options.email.senderDetails + * @param {undefined | null | string} options.email.senderDetails.senderName + * @param {undefined | null | string} options.email.senderDetails.senderEmail + * @param {undefined | null | string} options.email.senderDetails.senderReplyTo * @param {'welcome' | 'automation'} options.emailType * @returns {Promise} */ @@ -414,7 +425,7 @@ class MemberWelcomeEmailService { siteSettings: this.#getSiteSettings() }); - const senderOptions = await this.#getEffectiveSenderOptions(email); + const senderOptions = await this.#getEffectiveSenderOptions(email.senderDetails); await this.#mailer.send({ to: member.email, @@ -457,9 +468,6 @@ class MemberWelcomeEmailService { * @param {object} options.email * @param {null | string} options.email.designSettingId * @param {string} options.email.lexical - * @param {null | string} options.email.senderEmail - * @param {null | string} options.email.senderName - * @param {null | string} options.email.senderReplyTo * @param {string} options.email.subject * @param {object} options.member * @param {string} options.member.email @@ -472,6 +480,7 @@ class MemberWelcomeEmailService { const designSettings = email.designSettingId ? await EmailDesignSetting.findOne({id: email.designSettingId}) : null; + const designSettingsJson = designSettings?.id ? designSettings.toJSON() : null; await this.#sendEmail({ member, @@ -480,10 +489,8 @@ class MemberWelcomeEmailService { email: { lexical: email.lexical, subject: email.subject, - designSettings: designSettings?.id ? designSettings.toJSON() : null, - senderName: email.senderName, - senderEmail: email.senderEmail, - senderReplyTo: email.senderReplyTo + designSettings: designSettingsJson, + senderDetails: getSenderDetailsFromDesignSettings(designSettingsJson) } }); } diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/automations.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/automations.test.js.snap index e3797683e5e..bc9d216e963 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/automations.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/automations.test.js.snap @@ -135,9 +135,6 @@ Object { "data": Object { "email_design_setting_id": StringMatching /\\[a-f0-9\\]\\{24\\}/, "email_lexical": "{\\"root\\":{\\"children\\":[{\\"type\\":\\"paragraph\\",\\"children\\":[{\\"type\\":\\"text\\",\\"text\\":\\"Lorem ipsum.\\"}]}],\\"direction\\":null,\\"format\\":\\"\\",\\"indent\\":0,\\"type\\":\\"root\\",\\"version\\":1}}", - "email_sender_email": "hello@example.com", - "email_sender_name": "Ghost", - "email_sender_reply_to": "support@example.com", "email_subject": "Welcome!", }, "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, @@ -154,9 +151,6 @@ Object { "data": Object { "email_design_setting_id": StringMatching /\\[a-f0-9\\]\\{24\\}/, "email_lexical": "{\\"root\\":{\\"children\\":[{\\"type\\":\\"paragraph\\",\\"children\\":[{\\"type\\":\\"text\\",\\"text\\":\\"Lorem ipsum.\\"}]}],\\"direction\\":null,\\"format\\":\\"\\",\\"indent\\":0,\\"type\\":\\"root\\",\\"version\\":1}}", - "email_sender_email": "hello@example.com", - "email_sender_name": "Ghost", - "email_sender_reply_to": "support@example.com", "email_subject": "Follow up", }, "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, @@ -192,7 +186,7 @@ exports[`Automations API read returns the automation, ordered actions, and edges Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "1605", + "content-length": "1375", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, diff --git a/ghost/core/test/e2e-api/admin/automations.test.js b/ghost/core/test/e2e-api/admin/automations.test.js index 03c8c17ddb5..f65bec3fefb 100644 --- a/ghost/core/test/e2e-api/admin/automations.test.js +++ b/ghost/core/test/e2e-api/admin/automations.test.js @@ -76,9 +76,6 @@ const buildSendEmailAction = (dataOverrides = {}) => ({ data: { email_subject: 'Welcome', email_lexical: NON_EMPTY_EMAIL_LEXICAL, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: '64b6f7b7c8f1a2b3c4d5e6f7', ...dataOverrides } @@ -190,9 +187,6 @@ describe('Automations API', function () { data: { email_subject: 'Hello from the editor', email_lexical: emailLexical, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: '64b6f7b7c8f1a2b3c4d5e6f7' } }], @@ -559,9 +553,6 @@ describe('Automations API', function () { data: { email_subject: 'Changed type', email_lexical: JSON.stringify({root: {children: [], direction: null, format: '', indent: 0, type: 'root', version: 1}}), - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: '64b6f7b7c8f1a2b3c4d5e6f7' } } : { diff --git a/ghost/core/test/e2e-api/members/automations.test.js b/ghost/core/test/e2e-api/members/automations.test.js index b3835722141..b708954f1b1 100644 --- a/ghost/core/test/e2e-api/members/automations.test.js +++ b/ghost/core/test/e2e-api/members/automations.test.js @@ -3,6 +3,7 @@ const sinon = require('sinon'); const DomainEvents = require('@tryghost/domain-events'); const {agentProvider, fixtureManager, mockManager} = require('../../utils/e2e-framework'); const models = require('../../../core/server/models'); +const db = require('../../../core/server/data/db'); const automationsApi = require('../../../core/server/services/automations/automations-api'); const adapterManager = require('../../../core/server/services/adapter-manager'); const mailService = require('../../../core/server/services/mail'); @@ -11,6 +12,7 @@ const {getSignedAdminToken} = require('../../../core/server/adapters/scheduling/ const {MEMBER_WELCOME_EMAIL_SLUGS} = require('../../../core/server/services/member-welcome-emails/constants'); const HOUR_MS = 60 * 60 * 1000; +const AUTOMATION_EMAIL_REPLY_TO = 'support@example.com'; let agent; let schedulerKey; @@ -44,6 +46,67 @@ async function runSchedulerPoll() { await DomainEvents.allSettled(); } +async function upsertEmailDesignSetting({id, senderReplyTo}) { + const currentTime = new Date(); + + await db.knex('email_design_settings') + .insert({ + id, + slug: `automation-test-email-design-${id}`, + background_color: 'light', + header_background_color: 'transparent', + show_header_icon: true, + show_header_title: true, + button_color: 'accent', + button_corners: 'rounded', + button_style: 'fill', + link_color: 'accent', + link_style: 'underline', + body_font_category: 'sans_serif', + title_font_category: 'sans_serif', + title_font_weight: 'bold', + image_corners: 'square', + show_badge: true, + sender_reply_to: senderReplyTo, + created_at: currentTime, + updated_at: currentTime + }) + .onConflict('id') + .merge({ + sender_reply_to: senderReplyTo, + updated_at: currentTime + }); +} + +async function updateAutomationEmailDesignSetting(automation, emailDesignSettingId) { + const actions = automation.actions.map((action) => { + if (action.type !== 'send_email') { + return action; + } + + return { + ...action, + data: { + ...action.data, + email_design_setting_id: emailDesignSettingId + } + }; + }); + + const {body} = await agent + .put(`automations/${automation.id}`) + .body({ + automations: [{ + status: automation.status, + actions, + edges: automation.edges + }] + }) + .expectStatus(200); + + return body.automations[0]; +} + describe('Members Automations', function () { before(async function () { agent = await agentProvider.getAdminAPIAgent(); @@ -66,10 +129,34 @@ describe('Members Automations', function () { await DomainEvents.allSettled(); sinon.restore(); mockManager.restore(); + await db.knex('email_design_settings') + .where('slug', 'like', 'automation-test-email-design-%') + .del(); automationsApi._resetTestDatabase(); }); it('runs every step in the free member signup automation', async function () { + let automation = await getFreeMemberSignupAutomation(); + assert.equal(automation.actions.length, 4); + + const sendEmailActions = automation.actions.filter(action => action.type === 'send_email'); + assert.equal(sendEmailActions.length, 2); + + const emailDesignSettingId = sendEmailActions[0].data.email_design_setting_id; + assert(emailDesignSettingId, 'Expected send email action to have an email design setting'); + await upsertEmailDesignSetting({ + id: emailDesignSettingId, + senderReplyTo: AUTOMATION_EMAIL_REPLY_TO + }); + + automation = await updateAutomationEmailDesignSetting(automation, emailDesignSettingId); + assert.deepEqual( + automation.actions + .filter(action => action.type === 'send_email') + .map(action => action.data.email_design_setting_id), + [emailDesignSettingId, emailDesignSettingId] + ); + const email = `automation-free-member-${Date.now()}@test.example`; const member = await membersService.api.members.create({ email, @@ -79,9 +166,6 @@ describe('Members Automations', function () { }); assert.equal(member.get('status'), 'free'); - const automation = await getFreeMemberSignupAutomation(); - assert.equal(automation.actions.length, 4); - await DomainEvents.allSettled(); const clock = sinon.useFakeTimers({now: new Date(), shouldAdvanceTime: true, shouldClearNativeTimers: true}); @@ -109,7 +193,7 @@ describe('Members Automations', function () { assert.match(sentEmails[0].html, /Welcome!/); assert.match(sentEmails[1].html, /Follow up/); assert.deepEqual(sentEmails.map(({forceTextContent}) => forceTextContent), [true, true]); - assert.deepEqual(sentEmails.map(({replyTo}) => replyTo), ['support@example.com', 'support@example.com']); + assert.deepEqual(sentEmails.map(({replyTo}) => replyTo), [AUTOMATION_EMAIL_REPLY_TO, AUTOMATION_EMAIL_REPLY_TO]); assert.deepEqual(sentEmails.map(({tags}) => tags), [ ['member-welcome-email'], ['member-welcome-email'] diff --git a/ghost/core/test/integration/services/member-welcome-emails.test.js b/ghost/core/test/integration/services/member-welcome-emails.test.js index 62d1541277a..7d39c32a6ab 100644 --- a/ghost/core/test/integration/services/member-welcome-emails.test.js +++ b/ghost/core/test/integration/services/member-welcome-emails.test.js @@ -6,6 +6,7 @@ const models = require('../../../core/server/models'); const {OUTBOX_STATUSES} = require('../../../core/server/models/outbox'); const db = require('../../../core/server/data/db'); const mailService = require('../../../core/server/services/mail'); +const settingsHelpers = require('../../../core/server/services/settings-helpers'); const {MEMBER_WELCOME_EMAIL_SLUGS} = require('../../../core/server/services/member-welcome-emails/constants'); const memberWelcomeEmailService = require('../../../core/server/services/member-welcome-emails/service'); const processOutbox = require('../../../core/server/services/outbox/jobs/lib/process-outbox'); @@ -120,6 +121,13 @@ describe('Member Welcome Emails Integration', function () { await db.knex('automated_email_recipients').del(); await db.knex('outbox').del(); await db.knex('members').del(); + await db.knex('email_design_settings') + .where('id', defaultEmailDesignSettingId) + .update({ + sender_name: null, + sender_email: null, + sender_reply_to: null + }); await db.knex('automations').where('slug', MEMBER_WELCOME_EMAIL_SLUGS.free).del(); await db.knex('automations').where('slug', MEMBER_WELCOME_EMAIL_SLUGS.paid).del(); }); @@ -209,11 +217,35 @@ describe('Member Welcome Emails Integration', function () { await jobService.awaitCompletion(JOB_NAME); } - async function getAutomatedEmailBySlug(slug) { - return db.knex('welcome_email_automated_emails') - .join('automations', 'welcome_email_automated_emails.welcome_email_automation_id', 'automations.id') - .where('automations.slug', slug) - .first('welcome_email_automated_emails.*'); + async function sendAutomationEmail() { + memberWelcomeEmailService.api = null; + memberWelcomeEmailService.init(); + + await memberWelcomeEmailService.api.sendAutomationEmail({ + email: { + designSettingId: defaultEmailDesignSettingId, + lexical: JSON.stringify({ + root: { + children: [{ + type: 'paragraph', + children: [{type: 'text', text: 'Hello'}] + }], + direction: null, + format: '', + indent: 0, + type: 'root', + version: 1 + } + }), + subject: 'Automation email' + }, + member: { + email: 'automation-member@example.com', + name: 'Automation Member', + uuid: '99999999-9999-4999-8999-999999999999' + }, + memberStatus: 'free' + }); } it('does not send email when template is inactive', async function () { @@ -412,25 +444,35 @@ describe('Member Welcome Emails Integration', function () { assert.ok(sendCall.args[0].from.includes(senderEmail)); }); - it('uses automated email sender overrides when configured', async function () { + it('uses welcome email sender details instead of email design sender details for member welcome emails', async function () { const defaultNewsletter = await models.Newsletter.getDefaultNewsletter(); await db.knex('newsletters') .where('id', defaultNewsletter.id) .update({ - sender_name: 'Newsletter Sender', - sender_email: 'newsletter@example.com', - sender_reply_to: 'newsletter-reply@example.com' + sender_name: 'Ignored Newsletter Sender', + sender_email: 'ignored-newsletter@example.com', + sender_reply_to: 'ignored-newsletter-reply@example.com' + }); + + await db.knex('email_design_settings') + .where('id', defaultEmailDesignSettingId) + .update({ + sender_name: 'Design Sender', + sender_email: 'design@example.com', + sender_reply_to: 'design-reply@example.com' }); - const automatedEmail = await getAutomatedEmailBySlug(MEMBER_WELCOME_EMAIL_SLUGS.free); + const welcomeEmailAutomation = await db.knex('automations') + .where('slug', MEMBER_WELCOME_EMAIL_SLUGS.free) + .first('id'); await db.knex('welcome_email_automated_emails') - .where('id', automatedEmail.id) + .where('welcome_email_automation_id', welcomeEmailAutomation.id) .update({ - sender_name: 'Automation Sender', - sender_email: 'automation@example.com', - sender_reply_to: 'automation-reply@example.com' + sender_name: 'Welcome Sender', + sender_email: 'welcome@example.com', + sender_reply_to: 'welcome-reply@example.com' }); await models.Outbox.add({ @@ -449,8 +491,122 @@ describe('Member Welcome Emails Integration', function () { sinon.assert.calledOnce(mailService.GhostMailer.prototype.send); const sendCall = mailService.GhostMailer.prototype.send.firstCall; - assert.ok(sendCall.args[0].from.includes('automation@example.com')); - assert.equal(sendCall.args[0].replyTo, 'automation-reply@example.com'); + assert.equal(sendCall.args[0].from, '"Welcome Sender" '); + assert.equal(sendCall.args[0].replyTo, 'welcome-reply@example.com'); + }); + + it('uses newsletter sender details for member welcome emails when welcome email sender details are empty', async function () { + const defaultNewsletter = await models.Newsletter.getDefaultNewsletter(); + + await db.knex('newsletters') + .where('id', defaultNewsletter.id) + .update({ + sender_name: 'Newsletter Sender', + sender_email: 'newsletter@example.com', + sender_reply_to: 'newsletter-reply@example.com' + }); + + await db.knex('email_design_settings') + .where('id', defaultEmailDesignSettingId) + .update({ + sender_name: 'Design Sender', + sender_email: 'design@example.com', + sender_reply_to: 'design-reply@example.com' + }); + + await models.Outbox.add({ + event_type: 'MemberCreatedEvent', + payload: JSON.stringify({ + memberId: ObjectId().toHexString(), + uuid: '77777777-7777-4777-8777-777777777777', + email: 'legacy-design-sender-test@example.com', + name: 'Legacy Design Sender Test', + status: 'free' + }), + status: OUTBOX_STATUSES.PENDING + }); + + await scheduleInlineJob(); + + sinon.assert.calledOnceWithExactly(mailService.GhostMailer.prototype.send, sinon.match({ + from: '"Newsletter Sender" ', + replyTo: 'newsletter-reply@example.com' + })); + }); + + it('uses newsletter sender details for automation emails', async function () { + const defaultNewsletter = await models.Newsletter.getDefaultNewsletter(); + + await db.knex('newsletters') + .where('id', defaultNewsletter.id) + .update({ + sender_name: 'Newsletter Sender', + sender_email: 'newsletter@example.com', + sender_reply_to: 'newsletter-reply@example.com' + }); + + await sendAutomationEmail(); + + sinon.assert.calledOnceWithExactly(mailService.GhostMailer.prototype.send, sinon.match({ + from: '"Newsletter Sender" ', + replyTo: 'newsletter-reply@example.com' + })); + }); + + it('uses email design sender details for automation emails', async function () { + const defaultNewsletter = await models.Newsletter.getDefaultNewsletter(); + + await db.knex('newsletters') + .where('id', defaultNewsletter.id) + .update({ + sender_name: 'Ignored Newsletter Sender', + sender_email: 'ignored@example.com', + sender_reply_to: 'ignored-reply@example.com' + }); + + await db.knex('email_design_settings') + .where('id', defaultEmailDesignSettingId) + .update({ + sender_name: 'Design Sender', + sender_email: 'design@example.com', + sender_reply_to: 'design-reply@example.com' + }); + + await sendAutomationEmail(); + + sinon.assert.calledOnceWithExactly(mailService.GhostMailer.prototype.send, sinon.match({ + from: '"Design Sender" ', + replyTo: 'design-reply@example.com' + })); + }); + + it('falls back to site sender defaults for automation emails when newsletter sender fields are missing', async function () { + const defaultNewsletter = await models.Newsletter.getDefaultNewsletter(); + await db.knex('newsletters') + .where('id', defaultNewsletter.id) + .update({ + sender_name: null, + sender_email: null, + sender_reply_to: 'newsletter' + }); + + await sendAutomationEmail(); + + sinon.assert.calledOnceWithExactly(mailService.GhostMailer.prototype.send, sinon.match({ + from: settingsHelpers.getDefaultEmail().address, + replyTo: undefined + })); + }); + + it('falls back to site sender defaults for automation emails when no default newsletter exists', async function () { + sinon.stub(models.Newsletter, 'getDefaultNewsletter').resolves(null); + + await sendAutomationEmail(); + + sinon.assert.calledOnceWithExactly(mailService.GhostMailer.prototype.send, sinon.match({ + from: settingsHelpers.getDefaultEmail().address, + replyTo: undefined + })); }); it('uses mock member UUID when sending test welcome emails', async function () { @@ -489,20 +645,27 @@ describe('Member Welcome Emails Integration', function () { assert(!sendCall.args[0].html.includes('%7Buuid%7D')); }); - it('uses automated sender overrides for test welcome emails', async function () { + it('uses welcome email sender details instead of email design sender details for test welcome emails', async function () { memberWelcomeEmailService.init(); + await db.knex('email_design_settings') + .where('id', defaultEmailDesignSettingId) + .update({ + sender_name: 'Design Sender', + sender_email: 'design@example.com', + sender_reply_to: 'design-reply@example.com' + }); + const automation = await db.knex('automations') .where('slug', MEMBER_WELCOME_EMAIL_SLUGS.free) .first(); - const automatedEmail = await getAutomatedEmailBySlug(MEMBER_WELCOME_EMAIL_SLUGS.free); await db.knex('welcome_email_automated_emails') - .where('id', automatedEmail.id) + .where('welcome_email_automation_id', automation.id) .update({ - sender_name: 'Automation Sender', - sender_email: 'automation@example.com', - sender_reply_to: 'automation-reply@example.com' + sender_name: 'Welcome Sender', + sender_email: 'welcome@example.com', + sender_reply_to: 'welcome-reply@example.com' }); await memberWelcomeEmailService.api.sendTestEmail({ @@ -526,8 +689,8 @@ describe('Member Welcome Emails Integration', function () { sinon.assert.calledOnce(mailService.GhostMailer.prototype.send); const sendCall = mailService.GhostMailer.prototype.send.firstCall; - assert.ok(sendCall.args[0].from.includes('automation@example.com')); - assert.equal(sendCall.args[0].replyTo, 'automation-reply@example.com'); + assert.equal(sendCall.args[0].from, '"Welcome Sender" '); + assert.equal(sendCall.args[0].replyTo, 'welcome-reply@example.com'); }); }); diff --git a/ghost/core/test/unit/server/services/automations/automations-api.test.js b/ghost/core/test/unit/server/services/automations/automations-api.test.js index e915c02907c..857bf20e487 100644 --- a/ghost/core/test/unit/server/services/automations/automations-api.test.js +++ b/ghost/core/test/unit/server/services/automations/automations-api.test.js @@ -17,9 +17,6 @@ const buildSendEmailAction = (dataOverrides = {}) => ({ data: { email_subject: 'Welcome', email_lexical: NON_EMPTY_EMAIL_LEXICAL, - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: '64b6f7b7c8f1a2b3c4d5e6f7', ...dataOverrides } diff --git a/ghost/core/test/unit/server/services/automations/poll.test.ts b/ghost/core/test/unit/server/services/automations/poll.test.ts index 0be34d0ec28..7790a0b535a 100644 --- a/ghost/core/test/unit/server/services/automations/poll.test.ts +++ b/ghost/core/test/unit/server/services/automations/poll.test.ts @@ -17,9 +17,6 @@ type StepSpecificField = | 'wait_hours' | 'email_subject' | 'email_lexical' - | 'email_sender_name' - | 'email_sender_email' - | 'email_sender_reply_to' | 'email_design_setting_id'; type StepBase = Omit; type PollOptions = Parameters[0]; @@ -105,9 +102,6 @@ function buildEmailStep(attrs: Partial = {}): SendEmailStep { type: 'send_email', email_subject: 'Welcome!', email_lexical: JSON.stringify({root: {children: [], direction: null, format: '', indent: 0, type: 'root', version: 1}}), - email_sender_name: null, - email_sender_email: null, - email_sender_reply_to: null, email_design_setting_id: null, ...attrs } satisfies SendEmailStep; @@ -275,12 +269,7 @@ describe('automations poll', function () { it('sends email revision content and enqueues the next step', async function () { const nextReadyAt = new Date(Date.now() + 60 * 1000); - const step = buildEmailStep({ - email_design_setting_id: 'design-id', - email_sender_email: 'sender@example.com', - email_sender_name: 'Sender', - email_sender_reply_to: 'reply@example.com' - }); + const step = buildEmailStep({email_design_setting_id: 'design-id'}); automationsApi.fetchAndLockSteps.resolves({steps: [step], nextStepReadyAt: null}); automationsApi.finishStepAndEnqueueNext.resolves(nextReadyAt); @@ -291,9 +280,6 @@ describe('automations poll', function () { email: { designSettingId: 'design-id', lexical: step.email_lexical, - senderEmail: 'sender@example.com', - senderName: 'Sender', - senderReplyTo: 'reply@example.com', subject: 'Welcome!' }, memberStatus: 'free'