fix(ui): cancel in-flight settings refetches before seeding the saved values

Explicitly cancel active queries on the settings key before writing the
submitted body into the cache on save, per the standard TanStack Query
optimistic-update recipe, so a settings GET that started before the
PATCH resolved can never land afterwards and restore pre-save values.
The invalidateQueries call that follows already cancelled and restarted
in-flight refetches by default, so the race was not reachable in
practice; the explicit cancel stops relying on that default and both
form suites now pin the invariant with a stale in-flight refetch that
resolves only after the save.
This commit is contained in:
ryan-crabbe-berri 2026-08-14 08:48:56 -07:00
parent ed86414ab5
commit 41fceed6ac
4 changed files with 76 additions and 6 deletions

View file

@ -1,5 +1,5 @@
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { render, screen, waitFor } from "@testing-library/react";
import { act, render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import React from "react";
import { beforeEach, describe, expect, it, vi } from "vitest";
@ -56,7 +56,7 @@ const renderForm = (overrides?: {
</QueryClientProvider>,
);
return { fetchSettings, updateSettings };
return { fetchSettings, updateSettings, queryClient };
};
const saveButton = async () => await screen.findByRole("button", { name: "Save Changes" });
@ -222,6 +222,40 @@ describe("DefaultTeamSettingsForm", () => {
expect(NotificationsManager.success).toHaveBeenCalledWith("Default team settings updated successfully");
});
it("never lets a pre-save in-flight refetch restore the old values after saving", async () => {
const user = userEvent.setup();
const staleRefetch: { resolve: (value: DefaultTeamSettings) => void } = { resolve: () => {} };
const { updateSettings, queryClient } = renderForm({
fetchSettings: vi
.fn()
.mockResolvedValueOnce(SETTINGS)
.mockImplementationOnce(
() =>
new Promise<DefaultTeamSettings>((resolve) => {
staleRefetch.resolve = resolve;
}),
)
.mockImplementation(() => new Promise(() => {})),
});
await enterEditMode(user);
await user.clear(await screen.findByLabelText("Max Budget (USD)"));
await user.type(screen.getByLabelText("Max Budget (USD)"), "250");
void queryClient.refetchQueries({ queryKey: ["defaultTeamSettings"] });
await user.click(await saveButton());
await waitFor(() => expect(updateSettings).toHaveBeenCalledTimes(1));
expect(await screen.findByText("250")).toBeInTheDocument();
await act(async () => {
staleRefetch.resolve(SETTINGS);
await new Promise((resolve) => setTimeout(resolve, 0));
});
expect(screen.getByText("250")).toBeInTheDocument();
expect(screen.queryByText("100")).not.toBeInTheDocument();
});
it("keeps the edit and surfaces the backend error when the save fails", async () => {
const user = userEvent.setup();
const { updateSettings } = renderForm({

View file

@ -148,8 +148,9 @@ const SettingsForm = ({ initialValues, updateSettings, onCancel, onSaved }: Sett
const mutation = useMutation({
mutationFn: (values: DefaultTeamSettingsFormValues) => updateSettings(buildBody(values)),
onSuccess: (_result, values) => {
onSuccess: async (_result, values) => {
NotificationsManager.success("Default team settings updated successfully");
await queryClient.cancelQueries({ queryKey: SETTINGS_QUERY_KEY });
queryClient.setQueryData<DefaultTeamSettings>(SETTINGS_QUERY_KEY, (existing) => ({
field_schema: existing?.field_schema ?? {},
values: buildBody(values),

View file

@ -1,5 +1,5 @@
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { render, screen, waitFor } from "@testing-library/react";
import { act, render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import React from "react";
import { beforeEach, describe, expect, it, vi } from "vitest";
@ -83,7 +83,7 @@ const renderForm = (overrides?: {
</QueryClientProvider>,
);
return { fetchSettings, updateSettings };
return { fetchSettings, updateSettings, queryClient };
};
const saveButton = async () => await screen.findByRole("button", { name: "Save Changes" });
@ -273,6 +273,40 @@ describe("DefaultUserSettingsForm", () => {
expect(await saveButton()).toBeDisabled();
});
it("never lets a pre-save in-flight refetch restore the old values after saving", async () => {
const user = userEvent.setup();
const staleRefetch: { resolve: (value: InternalUserSettings) => void } = { resolve: () => {} };
const { updateSettings, queryClient } = renderForm({
fetchSettings: vi
.fn()
.mockResolvedValueOnce(SETTINGS)
.mockImplementationOnce(
() =>
new Promise<InternalUserSettings>((resolve) => {
staleRefetch.resolve = resolve;
}),
)
.mockImplementation(() => new Promise(() => {})),
});
await enterEditMode(user);
await user.clear(await screen.findByLabelText("Max Budget (USD)"));
await user.type(screen.getByLabelText("Max Budget (USD)"), "250");
void queryClient.refetchQueries({ queryKey: ["internalUserSettings"] });
await user.click(await saveButton());
await waitFor(() => expect(updateSettings).toHaveBeenCalledTimes(1));
expect(await screen.findByText("250")).toBeInTheDocument();
await act(async () => {
staleRefetch.resolve(SETTINGS);
await new Promise((resolve) => setTimeout(resolve, 0));
});
expect(screen.getByText("250")).toBeInTheDocument();
expect(screen.queryByText("100")).not.toBeInTheDocument();
});
it("keeps the edit and surfaces the backend error when the save fails", async () => {
const user = userEvent.setup();
const { updateSettings } = renderForm({

View file

@ -225,8 +225,9 @@ const SettingsForm = ({ initialValues, roleOptions, updateSettings, onCancel, on
const mutation = useMutation({
mutationFn: (values: DefaultUserSettingsFormValues) => updateSettings(buildBody(values)),
onSuccess: (_result, values) => {
onSuccess: async (_result, values) => {
NotificationsManager.success("Default user settings updated successfully");
await queryClient.cancelQueries({ queryKey: SETTINGS_QUERY_KEY });
queryClient.setQueryData<InternalUserSettings>(SETTINGS_QUERY_KEY, (existing) => ({
field_schema: existing?.field_schema ?? {},
values: buildBody(values),