fix(ui): move 'Store Prompts in Spend Logs' toggle to Admin Settings - #26631
Conversation
[Infra] Promote internal staging to main
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.
Greptile SummaryThis PR relocates the "Store Prompts in Spend Logs" and "Maximum Spend Logs Retention Period" controls from a gear-icon modal on the Logs page (visible to all authenticated users) into a new Logging Settings tab under Admin Settings (gated to admins), and replaces the The three previously-flagged P1 concerns are resolved in this version:
Confidence Score: 5/5Safe to merge — all previously flagged P1s are addressed, no new blocking issues found. No P0 or P1 findings remain. The three previously raised P1s (double-toast, stale values, test timeout) are each verifiably resolved in this diff. Only two P2 observations exist: a missing hook-level test for useStoreRequestInSpendLogs invalidation, and a cosmetic double-invalidation when clearing the retention period. No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx | New component replacing SpendLogsSettingsModal; uses mutate+callbacks (no double-toast), form key resets on cache invalidation, save flow correctly handles delete-then-update via onSettled chaining. |
| ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.ts | Exports proxyConfigKeys; adds invalidateQueries to useDeleteProxyConfigField.onSuccess — resolves the previously-flagged stale-form-values P1. |
| ui/litellm-dashboard/src/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs.ts | Adds invalidateQueries on success so the form key changes after a successful store-prompts update, triggering a form reset to server values. |
| ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx | New test suite; correctly mocks deleteField with onSettled, adds toHaveBeenCalledTimes(1) to catch double-toast regression; covers all key paths including loading states and delete-then-update flow. |
| ui/litellm-dashboard/src/components/view_logs/index.tsx | Removes gear button, SpendLogsSettingsModal, and onOpenSettings prop chain; cleans up associated state and imports. |
| ui/litellm-dashboard/src/components/view_logs/SpendLogsSettingsModal/SpendLogsSettingsModal.tsx | Deleted — replaced by LoggingSettings under Admin Settings; functionality preserved with improved save flow. |
| ui/litellm-dashboard/src/components/AdminPanel.tsx | Adds a new Logging Settings tab to the Admin Settings panel, correctly placed after UISettings. |
| ui/litellm-dashboard/src/components/view_logs/ConfigInfoMessage.tsx | Drops onOpenSettings prop; replaces the actionable button with a static reference to Admin Settings → Logging Settings. |
| ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.test.ts | Adds test verifying useDeleteProxyConfigField calls invalidateQueries on success; exports proxyConfigKeys for the assertion. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User clicks Save] --> B{retentionPeriod filled?}
B -- Yes --> C[mutate updateParams with retention]
B -- No --> D[deleteField retention period]
D -- onSuccess --> E[invalidateQueries proxyConfigKeys.all]
D -- onError --> F[console.warn, skip and continue]
D -- onSettled --> G[mutate updateParams no retention]
C -- onSuccess --> H[NotificationsManager.success]
G -- onSuccess --> H
C -- onError --> I[NotificationsManager.fromBackend]
G -- onError --> I
H --> J[invalidateQueries proxyConfigKeys.all]
J --> K[useProxyConfig refetches, proxyConfigData updates]
K --> L[initialValues recomputes, Form key changes]
L --> M[Form resets to server values]
Reviews (5): Last reviewed commit: "test(ui): reset mocks between LoggingSet..." | Re-trigger Greptile
…itellm_fix-logging-settings-admin-only
…backs 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.
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.
…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.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
62920a0
into
litellm_internal_staging
…tings-admin-only fix(ui): move 'Store Prompts in Spend Logs' toggle to Admin Settings
Summary
/config/updateand/config/listboth requirePROXY_ADMIN. Non-admins could open the modal, but the request would 403 on load and save, giving a confusing UX. Admin Settings is already gated to admins at the sidebar (all_admin_roles), so relocation is a defense-in-depth fix that also makes "global proxy setting" clearer than a gear on a per-user logs view.onOpenSettingsprop chain (ConfigInfoMessage→LogDetailContent→LogDetailsDrawer).ConfigInfoMessagenow tells users to toggle the setting under "Admin Settings → Logging Settings" inline.LoggingSettingsto use idiomatic React Query callbacks (mutate+onSuccess/onError/onSettled) instead ofmutateAsyncwrapped intry/catch. Eliminates a double-toast bug on save failure (both the per-callonErrorand the outer catch were notifying) and removes the awaited delete-then-update sequencing in favor ofonSettledchaining.Resolves LIT-2426
Screenshots
after
Test plan
LoggingSettingstest suite and the trimmedConfigInfoMessage/LogDetailContentsuites pass locally (npx vitest run src/components/Settings/AdminSettings/LoggingSettings).