Merge pull request #276 from Coffey-Labs/reply-to-my-own-message
Ask the folder, not just the identity list, whether a message was mine
This commit is contained in:
Coffey Labs
@@ -0,0 +1,165 @@
|
||||
import { beforeEach, afterEach, describe, expect, it } from "vitest";
|
||||
import { useCompose } from "@/store/compose";
|
||||
import { useMail } from "@/store/mail";
|
||||
import type { Email, Identity } from "@/jmap/types";
|
||||
|
||||
/*
|
||||
* Who a reply is addressed to.
|
||||
*
|
||||
* The hard half is replying to something *I* sent, which is what following up
|
||||
* on your own last message is. The conversation is with the people I wrote to;
|
||||
* addressing the reply to myself, or to my own Reply-To, sends it nowhere
|
||||
* useful -- and on a Reply all it quietly demotes everyone I was talking to
|
||||
* into Cc.
|
||||
*
|
||||
* There was a guard for this and it was sound. What it rested on was not: it
|
||||
* asked whether an address was in the identity list, which is empty before
|
||||
* identities load, misses an alias or a shared mailbox the server does not list
|
||||
* as an identity, and compared strings where the rest of the codebase uses
|
||||
* `sameAddress`. Each miss was silent. So the folder is asked first -- a
|
||||
* message in Sent is mine whatever address it went out as -- and the identity
|
||||
* list is the second opinion rather than the only one (#275).
|
||||
*/
|
||||
|
||||
const body = {
|
||||
messageId: ["<[email protected]>"], subject: "Numbers", references: [], inReplyTo: [],
|
||||
keywords: {}, htmlBody: [{ partId: "1", type: "text/html" }], textBody: [{ partId: "1", type: "text/html" }],
|
||||
bodyValues: { "1": { value: "<p>hi</p>", isEncodingProblem: false, isTruncated: false } },
|
||||
attachments: [], receivedAt: "2026-09-04T10:00:00Z", mailboxIds: {},
|
||||
};
|
||||
|
||||
const ME = { name: "John", email: "[email protected]" };
|
||||
const ANN = { name: "Ann", email: "[email protected]" };
|
||||
const BOB = { name: "Bob", email: "[email protected]" };
|
||||
|
||||
/** A message I sent: me in From, Ann in To, Bob in Cc. */
|
||||
const MINE = { ...body, id: "m1", from: [ME], to: [ANN], cc: [BOB] } as unknown as Email;
|
||||
/** The same conversation, but Ann's message to me. */
|
||||
const HERS = { ...body, id: "m2", from: [ANN], to: [ME], cc: [BOB] } as unknown as Email;
|
||||
|
||||
const IDENTITIES = [{ id: "i1", name: "John", email: "[email protected]", replyTo: null }] as unknown as Identity[];
|
||||
|
||||
/** In Sent, which is the signal that survives an unlisted alias. */
|
||||
const inSent = (e: Email) => ({ ...e, mailboxIds: { sent1: true } }) as Email;
|
||||
|
||||
function draftFor(email: Email, mode: "reply" | "replyAll" | "forward", opts: { identities?: Identity[] } = {}) {
|
||||
const identities = opts.identities ?? IDENTITIES;
|
||||
useMail.setState({
|
||||
accountId: "a1",
|
||||
identities: identities as never,
|
||||
getEmails: (async () => [email]) as never,
|
||||
defaultIdentity: (() => identities[0]) as never,
|
||||
loadIdentities: (async () => identities) as never,
|
||||
roleId: ((role: string) => (role === "sent" ? "sent1" : null)) as never,
|
||||
});
|
||||
return useCompose.getState().reply(email, mode).then((key) => useCompose.getState().drafts.find((d) => d.key === key)!);
|
||||
}
|
||||
|
||||
const addrs = (list: { email: string }[]) => list.map((a) => a.email);
|
||||
|
||||
beforeEach(() => useCompose.setState({ drafts: [], activeKey: null }));
|
||||
afterEach(() => useCompose.setState({ drafts: [], activeKey: null }));
|
||||
|
||||
describe("replying to a message somebody sent me", () => {
|
||||
it("replies to the sender", async () => {
|
||||
const d = await draftFor(HERS, "reply");
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
expect(d.cc).toEqual([]);
|
||||
});
|
||||
|
||||
it("reply all keeps the others and leaves me off", async () => {
|
||||
const d = await draftFor(HERS, "replyAll");
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
expect(addrs(d.cc)).toEqual([BOB.email]);
|
||||
});
|
||||
|
||||
it("honours the sender's Reply-To, which is what it is for", async () => {
|
||||
const d = await draftFor({ ...HERS, replyTo: [{ name: null, email: "[email protected]" }] } as Email, "reply");
|
||||
expect(addrs(d.to)).toEqual(["[email protected]"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("replying to a message I sent", () => {
|
||||
it("writes to the people I wrote to, not to me", async () => {
|
||||
const d = await draftFor(MINE, "reply");
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
expect(d.cc).toEqual([]);
|
||||
});
|
||||
|
||||
it("reply all keeps my Cc as Cc, rather than promoting me into To", async () => {
|
||||
const d = await draftFor(MINE, "replyAll");
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
expect(addrs(d.cc)).toEqual([BOB.email]);
|
||||
});
|
||||
|
||||
it("does not follow my own Reply-To back to my own desk", async () => {
|
||||
// The address replies to *me* belong at. My reply is not one of them.
|
||||
const d = await draftFor({ ...MINE, replyTo: [{ name: null, email: "[email protected]" }] } as Email, "replyAll");
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
expect(addrs(d.cc)).toEqual([BOB.email]);
|
||||
});
|
||||
|
||||
it("leaves me out even when I was a recipient of my own message", async () => {
|
||||
const d = await draftFor({ ...MINE, to: [ME, ANN] } as Email, "replyAll");
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
expect(addrs(d.cc)).toEqual([BOB.email]);
|
||||
});
|
||||
|
||||
it("recognises my address however the identity stored it", async () => {
|
||||
// A hand-typed identity address can carry whitespace, and comparing
|
||||
// strings rather than addresses made that enough to break the reply.
|
||||
const padded = [{ id: "i1", name: "John", email: " [email protected] " }] as unknown as Identity[];
|
||||
const d = await draftFor(MINE, "replyAll", { identities: padded });
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("when the identity list cannot answer", () => {
|
||||
it("takes a message in Sent as mine, whatever address it went out as", async () => {
|
||||
// An alias or a shared mailbox the server does not list as an identity.
|
||||
const alias = inSent({ ...MINE, from: [{ name: "Sales", email: "[email protected]" }] } as Email);
|
||||
const d = await draftFor(alias, "replyAll");
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
expect(addrs(d.cc)).toEqual([BOB.email]);
|
||||
});
|
||||
|
||||
it("takes a message in Sent as mine before the identities have loaded", async () => {
|
||||
const d = await draftFor(inSent(MINE), "replyAll", { identities: [] });
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
});
|
||||
|
||||
it("still replies to the sender of a message that is not mine and not in Sent", async () => {
|
||||
// The folder signal must not swallow the ordinary case when it is absent.
|
||||
const d = await draftFor(HERS, "replyAll", { identities: [] });
|
||||
expect(addrs(d.to)).toEqual([ANN.email]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("a message of mine with nobody obvious to reply to", () => {
|
||||
it("uses the Cc when I addressed it to nobody else", async () => {
|
||||
const d = await draftFor({ ...MINE, to: [ME] } as Email, "replyAll");
|
||||
expect(addrs(d.to)).toEqual([BOB.email]);
|
||||
expect(d.cc).toEqual([]);
|
||||
});
|
||||
|
||||
it("uses the Cc on a plain reply too, rather than leaving To empty", async () => {
|
||||
const d = await draftFor({ ...MINE, to: [] } as Email, "reply");
|
||||
expect(addrs(d.to)).toEqual([BOB.email]);
|
||||
});
|
||||
|
||||
it("falls back to my own address rather than a draft addressed to nobody", async () => {
|
||||
// A note I sent only to myself. Replying to it is odd, and an empty To is
|
||||
// worse than the only address there was.
|
||||
const d = await draftFor({ ...MINE, to: [ME], cc: [] } as Email, "reply");
|
||||
expect(addrs(d.to)).toEqual([ME.email]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("forwarding", () => {
|
||||
it("addresses nobody, whoever sent the message", async () => {
|
||||
for (const m of [MINE, HERS]) {
|
||||
const d = await draftFor(m, "forward");
|
||||
expect([d.to, d.cc]).toEqual([[], []]);
|
||||
}
|
||||
});
|
||||
});
|
||||
+37
-13
@@ -299,26 +299,50 @@ export const useCompose = create<ComposeState>((set, get) => ({
|
||||
const full = (await mail.getEmails([email.id], true))[0] ?? email;
|
||||
const identities = mail.identities.length ? mail.identities : await mail.loadIdentities();
|
||||
const ident = defaultIdentity(identities, full);
|
||||
const ownEmails = identities.map((i) => i.email.toLowerCase());
|
||||
const isOwn = (a: EmailAddress) => ownEmails.includes(a.email.toLowerCase());
|
||||
const ownEmails = identities.map((i) => i.email);
|
||||
/* `sameAddress` rather than a lowercased `includes`, because an identity
|
||||
address can carry whitespace and a hand-typed one does. */
|
||||
const isOwn = (a: EmailAddress) => ownEmails.some((e) => sameAddress(e, a.email));
|
||||
const withoutOwn = (list: EmailAddress[]) => uniqueAddresses(list).filter((a) => !isOwn(a));
|
||||
const s = settings();
|
||||
|
||||
/*
|
||||
* Was this message mine?
|
||||
*
|
||||
* The folder answers it before the addresses do, and has to: the address
|
||||
* test fails in exactly the cases where the mistake is least visible. A
|
||||
* message sent from an alias or a shared mailbox that `Identity/get` does
|
||||
* not list is not recognisably mine, and neither is anything at all if the
|
||||
* identities have not loaded yet -- and the failure is silent, addressing
|
||||
* the reply back to me with everyone I actually wrote to moved to Cc.
|
||||
*
|
||||
* A message in Sent is mine whatever address it went out as.
|
||||
*/
|
||||
const sentId = mail.roleId("sent");
|
||||
const sentByMe = (Boolean(full.from?.length) && (full.from ?? []).every(isOwn))
|
||||
|| Boolean(sentId && full.mailboxIds?.[sentId]);
|
||||
|
||||
let to: EmailAddress[] = [];
|
||||
let cc: EmailAddress[] = [];
|
||||
if (mode === "reply" || mode === "replyAll") {
|
||||
const replyTo = full.replyTo?.length ? full.replyTo : (full.from ?? []);
|
||||
to = uniqueAddresses(replyTo);
|
||||
if (sentByMe && (full.to?.length || full.cc?.length)) {
|
||||
/*
|
||||
* Replying to something I sent continues the conversation with the
|
||||
* people I wrote to. Not with myself, and not with my own Reply-To
|
||||
* either -- that address is where replies *to me* belong, and following
|
||||
* it here would send my own reply to my own desk.
|
||||
*/
|
||||
to = withoutOwn(full.to ?? []);
|
||||
cc = mode === "replyAll" ? withoutOwn(full.cc ?? []) : [];
|
||||
// Addressed only to myself, or only in Cc: there is still somebody this
|
||||
// is a reply to, and an empty To is not it.
|
||||
if (!to.length) { to = cc.length ? cc : withoutOwn(full.cc ?? []); cc = []; }
|
||||
if (!to.length) to = uniqueAddresses([...(full.to ?? []), ...(full.cc ?? [])]);
|
||||
} else {
|
||||
to = uniqueAddresses(full.replyTo?.length ? full.replyTo : (full.from ?? []));
|
||||
if (mode === "replyAll") {
|
||||
const others = uniqueAddresses([...(full.to ?? []), ...(full.cc ?? [])]).filter((a) => !isOwn(a) && !to.some((t) => sameAddress(t.email, a.email)));
|
||||
cc = others;
|
||||
// If the message was sent by me, reply to original recipients instead.
|
||||
if (to.every(isOwn) && full.to?.length) {
|
||||
to = uniqueAddresses(full.to);
|
||||
cc = uniqueAddresses(full.cc ?? []).filter((a) => !isOwn(a));
|
||||
cc = uniqueAddresses([...(full.to ?? []), ...(full.cc ?? [])]).filter((a) => !isOwn(a) && !to.some((t) => sameAddress(t.email, a.email)));
|
||||
}
|
||||
} else if (to.every(isOwn) && full.to?.length) {
|
||||
to = uniqueAddresses(full.to.filter((a) => !isOwn(a)));
|
||||
if (!to.length) to = uniqueAddresses(full.to);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user