Conversation
…"Next" button when a software title has multiple installer versions
Member
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes pagination metadata for Host Details → Software inventory when a single software title has multiple installer versions, which previously caused the UI “Next” button to be disabled incorrectly.
Changes:
- Updates
ListHostSoftwarepagination to computeHasNextResultsfrom the total unique title count rather than the post-deduped page length. - Adjusts the host software count query to count distinct title IDs.
- Adds a regression test covering pagination with multiple installers for the same software title.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/datastore/mysql/software.go | Fixes pagination metadata logic to avoid false “no next page” when installers create duplicate rows. |
| server/datastore/mysql/software_test.go | Adds a regression test reproducing the duplicate-installer pagination scenario. |
| changes/41233-host-software-has-results | Adds a user-visible changelog entry for the pagination fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
WalkthroughThis pull request fixes pagination logic in the host software listing feature. The Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/datastore/mysql/software_test.go (1)
11597-11605: Assert the actual page-0 titles as well.This only checks
len(sw)and metadata. A regression that still returns 5 rows but duplicatespagsw-04would pass even though dedup/pagination is wrong.Suggested test hardening
// Page 0: 5 unique titles, HasNextResults must be true (there are 10 total). opts.ListOptions.Page = 0 sw, meta, err := ds.ListHostSoftware(ctx, host, opts) require.NoError(t, err) require.Len(t, sw, perPage, "page 0 should have %d items", perPage) + gotNames := make([]string, 0, len(sw)) + for _, s := range sw { + gotNames = append(gotNames, s.Name) + } + require.Equal(t, []string{ + "pagsw-00", + "pagsw-01", + "pagsw-02", + "pagsw-03", + "pagsw-04", + }, gotNames) require.NotNil(t, meta) assert.Equal(t, uint(totalTitles), meta.TotalResults, "total results should reflect unique titles, not duplicated rows") assert.True(t, meta.HasNextResults, "page 0 of %d items with perPage=%d should have next results", totalTitles, perPage) assert.False(t, meta.HasPreviousResults)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/datastore/mysql/software_test.go` around lines 11597 - 11605, Add assertions to verify the actual page-0 titles returned by ListHostSoftware instead of only checking lengths and metadata: after calling ListHostSoftware (variables sw, meta), extract the Title field from each entry, assert the set size equals perPage and that all titles are unique (no duplicates like "pagsw-04"), and assert the set equals the expected first-page title list (or contains the known expected titles). This ensures the ListHostSoftware pagination/dedup logic (sw slice) returns the correct unique titles for page 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/datastore/mysql/software.go`:
- Around line 6070-6073: HasNextResults is computed from the distinct title
count but SQL pagination is still applied to raw rows (hostSoftwareList) before
deduplication, causing overlap/short pages; change the query flow so you first
page the distinct software_titles.id set (use software_titles.id as the
pagination key) to fetch only the titles for the requested page, then hydrate
installer/app details for those title ids to build hostSoftwareList and set
HasNextResults from the distinct count; update code paths around HasNextResults,
hostSoftwareList population, and the query that currently slices
hostSoftwareList (the pre-deduplication pagination) so pagination is done
against the deduplicated title ids before fetching installers.
---
Nitpick comments:
In `@server/datastore/mysql/software_test.go`:
- Around line 11597-11605: Add assertions to verify the actual page-0 titles
returned by ListHostSoftware instead of only checking lengths and metadata:
after calling ListHostSoftware (variables sw, meta), extract the Title field
from each entry, assert the set size equals perPage and that all titles are
unique (no duplicates like "pagsw-04"), and assert the set equals the expected
first-page title list (or contains the known expected titles). This ensures the
ListHostSoftware pagination/dedup logic (sw slice) returns the correct unique
titles for page 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3af2bd60-7e19-487a-b161-2c618a3f7bff
📒 Files selected for processing (3)
changes/41233-host-software-has-resultsserver/datastore/mysql/software.goserver/datastore/mysql/software_test.go
Comment on lines
+6070
to
+6073
| HasNextResults: titleCount > (opts.ListOptions.Page+1)*perPage, | ||
| } | ||
| if len(hostSoftwareList) > int(perPage) { //nolint:gosec // dismiss G115 | ||
| metaData.HasNextResults = true | ||
| hostSoftwareList = hostSoftwareList[:len(hostSoftwareList)-1] | ||
| hostSoftwareList = hostSoftwareList[:perPage] |
Contributor
There was a problem hiding this comment.
Page boundaries are still based on pre-deduped rows.
HasNextResults is now computed from the distinct title count, but the SQL pagination is still applied earlier at Line 5687, before Lines 5771-6059 collapse duplicate installer rows into one item per title. That means this change makes later pages reachable, but those pages can still overlap or come back short whenever one title expands into multiple raw rows. Page the distinct software_titles.id set first, then hydrate installer/app details for just that page.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/datastore/mysql/software.go` around lines 6070 - 6073, HasNextResults
is computed from the distinct title count but SQL pagination is still applied to
raw rows (hostSoftwareList) before deduplication, causing overlap/short pages;
change the query flow so you first page the distinct software_titles.id set (use
software_titles.id as the pagination key) to fetch only the titles for the
requested page, then hydrate installer/app details for those title ids to build
hostSoftwareList and set HasNextResults from the distinct count; update code
paths around HasNextResults, hostSoftwareList population, and the query that
currently slices hostSoftwareList (the pre-deduplication pagination) so
pagination is done against the deduplicated title ids before fetching
installers.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #41271 +/- ##
==========================================
+ Coverage 66.34% 66.36% +0.01%
==========================================
Files 2477 2477
Lines 198439 198459 +20
Branches 8740 8740
==========================================
+ Hits 131660 131699 +39
+ Misses 54877 54862 -15
+ Partials 11902 11898 -4
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:
|
jahzielv
approved these changes
Mar 9, 2026
getvictor
added a commit
that referenced
this pull request
Mar 9, 2026
…"Next" button when a software title has multiple installer versions (#41271) <!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #41233 # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Bug Fixes * Fixed pagination on the host software page to prevent the "Next" button from being incorrectly disabled when a software title has multiple installer versions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit f7595ba)
getvictor
added a commit
that referenced
this pull request
Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #41233
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing
Added/updated automated tests
QA'd all new/changed functionality manually
For unreleased bug fixes in a release candidate, one of:
Summary by CodeRabbit
Bug Fixes