mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-04 00:14:12 +00:00
fix: sort senders regardless of case on 0.16, mark shared threads read
44c77b26kept the server's order for sender and subject sorted lists. That is right for Stalwart 1.0, which sorts lowercased byte-order keys, but 0.16 compares the raw bytes, capitals first, so on every 0.16 server a sender list read "Adam, Carla, Eve, bea, dave, zed". - keep the server's order only for accounts that list `urn:ietf:params:jmap:principals:owner` (every 1.0 account, never 0.16); elsewhere pages and thread groups are re-sorted case-insensitively, as before44c77b26- the client comparator puts a message without a sender or subject last in both directions, as both versions do Expanding a thread marked it read in the login's own account instead of the shared or group account it lives in, so Stalwart answered notFound or marked an unrelated message with the same id, and sinced7004b38the thread also stayed unread. The write now names the owning account. Moving mail between category tabs had the same fault: it took a listed message's display-only account reference, and now routes each message like the other keyword writes.
This commit is contained in:
@@ -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<Email>();
|
||||
/**
|
||||
|
||||
@@ -12,10 +12,10 @@ type MethodCall = [string, Record<string, unknown>, string];
|
||||
|
||||
const UNREAD_FIRST: SortLevel[] = [{ criterion: 'unread', direction: 'desc' }];
|
||||
|
||||
function makeSession(mailCaps: Record<string, unknown> = {}) {
|
||||
function makeSession(mailCaps: Record<string, unknown> = {}, accountCaps: Record<string, unknown> = {}) {
|
||||
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<string, unknown>): Promise<JMAPClient> {
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse(makeSession(mailCaps)));
|
||||
async function connectedClient(mailCaps?: Record<string, unknown>, accountCaps?: Record<string, unknown>): Promise<JMAPClient> {
|
||||
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<typeof message>;
|
||||
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']);
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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([
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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. */
|
||||
|
||||
+19
-5
@@ -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;
|
||||
|
||||
@@ -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<SortCriterion> = 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) {
|
||||
|
||||
+8
-3
@@ -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);
|
||||
|
||||
@@ -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>): 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<typeof vi.fn> };
|
||||
}
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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));
|
||||
|
||||
|
||||
+27
-13
@@ -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<EmailStore>((set, get) => ({
|
||||
hasMoreEmails: false,
|
||||
totalEmails: 0,
|
||||
listOrder: [],
|
||||
listKeepsServerTextOrder: false,
|
||||
isPushConnected: false,
|
||||
lastPushUpdate: null,
|
||||
emailListSync: null,
|
||||
@@ -1764,6 +1770,8 @@ export const useEmailStore = create<EmailStore>((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<EmailStore>((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<EmailStore>((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<EmailStore>((set, get) => ({
|
||||
|
||||
if (unreadIds.length === 0) return;
|
||||
|
||||
// Group by account for unified view support
|
||||
const emailsById = new Map<string, Email>();
|
||||
for (const e of allEmailMap.values()) emailsById.set(e.id, e);
|
||||
|
||||
// Group unread IDs by account client
|
||||
const groups = new Map<IJMAPClient, string[]>();
|
||||
// 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<IJMAPClient, Map<string | undefined, string[]>>();
|
||||
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<string | undefined, string[]>();
|
||||
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<EmailStore>((set, get) => ({
|
||||
hasMoreEmails: result.hasMore,
|
||||
totalEmails: result.total,
|
||||
listOrder: order,
|
||||
listKeepsServerTextOrder: false,
|
||||
isLoading: false,
|
||||
unifiedErrors: result.errors,
|
||||
});
|
||||
|
||||
@@ -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<MessageListTabsStore>()((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<string | undefined, string[]>();
|
||||
// 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<IJMAPClient, Map<string | undefined, string[]>>();
|
||||
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<string | undefined, string[]>();
|
||||
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<MessageListTabsStore>()((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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user