From ff70ffec67979db65475477b417d089d0c8c9fb8 Mon Sep 17 00:00:00 2001 From: Sag Date: Wed, 22 Nov 2023 23:07:14 -0300 Subject: [PATCH] Updated newsletter settings UI for managed email (#19082) refs GRO-59 refs GRO-56 refs GRO-52 - When email is managed without a custom domain, do not allow the Sender Email address to be changed, but allow Reply-to address to be changed to any address the publisher can verify - When email is managed with a custom domain, allow both Sender and Reply-to addresses to be changed without verification, but not their domain names --------- Co-authored-by: Djordje Vlaisavljevic --- apps/admin-x-framework/src/api/config.ts | 22 +- .../newsletters/NewsletterDetailModal.tsx | 198 +++++++++++--- .../email/newsletters/NewsletterPreview.tsx | 13 +- .../test/acceptance/email/newsletters.test.ts | 246 ++++++++++++++---- 4 files changed, 385 insertions(+), 94 deletions(-) diff --git a/apps/admin-x-framework/src/api/config.ts b/apps/admin-x-framework/src/api/config.ts index e07c0c2db5..58b1155c70 100644 --- a/apps/admin-x-framework/src/api/config.ts +++ b/apps/admin-x-framework/src/api/config.ts @@ -48,7 +48,11 @@ export type Config = { pintura?: { js?: string css?: string - } + }, + managedEmail?: { + enabled?: boolean + sendingDomain?: string + }, } // Config is relatively fluid, so we only type used properties above and still support arbitrary property access when needed @@ -67,3 +71,19 @@ export const useBrowseConfig = createQuery({ dataType, path: '/config/' }); + +// Helpers + +export const isManagedEmail = (config: Config) => { + return !!config?.hostSettings?.managedEmail?.enabled; +}; + +export const hasSendingDomain = (config: Config) => { + const isDomain = /[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}/; + const sendingDomain = config?.hostSettings?.managedEmail?.sendingDomain; + return typeof sendingDomain === 'string' && isDomain.test(sendingDomain); +}; + +export const sendingDomain = (config: Config) => { + return config?.hostSettings?.managedEmail?.sendingDomain; +}; diff --git a/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx b/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx index 9faee5d6c4..f3c0493f4c 100644 --- a/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx +++ b/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx @@ -4,17 +4,33 @@ import React, {useEffect, useState} from 'react'; import useFeatureFlag from '../../../../hooks/useFeatureFlag'; import useSettingGroup from '../../../../hooks/useSettingGroup'; import validator from 'validator'; -import {Button, ButtonGroup, ColorPickerField, ConfirmationModal, Form, Heading, Hint, HtmlField, Icon, ImageUpload, LimitModal, PreviewModalContent, Select, SelectOption, Separator, Tab, TabView, TextArea, TextField, Toggle, ToggleGroup, showToast} from '@tryghost/admin-x-design-system'; +import {Button, ButtonGroup, ColorPickerField, ConfirmationModal, Form, Heading, Hint, HtmlField, Icon, ImageUpload, LimitModal, PreviewModalContent, Select, SelectOption, Separator, SettingGroupContent, Tab, TabView, TextArea, TextField, Toggle, ToggleGroup, showToast} from '@tryghost/admin-x-design-system'; import {ErrorMessages, useForm, useHandleError} from '@tryghost/admin-x-framework/hooks'; import {HostLimitError, useLimiter} from '../../../../hooks/useLimiter'; import {Newsletter, useBrowseNewsletters, useEditNewsletter} from '@tryghost/admin-x-framework/api/newsletters'; import {RoutingModalProps, useRouting} from '@tryghost/admin-x-framework/routing'; +import {SiteData} from '@tryghost/admin-x-framework/api/site'; import {fullEmailAddress} from '@tryghost/admin-x-framework/api/site'; import {getImageUrl, useUploadImage} from '@tryghost/admin-x-framework/api/images'; import {getSettingValues} from '@tryghost/admin-x-framework/api/settings'; +import {hasSendingDomain, isManagedEmail, sendingDomain} from '@tryghost/admin-x-framework/api/config'; import {textColorForBackgroundColor} from '@tryghost/color-utils'; import {useGlobalData} from '../../../providers/GlobalDataProvider'; +const renderReplyToEmail = (newsletter: Newsletter, siteData: SiteData, membersSupportAddress?: string) => { + if (!newsletter.sender_reply_to) { + return ''; + } + + if (newsletter.sender_reply_to === 'newsletter') { + return fullEmailAddress(newsletter.sender_email || 'noreply', siteData); + } else if (newsletter.sender_reply_to === 'support') { + return fullEmailAddress(membersSupportAddress || 'noreply', siteData); + } else { + return newsletter.sender_reply_to; + } +}; + const Sidebar: React.FC<{ newsletter: Newsletter; onlyOne: boolean; @@ -25,7 +41,7 @@ const Sidebar: React.FC<{ }> = ({newsletter, onlyOne, updateNewsletter, validate, errors, clearError}) => { const {mutateAsync: editNewsletter} = useEditNewsletter(); const limiter = useLimiter(); - const {settings, siteData} = useGlobalData(); + const {settings, siteData, config} = useGlobalData(); const [membersSupportAddress, icon] = getSettingValues(settings, ['members_support_address', 'icon']); const {mutateAsync: uploadImage} = useUploadImage(); const [selectedTab, setSelectedTab] = useState('generalSettings'); @@ -34,9 +50,12 @@ const Sidebar: React.FC<{ const [siteTitle] = getSettingValues(localSettings, ['title']) as string[]; const handleError = useHandleError(); + const newsletterAddress = fullEmailAddress(newsletter.sender_email || 'noreply', siteData); + const supportAddress = fullEmailAddress(membersSupportAddress || 'noreply', siteData); + const replyToEmails = [ - {label: `Newsletter address (${fullEmailAddress(newsletter.sender_email || 'noreply', siteData)})`, value: 'newsletter'}, - {label: `Support address (${fullEmailAddress(membersSupportAddress || 'noreply', siteData)})`, value: 'support'} + {label: `Newsletter address (${newsletterAddress})`, value: 'newsletter'}, + {label: `Support address (${supportAddress})`, value: 'support'} ]; const fontOptions: SelectOption[] = [ @@ -109,6 +128,103 @@ const Sidebar: React.FC<{ } }; + const renderSenderEmailField = () => { + if (isManagedEmail(config)) { + if (hasSendingDomain(config)) { + const sendingEmailUsername = newsletter.sender_email?.split('@')[0]; + + return ( + { + const username = e.target.value?.split('@')[0]; + const newEmail = username ? `${username}@${sendingDomain(config)}` : ''; + updateNewsletter({sender_email: newEmail}); + }} + onKeyDown={() => clearError('sender_email')} + /> + ); + } else { + return ( + To customise, set up a custom sending domain + } + ]} + /> + ); + } + } + + return ( + updateNewsletter({sender_email: e.target.value})} + onKeyDown={() => clearError('sender_email')} + /> + ); + }; + + const renderReplyToEmailField = () => { + if (isManagedEmail(config)) { + if (hasSendingDomain(config)) { + const replyToEmailUsername = ['newsletter', 'support'].includes(newsletter.sender_reply_to) ? '' : newsletter.sender_reply_to?.split('@')[0]; + return ( + { + const username = e.target.value?.split('@')[0]; + const newEmail = username ? `${username}@${sendingDomain(config)}` : ''; + updateNewsletter({sender_reply_to: newEmail}); + }} + onKeyDown={() => clearError('sender_reply_to')} + /> + ); + } else { + return ( + updateNewsletter({sender_reply_to: e.target.value})} + onKeyDown={() => clearError('sender_reply_to')} + /> + ); + } + } + + return ( + option.value === newsletter.sender_reply_to)} - title="Reply-to email" - onSelect={option => updateNewsletter({sender_reply_to: option?.value})} - /> + {renderSenderEmailField()} + {renderReplyToEmailField()}
= ({newsletter, onlyOne}) => { const modal = useModal(); - const {siteData} = useGlobalData(); + const {siteData, settings, config} = useGlobalData(); const {mutateAsync: editNewsletter} = useEditNewsletter(); const {updateRoute} = useRouting(); const handleError = useHandleError(); + const [membersSupportAddress] = getSettingValues(settings, ['members_support_address', 'icon']); const {formState, saveState, updateForm, setFormState, handleSave, validate, errors, clearError, okProps} = useForm({ initialState: newsletter, @@ -423,21 +526,40 @@ const NewsletterDetailModalContent: React.FC<{newsletter: Newsletter; onlyOne: b onSave: async () => { const {newsletters, meta} = await editNewsletter(formState); if (meta?.sent_email_verification) { - NiceModal.show(ConfirmationModal, { - title: 'Confirm newsletter email address', - prompt: <> - We‘ve sent a confirmation email to {formState.sender_email}. - Until the address has been verified newsletters will be sent from the - {newsletters[0].sender_email ? ' previous' : ' default'} email address - ({fullEmailAddress(newsletters[0].sender_email || 'noreply', siteData)}). - , - cancelLabel: '', - onOk: (confirmModal) => { - confirmModal?.remove(); - modal.remove(); - updateRoute('newsletters'); - } - }); + if (meta?.sent_email_verification[0] === 'sender_email') { + NiceModal.show(ConfirmationModal, { + title: 'Confirm newsletter email address', + prompt: <> + We‘ve sent a confirmation email to {formState.sender_email}. + Until the address has been verified newsletters will be sent from the + {newsletters[0].sender_email ? ' previous' : ' default'} email address + ({fullEmailAddress(newsletters[0].sender_email || 'noreply', siteData)}). + , + cancelLabel: '', + onOk: (confirmModal) => { + confirmModal?.remove(); + modal.remove(); + updateRoute('newsletters'); + } + }); + } else if (meta?.sent_email_verification[0] === 'sender_reply_to') { + const previousReplyTo = renderReplyToEmail(newsletters[0], siteData, membersSupportAddress); + + NiceModal.show(ConfirmationModal, { + title: 'Confirm reply-to address', + prompt: <> + We‘ve sent a confirmation email to {formState.sender_reply_to}. + Until the address has been verified, newsletters will use the previous reply-to address + {previousReplyTo ? ` (${previousReplyTo})` : ''}. + , + cancelLabel: '', + onOk: (confirmModal) => { + confirmModal?.remove(); + modal.remove(); + updateRoute('newsletters'); + } + }); + } } }, onSaveError: handleError, @@ -452,6 +574,10 @@ const NewsletterDetailModalContent: React.FC<{newsletter: Newsletter; onlyOne: b newErrors.sender_email = 'Invalid email.'; } + if (isManagedEmail(config) && formState.sender_reply_to && (!validator.isEmail(formState.sender_reply_to))) { + newErrors.sender_reply_to = 'Invalid email.'; + } + return newErrors; } }); diff --git a/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterPreview.tsx b/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterPreview.tsx index a4cf9cb2d3..d2cd901572 100644 --- a/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterPreview.tsx +++ b/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterPreview.tsx @@ -4,6 +4,7 @@ import useFeatureFlag from '../../../../hooks/useFeatureFlag'; import {Newsletter} from '@tryghost/admin-x-framework/api/newsletters'; import {fullEmailAddress} from '@tryghost/admin-x-framework/api/site'; import {getSettingValues} from '@tryghost/admin-x-framework/api/settings'; +import {hasSendingDomain, isManagedEmail, sendingDomain} from '@tryghost/admin-x-framework/api/config'; import {textColorForBackgroundColor} from '@tryghost/color-utils'; import {useGlobalData} from '../../../providers/GlobalDataProvider'; @@ -91,6 +92,16 @@ const NewsletterPreview: React.FC<{newsletter: Newsletter}> = ({newsletter}) => secondaryTextColor } : {}; + const renderSenderEmail = () => { + if (isManagedEmail(config)) { + if (hasSendingDomain(config)) { + return newsletter.sender_email || 'noreply@' + sendingDomain(config); + } + } + + return fullEmailAddress(newsletter.sender_email || 'noreply', siteData); + }; + return = ({newsletter}) => headerImage={newsletter.header_image} headerSubtitle={headerSubtitle} headerTitle={headerTitle} - senderEmail={fullEmailAddress(newsletter.sender_email || 'noreply', siteData)} + senderEmail={renderSenderEmail()} senderName={newsletter.sender_name || title} showBadge={newsletter.show_badge} showCommentCta={showCommentCta} diff --git a/apps/admin-x-settings/test/acceptance/email/newsletters.test.ts b/apps/admin-x-settings/test/acceptance/email/newsletters.test.ts index c32cd808f7..4dbd3316aa 100644 --- a/apps/admin-x-settings/test/acceptance/email/newsletters.test.ts +++ b/apps/admin-x-settings/test/acceptance/email/newsletters.test.ts @@ -91,80 +91,214 @@ test.describe('Newsletter settings', async () => { }); }); - test('Displays a prompt when email verification is required', 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_email'] - } - }} - }}); + test.describe('Email addresses', async () => { + test.describe('For self-hosters', async () => { + test('Displays a prompt when email verification is required', 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_email'] + } + }} + }}); - await page.goto('/'); + await page.goto('/'); - const section = page.getByTestId('newsletters'); + const section = page.getByTestId('newsletters'); - await section.getByText('Awesome newsletter').click(); + await section.getByText('Awesome newsletter').click(); - const modal = page.getByTestId('newsletter-modal'); + const modal = page.getByTestId('newsletter-modal'); - await modal.getByLabel('Sender email').fill('not-an-email'); - await modal.getByRole('button', {name: 'Save'}).click(); + await modal.getByLabel('Sender email').fill('not-an-email'); + await modal.getByRole('button', {name: 'Save'}).click(); - await expect(page.getByTestId('toast-error')).toHaveText(/Can't save newsletter/); - await expect(modal).toHaveText(/Invalid email/); + await expect(page.getByTestId('toast-error')).toHaveText(/Can't save newsletter/); + await expect(modal).toHaveText(/Invalid email/); - await modal.getByLabel('Sender email').fill('test@test.com'); - await modal.getByRole('button', {name: 'Save'}).click(); + await modal.getByLabel('Sender email').fill('test@test.com'); + await modal.getByRole('button', {name: 'Save'}).click(); - await expect(page.getByTestId('confirmation-modal')).toHaveCount(1); - await expect(page.getByTestId('confirmation-modal')).toHaveText(/Confirm newsletter email address/); - await expect(page.getByTestId('confirmation-modal')).toHaveText(/default email address \(noreply@test.com\)/); - }); + await expect(page.getByTestId('confirmation-modal')).toHaveCount(1); + await expect(page.getByTestId('confirmation-modal')).toHaveText(/Confirm newsletter email address/); + await expect(page.getByTestId('confirmation-modal')).toHaveText(/default email address \(noreply@test.com\)/); + }); - test('Displays the current email when changing sender address', async ({page}) => { - const response = { - ...responseFixtures.newsletters, - newsletters: [{ - ...responseFixtures.newsletters.newsletters[0], - sender_email: 'current@test.com' - }] - } satisfies NewslettersResponseType; + test('Displays the current email when changing sender address', async ({page}) => { + const response = { + ...responseFixtures.newsletters, + newsletters: [{ + ...responseFixtures.newsletters.newsletters[0], + sender_email: 'current@test.com' + }] + } satisfies NewslettersResponseType; - await mockApi({page, requests: { - ...globalDataRequests, - browseNewsletters: {method: 'GET', path: '/newsletters/?include=count.active_members%2Ccount.posts&limit=50', response}, - editNewsletter: {method: 'PUT', path: `/newsletters/${responseFixtures.newsletters.newsletters[0].id}/?include=count.active_members%2Ccount.posts`, response: { - newsletters: response.newsletters, - meta: { - sent_email_verification: ['sender_email'] - } - }} - }}); + await mockApi({page, requests: { + ...globalDataRequests, + browseNewsletters: {method: 'GET', path: '/newsletters/?include=count.active_members%2Ccount.posts&limit=50', response}, + editNewsletter: {method: 'PUT', path: `/newsletters/${responseFixtures.newsletters.newsletters[0].id}/?include=count.active_members%2Ccount.posts`, response: { + newsletters: response.newsletters, + meta: { + sent_email_verification: ['sender_email'] + } + }} + }}); - await page.goto('/'); + await page.goto('/'); - const section = page.getByTestId('newsletters'); + const section = page.getByTestId('newsletters'); - await section.getByText('Awesome newsletter').click(); + await section.getByText('Awesome newsletter').click(); - const modal = page.getByTestId('newsletter-modal'); + const modal = page.getByTestId('newsletter-modal'); - await modal.getByLabel('Sender email').fill('not-an-email'); - await modal.getByRole('button', {name: 'Save'}).click(); + await modal.getByLabel('Sender email').fill('not-an-email'); + await modal.getByRole('button', {name: 'Save'}).click(); - await expect(page.getByTestId('toast-error')).toHaveText(/Can't save newsletter/); - await expect(modal).toHaveText(/Invalid email/); + await expect(page.getByTestId('toast-error')).toHaveText(/Can't save newsletter/); + await expect(modal).toHaveText(/Invalid email/); - await modal.getByLabel('Sender email').fill('test@test.com'); - await modal.getByRole('button', {name: 'Save'}).click(); + await modal.getByLabel('Sender email').fill('test@test.com'); + await modal.getByRole('button', {name: 'Save'}).click(); - await expect(page.getByTestId('confirmation-modal')).toHaveCount(1); - await expect(page.getByTestId('confirmation-modal')).toHaveText(/Confirm newsletter email address/); - await expect(page.getByTestId('confirmation-modal')).toHaveText(/previous email address \(current@test.com\)/); + await expect(page.getByTestId('confirmation-modal')).toHaveCount(1); + await expect(page.getByTestId('confirmation-modal')).toHaveText(/Confirm newsletter email address/); + await expect(page.getByTestId('confirmation-modal')).toHaveText(/previous email address \(current@test.com\)/); + }); + }); + + test.describe('For Ghost (Pro) users without custom domain', () => { + test('Does not allow the Sender email address to be edited', async ({page}) => { + await mockApi({page, requests: { + ...globalDataRequests, + browseNewsletters: {method: 'GET', path: '/newsletters/?include=count.active_members%2Ccount.posts&limit=50', response: responseFixtures.newsletters}, + browseConfig: { + ...globalDataRequests.browseConfig, + response: { + config: { + ...responseFixtures.config.config, + hostSettings: { + managedEmail: { + enabled: true + } + } + } + } + } + }}); + + await page.goto('/'); + const section = page.getByTestId('newsletters'); + await section.getByText('Awesome newsletter').click(); + const modal = page.getByTestId('newsletter-modal'); + const senderEmailField = modal.getByLabel('Sender email'); + + // Test that there is no input field near "Sender email" + const parentElementLocator = senderEmailField.locator('xpath=..'); + const inputElementsNearby = await parentElementLocator.locator('input').count(); + + expect(inputElementsNearby).toBe(0); + }); + + test('Allow full customisation of the reply-to address', 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 + } + } + } + } + } + }}); + + await page.goto('/'); + const section = page.getByTestId('newsletters'); + await section.getByText('Awesome newsletter').click(); + const modal = page.getByTestId('newsletter-modal'); + const replyToEmail = modal.getByLabel('Reply-to email'); + + await replyToEmail.fill('not-an-email'); + await modal.getByRole('button', {name: 'Save'}).click(); + + await expect(page.getByTestId('toast-error')).toHaveText(/Can't save newsletter/); + await expect(modal).toHaveText(/Invalid email/); + + await replyToEmail.fill('test@test.com'); + 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(/previous reply-to address \(noreply@test.com\)/); + }); + }); + + test.describe('For Ghost (Pro) users with custom domain', () => { + test('Allow sender and reply-to addresses to be changed without verification, but not their domain name', 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: [] + } + }}, + 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 address'); + + // 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'); + + // 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'); + + // 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('Supports archiving newsletters', async ({page}) => {