mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: keep a reply draft in its thread when it is re-opened or undone
Drafts were saved without In-Reply-To and References, and a draft re-opens in compose mode, where the composer has no original message to rebuild them from. A reply that was saved and re-opened, or sent and brought back with Undo, therefore went out as a new conversation. `createDraft` now stores the threading headers, the composer saves them with every draft of a reply, and re-opening a draft carries them into the composer for later saves and the send.
This commit is contained in:
@@ -0,0 +1,231 @@
|
||||
import { render, screen, fireEvent, waitFor, act, within } from '@testing-library/react';
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import React from 'react';
|
||||
import { EmailComposer } from '../email-composer';
|
||||
import { useAuthStore } from '@/stores/auth-store';
|
||||
|
||||
// A reply draft re-opens in compose mode, where the composer used to have
|
||||
// nothing to rebuild In-Reply-To / References from: every later save (and the
|
||||
// send) dropped the reply out of its thread.
|
||||
|
||||
// ─── Heavy component mocks (mirrors composer-close-guard.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<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: () => {}, 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: { intercept: async () => true },
|
||||
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 ────────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* A host that replaces the compose session - a mailto: click, say - must not
|
||||
* drop text the user has not sent. It hands a follow-up to requestCloseRef and
|
||||
* the composer runs it only once it has really closed, so "Save" and "Discard"
|
||||
* continue into the new session while "Cancel" leaves the draft standing.
|
||||
*/
|
||||
|
||||
const REOPENED_REPLY = {
|
||||
to: 'bob@example.com',
|
||||
cc: '',
|
||||
bcc: '',
|
||||
subject: 'Re: Plans',
|
||||
body: '<p>Still typing</p>',
|
||||
showCc: false,
|
||||
showBcc: false,
|
||||
selectedIdentityId: 'id-me',
|
||||
subAddressTag: '',
|
||||
mode: 'compose' as const,
|
||||
draftId: 'draft-7',
|
||||
inReplyTo: ['parent@example.com'],
|
||||
references: ['root@example.com', 'parent@example.com'],
|
||||
};
|
||||
|
||||
describe('re-opened reply draft', () => {
|
||||
it('saves with the threading headers it was opened with', async () => {
|
||||
const createDraft = vi.fn().mockResolvedValue('draft-8');
|
||||
useAuthStore.setState({
|
||||
client: {
|
||||
createDraft,
|
||||
getEmail: vi.fn().mockResolvedValue({ id: 'draft-8', attachments: [] }),
|
||||
hasDelayedSend: () => false,
|
||||
getMaxDelayedSend: () => 0,
|
||||
} as never,
|
||||
activeAccountId: 'acct-A' as never,
|
||||
});
|
||||
|
||||
const requestCloseRef = { current: null } as React.MutableRefObject<((afterClose?: () => void) => void) | null>;
|
||||
render(<EmailComposer initialData={REOPENED_REPLY} requestCloseRef={requestCloseRef} onClose={vi.fn()} />);
|
||||
fireEvent.change(screen.getByDisplayValue('Re: Plans'), { target: { value: 'Re: Plans, edited' } });
|
||||
|
||||
act(() => requestCloseRef.current!(vi.fn()));
|
||||
const dialog = await screen.findByRole('alertdialog');
|
||||
fireEvent.click(within(dialog).getByText('save'));
|
||||
|
||||
await waitFor(() => expect(createDraft).toHaveBeenCalled());
|
||||
const args = createDraft.mock.calls[0];
|
||||
expect(args[11]).toEqual(['parent@example.com']);
|
||||
expect(args[12]).toEqual(['root@example.com', 'parent@example.com']);
|
||||
useAuthStore.setState({ client: null, activeAccountId: null });
|
||||
});
|
||||
});
|
||||
@@ -45,7 +45,7 @@ import type { EmailTemplate } from "@/lib/template-types";
|
||||
import { appendPlainTextSignature, getPlainTextSignature, plainTextBodyHasSignature, plainTextBodyWithoutSignature } from "@/lib/signature-utils";
|
||||
import { findComposeIdentityId, findDraftIdentityId, findReplyIdentityId, resolveReplyFrom } from "@/lib/reply-identity";
|
||||
import { buildReplyRecipients, isSelfSent } from "@/lib/reply-recipients";
|
||||
import { computeReplyThreadingHeaders } from "@/lib/email-threading";
|
||||
import { computeReplyThreadingHeaders, type ReplyThreadingHeaders } from "@/lib/email-threading";
|
||||
import { RequestTimeoutError, ScheduleTooLateError } from "@/lib/jmap/client";
|
||||
import {
|
||||
rewriteCidImagesForEditor,
|
||||
@@ -142,6 +142,13 @@ export interface ComposerDraftData {
|
||||
* format it was written in (#1022).
|
||||
*/
|
||||
plainTextMode?: boolean;
|
||||
/**
|
||||
* Threading headers of a re-opened reply draft (RFC 5322 In-Reply-To /
|
||||
* References). A draft re-opens in "compose" mode, where nothing else
|
||||
* rebuilds them, so they ride along or the reply drops out of its thread.
|
||||
*/
|
||||
inReplyTo?: string[];
|
||||
references?: string[];
|
||||
}
|
||||
|
||||
interface EmailComposerProps {
|
||||
@@ -730,6 +737,17 @@ export function EmailComposer({
|
||||
const composerClientRef = useRef(composerClient);
|
||||
composerClientRef.current = composerClient;
|
||||
const currentIdentityRawId = currentIdentityParts.rawId ?? currentIdentity?.id;
|
||||
// RFC 5322 §3.6.4 threading for this message: computed from the original
|
||||
// on a reply (not a forward), carried over from a re-opened reply draft.
|
||||
// Drafts store it too, so a reply stays in its thread when re-opened or
|
||||
// re-sent after Undo.
|
||||
const replyThreadingHeaders = useMemo<ReplyThreadingHeaders | null>(() => {
|
||||
if (mode === 'reply' || mode === 'replyAll') return computeReplyThreadingHeaders(replyTo);
|
||||
if (initialData?.inReplyTo?.length) {
|
||||
return { inReplyTo: initialData.inReplyTo, references: initialData.references ?? [] };
|
||||
}
|
||||
return null;
|
||||
}, [mode, replyTo, initialData?.inReplyTo, initialData?.references]);
|
||||
// A From override is asked for as the envelope MAIL FROM too, but a server
|
||||
// may only accept the identity's own address there (Stalwart does), which
|
||||
// then shows in the Return-Path. Unless one of the account's identities owns
|
||||
@@ -1894,7 +1912,9 @@ export function EmailComposer({
|
||||
savedDraft.draftId,
|
||||
savedDraft.attachments,
|
||||
savedDraft.fromName,
|
||||
savedDraft.htmlBody
|
||||
savedDraft.htmlBody,
|
||||
replyThreadingHeaders?.inReplyTo,
|
||||
replyThreadingHeaders?.references,
|
||||
);
|
||||
|
||||
// Update the ref synchronously so a queued save sees the new id and
|
||||
@@ -2285,9 +2305,7 @@ export function EmailComposer({
|
||||
};
|
||||
|
||||
// RFC 5322 §3.6.4 threading - only continues the chain on a reply, not a forward.
|
||||
const threadingHeaders = (mode === 'reply' || mode === 'replyAll')
|
||||
? computeReplyThreadingHeaders(replyTo)
|
||||
: null;
|
||||
const threadingHeaders = replyThreadingHeaders;
|
||||
|
||||
// In plain text mode, send text/plain only (no HTML body)
|
||||
const signatureOpts = { separator: signatureSeparatorEnabled };
|
||||
|
||||
@@ -1959,6 +1959,10 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
|
||||
subAddressTag: '',
|
||||
mode: 'compose',
|
||||
draftId: draft.id,
|
||||
// A reply draft re-opens in compose mode; its threading headers must
|
||||
// come along or the reply leaves its thread.
|
||||
inReplyTo: draft.inReplyTo ?? undefined,
|
||||
references: draft.references ?? undefined,
|
||||
// Existing server-side attachments must ride along, or the composer
|
||||
// starts empty and the next save/send silently rebuilds the draft
|
||||
// without them (#849).
|
||||
|
||||
@@ -103,6 +103,23 @@ describe('draft replace lifecycle (#849)', () => {
|
||||
expect(sets[0].destroy).toBeUndefined();
|
||||
});
|
||||
|
||||
it('createDraft keeps a reply draft in its thread', async () => {
|
||||
// A reply draft re-opened (or restored by Undo) has nothing else to
|
||||
// rebuild In-Reply-To / References from, so the draft must carry them.
|
||||
const client = createClient();
|
||||
const captured = mockFlow();
|
||||
|
||||
await client.createDraft(
|
||||
['bob@example.com'], 'Re: Subject', 'body',
|
||||
undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined,
|
||||
['<parent@example.com>'], ['root@example.com', 'parent@example.com'],
|
||||
);
|
||||
|
||||
const created = Object.values(emailSetCalls(captured)[0].create as Record<string, Record<string, unknown>>)[0];
|
||||
expect(created.inReplyTo).toEqual(['parent@example.com']);
|
||||
expect(created.references).toEqual(['root@example.com', 'parent@example.com']);
|
||||
});
|
||||
|
||||
it('createDraft destroys the previous version only after the create succeeded', async () => {
|
||||
const client = createClient();
|
||||
const captured = mockFlow();
|
||||
|
||||
@@ -287,6 +287,8 @@ export interface IJMAPClient {
|
||||
attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>,
|
||||
fromName?: string,
|
||||
htmlBody?: string,
|
||||
inReplyTo?: string[],
|
||||
references?: string[],
|
||||
): Promise<string>;
|
||||
|
||||
sendEmail(
|
||||
|
||||
+11
-1
@@ -3576,7 +3576,9 @@ export class JMAPClient implements IJMAPClient {
|
||||
draftId?: string,
|
||||
attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>,
|
||||
fromName?: string,
|
||||
htmlBody?: string
|
||||
htmlBody?: string,
|
||||
inReplyTo?: string[],
|
||||
references?: string[],
|
||||
): Promise<string> {
|
||||
const mailboxes = await this.getMailboxes();
|
||||
const draftsMailbox = mailboxes.find(mb => mb.role === 'drafts');
|
||||
@@ -3598,9 +3600,15 @@ export class JMAPClient implements IJMAPClient {
|
||||
textBody: { partId: string; type?: string }[];
|
||||
htmlBody?: { partId: string; type: string }[];
|
||||
attachments?: { blobId: string; type: string; name: string; disposition: string; cid?: string }[];
|
||||
inReplyTo?: string[];
|
||||
references?: string[];
|
||||
}
|
||||
|
||||
const sanitizedFromName = sanitizeIdentityDisplayName(fromName);
|
||||
// A reply draft keeps its thread: re-opened or re-sent after Undo, it has
|
||||
// nothing else to rebuild In-Reply-To / References from.
|
||||
const draftInReplyTo = inReplyTo?.map(stripMessageIdBrackets).filter(Boolean);
|
||||
const draftReferences = references?.map(stripMessageIdBrackets).filter(Boolean);
|
||||
const emailData: EmailDraft = {
|
||||
from: [{ ...(sanitizedFromName ? { name: sanitizedFromName } : {}), email: fromEmail || this.username }],
|
||||
// "Name <addr>" must be split into the two JMAP fields: storing the whole
|
||||
@@ -3619,6 +3627,8 @@ export class JMAPClient implements IJMAPClient {
|
||||
? [{ partId: "text", type: "text/plain" }]
|
||||
: [{ partId: "1" }],
|
||||
...(htmlBody ? { htmlBody: [{ partId: "html", type: "text/html" }] } : {}),
|
||||
...(draftInReplyTo?.length ? { inReplyTo: draftInReplyTo } : {}),
|
||||
...(draftReferences?.length ? { references: draftReferences } : {}),
|
||||
};
|
||||
|
||||
if (attachments?.length) {
|
||||
|
||||
Reference in New Issue
Block a user