refactor: replace preloaded teams/customers with TeamSelector/CustomerSelector in virtual keys table and sheet - #5644
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe virtual key governance page now queries only virtual keys. Entity assignment and filtering use dedicated team/customer selectors, while locked team details load by ID and ChangesVirtual key entity flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
d228c80 to
c661edc
Compare
TeamSelector/CustomerSelector in virtual keys table and sheet
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ui/app/workspace/governance/virtual-keys/page.tsx (1)
65-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe dedupe key is a constant, so
shownErrorsRefno longer adds anything.
errorKeyis`${!!vkError}`which is always"true"inside the guarded branch, so theSetholds at most one entry and is cleared whenevervkErrorclears. Since the effect only re-runs whenvkErroridentity changes, a plainuseRef<boolean>(or just toasting on change) expresses the same behavior with less machinery. Note the side effect: if a subsequent poll returns a different error message while the previous error is still set, no new toast is shown — acceptable, but worth being deliberate about.♻️ Simplification
- useEffect(() => { - if (!vkError) { - shownErrorsRef.current.clear(); - return; - } - const errorKey = `${!!vkError}`; - if (shownErrorsRef.current.has(errorKey)) return; - shownErrorsRef.current.add(errorKey); - toast.error(`Failed to load virtual keys: ${getErrorMessage(vkError)}`); - }, [vkError]); + useEffect(() => { + if (!vkError) { + hasShownVkErrorRef.current = false; + return; + } + if (hasShownVkErrorRef.current) return; + hasShownVkErrorRef.current = true; + toast.error(`Failed to load virtual keys: ${getErrorMessage(vkError)}`); + }, [vkError]);Also update line 16 to
const hasShownVkErrorRef = useRef(false);.🤖 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/governance/virtual-keys/page.tsx` around lines 65 - 74, In the virtual-key error effect, replace the constant-key Set-based deduplication with the boolean ref `hasShownVkErrorRef` initialized via `useRef(false)`. Clear it when `vkError` is absent, and when an error is present, toast only if the ref is false before marking it true; preserve the existing error message and dependency behavior.ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)
1570-1606: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAll three branches of
onValueChangedo the same thing.
team,customer, and theelsebranch each clear bothteamIdandcustomerIdand trigger the same fields — only thesetValueordering differs, which has no effect. Collapse to one path.♻️ Proposed simplification
onValueChange={async (value) => { const val = value ?? "none"; field.onChange(val); // Switching type clears both ids and lets the user pick; // there is no entity list loaded to default from. - if (val === "team") { - form.setValue("teamId", "", { - shouldDirty: true, - shouldValidate: true, - }); - form.setValue("customerId", "", { - shouldDirty: true, - shouldValidate: true, - }); - await form.trigger(["teamId", "customerId", "entityType"]); - } else if (val === "customer") { - ... - } else { - ... - } + form.setValue("teamId", "", { shouldDirty: true, shouldValidate: true }); + form.setValue("customerId", "", { shouldDirty: true, shouldValidate: true }); + await form.trigger(["teamId", "customerId", "entityType"]); }}🤖 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/virtual-keys/views/virtualKeySheet.tsx` around lines 1570 - 1606, In the onValueChange handler for the entity type field, remove the redundant team/customer/else branching and execute one shared path that clears both teamId and customerId, then triggers teamId, customerId, and entityType validation. Preserve the existing field.onChange call and update behavior for every value.
🤖 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.
Nitpick comments:
In `@ui/app/workspace/governance/virtual-keys/page.tsx`:
- Around line 65-74: In the virtual-key error effect, replace the constant-key
Set-based deduplication with the boolean ref `hasShownVkErrorRef` initialized
via `useRef(false)`. Clear it when `vkError` is absent, and when an error is
present, toast only if the ref is false before marking it true; preserve the
existing error message and dependency behavior.
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 1570-1606: In the onValueChange handler for the entity type field,
remove the redundant team/customer/else branching and execute one shared path
that clears both teamId and customerId, then triggers teamId, customerId, and
entityType validation. Preserve the existing field.onChange call and update
behavior for every value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 45dec4c8-ff49-402b-80f9-a269911c4734
📒 Files selected for processing (3)
ui/app/workspace/governance/virtual-keys/page.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/app/workspace/virtual-keys/views/virtualKeysTable.tsx
Merge activity
|
…omerSelector` in virtual keys table and sheet (maximhq#5644) ## Summary The virtual keys page and sheet previously fetched the full teams and customers lists upfront and passed them down as props for filtering and entity assignment. This PR removes those bulk fetches in favour of server-side search selectors (`TeamSelector` / `CustomerSelector`) that resolve their own labels on demand, eliminating unnecessary data loading and prop drilling. ## Changes - Removed `useGetTeamsQuery` and `useGetCustomersQuery` calls from the governance virtual keys page; the page no longer fetches or holds teams/customers lists. - Dropped `teams` and `customers` props from `VirtualKeysTable` and `VirtualKeySheet`. - Replaced the static `ComboboxSelect` team/customer filters in the table toolbar with `TeamSelector` and `CustomerSelector` components, which search server-side. Added a `FilterClearButton` helper component to restore the "clear to all" affordance that `ComboboxSelect` previously provided for free. - In `VirtualKeySheet`, the entity assignment section is now always rendered (previously hidden when no teams/customers were loaded). The assignment type dropdown always shows both "Assign to Team" and "Assign to Customer" options. Switching type clears the id fields rather than defaulting to the first item in a list. - The locked-team banner in `VirtualKeySheet` now resolves the team name via a single `useGetTeamQuery(attachedTeamId)` call instead of searching through the full teams list. - The team/customer fallback labels in the selectors are now sourced from the `team` and `customer` objects embedded on the `VirtualKey` itself rather than from the previously fetched lists. - RBAC checks for `Teams` and `Customers` view permissions were removed from the governance page since no bulk fetches are gated on them anymore. - Error handling in the governance page was simplified to only track virtual key fetch errors. ## 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 Governance → Virtual Keys page and confirm it loads without fetching `/teams` or `/customers` on mount. 2. Use the Team and Customer filter dropdowns — verify they search server-side and display results correctly. 3. Select a filter value and confirm the clear (`×`) button resets the filter back to "All". 4. Open the virtual key sheet for an existing key assigned to a team; confirm the team name resolves correctly in the locked banner and in the team selector fallback label. 5. Create a new virtual key, switch the assignment type between Team and Customer, and confirm the id fields are cleared on each switch. 6. Confirm error toasts still appear when the virtual keys fetch fails. ```sh cd ui pnpm i pnpm build ``` ## Screenshots/Recordings _Add before/after screenshots of the filter toolbar and entity assignment section if available._ ## Breaking changes - [x] No ## Related issues ## Security considerations No new auth surfaces introduced. Removed RBAC checks for teams/customers on this page are safe because the underlying selectors enforce their own access controls server-side. ## 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
…omerSelector` in virtual keys table and sheet (maximhq#5644) ## Summary The virtual keys page and sheet previously fetched the full teams and customers lists upfront and passed them down as props for filtering and entity assignment. This PR removes those bulk fetches in favour of server-side search selectors (`TeamSelector` / `CustomerSelector`) that resolve their own labels on demand, eliminating unnecessary data loading and prop drilling. ## Changes - Removed `useGetTeamsQuery` and `useGetCustomersQuery` calls from the governance virtual keys page; the page no longer fetches or holds teams/customers lists. - Dropped `teams` and `customers` props from `VirtualKeysTable` and `VirtualKeySheet`. - Replaced the static `ComboboxSelect` team/customer filters in the table toolbar with `TeamSelector` and `CustomerSelector` components, which search server-side. Added a `FilterClearButton` helper component to restore the "clear to all" affordance that `ComboboxSelect` previously provided for free. - In `VirtualKeySheet`, the entity assignment section is now always rendered (previously hidden when no teams/customers were loaded). The assignment type dropdown always shows both "Assign to Team" and "Assign to Customer" options. Switching type clears the id fields rather than defaulting to the first item in a list. - The locked-team banner in `VirtualKeySheet` now resolves the team name via a single `useGetTeamQuery(attachedTeamId)` call instead of searching through the full teams list. - The team/customer fallback labels in the selectors are now sourced from the `team` and `customer` objects embedded on the `VirtualKey` itself rather than from the previously fetched lists. - RBAC checks for `Teams` and `Customers` view permissions were removed from the governance page since no bulk fetches are gated on them anymore. - Error handling in the governance page was simplified to only track virtual key fetch errors. ## 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 Governance → Virtual Keys page and confirm it loads without fetching `/teams` or `/customers` on mount. 2. Use the Team and Customer filter dropdowns — verify they search server-side and display results correctly. 3. Select a filter value and confirm the clear (`×`) button resets the filter back to "All". 4. Open the virtual key sheet for an existing key assigned to a team; confirm the team name resolves correctly in the locked banner and in the team selector fallback label. 5. Create a new virtual key, switch the assignment type between Team and Customer, and confirm the id fields are cleared on each switch. 6. Confirm error toasts still appear when the virtual keys fetch fails. ```sh cd ui pnpm i pnpm build ``` ## Screenshots/Recordings _Add before/after screenshots of the filter toolbar and entity assignment section if available._ ## Breaking changes - [x] No ## Related issues ## Security considerations No new auth surfaces introduced. Removed RBAC checks for teams/customers on this page are safe because the underlying selectors enforce their own access controls server-side. ## 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
The virtual keys page and sheet previously fetched the full teams and customers lists upfront and passed them down as props for filtering and entity assignment. This PR removes those bulk fetches in favour of server-side search selectors (
TeamSelector/CustomerSelector) that resolve their own labels on demand, eliminating unnecessary data loading and prop drilling.Changes
useGetTeamsQueryanduseGetCustomersQuerycalls from the governance virtual keys page; the page no longer fetches or holds teams/customers lists.teamsandcustomersprops fromVirtualKeysTableandVirtualKeySheet.ComboboxSelectteam/customer filters in the table toolbar withTeamSelectorandCustomerSelectorcomponents, which search server-side. Added aFilterClearButtonhelper component to restore the "clear to all" affordance thatComboboxSelectpreviously provided for free.VirtualKeySheet, the entity assignment section is now always rendered (previously hidden when no teams/customers were loaded). The assignment type dropdown always shows both "Assign to Team" and "Assign to Customer" options. Switching type clears the id fields rather than defaulting to the first item in a list.VirtualKeySheetnow resolves the team name via a singleuseGetTeamQuery(attachedTeamId)call instead of searching through the full teams list.teamandcustomerobjects embedded on theVirtualKeyitself rather than from the previously fetched lists.TeamsandCustomersview permissions were removed from the governance page since no bulk fetches are gated on them anymore.Type of change
Affected areas
How to test
/teamsor/customerson mount.×) button resets the filter back to "All".cd ui pnpm i pnpm buildScreenshots/Recordings
Add before/after screenshots of the filter toolbar and entity assignment section if available.
Breaking changes
Related issues
Security considerations
No new auth surfaces introduced. Removed RBAC checks for teams/customers on this page are safe because the underlying selectors enforce their own access controls server-side.
Checklist
docs/contributing/README.mdand followed the guidelines