fix(vendors): bypass the query cache when reloading after a conflict (SH-250)

The reload fix re-seeded the form from queryClient.fetchQuery, which inherits the
global 5-minute staleTime. A 409 never invalidates that key -
useSaveVendorCompanyRoster invalidates only in onSuccess and its onError returns
early for conflicts - and the Add flow has no mounted observer on it, so reload
returned the cached roster with the same stale rowVersion and every retry 409'd
again. Update mode was unaffected because it recovers via query.refetch(), which
bypasses staleTime.

Pass staleTime: 0 on that fetch, since its sole purpose is the freshest
rowVersion. The regression test uses a client with the app's real staleTime; the
default test client uses 0, which masked this entirely.
This commit is contained in:
Codex Review Integration 2026-08-19 10:26:28 -03:00
parent 0de695362a
commit 1305bf35dd
2 changed files with 53 additions and 0 deletions

View file

@ -87,6 +87,13 @@ export function useVendorRosterSelection({
queryClient.fetchQuery({
queryKey: queryKeys.vendors.roster("companyId", id),
queryFn: () => vendorCompanyRosterApi.get({ companyId: id }),
// SH-250: this fetch exists to obtain the freshest rowVersion, so it must not
// serve the global 5-minute staleTime cache. A conflict never invalidates this
// key (useSaveVendorCompanyRoster invalidates only on success, and its onError
// returns early for conflicts), and the Add flow has no mounted observer on it.
// Without staleTime: 0 the Reload button re-seeds the same stale rowVersion and
// the retry 409s again indefinitely.
staleTime: 0,
}),
[queryClient],
);

View file

@ -156,6 +156,17 @@ function createClient(): QueryClient {
return new QueryClient({ defaultOptions: { queries: { retry: false } } });
}
/**
* SH-250: mirrors the app's real staleTime (src/lib/query/query-client.ts). The default
* test client uses staleTime 0, which silently masks cache-staleness defects on the
* reload path — the conflict recovery must not depend on an empty cache.
*/
function createCachingClient(): QueryClient {
return new QueryClient({
defaultOptions: { queries: { retry: false, staleTime: 5 * 60 * 1000 } },
});
}
function makeWrapper(client: QueryClient) {
return function Wrapper({ children }: { children: ReactNode }) {
return <QueryClientProvider client={client}>{children}</QueryClientProvider>;
@ -534,6 +545,41 @@ describe("useVendorRosterForm additive add flow (SH-250)", () => {
await waitFor(() => expect(result.current.name.field.value).toBe("Gateway Plumbing & Drain"));
});
it("reload bypasses the cache so the retry carries the fresh rowVersion", async () => {
rosterGet.mockResolvedValueOnce(roster);
const { result } = renderHook(
() => useVendorRosterForm({ mode: "create", startWithTechnician: true }),
{ wrapper: makeWrapper(createCachingClient()) },
);
await act(async () => {
await result.current.selectCompany(company);
});
expect(rosterGet).toHaveBeenCalledTimes(1);
act(() => {
result.current.submit(submitValues([newTechnician]));
});
expect(saveMutate.mock.calls[0]?.[0]).toEqual(expect.objectContaining({ rowVersion: "rv-1" }));
// The 409 never invalidates this key, and in the Add flow nothing else observes it.
// Without staleTime: 0 on the reload fetch, the cached rv-1 is served straight back
// and every retry 409s again.
rosterGet.mockResolvedValueOnce({ ...roster, rowVersion: "rv-2" });
await act(async () => {
result.current.reload();
});
await waitFor(() => expect(rosterGet).toHaveBeenCalledTimes(2));
act(() => {
result.current.submit(submitValues([newTechnician]));
});
expect(saveMutate.mock.calls[1]?.[0]).toEqual(
expect.objectContaining({ mode: "add", rowVersion: "rv-2" }),
);
});
it("does not regress the Edit Vendor reconcile path (mode update)", () => {
rosterQueryResult = {
data: rosterWithTechnicians,