Skip to content

Commit 8650a2e

Browse files
authored
fix: rollback appearance mutation cache updates (coder#25182)
## Summary - Add appearance mutation context so optimistic cache updates can be rolled back on failure. - Restore previous appearance settings, or remove the optimistic cache entry when no previous data exists. - Merge successful server responses back into the appearance cache while preserving request fields when responses are partial. ## Dependencies - Stacked on coder#25076 because the tests and generated types use the new appearance theme mode fields. ## Validation - `pnpm -C site exec vitest run --project=unit src/api/queries/users.test.ts` - `pnpm -C site lint:types` - Pre-commit hook passed on the branch commit.
1 parent 5f9b322 commit 8650a2e

2 files changed

Lines changed: 151 additions & 9 deletions

File tree

site/src/api/queries/users.test.ts

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { QueryClient } from "react-query";
2+
import { describe, expect, it } from "vitest";
3+
import type {
4+
UpdateUserAppearanceSettingsRequest,
5+
UserAppearanceSettings,
6+
} from "#/api/typesGenerated";
7+
import { myAppearanceKey, updateAppearanceSettings } from "./users";
8+
9+
const appearanceSettings = (
10+
overrides: Partial<UserAppearanceSettings> = {},
11+
): UserAppearanceSettings => ({
12+
theme_preference: "dark-tritan",
13+
theme_mode: "sync",
14+
theme_light: "light-tritan",
15+
theme_dark: "dark-tritan",
16+
terminal_font: "geist-mono",
17+
...overrides,
18+
});
19+
20+
const updateRequest = (
21+
overrides: Partial<UpdateUserAppearanceSettingsRequest> = {},
22+
): UpdateUserAppearanceSettingsRequest => ({
23+
theme_preference: "dark",
24+
theme_mode: "single",
25+
theme_light: "light-tritan",
26+
theme_dark: "dark-tritan",
27+
terminal_font: "fira-code",
28+
...overrides,
29+
});
30+
31+
describe("updateAppearanceSettings", () => {
32+
it("rolls back optimistic appearance updates when the mutation fails", async () => {
33+
const queryClient = new QueryClient();
34+
const previousSettings = appearanceSettings({
35+
theme_light: "light-protan-deuter",
36+
theme_dark: "dark-protan-deuter",
37+
});
38+
const optimisticSettings = updateRequest();
39+
40+
queryClient.setQueryData<UserAppearanceSettings>(
41+
myAppearanceKey,
42+
previousSettings,
43+
);
44+
45+
const mutation = updateAppearanceSettings(queryClient);
46+
const context = await mutation.onMutate?.(optimisticSettings);
47+
expect(queryClient.getQueryData(myAppearanceKey)).toEqual(
48+
optimisticSettings,
49+
);
50+
51+
mutation.onError?.(new Error("failed"), optimisticSettings, context);
52+
53+
expect(queryClient.getQueryData(myAppearanceKey)).toEqual(previousSettings);
54+
});
55+
56+
it("removes optimistic appearance data when rollback has no prior cache", async () => {
57+
const queryClient = new QueryClient();
58+
const optimisticSettings = updateRequest();
59+
const mutation = updateAppearanceSettings(queryClient);
60+
61+
const context = await mutation.onMutate?.(optimisticSettings);
62+
expect(queryClient.getQueryData(myAppearanceKey)).toEqual(
63+
optimisticSettings,
64+
);
65+
66+
mutation.onError?.(new Error("failed"), optimisticSettings, context);
67+
68+
expect(queryClient.getQueryData(myAppearanceKey)).toBeUndefined();
69+
});
70+
71+
it("stores the server response after a successful appearance update", async () => {
72+
const queryClient = new QueryClient();
73+
const optimisticSettings = updateRequest();
74+
const serverSettings = appearanceSettings({
75+
theme_preference: "dark-protan-deuter",
76+
theme_light: "light-protan-deuter",
77+
theme_dark: "dark-protan-deuter",
78+
});
79+
const mutation = updateAppearanceSettings(queryClient);
80+
81+
const context = await mutation.onMutate?.(optimisticSettings);
82+
if (!context) {
83+
throw new Error("expected mutation context");
84+
}
85+
expect(queryClient.getQueryData(myAppearanceKey)).toEqual(
86+
optimisticSettings,
87+
);
88+
89+
mutation.onSuccess?.(serverSettings, optimisticSettings, context);
90+
91+
expect(queryClient.getQueryData(myAppearanceKey)).toEqual(serverSettings);
92+
});
93+
94+
it("keeps patch values when a successful appearance update response is partial", async () => {
95+
const queryClient = new QueryClient();
96+
const optimisticSettings = updateRequest({
97+
theme_mode: "sync",
98+
theme_light: "light-protan-deuter",
99+
theme_dark: "dark-protan-deuter",
100+
});
101+
const serverSettings = {
102+
theme_preference: "dark-tritan",
103+
terminal_font: "jetbrains-mono",
104+
} satisfies Partial<UserAppearanceSettings>;
105+
const mutation = updateAppearanceSettings(queryClient);
106+
107+
const context = await mutation.onMutate?.(optimisticSettings);
108+
if (!context) {
109+
throw new Error("expected mutation context");
110+
}
111+
112+
mutation.onSuccess?.(
113+
serverSettings as UserAppearanceSettings,
114+
optimisticSettings,
115+
context,
116+
);
117+
118+
expect(queryClient.getQueryData(myAppearanceKey)).toEqual({
119+
...optimisticSettings,
120+
...serverSettings,
121+
});
122+
});
123+
});

site/src/api/queries/users.ts

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,11 @@ export const updateProfile = (userId: string) => {
262262
};
263263
};
264264

265-
const myAppearanceKey = ["me", "appearance"];
265+
export const myAppearanceKey = ["me", "appearance"] as const;
266+
267+
type AppearanceMutationContext = {
268+
previousAppearanceSettings: UserAppearanceSettings | undefined;
269+
};
266270

267271
export const appearanceSettings = (
268272
metadata: MetadataState<UserAppearanceSettings>,
@@ -280,27 +284,42 @@ export const updateAppearanceSettings = (
280284
UserAppearanceSettings,
281285
unknown,
282286
UpdateUserAppearanceSettingsRequest,
283-
unknown
287+
AppearanceMutationContext
284288
> => {
285289
return {
286290
mutationFn: (req) => API.updateAppearanceSettings(req),
287291
onMutate: async (patch) => {
292+
await queryClient.cancelQueries({ queryKey: myAppearanceKey });
293+
const previousAppearanceSettings =
294+
queryClient.getQueryData<UserAppearanceSettings>(myAppearanceKey);
295+
288296
// Mutate the `queryClient` optimistically to make the theme switcher
289297
// more responsive.
290-
queryClient.setQueryData(myAppearanceKey, {
298+
queryClient.setQueryData<UserAppearanceSettings>(myAppearanceKey, {
291299
theme_preference: patch.theme_preference,
292300
theme_mode: patch.theme_mode,
293301
theme_light: patch.theme_light,
294302
theme_dark: patch.theme_dark,
295303
terminal_font: patch.terminal_font,
296304
});
305+
return { previousAppearanceSettings };
306+
},
307+
onError: (_error, _patch, context) => {
308+
if (context?.previousAppearanceSettings) {
309+
queryClient.setQueryData<UserAppearanceSettings>(
310+
myAppearanceKey,
311+
context.previousAppearanceSettings,
312+
);
313+
return;
314+
}
315+
queryClient.removeQueries({ queryKey: myAppearanceKey, exact: true });
316+
},
317+
onSuccess: (settings, patch) => {
318+
queryClient.setQueryData<UserAppearanceSettings>(myAppearanceKey, {
319+
...patch,
320+
...settings,
321+
});
297322
},
298-
onSuccess: async () =>
299-
// Could technically invalidate more, but we only ever care about the
300-
// `theme_preference` for the `me` query.
301-
await queryClient.invalidateQueries({
302-
queryKey: myAppearanceKey,
303-
}),
304323
};
305324
};
306325

0 commit comments

Comments
 (0)