From 011ff0e93aec3cf52ff29cb3fb7703d376de53a2 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:30:33 +0100 Subject: [PATCH] feat: warn before sending a message with unfilled placeholders --- .../__tests__/placeholder-warning.test.tsx | 332 ++++++++++++++++++ components/email/email-composer.tsx | 138 ++++++++ locales/en/common.json | 9 + stores/settings-store.ts | 5 + 4 files changed, 484 insertions(+) create mode 100644 components/email/__tests__/placeholder-warning.test.tsx diff --git a/components/email/__tests__/placeholder-warning.test.tsx b/components/email/__tests__/placeholder-warning.test.tsx new file mode 100644 index 000000000..0cda741ea --- /dev/null +++ b/components/email/__tests__/placeholder-warning.test.tsx @@ -0,0 +1,332 @@ +import { render, screen, fireEvent, waitFor, within } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { EmailComposer } from '../email-composer'; +import { useSettingsStore } from '@/stores/settings-store'; + +// ─── Heavy component mocks (mirrors empty-subject-warning.test.tsx) ────────── + +vi.mock('@/components/email/rich-text-editor', () => ({ + RichTextEditor: () => React.createElement('div', { 'data-testid': 'rich-text-editor' }), +})); + +vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null })); +vi.mock('@/components/identity/sub-address-helper', () => ({ SubAddressHelper: () => null })); +vi.mock('@/components/templates/template-picker', () => ({ TemplatePicker: () => null })); +vi.mock('@/components/templates/template-form', () => ({ TemplateForm: () => null })); +vi.mock('@/components/files/file-preview-modal', () => ({ FilePreviewModal: () => null })); +vi.mock('@/hooks/use-focus-trap', () => ({ + useFocusTrap: () => ({ current: null }), +})); +vi.mock('@/hooks/use-pro-multi-account-identities', () => ({ + useProMultiAccountIdentities: () => ({ enabled: false, groups: [], allIdentities: [] }), + stripCrossAccountIdentityPrefix: (id: string) => ({ localAccountId: null, rawId: id }), +})); + +// ─── Store mocks ────────────────────────────────────────────────────────────── + +vi.mock('@/stores/auth-store', () => { + const state = { + client: null, + identities: [], + primaryIdentity: null, + isAuthenticated: false, + isDemoMode: false, + activeAccountId: null, + connectionLost: false, + getClientForAccount: () => undefined, + getAllConnectedClients: () => new Map(), + syncIdentities: () => {}, + refreshIdentities: async () => {}, + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useAuthStore: hook }; +}); + +vi.mock('@/stores/identity-store', () => { + const state = { + identities: [{ id: 'id-me', email: 'me@example.com', name: 'Me' }], + defaultIdentityId: 'id-me', + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useIdentityStore: hook }; +}); + +vi.mock('@/stores/account-store', () => { + const state = { accounts: [], getAccountById: () => undefined }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useAccountStore: hook }; +}); + +vi.mock('@/stores/email-store', () => { + const state = { + draftSaveEnabled: false, + sendRawEmail: async () => ({ sent: true }), + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useEmailStore: hook }; +}); + +const updateSetting = vi.fn(); + +vi.mock('@/stores/settings-store', () => { + const state = { + timeFormat: '24h', + plainTextMode: false, + subAddressDelimiter: '+', + autoSelectReplyIdentity: true, + attachmentReminderEnabled: false, + attachmentReminderKeywords: [], + emptySubjectWarningEnabled: true, + placeholderWarningEnabled: true, + sendDelaySeconds: 0, + signaturePosition: 'above_quote', + signatureSeparatorEnabled: false, + requestReadReceiptDefault: false, + addTrustedSender: () => {}, + trustedSendersAddressBook: null, + updateSetting: (...args: unknown[]) => updateSetting(...args), + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useSettingsStore: hook }; +}); + +vi.mock('@/stores/contact-store', () => { + const state = { + contacts: [], + getAutocomplete: async () => [], + addToTrustedSendersBook: async () => {}, + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useContactStore: hook }; +}); + +vi.mock('@/stores/template-store', () => { + const state = { templates: [], addTemplate: async () => {} }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useTemplateStore: hook }; +}); + +// ─── Misc dependency mocks ──────────────────────────────────────────────────── + +vi.mock('@/stores/toast-store', () => ({ + toast: { info: () => {}, error: () => {}, success: () => {} }, +})); + +vi.mock('@/lib/plugin-hooks', () => ({ + identityHooks: { + onSignatureRender: { size: 0, transform: async (value: unknown) => value }, + onIdentitySelect: { size: 0, emit: async () => {} }, + }, + templateHooks: { + onTemplateRender: { size: 0, transform: async (value: unknown) => value }, + onTemplateApply: { size: 0, emit: async () => {} }, + }, + emailHooks: { + onComposerOpen: { call: async () => [] }, + onRecipientChange: { call: async () => [] }, + getRecipientSuggestions: { call: async () => [] }, + onRecipientChipsChange: { transform: async (chips: unknown) => chips }, + onDraftChange: { emit: () => {} }, + onBeforeDraftAutoSave: { transform: async (draft: unknown) => draft }, + onBeforeEmailSend: { intercept: async () => true, interceptOutcome: async () => 'allowed' }, + onComposeSend: { takeOver: async () => 'default' }, + onTransformOutgoingEmail: { transform: async (email: unknown) => email }, + }, + contactHooks: { + search: { call: async () => [] }, + onProvideRecipientSuggestions: { transform: async (initial: unknown) => initial }, + }, +})); + +vi.mock('@/lib/email-sanitization', () => ({ + sanitizeSignatureHtml: (v: string) => v, + sanitizeEmailHtml: (v: string) => v, + parseHtmlSafely: (html: string) => new DOMParser().parseFromString(html, 'text/html'), +})); + +vi.mock('@/lib/email-threading', () => ({ + computeReplyThreadingHeaders: () => ({ inReplyTo: [], references: [] }), +})); +vi.mock('@/lib/signature-utils', () => ({ + appendPlainTextSignature: (body: string) => body, + getPlainTextSignature: () => '', + plainTextBodyWithoutSignature: (body: string) => body, +})); +vi.mock('@/lib/sub-addressing', () => ({ generateSubAddress: () => '' })); +vi.mock('@/lib/debug', () => ({ debug: { log: () => {}, warn: () => {}, error: () => {} } })); +vi.mock('@/components/email/quoted-html', () => ({ + buildQuotedHtmlBlock: () => '', + serializeEditorContent: () => '', +})); +vi.mock('@/lib/template-utils', () => ({ substitutePlaceholders: (s: string) => s })); + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +/** A ready-to-send draft whose body still has a fill-in. */ +const DRAFT_WITH_PLACEHOLDER = { + to: 'bob@example.com', + cc: '', + bcc: '', + subject: 'Lunch', + body: '

See you at [TIME] in TODO.

', + showCc: false, + showBcc: false, + selectedIdentityId: 'id-me', + subAddressTag: '', + mode: 'compose' as const, + draftId: null, +}; + +const sendButton = () => screen.getAllByTestId('composer-send')[0] as HTMLButtonElement; + +describe('composer placeholder warning', () => { + beforeEach(() => { + updateSetting.mockClear(); + }); + + afterEach(() => { + useSettingsStore.setState({ + placeholderWarningEnabled: true, + attachmentReminderEnabled: false, + attachmentReminderKeywords: [], + } as never); + }); + + it('asks before sending and lists the placeholders', async () => { + const onSend = vi.fn(); + render(); + + fireEvent.click(sendButton()); + + const dialog = await screen.findByRole('alertdialog'); + expect(within(dialog).getByText('placeholder_warning.title')).toBeInTheDocument(); + const found = within(dialog).getAllByRole('listitem').map((li) => li.textContent); + expect(found).toEqual(['[TIME]', 'TODO']); + expect(onSend).not.toHaveBeenCalled(); + }); + + it('sends once confirmed', async () => { + const onSend = vi.fn(); + render(); + + fireEvent.click(sendButton()); + fireEvent.click(await screen.findByText('placeholder_warning.send_anyway')); + + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + expect(updateSetting).not.toHaveBeenCalled(); + }); + + it('turns the warning off when "don\'t ask again" is checked', async () => { + const onSend = vi.fn(); + render(); + + fireEvent.click(sendButton()); + fireEvent.click(await screen.findByText('placeholder_warning.dont_ask_again')); + fireEvent.click(screen.getByText('placeholder_warning.send_anyway')); + + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + expect(updateSetting).toHaveBeenCalledWith('placeholderWarningEnabled', false); + }); + + it('goes back to editing with the placeholder selected', async () => { + const onSend = vi.fn(); + render( + Hello

' }} + onSend={onSend} + /> + ); + + fireEvent.click(sendButton()); + fireEvent.click(await screen.findByText('placeholder_warning.back')); + + await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + const subject = screen.getByDisplayValue('Hi {{name}}') as HTMLInputElement; + await waitFor(() => expect(document.activeElement).toBe(subject)); + expect([subject.selectionStart, subject.selectionEnd]).toEqual([3, 11]); + expect(onSend).not.toHaveBeenCalled(); + }); + + it('does not ask again after Send anyway and a trip back to add an attachment', async () => { + useSettingsStore.setState({ attachmentReminderEnabled: true, attachmentReminderKeywords: ['see you'] } as never); + const onSend = vi.fn(); + render(); + + fireEvent.click(sendButton()); + fireEvent.click(await screen.findByText('placeholder_warning.send_anyway')); + // The body says "See you", so the attachment reminder stops the send. + fireEvent.click(await screen.findByText('forgot_attachment.back')); + await waitFor(() => expect(screen.queryByText('forgot_attachment.title')).not.toBeInTheDocument()); + + fireEvent.click(sendButton()); + + expect(await screen.findByText('forgot_attachment.title')).toBeInTheDocument(); + expect(screen.queryByText('placeholder_warning.title')).not.toBeInTheDocument(); + expect(onSend).not.toHaveBeenCalled(); + }); + + it('asks again when a new placeholder shows up', async () => { + useSettingsStore.setState({ attachmentReminderEnabled: true, attachmentReminderKeywords: ['see you'] } as never); + const onSend = vi.fn(); + render(); + + fireEvent.click(sendButton()); + fireEvent.click(await screen.findByText('placeholder_warning.send_anyway')); + fireEvent.click(await screen.findByText('forgot_attachment.back')); + await waitFor(() => expect(screen.queryByText('forgot_attachment.title')).not.toBeInTheDocument()); + + fireEvent.change(screen.getByDisplayValue('Lunch'), { target: { value: 'Lunch with {{name}}' } }); + fireEvent.click(sendButton()); + + const dialog = await screen.findByRole('alertdialog'); + expect(within(dialog).getByText('placeholder_warning.title')).toBeInTheDocument(); + expect(within(dialog).getByText('{{name}}')).toBeInTheDocument(); + expect(onSend).not.toHaveBeenCalled(); + }); + + it('ignores placeholders in quoted text and in the signature', async () => { + const onSend = vi.fn(); + const body = '

Thanks!

' + + '
[COMPANY]
' + + '

Meet at [TIME]? TODO

'; + render(); + + fireEvent.click(sendButton()); + + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + expect(screen.queryByText('placeholder_warning.title')).not.toBeInTheDocument(); + }); + + it('sends without asking when the warning is off', async () => { + useSettingsStore.setState({ placeholderWarningEnabled: false } as never); + const onSend = vi.fn(); + render(); + + fireEvent.click(sendButton()); + + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + expect(screen.queryByText('placeholder_warning.title')).not.toBeInTheDocument(); + }); +}); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 81495cf59..1b538ec45 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -89,6 +89,7 @@ import { usePluginSignature } from "@/hooks/use-plugin-signature"; import { SubAddressHelper } from "@/components/identity/sub-address-helper"; import { generateSubAddress } from "@/lib/sub-addressing"; import { substitutePlaceholders, spliceTemplateAboveSignature, composeBodyHasUserContent } from "@/lib/template-utils"; +import { findUnfilledPlaceholders, stripEmbeddedSignatureHtml } from "@/lib/placeholder-check"; import { TemplatePicker } from "@/components/templates/template-picker"; import { TemplateForm } from "@/components/templates/template-form"; import type { EmailTemplate } from "@/lib/template-types"; @@ -478,6 +479,7 @@ export function EmailComposer({ const attachmentReminderEnabled = useSettingsStore((state) => state.attachmentReminderEnabled); const attachmentReminderKeywords = useSettingsStore((state) => state.attachmentReminderKeywords); const emptySubjectWarningEnabled = useSettingsStore((state) => state.emptySubjectWarningEnabled); + const placeholderWarningEnabled = useSettingsStore((state) => state.placeholderWarningEnabled); const recipientMentionsEnabled = useSettingsStore((state) => state.recipientMentionsEnabled); const mentionAddsRecipientTo = useSettingsStore((state) => state.mentionAddsRecipientTo); const updateSetting = useSettingsStore((state) => state.updateSetting); @@ -966,6 +968,10 @@ export function EmailComposer({ const [showEmptySubjectWarning, setShowEmptySubjectWarning] = useState(false); const [emptySubjectDelayedUntil, setEmptySubjectDelayedUntil] = useState(); const [emptySubjectDontAskAgain, setEmptySubjectDontAskAgain] = useState(false); + const [showPlaceholderWarning, setShowPlaceholderWarning] = useState(false); + const [placeholderWarningMatches, setPlaceholderWarningMatches] = useState([]); + const [placeholderWarningDelayedUntil, setPlaceholderWarningDelayedUntil] = useState(); + const [placeholderDontAskAgain, setPlaceholderDontAskAgain] = useState(false); const [showScheduleDialog, setShowScheduleDialog] = useState(false); const [scheduleValue, setScheduleValue] = useState(''); const [scheduleError, setScheduleError] = useState(''); @@ -997,6 +1003,12 @@ export function EmailComposer({ restoreFocus: true, }); + const placeholderWarningRef = useFocusTrap({ + isActive: showPlaceholderWarning, + onEscape: () => setShowPlaceholderWarning(false), + restoreFocus: true, + }); + const { client: activeClient } = useAuthStore(); const composerOwnClientRef = useRef(activeClient); if (onComposerAccount && activeClient) composerOwnClientRef.current = activeClient; @@ -3093,6 +3105,9 @@ export function EmailComposer({ const sendCancelledRef = useRef(false); // The user confirmed sending without a subject for this message. const emptySubjectConfirmedRef = useRef(false); + // Placeholders the user chose to send as they are. The warning comes back + // only for one that is not in here. + const confirmedPlaceholdersRef = useRef>(new Set()); // Toasts sit in the bottom corner, right over the composer's Send button. // The attribute lifts them clear while a composer is open (see toast.tsx). @@ -3107,6 +3122,41 @@ export function EmailComposer({ }; }, []); + // "Back to editing" from the placeholder warning selects the first + // placeholder, so the user can type over it. + const selectPlaceholder = (placeholder: string) => { + const subjectInput = subjectInputRef.current; + const subjectIndex = subjectInput?.value.indexOf(placeholder) ?? -1; + if (subjectInput && subjectIndex >= 0) { + subjectInput.focus(); + subjectInput.setSelectionRange(subjectIndex, subjectIndex + placeholder.length); + return; + } + if (plainTextModeRef.current) { + const textarea = bodyRef.current; + if (!textarea) return; + const index = textarea.value.indexOf(placeholder); + textarea.focus(); + if (index >= 0) textarea.setSelectionRange(index, index + placeholder.length); + return; + } + const editor = editorRef.current; + if (!editor) return; + const range = { from: -1, to: -1 }; + editor.state.doc.descendants((node, pos) => { + if (range.from >= 0) return false; + const index = node.isText && node.text ? node.text.indexOf(placeholder) : -1; + if (index >= 0) { + range.from = pos + index; + range.to = range.from + placeholder.length; + } + }); + // A placeholder split across formatting has no single text node; then + // the caret just goes back into the body. + if (range.from >= 0) editor.chain().focus().setTextSelection(range).scrollIntoView().run(); + else editor.commands.focus(); + }; + const handleSend = async ({ skipAttachmentCheck = false, skipSubjectCheck = false, delayedUntil }: { skipAttachmentCheck?: boolean; skipSubjectCheck?: boolean; @@ -3176,6 +3226,27 @@ export function EmailComposer({ return; } + // Unfilled placeholders: template fields left as {{name}}, fill-ins like + // [TIME] from AI drafts, and markers like TODO. The signature is left + // out, or a placeholder in it would stop every message. + if (placeholderWarningEnabled) { + const authored = plainTextMode + ? plainTextBodyWithoutSignature(body, signatureIdentity) + : stripEmbeddedSignatureHtml(body); + const bodyText = extractUserAuthoredText(authored, { + plainTextMode, + forwardedSeparator: tQuote('forwarded_separator'), + }); + const found = findUnfilledPlaceholders(subject, bodyText); + if (found.some(p => !confirmedPlaceholdersRef.current.has(p))) { + setPlaceholderWarningMatches(found); + setPlaceholderDontAskAgain(false); + setPlaceholderWarningDelayedUntil(delayedUntil); + setShowPlaceholderWarning(true); + return; + } + } + // Attachment reminder check if (!skipAttachmentCheck && attachmentReminderEnabled) { const hasAttachments = attachmentsRef.current.some(att => att.blobId && !att.uploading && !att.error); @@ -3688,6 +3759,7 @@ export function EmailComposer({ showScheduleDialog || showAttachmentWarning || showEmptySubjectWarning || + showPlaceholderWarning || showCloseDialog ) return; @@ -4824,6 +4896,72 @@ export function EmailComposer({ )} + {showPlaceholderWarning && ( +
setShowPlaceholderWarning(false)} + > +
e.stopPropagation()} + className="bg-background border border-border rounded-lg shadow-xl w-full max-w-md animate-in zoom-in-95 duration-200" + > +
+

{t('placeholder_warning.title')}

+

{t('placeholder_warning.message')}

+
    + {placeholderWarningMatches.slice(0, 5).map((p) => ( +
  • {p}
    • + ))} + {placeholderWarningMatches.length > 5 && ( +
  • + {t('placeholder_warning.more', { count: placeholderWarningMatches.length - 5 })} +
    • + )} +
+ +
+
+ + +
+
+
+ )} + {showCloseDialog && (
()( attachmentReminderEnabled: state.attachmentReminderEnabled, attachmentReminderKeywords: state.attachmentReminderKeywords, emptySubjectWarningEnabled: state.emptySubjectWarningEnabled, + placeholderWarningEnabled: state.placeholderWarningEnabled, recipientMentionsEnabled: state.recipientMentionsEnabled, mentionAddsRecipientTo: state.mentionAddsRecipientTo, hideInlineImageAttachments: state.hideInlineImageAttachments,