Skip to content

Commit af8ddfd

Browse files
cmraiblecmraible
committed
🐛 Fixed welcome emails using noreply address instead of default newsletter reply-to address (#26358)
ref https://linear.app/ghost/issue/NY-1028/welcome-email-uses-noreply-instead-of-configured-sender ## Problem Welcome emails aren't currently respecting the sender info of the default newsletter as intended. Instead, they are being sent with a fallback `sender_name` and `sender_reply_to` address: - For a site with custom sending domain, this is `noreply@example.com` - For a site without a custom sending domain, this is `subdomain@ghost.io` ## Fix Welcome emails should use the sender info from the default newsletter's settings, so that new members can reply directly to the welcome email and the publisher will receive that email. This fix gets the settings from the default newsletter, and passes this through to mailgun when sending welcome emails (real ones and test emails).
1 parent 9f6786c commit af8ddfd

File tree

3 files changed

+162
-5
lines changed

3 files changed

+162
-5
lines changed

‎ghost/core/core/server/services/member-welcome-emails/service.js‎

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,19 @@ const errors = require('@tryghost/errors');
33
const urlUtils = require('../../../shared/url-utils');
44
const settingsCache = require('../../../shared/settings-cache');
55
const emailAddressService = require('../email-address');
6+
const settingsHelpers = require('../settings-helpers');
7+
const EmailAddressParser = require('../email-address/email-address-parser');
68
const mail = require('../mail');
79
// @ts-expect-error type checker has trouble with the dynamic exporting in models
8-
const {AutomatedEmail} = require('../../models');
10+
const {AutomatedEmail, Newsletter} = require('../../models');
911
const MemberWelcomeEmailRenderer = require('./member-welcome-email-renderer');
1012
const {MEMBER_WELCOME_EMAIL_LOG_KEY, MEMBER_WELCOME_EMAIL_SLUGS, MESSAGES} = require('./constants');
1113

1214
class MemberWelcomeEmailService {
1315
#mailer;
1416
#renderer;
1517
#memberWelcomeEmails = {free: null, paid: null};
18+
#defaultNewsletterSenderOptions = null;
1619

1720
constructor() {
1821
emailAddressService.init();
@@ -28,7 +31,71 @@ class MemberWelcomeEmailService {
2831
};
2932
}
3033

34+
async #getDefaultNewsletterSenderOptions() {
35+
const newsletter = await Newsletter.getDefaultNewsletter();
36+
if (!newsletter) {
37+
return {};
38+
}
39+
40+
let senderName = settingsCache.get('title') || '';
41+
if (newsletter.get('sender_name')) {
42+
senderName = newsletter.get('sender_name');
43+
}
44+
45+
let fromAddress = settingsHelpers.getNoReplyAddress();
46+
if (newsletter.get('sender_email')) {
47+
fromAddress = newsletter.get('sender_email');
48+
}
49+
50+
const fromAddresses = emailAddressService.service.getAddress({
51+
from: {
52+
address: fromAddress,
53+
name: senderName || undefined
54+
}
55+
});
56+
57+
const from = EmailAddressParser.stringify(fromAddresses.from);
58+
const replyToSetting = newsletter.get('sender_reply_to');
59+
let replyTo = null;
60+
61+
if (replyToSetting === 'support') {
62+
replyTo = settingsHelpers.getMembersSupportAddress();
63+
} else if (replyToSetting === 'newsletter' && !emailAddressService.service.managedEmailEnabled) {
64+
replyTo = from;
65+
} else {
66+
const addresses = emailAddressService.service.getAddress({
67+
from: {
68+
address: fromAddress,
69+
name: senderName || undefined
70+
},
71+
replyTo: replyToSetting === 'newsletter' ? undefined : {address: replyToSetting}
72+
});
73+
74+
if (addresses.replyTo) {
75+
replyTo = EmailAddressParser.stringify(addresses.replyTo);
76+
}
77+
}
78+
79+
return {
80+
from,
81+
...(replyTo ? {
82+
replyTo
83+
} : {})
84+
};
85+
}
86+
87+
async #getSenderOptions() {
88+
if (this.#defaultNewsletterSenderOptions) {
89+
return this.#defaultNewsletterSenderOptions;
90+
}
91+
92+
this.#defaultNewsletterSenderOptions = await this.#getDefaultNewsletterSenderOptions();
93+
return this.#defaultNewsletterSenderOptions;
94+
}
95+
3196
async loadMemberWelcomeEmails() {
97+
this.#defaultNewsletterSenderOptions = await this.#getDefaultNewsletterSenderOptions();
98+
3299
for (const [memberStatus, slug] of Object.entries(MEMBER_WELCOME_EMAIL_SLUGS)) {
33100
const row = await AutomatedEmail.findOne({slug});
34101

@@ -82,12 +149,15 @@ class MemberWelcomeEmailService {
82149
siteSettings: this.#getSiteSettings()
83150
});
84151

152+
const senderOptions = await this.#getSenderOptions();
153+
85154
await this.#mailer.send({
86155
to: member.email,
87156
subject,
88157
html,
89158
text,
90-
forceTextContent: true
159+
forceTextContent: true,
160+
...senderOptions
91161
});
92162
}
93163

@@ -136,12 +206,16 @@ class MemberWelcomeEmailService {
136206
siteSettings: this.#getSiteSettings()
137207
});
138208

209+
// Test sends should always reflect the latest newsletter sender settings.
210+
const senderOptions = await this.#getDefaultNewsletterSenderOptions();
211+
139212
await this.#mailer.send({
140213
to: email,
141214
subject: `[Test] ${renderedSubject}`,
142215
html,
143216
text,
144-
forceTextContent: true
217+
forceTextContent: true,
218+
...senderOptions
145219
});
146220
}
147221
}
@@ -156,4 +230,3 @@ class MemberWelcomeEmailServiceWrapper {
156230
}
157231

158232
module.exports = new MemberWelcomeEmailServiceWrapper();
159-

‎ghost/core/test/e2e-api/admin/automated-emails.test.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,36 @@ describe('Automated Emails API', function () {
502502
});
503503
});
504504

505+
it('Uses configured sender and reply-to for test email', async function () {
506+
const senderEmail = 'editor@example.com';
507+
const senderReplyTo = 'reply@example.com';
508+
const {body: newslettersBody} = await agent.get('newsletters/?limit=1').expectStatus(200);
509+
const defaultNewsletterId = newslettersBody.newsletters[0].id;
510+
511+
await agent
512+
.put(`newsletters/${defaultNewsletterId}`)
513+
.body({newsletters: [{
514+
sender_email: senderEmail,
515+
sender_reply_to: senderReplyTo
516+
}]})
517+
.expectStatus(200);
518+
519+
await agent
520+
.post(`automated_emails/${automatedEmailId}/test/`)
521+
.body({
522+
email: 'test@ghost.org',
523+
subject: 'Test Subject',
524+
lexical: validLexical
525+
})
526+
.expectStatus(204);
527+
528+
sinon.assert.calledOnce(mailService.GhostMailer.prototype.send);
529+
sinon.assert.calledWithMatch(mailService.GhostMailer.prototype.send, {
530+
from: sinon.match(new RegExp(senderEmail)),
531+
replyTo: senderReplyTo
532+
});
533+
});
534+
505535
it('Cannot send test email for non-existent automated email', async function () {
506536
await agent
507537
.post('automated_emails/abcd1234abcd1234abcd1234/test/')

‎ghost/core/test/integration/services/member-welcome-emails.test.js‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,26 @@ const processOutbox = require('../../../core/server/services/outbox/jobs/lib/pro
1212

1313
describe('Member Welcome Emails Integration', function () {
1414
let membersService;
15+
let defaultNewsletterSenderState = null;
1516

1617
before(async function () {
1718
await testUtils.setup('default')();
1819
membersService = require('../../../core/server/services/members');
1920
});
2021

2122
beforeEach(async function () {
23+
const defaultNewsletter = await models.Newsletter.getDefaultNewsletter();
24+
if (defaultNewsletter) {
25+
defaultNewsletterSenderState = {
26+
id: defaultNewsletter.id,
27+
sender_name: defaultNewsletter.get('sender_name'),
28+
sender_email: defaultNewsletter.get('sender_email'),
29+
sender_reply_to: defaultNewsletter.get('sender_reply_to')
30+
};
31+
} else {
32+
defaultNewsletterSenderState = null;
33+
}
34+
2235
await db.knex('outbox').del();
2336
await db.knex('members').del();
2437

@@ -58,6 +71,16 @@ describe('Member Welcome Emails Integration', function () {
5871
});
5972

6073
afterEach(async function () {
74+
if (defaultNewsletterSenderState) {
75+
await db.knex('newsletters')
76+
.where('id', defaultNewsletterSenderState.id)
77+
.update({
78+
sender_name: defaultNewsletterSenderState.sender_name,
79+
sender_email: defaultNewsletterSenderState.sender_email,
80+
sender_reply_to: defaultNewsletterSenderState.sender_reply_to
81+
});
82+
}
83+
6184
await db.knex('automated_email_recipients').del();
6285
await db.knex('outbox').del();
6386
await db.knex('members').del();
@@ -349,6 +372,37 @@ describe('Member Welcome Emails Integration', function () {
349372
const sendCall = mailService.GhostMailer.prototype.send.firstCall;
350373
assert.equal(sendCall.args[0].to, memberEmail);
351374
});
375+
376+
it('uses configured sender and reply-to when sending member welcome email', async function () {
377+
const senderEmail = 'editor@example.com';
378+
const senderReplyTo = 'reply@example.com';
379+
const defaultNewsletter = await models.Newsletter.getDefaultNewsletter();
380+
381+
await db.knex('newsletters')
382+
.where('id', defaultNewsletter.id)
383+
.update({
384+
sender_name: 'Scott',
385+
sender_email: senderEmail,
386+
sender_reply_to: senderReplyTo
387+
});
388+
389+
await models.Outbox.add({
390+
event_type: 'MemberCreatedEvent',
391+
payload: JSON.stringify({
392+
memberId: ObjectId().toHexString(),
393+
email: 'sender-test@example.com',
394+
name: 'Sender Test',
395+
status: 'free'
396+
}),
397+
status: OUTBOX_STATUSES.PENDING
398+
});
399+
400+
await scheduleInlineJob();
401+
402+
assert.equal(mailService.GhostMailer.prototype.send.callCount, 1);
403+
const sendCall = mailService.GhostMailer.prototype.send.firstCall;
404+
assert.equal(sendCall.args[0].replyTo, senderReplyTo);
405+
assert.ok(sendCall.args[0].from.includes(senderEmail));
406+
});
352407
});
353408
});
354-

0 commit comments

Comments
 (0)