mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(ui): migrate tag deletion to shared DeleteResourceModal (#33795)
The tag delete action moved into a Base UI dropdown menu when the tags table was migrated onto the shared DataTable. That menu is modal by default and holds a pointer-events lock on the page while it opens and closes, which left the hand-rolled inline confirmation modal unclickable, so deleting a tag stopped working Replace the inline modal with the shared DeleteResourceModal, which renders through an antd Modal portal that manages its own pointer-events and z-index, matching every other table's delete flow. Add a deleting loading state so the confirm button reflects progress and cannot be double-clicked Cover the wiring with a regression test that drives the delete flow through the shared modal and asserts tagDeleteCall runs with the tag name
This commit is contained in:
parent
966ff65fec
commit
b94311481e
2 changed files with 76 additions and 39 deletions
|
|
@ -1,7 +1,8 @@
|
|||
import { render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { tagListCall } from "@/components/networking";
|
||||
import { tagDeleteCall, tagListCall } from "@/components/networking";
|
||||
|
||||
import TagManagement from "./index";
|
||||
|
||||
|
|
@ -12,10 +13,23 @@ vi.mock("@/components/networking", () => ({
|
|||
modelInfoCall: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/components/molecules/notifications_manager", () => ({
|
||||
__esModule: true,
|
||||
default: {
|
||||
success: vi.fn(),
|
||||
fromBackend: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("./TagTable", () => ({
|
||||
__esModule: true,
|
||||
default: ({ isLoading }: { isLoading?: boolean }) => (
|
||||
<div data-testid="tag-table">{isLoading ? "table-loading" : "table-loaded"}</div>
|
||||
default: ({ isLoading, onDelete }: { isLoading?: boolean; onDelete: (tagName: string) => void }) => (
|
||||
<div data-testid="tag-table">
|
||||
{isLoading ? "table-loading" : "table-loaded"}
|
||||
<button data-testid="mock-delete-trigger" onClick={() => onDelete("test-tag")}>
|
||||
trigger
|
||||
</button>
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
|
||||
|
|
@ -30,6 +44,7 @@ vi.mock("./components/CreateTagModal", () => ({
|
|||
}));
|
||||
|
||||
const mockTagListCall = vi.mocked(tagListCall);
|
||||
const mockTagDeleteCall = vi.mocked(tagDeleteCall);
|
||||
|
||||
describe("TagManagement loading state", () => {
|
||||
beforeEach(() => {
|
||||
|
|
@ -57,3 +72,41 @@ describe("TagManagement loading state", () => {
|
|||
expect(mockTagListCall).toHaveBeenCalledWith("sk-test");
|
||||
});
|
||||
});
|
||||
|
||||
describe("TagManagement delete flow", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockTagListCall.mockResolvedValue({});
|
||||
});
|
||||
|
||||
it("should confirm deletion through the shared DeleteResourceModal and call tagDeleteCall with the tag name", async () => {
|
||||
const user = userEvent.setup();
|
||||
mockTagDeleteCall.mockResolvedValue({});
|
||||
render(<TagManagement accessToken="sk-test" userID="user-1" userRole="Admin" />);
|
||||
await screen.findByText("table-loaded");
|
||||
|
||||
expect(screen.queryByText("Tag Information")).not.toBeInTheDocument();
|
||||
|
||||
await user.click(screen.getByTestId("mock-delete-trigger"));
|
||||
|
||||
expect(await screen.findByText("Tag Information")).toBeInTheDocument();
|
||||
expect(screen.getByText("test-tag")).toBeInTheDocument();
|
||||
|
||||
await user.click(screen.getByRole("button", { name: /delete/i }));
|
||||
|
||||
expect(mockTagDeleteCall).toHaveBeenCalledWith("sk-test", "test-tag");
|
||||
});
|
||||
|
||||
it("should not call tagDeleteCall when the deletion is cancelled", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<TagManagement accessToken="sk-test" userID="user-1" userRole="Admin" />);
|
||||
await screen.findByText("table-loaded");
|
||||
|
||||
await user.click(screen.getByTestId("mock-delete-trigger"));
|
||||
await screen.findByText("Tag Information");
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Cancel" }));
|
||||
|
||||
expect(mockTagDeleteCall).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import { tagCreateCall, tagListCall, tagDeleteCall } from "@/components/networki
|
|||
import { Tag } from "@/components/tag_management/types";
|
||||
import TagTable from "./TagTable";
|
||||
import NotificationsManager from "@/components/molecules/notifications_manager";
|
||||
import DeleteResourceModal from "@/components/common_components/DeleteResourceModal";
|
||||
import CreateTagModal from "./components/CreateTagModal";
|
||||
|
||||
interface ModelInfo {
|
||||
|
|
@ -33,6 +34,7 @@ const TagManagement: React.FC<TagProps> = ({ accessToken, userID, userRole }) =>
|
|||
const [editTag, setEditTag] = useState<boolean>(false);
|
||||
const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false);
|
||||
const [tagToDelete, setTagToDelete] = useState<string | null>(null);
|
||||
const [isDeleting, setIsDeleting] = useState(false);
|
||||
const [lastRefreshed, setLastRefreshed] = useState("");
|
||||
const [availableModels, setAvailableModels] = useState<ModelInfo[]>([]);
|
||||
|
||||
|
|
@ -87,6 +89,7 @@ const TagManagement: React.FC<TagProps> = ({ accessToken, userID, userRole }) =>
|
|||
|
||||
const confirmDelete = async () => {
|
||||
if (!accessToken || !tagToDelete) return;
|
||||
setIsDeleting(true);
|
||||
try {
|
||||
await tagDeleteCall(accessToken, tagToDelete);
|
||||
NotificationsManager.success("Tag deleted successfully");
|
||||
|
|
@ -94,9 +97,11 @@ const TagManagement: React.FC<TagProps> = ({ accessToken, userID, userRole }) =>
|
|||
} catch (error) {
|
||||
console.error("Error deleting tag:", error);
|
||||
NotificationsManager.fromBackend("Error deleting tag: " + error);
|
||||
} finally {
|
||||
setIsDeleting(false);
|
||||
setIsDeleteModalOpen(false);
|
||||
setTagToDelete(null);
|
||||
}
|
||||
setIsDeleteModalOpen(false);
|
||||
setTagToDelete(null);
|
||||
};
|
||||
|
||||
useEffect(() => {
|
||||
|
|
@ -189,40 +194,19 @@ const TagManagement: React.FC<TagProps> = ({ accessToken, userID, userRole }) =>
|
|||
/>
|
||||
|
||||
{/* Delete Confirmation Modal */}
|
||||
{isDeleteModalOpen && (
|
||||
<div className="fixed z-10 inset-0 overflow-y-auto">
|
||||
<div className="flex items-end justify-center min-h-screen pt-4 px-4 pb-20 text-center sm:block sm:p-0">
|
||||
<div className="fixed inset-0 transition-opacity" aria-hidden="true">
|
||||
<div className="absolute inset-0 bg-gray-500 opacity-75"></div>
|
||||
</div>
|
||||
<div className="inline-block align-bottom bg-white rounded-lg text-left overflow-hidden shadow-xl transform transition-all sm:my-8 sm:align-middle sm:max-w-lg sm:w-full">
|
||||
<div className="bg-white px-4 pt-5 pb-4 sm:p-6 sm:pb-4">
|
||||
<div className="sm:flex sm:items-start">
|
||||
<div className="mt-3 text-center sm:mt-0 sm:ml-4 sm:text-left">
|
||||
<h3 className="text-lg leading-6 font-medium text-gray-900">Delete Tag</h3>
|
||||
<div className="mt-2">
|
||||
<p className="text-sm text-gray-500">Are you sure you want to delete this tag?</p>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<div className="bg-gray-50 px-4 py-3 sm:px-6 sm:flex sm:flex-row-reverse">
|
||||
<Button onClick={confirmDelete} color="red" className="ml-2">
|
||||
Delete
|
||||
</Button>
|
||||
<Button
|
||||
onClick={() => {
|
||||
setIsDeleteModalOpen(false);
|
||||
setTagToDelete(null);
|
||||
}}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
<DeleteResourceModal
|
||||
isOpen={isDeleteModalOpen}
|
||||
title="Delete Tag"
|
||||
message="Are you sure you want to delete this tag? This action cannot be undone."
|
||||
resourceInformationTitle="Tag Information"
|
||||
resourceInformation={[{ label: "Tag Name", value: tagToDelete, code: true }]}
|
||||
onCancel={() => {
|
||||
setIsDeleteModalOpen(false);
|
||||
setTagToDelete(null);
|
||||
}}
|
||||
onOk={confirmDelete}
|
||||
confirmLoading={isDeleting}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue