mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 08:13:50 +00:00
fix: split typed addresses on commas and stop sends to invalid addresses
This commit is contained in:
@@ -0,0 +1,260 @@
|
||||
import { render, screen, fireEvent, waitFor, act, within } from '@testing-library/react';
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import React from 'react';
|
||||
import { EmailComposer } from '../email-composer';
|
||||
|
||||
// ─── Heavy component mocks (mirrors reply-addressing.test.tsx) ────────────────
|
||||
|
||||
const editorKeyDown = vi.hoisted(() => vi.fn());
|
||||
const toastError = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock('@/components/email/rich-text-editor', () => ({
|
||||
RichTextEditor: () => React.createElement('div', {
|
||||
'data-testid': 'rich-text-editor',
|
||||
onKeyDown: (event: React.KeyboardEvent) => {
|
||||
if (!event.defaultPrevented) editorKeyDown();
|
||||
},
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null }));
|
||||
vi.mock('@/components/ui/avatar', () => ({ Avatar: () => 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<typeof state>) => 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<typeof state>) => 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<typeof state>) => 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<typeof state>) => 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,
|
||||
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<typeof state>) => 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<typeof state>) => 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<typeof state>) => Object.assign(state, p);
|
||||
return { useTemplateStore: hook };
|
||||
});
|
||||
|
||||
// ─── Misc dependency mocks ────────────────────────────────────────────────────
|
||||
|
||||
vi.mock('@/stores/toast-store', () => ({
|
||||
toast: { info: () => {}, error: toastError, success: () => {} },
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/plugin-hooks', () => ({
|
||||
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 },
|
||||
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: () => '',
|
||||
}));
|
||||
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 ────────────────────────────────────────────────────────────────────
|
||||
|
||||
const DRAFT = {
|
||||
to: '',
|
||||
cc: '',
|
||||
bcc: '',
|
||||
subject: 'Lunch',
|
||||
body: '<p>Hello there</p>',
|
||||
showCc: false,
|
||||
showBcc: false,
|
||||
selectedIdentityId: 'id-me',
|
||||
subAddressTag: '',
|
||||
mode: 'compose' as const,
|
||||
draftId: null,
|
||||
};
|
||||
|
||||
const toInput = () => screen.getByPlaceholderText('to_placeholder') as HTMLInputElement;
|
||||
const sendButton = () => screen.getAllByTestId('composer-send')[0] as HTMLButtonElement;
|
||||
|
||||
describe('composer recipient validation', () => {
|
||||
beforeEach(() => {
|
||||
toastError.mockClear();
|
||||
updateSetting.mockClear();
|
||||
editorKeyDown.mockClear();
|
||||
});
|
||||
|
||||
it('adds a typed address when a comma follows it', () => {
|
||||
render(<EmailComposer initialData={DRAFT} />);
|
||||
const input = toInput();
|
||||
fireEvent.change(input, { target: { value: 'bob@example.com' } });
|
||||
fireEvent.keyDown(input, { key: ',' });
|
||||
|
||||
expect(screen.getByText('bob@example.com')).toBeInTheDocument();
|
||||
expect(input.value).toBe('');
|
||||
});
|
||||
|
||||
it('leaves a comma inside a display name alone', () => {
|
||||
render(<EmailComposer initialData={DRAFT} />);
|
||||
const input = toInput();
|
||||
fireEvent.change(input, { target: { value: '"Doe' } });
|
||||
const event = fireEvent.keyDown(input, { key: ',' });
|
||||
|
||||
// Not prevented, so the browser types the comma.
|
||||
expect(event).toBe(true);
|
||||
expect(input.value).toBe('"Doe');
|
||||
});
|
||||
|
||||
it('splits a typed list into one chip per entry on blur and marks the bad one', () => {
|
||||
render(<EmailComposer initialData={DRAFT} />);
|
||||
const input = toInput();
|
||||
fireEvent.change(input, { target: { value: 'bob@example.com,notanemail' } });
|
||||
fireEvent.blur(input);
|
||||
|
||||
expect(screen.getByText('bob@example.com').closest('[data-invalid]')).toBeNull();
|
||||
expect(screen.getByText('notanemail').closest('[data-invalid]')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('does not send while a recipient is not an address and names it', async () => {
|
||||
const onSend = vi.fn();
|
||||
render(<EmailComposer initialData={{ ...DRAFT, to: 'bob@example.com, user' }} onSend={onSend} />);
|
||||
|
||||
fireEvent.click(sendButton());
|
||||
await act(async () => {});
|
||||
|
||||
expect(onSend).not.toHaveBeenCalled();
|
||||
expect(toastError).toHaveBeenCalledWith('validation.invalid_recipients');
|
||||
});
|
||||
|
||||
it('sends when every recipient is an address', async () => {
|
||||
const onSend = vi.fn();
|
||||
render(<EmailComposer initialData={{ ...DRAFT, to: 'bob@example.com' }} onSend={onSend} />);
|
||||
|
||||
fireEvent.click(sendButton());
|
||||
|
||||
await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1));
|
||||
expect(toastError).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -2378,6 +2378,18 @@ export function EmailComposer({
|
||||
return;
|
||||
}
|
||||
|
||||
// Catch addresses the server would reject before asking it. Its answer
|
||||
// ("No recipients found in email") does not say which one is wrong.
|
||||
const invalidRecipients = [
|
||||
...withInput(to, toInput),
|
||||
...withInput(cc, ccInput),
|
||||
...withInput(bcc, bccInput),
|
||||
].filter(r => !r.group && !isValidEmail(r.email)).map(r => r.email);
|
||||
if (invalidRecipients.length > 0) {
|
||||
toast.error(t('validation.invalid_recipients', { addresses: invalidRecipients.join(', ') }));
|
||||
return;
|
||||
}
|
||||
|
||||
// Empty subject confirmation (#684)
|
||||
if (!skipSubjectCheck && emptySubjectWarningEnabled && !subject.trim()) {
|
||||
setEmptySubjectDontAskAgain(false);
|
||||
@@ -3982,7 +3994,13 @@ function RecipientChipInput({
|
||||
|
||||
const commitCurrentInput = () => {
|
||||
if (inputText.trim()) {
|
||||
onChipsChange([...chips, parseRecipient(inputText)]);
|
||||
// A typed list ("a@x.com, b@y.com") becomes one chip per entry, like a
|
||||
// paste. Entries that are not addresses still become chips so they show
|
||||
// up as invalid instead of vanishing.
|
||||
const { valid, invalid } = /[,;]/.test(inputText)
|
||||
? splitPastedRecipients(inputText, chips.map(c => c.email))
|
||||
: { valid: [parseRecipient(inputText)], invalid: [] as string[] };
|
||||
onChipsChange([...chips, ...valid, ...invalid.map(parseRecipient)]);
|
||||
onInputChange('');
|
||||
// Also drops the debounced lookup for the text just committed, which
|
||||
// would otherwise reopen the dropdown over the next field.
|
||||
@@ -4021,6 +4039,15 @@ function RecipientChipInput({
|
||||
}
|
||||
}
|
||||
|
||||
// A comma or semicolon after a complete address adds it, as in other mail
|
||||
// clients. Inside a name ("Doe, John" <...>) it is typed normally.
|
||||
if ((e.key === ',' || e.key === ';') && isValidEmail(parseRecipient(inputText.trim()).email)) {
|
||||
e.preventDefault();
|
||||
if (highlightIndex >= 0) onInsertAutocomplete(autocompleteResults[highlightIndex], field);
|
||||
else commitCurrentInput();
|
||||
return;
|
||||
}
|
||||
|
||||
// Enter / Tab commit whatever is typed. Space only commits when the input
|
||||
// is already a complete email address; otherwise Space is a normal
|
||||
// character so a name search like "John Doe" can continue past the space
|
||||
@@ -4202,7 +4229,8 @@ function RecipientChipInput({
|
||||
if(chip.extra?.icon){
|
||||
IconComponent = ICON_MAP[chip.extra?.icon];
|
||||
}
|
||||
const customColor = chip.extra?.color;
|
||||
const isInvalid = !chip.group && !isValidEmail(chip.email);
|
||||
const customColor = isInvalid ? 'destructive' : chip.extra?.color;
|
||||
|
||||
return (
|
||||
<React.Fragment key={`${chip.email}-${i}`}>
|
||||
@@ -4238,6 +4266,8 @@ function RecipientChipInput({
|
||||
!isEditing && draggingIndex === i && "opacity-50"
|
||||
)}
|
||||
onContextMenu={isEditing ? undefined : (e) => handleContextMenu(e, i, chip)}
|
||||
data-invalid={isInvalid || undefined}
|
||||
title={isInvalid ? t('invalid_recipient') : undefined}
|
||||
>
|
||||
{IconComponent ? (
|
||||
<IconComponent
|
||||
|
||||
@@ -692,6 +692,7 @@
|
||||
"validation": {
|
||||
"recipient_required": "Add a recipient to send",
|
||||
"body_required": "Write a message or attach a file",
|
||||
"invalid_recipients": "Not sent - check these addresses: {addresses}",
|
||||
"attachments_uploading": "Attachments are still uploading — sending as soon as they finish",
|
||||
"attachment_upload_failed": "Not sent — an attachment failed to upload. Remove it and try again."
|
||||
},
|
||||
@@ -746,6 +747,7 @@
|
||||
"recipient_edit_name": "Edit display name",
|
||||
"recipient_email_placeholder": "Email address",
|
||||
"recipient_name_placeholder": "Display name",
|
||||
"invalid_recipient": "Not a valid email address",
|
||||
"autocomplete_search_server": "Search the server",
|
||||
"autocomplete_searching": "Searching...",
|
||||
"mention_recipients": "Recipients",
|
||||
|
||||
Reference in New Issue
Block a user