From 5a7cc5cc5a8c47f54f92b02693c9f957f932918f Mon Sep 17 00:00:00 2001 From: John Coffey Date: Thu, 27 Aug 2026 15:28:26 -0700 Subject: [PATCH] Say a missing folder is missing, not empty A folder id the account does not have rendered the ordinary empty state -- "Nothing here. This folder is empty." That is a claim about a folder that is not there, so a stale link read as a folder that had emptied itself rather than one that was gone (#111). It now goes to the inbox and says why. Inbox is the kinder landing than a dead end for a bookmark that has outlived its folder, but swapping one folder for another without a word would be its own small lie, so it does not do that either. The condition worth writing a test around is not the unknown id, it is the one guarding it. The folder list arrives after the first paint, so for a moment *every* id is unknown, the right one included. Without that gate this redirects on every cold load, from the folder the reader actually asked for, and looks exactly like a flaky link -- a worse bug than the one being fixed and a harder one to see. `isUnknownMailbox` is a small pure function so that case can be pinned down rather than reasoned about. Only ever reachable from outside the app, which is why it went unnoticed: the sidebar links to ids that exist. A bookmark to a deleted folder, or a folder link passed between accounts, is where it bites. Verified against the mock: an unknown id lands on the inbox with the message and a full list rather than an empty one, and a cold load straight into a real folder stays in that folder with nothing said. Closes #111. --- web/src/lib/__tests__/mailboxRoute.test.ts | 46 ++++++++++++++++++++++ web/src/lib/mailboxRoute.ts | 27 +++++++++++++ web/src/views/mail/MailView.tsx | 21 ++++++++++ 3 files changed, 94 insertions(+) create mode 100644 web/src/lib/__tests__/mailboxRoute.test.ts create mode 100644 web/src/lib/mailboxRoute.ts diff --git a/web/src/lib/__tests__/mailboxRoute.test.ts b/web/src/lib/__tests__/mailboxRoute.test.ts new file mode 100644 index 0000000..a7357e3 --- /dev/null +++ b/web/src/lib/__tests__/mailboxRoute.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; +import { isUnknownMailbox } from "@/lib/mailboxRoute"; +import type { Mailbox } from "@/jmap/types"; + +/** + * Issue #111: a folder id the account does not have rendered the ordinary + * empty state — "Nothing here. This folder is empty" — which is a claim about + * a folder that is not there. A stale link read as a folder that had emptied + * itself rather than one that was gone. + * + * The interesting case is not the unknown id. It is `loaded`: the folder list + * arrives after the first paint, so for a moment *every* id is unknown, + * including the right one. A version without that gate sends the reader to + * their inbox from the folder they asked for, on every cold load, and looks + * exactly like a flaky link. + */ + +const boxes = (...ids: string[]): Record => + Object.fromEntries(ids.map((id) => [id, { id, name: id } as Mailbox])); + +describe("spotting a folder the account does not have", () => { + it("is unknown when the list is loaded and does not contain it", () => { + expect(isUnknownMailbox({ mailboxId: "ghost", mailboxes: boxes("a", "b"), loaded: true })).toBe(true); + }); + + it("is not unknown when the list contains it", () => { + expect(isUnknownMailbox({ mailboxId: "a", mailboxes: boxes("a", "b"), loaded: true })).toBe(false); + }); +}); + +describe("what it refuses to call unknown", () => { + it("says nothing before the folder list has arrived", () => { + // The whole point. Every id is unknown at this moment, the real one too. + expect(isUnknownMailbox({ mailboxId: "a", mailboxes: {}, loaded: false })).toBe(false); + expect(isUnknownMailbox({ mailboxId: "ghost", mailboxes: {}, loaded: false })).toBe(false); + }); + + it("says nothing when there is no folder in the address", () => { + // /mail has its own redirect to the inbox; this must not race it. + expect(isUnknownMailbox({ mailboxId: undefined, mailboxes: boxes("a"), loaded: true })).toBe(false); + }); + + it("says nothing on a search, which has no folder to be wrong about", () => { + expect(isUnknownMailbox({ mailboxId: "ghost", mailboxes: boxes("a"), loaded: true, search: true })).toBe(false); + }); +}); diff --git a/web/src/lib/mailboxRoute.ts b/web/src/lib/mailboxRoute.ts new file mode 100644 index 0000000..f7f8e11 --- /dev/null +++ b/web/src/lib/mailboxRoute.ts @@ -0,0 +1,27 @@ +import type { Id, Mailbox } from "@/jmap/types"; + +/** + * Whether the folder in the address is one this account does not have. + * + * Rendering it as an empty folder was the bug (#111): "Nothing here. This + * folder is empty" is a claim about a folder that is not there, so a stale link + * read as a folder that had emptied itself rather than one that was gone. + * + * The condition that matters is `loaded`. The folder list arrives after the + * first paint, so for a moment every id is unknown -- including the right one. + * Without that gate this answers true on every cold load and sends the reader + * to their inbox from the folder they asked for, which is a worse bug than the + * one it fixes and would look exactly like a flaky link. + */ +export function isUnknownMailbox(args: { + mailboxId: Id | undefined; + mailboxes: Record; + loaded: boolean; + search?: boolean; +}): boolean { + const { mailboxId, mailboxes, loaded, search } = args; + if (search) return false; + if (!mailboxId) return false; + if (!loaded) return false; + return !mailboxes[mailboxId]; +} diff --git a/web/src/views/mail/MailView.tsx b/web/src/views/mail/MailView.tsx index 581513e..62a43de 100644 --- a/web/src/views/mail/MailView.tsx +++ b/web/src/views/mail/MailView.tsx @@ -14,6 +14,7 @@ import { LabelPicker } from "./LabelPicker"; import type { Id } from "@/jmap/types"; import { confirmDialog } from "@/ui/dialog"; import { toast } from "@/ui/toast"; +import { isUnknownMailbox } from "@/lib/mailboxRoute"; import { scheduledMailboxIdFrom, useScheduled } from "@/store/scheduled"; export function MailView({ mailboxId, threadId, search }: { mailboxId?: string; threadId?: string; search?: boolean }) { @@ -39,6 +40,26 @@ export function MailView({ mailboxId, threadId, search }: { mailboxId?: string; if (!search && !mailboxId && inboxId) navigate(`/mail/${inboxId}`, { replace: true }); }, [search, mailboxId, inboxId, navigate]); + /* + * A folder id this account does not have. + * + * It used to render the ordinary empty state -- "Nothing here. This folder is + * empty." -- which is a claim about a folder that is not there, so a stale + * link read as a folder that had emptied itself rather than one that was + * gone (#111). Only reachable from outside the app: the sidebar links to ids + * that exist. + * + * Inbox is the kinder landing than a dead end, but silently swapping one + * folder for another would be its own small lie, so it says what happened. + * `mailboxesLoaded` gates it: without that, every cold load redirects in the + * moment before the folder list arrives. + */ + useEffect(() => { + if (!isUnknownMailbox({ mailboxId, mailboxes, loaded: mailboxesLoaded, search }) || !inboxId) return; + toast.show("That folder no longer exists. Showing your inbox instead."); + navigate(`/mail/${inboxId}`, { replace: true }); + }, [search, mailboxId, mailboxesLoaded, mailboxes, inboxId, navigate]); + // Build & run the list query const listQuery = useMemo(() => { if (search) {