Fix My device software list appending macos_applications param on pagination#48145
Conversation
…ination The macos_applications query param (added in #46223 for the host details page filter) was being appended to the URL when paginating the My device software list, even though that page has no /Applications filter dropdown. Gate macosApplicationsFilter on !isMyDevicePage at its source so it stays undefined on the My device page, preventing the param from being sent to the API or appended to the URL. Resolves #39017
![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.
dantecatalfamo
had a problem deploying
to
Docker Hub
GitHub Actions
Failure
|
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 fix addresses pagination on the My device > Software tab incorrectly appending a 🚥 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.
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
Fixes an unintended macos_applications=true query param being appended during pagination on My device > Software for macOS devices, even though that page intentionally hides the /Applications filter UI. The fix ensures the filter is only computed/applied on the host details software page.
Changes:
- Gate
macosApplicationsFilterso it’sundefinedon the My device page (prevents URL/API leakage during pagination). - Add regression tests covering pagination URL behavior when the filter is set vs. undefined.
- Add a
changes/entry for the user-visible fix (content not visible due to repo policy).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/pages/hosts/details/cards/Software/HostSoftware.tsx | Gates macosApplicationsFilter on !isMyDevicePage so My device pagination doesn’t append macos_applications. |
| frontend/pages/hosts/details/cards/Software/HostSoftwareTable/HostSoftwareTable.tests.tsx | Adds regression tests verifying pagination appends/omits macos_applications appropriately. |
| changes/39017-mydevice-macos-applications-query-param | Release note entry (diff excluded by policy). |
Files excluded by content exclusion policy (1)
- changes/39017-mydevice-macos-applications-query-param
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Server-side pagination only renders when a full page of rows is present | ||
| // (DEFAULT_PAGE_SIZE = 20) and there are further results. | ||
| const fullPageWithNextResults = createMockGetHostSoftwareResponse({ | ||
| software: Array.from({ length: 20 }, (_, index) => | ||
| createMockHostSoftware({ id: index + 1 }) | ||
| ), | ||
| meta: { has_next_results: true, has_previous_results: false }, | ||
| }); |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #48145 +/- ##
=======================================
Coverage 67.32% 67.32%
=======================================
Files 3657 3657
Lines 231233 231233
Branches 12062 12062
=======================================
+ Hits 155676 155687 +11
+ Misses 61590 61579 -11
Partials 13967 13967
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:
|
Use a fresh createMockRouter() per pagination test and assert on its replace mock directly, instead of calling .mockClear() on the shared router (typed as LocationFunction, so .mockClear() failed tsc).
dantecatalfamo
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
| data: fullPageWithNextResults, | ||
| }); | ||
|
|
||
| fireEvent.click(screen.getByRole("button", { name: /next/i })); |
There was a problem hiding this comment.
we're moving away from fireEvent and using user.click, not worth blocking
Related issue: Resolves #39017
Related issue: Resolves #47846
Follow up for #46223
Summary
PR #46223 introduced a
macos_applicationsquery param for the host details software list, controlled by an/Applicationsfilter dropdown. On the My device > Software tab, that dropdown is intentionally hidden — butmacosApplicationsFilterwas still computed as defined (true) for macOS devices, so paginating the list appendedmacos_applications=trueto the URL.The fix gates
macosApplicationsFilteron!isMyDevicePageat its source inHostSoftware.tsx, leaving itundefinedon the My device page. This is a single root-cause change: it stops the param from being appended to the URL on pagination, sent to the API, or otherwise leaking into a page that has no filter control.Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Added two regression tests in
HostSoftwareTable.tests.tsx: pagination appendsmacos_applicationswhen the filter is set (host details page), and does not append it when the filter is undefined (My device page).Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
macos_applicationsquery parameter when the current page shouldn’t use the macOS Applications filter.Tests
macos_applicationsparameter is included in navigation URLs.