Fix: stop resurrecting deleted rows in the mailbox refresh merge
Fixes #592. refreshCurrentMailbox merges the refreshed first page with the already loaded list, appending existing entries beyond a cutoff. That cutoff was derived from the refreshed list's length - so whenever a folder shrank, the fresh page was shorter than the stale list and the loop re-appended the deleted rows from stale local state, despite the comment right above promising the opposite. The visible result is the reported bug: after sending a draft, the Drafts view keeps showing a ghost row for the already-destroyed draft. The send actually succeeded - resending the ghost delivers the mail again, which we reproduced with a live JMAP trace: four successful submissions, an empty server-side Drafts folder, a notFound ghost id, and five delivered copies. Deriving the cutoff from the page size fixes the shrink case while preserving the merge's intent for arrivals and loaded deeper pages; regression tests cover all three shapes. Also surface post-send filing failures instead of dropping them, as flagged in 4dc76bbb's follow-up note: a rejected onSuccessUpdateEmail patch or old-draft destroy now logs the server's error details and returns a filingError on SendEmailResult, and the UI shows a warning toast (all 23 locales) so a stale draft row is never again mistaken for a failed send. A plugin veto of the send leaves a debug trace.
This commit is contained in:
@@ -0,0 +1,102 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { useEmailStore } from '../email-store';
|
||||
import { useSettingsStore } from '../settings-store';
|
||||
import type { Email, Mailbox } from '@/lib/jmap/types';
|
||||
import type { IJMAPClient } from '@/lib/jmap/client-interface';
|
||||
|
||||
/**
|
||||
* refreshCurrentMailbox merges the refreshed first page with the already
|
||||
* loaded list. The append cutoff must derive from the page size, not from
|
||||
* the refreshed list's length: when the folder shrank (a deletion - e.g.
|
||||
* the draft of a just-sent mail), a length-based cutoff re-appends the
|
||||
* deleted rows from stale local state. That ghost row is how "sent mail
|
||||
* still shows as draft" reports happen (#592) - and re-sending the ghost
|
||||
* delivers the mail again.
|
||||
*/
|
||||
|
||||
const makeEmail = (id: string): Email =>
|
||||
({
|
||||
id,
|
||||
threadId: `t-${id}`,
|
||||
mailboxIds: { d: true },
|
||||
keywords: {},
|
||||
from: [{ email: 'a@example.com' }],
|
||||
to: [{ email: 'b@example.com' }],
|
||||
subject: `mail ${id}`,
|
||||
receivedAt: '2026-07-23T10:00:00Z',
|
||||
preview: '',
|
||||
hasAttachment: false,
|
||||
size: 1,
|
||||
}) as unknown as Email;
|
||||
|
||||
const draftsMailbox = {
|
||||
id: 'd',
|
||||
name: 'Drafts',
|
||||
role: 'drafts',
|
||||
totalEmails: 1,
|
||||
unreadEmails: 0,
|
||||
totalThreads: 1,
|
||||
unreadThreads: 0,
|
||||
} as unknown as Mailbox;
|
||||
|
||||
function makeClient(page: Email[], total: number): IJMAPClient {
|
||||
return {
|
||||
getEmails: vi.fn(async () => ({ emails: page, hasMore: false, total })),
|
||||
} as unknown as IJMAPClient;
|
||||
}
|
||||
|
||||
describe('refreshCurrentMailbox merge', () => {
|
||||
beforeEach(() => {
|
||||
useSettingsStore.setState({ emailsPerPage: 3 });
|
||||
// Only override what the tests need - the store's initial state already
|
||||
// carries the correct empty search filters, view flags and caches.
|
||||
useEmailStore.setState({
|
||||
selectedMailbox: 'd',
|
||||
mailboxes: [draftsMailbox],
|
||||
accountMailboxes: {},
|
||||
emails: [],
|
||||
totalEmails: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it('drops a deleted row when the folder shrank below a full page (#592 ghost draft)', async () => {
|
||||
// Client state still lists the old draft; the server already deleted it.
|
||||
useEmailStore.setState({ emails: [makeEmail('ghost')], totalEmails: 1 });
|
||||
const client = makeClient([], 0);
|
||||
|
||||
await useEmailStore.getState().refreshCurrentMailbox(client);
|
||||
|
||||
expect(useEmailStore.getState().emails).toEqual([]);
|
||||
expect(useEmailStore.getState().totalEmails).toBe(0);
|
||||
});
|
||||
|
||||
it('still preserves the item a new arrival pushes off the first page', async () => {
|
||||
const a = makeEmail('a');
|
||||
const b = makeEmail('b');
|
||||
const c = makeEmail('c');
|
||||
const fresh = makeEmail('new');
|
||||
useEmailStore.setState({ emails: [a, b, c], totalEmails: 3 });
|
||||
// New mail arrived: first page (size 3) now starts with it, c fell off.
|
||||
const client = makeClient([fresh, a, b], 4);
|
||||
|
||||
await useEmailStore.getState().refreshCurrentMailbox(client);
|
||||
|
||||
expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['new', 'a', 'b', 'c']);
|
||||
});
|
||||
|
||||
it('keeps loaded deeper pages while dropping a first-page deletion', async () => {
|
||||
const a = makeEmail('a');
|
||||
const b = makeEmail('b');
|
||||
const c = makeEmail('c');
|
||||
const d2 = makeEmail('d2');
|
||||
// Two loaded pages (page size 3); server deleted b from page one.
|
||||
useEmailStore.setState({ emails: [a, b, c, d2], totalEmails: 4 });
|
||||
const client = makeClient([a, c, d2], 3);
|
||||
|
||||
await useEmailStore.getState().refreshCurrentMailbox(client);
|
||||
|
||||
const ids = useEmailStore.getState().emails.map((e) => e.id);
|
||||
expect(ids).toContain('d2');
|
||||
expect(ids).not.toContain('b');
|
||||
});
|
||||
});
|
||||
@@ -2884,7 +2884,14 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
const merged: Email[] = [...refreshedEmails];
|
||||
const mergedIds = new Set(refreshedEmails.map((e: Email) => e.id));
|
||||
const insertedCount = Math.max((result.total || 0) - previousTotal, 0);
|
||||
const appendFromIndex = Math.max(refreshedEmails.length - insertedCount, 0);
|
||||
// Derive the cutoff from the page size, not from the refreshed list's
|
||||
// length: when the folder shrank (a deletion - e.g. the draft of a just
|
||||
// sent mail), the fresh page is shorter than the stale list and a
|
||||
// length-based cutoff re-appends the deleted rows from stale local
|
||||
// state. That ghost row is how "sent mail still shows as draft"
|
||||
// reports happen (#592) - and re-sending the ghost delivers the mail
|
||||
// again.
|
||||
const appendFromIndex = Math.max(emailsPerPage - insertedCount, 0);
|
||||
|
||||
for (const email of currentEmails.slice(appendFromIndex)) {
|
||||
if (!mergedIds.has(email.id)) {
|
||||
|
||||
Reference in New Issue
Block a user