Skip to content

Commit c8d7ec6

Browse files
committed
fix(webapp): address the review findings on the appearance work
Restores the Logs entry for teams with logs access but not query access. Extracting the side menu's sections dropped a clause from the Observability gate, so the whole group disappeared for them. Gives the profile page's "Customize sidebar" dialog the same feature flags the side menu sees. /account sits outside an org, so `useFeatureFlags` returned nothing and the dialog offered a shorter section list - and saving it replaced the stored order and hidden items, dropping the sections it had never shown. Flags are now resolved in the loader through a shared helper so the two can't drift. Rate-limits the appearance writes. Every profile row saves on its own, and the five appearance actions were the only ones a scripted POST could drive unthrottled - the contrast slider being the easiest. The repeated flag gate moves into that same helper. `update-theme` now rejects an unknown theme instead of quietly resetting it to dark, matching its sibling action and the /resources/preferences/theme route. Keeps the span title legible on the light themes. The run-type accents are drawn to clear 3:1 as icons; as 16px semibold text tasks (3.38) and agents (3.96) fall under 4.5:1 on Light and White, so the title takes the text colour there and the icon carries the type. Shows the contrast slider's value label on keyboard focus, via focus-visible so a mouse click doesn't strand it on screen.
1 parent cd9e7af commit c8d7ec6

6 files changed

Lines changed: 128 additions & 85 deletions

File tree

apps/webapp/app/components/navigation/sideMenuSections.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ export function buildSideMenuSections({
122122
});
123123
}
124124

125-
if (isAdmin || featureFlags.hasQueryAccess) {
125+
if (isAdmin || featureFlags.hasQueryAccess || featureFlags.hasLogsPageAccess) {
126126
staticSections.push({
127127
id: "metrics",
128128
title: "Observability",

apps/webapp/app/components/primitives/Slider.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,11 @@ export function Slider({
166166
<span
167167
className={cn(
168168
"pointer-events-none absolute bottom-full left-1/2 mb-2.5 -translate-x-1/2 rounded border border-grid-bright bg-background-bright px-1.5 py-0.5 text-xs tabular-nums text-text-bright shadow-md transition-opacity",
169-
// Not keyed off focus: the thumb keeps it after a click.
170-
isDragging ? "opacity-100" : "opacity-0 group-hover/thumb:opacity-100"
169+
// focus-visible, not focus: a click leaves the thumb focused,
170+
// which would strand the label on screen.
171+
isDragging
172+
? "opacity-100"
173+
: "opacity-0 group-hover/thumb:opacity-100 group-focus-visible/thumb:opacity-100"
171174
)}
172175
>
173176
{valueTooltip(currentValue)}

apps/webapp/app/presenters/OrganizationsPresenter.server.ts

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ import { newOrganizationPath, newProjectPath } from "~/utils/pathBuilder";
77
import { SelectBestEnvironmentPresenter } from "./SelectBestEnvironmentPresenter.server";
88
import { sortEnvironments } from "~/utils/environmentSort";
99
import { defaultAvatar, parseAvatar } from "~/components/primitives/Avatar";
10-
import { env } from "~/env.server";
11-
import { flags } from "~/v3/featureFlags.server";
12-
import { validatePartialFeatureFlags } from "~/v3/featureFlags";
10+
import { globalFeatureFlags, mergeOrgFeatureFlags } from "~/v3/featureFlags.server";
1311
import { hydrateEnvsWithActivity } from "./v3/BranchesPresenter.server";
1412

1513
export class OrganizationsPresenter {
@@ -155,23 +153,10 @@ export class OrganizationsPresenter {
155153
},
156154
});
157155

158-
// Get global feature flags with env-var-based defaults
159-
const globalFlags = await flags({
160-
defaultValues: {
161-
hasAiAccess: env.AI_FEATURES_ENABLED === "1",
162-
hasDashboardAgentAccess: env.DASHBOARD_AGENT_ENABLED === "1",
163-
hasPrivateConnections: env.PRIVATE_CONNECTIONS_ENABLED === "1",
164-
},
165-
});
156+
const globalFlags = await globalFeatureFlags();
166157

167158
return orgs.map((org) => {
168-
const orgFlagsResult = org.featureFlags
169-
? validatePartialFeatureFlags(org.featureFlags as Record<string, unknown>)
170-
: ({ success: false } as const);
171-
const orgFlags = orgFlagsResult.success ? orgFlagsResult.data : {};
172-
173-
// Combine global flags with org flags (org flags win)
174-
const combinedFlags = { ...globalFlags, ...orgFlags };
159+
const combinedFlags = mergeOrgFeatureFlags(globalFlags, org.featureFlags);
175160

176161
return {
177162
id: org.id,

apps/webapp/app/routes/account._index/route.tsx

Lines changed: 81 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { useEffect, useRef, useState } from "react";
22
import { useFetcher, useLoaderData } from "@remix-run/react";
3-
import { type ActionFunction, json, type LoaderFunctionArgs } from "@remix-run/server-runtime";
3+
import {
4+
type ActionFunction,
5+
json,
6+
type LoaderFunctionArgs,
7+
type SerializeFrom,
8+
} from "@remix-run/server-runtime";
49
import { z } from "zod";
510
import { EditPencilIcon } from "~/assets/icons/EditPencilIcon";
611
import { UserProfilePhoto } from "~/components/UserProfilePhoto";
@@ -54,7 +59,6 @@ import {
5459
} from "~/components/themeOptions";
5560
import { prisma } from "~/db.server";
5661
import { SelectBestEnvironmentPresenter } from "~/presenters/SelectBestEnvironmentPresenter.server";
57-
import { useFeatureFlags } from "~/hooks/useFeatureFlags";
5862
import {
5963
applyThemeContrast,
6064
applyThemePreference,
@@ -64,6 +68,7 @@ import {
6468
import { useFeatures } from "~/hooks/useFeatures";
6569
import { useHasAdminAccess, useUser } from "~/hooks/useUser";
6670
import { updateUserEmail, updateUserMarketingEmails, updateUserName } from "~/models/user.server";
71+
import { logger } from "~/services/logger.server";
6772
import { profileUpdateRateLimiter } from "~/services/profileUpdateRateLimiter.server";
6873
import { type EmailOwnership, getEmailOwnership } from "~/services/ssoManagedIdentity.server";
6974
import {
@@ -82,9 +87,9 @@ import {
8287
normalizeThemePreference,
8388
SystemDarkTheme,
8489
SystemLightTheme,
85-
type ThemePreference,
90+
ThemePreference,
8691
} from "~/utils/themePreference";
87-
import { cachedFlag } from "~/v3/featureFlags.server";
92+
import { cachedFlag, resolveOrganizationFeatureFlags } from "~/v3/featureFlags.server";
8893
import { requireUser, requireUserId } from "~/services/session.server";
8994
import { emailSchema, MAX_EMAIL_LENGTH } from "~/utils/emailValidation";
9095
import { pageMeta } from "~/utils/pageTitle";
@@ -194,6 +199,25 @@ function profileUpdateError(error: string, status: number) {
194199
return json({ success: false as const, error }, { status });
195200
}
196201

202+
/**
203+
* Shared gate for the appearance writes: same rate limit as the profile writes,
204+
* then the theme-switcher flag. Returns the user so the caller needn't load it
205+
* a second time.
206+
*/
207+
async function requireAppearanceAccess(request: Request, userId: string) {
208+
const rateLimited = await checkProfileUpdateRateLimit(userId);
209+
if (rateLimited) return { error: rateLimited };
210+
211+
const user = await requireUser(request);
212+
const showThemeSwitcher =
213+
user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false }));
214+
if (!showThemeSwitcher) {
215+
return { error: profileUpdateError("Not available", 404) };
216+
}
217+
218+
return { user };
219+
}
220+
197221
/** The only limit a scripted POST can't skip. */
198222
async function checkProfileUpdateRateLimit(userId: string) {
199223
const limit = await profileUpdateRateLimiter.limit(`user:${userId}`);
@@ -216,6 +240,10 @@ export async function loader({ request }: LoaderFunctionArgs) {
216240
organization: { slug: string };
217241
project: { slug: string };
218242
environment: { slug: string };
243+
// Resolved here, not from route matches: /account sits outside an org, so
244+
// `useFeatureFlags` would see none and the dialog would offer a shorter
245+
// section list than the side menu - and saving it drops the rest.
246+
featureFlags: Awaited<ReturnType<typeof resolveOrganizationFeatureFlags>>;
219247
} | null = null;
220248
try {
221249
const { organization, project, environment } = await new SelectBestEnvironmentPresenter().call({
@@ -225,8 +253,14 @@ export async function loader({ request }: LoaderFunctionArgs) {
225253
organization: { slug: organization.slug },
226254
project: { slug: project.slug },
227255
environment: { slug: environment.slug },
256+
featureFlags: await resolveOrganizationFeatureFlags(organization.featureFlags),
228257
};
229-
} catch {}
258+
} catch (error) {
259+
logger.debug("Account page: no sidebar context for this user", {
260+
userId: user.id,
261+
error: error instanceof Error ? error.message : error,
262+
});
263+
}
230264

231265
return json({ showThemeSwitcher, sidebarContext, emailOwnership });
232266
}
@@ -237,79 +271,70 @@ export const action: ActionFunction = async ({ request }) => {
237271
const formData = await request.formData();
238272

239273
if (formData.get("action") === "update-theme") {
240-
const user = await requireUser(request);
241-
const showThemeSwitcher =
242-
user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false }));
243-
if (!showThemeSwitcher) {
244-
return json({ error: "Not available" }, { status: 404 });
245-
}
246-
const theme = normalizeThemePreference(formData.get("theme"));
247-
await updateThemePreference({ user, theme });
274+
const gate = await requireAppearanceAccess(request, userId);
275+
if ("error" in gate) return gate.error;
276+
// Strict, matching /resources/preferences/theme: an unknown value must fail
277+
// rather than quietly resetting a saved theme to the default.
278+
const theme = ThemePreference.safeParse(formData.get("theme"));
279+
if (!theme.success) return profileUpdateError("Invalid theme", 400);
280+
await updateThemePreference({ user: gate.user, theme: theme.data });
248281
return json({ success: true });
249282
}
250283

251284
if (formData.get("action") === "update-contrast") {
252-
const user = await requireUser(request);
253-
const showThemeSwitcher =
254-
user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false }));
255-
if (!showThemeSwitcher) {
256-
return json({ error: "Not available" }, { status: 404 });
257-
}
285+
const gate = await requireAppearanceAccess(request, userId);
286+
if ("error" in gate) return gate.error;
258287
const contrast = normalizeThemeContrast(formData.get("contrast"));
259-
await updateContrastPreference({ user, contrast });
288+
await updateContrastPreference({ user: gate.user, contrast });
260289
return json({ success: true });
261290
}
262291

263292
if (formData.get("action") === "update-icon-contrast") {
264-
const user = await requireUser(request);
265-
const showThemeSwitcher =
266-
user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false }));
267-
if (!showThemeSwitcher) {
268-
return json({ error: "Not available" }, { status: 404 });
269-
}
293+
const gate = await requireAppearanceAccess(request, userId);
294+
if ("error" in gate) return gate.error;
270295
await updateIconContrastPreference({
271-
user,
296+
user: gate.user,
272297
iconContrast: formData.get("iconContrast") === "true",
273298
});
274299
return json({ success: true });
275300
}
276301

277302
if (formData.get("action") === "update-underline-links") {
278-
const user = await requireUser(request);
279-
const showThemeSwitcher =
280-
user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false }));
281-
if (!showThemeSwitcher) {
282-
return json({ error: "Not available" }, { status: 404 });
283-
}
303+
const gate = await requireAppearanceAccess(request, userId);
304+
if ("error" in gate) return gate.error;
284305
await updateUnderlineLinksPreference({
285-
user,
306+
user: gate.user,
286307
underlineLinks: formData.get("underlineLinks") === "true",
287308
});
288309
return json({ success: true });
289310
}
290311

291312
if (formData.get("action") === "update-system-theme") {
292-
const user = await requireUser(request);
293-
const showThemeSwitcher =
294-
user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false }));
295-
if (!showThemeSwitcher) {
296-
return json({ error: "Not available" }, { status: 404 });
297-
}
313+
const gate = await requireAppearanceAccess(request, userId);
314+
if ("error" in gate) return gate.error;
298315
// Strict: an unknown value must fail, not silently reset.
299316
const end = formData.get("end");
300317
if (end === "light") {
301318
const theme = SystemLightTheme.safeParse(formData.get("theme"));
302-
if (!theme.success) return json({ error: "Invalid theme" }, { status: 400 });
303-
await updateSystemThemePreference({ user, end: "systemLightTheme", theme: theme.data });
319+
if (!theme.success) return profileUpdateError("Invalid theme", 400);
320+
await updateSystemThemePreference({
321+
user: gate.user,
322+
end: "systemLightTheme",
323+
theme: theme.data,
324+
});
304325
return json({ success: true });
305326
}
306327
if (end === "dark") {
307328
const theme = SystemDarkTheme.safeParse(formData.get("theme"));
308-
if (!theme.success) return json({ error: "Invalid theme" }, { status: 400 });
309-
await updateSystemThemePreference({ user, end: "systemDarkTheme", theme: theme.data });
329+
if (!theme.success) return profileUpdateError("Invalid theme", 400);
330+
await updateSystemThemePreference({
331+
user: gate.user,
332+
end: "systemDarkTheme",
333+
theme: theme.data,
334+
});
310335
return json({ success: true });
311336
}
312-
return json({ error: "Invalid end" }, { status: 400 });
337+
return profileUpdateError("Invalid end", 400);
313338
}
314339

315340
if (formData.get("action") === "update-name") {
@@ -648,15 +673,10 @@ function MarketingEmailsSwitch() {
648673
function CustomizeSidebarButton({
649674
context,
650675
}: {
651-
context: {
652-
organization: { slug: string };
653-
project: { slug: string };
654-
environment: { slug: string };
655-
};
676+
context: NonNullable<SerializeFrom<typeof loader>["sidebarContext"]>;
656677
}) {
657678
const user = useUser();
658679
const isAdmin = useHasAdminAccess();
659-
const featureFlags = useFeatureFlags();
660680
const { isManagedCloud } = useFeatures();
661681
const favorites = useFavorites();
662682
const [isOpen, setIsOpen] = useState(false);
@@ -699,18 +719,16 @@ function CustomizeSidebarButton({
699719
},
700720
]
701721
: []),
702-
...buildSideMenuSections({ ...context, isAdmin, featureFlags, isManagedCloud }).map(
703-
(section) => ({
704-
id: section.id,
705-
title: section.title,
706-
items: section.items.map((item) => ({
707-
id: item.id,
708-
name: item.name,
709-
icon: item.icon,
710-
defaultHidden: item.defaultHidden,
711-
})),
712-
})
713-
),
722+
...buildSideMenuSections({ ...context, isAdmin, isManagedCloud }).map((section) => ({
723+
id: section.id,
724+
title: section.title,
725+
items: section.items.map((item) => ({
726+
id: item.id,
727+
name: item.name,
728+
icon: item.icon,
729+
defaultHidden: item.defaultHidden,
730+
})),
731+
})),
714732
];
715733

716734
return (

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,11 @@ function RunBody({
449449
<Header2
450450
className={cn(
451451
"overflow-x-hidden",
452-
run.isAgentRun ? "text-agents" : run.isScheduled ? "text-schedules" : "text-tasks"
452+
// The run-type accents are drawn for 3:1 as icons; as 16px text the
453+
// tasks and agents blues fall under 4.5:1 on the light themes, so
454+
// the title takes the text colour there and the icon carries type.
455+
run.isAgentRun ? "text-agents" : run.isScheduled ? "text-schedules" : "text-tasks",
456+
"light:text-text-bright"
453457
)}
454458
>
455459
<span className="truncate">

apps/webapp/app/v3/featureFlags.server.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import {
66
type FeatureFlagCatalogSchema,
77
type FeatureFlagKey,
88
FeatureFlagCatalog,
9+
validatePartialFeatureFlags,
910
} from "~/v3/featureFlags";
11+
import { env } from "~/env.server";
1012
import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace";
1113

1214
export type FlagsOptions<T extends FeatureFlagKey> = {
@@ -220,3 +222,34 @@ export async function applyGlobalMintKindFlip(
220222
return makeSetMultipleFlags(tx)(stamped);
221223
});
222224
}
225+
226+
/** The global flag set, with the env-var defaults this app applies. */
227+
export async function globalFeatureFlags() {
228+
return flags({
229+
defaultValues: {
230+
hasAiAccess: env.AI_FEATURES_ENABLED === "1",
231+
hasDashboardAgentAccess: env.DASHBOARD_AGENT_ENABLED === "1",
232+
hasPrivateConnections: env.PRIVATE_CONNECTIONS_ENABLED === "1",
233+
},
234+
});
235+
}
236+
237+
/** The global set with one org's overrides on top. */
238+
export function mergeOrgFeatureFlags(
239+
globalFlags: Partial<FeatureFlagCatalog>,
240+
orgFeatureFlags: unknown
241+
) {
242+
const parsed = orgFeatureFlags
243+
? validatePartialFeatureFlags(orgFeatureFlags as Record<string, unknown>)
244+
: ({ success: false } as const);
245+
return { ...globalFlags, ...(parsed.success ? parsed.data : {}) };
246+
}
247+
248+
/**
249+
* The flags that apply to one organization. Server-side callers that need the
250+
* same set the side menu sees should use this rather than assembling their own,
251+
* so a partial set can't silently disagree with it.
252+
*/
253+
export async function resolveOrganizationFeatureFlags(orgFeatureFlags: unknown) {
254+
return mergeOrgFeatureFlags(await globalFeatureFlags(), orgFeatureFlags);
255+
}

0 commit comments

Comments
 (0)