fix: drop a cancelled scheduled send from the list when it can't go back to Drafts, and say it stayed in Sent

This commit is contained in:
Linus Rath
2026-10-29 04:59:32 +01:00
parent 7594e45937
commit 51b3208b3b
6 changed files with 88 additions and 18 deletions
+2
View File
@@ -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.",
@@ -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);
+11 -2
View File
@@ -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');
@@ -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\) \{/);
}
});
});
+28 -12
View File
@@ -12,26 +12,42 @@ export function scheduledAccountOverride(item: Pick<ScheduledEmail, 'accountId'>
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<ScheduledEmail, 'accountId' | 'emailSubmissionId' | 'emailId'>,
mailboxes: Mailbox[],
ownAccountId: string,
): Promise<boolean> {
): Promise<CancelledToDrafts> {
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 };
}
+19 -2
View File
@@ -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)) });