Fleet UI: Add combined include/exclude label targeting for configuration profiles - #46444
Conversation
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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #46444 +/- ##
==========================================
+ Coverage 66.91% 66.97% +0.05%
==========================================
Files 2832 2814 -18
Lines 224970 221218 -3752
Branches 11663 11651 -12
==========================================
- Hits 150531 148151 -2380
+ Misses 60783 59776 -1007
+ Partials 13656 13291 -365
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:
|
There was a problem hiding this comment.
Pull request overview
Updates the frontend MDM configuration profile targeting UX to support selecting include + exclude labels together (per #45179), along with related UI refinements and copy changes around label deletion and profile label display.
Changes:
- Reworked the “Add profile” modal targeting UI to support separate Include/Exclude label selection (with search, tabs, and selected “badge” affordances).
- Updated MDM profile upload request-building to send include-all/include-any/exclude-any label arrays.
- Updated profile label display/modals and delete-label copy/error messaging; added unit tests for new helper logic and messaging.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/styles/var/mixins.scss | Makes a sticky layout section responsive via clamp() sizing. |
| frontend/services/entities/mdm.ts | Changes profile upload FormData building to append include/exclude label fields. |
| frontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/PlatformTabs.tsx | Replaces “dot” configured indicator with a check icon in tab labels. |
| frontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/_styles.scss | Styles the new tab check icon. |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tsx | Updates targeting helper to generate include + exclude label keys. |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tests.ts | Adds unit tests for new targeting helper behavior. |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsx | Replaces TargetLabelSelector with a custom include/exclude targeting UI (tabs + search + badges). |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/_styles.scss | Styles the new targeting UI (tabs, panels, selected badges). |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileListItem/ProfileListItem.tsx | Updates label aggregation for displaying a single label count when include/exclude labels exist. |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileListItem/_styles.scss | Adds truncation/overflow behavior and responsive tweaks for the profiles list item layout. |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileLabelsModal/ProfileLabelsModal.tsx | Updates label modal to show include and exclude sections separately with new badge layout. |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileLabelsModal/ProfileLabelsModal.tests.tsx | Adds tests for the updated profile labels modal behavior. |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileLabelsModal/_styles.scss | Styles new badge-based, scrollable labels layout and section titles. |
| frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/_styles.scss | Adjusts action button spacing in the configuration profiles card. |
| frontend/pages/labels/helpers.ts | Adds a new error-message branch for “configuration profile” label-delete failures. |
| frontend/pages/labels/helpers.tests.ts | Adds unit tests for updated delete-label error message behavior. |
| frontend/pages/hosts/ManageHostsPage/components/DeleteLabelModal/DeleteLabelModal.tsx | Updates premium delete-label modal copy re: configuration profile targeting. |
| changes/45179-cpie-frontend-include-exclude-targeting | Adds changelog entry for the user-visible frontend updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThis PR implements frontend changes enabling MDM configuration profiles to target both include and exclude labels simultaneously. The core changes include a redesigned label selection interface in the Add Profile modal with tabbed include/exclude selection, updated profile display to show include and exclude labels in separate sections, modified label deletion messaging to clarify the profile constraint, and supporting platform configuration status indicators. The implementation updates server integration to independently append all three label array types to the upload payload. 🚥 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.
🧹 Nitpick comments (2)
frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileLabelsModal/ProfileLabelsModal.tsx (1)
63-64: ⚡ Quick winPrefer nullish coalescing operator for consistency and safety.
Line 63 uses the OR operator (
||), butProfileListItem.tsx(lines 128-132) uses the nullish coalescing operator (??) for similar label array handling. The OR operator will select an empty array[]over a populated array, whereas??only falls through fornull/undefined. For consistency and to avoid edge-case bugs if the backend ever returns empty arrays:♻️ Use nullish coalescing for safer fallback
- const includeLabels = labels_include_all || labels_include_any; + const includeLabels = labels_include_all ?? labels_include_any;🤖 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/ProfileLabelsModal/ProfileLabelsModal.tsx` around lines 63 - 64, Replace the use of the logical OR for computing includeLabels with nullish coalescing to match the pattern in ProfileListItem.tsx: change the expression that defines includeLabels (currently using labels_include_all || labels_include_any) to use labels_include_all ?? labels_include_any so only null/undefined fall through; leave excludeLabels (labels_exclude_any) unchanged.frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileListItem/ProfileListItem.tsx (1)
173-173: ⚡ Quick winRemove redundant undefined check.
The condition
labels !== undefinedis now always true sincelabelsis initialized as an array on lines 128-132 and can never beundefined. Simplify to just check the length:♻️ Simplify condition
- {isPremium && labels !== undefined && labels.length && ( + {isPremium && labels.length > 0 && (🤖 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/ProfileListItem/ProfileListItem.tsx` at line 173, In ProfileListItem, the JSX condition currently checks `isPremium && labels !== undefined && labels.length` but `labels` is always initialized as an array in the ProfileListItem component, so remove the redundant `labels !== undefined` check and simplify the conditional to only test `isPremium` and whether `labels.length` (e.g., `isPremium && labels.length > 0`) to render the labels.
🤖 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/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileLabelsModal/ProfileLabelsModal.tsx`:
- Around line 63-64: Replace the use of the logical OR for computing
includeLabels with nullish coalescing to match the pattern in
ProfileListItem.tsx: change the expression that defines includeLabels (currently
using labels_include_all || labels_include_any) to use labels_include_all ??
labels_include_any so only null/undefined fall through; leave excludeLabels
(labels_exclude_any) unchanged.
In
`@frontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileListItem/ProfileListItem.tsx`:
- Line 173: In ProfileListItem, the JSX condition currently checks `isPremium &&
labels !== undefined && labels.length` but `labels` is always initialized as an
array in the ProfileListItem component, so remove the redundant `labels !==
undefined` check and simplify the conditional to only test `isPremium` and
whether `labels.length` (e.g., `isPremium && labels.length > 0`) to render the
labels.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c12716d7-43a3-4b73-a26f-e7109db8eaaf
📒 Files selected for processing (18)
changes/45179-cpie-frontend-include-exclude-targetingfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileLabelsModal/ProfileLabelsModal.tests.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileLabelsModal/ProfileLabelsModal.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileLabelsModal/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileListItem/ProfileListItem.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileListItem/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/AddProfileModal.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tests.tsfrontend/pages/ManageControlsPage/OSSettings/cards/ConfigurationProfiles/components/ProfileUploader/components/AddProfileModal/helpers.tsxfrontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/PlatformTabs.tsxfrontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/_styles.scssfrontend/pages/hosts/ManageHostsPage/components/DeleteLabelModal/DeleteLabelModal.tsxfrontend/pages/labels/helpers.tests.tsfrontend/pages/labels/helpers.tsfrontend/services/entities/mdm.tsfrontend/styles/var/mixins.scss
…implify redundant undefined check
MagnusHJensen
left a comment
There was a problem hiding this comment.
Looks great, just a few small things
MagnusHJensen
left a comment
There was a problem hiding this comment.
Just a couple small changes left, then we are good to go
6532271 to
e1c4769
Compare
Related issue: Resolves #45179
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Added/updated automated tests
QA'd all new/changed functionality manually
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests