Fix My device page software sorting by display name (#45836)
**Related issue:** Closes #43673 (remaining issue reported by @getvictor after PR #44873) ## Changes The "My device" page / host details software tab sorts software by `software_titles.name` (often an installer filename) instead of the custom display name. PR #44873 fixed this for the global `/software/titles` endpoint but missed the host-specific `ListHostSoftware` query path. **Fix:** Add a `LEFT JOIN software_title_display_names` to the outer query wrapper in `ListHostSoftware`, and update `hostSoftwareAllowedOrderKeys` to use `COALESCE(NULLIF(stdn.display_name, ''), name)` so display names are used for sorting when set. **1 file changed:** `server/datastore/mysql/software.go` # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Dante Catalfamo <43040593+dantecatalfamo@users.noreply.github.com>
This commit is contained in:
co-authored by
copilot-swe-agent[bot]
Dante Catalfamo
parent
52c176f3ad
commit
faff41e41d
@@ -0,0 +1 @@
|
||||
- Fixed "My device" page to sort software by display name instead of installer filename when a custom display name is set.
|
||||
@@ -4683,9 +4683,11 @@ func promoteSoftwareTitleInHouseApp(softwareTitleRecord *hostSoftware) {
|
||||
|
||||
// hostSoftwareAllowedOrderKeys is minimal: the service layer pins OrderKey to "name".
|
||||
// "source" is included for test determinism (used as the secondary order key in tests).
|
||||
// "name" uses COALESCE(NULLIF(...)) so that a custom display name (when set) is used
|
||||
// for sorting, falling back to the software title name (often an installer filename).
|
||||
var hostSoftwareAllowedOrderKeys = common_mysql.OrderKeyAllowlist{
|
||||
"name": "name",
|
||||
"source": "source",
|
||||
"name": "COALESCE(NULLIF(stdn.display_name, ''), combined_results.name)",
|
||||
"source": "combined_results.source",
|
||||
}
|
||||
|
||||
// hostSoftwareTitleAssembler accumulates and de-duplicates host software title records
|
||||
@@ -6522,7 +6524,11 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt
|
||||
`)
|
||||
}
|
||||
stmt = fmt.Sprintf(stmt, replacements...)
|
||||
stmt = fmt.Sprintf("SELECT * FROM (%s) AS combined_results", stmt)
|
||||
stmt = fmt.Sprintf(
|
||||
"SELECT combined_results.* FROM (%s) AS combined_results LEFT JOIN software_title_display_names stdn ON stdn.software_title_id = combined_results.id AND stdn.team_id = ?",
|
||||
stmt,
|
||||
)
|
||||
args = append(args, globalOrTeamID)
|
||||
stmt, _, err = appendListOptionsToSQLSecure(stmt, &opts.ListOptions, hostSoftwareAllowedOrderKeys)
|
||||
if err != nil {
|
||||
return nil, nil, ctxerr.Wrap(ctx, err, "list host software")
|
||||
|
||||
@@ -137,6 +137,7 @@ func TestSoftware(t *testing.T) {
|
||||
{"GetSoftwareCategoryNameToIDMap", testGetSoftwareCategoryNameToIDMap},
|
||||
{"BatchNewSoftwareCategoriesIdempotent", testBatchNewSoftwareCategoriesIdempotent},
|
||||
{"CreateIntermediateInstallFailureRecordAfterDeletion", testCreateIntermediateInstallFailureRecordAfterDeletion},
|
||||
{"ListHostSoftwareSortByDisplayName", testListHostSoftwareSortByDisplayName},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
@@ -13027,6 +13028,84 @@ func testGetSoftwareCategoryNameToIDMap(t *testing.T, ds *Datastore) {
|
||||
assert.Empty(t, got)
|
||||
}
|
||||
|
||||
func testListHostSoftwareSortByDisplayName(t *testing.T, ds *Datastore) {
|
||||
ctx := context.Background()
|
||||
|
||||
// Create a team.
|
||||
team, err := ds.NewTeam(ctx, &fleet.Team{Name: "Display Name Sort Team"})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a host on the team.
|
||||
host := test.NewHost(t, ds, "sorthost", "", "sorthostkey", "sorthostuuid", time.Now())
|
||||
err = ds.AddHostsToTeam(ctx, fleet.NewAddHostsToTeamParams(&team.ID, []uint{host.ID}))
|
||||
require.NoError(t, err)
|
||||
// Reload host to get TeamID set.
|
||||
host, err = ds.Host(ctx, host.ID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Install software on the host.
|
||||
sw := []fleet.Software{
|
||||
{Name: "alpha", Version: "1.0", Source: "apps"},
|
||||
{Name: "bravo", Version: "1.0", Source: "apps"},
|
||||
{Name: "zzz-installer", Version: "1.0", Source: "apps"},
|
||||
}
|
||||
_, err = ds.UpdateHostSoftware(ctx, host.ID, sw)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
|
||||
require.NoError(t, ds.CleanupSoftwareTitles(ctx))
|
||||
require.NoError(t, ds.SyncHostsSoftwareTitles(ctx, time.Now()))
|
||||
|
||||
// Look up the title IDs via ListSoftwareTitles.
|
||||
adminFilter := fleet.TeamFilter{User: &fleet.User{GlobalRole: new(fleet.RoleAdmin)}}
|
||||
titles, _, _, err := ds.ListSoftwareTitles(ctx, fleet.SoftwareTitleListOptions{
|
||||
ListOptions: fleet.ListOptions{OrderKey: "name", OrderDirection: fleet.OrderAscending},
|
||||
TeamID: &team.ID,
|
||||
}, adminFilter)
|
||||
require.NoError(t, err)
|
||||
|
||||
titleByName := func(name string) uint {
|
||||
for _, tt := range titles {
|
||||
if tt.Name == name {
|
||||
return tt.ID
|
||||
}
|
||||
}
|
||||
t.Fatalf("title %q not found", name)
|
||||
return 0
|
||||
}
|
||||
|
||||
alphaID := titleByName("alpha")
|
||||
scriptID := titleByName("zzz-installer")
|
||||
|
||||
bravoID := titleByName("bravo")
|
||||
|
||||
// Set display names that reorder the titles:
|
||||
// alpha -> "Zulu" (should sort last)
|
||||
// bravo -> "" (empty string, NULLIF falls back to "bravo")
|
||||
// zzz-installer -> "AAA Script" (should sort first despite filename)
|
||||
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
|
||||
if err := updateSoftwareTitleDisplayName(ctx, q, &team.ID, alphaID, "Zulu"); err != nil {
|
||||
return err
|
||||
}
|
||||
// Explicitly set empty display name to exercise the NULLIF(display_name, '') fallback.
|
||||
if err := updateSoftwareTitleDisplayName(ctx, q, &team.ID, bravoID, ""); err != nil {
|
||||
return err
|
||||
}
|
||||
return updateSoftwareTitleDisplayName(ctx, q, &team.ID, scriptID, "AAA Script")
|
||||
})
|
||||
|
||||
// List host software sorted by name ASC.
|
||||
// Expected order: AAA Script (zzz-installer), bravo, Zulu (alpha).
|
||||
hostSw, _, err := ds.ListHostSoftware(ctx, host, fleet.HostSoftwareTitleListOptions{
|
||||
ListOptions: fleet.ListOptions{OrderKey: "name", OrderDirection: fleet.OrderAscending},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, hostSw, 3)
|
||||
assert.Equal(t, "zzz-installer", hostSw[0].Name, "AAA Script (zzz-installer) should sort first")
|
||||
assert.Equal(t, "bravo", hostSw[1].Name, "bravo (no display name) should sort second")
|
||||
assert.Equal(t, "alpha", hostSw[2].Name, "Zulu (alpha) should sort last")
|
||||
}
|
||||
|
||||
func testBatchNewSoftwareCategoriesIdempotent(t *testing.T, ds *Datastore) {
|
||||
ctx := context.Background()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user