tiennm99/litellm
1
0
Fork 0
mirror of https://github.com/tiennm99/litellm.git synced 2026-07-18 08:18:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(ui): simplify LoggingSettings save flow via React Query callbacks

Browse Source 
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.
This commit is contained in:
Ryan Crabbe
2026-04-27 14:27:11 -07:00
parent be248627b9
commit adff1c93d0
2 changed files with 61 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx
+30 -49
View File
@@ -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(<LoggingSettings />);
@@ -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(<LoggingSettings />);
@@ -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(<LoggingSettings />);
@@ -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(<LoggingSettings />);
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(<LoggingSettings />);
@@ -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(<LoggingSettings />);
@@ -340,7 +321,7 @@ describe("LoggingSettings", () => {
await waitFor(() => {
expect(mockDeleteField).toHaveBeenCalled();
expect(mockMutateAsync).toHaveBeenCalledWith(
expect(mockMutate).toHaveBeenCalledWith(
{
store_prompts_in_spend_logs: true,
},
ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx
+31 -36
View File
@@ -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 (