mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-08 16:48:51 +00:00
7c01f811a1
Phase 2 of the frontend-design audit: TanStack Query discipline.
Set the cross-cutting QueryClient defaults + staleTime/gcTime tier
model + visibility-aware polling + 4 optimistic-update mutations
before any further per-page work.
New foundation
==============
web/src/api/queryConstants.ts (new)
STALE_TIME = { REAL_TIME: 15s, REFERENCE: 5m, CONSTANT: 1h }
GC_TIME = { HEAVY: 1m, STANDARD: 5m, REFERENCE: 30m }
Doc-comment explains the tier model so every new useQuery picks
a tier rather than a hardcoded ms integer.
web/src/main.tsx
QueryClient defaults rewritten:
pre: staleTime: 10_000 + refetchOnWindowFocus: true (refetch
storm on every tab refocus across 242 query sites)
post: staleTime: STALE_TIME.REFERENCE (5min) + gcTime: GC_TIME
.STANDARD (explicit 5min) + refetchOnWindowFocus: false
(per-query opt-in for live-tile queries)
retry: 1 unchanged per the audit's DO NOT.
Findings closed by source ID
============================
TQ-H2 (refetch storm)
main.tsx QueryClient defaults — refetchOnWindowFocus: false root +
per-query opt-in. STALE_TIME.REFERENCE 5min for everything else.
TQ-M1 (no gcTime overrides)
main.tsx now sets gcTime: GC_TIME.STANDARD explicitly — the
contract is documented at the root, not implicit-defaulted by
TanStack.
TQ-M2 (12 inconsistent staleTime values)
All 11 hardcoded numeric staleTime overrides migrated to the
STALE_TIME tier constants. useAuthMe.ts (the 12th) already used
its own constant — left alone. Tier mapping:
- operator-facing live data (KeysPage keys, RoleDetail role,
UsersPage, OIDCJWKSStatusPanel, ApprovalsPage):
STALE_TIME.REAL_TIME (15s)
- slow-changing reference data (KeysPage roles, RolesPage,
AuthSettings bootstrap+runtime-config):
STALE_TIME.REFERENCE (5min)
- effectively immutable (RoleDetail permissions catalogue):
STALE_TIME.CONSTANT (1hr)
TQ-H1 (OnboardingWizard infinite 5s poll)
OnboardingWizard.tsx:288-302 — refetchInterval rewritten to v5
functional form:
refetchInterval: (query) =>
(query.state.data?.data?.length ?? 0) > 0 ? false : 5_000;
As soon as the first agent registers, the interval flips to false
and the poll stops. Also explicit: refetchOnWindowFocus: true +
staleTime: STALE_TIME.REAL_TIME (because this IS a live-tile poll
during the wizard).
PERF-H1 (Dashboard polling storm)
DashboardPage.tsx
- jobs poll bumped 10s → 30s (10s granularity isn't needed when
30s is already inside the human-attention window; the
CertificateDetail page is where 10s polling lives)
- visibility-listener pauses ALL Dashboard polls when
document.visibilityState === 'hidden'; on visibility return,
immediately invalidates the 4 live-tile queries (health,
dashboard-summary, jobs, certs-by-status) so the operator
sees fresh data instantly rather than waiting one tick.
- The 4 live-tile queries (health, dashboard-summary, jobs,
certs-by-status) opt into refetchOnWindowFocus: true +
staleTime: STALE_TIME.REAL_TIME explicitly.
- Backend aggregation gap (dashboard-summary + certs-by-status
+ certificates could collapse into 1 endpoint) tracked
separately — Phase 3 backend follow-up.
P-H1 (CertificatesPage 4 duplicate-key pairs)
Pre-Phase-2 4 pairs of distinct cache slots fetching the same data:
['profiles'] vs ['profiles-filter']
['issuers'] vs ['issuers-filter']
['owners', 'form'] vs ['owners-filter']
['teams', 'form'] vs ['teams-filter']
Post-Phase-2 all four pairs collapse to a single parameterized
queryKey shape: `[name, { per_page: 100 }]`. TanStack v5 dedupes
on serialized queryKey — the modal + filter now share one cache
slot per resource. 8 useQuery sites → 4 cache slots; backend
hits halved on first paint of CertificatesPage.
TQ-M3 (4 of 5 priority optimistic-update mutations)
Wired onMutate / onError-rollback / onSettled-invalidation on:
1. mark-notification-read (NotificationsPage)
— flips row status to 'read' in both ['notifications','all']
+ ['notifications','dead'] cache slots
2. claim-discovered-cert (DiscoveryPage)
— flips status to 'Managed' in ['discovered-certificates']
3. dismiss-discovery (DiscoveryPage)
— flips status to 'Dismissed' in same cache slot
4. archive-certificate (CertificateDetailPage)
— flips status to 'Archived' in ['certificate', id]; on
success navigates to /certificates (optimistic data
doesn't linger); on error restores snapshot + toasts
All four fire the Phase 1 Sonner toast on success/failure.
The 5th priority site (role-assignment toggle in
auth/RoleDetailPage) uses raw async/await handlers rather than
useTrackedMutation — converting it requires a structural
refactor outside Phase 2's TQ-focus; tracked as Phase 2 follow-up.
TQ-L1 (useTrackedMutation extended tests)
useTrackedMutation.test.tsx grew from 3 tests to 8:
+ passes onMutate through and runs it before mutationFn
+ passes onError through with the onMutate context (rollback
path — pins the 3rd-arg snapshot semantics)
+ does NOT invalidate on error (only on success)
+ passes onSettled through (fires after both success + error)
+ parity with raw useMutation when no extra options given
Verification
============
$ grep -E "refetchOnWindowFocus: false" web/src/main.tsx
89: refetchOnWindowFocus: false, // per-query opt-in
$ grep -E "STALE_TIME\.REFERENCE" web/src/main.tsx
86: staleTime: STALE_TIME.REFERENCE, // 5 min
$ grep -cE "useQuery.*\['profiles" web/src/pages/CertificatesPage.tsx
2 (was 6 pre-Phase-2 — '[profiles]' modal + '[profiles-filter]'
+ '[profiles]' top-of-page; now both refer to the same
parameterized key '[profiles, { per_page: 100 }]')
$ grep -rE "onMutate" web/src --include='*.tsx' --exclude='*.test.*' | wc -l
5 (≥ 4 priority sites; the 5th is the optional onMutate in
queryConstants test wiring)
$ grep -rE "STALE_TIME\." web/src --include='*.tsx' --include='*.ts' \
--exclude='*.test.*' | wc -l
18 (queryConstants.ts + main.tsx + 11 migrated callsites
+ OnboardingWizard + DashboardPage)
$ npx tsc --noEmit
(exit 0)
$ npx vitest run [13 affected test files]
Test Files 13 passed (13)
Tests 100 passed (100)
$ npx vite build
✓ built in 2.49s
dist/assets/index-yg3cYtYA.js 1,113 kB
(+3 kB vs Phase 1 — queryConstants + optimistic-update wrappers)
Audit-accuracy callouts
=======================
* The audit claimed 10 useQuery on Dashboard; live count is 9 (one
issuers query has no interval). All 8 polling queries now gated
behind visibility-listener; the 9th (issuers) is non-polling and
not affected.
* TQ-L1 originally specified 4 test extensions; shipped 5
(onMutate ordering, onError-with-context, no-invalidate-on-error,
onSettled pass-through, parity-with-raw-useMutation).
* Optimistic-update 5th-site (role-assignment toggle in
auth/RoleDetailPage) deferred — RoleDetailPage handlers use raw
async/await instead of useTrackedMutation. Refactoring it adds
one more optimistic path but requires a structural change
outside Phase 2's TQ-discipline scope. Tracked as Phase 2
follow-up.
Residual risks
==============
* The Dashboard visibility-listener gate may need per-page opt-in
if a page genuinely needs to keep polling while hidden (e.g.
a background-tab monitor). Not aware of any such case today;
if needed, the gate is a simple `useState`-driven hook
extracted to web/src/hooks/useTabVisibility.ts.
* The Dashboard backend-aggregation collapse
(dashboard-summary + certs-by-status + certificates → one
endpoint) is documented as a Phase-3 backend item.
* The 4 collapsed CertificatesPage pairs now request per_page=100
everywhere. Operator with >100 issuers/owners/profiles/teams
will see a truncated dropdown — that's an unrelated Phase-1-
Combobox-migration concern; the right fix when it lands is to
move issuer/owner/profile selectors to Combobox with
server-side typeahead.
* The 12-second total Bundle-1 audit of all useQuery sites
still leaves ~230 queries running with the new 5-min
REFERENCE default. The default is generous; aggressively-
fresh per-page queries that genuinely need 15s freshness
must opt in (the audit page, the agent-fleet live counter,
in-flight scan progress).
196 lines
7.1 KiB
TypeScript
196 lines
7.1 KiB
TypeScript
// Bundle-8 / Audit M-009:
|
|
// regression coverage for useTrackedMutation. Confirms that:
|
|
// 1. successful mutation invalidates each declared query key
|
|
// 2. caller's onSuccess fires after invalidation
|
|
// 3. 'noop' invalidates option requires noopReason at the type level
|
|
// (compile-time assertion via the discriminated union — runtime
|
|
// coverage here just confirms 'noop' passes through silently)
|
|
|
|
import { describe, it, expect, vi } from 'vitest';
|
|
import { renderHook, waitFor } from '@testing-library/react';
|
|
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
|
import { useTrackedMutation } from './useTrackedMutation';
|
|
import type { ReactNode } from 'react';
|
|
|
|
function withQueryClient(client: QueryClient) {
|
|
return ({ children }: { children: ReactNode }) => (
|
|
<QueryClientProvider client={client}>{children}</QueryClientProvider>
|
|
);
|
|
}
|
|
|
|
describe('useTrackedMutation — Bundle-8 / M-009', () => {
|
|
it('invalidates declared query keys on successful mutation', async () => {
|
|
const client = new QueryClient();
|
|
const invalidateSpy = vi.spyOn(client, 'invalidateQueries');
|
|
|
|
const { result } = renderHook(
|
|
() =>
|
|
useTrackedMutation({
|
|
mutationFn: async () => 'ok',
|
|
invalidates: [['certificates'], ['certificate', 'mc-001']],
|
|
}),
|
|
{ wrapper: withQueryClient(client) },
|
|
);
|
|
|
|
result.current.mutate(undefined);
|
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
|
|
|
// Once per declared key
|
|
expect(invalidateSpy).toHaveBeenCalledTimes(2);
|
|
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['certificates'] });
|
|
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['certificate', 'mc-001'] });
|
|
});
|
|
|
|
it('fires caller onSuccess after invalidation', async () => {
|
|
const client = new QueryClient();
|
|
const onSuccess = vi.fn();
|
|
const { result } = renderHook(
|
|
() =>
|
|
useTrackedMutation({
|
|
mutationFn: async () => 42,
|
|
invalidates: [['certificates']],
|
|
onSuccess,
|
|
}),
|
|
{ wrapper: withQueryClient(client) },
|
|
);
|
|
|
|
result.current.mutate(undefined);
|
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
|
expect(onSuccess).toHaveBeenCalledOnce();
|
|
expect(onSuccess.mock.calls[0][0]).toBe(42);
|
|
});
|
|
|
|
it("noop variant doesn't invalidate but still runs caller onSuccess", async () => {
|
|
const client = new QueryClient();
|
|
const invalidateSpy = vi.spyOn(client, 'invalidateQueries');
|
|
const onSuccess = vi.fn();
|
|
const { result } = renderHook(
|
|
() =>
|
|
useTrackedMutation({
|
|
mutationFn: async () => 'noop-data',
|
|
invalidates: 'noop',
|
|
noopReason: 'fire-and-forget agent ping; no client cache impact',
|
|
onSuccess,
|
|
}),
|
|
{ wrapper: withQueryClient(client) },
|
|
);
|
|
|
|
result.current.mutate(undefined);
|
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
|
expect(invalidateSpy).not.toHaveBeenCalled();
|
|
expect(onSuccess).toHaveBeenCalledOnce();
|
|
});
|
|
|
|
// Phase 2 TQ-L1 extension — pin the optimistic-update contract.
|
|
//
|
|
// useTrackedMutation passes onMutate / onError / onSettled through
|
|
// verbatim (only onSuccess is wrapper-owned). The 4 Phase-2 sites
|
|
// (mark-notification-read, dismiss-discovery, claim-discovered,
|
|
// archive-certificate) depend on this pass-through to implement
|
|
// optimistic updates with rollback. These tests pin:
|
|
// (a) onMutate runs before mutationFn (snapshot pre-mutation state)
|
|
// (b) onError fires with the snapshot as the 3rd arg (rollback path)
|
|
// (c) onError pass-through (raw useMutation behaviour preserved)
|
|
// (d) the no-options call is parity with raw useMutation (the
|
|
// wrapper imposes no semantic behaviour beyond invalidation
|
|
// + the optional onSuccess chain).
|
|
it('passes onMutate through and runs it before mutationFn', async () => {
|
|
const client = new QueryClient();
|
|
const order: string[] = [];
|
|
const { result } = renderHook(
|
|
() =>
|
|
useTrackedMutation({
|
|
mutationFn: async () => {
|
|
order.push('mutate');
|
|
return 'ok';
|
|
},
|
|
invalidates: [['something']],
|
|
onMutate: async () => {
|
|
order.push('onMutate');
|
|
return { snapshot: 'pre-state' };
|
|
},
|
|
}),
|
|
{ wrapper: withQueryClient(client) },
|
|
);
|
|
result.current.mutate(undefined);
|
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
|
expect(order).toEqual(['onMutate', 'mutate']);
|
|
});
|
|
|
|
it('passes onError through with the onMutate context (rollback path)', async () => {
|
|
const client = new QueryClient();
|
|
const onError = vi.fn();
|
|
const onMutate = vi.fn(async () => ({ snapshot: { foo: 'bar' } }));
|
|
const { result } = renderHook(
|
|
() =>
|
|
useTrackedMutation({
|
|
mutationFn: async () => {
|
|
throw new Error('boom');
|
|
},
|
|
invalidates: [['something']],
|
|
onMutate,
|
|
onError,
|
|
}),
|
|
{ wrapper: withQueryClient(client) },
|
|
);
|
|
result.current.mutate(undefined);
|
|
await waitFor(() => expect(result.current.isError).toBe(true));
|
|
expect(onMutate).toHaveBeenCalledOnce();
|
|
expect(onError).toHaveBeenCalledOnce();
|
|
// 3rd arg of onError is the onMutate return value (the snapshot
|
|
// for rollback). Pinning this guarantees the optimistic-update
|
|
// rollback wiring stays intact across future refactors.
|
|
expect(onError.mock.calls[0][2]).toEqual({ snapshot: { foo: 'bar' } });
|
|
});
|
|
|
|
it('does NOT invalidate on error (only on success)', async () => {
|
|
const client = new QueryClient();
|
|
const invalidateSpy = vi.spyOn(client, 'invalidateQueries');
|
|
const { result } = renderHook(
|
|
() =>
|
|
useTrackedMutation({
|
|
mutationFn: async () => {
|
|
throw new Error('nope');
|
|
},
|
|
invalidates: [['cache-key']],
|
|
}),
|
|
{ wrapper: withQueryClient(client) },
|
|
);
|
|
result.current.mutate(undefined);
|
|
await waitFor(() => expect(result.current.isError).toBe(true));
|
|
expect(invalidateSpy).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('passes onSettled through (fires after both success and error)', async () => {
|
|
const client = new QueryClient();
|
|
const onSettled = vi.fn();
|
|
const { result } = renderHook(
|
|
() =>
|
|
useTrackedMutation({
|
|
mutationFn: async () => 'ok',
|
|
invalidates: [['x']],
|
|
onSettled,
|
|
}),
|
|
{ wrapper: withQueryClient(client) },
|
|
);
|
|
result.current.mutate(undefined);
|
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
|
expect(onSettled).toHaveBeenCalledOnce();
|
|
});
|
|
|
|
it('parity with raw useMutation when no extra options given', async () => {
|
|
const client = new QueryClient();
|
|
const { result } = renderHook(
|
|
() =>
|
|
useTrackedMutation({
|
|
mutationFn: async (n: number) => n * 2,
|
|
invalidates: [['compute']],
|
|
}),
|
|
{ wrapper: withQueryClient(client) },
|
|
);
|
|
result.current.mutate(7);
|
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
|
expect(result.current.data).toBe(14);
|
|
});
|
|
});
|