diff --git a/src/backend/database/routes/ui-preferences.ts b/src/backend/database/routes/ui-preferences.ts index 6360fc0e0..9f593dc04 100644 --- a/src/backend/database/routes/ui-preferences.ts +++ b/src/backend/database/routes/ui-preferences.ts @@ -21,13 +21,12 @@ const authenticateJWT = authManager.createAuthMiddleware(); const NEW_ACCOUNT_WINDOW_MS = 24 * 60 * 60 * 1000; /** - * Existing users must never be ambushed by onboarding. A user with no - * ui_preferences row who registered more than a day ago predates this feature, - * so they are handed a completed onboarding state. This is a read-time default - * rather than a migration write: nothing is persisted until the user actually - * changes something, so it stays correct on a fresh database too. + * Existing users must never be ambushed by onboarding. A user who never + * finished it and registered more than a day ago predates this feature, so + * they are handed a completed onboarding state. This also repairs rows that an + * earlier PUT wrote with the not-completed default. */ -function withOnboardingBackfill( +export function withOnboardingBackfill( preferences: UiPreferences, registeredAt: string | null | undefined, ): UiPreferences { @@ -36,7 +35,7 @@ function withOnboardingBackfill( Number.isFinite(registeredMs) && Date.now() - registeredMs < NEW_ACCOUNT_WINDOW_MS; - if (isNewAccount) return preferences; + if (isNewAccount || preferences.onboarding.completedAt) return preferences; return { ...preferences, @@ -65,18 +64,16 @@ router.get("/", authenticateJWT, async (req: Request, res: Response) => { const existing = await createCurrentUiPreferenceRepository().findByUserId(userId); - if (existing) { - return res.json({ - preferences: sanitizeUiPreferences(JSON.parse(existing.data)), - }); + const stored = existing + ? sanitizeUiPreferences(JSON.parse(existing.data)) + : defaultUiPreferences(); + if (stored.onboarding.completedAt) { + return res.json({ preferences: stored }); } const user = await createCurrentUserRepository().findById(userId); return res.json({ - preferences: withOnboardingBackfill( - defaultUiPreferences(), - user?.registeredAt, - ), + preferences: withOnboardingBackfill(stored, user?.registeredAt), }); } catch (e) { databaseLogger.error("Failed to get UI preferences", e, { @@ -117,9 +114,15 @@ router.put("/", authenticateJWT, async (req: Request, res: Response) => { try { const repository = createCurrentUiPreferenceRepository(); const existing = await repository.findByUserId(userId); - const base = existing + const stored = existing ? sanitizeUiPreferences(JSON.parse(existing.data)) : defaultUiPreferences(); + const base = stored.onboarding.completedAt + ? stored + : withOnboardingBackfill( + stored, + (await createCurrentUserRepository().findById(userId))?.registeredAt, + ); const body = req.body as Record; diff --git a/src/backend/tests/database/routes/ui-preferences.test.ts b/src/backend/tests/database/routes/ui-preferences.test.ts new file mode 100644 index 000000000..a9d84a469 --- /dev/null +++ b/src/backend/tests/database/routes/ui-preferences.test.ts @@ -0,0 +1,128 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import http from "node:http"; +import type { AddressInfo } from "node:net"; +import express from "express"; +import { UI_ONBOARDING_VERSION } from "../../../../types/ui-preferences.js"; + +const state = vi.hoisted(() => ({ + row: null as { data: string } | null, + registeredAt: null as string | null, +})); + +vi.mock("../../../utils/logger.js", () => ({ + databaseLogger: { warn: vi.fn(), error: vi.fn(), info: vi.fn() }, +})); + +vi.mock("../../../utils/auth-manager.js", () => ({ + AuthManager: { + getInstance: () => ({ + createAuthMiddleware: + () => (req: express.Request, _res: unknown, next: () => void) => { + (req as unknown as { userId: string }).userId = "user-1"; + next(); + }, + }), + }, +})); + +vi.mock("../../../database/repositories/factory.js", () => ({ + createCurrentUiPreferenceRepository: () => ({ + findByUserId: async () => state.row, + upsert: async (_userId: string, data: string) => { + state.row = { data }; + }, + }), + createCurrentUserRepository: () => ({ + findById: async () => ({ registeredAt: state.registeredAt }), + }), +})); + +const { default: uiPreferenceRoutes } = + await import("../../../database/routes/ui-preferences.js"); + +let server: http.Server; +let baseUrl: string; + +const OLD = new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(); +const NEW = new Date().toISOString(); + +async function put(body: unknown) { + const res = await fetch(`${baseUrl}/ui-preferences`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + return res.json(); +} + +async function get() { + const res = await fetch(`${baseUrl}/ui-preferences`); + return (await res.json()).preferences; +} + +beforeEach(async () => { + state.row = null; + state.registeredAt = OLD; + + const app = express(); + app.use(express.json()); + app.use("/ui-preferences", uiPreferenceRoutes); + + server = http.createServer(app); + await new Promise((resolve) => server.listen(0, resolve)); + const { port } = server.address() as AddressInfo; + baseUrl = `http://127.0.0.1:${port}`; +}); + +afterEach(async () => { + await new Promise((resolve) => server.close(() => resolve())); +}); + +describe("ui-preferences onboarding backfill", () => { + it("treats an old account with no row as onboarded", async () => { + expect((await get()).onboarding.completedVersion).toBe( + UI_ONBOARDING_VERSION, + ); + }); + + it("keeps an old account onboarded after its first write", async () => { + await put({ overrides: { "plugin:x": { a: 1 } } }); + const stored = JSON.parse(state.row!.data); + expect(stored.onboarding.completedVersion).toBe(UI_ONBOARDING_VERSION); + expect((await get()).onboarding.completedVersion).toBe( + UI_ONBOARDING_VERSION, + ); + }); + + it("repairs a row an earlier write stored as not completed", async () => { + state.row = { + data: JSON.stringify({ + onboarding: { completedVersion: 0, completedAt: null, skipped: false }, + }), + }; + expect((await get()).onboarding.completedVersion).toBe( + UI_ONBOARDING_VERSION, + ); + }); + + it("still shows onboarding to a new account", async () => { + state.registeredAt = NEW; + await put({ preset: "simple" }); + expect((await get()).onboarding.completedVersion).toBe(0); + }); + + it("does not hide a newer onboarding version from someone who finished an older one", async () => { + state.row = { + data: JSON.stringify({ + onboarding: { + completedVersion: UI_ONBOARDING_VERSION - 1, + completedAt: OLD, + skipped: false, + }, + }), + }; + expect((await get()).onboarding.completedVersion).toBe( + UI_ONBOARDING_VERSION - 1, + ); + }); +}); diff --git a/src/ui/contexts/UiPreferencesContext.tsx b/src/ui/contexts/UiPreferencesContext.tsx index 1adc5228f..1ee00878b 100644 --- a/src/ui/contexts/UiPreferencesContext.tsx +++ b/src/ui/contexts/UiPreferencesContext.tsx @@ -31,7 +31,6 @@ import { const SAVE_DEBOUNCE_MS = 500; const LS_KEY = "uiPreferences"; -const SYNC_EVENT = "uiPreferencesChanged"; function readCache(): UiPreferences | null { try { @@ -115,15 +114,6 @@ export function UiPreferencesProvider({ children }: { children: ReactNode }) { }; }, []); - useEffect(() => { - const handler = () => { - const cached = readCache(); - if (cached) setPreferences(cached); - }; - window.addEventListener(SYNC_EVENT, handler); - return () => window.removeEventListener(SYNC_EVENT, handler); - }, []); - useEffect(() => { return () => { if (saveTimer.current) clearTimeout(saveTimer.current); @@ -152,8 +142,13 @@ export function UiPreferencesProvider({ children }: { children: ReactNode }) { pendingPatch.current = {}; getUserPreferences() .then((prefs) => { - if (prefs.storageMode !== "cloud") return; - return saveUiPreferences(body); + if (prefs.storageMode === "cloud") return saveUiPreferences(body); + // The server copy replaces the cache on load, so onboarding has to + // reach it even in local mode or it reruns every time. + if (body.onboarding) + return saveUiPreferences({ + onboarding: body.onboarding as UiPreferences["onboarding"], + }); }) .catch(() => { /* best-effort; cache already holds it */ @@ -170,7 +165,6 @@ export function UiPreferencesProvider({ children }: { children: ReactNode }) { setPreferences((prev) => { const next = sanitizeUiPreferences(mutate(prev)); writeCache(next); - window.dispatchEvent(new Event(SYNC_EVENT)); return next; }); queueSave(patch);