diff --git a/apps/webapp/app/components/navigation/AppearanceMenuItem.tsx b/apps/webapp/app/components/navigation/AppearanceMenuItem.tsx index 4b1f056d22d..6894d787d2e 100644 --- a/apps/webapp/app/components/navigation/AppearanceMenuItem.tsx +++ b/apps/webapp/app/components/navigation/AppearanceMenuItem.tsx @@ -65,6 +65,7 @@ export function AppearanceMenuItem() { icon={EllipsisHorizontalIcon} leadingIconClassName={SIDE_MENU_POPOVER_ITEM_ICON} className={SIDE_MENU_POPOVER_ITEM_LABEL} + isSelected={!THEME_OPTIONS.some((option) => option.value === theme)} /> diff --git a/apps/webapp/app/components/navigation/CustomizeSidebarDialog.tsx b/apps/webapp/app/components/navigation/CustomizeSidebarDialog.tsx index ff892b7071b..89d365cd3f5 100644 --- a/apps/webapp/app/components/navigation/CustomizeSidebarDialog.tsx +++ b/apps/webapp/app/components/navigation/CustomizeSidebarDialog.tsx @@ -45,6 +45,8 @@ export type SidebarCustomizationPayload = { sectionItemOrder: Record | null; favorites?: Array<{ id: string; label: string }>; removedFavoriteIds?: string[]; + /** Item ids this dialog rendered, so the write leaves ids it never saw alone. */ + knownItemIds: string[]; }; type DialogState = { @@ -248,6 +250,7 @@ export function CustomizeSidebarDialog({ ? favoriteOrder.map((id) => ({ id, label: state.labels[id] ?? "" })) : undefined, removedFavoriteIds: state.removed.length > 0 ? state.removed : undefined, + knownItemIds: sections.flatMap((section) => section.items.map((item) => item.id)), }; onConfirm(payload); diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 39456e529ea..9187d361dba 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -69,7 +69,6 @@ import { useFeatures } from "~/hooks/useFeatures"; import { useHasAdminAccess, useUser } from "~/hooks/useUser"; import { updateUserEmail, updateUserMarketingEmails, updateUserName } from "~/models/user.server"; import { logger } from "~/services/logger.server"; -import { profileUpdateRateLimiter } from "~/services/profileUpdateRateLimiter.server"; import { type EmailOwnership, getEmailOwnership } from "~/services/ssoManagedIdentity.server"; import { updateContrastPreference, @@ -90,7 +89,7 @@ import { ThemePreference, } from "~/utils/themePreference"; import { cachedFlag, resolveOrganizationFeatureFlags } from "~/v3/featureFlags.server"; -import { requireUser, requireUserId } from "~/services/session.server"; +import { requireUser } from "~/services/session.server"; import { emailSchema, MAX_EMAIL_LENGTH } from "~/utils/emailValidation"; import { pageMeta } from "~/utils/pageTitle"; import { cn } from "~/utils/cn"; @@ -101,6 +100,8 @@ const MIN_CONTRAST = 0; const DEFAULT_CONTRAST_MARK = 0; +const CONTRAST_SAVE_DEBOUNCE_MS = 400; + function themeIcon(value: ThemePreference, appearance: ThemeAppearance) { const Icon = themeOptionIcon(THEME_OPTIONS_BY_VALUE[value], appearance); // shrink-0 stops a long label squashing the icon. @@ -200,31 +201,32 @@ function profileUpdateError(error: string, status: number) { } /** - * Shared gate for the appearance writes: same rate limit as the profile writes, - * then the theme-switcher flag. Returns the user so the caller needn't load it - * a second time. + * Shared gate for every write on this page. Returns the user so the caller + * needn't load it a second time. */ -async function requireAppearanceAccess(request: Request, userId: string) { - const rateLimited = await checkProfileUpdateRateLimit(userId); - if (rateLimited) return { error: rateLimited }; - +async function requireOwnAccountWrite(request: Request) { const user = await requireUser(request); - const showThemeSwitcher = - user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false })); - if (!showThemeSwitcher) { - return { error: profileUpdateError("Not available", 404) }; + if (user.isImpersonating) { + return { + error: profileUpdateError("You can't change this while impersonating another user.", 403), + }; } return { user }; } -/** The only limit a scripted POST can't skip. */ -async function checkProfileUpdateRateLimit(userId: string) { - const limit = await profileUpdateRateLimiter.limit(`user:${userId}`); - if (limit.success) { - return undefined; +/** The gate above, plus the theme-switcher flag. */ +async function requireAppearanceAccess(request: Request) { + const gate = await requireOwnAccountWrite(request); + if ("error" in gate) return gate; + + const showThemeSwitcher = + gate.user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false })); + if (!showThemeSwitcher) { + return { error: profileUpdateError("Not available", 404) }; } - return profileUpdateError("Too many changes at once. Please wait a moment and try again.", 429); + + return gate; } export async function loader({ request }: LoaderFunctionArgs) { @@ -232,9 +234,6 @@ export async function loader({ request }: LoaderFunctionArgs) { const showThemeSwitcher = user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false })); - // Picks the modal only; the action re-checks before writing. - const emailOwnership = await getEmailOwnership(user); - // Null when the user has no project yet; the row hides itself. let sidebarContext: { organization: { slug: string }; @@ -262,16 +261,14 @@ export async function loader({ request }: LoaderFunctionArgs) { }); } - return json({ showThemeSwitcher, sidebarContext, emailOwnership }); + return json({ showThemeSwitcher, sidebarContext }); } export const action: ActionFunction = async ({ request }) => { - const userId = await requireUserId(request); - const formData = await request.formData(); if (formData.get("action") === "update-theme") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; // Strict, matching /resources/preferences/theme: an unknown value must fail // rather than quietly resetting a saved theme to the default. @@ -282,7 +279,7 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-contrast") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; const contrast = normalizeThemeContrast(formData.get("contrast")); await updateContrastPreference({ user: gate.user, contrast }); @@ -290,7 +287,7 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-icon-contrast") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; await updateIconContrastPreference({ user: gate.user, @@ -300,7 +297,7 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-underline-links") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; await updateUnderlineLinksPreference({ user: gate.user, @@ -310,7 +307,7 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-system-theme") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; // Strict: an unknown value must fail, not silently reset. const end = formData.get("end"); @@ -338,8 +335,8 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-name") { - const rateLimited = await checkProfileUpdateRateLimit(userId); - if (rateLimited) return rateLimited; + const gate = await requireOwnAccountWrite(request); + if ("error" in gate) return gate.error; const submission = NameSchema.safeParse({ name: formData.get("name") }); if (!submission.success) { @@ -349,17 +346,26 @@ export const action: ActionFunction = async ({ request }) => { ); } - await updateUserName({ id: userId, name: submission.data.name }); + await updateUserName({ id: gate.user.id, name: submission.data.name }); return json({ success: true as const }); } if (formData.get("action") === "update-email") { - const rateLimited = await checkProfileUpdateRateLimit(userId); - if (rateLimited) return rateLimited; + const gate = await requireOwnAccountWrite(request); + if ("error" in gate) return gate.error; // Re-checked: the loader only picked the modal. - const user = await requireUser(request); - const ownership = await getEmailOwnership(user); + const submission = EmailSchema.safeParse({ email: formData.get("email") }); + if (!submission.success) { + return profileUpdateError( + submission.error.issues[0]?.message ?? "That email address isn't valid.", + 400 + ); + } + + const { email } = submission.data; + + const ownership = await getEmailOwnership(gate.user, email); if (ownership === "idp") { return profileUpdateError( "Your email address is managed by your organization's identity provider.", @@ -372,28 +378,18 @@ export const action: ActionFunction = async ({ request }) => { 503 ); } - - const submission = EmailSchema.safeParse({ email: formData.get("email") }); - if (!submission.success) { - return profileUpdateError( - submission.error.issues[0]?.message ?? "That email address isn't valid.", - 400 - ); - } - - const { email } = submission.data; const existingUser = await prisma.user.findFirst({ where: { email } }); - if (existingUser && existingUser.id !== userId) { + if (existingUser && existingUser.id !== gate.user.id) { return profileUpdateError("Email is already being used by a different account", 400); } - await updateUserEmail({ id: userId, email }); + await updateUserEmail({ id: gate.user.id, email }); return json({ success: true as const }); } if (formData.get("action") === "update-marketing-emails") { - const rateLimited = await checkProfileUpdateRateLimit(userId); - if (rateLimited) return rateLimited; + const gate = await requireOwnAccountWrite(request); + if ("error" in gate) return gate.error; const submission = MarketingEmailsSchema.safeParse({ marketingEmails: formData.get("marketingEmails"), @@ -404,7 +400,7 @@ export const action: ActionFunction = async ({ request }) => { // No-op when the stored value already matches. await updateUserMarketingEmails({ - id: userId, + id: gate.user.id, marketingEmails: submission.data.marketingEmails, }); return json({ success: true as const }); @@ -519,13 +515,17 @@ function EditNameButton() { ); } -function EditEmailButton({ ownership }: { ownership: EmailOwnership }) { +const EMAIL_OWNERSHIP_PATH = "/resources/account/email-ownership"; + +function EditEmailButton() { const user = useUser(); const [isOpen, setIsOpen] = useState(false); const { fetcher, error, setError, isSubmitting } = useProfileFieldUpdate({ successMessage: "Your email address has been updated.", onSuccess: () => setIsOpen(false), }); + const ownershipFetcher = useFetcher<{ ownership: EmailOwnership }>(); + const ownership = ownershipFetcher.data?.ownership; return ( { setIsOpen(open); if (!open) setError(undefined); + if (open && ownershipFetcher.state === "idle" && !ownershipFetcher.data) { + ownershipFetcher.load(EMAIL_OWNERSHIP_PATH); + } }} > @@ -550,7 +553,11 @@ function EditEmailButton({ ownership }: { ownership: EmailOwnership }) { Email address - {ownership === "idp" ? ( + {ownership === undefined ? ( + + Checking your sign-in settings… + + ) : ownership === "idp" ? ( Your organization uses single sign-on, so your email address is managed by your identity provider rather than here. To change it, ask an organization admin to update @@ -773,8 +780,8 @@ function CustomizeSidebarButton({ export default function Page() { const user = useUser(); - const { showThemeSwitcher, sidebarContext, emailOwnership } = useLoaderData(); - const themeFetcher = useFetcher(); + const { showThemeSwitcher, sidebarContext } = useLoaderData(); + const themeFetcher = useFetcher(); const contrastFetcher = useFetcher(); const iconContrastFetcher = useFetcher(); const pendingIconContrast = iconContrastFetcher.formData?.get("iconContrast"); @@ -802,8 +809,8 @@ export default function Page() { const appearance = useThemeAppearance(theme); // One fetcher per end, so picking both quickly can't cancel the first. - const systemLightFetcher = useFetcher(); - const systemDarkFetcher = useFetcher(); + const systemLightFetcher = useFetcher(); + const systemDarkFetcher = useFetcher(); const pendingSystemLight = systemLightFetcher.formData?.get("theme"); const pendingSystemDark = systemDarkFetcher.formData?.get("theme"); const systemLightTheme = normalizeSystemLightTheme( @@ -828,25 +835,51 @@ export default function Page() { fetcher.submit({ action: "update-system-theme", end, theme: value }, { method: "post" }); }; + const storedTheme = normalizeThemePreference(user.dashboardPreferences.theme); + const storedSystemLight = normalizeSystemLightTheme(user.dashboardPreferences.systemLightTheme); + const storedSystemDark = normalizeSystemDarkTheme(user.dashboardPreferences.systemDarkTheme); + const themeWriteFailed = [themeFetcher, systemLightFetcher, systemDarkFetcher].some( + (fetcher) => fetcher.state === "idle" && fetcher.data && !fetcher.data.success + ); + useEffect(() => { + if (themeWriteFailed) { + applyThemePreference(storedTheme, { light: storedSystemLight, dark: storedSystemDark }); + } + }, [themeWriteFailed, storedTheme, storedSystemLight, storedSystemDark]); + // Resnap to the stored value so a failed save leaves no phantom contrast. const [contrastPreview, setContrastPreview] = useState(contrast); + const [contrastToSave, setContrastToSave] = useState(undefined); useEffect(() => { - if (contrastFetcher.state === "idle") { + if (contrastFetcher.state === "idle" && contrastToSave === undefined) { // oxlint-disable-next-line react/set-state-in-effect -- This effect intentionally synchronizes route state after an external or lifecycle change. setContrastPreview(contrast); applyThemeContrast(contrast); } - }, [contrastFetcher.state, contrast]); + }, [contrastFetcher.state, contrast, contrastToSave]); + + const contrastSubmitRef = useRef(contrastFetcher.submit); + useEffect(() => { + contrastSubmitRef.current = contrastFetcher.submit; + }); + useEffect(() => { + if (contrastToSave === undefined) return; + const timer = setTimeout(() => { + contrastSubmitRef.current( + { action: "update-contrast", contrast: String(contrastToSave) }, + { method: "post" } + ); + // oxlint-disable-next-line react/set-state-in-effect -- Clears the debounce slot once the write is away. + setContrastToSave(undefined); + }, CONTRAST_SAVE_DEBOUNCE_MS); + return () => clearTimeout(timer); + }, [contrastToSave]); const previewContrast = (value: number) => { setContrastPreview(value); applyThemeContrast(value); }; - const saveContrast = (value: number) => - contrastFetcher.submit( - { action: "update-contrast", contrast: String(value) }, - { method: "post" } - ); + const saveContrast = (value: number) => setContrastToSave(value); return ( @@ -891,7 +924,7 @@ export default function Page() { {user.email} - + diff --git a/apps/webapp/app/routes/resources.account.email-ownership.ts b/apps/webapp/app/routes/resources.account.email-ownership.ts new file mode 100644 index 00000000000..a6b6ab3f308 --- /dev/null +++ b/apps/webapp/app/routes/resources.account.email-ownership.ts @@ -0,0 +1,9 @@ +import { json, type LoaderFunctionArgs } from "@remix-run/node"; +import { getEmailOwnership } from "~/services/ssoManagedIdentity.server"; +import { requireUser } from "~/services/session.server"; + +export async function loader({ request }: LoaderFunctionArgs) { + const user = await requireUser(request); + + return json({ ownership: await getEmailOwnership(user) }); +} diff --git a/apps/webapp/app/routes/resources.preferences.sidemenu.tsx b/apps/webapp/app/routes/resources.preferences.sidemenu.tsx index 35b11a4cf1c..c9314b9e1d1 100644 --- a/apps/webapp/app/routes/resources.preferences.sidemenu.tsx +++ b/apps/webapp/app/routes/resources.preferences.sidemenu.tsx @@ -41,6 +41,7 @@ const CustomizationSchema = z.object({ .max(100) .optional(), removedFavoriteIds: z.array(z.string().max(64)).max(100).optional(), + knownItemIds: z.array(z.string().max(64)).max(500).optional(), }); export async function action({ request }: ActionFunctionArgs) { @@ -73,8 +74,14 @@ export async function action({ request }: ActionFunctionArgs) { if (!customizationResult.success) { return json({ success: false, error: "Invalid request data" }, { status: 400 }); } - const { sectionOrder, hiddenItems, sectionItemOrder, favorites, removedFavoriteIds } = - customizationResult.data; + const { + sectionOrder, + hiddenItems, + sectionItemOrder, + favorites, + removedFavoriteIds, + knownItemIds, + } = customizationResult.data; // The modal keeps its "Confirm" pending until this responds, so failures must come back as a // response (never a throw, which would escalate a preferences write to the error boundary). try { @@ -85,6 +92,7 @@ export async function action({ request }: ActionFunctionArgs) { sectionItemOrder, favorites, removedFavoriteIds, + knownItemIds, }); // undefined means nothing was written (impersonating, or the user row is gone) if (!updated) { diff --git a/apps/webapp/app/services/dashboardPreferences.server.ts b/apps/webapp/app/services/dashboardPreferences.server.ts index 2710145b0f1..ed63a397184 100644 --- a/apps/webapp/app/services/dashboardPreferences.server.ts +++ b/apps/webapp/app/services/dashboardPreferences.server.ts @@ -4,6 +4,8 @@ import { type UserFromSession } from "./session.server"; import { type DashboardPreferences, type FavoritePage, + mergeHiddenItems, + preserveUnknownKeys, parseDashboardPreferences, SideMenuPreferences, } from "~/utils/dashboardPreferences"; @@ -50,7 +52,8 @@ async function mutateDashboardPreferences( return undefined; } - const updated = mutate(getDashboardPreferences(rows[0].dashboardPreferences)); + const raw = rows[0].dashboardPreferences; + const updated = mutate(getDashboardPreferences(raw)); if (!updated) { return undefined; } @@ -60,7 +63,7 @@ async function mutateDashboardPreferences( id: userId, }, data: { - dashboardPreferences: updated, + dashboardPreferences: preserveUnknownKeys(raw, updated), }, }); }, @@ -468,6 +471,7 @@ export async function updateSideMenuCustomization({ sectionItemOrder, favorites, removedFavoriteIds, + knownItemIds, }: { user: UserFromSession; /** undefined = leave unchanged, null = reset to default */ @@ -480,6 +484,13 @@ export async function updateSideMenuCustomization({ favorites?: Array<{ id: string; label: string }>; /** Favorites deleted from the customize modal. */ removedFavoriteIds?: string[]; + /** + * Item ids the submitting dialog rendered. `hiddenItems` only describes these, + * so ids outside the list keep whatever they had: the dialog's section list + * depends on the org whose feature flags were in scope, and a narrower list + * must not un-hide items belonging to a wider one. + */ + knownItemIds?: string[]; }) { if (user.isImpersonating) { return; @@ -494,8 +505,7 @@ export async function updateSideMenuCustomization({ } if (hiddenItems !== undefined) { - next.hiddenItems = - hiddenItems && Object.keys(hiddenItems).length > 0 ? hiddenItems : undefined; + next.hiddenItems = mergeHiddenItems(currentSideMenu.hiddenItems, hiddenItems, knownItemIds); } if (sectionItemOrder !== undefined) { diff --git a/apps/webapp/app/services/profileUpdateRateLimiter.server.ts b/apps/webapp/app/services/profileUpdateRateLimiter.server.ts deleted file mode 100644 index 988979adad4..00000000000 --- a/apps/webapp/app/services/profileUpdateRateLimiter.server.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { Ratelimit } from "@upstash/ratelimit"; -import { type RedisWithClusterOptions } from "~/redis.server"; -import { createRedisRateLimitClient, RateLimiter } from "~/services/rateLimiter.server"; -import { singleton } from "~/utils/singleton"; - -// Every profile row writes on its own, with no submit button pacing them. The -// client debounce is a courtesy a scripted POST skips, and `/account` sits -// outside `/api/*` so apiRateLimiter doesn't cover it. Exported for the tests. -const PROFILE_UPDATE_RATE_LIMIT_ATTEMPTS = 20; -const PROFILE_UPDATE_RATE_LIMIT_WINDOW = "1 m" as const; - -/** Production uses the env-derived Redis; tests inject a container one. */ -function createProfileUpdateRateLimiter(redisOptions?: RedisWithClusterOptions): RateLimiter { - return new RateLimiter({ - ...(redisOptions ? { redisClient: createRedisRateLimitClient(redisOptions) } : {}), - keyPrefix: "account.profile-update", - limiter: Ratelimit.slidingWindow( - PROFILE_UPDATE_RATE_LIMIT_ATTEMPTS, - PROFILE_UPDATE_RATE_LIMIT_WINDOW - ), - logFailure: true, - }); -} - -export const profileUpdateRateLimiter = singleton("profileUpdateRateLimiter", () => - createProfileUpdateRateLimiter() -); diff --git a/apps/webapp/app/services/ssoManagedIdentity.server.ts b/apps/webapp/app/services/ssoManagedIdentity.server.ts index afae8d60579..38869748ee0 100644 --- a/apps/webapp/app/services/ssoManagedIdentity.server.ts +++ b/apps/webapp/app/services/ssoManagedIdentity.server.ts @@ -28,21 +28,34 @@ export function idpOwnsEmailDomain(status: OrgSsoStatus, emailDomain: string): b ); } -function domainOf(email: string): string | undefined { - const domain = email.toLowerCase().trim().split("@")[1]; - return domain || undefined; +export function emailDomainOf(email: string): string | undefined { + const normalized = email.toLowerCase().trim(); + const at = normalized.lastIndexOf("@"); + return at === -1 ? undefined : normalized.slice(at + 1) || undefined; } -export async function getEmailOwnership(user: { - id: string; - email: string; -}): Promise { +/** + * `candidateEmail` is the address being moved to, when there is one. An org that + * owns either end owns the change: checking only the current address would let a + * member on an unverified domain move onto the org's IdP-managed one. + */ +export async function getEmailOwnership( + user: { + id: string; + email: string; + }, + candidateEmail?: string +): Promise { if (!(await ssoController.isUsingPlugin())) { return "user"; } - const emailDomain = domainOf(user.email); - if (!emailDomain) { + const domains = [ + emailDomainOf(user.email), + candidateEmail ? emailDomainOf(candidateEmail) : undefined, + ]; + const emailDomains = [...new Set(domains.filter((domain): domain is string => !!domain))]; + if (emailDomains.length === 0) { return "user"; } @@ -74,7 +87,7 @@ export async function getEmailOwnership(user: { continue; } - if (idpOwnsEmailDomain(status.value, emailDomain)) { + if (emailDomains.some((domain) => idpOwnsEmailDomain(status.value, domain))) { return "idp"; } } diff --git a/apps/webapp/app/utils/dashboardPreferences.ts b/apps/webapp/app/utils/dashboardPreferences.ts index de858a901c5..fbe5ddecfa3 100644 --- a/apps/webapp/app/utils/dashboardPreferences.ts +++ b/apps/webapp/app/utils/dashboardPreferences.ts @@ -97,3 +97,45 @@ export function parseDashboardPreferences( return result.data; } + +/** + * Re-attach keys the schema dropped, so a full-blob write preserves fields this + * deploy was not compiled against. The parsed result wins for every key it + * carries, including ones it deliberately cleared to undefined. + */ +export function preserveUnknownKeys( + raw: unknown, + updated: DashboardPreferences +): DashboardPreferences { + if (!raw || typeof raw !== "object" || Array.isArray(raw)) { + return updated; + } + + const known = new Set(Object.keys(DashboardPreferences.shape)); + const unknownKeys = Object.entries(raw as Record).filter( + ([key]) => !known.has(key) + ); + + return unknownKeys.length > 0 ? { ...Object.fromEntries(unknownKeys), ...updated } : updated; +} + +/** + * Fold a customize-sidebar submission into the stored hidden map. `submitted` + * only describes `knownItemIds`, so ids outside that list keep what they had - + * the dialog's section list depends on which org's feature flags were in scope, + * and a narrower list must not un-hide items belonging to a wider one. Without + * the list the submission is authoritative, as it was before. + */ +export function mergeHiddenItems( + current: Record | undefined, + submitted: Record | null, + knownItemIds: string[] | undefined +): Record | undefined { + const known = knownItemIds ? new Set(knownItemIds) : undefined; + const preserved: Array<[string, boolean]> = known + ? Object.entries(current ?? {}).filter(([id]) => !known.has(id)) + : []; + const merged = { ...Object.fromEntries(preserved), ...(submitted ?? {}) }; + + return Object.keys(merged).length > 0 ? merged : undefined; +} diff --git a/apps/webapp/test/mergeHiddenItems.test.ts b/apps/webapp/test/mergeHiddenItems.test.ts new file mode 100644 index 00000000000..4cacacaeb30 --- /dev/null +++ b/apps/webapp/test/mergeHiddenItems.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { mergeHiddenItems } from "~/utils/dashboardPreferences"; + +describe("mergeHiddenItems", () => { + it("leaves ids the dialog never rendered alone", () => { + const result = mergeHiddenItems({ logs: true, queues: true }, { queues: false }, ["queues"]); + expect(result).toEqual({ logs: true, queues: false }); + }); + + it("keeps out-of-scope ids when the submission resets to defaults", () => { + const result = mergeHiddenItems({ logs: true, queues: true }, null, ["queues"]); + expect(result).toEqual({ logs: true }); + }); + + it("treats the submission as authoritative without a known-id list", () => { + const result = mergeHiddenItems({ logs: true, queues: true }, { queues: false }, undefined); + expect(result).toEqual({ queues: false }); + }); + + it("clears the stored map when nothing is left hidden", () => { + expect(mergeHiddenItems({ queues: true }, null, ["queues"])).toBeUndefined(); + expect(mergeHiddenItems(undefined, null, undefined)).toBeUndefined(); + }); + + it("lets the submission win for ids it did render", () => { + const result = mergeHiddenItems({ queues: true }, { queues: false }, ["queues", "logs"]); + expect(result).toEqual({ queues: false }); + }); +}); diff --git a/apps/webapp/test/ssoManagedIdentity.test.ts b/apps/webapp/test/ssoManagedIdentity.test.ts index fda4f8ec9da..51cb5a28d8c 100644 --- a/apps/webapp/test/ssoManagedIdentity.test.ts +++ b/apps/webapp/test/ssoManagedIdentity.test.ts @@ -1,6 +1,6 @@ import type { OrgSsoStatus } from "@trigger.dev/plugins"; import { describe, expect, it } from "vitest"; -import { idpOwnsEmailDomain } from "~/services/ssoManagedIdentity.server"; +import { emailDomainOf, idpOwnsEmailDomain } from "~/services/ssoManagedIdentity.server"; function status(overrides: Partial = {}): OrgSsoStatus { return { @@ -84,3 +84,22 @@ describe("idpOwnsEmailDomain", () => { expect(idpOwnsEmailDomain(status(), "mail.acme.com")).toBe(false); }); }); + +describe("emailDomainOf", () => { + it("reads the domain off an ordinary address", () => { + expect(emailDomainOf("alice@acme.com")).toBe("acme.com"); + }); + + it("lowercases and trims", () => { + expect(emailDomainOf(" Alice@ACME.com ")).toBe("acme.com"); + }); + + it("splits on the last @, so a quoted local part can't hide the domain", () => { + expect(emailDomainOf('"a@b"@acme.com')).toBe("acme.com"); + }); + + it("returns undefined when there is no domain to read", () => { + expect(emailDomainOf("alice")).toBeUndefined(); + expect(emailDomainOf("alice@")).toBeUndefined(); + }); +}); diff --git a/apps/webapp/test/themePreference.test.ts b/apps/webapp/test/themePreference.test.ts index f89ecf49b79..a3a4c438b85 100644 --- a/apps/webapp/test/themePreference.test.ts +++ b/apps/webapp/test/themePreference.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { parseDashboardPreferences } from "~/utils/dashboardPreferences"; +import { parseDashboardPreferences, preserveUnknownKeys } from "~/utils/dashboardPreferences"; import { normalizeThemePreference, type ThemePreference } from "~/utils/themePreference"; const VALID_THEMES: ThemePreference[] = ["system", "dark", "light", "black", "white"]; @@ -59,3 +59,31 @@ describe("DashboardPreferences theme schema", () => { 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); + }); +});