From 51b3208b3bf5f5a97b095a3e1baf7ed901705693 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 04:59:32 +0100 Subject: [PATCH] fix: drop a cancelled scheduled send from the list when it can't go back to Drafts, and say it stayed in Sent --- locales/rn/en.json | 2 + .../__tests__/live-stalwart-compose.test.ts | 4 +- src/lib/__tests__/compose-scheduled.test.ts | 13 +++++- .../__tests__/scheduled-screen-wiring.test.ts | 26 ++++++++++++ src/lib/compose-scheduled.ts | 40 +++++++++++++------ src/screens/ScheduledScreen.tsx | 21 +++++++++- 6 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 src/lib/__tests__/scheduled-screen-wiring.test.ts diff --git a/locales/rn/en.json b/locales/rn/en.json index 679fe44d..b5ad94de 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -758,6 +758,8 @@ "cancel_body": "The message will not be delivered. A copy stays in your Sent folder.", "cancel_failed": "Cancel failed", "cancel_to_drafts_body": "The message will not be sent. It goes back to your Drafts.", + "cancelled_not_moved": "Send cancelled, but not moved to Drafts", + "cancelled_not_moved_body": "The message will not be sent. It is still in Sent.", "edit_failed": "Could not open the message for editing", "keep_scheduled": "Keep scheduled", "no_longer_scheduled": "This message is no longer scheduled.", diff --git a/src/api/__tests__/live-stalwart-compose.test.ts b/src/api/__tests__/live-stalwart-compose.test.ts index 123f26e3..e916e217 100644 --- a/src/api/__tests__/live-stalwart-compose.test.ts +++ b/src/api/__tests__/live-stalwart-compose.test.ts @@ -110,8 +110,8 @@ live('live Stalwart: composer and sending (0.16 and 1.0)', () => { expect(filed.mailboxIds[sent]).toBe(true); // Cancel send: nothing goes out, and the message is a draft again. - const inDrafts = await cancelScheduledToDrafts(second, await getMailboxes(), singleton.accountId); - expect(inDrafts).toBe(true); + const cancelled = await cancelScheduledToDrafts(second, await getMailboxes(), singleton.accountId); + expect(cancelled).toEqual({ inDrafts: true }); expect((await listScheduledEmails()).some((s) => s.emailId === result.emailId)).toBe(false); const [restored] = await getFullEmails([result.emailId!]); expect(restored.mailboxIds[drafts]).toBe(true); diff --git a/src/lib/__tests__/compose-scheduled.test.ts b/src/lib/__tests__/compose-scheduled.test.ts index 99b0d206..28cc24ee 100644 --- a/src/lib/__tests__/compose-scheduled.test.ts +++ b/src/lib/__tests__/compose-scheduled.test.ts @@ -36,7 +36,7 @@ beforeEach(() => vi.clearAllMocks()); describe('Cancel send of a scheduled message', () => { it('cancels the submission and moves an own message back into the own Drafts', async () => { const item = { accountId: 'own', emailSubmissionId: 'sub-1', emailId: 'mail-1' }; - await expect(cancelScheduledToDrafts(item, MAILBOXES, 'own')).resolves.toBe(true); + await expect(cancelScheduledToDrafts(item, MAILBOXES, 'own')).resolves.toEqual({ inDrafts: true }); expect(api.cancelScheduledSend).toHaveBeenCalledWith('sub-1', 'own'); expect(api.restoreEmailToDraft).toHaveBeenCalledWith('mail-1', 'own-drafts', 'own-sent', 'own'); }); @@ -57,10 +57,19 @@ describe('Cancel send of a scheduled message', () => { it('says so when the account has no Drafts folder', async () => { const item = { accountId: 'own', emailSubmissionId: 'sub-1', emailId: 'mail-1' }; - await expect(cancelScheduledToDrafts(item, [mailbox('sent', 'own-sent')], 'own')).resolves.toBe(false); + await expect(cancelScheduledToDrafts(item, [mailbox('sent', 'own-sent')], 'own')).resolves.toEqual({ inDrafts: false }); expect(api.restoreEmailToDraft).not.toHaveBeenCalled(); }); + // The cancel went through, so it must not read as "Cancel failed" with the + // row still listed as scheduled. + it('reports a cancelled send it could not move back to Drafts, without failing', async () => { + const moveError = new Error('Network request failed'); + api.restoreEmailToDraft.mockRejectedValueOnce(moveError); + const item = { accountId: 'own', emailSubmissionId: 'sub-1', emailId: 'mail-1' }; + await expect(cancelScheduledToDrafts(item, MAILBOXES, 'own')).resolves.toEqual({ inDrafts: false, moveError }); + }); + it('names the own account as no override', () => { expect(scheduledAccountOverride({ accountId: 'own' }, 'own')).toBeUndefined(); expect(scheduledAccountOverride({ accountId: 'team' }, 'own')).toBe('team'); diff --git a/src/lib/__tests__/scheduled-screen-wiring.test.ts b/src/lib/__tests__/scheduled-screen-wiring.test.ts new file mode 100644 index 00000000..9b1c8887 --- /dev/null +++ b/src/lib/__tests__/scheduled-screen-wiring.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; + +// The Scheduled screen is a component the node test environment can't +// render; these check its wiring to the helpers tested elsewhere. +const source = fs.readFileSync(path.resolve(__dirname, '../../screens/ScheduledScreen.tsx'), 'utf8'); +const body = (name: string) => { + const start = source.indexOf(`const ${name} = `); + expect(start).toBeGreaterThan(-1); + return source.slice(start, source.indexOf('\n };', start)); +}; + +describe('Scheduled screen', () => { + // A cancel that went through, with the move to Drafts failing after it, + // read "Cancel failed" and left the row listed as scheduled. + it('drops the row of a send cancelled but not moved to Drafts, and says so', () => { + for (const name of ['onCancel', 'onEdit']) { + const handler = body(name); + expect(handler).toMatch(/\{ (inDrafts, )?moveError \} = await cancelScheduledToDrafts\(/); + const handled = handler.slice(handler.indexOf('cancelScheduledToDrafts('), handler.indexOf('scheduled.cancelled_not_moved\'')); + expect(handled).toMatch(/removeItem\(item\);/); + expect(handled).toMatch(/if \(moveError\) \{/); + } + }); +}); diff --git a/src/lib/compose-scheduled.ts b/src/lib/compose-scheduled.ts index 3d815f06..2dada447 100644 --- a/src/lib/compose-scheduled.ts +++ b/src/lib/compose-scheduled.ts @@ -12,26 +12,42 @@ export function scheduledAccountOverride(item: Pick return item.accountId !== ownAccountId ? item.accountId : undefined; } +/** Where a cancelled scheduled message is now. */ +export interface CancelledToDrafts { + /** Back in Drafts. False: still in Sent, but no longer scheduled. */ + inDrafts: boolean; + /** + * Why the move to Drafts failed after the cancel went through. The send is + * cancelled all the same: the row goes, and the user hears it stayed in + * Sent rather than "Cancel failed". + */ + moveError?: unknown; +} + /** - * Cancel the scheduled send and move the message back into Drafts. Resolves - * with whether it is in Drafts now: false when the account has no Drafts - * folder, where it stays in Sent. A failed cancel throws and moves nothing. + * Cancel the scheduled send and move the message back into Drafts. It stays + * in Sent when the account has no Drafts folder or the move fails (see + * CancelledToDrafts). A failed cancel throws and moves nothing. */ export async function cancelScheduledToDrafts( item: Pick, mailboxes: Mailbox[], ownAccountId: string, -): Promise { +): Promise { await cancelScheduledSend(item.emailSubmissionId, item.accountId); const scope = mailboxesOfAccount(mailboxes, scheduledAccountOverride(item, ownAccountId)); const drafts = scope.find((m) => m.role === 'drafts'); const sent = scope.find((m) => m.role === 'sent'); - if (!drafts) return false; - await restoreEmailToDraft( - item.emailId, - drafts.originalId ?? drafts.id, - sent ? (sent.originalId ?? sent.id) : undefined, - item.accountId, - ); - return true; + if (!drafts) return { inDrafts: false }; + try { + await restoreEmailToDraft( + item.emailId, + drafts.originalId ?? drafts.id, + sent ? (sent.originalId ?? sent.id) : undefined, + item.accountId, + ); + } catch (moveError) { + return { inDrafts: false, moveError }; + } + return { inDrafts: true }; } diff --git a/src/screens/ScheduledScreen.tsx b/src/screens/ScheduledScreen.tsx index a3f6c770..62c9bedf 100644 --- a/src/screens/ScheduledScreen.tsx +++ b/src/screens/ScheduledScreen.tsx @@ -194,9 +194,17 @@ export default function ScheduledScreen({ navigation }: Props) { style: 'destructive', onPress: () => { void runAction(item, async () => { - const inDrafts = await cancelScheduledToDrafts(item, mailboxes, jmapClient.accountId); + const { inDrafts, moveError } = await cancelScheduledToDrafts(item, mailboxes, jmapClient.accountId); clearUndoFor(item); removeItem(item); + if (moveError) { + // Not scheduled any more either way: it just stayed in Sent. + toast.warning( + t('scheduled.cancelled_not_moved', 'Send cancelled, but not moved to Drafts'), + { message: t('scheduled.cancelled_not_moved_body', 'The message will not be sent. It is still in Sent.') }, + ); + return; + } toast.success( t('scheduled.send_cancelled', 'Send cancelled'), inDrafts @@ -252,8 +260,17 @@ export default function ScheduledScreen({ navigation }: Props) { const onEdit = (item: ScheduledEmail) => { void runAction(item, async () => { // A message held in a shared account goes back to that account's Drafts. - await cancelScheduledToDrafts(item, mailboxes, jmapClient.accountId); + const { moveError } = await cancelScheduledToDrafts(item, mailboxes, jmapClient.accountId); clearUndoFor(item); + if (moveError) { + // Opening it in the composer would edit the copy in Sent. + removeItem(item); + toast.warning( + t('scheduled.cancelled_not_moved', 'Send cancelled, but not moved to Drafts'), + { message: t('scheduled.cancelled_not_moved_body', 'The message will not be sent. It is still in Sent.') }, + ); + return; + } const email = await getFullEmail(item.emailId, item.accountId); removeItem(item); navigation.replace('Compose', { draft: draftContextFromEmail(email, scheduledAccountOverride(item, jmapClient.accountId)) });