Use new MDM status on hosts page and show tooltip; show "Not supported" for Linux - #46377
Conversation
We now use "company-owned" instead of "automatic". Instead of taking the raw value from the API, we should map it to the new user-facing value.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
There was a problem hiding this comment.
Pull request overview
Updates the host table MDM Status column so the API value "On (automatic)" is displayed as "On (company-owned)" via the existing MDM_ENROLLMENT_STATUS_UI_MAP, and renders the MDM_STATUS_TOOLTIP text on hover with a no-wrap style.
Changes:
- Map raw enrollment status to its UI display name before rendering.
- Wrap the displayed status in
TooltipWrapperwhen a tooltip is defined for that status. - Add CSS to keep the status name on a single line within the tooltip wrapper.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/components/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tsx | Resolves displayName via MDM_ENROLLMENT_STATUS_UI_MAP and conditionally renders a tooltip from MDM_STATUS_TOOLTIP. |
| frontend/components/TableContainer/DataTable/HostMdmStatusCell/_styles.scss | Adds white-space: nowrap on the tooltip wrapper element to prevent wrapping. |
| changes/46066-fix-on-automatic-mdm-status-display | Changelog entry describing the user-visible fix and new tooltip. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThis PR updates HostMdmStatusCell to map raw MDM enrollment values to user-facing labels using MDM_ENROLLMENT_STATUS_UI_MAP, broadens the unsupported-platform guard, and conditionally wraps the display label in a TooltipWrapper when MDM_STATUS_TOOLTIP exists. It adds a CSS rule to prevent tooltip text wrapping and includes tests covering multiple platforms and enrollment statuses. Possibly related issues
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/components/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tsx`:
- Line 2: The import statements for DEFAULT_EMPTY_CELL_VALUE and
MDM_STATUS_TOOLTIP (and the other multiline import around line 9) are not
formatted to satisfy Prettier/ESLint; reformat the imports to follow Prettier
rules (e.g., break the named imports into properly indented multiline braces or
run Prettier to auto-fix) so the import of DEFAULT_EMPTY_CELL_VALUE,
MDM_STATUS_TOOLTIP (and the second multiline import referenced on line 9) is
correctly split/indented and passes linting.
🪄 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: 437282fc-e135-4716-b159-9bdf728980bf
📒 Files selected for processing (3)
changes/46066-fix-on-automatic-mdm-status-displayfrontend/components/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tsxfrontend/components/TableContainer/DataTable/HostMdmStatusCell/_styles.scss
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #46377 +/- ##
==========================================
+ Coverage 66.89% 66.92% +0.02%
==========================================
Files 2782 2798 +16
Lines 221734 221896 +162
Branches 11221 11316 +95
==========================================
+ Hits 148334 148495 +161
+ Misses 60000 59998 -2
- Partials 13400 13403 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks @MagnusHJensen! Just made those changes. While looking at this again, would it be worth it to use |
|
Thanks @spalmesano0 Yeah I think that is a good callout, let's add that in as well, now we're here, maybe a small test case for Linux as well. |
spalmesano0
changed the title
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.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/components/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tests.tsx (1)
15-59: ⚡ Quick winConsider adding tests for tooltip rendering.
The PR objectives mention "Adds a tooltip for the MDM status on the Hosts page," but there are no tests verifying that tooltips are rendered when expected. Adding tests to check for tooltip presence would help prevent regressions of this new feature.
Example test for tooltip
+ it("renders tooltip for supported MDM status values", () => { + renderCell("darwin", "On (automatic)"); + const tooltip = screen.getByRole("tooltip") || screen.queryByTestId("tooltip-wrapper"); + expect(tooltip).toBeInTheDocument(); + });Note: The exact query selector depends on how TooltipWrapper is implemented. Adjust based on the component's actual structure or add a test-id to the TooltipWrapper.
🤖 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/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tests.tsx` around lines 15 - 59, Add tests in HostMdmStatusCell.tests.tsx to assert the new tooltip is rendered for statuses that should show it: use the existing renderCell helper to render rows with MDM statuses that trigger tooltips (e.g., "On (manual)", "On (automatic)", pending states), then simulate hover (userEvent.hover or fireEvent.mouseOver) on the rendered status element and assert the TooltipWrapper output appears (query by role="tooltip", aria-describedby on the status, or a test-id on TooltipWrapper). If the TooltipWrapper has no test hook, add a data-testid to TooltipWrapper in HostMdmStatusCell so tests can reliably query it and then assert tooltip text content and visibility after hover.
🤖 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/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tests.tsx`:
- Around line 15-59: Add tests in HostMdmStatusCell.tests.tsx to assert the new
tooltip is rendered for statuses that should show it: use the existing
renderCell helper to render rows with MDM statuses that trigger tooltips (e.g.,
"On (manual)", "On (automatic)", pending states), then simulate hover
(userEvent.hover or fireEvent.mouseOver) on the rendered status element and
assert the TooltipWrapper output appears (query by role="tooltip",
aria-describedby on the status, or a test-id on TooltipWrapper). If the
TooltipWrapper has no test hook, add a data-testid to TooltipWrapper in
HostMdmStatusCell so tests can reliably query it and then assert tooltip text
content and visibility after hover.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1019ccb0-e7dd-46f8-a440-0bb46ef9faa2
📒 Files selected for processing (2)
frontend/components/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tests.tsxfrontend/components/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/components/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tsx
Related issue: Resolves #46066
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing
Summary by CodeRabbit
Bug Fixes
New Features
Style