mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
fix: list and cancel scheduled sends held in shared accounts
A send from a shared or group account's identity (quick reply with the undo-send delay, or a message scheduled in the webmail) is held in that account. The Scheduled screen only looked in the primary account, so it never listed such a message, and it could not be cancelled, rescheduled or edited before it went out (webmail #874). The Scheduled list now covers every account that advertises submission and can hold mail. Each entry carries its account, and cancel, reschedule, "Send now" and edit act there. Edit restores the message into that account's Drafts and opens it as a shared draft. A shared account that fails to answer is skipped, so the user's own scheduled mail still shows.
This commit is contained in:
@@ -1,10 +1,12 @@
|
|||||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||||
|
|
||||||
vi.mock('../jmap-client', () => ({
|
vi.mock('../jmap-client', () => ({
|
||||||
jmapClient: {
|
jmapClient: {
|
||||||
accountId: 'acc-1',
|
accountId: 'acc-1',
|
||||||
request: vi.fn(),
|
request: vi.fn(),
|
||||||
learnHoldLimit: vi.fn(),
|
learnHoldLimit: vi.fn(),
|
||||||
|
getSubmissionAccountIds: vi.fn(() => ['acc-1']),
|
||||||
|
hasDelayedSend: vi.fn(() => true),
|
||||||
getMaxCallsInRequest: vi.fn(() => 16),
|
getMaxCallsInRequest: vi.fn(() => 16),
|
||||||
getMaxObjectsInGet: vi.fn(() => 500),
|
getMaxObjectsInGet: vi.fn(() => 500),
|
||||||
getMaxObjectsInSet: vi.fn(() => 500),
|
getMaxObjectsInSet: vi.fn(() => 500),
|
||||||
@@ -12,7 +14,7 @@ vi.mock('../jmap-client', () => ({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
import { jmapClient } from '../jmap-client';
|
import { jmapClient } from '../jmap-client';
|
||||||
import { rescheduleScheduledSend, sendEmail } from '../email';
|
import { cancelScheduledSend, listScheduledEmails, rescheduleScheduledSend, sendEmail } from '../email';
|
||||||
import { ScheduleTooLateError } from '../jmap-result';
|
import { ScheduleTooLateError } from '../jmap-result';
|
||||||
|
|
||||||
const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
|
const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
|
||||||
@@ -223,3 +225,89 @@ describe('hold-limit rejections', () => {
|
|||||||
expect(jmapClient.learnHoldLimit).toHaveBeenCalledWith(172_800);
|
expect(jmapClient.learnHoldLimit).toHaveBeenCalledWith(172_800);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('scheduled sends in shared accounts (webmail #874)', () => {
|
||||||
|
const FUTURE = new Date(Date.now() + 3600_000).toISOString();
|
||||||
|
const LATER = new Date(Date.now() + 7200_000).toISOString();
|
||||||
|
const byAccount: Record<string, { submissions: unknown[]; emails: unknown[] } | Error> = {};
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
(jmapClient.getSubmissionAccountIds as ReturnType<typeof vi.fn>).mockReturnValue(['acc-1', 'grp-1', 'grp-2']);
|
||||||
|
// grp-2 advertises submission but can't hold mail.
|
||||||
|
(jmapClient.hasDelayedSend as ReturnType<typeof vi.fn>).mockImplementation((id: string) => id !== 'grp-2');
|
||||||
|
byAccount['acc-1'] = {
|
||||||
|
submissions: [{ id: 's-own', emailId: 'e-own', identityId: 'i-own', sendAt: LATER, undoStatus: 'pending' }],
|
||||||
|
emails: [{ id: 'e-own', subject: 'Own' }],
|
||||||
|
};
|
||||||
|
byAccount['grp-1'] = {
|
||||||
|
submissions: [
|
||||||
|
{ id: 's-grp', emailId: 'e-grp', identityId: 'i-grp', sendAt: FUTURE, undoStatus: 'pending' },
|
||||||
|
{ id: 's-done', emailId: 'e-done', identityId: 'i-grp', sendAt: FUTURE, undoStatus: 'final' },
|
||||||
|
],
|
||||||
|
emails: [{ id: 'e-grp', subject: 'Team' }],
|
||||||
|
};
|
||||||
|
mockRequest.mockImplementation(async (calls: Array<[string, { accountId: string }, string]>) => {
|
||||||
|
const [method, args] = calls[0];
|
||||||
|
const data = byAccount[args.accountId];
|
||||||
|
if (data instanceof Error) throw data;
|
||||||
|
if (!data) throw new Error(`unexpected account ${args.accountId}`);
|
||||||
|
if (method === 'EmailSubmission/query') {
|
||||||
|
return { methodResponses: [[method, { ids: (data.submissions as Array<{ id: string }>).map((s) => s.id) }, '0']] };
|
||||||
|
}
|
||||||
|
if (method === 'EmailSubmission/get') return { methodResponses: [[method, { list: data.submissions }, '0']] };
|
||||||
|
if (method === 'Email/get') return { methodResponses: [[method, { list: data.emails }, '0']] };
|
||||||
|
throw new Error(`unexpected ${method}`);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
mockRequest.mockReset();
|
||||||
|
(jmapClient.getSubmissionAccountIds as ReturnType<typeof vi.fn>).mockReturnValue(['acc-1']);
|
||||||
|
(jmapClient.hasDelayedSend as ReturnType<typeof vi.fn>).mockReturnValue(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('lists the pending sends of every account that can hold mail, tagged with their account', async () => {
|
||||||
|
const list = await listScheduledEmails();
|
||||||
|
|
||||||
|
expect(list.map((s) => [s.accountId, s.emailSubmissionId, s.subject])).toEqual([
|
||||||
|
['grp-1', 's-grp', 'Team'],
|
||||||
|
['acc-1', 's-own', 'Own'],
|
||||||
|
]);
|
||||||
|
const asked = mockRequest.mock.calls.map((c) => c[0][0][1].accountId);
|
||||||
|
expect(asked).not.toContain('grp-2');
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps the user's own scheduled sends when a shared account fails", async () => {
|
||||||
|
byAccount['grp-1'] = new Error('forbidden');
|
||||||
|
const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
|
||||||
|
|
||||||
|
const list = await listScheduledEmails();
|
||||||
|
|
||||||
|
expect(list.map((s) => s.emailSubmissionId)).toEqual(['s-own']);
|
||||||
|
warn.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still fails when the primary account fails', async () => {
|
||||||
|
byAccount['acc-1'] = new Error('server down');
|
||||||
|
|
||||||
|
await expect(listScheduledEmails()).rejects.toThrow('server down');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('cancels and reschedules in the account holding the submission', async () => {
|
||||||
|
mockRequest.mockReset();
|
||||||
|
mockRequest.mockResolvedValueOnce({
|
||||||
|
methodResponses: [['EmailSubmission/set', { updated: { 's-grp': null } }, '0']],
|
||||||
|
});
|
||||||
|
await cancelScheduledSend('s-grp', 'grp-1');
|
||||||
|
expect(mockRequest.mock.calls[0][0][0][1].accountId).toBe('grp-1');
|
||||||
|
|
||||||
|
mockRequest.mockResolvedValueOnce({
|
||||||
|
methodResponses: [['EmailSubmission/set', {
|
||||||
|
updated: { 's-grp': null },
|
||||||
|
created: { replacement: { id: 's-new', sendAt: FUTURE } },
|
||||||
|
}, '0']],
|
||||||
|
});
|
||||||
|
await rescheduleScheduledSend({ ...SCHEDULED, emailSubmissionId: 's-grp', accountId: 'grp-1' }, 0);
|
||||||
|
expect(mockRequest.mock.calls[1][0][0][1].accountId).toBe('grp-1');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -352,6 +352,13 @@ describe('JMAPClient', () => {
|
|||||||
expect(client.getMaxDelayedSend()).toBe(0);
|
expect(client.getMaxDelayedSend()).toBe(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('lists every account that can hold submissions, primary first (webmail #874)', async () => {
|
||||||
|
global.fetch = mockFetch([{ status: 200, json: STALWART_SESSION }]) as any;
|
||||||
|
await client.connect('https://mail.example.com', 'user', 'pass');
|
||||||
|
|
||||||
|
expect(client.getSubmissionAccountIds()).toEqual(['acc-1', 'shared-no-hold']);
|
||||||
|
});
|
||||||
|
|
||||||
describe('hold limit', () => {
|
describe('hold limit', () => {
|
||||||
const withSubmission = (maxDelayedSend: number, stalwart: boolean): JMAPSession => ({
|
const withSubmission = (maxDelayedSend: number, stalwart: boolean): JMAPSession => ({
|
||||||
...STALWART_SESSION,
|
...STALWART_SESSION,
|
||||||
|
|||||||
+38
-10
@@ -1331,6 +1331,8 @@ export async function createDraft(
|
|||||||
}
|
}
|
||||||
|
|
||||||
export interface ScheduledEmail {
|
export interface ScheduledEmail {
|
||||||
|
/** Account holding the submission: a shared account for a send from its identity. */
|
||||||
|
accountId: string;
|
||||||
emailSubmissionId: string;
|
emailSubmissionId: string;
|
||||||
emailId: string;
|
emailId: string;
|
||||||
identityId: string;
|
identityId: string;
|
||||||
@@ -1345,10 +1347,27 @@ export interface ScheduledEmail {
|
|||||||
|
|
||||||
// List pending (not-yet-delivered, not cancelled) scheduled submissions whose
|
// List pending (not-yet-delivered, not cancelled) scheduled submissions whose
|
||||||
// send time is still in the future, joined with a light Email/get so the UI
|
// send time is still in the future, joined with a light Email/get so the UI
|
||||||
// can show subject/recipients. Empty when the server lacks FUTURERELEASE.
|
// can show subject/recipients. Covers every account that can hold mail: a
|
||||||
|
// send from a shared identity is held in the shared account, where a
|
||||||
|
// primary-only lookup never found it, so it could not be cancelled (webmail
|
||||||
|
// #874). Empty when no account supports FUTURERELEASE.
|
||||||
export async function listScheduledEmails(): Promise<ScheduledEmail[]> {
|
export async function listScheduledEmails(): Promise<ScheduledEmail[]> {
|
||||||
if (!jmapClient.hasDelayedSend()) return [];
|
const accountIds = jmapClient.getSubmissionAccountIds().filter((id) => jmapClient.hasDelayedSend(id));
|
||||||
const accountId = jmapClient.accountId;
|
const out: ScheduledEmail[] = [];
|
||||||
|
for (const accountId of accountIds) {
|
||||||
|
try {
|
||||||
|
out.push(...(await listScheduledEmailsIn(accountId)));
|
||||||
|
} catch (err) {
|
||||||
|
// A shared account that can't answer (revoked access, server error)
|
||||||
|
// must not hide the user's own scheduled mail.
|
||||||
|
if (accountId === jmapClient.accountId) throw err;
|
||||||
|
console.warn(`[email] scheduled sends of account ${accountId} unavailable:`, err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out.sort((a, b) => new Date(a.sendAt).getTime() - new Date(b.sendAt).getTime());
|
||||||
|
}
|
||||||
|
|
||||||
|
async function listScheduledEmailsIn(accountId: string): Promise<ScheduledEmail[]> {
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
|
|
||||||
const queryRes = await jmapClient.request(
|
const queryRes = await jmapClient.request(
|
||||||
@@ -1406,6 +1425,7 @@ export async function listScheduledEmails(): Promise<ScheduledEmail[]> {
|
|||||||
const e = emailById.get(s.emailId);
|
const e = emailById.get(s.emailId);
|
||||||
if (!s.sendAt) return null;
|
if (!s.sendAt) return null;
|
||||||
return {
|
return {
|
||||||
|
accountId,
|
||||||
emailSubmissionId: s.id,
|
emailSubmissionId: s.id,
|
||||||
emailId: s.emailId,
|
emailId: s.emailId,
|
||||||
identityId: s.identityId,
|
identityId: s.identityId,
|
||||||
@@ -1418,14 +1438,14 @@ export async function listScheduledEmails(): Promise<ScheduledEmail[]> {
|
|||||||
preview: e?.preview,
|
preview: e?.preview,
|
||||||
};
|
};
|
||||||
})
|
})
|
||||||
.filter((s): s is ScheduledEmail => s !== null)
|
.filter((s): s is ScheduledEmail => s !== null);
|
||||||
.sort((a, b) => new Date(a.sendAt).getTime() - new Date(b.sendAt).getTime());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Cancel a pending scheduled send. The held message copy stays in Sent; only
|
// Cancel a pending scheduled send. The held message copy stays in Sent; only
|
||||||
// delivery is stopped (matches the webmail behaviour).
|
// delivery is stopped (matches the webmail behaviour). `accountIdOverride` is
|
||||||
export async function cancelScheduledSend(emailSubmissionId: string): Promise<void> {
|
// the account holding the submission (see `ScheduledEmail.accountId`).
|
||||||
const accountId = jmapClient.accountId;
|
export async function cancelScheduledSend(emailSubmissionId: string, accountIdOverride?: string): Promise<void> {
|
||||||
|
const accountId = accountIdOverride ?? jmapClient.accountId;
|
||||||
const res = await jmapClient.request(
|
const res = await jmapClient.request(
|
||||||
[['EmailSubmission/set', {
|
[['EmailSubmission/set', {
|
||||||
accountId,
|
accountId,
|
||||||
@@ -1449,11 +1469,19 @@ export async function cancelScheduledSend(emailSubmissionId: string): Promise<vo
|
|||||||
* submission id and resolved send time.
|
* submission id and resolved send time.
|
||||||
*/
|
*/
|
||||||
export async function rescheduleScheduledSend(
|
export async function rescheduleScheduledSend(
|
||||||
scheduled: { emailSubmissionId: string; emailId: string; identityId: string; from?: EmailAddress[]; to?: EmailAddress[] },
|
scheduled: {
|
||||||
|
emailSubmissionId: string;
|
||||||
|
emailId: string;
|
||||||
|
identityId: string;
|
||||||
|
from?: EmailAddress[];
|
||||||
|
to?: EmailAddress[];
|
||||||
|
/** Account holding the submission; the replacement is created there too. */
|
||||||
|
accountId?: string;
|
||||||
|
},
|
||||||
holdForSeconds: number,
|
holdForSeconds: number,
|
||||||
recipients?: EmailAddress[],
|
recipients?: EmailAddress[],
|
||||||
): Promise<{ emailSubmissionId?: string; sendAt?: string }> {
|
): Promise<{ emailSubmissionId?: string; sendAt?: string }> {
|
||||||
const accountId = jmapClient.accountId;
|
const accountId = scheduled.accountId ?? jmapClient.accountId;
|
||||||
const create: Record<string, unknown> = { emailId: scheduled.emailId, identityId: scheduled.identityId };
|
const create: Record<string, unknown> = { emailId: scheduled.emailId, identityId: scheduled.identityId };
|
||||||
if (holdForSeconds > 0) {
|
if (holdForSeconds > 0) {
|
||||||
// Reuse the held submission's envelope: it names every recipient,
|
// Reuse the held submission's envelope: it names every recipient,
|
||||||
|
|||||||
@@ -1043,6 +1043,23 @@ export class JMAPClient {
|
|||||||
return this.session?.primaryAccounts?.[CAPABILITIES.SUBMISSION] || id;
|
return this.session?.primaryAccounts?.[CAPABILITIES.SUBMISSION] || id;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Every account whose scheduled sends this session can see: the primary
|
||||||
|
* submission account plus each shared/group account that advertises
|
||||||
|
* submission itself. A send from a shared identity is held in that
|
||||||
|
* account, so a lookup in the primary one alone never found it (webmail
|
||||||
|
* #874).
|
||||||
|
*/
|
||||||
|
getSubmissionAccountIds(): string[] {
|
||||||
|
const ids: string[] = [];
|
||||||
|
const primary = this.submissionAccountId();
|
||||||
|
if (primary) ids.push(primary);
|
||||||
|
for (const [id, account] of Object.entries(this.session?.accounts ?? {})) {
|
||||||
|
if (!ids.includes(id) && account?.accountCapabilities?.[CAPABILITIES.SUBMISSION]) ids.push(id);
|
||||||
|
}
|
||||||
|
return ids;
|
||||||
|
}
|
||||||
|
|
||||||
private submissionCapability(accountId?: string):
|
private submissionCapability(accountId?: string):
|
||||||
| { maxDelayedSend?: number; submissionExtensions?: unknown }
|
| { maxDelayedSend?: number; submissionExtensions?: unknown }
|
||||||
| undefined {
|
| undefined {
|
||||||
|
|||||||
@@ -104,7 +104,7 @@ export default function ScheduledScreen({ navigation }: Props) {
|
|||||||
style: 'destructive',
|
style: 'destructive',
|
||||||
onPress: () => {
|
onPress: () => {
|
||||||
void runAction(item, async () => {
|
void runAction(item, async () => {
|
||||||
await cancelScheduledSend(item.emailSubmissionId);
|
await cancelScheduledSend(item.emailSubmissionId, item.accountId);
|
||||||
if (useSendUndoStore.getState().pending?.emailSubmissionId === item.emailSubmissionId) {
|
if (useSendUndoStore.getState().pending?.emailSubmissionId === item.emailSubmissionId) {
|
||||||
useSendUndoStore.getState().clear();
|
useSendUndoStore.getState().clear();
|
||||||
}
|
}
|
||||||
@@ -135,7 +135,7 @@ export default function ScheduledScreen({ navigation }: Props) {
|
|||||||
);
|
);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const max = jmapClient.getMaxDelayedSend();
|
const max = jmapClient.getMaxDelayedSend(item.accountId);
|
||||||
if (max > 0 && seconds > max) {
|
if (max > 0 && seconds > max) {
|
||||||
Alert.alert(
|
Alert.alert(
|
||||||
t('email_composer.schedule_too_late_title', 'Too far ahead'),
|
t('email_composer.schedule_too_late_title', 'Too far ahead'),
|
||||||
@@ -159,17 +159,28 @@ export default function ScheduledScreen({ navigation }: Props) {
|
|||||||
// the composer (webmail cancelScheduledEmailForEdit).
|
// the composer (webmail cancelScheduledEmailForEdit).
|
||||||
const onEdit = (item: ScheduledEmail) => {
|
const onEdit = (item: ScheduledEmail) => {
|
||||||
void runAction(item, async () => {
|
void runAction(item, async () => {
|
||||||
await cancelScheduledSend(item.emailSubmissionId);
|
await cancelScheduledSend(item.emailSubmissionId, item.accountId);
|
||||||
if (useSendUndoStore.getState().pending?.emailSubmissionId === item.emailSubmissionId) {
|
if (useSendUndoStore.getState().pending?.emailSubmissionId === item.emailSubmissionId) {
|
||||||
useSendUndoStore.getState().clear();
|
useSendUndoStore.getState().clear();
|
||||||
}
|
}
|
||||||
const own = ownMailboxes(mailboxes);
|
// A message held in a shared account goes back to that account's Drafts.
|
||||||
const drafts = own.find((m) => m.role === 'drafts');
|
const shared = item.accountId !== jmapClient.accountId;
|
||||||
const sent = own.find((m) => m.role === 'sent');
|
const scope = shared
|
||||||
if (drafts) await restoreEmailToDraft(item.emailId, drafts.id, sent?.id);
|
? mailboxes.filter((m) => m.isShared && m.accountId === item.accountId)
|
||||||
const email = await getFullEmail(item.emailId);
|
: ownMailboxes(mailboxes);
|
||||||
|
const drafts = scope.find((m) => m.role === 'drafts');
|
||||||
|
const sent = scope.find((m) => m.role === 'sent');
|
||||||
|
if (drafts) {
|
||||||
|
await restoreEmailToDraft(
|
||||||
|
item.emailId,
|
||||||
|
drafts.originalId ?? drafts.id,
|
||||||
|
sent ? (sent.originalId ?? sent.id) : undefined,
|
||||||
|
item.accountId,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
const email = await getFullEmail(item.emailId, item.accountId);
|
||||||
removeItem(item.emailSubmissionId);
|
removeItem(item.emailSubmissionId);
|
||||||
navigation.replace('Compose', { draft: draftContextFromEmail(email) });
|
navigation.replace('Compose', { draft: draftContextFromEmail(email, shared ? item.accountId : undefined) });
|
||||||
}, t('scheduled.edit_failed', 'Could not open the message for editing'));
|
}, t('scheduled.edit_failed', 'Could not open the message for editing'));
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -180,7 +191,7 @@ export default function ScheduledScreen({ navigation }: Props) {
|
|||||||
tomorrowMorning.setDate(tomorrowMorning.getDate() + 1);
|
tomorrowMorning.setDate(tomorrowMorning.getDate() + 1);
|
||||||
tomorrowMorning.setHours(8, 0, 0, 0);
|
tomorrowMorning.setHours(8, 0, 0, 0);
|
||||||
// Only offer times within the server's hold limit.
|
// Only offer times within the server's hold limit.
|
||||||
const maxMs = jmapClient.getMaxDelayedSend() * 1000;
|
const maxMs = jmapClient.getMaxDelayedSend(rescheduleFor?.accountId) * 1000;
|
||||||
return [
|
return [
|
||||||
{ label: t('email_composer.schedule_in_1h', 'In 1 hour'), date: inHours(1) },
|
{ label: t('email_composer.schedule_in_1h', 'In 1 hour'), date: inHours(1) },
|
||||||
{ label: t('email_composer.schedule_in_3h', 'In 3 hours'), date: inHours(3) },
|
{ label: t('email_composer.schedule_in_3h', 'In 3 hours'), date: inHours(3) },
|
||||||
|
|||||||
Reference in New Issue
Block a user