Update API calls in front-end to use new, non-deprecated URLs and params - #41515
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #41515 +/- ##
==========================================
- Coverage 66.36% 66.35% -0.01%
==========================================
Files 2488 2489 +1
Lines 198855 198859 +4
Branches 8792 8904 +112
==========================================
- Hits 131962 131960 -2
- Misses 54972 54978 +6
Partials 11921 11921
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:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request systematically renames Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can customize the tone of the review comments and chat replies.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/services/entities/operating_systems.ts (1)
76-86:⚠️ Potential issue | 🔴 CriticalBackend API parameter mismatch: frontend sends
fleet_idbut backend handler expectsteam_id.The backend HTTP handler for OS versions endpoints is defined with
query:"team_id"(seeserver/service/vulnerabilities.goline 129), meaning it expects the query parameter to be namedteam_id. However, the frontend code now sendsfleet_id. This mismatch will cause the OS versions API calls to fail or ignore the team filtering parameter.The backend handler must be updated to accept
fleet_idinstead ofteam_idfor these endpoints, or the frontend change must be reverted to continue usingteam_id.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/services/entities/operating_systems.ts` around lines 76 - 86, The frontend is sending fleet_id but the backend expects team_id; update the params object in operating_systems.ts so the query uses team_id instead of fleet_id (i.e., replace the fleet_id key with team_id: teamId) and ensure the IGetOSVersionsRequestQueryParams type aligns with the team_id property so the outgoing query matches the backend handler.frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tsx (1)
58-69:⚠️ Potential issue | 🟠 MajorReturn type is narrower than the values produced here.
byName[...]can beundefined, butSelectedTeamIdsis inferred frommdmAbmAPI.editTeamsand requires numbers. That masks an invalid payload path from TypeScript and can end up PATCHingundefinedfleet ids if the token names ever drift fromavailableTeams. Please either validate before submit or make this helper return optional ids until validation happens.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tsx` around lines 58 - 69, getSelectedTeamIds may return undefined for byName[...] but currently returns a strict SelectedTeamIds expected by mdmAbmAPI.editTeams; update the helper to return optional ids instead so TypeScript surfaces missing matches. Change getSelectedTeamIds to return a type where ios_fleet_id, ipados_fleet_id, macos_fleet_id are number | undefined (e.g., SelectedTeamIdsOptional or Partial<SelectedTeamIds>), keep the same mapping logic using availableTeams and byName, and update callers (places that call getSelectedTeamIds before invoking mdmAbmAPI.editTeams) to validate/throw or transform to the required non-optional SelectedTeamIds before sending the PATCH.frontend/pages/queries/edit/components/SaveAsNewQueryModal/SaveAsNewQueryModal.tsx (1)
161-168:⚠️ Potential issue | 🟠 MajorUse the renamed field in the post-create redirect.
This flow now creates with
fleet_id, but Line 166 still readsnewQuery.team_id. That leaves the success redirect on the legacy field name and can drop the fleet scope after save.Suggested fix
router.push( getPathWithQueryParams(PATHS.REPORT_DETAILS(newQuery.id), { - fleet_id: newQuery.team_id, + fleet_id: newQuery.fleet_id, host_id: hostId, }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/queries/edit/components/SaveAsNewQueryModal/SaveAsNewQueryModal.tsx` around lines 161 - 168, The redirect after creating a query uses the legacy field newQuery.team_id for fleet scope; update the post-create redirect in SaveAsNewQueryModal to use the new fleet field (newQuery.fleet_id) when building params for getPathWithQueryParams/PATHS.REPORT_DETAILS after queryAPI.create returns newQuery so the saved report retains the correct fleet scope.
🧹 Nitpick comments (3)
frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tests.ts (1)
27-39: Please add a missing-name case for the renamed payload.This only covers the happy path. A test where one selected name is absent from
availableTeamswould guard against silently carryingundefinedin one of the new*_fleet_idfields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tests.ts` around lines 27 - 39, Add a test case to getSelectedTeamIds that covers the missing-name scenario: construct selectedTeamNames where one of the entries (e.g., macos_team) is a name that does not exist in availableTeams and assert the returned object maps that platform's *_fleet_id to 0 (or the intended fallback) rather than undefined; update the test file (EditTeamsAbmModal.tests.ts) to include this additional it block referencing getSelectedTeamIds and availableTeams so the function’s behavior for missing team names is validated.frontend/services/entities/targets.ts (1)
48-59: Rename the caller-facingqueryIdinput here too.These methods now send
report_id, but their public input is still namedqueryId. That mismatch makes it easy for callers to pass the saved query id instead of the report id the API now expects.Also applies to: 81-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/services/entities/targets.ts` around lines 48 - 59, The public parameter name queryId in loadAll (and the other targets methods around the second occurrence) is misleading because the payload uses report_id; rename the caller-facing parameter from queryId to reportId in the method signatures and in the ITargetsProps type, and update all internal references so the POST body sends report_id: reportId (not report_id: queryId) to keep the API contract and caller intent consistent (update both the loadAll function and the other method at the later occurrence).frontend/services/entities/software.ts (1)
783-785: Keep this payload strongly typed.Switching
bodytoRecord<string, unknown>removes TypeScript coverage for theteam_id→fleet_idrename on a write path. Updating the dedicated payload type and using it here would let the compiler catch any leftover field-name drift.Suggested cleanup
-interface IAddFleetMaintainedAppPostBody { - team_id: number; +interface IAddFleetMaintainedAppPostBody { + fleet_id: number; fleet_maintained_app_id: number; pre_install_query?: string; install_script?: string; post_install_script?: string; uninstall_script?: string; self_service?: boolean; automatic_install?: boolean; labels_include_any?: string[]; labels_exclude_any?: string[]; categories: string[]; }- const body: Record<string, unknown> = { + const body: IAddFleetMaintainedAppPostBody = { fleet_id: teamId, fleet_maintained_app_id: formData.appId, pre_install_query: encodeScriptBase64(formData.preInstallQuery), install_script: encodeScriptBase64(formData.installScript), post_install_script: encodeScriptBase64(formData.postInstallScript), uninstall_script: encodeScriptBase64(formData.uninstallScript), self_service: formData.selfService, automatic_install: formData.automaticInstall, categories: formData.categories, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/services/entities/software.ts` around lines 783 - 785, The payload is currently typed as Record<string, unknown> which loses compile-time checks; replace the ad-hoc typing for the body object with a dedicated interface/type (e.g., UpdateFleetMaintainedAppPayload) that declares fleet_id: string (or number) and fleet_maintained_app_id: string (or appropriate types), then change the variable declaration of body to that concrete type and use it where the request is sent so the compiler will catch any accidental field-name drift like team_id → fleet_id; locate the code building the body object (variable name body, values teamId and formData.appId) and update callers to expect the new payload type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/LiveQuery/SelectTargets.tsx`:
- Around line 224-232: The count query can fire without a report_id; update the
guard so targetsAPI.count only runs when a report_id exists by adding a truthy
check for queryKey[0].query_id to the enabled condition (in addition to
!!selectedTargets.length) or by early-returning/throwing from the query function
when query_id is falsy; reference the queryKey/query_id destructure and the
targetsAPI.count call so the request never sends with an undefined report_id.
In `@frontend/pages/admin/OrgSettingsPage/cards/Advanced/Advanced.tsx`:
- Around line 172-173: The form hydrator and type need to be updated to match
the new submit keys: change the initialization logic that sets the form state
(where you currently read appConfig.server_settings.live_query_disabled and
query_reports_disabled) to first read
appConfig.server_settings.live_reporting_disabled and
appConfig.server_settings.discard_reports_data and fall back to the old keys
(live_query_disabled and query_reports_disabled) for rollout compatibility; also
update the IConfigServerSettings interface to include live_reporting_disabled?:
boolean and discard_reports_data?: boolean so the fetched config maps to the new
names. Ensure the variables referenced at submit (disableLiveQuery,
disableQueryReports) are still driven by this updated hydration logic so the
round-trip remains consistent.
In `@frontend/pages/admin/OrgSettingsPage/cards/Statistics/Statistics.tsx`:
- Line 44: The code reads appConfig.server_settings.query_reports_disabled but
always submits discard_reports_data, which can erase the setting if the backend
switches field names; update the read to use a fallback like const
preserveDiscard = appConfig.server_settings.discard_reports_data ??
appConfig.server_settings.query_reports_disabled ?? false and submit that value
as discard_reports_data, update the IConfigServerSettings type to include
optional discard_reports_data?: boolean, and make the same fallback change in
Advanced.tsx (where query_reports_disabled is used) so both components preserve
the existing stored-results setting during the field-name transition.
In `@frontend/services/entities/scripts.ts`:
- Around line 262-263: Replace the truthy guard on teamId so teamId === 0 is
preserved: change the conditional that wraps formData.append("fleet_id",
teamId.toString()) to check teamId !== undefined (or typeof teamId !==
'undefined') instead of a truthy check; update the code around the teamId
variable and the call to formData.append("fleet_id", ...) so zero is treated as
a valid value.
In `@frontend/services/entities/team_scheduled_queries.ts`:
- Around line 17-19: The code is coercing optional IDs (report_id and fleet_id)
into Number(...) and defaulting to TEAM_SCHEDULE(0), which turns missing IDs
into NaN or 0; update the create payload logic to only convert and include
report_id and fleet_id when they are actually provided (check for !==
undefined/null/''), e.g. if (report_id != null && report_id !== '') use
Number(report_id) in the payload, otherwise omit the fields (or set them to
undefined), and remove the forced TEAM_SCHEDULE(0) fallback so TEAM_SCHEDULE is
only used when a valid id is present; apply the same change to all occurrences
referenced (the coercions around report_id, fleet_id and the TEAM_SCHEDULE(0)
default).
In `@frontend/utilities/helpers.tsx`:
- Around line 244-246: The payload still carries the legacy query_id alongside
the new report_id; in the helpers where you set (result as any).report_id =
Number(queryID) (and the analogous block at the other occurrence around lines
306-308), remove the old property from the payload before returning/sending it
by deleting (or setting to undefined) the result.query_id so only report_id is
sent; ensure you do this in both helper locations and keep logging_type omission
logic unchanged.
- Around line 369-379: The code currently assigns report_id from queryID then
incorrectly overwrites it with teamID and leaves legacy query_id/team_id on
result; fix by keeping report_id = Number(queryID) only when queryID exists,
assign the fleet/team identifier to a separate property (e.g., (result as
any).team_id = Number(teamID)) instead of overwriting report_id, ensure shard is
still set via result.shard = Number(shard), and remove any legacy keys by
deleting (result as any).query_id and (result as any).team_id if they clash with
the new properties so the request targets the correct report and does not retain
legacy fields.
---
Outside diff comments:
In
`@frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tsx`:
- Around line 58-69: getSelectedTeamIds may return undefined for byName[...] but
currently returns a strict SelectedTeamIds expected by mdmAbmAPI.editTeams;
update the helper to return optional ids instead so TypeScript surfaces missing
matches. Change getSelectedTeamIds to return a type where ios_fleet_id,
ipados_fleet_id, macos_fleet_id are number | undefined (e.g.,
SelectedTeamIdsOptional or Partial<SelectedTeamIds>), keep the same mapping
logic using availableTeams and byName, and update callers (places that call
getSelectedTeamIds before invoking mdmAbmAPI.editTeams) to validate/throw or
transform to the required non-optional SelectedTeamIds before sending the PATCH.
In
`@frontend/pages/queries/edit/components/SaveAsNewQueryModal/SaveAsNewQueryModal.tsx`:
- Around line 161-168: The redirect after creating a query uses the legacy field
newQuery.team_id for fleet scope; update the post-create redirect in
SaveAsNewQueryModal to use the new fleet field (newQuery.fleet_id) when building
params for getPathWithQueryParams/PATHS.REPORT_DETAILS after queryAPI.create
returns newQuery so the saved report retains the correct fleet scope.
In `@frontend/services/entities/operating_systems.ts`:
- Around line 76-86: The frontend is sending fleet_id but the backend expects
team_id; update the params object in operating_systems.ts so the query uses
team_id instead of fleet_id (i.e., replace the fleet_id key with team_id:
teamId) and ensure the IGetOSVersionsRequestQueryParams type aligns with the
team_id property so the outgoing query matches the backend handler.
---
Nitpick comments:
In
`@frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tests.ts`:
- Around line 27-39: Add a test case to getSelectedTeamIds that covers the
missing-name scenario: construct selectedTeamNames where one of the entries
(e.g., macos_team) is a name that does not exist in availableTeams and assert
the returned object maps that platform's *_fleet_id to 0 (or the intended
fallback) rather than undefined; update the test file
(EditTeamsAbmModal.tests.ts) to include this additional it block referencing
getSelectedTeamIds and availableTeams so the function’s behavior for missing
team names is validated.
In `@frontend/services/entities/software.ts`:
- Around line 783-785: The payload is currently typed as Record<string, unknown>
which loses compile-time checks; replace the ad-hoc typing for the body object
with a dedicated interface/type (e.g., UpdateFleetMaintainedAppPayload) that
declares fleet_id: string (or number) and fleet_maintained_app_id: string (or
appropriate types), then change the variable declaration of body to that
concrete type and use it where the request is sent so the compiler will catch
any accidental field-name drift like team_id → fleet_id; locate the code
building the body object (variable name body, values teamId and formData.appId)
and update callers to expect the new payload type.
In `@frontend/services/entities/targets.ts`:
- Around line 48-59: The public parameter name queryId in loadAll (and the other
targets methods around the second occurrence) is misleading because the payload
uses report_id; rename the caller-facing parameter from queryId to reportId in
the method signatures and in the ITargetsProps type, and update all internal
references so the POST body sends report_id: reportId (not report_id: queryId)
to keep the API contract and caller intent consistent (update both the loadAll
function and the other method at the later occurrence).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e4ba83f-4594-4589-a615-3ba8dae1d22f
📒 Files selected for processing (39)
frontend/components/LiveQuery/SelectTargets.tests.tsxfrontend/components/LiveQuery/SelectTargets.tsxfrontend/interfaces/schedulable_query.tsfrontend/pages/ManageControlsPage/OSUpdates/components/AppleOSTargetForm/AppleOSTargetForm.tests.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tests.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tests.tsfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tsxfrontend/pages/admin/OrgSettingsPage/cards/Advanced/Advanced.tsxfrontend/pages/admin/OrgSettingsPage/cards/Statistics/Statistics.tsxfrontend/pages/policies/ManagePoliciesPage/components/CalendarEventsModal/CalendarEventsModal.tests.tsxfrontend/pages/policies/ManagePoliciesPage/components/PoliciesPaginatedList/PoliciesPaginatedList.tests.tsxfrontend/pages/queries/edit/components/EditQueryForm/EditQueryForm.tsxfrontend/pages/queries/edit/components/SaveAsNewQueryModal/SaveAsNewQueryModal.tsxfrontend/pages/queries/edit/components/SaveNewQueryModal/SaveNewQueryModal.tsxfrontend/services/entities/certificates.tsfrontend/services/entities/disk_encryption.tsfrontend/services/entities/global_scheduled_queries.tsfrontend/services/entities/host_summary.tsfrontend/services/entities/hosts.tsfrontend/services/entities/labels.tsfrontend/services/entities/macadmins.tsfrontend/services/entities/mdm.tsfrontend/services/entities/mdm_apple.tsfrontend/services/entities/mdm_apple_bm.tsfrontend/services/entities/operating_systems.tsfrontend/services/entities/queries.tsfrontend/services/entities/query_report.tsfrontend/services/entities/scripts.tsfrontend/services/entities/software.tsfrontend/services/entities/targets.tsfrontend/services/entities/team_scheduled_queries.tsfrontend/services/entities/users.tsfrontend/services/entities/vulnerabilities.tsfrontend/test/handlers/script-handlers.tsfrontend/test/handlers/team-handlers.tsfrontend/utilities/endpoints.tsfrontend/utilities/helpers.tsxfrontend/utilities/url/index.tsfrontend/utilities/url/url.tests.ts
| } | ||
|
|
||
| export default { | ||
| create: (formData: ICreateTeamScheduledQueryFormData) => { |
There was a problem hiding this comment.
i don't think this method is used anywhere
There was a problem hiding this comment.
Doesn't look to be. If you want to nuke this in a subsequent PR go for it.
|
|
||
| if (queryID) { | ||
| result.query_id = Number(queryID); | ||
| (result as any).report_id = Number(queryID); |
There was a problem hiding this comment.
the as any here and below is required because we're starting from the scheduleQuery type which we're not updating response types in this PR, so report_id is an unknown field. The blast radius of updating scheduleQuery is fairly large (17 files), so this slightly ugly fix is better IMO.
iansltx
left a comment
There was a problem hiding this comment.
Not much feedback on this one. Scope limitations make sense; I'll review the cleanup when it comes across.
Related issue: Resolves #41391
Details
This PR updates front-end API calls to use new URLs and API params, so that the front end doesn't cause deprecation warnings to appear on the server.
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
n/a, should not be user-visible
Testing
The biggest risk here is not that we missed a spot that still causes a deprecation warning, but that we might inadvertently make a change that breaks the front end, for instance by sending
fleet_idto a function that drops it silently and thus sends no ID to the server. Fortunately we use TypeScript in virtually every place affected by these changes, so the code would not compile if there were mismatches between the API expectation and what we're sending. Still, spot checking as many places as possible both for deprecation-warning leaks and loss of functionality is important.Summary by CodeRabbit