mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
fix: send group mail through the group's identity on Send now and quick reply
This commit is contained in:
@@ -107,7 +107,7 @@ describe('rescheduleScheduledSend', () => {
|
|||||||
expect(result).toEqual({ emailSubmissionId: 'sub-2', sendAt: '2026-09-25T08:00:00Z' });
|
expect(result).toEqual({ emailSubmissionId: 'sub-2', sendAt: '2026-09-25T08:00:00Z' });
|
||||||
const [lookup] = mockRequest.mock.calls[0][0];
|
const [lookup] = mockRequest.mock.calls[0][0];
|
||||||
expect(lookup).toEqual([
|
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({
|
expect(createdArgs().create.replacement.envelope).toEqual({
|
||||||
mailFrom: { email: 'me@example.com', parameters: { HOLDFOR: '7200' } },
|
mailFrom: { email: 'me@example.com', parameters: { HOLDFOR: '7200' } },
|
||||||
@@ -218,6 +218,37 @@ describe('rescheduleScheduledSend', () => {
|
|||||||
vi.mocked(Date.now).mockRestore();
|
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<Record<string, unknown>> }).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 () => {
|
it('moves the message back to Drafts when neither the replacement nor the original can be made', async () => {
|
||||||
vi.spyOn(console, 'warn').mockImplementation(() => undefined);
|
vi.spyOn(console, 'warn').mockImplementation(() => undefined);
|
||||||
mockRequest
|
mockRequest
|
||||||
|
|||||||
+8
-2
@@ -2230,7 +2230,7 @@ export async function rescheduleScheduledSend(
|
|||||||
const lookup = await jmapClient.request(
|
const lookup = await jmapClient.request(
|
||||||
[
|
[
|
||||||
['EmailSubmission/get', {
|
['EmailSubmission/get', {
|
||||||
accountId, ids: [scheduled.emailSubmissionId], properties: ['envelope', 'sendAt'],
|
accountId, ids: [scheduled.emailSubmissionId], properties: ['envelope', 'sendAt', 'identityId'],
|
||||||
}, '0'],
|
}, '0'],
|
||||||
['Email/get', { accountId, ids: [scheduled.emailId], properties: ['to', 'cc', 'bcc'] }, '1'],
|
['Email/get', { accountId, ids: [scheduled.emailId], properties: ['to', 'cc', 'bcc'] }, '1'],
|
||||||
['Mailbox/get', { accountId, ids: null, properties: ['id', 'role'] }, '2'],
|
['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<{
|
const original = (requireMethodResult(lookup, '0', 'EmailSubmission/get').list as Array<{
|
||||||
envelope?: { mailFrom?: { email?: string }; rcptTo?: Array<{ email?: string }> } | null;
|
envelope?: { mailFrom?: { email?: string }; rcptTo?: Array<{ email?: string }> } | null;
|
||||||
sendAt?: string;
|
sendAt?: string;
|
||||||
|
identityId?: string;
|
||||||
}> | undefined)?.[0];
|
}> | undefined)?.[0];
|
||||||
const envelope = original?.envelope;
|
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 held = requireMethodResult(lookup, '1', 'Email/get').list as Email[] | undefined;
|
||||||
const headerRecipients = [...(held?.[0]?.to ?? []), ...(held?.[0]?.cc ?? []), ...(held?.[0]?.bcc ?? [])];
|
const headerRecipients = [...(held?.[0]?.to ?? []), ...(held?.[0]?.cc ?? []), ...(held?.[0]?.bcc ?? [])];
|
||||||
const source = recipients ?? (envelope?.rcptTo?.length ? envelope.rcptTo : headerRecipients);
|
const source = recipients ?? (envelope?.rcptTo?.length ? envelope.rcptTo : headerRecipients);
|
||||||
@@ -2271,7 +2277,7 @@ export async function rescheduleScheduledSend(
|
|||||||
create: {
|
create: {
|
||||||
replacement: {
|
replacement: {
|
||||||
emailId: scheduled.emailId,
|
emailId: scheduled.emailId,
|
||||||
identityId: scheduled.identityId,
|
identityId,
|
||||||
envelope: {
|
envelope: {
|
||||||
mailFrom: seconds === undefined
|
mailFrom: seconds === undefined
|
||||||
? { email: mailFrom }
|
? { email: mailFrom }
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ import { resolveReplyIdentity } from '../../lib/reply-identity';
|
|||||||
import { signatureIdentityFor, signPlainTextReply } from '../../lib/signature-utils';
|
import { signatureIdentityFor, signPlainTextReply } from '../../lib/signature-utils';
|
||||||
import { pickEmailBody, plainTextBody } from '../../lib/email-body';
|
import { pickEmailBody, plainTextBody } from '../../lib/email-body';
|
||||||
import { htmlToPlainText } from '../../lib/compose-html';
|
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 { emailDisplayDate } from '../../lib/email-date';
|
||||||
import { buildQuoteHeader, quoteHeaderLabels } from '../../lib/quote-header';
|
import { buildQuoteHeader, quoteHeaderLabels } from '../../lib/quote-header';
|
||||||
|
|
||||||
@@ -66,16 +66,19 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF
|
|||||||
const from = email.from?.[0];
|
const from = email.from?.[0];
|
||||||
if (!from?.email || email.keywords?.$draft) return null;
|
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 send = async () => {
|
||||||
const body = text.trim();
|
const body = text.trim();
|
||||||
if (!body || sending) return;
|
if (!body || sending) return;
|
||||||
if (loginMovedFrom(ownerLoginRef.current)) {
|
if (refuseAfterSwitch()) return;
|
||||||
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;
|
|
||||||
}
|
|
||||||
const ownEmails = identities.map((i) => i.email).filter(Boolean);
|
const ownEmails = identities.map((i) => i.email).filter(Boolean);
|
||||||
// The own identity the message was delivered to (or, for our own message,
|
// 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
|
// 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'));
|
Alert.alert(t('common.error', 'Error'), t('email_viewer.unsubscribe_banner.no_identity', 'No sending identity available'));
|
||||||
return;
|
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);
|
setSending(true);
|
||||||
try {
|
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(
|
const recipients = buildReplyRecipients(
|
||||||
{ from: email.from, replyToAddresses: email.replyTo, to: email.to, cc: email.cc },
|
{ from: email.from, replyToAddresses: email.replyTo, to: email.to, cc: email.cc },
|
||||||
'reply',
|
'reply',
|
||||||
@@ -116,10 +123,10 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF
|
|||||||
labels: quoteHeaderLabels(t),
|
labels: quoteHeaderLabels(t),
|
||||||
});
|
});
|
||||||
const threading = computeReplyThreadingHeaders(email);
|
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
|
// Where the reply goes, read before it goes: Undo must reach that login
|
||||||
// and account whatever the app switches to meanwhile.
|
// 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(
|
const result = await sendEmail(
|
||||||
{
|
{
|
||||||
from: [{ name: identity.name, email: identity.email }],
|
from: [{ name: identity.name, email: identity.email }],
|
||||||
@@ -135,15 +142,15 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF
|
|||||||
inReplyTo: threading?.inReplyTo,
|
inReplyTo: threading?.inReplyTo,
|
||||||
references: threading?.references,
|
references: threading?.references,
|
||||||
},
|
},
|
||||||
identity.id,
|
target.identityId,
|
||||||
sent.originalId ?? sent.id,
|
target.sentMailboxId,
|
||||||
holdFor,
|
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.
|
// 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.
|
// Recorded before the flag below so its round trip doesn't eat the window.
|
||||||
useSendUndoStore.getState().recordHeldSend(result, holdFor, {
|
useSendUndoStore.getState().recordHeldSend(result, holdFor, {
|
||||||
identityId: identity.id,
|
identityId: target.identityId,
|
||||||
...sentThrough,
|
...sentThrough,
|
||||||
from: [{ name: identity.name, email: identity.email }],
|
from: [{ name: identity.name, email: identity.email }],
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -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\./);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -2407,7 +2407,9 @@ export default function ComposeScreen({ route, navigation }: Props) {
|
|||||||
} else if (result.scheduled) {
|
} else if (result.scheduled) {
|
||||||
// Undo-send window: the undo bar offers Undo / Send now.
|
// Undo-send window: the undo bar offers Undo / Send now.
|
||||||
useSendUndoStore.getState().recordHeldSend(result, holdForSeconds, {
|
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,
|
accountId: submissionAccountId,
|
||||||
login: ownerLoginRef.current,
|
login: ownerLoginRef.current,
|
||||||
from: outgoing.from,
|
from: outgoing.from,
|
||||||
|
|||||||
Reference in New Issue
Block a user