feat(FN-3711): consolidate graph position storage and remove scopedStorage
Consolidated graph position storage into a canonical shared helper (`projectStorage.ts`) with hardened persistence logic and removed the duplicate `scopedStorage` module. Updated integration tests and documented the canonical storage approach. A smaller fix (`FN-3628`) addressed mobile touch targets Fusion-Task-Id: FN-3711
This commit is contained in:
@@ -27,11 +27,12 @@ Plugin-provided top-level **Graph** dashboard view for Fusion.
|
||||
|
||||
## Position persistence
|
||||
|
||||
- Storage key format: `kb:${projectId}:dependency-graph-positions` (falls back to `dependency-graph-positions` when no project is selected)
|
||||
- Canonical base key: `fusion-plugin-dependency-graph:positions`
|
||||
- Storage key format: `kb:${projectId}:fusion-plugin-dependency-graph:positions` (falls back to `fusion-plugin-dependency-graph:positions` when no project is selected)
|
||||
- Read path: positions load on graph mount and whenever `projectId` changes, then merge with fresh auto-layout so new tasks still receive layout defaults
|
||||
- Write path: positions persist on drag end only (not on every drag frame), filtered to currently visible tasks for stale cleanup
|
||||
- Reset behavior: Fit to graph / Reset view clear persisted positions and re-apply auto-layout
|
||||
- Implementation detail: the plugin ships local `scopedStorage` helpers duplicated from dashboard `projectStorage` to stay plugin-isolated while preserving the same `kb:${projectId}:${baseKey}` convention
|
||||
- Implementation detail: the plugin now reuses dashboard `projectStorage` helpers (`getScopedItem` / `setScopedItem` / `removeScopedItem`) instead of duplicating scoped localStorage logic
|
||||
|
||||
## Dependency chain highlighting
|
||||
|
||||
|
||||
@@ -74,7 +74,7 @@ describe("DependencyGraph persistence", () => {
|
||||
fireEvent.pointerMove(node, { pointerId: 1, isPrimary: true, clientX: 30, clientY: 40 });
|
||||
fireEvent.pointerUp(node, { pointerId: 1, isPrimary: true, clientX: 30, clientY: 40 });
|
||||
|
||||
expect(window.localStorage.getItem("kb:p1:dependency-graph-positions")).toContain('"A":{"x":20,"y":30}');
|
||||
expect(window.localStorage.getItem("kb:p1:fusion-plugin-dependency-graph:positions")).toContain('"A":{"x":20,"y":30}');
|
||||
|
||||
unmount();
|
||||
render(<DependencyGraph tasks={[createTask("A")]} projectId="p1" />);
|
||||
@@ -84,7 +84,7 @@ describe("DependencyGraph persistence", () => {
|
||||
});
|
||||
|
||||
it("merges saved positions with auto-layout for new tasks", () => {
|
||||
window.localStorage.setItem("kb:p1:dependency-graph-positions", JSON.stringify({ A: { x: 25, y: 35 } }));
|
||||
window.localStorage.setItem("kb:p1:fusion-plugin-dependency-graph:positions", JSON.stringify({ A: { x: 25, y: 35 } }));
|
||||
|
||||
render(<DependencyGraph tasks={[createTask("A"), createTask("B")]} projectId="p1" />);
|
||||
|
||||
@@ -93,18 +93,18 @@ describe("DependencyGraph persistence", () => {
|
||||
});
|
||||
|
||||
it("fit to graph clears saved positions and reapplies auto-layout", () => {
|
||||
window.localStorage.setItem("kb:p1:dependency-graph-positions", JSON.stringify({ A: { x: 25, y: 35 } }));
|
||||
window.localStorage.setItem("kb:p1:fusion-plugin-dependency-graph:positions", JSON.stringify({ A: { x: 25, y: 35 } }));
|
||||
|
||||
render(<DependencyGraph tasks={[createTask("A")]} projectId="p1" />);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Fit to graph" }));
|
||||
|
||||
expect(window.localStorage.getItem("kb:p1:dependency-graph-positions")).toBeNull();
|
||||
expect(window.localStorage.getItem("kb:p1:fusion-plugin-dependency-graph:positions")).toBeNull();
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 0px");
|
||||
});
|
||||
|
||||
it("switching projects loads project-scoped positions", () => {
|
||||
window.localStorage.setItem("kb:p1:dependency-graph-positions", JSON.stringify({ A: { x: 11, y: 22 } }));
|
||||
window.localStorage.setItem("kb:p2:dependency-graph-positions", JSON.stringify({ A: { x: 33, y: 44 } }));
|
||||
window.localStorage.setItem("kb:p1:fusion-plugin-dependency-graph:positions", JSON.stringify({ A: { x: 11, y: 22 } }));
|
||||
window.localStorage.setItem("kb:p2:fusion-plugin-dependency-graph:positions", JSON.stringify({ A: { x: 33, y: 44 } }));
|
||||
|
||||
const { rerender } = render(<DependencyGraph tasks={[createTask("A")]} projectId="p1" />);
|
||||
expect(screen.getByTestId("graph-task-node-A").getAttribute("style")).toContain("left: 11px");
|
||||
|
||||
@@ -21,7 +21,7 @@ describe("graphPositionStorage", () => {
|
||||
});
|
||||
|
||||
it("loadPositions returns parsed positions from localStorage", () => {
|
||||
window.localStorage.setItem("kb:p1:dependency-graph-positions", JSON.stringify({ a: { x: 1, y: 2 } }));
|
||||
window.localStorage.setItem("kb:p1:fusion-plugin-dependency-graph:positions", JSON.stringify({ a: { x: 1, y: 2 } }));
|
||||
expect(loadPositions("p1")).toEqual({ a: { x: 1, y: 2 } });
|
||||
});
|
||||
|
||||
@@ -30,13 +30,13 @@ describe("graphPositionStorage", () => {
|
||||
});
|
||||
|
||||
it("loadPositions returns empty object for invalid json", () => {
|
||||
window.localStorage.setItem("kb:p1:dependency-graph-positions", "{oops");
|
||||
window.localStorage.setItem("kb:p1:fusion-plugin-dependency-graph:positions", "{oops");
|
||||
expect(loadPositions("p1")).toEqual({});
|
||||
});
|
||||
|
||||
it("loadPositions skips entries with invalid position shape", () => {
|
||||
window.localStorage.setItem(
|
||||
"kb:p1:dependency-graph-positions",
|
||||
"kb:p1:fusion-plugin-dependency-graph:positions",
|
||||
JSON.stringify({
|
||||
good: { x: 1, y: 2 },
|
||||
badX: { x: "1", y: 2 },
|
||||
@@ -49,13 +49,13 @@ describe("graphPositionStorage", () => {
|
||||
|
||||
it("savePositions writes filtered positions json to scoped localStorage key", () => {
|
||||
savePositions({ a: { x: 1, y: 2 }, b: { x: 3, y: 4 } }, new Set(["a"]), "p1");
|
||||
expect(window.localStorage.getItem("kb:p1:dependency-graph-positions")).toBe(JSON.stringify({ a: { x: 1, y: 2 } }));
|
||||
expect(window.localStorage.getItem("kb:p1:fusion-plugin-dependency-graph:positions")).toBe(JSON.stringify({ a: { x: 1, y: 2 } }));
|
||||
});
|
||||
|
||||
it("clearPositions removes scoped localStorage key", () => {
|
||||
window.localStorage.setItem("kb:p1:dependency-graph-positions", JSON.stringify({ a: { x: 1, y: 2 } }));
|
||||
window.localStorage.setItem("kb:p1:fusion-plugin-dependency-graph:positions", JSON.stringify({ a: { x: 1, y: 2 } }));
|
||||
clearPositions("p1");
|
||||
expect(window.localStorage.getItem("kb:p1:dependency-graph-positions")).toBeNull();
|
||||
expect(window.localStorage.getItem("kb:p1:fusion-plugin-dependency-graph:positions")).toBeNull();
|
||||
});
|
||||
|
||||
it("mergePositions prefers saved for overlap and keeps auto-layout for new tasks", () => {
|
||||
@@ -81,4 +81,15 @@ describe("graphPositionStorage", () => {
|
||||
it("mergePositions returns auto-layout unchanged when saved is empty", () => {
|
||||
expect(mergePositions({ a: { x: 1, y: 2 } }, {}, new Set(["a"]))).toEqual({ a: { x: 1, y: 2 } });
|
||||
});
|
||||
|
||||
it("loadPositions returns empty object when localStorage.getItem is unavailable", () => {
|
||||
vi.stubGlobal("window", { localStorage: {} });
|
||||
expect(loadPositions("p1")).toEqual({});
|
||||
});
|
||||
|
||||
it("savePositions and clearPositions are no-ops when localStorage methods are unavailable", () => {
|
||||
vi.stubGlobal("window", { localStorage: {} });
|
||||
expect(() => savePositions({ a: { x: 1, y: 2 } }, new Set(["a"]), "p1")).not.toThrow();
|
||||
expect(() => clearPositions("p1")).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -38,7 +38,7 @@ describe("dependency graph position persistence", () => {
|
||||
it("saves/restores project-scoped position shape", () => {
|
||||
savePositions({ A: { x: 10, y: 20 }, B: { x: 30, y: 40 } }, new Set(["A", "B"]), "p1");
|
||||
|
||||
expect(window.localStorage.getItem("kb:p1:dependency-graph-positions")).toBe(
|
||||
expect(window.localStorage.getItem("kb:p1:fusion-plugin-dependency-graph:positions")).toBe(
|
||||
JSON.stringify({ A: { x: 10, y: 20 }, B: { x: 30, y: 40 } }),
|
||||
);
|
||||
expect(loadPositions("p1")).toEqual({ A: { x: 10, y: 20 }, B: { x: 30, y: 40 } });
|
||||
@@ -53,7 +53,7 @@ describe("dependency graph position persistence", () => {
|
||||
});
|
||||
|
||||
it("falls back to auto-layout with corrupt storage and does not crash", () => {
|
||||
window.localStorage.setItem("kb:p1:dependency-graph-positions", "{broken");
|
||||
window.localStorage.setItem("kb:p1:fusion-plugin-dependency-graph:positions", "{broken");
|
||||
|
||||
render(<DependencyGraph tasks={[createTask("A")]} projectId="p1" />);
|
||||
expect(screen.getByTestId("graph-task-node-A")).toBeTruthy();
|
||||
@@ -68,7 +68,7 @@ describe("dependency graph position persistence", () => {
|
||||
fireEvent.pointerMove(node, { pointerId: 1, isPrimary: true, clientX: 30, clientY: 40 });
|
||||
fireEvent.pointerUp(node, { pointerId: 1, isPrimary: true, clientX: 30, clientY: 40 });
|
||||
|
||||
expect(window.localStorage.getItem("kb:p1:dependency-graph-positions")).toContain('"A"');
|
||||
expect(window.localStorage.getItem("kb:p1:fusion-plugin-dependency-graph:positions")).toContain('"A"');
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -79,11 +79,11 @@ describe("dependency graph position persistence", () => {
|
||||
fireEvent.pointerMove(node, { pointerId: 1, isPrimary: true, clientX: 40, clientY: 50 });
|
||||
fireEvent.pointerUp(node, { pointerId: 1, isPrimary: true, clientX: 40, clientY: 50 });
|
||||
|
||||
window.localStorage.removeItem("kb:p1:dependency-graph-positions");
|
||||
window.localStorage.removeItem("kb:p1:fusion-plugin-dependency-graph:positions");
|
||||
unmount();
|
||||
|
||||
render(<DependencyGraph tasks={[createTask("A")]} projectId="p1" />);
|
||||
expect(window.localStorage.getItem("kb:p1:dependency-graph-positions")).toBeNull();
|
||||
expect(window.localStorage.getItem("kb:p1:fusion-plugin-dependency-graph:positions")).toBeNull();
|
||||
expect(screen.getByTestId("graph-task-node-A")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,58 +0,0 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { getScopedItem, removeScopedItem, scopedKey, setScopedItem } from "../utils/scopedStorage";
|
||||
|
||||
function createStorage() {
|
||||
const store = new Map<string, string>();
|
||||
return {
|
||||
getItem: (key: string) => store.get(key) ?? null,
|
||||
setItem: (key: string, value: string) => {
|
||||
store.set(key, value);
|
||||
},
|
||||
removeItem: (key: string) => {
|
||||
store.delete(key);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("scopedStorage", () => {
|
||||
beforeEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
vi.stubGlobal("window", { localStorage: createStorage() });
|
||||
});
|
||||
|
||||
it("scopedKey uses kb project prefix when project id is provided", () => {
|
||||
expect(scopedKey("baseKey", "project-1")).toBe("kb:project-1:baseKey");
|
||||
});
|
||||
|
||||
it("scopedKey falls back to unscoped key for undefined/null/empty project id", () => {
|
||||
expect(scopedKey("baseKey", undefined)).toBe("baseKey");
|
||||
expect(scopedKey("baseKey", null)).toBe("baseKey");
|
||||
expect(scopedKey("baseKey", "")).toBe("baseKey");
|
||||
});
|
||||
|
||||
it("getScopedItem reads from localStorage using scoped key", () => {
|
||||
window.localStorage.setItem("kb:project-1:baseKey", "value");
|
||||
expect(getScopedItem("baseKey", "project-1")).toBe("value");
|
||||
});
|
||||
|
||||
it("getScopedItem returns null when window is undefined", () => {
|
||||
vi.stubGlobal("window", undefined);
|
||||
expect(getScopedItem("baseKey", "project-1")).toBeNull();
|
||||
});
|
||||
|
||||
it("setScopedItem writes to localStorage using scoped key", () => {
|
||||
setScopedItem("baseKey", "value", "project-1");
|
||||
expect(window.localStorage.getItem("kb:project-1:baseKey")).toBe("value");
|
||||
});
|
||||
|
||||
it("setScopedItem is a no-op when window is undefined", () => {
|
||||
vi.stubGlobal("window", undefined);
|
||||
expect(() => setScopedItem("baseKey", "value", "project-1")).not.toThrow();
|
||||
});
|
||||
|
||||
it("removeScopedItem removes from localStorage using scoped key", () => {
|
||||
window.localStorage.setItem("kb:project-1:baseKey", "value");
|
||||
removeScopedItem("baseKey", "project-1");
|
||||
expect(window.localStorage.getItem("kb:project-1:baseKey")).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -1,8 +1,8 @@
|
||||
import { getScopedItem, removeScopedItem, setScopedItem } from "./scopedStorage";
|
||||
import { getScopedItem, removeScopedItem, setScopedItem } from "@fusion/dashboard/app/utils/projectStorage";
|
||||
|
||||
export type NodePositions = Record<string, { x: number; y: number }>;
|
||||
|
||||
const STORAGE_KEY = "dependency-graph-positions";
|
||||
const STORAGE_KEY = "fusion-plugin-dependency-graph:positions";
|
||||
|
||||
function isPosition(value: unknown): value is { x: number; y: number } {
|
||||
if (!value || typeof value !== "object") return false;
|
||||
|
||||
@@ -1,49 +0,0 @@
|
||||
// Duplicated from packages/dashboard/app/utils/projectStorage.ts for plugin isolation.
|
||||
// Keeps the same project-scoped key convention: kb:${projectId}:${baseKey}.
|
||||
|
||||
export function scopedKey(baseKey: string, projectId?: string | null): string {
|
||||
if (typeof projectId !== "string" || projectId.length === 0) {
|
||||
return baseKey;
|
||||
}
|
||||
|
||||
return `kb:${projectId}:${baseKey}`;
|
||||
}
|
||||
|
||||
export function getScopedItem(baseKey: string, projectId?: string | null): string | null {
|
||||
if (typeof window === "undefined") {
|
||||
return null;
|
||||
}
|
||||
|
||||
const getItem = window.localStorage?.getItem;
|
||||
if (typeof getItem !== "function") {
|
||||
return null;
|
||||
}
|
||||
|
||||
return getItem.call(window.localStorage, scopedKey(baseKey, projectId));
|
||||
}
|
||||
|
||||
export function setScopedItem(baseKey: string, value: string, projectId?: string | null): void {
|
||||
if (typeof window === "undefined") {
|
||||
return;
|
||||
}
|
||||
|
||||
const setItem = window.localStorage?.setItem;
|
||||
if (typeof setItem !== "function") {
|
||||
return;
|
||||
}
|
||||
|
||||
setItem.call(window.localStorage, scopedKey(baseKey, projectId), value);
|
||||
}
|
||||
|
||||
export function removeScopedItem(baseKey: string, projectId?: string | null): void {
|
||||
if (typeof window === "undefined") {
|
||||
return;
|
||||
}
|
||||
|
||||
const removeItem = window.localStorage?.removeItem;
|
||||
if (typeof removeItem !== "function") {
|
||||
return;
|
||||
}
|
||||
|
||||
removeItem.call(window.localStorage, scopedKey(baseKey, projectId));
|
||||
}
|
||||
Reference in New Issue
Block a user