mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: keep your own address out of reply-all before your identities have loaded #803
This commit is contained in:
@@ -1,10 +1,11 @@
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { render, screen, fireEvent } 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';
|
||||
import { useAuthStore } from '@/stores/auth-store';
|
||||
import { useAccountStore } from '@/stores/account-store';
|
||||
import { useIdentityStore } from '@/stores/identity-store';
|
||||
import { usePolicyStore } from '@/stores/policy-store';
|
||||
|
||||
// ─── Heavy component mocks (mirrors recipient-paste.test.tsx) ─────────────────
|
||||
@@ -491,6 +492,59 @@ describe('composer reply addressing', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// #803: identities load from the server after the app starts and aren't
|
||||
// persisted. A reply-all opened before they arrive used to keep our own
|
||||
// address in To, and nothing fixed it once they loaded.
|
||||
describe('reply-all before identities have loaded', () => {
|
||||
const identityStore = useIdentityStore as unknown as {
|
||||
getState: () => { identities: unknown[] };
|
||||
setState: (p: Record<string, unknown>) => void;
|
||||
};
|
||||
const accountStore = useAccountStore as unknown as { setState: (p: Record<string, unknown>) => void };
|
||||
const authStore = useAuthStore as unknown as { setState: (p: Record<string, unknown>) => void };
|
||||
let loaded: unknown[];
|
||||
|
||||
beforeEach(() => {
|
||||
loaded = identityStore.getState().identities;
|
||||
identityStore.setState({ identities: [] });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
identityStore.setState({ identities: loaded });
|
||||
accountStore.setState({ accounts: [] });
|
||||
authStore.setState({ activeAccountId: null });
|
||||
});
|
||||
|
||||
it('drops our own address using the account address', () => {
|
||||
accountStore.setState({
|
||||
accounts: [{ id: 'local-1', email: 'me@example.com', username: 'me@example.com', isConnected: true }],
|
||||
});
|
||||
authStore.setState({ activeAccountId: 'local-1' });
|
||||
render(<EmailComposer mode="replyAll" replyTo={RECEIVED} />);
|
||||
expect(toChips()).toEqual(['Bob (bob@other.com)', 'Carol (carol@other.com)']);
|
||||
});
|
||||
|
||||
it('drops our own address once the identities arrive', () => {
|
||||
const { rerender } = render(<EmailComposer mode="replyAll" replyTo={RECEIVED} />);
|
||||
expect(toChips()).toContain('Me (me@example.com)');
|
||||
identityStore.setState({ identities: loaded });
|
||||
rerender(<EmailComposer mode="replyAll" replyTo={RECEIVED} />);
|
||||
expect(toChips()).toEqual(['Bob (bob@other.com)', 'Carol (carol@other.com)']);
|
||||
expect(ccChips()).toEqual(['Dave (dave@other.com)']);
|
||||
});
|
||||
|
||||
it('leaves the recipients alone once the user has changed them', () => {
|
||||
const { rerender } = render(<EmailComposer mode="replyAll" replyTo={RECEIVED} />);
|
||||
const carol = Array.from(screen.getByTestId('composer-to').querySelectorAll('[draggable]'))
|
||||
.find((el) => el.textContent?.includes('carol@other.com')) as HTMLElement;
|
||||
fireEvent.click(carol.querySelector('button') as HTMLElement);
|
||||
expect(toChips()).toEqual(['Bob (bob@other.com)', 'Me (me@example.com)']);
|
||||
identityStore.setState({ identities: loaded });
|
||||
rerender(<EmailComposer mode="replyAll" replyTo={RECEIVED} />);
|
||||
expect(toChips()).toEqual(['Bob (bob@other.com)', 'Me (me@example.com)']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('composer @-mention candidates', () => {
|
||||
const offered = () => editorProps.mentionCandidates.map((c) => `${c.label} <${c.email}>`);
|
||||
|
||||
|
||||
@@ -91,7 +91,7 @@ import type { EmailTemplate } from "@/lib/template-types";
|
||||
import { appendPlainTextSignature, getPlainTextSignature, plainTextBodyHasSignature, plainTextBodyWithoutSignature } from "@/lib/signature-utils";
|
||||
import { wantsSignatureSeparator } from "@/lib/signature-delimiter";
|
||||
import { findComposeIdentityId, findDraftIdentityId, findReplyIdentityId, resolveReplyFrom } from "@/lib/reply-identity";
|
||||
import { buildReplyRecipients, isSelfSent } from "@/lib/reply-recipients";
|
||||
import { buildReplyRecipients, isSelfSent, ownReplyAddresses } from "@/lib/reply-recipients";
|
||||
import { computeReplyThreadingHeaders, type ReplyThreadingHeaders } from "@/lib/email-threading";
|
||||
import { RecipientsRejectedError, RequestTimeoutError, ScheduleTooLateError, SendUnconfirmedError, formatRejectedRecipients } from "@/lib/jmap/client";
|
||||
import {
|
||||
@@ -520,7 +520,17 @@ export function EmailComposer({
|
||||
const toRecipient = (r: { name?: string; email?: string }): Recipient =>
|
||||
({ name: r.name && r.name !== r.email ? r.name : undefined, email: r.email ?? "" });
|
||||
|
||||
const ownIdentityEmails = identities.map(i => i.email).filter((e): e is string => Boolean(e));
|
||||
// Identities can still be loading when the composer opens (cold start, PWA
|
||||
// relaunch), so the account's own addresses stand in until they arrive (#803).
|
||||
const composerAccount = activeAccountId ? useAccountStore.getState().getAccountById(activeAccountId) : undefined;
|
||||
const replyAccount = sourceAccountId && sourceAccountId !== activeAccountId
|
||||
? useAccountStore.getState().getAccountById(sourceAccountId)
|
||||
: undefined;
|
||||
const ownIdentityEmails = ownReplyAddresses(
|
||||
identities.map(i => i.email),
|
||||
[composerAccount?.email, composerAccount?.username, replyAccount?.email, replyAccount?.username],
|
||||
);
|
||||
const ownIdentityEmailsKey = ownIdentityEmails.map((e) => e.toLowerCase()).join(',');
|
||||
|
||||
// Initialize with reply/forward data if provided
|
||||
const getInitialTo = (): Recipient[] => {
|
||||
@@ -726,6 +736,34 @@ export function EmailComposer({
|
||||
const [showCc, setShowCc] = useState(() => initialData?.showCc
|
||||
?? (prefill?.cc ? prefill.cc.length > 0 : getInitialCc().length > 0));
|
||||
const [showBcc, setShowBcc] = useState(initialData?.showBcc ?? false);
|
||||
// The reply recipients worked out when the composer opened. When the
|
||||
// identities arrive later (#803) they're worked out again, but only while
|
||||
// To and Cc still hold exactly what was filled in, so nothing the user
|
||||
// changed is overwritten. Drafts and prefilled composes are left alone.
|
||||
const autoRecipientsRef = useRef<{ to: Recipient[]; cc: Recipient[] } | null>(
|
||||
!initialData && !prefill && (mode === 'reply' || mode === 'replyAll')
|
||||
? { to, cc }
|
||||
: null,
|
||||
);
|
||||
useEffect(() => {
|
||||
const auto = autoRecipientsRef.current;
|
||||
if (!auto) return;
|
||||
const same = (a: Recipient[], b: Recipient[]) =>
|
||||
a.length === b.length && a.every((r, i) => r.email === b[i].email && r.name === b[i].name);
|
||||
if (!same(to, auto.to) || !same(cc, auto.cc)) {
|
||||
autoRecipientsRef.current = null;
|
||||
return;
|
||||
}
|
||||
const nextTo = getInitialTo();
|
||||
const nextCc = getInitialCc();
|
||||
if (same(nextTo, auto.to) && same(nextCc, auto.cc)) return;
|
||||
autoRecipientsRef.current = { to: nextTo, cc: nextCc };
|
||||
setTo(nextTo);
|
||||
setCc(nextCc);
|
||||
if (nextCc.length > 0) setShowCc(true);
|
||||
// Only a change in the user's own addresses should redo the recipients.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [ownIdentityEmailsKey]);
|
||||
// Committed recipients plus any not-yet-committed text the user has typed.
|
||||
// Send/validation/draft paths treat a typed-but-uncommitted address as a
|
||||
// real recipient, matching the previous string-based behavior.
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { buildReplyRecipients, isSelfSent } from '@/lib/reply-recipients';
|
||||
import { buildReplyRecipients, isSelfSent, ownReplyAddresses } from '@/lib/reply-recipients';
|
||||
|
||||
const OWN = ['me@example.com', 'info@example.com'];
|
||||
|
||||
@@ -135,3 +135,29 @@ describe('buildReplyRecipients', () => {
|
||||
expect(isSelfSent(undefined, OWN)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// #803: identities load after the app starts, so a composer opened before
|
||||
// they arrive had no own addresses and reply-all kept the user in To.
|
||||
describe('ownReplyAddresses', () => {
|
||||
const received = {
|
||||
from: [{ email: 'bob@other.com' }],
|
||||
to: [{ email: 'me@example.com' }, { email: 'carol@other.com' }],
|
||||
cc: [{ email: 'Me@Example.com' }],
|
||||
};
|
||||
|
||||
it('drops our own address on reply-all before identities have loaded', () => {
|
||||
const own = ownReplyAddresses([], ['me@example.com', 'me']);
|
||||
const { to, cc } = buildReplyRecipients(received, 'replyAll', own);
|
||||
expect(emails(to)).toEqual(['bob@other.com', 'carol@other.com']);
|
||||
expect(cc).toEqual([]);
|
||||
});
|
||||
|
||||
it('merges identities with the account addresses, once each', () => {
|
||||
expect(ownReplyAddresses(['me@example.com', undefined, ''], ['ME@example.com ', 'info@example.com']))
|
||||
.toEqual(['me@example.com', 'info@example.com']);
|
||||
});
|
||||
|
||||
it('skips a login name without a domain', () => {
|
||||
expect(ownReplyAddresses([], ['linus', null, undefined])).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -44,6 +44,31 @@ function isOwnAddress(email: string | undefined, ownEmails: string[]): boolean {
|
||||
return ownEmails.some((own) => normalizeBase(own) === base);
|
||||
}
|
||||
|
||||
/**
|
||||
* The addresses a reply treats as the user's own: their identities plus the
|
||||
* account's login and primary address. Identities load from the server after
|
||||
* the app starts and aren't persisted, so a composer opened before they arrive
|
||||
* saw none and reply-all kept the user in To/Cc (#803). The account's
|
||||
* addresses are known from the start and cover the usual case until then.
|
||||
* A bare login name without a domain can't match an address, so it's skipped.
|
||||
*/
|
||||
export function ownReplyAddresses(
|
||||
identityEmails: (string | undefined | null)[],
|
||||
accountAddresses: (string | undefined | null)[] = [],
|
||||
): string[] {
|
||||
const seen = new Set<string>();
|
||||
const result: string[] = [];
|
||||
for (const email of [...identityEmails, ...accountAddresses]) {
|
||||
const trimmed = email?.trim();
|
||||
if (!trimmed || !trimmed.includes('@')) continue;
|
||||
const key = normalize(trimmed);
|
||||
if (seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
result.push(trimmed);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this a message the user themself sent? True when the From address is one
|
||||
* of their own identities - the case that shows up when browsing a thread and
|
||||
|
||||
Reference in New Issue
Block a user