feat: replace inline edit/delete buttons with dropdown menu in model limits table - #3483
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesDelete Dialog Centralization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Confidence Score: 5/5This is a well-scoped UI refactor with no new logic errors; safe to merge. The change is a straightforward UI consolidation. The deletingModelConfig memo correctly derives from the RTK cache prop, RBAC checks are intact on both new menu items, and the single top-level AlertDialog is a strict improvement over per-row instances. No new data paths or async race conditions are introduced beyond what was already present and flagged in earlier review threads. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "chore: made actions sticky on the worksp..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/app/workspace/model-limits/views/modelLimitsTable.tsx (1)
390-406: ⚡ Quick winPrefer
onSelectoveronClickforDropdownMenuItemactions.Using
onSelectaligns with Radix menu-item activation semantics and avoids input-method edge cases for keyboard-triggered selection.Suggested patch
<DropdownMenuItem className="cursor-pointer" disabled={!hasUpdateAccess} - onClick={(e) => { + onSelect={(e) => { e.stopPropagation(); handleEditModelLimit(config); }} data-testid={`model-limit-button-edit-${toTestIdPart(config.model_name)}-${toTestIdPart(config.provider || "all")}`} > @@ <DropdownMenuItem variant="destructive" className="cursor-pointer" disabled={!hasDeleteAccess} - onClick={(e) => { + onSelect={(e) => { e.stopPropagation(); setDeleteModelConfigId(config.id); }} data-testid={`model-limit-button-delete-${toTestIdPart(config.model_name)}-${toTestIdPart(config.provider || "all")}`} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/app/workspace/model-limits/views/modelLimitsTable.tsx` around lines 390 - 406, Replace the onClick handlers on the DropdownMenuItem instances with onSelect handlers to follow Radix activation semantics: move the existing inline functions to onSelect (e.g., change onClick={(e)=>{e.stopPropagation(); handleEditModelLimit(config);}} to onSelect={(e)=>{e.stopPropagation(); handleEditModelLimit(config);}} for the Edit item and similarly replace setDeleteModelConfigId(config.id) on the destructive item; keep the e.stopPropagation() call, preserve the disabled prop (disabled={!hasDeleteAccess}) and existing data-testid usage so behavior and test hooks remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/app/workspace/model-limits/views/modelLimitsTable.tsx`:
- Around line 141-148: The new delete dialog buttons (AlertDialogCancel and
AlertDialogAction) need test selectors: add data-testid attributes to the Cancel
and Delete controls in the modelLimitsTable component so E2E tests can target
them; specifically, update the AlertDialogCancel element to include
data-testid="model-delete-cancel" and the AlertDialogAction (the delete button
that calls handleDelete using deletingModelConfig.id and respects isDeleting) to
include data-testid="model-delete-confirm" so tests can reliably locate these
interactive elements.
---
Nitpick comments:
In `@ui/app/workspace/model-limits/views/modelLimitsTable.tsx`:
- Around line 390-406: Replace the onClick handlers on the DropdownMenuItem
instances with onSelect handlers to follow Radix activation semantics: move the
existing inline functions to onSelect (e.g., change
onClick={(e)=>{e.stopPropagation(); handleEditModelLimit(config);}} to
onSelect={(e)=>{e.stopPropagation(); handleEditModelLimit(config);}} for the
Edit item and similarly replace setDeleteModelConfigId(config.id) on the
destructive item; keep the e.stopPropagation() call, preserve the disabled prop
(disabled={!hasDeleteAccess}) and existing data-testid usage so behavior and
test hooks remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26a1213b-ed6c-4a0f-a7ef-25c43742c3b1
📒 Files selected for processing (1)
ui/app/workspace/model-limits/views/modelLimitsTable.tsx
9b9032e to
8ef4b61
Compare
8aa7789 to
9dd44b0
Compare
Merge activity
|
The base branch was changed.
…limits table (#3483) ## Summary Replaces the individual inline Edit and Delete action buttons in the model limits table with a consolidated `DropdownMenu` (three-dot menu). The delete confirmation dialog is also lifted out of the per-row render loop and rendered once at the table level, driven by a `deleteModelConfigId` state value. ## Changes - Replaced per-row Edit and Delete buttons with a single `MoreHorizontal` icon button that opens a `DropdownMenu` containing Edit and Delete items. - Moved the `AlertDialog` for delete confirmation out of the table row loop into a single top-level instance, controlled by `deleteModelConfigId` state. This prevents multiple dialog instances from being mounted simultaneously. - Added `deletingModelConfig` derived from `deleteModelConfigId` via `useMemo`, keeping it in sync with the RTK cache similarly to `editingModelConfig`. - Cleared `deleteModelConfigId` on successful deletion to close the dialog automatically. - Removed the hover/focus-dependent opacity animation on the action cell since the dropdown replaces that pattern. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Navigate to the Model Limits table in the workspace. 2. Hover over any model limit row and click the `...` (MoreHorizontal) button. 3. Verify the dropdown shows **Edit** and **Delete** options. 4. Click **Edit** and confirm the model limit sheet opens with the correct config pre-populated. 5. Click **Delete** and confirm the confirmation dialog appears with the correct model name (truncated if over 30 characters). 6. Confirm deletion succeeds, the dialog closes, and a success toast is shown. 7. Confirm that users without update/delete RBAC access see the respective menu items disabled. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots showing the old separate Edit/Delete buttons vs. the new dropdown menu._ ## Breaking changes - [x] No ## Related issues ## Security considerations RBAC checks for `Governance` update and delete operations are preserved on the new dropdown menu items. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…limits table (#3483) ## Summary Replaces the individual inline Edit and Delete action buttons in the model limits table with a consolidated `DropdownMenu` (three-dot menu). The delete confirmation dialog is also lifted out of the per-row render loop and rendered once at the table level, driven by a `deleteModelConfigId` state value. ## Changes - Replaced per-row Edit and Delete buttons with a single `MoreHorizontal` icon button that opens a `DropdownMenu` containing Edit and Delete items. - Moved the `AlertDialog` for delete confirmation out of the table row loop into a single top-level instance, controlled by `deleteModelConfigId` state. This prevents multiple dialog instances from being mounted simultaneously. - Added `deletingModelConfig` derived from `deleteModelConfigId` via `useMemo`, keeping it in sync with the RTK cache similarly to `editingModelConfig`. - Cleared `deleteModelConfigId` on successful deletion to close the dialog automatically. - Removed the hover/focus-dependent opacity animation on the action cell since the dropdown replaces that pattern. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Navigate to the Model Limits table in the workspace. 2. Hover over any model limit row and click the `...` (MoreHorizontal) button. 3. Verify the dropdown shows **Edit** and **Delete** options. 4. Click **Edit** and confirm the model limit sheet opens with the correct config pre-populated. 5. Click **Delete** and confirm the confirmation dialog appears with the correct model name (truncated if over 30 characters). 6. Confirm deletion succeeds, the dialog closes, and a success toast is shown. 7. Confirm that users without update/delete RBAC access see the respective menu items disabled. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots showing the old separate Edit/Delete buttons vs. the new dropdown menu._ ## Breaking changes - [x] No ## Related issues ## Security considerations RBAC checks for `Governance` update and delete operations are preserved on the new dropdown menu items. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…limits table (#3483) ## Summary Replaces the individual inline Edit and Delete action buttons in the model limits table with a consolidated `DropdownMenu` (three-dot menu). The delete confirmation dialog is also lifted out of the per-row render loop and rendered once at the table level, driven by a `deleteModelConfigId` state value. ## Changes - Replaced per-row Edit and Delete buttons with a single `MoreHorizontal` icon button that opens a `DropdownMenu` containing Edit and Delete items. - Moved the `AlertDialog` for delete confirmation out of the table row loop into a single top-level instance, controlled by `deleteModelConfigId` state. This prevents multiple dialog instances from being mounted simultaneously. - Added `deletingModelConfig` derived from `deleteModelConfigId` via `useMemo`, keeping it in sync with the RTK cache similarly to `editingModelConfig`. - Cleared `deleteModelConfigId` on successful deletion to close the dialog automatically. - Removed the hover/focus-dependent opacity animation on the action cell since the dropdown replaces that pattern. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Navigate to the Model Limits table in the workspace. 2. Hover over any model limit row and click the `...` (MoreHorizontal) button. 3. Verify the dropdown shows **Edit** and **Delete** options. 4. Click **Edit** and confirm the model limit sheet opens with the correct config pre-populated. 5. Click **Delete** and confirm the confirmation dialog appears with the correct model name (truncated if over 30 characters). 6. Confirm deletion succeeds, the dialog closes, and a success toast is shown. 7. Confirm that users without update/delete RBAC access see the respective menu items disabled. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots showing the old separate Edit/Delete buttons vs. the new dropdown menu._ ## Breaking changes - [x] No ## Related issues ## Security considerations RBAC checks for `Governance` update and delete operations are preserved on the new dropdown menu items. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…limits table (#3483) ## Summary Replaces the individual inline Edit and Delete action buttons in the model limits table with a consolidated `DropdownMenu` (three-dot menu). The delete confirmation dialog is also lifted out of the per-row render loop and rendered once at the table level, driven by a `deleteModelConfigId` state value. ## Changes - Replaced per-row Edit and Delete buttons with a single `MoreHorizontal` icon button that opens a `DropdownMenu` containing Edit and Delete items. - Moved the `AlertDialog` for delete confirmation out of the table row loop into a single top-level instance, controlled by `deleteModelConfigId` state. This prevents multiple dialog instances from being mounted simultaneously. - Added `deletingModelConfig` derived from `deleteModelConfigId` via `useMemo`, keeping it in sync with the RTK cache similarly to `editingModelConfig`. - Cleared `deleteModelConfigId` on successful deletion to close the dialog automatically. - Removed the hover/focus-dependent opacity animation on the action cell since the dropdown replaces that pattern. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Navigate to the Model Limits table in the workspace. 2. Hover over any model limit row and click the `...` (MoreHorizontal) button. 3. Verify the dropdown shows **Edit** and **Delete** options. 4. Click **Edit** and confirm the model limit sheet opens with the correct config pre-populated. 5. Click **Delete** and confirm the confirmation dialog appears with the correct model name (truncated if over 30 characters). 6. Confirm deletion succeeds, the dialog closes, and a success toast is shown. 7. Confirm that users without update/delete RBAC access see the respective menu items disabled. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots showing the old separate Edit/Delete buttons vs. the new dropdown menu._ ## Breaking changes - [x] No ## Related issues ## Security considerations RBAC checks for `Governance` update and delete operations are preserved on the new dropdown menu items. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…limits table (maximhq#3483) ## Summary Replaces the individual inline Edit and Delete action buttons in the model limits table with a consolidated `DropdownMenu` (three-dot menu). The delete confirmation dialog is also lifted out of the per-row render loop and rendered once at the table level, driven by a `deleteModelConfigId` state value. ## Changes - Replaced per-row Edit and Delete buttons with a single `MoreHorizontal` icon button that opens a `DropdownMenu` containing Edit and Delete items. - Moved the `AlertDialog` for delete confirmation out of the table row loop into a single top-level instance, controlled by `deleteModelConfigId` state. This prevents multiple dialog instances from being mounted simultaneously. - Added `deletingModelConfig` derived from `deleteModelConfigId` via `useMemo`, keeping it in sync with the RTK cache similarly to `editingModelConfig`. - Cleared `deleteModelConfigId` on successful deletion to close the dialog automatically. - Removed the hover/focus-dependent opacity animation on the action cell since the dropdown replaces that pattern. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Navigate to the Model Limits table in the workspace. 2. Hover over any model limit row and click the `...` (MoreHorizontal) button. 3. Verify the dropdown shows **Edit** and **Delete** options. 4. Click **Edit** and confirm the model limit sheet opens with the correct config pre-populated. 5. Click **Delete** and confirm the confirmation dialog appears with the correct model name (truncated if over 30 characters). 6. Confirm deletion succeeds, the dialog closes, and a success toast is shown. 7. Confirm that users without update/delete RBAC access see the respective menu items disabled. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots showing the old separate Edit/Delete buttons vs. the new dropdown menu._ ## Breaking changes - [x] No ## Related issues ## Security considerations RBAC checks for `Governance` update and delete operations are preserved on the new dropdown menu items. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…limits table (maximhq#3483) ## Summary Replaces the individual inline Edit and Delete action buttons in the model limits table with a consolidated `DropdownMenu` (three-dot menu). The delete confirmation dialog is also lifted out of the per-row render loop and rendered once at the table level, driven by a `deleteModelConfigId` state value. ## Changes - Replaced per-row Edit and Delete buttons with a single `MoreHorizontal` icon button that opens a `DropdownMenu` containing Edit and Delete items. - Moved the `AlertDialog` for delete confirmation out of the table row loop into a single top-level instance, controlled by `deleteModelConfigId` state. This prevents multiple dialog instances from being mounted simultaneously. - Added `deletingModelConfig` derived from `deleteModelConfigId` via `useMemo`, keeping it in sync with the RTK cache similarly to `editingModelConfig`. - Cleared `deleteModelConfigId` on successful deletion to close the dialog automatically. - Removed the hover/focus-dependent opacity animation on the action cell since the dropdown replaces that pattern. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Navigate to the Model Limits table in the workspace. 2. Hover over any model limit row and click the `...` (MoreHorizontal) button. 3. Verify the dropdown shows **Edit** and **Delete** options. 4. Click **Edit** and confirm the model limit sheet opens with the correct config pre-populated. 5. Click **Delete** and confirm the confirmation dialog appears with the correct model name (truncated if over 30 characters). 6. Confirm deletion succeeds, the dialog closes, and a success toast is shown. 7. Confirm that users without update/delete RBAC access see the respective menu items disabled. ```sh cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots showing the old separate Edit/Delete buttons vs. the new dropdown menu._ ## Breaking changes - [x] No ## Related issues ## Security considerations RBAC checks for `Governance` update and delete operations are preserved on the new dropdown menu items. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Replaces the individual inline Edit and Delete action buttons in the model limits table with a consolidated
DropdownMenu(three-dot menu). The delete confirmation dialog is also lifted out of the per-row render loop and rendered once at the table level, driven by adeleteModelConfigIdstate value.Changes
MoreHorizontalicon button that opens aDropdownMenucontaining Edit and Delete items.AlertDialogfor delete confirmation out of the table row loop into a single top-level instance, controlled bydeleteModelConfigIdstate. This prevents multiple dialog instances from being mounted simultaneously.deletingModelConfigderived fromdeleteModelConfigIdviauseMemo, keeping it in sync with the RTK cache similarly toeditingModelConfig.deleteModelConfigIdon successful deletion to close the dialog automatically.Type of change
Affected areas
How to test
...(MoreHorizontal) button.Screenshots/Recordings
Add before/after screenshots showing the old separate Edit/Delete buttons vs. the new dropdown menu.
Breaking changes
Related issues
Security considerations
RBAC checks for
Governanceupdate and delete operations are preserved on the new dropdown menu items.Checklist
docs/contributing/README.mdand followed the guidelines