fix: stop reopened drafts from deleting another login's mail

A reopened draft is saved over and removed by id through the login and
account it is stamped with, and message ids repeat across servers. Two
ways of opening a draft stamped it with a login that was only on
screen, not the one it was read through:

- A Pro email tab opened by double-click named no login. It read the
  message through whatever login the main list was browsing and stamped
  the active one: with the 0.16.23 login active and the 1.0 login's
  Drafts browsed, the first autosave destroyed the draft's id in the
  0.16.23 account and left the draft itself behind. A Pro view now
  stamps only a login its host names, and reads the message through
  that login's client. A double-click on a row of the mail list or of a
  folder tab names the row's login and account, and the search preview
  and a folder tab's reading pane name theirs for the active login
  too. Without a login the composer goes by the draft's From, as it did
  before.
- "Cancel and edit" in Scheduled took the login browsed before, which
  stays set while Scheduled lists the active login's mail. The draft
  was restored through the active login and then saved over and
  discarded through the other one. It now takes the active login, and
  a send undone or cancelled for editing never falls back to what is on
  screen.

Also:

- A draft version saved through another login is removed only once the
  message is out. A host without a connection sent nothing and the
  version was removed anyway; it now says so and the draft stays.
- An email tab opened for one login's message no longer focuses the tab
  of another login's message with the same id and account.
This commit is contained in:
Linus Rath
2026-09-26 04:32:25 +02:00
parent 021a67327b
commit 5f0669eb92
15 changed files with 424 additions and 42 deletions
@@ -316,6 +316,20 @@ describe('a draft and the login it was saved with', () => {
expect(clientA.deleteEmail).not.toHaveBeenCalled();
});
it('keeps a draft saved through another login when the host sent nothing', async () => {
// The host has no connection and says so.
const onSend = vi.fn().mockResolvedValue(false);
const composer = open(DRAFT, { onSend });
await composer.editSubject('Hello', 'Hello 2');
composer.switchFrom('login-b::id-b');
await composer.send();
expect(onSend).toHaveBeenCalled();
expect(clientA.deleteEmail).not.toHaveBeenCalled();
expect(clientB.deleteEmail).not.toHaveBeenCalled();
});
it('removes a draft opened from another login\'s group account through that login', async () => {
const onDiscardDraft = vi.fn();
// Opened from login B's unified-view row in the team account `c`; the
+9 -3
View File
@@ -162,6 +162,11 @@ export interface ComposerDraftData {
}
interface EmailComposerProps {
/**
* Submits the message. Resolves `false` when the host sent nothing (it has
* no connection): a draft version that only goes once the message is out
* stays then.
*/
onSend?: (data: {
to: string[];
cc: string[];
@@ -190,7 +195,7 @@ interface EmailComposerProps {
requestDsn?: boolean;
/** Refuse delivery over an unencrypted hop (RFC 8689 REQUIRETLS). */
requireTls?: boolean;
}) => void | Promise<void>;
}) => void | false | Promise<void | false>;
onScheduledSendCreated?: () => void | Promise<void>;
onClose?: () => void;
/**
@@ -2440,7 +2445,7 @@ export function EmailComposer({
const draftAccountAtSend = draftAccountIdRef.current;
const draftElsewhere = !!finalDraftId && !!draftLogin && draftLogin !== loginFor(identityLocalAccountId);
await onSend?.({
const outcome = await onSend?.({
to: outgoing.to,
cc: outgoing.cc,
bcc: outgoing.bcc,
@@ -2462,7 +2467,8 @@ export function EmailComposer({
requireTls: requireTls || undefined,
delayedUntil: effectiveDelayedUntil,
});
if (draftElsewhere) await removeDraftVersion(finalDraftId!, draftLogin, draftAccountAtSend);
// Not when nothing went: no host, or one that sent nothing.
if (draftElsewhere && onSend && outcome !== false) await removeDraftVersion(finalDraftId!, draftLogin, draftAccountAtSend);
if (mode === 'reply' || mode === 'replyAll') {
for (const recipient of [...outgoing.to, ...outgoing.cc].filter(Boolean)) {
@@ -0,0 +1,55 @@
import { render } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import React from 'react';
// The search tab's preview of a mail hit reads the message through the
// login the hit came through, and a draft reopened there is saved over and
// removed through it - the active login's hits included: userb is account
// `e` on the 0.16.23 and the 1.0 test server alike.
const viewProps = vi.hoisted(() => ({ current: null as Record<string, unknown> | null }));
vi.mock('@/components/pro/pro-email-tab-body', () => ({
ProEmailView: (props: Record<string, unknown>) => {
viewProps.current = props;
return null;
},
}));
vi.mock('next-intl', () => ({ useTranslations: () => (key: string) => key }));
const clients = vi.hoisted(() => ({ v016: { label: '0.16.23' }, v10: { label: '1.0' } }));
vi.mock('@/stores/auth-store', () => {
const state = {
activeAccountId: 'userb@v016',
getClientForAccount: (id: string) => (id === 'userb@v016' ? clients.v016 : id === 'userb@v10' ? clients.v10 : undefined),
};
const hook = (sel?: (s: typeof state) => unknown) => (typeof sel === 'function' ? sel(state) : state);
hook.getState = () => state;
return { useAuthStore: hook };
});
import { SearchPreview } from '../search-preview';
import type { MailHit } from '@/lib/global-search/types';
const hit = (localAccountId: string): MailHit => ({
kind: 'mail', localAccountId, jmapAccountId: 'e', id: 'eaaaaab', accountLabel: localAccountId,
title: 'Draft', subtitle: 'Drafts', date: null, source: 'remote', snippet: null,
email: { id: 'eaaaaab' } as MailHit['email'],
});
describe('search preview of a mail hit', () => {
beforeEach(() => {
viewProps.current = null;
});
it('names the active login for a hit of the active login', () => {
render(<SearchPreview hit={hit('userb@v016')} onOpen={vi.fn()} onClose={vi.fn()} />);
expect(viewProps.current).toMatchObject({ client: clients.v016, clientAccountId: 'userb@v016', accountId: 'e' });
});
it('names the other login for a hit of another login', () => {
render(<SearchPreview hit={hit('userb@v10')} onOpen={vi.fn()} onClose={vi.fn()} />);
expect(viewProps.current).toMatchObject({ client: clients.v10, clientAccountId: 'userb@v10', accountId: 'e' });
});
});
+3 -2
View File
@@ -158,7 +158,6 @@ export interface SearchPreviewProps {
export function SearchPreview({ hit, onOpen, onClose }: SearchPreviewProps) {
const t = useTranslations('global_search');
const getClientForAccount = useAuthStore((s) => s.getClientForAccount);
const activeAccountId = useAuthStore((s) => s.activeAccountId);
if (!hit) {
return (
@@ -169,7 +168,9 @@ export function SearchPreview({ hit, onOpen, onClose }: SearchPreviewProps) {
}
if (hit.kind === 'mail') {
const clientOverride = hit.localAccountId !== activeAccountId ? getClientForAccount(hit.localAccountId) ?? undefined : undefined;
// The hit's own login, the active one too: the view names it, so a draft
// reopened from here is saved over and removed through it.
const clientOverride = getClientForAccount(hit.localAccountId) ?? undefined;
return (
<ProEmailView
key={`${hit.localAccountId}-${hit.id}`}
+26 -6
View File
@@ -32,7 +32,7 @@ import type { UnifiedAccountClient } from "@/lib/unified-mailbox";
import { connectedAccountsGrew } from "@/lib/unified-mailbox";
import { KeyboardShortcutsModal } from "@/components/keyboard-shortcuts-modal";
import { useEmailStore, buildUnifiedAccountClients, ArchiveMailboxNotFoundError, findArchiveMailbox, resolveUnstampedEmailAccountId, isAggregateListView } from "@/stores/email-store";
import { findDraftIdentityIdForLogin, resolveDraftSource, type DraftSource } from "@/lib/draft-source";
import { findDraftIdentityIdForLogin, listRowSource, resolveDraftSource, type DraftSource } from "@/lib/draft-source";
import { groupSearchScopeFolders } from "@/lib/search-scope-folders";
import { toast } from "@/stores/toast-store";
import { cancelFailureToast, partlyRefusedWarning, rescheduleFailureToast } from "@/lib/send-failure-message";
@@ -1713,7 +1713,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
requestDsn?: boolean;
requireTls?: boolean;
}) => {
if (!client) return;
// Nothing is sent: the composer keeps what only goes once the mail is out.
if (!client) return false;
// Sending from an identity that belongs to another connected account has
// to go through that account's own client: the identity id is only valid
@@ -4190,7 +4191,9 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
setComposerMode('compose');
setPendingDraft(null);
} else if (restored) {
await handleEditDraft(restored, { accountId: email.scheduledAccountId }).catch(reportDraftOpenFailure);
// Restored through the active login's client, the one
// Scheduled lists - not a login browsed before.
await handleEditDraft(restored, { accountId: email.scheduledAccountId, localAccountId: activeAccountId ?? undefined }).catch(reportDraftOpenFailure);
return;
}
setShowComposer(true);
@@ -4222,8 +4225,23 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const targetPane = hasSplit
? (sourcePane === 'main' ? 'split' : 'main')
: sourcePane;
// The tab reads the row again through the login and account
// it was listed from, and a draft reopened there is saved
// over and removed through them.
const source = listRowSource(email, {
isScheduledView,
viewingAccountId,
activeAccountId,
accountId: resolveUnstampedEmailAccountId({
mailboxes: viewMailboxes,
selectedMailbox,
searchActive: !!searchQuery || !isFilterEmpty(searchFilters),
searchMailboxId,
}),
});
store.openEmailTab({
accountId: email.accountId ?? '',
accountId: source.accountId ?? '',
clientAccountId: source.localAccountId,
emailId: email.id,
mailboxId: selectedMailbox,
title: email.subject?.trim() || t('email_composer.new_message'),
@@ -4393,8 +4411,9 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
setPendingDraft(data);
}}
onSend={async (data) => {
await handleEmailSend(data);
const outcome = await handleEmailSend(data);
setPendingDraft(null);
return outcome;
}}
onScheduledSendCreated={async () => {
if (client) {
@@ -4541,7 +4560,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
setShowComposer(true);
return;
}
if (restored) await handleEditDraft(restored, { accountId: selectedEmail.scheduledAccountId }).catch(reportDraftOpenFailure);
// Restored through the active login's client (see the list's Cancel and edit).
if (restored) await handleEditDraft(restored, { accountId: selectedEmail.scheduledAccountId, localAccountId: activeAccountId ?? undefined }).catch(reportDraftOpenFailure);
}}
onRescheduleScheduled={async (delayedUntil) => {
if (client && selectedEmail?.emailSubmissionId && selectedEmail.scheduledIdentityId) {
@@ -13,13 +13,13 @@ import React from 'react';
// The composer itself is irrelevant here - stub it down to a button that
// fires the onSend payload the real composer builds for a cross-account
// identity (raw identity id + owning local account id).
const payloadRef = vi.hoisted(() => ({ current: {} as Record<string, unknown> }));
const payloadRef = vi.hoisted(() => ({ current: {} as Record<string, unknown>, outcome: null as Promise<unknown> | null }));
vi.mock('@/components/email/email-composer', () => ({
EmailComposer: ({ onSend }: { onSend?: (d: Record<string, unknown>) => Promise<void> }) =>
EmailComposer: ({ onSend }: { onSend?: (d: Record<string, unknown>) => Promise<unknown> }) =>
React.createElement(
'button',
{ onClick: () => { void onSend?.(payloadRef.current); } },
{ onClick: () => { payloadRef.outcome = onSend?.(payloadRef.current) ?? null; } },
'send',
),
}));
@@ -107,6 +107,7 @@ vi.mock('@/stores/pro-tab-store', () => {
});
import { ProComposeTabBody } from '../pro-compose-tab-body';
import { useAuthStore } from '@/stores/auth-store';
const TAB_DATA = { sessionId: 1, mode: 'compose' as const, title: 'New message' };
@@ -159,4 +160,23 @@ describe('Pro compose tab - cross-account send routing (#461)', () => {
await waitFor(() => expect(sendEmail).toHaveBeenCalled());
expect(sendEmail.mock.calls[0][0]).toBe(activeClient);
});
// The composer removes a draft version saved through another login only
// once the message is out; without a connection nothing went.
it('tells the composer nothing was sent when there is no connection', async () => {
const auth = useAuthStore.getState() as unknown as { client: unknown };
const connected = auth.client;
auth.client = null;
try {
payloadRef.current = { ...basePayload, localAccountId: 'local-2' };
render(<ProComposeTabBody tabId="tab-1" data={TAB_DATA} />);
fireEvent.click(screen.getByText('send'));
await expect(payloadRef.outcome).resolves.toBe(false);
expect(sendEmail).not.toHaveBeenCalled();
} finally {
auth.client = connected;
}
});
});
@@ -42,13 +42,21 @@ function selectorStore<T extends object>(state: T) {
return hook;
}
vi.mock('@/stores/auth-store', () => ({
useAuthStore: selectorStore({ client: null, getClientForAccount: () => undefined }),
// userb on the 0.16.23 server is the active login.
const auth = vi.hoisted(() => ({
state: { client: null as unknown, activeAccountId: 'userb@v016', getClientForAccount: () => undefined },
}));
vi.mock('@/stores/auth-store', () => ({
useAuthStore: selectorStore(auth.state),
}));
const store = vi.hoisted(() => ({ fetched: null as Record<string, unknown> | null, fetchEmailContent: vi.fn() }));
vi.mock('@/stores/email-store', () => ({
useEmailStore: selectorStore({
// An own-folder message is read through the store, not by account.
fetchEmailContent: async (_client: unknown, id: string) => ({ ...DRAFT, id, mailboxIds: { d: true } }),
fetchEmailContent: async (client: unknown, id: string) => {
store.fetchEmailContent(client, id);
return store.fetched ?? ({ ...DRAFT, id, mailboxIds: { d: true } });
},
deleteEmail: async () => {},
markAsRead: async () => {},
toggleStar: async () => {},
@@ -86,6 +94,9 @@ describe('Pro email view - editing a draft', () => {
viewerProps.current = null;
openComposeTab.mockClear();
multiAccount.value = { enabled: false, groups: [], allIdentities: [] };
auth.state.client = null;
store.fetched = null;
store.fetchEmailContent.mockClear();
});
async function openAndEdit(accountId?: string, clientAccountId?: string, draft: object = DRAFT) {
@@ -127,4 +138,42 @@ describe('Pro email view - editing a draft', () => {
expect(initialData.draftId).toBe('eaaaaab');
expect(initialData.draftAccountId).toBeUndefined();
});
// Pro shell with the 0.16.23 login active and the 1.0 login's Drafts
// browsed: the store reads a row of that list through the 1.0 login. A
// view that names no login can't tell, so it names none - the active one
// made the composer destroy the draft's id in the 0.16.23 account.
it('names no login for a draft of a view without a login of its own', async () => {
auth.state.client = { getEmail: vi.fn(), getUsername: () => 'userb@example.org' };
multiAccount.value = {
enabled: true,
groups: [],
allIdentities: [
{ id: 'userb@v016::c', email: 'userb@example.org', name: 'userb' },
{ id: 'userb@v10::d', email: 'me@v10.example.org', name: 'Me' },
],
};
store.fetched = { ...DRAFT, from: [{ email: 'me@v10.example.org', name: 'Me' }] };
render(<ProEmailView emailId="eaaaaab" onClose={vi.fn()} />);
await waitFor(() => expect(viewerProps.current?.email).toBeTruthy());
(viewerProps.current!.onEditDraft as () => void)();
const initialData = openComposeTab.mock.calls[0][0].initialData as Record<string, unknown>;
expect(initialData.draftLocalAccountId).toBeUndefined();
// The draft's From decides, as for a draft of unknown origin.
expect(initialData.selectedIdentityId).toBe('userb@v10::d');
});
it('reads a message of the login it names through that login\'s client, not through the main list', async () => {
const client = { getEmail: vi.fn().mockResolvedValue(DRAFT), getUsername: () => 'userb@example.org' };
render(<ProEmailView emailId="eaaaaab" client={client as never} clientAccountId="userb@v10" onClose={vi.fn()} />);
await waitFor(() => expect(viewerProps.current?.email).toBeTruthy());
expect(client.getEmail).toHaveBeenCalledWith('eaaaaab', undefined);
expect(store.fetchEmailContent).not.toHaveBeenCalled();
(viewerProps.current!.onEditDraft as () => void)();
expect(openComposeTab.mock.calls[0][0].initialData).toMatchObject({ draftLocalAccountId: 'userb@v10', draftAccountId: undefined });
});
});
@@ -0,0 +1,103 @@
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import React from 'react';
/**
* A Pro folder tab lists one folder through the client of the login it
* belongs to. The message is read again through that client - in its
* reading pane or in an email tab - and a draft reopened there is saved
* over and removed through that login: userb is account `e` on the 0.16.23
* and the 1.0 test server alike, and message ids repeat across both.
*/
const viewProps = vi.hoisted(() => ({ current: null as Record<string, unknown> | null }));
vi.mock('@/components/pro/pro-email-tab-body', () => ({
ProEmailView: (props: Record<string, unknown>) => {
viewProps.current = props;
return null;
},
}));
vi.mock('@/components/email/email-viewer', () => ({ EmailViewer: () => null }));
vi.mock('next-intl', () => ({ useTranslations: () => (key: string) => key }));
vi.mock('@/hooks/use-media-query', () => ({ useDeviceDetection: () => ({ isMobile: false }) }));
vi.mock('@/hooks/use-pane-context', () => ({ usePaneId: () => 'main' }));
vi.mock('@/lib/utils', async (importOriginal) => ({
...await importOriginal<typeof import('@/lib/utils')>(),
formatDate: () => 'today',
}));
function selectorStore<T extends object>(state: T) {
const hook = (sel?: (s: T) => unknown) => (typeof sel === 'function' ? sel(state) : state);
hook.getState = () => state;
return hook;
}
const row = { id: 'eaaaaab', threadId: 't', subject: 'Draft', receivedAt: '2026-09-26T10:00:00Z', keywords: { $draft: true, $seen: true }, mailboxIds: { d: true } };
const makeClient = () => ({ getEmails: vi.fn().mockResolvedValue({ emails: [row], total: 1, hasMore: false }), markAsRead: vi.fn() });
const clients = vi.hoisted(() => ({ v016: null as unknown, v10: null as unknown }));
vi.mock('@/stores/auth-store', () => ({
useAuthStore: selectorStore({
get client() { return clients.v016; },
activeAccountId: 'userb@v016',
getClientForAccount: (id: string) => (id === 'userb@v016' ? clients.v016 : id === 'userb@v10' ? clients.v10 : undefined),
}),
}));
// The active login's list holds its own Drafts `d` and the team's `c:d`; the
// 1.0 login's list its own Drafts, under the same id.
const folders = vi.hoisted(() => {
const drafts = (id: string, extra: Record<string, unknown> = {}) => ({ id, name: 'Drafts', role: 'drafts', totalEmails: 1, unreadEmails: 0, ...extra });
return {
mailboxes: [drafts('d', { accountId: 'e' }), drafts('c:d', { originalId: 'd', accountId: 'c', isShared: true })],
accountMailboxes: { 'userb@v10': [drafts('d', { accountId: 'e' })] },
};
});
vi.mock('@/stores/email-store', () => ({ useEmailStore: selectorStore(folders) }));
vi.mock('@/stores/settings-store', () => ({
useSettingsStore: selectorStore({ emailsPerPage: 50 }),
getMessageListOrderFor: () => [],
}));
const tabs = vi.hoisted(() => ({ openEmailTab: vi.fn(), updateTabTitle: vi.fn() }));
vi.mock('@/stores/pro-tab-store', () => ({ useProTabStore: selectorStore(tabs) }));
import { ProFolderTabBody } from '../pro-folder-tab-body';
describe('Pro folder tab - the login a message is read and edited through', () => {
beforeEach(() => {
viewProps.current = null;
tabs.openEmailTab.mockClear();
clients.v016 = makeClient();
clients.v10 = makeClient();
});
async function openTab(accountId: string | null, mailboxId: string) {
render(<ProFolderTabBody tabId="tab-1" data={{ accountId, mailboxId, title: 'Drafts' }} />);
return waitFor(() => screen.getByText('Draft'));
}
it('names the active login for a message of its own folder in the reading pane', async () => {
fireEvent.click(await openTab(null, 'd'));
expect(viewProps.current).toMatchObject({ emailId: 'eaaaaab', client: clients.v016, clientAccountId: 'userb@v016', accountId: undefined });
});
it('opens a message of another login\'s folder in an email tab with that login', async () => {
fireEvent.doubleClick(await openTab('userb@v10', 'd'));
expect((clients.v10 as ReturnType<typeof makeClient>).getEmails).toHaveBeenCalled();
expect(tabs.openEmailTab).toHaveBeenCalledWith(
expect.objectContaining({ emailId: 'eaaaaab', clientAccountId: 'userb@v10', accountId: '' }),
expect.anything(),
);
});
it('opens a message of the active login\'s group folder in an email tab with the group account', async () => {
fireEvent.doubleClick(await openTab(null, 'c:d'));
expect(tabs.openEmailTab).toHaveBeenCalledWith(
expect.objectContaining({ emailId: 'eaaaaab', clientAccountId: 'userb@v016', accountId: 'c' }),
expect.anything(),
);
});
});
+2 -1
View File
@@ -55,7 +55,8 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) {
}, [client, refreshScheduledMetadata, isScheduledView, fetchScheduledEmails, closeTab]);
const handleSend = useCallback(async (sendData: Parameters<NonNullable<React.ComponentProps<typeof EmailComposer>['onSend']>>[0]) => {
if (!client) return;
// Nothing is sent: the composer keeps what only goes once the mail is out.
if (!client) return false;
// The Pro From dropdown lists identities from every connected account, so
// the chosen identity may belong to a different account than the active
// one. Submit through that account's own client - the identity id is only
+18 -11
View File
@@ -33,8 +33,10 @@ interface ProEmailViewProps {
*/
client?: IJMAPClient | null;
/**
* The connected account `client` belongs to; absent, the active one. A
* draft reopened from here is saved over and removed through it.
* The connected account `client` belongs to. Given, the message is read
* through `client` and a draft reopened from here is saved over and removed
* through that login. Absent, nothing says which login the message came
* through, and the composer goes by the draft's From.
*/
clientAccountId?: string;
/**
@@ -85,9 +87,11 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId,
const tNotifications = useTranslations('notifications');
const activeClient = useAuthStore((s) => s.client);
const activeAccountId = useAuthStore((s) => s.activeAccountId);
const client = clientOverride ?? activeClient;
const localAccountId = (clientOverride ? clientAccountId : undefined) ?? activeAccountId ?? undefined;
// Only a login the host names: without one the message is read through
// fetchEmailContent, which routes by the main list (the login browsed
// there, a row with the same id) - not necessarily the active login.
const localAccountId = clientOverride ? clientAccountId : undefined;
const fetchEmailContent = useEmailStore((s) => s.fetchEmailContent);
const deleteEmail = useEmailStore((s) => s.deleteEmail);
const markAsRead = useEmailStore((s) => s.markAsRead);
@@ -113,7 +117,9 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId,
let cancelled = false;
if (!client) return;
setIsLoading(true);
(accountId ? client.getEmail(emailId, accountId) : fetchEmailContent(client, emailId))
// A named login is read through its own client, so the login a draft
// reopened from here is stamped with is the one it was read through.
(accountId || localAccountId ? client.getEmail(emailId, accountId) : fetchEmailContent(client, emailId))
.then((loaded) => {
if (cancelled) return;
setEmail(loaded);
@@ -128,7 +134,7 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId,
return () => {
cancelled = true;
};
}, [client, emailId, accountId, fetchEmailContent]);
}, [client, emailId, accountId, localAccountId, fetchEmailContent]);
const currentMailboxRole = useMemo(() => {
if (!email) return undefined;
@@ -367,8 +373,8 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId,
mode: 'compose',
draftId: email.id,
// A draft in a shared or group folder is removed there when the
// composer replaces, sends or discards it, through this view's
// login (see ComposerDraftData).
// composer replaces, sends or discards it, through the login this
// view names (see ComposerDraftData).
draftAccountId: accountId,
draftLocalAccountId: localAccountId,
// The draft's already-uploaded parts - the classic path carries them
@@ -435,8 +441,9 @@ export function ProEmailTabBody({ tabId, data }: ProEmailTabBodyProps) {
return mb?.isShared ? mb.accountId : undefined;
}, [mailboxes, data.mailboxId]);
// A hit opened from another login (global search) is fetched through that
// login's client, targeting the owner accountId stamped on the hit (#847).
// A message opened with its login (a list row, a search hit of another
// login) is fetched through that login's client, targeting the owner
// accountId it came with (#847).
const clientOverride = data.clientAccountId ? getClientForAccount(data.clientAccountId) ?? null : null;
return (
@@ -444,7 +451,7 @@ export function ProEmailTabBody({ tabId, data }: ProEmailTabBodyProps) {
emailId={data.emailId}
client={clientOverride ?? undefined}
clientAccountId={clientOverride ? data.clientAccountId : undefined}
accountId={clientOverride ? data.accountId : ownerAccountId}
accountId={clientOverride ? (data.accountId || undefined) : ownerAccountId}
onLoaded={handleLoaded}
onClose={handleClose}
className="w-full"
+9 -3
View File
@@ -36,6 +36,7 @@ export function ProFolderTabBody({ tabId, data }: ProFolderTabBodyProps) {
const tSidebar = useTranslations('sidebar');
const activeClient = useAuthStore((s) => s.client);
const activeAccountId = useAuthStore((s) => s.activeAccountId);
const getClientForAccount = useAuthStore((s) => s.getClientForAccount);
const activeMailboxes = useEmailStore((s) => s.mailboxes);
const accountMailboxes = useEmailStore((s) => s.accountMailboxes);
@@ -49,6 +50,10 @@ export function ProFolderTabBody({ tabId, data }: ProFolderTabBodyProps) {
() => (data.accountId ? getClientForAccount(data.accountId) ?? activeClient : activeClient),
[data.accountId, getClientForAccount, activeClient],
);
// The login `client` belongs to. A message of this folder is read again
// through it, and a draft reopened from it is saved over and removed
// through it: the same ids name other messages on another server.
const clientLogin = (data.accountId && getClientForAccount(data.accountId) ? data.accountId : activeAccountId) ?? undefined;
// The folder's metadata, resolved from whichever account's mailbox list it
// was dragged from. Shared folders are namespaced in the store, so JMAP
@@ -126,14 +131,15 @@ export function ProFolderTabBody({ tabId, data }: ProFolderTabBodyProps) {
const openAsEmailTab = useCallback((email: Email, reuseReader: boolean) => {
useProTabStore.getState().openEmailTab(
{
accountId: email.accountId ?? '',
accountId: jmapAccountId ?? '',
clientAccountId: clientLogin,
emailId: email.id,
mailboxId: data.mailboxId,
title: email.subject?.trim() || t('email_composer.new_message'),
},
{ pane: paneId ?? 'main', reuseReader },
);
}, [data.mailboxId, paneId, t]);
}, [jmapAccountId, clientLogin, data.mailboxId, paneId, t]);
const handleRowClick = useCallback((email: Email) => {
markRowAsRead(email);
@@ -265,7 +271,7 @@ export function ProFolderTabBody({ tabId, data }: ProFolderTabBodyProps) {
key={selectedEmailId}
emailId={selectedEmailId}
client={client}
clientAccountId={data.accountId && getClientForAccount(data.accountId) ? data.accountId : undefined}
clientAccountId={clientLogin}
accountId={jmapAccountId}
onClose={handleViewerClose}
className="min-w-0 flex-1"
+55 -1
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest';
import { findDraftIdentityIdForLogin, resolveDraftSource } from '../draft-source';
import { findDraftIdentityIdForLogin, listRowSource, resolveDraftSource } from '../draft-source';
import type { Identity } from '../jmap/types';
// userb is account `e` on the Stalwart 0.16.23 and the 1.0 test server alike,
@@ -44,6 +44,60 @@ describe('resolveDraftSource', () => {
{ source: { accountId: 'c', localAccountId: 'userb@v10' }, aggregate: true, activeAccountId: 'userb@v016' },
)).toEqual({ localAccountId: 'userb@v10', accountId: 'c' });
});
// Pro shell: the 1.0 login's folder was browsed, then Scheduled opened; it
// lists the active 0.16.23 login's mail, and "Cancel and edit" restores the
// message through that login's client. The browsed login stays "viewed".
it('takes a scheduled send cancelled for editing from the login that restored it, not the one browsed before', () => {
expect(resolveDraftSource({}, {
source: { accountId: 'e', localAccountId: 'userb@v016' },
aggregate: false,
viewingAccountId: 'userb@v10',
activeAccountId: 'userb@v016',
})).toEqual({ localAccountId: 'userb@v016', accountId: 'e' });
});
it('leaves the login of a source that names none unset rather than take the login browsed before', () => {
expect(resolveDraftSource({}, {
source: { accountId: 'e' },
aggregate: false,
viewingAccountId: 'userb@v10',
activeAccountId: 'userb@v016',
})).toEqual({ localAccountId: undefined, accountId: 'e' });
});
it('never takes the folder selected underneath for a send undone from the own account', () => {
expect(resolveDraftSource({}, {
source: { localAccountId: 'userb@v016' },
aggregate: false,
mailbox: teamInbox,
activeAccountId: 'userb@v016',
})).toEqual({ localAccountId: 'userb@v016', accountId: undefined });
});
});
describe('listRowSource', () => {
it('takes a row of a list that mixes accounts from its stamps', () => {
expect(listRowSource(
{ sourceClientAccountId: 'userb@v10', sourceAccountId: 'c' },
{ isScheduledView: false, viewingAccountId: null, activeAccountId: 'userb@v016', accountId: undefined },
)).toEqual({ localAccountId: 'userb@v10', accountId: 'c' });
});
it('takes a row of another login\'s folder from the login browsed', () => {
expect(listRowSource({}, { isScheduledView: false, viewingAccountId: 'userb@v10', activeAccountId: 'userb@v016' }))
.toEqual({ localAccountId: 'userb@v10', accountId: undefined });
});
it('takes a row of the active login\'s group folder from the active login and the group account', () => {
expect(listRowSource({}, { isScheduledView: false, viewingAccountId: null, activeAccountId: 'userb@v016', accountId: 'c' }))
.toEqual({ localAccountId: 'userb@v016', accountId: 'c' });
});
it('names no login for a row of Scheduled, whatever login was browsed before', () => {
expect(listRowSource({}, { isScheduledView: true, viewingAccountId: 'userb@v10', activeAccountId: 'userb@v016' }))
.toEqual({});
});
});
describe('findDraftIdentityIdForLogin', () => {
+34 -6
View File
@@ -16,8 +16,11 @@ export interface DraftSource {
/**
* The DraftSource of `draft`, a message of the list on screen:
* - `source` when the caller knows it: a send undone or a scheduled send
* cancelled for editing is back in the account it was sent from;
* - `source` as it is when the caller knows it: a send undone or a scheduled
* send cancelled for editing is back in the account it was sent from, put
* there through the login `source` names (none: the composing one). What is
* on screen says nothing about it: Scheduled lists the active login's mail
* while a login browsed before still counts as viewed;
* - in a list that mixes accounts (the unified views, a tag view, an "All
* folders" search) the login and account the message is stamped with. The
* folder still selected underneath a tag view or a search says nothing
@@ -35,19 +38,44 @@ export function resolveDraftSource(
activeAccountId?: string | null;
},
): DraftSource {
if (context.source) {
return { localAccountId: context.source.localAccountId, accountId: context.source.accountId };
}
const mailbox = context.aggregate ? undefined : context.mailbox;
return {
localAccountId: context.source?.localAccountId
?? draft.sourceClientAccountId
localAccountId: draft.sourceClientAccountId
?? context.viewingAccountId
?? context.activeAccountId
?? undefined,
accountId: context.source?.accountId
?? draft.sourceAccountId
accountId: draft.sourceAccountId
?? (mailbox?.isShared ? mailbox.accountId : undefined),
};
}
/**
* Where a row of the mail list was listed from, for a view that reads it
* again on its own (a Pro email tab): the login and account the row is
* stamped with in a list that mixes accounts, else the login browsed - the
* active one when no other is - and `list.accountId`, the account the list
* shows (a shared or group folder's). Nothing for a row of Scheduled: that
* list is the active login's, whatever login was browsed before.
*/
export function listRowSource(
row: Pick<Email, 'sourceClientAccountId' | 'sourceAccountId'>,
list: {
isScheduledView: boolean;
viewingAccountId?: string | null;
activeAccountId?: string | null;
accountId?: string;
},
): DraftSource {
if (row.sourceClientAccountId && row.sourceAccountId) {
return { localAccountId: row.sourceClientAccountId, accountId: row.sourceAccountId };
}
if (list.isScheduledView) return {};
return { localAccountId: list.viewingAccountId ?? list.activeAccountId ?? undefined, accountId: list.accountId };
}
/**
* The identity to reopen a draft with (findDraftIdentityId), taken from the
* identities of the login the draft lives in first: two logins can share an
+15
View File
@@ -164,6 +164,21 @@ describe('openEmailTab', () => {
assertInvariants();
});
// userb is account `e` on the 0.16.23 and the 1.0 test server, and message
// ids repeat across both: the same id and account on two logins are two
// messages.
it('opens the same id and account of another login in a tab of its own', () => {
const open = (clientAccountId: string) => useProTabStore.getState().openEmailTab({
accountId: 'e', clientAccountId, emailId: 'eaaaaab', mailboxId: null, title: clientAccountId,
});
const first = open('userb@v016');
const second = open('userb@v10');
expect(second).not.toBe(first);
expect(open('userb@v016')).toBe(first);
assertInvariants();
});
it("pane: 'split' with no split creates one", () => {
const id = useProTabStore.getState().openEmailTab(data(2), { pane: 'split' });
const s = useProTabStore.getState();
+6 -3
View File
@@ -66,9 +66,10 @@ export interface ProEmailTabData {
mailboxId: string | null;
title: string;
/**
* Connected-account id (login) whose client fetches this email; unset = the
* active account. Set when a hit from another login is opened (global
* search) - bare email ids are ambiguous across logins (#847).
* Connected-account id (login) whose client fetches this email, with
* `accountId` the JMAP account there; unset = the active account. Set for
* a list row and a hit from another login (global search) - bare email ids
* are ambiguous across logins (#847).
*/
clientAccountId?: string;
}
@@ -414,10 +415,12 @@ export const useProTabStore = create<ProTabState>()(
const state = core(get());
// Same email already open anywhere: focus it, never open twice.
// Of the same login: ids and account ids repeat across servers.
const existing = state.tabs.find(
(tab) => tab.kind === 'email'
&& tab.emailData?.emailId === data.emailId
&& tab.emailData?.accountId === data.accountId
&& tab.emailData?.clientAccountId === data.clientAccountId
);
if (existing) {
commit((s) => withTabActivated(s, existing));