tqt/Ghost
2
1
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2023-12-13 21:00:40 +01:00
Code Issues 1 Releases Wiki Activity

Allowed custom/empty replyTo for newsletters with managed sending domain (#19183)

Browse source 
fixes GRO-75
fixes GRO-100

And allow them to be empty
This commit is contained in:
Simon Backx 2023-11-30 10:16:03 +01:00 committed by GitHub
parent 79081686b1
commit ab21b8ae1d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 90 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx
View file

@ -189,35 +189,13 @@ const Sidebar: React.FC<{
);
}
// Pro users with custom sending domains
if (hasSendingDomain(config)) {
const replyToEmail = ['newsletter', 'support'].includes(newsletter.sender_reply_to) ? '' : renderReplyToEmail(newsletter, config, supportEmailAddress, defaultEmailAddress);
const replyToEmailUsername = replyToEmail?.split('@')[0] || '';
return (
<TextField
error={Boolean(errors.sender_reply_to)}
hint={errors.sender_reply_to}
rightPlaceholder={`@${sendingDomain(config)}`}
title="Reply-to address"
value={replyToEmailUsername}
onBlur={validate}
onChange={(e) => {
const username = e.target.value?.split('@')[0];
const newEmail = username ? `${username}@${sendingDomain(config)}` : 'newsletter';
updateNewsletter({sender_reply_to: newEmail});
}}
onKeyDown={() => clearError('sender_reply_to')}
/>
);
}
const replyToRequired = !hasSendingDomain(config);
// Pro users without custom sending domains
return (
<TextField
error={Boolean(errors.sender_reply_to)}
hint={errors.sender_reply_to}
placeholder={newsletterAddress || ''}
placeholder={replyToRequired ? (newsletterAddress || '') : ''}
title="Reply-to email"
value={renderReplyToEmail(newsletter, config, supportEmailAddress, defaultEmailAddress) || ''}
onBlur={validate}

14
apps/admin-x-settings/src/utils/newsletterEmails.ts
View file

@ -21,6 +21,11 @@ export const renderSenderEmail = (newsletter: Newsletter, config: Config, defaul
export const renderReplyToEmail = (newsletter: Newsletter, config: Config, supportEmailAddress: string|undefined, defaultEmailAddress: string|undefined) => {
if (newsletter.sender_reply_to === 'newsletter') {
if (isManagedEmail(config) && hasSendingDomain(config)) {
// No reply-to set
// sender_reply_to currently doesn't allow empty values, we need to set it to 'newsletter'
return '';
}
return renderSenderEmail(newsletter, config, defaultEmailAddress);
}
@ -28,14 +33,5 @@ export const renderReplyToEmail = (newsletter: Newsletter, config: Config, suppo
return supportEmailAddress || defaultEmailAddress || '';
}
if (isManagedEmail(config) && hasSendingDomain(config)) {
// Only return sender_reply_to if the domain names match
if (newsletter.sender_reply_to.split('@')[1] === sendingDomain(config)) {
return newsletter.sender_reply_to;
} else {
return '';
}
}
return newsletter.sender_reply_to;
};

61
apps/admin-x-settings/test/acceptance/email/newsletters.test.ts
View file

@ -246,6 +246,7 @@ test.describe('Newsletter settings', async () => {
await expect(page.getByTestId('confirmation-modal')).toHaveCount(1);
await expect(page.getByTestId('confirmation-modal')).toHaveText(/Confirm reply-to address/);
await expect(page.getByTestId('confirmation-modal')).toHaveText(/sent a confirmation email to test@test.com/);
await expect(page.getByTestId('confirmation-modal')).toHaveText(/previous reply-to address \(support@example.com\)/);
});
});
@ -282,7 +283,7 @@ test.describe('Newsletter settings', async () => {
await section.getByText('Awesome newsletter').click();
const modal = page.getByTestId('newsletter-modal');
const senderEmail = modal.getByLabel('Sender email');
const replyToEmail = modal.getByLabel('Reply-to address');
const replyToEmail = modal.getByLabel('Reply-to email');
// The sending domain is rendered as placeholder text
expect(modal).toHaveText(/@customdomain\.com/);
@ -291,14 +292,66 @@ test.describe('Newsletter settings', async () => {
await senderEmail.fill('harry@potter.com');
expect(await senderEmail.inputValue()).toBe('harry');
// The sender email field should keep the username part of the email address
await replyToEmail.fill('hermione@granger.com');
expect(await replyToEmail.inputValue()).toBe('hermione');
// Full flexibility for the reply-to address
await replyToEmail.fill('hermione@customdomain.com');
expect(await replyToEmail.inputValue()).toBe('hermione@customdomain.com');
// The new username is saved without a confirmation popup
await modal.getByRole('button', {name: 'Save'}).click();
await expect(page.getByTestId('confirmation-modal')).toHaveCount(0);
});
test('Reply-To addresses without a matching domain require verification', async ({page}) => {
await mockApi({page, requests: {
...globalDataRequests,
browseNewsletters: {method: 'GET', path: '/newsletters/?include=count.active_members%2Ccount.posts&limit=50', response: responseFixtures.newsletters},
editNewsletter: {method: 'PUT', path: `/newsletters/${responseFixtures.newsletters.newsletters[0].id}/?include=count.active_members%2Ccount.posts`, response: {
newsletters: [responseFixtures.newsletters.newsletters[0]],
meta: {
sent_email_verification: ['sender_reply_to']
}
}},
browseConfig: {
...globalDataRequests.browseConfig,
response: {
config: {
...responseFixtures.config.config,
hostSettings: {
managedEmail: {
enabled: true,
sendingDomain: 'customdomain.com'
}
}
}
}
}
}});
await page.goto('/');
const section = page.getByTestId('newsletters');
await section.getByText('Awesome newsletter').click();
const modal = page.getByTestId('newsletter-modal');
const senderEmail = modal.getByLabel('Sender email');
const replyToEmail = modal.getByLabel('Reply-to email');
// The sending domain is rendered as placeholder text
expect(modal).toHaveText(/@customdomain\.com/);
// The sender email field should keep the username part of the email address
await senderEmail.fill('harry@potter.com');
expect(await senderEmail.inputValue()).toBe('harry');
// Full flexibility for the reply-to address
await replyToEmail.fill('hermione@granger.com');
expect(await replyToEmail.inputValue()).toBe('hermione@granger.com');
// The new username is saved without a confirmation popup
await modal.getByRole('button', {name: 'Save'}).click();
await expect(page.getByTestId('confirmation-modal')).toHaveCount(1);
await expect(page.getByTestId('confirmation-modal')).toHaveText(/Confirm reply-to address/);
await expect(page.getByTestId('confirmation-modal')).toHaveText(/sent a confirmation email to hermione@granger.com/);
await expect(page.getByTestId('confirmation-modal')).toHaveText(/previous reply-to address \(support@example.com\)/);
});
});
});

10
ghost/core/test/integration/services/q-email-addresses.test.js
View file

@ -269,7 +269,7 @@ describe('Email addresses', function () {
sender_reply_to: 'newsletter'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Anything Possible" <default@sendingdomain.com>', '"Anything Possible" <default@sendingdomain.com>');
await assertFromAddressNewsletter('"Anything Possible" <default@sendingdomain.com>');
});
it('[NEWSLETTER] Does allow to send a newsletter from a custom sending domain', async function () {
@ -279,7 +279,7 @@ describe('Email addresses', function () {
sender_reply_to: 'newsletter'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Anything Possible" <anything@sendingdomain.com>', '"Anything Possible" <anything@sendingdomain.com>');
await assertFromAddressNewsletter('"Anything Possible" <anything@sendingdomain.com>');
});
it('[NEWSLETTER] Does allow to set the replyTo address to any address', async function () {
@ -309,7 +309,7 @@ describe('Email addresses', function () {
sender_reply_to: 'newsletter'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Example Site" <default@sendingdomain.com>', '"Example Site" <default@sendingdomain.com>');
await assertFromAddressNewsletter('"Example Site" <default@sendingdomain.com>');
});
});
@ -365,7 +365,7 @@ describe('Email addresses', function () {
sender_reply_to: 'newsletter'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Anything Possible" <default@sendingdomain.com>', '"Anything Possible" <default@sendingdomain.com>');
await assertFromAddressNewsletter('"Anything Possible" <default@sendingdomain.com>');
});
it('[NEWSLETTER] Does allow to set the replyTo address to any address', async function () {
@ -395,7 +395,7 @@ describe('Email addresses', function () {
sender_reply_to: 'newsletter'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Example Site" <default@sendingdomain.com>', '"Example Site" <default@sendingdomain.com>');
await assertFromAddressNewsletter('"Example Site" <default@sendingdomain.com>');
});
});

4
ghost/email-service/lib/EmailRenderer.js
View file

@ -215,6 +215,10 @@ class EmailRenderer {
return this.#settingsHelpers.getMembersSupportAddress();
}
if (newsletter.get('sender_reply_to') === 'newsletter') {
if (this.#emailAddressService.managedEmailEnabled) {
// Don't duplicate the same replyTo addres if it already in the FROM address
return null;
}
return this.getFromAddress(post, newsletter);
}

20
ghost/email-service/test/email-renderer.test.js
View file

@ -731,7 +731,8 @@ describe('Email renderer', function () {
let emailAddressService = {
getAddress(addresses) {
return addresses;
}
},
managedEmailEnabled: true
};
let emailRenderer = new EmailRenderer({
settingsCache: {
@ -765,14 +766,27 @@ describe('Email renderer', function () {
response.should.equal('support@example.com');
});
it('returns correct reply to address for newsletter', function () {
it('[legacy] returns correct reply to address for newsletter', function () {
emailAddressService.managedEmailEnabled = false;
const newsletter = createModel({
sender_email: 'ghost@example.com',
sender_name: 'Ghost',
sender_reply_to: 'newsletter'
});
const response = emailRenderer.getReplyToAddress({}, newsletter);
response.should.equal(`"Ghost" <ghost@example.com>`);
assert.equal(response, `"Ghost" <ghost@example.com>`);
emailAddressService.managedEmailEnabled = true;
});
it('returns null when set to newsletter', function () {
emailAddressService.managedEmailEnabled = true;
const newsletter = createModel({
sender_email: 'ghost@example.com',
sender_name: 'Ghost',
sender_reply_to: 'newsletter'
});
const response = emailRenderer.getReplyToAddress({}, newsletter);
assert.equal(response, null);
});
it('returns correct custom reply to address', function () {