Show the primary user email on the Hosts page when multiple are present#48869
Show the primary user email on the Hosts page when multiple are present#48869spalmesano0 wants to merge 5 commits into
Conversation
Priority for email: IdP email, Chrome profile, anything else.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48869 +/- ##
==========================================
+ Coverage 68.03% 68.08% +0.05%
==========================================
Files 3689 3692 +3
Lines 234203 234265 +62
Branches 12303 12461 +158
==========================================
+ Hits 159338 159510 +172
+ Misses 60547 60436 -111
- Partials 14318 14319 +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:
|
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Updates the Hosts page “User email” column to show a primary (preferred-source) email address directly in the table, with a +N suffix and tooltip to reveal additional emails when multiple are present. This improves host ownership discoverability in environments where “Used by” can be ambiguous.
Changes:
- Replaces the “N users” display with a primary email + optional
+Nsuffix and tooltip listing additional emails. - Adds
getPrimaryDeviceUserhelper (with unit tests) to pick an IdP email first, then Chrome profile, then first available. - Extends
TooltipTruncatedTextCellwithjustifySuffixEndandshowTooltipWithSuffixoptions, plus styling support.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| frontend/pages/hosts/ManageHostsPage/HostTableConfig.tsx | Picks a primary device user email and renders it with suffix + tooltip in the Hosts table. |
| frontend/pages/hosts/ManageHostsPage/HostTableConfig.tests.tsx | Adds unit tests for primary-email selection and rendering behavior in the Hosts table column. |
| frontend/components/TableContainer/DataTable/TooltipTruncatedTextCell/TooltipTruncatedTextCell.tsx | Adds props to control suffix alignment and tooltip enabling when suffix is present. |
| frontend/components/TableContainer/DataTable/TooltipTruncatedTextCell/_styles.scss | Adds CSS to pin the suffix to the right edge when requested. |
| changes/30758-idp-username-hosts-column | User-visible change note (content excluded from review by policy). |
Files excluded by content exclusion policy (1)
- changes/30758-idp-username-hosts-column
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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 for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Hosts table "User email" column now renders a primary email chosen from IdP accounts, then Chrome profiles, then the first available user, with a Changes
Sequence Diagram(s)flowchart TD
DeviceUsers --> getPrimaryDeviceUser
getPrimaryDeviceUser --> SelectByPriority
SelectByPriority --> ComputeSuffixAndTooltipLines
ComputeSuffixAndTooltipLines --> TooltipTruncatedTextCellRender
TooltipTruncatedTextCellRender --> TooltipVisibilityCheck
TooltipVisibilityCheck --> RenderedHostRow
Related issues: Suggested labels: frontend, enhancement Suggested reviewers: noahtalerman 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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
🧹 Nitpick comments (1)
frontend/components/TableContainer/DataTable/TooltipTruncatedTextCell/TooltipTruncatedTextCell.tsx (1)
41-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider widening the return type to
React.ReactNode.Static analysis flags
JSX.Elementas too narrow at Line 43; it excludesnull, strings, and fragments that this component could plausibly return in the future.♻️ Proposed fix
-}: ITooltipTruncatedTextCellProps): JSX.Element => { +}: ITooltipTruncatedTextCellProps): React.ReactNode => {🤖 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/TooltipTruncatedTextCell/TooltipTruncatedTextCell.tsx` around lines 41 - 47, The return type on TooltipTruncatedTextCell is too narrow with JSX.Element and should be widened to React.ReactNode. Update the component signature for TooltipTruncatedTextCell so it uses React.ReactNode instead, keeping it compatible with null, strings, and fragments while preserving the existing props and classNames logic.Source: Linters/SAST tools
🤖 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/hosts/ManageHostsPage/HostTableConfig.tests.tsx`:
- Around line 85-93: The test in getPrimaryDeviceUser is using toContain with
expect.stringContaining("more"), which compares the matcher object directly
instead of checking each tooltip line’s text. Update the assertion to use a
predicate on tooltipLines so it actually verifies that no entry contains “more”,
while keeping the existing exact-length check for the cap case.
---
Nitpick comments:
In
`@frontend/components/TableContainer/DataTable/TooltipTruncatedTextCell/TooltipTruncatedTextCell.tsx`:
- Around line 41-47: The return type on TooltipTruncatedTextCell is too narrow
with JSX.Element and should be widened to React.ReactNode. Update the component
signature for TooltipTruncatedTextCell so it uses React.ReactNode instead,
keeping it compatible with null, strings, and fragments while preserving the
existing props and classNames logic.
🪄 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: 94fda647-8f5e-4944-96b5-fbfb6e49863a
📒 Files selected for processing (5)
changes/30758-idp-username-hosts-columnfrontend/components/TableContainer/DataTable/TooltipTruncatedTextCell/TooltipTruncatedTextCell.tsxfrontend/components/TableContainer/DataTable/TooltipTruncatedTextCell/_styles.scssfrontend/pages/hosts/ManageHostsPage/HostTableConfig.tests.tsxfrontend/pages/hosts/ManageHostsPage/HostTableConfig.tsx
Related issue: Resolves #30758
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing
Summary by CodeRabbit
+Nindicator and a tooltip listing the remaining emails (capped with+N morewhen applicable).---) when no emails exist.