fix(email): restore the sender identity when reopening a draft
Reopening a draft reset the composer's From to the default identity. The edit-draft handler matched the draft's saved From against the active-account identity list by email only, so two identities sharing an address (a default + an alias differing by name) collided — the wrong one was picked, or with cross-account namespaced ids none was. Add findDraftIdentityId (name+email, normalized, +tag fallback) and match against the same list the composer renders (the flat cross-account list when multi- account is on). Wired into both the classic and Pro edit-draft paths. Unit tests plus the un-pinned 07-drafts integration spec.
This commit is contained in:
@@ -61,7 +61,8 @@ import { isFilePreviewable } from "@/lib/file-preview";
|
|||||||
import { appendHtmlSignature, appendPlainTextSignature } from "@/lib/signature-utils";
|
import { appendHtmlSignature, appendPlainTextSignature } from "@/lib/signature-utils";
|
||||||
import { computeReplyThreadingHeaders } from "@/lib/email-threading";
|
import { computeReplyThreadingHeaders } from "@/lib/email-threading";
|
||||||
import { EML_IMPORT_ACCEPT, expandImportableEmails } from "@/lib/eml-import";
|
import { EML_IMPORT_ACCEPT, expandImportableEmails } from "@/lib/eml-import";
|
||||||
import { resolveReplyFrom } from "@/lib/reply-identity";
|
import { findDraftIdentityId, resolveReplyFrom } from "@/lib/reply-identity";
|
||||||
|
import { useProMultiAccountIdentities } from "@/hooks/use-pro-multi-account-identities";
|
||||||
import { Search, Filter, ChevronDown, X, Paperclip, Star, Mail, MailOpen, RotateCcw, PenSquare, PenLine, CheckSquare, Square, AlertTriangle } from "lucide-react";
|
import { Search, Filter, ChevronDown, X, Paperclip, Star, Mail, MailOpen, RotateCcw, PenSquare, PenLine, CheckSquare, Square, AlertTriangle } from "lucide-react";
|
||||||
import { ResizeHandle } from "@/components/layout/resize-handle";
|
import { ResizeHandle } from "@/components/layout/resize-handle";
|
||||||
import { Button } from "@/components/ui/button";
|
import { Button } from "@/components/ui/button";
|
||||||
@@ -119,6 +120,7 @@ export default function Home() {
|
|||||||
const initialMailLoadClientRef = useRef<object | null>(null);
|
const initialMailLoadClientRef = useRef<object | null>(null);
|
||||||
const { isAuthenticated, client, logout, checkAuth, switchAccount, activeAccountId, isLoading: authLoading, connectionLost, isRateLimited, rateLimitUntil } = useAuthStore();
|
const { isAuthenticated, client, logout, checkAuth, switchAccount, activeAccountId, isLoading: authLoading, connectionLost, isRateLimited, rateLimitUntil } = useAuthStore();
|
||||||
const { identities } = useIdentityStore();
|
const { identities } = useIdentityStore();
|
||||||
|
const multiAccountIdentities = useProMultiAccountIdentities();
|
||||||
useIdentitySync();
|
useIdentitySync();
|
||||||
const trustedSendersAddressBook = useSettingsStore((state) => state.trustedSendersAddressBook);
|
const trustedSendersAddressBook = useSettingsStore((state) => state.trustedSendersAddressBook);
|
||||||
const sendDelaySeconds = useSettingsStore((state) => state.sendDelaySeconds);
|
const sendDelaySeconds = useSettingsStore((state) => state.sendDelaySeconds);
|
||||||
@@ -1409,11 +1411,15 @@ export default function Home() {
|
|||||||
? draft.bodyValues[draftHtmlPart.partId].value
|
? draft.bodyValues[draftHtmlPart.partId].value
|
||||||
: undefined;
|
: undefined;
|
||||||
|
|
||||||
// Try to find the identity that matches the draft's from address to preserve it
|
// Restore the identity the draft was composed with. Match the saved From
|
||||||
const draftFromEmail = draft.from?.[0]?.email;
|
// (name + address) against the same list the composer renders — the flat
|
||||||
const matchedIdentity = draftFromEmail
|
// cross-account list when multi-account is on — so a draft from a non-active
|
||||||
? identities.find(id => id.email === draftFromEmail)
|
// account, or one of two identities sharing an address, is restored rather
|
||||||
: null;
|
// than reset to the default.
|
||||||
|
const composerIdentities = multiAccountIdentities.enabled
|
||||||
|
? multiAccountIdentities.allIdentities
|
||||||
|
: identities;
|
||||||
|
const matchedIdentityId = findDraftIdentityId(composerIdentities, draft.from?.[0]);
|
||||||
|
|
||||||
// Increment session ID to force the composer to remount with fresh state,
|
// Increment session ID to force the composer to remount with fresh state,
|
||||||
// even if it was already open (e.g. right-clicking a draft while composing).
|
// even if it was already open (e.g. right-clicking a draft while composing).
|
||||||
@@ -1426,7 +1432,7 @@ export default function Home() {
|
|||||||
body: htmlBody || bodyText,
|
body: htmlBody || bodyText,
|
||||||
showCc: (draft.cc?.length || 0) > 0,
|
showCc: (draft.cc?.length || 0) > 0,
|
||||||
showBcc: (draft.bcc?.length || 0) > 0,
|
showBcc: (draft.bcc?.length || 0) > 0,
|
||||||
selectedIdentityId: matchedIdentity?.id ?? null,
|
selectedIdentityId: matchedIdentityId,
|
||||||
subAddressTag: '',
|
subAddressTag: '',
|
||||||
mode: 'compose',
|
mode: 'compose',
|
||||||
draftId: draft.id,
|
draftId: draft.id,
|
||||||
|
|||||||
@@ -7,6 +7,8 @@ import { ErrorBoundary, EmailViewerErrorFallback } from "@/components/error";
|
|||||||
import { useAuthStore } from "@/stores/auth-store";
|
import { useAuthStore } from "@/stores/auth-store";
|
||||||
import { useEmailStore } from "@/stores/email-store";
|
import { useEmailStore } from "@/stores/email-store";
|
||||||
import { useIdentityStore } from "@/stores/identity-store";
|
import { useIdentityStore } from "@/stores/identity-store";
|
||||||
|
import { useProMultiAccountIdentities } from "@/hooks/use-pro-multi-account-identities";
|
||||||
|
import { findDraftIdentityId } from "@/lib/reply-identity";
|
||||||
import { useSettingsStore } from "@/stores/settings-store";
|
import { useSettingsStore } from "@/stores/settings-store";
|
||||||
import { toast } from "@/stores/toast-store";
|
import { toast } from "@/stores/toast-store";
|
||||||
import { useProTabStore, type ProEmailTabData, type ProReplyContext } from "@/stores/pro-tab-store";
|
import { useProTabStore, type ProEmailTabData, type ProReplyContext } from "@/stores/pro-tab-store";
|
||||||
@@ -56,6 +58,7 @@ export function ProEmailTabBody({ tabId, data }: ProEmailTabBodyProps) {
|
|||||||
const mailboxes = useEmailStore((s) => s.mailboxes);
|
const mailboxes = useEmailStore((s) => s.mailboxes);
|
||||||
const settingsKeywords = useSettingsStore((s) => s.emailKeywords);
|
const settingsKeywords = useSettingsStore((s) => s.emailKeywords);
|
||||||
const identities = useIdentityStore((s) => s.identities);
|
const identities = useIdentityStore((s) => s.identities);
|
||||||
|
const multiAccountIdentities = useProMultiAccountIdentities();
|
||||||
|
|
||||||
const closeTab = useProTabStore((s) => s.closeTab);
|
const closeTab = useProTabStore((s) => s.closeTab);
|
||||||
const openComposeTab = useProTabStore((s) => s.openComposeTab);
|
const openComposeTab = useProTabStore((s) => s.openComposeTab);
|
||||||
@@ -242,11 +245,12 @@ export function ProEmailTabBody({ tabId, data }: ProEmailTabBodyProps) {
|
|||||||
? email.bodyValues[draftHtmlPart.partId].value
|
? email.bodyValues[draftHtmlPart.partId].value
|
||||||
: undefined;
|
: undefined;
|
||||||
|
|
||||||
// Preserve the identity that matches the draft's From address.
|
// Preserve the identity the draft was composed with — match name + address
|
||||||
const draftFromEmail = email.from?.[0]?.email;
|
// against the same list the composer renders (see findDraftIdentityId).
|
||||||
const matchedIdentity = draftFromEmail
|
const composerIdentities = multiAccountIdentities.enabled
|
||||||
? identities.find((id) => id.email === draftFromEmail)
|
? multiAccountIdentities.allIdentities
|
||||||
: null;
|
: identities;
|
||||||
|
const matchedIdentityId = findDraftIdentityId(composerIdentities, email.from?.[0]);
|
||||||
|
|
||||||
composerSessionIdRef.current += 1;
|
composerSessionIdRef.current += 1;
|
||||||
openComposeTab({
|
openComposeTab({
|
||||||
@@ -261,14 +265,14 @@ export function ProEmailTabBody({ tabId, data }: ProEmailTabBodyProps) {
|
|||||||
body: htmlBody || bodyText,
|
body: htmlBody || bodyText,
|
||||||
showCc: (email.cc?.length || 0) > 0,
|
showCc: (email.cc?.length || 0) > 0,
|
||||||
showBcc: (email.bcc?.length || 0) > 0,
|
showBcc: (email.bcc?.length || 0) > 0,
|
||||||
selectedIdentityId: matchedIdentity?.id ?? null,
|
selectedIdentityId: matchedIdentityId,
|
||||||
subAddressTag: '',
|
subAddressTag: '',
|
||||||
mode: 'compose',
|
mode: 'compose',
|
||||||
draftId: email.id,
|
draftId: email.id,
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
closeTab(tabId);
|
closeTab(tabId);
|
||||||
}, [email, identities, openComposeTab, closeTab, tabId, t]);
|
}, [email, identities, multiAccountIdentities, openComposeTab, closeTab, tabId, t]);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="flex h-full w-full flex-col bg-background">
|
<div className="flex h-full w-full flex-col bg-background">
|
||||||
|
|||||||
@@ -120,12 +120,10 @@ test.describe('Drafts', () => {
|
|||||||
expect((draft.from ?? [])[0]?.name, 'draft From carries the selected identity').toBe('Alice Team');
|
expect((draft.from ?? [])[0]?.name, 'draft From carries the selected identity').toBe('Alice Team');
|
||||||
});
|
});
|
||||||
|
|
||||||
// KNOWN BUG (documented via test.fail): a draft composed with a non-default
|
// Regression: reopening a draft must restore the identity it was written with
|
||||||
// identity is saved with the right From on the server (see the test above),
|
// (drafts store the From address+name, not an identity id, so the reopen path
|
||||||
// but reopening the draft resets the composer's From selector to the default
|
// matches name+address against the identity list — see findDraftIdentityId).
|
||||||
// identity instead of restoring the one the draft was written with. If this
|
test('reopening a draft restores the changed sender in the From selector', async ({ page }) => {
|
||||||
// starts passing, the reopen path was fixed — flip this back to a plain test.
|
|
||||||
test.fail('reopening a draft restores the changed sender in the From selector', async ({ page }) => {
|
|
||||||
const altId = await jmap.ensureIdentity('Alice Team', alice.email);
|
const altId = await jmap.ensureIdentity('Alice Team', alice.email);
|
||||||
const subject = subj('draft-from-reopen');
|
const subject = subj('draft-from-reopen');
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { describe, expect, it } from 'vitest';
|
import { describe, expect, it } from 'vitest';
|
||||||
import { findComposeIdentityId, findReplyIdentityId, resolveReplyFrom } from '../reply-identity';
|
import { findComposeIdentityId, findDraftIdentityId, findReplyIdentityId, resolveReplyFrom } from '../reply-identity';
|
||||||
import type { Identity } from '../jmap/types';
|
import type { Identity } from '../jmap/types';
|
||||||
|
|
||||||
const identities: Identity[] = [
|
const identities: Identity[] = [
|
||||||
@@ -17,6 +17,36 @@ const identities: Identity[] = [
|
|||||||
},
|
},
|
||||||
];
|
];
|
||||||
|
|
||||||
|
describe('findDraftIdentityId', () => {
|
||||||
|
// Two identities on the SAME address, differing only by display name — the
|
||||||
|
// reopen-draft regression: an email-only match picks the default, not the one
|
||||||
|
// the draft was written with.
|
||||||
|
const sameAddress: Identity[] = [
|
||||||
|
{ id: 'default', name: 'Harry Primary', email: 'harry@primary.com', mayDelete: false },
|
||||||
|
{ id: 'team', name: 'Harry Team', email: 'harry@primary.com', mayDelete: false },
|
||||||
|
];
|
||||||
|
|
||||||
|
it('restores the exact identity by name when several share an address', () => {
|
||||||
|
expect(findDraftIdentityId(sameAddress, { name: 'Harry Team', email: 'harry@primary.com' })).toBe('team');
|
||||||
|
expect(findDraftIdentityId(sameAddress, { name: 'Harry Primary', email: 'harry@primary.com' })).toBe('default');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('matches by email (normalized) when the name is absent or unique', () => {
|
||||||
|
expect(findDraftIdentityId(identities, { email: 'HARRY@Secondary.com' })).toBe('secondary');
|
||||||
|
expect(findDraftIdentityId(identities, { name: 'Whatever', email: 'harry@secondary.com' })).toBe('secondary');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to the +tag-stripped base address', () => {
|
||||||
|
expect(findDraftIdentityId(identities, { email: 'harry+promo@secondary.com' })).toBe('secondary');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns null when nothing matches or there is no From', () => {
|
||||||
|
expect(findDraftIdentityId(identities, { email: 'nobody@elsewhere.com' })).toBeNull();
|
||||||
|
expect(findDraftIdentityId(identities, null)).toBeNull();
|
||||||
|
expect(findDraftIdentityId([], { email: 'harry@primary.com' })).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('findReplyIdentityId', () => {
|
describe('findReplyIdentityId', () => {
|
||||||
it('matches the identity that received the original message', () => {
|
it('matches the identity that received the original message', () => {
|
||||||
const selected = findReplyIdentityId(identities, {
|
const selected = findReplyIdentityId(identities, {
|
||||||
|
|||||||
@@ -95,6 +95,45 @@ export function findComposeIdentityId(
|
|||||||
return baseIdentity?.id ?? null;
|
return baseIdentity?.id ?? null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Restore the identity a draft was composed with from its saved From. A draft
|
||||||
|
* stores only the From address+name, not an identityId, so when two identities
|
||||||
|
* share an address (a default + an alias with a different display name) the name
|
||||||
|
* has to disambiguate — an email-only match picks the wrong one. Falls back to
|
||||||
|
* email, then `+tag`-stripped email. Returns null when none match (caller keeps
|
||||||
|
* the default). Pass the same identity list the composer renders (the flat
|
||||||
|
* cross-account list when multi-account is on) so the returned id is usable
|
||||||
|
* there.
|
||||||
|
*/
|
||||||
|
export function findDraftIdentityId(
|
||||||
|
identities: Identity[],
|
||||||
|
from?: { email?: string | null; name?: string | null } | null,
|
||||||
|
): string | null {
|
||||||
|
const email = from?.email?.trim();
|
||||||
|
if (identities.length === 0 || !email) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
const wantEmail = normalizeEmailAddress(email);
|
||||||
|
const wantName = (from?.name ?? '').trim();
|
||||||
|
|
||||||
|
const nameAndEmail = identities.find(
|
||||||
|
(i) => normalizeEmailAddress(i.email) === wantEmail && (i.name ?? '').trim() === wantName,
|
||||||
|
);
|
||||||
|
if (nameAndEmail) {
|
||||||
|
return nameAndEmail.id;
|
||||||
|
}
|
||||||
|
|
||||||
|
const exact = identities.find((i) => normalizeEmailAddress(i.email) === wantEmail);
|
||||||
|
if (exact) {
|
||||||
|
return exact.id;
|
||||||
|
}
|
||||||
|
|
||||||
|
const wantBase = normalizeBaseEmailAddress(email);
|
||||||
|
const base = identities.find((i) => normalizeBaseEmailAddress(i.email) === wantBase);
|
||||||
|
return base?.id ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
export interface ReplyFromResolution {
|
export interface ReplyFromResolution {
|
||||||
/** Identity to use for JMAP `identityId` and the SMTP envelope MAIL FROM. */
|
/** Identity to use for JMAP `identityId` and the SMTP envelope MAIL FROM. */
|
||||||
identityId: string;
|
identityId: string;
|
||||||
|
|||||||
Reference in New Issue
Block a user