nicktrn
b082e44389
Fixes found while reviewing #4547, stacked on that branch so they can be reviewed on their own and merged into it. One commit per fix. ## Write-path correctness **Refuse account writes while impersonating.** The five `dashboardPreferences` writers already no-op for an impersonating admin, but the three profile writers added next to them did not, and `requireUserId` returns the impersonated user's id. Both gates now refuse up front and say so, rather than the preference writers silently no-opping while the page reports success. **Preserve unknown keys on a full-blob write.** `mutateDashboardPreferences` parses the JSON column, hands the result to a mutator and persists the whole object back. zod strips keys it does not declare, so a deploy that predates a preference field drops it on the next write through that path — and `updateCurrentProjectEnvironmentId` sits on the navigation hot path. `preserveUnknownKeys` re-attaches them at the write. Note this cannot help deploys already running, so it makes this the last release able to strip rather than retroactively protecting the fields added in #4547. **Scope hidden-sidebar writes to what was shown.** The customize dialog builds its hidden map from the sections it can see and the write replaced `hiddenItems` wholesale. The profile page has no org in scope, so it resolves sections from the most-recently-updated project's org: confirming there dropped hidden ids belonging to sections that org's flags exclude. The payload now carries the ids the dialog rendered and the write only replaces those. Submissions without the list stay authoritative. **Consider both addresses when checking email ownership.** The check only looked at the address the user already had; it now considers the current and submitted address together, so an org managing either one governs the change. Validation moved ahead of the check, and `emailDomainOf` splits on the last `@`. ## Interaction **Revert unsaved themes, debounce contrast saves.** The theme and system-theme selects stamp `data-theme` before the write lands. When it fails, the loader returns the value it always had — so `useSystemThemeSync`'s effect deps are unchanged and React's vdom diff sees no change either, and nothing rewrites the attribute. The page kept rendering a theme that was never stored while the select showed the stored one. The stored pair is now re-applied explicitly, as the side menu's switcher already did. The contrast slider is debounced because Radix commits on every arrow keypress, so a keyboard user crossing the range fired one write per step. **Tick More options for themes outside the short list.** The appearance submenu offers System, Light and Dark; Black and White live on the profile page. With one of those stored, every row read as unselected. ## Subtraction **Drop the profile update rate limiter.** It covered one of four paths that write the same column — `resources.preferences.sidemenu` and `.favorites` take unlimited authenticated writes and go through the locked read-modify-write, which is more expensive than the single narrow `jsonb_set` this capped. It was also what made the contrast slider unusable by keyboard. If preference writes want limiting, it belongs in one place covering all of them. **Resolve email ownership when the dialog opens.** It fans out one SSO status lookup per organization the user belongs to and ran in the profile loader on every page view, purely to pick which body the dialog renders. The action re-derives it before writing either way, so the check that guards the write now has one call site instead of two. ## Testing `typecheck --filter webapp` and `lint` clean. New unit tests for `preserveUnknownKeys`, `mergeHiddenItems` and `emailDomainOf`; `themePreference`, `mergeHiddenItems` and `ssoManagedIdentity` suites pass locally (26 tests). The rest of the webapp suite needs testcontainers and is left to CI. No changeset or `.server-changes` entry: everything here fixes code on the parent branch that has not shipped. The one exception worth a maintainer's call is `mergeHiddenItems`, which also touches the side menu's own customize path.
90 lines
3.4 KiB
TypeScript
90 lines
3.4 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
|
import { parseDashboardPreferences, preserveUnknownKeys } from "~/utils/dashboardPreferences";
|
|
import { normalizeThemePreference, type ThemePreference } from "~/utils/themePreference";
|
|
|
|
const VALID_THEMES: ThemePreference[] = ["system", "dark", "light", "black", "white"];
|
|
|
|
describe("normalizeThemePreference", () => {
|
|
it("returns each valid value unchanged", () => {
|
|
for (const theme of VALID_THEMES) {
|
|
expect(normalizeThemePreference(theme)).toBe(theme);
|
|
}
|
|
});
|
|
|
|
it("falls back to dark for legacy/unknown values", () => {
|
|
// Classic is retired. Anyone still holding it lands on Dark, which at
|
|
// contrast 0 renders the palette Classic used to ship.
|
|
expect(normalizeThemePreference("classic")).toBe("dark");
|
|
expect(normalizeThemePreference("solarized")).toBe("dark");
|
|
expect(normalizeThemePreference("")).toBe("dark");
|
|
expect(normalizeThemePreference(42)).toBe("dark");
|
|
expect(normalizeThemePreference(null)).toBe("dark");
|
|
});
|
|
|
|
it("falls back to dark for undefined", () => {
|
|
expect(normalizeThemePreference(undefined)).toBe("dark");
|
|
});
|
|
});
|
|
|
|
describe("DashboardPreferences theme schema", () => {
|
|
it("accepts every theme value", () => {
|
|
for (const theme of VALID_THEMES) {
|
|
const result = parseDashboardPreferences({ version: "1", projects: {}, theme });
|
|
expect(result.theme).toBe(theme);
|
|
}
|
|
});
|
|
|
|
it("drops a stored classic theme", () => {
|
|
const result = parseDashboardPreferences({ version: "1", projects: {}, theme: "classic" });
|
|
expect(result.theme).toBeUndefined();
|
|
});
|
|
|
|
it("accepts preferences without a theme", () => {
|
|
const result = parseDashboardPreferences({ version: "1", projects: {} });
|
|
expect(result.theme).toBeUndefined();
|
|
});
|
|
|
|
it("drops an invalid theme without erasing the rest of the preferences", () => {
|
|
const result = parseDashboardPreferences({
|
|
version: "1",
|
|
projects: {},
|
|
theme: "neon",
|
|
contrast: 999,
|
|
currentProjectId: "proj_123",
|
|
sideMenu: { isCollapsed: true },
|
|
});
|
|
expect(result.theme).toBeUndefined();
|
|
expect(result.contrast).toBeUndefined();
|
|
expect(result.currentProjectId).toBe("proj_123");
|
|
expect(result.sideMenu?.isCollapsed).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe("preserveUnknownKeys", () => {
|
|
it("re-attaches a key the schema dropped, so a full-blob write can't erase it", () => {
|
|
const raw = {
|
|
version: "1",
|
|
projects: {},
|
|
theme: "dark",
|
|
somethingANewerDeployAdded: { nested: true },
|
|
};
|
|
const result = preserveUnknownKeys(raw, parseDashboardPreferences(raw));
|
|
expect(result).toHaveProperty("somethingANewerDeployAdded", { nested: true });
|
|
expect(result.theme).toBe("dark");
|
|
});
|
|
|
|
it("lets the parsed value win for keys the schema does know", () => {
|
|
const raw = { version: "1", projects: {}, theme: "dark", contrast: 40 };
|
|
const result = preserveUnknownKeys(raw, { ...parseDashboardPreferences(raw), contrast: 10 });
|
|
expect(result.contrast).toBe(10);
|
|
});
|
|
|
|
it("passes the update straight through when there is nothing extra to keep", () => {
|
|
const raw = { version: "1", projects: {} };
|
|
const parsed = parseDashboardPreferences(raw);
|
|
expect(preserveUnknownKeys(raw, parsed)).toBe(parsed);
|
|
expect(preserveUnknownKeys(null, parsed)).toBe(parsed);
|
|
expect(preserveUnknownKeys("nonsense", parsed)).toBe(parsed);
|
|
});
|
|
});
|