diff --git a/components/email/__tests__/composer-draft-login.test.tsx b/components/email/__tests__/composer-draft-login.test.tsx index 5558f84be..06b45e631 100644 --- a/components/email/__tests__/composer-draft-login.test.tsx +++ b/components/email/__tests__/composer-draft-login.test.tsx @@ -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 diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 51d4b37d6..2ca95131f 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -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 | false | Promise; onScheduledSendCreated?: () => void | Promise; 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)) { diff --git a/components/global-search/__tests__/search-preview.test.tsx b/components/global-search/__tests__/search-preview.test.tsx new file mode 100644 index 000000000..aca9ef278 --- /dev/null +++ b/components/global-search/__tests__/search-preview.test.tsx @@ -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 | null })); +vi.mock('@/components/pro/pro-email-tab-body', () => ({ + ProEmailView: (props: Record) => { + 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(); + + expect(viewProps.current).toMatchObject({ client: clients.v016, clientAccountId: 'userb@v016', accountId: 'e' }); + }); + + it('names the other login for a hit of another login', () => { + render(); + + expect(viewProps.current).toMatchObject({ client: clients.v10, clientAccountId: 'userb@v10', accountId: 'e' }); + }); +}); diff --git a/components/global-search/search-preview.tsx b/components/global-search/search-preview.tsx index 1052df573..2b519e1ae 100644 --- a/components/global-search/search-preview.tsx +++ b/components/global-search/search-preview.tsx @@ -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 ( { - 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) { diff --git a/components/pro/__tests__/cross-account-send-routing.test.tsx b/components/pro/__tests__/cross-account-send-routing.test.tsx index 4561119ae..90dc2ad4c 100644 --- a/components/pro/__tests__/cross-account-send-routing.test.tsx +++ b/components/pro/__tests__/cross-account-send-routing.test.tsx @@ -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 })); +const payloadRef = vi.hoisted(() => ({ current: {} as Record, outcome: null as Promise | null })); vi.mock('@/components/email/email-composer', () => ({ - EmailComposer: ({ onSend }: { onSend?: (d: Record) => Promise }) => + EmailComposer: ({ onSend }: { onSend?: (d: Record) => Promise }) => 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(); + + fireEvent.click(screen.getByText('send')); + + await expect(payloadRef.outcome).resolves.toBe(false); + expect(sendEmail).not.toHaveBeenCalled(); + } finally { + auth.client = connected; + } + }); }); diff --git a/components/pro/__tests__/pro-email-edit-draft.test.tsx b/components/pro/__tests__/pro-email-edit-draft.test.tsx index 69d6883c9..070f464db 100644 --- a/components/pro/__tests__/pro-email-edit-draft.test.tsx +++ b/components/pro/__tests__/pro-email-edit-draft.test.tsx @@ -42,13 +42,21 @@ function selectorStore(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 | 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(); + await waitFor(() => expect(viewerProps.current?.email).toBeTruthy()); + (viewerProps.current!.onEditDraft as () => void)(); + const initialData = openComposeTab.mock.calls[0][0].initialData as Record; + + 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(); + 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 }); + }); }); diff --git a/components/pro/__tests__/pro-folder-tab-draft-login.test.tsx b/components/pro/__tests__/pro-folder-tab-draft-login.test.tsx new file mode 100644 index 000000000..0db466a36 --- /dev/null +++ b/components/pro/__tests__/pro-folder-tab-draft-login.test.tsx @@ -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 | null })); +vi.mock('@/components/pro/pro-email-tab-body', () => ({ + ProEmailView: (props: Record) => { + 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(), + formatDate: () => 'today', +})); + +function selectorStore(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 = {}) => ({ 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(); + 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).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(), + ); + }); +}); diff --git a/components/pro/pro-compose-tab-body.tsx b/components/pro/pro-compose-tab-body.tsx index 64869243c..fd44d9a4f 100644 --- a/components/pro/pro-compose-tab-body.tsx +++ b/components/pro/pro-compose-tab-body.tsx @@ -55,7 +55,8 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) { }, [client, refreshScheduledMetadata, isScheduledView, fetchScheduledEmails, closeTab]); const handleSend = useCallback(async (sendData: Parameters['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 diff --git a/components/pro/pro-email-tab-body.tsx b/components/pro/pro-email-tab-body.tsx index d3d98bc5f..71e487217 100644 --- a/components/pro/pro-email-tab-body.tsx +++ b/components/pro/pro-email-tab-body.tsx @@ -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" diff --git a/components/pro/pro-folder-tab-body.tsx b/components/pro/pro-folder-tab-body.tsx index 58e38237a..f98ce4dfe 100644 --- a/components/pro/pro-folder-tab-body.tsx +++ b/components/pro/pro-folder-tab-body.tsx @@ -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" diff --git a/lib/__tests__/draft-source.test.ts b/lib/__tests__/draft-source.test.ts index 368bc941f..2f6ee5094 100644 --- a/lib/__tests__/draft-source.test.ts +++ b/lib/__tests__/draft-source.test.ts @@ -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', () => { diff --git a/lib/draft-source.ts b/lib/draft-source.ts index 9b7522d3a..832485063 100644 --- a/lib/draft-source.ts +++ b/lib/draft-source.ts @@ -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, + 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 diff --git a/stores/__tests__/pro-tab-store.test.ts b/stores/__tests__/pro-tab-store.test.ts index 2fe352e1c..c473f08a3 100644 --- a/stores/__tests__/pro-tab-store.test.ts +++ b/stores/__tests__/pro-tab-store.test.ts @@ -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(); diff --git a/stores/pro-tab-store.ts b/stores/pro-tab-store.ts index 017ee8e1a..6ad726fd8 100644 --- a/stores/pro-tab-store.ts +++ b/stores/pro-tab-store.ts @@ -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()( 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));