Count FMAs by slug (#48783)

**Related issue:** Resolves #48528

This keys the count, pagination, and the frontend row-combining on the
app's slug token (the prefix before `/`, shared across an app's platform
entries but distinct across apps). The count now equals the rows shown
in every view (macOS, Windows, All), and name-colliding apps stay as
separate rows.

  # Checklist for submitter

- [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

  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

* **New Features**
* Software listings now group platform-specific installers into a single
app row based on the app identifier, improving how macOS and Windows
entries appear together.

* **Bug Fixes**
* Apps with the same display name but different identifiers now stay
separate instead of being merged incorrectly.
* List counts and pagination now match the combined app view more
accurately across the software pages.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Carlo
2026-07-06 17:05:59 -04:00
committed by GitHub
parent a19a1c77ed
commit b3c2a6368d
8 changed files with 183 additions and 60 deletions
+1
View File
@@ -369,6 +369,7 @@ const DEFAULT_FLEET_MAINTAINED_APPS_MOCK: IFleetMaintainedApp = {
name: "test app",
version: "1.2.3",
platform: "darwin",
slug: "test-app/darwin",
};
export const createMockFleetMaintainedApp = (
@@ -5,7 +5,11 @@ import InstallerActionCell from "./InstallerActionCell";
describe("InstallerAction cell", () => {
it("renders add button if installer is available", async () => {
render(<InstallerActionCell value={{ id: 1, platform: "darwin" }} />);
render(
<InstallerActionCell
value={{ id: 1, platform: "darwin", slug: "test-app/darwin" }}
/>
);
expect(screen.getByText(/add/i)).toBeInTheDocument();
});
@@ -17,7 +21,12 @@ describe("InstallerAction cell", () => {
it("renders checkmark if installer is already added", async () => {
render(
<InstallerActionCell
value={{ id: 1, platform: "darwin", software_title_id: 1 }}
value={{
id: 1,
platform: "darwin",
slug: "test-app/darwin",
software_title_id: 1,
}}
/>
);
+1
View File
@@ -891,6 +891,7 @@ export interface IFleetMaintainedApp {
name: string;
version: string;
platform: FleetMaintainedAppPlatform;
slug: string; // "<app-token>/<platform>", e.g. "figma/darwin"; the token uniquely identifies an app across its platform entries
software_title_id?: number; // null unless the team already has the software added (as a Fleet-maintained app, App Store (app), or custom package)
}
@@ -0,0 +1,48 @@
import { IFleetMaintainedApp } from "interfaces/software";
import { combineAppsByPlatform } from "./FleetMaintainedAppsTable";
const app = (overrides: Partial<IFleetMaintainedApp>): IFleetMaintainedApp => ({
id: 1,
name: "App",
version: "1.0",
platform: "darwin",
slug: "app/darwin",
...overrides,
});
describe("combineAppsByPlatform", () => {
it("combines an app's macOS and Windows entries into a single row", () => {
const combined = combineAppsByPlatform([
app({ id: 1, name: "Figma", slug: "figma/darwin", platform: "darwin" }),
app({ id: 2, name: "Figma", slug: "figma/windows", platform: "windows" }),
]);
expect(combined).toHaveLength(1);
expect(combined[0].name).toBe("Figma");
expect(combined[0].macos?.id).toBe(1);
expect(combined[0].windows?.id).toBe(2);
});
it("keeps two distinct apps that share a display name as separate rows", () => {
// MacPaw Gemini and Google Gemini share the name "Gemini" but have
// different slug tokens, so they must not collapse into one row.
const combined = combineAppsByPlatform([
app({ id: 1, name: "Gemini", slug: "gemini/darwin", platform: "darwin" }),
app({
id: 2,
name: "Gemini",
slug: "google-gemini/darwin",
platform: "darwin",
}),
]);
expect(combined).toHaveLength(2);
// Each row keeps its own macOS entry (neither Gemini is hidden/overwritten).
expect(combined.map((c) => c.macos?.id).sort()).toEqual([1, 2]);
expect(combined.map((c) => c.macos?.slug).sort()).toEqual([
"gemini/darwin",
"google-gemini/darwin",
]);
});
});
@@ -42,28 +42,31 @@ const EmptyFleetAppsTable = () => (
/>
);
/** Used to convert FleetMaintainedApp API response which has separate entries
* for Windows FMA and macOS FMA into table friendly format that combines
* entries for the same app for different platforms */
const combineAppsByPlatform = (
/** Converts the FleetMaintainedApp API response, which has separate macOS and
* Windows entries, into a table-friendly format that combines an app's entries
* for different platforms into one row. Apps are keyed by their slug token (the
* prefix before "/"), not by name, so two distinct apps that share a display
* name stay as separate rows. */
export const combineAppsByPlatform = (
fmaList: IFleetMaintainedApp[]
): ICombinedFMA[] => {
const combinedApps: { [name: string]: ICombinedFMA } = {};
const combinedApps: { [appToken: string]: ICombinedFMA } = {};
fmaList.forEach((app: IFleetMaintainedApp) => {
const { name, platform, ...rest } = app;
const appToken = app.slug.split("/")[0];
if (!combinedApps[name]) {
combinedApps[name] = { name, macos: null, windows: null };
if (!combinedApps[appToken]) {
combinedApps[appToken] = { name, macos: null, windows: null };
}
if (platform === "darwin") {
combinedApps[name].macos = {
combinedApps[appToken].macos = {
platform: platform as FleetMaintainedAppPlatform,
...rest,
};
} else if (platform === "windows") {
combinedApps[name].windows = {
combinedApps[appToken].windows = {
platform: platform as FleetMaintainedAppPlatform,
...rest,
};
+31 -30
View File
@@ -13,8 +13,9 @@ import (
)
// maintainedAppsAllowedOrderKeys allowlists order keys for listing
// Fleet-maintained apps. The list is a combined-by-name view, so name is the
// only meaningful key; it's validation-only, since ORDER BY is hard-coded below.
// Fleet-maintained apps. The list is a combined-by-app view (see
// ListAvailableFleetMaintainedApps), so name is the only meaningful key; it's
// validation-only, since ORDER BY is hard-coded below.
var maintainedAppsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{
"name": "fma.name",
}
@@ -223,12 +224,14 @@ func (ds *Datastore) GetMaintainedAppBySlug(ctx context.Context, slug string, te
func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamID *uint, opt fleet.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) {
dbReader := ds.reader(ctx)
// We paginate by distinct app NAME, because the UI combines an app's macOS
// and Windows entries into a single row and an app must not be split across a
// page boundary. The count, by contrast, is the total number of apps (each
// platform entry is its own installable app). The team join lets us tell
// whether each app has already been added, which the "available only" filter
// needs.
// We paginate and count by distinct app token (the slug prefix, e.g. "figma"
// in "figma/darwin"), which identifies an app across its platform entries.
// The UI combines an app's macOS and Windows entries into one row, so an app
// must not be split across a page boundary and the count must equal the rows
// shown. Keying on the token rather than the name keeps two distinct apps that
// share a name (e.g. gemini/darwin and google-gemini/darwin) separate. The
// team join tells us whether each app is already added, for the "available
// only" filter.
fromClause := `FROM fleet_maintained_apps fma`
var fromArgs []any
if teamID != nil {
@@ -252,14 +255,10 @@ func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamI
where += ` AND team_titles.id IS NULL`
}
// Total count of matching apps. We count distinct rows (by primary key), not
// distinct names: an app's macOS and Windows entries are separate installable
// apps and are each counted, even though the UI combines them into one row.
// DISTINCT fma.id also collapses any duplicate rows from the team join's
// fan-out.
// Count by distinct token; DISTINCT also collapses the team join's fan-out.
countArgs := append(append([]any{}, fromArgs...), whereArgs...)
var filteredCount int
if err := sqlx.GetContext(ctx, dbReader, &filteredCount, `SELECT COUNT(DISTINCT fma.id) `+fromClause+where, countArgs...); err != nil {
if err := sqlx.GetContext(ctx, dbReader, &filteredCount, `SELECT COUNT(DISTINCT SUBSTRING_INDEX(fma.slug, '/', 1)) `+fromClause+where, countArgs...); err != nil {
return nil, nil, ctxerr.Wrap(ctx, err, "get fleet maintained apps count")
}
@@ -290,24 +289,26 @@ func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamI
direction = "DESC"
}
// Select the page of app names, fetching one extra to detect a next page.
// Select the page of app tokens, fetching one extra to detect a next page.
// Group by the token and order by the app's name (the token maps to a single
// name), with the token as a deterministic tiebreaker for same-named apps.
perPage := opt.GetPerPage()
pageNamesStmt := fmt.Sprintf(
`SELECT DISTINCT fma.name %s%s ORDER BY fma.name %s LIMIT %d OFFSET %d`,
fromClause, where, direction, perPage+1, perPage*opt.Page,
pageTokensStmt := fmt.Sprintf(
`SELECT SUBSTRING_INDEX(fma.slug, '/', 1) AS app_token %s%s GROUP BY app_token ORDER BY MIN(fma.name) %s, app_token %s LIMIT %d OFFSET %d`,
fromClause, where, direction, direction, perPage+1, perPage*opt.Page,
)
pageNamesArgs := append(append([]any{}, fromArgs...), whereArgs...)
var pageNames []string
if err := sqlx.SelectContext(ctx, dbReader, &pageNames, pageNamesStmt, pageNamesArgs...); err != nil {
return nil, nil, ctxerr.Wrap(ctx, err, "selecting fleet maintained app page names")
pageTokensArgs := append(append([]any{}, fromArgs...), whereArgs...)
var pageTokens []string
if err := sqlx.SelectContext(ctx, dbReader, &pageTokens, pageTokensStmt, pageTokensArgs...); err != nil {
return nil, nil, ctxerr.Wrap(ctx, err, "selecting fleet maintained app page tokens")
}
meta := &fleet.PaginationMetadata{HasPreviousResults: opt.Page > 0, TotalResults: uint(filteredCount)} //nolint:gosec // dismiss G115
if uint(len(pageNames)) > perPage { //nolint:gosec // dismiss G115
if uint(len(pageTokens)) > perPage { //nolint:gosec // dismiss G115
meta.HasNextResults = true
pageNames = pageNames[:perPage]
pageTokens = pageTokens[:perPage]
}
if len(pageNames) == 0 {
if len(pageTokens) == 0 {
// Page is past the last result.
return []fleet.MaintainedApp{}, meta, nil
}
@@ -317,13 +318,13 @@ func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamI
selectStmt := `SELECT fma.id, fma.name, fma.platform, fma.slug, `
var rowsArgs []any
if teamID != nil {
selectStmt += teamFMATitlesJoin + ` WHERE fma.name IN (?)`
rowsArgs = []any{teamID, teamID, pageNames}
selectStmt += teamFMATitlesJoin + ` WHERE SUBSTRING_INDEX(fma.slug, '/', 1) IN (?)`
rowsArgs = []any{teamID, teamID, pageTokens}
} else {
selectStmt += `NULL software_title_id FROM fleet_maintained_apps fma WHERE fma.name IN (?)`
rowsArgs = []any{pageNames}
selectStmt += `NULL software_title_id FROM fleet_maintained_apps fma WHERE SUBSTRING_INDEX(fma.slug, '/', 1) IN (?)`
rowsArgs = []any{pageTokens}
}
selectStmt += fmt.Sprintf(` ORDER BY fma.name %s, fma.platform ASC`, direction)
selectStmt += fmt.Sprintf(` ORDER BY fma.name %s, fma.slug ASC`, direction)
selectStmt, rowsArgs, err := sqlx.In(selectStmt, rowsArgs...)
if err != nil {
+70 -14
View File
@@ -24,6 +24,7 @@ func TestMaintainedApps(t *testing.T) {
{"Sync", testSync},
{"ListAndGetAvailableApps", testListAndGetAvailableApps},
{"ListAvailableAppsByNameAndFilters", testListAvailableAppsByNameAndFilters},
{"ListAvailableAppsSharedName", testListAvailableAppsSharedName},
{"SyncAndRemoveApps", testSyncAndRemoveApps},
{"GetMaintainedAppBySlug", testGetMaintainedAppBySlug},
{"ListAvailableAppsWindows", testListAvailableAppsWindows},
@@ -456,7 +457,7 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) {
maintained3.TitleID = nil
require.Equal(t, maintained3, gotApp)
// Ordering: the combined-by-name view is only meaningfully sortable by name,
// Ordering: the combined-by-app view is only meaningfully sortable by name,
// so "name" is the one allowed order key. expectedApps is declared in
// ascending name order, so we derive the expected name sequences from it.
appNames := func(apps []fleet.MaintainedApp) []string {
@@ -681,11 +682,11 @@ func testListAvailableAppsWindows(t *testing.T, ds *Datastore) {
require.Nil(t, apps[1].TitleID)
}
// testListAvailableAppsByNameAndFilters verifies that the list paginates by
// distinct app NAME (an app's macOS and Windows entries are combined into one
// logical app in the UI) while the total count is by distinct app row (each
// platform entry counted separately), and that the platform and available-only
// filters work server-side.
// testListAvailableAppsByNameAndFilters verifies that the list paginates and
// counts by distinct app TOKEN (the slug prefix), so an app's macOS and Windows
// entries are combined into one logical app for both pagination and the count
// (matching the single combined row the UI renders), and that the platform and
// available-only filters work server-side.
func testListAvailableAppsByNameAndFilters(t *testing.T, ds *Datastore) {
ctx := context.Background()
@@ -721,20 +722,21 @@ func testListAvailableAppsByNameAndFilters(t *testing.T, ds *Datastore) {
return fleet.MaintainedAppListOptions{ListOptions: o}
}
// Unfiltered: 6 apps (the count, one per platform entry) across 4 names, 6
// rows returned.
// Unfiltered: 4 apps (the count, one per app token: alpha, beta, gamma,
// delta) across 4 names, 6 rows returned (the raw per-platform entries the UI
// combines into 4 rows).
apps, meta, err := ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, listOpts(fleet.ListOptions{}))
require.NoError(t, err)
require.EqualValues(t, 6, meta.TotalResults)
require.EqualValues(t, 4, meta.TotalResults)
require.Len(t, apps, 6)
require.False(t, meta.HasNextResults)
// Pagination is by app name: a page of 2 names that includes a dual-platform
// Pagination is by app token: a page of 2 apps that includes a dual-platform
// app returns ALL of that app's rows, so an app is never split across a page
// boundary. Page 0 => Alpha (darwin+windows) + Beta (darwin) = 3 rows.
apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, listOpts(fleet.ListOptions{PerPage: 2}))
require.NoError(t, err)
require.EqualValues(t, 6, meta.TotalResults)
require.EqualValues(t, 4, meta.TotalResults)
require.True(t, meta.HasNextResults)
require.False(t, meta.HasPreviousResults)
require.Equal(t, []string{"Alpha", "Alpha", "Beta"}, appNames(apps))
@@ -774,11 +776,10 @@ func testListAvailableAppsByNameAndFilters(t *testing.T, ds *Datastore) {
// Available-only hides Beta (its only platform is added) but keeps the other
// three apps, which still have at least one not-yet-added platform. The count
// is the 5 not-yet-added platform entries (Alpha macOS+Windows, Gamma
// Windows, Delta macOS+Windows).
// is the 3 remaining app tokens (Alpha, Gamma, Delta).
apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, fleet.MaintainedAppListOptions{AvailableOnly: true, ListOptions: fleet.ListOptions{IncludeMetadata: true}})
require.NoError(t, err)
require.EqualValues(t, 5, meta.TotalResults)
require.EqualValues(t, 3, meta.TotalResults)
require.NotContains(t, appNames(apps), "Beta")
// Without the filter, Beta is still listed (as added).
@@ -794,6 +795,61 @@ func testListAvailableAppsByNameAndFilters(t *testing.T, ds *Datastore) {
require.ElementsMatch(t, []string{"Alpha", "Alpha", "Delta", "Delta"}, appNames(apps))
}
// testListAvailableAppsSharedName verifies that two distinct apps sharing a
// display name (e.g. "Gemini": gemini/darwin and google-gemini/darwin) are
// counted and listed as two separate apps. Keying on the slug token keeps them
// distinct, so the count matches the row count and neither app is hidden.
func testListAvailableAppsSharedName(t *testing.T, ds *Datastore) {
ctx := context.Background()
team, err := ds.NewTeam(ctx, &fleet.Team{Name: "Team Shared Name"})
require.NoError(t, err)
mkApp := func(name, slug, platform, ident string) {
_, err := ds.UpsertMaintainedApp(ctx, &fleet.MaintainedApp{
Name: name, Slug: slug, Platform: platform, UniqueIdentifier: ident,
})
require.NoError(t, err)
}
// Two different macOS apps that share the display name "Gemini".
mkApp("Gemini", "gemini/darwin", "darwin", "com.macpaw.site.Gemini2")
mkApp("Gemini", "google-gemini/darwin", "darwin", "com.google.GeminiMacOS")
assertTwoGeminis := func(opt fleet.MaintainedAppListOptions) {
apps, meta, err := ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, opt)
require.NoError(t, err)
// Both apps counted (not collapsed by shared name) ...
require.EqualValues(t, 2, meta.TotalResults)
// ... and both rows returned, with distinct slugs.
require.Len(t, apps, 2)
slugs := []string{apps[0].Slug, apps[1].Slug}
require.ElementsMatch(t, []string{"gemini/darwin", "google-gemini/darwin"}, slugs)
require.Equal(t, "Gemini", apps[0].Name)
require.Equal(t, "Gemini", apps[1].Name)
}
// Count must equal rows unfiltered and with the macOS platform filter.
assertTwoGeminis(fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}})
assertTwoGeminis(fleet.MaintainedAppListOptions{Platform: "darwin", ListOptions: fleet.ListOptions{IncludeMetadata: true}})
// Paginating one app at a time yields each Gemini on its own page, never
// splitting or dropping one.
page0, meta0, err := ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{PerPage: 1, IncludeMetadata: true}})
require.NoError(t, err)
require.EqualValues(t, 2, meta0.TotalResults)
require.Len(t, page0, 1)
require.True(t, meta0.HasNextResults)
page1, meta1, err := ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{PerPage: 1, Page: 1, IncludeMetadata: true}})
require.NoError(t, err)
require.EqualValues(t, 2, meta1.TotalResults)
require.Len(t, page1, 1)
require.False(t, meta1.HasNextResults)
require.True(t, meta1.HasPreviousResults)
// The two pages cover the two distinct apps.
require.NotEqual(t, page0[0].Slug, page1[0].Slug)
}
func testSoftwareTitleRenamingWindows(t *testing.T, ds *Datastore) {
ctx := context.Background()
@@ -21025,11 +21025,15 @@ func (s *integrationEnterpriseTestSuite) TestMaintainedApps() {
require.False(t, listMAResp.Meta.HasNextResults)
require.Len(t, listMAResp.FleetMaintainedApps, len(expectedApps))
// Count is the total number of platform-specific maintained apps: an app's
// macOS and Windows entries are counted separately, even though the UI
// combines them into a single row. The full unfiltered list returns one row
// per app, so the count equals the number of rows returned.
require.Equal(t, len(expectedApps), listMAResp.Count)
// Count is the number of distinct apps (by slug token), matching the combined
// rows the UI renders: an app's macOS and Windows entries share a token and
// count once. The unfiltered list returns every platform row for the UI to
// combine, so the row count can exceed the count.
distinctAppTokens := make(map[string]struct{}, len(expectedApps))
for _, a := range expectedApps {
distinctAppTokens[strings.SplitN(a.Slug, "/", 2)[0]] = struct{}{}
}
require.Equal(t, len(distinctAppTokens), listMAResp.Count)
sortFMAs := func(a, b fleet.MaintainedApp) int {
if c := cmp.Compare(a.Name, b.Name); c != 0 {