Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .server-changes/billing-alerts-review-fixes.md
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 2 additions & 1 deletion apps/webapp/app/components/billing/BillingAlertsSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ export function BillingAlertsSection({
const alertPreviewLimitCents = getAlertPreviewLimitCents(
alerts,
effectiveLimitCents,
planLimitCents
planLimitCents,
isPercentageMode
);
const maxAlerts = isPercentageMode ? MAX_PERCENTAGE_ALERTS : MAX_ABSOLUTE_ALERTS;

Expand Down
16 changes: 13 additions & 3 deletions apps/webapp/app/components/billing/billingAlertsFormat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
57 changes: 29 additions & 28 deletions apps/webapp/app/components/primitives/CopyableText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export function CopyableText({
asChild,
variant,
hideTooltip,
ariaLabel,
}: {
value: string;
copyValue?: string;
Expand All @@ -24,18 +25,27 @@ 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);

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 = (
<span
<button
type="button"
onClick={copy}
onMouseDown={(e) => 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"
Expand All @@ -46,35 +56,26 @@ export function CopyableText({
) : (
<ClipboardIcon className="size-3.5" />
)}
</span>
</button>
);

return (
<span
className={cn("group relative inline-flex h-6 items-center", className)}
onMouseLeave={() => setIsHovered(false)}
>
<span onMouseEnter={() => setIsHovered(true)}>{value}</span>
<span
onClick={copy}
onMouseDown={(e) => e.stopPropagation()}
className={cn(
"absolute -right-6 top-0 z-10 size-6 font-sans",
isHovered ? "flex" : "hidden"
)}
>
{hideTooltip ? (
iconButton
) : (
<SimpleTooltip
button={iconButton}
content={copied ? "Copied!" : "Copy"}
className="font-sans"
disableHoverableContent
asChild={asChild}
/>
)}
</span>
<span className={cn("group relative inline-flex h-6 items-center", className)}>
<span>{value}</span>
{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).
<SimpleTooltip
button={iconButton}
content={copied ? "Copied!" : "Copy"}
className="font-sans"
disableHoverableContent
tabbable
asChild
/>
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
)}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
</span>
);
}
Expand Down
36 changes: 34 additions & 2 deletions apps/webapp/app/models/organization.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -145,12 +146,18 @@ async function seedDefaultBillingAlerts(organizationId: string): Promise<void> {
}

let timer: NodeJS.Timeout | undefined;
const abort = new AbortController();
const timeout = new Promise<never>((_, 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)
)
);
Expand All @@ -162,6 +169,31 @@ async function seedDefaultBillingAlerts(organizationId: string): Promise<void> {
}
}

/**
* 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<void> {
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,
Expand Down
29 changes: 28 additions & 1 deletion apps/webapp/test/billingAlertsDefaults.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand All @@ -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
);
});
});
20 changes: 11 additions & 9 deletions apps/webapp/test/billingAlertsFormat.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down Expand Up @@ -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", () => {
Expand Down