Skip to content

Policies: Include/Exclude label targeting in Save policy modal (#33441) - #47213

Merged
nulmete merged 4 commits into
mainfrom
33441-policies-save-modal-custom-targets
Jun 11, 2026
Merged

Policies: Include/Exclude label targeting in Save policy modal (#33441)#47213
nulmete merged 4 commits into
mainfrom
33441-policies-save-modal-custom-targets

Conversation

@nulmete

@nulmete nulmete commented Jun 9, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #46583

Figma: https://www.figma.com/design/0F1sw63SuYaKVWlcL7mnc6/-33441-Policies--Custom-targets-with-%22Include-any%22-and-%22Exclude-any%22?node-id=5303-5687&t=Fszpf83KhcZ7ViWh-0

Checklist for submitter

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.

Testing

  • Added/updated automated tests

  • QA'd all new/changed functionality manually

Note: policy creation will fail if more than one inclusion/exclusion option is provided. This will be addressed as part of #46582 (we'll relax that check to make sure we can accept a combination).

Screen.Recording.2026-06-09.at.3.48.45.PM.mov
Screen.Recording.2026-06-09.at.3.49.30.PM.mov

Empty state:

Screenshot 2026-06-10 at 11 30 45 AM

Summary by CodeRabbit

  • New Features

    • Add optional "exclude all" label targeting for policies (labels_exclude_all) and tab-based Include/Exclude targeting UI.
  • Improvements

    • In-modal navigation to add new labels; Custom targeting requires at least one selected label to enable Save.
    • Payloads now include include/exclude label fields when using Custom targeting.
  • Tests

    • Updated modal tests to cover exclude-tab label selection.
  • Style

    • Improved empty-state border styling in label selector.

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.19%. Comparing base (8ba889f) to head (f308fec).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
...mponents/SaveNewPolicyModal/SaveNewPolicyModal.tsx 95.65% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #47213      +/-   ##
==========================================
- Coverage   67.20%   67.19%   -0.02%     
==========================================
  Files        3069     3147      +78     
  Lines      226869   227029     +160     
  Branches    11887    11893       +6     
==========================================
+ Hits       152467   152550      +83     
- Misses      60661    60737      +76     
- Partials    13741    13742       +1     
Flag Coverage Δ
frontend 57.85% <95.65%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nulmete
nulmete marked this pull request as ready for review June 9, 2026 19:29
@nulmete
nulmete requested a review from a team as a code owner June 9, 2026 19:29
@nulmete
nulmete force-pushed the 33441-policies-save-modal-custom-targets branch from dccdba8 to b5ee545 Compare June 9, 2026 20:41
@nulmete
nulmete marked this pull request as draft June 9, 2026 20:42
Base automatically changed from 33441-policies-custom-targets-with-include to main June 10, 2026 14:14
Replace the single-mode dropdown target selector in SaveNewPolicyModal
with the tabbed Include/Exclude TargetLabelSelector, supporting a combined
include + exclude scope with any/all modes. Add labels_exclude_all to the
policy form types and forward it in team_policies create/update.
@nulmete
nulmete force-pushed the 33441-policies-save-modal-custom-targets branch from b5ee545 to 89fd02d Compare June 10, 2026 14:17
@nulmete
nulmete marked this pull request as ready for review June 10, 2026 14:18
Copilot AI review requested due to automatic review settings June 10, 2026 14:18
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Adds an exclude-all label condition to policy types and includes it in team_policies create/update requests. Refactors SaveNewPolicyModal to use include/exclude tabs, separate include/exclude label state, and router-driven Add Label navigation; PolicyForm now passes the router prop. Tests are updated to mock the router and exercise the Exclude tab. A small style change updates TargetLabelSelector’s empty-state border.

Possibly related PRs

  • fleetdm/fleet#47212: The main PR’s policy modal changes (rebuilding labels_include_*/labels_exclude_* from an include/exclude targeting UI and the router-driven “add label” flow) directly align with #47212’s extraction/refactor of the reusable include/exclude TargetLabelSelector/DropdownTargetLabelSelector used for those same targeting modes.
  • fleetdm/fleet#46444: Both PRs implement combined include/exclude label targeting by updating label selection state and rebuilding labels_* targeting payload fields (including labels_exclude_all) in their respective modals.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding include/exclude label targeting functionality to the Save policy modal.
Linked Issues check ✅ Passed The PR implements frontend changes for custom label targeting with Include/Exclude tabs and any/all modes, fulfilling the objectives from issue #46583.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing include/exclude label targeting in the Save policy modal as specified in the linked issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed PR description includes related issue, changes file confirmation, testing confirmation, and Figma design reference.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 33441-policies-save-modal-custom-targets

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the “Save policy” modal to use the newer tabbed Include/Exclude label targeting UI (with Any/All modes) and extends the policy payload/types to include labels_exclude_all, aligning policy targeting capabilities with the newer TargetLabelSelector component.

Changes:

  • Replace the old dropdown-based label targeting in SaveNewPolicyModal with TargetLabelSelector include/exclude tabs and Any/All mode toggles.
  • Extend policy payload plumbing and TS types to include labels_exclude_all.
  • Update the modal’s tests and pass a router instance down for “Add label” navigation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontend/services/entities/team_policies.ts Adds labels_exclude_all to the create/update request payloads for team policies.
frontend/pages/policies/edit/components/SaveNewPolicyModal/SaveNewPolicyModal.tsx Switches to TargetLabelSelector with include/exclude tabs + Any/All toggles and builds the new label-scope payload.
frontend/pages/policies/edit/components/SaveNewPolicyModal/SaveNewPolicyModal.tests.tsx Adds/updates tests for include/exclude label payload behavior and new modes.
frontend/pages/policies/edit/components/PolicyForm/PolicyForm.tsx Passes router into SaveNewPolicyModal.
frontend/interfaces/policy.ts Extends IPolicy/IPolicyFormData with labels_exclude_all.
Comments suppressed due to low confidence (1)

frontend/pages/policies/edit/components/SaveNewPolicyModal/SaveNewPolicyModal.tsx:436

  • PATHS.LABEL_NEW_DYNAMIC is marked deprecated in router/paths.ts (handled by the /labels/new route). For new navigation from this modal, prefer PATHS.NEW_LABEL to avoid introducing new uses of the deprecated path.
          autofocus
          disabled={disableForm}
        />

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +186 to +195
const hasCustomLabels =
listNamesFromSelectedLabels(selectedIncludeLabels).length > 0 ||
listNamesFromSelectedLabels(selectedExcludeLabels).length > 0;

const disableForm =
isFetchingAutofillDescription || isFetchingAutofillResolution;
const disableSave =
!platformSelector.isAnyPlatformSelected ||
disableForm ||
(selectedTargetType === "Custom" &&
!Object.entries(selectedLabels).some(([, value]) => {
return value;
}));
(selectedTargetType === "Custom" && !hasCustomLabels);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comment below, I'll tackle the backend changes in a follow-up PR. Merging this to main won't land to users yet and we can still save a single inclusion/exclusion as before.

Comment on lines +259 to +263
it("should send both include and exclude labels when both tabs have selections", async () => {
const onCreatePolicy = jest.fn();
const props = { ...defaultProps, onCreatePolicy };
render(
<PolicyProvider>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll tackle the backend in a separate PR. This is fine.

@@ -0,0 +1 @@
* Added the ability to target a policy to hosts using a combination of "include" and "exclude" labels.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the changes file in this first PR of the series (will have follow-ups for Edit policy page + BE changes and gitops) so that I don't forget later.

@juan-fdz-hawa juan-fdz-hawa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - The only tweak I'd suggest is using a callback (e.g., onAddLabelClicked) for the Add Label CTA when no labels exist. This keeps it consistent with the component's existing pattern.

@nulmete
nulmete merged commit b4c32d2 into main Jun 11, 2026
28 checks passed
@nulmete
nulmete deleted the 33441-policies-save-modal-custom-targets branch June 11, 2026 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Policies custom label targets] Frontend

3 participants