diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index c00783854a7..0c487de2b42 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -21647,6 +21647,7 @@ async def get_config( # Check if slack alerting is on _alerting: Final = _general_settings.get("alerting", []) + _alerting_types: Final = proxy_logging_obj.slack_alerting_instance.alert_types alerting_data: Final = [] if "slack" in _alerting: _slack_values, _ = resolve_fields( @@ -21654,7 +21655,6 @@ async def get_config( ) _slack_env_vars: Final = _apply_alerting_env_role_gate(_slack_values, is_full_admin) - _alerting_types: Final = proxy_logging_obj.slack_alerting_instance.alert_types _all_alert_types: Final = proxy_logging_obj.slack_alerting_instance._all_possible_alert_types() _alerts_to_webhook: Final = _apply_webhook_role_gate( proxy_logging_obj.slack_alerting_instance.alert_to_webhook_url, is_full_admin @@ -21699,6 +21699,7 @@ async def get_config( "callbacks": _data_to_return, "alerts": alerting_data, "active_alerting_destinations": tuple(_alerting), + "alert_types": _alerting_types, "router_settings": _router_settings, "available_callbacks": all_available_callbacks, } diff --git a/tests/unit/proxy/proxy_server/test_routes_config.py b/tests/unit/proxy/proxy_server/test_routes_config.py index e3b8aff8fcb..8c0adab3ee8 100644 --- a/tests/unit/proxy/proxy_server/test_routes_config.py +++ b/tests/unit/proxy/proxy_server/test_routes_config.py @@ -1413,6 +1413,34 @@ def test_get_config_callbacks_redacts_slack_webhook_urls_for_view_only_admin(cli assert admin_slack["variables"]["SLACK_WEBHOOK_URL"] != "REDACTED" +def test_get_config_callbacks_returns_alert_types_when_slack_alerting_is_off(client, auth_as, mock_prisma, monkeypatch): + from litellm.proxy import proxy_server as ps + from litellm.proxy._types import AlertType, LitellmUserRoles + + _install_litellm_config(mock_prisma) + monkeypatch.setattr(ps, "prisma_client", mock_prisma) + monkeypatch.setattr(ps, "llm_router", None) + fake_proxy_config = MagicMock() + fake_proxy_config.get_config = AsyncMock( + return_value={ + "litellm_settings": {"success_callback": []}, + "general_settings": {"alerting": ["ms_teams"]}, + "environment_variables": {}, + } + ) + monkeypatch.setattr(ps, "proxy_config", fake_proxy_config) + teams_alert_types: Final = [AlertType.llm_exceptions, AlertType.spend_reports] + monkeypatch.setattr(ps.proxy_logging_obj.slack_alerting_instance, "alert_types", teams_alert_types) + + with auth_as(LitellmUserRoles.PROXY_ADMIN): + response = client.get("/get/config/callbacks") + assert response.status_code == 200 + body = response.json() + + assert body["alert_types"] == ["llm_exceptions", "spend_reports"] + assert all(alert["name"] != "slack" for alert in body["alerts"]) + + def test_redact_callback_env_vars_helper_handles_none_and_non_secret_keys(): from litellm.proxy import proxy_server as ps diff --git a/ui/litellm-dashboard/src/components/settings.test.tsx b/ui/litellm-dashboard/src/components/settings.test.tsx index 027db2e2aca..47cdd5d91ed 100644 --- a/ui/litellm-dashboard/src/components/settings.test.tsx +++ b/ui/litellm-dashboard/src/components/settings.test.tsx @@ -59,6 +59,33 @@ beforeAll(() => { }); }); +const teamsOnlyCallbacksResponse = { + callbacks: [], + available_callbacks: [], + alerts: [ + { name: "email", variables: { EMAIL_ADDRESS: null } }, + { name: "ms_teams", variables: { MS_TEAMS_WEBHOOK_URL: null } }, + ], + active_alerting_destinations: ["ms_teams"], + alert_types: ["llm_exceptions", "spend_reports"], +}; + +const slackCallbacksResponseWithoutTopLevelAlertTypes = { + callbacks: [], + available_callbacks: [], + alerts: [ + { + name: "slack", + active_alerts: ["llm_exceptions"], + variables: { SLACK_WEBHOOK_URL: null }, + alerts_to_webhook: { model_deprecation_warnings: "https://hooks.example.com/dep" }, + }, + { name: "email", variables: { EMAIL_ADDRESS: null } }, + { name: "ms_teams", variables: { MS_TEAMS_WEBHOOK_URL: null } }, + ], + active_alerting_destinations: ["slack"], +}; + describe("Settings", () => { const defaultProps = { accessToken: "token", @@ -518,16 +545,56 @@ describe("Settings", () => { }); }); - it("should allow alert types to be enabled when Slack alerting is not configured", async () => { - mockGetCallbacksCall.mockResolvedValue({ - callbacks: [], - available_callbacks: [], - alerts: [ - { name: "email", variables: { EMAIL_ADDRESS: null } }, - { name: "ms_teams", variables: { MS_TEAMS_WEBHOOK_URL: null } }, - ], + it("should start alert type switches from the proxy's alert types and keep them on save when Slack is not configured", async () => { + mockGetCallbacksCall.mockResolvedValue(teamsOnlyCallbacksResponse); + + const user = userEvent.setup(); + render(); + await user.click(await screen.findByRole("tab", { name: "Alerting Types" })); + + const llmExceptionsRow = screen.getByRole("row", { name: "LLM Exceptions" }); + const llmExceptionsSwitch = within(llmExceptionsRow).getByRole("switch"); + expect(llmExceptionsSwitch).toBeChecked(); + + const deprecationWarningsRow = screen.getByRole("row", { name: "Model Deprecation Warnings" }); + const deprecationWarningsSwitch = within(deprecationWarningsRow).getByRole("switch"); + expect(deprecationWarningsSwitch).not.toBeChecked(); + await user.click(deprecationWarningsSwitch); + await user.click(llmExceptionsSwitch); + + expect(deprecationWarningsSwitch).toBeChecked(); + expect(llmExceptionsSwitch).not.toBeChecked(); + await user.click(screen.getByRole("button", { name: "Save Changes" })); + + await waitFor(() => { + expect(vi.mocked(setCallbacksCall)).toHaveBeenCalledWith( + "token", + expect.objectContaining({ + general_settings: expect.objectContaining({ + alert_types: ["spend_reports", "model_deprecation_warnings"], + }), + }), + ); }); + await user.click(deprecationWarningsSwitch); + await user.click(screen.getByRole("button", { name: "Save Changes" })); + + await waitFor(() => { + expect(vi.mocked(setCallbacksCall)).toHaveBeenLastCalledWith( + "token", + expect.objectContaining({ + general_settings: expect.objectContaining({ + alert_types: ["spend_reports"], + }), + }), + ); + }); + }); + + it("should leave the proxy's alert types unset when only webhooks change on save", async () => { + mockGetCallbacksCall.mockResolvedValue(teamsOnlyCallbacksResponse); + const user = userEvent.setup(); render(); await user.click(await screen.findByRole("tab", { name: "Alerting Types" })); @@ -535,36 +602,19 @@ describe("Settings", () => { const deprecationWarningsRow = screen.getByRole("row", { name: "Model Deprecation Warnings" }); const deprecationWarningsSwitch = within(deprecationWarningsRow).getByRole("switch"); await user.click(deprecationWarningsSwitch); - - expect(deprecationWarningsSwitch).toBeChecked(); + await user.click(deprecationWarningsSwitch); await user.click(screen.getByRole("button", { name: "Save Changes" })); await waitFor(() => { - expect(vi.mocked(setCallbacksCall)).toHaveBeenCalledWith( - "token", - expect.objectContaining({ - general_settings: expect.objectContaining({ - alert_types: ["model_deprecation_warnings"], - }), - }), - ); + expect(vi.mocked(setCallbacksCall)).toHaveBeenCalledTimes(1); }); + const savedSettings = vi.mocked(setCallbacksCall).mock.calls[0][1].general_settings; + expect(savedSettings).toHaveProperty("alert_to_webhook_url"); + expect(savedSettings).not.toHaveProperty("alert_types"); }); it("should load alert types and webhook values from the Slack callback row", async () => { - mockGetCallbacksCall.mockResolvedValue({ - callbacks: [], - available_callbacks: [], - alerts: [ - { name: "email", variables: { EMAIL_ADDRESS: null } }, - { - name: "slack", - active_alerts: ["llm_exceptions"], - variables: { SLACK_WEBHOOK_URL: null }, - alerts_to_webhook: { model_deprecation_warnings: "https://hooks.example.com/dep" }, - }, - ], - }); + mockGetCallbacksCall.mockResolvedValue(slackCallbacksResponseWithoutTopLevelAlertTypes); const user = userEvent.setup(); render(); diff --git a/ui/litellm-dashboard/src/components/settings.tsx b/ui/litellm-dashboard/src/components/settings.tsx index db6cf918b94..93f0a8bde6b 100644 --- a/ui/litellm-dashboard/src/components/settings.tsx +++ b/ui/litellm-dashboard/src/components/settings.tsx @@ -323,6 +323,7 @@ const Settings: React.FC = ({ accessToken, userRole, userID, const [catchAllWebhookURL, setCatchAllWebhookURL] = useState(""); const [alertToWebhooks, setAlertToWebhooks] = useState>({}); const [activeAlerts, setActiveAlerts] = useState([]); + const [savedAlerts, setSavedAlerts] = useState([]); const [showAddCallbacksModal, setShowAddCallbacksModal] = useState(false); const [callbackConfigs, setCallbackConfigs] = useState([]); @@ -429,8 +430,10 @@ const Settings: React.FC = ({ accessToken, userRole, userID, alerts_to_webhook?: Record | null; }) => alert.name === "slack", ); + const loadedAlerts: string[] = data.alert_types ?? slack_alert_info?.active_alerts ?? []; + setActiveAlerts(loadedAlerts); + setSavedAlerts(loadedAlerts); if (slack_alert_info) { - setActiveAlerts(slack_alert_info.active_alerts ?? []); setCatchAllWebhookURL(slack_alert_info.variables?.SLACK_WEBHOOK_URL ?? ""); setAlertToWebhooks(slack_alert_info.alerts_to_webhook ?? {}); } @@ -546,10 +549,12 @@ const Settings: React.FC = ({ accessToken, userRole, userID, updatedAlertToWebhooks[key] = newWebhookValue; }); + const alertTypesChanged = + activeAlerts.length !== savedAlerts.length || activeAlerts.some((alert) => !savedAlerts.includes(alert)); const payload = { general_settings: { alert_to_webhook_url: updatedAlertToWebhooks, - alert_types: activeAlerts, + ...(alertTypesChanged ? { alert_types: activeAlerts } : {}), }, }; @@ -559,6 +564,7 @@ const Settings: React.FC = ({ accessToken, userRole, userID, toast.fromError(error); return; } + setSavedAlerts(activeAlerts); toast.success(t("loggingAndAlerts.alertTypes.toasts.saved")); };