Skip to content

Commit 4974290

Browse files
committed
fix(ui): keep the cost tracking removal confirmation open until it settles
The discount and margin removal confirmation used AlertDialogAction, which renders AlertDialogPrimitive.Close and dismisses the dialog on click. The dialog therefore disappeared while the removal request was still in flight, leaving the admin with no sign that anything happened and free to fire a duplicate removal. Swap the confirm control for a plain destructive Button, track an isRemoving pending state that disables Cancel and relabels Remove to "Removing...", and clear the pending removal in a finally block once the request settles.
1 parent e1f3d6e commit 4974290

2 files changed

Lines changed: 44 additions & 13 deletions

File tree

ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/cost_tracking_settings.test.tsx

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from "react";
22
import { describe, it, expect, vi, beforeEach } from "vitest";
3-
import { screen } from "@testing-library/react";
3+
import { act, screen, waitFor } from "@testing-library/react";
44
import userEvent from "@testing-library/user-event";
55
import { renderWithProviders } from "../../../../../tests/test-utils";
66
import CostTrackingSettings from "./cost_tracking_settings";
@@ -197,6 +197,32 @@ describe("CostTrackingSettings", () => {
197197
expect(screen.queryByRole("button", { name: "Remove" })).not.toBeInTheDocument();
198198
});
199199

200+
it("should hold the confirmation open while the removal is still in flight", async () => {
201+
mockDiscountConfig.mockReturnValue({ openai: 0.05 });
202+
let settleRemoval: () => void = () => {};
203+
mockRemoveDiscount.mockReturnValue(
204+
new Promise<void>((resolve) => {
205+
settleRemoval = resolve;
206+
}),
207+
);
208+
209+
const user = await expandAndRemove("Provider Discounts", "Remove discount for openai");
210+
await user.click(await screen.findByRole("button", { name: "Remove" }));
211+
212+
const removing = await screen.findByRole("button", { name: "Removing…" });
213+
expect(removing).toBeDisabled();
214+
expect(screen.getByRole("button", { name: "Cancel" })).toBeDisabled();
215+
216+
await act(async () => {
217+
settleRemoval();
218+
});
219+
220+
await waitFor(() => {
221+
expect(screen.queryByRole("alertdialog")).not.toBeInTheDocument();
222+
});
223+
expect(mockRemoveDiscount).toHaveBeenCalledWith("openai");
224+
});
225+
200226
it("should remove the margin once removal is confirmed", async () => {
201227
mockMarginConfig.mockReturnValue({ openai: 0.1 });
202228

ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/cost_tracking_settings.tsx

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { ChevronDown } from "lucide-react";
33
import { Modal, Form } from "antd";
44
import {
55
AlertDialog,
6-
AlertDialogAction,
76
AlertDialogCancel,
87
AlertDialogContent,
98
AlertDialogDescription,
@@ -66,6 +65,7 @@ const CostTrackingSettings: React.FC<CostTrackingSettingsProps> = ({ userID, use
6665
const [fixedAmountValue, setFixedAmountValue] = useState<string>("");
6766
const [models, setModels] = useState<string[]>([]);
6867
const [pendingRemoval, setPendingRemoval] = useState<PendingRemoval | null>(null);
68+
const [isRemoving, setIsRemoving] = useState(false);
6969
const [form] = Form.useForm();
7070
const [marginForm] = Form.useForm();
7171

@@ -131,14 +131,19 @@ const CostTrackingSettings: React.FC<CostTrackingSettingsProps> = ({ userID, use
131131
setPendingRemoval({ kind: "discount", provider, displayName: providerDisplayName });
132132
};
133133

134-
const handleConfirmRemoval = () => {
134+
const handleConfirmRemoval = async () => {
135135
if (!pendingRemoval) return;
136-
if (pendingRemoval.kind === "discount") {
137-
removeProvider(pendingRemoval.provider);
138-
} else {
139-
removeMargin(pendingRemoval.provider);
136+
setIsRemoving(true);
137+
try {
138+
if (pendingRemoval.kind === "discount") {
139+
await removeProvider(pendingRemoval.provider);
140+
} else {
141+
await removeMargin(pendingRemoval.provider);
142+
}
143+
} finally {
144+
setIsRemoving(false);
145+
setPendingRemoval(null);
140146
}
141-
setPendingRemoval(null);
142147
};
143148

144149
const handleAddMargin = async () => {
@@ -311,7 +316,7 @@ const CostTrackingSettings: React.FC<CostTrackingSettingsProps> = ({ userID, use
311316
</div>
312317

313318
{pendingRemoval && (
314-
<AlertDialog open onOpenChange={(open) => !open && setPendingRemoval(null)}>
319+
<AlertDialog open onOpenChange={(open) => !open && !isRemoving && setPendingRemoval(null)}>
315320
<AlertDialogContent>
316321
<AlertDialogHeader>
317322
<AlertDialogTitle>{REMOVAL_COPY[pendingRemoval.kind].title}</AlertDialogTitle>
@@ -321,10 +326,10 @@ const CostTrackingSettings: React.FC<CostTrackingSettingsProps> = ({ userID, use
321326
</AlertDialogDescription>
322327
</AlertDialogHeader>
323328
<AlertDialogFooter>
324-
<AlertDialogCancel>Cancel</AlertDialogCancel>
325-
<AlertDialogAction variant="destructive" onClick={handleConfirmRemoval}>
326-
Remove
327-
</AlertDialogAction>
329+
<AlertDialogCancel disabled={isRemoving}>Cancel</AlertDialogCancel>
330+
<Button variant="destructive" onClick={handleConfirmRemoval} disabled={isRemoving}>
331+
{isRemoving ? "Removing…" : "Remove"}
332+
</Button>
328333
</AlertDialogFooter>
329334
</AlertDialogContent>
330335
</AlertDialog>

0 commit comments

Comments
 (0)