Skip to content

Extract reusable Include/Exclude TargetLabelSelector component#47212

Merged
nulmete merged 4 commits into
mainfrom
33441-policies-custom-targets-with-include
Jun 10, 2026
Merged

Extract reusable Include/Exclude TargetLabelSelector component#47212
nulmete merged 4 commits into
mainfrom
33441-policies-custom-targets-with-include

Conversation

@nulmete

@nulmete nulmete commented Jun 9, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #46583

Before implementing changes in the Save policy / Edit policy modal (see Figma), I though of making the label selection a reusable component that we could reuse both in Configuration Profiles and in Policies, so that we don't repeat ourselves.

Testing

  • Added/updated automated tests

  • QA'd all new/changed functionality manually

I don't have MDM fully wired up but I checked the payloads were sent as expected.

cp_qa.mov

Summary by CodeRabbit

  • New Features

    • Added a dropdown-based target selector and a tabbed Include/Exclude label selector with Any/All mode.
  • Improvements

    • Unified target/label selection across packages, policies, queries, and profiles.
    • Better empty/loading/error states, selectable badges, help/subtitle support, and cross-tab disabling of selected labels.
  • Tests

    • Added and updated test coverage for both dropdown and tabbed selector behaviors.

nulmete added 2 commits June 9, 2026 15:29
Extract the tabbed Include/Exclude target selector out of AddProfileModal
into a props-driven, reusable TargetLabelSelector component; move the
existing single-mode (dropdown) selector to DropdownTargetLabelSelector.
Relocate listNamesFromSelectedLabels to services/entities/labels to fix a
service->component import. No user-facing changes.
No consumer used hideAllOption: configuration profiles gate the whole
selector behind premium, and the Save policy modal does the same, so the
free-tier 'hide the All radio' case never arises. Drop the dead prop.
@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.35088% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.14%. Comparing base (760c787) to head (8d6d14b).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
...onents/TargetLabelSelector/TargetLabelSelector.tsx 88.00% 6 Missing ⚠️
...argetLabelSelector/DropdownTargetLabelSelector.tsx 90.24% 4 Missing ⚠️
...ePage/components/forms/PackageForm/PackageForm.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #47212      +/-   ##
==========================================
- Coverage   67.15%   67.14%   -0.01%     
==========================================
  Files        2970     3069      +99     
  Lines      226456   226705     +249     
  Branches    11721    11741      +20     
==========================================
+ Hits       152075   152223     +148     
- Misses      60643    60744     +101     
  Partials    13738    13738              
Flag Coverage Δ
frontend 57.89% <90.35%> (+0.01%) ⬆️

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:22
@nulmete nulmete requested a review from a team as a code owner June 9, 2026 19:22
Copilot AI review requested due to automatic review settings June 9, 2026 19:22

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

This PR refactors Fleet’s label-targeting UI by extracting the previously dropdown-based TargetLabelSelector into two explicit, reusable variants: a new tabbed Include/Exclude selector (for configuration profiles) and a preserved dropdown-based selector (for existing forms). It also centralizes shared label-selection helpers into the labels service and adds/updates tests accordingly.

Changes:

  • Introduced a tabbed TargetLabelSelector (Include/Exclude) and extracted the legacy dropdown UX into DropdownTargetLabelSelector.
  • Moved listNamesFromSelectedLabels into frontend/services/entities/labels.ts and updated call sites.
  • Updated configuration profile upload targeting to use the new tabbed selector; updated styles and tests.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
frontend/services/entities/software.ts Updates import to use the centralized listNamesFromSelectedLabels helper.
frontend/services/entities/labels.ts Adds listNamesFromSelectedLabels export.
frontend/services/entities/labels.tests.ts Adds unit tests for listNamesFromSelectedLabels.
frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsx Switches to DropdownTargetLabelSelector import/usage.
frontend/pages/SoftwarePage/components/forms/SoftwareVppForm/SoftwareVppForm.tsx Switches to DropdownTargetLabelSelector import/usage.
frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tsx Switches to DropdownTargetLabelSelector import/usage.
frontend/pages/queries/edit/components/SaveNewQueryModal/SaveNewQueryModal.tsx Switches to DropdownTargetLabelSelector import/usage.
frontend/pages/queries/edit/components/EditQueryForm/EditQueryForm.tsx Switches to DropdownTargetLabelSelector import/usage.
frontend/pages/policies/edit/components/SaveNewPolicyModal/SaveNewPolicyModal.tsx Switches to DropdownTargetLabelSelector import/usage.
frontend/pages/policies/edit/components/PolicyForm/PolicyForm.tsx Switches to DropdownTargetLabelSelector import/usage.
frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tsx Removes local helpers in favor of shared selector/helpers.
frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tests.ts Removes tests for deleted local helpers (replaced by new helper tests).
frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsx Migrates targeting UI to the new tabbed TargetLabelSelector and shared label-key generator.
frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/_styles.scss Removes selector-specific styles now owned by TargetLabelSelector styles.
frontend/components/TargetLabelSelector/TargetLabelSelector.tsx Implements the new tabbed Include/Exclude selector and new prop API.
frontend/components/TargetLabelSelector/TargetLabelSelector.tests.tsx Updates tests for the new tabbed selector behavior.
frontend/components/TargetLabelSelector/index.ts Changes exports to named exports; exposes both selector variants and helpers/types.
frontend/components/TargetLabelSelector/helpers.ts Adds generateCustomTargetLabelKey and shared types.
frontend/components/TargetLabelSelector/helpers.tests.ts Adds unit tests for generateCustomTargetLabelKey.
frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx New file: extracted legacy dropdown-based selector.
frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tests.tsx New tests for the dropdown-based selector.
frontend/components/TargetLabelSelector/_styles.scss Adds styles for the new tabbed selector (badges, empty state, panel layout).

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

Comment thread frontend/components/TargetLabelSelector/TargetLabelSelector.tsx
Comment on lines +36 to +39
if (excludeNames.length) {
result[
excludeMode === "all" ? "labelsExcludeAll" : "labelsExcludeAny"
] = excludeNames;
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f81edd42-f3b6-4373-a4f4-2e07641bfd68

📥 Commits

Reviewing files that changed from the base of the PR and between 37e4402 and 8d6d14b.

📒 Files selected for processing (2)
  • frontend/components/TargetLabelSelector/_styles.scss
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/_styles.scss
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/components/TargetLabelSelector/_styles.scss

Walkthrough

The PR introduces a dual-component architecture for label targeting: DropdownTargetLabelSelector for simple target/label selection (All hosts/Custom, optional custom-target dropdown, and checkboxes) and a refactored TargetLabelSelector providing tabbed Include/Exclude workflows (mode toggle, search, badges, cross-tab disabling). It centralizes label utilities (listNamesFromSelectedLabels, generateCustomTargetLabelKey), updates consumer pages to the dropdown selector, and adds tests and styles.

Possibly related PRs

  • fleetdm/fleet#46444: Updates configuration-profile AddProfileModal to support combined include/exclude label targeting with refactored state representation and label-key generation logic.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Extract reusable Include/Exclude TargetLabelSelector component' clearly summarizes the main objective of the PR—extracting a reusable component for label selection with include/exclude tabs.
Description check ✅ Passed The PR description provides context for the extraction (planned reuse in Policies/Configuration Profiles), references the related issue, includes testing confirmation, and links supporting documentation.
Linked Issues check ✅ Passed The PR successfully delivers frontend changes supporting custom label targets for policies with Include/Exclude functionality. A reusable TargetLabelSelector component is extracted and integrated across multiple modals, fulfilling the linked issue #46583 requirements and supporting the broader #33441 initiative.
Out of Scope Changes check ✅ Passed All changes are within scope: extracting/refactoring TargetLabelSelector into reusable include/exclude tabs component, creating DropdownTargetLabelSelector variant, migrating dependent modules to use new components, and updating related services/helpers accordingly.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 33441-policies-custom-targets-with-include

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.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsx (1)

155-161: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reset the new targeting state in onDone.

This cleanup clears the selected labels, but it leaves selectedTargetType and selectedLabelIncludeMode intact. If the modal is hidden instead of unmounted, reopening it can inherit the previous targeting mode and change the next upload's defaults unexpectedly.

Suggested fix
   const onDone = useCallback(() => {
     fileRef.current = null;
     setFileDetails(null);
+    setSelectedTargetType("All hosts");
+    setSelectedLabelIncludeMode("any");
     setSelectedIncludeLabels({});
     setSelectedExcludeLabels({});
     setShowModal(false);
   }, [fileRef, setShowModal]);
🤖 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
`@frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsx`
around lines 155 - 161, The onDone callback currently clears fileRef,
fileDetails, selectedIncludeLabels and selectedExcludeLabels but does not reset
targeting state; update the onDone function to also reset selectedTargetType and
selectedLabelIncludeMode to their initial/default values (use the state setters
for selectedTargetType and selectedLabelIncludeMode inside the same useCallback)
so when the modal is closed and reopened it doesn't inherit prior targeting
selections; ensure you reference the existing onDone useCallback and call the
appropriate setters for selectedTargetType and selectedLabelIncludeMode.
🧹 Nitpick comments (3)
frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsx (1)

87-87: 💤 Low value

Update comment to reflect new component name.

The comment references TargetLabelSelector but the component has been renamed to DropdownTargetLabelSelector.

📝 Suggested fix
-  // Fetch labels for TargetLabelSelector
+  // Fetch labels for DropdownTargetLabelSelector
🤖 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
`@frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsx`
at line 87, The inline comment "// Fetch labels for TargetLabelSelector" is
outdated; update it to reference the renamed component by changing the comment
to mention "DropdownTargetLabelSelector" (e.g., "// Fetch labels for
DropdownTargetLabelSelector") so it accurately reflects the component name used
in EditAutoUpdateConfigModal and any related logic for fetching labels.
frontend/components/TargetLabelSelector/_styles.scss (1)

68-79: ⚡ Quick win

Consider responsive width for truncated checkbox labels.

The fixed width: 490px on line 74 could cause layout issues if the parent container is narrower than 490px (e.g., in sidebars, modals on smaller screens, or responsive breakpoints). Consider using max-width: 490px instead, or a percentage-based width that adapts to the container.

♻️ Proposed fix using max-width
   .react-tabs__tab-panel .fleet-checkbox {
     height: 20px;
     display: flex;
     align-items: center;
 
     &__label {
-      width: 490px;
+      max-width: 490px;
       overflow: hidden;
       text-overflow: ellipsis;
       white-space: nowrap;
     }
   }
🤖 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 `@frontend/components/TargetLabelSelector/_styles.scss` around lines 68 - 79,
The .react-tabs__tab-panel .fleet-checkbox__label currently uses a fixed width:
490px which breaks on smaller containers; change that rule to use a responsive
constraint (e.g., replace width: 490px with max-width: 490px and allow the
element to shrink by using width: 100% or flex: 1) so the
overflow/text-overflow/white-space rules still apply but the label can adapt to
narrower parent containers; update the CSS selector .react-tabs__tab-panel
.fleet-checkbox__label accordingly.
frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx (1)

8-9: ⚡ Quick win

Replace @ts-ignore with proper typing or @ts-expect-error.

The @ts-ignore directive suppresses all TypeScript errors for the next line, which can hide legitimate issues and makes the code harder to maintain. Consider one of these approaches:

  1. Preferred: Add proper TypeScript definitions for the Dropdown component
  2. Alternative: Use @ts-expect-error instead, which will fail if the error no longer exists, alerting you when types are fixed
♻️ Suggested improvement
-// `@ts-ignore`
+// `@ts-expect-error` - Dropdown component lacks TypeScript definitions
 import Dropdown from "components/forms/fields/Dropdown";
🤖 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 `@frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx`
around lines 8 - 9, The import line currently preceded by "// `@ts-ignore`" should
be fixed: remove the `@ts-ignore` and either (preferred) add proper TypeScript
types for the Dropdown component (export correct typings from the Dropdown
module or add a declaration for its props/interface used by
DropdownTargetLabelSelector) or (alternative) replace "// `@ts-ignore`" with "//
`@ts-expect-error`" to ensure the build fails when the underlying error is
resolved; update the Dropdown import usage in DropdownTargetLabelSelector (and
any related prop types or JSX) to match the declared types so TypeScript no
longer reports an error.
🤖 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/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx`:
- Around line 37-56: The "All hosts" Radio's checked prop includes
!disableOptions which makes its visual checked state depend on disabled state;
change the checked logic so both Radio components derive checked solely from
selectedTarget (i.e., set checked={selectedTarget === "All hosts"} for the "All
hosts" Radio to match the "Custom" Radio), leaving disabled={disableOptions} and
onChange={onSelect} as-is so selection and disabling remain independent; update
the Radio usages in DropdownTargetLabelSelector.tsx accordingly.

In `@frontend/components/TargetLabelSelector/TargetLabelSelector.tsx`:
- Around line 333-352: The checked logic for the "All hosts" Radio is
inconsistent with "Custom": update the checked expression in TargetLabelSelector
for the "All hosts" radio (the Radio with id "all-hosts-target-radio-btn") to
mirror the "Custom" radio by using checked={selectedTargetType === "All hosts"}
(remove the !disableOptions guard) so both radios reflect selection state
consistently even when disabled; ensure onSelectTargetType and value/name props
remain unchanged.

---

Outside diff comments:
In
`@frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsx`:
- Around line 155-161: The onDone callback currently clears fileRef,
fileDetails, selectedIncludeLabels and selectedExcludeLabels but does not reset
targeting state; update the onDone function to also reset selectedTargetType and
selectedLabelIncludeMode to their initial/default values (use the state setters
for selectedTargetType and selectedLabelIncludeMode inside the same useCallback)
so when the modal is closed and reopened it doesn't inherit prior targeting
selections; ensure you reference the existing onDone useCallback and call the
appropriate setters for selectedTargetType and selectedLabelIncludeMode.

---

Nitpick comments:
In `@frontend/components/TargetLabelSelector/_styles.scss`:
- Around line 68-79: The .react-tabs__tab-panel .fleet-checkbox__label currently
uses a fixed width: 490px which breaks on smaller containers; change that rule
to use a responsive constraint (e.g., replace width: 490px with max-width: 490px
and allow the element to shrink by using width: 100% or flex: 1) so the
overflow/text-overflow/white-space rules still apply but the label can adapt to
narrower parent containers; update the CSS selector .react-tabs__tab-panel
.fleet-checkbox__label accordingly.

In `@frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx`:
- Around line 8-9: The import line currently preceded by "// `@ts-ignore`" should
be fixed: remove the `@ts-ignore` and either (preferred) add proper TypeScript
types for the Dropdown component (export correct typings from the Dropdown
module or add a declaration for its props/interface used by
DropdownTargetLabelSelector) or (alternative) replace "// `@ts-ignore`" with "//
`@ts-expect-error`" to ensure the build fails when the underlying error is
resolved; update the Dropdown import usage in DropdownTargetLabelSelector (and
any related prop types or JSX) to match the declared types so TypeScript no
longer reports an error.

In
`@frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsx`:
- Line 87: The inline comment "// Fetch labels for TargetLabelSelector" is
outdated; update it to reference the renamed component by changing the comment
to mention "DropdownTargetLabelSelector" (e.g., "// Fetch labels for
DropdownTargetLabelSelector") so it accurately reflects the component name used
in EditAutoUpdateConfigModal and any related logic for fetching labels.
🪄 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: ae1228be-c295-4711-8452-e640cea930a4

📥 Commits

Reviewing files that changed from the base of the PR and between 760c787 and 5c5a2c2.

📒 Files selected for processing (22)
  • frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tests.tsx
  • frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx
  • frontend/components/TargetLabelSelector/TargetLabelSelector.tests.tsx
  • frontend/components/TargetLabelSelector/TargetLabelSelector.tsx
  • frontend/components/TargetLabelSelector/_styles.scss
  • frontend/components/TargetLabelSelector/helpers.tests.ts
  • frontend/components/TargetLabelSelector/helpers.ts
  • frontend/components/TargetLabelSelector/index.ts
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsx
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/_styles.scss
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tests.ts
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tsx
  • frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsx
  • frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tsx
  • frontend/pages/SoftwarePage/components/forms/SoftwareVppForm/SoftwareVppForm.tsx
  • frontend/pages/policies/edit/components/PolicyForm/PolicyForm.tsx
  • frontend/pages/policies/edit/components/SaveNewPolicyModal/SaveNewPolicyModal.tsx
  • frontend/pages/queries/edit/components/EditQueryForm/EditQueryForm.tsx
  • frontend/pages/queries/edit/components/SaveNewQueryModal/SaveNewQueryModal.tsx
  • frontend/services/entities/labels.tests.ts
  • frontend/services/entities/labels.ts
  • frontend/services/entities/software.ts
💤 Files with no reviewable changes (2)
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tests.ts
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tsx

Comment thread frontend/components/TargetLabelSelector/TargetLabelSelector.tsx
- Radio 'All hosts' now reflects selection even when disabled (both the
  tabbed and dropdown selectors), matching the 'Custom' radio.
- Remove unused excludeMode/labelsExcludeAll from the profile label-key
  helper (no consumer; policies build their payload separately).
- Move generateCustomTargetLabelKey to AddProfileModal, its only consumer;
  move LabelTargetMode/TargetType into TargetLabelSelector and drop the
  now-empty helpers module.
- Use max-width for truncated checkbox labels; fix a stale comment.

const baseClass = "target-label-selector";

export const listNamesFromSelectedLabels = (dict: Record<string, boolean>) => {

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.

moved this utility to services/entities/labels.ts since it's also consumed in services/entities/software.ts (didn't make sense to import this from components into `services IMHO)

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (2)
frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx (1)

18-24: ⚡ Quick win

Align target props with the shared TargetType union.

This keeps the dropdown selector contract consistent with the tabbed selector and prevents invalid target values from being passed through typed call sites.

Suggested diff
 import React, { ReactNode } from "react";
 import classnames from "classnames";
+import type { TargetType } from "./TargetLabelSelector";
@@
 interface ITargetChooserProps {
-  selectedTarget: string;
-  onSelect: (val: string) => void;
+  selectedTarget: TargetType;
+  onSelect: (val: TargetType) => void;
@@
         value="All hosts"
         name="target-type"
-        onChange={onSelect}
+        onChange={(val: string) => onSelect(val as TargetType)}
         disabled={disableOptions}
       />
@@
         value="Custom"
         name="target-type"
-        onChange={onSelect}
+        onChange={(val: string) => onSelect(val as TargetType)}
         disabled={disableOptions}
       />
@@
 interface IDropdownTargetLabelSelectorProps {
-  selectedTargetType: string;
+  selectedTargetType: TargetType;
@@
-  onSelectTargetType: (val: string) => void;
+  onSelectTargetType: (val: TargetType) => void;

Also applies to: 150-165

🤖 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 `@frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx`
around lines 18 - 24, The props interface ITargetChooserProps uses plain string
for selectedTarget and onSelect(val: string), which allows invalid values;
change those types to the shared TargetType union used by the tabbed selector so
selectedTarget: TargetType and onSelect: (val: TargetType) => void (update any
usages in DropdownTargetLabelSelector and related components to accept
TargetType as well). Locate ITargetChooserProps, DropdownTargetLabelSelector,
and any call sites mentioned around lines ~150-165 and update type annotations
and event handlers to use TargetType for consistency.
frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.ts (1)

21-33: ⚡ Quick win

Narrow the helper return type to known payload keys.

Using Record<string, string[]> permits arbitrary keys and weakens payload safety for mdmAPI.uploadProfile.

Suggested diff
+type ICustomTargetLabelKey = Partial<{
+  labelsIncludeAny: string[];
+  labelsIncludeAll: string[];
+  labelsExcludeAny: string[];
+}>;
+
 const generateCustomTargetLabelKey = ({
   targetType,
   includeMode,
   includeLabels,
   excludeLabels,
-}: IGenerateCustomTargetLabelKeyArgs) => {
+}: IGenerateCustomTargetLabelKeyArgs): ICustomTargetLabelKey => {
   if (targetType !== "Custom") {
     return {};
   }
 
-  const result: Record<string, string[]> = {};
+  const result: ICustomTargetLabelKey = {};
🤖 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
`@frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.ts`
around lines 21 - 33, The helper currently types result as
Record<string,string[]>, which allows arbitrary keys; narrow it to a specific
payload type (e.g., a Partial or explicit interface containing only
labelsIncludeAll, labelsIncludeAny, and labelsExcludeAny) and use that type for
the result variable and the function return type. Update the helper that calls
listNamesFromSelectedLabels and uses includeMode/includeLabels/excludeLabels to
construct and return this typed payload so mdmAPI.uploadProfile receives a
strongly-typed object.
🤖 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 `@frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx`:
- Around line 18-24: The props interface ITargetChooserProps uses plain string
for selectedTarget and onSelect(val: string), which allows invalid values;
change those types to the shared TargetType union used by the tabbed selector so
selectedTarget: TargetType and onSelect: (val: TargetType) => void (update any
usages in DropdownTargetLabelSelector and related components to accept
TargetType as well). Locate ITargetChooserProps, DropdownTargetLabelSelector,
and any call sites mentioned around lines ~150-165 and update type annotations
and event handlers to use TargetType for consistency.

In
`@frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.ts`:
- Around line 21-33: The helper currently types result as
Record<string,string[]>, which allows arbitrary keys; narrow it to a specific
payload type (e.g., a Partial or explicit interface containing only
labelsIncludeAll, labelsIncludeAny, and labelsExcludeAny) and use that type for
the result variable and the function return type. Update the helper that calls
listNamesFromSelectedLabels and uses includeMode/includeLabels/excludeLabels to
construct and return this typed payload so mdmAPI.uploadProfile receives a
strongly-typed object.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0d49f54f-0a11-4f12-8d08-50eb2e6d9b57

📥 Commits

Reviewing files that changed from the base of the PR and between 5c5a2c2 and 37e4402.

📒 Files selected for processing (8)
  • frontend/components/TargetLabelSelector/DropdownTargetLabelSelector.tsx
  • frontend/components/TargetLabelSelector/TargetLabelSelector.tsx
  • frontend/components/TargetLabelSelector/_styles.scss
  • frontend/components/TargetLabelSelector/index.ts
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsx
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tests.ts
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.ts
  • frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsx
  • frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsx
  • frontend/components/TargetLabelSelector/_styles.scss

isErrorLabels={isErrorLabels}
emptyStateDescription="Add a label to target your configuration profile."
onAddLabel={() => {
window.location.href = PATHS.LABEL_NEW_DYNAMIC;

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 think we should use the router here. (I left as-is, can follow-up.)

@@ -1,51 +1,69 @@
import { listNamesFromSelectedLabels, generateLabelKey } from "./helpers";
import generateCustomTargetLabelKey from "./helpers";

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 believe there was a linting issue when having a single export and it being a named one. So had to change to default.

@nulmete nulmete assigned andymFleet and MagnusHJensen and unassigned andymFleet Jun 9, 2026

@MagnusHJensen MagnusHJensen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall a great change, just

Comment thread frontend/components/TargetLabelSelector/TargetLabelSelector.tsx
Comment thread frontend/components/TargetLabelSelector/TargetLabelSelector.tsx
@nulmete nulmete merged commit 8ba889f into main Jun 10, 2026
26 checks passed
@nulmete nulmete deleted the 33441-policies-custom-targets-with-include branch June 10, 2026 14:14
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

4 participants