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.
This commit is contained in:
@@ -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<string, Mailbox> =>
|
||||||
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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<Id, Mailbox>;
|
||||||
|
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];
|
||||||
|
}
|
||||||
@@ -14,6 +14,7 @@ import { LabelPicker } from "./LabelPicker";
|
|||||||
import type { Id } from "@/jmap/types";
|
import type { Id } from "@/jmap/types";
|
||||||
import { confirmDialog } from "@/ui/dialog";
|
import { confirmDialog } from "@/ui/dialog";
|
||||||
import { toast } from "@/ui/toast";
|
import { toast } from "@/ui/toast";
|
||||||
|
import { isUnknownMailbox } from "@/lib/mailboxRoute";
|
||||||
import { scheduledMailboxIdFrom, useScheduled } from "@/store/scheduled";
|
import { scheduledMailboxIdFrom, useScheduled } from "@/store/scheduled";
|
||||||
|
|
||||||
export function MailView({ mailboxId, threadId, search }: { mailboxId?: string; threadId?: string; search?: boolean }) {
|
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 });
|
if (!search && !mailboxId && inboxId) navigate(`/mail/${inboxId}`, { replace: true });
|
||||||
}, [search, mailboxId, inboxId, navigate]);
|
}, [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
|
// Build & run the list query
|
||||||
const listQuery = useMemo<ListQuery | null>(() => {
|
const listQuery = useMemo<ListQuery | null>(() => {
|
||||||
if (search) {
|
if (search) {
|
||||||
|
|||||||
Reference in New Issue
Block a user