diff --git a/web/src/store/__tests__/files-account-switch.test.ts b/web/src/store/__tests__/files-account-switch.test.ts new file mode 100644 index 0000000..0fdca95 --- /dev/null +++ b/web/src/store/__tests__/files-account-switch.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { emptyForAccount } from "../files"; + +/** + * Switching to an account somebody shared with you showed an empty folder tree. + * + * The switch cleared `nodes` and `children` and stopped there, so `treeLoaded` + * stayed true from the previous account — the sidebar never asked the new one + * for its folders — while `dirIds` still named the old account's folders, which + * no longer resolved against the cleared `nodes`. The result was a tree with + * nothing in it and no error to explain it, in the one place a tree matters + * most: someone else's files, where you have no idea what the shape should be. + * + * The test that matters is the last one. The bug was not bad logic, it was a + * field nobody remembered, and the only durable guard is asserting the whole + * set rather than the fields we happen to think of today. + */ + +describe("what a switch to another account keeps", () => { + it("keeps nothing but the new account's own id", () => { + expect(emptyForAccount("b")).toEqual({ + accountId: "b", + nodes: {}, + children: {}, + dirIds: [], + treeLoaded: false, + draggingId: null, + error: null, + }); + }); + + it("asks the new account for its tree", () => { + // The sidebar loads when `treeLoaded` is false. True here means an empty + // tree for as long as the account stays selected. + expect(emptyForAccount("b").treeLoaded).toBe(false); + }); + + it("carries no folder ids over from the account before it", () => { + expect(emptyForAccount("b").dirIds).toEqual([]); + }); + + it("drops a drag that was in flight", () => { + // Its id belongs to the other account and would name a different node here. + expect(emptyForAccount("b").draggingId).toBeNull(); + }); + + it("names every piece of per-account state", () => { + // Add a per-account field to the store and forget it here, and this fails + // rather than the field quietly following someone into another account. + expect(Object.keys(emptyForAccount(null)).sort()).toEqual( + ["accountId", "children", "dirIds", "draggingId", "error", "nodes", "treeLoaded"], + ); + }); +}); diff --git a/web/src/store/files.ts b/web/src/store/files.ts index 37b2b2c..5c262cd 100644 --- a/web/src/store/files.ts +++ b/web/src/store/files.ts @@ -75,6 +75,22 @@ export function withoutAppFolder(nodes: FileNode[]): FileNode[] { return nodes.filter((n) => !hidden.has(n.id)); } +/** + * The state that belongs to one account, emptied when the selection moves. + * + * Every field here describes somebody's files, so none of it survives a switch + * to somebody else's. `treeLoaded` is the one that bites: leave it true and the + * sidebar never asks the new account for its folders, while `dirIds` still + * names the old account's, which no longer resolve -- so the tree is simply + * empty, with nothing to say why. That shipped, and is what this exists to stop + * happening again: the test asserts the whole set, so a field added to the + * store and forgotten here fails rather than quietly persisting across + * accounts. + */ +export function emptyForAccount(accountId: Id | null) { + return { accountId, nodes: {}, children: {}, dirIds: [], treeLoaded: false, draggingId: null, error: null }; +} + export const useFiles = create((set, get) => ({ accountId: null, available: false, @@ -90,7 +106,7 @@ export const useFiles = create((set, get) => ({ async init() { const accountId = useSession.getState().accountFor(CAP.filenode); const available = Boolean(accountId && client.hasCapability(CAP.filenode)); - if (accountId !== get().accountId) set({ accountId, nodes: {}, children: {} }); + if (accountId !== get().accountId) set(emptyForAccount(accountId)); set({ available }); },