jcoffey-dev
2360e40733
Switching to an account somebody had shared showed an empty folder tree. Their files listed perfectly well; the sidebar beside them was blank, with nothing to say why. Switching accounts cleared `nodes` and `children` and stopped there. So `treeLoaded` stayed true from the account before -- the sidebar only asks for folders when it is false, and it never asked again -- while `dirIds` still named the previous account's folders, which no longer resolved against the cleared `nodes`. An empty tree either way, and no error, because nothing had failed. The fields that belong to one account are now named in one place, `emptyForAccount`, and the test asserts the whole set rather than the ones that come to mind. The bug was not bad logic, it was a field nobody remembered when two more were added a commit earlier, and asserting the set is the only guard that survives the next two. Found by the person it was built for, on a real share between two accounts, which is where it was always going to show up: the tree is built from a query that had already run for their own account, so it only breaks on the switch.
55 lines
2.1 KiB
TypeScript
55 lines
2.1 KiB
TypeScript
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"],
|
|
);
|
|
});
|
|
});
|