Follow-up fix for FMA counts (#48818)
**Related issue:** Resolves #48528 Follow-up to #48783, which changed the Fleet-maintained apps "items" count from per-platform entries to per-app, dropping it from 1,263 to 1,023. This restores the count to `COUNT(DISTINCT fma.id)`: macOS and Windows entries are separately installable (each its own Add button), so each counts (1,263 / 960 macOS / 303 Windows). The token-based row-combining and pagination from #48783 are kept. # 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 * **Bug Fixes** * Corrected available-app counts in listings so pagination totals now match what users can actually add. * Improved pagination consistency for apps with multiple platform variants, reducing confusion where totals did not align with visible entries. * Updated team-based filtering so already-added apps are excluded more accurately from available results. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -224,14 +224,16 @@ 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 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.
|
||||
// We paginate 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. Keying on the token rather than the name
|
||||
// keeps two distinct apps that share a name (e.g. gemini/darwin and
|
||||
// google-gemini/darwin) as separate rows. The count, by contrast, is the
|
||||
// number of installable platform entries: each is separately installable (its
|
||||
// own Add button), so an app shipped on both platforms counts twice. 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 {
|
||||
@@ -255,10 +257,11 @@ func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamI
|
||||
where += ` AND team_titles.id IS NULL`
|
||||
}
|
||||
|
||||
// Count by distinct token; DISTINCT also collapses the team join's fan-out.
|
||||
// Count the installable platform entries (each Add button); DISTINCT id 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 SUBSTRING_INDEX(fma.slug, '/', 1)) `+fromClause+where, countArgs...); err != nil {
|
||||
if err := sqlx.GetContext(ctx, dbReader, &filteredCount, `SELECT COUNT(DISTINCT fma.id) `+fromClause+where, countArgs...); err != nil {
|
||||
return nil, nil, ctxerr.Wrap(ctx, err, "get fleet maintained apps count")
|
||||
}
|
||||
|
||||
|
||||
@@ -682,11 +682,11 @@ func testListAvailableAppsWindows(t *testing.T, ds *Datastore) {
|
||||
require.Nil(t, apps[1].TitleID)
|
||||
}
|
||||
|
||||
// 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.
|
||||
// testListAvailableAppsByNameAndFilters verifies that the list paginates by
|
||||
// distinct app TOKEN (the slug prefix), so an app's macOS and Windows entries
|
||||
// are combined into one row and never split across a page boundary, while the
|
||||
// count is the number of installable platform entries (each Add button counts
|
||||
// once), and that the platform and available-only filters work server-side.
|
||||
func testListAvailableAppsByNameAndFilters(t *testing.T, ds *Datastore) {
|
||||
ctx := context.Background()
|
||||
|
||||
@@ -722,12 +722,12 @@ func testListAvailableAppsByNameAndFilters(t *testing.T, ds *Datastore) {
|
||||
return fleet.MaintainedAppListOptions{ListOptions: o}
|
||||
}
|
||||
|
||||
// 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).
|
||||
// Unfiltered: 6 installable platform entries (the count: alpha+delta each ship
|
||||
// on two platforms) across 4 app tokens, 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, 4, meta.TotalResults)
|
||||
require.EqualValues(t, 6, meta.TotalResults)
|
||||
require.Len(t, apps, 6)
|
||||
require.False(t, meta.HasNextResults)
|
||||
|
||||
@@ -736,7 +736,7 @@ func testListAvailableAppsByNameAndFilters(t *testing.T, ds *Datastore) {
|
||||
// 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, 4, meta.TotalResults)
|
||||
require.EqualValues(t, 6, meta.TotalResults)
|
||||
require.True(t, meta.HasNextResults)
|
||||
require.False(t, meta.HasPreviousResults)
|
||||
require.Equal(t, []string{"Alpha", "Alpha", "Beta"}, appNames(apps))
|
||||
@@ -776,10 +776,11 @@ 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 3 remaining app tokens (Alpha, Gamma, Delta).
|
||||
// is the 5 not-yet-added platform entries (Alpha macOS+Windows, Gamma Windows,
|
||||
// Delta macOS+Windows).
|
||||
apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, fleet.MaintainedAppListOptions{AvailableOnly: true, ListOptions: fleet.ListOptions{IncludeMetadata: true}})
|
||||
require.NoError(t, err)
|
||||
require.EqualValues(t, 3, meta.TotalResults)
|
||||
require.EqualValues(t, 5, meta.TotalResults)
|
||||
require.NotContains(t, appNames(apps), "Beta")
|
||||
|
||||
// Without the filter, Beta is still listed (as added).
|
||||
|
||||
@@ -21025,15 +21025,12 @@ func (s *integrationEnterpriseTestSuite) TestMaintainedApps() {
|
||||
require.False(t, listMAResp.Meta.HasNextResults)
|
||||
require.Len(t, listMAResp.FleetMaintainedApps, len(expectedApps))
|
||||
|
||||
// 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)
|
||||
// Count is the total number of installable platform entries: an app's macOS
|
||||
// and Windows entries are separately installable (one Add button each), so
|
||||
// they count separately even though the UI combines them into a single row.
|
||||
// The unfiltered list returns every platform row, so here the count equals the
|
||||
// number of rows returned.
|
||||
require.Equal(t, len(expectedApps), listMAResp.Count)
|
||||
|
||||
sortFMAs := func(a, b fleet.MaintainedApp) int {
|
||||
if c := cmp.Compare(a.Name, b.Name); c != 0 {
|
||||
|
||||
Reference in New Issue
Block a user