Include mobile hosts by default in "Hosts online" chart#48769
Include mobile hosts by default in "Hosts online" chart#48769spalmesano0 wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48769 +/- ##
==========================================
- Coverage 68.02% 68.01% -0.02%
==========================================
Files 3680 3681 +1
Lines 233960 233961 +1
Branches 12419 12270 -149
==========================================
- Hits 159146 159121 -25
- Misses 60496 60523 +27
+ Partials 14318 14317 -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
This PR updates the Dashboard “Hosts online” chart to include mobile platforms (iOS/iPadOS/Android) by default on page load, matching the intended post-4.89 behavior described in #47661.
Changes:
- Removes the chart’s default platform filter so the initial request includes all platforms (mobile + non-mobile).
- Updates the chart tooltip/copy to remove the “mobile excluded by default” messaging.
- Updates the ChartCard test to reflect the new default behavior (no “Filtered” badge, no
platformsquery param).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/pages/DashboardPage/cards/ChartCard/ChartFilterModal/ChartFilterModal.tsx | Simplifies comment above PLATFORM_OPTIONS now that ChartCard no longer hard-codes default platform exclusions. |
| frontend/pages/DashboardPage/cards/ChartCard/ChartCard.tsx | Removes the seeded default platforms filter; updates tooltip text accordingly. |
| frontend/pages/DashboardPage/cards/ChartCard/ChartCard.tests.tsx | Updates default-platform test expectations; currently needs a small adjustment to ensure the request handler is actually hit. |
| changes/47661-hosts-online-chart-include-mobile-by-default | User-visible change note (content excluded from this review by policy). |
Files excluded by content exclusion policy (1)
- changes/47661-hosts-online-chart-include-mobile-by-default
Comments suppressed due to low confidence (1)
frontend/pages/DashboardPage/cards/ChartCard/ChartCard.tests.tsx:207
- This test can pass even if the chart request never fires because
requestedPlatformsstarts asnull(the same value you'd get when the query param is absent). If the handler isn't hit (e.g., due to React Query cache), the assertiontoBeNull()still passes. Initialize toundefinedand assert it becomesnullafter rendering; also prefer queryingrectelements from the rendered container rather thandocumentfor isolation.
it("includes mobile platforms by default and does not show the Filtered badge", async () => {
let requestedPlatforms: string | null = null;
mockServer.use(
http.get(baseUrl("/charts/:metric"), ({ params, request }) => {
requestedPlatforms = new URL(request.url).searchParams.get("platforms");
💡 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 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.
|
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 (4)
WalkthroughThe default platform filter on the "hosts online" chart is removed so mobile platforms (iOS, iPadOS, Android) are included by default alongside desktop platforms. Changes
Related issues
Possibly related PRs
Suggested labels: Suggested reviewers: noahtalerman 🐰 A chart once hid its mobile friends away, 🚥 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 |
Related issue: Resolves #47661
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing
Summary by CodeRabbit
New Features
Bug Fixes
Tests