mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-01 00:07:01 +00:00
fix: drop the deleted messages from the list when the server refuses others of a bulk delete
This commit is contained in:
@@ -0,0 +1,42 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
vi.mock('../jmap-client', () => ({
|
||||
jmapClient: {
|
||||
accountId: 'acc-1',
|
||||
request: vi.fn(),
|
||||
getMaxObjectsInGet: vi.fn(() => 500),
|
||||
getMaxObjectsInSet: vi.fn(() => 2),
|
||||
},
|
||||
}));
|
||||
|
||||
import { jmapClient } from '../jmap-client';
|
||||
import { destroyEmails } from '../email';
|
||||
import { failedIdsOf } from '../jmap-result';
|
||||
|
||||
const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('destroyEmails', () => {
|
||||
it('sends every slice and names only the refused messages', async () => {
|
||||
mockRequest.mockImplementation(async ([[, args]]: Array<[string, { destroy: string[] }, string]>) => {
|
||||
const refused: string[] = args.destroy.filter((id) => id === 'e2');
|
||||
const gone: string[] = args.destroy.filter((id) => id === 'e4');
|
||||
return {
|
||||
methodResponses: [['Email/set', {
|
||||
destroyed: args.destroy.filter((id) => !refused.includes(id) && !gone.includes(id)),
|
||||
notDestroyed: Object.fromEntries([
|
||||
...refused.map((id) => [id, { type: 'forbidden' }]),
|
||||
...gone.map((id) => [id, { type: 'notFound' }]),
|
||||
]),
|
||||
}, '0']],
|
||||
};
|
||||
});
|
||||
const err = await destroyEmails(['e1', 'e2', 'e3', 'e4']).then(() => null, (e: unknown) => e);
|
||||
expect(mockRequest).toHaveBeenCalledTimes(2);
|
||||
// A message that is already gone counts as deleted.
|
||||
expect(failedIdsOf(err)).toEqual(['e2']);
|
||||
});
|
||||
});
|
||||
+21
-2
@@ -1270,9 +1270,25 @@ export async function setEmailMailboxes(
|
||||
export async function destroyEmails(ids: string[], accountIdOverride?: string): Promise<void> {
|
||||
if (ids.length === 0) return;
|
||||
const accountId = accountIdOverride ?? jmapClient.accountId;
|
||||
// Every slice goes out before a refusal is thrown, and the error names the
|
||||
// ids that are still there (failedIds): the server destroyed the rest, and
|
||||
// the list must drop those rather than keep them until the next refresh
|
||||
// (webmail ac99c68ef).
|
||||
let refusal: JMAPSetError | null = null;
|
||||
const failed: string[] = [];
|
||||
for (const slice of batched(ids, maxInSet())) {
|
||||
const res = await jmapClient.request([['Email/set', { accountId, destroy: slice }, '0']]);
|
||||
assertEmailsDestroyed(requireMethodResult(res, '0', 'Email/set'));
|
||||
try {
|
||||
assertEmailsDestroyed(requireMethodResult(res, '0', 'Email/set'));
|
||||
} catch (err) {
|
||||
if (!(err instanceof JMAPSetError)) throw err;
|
||||
refusal ??= err;
|
||||
failed.push(...(err.failedIds ?? []));
|
||||
}
|
||||
}
|
||||
if (refusal) {
|
||||
refusal.failedIds = failed;
|
||||
throw refusal;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1282,9 +1298,12 @@ function assertEmailsDestroyed(body: Record<string, any>): void {
|
||||
const real = Object.entries(notDestroyed).filter(([, err]) => err?.type !== 'notFound');
|
||||
if (real.length > 0) {
|
||||
const [id, err] = real[0];
|
||||
throw new Error(
|
||||
const error = new JMAPSetError(
|
||||
err.type || 'setError',
|
||||
`Failed to delete ${real.length} email(s), first: ${id} – ${err.type || 'unknown'}${err.description ? ` (${err.description})` : ''}`,
|
||||
);
|
||||
error.failedIds = real.map(([failedId]) => failedId);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1032,6 +1032,27 @@ describe('email-store', () => {
|
||||
expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $seen: true }, undefined);
|
||||
useSettingsStore.getState().updateSetting('deleteAction', 'trash');
|
||||
});
|
||||
|
||||
it('drops the rows the server deleted when it refused others of the selection', async () => {
|
||||
const { JMAPSetError } = await import('../../api/jmap-result');
|
||||
const refusal = new JMAPSetError('forbidden', 'Failed to delete 1 email(s), first: e2 – forbidden');
|
||||
refusal.failedIds = ['e2'];
|
||||
(emailApi.deleteEmails as ReturnType<typeof vi.fn>).mockRejectedValueOnce(refusal);
|
||||
useEmailStore.setState({
|
||||
mailboxes: [
|
||||
{ id: 'mb-1', name: 'Inbox', role: 'inbox', isShared: false } as any,
|
||||
{ id: 'mb-trash', name: 'Trash', role: 'trash', isShared: false } as any,
|
||||
],
|
||||
currentMailboxId: 'mb-trash',
|
||||
emails: [
|
||||
{ id: 'e1', keywords: {}, mailboxIds: { 'mb-trash': true } } as any,
|
||||
{ id: 'e2', keywords: {}, mailboxIds: { 'mb-trash': true } } as any,
|
||||
],
|
||||
});
|
||||
|
||||
await expect(useEmailStore.getState().deleteEmailsBatch(['e1', 'e2'], 'mb-trash', 'mb-trash')).rejects.toBe(refusal);
|
||||
expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['e2']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bulk keyword actions', () => {
|
||||
|
||||
+30
-10
@@ -1885,43 +1885,63 @@ export const useEmailStore = create<EmailState>()(
|
||||
patch: markReadPatch,
|
||||
})),
|
||||
];
|
||||
// The server refuses messages one by one and applies the rest. The
|
||||
// refused ones keep their rows, the others are reconciled as done, and
|
||||
// the refusal is still reported (webmail ac99c68ef): before, one refused
|
||||
// message kept every row of the selection, the deleted ones included.
|
||||
let refusal: unknown = null;
|
||||
const kept = new Set<string>();
|
||||
const step = async (run: () => Promise<void>, keepRefused: boolean) => {
|
||||
try {
|
||||
await run();
|
||||
} catch (err) {
|
||||
const failed = failedIdsOf(err);
|
||||
if (!failed) throw err;
|
||||
refusal ??= err;
|
||||
if (keepRefused) for (const id of failed) kept.add(id);
|
||||
}
|
||||
};
|
||||
await applyOrQueueBatch(ops, async () => {
|
||||
if (toDestroy.length > 0) {
|
||||
await apiDeleteEmails(toDestroy.map((e) => e.id), trash.id, trash.id, trash.accountId);
|
||||
await step(() => apiDeleteEmails(toDestroy.map((e) => e.id), trash.id, trash.id, trash.accountId), true);
|
||||
}
|
||||
if (toTrash.length > 0) {
|
||||
await apiMoveEmails(toTrash.map((e) => e.id), source.id, trash.id, source.accountId);
|
||||
await step(() => apiMoveEmails(toTrash.map((e) => e.id), source.id, trash.id, source.accountId), true);
|
||||
}
|
||||
if (toMarkRead.length > 0) {
|
||||
await patchKeywordsForEmails([...markReadIds], markReadPatch, source.accountId);
|
||||
// Moved but left unread: the row has still left the folder.
|
||||
await step(() => patchKeywordsForEmails([...markReadIds], markReadPatch, source.accountId), false);
|
||||
}
|
||||
});
|
||||
const destroyed = toDestroy.filter((e) => !kept.has(e.id));
|
||||
const trashed = toTrash.filter((e) => !kept.has(e.id));
|
||||
|
||||
if (toDestroy.length > 0) dropFromCache(toDestroy.map((e) => e.id), trash.accountId);
|
||||
for (const e of toTrash) {
|
||||
if (destroyed.length > 0) dropFromCache(destroyed.map((e) => e.id), trash.accountId);
|
||||
for (const e of trashed) {
|
||||
patchCache(e.id, {
|
||||
mailboxIds: mailboxesAfterMove(e.mailboxIds, source.id, trash.id),
|
||||
...(markReadIds.has(e.id) ? { keywords: markReadPatch } : {}),
|
||||
}, source.accountId);
|
||||
}
|
||||
|
||||
const removed = new Set(targets.map(rowKeyOf));
|
||||
const removed = new Set([...destroyed, ...trashed].map(rowKeyOf));
|
||||
set({ emails: get().emails.filter((e) => !removed.has(rowKeyOf(e))) });
|
||||
|
||||
// Only the moved-to-trash items are recoverable; destroyed ones are gone.
|
||||
if (toTrash.length > 0) {
|
||||
if (trashed.length > 0) {
|
||||
set({
|
||||
pendingUndo: {
|
||||
kind: 'delete',
|
||||
label: toTrash.length === 1
|
||||
label: trashed.length === 1
|
||||
? t('email_list.moved_to_trash', 'Email moved to Trash')
|
||||
: t('email_list.moved_to_trash_count', `${toTrash.length} emails moved to Trash`, { count: toTrash.length }),
|
||||
: t('email_list.moved_to_trash_count', `${trashed.length} emails moved to Trash`, { count: trashed.length }),
|
||||
createdAt: Date.now(),
|
||||
accountId: source.accountId,
|
||||
items: toTrash.map((e) => ({ email: e, originalMailboxIds: { ...e.mailboxIds } })),
|
||||
items: trashed.map((e) => ({ email: e, originalMailboxIds: { ...e.mailboxIds } })),
|
||||
},
|
||||
});
|
||||
}
|
||||
if (refusal) throw refusal;
|
||||
},
|
||||
|
||||
setKeywordForEmails: async (emailIds, token, on, viewed) => {
|
||||
|
||||
Reference in New Issue
Block a user