mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-03 11:10:23 +02:00
* fix(workspace): drop workspace from list cache while delete is pending (MUL-4129) The delete-workspace flow navigates away before awaiting the DELETE (required ordering — see navigateAwayFromCurrentWorkspace's CancelledError notes), but useDeleteWorkspace left the workspace in the list cache until onSettled. During the pending window any list refetch re-presented the deleting workspace as a selectable/current option. Optimistically remove it in onMutate (after cancelling in-flight list fetches), roll the snapshot back in onError so a failed delete restores the workspace alongside the existing error toast, and keep the onSettled invalidate as the server-truth reconcile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> * fix(workspace): own storage cleanup on delete success and tombstone pending deletes (MUL-4129) Address final-review blockers on #4980: 1. The realtime workspace:deleted handler reverse-looks-up the slug from the list cache to clear the ${key}:${slug} persisted namespace; the optimistic removal empties that row on the initiating client, so the lookup misses and cleanup was silently skipped. Capture the slug in onMutate before removal and clear storage in onSuccess only — a failed DELETE rolls back and must not touch persisted state. 2. cancelQueries only covered fetches already in flight at onMutate. Add a pending-delete tombstone (marked onMutate, lifted onSettled before the reconcile invalidate) filtered inside workspaceListOptions' queryFn, so invalidation/reconnect/fetchQuery refetches that land mid-pending cannot write the not-yet-committed row back into cache. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> * docs(claude-md): scope optimistic updates to same-screen field patches Replace the blanket "mutations optimistic by default" state rule with three scoped rules: optimistic only for predictable same-screen field patches; navigating/confirming flows (create/delete/leave) await the server first; chat send uses the pending-message pattern. Aligned with TanStack Query maintainer guidance and React Router's pending-UI criteria; the old blanket rule is what steered the original MUL-4129 fix toward optimistic entity removal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(workspace): await delete before navigating; drop optimistic removal (MUL-4129) Rework of the previous approach on this branch. The optimistic removal emptied the workspace list cache at click time, while the settings page was still mounted on the old slug — useWorkspaceId (URL slug + list lookup) then threw 'no workspace selected'. Root cause of the original navigate-first ordering was dual ownership of delete handling between the initiating flow and the realtime workspace:deleted handler. - useDeleteWorkspace: no optimistic removal, no rollback; onMutate only marks the delete self-initiated and captures the slug; onSuccess owns storage cleanup; onSettled invalidates. - pending-delete.ts: repurposed from tombstone filter to self-initiated marker; kept on success (suppresses the WS echo), lifted on failure. - use-realtime-sync: workspace:deleted no-ops for self-initiated deletes; it now only serves deletes initiated elsewhere. - workspace-tab: confirm dialog stays open in loading state, navigate only after the DELETE succeeds; failure leaves the user in place with nothing to roll back. Replace throwing useWorkspaceId with workspace?.id + enabled gating (independent crash on external deletes of the current workspace). Known debt: useLeaveWorkspace still navigates before awaiting (member:removed has no self-initiated marker yet). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
252 lines
9.1 KiB
TypeScript
252 lines
9.1 KiB
TypeScript
/**
|
|
* @vitest-environment jsdom
|
|
*/
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
import { renderHook } from "@testing-library/react";
|
|
import type { ReactNode } from "react";
|
|
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
|
|
import type { WSClient } from "../api/ws-client";
|
|
import { defaultStorage } from "../platform/storage";
|
|
import { workspaceKeys } from "../workspace/queries";
|
|
import {
|
|
markWorkspaceDeletePending,
|
|
unmarkWorkspaceDeletePending,
|
|
} from "../workspace/pending-delete";
|
|
import { useRealtimeSync, type RealtimeSyncStores } from "./use-realtime-sync";
|
|
|
|
vi.mock("../platform/workspace-storage", () => ({
|
|
getCurrentWsId: () => "ws-1",
|
|
getCurrentSlug: () => "test-ws",
|
|
}));
|
|
|
|
vi.mock("../paths", () => ({
|
|
useHasOnboarded: () => true,
|
|
resolvePostAuthDestination: () => "/",
|
|
}));
|
|
|
|
function createMockWs(): WSClient {
|
|
return {
|
|
on: vi.fn(() => () => {}),
|
|
onAny: vi.fn(() => () => {}),
|
|
onReconnect: vi.fn(() => () => {}),
|
|
} as unknown as WSClient;
|
|
}
|
|
|
|
function createStores(): RealtimeSyncStores {
|
|
return {
|
|
authStore: Object.assign(() => ({}), {
|
|
getState: () => ({ user: { id: "u1" } }),
|
|
subscribe: () => () => {},
|
|
setState: () => {},
|
|
destroy: () => {},
|
|
}),
|
|
} as unknown as RealtimeSyncStores;
|
|
}
|
|
|
|
function createWrapper(qc: QueryClient) {
|
|
// Named function (not arrow) so react/display-name lint rule passes —
|
|
// anonymous render-fn components break that rule even in test files.
|
|
return function Wrapper({ children }: { children: ReactNode }) {
|
|
return <QueryClientProvider client={qc}>{children}</QueryClientProvider>;
|
|
};
|
|
}
|
|
|
|
describe("useRealtimeSync — ws instance change", () => {
|
|
let qc: QueryClient;
|
|
let stores: RealtimeSyncStores;
|
|
let invalidateSpy: ReturnType<typeof vi.spyOn>;
|
|
|
|
beforeEach(() => {
|
|
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
|
stores = createStores();
|
|
invalidateSpy = vi.spyOn(qc, "invalidateQueries");
|
|
});
|
|
|
|
it("skips invalidation on first non-null ws instance", () => {
|
|
const ws = createMockWs();
|
|
renderHook(() => useRealtimeSync(ws, stores), {
|
|
wrapper: createWrapper(qc),
|
|
});
|
|
|
|
// The main effect calls invalidateQueries for its own setup, but the
|
|
// ws-instance-change effect should NOT have fired invalidation.
|
|
// The only invalidateQueries calls should come from the main effect's
|
|
// event handlers, not from the instance-change effect.
|
|
// We verify by checking that no call was made with workspaceKeys.list()
|
|
// pattern from the instance-change path (it logs a specific message).
|
|
// Simpler: count calls — first mount with a ws should not trigger the
|
|
// workspace-scoped bulk invalidation.
|
|
expect(invalidateSpy).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("does not invalidate when ws goes from instance to null", () => {
|
|
const ws1 = createMockWs();
|
|
const { rerender } = renderHook(
|
|
({ ws }) => useRealtimeSync(ws, stores),
|
|
{ initialProps: { ws: ws1 as WSClient | null }, wrapper: createWrapper(qc) },
|
|
);
|
|
|
|
invalidateSpy.mockClear();
|
|
rerender({ ws: null });
|
|
|
|
expect(invalidateSpy).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("invalidates exactly once when a new ws instance appears after null gap", () => {
|
|
const ws1 = createMockWs();
|
|
const { rerender } = renderHook(
|
|
({ ws }) => useRealtimeSync(ws, stores),
|
|
{ initialProps: { ws: ws1 as WSClient | null }, wrapper: createWrapper(qc) },
|
|
);
|
|
|
|
// Simulate workspace switch: ws -> null -> new ws
|
|
invalidateSpy.mockClear();
|
|
rerender({ ws: null });
|
|
expect(invalidateSpy).not.toHaveBeenCalled();
|
|
|
|
const ws2 = createMockWs();
|
|
rerender({ ws: ws2 });
|
|
|
|
// Should have called invalidateQueries for all workspace-scoped keys
|
|
// (15 workspace-scoped + 6 per-issue prefixes + 4 per-chat prefixes
|
|
// + 1 workspaceKeys.list() + 1 cross-workspace inbox unread summary = 27 calls)
|
|
expect(invalidateSpy).toHaveBeenCalledTimes(27);
|
|
});
|
|
|
|
it("does not re-invalidate when rerendered with the same ws instance", () => {
|
|
const ws1 = createMockWs();
|
|
const { rerender } = renderHook(
|
|
({ ws }) => useRealtimeSync(ws, stores),
|
|
{ initialProps: { ws: ws1 as WSClient | null }, wrapper: createWrapper(qc) },
|
|
);
|
|
|
|
invalidateSpy.mockClear();
|
|
// Rerender with same instance
|
|
rerender({ ws: ws1 });
|
|
|
|
expect(invalidateSpy).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("invalidates chat, pins, labels, and invitations queries on ws instance change", () => {
|
|
const ws1 = createMockWs();
|
|
const { rerender } = renderHook(
|
|
({ ws }) => useRealtimeSync(ws, stores),
|
|
{ initialProps: { ws: ws1 as WSClient | null }, wrapper: createWrapper(qc) },
|
|
);
|
|
|
|
invalidateSpy.mockClear();
|
|
rerender({ ws: null });
|
|
|
|
const ws2 = createMockWs();
|
|
rerender({ ws: ws2 });
|
|
|
|
const calls = invalidateSpy.mock.calls.map((call: [{ queryKey?: unknown }, ...unknown[]]) => call[0].queryKey);
|
|
expect(calls).toContainEqual(["chat", "ws-1"]);
|
|
expect(calls).toContainEqual(["labels", "ws-1"]);
|
|
expect(calls).toContainEqual(["workspaces", "ws-1", "invitations"]);
|
|
});
|
|
|
|
it("invalidates per-issue caches (no wsId in key) on ws instance change", () => {
|
|
// These keys are not under the ["issues", wsId] prefix, so they need
|
|
// their own invalidation on recovery — otherwise events missed while
|
|
// disconnected leave them stale forever (staleTime: Infinity, #3953).
|
|
const ws1 = createMockWs();
|
|
const { rerender } = renderHook(
|
|
({ ws }) => useRealtimeSync(ws, stores),
|
|
{ initialProps: { ws: ws1 as WSClient | null }, wrapper: createWrapper(qc) },
|
|
);
|
|
|
|
invalidateSpy.mockClear();
|
|
rerender({ ws: null });
|
|
|
|
const ws2 = createMockWs();
|
|
rerender({ ws: ws2 });
|
|
|
|
const calls = invalidateSpy.mock.calls.map((call: [{ queryKey?: unknown }, ...unknown[]]) => call[0].queryKey);
|
|
expect(calls).toContainEqual(["issues", "timeline"]);
|
|
expect(calls).toContainEqual(["issues", "reactions"]);
|
|
expect(calls).toContainEqual(["issues", "subscribers"]);
|
|
expect(calls).toContainEqual(["issues", "usage"]);
|
|
expect(calls).toContainEqual(["issues", "attachments"]);
|
|
expect(calls).toContainEqual(["issues", "tasks"]);
|
|
});
|
|
|
|
it("invalidates per-chat-session caches (no wsId in key) on ws instance change", () => {
|
|
// These keys are not under the ["chat", wsId] prefix, so they need their
|
|
// own recovery invalidation when reconnecting after missed chat/task events.
|
|
const ws1 = createMockWs();
|
|
const { rerender } = renderHook(
|
|
({ ws }) => useRealtimeSync(ws, stores),
|
|
{ initialProps: { ws: ws1 as WSClient | null }, wrapper: createWrapper(qc) },
|
|
);
|
|
|
|
invalidateSpy.mockClear();
|
|
rerender({ ws: null });
|
|
|
|
const ws2 = createMockWs();
|
|
rerender({ ws: ws2 });
|
|
|
|
const calls = invalidateSpy.mock.calls.map((call: [{ queryKey?: unknown }, ...unknown[]]) => call[0].queryKey);
|
|
expect(calls).toContainEqual(["chat", "messages"]);
|
|
expect(calls).toContainEqual(["chat", "messages-page"]);
|
|
expect(calls).toContainEqual(["chat", "pending-task"]);
|
|
expect(calls).toContainEqual(["task-messages"]);
|
|
});
|
|
});
|
|
|
|
describe("useRealtimeSync — workspace:deleted self-initiated suppression", () => {
|
|
let qc: QueryClient;
|
|
let stores: RealtimeSyncStores;
|
|
|
|
beforeEach(() => {
|
|
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
|
stores = createStores();
|
|
});
|
|
|
|
afterEach(() => {
|
|
unmarkWorkspaceDeletePending("ws-2");
|
|
localStorage.clear();
|
|
});
|
|
|
|
// getCurrentWsId is mocked to "ws-1" at module level, so deleting "ws-2"
|
|
// never enters the relocate branch — these tests only exercise the
|
|
// storage-cleanup path, which is the observable difference between a
|
|
// handled and a suppressed event.
|
|
const dispatchWorkspaceDeleted = (ws: WSClient, workspaceId: string) => {
|
|
const call = vi
|
|
.mocked(ws.on)
|
|
.mock.calls.find(([event]) => event === "workspace:deleted");
|
|
expect(call).toBeDefined();
|
|
(call![1] as (p: unknown) => void)({ workspace_id: workspaceId });
|
|
};
|
|
|
|
it("ignores the event for a delete this client initiated", () => {
|
|
const ws = createMockWs();
|
|
renderHook(() => useRealtimeSync(ws, stores), {
|
|
wrapper: createWrapper(qc),
|
|
});
|
|
qc.setQueryData(workspaceKeys.list(), [{ id: "ws-2", slug: "delete-me" }]);
|
|
defaultStorage.setItem("multica_issue_draft:delete-me", "draft");
|
|
|
|
markWorkspaceDeletePending("ws-2");
|
|
dispatchWorkspaceDeleted(ws, "ws-2");
|
|
|
|
// useDeleteWorkspace.onSuccess owns cleanup for self-initiated deletes;
|
|
// the handler must not have touched storage.
|
|
expect(defaultStorage.getItem("multica_issue_draft:delete-me")).toBe("draft");
|
|
});
|
|
|
|
it("still cleans up for a delete initiated elsewhere", () => {
|
|
const ws = createMockWs();
|
|
renderHook(() => useRealtimeSync(ws, stores), {
|
|
wrapper: createWrapper(qc),
|
|
});
|
|
qc.setQueryData(workspaceKeys.list(), [{ id: "ws-2", slug: "delete-me" }]);
|
|
defaultStorage.setItem("multica_issue_draft:delete-me", "draft");
|
|
|
|
dispatchWorkspaceDeleted(ws, "ws-2");
|
|
|
|
expect(defaultStorage.getItem("multica_issue_draft:delete-me")).toBeNull();
|
|
});
|
|
});
|