mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-01 08:12:43 +00:00
fix: mark every unread message read and report refused spam moves
"Mark all as read" paged through the unread messages by position, but each page it marked dropped out of the unread query, so every other page was skipped: with more unread mail than one page (500), part of it stayed unread. The next page now starts after the messages that were left unread (those in excluded folders) only. Neither mark-as-read loop checked the Email/set result. A refused update made "mark folder as read" fetch the same unread ids forever; it now stops with an error. Marking as spam and "not spam" also ignored `notUpdated` (e.g. a Junk folder the server no longer knows) and showed success; they now report the failure.
This commit is contained in:
@@ -0,0 +1,60 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { JMAPClient } from '../jmap/client';
|
||||
|
||||
type Call = [string, Record<string, unknown>, string];
|
||||
|
||||
// A tiny server: `unread` is the set of unread message ids, each in one
|
||||
// mailbox. Email/query {notKeyword: $seen} pages over it in id order.
|
||||
function fakeServer(mailboxOf: Record<string, string>, refuse = false) {
|
||||
const unread = new Set(Object.keys(mailboxOf));
|
||||
const client = new JMAPClient('https://jmap.example.com', 'user@example.com', 'pass');
|
||||
Object.assign(client, { accountId: 'me', capabilities: { 'urn:ietf:params:jmap:core': { maxObjectsInGet: 2, maxObjectsInSet: 2 } } });
|
||||
const request = vi.spyOn(client as unknown as { request: (c: Call[]) => Promise<unknown> }, 'request')
|
||||
.mockImplementation(async (calls: Call[]) => {
|
||||
const responses: unknown[] = [];
|
||||
let lastIds: string[] = [];
|
||||
for (const [method, args, id] of calls) {
|
||||
if (method === 'Email/query') {
|
||||
const filter = args.filter as { conditions?: Array<{ inMailbox?: string }> };
|
||||
const inMailbox = filter.conditions?.find(c => c.inMailbox)?.inMailbox;
|
||||
const all = [...unread].sort().filter(m => !inMailbox || mailboxOf[m] === inMailbox);
|
||||
const position = (args.position as number) ?? 0;
|
||||
lastIds = all.slice(position, position + (args.limit as number));
|
||||
responses.push(['Email/query', { ids: lastIds }, id]);
|
||||
} else if (method === 'Email/get') {
|
||||
responses.push(['Email/get', { list: lastIds.map(m => ({ id: m, mailboxIds: { [mailboxOf[m]]: true } })) }, id]);
|
||||
} else if (method === 'Email/set') {
|
||||
const ids = Object.keys(args.update as object);
|
||||
if (refuse) {
|
||||
responses.push(['Email/set', { notUpdated: Object.fromEntries(ids.map(m => [m, { type: 'forbidden' }])) }, id]);
|
||||
} else {
|
||||
ids.forEach(m => unread.delete(m));
|
||||
responses.push(['Email/set', { updated: Object.fromEntries(ids.map(m => [m, null])) }, id]);
|
||||
}
|
||||
}
|
||||
}
|
||||
return { methodResponses: responses };
|
||||
});
|
||||
return { client, unread, request };
|
||||
}
|
||||
|
||||
describe('mark as read paging', () => {
|
||||
it('marks every unread message, not every other page', async () => {
|
||||
const mailboxOf = Object.fromEntries(['a', 'b', 'c', 'd', 'e', 'f', 'g'].map(m => [m, 'inbox']));
|
||||
const { client, unread } = fakeServer(mailboxOf);
|
||||
expect(await client.markAllAsRead()).toBe(7);
|
||||
expect(unread.size).toBe(0);
|
||||
});
|
||||
|
||||
it('pages past messages left unread in excluded folders', async () => {
|
||||
const { client, unread } = fakeServer({ a: 'junk', b: 'inbox', c: 'junk', d: 'inbox', e: 'inbox' });
|
||||
expect(await client.markAllAsRead(['junk'])).toBe(3);
|
||||
expect([...unread].sort()).toEqual(['a', 'c']);
|
||||
});
|
||||
|
||||
it('stops with an error when the server refuses instead of looping', async () => {
|
||||
const { client, request } = fakeServer({ a: 'inbox', b: 'inbox', c: 'inbox' }, true);
|
||||
await expect(client.markMailboxAsRead('inbox')).rejects.toThrow(/mark the folder as read/);
|
||||
expect(request).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
+14
-5
@@ -2750,9 +2750,12 @@ export class JMAPClient implements IJMAPClient {
|
||||
ids.map((id) => [id, { "keywords/$seen": true }])
|
||||
);
|
||||
|
||||
await this.request([
|
||||
// A refused update leaves the ids unread, and the next query would
|
||||
// return them again forever.
|
||||
const setResponse = await this.request([
|
||||
["Email/set", { accountId: targetAccountId, update: updates }, "0"],
|
||||
]);
|
||||
this.assertEmailSetSucceeded(setResponse, "mark the folder as read");
|
||||
|
||||
totalMarked += ids.length;
|
||||
hasMore = ids.length === pageSize;
|
||||
@@ -2804,14 +2807,18 @@ export class JMAPClient implements IJMAPClient {
|
||||
const updates = Object.fromEntries(
|
||||
targetIds.map((id) => [id, { "keywords/$seen": true }])
|
||||
);
|
||||
await this.request([
|
||||
const setResponse = await this.request([
|
||||
["Email/set", { accountId: targetAccountId, update: updates }, "0"],
|
||||
]);
|
||||
this.assertEmailSetSucceeded(setResponse, "mark all as read");
|
||||
totalMarked += targetIds.length;
|
||||
}
|
||||
|
||||
hasMore = ids.length === pageSize;
|
||||
position += ids.length;
|
||||
// Marked messages drop out of the "unread" query, so the next page
|
||||
// starts after the ones left unread (excluded folders) only. Advancing
|
||||
// by the whole page skipped a page of unread mail every round.
|
||||
position += ids.length - targetIds.length;
|
||||
}
|
||||
|
||||
return totalMarked;
|
||||
@@ -2845,18 +2852,19 @@ export class JMAPClient implements IJMAPClient {
|
||||
};
|
||||
if (markAsRead) patch["keywords/$seen"] = true;
|
||||
|
||||
await this.request([
|
||||
const response = await this.request([
|
||||
["Email/set", {
|
||||
accountId: targetAccountId,
|
||||
update: { [emailId]: patch },
|
||||
}, "0"],
|
||||
]);
|
||||
this.assertEmailSetSucceeded(response, "mark as spam");
|
||||
}
|
||||
|
||||
async undoSpam(emailId: string, originalMailboxId: string, accountId?: string): Promise<void> {
|
||||
const targetAccountId = accountId || this.accountId;
|
||||
|
||||
await this.request([
|
||||
const response = await this.request([
|
||||
["Email/set", {
|
||||
accountId: targetAccountId,
|
||||
update: {
|
||||
@@ -2870,6 +2878,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
},
|
||||
}, "0"],
|
||||
]);
|
||||
this.assertEmailSetSucceeded(response, "mark as not spam");
|
||||
}
|
||||
|
||||
async createMailbox(name: string, parentId?: string, accountId?: string): Promise<Mailbox> {
|
||||
|
||||
Reference in New Issue
Block a user