fix: keep an open draft on its own account across an account switch

The composer took its client, identities and active account live from the stores. Switching accounts with a draft open moved it: autosave created the draft in the other account, and replacing the previous save sent that account a delete for the old draft's id, which there names an unrelated message. The composer now stays on the account it was opened in; the other account's identities and client are ignored until it is closed.
2026-10-31 00:12:39 +00:00 · 2026-09-26 15:37:18 +02:00
parent 9033aa7cca
commit d583443292
2 changed files with 255 additions and 3 deletions
@@ -0,0 +1,236 @@
+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 draft belongs to the account the composer was opened in. The composer
+// used to take its client and identities live from the stores, so after an
+// account switch with the composer open its saves went to the other
+// account - and replacing the previous save deleted that account's message
+// with the same id.
+
+// ─── 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 DRAFT = {
+  to: 'bob@example.com',
+  cc: '',
+  bcc: '',
+  subject: 'Half-written',
+  body: '<p>Still typing</p>',
+  showCc: false,
+  showBcc: false,
+  selectedIdentityId: 'id-me',
+  subAddressTag: '',
+  mode: 'compose' as const,
+  draftId: 'draft-7',
+};
+
+function client(draftId: string) {
+  return {
+    createDraft: vi.fn().mockResolvedValue(draftId),
+    getEmail: vi.fn().mockResolvedValue({ id: draftId, attachments: [] }),
+    hasDelayedSend: () => false,
+    getMaxDelayedSend: () => 0,
+  };
+}
+
+describe('composer across an account switch', () => {
+  it('keeps saving the draft to the account it was opened in', async () => {
+    const clientA = client('draft-8');
+    const clientB = client('draft-9');
+    useAuthStore.setState({ client: clientA as never, activeAccountId: 'acct-A' as never });
+
+    const requestCloseRef = { current: null } as React.MutableRefObject<((afterClose?: () => void) => void) | null>;
+    const props = { initialData: DRAFT, requestCloseRef, onClose: vi.fn() };
+    const view = render(<EmailComposer {...props} />);
+    fireEvent.change(screen.getByDisplayValue('Half-written'), { target: { value: 'Half-written more' } });
+
+    // The user switches to account B with the composer still open.
+    useAuthStore.setState({ client: clientB as never, activeAccountId: 'acct-B' as never });
+    view.rerender(<EmailComposer {...props} />);
+
+    act(() => requestCloseRef.current!(vi.fn()));
+    const dialog = await screen.findByRole('alertdialog');
+    fireEvent.click(within(dialog).getByText('save'));
+
+    await waitFor(() => expect(clientA.createDraft).toHaveBeenCalled());
+    expect(clientB.createDraft).not.toHaveBeenCalled();
+    useAuthStore.setState({ client: null, activeAccountId: null });
+  });
+});
@@ -305,7 +305,19 @@ export function EmailComposer({
  const signaturePosition = useSettingsStore((state) => state.signaturePosition);
  const signatureSeparatorEnabled = useSettingsStore((state) => state.signatureSeparatorEnabled);
  const requestReadReceiptDefault = useSettingsStore((state) => state.requestReadReceiptDefault);
-  const activeIdentities = useIdentityStore((s) => s.identities);
+  // A draft belongs to the account the composer was opened in. Switching
+  // accounts with the composer open must not move it: its saves would land in
+  // the other account, and replacing the previous save would delete that
+  // account's message with the same id. So the identities (and, below, the
+  // client) stop following the active account once it changes.
+  const activeAccountIdNow = useAuthStore((s) => s.activeAccountId);
+  const composerAccountIdRef = useRef(activeAccountIdNow);
+  if (composerAccountIdRef.current === null) composerAccountIdRef.current = activeAccountIdNow;
+  const onComposerAccount = activeAccountIdNow === composerAccountIdRef.current;
+  const liveIdentities = useIdentityStore((s) => s.identities);
+  const composerIdentitiesRef = useRef(liveIdentities);
+  if (onComposerAccount) composerIdentitiesRef.current = liveIdentities;
+  const activeIdentities = composerIdentitiesRef.current;
  // More than one connected account: surface identities from every one,
  // grouped for the From dropdown's <optgroup>s. With a single account this
  // collapses to the active account's identities only.
@@ -317,7 +329,8 @@ export function EmailComposer({
    ? multiAccountIdentities.groups
    : [];
  const primaryIdentity = activeIdentities[0] ?? null;
-  const activeAccountId = useAuthStore((s) => s.activeAccountId);
+  // The account this composer belongs to (see composerAccountIdRef above).
+  const activeAccountId = composerAccountIdRef.current;
  // Automatic selection stays on the account that holds the original message:
  // `composerClient` follows the chosen identity, and a reply/forward still
  // carries the original message's blobIds, which only that account's server
@@ -691,7 +704,10 @@ export function EmailComposer({
    restoreFocus: true,
  });

-  const { client } = useAuthStore();
+  const { client: activeClient } = useAuthStore();
+  const composerOwnClientRef = useRef(activeClient);
+  if (onComposerAccount && activeClient) composerOwnClientRef.current = activeClient;
+  const client = composerOwnClientRef.current;
  const currentIdentity = selectedIdentityId
    ? identities.find((identity) => identity.id === selectedIdentityId) || primaryIdentity
    : primaryIdentity;