From 893f40a21ddfa75d5e68c330f8ab6936ad44d6d9 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 04:40:19 +0100 Subject: [PATCH] fix: send group mail through the group's identity on Send now and quick reply --- src/api/__tests__/email-submission.test.ts | 33 +++++++++++- src/api/email.ts | 10 +++- src/components/email/QuickReplyBox.tsx | 51 +++++++++++-------- .../compose-send-target-wiring.test.ts | 29 +++++++++++ src/screens/ComposeScreen.tsx | 4 +- 5 files changed, 101 insertions(+), 26 deletions(-) create mode 100644 src/lib/__tests__/compose-send-target-wiring.test.ts diff --git a/src/api/__tests__/email-submission.test.ts b/src/api/__tests__/email-submission.test.ts index f0f55ded..e148012a 100644 --- a/src/api/__tests__/email-submission.test.ts +++ b/src/api/__tests__/email-submission.test.ts @@ -107,7 +107,7 @@ describe('rescheduleScheduledSend', () => { expect(result).toEqual({ emailSubmissionId: 'sub-2', sendAt: '2026-09-25T08:00:00Z' }); const [lookup] = mockRequest.mock.calls[0][0]; expect(lookup).toEqual([ - 'EmailSubmission/get', { accountId: 'acc-1', ids: ['sub-1'], properties: ['envelope', 'sendAt'] }, '0', + 'EmailSubmission/get', { accountId: 'acc-1', ids: ['sub-1'], properties: ['envelope', 'sendAt', 'identityId'] }, '0', ]); expect(createdArgs().create.replacement.envelope).toEqual({ mailFrom: { email: 'me@example.com', parameters: { HOLDFOR: '7200' } }, @@ -218,6 +218,37 @@ describe('rescheduleScheduledSend', () => { vi.mocked(Date.now).mockRestore(); }); + // A group send's undo entry carried the login's own identity id, which + // names nothing in the group account: both creates were refused and the + // message went back to Drafts unsent. + it("submits the replacement and the put-back through the original's identity, not the caller's", async () => { + const lookup = structuredClone(LOOKUP); + (lookup.methodResponses[0][1] as { list: Array> }).list[0].identityId = 'grp-identity'; + mockRequest + .mockResolvedValueOnce(lookup) + .mockResolvedValueOnce(CANCELLED) + .mockResolvedValueOnce(DESTROYED('sub-1')) + .mockResolvedValueOnce(REFUSED('Quota exceeded')) + .mockResolvedValueOnce(set({ created: { replacement: { id: 'sub-3' } } })); + + await rescheduleScheduledSend({ ...SCHEDULED, identityId: 'own-identity', accountId: 'grp-1' }, 0).catch(() => undefined); + + expect(createdArgs(0).create.replacement.identityId).toBe('grp-identity'); + expect(createdArgs(1).create.replacement.identityId).toBe('grp-identity'); + }); + + it("keeps the caller's identity when the server does not name one", async () => { + mockRequest + .mockResolvedValueOnce(LOOKUP) + .mockResolvedValueOnce(CANCELLED) + .mockResolvedValueOnce(DESTROYED('sub-1')) + .mockResolvedValueOnce({ methodResponses: REPLACED }); + + await rescheduleScheduledSend(SCHEDULED, 0); + + expect(createdArgs().create.replacement.identityId).toBe('id-1'); + }); + it('moves the message back to Drafts when neither the replacement nor the original can be made', async () => { vi.spyOn(console, 'warn').mockImplementation(() => undefined); mockRequest diff --git a/src/api/email.ts b/src/api/email.ts index 75fedc13..f4c4ecce 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -2230,7 +2230,7 @@ export async function rescheduleScheduledSend( const lookup = await jmapClient.request( [ ['EmailSubmission/get', { - accountId, ids: [scheduled.emailSubmissionId], properties: ['envelope', 'sendAt'], + accountId, ids: [scheduled.emailSubmissionId], properties: ['envelope', 'sendAt', 'identityId'], }, '0'], ['Email/get', { accountId, ids: [scheduled.emailId], properties: ['to', 'cc', 'bcc'] }, '1'], ['Mailbox/get', { accountId, ids: null, properties: ['id', 'role'] }, '2'], @@ -2240,8 +2240,14 @@ export async function rescheduleScheduledSend( const original = (requireMethodResult(lookup, '0', 'EmailSubmission/get').list as Array<{ envelope?: { mailFrom?: { email?: string }; rcptTo?: Array<{ email?: string }> } | null; sendAt?: string; + identityId?: string; }> | undefined)?.[0]; const envelope = original?.envelope; + // The original's identity is one of the submission account's. The caller's + // may be the composer's, from the login's own account: for a group + // identity's message that id names nothing in the group account, and the + // replacement and the put-back were both refused (webmail parity). + const identityId = original?.identityId || scheduled.identityId; const held = requireMethodResult(lookup, '1', 'Email/get').list as Email[] | undefined; const headerRecipients = [...(held?.[0]?.to ?? []), ...(held?.[0]?.cc ?? []), ...(held?.[0]?.bcc ?? [])]; const source = recipients ?? (envelope?.rcptTo?.length ? envelope.rcptTo : headerRecipients); @@ -2271,7 +2277,7 @@ export async function rescheduleScheduledSend( create: { replacement: { emailId: scheduled.emailId, - identityId: scheduled.identityId, + identityId, envelope: { mailFrom: seconds === undefined ? { email: mailFrom } diff --git a/src/components/email/QuickReplyBox.tsx b/src/components/email/QuickReplyBox.tsx index 3d3f7068..420c8f77 100644 --- a/src/components/email/QuickReplyBox.tsx +++ b/src/components/email/QuickReplyBox.tsx @@ -22,7 +22,7 @@ import { resolveReplyIdentity } from '../../lib/reply-identity'; import { signatureIdentityFor, signPlainTextReply } from '../../lib/signature-utils'; import { pickEmailBody, plainTextBody } from '../../lib/email-body'; import { htmlToPlainText } from '../../lib/compose-html'; -import { mailboxesOfAccount } from '../../lib/mailbox-tree'; +import { sendTargetFor } from '../../lib/compose-send-account'; import { emailDisplayDate } from '../../lib/email-date'; import { buildQuoteHeader, quoteHeaderLabels } from '../../lib/quote-header'; @@ -66,16 +66,19 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF const from = email.from?.[0]; if (!from?.email || email.keywords?.$draft) return null; + const refuseAfterSwitch = () => { + if (!loginMovedFrom(ownerLoginRef.current)) return false; + Alert.alert( + t('email_composer.login_switched_title', 'Account switched'), + t('email_viewer.quick_reply_login_switched', 'This message is in another account. Switch back to it to reply.'), + ); + return true; + }; + const send = async () => { const body = text.trim(); if (!body || sending) return; - if (loginMovedFrom(ownerLoginRef.current)) { - Alert.alert( - t('email_composer.login_switched_title', 'Account switched'), - t('email_viewer.quick_reply_login_switched', 'This message is in another account. Switch back to it to reply.'), - ); - return; - } + if (refuseAfterSwitch()) return; const ownEmails = identities.map((i) => i.email).filter(Boolean); // The own identity the message was delivered to (or, for our own message, // the one that sent it). Never the catch-all From rewrite: this box has no @@ -88,16 +91,20 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF Alert.alert(t('common.error', 'Error'), t('email_viewer.unsubscribe_banner.no_identity', 'No sending identity available')); return; } - // Sent/Drafts of the account the reply is submitted from: the message's. - const scoped = mailboxesOfAccount(mailboxes, jmapAccountId); - const sent = scoped.find((m) => m.role === 'sent'); - const drafts = scoped.find((m) => m.role === 'drafts'); - if (!sent) { - Alert.alert(t('common.error', 'Error'), t('email_composer.no_sent_folder', 'No Sent folder found')); - return; - } setSending(true); try { + // The account, identity and folders the reply goes out through, as the + // composer picks them (#1090): a group address sends from the group's + // account with that account's identity id. The message's own account + // with the login's identity id named nothing there, or another + // identity. + const target = await sendTargetFor(identity, identity.email, mailboxes); + if (!target.sentMailboxId) { + Alert.alert(t('common.error', 'Error'), t('email_composer.no_sent_folder', 'No Sent folder found')); + return; + } + // The lookup may have outlasted a switch of logins. + if (refuseAfterSwitch()) return; const recipients = buildReplyRecipients( { from: email.from, replyToAddresses: email.replyTo, to: email.to, cc: email.cc }, 'reply', @@ -116,10 +123,10 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF labels: quoteHeaderLabels(t), }); const threading = computeReplyThreadingHeaders(email); - const holdFor = jmapClient.undoSendHold(sendDelaySeconds, jmapAccountId); + const holdFor = jmapClient.undoSendHold(sendDelaySeconds, target.accountId); // 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 sentThrough = { login: currentSendLogin(), accountId: target.accountId ?? jmapClient.connectedAccountId ?? undefined }; const result = await sendEmail( { from: [{ name: identity.name, email: identity.email }], @@ -135,15 +142,15 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF inReplyTo: threading?.inReplyTo, references: threading?.references, }, - identity.id, - sent.originalId ?? sent.id, + target.identityId, + target.sentMailboxId, holdFor, - { draftsMailboxId: drafts ? (drafts.originalId ?? drafts.id) : undefined, accountId: jmapAccountId }, + { draftsMailboxId: target.draftsMailboxId, accountId: target.accountId }, ); // Held for the undo-send delay: the undo bar offers Undo / Send now. // Recorded before the flag below so its round trip doesn't eat the window. useSendUndoStore.getState().recordHeldSend(result, holdFor, { - identityId: identity.id, + identityId: target.identityId, ...sentThrough, from: [{ name: identity.name, email: identity.email }], }); diff --git a/src/lib/__tests__/compose-send-target-wiring.test.ts b/src/lib/__tests__/compose-send-target-wiring.test.ts new file mode 100644 index 00000000..1495b893 --- /dev/null +++ b/src/lib/__tests__/compose-send-target-wiring.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; + +// A group or shared identity sends from the group's account with that +// account's identity id (lib/compose-send-account). Both places below used +// the login's own identity id, which names nothing in the group account: +// "Send now" from the undo bar lost the message, and a quick reply was +// refused or went out through another identity. +const read = (rel: string) => fs.readFileSync(path.resolve(__dirname, rel), 'utf8'); + +describe('send target wiring', () => { + it("records the target's identity for the undo bar of a composer send", () => { + const source = read('../../screens/ComposeScreen.tsx'); + const held = source.slice(source.indexOf('recordHeldSend(result, holdForSeconds')); + expect(held).toMatch(/^recordHeldSend\(result, holdForSeconds, \{[\s\S]*?identityId: target\.identityId,/); + expect(held.slice(0, 300)).not.toContain('primaryIdentity.id'); + }); + + it('sends a quick reply through the target the composer would pick', () => { + const source = read('../../components/email/QuickReplyBox.tsx'); + expect(source).toMatch(/const target = await sendTargetFor\(identity, identity\.email, mailboxes\)/); + const call = source.slice(source.indexOf('await sendEmail(')); + expect(call).toMatch(/\},\s+target\.identityId,\s+target\.sentMailboxId,/); + expect(call).toMatch(/accountId: target\.accountId \}/); + expect(source).toMatch(/recordHeldSend\(result, holdFor, \{\s*identityId: target\.identityId,/); + expect(source).not.toMatch(/identity\.id,\s+sent\./); + }); +}); diff --git a/src/screens/ComposeScreen.tsx b/src/screens/ComposeScreen.tsx index ea5f054e..ab038b05 100644 --- a/src/screens/ComposeScreen.tsx +++ b/src/screens/ComposeScreen.tsx @@ -2407,7 +2407,9 @@ export default function ComposeScreen({ route, navigation }: Props) { } else if (result.scheduled) { // Undo-send window: the undo bar offers Undo / Send now. useSendUndoStore.getState().recordHeldSend(result, holdForSeconds, { - identityId: primaryIdentity.id, + // The identity the message went out through, as the submission + // account names it: "Send now" submits again with it. + identityId: target.identityId, accountId: submissionAccountId, login: ownerLoginRef.current, from: outgoing.from,