Merge pull request #93 from LINUXexpert-org/fix-tree-on-account-switch

Show the folder tree in a shared account

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"],
+    );
+  });
+});

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<FilesState>((set, get) => ({
   accountId: null,
   available: false,
@@ -90,7 +106,7 @@ export const useFiles = create<FilesState>((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 });
   },