Command palette: deep-link Manage policy automations to AutomationsModal - #46884
Conversation
The five Policy automation sub-items (Tickets & webhooks, Install software, Run script, Calendar events, Conditional access) were dead links — ManagePoliciesPage declared a `?manage_automations` query param but never parsed it, and the team-level AutomationsModal also dropped its Install software / Run script sections (per-policy now). Replace the parent + five sub-items with a single flat entry whose path is `/policies?manage_automations=1`, mirroring the reports pattern. ManagePoliciesPage now reads the param, re-checks role + hasPoliciesToAutomate, opens AutomationsModal, and strips the param. Keywords scope to the sections the modal actually renders for the current fleet: All fleets gets webhook-only keywords, Unassigned adds conditional access, a specific team adds calendar too.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
| // Manage automations — policies (admins and maintainers). Mirrors | ||
| // the reports pattern: ManagePoliciesPage reads ?manage_automations=1 | ||
| // and opens AutomationsModal. The page re-checks role + | ||
| // hasPoliciesToAutomate before opening, then strips the param. |
There was a problem hiding this comment.
❯ do we still care about hasPoliciesToAutomate in the command-palette-4.87
⏺ Yes — keep it. Two reasons:
- Symmetry with the in-page button. ManagePoliciesPage.tsx:852 disables when !hasPoliciesToAutomate. Our useEffect's
canEditAutomationsSettings && hasPoliciesToAutomate gate is the URL-driven equivalent of that disabled state. Dropping it would mean clicking the
palette opens a modal that clicking the button refuses to open — same user, same scope, different outcome based on entry path. - The modal isn't useful with zero non-inherited policies. The webhook/calendar/conditional-access automations are team-level configs attached to
policies. With no policies, there's nothing to attach them to — which is why the product gates the button in the first place.
The palette itself can't (cheaply) check this — the context has no policy count — so the page is the right place for the check. If the user clicks
the palette item on an empty team, they land on /policies?manage_automations=1, the param gets stripped, no modal opens, and they see the
empty-policies page with the grayed-out Automations button. That's a coherent dead-end-with-explanation, not a silent failure.
If you wanted to make it punchier, the only real lever would be hiding the palette item itself for premium teams with zero policies — but that
needs a fetched count and probably isn't worth it given how rare "premium tier + admin role + zero policies" is.
There was a problem hiding this comment.
Pull request overview
This PR fixes broken command-palette links for policy automations by deep-linking to the Policies page with a ?manage_automations=1 query param and having ManagePoliciesPage open the AutomationsModal when appropriate, then stripping the param to avoid reopening on refresh.
Changes:
- Add a deep-link handler to
ManagePoliciesPagethat opensAutomationsModal(only when authorized and there are policies to automate) and then removesmanage_automationsfrom the URL. - Flatten “Manage policy automations” in the command palette to a single item (
/policies?manage_automations=1) and scope keywords to what the modal actually renders for the selected fleet scope. - Update command palette unit tests to reflect the flattened entry and keyword scoping.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx | Parses ?manage_automations=1, gates opening the modal on authorization + counts loaded, and strips the param via router.replace. |
| frontend/components/CommandPalette/helpers.tests.ts | Updates tests to validate the flattened policy-automations entry and keyword behavior across fleet scopes. |
| frontend/components/CommandPalette/groups/automations.ts | Replaces policy-automation sub-items with a single deep-link entry and adjusts keywords by scope. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Manage automations — policies (admins and maintainers). Mirrors | ||
| // the reports pattern: ManagePoliciesPage reads ?manage_automations=1 | ||
| // and opens AutomationsModal. The page re-checks role + | ||
| // hasPoliciesToAutomate before opening, then strips the param. | ||
| // |
There was a problem hiding this comment.
addressing in next commit
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #46884 +/- ##
==========================================
- Coverage 67.01% 67.01% -0.01%
==========================================
Files 2857 2857
Lines 224601 224605 +4
Branches 11732 11733 +1
==========================================
- Hits 150521 150515 -6
- Misses 60438 60447 +9
- Partials 13642 13643 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR consolidates the "Manage automations" workflow by restructuring the Command Palette entry from a multi-subitem parent into a single flat palette item that deep-links to the automations modal via Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx`:
- Around line 627-645: The current effect only strips the manage_automations
query when countLoaded is true, so if the count request errors the deep-link
param remains; update the effect so it still always removes manage_automations
from location.query but only opens the automations modal when countLoaded and
the other conditions hold. Concretely, keep computing countLoaded
(isAllTeamsSelected ? globalPoliciesCount !== undefined :
teamPoliciesCountResponse !== undefined), then if (countLoaded &&
canEditAutomationsSettings && hasPoliciesToAutomate) call
setShowAutomationsModal(true), and regardless of countLoaded always do "const {
manage_automations, ...rest } = location.query; router.replace({ pathname:
location.pathname, query: rest });" so the router.replace runs even when data
failed; preserve the same effect dependencies (location.query,
location.pathname, router, canEditAutomationsSettings, hasPoliciesToAutomate,
isAllTeamsSelected, globalPoliciesCount, teamPoliciesCountResponse).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: adb8160b-f5b8-41e6-8b01-49f148b5589a
📒 Files selected for processing (3)
frontend/components/CommandPalette/groups/automations.tsfrontend/components/CommandPalette/helpers.tests.tsfrontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx
Issue
Closes #43757
Description
in 4.87.0 The five Policy automation sub-items (Tickets & webhooks, Install software, Run script, Calendar events, Conditional access) are now dead links — ManagePoliciesPage declared a
?manage_automationsquery param but never parsed it, and the team-level AutomationsModal also dropped its Install software / Run script sections (per-policy now). Replace the parent + five sub-items with a single flat entry whose path is/policies?manage_automations=1, mirroring the reports pattern.ManagePoliciesPage now reads the param, re-checks role + hasPoliciesToAutomate, opens AutomationsModal, and strips the param. Keywords scope to the sections the modal actually renders for the current fleet: All fleets gets webhook-only keywords, Unassigned adds conditional access, a specific team adds calendar too.
Screen recording of fix
Screen.Recording.2026-06-05.at.7.32.15.AM.mov
Related issue: Resolves #
Testing
Summary by CodeRabbit
New Features
Bug Fixes / Behavior Changes
Tests