Updated design of the reply-to field for custom sending domains (#19515)

fixes PROD-205
fixes PROD-219

- removed the right placeholder complexity
- the preview renders the current sender / reply-to address in
real-time, regardless of whether it's valid
- if the reply-to address does not match the custom sending domain, show
an inline error
This commit is contained in:
Sag 2024-01-18 14:39:43 +01:00 committed by GitHub
parent f34999a51f
commit 0f17f9a937
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 19 additions and 30 deletions

View File

@ -197,28 +197,15 @@ const Sidebar: React.FC<{
// Pro users with custom sending domains // Pro users with custom sending domains
if (hasSendingDomain(config)) { if (hasSendingDomain(config)) {
let sendingEmail = newsletter.sender_email || ''; // Do not use the rendered address here, because this field is editable and we otherwise can't have an empty field
// It is possible we have an invalid saved email address, in that case it won't get used
// so we should display as if we are using the default = an empty address
if (sendingEmail && sendingEmail !== newsletterAddress) {
sendingEmail = '';
}
const sendingEmailUsername = sendingEmail?.split('@')[0] || '';
return ( return (
<TextField <TextField
error={Boolean(errors.sender_email)} error={Boolean(errors.sender_email)}
hint={errors.sender_email} hint={errors.sender_email}
placeholder={defaultEmailAddress} placeholder={defaultEmailAddress}
rightPlaceholder={sendingEmailUsername ? `@${sendingDomain(config)}` : `` }
title="Sender email address" title="Sender email address"
value={sendingEmailUsername || ''} value={newsletter.sender_email || ''}
onChange={(e) => { onChange={(e) => {
const username = e.target.value?.split('@')[0]; updateNewsletter({sender_email: e.target.value});
const newEmail = username ? `${username}@${sendingDomain(config)}` : '';
updateNewsletter({sender_email: newEmail});
}} }}
onKeyDown={() => clearError('sender_email')} onKeyDown={() => clearError('sender_email')}
/> />
@ -573,6 +560,8 @@ const NewsletterDetailModalContent: React.FC<{newsletter: Newsletter; onlyOne: b
if (formState.sender_email && !validator.isEmail(formState.sender_email)) { if (formState.sender_email && !validator.isEmail(formState.sender_email)) {
newErrors.sender_email = 'Invalid email'; newErrors.sender_email = 'Invalid email';
} else if (formState.sender_email && hasSendingDomain(config) && formState.sender_email.split('@')[1] !== sendingDomain(config)) {
newErrors.sender_email = `Email must end with @${sendingDomain(config)}`;
} }
if (formState.sender_reply_to && !validator.isEmail(formState.sender_reply_to) && !['newsletter', 'support'].includes(formState.sender_reply_to)) { if (formState.sender_reply_to && !validator.isEmail(formState.sender_reply_to) && !['newsletter', 'support'].includes(formState.sender_reply_to)) {

View File

@ -1,4 +1,4 @@
import {Config, hasSendingDomain, isManagedEmail, sendingDomain} from '@tryghost/admin-x-framework/api/config'; import {Config, hasSendingDomain, isManagedEmail} from '@tryghost/admin-x-framework/api/config';
import {Newsletter} from '@tryghost/admin-x-framework/api/newsletters'; import {Newsletter} from '@tryghost/admin-x-framework/api/newsletters';
export const renderSenderEmail = (newsletter: Newsletter, config: Config, defaultEmailAddress: string|undefined) => { export const renderSenderEmail = (newsletter: Newsletter, config: Config, defaultEmailAddress: string|undefined) => {
@ -7,15 +7,6 @@ export const renderSenderEmail = (newsletter: Newsletter, config: Config, defaul
return defaultEmailAddress; return defaultEmailAddress;
} }
if (isManagedEmail(config) && hasSendingDomain(config)) {
// Only return sender_email if the domain names match
if (newsletter.sender_email?.split('@')[1] === sendingDomain(config)) {
return newsletter.sender_email;
} else {
return defaultEmailAddress || '';
}
}
return newsletter.sender_email || defaultEmailAddress || ''; return newsletter.sender_email || defaultEmailAddress || '';
}; };

View File

@ -252,7 +252,7 @@ test.describe('Newsletter settings', async () => {
}); });
test.describe('For Ghost (Pro) users with custom sending domain', () => { test.describe('For Ghost (Pro) users with custom sending domain', () => {
test('Allow sender address to be changed partially (username but not domain name)', async ({page}) => { test('The sender email address can be changed partially (username but not domain name)', async ({page}) => {
await mockApi({page, requests: { await mockApi({page, requests: {
...globalDataRequests, ...globalDataRequests,
browseNewsletters: {method: 'GET', path: '/newsletters/?include=count.active_members%2Ccount.posts&limit=50', response: responseFixtures.newsletters}, browseNewsletters: {method: 'GET', path: '/newsletters/?include=count.active_members%2Ccount.posts&limit=50', response: responseFixtures.newsletters},
@ -284,11 +284,20 @@ test.describe('Newsletter settings', async () => {
const modal = page.getByTestId('newsletter-modal'); const modal = page.getByTestId('newsletter-modal');
const senderEmail = modal.getByLabel('Sender email'); const senderEmail = modal.getByLabel('Sender email');
// The sender email field should keep the username part of the email address // Error case #1: add invalid email address
await senderEmail.fill('harry@potter.com'); await senderEmail.fill('Harry Potter');
expect(await senderEmail.inputValue()).toBe('harry'); await modal.getByRole('button', {name: 'Save'}).click();
await expect(page.getByTestId('toast-error').first()).toHaveText(/Can't save newsletter/);
await expect(modal).toHaveText(/Invalid email/);
// The new username is saved without a confirmation popup // Error case #2: the sender email address doesn't match the custom sending domain
await senderEmail.fill('harry@potter.com');
await modal.getByRole('button', {name: 'Save'}).click();
await expect(page.getByTestId('toast-error').first()).toHaveText(/Can't save newsletter/);
await expect(modal).toHaveText(/Email must end with @customdomain.com/);
// But can have any address on the same domain, without verification
await senderEmail.fill('harry@customdomain.com');
await modal.getByRole('button', {name: 'Save'}).click(); await modal.getByRole('button', {name: 'Save'}).click();
await expect(page.getByTestId('confirmation-modal')).toHaveCount(0); await expect(page.getByTestId('confirmation-modal')).toHaveCount(0);
}); });