refactor(ui): migrate policy attachments table onto shared DataTable - #33827
Conversation
Greptile SummaryThis PR migrates the Policy Attachments table from a hand-rolled TanStack + Tremor renderer onto the shared
Confidence Score: 5/5Safe to merge — this is a pure UI refactor with no backend changes, no new API calls, and no altered data flow. The change is a straightforward component migration: the old hand-rolled table is deleted and replaced by the shared DataTable with identical data behaviour. All interaction patterns (delete flow, clipboard copy, blast-radius popover) are covered by updated tests that verify the correct UI elements and user flows. No logic has been introduced that could cause regressions in the policy attachment delete or display paths. No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/AttachmentTableColumns.tsx | New column definitions file; uses shared cell primitives and dropdown pattern consistently. copyToClipboard is the established codebase pattern that handles success/failure internally. |
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/AttachmentTable.tsx | Thin DataTable wrapper with client-side sorting and a stable DEFAULT_SORTING constant; columns are memoized on isAdmin/accessToken/onDeleteClick. Clean. |
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/AttachmentTable.test.tsx | Tests rewritten for the new dropdown-menu interaction pattern; covers default sort, non-admin gate, clipboard copy, and blast-radius availability. No coverage gaps or weakened assertions visible. |
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/index.test.tsx | Panel-level delete-flow tests updated from the old TrashIcon button to the new data-testid-driven menu; stale heroicons and tremor Icon mocks removed. |
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/index.tsx | Single import path change from attachment_table to AttachmentTable; all other logic untouched. |
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/attachment_table.tsx | Deleted — replaced by AttachmentTable.tsx (PascalCase) and AttachmentTableColumns.tsx. Removal is clean. |
| ui/litellm-dashboard/eslint-suppressions.json | Stale suppression entries for the deleted snake_case files removed; no new suppressions added. |
Reviews (2): Last reviewed commit: "refactor(ui): pass a specific success me..." | Re-trigger Greptile
|
@greptile the copy to clipboard reuses what we already have. this is known pattern. Please review again |
QA: Policy Attachments table migration to shared DataTableQA'd live against a local proxy + dashboard on this branch. Seeded Populated tableMono truncated attachment IDs, +N overflow tooltipHovering Blast-radius popoverInline eye control still opens the Blast Radius popover with affected keys/teams. Row actions menuShared overflow menu shows Copy attachment ID (all users) and admin-only Delete attachment. Copy attachment IDCopy fires the specific success toast "Attachment ID copied". Delete attachmentDelete opens the existing Delete Attachment modal with attachment info, confirming removes the row and refetches the list ("Attachment deleted successfully", 4 to 3 rows). Empty stateAfter deleting all attachments, the table shows the rich empty state "No attachments found" with guidance text. WalkthroughVerified: render, sort default + sortable headers, chip overflow + tooltip, blast-radius popover, copy toast, admin-only delete + modal + refetch, empty state. No regressions found. |
Relevant issues
Related: #25546 (closed as stale); the broken delete control it tried to repair is replaced here by a real button inside the row actions menu
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Verified live against a proxy on localhost:4000 (dev config, DB-backed) and the dashboard dev server, at commit 45f4567. Steps to reproduce the states:
npm run devinui/litellm-dashboard)curl -X DELETE "http://localhost:4000/policies/attachments/<id>" -H "Authorization: Bearer sk-1234") and reload/policies/attachments/list, then delete a row to trigger a refetch; compact skeleton rows render at the same height as data rowsType
🧹 Refactoring
Changes
Rewrites the Policy Attachments table (Policies page, Attachments tab) from a hand-rolled TanStack + tremor renderer onto the shared
DataTableand cell library, matching the sibling PolicyTable that migrated in #33357. The container becomes a thin client-modeDataTableconsumer and the column defs move to a newAttachmentTableColumns.tsx; the legacy files are renamed to PascalCase (AttachmentTable.tsx,AttachmentTable.test.tsx) via git mvData behavior is preserved: default sort stays
created_atdescending, the attachment ID renders as plain truncated mono text, Global (*) scope keeps an amber badge, the four list columns (teams, keys, models, tags) keep the show-2-then-+N-overflow-with-tooltip pattern, and delete stays admin-only. Sortable headers are now deliberate (Policy and Created At) instead of every column, per the unified table designRow actions unify to the shared pattern: a dropdown menu with Copy attachment ID for everyone and a destructive Delete attachment item appended for admins. The tremor
Icondelete control (not a real button; the thing #25546 tried to repair before going stale) is gone. The blast radius popover stays as an inline control next to the menu trigger because an antd Popover must stay anchored to a mounted element; a menu item unmounts when the menu closes on selectThe parent panel only changes its import path; its loading and refetch semantics are untouched. Tests: the table test is rewritten for the menu pattern (menu-open flow, clipboard assertion via
readText, admin gate mutation-checked by forcing it open and confirming the non-admin test fails), the two panel-level delete-flow tests now drive the menu instead of the old icon, and a new test pins the created_at descending default sort. The renamed file's stale entries ineslint-suppressions.jsonare prunedFinal Attestation