From 2c3c8aa4ea2c0bdbee1e7d80e60ea1923b8c7f99 Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Thu, 23 Apr 2026 21:04:13 -0700 Subject: [PATCH 1/4] Move "Store Prompts in Spend Logs" toggle to Admin Settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the "Store Prompts in Spend Logs" and "Maximum Spend Logs Retention Period" settings were surfaced via a gear-icon modal on the Logs page. The gear was visible to every authenticated user even though the backend endpoints (/config/update, /config/list) require PROXY_ADMIN — so non-admins could open the modal but the request would 403 on load and save, giving a confusing UX. Move the controls into a new "Logging Settings" tab under Admin Settings, which is already gated to admins at the sidebar. Remove the gear button and the onOpenSettings prop chain (ConfigInfoMessage → LogDetailContent → LogDetailsDrawer). ConfigInfoMessage now points users to "Admin Settings → Logging Settings" inline. --- .../src/components/AdminPanel.tsx | 6 + .../LoggingSettings/LoggingSettings.test.tsx} | 255 +++++------------- .../LoggingSettings/LoggingSettings.tsx | 150 +++++++++++ .../view_logs/ConfigInfoMessage.test.tsx | 22 +- .../view_logs/ConfigInfoMessage.tsx | 17 +- .../LogDetailContent.test.tsx | 21 -- .../LogDetailsDrawer/LogDetailContent.tsx | 5 +- .../LogDetailsDrawer/LogDetailsDrawer.tsx | 3 - .../SpendLogsSettingsModal.tsx | 156 ----------- .../src/components/view_logs/index.tsx | 19 +- 10 files changed, 228 insertions(+), 426 deletions(-) rename ui/litellm-dashboard/src/components/{view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.test.tsx => Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx} (53%) create mode 100644 ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx delete mode 100644 ui/litellm-dashboard/src/components/view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.tsx diff --git a/ui/litellm-dashboard/src/components/AdminPanel.tsx b/ui/litellm-dashboard/src/components/AdminPanel.tsx index 7528b081ff3..e4aec7706a0 100644 --- a/ui/litellm-dashboard/src/components/AdminPanel.tsx +++ b/ui/litellm-dashboard/src/components/AdminPanel.tsx @@ -21,6 +21,7 @@ import { useBaseUrl } from "./constants"; import NotificationsManager from "./molecules/notifications_manager"; import { addAllowedIP, deleteAllowedIP, getAllowedIPs, getSSOSettings } from "./networking"; import SCIMConfig from "./SCIM"; +import LoggingSettings from "./Settings/AdminSettings/LoggingSettings/LoggingSettings"; import SSOSettings from "./Settings/AdminSettings/SSOSettings/SSOSettings"; import UISettings from "./Settings/AdminSettings/UISettings/UISettings"; import HashicorpVault from "./Settings/AdminSettings/HashicorpVault/HashicorpVault"; @@ -362,6 +363,11 @@ const AdminPanel: React.FC = ({ proxySettings }) => { ), children: , }, + { + key: "logging-settings", + label: "Logging Settings", + children: , + }, { key: "hashicorp-vault", label: "Hashicorp Vault", diff --git a/ui/litellm-dashboard/src/components/view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx similarity index 53% rename from ui/litellm-dashboard/src/components/view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.test.tsx rename to ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx index 40d06d90461..228e899a3a2 100644 --- a/ui/litellm-dashboard/src/components/view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx @@ -5,11 +5,20 @@ import { parseErrorMessage } from "@/components/shared/errorUtils"; import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { renderWithProviders } from "../../../../tests/test-utils"; -import SpendLogsSettingsModal from "./SpendLogsSettingsModal"; +import { renderWithProviders } from "../../../../../tests/test-utils"; +import LoggingSettings from "./LoggingSettings"; vi.mock("@/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs"); -vi.mock("@/app/(dashboard)/hooks/proxyConfig/useProxyConfig"); +vi.mock("@/app/(dashboard)/hooks/proxyConfig/useProxyConfig", async () => { + const actual = await vi.importActual( + "@/app/(dashboard)/hooks/proxyConfig/useProxyConfig", + ); + return { + ...actual, + useProxyConfig: vi.fn(), + useDeleteProxyConfigField: vi.fn(), + }; +}); vi.mock("@/components/molecules/notifications_manager", () => ({ default: { success: vi.fn(), @@ -26,19 +35,11 @@ const mockUseDeleteProxyConfigField = vi.mocked(useDeleteProxyConfigField); const mockNotificationsManager = vi.mocked(NotificationsManager); const mockParseErrorMessage = vi.mocked(parseErrorMessage); -describe("SpendLogsSettingsModal", () => { - const mockOnCancel = vi.fn(); - const mockOnSuccess = vi.fn(); +describe("LoggingSettings", () => { const mockMutateAsync = vi.fn(); const mockDeleteField = vi.fn(); const mockRefetch = vi.fn(); - const defaultProps = { - isVisible: true, - onCancel: mockOnCancel, - onSuccess: mockOnSuccess, - }; - beforeEach(() => { vi.clearAllMocks(); mockUseStoreRequestInSpendLogs.mockReturnValue({ @@ -57,50 +58,19 @@ describe("SpendLogsSettingsModal", () => { mockParseErrorMessage.mockImplementation((error: any) => error?.message || String(error)); }); - it("should render the modal", () => { - renderWithProviders(); - expect(screen.getByRole("dialog")).toBeInTheDocument(); - expect(screen.getByText("Spend Logs Settings")).toBeInTheDocument(); - }); - - it("should render form fields with initial values", () => { - renderWithProviders(); + it("should render the card with title and form fields", () => { + renderWithProviders(); + expect(screen.getByText("Logging Settings")).toBeInTheDocument(); expect(screen.getByText("Store Prompts in Spend Logs")).toBeInTheDocument(); expect(screen.getByLabelText("Maximum Spend Logs Retention Period (Optional)")).toBeInTheDocument(); expect(screen.getByPlaceholderText("e.g., 7d, 30d")).toBeInTheDocument(); - }); - - it("should render cancel and save buttons", () => { - renderWithProviders(); - - expect(screen.getByRole("button", { name: "Cancel" })).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Save Settings" })).toBeInTheDocument(); }); - it("should call onCancel when cancel button is clicked", async () => { - const user = userEvent.setup(); - renderWithProviders(); - - const cancelButton = screen.getByRole("button", { name: "Cancel" }); - await user.click(cancelButton); - - expect(mockOnCancel).toHaveBeenCalledTimes(1); - }); - - it("should call onCancel when modal close button is clicked", async () => { - const user = userEvent.setup(); - renderWithProviders(); - - const closeButton = screen.getByRole("button", { name: /close/i }); - await user.click(closeButton); - - expect(mockOnCancel).toHaveBeenCalledTimes(1); - }); - it("should toggle store prompts switch", async () => { const user = userEvent.setup(); - renderWithProviders(); + renderWithProviders(); const switchElement = screen.getByRole("switch"); expect(switchElement).not.toBeChecked(); @@ -114,7 +84,7 @@ describe("SpendLogsSettingsModal", () => { it("should update retention period input", async () => { const user = userEvent.setup(); - renderWithProviders(); + renderWithProviders(); const retentionInput = screen.getByPlaceholderText("e.g., 7d, 30d"); await user.type(retentionInput, "30d"); @@ -124,13 +94,13 @@ describe("SpendLogsSettingsModal", () => { it("should submit form with store prompts enabled and retention period", async () => { const user = userEvent.setup(); - mockMutateAsync.mockImplementation(async (params, options) => { + mockMutateAsync.mockImplementation(async (_params, options) => { await Promise.resolve(); options?.onSuccess?.(); return { message: "Success" }; }); - renderWithProviders(); + renderWithProviders(); const switchElement = screen.getByRole("switch"); await user.click(switchElement); @@ -148,21 +118,21 @@ describe("SpendLogsSettingsModal", () => { store_prompts_in_spend_logs: true, maximum_spend_logs_retention_period: "30d", }, - expect.any(Object) + expect.any(Object), ); }); }); - it("should submit form with store prompts disabled and no retention period", async () => { + it("should delete retention period field when left empty on submit", async () => { const user = userEvent.setup(); mockDeleteField.mockResolvedValue({ message: "Field deleted successfully" }); - mockMutateAsync.mockImplementation(async (params, options) => { + mockMutateAsync.mockImplementation(async (_params, options) => { await Promise.resolve(); options?.onSuccess?.(); return { message: "Success" }; }); - renderWithProviders(); + renderWithProviders(); const saveButton = screen.getByRole("button", { name: "Save Settings" }); await user.click(saveButton); @@ -173,207 +143,106 @@ describe("SpendLogsSettingsModal", () => { { store_prompts_in_spend_logs: false, }, - expect.any(Object) + expect.any(Object), ); }); }); - it("should show success notification and call onSuccess on successful submission", async () => { + it("should show success notification on successful submission", async () => { const user = userEvent.setup(); mockDeleteField.mockResolvedValue({ message: "Field deleted successfully" }); - mockMutateAsync.mockImplementation(async (params, options) => { + mockMutateAsync.mockImplementation(async (_params, options) => { await Promise.resolve(); options?.onSuccess?.(); return { message: "Success" }; }); - renderWithProviders(); + renderWithProviders(); const saveButton = screen.getByRole("button", { name: "Save Settings" }); await user.click(saveButton); await waitFor(() => { expect(mockNotificationsManager.success).toHaveBeenCalledWith("Spend logs settings updated successfully"); - expect(mockRefetch).toHaveBeenCalled(); - expect(mockOnSuccess).toHaveBeenCalledTimes(1); }); }); - it("should show error notification when submission fails", async () => { + it("should show error notification when submission throws", async () => { const user = userEvent.setup(); const error = new Error("Network error"); mockMutateAsync.mockRejectedValue(error); mockParseErrorMessage.mockReturnValue("Network error"); - renderWithProviders(); + renderWithProviders(); const saveButton = screen.getByRole("button", { name: "Save Settings" }); await user.click(saveButton); await waitFor(() => { - expect(mockNotificationsManager.fromBackend).toHaveBeenCalledWith("Failed to save spend logs settings: Network error"); + expect(mockNotificationsManager.fromBackend).toHaveBeenCalledWith( + "Failed to save spend logs settings: Network error", + ); }); }); - it("should show error notification from onError callback", async () => { + it("should show error notification via onError callback", async () => { const user = userEvent.setup(); const error = new Error("Backend error"); - mockMutateAsync.mockImplementation((params, options) => { + mockMutateAsync.mockImplementation((_params, options) => { options?.onError?.(error); return Promise.reject(error); }); mockParseErrorMessage.mockReturnValue("Backend error"); - renderWithProviders(); + renderWithProviders(); const saveButton = screen.getByRole("button", { name: "Save Settings" }); await user.click(saveButton); await waitFor(() => { - expect(mockNotificationsManager.fromBackend).toHaveBeenCalledWith("Failed to save spend logs settings: Backend error"); + expect(mockNotificationsManager.fromBackend).toHaveBeenCalledWith( + "Failed to save spend logs settings: Backend error", + ); }); }); - it("should disable cancel button when pending", () => { + it("should show loading state on save button when update pending", () => { mockUseStoreRequestInSpendLogs.mockReturnValue({ mutateAsync: mockMutateAsync, isPending: true, } as any); - renderWithProviders(); + renderWithProviders(); - const cancelButton = screen.getByRole("button", { name: "Cancel" }); - expect(cancelButton).toBeDisabled(); + const saveButton = screen.getByRole("button", { name: /Saving/i }); + expect(saveButton).toBeInTheDocument(); + expect(saveButton.className).toContain("ant-btn-loading"); }); - it("should disable cancel button when deleting field", () => { + it("should show loading state on save button when delete pending", () => { mockUseDeleteProxyConfigField.mockReturnValue({ mutateAsync: mockDeleteField, isPending: true, } as any); - renderWithProviders(); + renderWithProviders(); - const cancelButton = screen.getByRole("button", { name: "Cancel" }); - expect(cancelButton).toBeDisabled(); + const saveButton = screen.getByRole("button", { name: /Saving/i }); + expect(saveButton).toBeInTheDocument(); + expect(saveButton.className).toContain("ant-btn-loading"); }); - it("should disable cancel button when loading config", () => { + it("should disable save button while config is loading", () => { mockUseProxyConfig.mockReturnValue({ data: undefined, isLoading: true, refetch: mockRefetch, } as any); - renderWithProviders(); - - const cancelButton = screen.getByRole("button", { name: "Cancel" }); - expect(cancelButton).toBeDisabled(); - }); - - it("should show loading state on save button when pending", () => { - mockUseStoreRequestInSpendLogs.mockReturnValue({ - mutateAsync: mockMutateAsync, - isPending: true, - } as any); - - renderWithProviders(); - - const saveButton = screen.getByRole("button", { name: /Saving/i }); - expect(saveButton).toBeInTheDocument(); - expect(saveButton.className).toContain("ant-btn-loading"); - }); - - it("should show loading state on save button when deleting field", () => { - mockUseDeleteProxyConfigField.mockReturnValue({ - mutateAsync: mockDeleteField, - isPending: true, - } as any); - - renderWithProviders(); - - const saveButton = screen.getByRole("button", { name: /Saving/i }); - expect(saveButton).toBeInTheDocument(); - expect(saveButton.className).toContain("ant-btn-loading"); - }); - - it("should call onCancel when cancel button is clicked after modifying form", async () => { - const user = userEvent.setup(); - renderWithProviders(); - - const switchElement = screen.getByRole("switch"); - await user.click(switchElement); - - const retentionInput = screen.getByPlaceholderText("e.g., 7d, 30d"); - await user.type(retentionInput, "30d"); - - expect(switchElement).toBeChecked(); - expect(retentionInput).toHaveValue("30d"); - - const cancelButton = screen.getByRole("button", { name: "Cancel" }); - await user.click(cancelButton); - - expect(mockOnCancel).toHaveBeenCalledTimes(1); - }); - - it("should call refetch after successful submission", async () => { - const user = userEvent.setup(); - mockDeleteField.mockResolvedValue({ message: "Field deleted successfully" }); - mockMutateAsync.mockImplementation(async (params, options) => { - await Promise.resolve(); - options?.onSuccess?.(); - return { message: "Success" }; - }); - - renderWithProviders(); - - const switchElement = screen.getByRole("switch"); - await user.click(switchElement); - - const retentionInput = screen.getByPlaceholderText("e.g., 7d, 30d"); - await user.type(retentionInput, "30d"); - - expect(switchElement).toBeChecked(); - expect(retentionInput).toHaveValue("30d"); + renderWithProviders(); const saveButton = screen.getByRole("button", { name: "Save Settings" }); - await user.click(saveButton); - - await waitFor(() => { - expect(mockNotificationsManager.success).toHaveBeenCalled(); - expect(mockRefetch).toHaveBeenCalled(); - }); - }); - - it("should not call onSuccess when it is not provided", async () => { - const user = userEvent.setup(); - mockDeleteField.mockResolvedValue({ message: "Field deleted successfully" }); - mockMutateAsync.mockImplementation(async (params, options) => { - await Promise.resolve(); - options?.onSuccess?.(); - return { message: "Success" }; - }); - - renderWithProviders(); - - const saveButton = screen.getByRole("button", { name: "Save Settings" }); - await user.click(saveButton); - - await waitFor(() => { - expect(mockNotificationsManager.success).toHaveBeenCalled(); - }); - }); - - it("should not render modal when isVisible is false", () => { - renderWithProviders(); - - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - }); - - it("should call refetch when modal opens", () => { - renderWithProviders(); - - expect(mockRefetch).toHaveBeenCalledTimes(1); + expect(saveButton).toBeDisabled(); }); it("should render form with initial values from config data", () => { @@ -400,7 +269,7 @@ describe("SpendLogsSettingsModal", () => { refetch: mockRefetch, } as any); - renderWithProviders(); + renderWithProviders(); const switchElement = screen.getByRole("switch"); const retentionInput = screen.getByPlaceholderText("e.g., 7d, 30d"); @@ -416,13 +285,11 @@ describe("SpendLogsSettingsModal", () => { refetch: mockRefetch, } as any); - renderWithProviders(); + renderWithProviders(); - // Check that switch and input are not present when loading (skeletons are shown instead) expect(screen.queryByRole("switch")).not.toBeInTheDocument(); expect(screen.queryByPlaceholderText("e.g., 7d, 30d")).not.toBeInTheDocument(); - // Check for skeleton elements (Ant Design Skeleton.Input renders with ant-skeleton class) const skeletons = document.querySelectorAll(".ant-skeleton"); expect(skeletons.length).toBeGreaterThan(0); }); @@ -431,13 +298,13 @@ describe("SpendLogsSettingsModal", () => { const user = userEvent.setup(); const deleteError = new Error("Field does not exist"); mockDeleteField.mockRejectedValue(deleteError); - mockMutateAsync.mockImplementation(async (params, options) => { + mockMutateAsync.mockImplementation(async (_params, options) => { await Promise.resolve(); options?.onSuccess?.(); return { message: "Success" }; }); - renderWithProviders(); + renderWithProviders(); const saveButton = screen.getByRole("button", { name: "Save Settings" }); await user.click(saveButton); @@ -448,22 +315,22 @@ describe("SpendLogsSettingsModal", () => { { store_prompts_in_spend_logs: false, }, - expect.any(Object) + expect.any(Object), ); expect(mockNotificationsManager.success).toHaveBeenCalled(); }); }); - it("should submit form with only store prompts enabled and no retention period", async () => { + it("should submit with only store prompts enabled when retention is empty", async () => { const user = userEvent.setup(); mockDeleteField.mockResolvedValue({ message: "Field deleted successfully" }); - mockMutateAsync.mockImplementation(async (params, options) => { + mockMutateAsync.mockImplementation(async (_params, options) => { await Promise.resolve(); options?.onSuccess?.(); return { message: "Success" }; }); - renderWithProviders(); + renderWithProviders(); const switchElement = screen.getByRole("switch"); await user.click(switchElement); @@ -477,7 +344,7 @@ describe("SpendLogsSettingsModal", () => { { store_prompts_in_spend_logs: true, }, - expect.any(Object) + expect.any(Object), ); }); }); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx new file mode 100644 index 00000000000..c3aaebd3bf2 --- /dev/null +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx @@ -0,0 +1,150 @@ +"use client"; + +import { + ConfigType, + GeneralSettingsFieldName, + useDeleteProxyConfigField, + useProxyConfig, +} from "@/app/(dashboard)/hooks/proxyConfig/useProxyConfig"; +import { + StoreRequestInSpendLogsParams, + useStoreRequestInSpendLogs, +} from "@/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs"; +import NotificationsManager from "@/components/molecules/notifications_manager"; +import { parseErrorMessage } from "@/components/shared/errorUtils"; +import { ClockCircleOutlined } from "@ant-design/icons"; +import { Button, Card, Form, Input, Skeleton, Space, Switch, Typography } from "antd"; +import React, { useMemo } from "react"; + +const LoggingSettings: React.FC = () => { + const [form] = Form.useForm(); + const { mutateAsync, isPending } = useStoreRequestInSpendLogs(); + const { mutateAsync: deleteField, isPending: isDeletingField } = useDeleteProxyConfigField(); + const { data: proxyConfigData, isLoading: isLoadingConfig } = useProxyConfig(ConfigType.GENERAL_SETTINGS); + const storePromptsValue = Form.useWatch("store_prompts_in_spend_logs", form); + + const initialValues = useMemo(() => { + if (!proxyConfigData) { + return { + store_prompts_in_spend_logs: false, + maximum_spend_logs_retention_period: undefined, + }; + } + + const storePromptsField = proxyConfigData.find((field) => field.field_name === "store_prompts_in_spend_logs"); + const retentionPeriodField = proxyConfigData.find( + (field) => field.field_name === "maximum_spend_logs_retention_period", + ); + + return { + store_prompts_in_spend_logs: storePromptsField?.field_value ?? false, + maximum_spend_logs_retention_period: retentionPeriodField?.field_value ?? undefined, + }; + }, [proxyConfigData]); + + const handleFormSubmit = async (formValues: StoreRequestInSpendLogsParams) => { + try { + const retentionPeriodValue = formValues.maximum_spend_logs_retention_period; + const shouldDeleteRetentionPeriod = + !retentionPeriodValue || + (typeof retentionPeriodValue === "string" && retentionPeriodValue.trim() === ""); + + if (shouldDeleteRetentionPeriod) { + try { + await deleteField({ + config_type: ConfigType.GENERAL_SETTINGS, + field_name: GeneralSettingsFieldName.MAXIMUM_SPEND_LOGS_RETENTION_PERIOD, + }); + } catch (deleteError) { + console.warn("Failed to delete retention period field (may not exist):", deleteError); + } + } + + const updateParams: StoreRequestInSpendLogsParams = { + store_prompts_in_spend_logs: formValues.store_prompts_in_spend_logs, + ...(retentionPeriodValue && + typeof retentionPeriodValue === "string" && + retentionPeriodValue.trim() !== "" && { + maximum_spend_logs_retention_period: retentionPeriodValue, + }), + }; + + await mutateAsync(updateParams, { + onSuccess: () => { + NotificationsManager.success("Spend logs settings updated successfully"); + }, + onError: (error) => { + NotificationsManager.fromBackend("Failed to save spend logs settings: " + parseErrorMessage(error)); + }, + }); + } catch (error) { + NotificationsManager.fromBackend("Failed to save spend logs settings: " + parseErrorMessage(error)); + } + }; + + return ( + + + + Proxy-wide settings that control how request and response data are written to spend logs. + + +
+ f.field_name === "store_prompts_in_spend_logs")?.field_description || + "When enabled, prompts will be stored in spend logs for tracking and analysis purposes." + } + valuePropName="checked" + > + {isLoadingConfig ? ( + + ) : ( + form.setFieldValue("store_prompts_in_spend_logs", checked)} + /> + )} + + + f.field_name === "maximum_spend_logs_retention_period") + ?.field_description || + "Set the maximum retention period for spend logs (e.g., '7d' for 7 days, '30d' for 30 days). Leave empty for no limit." + } + > + {isLoadingConfig ? ( + + ) : ( + } /> + )} + + + + + +
+
+
+ ); +}; + +export default LoggingSettings; diff --git a/ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.test.tsx b/ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.test.tsx index 9e28b27cece..ad9b724ba80 100644 --- a/ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.test.tsx +++ b/ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.test.tsx @@ -1,6 +1,5 @@ import { render, screen } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; -import userEvent from "@testing-library/user-event"; +import { describe, expect, it } from "vitest"; import { ConfigInfoMessage } from "./ConfigInfoMessage"; describe("ConfigInfoMessage", () => { @@ -19,23 +18,8 @@ describe("ConfigInfoMessage", () => { expect(screen.getByText(/store_prompts_in_spend_logs: true/)).toBeInTheDocument(); }); - it("should render the settings button when onOpenSettings is provided", () => { - render( {}} />); - expect(screen.getByText("open the settings")).toBeInTheDocument(); - }); - - it("should not render the settings button when onOpenSettings is omitted", () => { + it("should reference Admin Settings \u2192 Logging Settings", () => { render(); - expect(screen.queryByText("open the settings")).not.toBeInTheDocument(); - }); - - it("should call onOpenSettings when the settings button is clicked", async () => { - const user = userEvent.setup(); - const onOpenSettings = vi.fn(); - - render(); - await user.click(screen.getByText("open the settings")); - - expect(onOpenSettings).toHaveBeenCalledOnce(); + expect(screen.getByText(/Admin Settings → Logging Settings/)).toBeInTheDocument(); }); }); diff --git a/ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.tsx b/ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.tsx index 509b1c73de2..a231a099716 100644 --- a/ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.tsx +++ b/ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.tsx @@ -2,10 +2,9 @@ import React from "react"; interface ConfigInfoMessageProps { show: boolean; - onOpenSettings?: () => void; } -export const ConfigInfoMessage: React.FC = ({ show, onOpenSettings }) => { +export const ConfigInfoMessage: React.FC = ({ show }) => { if (!show) return null; return ( @@ -31,18 +30,8 @@ export const ConfigInfoMessage: React.FC = ({ show, onOp

Request/Response Data Not Available

To view request and response details, enable prompt storage in your LiteLLM configuration by adding the - following to your proxy_config.yaml file - {onOpenSettings && ( - <> or{" "} - - {" "}to configure this directly. - - )} + following to your proxy_config.yaml file, or toggle + the setting in Admin Settings → Logging Settings.

           {`general_settings:
diff --git a/ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailContent.test.tsx b/ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailContent.test.tsx
index 992063fb69b..a2da5136755 100644
--- a/ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailContent.test.tsx
+++ b/ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailContent.test.tsx
@@ -171,27 +171,6 @@ describe("LogDetailContent", () => {
     expect(screen.queryByText("Request/Response Data Not Available")).not.toBeInTheDocument();
   });
 
-  it("should call onOpenSettings when user clicks open settings in ConfigInfoMessage", async () => {
-    const onOpenSettings = vi.fn();
-    const user = userEvent.setup();
-
-    render(
-      ,
-    );
-
-    const settingsButton = screen.getByRole("button", { name: /open the settings/i });
-    await user.click(settingsButton);
-
-    expect(onOpenSettings).toHaveBeenCalledTimes(1);
-  });
-
   it("should display loading state when isLoadingDetails is true", () => {
     render(
        void;
   /** When true, log details (messages/response) are still being lazy-loaded. */
   isLoadingDetails?: boolean;
   accessToken?: string | null;
@@ -51,7 +50,7 @@ export interface LogDetailContentProps {
  * Designed to be placed inside LogDetailsDrawer's right panel so it can
  * be reused for both single-log and session-mode views.
  */
-export function LogDetailContent({ logEntry, onOpenSettings, isLoadingDetails = false, accessToken }: LogDetailContentProps) {
+export function LogDetailContent({ logEntry, isLoadingDetails = false, accessToken }: LogDetailContentProps) {
   const metadata = logEntry.metadata || {};
   const hasError = metadata.status === "failure";
   const errorInfo = hasError ? metadata.error_information : null;
@@ -153,7 +152,7 @@ export function LogDetailContent({ logEntry, onOpenSettings, isLoadingDetails =
       {/* Configuration Info Message */}
       {missingData && (
         
- +
)} diff --git a/ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx b/ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx index 036a24c045a..39315b0b585 100644 --- a/ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx +++ b/ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx @@ -26,7 +26,6 @@ export interface LogDetailsDrawerProps { logEntry: LogEntry | null; sessionId?: string | null; accessToken?: string | null; - onOpenSettings?: () => void; allLogs?: LogEntry[]; onSelectLog?: (log: LogEntry) => void; startTime?: string; @@ -109,7 +108,6 @@ export function LogDetailsDrawer({ logEntry, sessionId, accessToken, - onOpenSettings, allLogs = [], onSelectLog, startTime, @@ -399,7 +397,6 @@ export function LogDetailsDrawer({
diff --git a/ui/litellm-dashboard/src/components/view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.tsx b/ui/litellm-dashboard/src/components/view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.tsx deleted file mode 100644 index b3f73af0602..00000000000 --- a/ui/litellm-dashboard/src/components/view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.tsx +++ /dev/null @@ -1,156 +0,0 @@ -"use client"; - -import { ConfigType, GeneralSettingsFieldName, useDeleteProxyConfigField, useProxyConfig } from "@/app/(dashboard)/hooks/proxyConfig/useProxyConfig"; -import { StoreRequestInSpendLogsParams, useStoreRequestInSpendLogs } from "@/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs"; -import NotificationsManager from "@/components/molecules/notifications_manager"; -import { parseErrorMessage } from "@/components/shared/errorUtils"; -import { ClockCircleOutlined } from "@ant-design/icons"; -import { Button, Form, Input, Modal, Skeleton, Space, Switch, Typography } from "antd"; -import React, { useEffect, useMemo } from "react"; - -interface SpendLogsSettingsModalProps { - isVisible: boolean; - onCancel: () => void; - onSuccess?: () => void; -} - -const SpendLogsSettingsModal: React.FC = ({ isVisible, onCancel, onSuccess }) => { - const [form] = Form.useForm(); - const { mutateAsync, isPending } = useStoreRequestInSpendLogs(); - const { mutateAsync: deleteField, isPending: isDeletingField } = useDeleteProxyConfigField(); - const { data: proxyConfigData, isLoading: isLoadingConfig, refetch } = useProxyConfig(ConfigType.GENERAL_SETTINGS); - const storePromptsValue = Form.useWatch('store_prompts_in_spend_logs', form); - - // Refetch config when modal opens to ensure we have the latest values - useEffect(() => { - if (isVisible) { - refetch(); - } - }, [isVisible, refetch]); - - // Compute initial values from fetched config data - const initialValues = useMemo(() => { - if (!proxyConfigData) { - return { - store_prompts_in_spend_logs: false, - maximum_spend_logs_retention_period: undefined, - }; - } - - const storePromptsField = proxyConfigData.find(field => field.field_name === 'store_prompts_in_spend_logs'); - const retentionPeriodField = proxyConfigData.find(field => field.field_name === 'maximum_spend_logs_retention_period'); - - return { - store_prompts_in_spend_logs: storePromptsField?.field_value ?? false, - maximum_spend_logs_retention_period: retentionPeriodField?.field_value ?? undefined, - }; - }, [proxyConfigData]); - - const handleFormSubmit = async (formValues: StoreRequestInSpendLogsParams) => { - try { - // If maximum_spend_logs_retention_period is empty/null, delete the field first - const retentionPeriodValue = formValues.maximum_spend_logs_retention_period; - const shouldDeleteRetentionPeriod = - !retentionPeriodValue || - (typeof retentionPeriodValue === "string" && retentionPeriodValue.trim() === ""); - - if (shouldDeleteRetentionPeriod) { - try { - await deleteField({ - config_type: ConfigType.GENERAL_SETTINGS, - field_name: GeneralSettingsFieldName.MAXIMUM_SPEND_LOGS_RETENTION_PERIOD, - }); - } catch (deleteError) { - // If field doesn't exist, that's okay - continue with update - console.warn("Failed to delete retention period field (may not exist):", deleteError); - } - } - - // Update the settings (excluding maximum_spend_logs_retention_period if it's empty) - const updateParams: StoreRequestInSpendLogsParams = { - store_prompts_in_spend_logs: formValues.store_prompts_in_spend_logs, - ...(retentionPeriodValue && - typeof retentionPeriodValue === "string" && - retentionPeriodValue.trim() !== "" && { - maximum_spend_logs_retention_period: retentionPeriodValue, - }), - }; - - await mutateAsync(updateParams, { - onSuccess: () => { - NotificationsManager.success("Spend logs settings updated successfully"); - refetch(); // Refetch config to get updated values - onSuccess?.(); - }, - onError: (error) => { - NotificationsManager.fromBackend("Failed to save spend logs settings: " + parseErrorMessage(error)); - }, - }); - } catch (error) { - NotificationsManager.fromBackend("Failed to save spend logs settings: " + parseErrorMessage(error)); - } - }; - - const handleCancel = () => { - form.resetFields(); - onCancel(); - }; - - return ( - Spend Logs Settings} - open={isVisible} - footer={ - - - - - } - onCancel={handleCancel} - > - -
- f.field_name === 'store_prompts_in_spend_logs')?.field_description || - "When enabled, prompts will be stored in spend logs for tracking and analysis purposes." - } - valuePropName="checked" - > -
- - {isLoadingConfig ? : form.setFieldValue('store_prompts_in_spend_logs', checked)} />} -
-
- - f.field_name === 'maximum_spend_logs_retention_period')?.field_description || - "Set the maximum retention period for spend logs (e.g., '7d' for 7 days, '30d' for 30 days). Leave empty for no limit." - } - > - {isLoadingConfig ? : } - />} - -
-
- ); -}; - -export default SpendLogsSettingsModal; diff --git a/ui/litellm-dashboard/src/components/view_logs/index.tsx b/ui/litellm-dashboard/src/components/view_logs/index.tsx index 8205c0b4b86..8a015d83057 100644 --- a/ui/litellm-dashboard/src/components/view_logs/index.tsx +++ b/ui/litellm-dashboard/src/components/view_logs/index.tsx @@ -4,7 +4,7 @@ import { useCallback, useDeferredValue, useEffect, useRef, useState } from "reac import GuardrailViewer from "@/components/view_logs/GuardrailViewer/GuardrailViewer"; import { formatNumberWithCommas } from "@/utils/dataUtils"; import { truncateString } from "@/utils/textUtils"; -import { SettingOutlined, SyncOutlined } from "@ant-design/icons"; +import { SyncOutlined } from "@ant-design/icons"; import { Row } from "@tanstack/react-table"; import { Switch, Tab, TabGroup, TabList, TabPanel, TabPanels } from "@tremor/react"; import { Button, Tag, Tooltip } from "antd"; @@ -28,7 +28,6 @@ import { useLogFilterLogic } from "./log_filter_logic"; import { LogDetailsDrawer } from "./LogDetailsDrawer"; import { getTimeRangeDisplay } from "./logs_utils"; import { RequestResponsePanel } from "./RequestResponsePanel"; -import SpendLogsSettingsModal from "./SpendLogsSettingsModal/SpendLogsSettingsModal"; import { DataTable } from "./table"; import { VectorStoreViewer } from "./VectorStoreViewer"; @@ -85,7 +84,6 @@ export default function SpendLogsTable({ const [selectedLog, setSelectedLog] = useState(null); const [isDrawerOpen, setIsDrawerOpen] = useState(false); const [selectedSessionId, setSelectedSessionId] = useState(null); - const [isSpendLogsSettingsModalVisible, setIsSpendLogsSettingsModalVisible] = useState(false); const [sortBy, setSortBy] = useState("startTime"); const [sortOrder, setSortOrder] = useState<"asc" | "desc">("desc"); @@ -490,11 +488,6 @@ export default function SpendLogsTable({

Request Logs

-
{selectedKeyInfo && selectedKeyIdInfoView && selectedKeyInfo.api_key === selectedKeyIdInfoView ? ( - setIsSpendLogsSettingsModalVisible(false)} - onSuccess={() => setIsSpendLogsSettingsModalVisible(false)} - />
@@ -725,7 +713,6 @@ export default function SpendLogsTable({ logEntry={selectedLog} sessionId={selectedSessionId} accessToken={accessToken} - onOpenSettings={() => setIsSpendLogsSettingsModalVisible(true)} allLogs={filteredData} onSelectLog={handleSelectLog} startTime={moment(startTime).utc().format("YYYY-MM-DD HH:mm:ss")} @@ -734,7 +721,7 @@ export default function SpendLogsTable({ ); } -export function RequestViewer({ row, onOpenSettings }: { row: Row; onOpenSettings?: () => void }) { +export function RequestViewer({ row }: { row: Row }) { // Helper function to clean metadata by removing specific fields const formatData = (input: any) => { if (typeof input === "string") { @@ -961,7 +948,7 @@ export function RequestViewer({ row, onOpenSettings }: { row: Row; onO /> {/* Configuration Info Message - Show when data is missing */} - + {/* Request/Response Panel */}
From adff1c93d0a987b95eec0cf5ab28dad2fc76c324 Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Mon, 27 Apr 2026 14:27:11 -0700 Subject: [PATCH 2/4] refactor(ui): simplify LoggingSettings save flow via React Query callbacks Switch the spend-logs save flow from mutateAsync + try/catch to mutate + callbacks. Errors now surface through a single onError path (no more double toast on failure), and the delete-then-update sequencing runs through onSettled instead of awaited promises. handleFormSubmit is no longer async. Tighten the corresponding test to assert exactly one error toast fires. --- .../LoggingSettings/LoggingSettings.test.tsx | 79 +++++++------------ .../LoggingSettings/LoggingSettings.tsx | 67 ++++++++-------- 2 files changed, 61 insertions(+), 85 deletions(-) diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx index 228e899a3a2..74413333ec4 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx @@ -36,18 +36,18 @@ const mockNotificationsManager = vi.mocked(NotificationsManager); const mockParseErrorMessage = vi.mocked(parseErrorMessage); describe("LoggingSettings", () => { - const mockMutateAsync = vi.fn(); + const mockMutate = vi.fn(); const mockDeleteField = vi.fn(); const mockRefetch = vi.fn(); beforeEach(() => { vi.clearAllMocks(); mockUseStoreRequestInSpendLogs.mockReturnValue({ - mutateAsync: mockMutateAsync, + mutate: mockMutate, isPending: false, } as any); mockUseDeleteProxyConfigField.mockReturnValue({ - mutateAsync: mockDeleteField, + mutate: mockDeleteField, isPending: false, } as any); mockUseProxyConfig.mockReturnValue({ @@ -94,10 +94,8 @@ describe("LoggingSettings", () => { it("should submit form with store prompts enabled and retention period", async () => { const user = userEvent.setup(); - mockMutateAsync.mockImplementation(async (_params, options) => { - await Promise.resolve(); + mockMutate.mockImplementation((_params, options) => { options?.onSuccess?.(); - return { message: "Success" }; }); renderWithProviders(); @@ -113,7 +111,7 @@ describe("LoggingSettings", () => { await waitFor(() => { expect(mockDeleteField).not.toHaveBeenCalled(); - expect(mockMutateAsync).toHaveBeenCalledWith( + expect(mockMutate).toHaveBeenCalledWith( { store_prompts_in_spend_logs: true, maximum_spend_logs_retention_period: "30d", @@ -125,11 +123,11 @@ describe("LoggingSettings", () => { it("should delete retention period field when left empty on submit", async () => { const user = userEvent.setup(); - mockDeleteField.mockResolvedValue({ message: "Field deleted successfully" }); - mockMutateAsync.mockImplementation(async (_params, options) => { - await Promise.resolve(); + mockDeleteField.mockImplementation((_params, options) => { + options?.onSettled?.(); + }); + mockMutate.mockImplementation((_params, options) => { options?.onSuccess?.(); - return { message: "Success" }; }); renderWithProviders(); @@ -139,7 +137,7 @@ describe("LoggingSettings", () => { await waitFor(() => { expect(mockDeleteField).toHaveBeenCalled(); - expect(mockMutateAsync).toHaveBeenCalledWith( + expect(mockMutate).toHaveBeenCalledWith( { store_prompts_in_spend_logs: false, }, @@ -150,11 +148,11 @@ describe("LoggingSettings", () => { it("should show success notification on successful submission", async () => { const user = userEvent.setup(); - mockDeleteField.mockResolvedValue({ message: "Field deleted successfully" }); - mockMutateAsync.mockImplementation(async (_params, options) => { - await Promise.resolve(); + mockDeleteField.mockImplementation((_params, options) => { + options?.onSettled?.(); + }); + mockMutate.mockImplementation((_params, options) => { options?.onSuccess?.(); - return { message: "Success" }; }); renderWithProviders(); @@ -167,30 +165,11 @@ describe("LoggingSettings", () => { }); }); - it("should show error notification when submission throws", async () => { - const user = userEvent.setup(); - const error = new Error("Network error"); - mockMutateAsync.mockRejectedValue(error); - mockParseErrorMessage.mockReturnValue("Network error"); - - renderWithProviders(); - - const saveButton = screen.getByRole("button", { name: "Save Settings" }); - await user.click(saveButton); - - await waitFor(() => { - expect(mockNotificationsManager.fromBackend).toHaveBeenCalledWith( - "Failed to save spend logs settings: Network error", - ); - }); - }); - - it("should show error notification via onError callback", async () => { + it("should show a single error notification via onError callback", async () => { const user = userEvent.setup(); const error = new Error("Backend error"); - mockMutateAsync.mockImplementation((_params, options) => { + mockMutate.mockImplementation((_params, options) => { options?.onError?.(error); - return Promise.reject(error); }); mockParseErrorMessage.mockReturnValue("Backend error"); @@ -204,11 +183,12 @@ describe("LoggingSettings", () => { "Failed to save spend logs settings: Backend error", ); }); + expect(mockNotificationsManager.fromBackend).toHaveBeenCalledTimes(1); }); it("should show loading state on save button when update pending", () => { mockUseStoreRequestInSpendLogs.mockReturnValue({ - mutateAsync: mockMutateAsync, + mutate: mockMutate, isPending: true, } as any); @@ -221,7 +201,7 @@ describe("LoggingSettings", () => { it("should show loading state on save button when delete pending", () => { mockUseDeleteProxyConfigField.mockReturnValue({ - mutateAsync: mockDeleteField, + mutate: mockDeleteField, isPending: true, } as any); @@ -297,11 +277,12 @@ describe("LoggingSettings", () => { it("should continue with update even if deleteField fails", async () => { const user = userEvent.setup(); const deleteError = new Error("Field does not exist"); - mockDeleteField.mockRejectedValue(deleteError); - mockMutateAsync.mockImplementation(async (_params, options) => { - await Promise.resolve(); + mockDeleteField.mockImplementation((_params, options) => { + options?.onError?.(deleteError); + options?.onSettled?.(); + }); + mockMutate.mockImplementation((_params, options) => { options?.onSuccess?.(); - return { message: "Success" }; }); renderWithProviders(); @@ -311,7 +292,7 @@ describe("LoggingSettings", () => { await waitFor(() => { expect(mockDeleteField).toHaveBeenCalled(); - expect(mockMutateAsync).toHaveBeenCalledWith( + expect(mockMutate).toHaveBeenCalledWith( { store_prompts_in_spend_logs: false, }, @@ -323,11 +304,11 @@ describe("LoggingSettings", () => { it("should submit with only store prompts enabled when retention is empty", async () => { const user = userEvent.setup(); - mockDeleteField.mockResolvedValue({ message: "Field deleted successfully" }); - mockMutateAsync.mockImplementation(async (_params, options) => { - await Promise.resolve(); + mockDeleteField.mockImplementation((_params, options) => { + options?.onSettled?.(); + }); + mockMutate.mockImplementation((_params, options) => { options?.onSuccess?.(); - return { message: "Success" }; }); renderWithProviders(); @@ -340,7 +321,7 @@ describe("LoggingSettings", () => { await waitFor(() => { expect(mockDeleteField).toHaveBeenCalled(); - expect(mockMutateAsync).toHaveBeenCalledWith( + expect(mockMutate).toHaveBeenCalledWith( { store_prompts_in_spend_logs: true, }, diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx index c3aaebd3bf2..456240ec5df 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx @@ -18,8 +18,8 @@ import React, { useMemo } from "react"; const LoggingSettings: React.FC = () => { const [form] = Form.useForm(); - const { mutateAsync, isPending } = useStoreRequestInSpendLogs(); - const { mutateAsync: deleteField, isPending: isDeletingField } = useDeleteProxyConfigField(); + const { mutate, isPending } = useStoreRequestInSpendLogs(); + const { mutate: deleteField, isPending: isDeletingField } = useDeleteProxyConfigField(); const { data: proxyConfigData, isLoading: isLoadingConfig } = useProxyConfig(ConfigType.GENERAL_SETTINGS); const storePromptsValue = Form.useWatch("store_prompts_in_spend_logs", form); @@ -42,44 +42,39 @@ const LoggingSettings: React.FC = () => { }; }, [proxyConfigData]); - const handleFormSubmit = async (formValues: StoreRequestInSpendLogsParams) => { - try { - const retentionPeriodValue = formValues.maximum_spend_logs_retention_period; - const shouldDeleteRetentionPeriod = - !retentionPeriodValue || - (typeof retentionPeriodValue === "string" && retentionPeriodValue.trim() === ""); + const handleFormSubmit = (formValues: StoreRequestInSpendLogsParams) => { + const retentionPeriodValue = formValues.maximum_spend_logs_retention_period; + const hasRetentionPeriod = + typeof retentionPeriodValue === "string" && retentionPeriodValue.trim() !== ""; - if (shouldDeleteRetentionPeriod) { - try { - await deleteField({ - config_type: ConfigType.GENERAL_SETTINGS, - field_name: GeneralSettingsFieldName.MAXIMUM_SPEND_LOGS_RETENTION_PERIOD, - }); - } catch (deleteError) { - console.warn("Failed to delete retention period field (may not exist):", deleteError); - } - } + const updateParams: StoreRequestInSpendLogsParams = { + store_prompts_in_spend_logs: formValues.store_prompts_in_spend_logs, + ...(hasRetentionPeriod && { maximum_spend_logs_retention_period: retentionPeriodValue }), + }; - const updateParams: StoreRequestInSpendLogsParams = { - store_prompts_in_spend_logs: formValues.store_prompts_in_spend_logs, - ...(retentionPeriodValue && - typeof retentionPeriodValue === "string" && - retentionPeriodValue.trim() !== "" && { - maximum_spend_logs_retention_period: retentionPeriodValue, - }), - }; - - await mutateAsync(updateParams, { - onSuccess: () => { - NotificationsManager.success("Spend logs settings updated successfully"); - }, - onError: (error) => { - NotificationsManager.fromBackend("Failed to save spend logs settings: " + parseErrorMessage(error)); - }, + const submitUpdate = () => + mutate(updateParams, { + onSuccess: () => NotificationsManager.success("Spend logs settings updated successfully"), + onError: (error) => + NotificationsManager.fromBackend("Failed to save spend logs settings: " + parseErrorMessage(error)), }); - } catch (error) { - NotificationsManager.fromBackend("Failed to save spend logs settings: " + parseErrorMessage(error)); + + if (hasRetentionPeriod) { + submitUpdate(); + return; } + + deleteField( + { + config_type: ConfigType.GENERAL_SETTINGS, + field_name: GeneralSettingsFieldName.MAXIMUM_SPEND_LOGS_RETENTION_PERIOD, + }, + { + onError: (deleteError) => + console.warn("Failed to delete retention period field (may not exist):", deleteError), + onSettled: submitUpdate, + }, + ); }; return ( From 325c74548df644dde1450fc65eb8f3cae63e796b Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Mon, 27 Apr 2026 15:48:27 -0700 Subject: [PATCH 3/4] refactor(ui): invalidate proxyConfig query after spend-logs mutations Previously, useStoreRequestInSpendLogs and useDeleteProxyConfigField did not refresh the proxyConfig cache on success, so the Logging Settings form continued to render the pre-save values until React Query refetched on its own. Wire both hooks to invalidate proxyConfigKeys on success so any active observer (currently the Logging Settings page) repulls fresh data. Export proxyConfigKeys for cross-hook reuse. --- .../hooks/proxyConfig/useProxyConfig.test.ts | 23 +++++++++++++++++++ .../hooks/proxyConfig/useProxyConfig.ts | 8 +++++-- .../useStoreRequestInSpendLogs.ts | 7 +++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.test.ts index a8ce55d2745..4ba80df5c6c 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.test.ts @@ -7,6 +7,7 @@ import { useDeleteProxyConfigField, getProxyConfigCall, deleteProxyConfigFieldCall, + proxyConfigKeys, ConfigType, GeneralSettingsFieldName, type ProxyConfigResponse, @@ -426,6 +427,28 @@ describe("useDeleteProxyConfigField", () => { expect(result.current.error).toBeDefined(); }); + + it("should invalidate proxyConfig queries after a successful delete", async () => { + (fetchSpy as any).mockResolvedValue({ + ok: true, + json: async () => mockDeleteResponse, + }); + + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); + + const { result } = renderHook(() => useDeleteProxyConfigField(), { wrapper }); + + result.current.mutate({ + config_type: ConfigType.GENERAL_SETTINGS, + field_name: GeneralSettingsFieldName.MAXIMUM_SPEND_LOGS_RETENTION_PERIOD, + }); + + await waitFor(() => { + expect(result.current.isSuccess).toBe(true); + }); + + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: proxyConfigKeys.all }); + }); }); describe("getProxyConfigCall", () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.ts index b823ce4ffd8..485c7cc1f92 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.ts @@ -1,4 +1,4 @@ -import { useQuery, useMutation, UseMutationResult } from "@tanstack/react-query"; +import { useQuery, useMutation, UseMutationResult, useQueryClient } from "@tanstack/react-query"; import { createQueryKeys } from "../common/queryKeysFactory"; import useAuthorized from "../useAuthorized"; import { proxyBaseUrl, getGlobalLitellmHeaderName, deriveErrorMessage, handleError } from "@/components/networking"; @@ -101,7 +101,7 @@ export const getProxyConfigCall = async (accessToken: string, configType: Config } }; -const proxyConfigKeys = createQueryKeys("proxyConfig"); +export const proxyConfigKeys = createQueryKeys("proxyConfig"); /** * Network call function to delete a proxy config field @@ -168,6 +168,7 @@ export const useDeleteProxyConfigField = (): UseMutationResult< DeleteProxyConfigFieldRequest > => { const { accessToken } = useAuthorized(); + const queryClient = useQueryClient(); return useMutation({ mutationFn: async (request: DeleteProxyConfigFieldRequest) => { @@ -176,5 +177,8 @@ export const useDeleteProxyConfigField = (): UseMutationResult< } return await deleteProxyConfigFieldCall(accessToken, request); }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: proxyConfigKeys.all }); + }, }); }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs.ts index 9c6211c3086..67b52997a01 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs.ts @@ -1,6 +1,7 @@ -import { useMutation, UseMutationResult } from "@tanstack/react-query"; +import { useMutation, UseMutationResult, useQueryClient } from "@tanstack/react-query"; import { getProxyBaseUrl, getGlobalLitellmHeaderName } from "@/components/networking"; import useAuthorized from "../useAuthorized"; +import { proxyConfigKeys } from "../proxyConfig/useProxyConfig"; export interface StoreRequestInSpendLogsParams { store_prompts_in_spend_logs: boolean; @@ -51,6 +52,7 @@ export const useStoreRequestInSpendLogs = (): UseMutationResult< StoreRequestInSpendLogsParams > => { const { accessToken } = useAuthorized(); + const queryClient = useQueryClient(); return useMutation({ mutationFn: async (params: StoreRequestInSpendLogsParams) => { @@ -59,5 +61,8 @@ export const useStoreRequestInSpendLogs = (): UseMutationResult< } return await performStoreRequestInSpendLogs(accessToken, params); }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: proxyConfigKeys.all }); + }, }); }; From 7f48284decb155c257445e27f77f98266b74737c Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Mon, 27 Apr 2026 16:28:48 -0700 Subject: [PATCH 4/4] test(ui): reset mocks between LoggingSettings tests to prevent bleed-through vi.clearAllMocks does not reset mockImplementation, so the error-notification test was inadvertently relying on a deleteField stub set up in earlier tests and would time out when run in isolation. --- .../AdminSettings/LoggingSettings/LoggingSettings.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx index 74413333ec4..1adf8039e6b 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx @@ -41,7 +41,7 @@ describe("LoggingSettings", () => { const mockRefetch = vi.fn(); beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); mockUseStoreRequestInSpendLogs.mockReturnValue({ mutate: mockMutate, isPending: false, @@ -168,6 +168,9 @@ describe("LoggingSettings", () => { it("should show a single error notification via onError callback", async () => { const user = userEvent.setup(); const error = new Error("Backend error"); + mockDeleteField.mockImplementation((_params, options) => { + options?.onSettled?.(); + }); mockMutate.mockImplementation((_params, options) => { options?.onError?.(error); });