test: cover features since 053a9c0d + repair test tooling

Add ~330 tests across server (Jest), client (Vitest), editor-ext (Vitest) and packages/mcp (node:test) for the gitmost features added since 053a9c0d: AI chat, AI agent roles, public-share assistant, MCP per-user auth, HTML embed, page templates/embed, realtime tree, tree expand/collapse, and the AI-settings UI. Test-tooling fixes (prerequisite, were silently hiding coverage): - Repair 3 page-template specs broken by the 11-arg TransclusionService constructor; they never compiled, so template access-control / content -leak / unsync-strip coverage was fictitious. - Build @docmost/editor-ext before server tests via a `pretest` hook; the stale dist omitted the new HtmlEmbed/PageEmbed exports (TS2305). - Let jest resolve the .tsx email templates: add `tsx` to moduleFileExtensions and widen the ts-jest transform to (t|j)sx?. Behaviour-preserving "extract pure core" refactors that the tests drive: - server: resolveShareAssistantRequest + uiMessageTextLength (public-share controller), decideBasicGate + mapAuthResultToResponse (mcp), buildErrorAssistantRecord (ai-chat), jsonbObject export (roles). - client: render-raw-html + shouldExecute/canEdit, decide-embed-state, page-embed picker utils, tree-socket reducers, open/close branch maps, isEndpointConfigured/resolveKeyField; buildTreeWithChildren now treats a permission-trimmed orphan as a root instead of crashing. Deferred (need a test DB or HTTP harness, documented in the specs): repo-level Postgres integration tests and the public-share XFF E2E. Pre-existing DI/lib0-ESM suite failures are untouched and out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-20 23:40:40 +03:00
parent 692c0abe13
commit 90d3fab483
56 changed files with 5668 additions and 447 deletions
@@ -0,0 +1,228 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { createRef } from "react";
+import { render, waitFor, cleanup } from "@testing-library/react";
+
+// --- Mocks for the heavy / networked module graph ---------------------------
+// SpaceTree pulls in query hooks, page services, i18n, notifications and two
+// child render components. The expandAll contract is exercised purely through
+// the imperative ref, so we mock everything that would otherwise need a real
+// server / router and stub the visual children to empty renders.
+
+const getSpaceTreeMock = vi.fn();
+const notificationsShowMock = vi.fn();
+
+vi.mock("@/features/page/services/page-service.ts", () => ({
+  getSpaceTree: (...args: unknown[]) => getSpaceTreeMock(...args),
+  getPageBreadcrumbs: vi.fn(),
+}));
+
+vi.mock("@/features/page/queries/page-query.ts", () => ({
+  // No root pages and no further pages — the data-load effect is inert so the
+  // test fully controls the tree through expandAll.
+  useGetRootSidebarPagesQuery: () => ({
+    data: undefined,
+    hasNextPage: false,
+    fetchNextPage: vi.fn(),
+    isFetching: false,
+  }),
+  usePageQuery: () => ({ data: undefined }),
+  fetchAllAncestorChildren: vi.fn(),
+}));
+
+vi.mock("@/features/page/tree/hooks/use-tree-mutation.ts", () => ({
+  useTreeMutation: () => ({ handleMove: vi.fn() }),
+}));
+
+vi.mock("@mantine/notifications", () => ({
+  notifications: { show: (...args: unknown[]) => notificationsShowMock(...args) },
+}));
+
+vi.mock("react-i18next", () => ({
+  useTranslation: () => ({ t: (key: string) => key }),
+}));
+
+vi.mock("react-router-dom", () => ({
+  useParams: () => ({ pageSlug: undefined }),
+}));
+
+vi.mock("@/lib", () => ({
+  extractPageSlugId: () => undefined,
+}));
+
+vi.mock("@/lib/config.ts", () => ({
+  isCompactPageTreeEnabled: () => false,
+}));
+
+// Stub the visual children so we don't drag in the full DnD / Mantine stack.
+vi.mock("./doc-tree", () => ({
+  DocTree: () => null,
+  ROW_HEIGHT_COMPACT: 28,
+  ROW_HEIGHT_STANDARD: 32,
+}));
+vi.mock("./space-tree-row", () => ({
+  SpaceTreeRow: () => null,
+}));
+
+vi.mock("@mantine/core", () => ({
+  Text: ({ children }: { children?: unknown }) => children ?? null,
+}));
+
+// The real openTreeNodesAtom is localStorage-backed (atomWithStorage +
+// getOnInit), which crashes under jsdom's localStorage shim here. Swap in a
+// plain in-memory atom with the same read value (OpenMap) and the same setter
+// shape (value OR functional updater) so the component's open-state logic runs
+// unchanged while staying inside the test store.
+vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => {
+  const { atom } = await import("jotai");
+  type OpenMap = Record<string, boolean>;
+  const base = atom<OpenMap>({});
+  const openTreeNodesAtom = atom(
+    (get) => get(base),
+    (get, set, update: OpenMap | ((prev: OpenMap) => OpenMap)) => {
+      const next =
+        typeof update === "function"
+          ? (update as (prev: OpenMap) => OpenMap)(get(base))
+          : update;
+      set(base, next);
+    },
+  );
+  return { openTreeNodesAtom };
+});
+
+import SpaceTree, { SpaceTreeApi } from "./space-tree";
+import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
+import { openTreeNodesAtom } from "@/features/page/tree/atoms/open-tree-nodes-atom.ts";
+import { createStore, Provider } from "jotai";
+import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
+
+// A flat space-tree response (parentPageId pointers) that buildTree +
+// buildTreeWithChildren nest into a multi-level tree. Depth > 1 lets us assert
+// expandAll never fans out into per-branch fetches (no N+1).
+function spaceTreeItems(): SpaceTreeNode[] {
+  const n = (
+    id: string,
+    parentPageId: string | null,
+    position: string,
+  ): SpaceTreeNode => ({
+    id,
+    slugId: `slug-${id}`,
+    name: id,
+    icon: undefined,
+    position,
+    spaceId: "space-1",
+    parentPageId: parentPageId as unknown as string,
+    hasChildren: false,
+    children: [],
+  });
+  return [
+    n("root", null, "a0"),
+    n("branch", "root", "a1"),
+    n("leaf", "branch", "a1"),
+  ];
+}
+
+function renderTree(store: ReturnType<typeof createStore>) {
+  const ref = createRef<SpaceTreeApi>();
+  render(
+    <Provider store={store}>
+      <SpaceTree ref={ref} spaceId="space-1" readOnly={false} />
+    </Provider>,
+  );
+  return ref;
+}
+
+beforeEach(() => {
+  getSpaceTreeMock.mockReset();
+  notificationsShowMock.mockReset();
+  // jsdom's localStorage shim here lacks `clear`; guard it. Each test uses a
+  // fresh jotai store anyway, so cross-test open-state never leaks.
+  try {
+    localStorage.clear?.();
+  } catch {
+    /* ignore — fresh store per test isolates state */
+  }
+});
+
+afterEach(() => {
+  cleanup();
+});
+
+describe("SpaceTree.expandAll (integration via ref)", () => {
+  it("makes exactly ONE getSpaceTree call regardless of depth (no N+1)", async () => {
+    getSpaceTreeMock.mockResolvedValue(spaceTreeItems());
+    const store = createStore();
+    const ref = renderTree(store);
+
+    await ref.current!.expandAll();
+
+    expect(getSpaceTreeMock).toHaveBeenCalledTimes(1);
+    expect(getSpaceTreeMock).toHaveBeenCalledWith({ spaceId: "space-1" });
+
+    // Every branch node (root, branch) is opened; the leaf needs no entry.
+    const openMap = store.get(openTreeNodesAtom);
+    expect(openMap["root"]).toBe(true);
+    expect(openMap["branch"]).toBe(true);
+    expect(openMap["leaf"]).toBeUndefined();
+
+    // The full tree replaced the current-space nodes.
+    const data = store.get(treeDataAtom);
+    expect(data.map((d) => d.id)).toEqual(["root"]);
+  });
+
+  it("shows a notification and still resets isExpanding when getSpaceTree rejects", async () => {
+    getSpaceTreeMock.mockRejectedValue(new Error("boom"));
+    const store = createStore();
+    const ref = renderTree(store);
+
+    await ref.current!.expandAll();
+
+    expect(notificationsShowMock).toHaveBeenCalledTimes(1);
+    expect(notificationsShowMock).toHaveBeenCalledWith(
+      expect.objectContaining({ color: "red" }),
+    );
+
+    // isExpanding must be reset in the finally block even on failure.
+    await waitFor(() => {
+      expect(ref.current!.isExpanding).toBe(false);
+    });
+  });
+
+  it("aborts the merge when the space switches mid-flight", async () => {
+    // getSpaceTree resolves only after we flip the tree to a different space,
+    // simulating the user navigating away while the request is in flight.
+    let resolveTree: (v: SpaceTreeNode[]) => void = () => {};
+    getSpaceTreeMock.mockImplementation(
+      () =>
+        new Promise<SpaceTreeNode[]>((resolve) => {
+          resolveTree = resolve;
+        }),
+    );
+
+    const store = createStore();
+    const ref = createRef<SpaceTreeApi>();
+    const { rerender } = render(
+      <Provider store={store}>
+        <SpaceTree ref={ref} spaceId="space-1" readOnly={false} />
+      </Provider>,
+    );
+
+    const promise = ref.current!.expandAll();
+
+    // Switch the space mid-flight: spaceIdRef.current becomes "space-2".
+    rerender(
+      <Provider store={store}>
+        <SpaceTree ref={ref} spaceId="space-2" readOnly={false} />
+      </Provider>,
+    );
+
+    // Now resolve the in-flight request for the OLD space.
+    resolveTree(spaceTreeItems());
+    await promise;
+
+    // The merge must have been aborted: no tree data written, no branches opened.
+    expect(store.get(treeDataAtom)).toEqual([]);
+    const openMap = store.get(openTreeNodesAtom);
+    expect(openMap["root"]).toBeUndefined();
+    expect(openMap["branch"]).toBeUndefined();
+  });
+});
@@ -27,6 +27,8 @@ import {
  mergeRootTrees,
  collectAllIds,
  collectBranchIds,
+  openBranches,
+  closeIds,
 } from "@/features/page/tree/utils/utils.ts";
 import { SpaceTreeNode } from "@/features/page/tree/types.ts";
 import { treeModel } from "@/features/page/tree/model/tree-model";
@@ -236,11 +238,7 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
      // Open every branch node (node with children) of the current space only.
      const branchIds = collectBranchIds(fullTree);

-      setOpenTreeNodes((prev) => {
-        const next = { ...prev };
-        for (const id of branchIds) next[id] = true;
-        return next;
-      });
+      setOpenTreeNodes((prev) => openBranches(prev, branchIds));
    } catch (err: any) {
      // Never swallow: log full error + surface the real reason.
      console.error("[tree] expandAll failed", err);
@@ -261,11 +259,7 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
    // other spaces' expanded state is left intact.
    const ids = collectAllIds(filteredData);

-    setOpenTreeNodes((prev) => {
-      const next = { ...prev };
-      for (const id of ids) next[id] = false;
-      return next;
-    });
+    setOpenTreeNodes((prev) => closeIds(prev, ids));
  }, [filteredData, setOpenTreeNodes]);

  useImperativeHandle(
@@ -196,6 +196,17 @@ describe('treeModel.insertByPosition', () => {
    const t = treeModel.insertByPosition(roots, null, node);
    expect(t.map((n) => n.id)).toEqual(['a', 'b', 'c', 'x']);
  });
+
+  it('tie-break: a node whose position EQUALS a sibling lands deterministically (strict >)', () => {
+    // The insertion index is the first sibling whose position sorts STRICTLY
+    // after the new node's. An equal sibling is not strictly after, so it is
+    // skipped — the new node lands immediately AFTER every equal-position
+    // sibling and before the first strictly-greater one. This is deterministic:
+    // a tie always resolves the same way on every client.
+    const node: P = { id: 'x', name: 'X', position: 'a2' }; // equals b's position
+    const t = treeModel.insertByPosition(roots, null, node);
+    expect(t.map((n) => n.id)).toEqual(['a', 'b', 'x', 'c']);
+  });
 });

 // addTreeNode idempotency: the receiver early-returns when the node id already
@@ -692,4 +703,45 @@ describe('treeModel.move', () => {
    });
    expect(out.tree).toBe(fixture);
  });
+
+  it('cross-parent move does NOT apply the same-parent adjust (no off-by-one)', () => {
+    // Source `x3` sits at index 2 in parent `x`; target `y1` sits at index 0 in
+    // parent `y`. sourceInfo.index (2) > info.index (0) AND the parents differ,
+    // so the `sameParent && source.index < info.index` adjust must be 0 — the
+    // node must land at index 0 in `y`, not at index -1 (which would silently
+    // drop it at a wrong slot / off-by-one).
+    const crossFixture: N[] = [
+      {
+        id: 'x',
+        name: 'X',
+        children: [
+          { id: 'x1', name: 'X1' },
+          { id: 'x2', name: 'X2' },
+          { id: 'x3', name: 'X3' },
+        ],
+      },
+      {
+        id: 'y',
+        name: 'Y',
+        children: [
+          { id: 'y1', name: 'Y1' },
+          { id: 'y2', name: 'Y2' },
+        ],
+      },
+    ];
+    const { tree: t, result } = treeModel.move(crossFixture, 'x3', {
+      kind: 'reorder-before',
+      targetId: 'y1',
+    });
+    expect(result).toEqual({ parentId: 'y', index: 0 });
+    expect(treeModel.find(t, 'y')?.children?.map((n) => n.id)).toEqual([
+      'x3',
+      'y1',
+      'y2',
+    ]);
+    expect(treeModel.find(t, 'x')?.children?.map((n) => n.id)).toEqual([
+      'x1',
+      'x2',
+    ]);
+  });
 });
@@ -1,6 +1,14 @@
 import { describe, it, expect } from "vitest";
-import { buildTree } from "./utils";
+import {
+  buildTree,
+  buildTreeWithChildren,
+  collectAllIds,
+  collectBranchIds,
+  openBranches,
+  closeIds,
+} from "./utils";
 import type { IPage } from "@/features/page/types/page.types.ts";
+import type { SpaceTreeNode } from "@/features/page/tree/types.ts";

 function page(id: string, position: string): IPage {
  return {
@@ -15,6 +23,44 @@ function page(id: string, position: string): IPage {
  } as IPage;
 }

+// Flat SpaceTreeNode factory for buildTreeWithChildren (it consumes a flat list
+// with parentPageId pointers and nests them).
+function flatNode(
+  id: string,
+  parentPageId: string | null,
+  position: string,
+): SpaceTreeNode {
+  return {
+    id,
+    slugId: `slug-${id}`,
+    name: id.toUpperCase(),
+    icon: undefined,
+    position,
+    spaceId: "space-1",
+    parentPageId: parentPageId as unknown as string,
+    hasChildren: false,
+    children: [],
+  };
+}
+
+// Nested SpaceTreeNode factory for collectAllIds / collectBranchIds.
+function treeNode(
+  id: string,
+  children: SpaceTreeNode[] = [],
+): SpaceTreeNode {
+  return {
+    id,
+    slugId: `slug-${id}`,
+    name: id.toUpperCase(),
+    icon: undefined,
+    position: "a0",
+    spaceId: "space-1",
+    parentPageId: null as unknown as string,
+    hasChildren: children.length > 0,
+    children,
+  };
+}
+
 describe("buildTree", () => {
  it("builds one node per unique page", () => {
    const tree = buildTree([page("a", "a1"), page("b", "a2")]);
@@ -38,3 +84,192 @@ describe("buildTree", () => {
    expect(new Set(ids).size).toBe(ids.length);
  });
 });
+
+describe("collectBranchIds", () => {
+  it("returns every node-with-children id in a multi-level tree", () => {
+    const tree = [
+      treeNode("root", [
+        treeNode("branch1", [treeNode("leaf1")]),
+        treeNode("leaf2"),
+      ]),
+      treeNode("root2", [treeNode("leaf3")]),
+    ];
+    expect(collectBranchIds(tree).sort()).toEqual([
+      "branch1",
+      "root",
+      "root2",
+    ]);
+  });
+
+  it("returns [] for a leaf-only tree", () => {
+    const tree = [treeNode("a"), treeNode("b"), treeNode("c")];
+    expect(collectBranchIds(tree)).toEqual([]);
+  });
+
+  it("does NOT include a node whose children is an empty array", () => {
+    // hasChildren-less / empty-children nodes are leaves for expansion purposes.
+    const tree = [treeNode("a", [])];
+    expect(collectBranchIds(tree)).toEqual([]);
+  });
+
+  it("returns every ancestor id in a deep single chain", () => {
+    const chain = treeNode("a", [
+      treeNode("b", [treeNode("c", [treeNode("d")])]),
+    ]);
+    // a, b, c are branches; d is the leaf.
+    expect(collectBranchIds([chain])).toEqual(["a", "b", "c"]);
+  });
+
+  it("returns [] for an empty tree", () => {
+    expect(collectBranchIds([])).toEqual([]);
+  });
+});
+
+describe("collectAllIds", () => {
+  it("returns every id (roots, branches, leaves)", () => {
+    const tree = [
+      treeNode("root", [
+        treeNode("branch1", [treeNode("leaf1")]),
+        treeNode("leaf2"),
+      ]),
+      treeNode("root2"),
+    ];
+    expect(collectAllIds(tree).sort()).toEqual([
+      "branch1",
+      "leaf1",
+      "leaf2",
+      "root",
+      "root2",
+    ]);
+  });
+
+  it("returns every id in a deep chain", () => {
+    const chain = treeNode("a", [
+      treeNode("b", [treeNode("c", [treeNode("d")])]),
+    ]);
+    expect(collectAllIds([chain])).toEqual(["a", "b", "c", "d"]);
+  });
+
+  it("returns [] for an empty tree", () => {
+    expect(collectAllIds([])).toEqual([]);
+  });
+
+  it("is a superset of collectBranchIds for the same tree (property)", () => {
+    const tree = [
+      treeNode("root", [
+        treeNode("branch1", [treeNode("leaf1"), treeNode("leaf2")]),
+        treeNode("branch2", [treeNode("leaf3")]),
+        treeNode("leaf4"),
+      ]),
+      treeNode("root2", [treeNode("leaf5")]),
+    ];
+    const all = new Set(collectAllIds(tree));
+    const branches = collectBranchIds(tree);
+    for (const id of branches) {
+      expect(all.has(id)).toBe(true);
+    }
+    // And the superset is strictly larger (it also has the leaves).
+    expect(all.size).toBeGreaterThan(branches.length);
+  });
+});
+
+describe("buildTreeWithChildren", () => {
+  it("nests a flat list and sorts siblings by position", () => {
+    // Provided out of position order to prove the sort.
+    const flat = [
+      flatNode("root", null, "a0"),
+      flatNode("c2", "root", "a4"),
+      flatNode("c1", "root", "a1"),
+    ];
+    const tree = buildTreeWithChildren(flat);
+    expect(tree.map((n) => n.id)).toEqual(["root"]);
+    expect(tree[0].children.map((n) => n.id)).toEqual(["c1", "c2"]);
+  });
+
+  it("recomputes hasChildren to true for nodes that gain children", () => {
+    // Parent ships with hasChildren=false; building must flip it true.
+    const flat = [
+      flatNode("root", null, "a0"),
+      flatNode("child", "root", "a1"),
+    ];
+    expect(flat[0].hasChildren).toBe(false);
+    const tree = buildTreeWithChildren(flat);
+    expect(tree[0].hasChildren).toBe(true);
+  });
+
+  it("treats a node whose parentPageId is ABSENT from the list as a root (no crash)", () => {
+    // Permission-trimmed response: `orphan`'s parent `missing` was filtered out
+    // server-side. The function must not throw and must surface the orphan as a
+    // root rather than dropping or crashing on it.
+    const flat = [
+      flatNode("root", null, "a0"),
+      flatNode("orphan", "missing", "a2"),
+    ];
+    let tree: SpaceTreeNode[] = [];
+    expect(() => {
+      tree = buildTreeWithChildren(flat);
+    }).not.toThrow();
+    expect(tree.map((n) => n.id).sort()).toEqual(["orphan", "root"]);
+  });
+});
+
+describe("openBranches", () => {
+  it("sets all given ids to true", () => {
+    const next = openBranches({}, ["a", "b", "c"]);
+    expect(next).toEqual({ a: true, b: true, c: true });
+  });
+
+  it("preserves pre-existing open ids and other-space ids", () => {
+    const prev = { existing: true, "other-space": true, closed: false };
+    const next = openBranches(prev, ["a"]);
+    expect(next).toEqual({
+      existing: true,
+      "other-space": true,
+      closed: false,
+      a: true,
+    });
+  });
+
+  it("does not mutate the input map", () => {
+    const prev = { a: false };
+    const next = openBranches(prev, ["a"]);
+    expect(prev).toEqual({ a: false });
+    expect(next).not.toBe(prev);
+  });
+
+  it("is idempotent", () => {
+    const once = openBranches({ z: true }, ["a", "b"]);
+    const twice = openBranches(once, ["a", "b"]);
+    expect(twice).toEqual(once);
+  });
+});
+
+describe("closeIds", () => {
+  it("flips current-space ids to false while leaving OTHER-space ids untouched", () => {
+    const prev = {
+      "current-1": true,
+      "current-2": true,
+      "other-space": true,
+    };
+    const next = closeIds(prev, ["current-1", "current-2"]);
+    expect(next).toEqual({
+      "current-1": false,
+      "current-2": false,
+      "other-space": true, // untouched
+    });
+  });
+
+  it("does not mutate the input map", () => {
+    const prev = { a: true };
+    const next = closeIds(prev, ["a"]);
+    expect(prev).toEqual({ a: true });
+    expect(next).not.toBe(prev);
+  });
+
+  it("is idempotent", () => {
+    const once = closeIds({ keep: true }, ["a", "b"]);
+    const twice = closeIds(once, ["a", "b"]);
+    expect(twice).toEqual(once);
+    expect(twice).toEqual({ keep: true, a: false, b: false });
+  });
+});
@@ -142,11 +142,17 @@ export function buildTreeWithChildren(items: SpaceTreeNode[]): SpaceTreeNode[] {
  // Build the tree array
  items.forEach((item) => {
    const node = nodeMap[item.id];
-    if (item.parentPageId !== null) {
+    // A permission-trimmed response can include a node whose `parentPageId` is
+    // not in the list (the parent was filtered out server-side). Treat such an
+    // orphan as a root instead of dereferencing an absent parent and throwing
+    // "Cannot read properties of undefined". Happy-path behaviour is unchanged:
+    // a node whose parent IS present still nests under it.
+    if (item.parentPageId !== null && nodeMap[item.parentPageId]) {
      // Find the parent node and add the current node to its children
      nodeMap[item.parentPageId].children.push(node);
    } else {
-      // If the item has no parent, it's a root node, so add it to the result array
+      // If the item has no parent (or its parent isn't loaded), it's a root
+      // node, so add it to the result array.
      result.push(node);
    }
  });
@@ -254,3 +260,30 @@ export function collectBranchIds(nodes: SpaceTreeNode[]): string[] {
  walk(nodes);
  return ids;
 }
+
+// The open-state map (`openTreeNodesAtom`) is shared across spaces. Pure
+// next-map helpers for expand/collapse so the merge logic can be unit-tested
+// without rendering SpaceTree. Both return a fresh map and never mutate the
+// input — ids not in `ids` (e.g. other spaces) are carried over untouched.
+
+// Set each id in `ids` to true (open). Pre-existing entries (including other
+// spaces' open state) are preserved.
+export function openBranches(
+  prevMap: Record<string, boolean>,
+  ids: string[],
+): Record<string, boolean> {
+  const next = { ...prevMap };
+  for (const id of ids) next[id] = true;
+  return next;
+}
+
+// Set each id in `ids` to false (closed). Entries not listed (e.g. other
+// spaces' ids) are left exactly as they were.
+export function closeIds(
+  prevMap: Record<string, boolean>,
+  ids: string[],
+): Record<string, boolean> {
+  const next = { ...prevMap };
+  for (const id of ids) next[id] = false;
+  return next;
+}