mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
fix: undo and send now of a delayed send only reach the login it was sent from
This commit is contained in:
@@ -340,6 +340,8 @@
|
||||
"remove_link": "Remove link"
|
||||
},
|
||||
"undo_send_label": "Sending in {seconds}s…",
|
||||
"undo_send_other_login": "This message was sent from another account",
|
||||
"undo_send_other_login_body": "Switch back to {account} to undo it or send it now. It goes out when the delay ends.",
|
||||
"upload_failed_short": "Upload failed",
|
||||
"uploading": "Uploading...",
|
||||
"uploading_pct": "Uploading {pct}%",
|
||||
|
||||
@@ -8,7 +8,7 @@ import { useSettingsStore } from '../../stores/settings-store';
|
||||
import { useLocaleStore } from '../../stores/locale-store';
|
||||
import { useEmailStore } from '../../stores/email-store';
|
||||
import { toast } from '../../stores/toast-store';
|
||||
import { useSendUndoStore } from '../../stores/send-undo-store';
|
||||
import { currentSendLogin, useSendUndoStore } from '../../stores/send-undo-store';
|
||||
import { sendEmail, patchKeywordsForEmails } from '../../api/email';
|
||||
import { jmapClient } from '../../api/jmap-client';
|
||||
import { pruneEmailSubmissions } from '../../api/submissions';
|
||||
@@ -103,6 +103,9 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF
|
||||
});
|
||||
const threading = computeReplyThreadingHeaders(email);
|
||||
const holdFor = jmapClient.undoSendHold(sendDelaySeconds, jmapAccountId);
|
||||
// Where the reply goes, read before it goes: Undo must reach that login
|
||||
// and account whatever the app switches to meanwhile.
|
||||
const sentThrough = { login: currentSendLogin(), accountId: jmapAccountId ?? jmapClient.connectedAccountId ?? undefined };
|
||||
const result = await sendEmail(
|
||||
{
|
||||
from: [{ name: identity.name, email: identity.email }],
|
||||
@@ -127,7 +130,7 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF
|
||||
// Recorded before the flag below so its round trip doesn't eat the window.
|
||||
useSendUndoStore.getState().recordHeldSend(result, holdFor, {
|
||||
identityId: identity.id,
|
||||
accountId: jmapAccountId,
|
||||
...sentThrough,
|
||||
from: [{ name: identity.name, email: identity.email }],
|
||||
});
|
||||
void pruneEmailSubmissions();
|
||||
|
||||
@@ -2184,6 +2184,9 @@ export default function ComposeScreen({ route, navigation }: Props) {
|
||||
setSending(true);
|
||||
try {
|
||||
const outgoing = buildOutgoing(primaryIdentity, liveBodyHtml, { forDraft: false });
|
||||
// The account the submission is held in, for Undo (read before the
|
||||
// request: a switch may land while it is out).
|
||||
const submissionAccountId = jmapClient.connectedAccountId ?? undefined;
|
||||
const result = await sendEmail(
|
||||
outgoing,
|
||||
primaryIdentity.id,
|
||||
@@ -2237,6 +2240,8 @@ export default function ComposeScreen({ route, navigation }: Props) {
|
||||
// Undo-send window: the undo bar offers Undo / Send now.
|
||||
useSendUndoStore.getState().recordHeldSend(result, holdForSeconds, {
|
||||
identityId: primaryIdentity.id,
|
||||
accountId: submissionAccountId,
|
||||
login: ownerLoginRef.current,
|
||||
from: outgoing.from,
|
||||
to: [...outgoing.to, ...(outgoing.cc ?? []), ...(outgoing.bcc ?? [])],
|
||||
});
|
||||
|
||||
@@ -7,11 +7,31 @@ vi.mock('../../api/email', () => ({
|
||||
getFullEmail: vi.fn(),
|
||||
}));
|
||||
|
||||
// The login the client serves: me@example.com on mail.example.com, whose
|
||||
// own JMAP account is 'primary'. Tests switch it to another server.
|
||||
const { client } = vi.hoisted(() => ({
|
||||
client: {
|
||||
accountId: 'primary',
|
||||
connectedAccountId: 'primary' as string | null,
|
||||
isConnected: true,
|
||||
username: 'me@example.com' as string | null,
|
||||
serverUrl: 'https://mail.example.com' as string | null,
|
||||
},
|
||||
}));
|
||||
vi.mock('../../api/jmap-client', () => ({
|
||||
jmapClient: { accountId: 'primary' },
|
||||
jmapClient: client,
|
||||
RequestTimeoutError: class RequestTimeoutError extends Error {},
|
||||
}));
|
||||
|
||||
const LOGIN = 'me@example.com@mail.example.com';
|
||||
const OTHER_LOGIN = 'me@example.com@other.example.org';
|
||||
|
||||
/** Switch the app to the same user on another server, whose own account is also 'primary'. */
|
||||
function switchToOtherServer() {
|
||||
useAccountStore.setState({ activeAccountId: OTHER_LOGIN });
|
||||
client.serverUrl = 'https://other.example.org';
|
||||
}
|
||||
|
||||
// Background work nobody waits for: it never settles here.
|
||||
vi.mock('../../api/submissions', () => ({
|
||||
pruneEmailSubmissions: vi.fn(() => new Promise<number>(() => undefined)),
|
||||
@@ -23,6 +43,7 @@ import { NoLongerScheduledError, RecipientsRefusedError, SubmissionQuotaError }
|
||||
import type { Email, Mailbox } from '../../api/types';
|
||||
import { useSendUndoStore, restoreUndoneSend, type PendingUndoSend } from '../send-undo-store';
|
||||
import { useToastStore } from '../toast-store';
|
||||
import { useAccountStore } from '../account-store';
|
||||
|
||||
const api = vi.mocked(emailApi);
|
||||
const prune = vi.mocked(submissions.pruneEmailSubmissions);
|
||||
@@ -70,6 +91,8 @@ beforeEach(() => {
|
||||
useSendUndoStore.setState({ pending: null, busy: false, restoredEmailId: null });
|
||||
useToastStore.getState().clearToasts();
|
||||
api.getFullEmail.mockResolvedValue({ id: 'mail-1', subject: 'Re: hi' } as Email);
|
||||
client.serverUrl = 'https://mail.example.com';
|
||||
useAccountStore.setState({ activeAccountId: LOGIN });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -93,9 +116,21 @@ describe('send-undo-store', () => {
|
||||
from: [SENDER],
|
||||
sendAt: HELD.sendAt,
|
||||
delaySeconds: 10,
|
||||
login: LOGIN,
|
||||
});
|
||||
});
|
||||
|
||||
it('names the own account and the login of an own-account send', () => {
|
||||
useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] });
|
||||
expect(useSendUndoStore.getState().pending).toMatchObject({ accountId: 'primary', login: LOGIN });
|
||||
});
|
||||
|
||||
it('keeps the login the sender sent through when a switch landed during the send', () => {
|
||||
client.serverUrl = 'https://other.example.org';
|
||||
useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', login: LOGIN, accountId: 'primary' });
|
||||
expect(useSendUndoStore.getState().pending?.login).toBe(LOGIN);
|
||||
});
|
||||
|
||||
it('records nothing for a send that went out at once', () => {
|
||||
const store = useSendUndoStore.getState();
|
||||
expect(store.recordHeldSend({ scheduled: false, emailSubmissionId: 'sub-1', emailId: 'mail-1' }, undefined, { identityId: 'id-1' })).toBe(false);
|
||||
@@ -121,9 +156,47 @@ describe('send-undo-store', () => {
|
||||
});
|
||||
|
||||
it('cancels an own-account send in the primary account', async () => {
|
||||
useSendUndoStore.getState().setPending(entry());
|
||||
useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] });
|
||||
await useSendUndoStore.getState().undo();
|
||||
expect(api.cancelScheduledSend).toHaveBeenCalledWith('sub-1', undefined);
|
||||
expect(api.cancelScheduledSend).toHaveBeenCalledWith('sub-1', 'primary');
|
||||
});
|
||||
|
||||
// GHSA-8x23: the same user on another server has an account 'primary'
|
||||
// too, with its own submission 'sub-1'.
|
||||
it('cancels nothing in another login, and says where the message was sent from', async () => {
|
||||
useAccountStore.setState({
|
||||
accounts: [{ id: LOGIN, email: 'me@example.com', displayName: 'Me' } as never],
|
||||
});
|
||||
useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] });
|
||||
const pending = useSendUndoStore.getState().pending;
|
||||
// The switch is still under way: the bar is up and the client serves the other server.
|
||||
client.serverUrl = 'https://other.example.org';
|
||||
|
||||
await expect(useSendUndoStore.getState().undo()).resolves.toBe(false);
|
||||
await expect(useSendUndoStore.getState().sendNow()).resolves.toBe(false);
|
||||
|
||||
expect(api.cancelScheduledSend).not.toHaveBeenCalled();
|
||||
expect(api.rescheduleScheduledSend).not.toHaveBeenCalled();
|
||||
expect(useToastStore.getState().toasts[0]).toMatchObject({
|
||||
type: 'error',
|
||||
message: expect.stringContaining('me@example.com'),
|
||||
});
|
||||
expect(useSendUndoStore.getState().pending).toBe(pending);
|
||||
await expect(restoreUndoneSend(pending!, MAILBOXES)).rejects.toThrow();
|
||||
expect(api.restoreEmailToDraft).not.toHaveBeenCalled();
|
||||
useAccountStore.setState({ accounts: [] });
|
||||
});
|
||||
|
||||
it('takes the undo bar away once another login is active', () => {
|
||||
useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] });
|
||||
switchToOtherServer();
|
||||
expect(useSendUndoStore.getState().pending).toBeNull();
|
||||
});
|
||||
|
||||
it('keeps the undo bar while the same login stays active', () => {
|
||||
useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] });
|
||||
useAccountStore.setState({ defaultAccountId: LOGIN });
|
||||
expect(useSendUndoStore.getState().pending).not.toBeNull();
|
||||
});
|
||||
|
||||
it('keeps the entry when the cancel fails, and says so', async () => {
|
||||
|
||||
@@ -14,7 +14,9 @@ import { NoLongerScheduledError } from '../api/jmap-result';
|
||||
import { pruneEmailSubmissions } from '../api/submissions';
|
||||
import type { EmailAddress, Mailbox } from '../api/types';
|
||||
import { draftContextFromEmail } from '../lib/draft-context';
|
||||
import { composerLoginMoved, servedLoginOf } from '../lib/draft-login';
|
||||
import { mailboxesOfAccount } from '../lib/mailbox-tree';
|
||||
import { useAccountStore } from './account-store';
|
||||
import { partlyRefusedWarning, rescheduleFailureNotice } from '../lib/send-failure-message';
|
||||
import type { ComposeDraftContext } from '../navigation/types';
|
||||
import { t } from './locale-store';
|
||||
@@ -24,8 +26,19 @@ export interface PendingUndoSend {
|
||||
emailSubmissionId: string;
|
||||
emailId: string;
|
||||
identityId: string;
|
||||
/** Account holding the submission: a shared account for a send from its identity. */
|
||||
/**
|
||||
* JMAP account holding the submission: a shared account for a send from
|
||||
* its identity, else the login's own. Recorded either way: submission ids
|
||||
* repeat across accounts and servers.
|
||||
*/
|
||||
accountId?: string;
|
||||
/**
|
||||
* The login (registry id, lib/draft-login) the message was sent through.
|
||||
* Undo and Send now act through that login only: after a switch the same
|
||||
* submission id names another message on the other server (webmail
|
||||
* GHSA-8x23).
|
||||
*/
|
||||
login?: string | null;
|
||||
from?: EmailAddress[];
|
||||
to?: EmailAddress[];
|
||||
/** ISO time the server will release the message. */
|
||||
@@ -36,7 +49,32 @@ export interface PendingUndoSend {
|
||||
}
|
||||
|
||||
/** What the sender knows about a held send beyond the server's answer. */
|
||||
export type HeldSendDetails = Pick<PendingUndoSend, 'identityId' | 'accountId' | 'from' | 'to'>;
|
||||
export type HeldSendDetails = Pick<PendingUndoSend, 'identityId' | 'accountId' | 'from' | 'to' | 'login'>;
|
||||
|
||||
/** The login the JMAP client works for now, or the registry's active one while it connects. */
|
||||
export function currentSendLogin(): string | null {
|
||||
return servedLoginOf(jmapClient) ?? useAccountStore.getState().activeAccountId;
|
||||
}
|
||||
|
||||
/** Whether Undo or Send now of `entry` would now go through another login. */
|
||||
export function pendingSendLoginMoved(entry: Pick<PendingUndoSend, 'login'>): boolean {
|
||||
return composerLoginMoved(entry.login, {
|
||||
activeAccountId: useAccountStore.getState().activeAccountId,
|
||||
servedLogin: servedLoginOf(jmapClient),
|
||||
});
|
||||
}
|
||||
|
||||
function refuseOtherLogin(entry: PendingUndoSend): void {
|
||||
const owner = entry.login ? useAccountStore.getState().getAccountById(entry.login) : undefined;
|
||||
toast.error(
|
||||
t('email_composer.undo_send_other_login', 'This message was sent from another account'),
|
||||
t(
|
||||
'email_composer.undo_send_other_login_body',
|
||||
'Switch back to {account} to undo it or send it now. It goes out when the delay ends.',
|
||||
{ account: owner?.email || owner?.displayName || '' },
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
interface SendUndoState {
|
||||
pending: PendingUndoSend | null;
|
||||
@@ -71,6 +109,10 @@ export const useSendUndoStore = create<SendUndoState>((set, get) => ({
|
||||
if (!holdForSeconds || !result.scheduled || !result.emailSubmissionId || !result.emailId) return false;
|
||||
get().setPending({
|
||||
...details,
|
||||
accountId: details.accountId ?? jmapClient.connectedAccountId ?? undefined,
|
||||
// The sender reads the login before it sends; a switch may land while
|
||||
// the request is out.
|
||||
login: details.login !== undefined ? details.login : currentSendLogin(),
|
||||
emailSubmissionId: result.emailSubmissionId,
|
||||
emailId: result.emailId,
|
||||
sendAt: result.sendAt,
|
||||
@@ -84,6 +126,10 @@ export const useSendUndoStore = create<SendUndoState>((set, get) => ({
|
||||
undo: async () => {
|
||||
const entry = get().pending;
|
||||
if (!entry || get().busy) return false;
|
||||
if (pendingSendLoginMoved(entry)) {
|
||||
refuseOtherLogin(entry);
|
||||
return false;
|
||||
}
|
||||
set({ busy: true });
|
||||
try {
|
||||
await cancelScheduledSend(entry.emailSubmissionId, entry.accountId);
|
||||
@@ -105,6 +151,10 @@ export const useSendUndoStore = create<SendUndoState>((set, get) => ({
|
||||
sendNow: async () => {
|
||||
const entry = get().pending;
|
||||
if (!entry || get().busy) return false;
|
||||
if (pendingSendLoginMoved(entry)) {
|
||||
refuseOtherLogin(entry);
|
||||
return false;
|
||||
}
|
||||
set({ busy: true });
|
||||
try {
|
||||
const result = await rescheduleScheduledSend(
|
||||
@@ -151,6 +201,8 @@ export async function restoreUndoneSend(
|
||||
entry: PendingUndoSend,
|
||||
mailboxes: Mailbox[],
|
||||
): Promise<ComposeDraftContext> {
|
||||
// The ids name other messages in another login (see pendingSendLoginMoved).
|
||||
if (pendingSendLoginMoved(entry)) throw new Error('The account was switched');
|
||||
const shared = !!entry.accountId && entry.accountId !== jmapClient.accountId;
|
||||
const accountId = shared ? entry.accountId : undefined;
|
||||
const scope = mailboxesOfAccount(mailboxes, accountId);
|
||||
@@ -166,3 +218,12 @@ export async function restoreUndoneSend(
|
||||
}
|
||||
return draftContextFromEmail(await getFullEmail(entry.emailId, accountId), accountId);
|
||||
}
|
||||
|
||||
// A switch to another login takes the undo bar away, since its actions
|
||||
// would reach the other server. The held message still goes out when the
|
||||
// delay ends; back in its account, Scheduled lists it until then.
|
||||
useAccountStore.subscribe((state, prev) => {
|
||||
if (state.activeAccountId === prev.activeAccountId) return;
|
||||
const { pending } = useSendUndoStore.getState();
|
||||
if (pending?.login && pending.login !== state.activeAccountId) useSendUndoStore.setState({ pending: null });
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user