From 1cb99ba3c90dadc5bc0b54654f985595493a3f45 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 23 Jul 2026 21:26:09 +0000 Subject: [PATCH 1/4] fix(webapp): billing alerts review fixes and accessible copy affordance - getAlertPreviewLimitCents takes the real percentage mode from BillingAlertsSection instead of inferring it from threshold conversion, so absolute-dollar default alerts are no longer read as percentages (regression test added) - CopyableText renders a real button with an aria-label, visible on focus and coarse pointers, not only on mouse hover - default billing alert seeding only writes when no alerts exist, so a slow seed can't clobber the user's first alert edit after the timeout --- .../billing/BillingAlertsSection.tsx | 3 +- .../components/billing/billingAlertsFormat.ts | 16 +++++- .../components/primitives/CopyableText.tsx | 55 +++++++++---------- apps/webapp/app/models/organization.server.ts | 18 +++++- .../webapp/test/billingAlertsDefaults.test.ts | 29 +++++++++- 5 files changed, 86 insertions(+), 35 deletions(-) diff --git a/apps/webapp/app/components/billing/BillingAlertsSection.tsx b/apps/webapp/app/components/billing/BillingAlertsSection.tsx index ce2f5e5e0fc..69979724730 100644 --- a/apps/webapp/app/components/billing/BillingAlertsSection.tsx +++ b/apps/webapp/app/components/billing/BillingAlertsSection.tsx @@ -136,7 +136,8 @@ export function BillingAlertsSection({ const alertPreviewLimitCents = getAlertPreviewLimitCents( alerts, effectiveLimitCents, - planLimitCents + planLimitCents, + isPercentageMode ); const maxAlerts = isPercentageMode ? MAX_PERCENTAGE_ALERTS : MAX_ABSOLUTE_ALERTS; diff --git a/apps/webapp/app/components/billing/billingAlertsFormat.ts b/apps/webapp/app/components/billing/billingAlertsFormat.ts index 5940405a84c..ebcd918dea2 100644 --- a/apps/webapp/app/components/billing/billingAlertsFormat.ts +++ b/apps/webapp/app/components/billing/billingAlertsFormat.ts @@ -313,15 +313,25 @@ function percentageAlertAmountMatches( return amountCents === effectiveLimitCents || amountCents === planLimitCents; } -/** Cents base for dollar preview when displaying saved percentage alerts. */ +/** + * Cents base for dollar preview when displaying saved percentage alerts. + * `isPercentageMode` must come from the billing limit mode, not from the alert + * levels: absolute dollar alerts (e.g. the seeded defaults) also convert to + * non-empty UI thresholds and must not be treated as percentages of the limit. + */ export function getAlertPreviewLimitCents( alerts: BillingAlertsFormData, effectiveLimitCents: number, - planLimitCents: number + planLimitCents: number, + isPercentageMode: boolean ): number { const amountCents = getSavedAlertAmountCents(alerts); // Percentages always apply to the current limit, not the base stored at last save. - if (amountCents > 0 && percentageAlertLevelsToUiThresholds(alerts.alertLevels).length > 0) { + if ( + isPercentageMode && + amountCents > 0 && + percentageAlertLevelsToUiThresholds(alerts.alertLevels).length > 0 + ) { return effectiveLimitCents; } if (percentageAlertAmountMatches(amountCents, effectiveLimitCents, planLimitCents)) { diff --git a/apps/webapp/app/components/primitives/CopyableText.tsx b/apps/webapp/app/components/primitives/CopyableText.tsx index 9634ad1ac01..5ca0c3a7b98 100644 --- a/apps/webapp/app/components/primitives/CopyableText.tsx +++ b/apps/webapp/app/components/primitives/CopyableText.tsx @@ -12,6 +12,7 @@ export function CopyableText({ asChild, variant, hideTooltip, + ariaLabel, }: { value: string; copyValue?: string; @@ -24,6 +25,8 @@ export function CopyableText({ * fire Radix's global "one tooltip open at a time" close and dismiss the parent. */ hideTooltip?: boolean; + /** Accessible label for the copy button. Defaults to "Copy". */ + ariaLabel?: string; }) { const [isHovered, setIsHovered] = useState(false); const { copy, copied } = useCopy(copyValue ?? value); @@ -31,11 +34,18 @@ export function CopyableText({ const resolvedVariant = variant ?? "icon-right"; if (resolvedVariant === "icon-right") { + // Real button semantics so keyboard and touch users can discover and trigger copying. + // The affordance is revealed on row hover, keyboard focus, and coarse (touch) pointers. const iconButton = ( - e.stopPropagation()} + aria-label={copied ? "Copied!" : (ariaLabel ?? "Copy")} className={cn( - "ml-1 flex size-6 items-center justify-center rounded border border-border-bright bg-background-hover", + "absolute -right-6 top-0 z-10 flex size-6 items-center justify-center rounded border border-border-bright bg-background-hover font-sans", asChild && "p-1", + "pointer-events-none opacity-0 transition-opacity group-hover:pointer-events-auto group-hover:opacity-100 focus-visible:opacity-100 [@media(pointer:coarse)]:pointer-events-auto [@media(pointer:coarse)]:opacity-100", copied ? "text-green-500" : "text-text-dimmed hover:border-border-bright hover:bg-background-raised hover:text-text-bright" @@ -46,35 +56,24 @@ export function CopyableText({ ) : ( )} - + ); return ( - setIsHovered(false)} - > - setIsHovered(true)}>{value} - e.stopPropagation()} - className={cn( - "absolute -right-6 top-0 z-10 size-6 font-sans", - isHovered ? "flex" : "hidden" - )} - > - {hideTooltip ? ( - iconButton - ) : ( - - )} - + + {value} + {hideTooltip ? ( + iconButton + ) : ( + // asChild so the Radix trigger merges onto our button instead of nesting a button. + + )} ); } diff --git a/apps/webapp/app/models/organization.server.ts b/apps/webapp/app/models/organization.server.ts index 9f7fb382315..a29f60d1789 100644 --- a/apps/webapp/app/models/organization.server.ts +++ b/apps/webapp/app/models/organization.server.ts @@ -15,6 +15,7 @@ import { env } from "~/env.server"; import { featuresForUrl } from "~/features.server"; import { createApiKeyForEnv, createPkApiKeyForEnv, envSlug } from "./api-key.server"; import { + getBillingAlerts, getDefaultEnvironmentConcurrencyLimit, isBillingConfigured, setBillingAlert, @@ -150,8 +151,8 @@ async function seedDefaultBillingAlerts(organizationId: string): Promise { }); const [error] = await tryCatch( - Promise.race([setBillingAlert(organizationId, buildDefaultBillingAlerts()), timeout]).finally( - () => clearTimeout(timer) + Promise.race([writeDefaultBillingAlertsIfUnset(organizationId), timeout]).finally(() => + clearTimeout(timer) ) ); if (error) { @@ -162,6 +163,19 @@ async function seedDefaultBillingAlerts(organizationId: string): Promise { } } +/** + * Only writes defaults when the org has no alerts yet. A slow seed that finishes + * after org creation returned would otherwise overwrite the user's first alert edit. + */ +async function writeDefaultBillingAlertsIfUnset(organizationId: string): Promise { + const existing = await getBillingAlerts(organizationId); + if (existing && existing.alertLevels.length > 0) { + return; + } + + await setBillingAlert(organizationId, buildDefaultBillingAlerts()); +} + export async function createEnvironment({ organization, project, diff --git a/apps/webapp/test/billingAlertsDefaults.test.ts b/apps/webapp/test/billingAlertsDefaults.test.ts index 6e655e38fea..73a6ffe0fc6 100644 --- a/apps/webapp/test/billingAlertsDefaults.test.ts +++ b/apps/webapp/test/billingAlertsDefaults.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from "vitest"; import { buildDefaultBillingAlerts } from "~/services/billingAlertsDefaults.server"; -import { ABSOLUTE_ALERT_BASE_CENTS } from "~/components/billing/billingAlertsFormat"; +import { + ABSOLUTE_ALERT_BASE_CENTS, + getAlertPreviewLimitCents, + storedAlertsToThresholds, + type BillingAlertsFormData, +} from "~/components/billing/billingAlertsFormat"; describe("buildDefaultBillingAlerts", () => { it("uses the absolute dollar base so alert levels read as dollar thresholds", () => { @@ -23,4 +28,26 @@ describe("buildDefaultBillingAlerts", () => { first.alertLevels.push(9999); expect(second.alertLevels).toEqual([5, 100, 500, 1000, 2500]); }); + + it("does not treat the default absolute-dollar payload as percentages of the limit", () => { + const defaults = buildDefaultBillingAlerts(); + const alerts: BillingAlertsFormData = { + amount: defaults.amount / 100, // API cents -> stored dollars ($1 base) + emails: defaults.emails ?? [], + alertLevels: [...(defaults.alertLevels ?? [])], + }; + + // Absolute (none) mode: levels stay dollar thresholds, not percentages of the limit. + expect(storedAlertsToThresholds(alerts, "none", 50_000, 50_000)).toEqual([ + 5, 100, 500, 1000, 2500, + ]); + + // With the real percentage mode (false for absolute alerts) the preview must not fall + // into the percentage branch, even though a level like 100 looks like a percent. Here a + // limit matches the $1 base, so inferring percentages would wrongly return the limit + // (50000) instead of the absolute base (100). + expect(getAlertPreviewLimitCents(alerts, 50_000, ABSOLUTE_ALERT_BASE_CENTS, false)).toBe( + ABSOLUTE_ALERT_BASE_CENTS + ); + }); }); From 558da58f11b8f0b45a5afd0dcb649e275b629522 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 23 Jul 2026 21:32:35 +0000 Subject: [PATCH 2/4] test(webapp): exercise isPercentageMode in alert preview raised-limit test Pass the isPercentageMode argument to getAlertPreviewLimitCents so the raised-limit regression test drives the percentage-mode branch instead of the fallback path, matching the current function signature. --- apps/webapp/test/billingAlertsFormat.test.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/apps/webapp/test/billingAlertsFormat.test.ts b/apps/webapp/test/billingAlertsFormat.test.ts index 98a61ca8b86..32815b8435c 100644 --- a/apps/webapp/test/billingAlertsFormat.test.ts +++ b/apps/webapp/test/billingAlertsFormat.test.ts @@ -86,20 +86,22 @@ describe("billingAlertsFormat", () => { ).toEqual([10, 50, 80]); expect( - getAlertPreviewLimitCents({ amount: 100, emails: [], alertLevels: [] }, 25_000, 10_000) + getAlertPreviewLimitCents({ amount: 100, emails: [], alertLevels: [] }, 25_000, 10_000, true) ).toBe(10_000); }); - it("previews saved percentage alerts against the current limit after it changes", () => { + it("previews saved percentage alerts against the current limit after it is raised", () => { // Percentage alerts saved against a $30 custom limit, limit later raised to $300. + // In percentage mode the preview must track the current limit (30_000 cents), not the + // $30 base snapshotted at the last alert save. const alerts = { amount: 30, emails: [], alertLevels: [0.75, 1.0] }; - expect(getAlertPreviewLimitCents(alerts, 30_000, 10_000)).toBe(30_000); + expect(getAlertPreviewLimitCents(alerts, 30_000, 10_000, true)).toBe(30_000); expect( - previewDollarAmountForPercent(100, getAlertPreviewLimitCents(alerts, 30_000, 10_000)) + previewDollarAmountForPercent(100, getAlertPreviewLimitCents(alerts, 30_000, 10_000, true)) ).toBe(300); expect( - previewDollarAmountForPercent(75, getAlertPreviewLimitCents(alerts, 30_000, 10_000)) + previewDollarAmountForPercent(75, getAlertPreviewLimitCents(alerts, 30_000, 10_000, true)) ).toBe(225); }); @@ -157,10 +159,10 @@ describe("billingAlertsFormat", () => { expect(storedAlertsToThresholds(normalized, "plan", 500, 500)).toEqual([75, 90]); - expect(getAlertPreviewLimitCents(normalized, 500, 500)).toBe(500); - expect(previewDollarAmountForPercent(75, getAlertPreviewLimitCents(normalized, 500, 500))).toBe( - 3.75 - ); + expect(getAlertPreviewLimitCents(normalized, 500, 500, true)).toBe(500); + expect( + previewDollarAmountForPercent(75, getAlertPreviewLimitCents(normalized, 500, 500, true)) + ).toBe(3.75); }); it("keeps seeded absolute defaults absolute when the plan limit is exactly $100", () => { From e991fa5a783333709e4deaaa5ff4f1ec5cd80667 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 23 Jul 2026 22:06:23 +0000 Subject: [PATCH 3/4] chore(webapp): server-changes note for the billing alerts fixes --- .server-changes/billing-alerts-review-fixes.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .server-changes/billing-alerts-review-fixes.md diff --git a/.server-changes/billing-alerts-review-fixes.md b/.server-changes/billing-alerts-review-fixes.md new file mode 100644 index 00000000000..2a2cd26b5c3 --- /dev/null +++ b/.server-changes/billing-alerts-review-fixes.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Fixes billing alert thresholds sometimes displaying as percentages when they were set as dollar amounts. Copy buttons are now reachable with the keyboard and visible on touch devices. From 40337e9c18c9e9b5061aef516c98d9a83ffbf99d Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 23 Jul 2026 22:46:31 +0000 Subject: [PATCH 4/4] fix(webapp): keep copy button tabbable and stop timed-out alert seed writing late Review feedback on #4359: the copy affordance sat inside a tooltip trigger that forced tabIndex -1, so keyboard users could not reach it; pass tabbable through. The default-alerts seed could also complete its write after the 5s timeout fired and overwrite a user's first alert edit; the timeout now aborts the seed before it writes. A fully atomic create-if-absent needs a platform API change and is out of scope here. --- .../components/primitives/CopyableText.tsx | 2 ++ apps/webapp/app/models/organization.server.ts | 26 ++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/components/primitives/CopyableText.tsx b/apps/webapp/app/components/primitives/CopyableText.tsx index 5ca0c3a7b98..9f4dca45b2b 100644 --- a/apps/webapp/app/components/primitives/CopyableText.tsx +++ b/apps/webapp/app/components/primitives/CopyableText.tsx @@ -66,11 +66,13 @@ export function CopyableText({ iconButton ) : ( // asChild so the Radix trigger merges onto our button instead of nesting a button. + // tabbable keeps the button in the tab order (the trigger sets tabIndex -1 otherwise). )} diff --git a/apps/webapp/app/models/organization.server.ts b/apps/webapp/app/models/organization.server.ts index a29f60d1789..9157da7f9af 100644 --- a/apps/webapp/app/models/organization.server.ts +++ b/apps/webapp/app/models/organization.server.ts @@ -146,13 +146,19 @@ async function seedDefaultBillingAlerts(organizationId: string): Promise { } let timer: NodeJS.Timeout | undefined; + const abort = new AbortController(); const timeout = new Promise((_, reject) => { - timer = setTimeout(() => reject(new Error("Timed out")), SEED_ALERTS_TIMEOUT_MS); + timer = setTimeout(() => { + // Stop the seed from writing after we give up: by then the user may have + // saved their own alerts, and a late default write would clobber them. + abort.abort(); + reject(new Error("Timed out")); + }, SEED_ALERTS_TIMEOUT_MS); }); const [error] = await tryCatch( - Promise.race([writeDefaultBillingAlertsIfUnset(organizationId), timeout]).finally(() => - clearTimeout(timer) + Promise.race([writeDefaultBillingAlertsIfUnset(organizationId, abort.signal), timeout]).finally( + () => clearTimeout(timer) ) ); if (error) { @@ -166,13 +172,25 @@ async function seedDefaultBillingAlerts(organizationId: string): Promise { /** * Only writes defaults when the org has no alerts yet. A slow seed that finishes * after org creation returned would otherwise overwrite the user's first alert edit. + * + * The read-then-write isn't atomic: the platform API has no conditional write and + * returns the same empty-levels shape for "no record" and "record cleared by the + * user". Both only matter inside the seconds-long seed window right after org + * creation; the abort check below keeps a timed-out seed from writing late. */ -async function writeDefaultBillingAlertsIfUnset(organizationId: string): Promise { +async function writeDefaultBillingAlertsIfUnset( + organizationId: string, + signal: AbortSignal +): Promise { const existing = await getBillingAlerts(organizationId); if (existing && existing.alertLevels.length > 0) { return; } + if (signal.aborted) { + return; + } + await setBillingAlert(organizationId, buildDefaultBillingAlerts()); }