Skip to content

Commit ead8a53

Browse files
ConnorYohFrooodle
andauthored
feat(editor): move signing sessions onto TanStack Query (Stirling-Tools#7436)
# Description of Changes Step 4 of the TanStack Query rollout, and the first of the polling hooks. Follows Stirling-Tools#7264, Stirling-Tools#7283, Stirling-Tools#7285. ## The problem `useSigningSessions` hand-rolled its own fetch, loading state and `setInterval`. Two consequences: - **A raw `setInterval` keeps polling a hidden tab.** Browsers throttle background timers, they do not stop them, so a backgrounded editor with Shared Sign open keeps hitting both endpoints for as long as it is open. - **No tests.** The hook had none, and its quietest behaviour (below) is the easiest thing to break without noticing. ## End state One query behind `qk.signingSessions()`, with the polling lifecycle handed to the library: - Polling stops while the tab is hidden, and refetches on return rather than leaving data up to a full interval stale. - Mounts render from cache while they revalidate, so moving between the tool picker and the signing tool no longer flashes an empty list. - 12 tests where there were none. Same return shape, so no consumer files change. ### What this is not This is not a deduplication win. The three consumers are never mounted at the same time: `ToolPanel` renders the tool picker or the active tool and never both, so the badge cannot be on screen with either of the others, and `SharedSigningLauncher` and `useSigningSessionController` sit inside two different tools. The shared key earns its keep on cache reuse across those transitions, not on concurrent fetches. ## The bit worth reviewing The hand-rolled `{ silent: true }` flag encoded three states, and no single Query flag reproduces them: | | Spinner | Toast on failure | |---|---|---| | First load | yes | yes | | Background poll | no | no | | Explicit refetch | **yes** | **yes** | `isLoading` is false during an explicit refetch when data is already on screen; `isFetching` is true during a background poll. Neither matches, so the user-initiated case is tracked with a small flag and the failure toast is gated on `isLoadingError` plus the explicit path. ## Testing Twelve tests. Rather than trust them, each claim was checked by breaking the implementation and confirming the relevant test fails: | Mutation | Caught by | |---|---| | `refetchIntervalInBackground: true` | hidden-tab test | | Drop `refetchOnWindowFocus` | returns-to-view test | | Drop the user-initiated spinner flag | manual-refresh test | | Toast on every error | background-failure-is-silent test | | Give each observer its own key | dedupe test | Three things worth knowing for the next conversion: - **`waitFor` flushes renders.** Recording an index *after* `waitFor(callCount === 2)` skips past the in-flight render, so a "did the spinner flip on" assertion passes vacuously. The marker has to go before the poll. - **Fake timers hide in-flight state.** The fetch settles inside the same `act()`, so the intermediate render never happens. That test uses real timers and a held-open promise. - **`visibilitychange` has to bubble.** query-core listens for it on `window`, and the real event bubbles from `document`. A test helper dispatching a non-bubbling event never reaches the focus manager, and the pause behaviour still appears to work because `refetchInterval` reads `document.visibilityState` directly at tick time rather than through the event. **One claim is deliberately unguarded.** `isLoading` vs `isFetching` for a background poll produces no re-render at all, so there is nothing observable for a test to assert and no user-visible difference to protect. ## Pre-existing failures `task frontend:check` passes typecheck, lint and oxfmt, and 2363 of 2365 editor tests. The two failures, `workbenchSession.test.ts` and `notificationActions.test.tsx`, fail identically with this branch's changes reverted and are untouched by it. ## Scope This is one of five pollers. The remaining four, `useLocalFolderPoller`, `WatchedFolderWorkbenchView`, `SessionDetailPanel` and cloud `TeamSection`, are separate files with their own consumers and follow separately, now that the silent-refresh pattern has a worked example. --------- Co-authored-by: Anthony Stirling <77850077+Frooodle@users.noreply.github.com>
1 parent c22d9ec commit ead8a53

4 files changed

Lines changed: 400 additions & 73 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import apiClient from "@app/services/apiClient";
2+
import type {
3+
SignRequestSummary,
4+
SessionSummary,
5+
} from "@app/types/signingSession";
6+
7+
export interface SigningSessions {
8+
signRequests: SignRequestSummary[];
9+
mySessions: SessionSummary[];
10+
}
11+
12+
/** The two lists the signing UI always needs together. */
13+
export async function fetchSigningSessions(): Promise<SigningSessions> {
14+
const [requests, sessions] = await Promise.all([
15+
apiClient.get<SignRequestSummary[]>(
16+
"/api/v1/security/cert-sign/sign-requests",
17+
),
18+
apiClient.get<SessionSummary[]>("/api/v1/security/cert-sign/sessions"),
19+
]);
20+
return { signRequests: requests.data, mySessions: sessions.data };
21+
}
Lines changed: 319 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,319 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
2+
import { renderHook, waitFor, act } from "@testing-library/react";
3+
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
4+
import type { ReactNode } from "react";
5+
import { baseQueryOptions } from "@app/query/queryClient";
6+
import { TestQueryProvider } from "@app/tests/utils/TestQueryProvider";
7+
import { useSigningSessions } from "@app/hooks/signing/useSigningSessions";
8+
import { fetchSigningSessions } from "@app/api/signing";
9+
import { alert } from "@app/components/toast";
10+
import { expectConsole } from "@app/tests/failOnConsole";
11+
12+
vi.mock("@app/api/signing", () => ({ fetchSigningSessions: vi.fn() }));
13+
vi.mock("@app/components/toast", () => ({ alert: vi.fn() }));
14+
vi.mock("react-i18next", () => ({
15+
useTranslation: () => ({
16+
t: (_k: string, fallback?: string) => fallback ?? _k,
17+
}),
18+
}));
19+
20+
const mockFetch = vi.mocked(fetchSigningSessions);
21+
const mockAlert = vi.mocked(alert);
22+
23+
const EMPTY = { signRequests: [], mySessions: [] };
24+
25+
function setVisibility(state: "visible" | "hidden") {
26+
Object.defineProperty(document, "visibilityState", {
27+
configurable: true,
28+
get: () => state,
29+
});
30+
// Bubbles, as the real event does: query-core listens for it on window.
31+
document.dispatchEvent(new Event("visibilitychange", { bubbles: true }));
32+
}
33+
34+
describe("useSigningSessions", () => {
35+
beforeEach(() => {
36+
vi.clearAllMocks();
37+
mockFetch.mockResolvedValue(EMPTY);
38+
});
39+
40+
afterEach(() => {
41+
vi.useRealTimers();
42+
setVisibility("visible");
43+
});
44+
45+
it("dedupes concurrent observers of the same key", async () => {
46+
const { result } = renderHook(
47+
() => ({
48+
badge: useSigningSessions({
49+
enabled: true,
50+
autoRefreshInterval: 60000,
51+
}),
52+
launcher: useSigningSessions({ enabled: true }),
53+
controller: useSigningSessions({
54+
enabled: true,
55+
autoRefreshInterval: 15000,
56+
}),
57+
}),
58+
{ wrapper: TestQueryProvider },
59+
);
60+
61+
await waitFor(() => expect(result.current.badge.loading).toBe(false));
62+
expect(mockFetch).toHaveBeenCalledTimes(1);
63+
});
64+
65+
it("does not fetch while disabled", async () => {
66+
vi.useFakeTimers();
67+
const { result } = renderHook(
68+
() => useSigningSessions({ enabled: false, autoRefreshInterval: 15000 }),
69+
{ wrapper: TestQueryProvider },
70+
);
71+
72+
expect(mockFetch).not.toHaveBeenCalled();
73+
await act(async () => {
74+
vi.advanceTimersByTime(60000);
75+
});
76+
expect(mockFetch).not.toHaveBeenCalled();
77+
expect(result.current.signRequests).toEqual([]);
78+
});
79+
80+
it("starts fetching when enabled flips on", async () => {
81+
const { result, rerender } = renderHook(
82+
({ on }: { on: boolean }) => useSigningSessions({ enabled: on }),
83+
{ wrapper: TestQueryProvider, initialProps: { on: false } },
84+
);
85+
86+
expect(mockFetch).not.toHaveBeenCalled();
87+
rerender({ on: true });
88+
await waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(1));
89+
await waitFor(() => expect(result.current.loading).toBe(false));
90+
});
91+
92+
it("polls on the interval", async () => {
93+
vi.useFakeTimers();
94+
const { result } = renderHook(
95+
() => useSigningSessions({ enabled: true, autoRefreshInterval: 15000 }),
96+
{ wrapper: TestQueryProvider },
97+
);
98+
99+
expect(result.current.loading).toBe(true);
100+
await act(async () => {
101+
await vi.advanceTimersByTimeAsync(0);
102+
});
103+
expect(mockFetch).toHaveBeenCalledTimes(1);
104+
105+
await act(async () => {
106+
await vi.advanceTimersByTimeAsync(15000);
107+
});
108+
expect(mockFetch).toHaveBeenCalledTimes(2);
109+
});
110+
111+
it("does not raise the spinner while a background poll is in flight", async () => {
112+
// Real timers, a held-open poll, and every render recorded. Asserting on
113+
// result.current alone is not enough: waitFor returns as soon as the fetch
114+
// count moves, before React has re-rendered, so a spinner that did flip on
115+
// would be missed.
116+
const seen: boolean[] = [];
117+
const { result } = renderHook(
118+
() => {
119+
const state = useSigningSessions({
120+
enabled: true,
121+
autoRefreshInterval: 50,
122+
});
123+
seen.push(state.loading);
124+
return state;
125+
},
126+
{ wrapper: TestQueryProvider },
127+
);
128+
await waitFor(() => expect(result.current.loading).toBe(false));
129+
130+
// Marked before the poll: waitFor flushes renders, so recording after it
131+
// would skip straight past the in-flight one.
132+
const fromPollStart = seen.length;
133+
134+
let release: (v: unknown) => void = () => {};
135+
mockFetch.mockReturnValueOnce(
136+
new Promise((resolve) => {
137+
release = resolve;
138+
}) as never,
139+
);
140+
141+
await waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(2));
142+
143+
// Give React room to render the in-flight state, if it produces one.
144+
await act(async () => {
145+
await new Promise((resolve) => setTimeout(resolve, 30));
146+
});
147+
148+
// Mid-poll: this is what the old `silent` flag bought.
149+
expect(seen.slice(fromPollStart)).not.toContain(true);
150+
expect(result.current.loading).toBe(false);
151+
152+
await act(async () => {
153+
release(EMPTY);
154+
});
155+
});
156+
157+
it("shows the spinner for a user-initiated refresh, not a background poll", async () => {
158+
// Real timers: the in-flight window has to be observable, which is exactly
159+
// what a fake-timer act() hides.
160+
const { result } = renderHook(() => useSigningSessions({ enabled: true }), {
161+
wrapper: TestQueryProvider,
162+
});
163+
await waitFor(() => expect(result.current.loading).toBe(false));
164+
165+
let release: (v: unknown) => void = () => {};
166+
mockFetch.mockReturnValueOnce(
167+
new Promise((resolve) => {
168+
release = resolve;
169+
}) as never,
170+
);
171+
172+
let done: Promise<void>;
173+
act(() => {
174+
done = result.current.refetch();
175+
});
176+
await waitFor(() => expect(result.current.loading).toBe(true));
177+
178+
await act(async () => {
179+
release(EMPTY);
180+
await done;
181+
});
182+
expect(result.current.loading).toBe(false);
183+
});
184+
185+
it("toasts a first-load failure", async () => {
186+
expectConsole.error(/Failed to fetch signing data/);
187+
mockFetch.mockRejectedValue(new Error("down"));
188+
189+
const { result } = renderHook(() => useSigningSessions({ enabled: true }), {
190+
wrapper: TestQueryProvider,
191+
});
192+
193+
await waitFor(() => expect(result.current.error).toBeTruthy());
194+
expect(mockAlert).toHaveBeenCalledTimes(1);
195+
});
196+
197+
it("stays silent when a background poll fails after a success", async () => {
198+
vi.useFakeTimers();
199+
mockFetch.mockResolvedValueOnce(EMPTY);
200+
201+
const { result } = renderHook(
202+
() => useSigningSessions({ enabled: true, autoRefreshInterval: 15000 }),
203+
{ wrapper: TestQueryProvider },
204+
);
205+
206+
await act(async () => {
207+
await vi.advanceTimersByTimeAsync(0);
208+
});
209+
expect(result.current.loading).toBe(false);
210+
expect(mockAlert).not.toHaveBeenCalled();
211+
212+
mockFetch.mockRejectedValue(new Error("flaky"));
213+
await act(async () => {
214+
await vi.advanceTimersByTimeAsync(15000);
215+
});
216+
217+
expect(mockFetch).toHaveBeenCalledTimes(2);
218+
expect(mockAlert).not.toHaveBeenCalled();
219+
});
220+
221+
it("toasts an explicit refetch failure even with data on screen", async () => {
222+
expectConsole.error(/Failed to fetch signing data/);
223+
const { result } = renderHook(() => useSigningSessions({ enabled: true }), {
224+
wrapper: TestQueryProvider,
225+
});
226+
await waitFor(() => expect(result.current.loading).toBe(false));
227+
expect(mockAlert).not.toHaveBeenCalled();
228+
229+
mockFetch.mockRejectedValue(new Error("nope"));
230+
await act(async () => {
231+
await result.current.refetch();
232+
});
233+
234+
expect(mockAlert).toHaveBeenCalledTimes(1);
235+
});
236+
237+
it("stops polling while the tab is hidden", async () => {
238+
vi.useFakeTimers();
239+
const { result } = renderHook(
240+
() => useSigningSessions({ enabled: true, autoRefreshInterval: 15000 }),
241+
{ wrapper: TestQueryProvider },
242+
);
243+
244+
await act(async () => {
245+
await vi.advanceTimersByTimeAsync(0);
246+
});
247+
expect(result.current.loading).toBe(false);
248+
expect(mockFetch).toHaveBeenCalledTimes(1);
249+
250+
setVisibility("hidden");
251+
await act(async () => {
252+
await vi.advanceTimersByTimeAsync(60000);
253+
});
254+
// Four intervals elapsed with the tab in the background.
255+
expect(mockFetch).toHaveBeenCalledTimes(1);
256+
257+
setVisibility("visible");
258+
await act(async () => {
259+
await vi.advanceTimersByTimeAsync(15000);
260+
});
261+
expect(mockFetch.mock.calls.length).toBeGreaterThan(1);
262+
});
263+
264+
it("refetches on becoming visible rather than waiting out the interval", async () => {
265+
vi.useFakeTimers();
266+
// The app client turns focus refetching off globally; TestQueryProvider
267+
// does not, and would pass this on the library default alone.
268+
const client = new QueryClient({
269+
defaultOptions: {
270+
queries: { ...baseQueryOptions, retry: false, gcTime: Infinity },
271+
},
272+
});
273+
const { result } = renderHook(
274+
() => useSigningSessions({ enabled: true, autoRefreshInterval: 15000 }),
275+
{
276+
wrapper: ({ children }: { children: ReactNode }) => (
277+
<QueryClientProvider client={client}>{children}</QueryClientProvider>
278+
),
279+
},
280+
);
281+
282+
await act(async () => {
283+
await vi.advanceTimersByTimeAsync(0);
284+
});
285+
expect(result.current.loading).toBe(false);
286+
expect(mockFetch).toHaveBeenCalledTimes(1);
287+
288+
setVisibility("hidden");
289+
await act(async () => {
290+
await vi.advanceTimersByTimeAsync(60000);
291+
});
292+
expect(mockFetch).toHaveBeenCalledTimes(1);
293+
294+
setVisibility("visible");
295+
await act(async () => {
296+
await vi.advanceTimersByTimeAsync(0);
297+
});
298+
expect(mockFetch).toHaveBeenCalledTimes(2);
299+
});
300+
301+
it("stops polling once unmounted", async () => {
302+
vi.useFakeTimers();
303+
const { unmount } = renderHook(
304+
() => useSigningSessions({ enabled: true, autoRefreshInterval: 15000 }),
305+
{ wrapper: TestQueryProvider },
306+
);
307+
308+
await act(async () => {
309+
await vi.advanceTimersByTimeAsync(0);
310+
});
311+
expect(mockFetch).toHaveBeenCalledTimes(1);
312+
313+
unmount();
314+
await act(async () => {
315+
await vi.advanceTimersByTimeAsync(60000);
316+
});
317+
expect(mockFetch).toHaveBeenCalledTimes(1);
318+
});
319+
});

0 commit comments

Comments
 (0)