Update URL params in UI to use fleet naming instead of team - #39870
Conversation
GitHub Actions
Failure
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 39329-rename-teams-to-fleets-frontend #39870 +/- ##
========================================================================
Coverage ? 66.21%
========================================================================
Files ? 2432
Lines ? 194828
Branches ? 8551
========================================================================
Hits ? 129000
Misses ? 54119
Partials ? 11709
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:
|
GitHub Actions
Failure
GitHub Actions
Failure
GitHub Actions
Failure
|
@CodeRabbit-ai full review |
|
✅ Actions performedFull review triggered. |
WalkthroughThis PR systematically renames the query parameter from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
frontend/pages/hosts/details/cards/HostSummary/HostSummary.tests.tsx (1)
31-56:⚠️ Potential issue | 🟡 MinorIncomplete rename: test descriptions on lines 32 and 49 still reference "team".
The
describeblock was renamed to"Fleet data", but the nesteditdescriptions were not updated to match. For consistency with the rest of the rename:Proposed fix
- it("renders the team name when present", () => { + it("renders the fleet name when present", () => {- it("renders 'No team' when team_name is '---'", () => { + it("renders 'No fleet' when team_name is '---'", () => {frontend/pages/SoftwarePage/SoftwareOSDetailsPage/SoftwareOSDetailsPage.tsx (1)
60-66:⚠️ Potential issue | 🔴 CriticalChange
team_idtofleet_idon line 64.The
queryParamsobject is passed toSoftwareDetailsSummary, which uses it to build a URL for the "View all hosts" navigation link topaths.MANAGE_HOSTS. TheManageHostsPagecomponent expects thefleet_idquery parameter, notteam_id. This inconsistency also appears in several other files within the SoftwarePage directory (e.g.,SoftwareVersionDetailsPage.tsx,SoftwareVulnerabilitiesTable.tsx,SoftwareSummaryCard.tsx), while most related components already usefleet_idcorrectly.frontend/pages/hosts/details/HostQueryReport/HostQueryReport.tsx (1)
112-142:⚠️ Potential issue | 🟡 MinorStale closure:
currentTeammissing fromuseCallbackdeps.
fullReportPathcapturescurrentTeam?.id(Line 115), butcurrentTeamis not in the dependency array on Line 142. If the current team changes while this component is mounted, the callback will use a stale value.Proposed fix
- }, [queryId, hostId, hqrError, hostName]); + }, [queryId, hostId, hqrError, hostName, currentTeam?.id]);frontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnSoftwareVersions/SwVulnSwTableConfig.tsx (1)
94-108:⚠️ Potential issue | 🟡 MinorAdd
fleet_idto ViewAllHostsLink queryParams for team scoping consistency.The ViewAllHostsLink is missing the
fleet_idparameter. The Name column at line 41 includesfleet_id: teamIdForApi, and similar ViewAllHostsLink usages throughout the codebase (TitleVersionsTableConfig.tsx, SoftwareVulnerabilitiesTableConfig.tsx, OSKernelsTableConfig.tsx, etc.) all includefleet_idfor proper team scoping. Without it, the "View all hosts" link may lose team filtering.Proposed fix
<ViewAllHostsLink queryParams={{ software_version_id: cellProps.row.original.id, + fleet_id: teamIdForApi, }} responsive rowHover />frontend/pages/ManageControlsPage/Scripts/ScriptBatchDetailsPage/ScriptBatchDetailsPage.tsx (1)
188-194:⚠️ Potential issue | 🟡 MinorMissed
team_id→fleet_idrename inViewAllHostsLinkquery params.Line 192 still passes
team_id: batchDetails?.team_idwhile the rest of the PR (e.g.,SoftwareVulnerabilitiesTableConfig.tsxline 221) usesfleet_idinViewAllHostsLinkquery params.Proposed fix
<ViewAllHostsLink queryParams={{ script_batch_execution_status: selectedHostStatus, script_batch_execution_id: batchExecutionId, - team_id: batchDetails?.team_id, + fleet_id: batchDetails?.team_id, }} />frontend/pages/SoftwarePage/SoftwareTitles/SoftwareTable/SoftwareTable.tsx (1)
228-244:⚠️ Potential issue | 🟡 MinorMissed rename in
handleCustomFilterDropdownChange:teamIdconverts toteam_idviaconvertParamsToSnakeCase.The other handlers in this file (
generateNewQueryParams,handleShowVersionsToggle,handleRowSelect) all explicitly usefleet_idas the query param key. However, this handler builds anISoftwareApiParamsobject with ateamIdproperty that gets converted toteam_idbyconvertParamsToSnakeCaseon line 242, producing the old param name in the URL.Proposed fix
const queryParams: ISoftwareApiParams = { query, - teamId, orderDirection, orderKey, page: 0, // resets page index ...buildSoftwareVulnFiltersQueryParams(vulnFilters), ...buildSoftwareFilterQueryParams(value), }; + const snakeCaseParams = convertParamsToSnakeCase(queryParams); + router.replace( getNextLocationPath({ pathPrefix: currentPath, routeTemplate: "", - queryParams: convertParamsToSnakeCase(queryParams), + queryParams: { ...snakeCaseParams, fleet_id: teamId }, }) );frontend/pages/SoftwarePage/SoftwareVersionDetailsPage/SoftwareVersionDetailsPage.tsx (1)
189-192:⚠️ Potential issue | 🟡 MinorChange
team_idtofleet_idin SoftwareDetailsSummary query params.Line 191 passes
team_id: teamIdForApito query params that are used for URL construction viagetPathWithQueryParams(paths.MANAGE_HOSTS, queryParams). This should befleet_idto match the route params interface (line 47) and align with other MANAGE_HOSTS URL constructions across the codebase (e.g., SoftwareVulnSummary, Dashboard cards).Proposed fix
queryParams={{ software_version_id: softwareVersion.id, - team_id: teamIdForApi, + fleet_id: teamIdForApi, }}frontend/hooks/useTeamIdParam.ts (1)
141-148:⚠️ Potential issue | 🟠 MajorPre-existing bug:
splice(-1, 1, newPart)whenparamIndex === -1.The condition on line 143 checks
paramIndex === -1(param not found) and then callsparts.splice(-1, 1, newPart), which removes the last element and replaces it—clearly unintended. TheshouldReplacebranch (lines 141–148) has theif/elseinverted:spliceshould run whenparamIndex !== -1, andpushwhen it is-1.This is a pre-existing bug (not introduced by this PR), but worth flagging since the backward-compat changes now funnel more traffic through this function.
Proposed fix
if (shouldReplace) { const newPart = `${paramName}=${replaceString}`; - if (paramIndex === -1) { - parts.splice(paramIndex, 1, newPart); - } else { + if (paramIndex !== -1) { + parts.splice(paramIndex, 1, newPart); + } else { parts.push(newPart); } }
🤖 Fix all issues with AI agents
In `@frontend/hooks/useTeamIdParam.ts`:
- Around line 364-372: After performing the legacy-URL redirect in the check
where search.includes("team_id=") and router.replace(...) is called, return
early from the hook so the rest of the logic (including
shouldRedirectToDefaultTeam and handleTeamChange) does not run against stale
query params; specifically, add an early return immediately after the
router.replace call (or move this redirect into a useEffect with a guard) to
prevent a second router.replace from overwriting the intended fleet_id redirect.
In
`@frontend/pages/hosts/ManageHostsPage/components/RunScriptBatchModal/RunScriptBatchModal.tsx`:
- Line 180: In RunScriptBatchModal remove the stray closing parenthesis from the
URL template literals so the generated links are well-formed; update the two
places that build the to prop using PATHS.CONTROLS_SCRIPTS_BATCH_PROGRESS (the
"scheduled" and "started" links) to produce
`${PATHS.CONTROLS_SCRIPTS_BATCH_PROGRESS}?status=scheduled&fleet_id=${teamId}`
and `${PATHS.CONTROLS_SCRIPTS_BATCH_PROGRESS}?status=started&fleet_id=${teamId}`
respectively (i.e., delete the trailing `)` after `${teamId}`).
🧹 Nitpick comments (6)
frontend/pages/DashboardPage/components/MdmSolutionModal/MdmSolutionModalTableConfig.tsx (1)
43-44: Nit: internal parameterteamIdstill uses old "team" naming.The function parameter on line 44 is still called
teamIdwhile it now maps to thefleet_idquery param. Consider renaming it tofleetIdfor consistency when the broader internal rename pass happens.export const generateSolutionsTableHeaders = ( - teamId?: number + fleetId?: number ): IDataColumn[] => [And on line 78:
- queryParams={{ mdm_id: cellProps.row.original.id, fleet_id: teamId }} + queryParams={{ mdm_id: cellProps.row.original.id, fleet_id: fleetId }}frontend/pages/DashboardPage/sections/PlatformHostCounts/PlatformHostCounts.tsx (1)
206-208: Consider usinggetPathWithQueryParamsfor consistency with the other platform cards.Every other platform card uses
getPathWithQueryParams(PATHS.MANAGE_HOSTS_LABEL(...), { fleet_id: teamId }), but Android manually concatenates the query string. This is a pre-existing inconsistency, but now would be a good time to align it.♻️ Suggested refactor
- path={PATHS.MANAGE_HOSTS_LABEL(androidLabelId).concat( - teamId !== undefined ? `?fleet_id=${teamId}` : "" - )} + path={getPathWithQueryParams(PATHS.MANAGE_HOSTS_LABEL(androidLabelId), { + fleet_id: teamId, + })}frontend/pages/ManageControlsPage/Scripts/cards/ScriptLibrary/ScriptLibrary.tsx (1)
78-85: Consider usingbuildQueryStringFromParamsfor consistency with the rest of the codebase.The manual string interpolation works but is fragile (e.g., trailing
&whenisPremiumTieris true, and thepageparam appended separately). Other files in this PR usegetPathWithQueryParamsorbuildQueryStringFromParamsfor URL construction. This pre-dates the PR but would be a good cleanup opportunity.♻️ Suggested refactor
- const path = PATHS.CONTROLS_SCRIPTS_LIBRARY; - const queryString = isPremiumTier ? `?fleet_id=${teamId}&` : "?"; - const onPrevPage = useCallback(() => { - router.push(path.concat(`${queryString}page=${currentPage - 1}`)); - }, [router, path, currentPage, queryString]); - const onNextPage = useCallback(() => { - router.push(path.concat(`${queryString}page=${currentPage + 1}`)); - }, [router, path, currentPage, queryString]); + const onPrevPage = useCallback(() => { + router.push( + getPathWithQueryParams(PATHS.CONTROLS_SCRIPTS_LIBRARY, { + ...(isPremiumTier && { fleet_id: teamId }), + page: currentPage - 1, + }) + ); + }, [router, isPremiumTier, teamId, currentPage]); + const onNextPage = useCallback(() => { + router.push( + getPathWithQueryParams(PATHS.CONTROLS_SCRIPTS_LIBRARY, { + ...(isPremiumTier && { fleet_id: teamId }), + page: currentPage + 1, + }) + ); + }, [router, isPremiumTier, teamId, currentPage]);frontend/pages/ManageControlsPage/Scripts/ScriptsNavItems.tsx (1)
31-39: Consider usinggetPathWithQueryParamsinstead of manual string interpolation.Other files in this PR (and the broader codebase) use
getPathWithQueryParamsfor building URLs with query parameters. These two paths manually interpolatefleet_idvia template literals, which is less consistent and doesn't benefit from the utility's encoding/handling.♻️ Suggested refactor
+import { getPathWithQueryParams } from "utilities/url"; + const useScriptNavItems = ( teamId: number | undefined ): ISideNavItem<IScriptsCardProps>[] => { return useMemo( () => [ { title: "Library", urlSection: "library", - path: `${PATHS.CONTROLS_SCRIPTS_LIBRARY}?fleet_id=${teamId || 0}`, + path: getPathWithQueryParams(PATHS.CONTROLS_SCRIPTS_LIBRARY, { fleet_id: teamId || 0 }), Card: ScriptLibrary, }, { title: "Batch progress", urlSection: "progress", - path: `${PATHS.CONTROLS_SCRIPTS_BATCH_PROGRESS}?fleet_id=${ - teamId || 0 - }`, + path: getPathWithQueryParams(PATHS.CONTROLS_SCRIPTS_BATCH_PROGRESS, { fleet_id: teamId || 0 }), Card: ScriptBatchProgress, }, ], [teamId] ); };frontend/pages/queries/live/LiveQueryPage/LiveQueryPage.tsx (1)
163-167: Pre-existing:goToQueryEditorhas stale closure due to empty dependency array.
useCallbackon line 163 capturesqueryId,currentTeamId, androuterat mount time only ([]deps). If any of these change, the callback will use stale values. This predates this PR but is worth noting since thefleet_idvalue passed here could be stale.Suggested fix
- const goToQueryEditor = useCallback(() => { - const path = queryId ? PATHS.EDIT_QUERY(queryId) : PATHS.NEW_QUERY; - - router.push(getPathWithQueryParams(path, { fleet_id: currentTeamId })); - }, []); + const goToQueryEditor = useCallback(() => { + const path = queryId ? PATHS.EDIT_QUERY(queryId) : PATHS.NEW_QUERY; + + router.push(getPathWithQueryParams(path, { fleet_id: currentTeamId })); + }, [queryId, currentTeamId, router]);frontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnOSVersions/SwVulnOSTableConfig.tsx (1)
93-106:ViewAllHostsLinkmissingfleet_idin query params — pre-existing but worth noting.The OS details link (line 42) now correctly passes
fleet_id, but theViewAllHostsLinkhere only passesos_version_idwithoutfleet_id. This is pre-existing behavior, but for consistency with the rest of the PR you may want to add it.Suggested diff
<ViewAllHostsLink queryParams={{ os_version_id: cellProps.row.original.os_version_id, + fleet_id: teamIdForApi, }}
8bdddc8
into
39329-rename-teams-to-fleets-frontend
1 participant
Summary by CodeRabbit
team_idtofleet_idacross the application for improved terminology clarity.