diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index 87d73e2f0..e89a5f625 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -140,13 +140,14 @@ export function EmailList({ // grouping mirrors the server order instead of re-sorting the page by date. // Search results and cross-account views are always chronological. const fetchedListOrder = useEmailStore((state) => state.listOrder); + const keepServerTextOrder = useEmailStore((state) => state.listKeepsServerTextOrder); const crossView = useEmailStore((state) => state.crossView); const threadGroups = useMemo(() => { const listOrder = searchQuery || crossView || !isFilterEmpty(searchFilters) ? [] : fetchedListOrder; const groups = groupEmailsByThread(emails, disableThreading || isScheduledView, threadEmailCounts); - return sortThreadGroups(groups, listOrder); - }, [emails, disableThreading, isScheduledView, threadEmailCounts, fetchedListOrder, searchQuery, crossView, searchFilters]); + return sortThreadGroups(groups, listOrder, { keepServerTextOrder }); + }, [emails, disableThreading, isScheduledView, threadEmailCounts, fetchedListOrder, keepServerTextOrder, searchQuery, crossView, searchFilters]); const { contextMenu, openContextMenu, closeContextMenu, menuRef } = useContextMenu(); /** diff --git a/lib/__tests__/jmap-list-order.test.ts b/lib/__tests__/jmap-list-order.test.ts index e8c633af2..ea6d21ca8 100644 --- a/lib/__tests__/jmap-list-order.test.ts +++ b/lib/__tests__/jmap-list-order.test.ts @@ -12,10 +12,10 @@ type MethodCall = [string, Record, string]; const UNREAD_FIRST: SortLevel[] = [{ criterion: 'unread', direction: 'desc' }]; -function makeSession(mailCaps: Record = {}) { +function makeSession(mailCaps: Record = {}, accountCaps: Record = {}) { return { capabilities: { 'urn:ietf:params:jmap:core': {} }, - accounts: { 'acct-1': { name: 'test', isPersonal: true, accountCapabilities: { 'urn:ietf:params:jmap:mail': mailCaps } } }, + accounts: { 'acct-1': { name: 'test', isPersonal: true, accountCapabilities: { 'urn:ietf:params:jmap:mail': mailCaps, ...accountCaps } } }, primaryAccounts: { 'urn:ietf:params:jmap:mail': 'acct-1' }, apiUrl: 'https://mail.example.com/jmap/api', downloadUrl: 'https://mail.example.com/jmap/download/{accountId}/{blobId}/{name}', @@ -75,8 +75,8 @@ describe('JMAPClient list ordering', () => { vi.restoreAllMocks(); }); - async function connectedClient(mailCaps?: Record): Promise { - fetchSpy.mockResolvedValueOnce(jsonResponse(makeSession(mailCaps))); + async function connectedClient(mailCaps?: Record, accountCaps?: Record): Promise { + fetchSpy.mockResolvedValueOnce(jsonResponse(makeSession(mailCaps, accountCaps))); const client = JMAPClient.withBearer('https://mail.example.com', 'token123', 'user@test.com'); await client.connect(); fetchSpy.mockReset(); @@ -192,17 +192,30 @@ describe('JMAPClient list ordering', () => { ]); }); - describe('sender and subject orders keep the server order', () => { - const sender = (id: string, name: string, address: string, receivedAt: string) => ({ + describe('sender and subject orders', () => { + // Every Stalwart 1.0 account lists principals:owner in its + // accountCapabilities (api/session.rs jmap_account); 0.16 lists it nowhere. + const STALWART_1_0 = { + 'urn:ietf:params:jmap:principals:owner': { accountIdForPrincipal: 'acct-1', principalId: 'acct-1' }, + }; + const message = (id: string, name: string, address: string, subject: string, receivedAt = '2026-01-01T00:00:00Z') => ({ id, threadId: `t-${id}`, keywords: { $seen: true }, receivedAt, mailboxIds: {}, size: 1, hasAttachment: false, - from: address ? [{ name, email: address }] : [], + from: address ? [{ name, email: address }] : [], subject, }); - const zoe = sender('zoe', 'Zoe', 'zoe@example.org', '2026-01-01T00:00:00Z'); - const emile = sender('emile', 'Émile', 'emile@example.org', '2026-01-02T00:00:00Z'); - const nobody = sender('nobody', '', '', '2026-01-03T00:00:00Z'); + type Message = ReturnType; + const zoe = message('zoe', 'Zoe', 'zoe@example.org', 'Zebra report', '2026-01-01T00:00:00Z'); + const emile = message('emile', 'Émile', 'emile@example.org', 'Éclair', '2026-01-02T00:00:00Z'); + const nobody = message('nobody', '', '', '', '2026-01-03T00:00:00Z'); + // The senders and subjects of a live check against both servers. + const adam = message('adam', 'Adam', 'adam@x.test', 'Banana split'); + const bea = message('bea', 'bea', 'bea@x.test', 'aardvark'); + const carla = message('carla', 'Carla', 'carla@x.test', 'Mango'); + const dave = message('dave', 'dave', 'dave@x.test', 'apple pie'); + const eve = message('eve', 'Eve', 'eve@x.test', 'Re: cherry tart'); + const zed = message('zed', 'zed', 'zed@x.test', 'Zebra report'); /** A folder query answered in `queryOrder`, with Email/get listing the objects in `getOrder`. */ - function replyInOrder(queryOrder: typeof zoe[], getOrder: typeof zoe[]) { + function replyInOrder(queryOrder: Message[], getOrder: Message[] = [...queryOrder].reverse()) { return (methodCalls: MethodCall[]) => ({ methodResponses: [ ['Email/query', { ids: queryOrder.map((e) => e.id), total: queryOrder.length }, methodCalls[0][2]], @@ -210,44 +223,63 @@ describe('JMAPClient list ordering', () => { ], }); } + const listed = async (client: JMAPClient, order: SortLevel[]) => + (await client.getEmails('inbox', undefined, 50, 0, undefined, false, undefined, order)).emails.map((e) => e.id); - it('keeps Stalwart 1.0 byte order with the empty sender last', async () => { + it('keeps the sender order of Stalwart 1.0: lowercased byte order, the empty sender last', async () => { // 1.0 ranks lowercased "name address" keys by bytes, so "zoe ..." sorts - // before "émile ..." and a message without a sender comes last. The - // client comparator (localeCompare, empty first) would reverse all three. - const client = await connectedClient(); + // before "émile ...". A client re-sort (localeCompare) would swap them. + const client = await connectedClient({}, STALWART_1_0); const sent = recordRequests(replyInOrder([zoe, emile, nobody], [nobody, emile, zoe])); - const { emails } = await client.getEmails('inbox', undefined, 50, 0, undefined, false, undefined, [{ criterion: 'from', direction: 'asc' }]); - + expect(await listed(client, [{ criterion: 'from', direction: 'asc' }])).toEqual(['zoe', 'emile', 'nobody']); expect(folderQueries(sent)).toEqual([[ { property: 'from', isAscending: true }, { property: 'receivedAt', isAscending: false }, ]]); - expect(emails.map((e) => e.id)).toEqual(['zoe', 'emile', 'nobody']); }); - it('keeps whatever order Stalwart 0.16 returns when a subject level follows another', async () => { - // 0.16 leaves the collation to its search store. - const client = await connectedClient(); - recordRequests(replyInOrder([emile, nobody, zoe], [zoe, nobody, emile])); + it('keeps the subject order of Stalwart 1.0 behind another level', async () => { + const client = await connectedClient({}, STALWART_1_0); + recordRequests(replyInOrder([zed, emile, nobody])); - const { emails } = await client.getEmails('inbox', undefined, 50, 0, undefined, false, undefined, [ + expect(await listed(client, [ { criterion: 'size', direction: 'desc' }, { criterion: 'subject', direction: 'asc' }, - ]); - - expect(emails.map((e) => e.id)).toEqual(['emile', 'nobody', 'zoe']); + ])).toEqual(['zed', 'emile', 'nobody']); }); - it('still re-sorts a page ordered without text levels', async () => { - // The safety net for servers that ignore the sort stays for everything else. + it('re-sorts a sender page of Stalwart 0.16 case-insensitively, the empty sender last', async () => { + // 0.16 compares the raw bytes: capitals first, "Eve" before "bea". const client = await connectedClient(); - recordRequests(replyInOrder([zoe, emile, nobody], [zoe, emile, nobody])); + recordRequests(replyInOrder([adam, carla, eve, bea, dave, zed, nobody])); - const { emails } = await client.getEmails('inbox', undefined, 50, 0, undefined, false, undefined, [{ criterion: 'receivedAt', direction: 'desc' }]); + expect(await listed(client, [{ criterion: 'from', direction: 'asc' }])) + .toEqual(['adam', 'bea', 'carla', 'dave', 'eve', 'zed', 'nobody']); + }); - expect(emails.map((e) => e.id)).toEqual(['nobody', 'emile', 'zoe']); + it('re-sorts a subject page of Stalwart 0.16 on the thread name, the empty subject last', async () => { + // 0.16 sorts the thread name ("cherry tart" for "Re: cherry tart") by raw + // bytes; both versions put a message without a subject last, descending too. + const client = await connectedClient(); + recordRequests(replyInOrder([adam, carla, zed, bea, dave, eve, nobody])); + + expect(await listed(client, [{ criterion: 'subject', direction: 'asc' }])) + .toEqual(['bea', 'dave', 'adam', 'eve', 'carla', 'zed', 'nobody']); + + recordRequests(replyInOrder([eve, dave, bea, zed, carla, adam, nobody])); + expect(await listed(client, [{ criterion: 'subject', direction: 'desc' }])) + .toEqual(['zed', 'carla', 'eve', 'adam', 'dave', 'bea', 'nobody']); + }); + + it('still re-sorts a page ordered without text levels, on 0.16 and 1.0', async () => { + // The safety net for servers that ignore the sort stays for everything else. + for (const accountCaps of [{}, STALWART_1_0]) { + const client = await connectedClient({}, accountCaps); + recordRequests(replyInOrder([zoe, emile, nobody], [zoe, emile, nobody])); + + expect(await listed(client, [{ criterion: 'receivedAt', direction: 'desc' }])).toEqual(['nobody', 'emile', 'zoe']); + } }); }); }); diff --git a/lib/__tests__/message-list-order.test.ts b/lib/__tests__/message-list-order.test.ts index 86f3cc683..db58cf160 100644 --- a/lib/__tests__/message-list-order.test.ts +++ b/lib/__tests__/message-list-order.test.ts @@ -215,6 +215,21 @@ describe('compareEmails', () => { expect([a, b, c].sort(compareEmails([{ criterion: 'size', direction: 'desc' }])).map(e => e.id)).toEqual(['b', 'c', 'a']); }); + it('puts a message without a sender or subject last in both directions, as Stalwart 0.16 and 1.0 do', () => { + const named = makeEmail({ id: 'named', from: [{ name: 'Eve', email: 'eve@x.test' }], subject: 'Mango' }); + const lower = makeEmail({ id: 'lower', from: [{ name: 'bea', email: 'bea@x.test' }], subject: 'apple pie' }); + const bare = makeEmail({ id: 'bare', from: [], subject: undefined }); + // A subject that is nothing but a reply prefix has an empty thread name. + const onlyPrefix = makeEmail({ id: 'only-prefix', from: [{ name: '', email: '' }], subject: 'Re:', receivedAt: '2025-01-01T00:00:00Z' }); + + for (const criterion of ['from', 'subject'] as const) { + expect([bare, named, onlyPrefix, lower].sort(compareEmails([{ criterion, direction: 'asc' }])).map(e => e.id)) + .toEqual(['lower', 'named', 'bare', 'only-prefix']); + expect([bare, lower, onlyPrefix, named].sort(compareEmails([{ criterion, direction: 'desc' }])).map(e => e.id)) + .toEqual(['named', 'lower', 'bare', 'only-prefix']); + } + }); + it('applies levels in priority order', () => { const starredOld = makeEmail({ id: 'starred-old', receivedAt: '2026-01-01T00:00:00Z', keywords: { $flagged: true, $seen: true } }); const sorted = [readNewest, starredOld, unreadNew].sort(compareEmails([ diff --git a/lib/__tests__/message-list-tabs-store.test.ts b/lib/__tests__/message-list-tabs-store.test.ts index 5203bc828..62747230e 100644 --- a/lib/__tests__/message-list-tabs-store.test.ts +++ b/lib/__tests__/message-list-tabs-store.test.ts @@ -11,7 +11,7 @@ import { useEmailStore } from '@/stores/email-store'; import { messageListTabHooks } from '@/lib/plugin-hooks'; import { jmapError } from '@/lib/jmap/jmap-error'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; -import type { Email } from '@/lib/jmap/types'; +import type { Email, Mailbox } from '@/lib/jmap/types'; import type { MessageListTab } from '@/lib/plugin-types'; const GMAIL_STYLE = { @@ -184,10 +184,14 @@ describe('store registration', () => { }); describe('categorizeEmails', () => { - const email = (id: string, accountId?: string) => ({ + // `sourceAccountId`: the JMAP account a unified-view message lives in, as + // the list stamps it; the display-only `accountId` names the login. + const email = (id: string, sourceAccountId?: string) => ({ id, threadId: `t-${id}`, keywords: { $seen: true }, mailboxIds: { inbox: true }, - from: [{ email: `${id}@example.com` }], accountId, + from: [{ email: `${id}@example.com` }], + ...(sourceAccountId ? { accountId: 'login-1', sourceClientAccountId: 'login-1', sourceAccountId } : {}), }) as unknown as Email; + const PATCH = { 'keywords/$category-updates': null, 'keywords/$category-promotions': true }; beforeEach(() => { useEmailStore.setState({ emails: [email('m1'), email('m2', 'shared')], mailboxes: [], selectedMailbox: '' }); @@ -237,4 +241,30 @@ describe('categorizeEmails', () => { // No per-sender override is recorded for a move that did not happen. expect(emitted).not.toHaveBeenCalled(); }); + + it('writes a unified-view message to its own JMAP account, not the login named for display', async () => { + const batchUpdateKeywords = vi.fn().mockResolvedValue(undefined); + + await useMessageListTabsStore.getState() + .categorizeEmails({ batchUpdateKeywords } as unknown as IJMAPClient, ['m2'], 'promotions'); + + expect(batchUpdateKeywords).toHaveBeenCalledWith(['m2'], PATCH, 'shared'); + expect(batchUpdateKeywords).not.toHaveBeenCalledWith(expect.anything(), expect.anything(), 'login-1'); + }); + + it('writes to the group account whose inbox is open in the shared section', async () => { + // Those messages carry no source stamps: the open folder names the + // account. An unrouted write lands in the login's own account. + const groupInbox = { + id: 'team:inbox', originalId: 'inbox', accountId: 'team', isShared: true, role: 'inbox', name: 'Inbox', + } as unknown as Mailbox; + useEmailStore.setState({ emails: [email('g1'), email('g2')], mailboxes: [groupInbox], selectedMailbox: groupInbox.id }); + const batchUpdateKeywords = vi.fn().mockResolvedValue(undefined); + + await useMessageListTabsStore.getState() + .categorizeEmails({ batchUpdateKeywords } as unknown as IJMAPClient, ['g1', 'g2'], 'promotions'); + + expect(batchUpdateKeywords).toHaveBeenCalledTimes(1); + expect(batchUpdateKeywords).toHaveBeenCalledWith(['g1', 'g2'], PATCH, 'team'); + }); }); diff --git a/lib/__tests__/thread-utils.test.ts b/lib/__tests__/thread-utils.test.ts index 4f2fb08bc..12af451e8 100644 --- a/lib/__tests__/thread-utils.test.ts +++ b/lib/__tests__/thread-utils.test.ts @@ -196,7 +196,7 @@ describe('sortThreadGroups', () => { it('keeps the fetched order of a sender order, as Stalwart 1.0 collates it', () => { // The server's byte order puts "zoe" before "émile" and the message - // without a sender last; a localeCompare re-sort would turn that around. + // without a sender last; a localeCompare re-sort would swap the first two. const fetched = [ makeEmail({ id: 'z', threadId: 'tz', from: [{ name: 'Zoe', email: 'zoe@example.org' }] }), makeEmail({ id: 'e1', threadId: 'te', from: [{ name: 'Émile', email: 'emile@example.org' }] }), @@ -204,19 +204,37 @@ describe('sortThreadGroups', () => { makeEmail({ id: 'e2', threadId: 'te', from: [{ name: 'Émile', email: 'emile@example.org' }], receivedAt: '2024-02-01T00:00:00Z' }), ]; const order = [{ criterion: 'from' as const, direction: 'asc' as const }]; - expect(sortThreadGroups(groupEmailsByThread(fetched), order).map(g => g.threadId)).toEqual(['tz', 'te', 'tn']); + expect(sortThreadGroups(groupEmailsByThread(fetched), order, { keepServerTextOrder: true }).map(g => g.threadId)) + .toEqual(['tz', 'te', 'tn']); }); - it('still lifts pinned threads above a subject order, and otherwise keeps the fetched order', () => { + it('still lifts pinned threads above a subject order on Stalwart 1.0, and otherwise keeps the fetched order', () => { // Same subject everywhere: a client re-sort would fall back to newest first. const groups = [ makeGroup('first', '2024-01-01T00:00:00Z'), makeGroup('second', '2024-06-01T00:00:00Z'), makeGroup('pinned', '2024-03-01T00:00:00Z', true), ]; - const sorted = sortThreadGroups(groups, [{ criterion: 'subject', direction: 'asc' }]); + const sorted = sortThreadGroups(groups, [{ criterion: 'subject', direction: 'asc' }], { keepServerTextOrder: true }); expect(sorted.map(g => g.threadId)).toEqual(['pinned', 'first', 'second']); }); + + it('re-sorts a sender order case-insensitively on Stalwart 0.16, the empty sender last', () => { + // 0.16 pages come in raw byte order, capitals first; the loaded pages are + // sorted as one list, as before 1.0. + const fetched = [ + makeEmail({ id: 'a', threadId: 'ta', from: [{ name: 'Adam', email: 'adam@x.test' }] }), + makeEmail({ id: 'c', threadId: 'tc', from: [{ name: 'Carla', email: 'carla@x.test' }] }), + makeEmail({ id: 'n', threadId: 'tn', from: [] }), + makeEmail({ id: 'b', threadId: 'tb', from: [{ name: 'bea', email: 'bea@x.test' }] }), + makeEmail({ id: 'd', threadId: 'td', from: [{ name: 'dave', email: 'dave@x.test' }] }), + ]; + const order = [{ criterion: 'from' as const, direction: 'asc' as const }]; + expect(sortThreadGroups(groupEmailsByThread(fetched), order).map(g => g.threadId)) + .toEqual(['ta', 'tb', 'tc', 'td', 'tn']); + expect(sortThreadGroups(groupEmailsByThread(fetched), [{ criterion: 'from', direction: 'desc' }]).map(g => g.threadId)) + .toEqual(['td', 'tc', 'tb', 'ta', 'tn']); + }); }); describe('getThreadParticipants', () => { diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index cda761d63..eeee708c3 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -84,6 +84,8 @@ export interface IJMAPClient { hasAccountCapability(capability: string, accountId?: string): boolean; /** How the session describes an account; null when it doesn't list the id. */ getAccountInfo(accountId: string): AccountInfo | null; + /** Whether sender and subject sorted lists of the account keep the server's order (Stalwart 1.0). */ + keepsServerTextOrder?(accountId?: string): boolean; /** FileNode naming rules the server publishes (Stalwart 0.16.6+), if any. */ getFileNameRules?(accountId?: string): FileNameRules | null; /** How deep a FileNode may sit (Stalwart 1.0: 64), or null for no limit. */ diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 706b027bc..99b7fb9c3 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -2273,17 +2273,19 @@ export class JMAPClient implements IJMAPClient { if (response.methodResponses?.[1]?.[0] === "Email/get" && getResponse) { const emails = (getResponse.list || []) as Email[]; - if (hasTextLevels(order)) { - // A sender or subject order is the server's collation, which the - // client comparator contradicts (Stalwart 1.0 compares byte-order - // sort keys, empty values last). Keep the query's order: /get may - // return the objects in any order (RFC 8620 section 5.1). + if (hasTextLevels(order) && this.keepsServerTextOrder(targetAccountId)) { + // Stalwart 1.0's sender or subject collation, which the client + // comparator would only disturb (see keepsServerTextOrder). Keep + // the query's order: /get may return the objects in any order + // (RFC 8620 section 5.1). const rank = new Map(((queryResponse?.ids ?? []) as string[]).map((id, i) => [id, i])); emails.sort((a, b) => (rank.get(a.id) ?? rank.size) - (rank.get(b.id) ?? rank.size)); } else { // Sort client-side as safety net - some servers may not honour // the query sort for large mailboxes without additional filters. // Must mirror the query sort, or it would undo the configured order. + // On Stalwart 0.16 this also turns a sender or subject page from + // raw byte order (capitals first) into a case-insensitive one. emails.sort(compareEmails(order, { pinnedFirst })); } const total = queryResponse?.total || 0; @@ -5310,6 +5312,18 @@ export class JMAPClient implements IJMAPClient { return !!caps && capability in caps; } + /** + * Whether a sender or subject sorted list of the account keeps the order + * the server returned. Stalwart 1.0 collates those itself (lowercased + * byte-order keys, empty values last), which the client comparator would + * only disturb. 0.16 compares the raw bytes, capitals before lowercase, so + * its pages are re-sorted case-insensitively. Every 1.0 account lists + * principals:owner in its accountCapabilities; 0.16 never does. + */ + keepsServerTextOrder(accountId?: string): boolean { + return this.hasAccountCapability("urn:ietf:params:jmap:principals:owner", accountId); + } + getAccountInfo(accountId: string): AccountInfo | null { const account = this.accounts[accountId]; if (!account) return null; diff --git a/lib/message-list-order.ts b/lib/message-list-order.ts index 1a660f007..4580ae081 100644 --- a/lib/message-list-order.ts +++ b/lib/message-list-order.ts @@ -149,15 +149,18 @@ export function hasKeywordLevels(levels: SortLevel[], pinnedFirst = false): bool return pinnedFirst || levels.some(l => levelKeyword(l) !== null); } -// Sender and subject orders are collated by the server. Stalwart 1.0 compares -// byte-order sort keys (lowercased name and address, the thread name for the -// subject) and puts empty values last in both directions; 0.16 leaves it to -// its search store. `compareEmails` approximates neither. +// Sender and subject orders are collated by the server, and the versions +// differ. Stalwart 1.0 compares byte-order sort keys: the lowercased name and +// address, the thread name for the subject. 0.16 compares the raw bytes of the +// same values, so capitals come before lowercase ("Eve" before "bea"). Both +// put a message without a sender or subject last in either direction. +// `compareEmails` sorts case-insensitively and puts those last too. const TEXT_CRITERIA: ReadonlySet = new Set(['from', 'subject']); /** - * True when the order compares text: the list keeps the order the server - * returned instead of being re-sorted on the client. + * True when the order compares text. A list from a server whose collation the + * client keeps (see JMAPClient.keepsServerTextOrder) stays in the order the + * server returned; any other list is re-sorted with `compareEmails`. */ export function hasTextLevels(levels: SortLevel[]): boolean { return levels.some(l => TEXT_CRITERIA.has(l.criterion)); @@ -259,8 +262,8 @@ function compareValues(a: number | string, b: number | string): number { * within-page safety net (some servers ignore the sort on unfiltered queries), * for merging per-account pages in the unified view, for the demo client, and * to keep thread grouping from undoing the server order. Sender and subject - * levels are an approximation (see `hasTextLevels`), so a single-account list - * with one keeps the server's order. + * levels only approximate the server's collation (see `hasTextLevels`), so a + * list from a server that collates them itself keeps the server's order. */ export function compareEmails( levels: SortLevel[], @@ -274,7 +277,15 @@ export function compareEmails( if (pa !== pb) return pb - pa; } for (const level of levels) { - const diff = compareValues(levelValue(level, a), levelValue(level, b)); + const va = levelValue(level, a); + const vb = levelValue(level, b); + // No sender or subject: last whatever the direction, as both Stalwart + // versions order it. Only the text criteria yield ''. + if (va === '' || vb === '') { + if (va !== vb) return va === '' ? 1 : -1; + continue; + } + const diff = compareValues(va, vb); if (diff !== 0) return level.direction === 'asc' ? diff : -diff; } if (!hasDateLevel) { diff --git a/lib/thread-utils.ts b/lib/thread-utils.ts index c77182674..48cb0f1bb 100644 --- a/lib/thread-utils.ts +++ b/lib/thread-utils.ts @@ -110,13 +110,18 @@ export function groupEmailsByThread( * (RFC 8621 §4.4.3 thread collapsing semantics) - with the default order that * is the latest email's receivedAt date, newest first. * - * A sender or subject order only the server can apply (see `hasTextLevels`). + * A sender or subject order the list keeps as the server collated it + * (`keepServerTextOrder`, Stalwart 1.0; see `hasTextLevels`) is not re-sorted. * The groups arrive in fetched order - groupEmailsByThread keeps the order in * which each thread first appears - so each thread already sits where its * first-ranked email does, and only pinned threads move. */ -export function sortThreadGroups(groups: ThreadGroup[], order: SortLevel[] = []): ThreadGroup[] { - if (hasTextLevels(order)) { +export function sortThreadGroups( + groups: ThreadGroup[], + order: SortLevel[] = [], + opts: { keepServerTextOrder?: boolean } = {}, +): ThreadGroup[] { + if (opts.keepServerTextOrder && hasTextLevels(order)) { return [...groups].sort((a, b) => (b.hasPinned ? 1 : 0) - (a.hasPinned ? 1 : 0)); } const compare = compareEmails(order); diff --git a/stores/__tests__/email-store-server-text-order.test.ts b/stores/__tests__/email-store-server-text-order.test.ts new file mode 100644 index 000000000..432e65d08 --- /dev/null +++ b/stores/__tests__/email-store-server-text-order.test.ts @@ -0,0 +1,92 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore } from '../email-store'; +import { useSettingsStore } from '../settings-store'; +import { useAuthStore } from '../auth-store'; +import { useMessageListTabsStore } from '../message-list-tabs-store'; +import type { SortLevel } from '@/lib/message-list-order'; +import type { Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +/** + * A sender or subject order is collated by the server, and the versions + * differ: Stalwart 1.0 lowercases, 0.16 compares raw bytes (capitals first). + * Thread grouping keeps the order of a 1.0 folder as fetched and re-sorts + * anything else (JMAPClient.keepsServerTextOrder), so the store records which + * one the list is. + */ + +const FROM_ASC: SortLevel[] = [{ criterion: 'from', direction: 'asc' }]; + +const mailbox = (overrides: Partial): Mailbox => ({ + id: 'inbox', name: 'Inbox', role: 'inbox', sortOrder: 0, + totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, + isSubscribed: true, isShared: false, + ...overrides, +}) as Mailbox; + +const inbox = mailbox({}); +// A group's Inbox, shown in the sidebar's shared section. +const groupInbox = mailbox({ id: 'team:inbox', originalId: 'inbox', accountId: 'team', isShared: true }); + +/** A client whose accounts collate text the way `stalwart10Accounts` says. */ +function makeClient(stalwart10Accounts: string[]) { + return { + getAccountId: () => 'acct', + getEmails: vi.fn(async () => ({ emails: [], total: 0, hasMore: false, state: 'email-state' })), + getThreads: vi.fn(async () => []), + keepsServerTextOrder: vi.fn((accountId?: string) => stalwart10Accounts.includes(accountId ?? 'acct')), + } as unknown as IJMAPClient & { keepsServerTextOrder: ReturnType }; +} + +describe('server text order of the message list', () => { + beforeEach(() => { + useAuthStore.setState(useAuthStore.getInitialState()); + useMessageListTabsStore.setState(useMessageListTabsStore.getInitialState()); + useSettingsStore.setState({ emailsPerPage: 50, emailKeywords: [], messageListOrder: FROM_ASC, messageListOrderScope: 'all' }); + useEmailStore.setState({ + ...useEmailStore.getInitialState(), + selectedMailbox: 'inbox', + mailboxes: [inbox, groupInbox], + }); + }); + + it('keeps the fetched sender order of a folder on Stalwart 1.0', async () => { + const client = makeClient(['acct']); + + await useEmailStore.getState().fetchEmails(client, 'inbox'); + + expect(useEmailStore.getState().listOrder).toEqual(FROM_ASC); + expect(useEmailStore.getState().listKeepsServerTextOrder).toBe(true); + }); + + it('re-sorts the sender order of a folder on Stalwart 0.16', async () => { + const client = makeClient([]); + + await useEmailStore.getState().fetchEmails(client, 'inbox'); + + expect(useEmailStore.getState().listOrder).toEqual(FROM_ASC); + expect(useEmailStore.getState().listKeepsServerTextOrder).toBe(false); + }); + + it('asks about the account of the shared folder that is open', async () => { + const client = makeClient(['team']); + useEmailStore.setState({ selectedMailbox: groupInbox.id }); + + await useEmailStore.getState().fetchEmails(client, groupInbox.id); + + expect(client.keepsServerTextOrder).toHaveBeenCalledWith('team'); + expect(useEmailStore.getState().listKeepsServerTextOrder).toBe(true); + }); + + it('re-sorts a tag view, which merges its accounts on the client, even on Stalwart 1.0', async () => { + const client = makeClient(['acct', 'team']); + await useEmailStore.getState().fetchEmails(client, 'inbox'); + expect(useEmailStore.getState().listKeepsServerTextOrder).toBe(true); + + useEmailStore.setState({ selectedKeyword: 'red' }); + await useEmailStore.getState().fetchEmails(client); + + expect(useEmailStore.getState().listOrder).toEqual(FROM_ASC); + expect(useEmailStore.getState().listKeepsServerTextOrder).toBe(false); + }); +}); diff --git a/stores/__tests__/email-store-thread-key.test.ts b/stores/__tests__/email-store-thread-key.test.ts index 76e6ad336..372ab46f4 100644 --- a/stores/__tests__/email-store-thread-key.test.ts +++ b/stores/__tests__/email-store-thread-key.test.ts @@ -134,13 +134,48 @@ describe('thread identity across accounts (#1012)', () => { it('marks only the owning account\'s messages read when a thread is expanded', async () => { await useEmailStore.getState().markThreadAsRead(clientA, threadKeyFor(a1)); - expect(clientA.batchMarkAsRead).toHaveBeenCalledWith(['a1'], true); + expect(clientA.batchMarkAsRead).toHaveBeenCalledWith(['a1'], true, 'acct-a'); expect(clientB.batchMarkAsRead).not.toHaveBeenCalled(); const byId = new Map(useEmailStore.getState().emails.map((e) => [e.id, e])); expect(byId.get('a1')?.keywords?.$seen).toBe(true); expect(byId.get('b1')?.keywords?.$seen).toBeUndefined(); }); + it('marks a thread of a shared account read in that account, through the login that reaches it', async () => { + // A group's mailbox in the unified view: reached through login a, owned + // by the group's JMAP account. Without the account id the write lands in + // the login's own account, where Stalwart answers notFound or marks an + // unrelated message with the same id. + const shared = inAccount('a', 'g1', { threadId: 'c', sourceAccountId: 'acct-team' }); + useEmailStore.setState({ emails: [a1, b1, shared] }); + + await useEmailStore.getState().markThreadAsRead(clientA, threadKeyFor(shared)); + + expect(clientA.batchMarkAsRead).toHaveBeenCalledTimes(1); + expect(clientA.batchMarkAsRead).toHaveBeenCalledWith(['g1'], true, 'acct-team'); + expect(useEmailStore.getState().emails.find((e) => e.id === 'g1')?.keywords?.$seen).toBe(true); + }); + + it('marks a thread read in the group account whose folder is open, outside the unified view', async () => { + // The sidebar's shared section lists a group folder's mail without source + // stamps; the open folder names the account. + const groupInbox = makeMailbox({ id: 'acct-team:inbox', originalId: 'inbox', accountId: 'acct-team', isShared: true }); + const plain = (id: string) => inAccount('a', id, { + threadId: 't7', sourceClientAccountId: undefined, sourceAccountId: undefined, accountId: undefined, + }); + useEmailStore.setState({ + isUnifiedView: false, + crossView: null, + selectedMailbox: groupInbox.id, + mailboxes: [makeMailbox(), groupInbox], + emails: [plain('m1'), plain('m2')], + }); + + await useEmailStore.getState().markThreadAsRead(clientA, threadKeyFor(plain('m1'))); + + expect(clientA.batchMarkAsRead).toHaveBeenCalledWith(['m1', 'm2'], true, 'acct-team'); + }); + it('keeps expansion state apart for same-id threads of different accounts', () => { useEmailStore.getState().toggleThreadExpansion(threadKeyFor(a1)); diff --git a/stores/email-store.ts b/stores/email-store.ts index ed83c333c..fa0b9fc59 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -82,6 +82,11 @@ interface EmailStore { // The configured message-list order the current folder view was fetched // with (#718); empty for chronological. Thread grouping mirrors it. listOrder: SortLevel[]; + // Whether the list keeps the server's order for listOrder's sender and + // subject levels: a folder of an account whose server collates them itself + // (Stalwart 1.0, see JMAPClient.keepsServerTextOrder). Otherwise thread + // grouping re-sorts them on the client, as the fetch did per page. + listKeepsServerTextOrder: boolean; isPushConnected: boolean; // Track if push notifications are connected lastPushUpdate: number | null; // Timestamp of last push update // Delta sync (RFC 8620 §5.2). The Email collection state the plain @@ -879,7 +884,7 @@ function resolveEmailActionContext( * unknown email keeps the previous behaviour instead: no explicit accountId, * i.e. the reaching client's own account. */ -function resolveKeywordActionContext( +export function resolveKeywordActionContext( emailId: string, passedClient: IJMAPClient, ): { client: IJMAPClient; accountId: string | undefined } { @@ -1422,6 +1427,7 @@ export const useEmailStore = create((set, get) => ({ hasMoreEmails: false, totalEmails: 0, listOrder: [], + listKeepsServerTextOrder: false, isPushConnected: false, lastPushUpdate: null, emailListSync: null, @@ -1764,6 +1770,8 @@ export const useEmailStore = create((set, get) => ({ hasMoreEmails: result.hasMore, totalEmails: result.total, listOrder: crossView || hasFilters || searchQuery ? [] : getMessageListOrderFor(unifiedRole), + // Merged from several accounts with the client comparator. + listKeepsServerTextOrder: false, emailListSync: null, threadEmailsCache: new Map(), expandedThreadIds: new Set(), @@ -1805,6 +1813,8 @@ export const useEmailStore = create((set, get) => ({ hasMoreEmails: result.hasMore, totalEmails: result.total, listOrder: order, + // Merged from several accounts with the client comparator. + listKeepsServerTextOrder: false, // A server-side keyword filter cannot be delta-synced. emailListSync: null, threadEmailsCache: new Map(), @@ -1862,6 +1872,7 @@ export const useEmailStore = create((set, get) => ({ hasMoreEmails: result.hasMore, totalEmails: result.total, listOrder: order, + listKeepsServerTextOrder: effectiveClient.keepsServerTextOrder?.(accountId) ?? false, emailListSync, // Clear thread caches since the email list was fully replaced threadEmailsCache: new Map(), @@ -4378,24 +4389,26 @@ export const useEmailStore = create((set, get) => ({ if (unreadIds.length === 0) return; - // Group by account for unified view support - const emailsById = new Map(); - for (const e of allEmailMap.values()) emailsById.set(e.id, e); - - // Group unread IDs by account client - const groups = new Map(); + // Group the unread ids by the client and the JMAP account that own them. + // Ids repeat across accounts (#847): a write without the owner's account + // id lands in the login's own account, where the server answers notFound + // or marks an unrelated message that happens to share the id. So a thread + // in a shared or group folder has to name its account. + const groups = new Map>(); for (const id of unreadIds) { - const email = emailsById.get(id)!; - const { client: actionClient } = resolveEmailActionContext(email, client); - if (!groups.has(actionClient)) groups.set(actionClient, []); - groups.get(actionClient)!.push(id); + const { client: actionClient, accountId } = resolveEmailActionContext(allEmailMap.get(id)!, client); + const byAccount = groups.get(actionClient) ?? new Map(); + byAccount.set(accountId, [...(byAccount.get(accountId) ?? []), id]); + groups.set(actionClient, byAccount); } // Mark all as read on the server try { await Promise.all( - Array.from(groups.entries()).map(([actionClient, emailIds]) => - actionClient.batchMarkAsRead(emailIds, true) + Array.from(groups.entries()).flatMap(([actionClient, byAccount]) => + Array.from(byAccount.entries()).map(([accountId, emailIds]) => + actionClient.batchMarkAsRead(emailIds, true, accountId) + ) ) ); } catch (error) { @@ -4815,6 +4828,7 @@ export const useEmailStore = create((set, get) => ({ hasMoreEmails: result.hasMore, totalEmails: result.total, listOrder: order, + listKeepsServerTextOrder: false, isLoading: false, unifiedErrors: result.errors, }); diff --git a/stores/message-list-tabs-store.ts b/stores/message-list-tabs-store.ts index fc4575cfa..3ebbde511 100644 --- a/stores/message-list-tabs-store.ts +++ b/stores/message-list-tabs-store.ts @@ -11,13 +11,12 @@ import { create } from 'zustand'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; -import type { Email } from '@/lib/jmap/types'; import type { MessageListTab, MessageListTabsConfig } from '@/lib/plugin-types'; import { MAX_MESSAGE_LIST_TABS } from '@/lib/plugin-types'; import { messageListTabHooks } from '@/lib/plugin-hooks'; import { keywordPointer } from '@/lib/jmap/patch-pointer'; import { SERVER_SET_KEYWORDS } from '@/lib/jmap/server-keywords'; -import { useEmailStore } from './email-store'; +import { resolveKeywordActionContext, useEmailStore } from './email-store'; // ─── Validation ────────────────────────────────────────────── @@ -331,14 +330,17 @@ export const useMessageListTabsStore = create()((set, get) for (const k of keywordsRemoved) patch[keywordPointer(k)] = null; if (keywordAdded) patch[keywordPointer(keywordAdded)] = true; - // Group by owning account so shared-mailbox messages patch correctly. - const byAccount = new Map(); + // Group by the client and JMAP account that own each message, the way the + // email store routes its keyword writes: a listed message's `accountId` + // is only a display reference, and a write without the owner's account id + // lands in the login's own account (a group inbox with tabs, the unified + // views). + const groups = new Map>(); for (const id of emailIds) { - const email = affected.find((e) => e.id === id) as (Email & { accountId?: string }) | undefined; - const acct = email?.accountId; - const list = byAccount.get(acct) ?? []; - list.push(id); - byAccount.set(acct, list); + const { client: writeClient, accountId } = resolveKeywordActionContext(id, client); + const byAccount = groups.get(writeClient) ?? new Map(); + byAccount.set(accountId, [...(byAccount.get(accountId) ?? []), id]); + groups.set(writeClient, byAccount); } // A refused write (a read-only shared folder; Stalwart 1.0 also refuses a // 101st keyword on a message) fails the move: the plugin's promise rejects @@ -346,12 +348,14 @@ export const useMessageListTabsStore = create()((set, get) // accounts are still written, and their messages still move locally. const written: string[] = []; let failure: unknown = null; - for (const [acct, ids] of byAccount) { - try { - await client.batchUpdateKeywords(ids, patch, acct); - written.push(...ids); - } catch (err) { - failure ??= err; + for (const [writeClient, byAccount] of groups) { + for (const [accountId, ids] of byAccount) { + try { + await writeClient.batchUpdateKeywords(ids, patch, accountId); + written.push(...ids); + } catch (err) { + failure ??= err; + } } }