From 8650a2ee22e9dbce5bff21d57d9bf1b7decdc1ab Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 15 May 2026 16:50:29 +0700 Subject: [PATCH] fix: rollback appearance mutation cache updates (#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 #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. --- site/src/api/queries/users.test.ts | 123 +++++++++++++++++++++++++++++ site/src/api/queries/users.ts | 37 ++++++--- 2 files changed, 151 insertions(+), 9 deletions(-) create mode 100644 site/src/api/queries/users.test.ts diff --git a/site/src/api/queries/users.test.ts b/site/src/api/queries/users.test.ts new file mode 100644 index 0000000000..9566b2d228 --- /dev/null +++ b/site/src/api/queries/users.test.ts @@ -0,0 +1,123 @@ +import { QueryClient } from "react-query"; +import { describe, expect, it } from "vitest"; +import type { + UpdateUserAppearanceSettingsRequest, + UserAppearanceSettings, +} from "#/api/typesGenerated"; +import { myAppearanceKey, updateAppearanceSettings } from "./users"; + +const appearanceSettings = ( + overrides: Partial = {}, +): UserAppearanceSettings => ({ + theme_preference: "dark-tritan", + theme_mode: "sync", + theme_light: "light-tritan", + theme_dark: "dark-tritan", + terminal_font: "geist-mono", + ...overrides, +}); + +const updateRequest = ( + overrides: Partial = {}, +): UpdateUserAppearanceSettingsRequest => ({ + theme_preference: "dark", + theme_mode: "single", + theme_light: "light-tritan", + theme_dark: "dark-tritan", + terminal_font: "fira-code", + ...overrides, +}); + +describe("updateAppearanceSettings", () => { + it("rolls back optimistic appearance updates when the mutation fails", async () => { + const queryClient = new QueryClient(); + const previousSettings = appearanceSettings({ + theme_light: "light-protan-deuter", + theme_dark: "dark-protan-deuter", + }); + const optimisticSettings = updateRequest(); + + queryClient.setQueryData( + myAppearanceKey, + previousSettings, + ); + + const mutation = updateAppearanceSettings(queryClient); + const context = await mutation.onMutate?.(optimisticSettings); + expect(queryClient.getQueryData(myAppearanceKey)).toEqual( + optimisticSettings, + ); + + mutation.onError?.(new Error("failed"), optimisticSettings, context); + + expect(queryClient.getQueryData(myAppearanceKey)).toEqual(previousSettings); + }); + + it("removes optimistic appearance data when rollback has no prior cache", async () => { + const queryClient = new QueryClient(); + const optimisticSettings = updateRequest(); + const mutation = updateAppearanceSettings(queryClient); + + const context = await mutation.onMutate?.(optimisticSettings); + expect(queryClient.getQueryData(myAppearanceKey)).toEqual( + optimisticSettings, + ); + + mutation.onError?.(new Error("failed"), optimisticSettings, context); + + expect(queryClient.getQueryData(myAppearanceKey)).toBeUndefined(); + }); + + it("stores the server response after a successful appearance update", async () => { + const queryClient = new QueryClient(); + const optimisticSettings = updateRequest(); + const serverSettings = appearanceSettings({ + theme_preference: "dark-protan-deuter", + theme_light: "light-protan-deuter", + theme_dark: "dark-protan-deuter", + }); + const mutation = updateAppearanceSettings(queryClient); + + const context = await mutation.onMutate?.(optimisticSettings); + if (!context) { + throw new Error("expected mutation context"); + } + expect(queryClient.getQueryData(myAppearanceKey)).toEqual( + optimisticSettings, + ); + + mutation.onSuccess?.(serverSettings, optimisticSettings, context); + + expect(queryClient.getQueryData(myAppearanceKey)).toEqual(serverSettings); + }); + + it("keeps patch values when a successful appearance update response is partial", async () => { + const queryClient = new QueryClient(); + const optimisticSettings = updateRequest({ + theme_mode: "sync", + theme_light: "light-protan-deuter", + theme_dark: "dark-protan-deuter", + }); + const serverSettings = { + theme_preference: "dark-tritan", + terminal_font: "jetbrains-mono", + } satisfies Partial; + const mutation = updateAppearanceSettings(queryClient); + + const context = await mutation.onMutate?.(optimisticSettings); + if (!context) { + throw new Error("expected mutation context"); + } + + mutation.onSuccess?.( + serverSettings as UserAppearanceSettings, + optimisticSettings, + context, + ); + + expect(queryClient.getQueryData(myAppearanceKey)).toEqual({ + ...optimisticSettings, + ...serverSettings, + }); + }); +}); diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index 3c35e072cb..d2dd38adc1 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -262,7 +262,11 @@ export const updateProfile = (userId: string) => { }; }; -const myAppearanceKey = ["me", "appearance"]; +export const myAppearanceKey = ["me", "appearance"] as const; + +type AppearanceMutationContext = { + previousAppearanceSettings: UserAppearanceSettings | undefined; +}; export const appearanceSettings = ( metadata: MetadataState, @@ -280,27 +284,42 @@ export const updateAppearanceSettings = ( UserAppearanceSettings, unknown, UpdateUserAppearanceSettingsRequest, - unknown + AppearanceMutationContext > => { return { mutationFn: (req) => API.updateAppearanceSettings(req), onMutate: async (patch) => { + await queryClient.cancelQueries({ queryKey: myAppearanceKey }); + const previousAppearanceSettings = + queryClient.getQueryData(myAppearanceKey); + // Mutate the `queryClient` optimistically to make the theme switcher // more responsive. - queryClient.setQueryData(myAppearanceKey, { + queryClient.setQueryData(myAppearanceKey, { theme_preference: patch.theme_preference, theme_mode: patch.theme_mode, theme_light: patch.theme_light, theme_dark: patch.theme_dark, terminal_font: patch.terminal_font, }); + return { previousAppearanceSettings }; + }, + onError: (_error, _patch, context) => { + if (context?.previousAppearanceSettings) { + queryClient.setQueryData( + myAppearanceKey, + context.previousAppearanceSettings, + ); + return; + } + queryClient.removeQueries({ queryKey: myAppearanceKey, exact: true }); + }, + onSuccess: (settings, patch) => { + queryClient.setQueryData(myAppearanceKey, { + ...patch, + ...settings, + }); }, - onSuccess: async () => - // Could technically invalidate more, but we only ever care about the - // `theme_preference` for the `me` query. - await queryClient.invalidateQueries({ - queryKey: myAppearanceKey, - }), }; };