fix(ui): delete policy attachments via controlled modal - #25324
Conversation
Replace static Modal.confirm with DeleteResourceModal so attachment delete reliably triggers the API call. Add a regression test covering the confirm->delete flow. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@greptileai review this PR again |
Greptile SummaryThis PR fixes a React 18 regression where Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/policies/index.tsx | Replaces Modal.confirm with a controlled DeleteResourceModal + useDeletePolicyAttachment hook; adds two pieces of state for the new modal. Contains a leftover unused useMutation import. |
| ui/litellm-dashboard/src/hooks/policies/useDeletePolicyAttachment.ts | New hook wrapping deletePolicyAttachmentCall in a useMutation; handles success/error messages and delegates callbacks cleanly. |
| ui/litellm-dashboard/src/components/policies/index.test.tsx | New integration test for the attachment delete flow; mocks Modal.confirm as a no-op to verify the regression (old path) and confirms the new controlled modal path calls the API. |
| ui/litellm-dashboard/src/hooks/policies/useDeletePolicyAttachment.test.tsx | Unit tests for the new hook covering success and error paths; missing import React for JSX wrapper function (minor). |
Sequence Diagram
sequenceDiagram
participant User
participant AttachmentTable
participant PoliciesPanel
participant DeleteResourceModal
participant useDeletePolicyAttachment
participant deletePolicyAttachmentCall
User->>AttachmentTable: clicks trash icon
AttachmentTable->>PoliciesPanel: onDeleteClick(attachmentId)
PoliciesPanel->>PoliciesPanel: "handleDeleteAttachmentClick()<br/>setAttachmentToDelete()<br/>setIsDeleteAttachmentModalOpen(true)"
PoliciesPanel->>DeleteResourceModal: "renders (isOpen=true)"
DeleteResourceModal->>User: shows confirm dialog
User->>DeleteResourceModal: clicks "Delete"
DeleteResourceModal->>PoliciesPanel: onOk()
PoliciesPanel->>useDeletePolicyAttachment: mutate(attachmentId)
useDeletePolicyAttachment->>deletePolicyAttachmentCall: call API
deletePolicyAttachmentCall-->>useDeletePolicyAttachment: resolves
alt success
useDeletePolicyAttachment-->>PoliciesPanel: onSuccess → fetchAttachments()
useDeletePolicyAttachment-->>PoliciesPanel: onSettled → close modal
else error
useDeletePolicyAttachment-->>PoliciesPanel: onError → MessageManager.error
useDeletePolicyAttachment-->>PoliciesPanel: onSettled → close modal
end
Reviews (7): Last reviewed commit: "fix(ui): remove unused useMutation impor..." | Re-trigger Greptile
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Tip: Greploops — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
Adopt a React Query mutation for policy attachment deletion and add a pending-state test on the policies index panel. This removes local delete-loading state and keeps modal loading tied to mutation status. Made-with: Cursor
| @@ -57,6 +58,8 @@ const PoliciesPanel: React.FC<PoliciesPanelProps> = ({ | |||
| const [isDeleting, setIsDeleting] = useState(false); | |||
There was a problem hiding this comment.
I think this will be handled by the new useMutation
There was a problem hiding this comment.
Only used by policy deletion not attachment, will create pr to refactor the policy deletion code
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 29203053 | Triggered | Generic Password | ef774a1 | .circleci/config.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
3f3760b
into
BerriAI:litellm_yj_apr14
…tachment-delete fix(ui): delete policy attachments via controlled modal
Replace static Modal.confirm with DeleteResourceModal so attachment delete reliably triggers the API call. Add a regression test covering the confirm->delete flow.
Issue:
#25274
fix_issue_policy_attachment.mp4
Made-with: Cursor
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🐛 Bug Fix
✅ Test
Changes