diff --git a/.server-changes/billing-alerts-review-fixes.md b/.server-changes/billing-alerts-review-fixes.md new file mode 100644 index 0000000000..2a2cd26b5c --- /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. diff --git a/apps/webapp/app/components/billing/BillingAlertsSection.tsx b/apps/webapp/app/components/billing/BillingAlertsSection.tsx index ce2f5e5e0f..6997972473 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 5940405a84..ebcd918dea 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 9634ad1ac0..9f4dca45b2 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,26 @@ 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. + // 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 9f7fb38231..9157da7f9a 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, @@ -145,12 +146,18 @@ 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([setBillingAlert(organizationId, buildDefaultBillingAlerts()), timeout]).finally( + Promise.race([writeDefaultBillingAlertsIfUnset(organizationId, abort.signal), timeout]).finally( () => clearTimeout(timer) ) ); @@ -162,6 +169,31 @@ 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, + signal: AbortSignal +): Promise { + const existing = await getBillingAlerts(organizationId); + if (existing && existing.alertLevels.length > 0) { + return; + } + + if (signal.aborted) { + 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 6e655e38fe..73a6ffe0fc 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 + ); + }); }); diff --git a/apps/webapp/test/billingAlertsFormat.test.ts b/apps/webapp/test/billingAlertsFormat.test.ts index 98a61ca8b8..32815b8435 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", () => {