From a4228cc45c312fe604b7e88ca13e4afe8973fdf6 Mon Sep 17 00:00:00 2001 From: Sarah Gillespie <73313222+gillespi314@users.noreply.github.com> Date: Tue, 6 Feb 2024 10:04:07 -0600 Subject: [PATCH] Fix UI bug in host query params reconciliation (#16432) --- .../hosts/ManageHostsPage/HostsPageConfig.tsx | 16 ++++++++++++++ .../hosts/ManageHostsPage/ManageHostsPage.tsx | 20 +++++------------ frontend/services/entities/hosts.ts | 2 ++ frontend/utilities/url/index.ts | 14 ++++++++++-- frontend/utilities/url/url.tests.ts | 22 +++++++++++++++++++ 5 files changed, 58 insertions(+), 16 deletions(-) diff --git a/frontend/pages/hosts/ManageHostsPage/HostsPageConfig.tsx b/frontend/pages/hosts/ManageHostsPage/HostsPageConfig.tsx index be51c446ce..8a7fb4c61b 100644 --- a/frontend/pages/hosts/ManageHostsPage/HostsPageConfig.tsx +++ b/frontend/pages/hosts/ManageHostsPage/HostsPageConfig.tsx @@ -23,6 +23,22 @@ export const MANAGE_HOSTS_PAGE_FILTER_KEYS = [ "bootstrap_package", ] as const; +/* + * These are the URL query params that are incompatible with non-status labels on the manage hosts page. + * They should be stripped from the URL when a non-status label is selected. + */ +export const MANAGE_HOSTS_PAGE_LABEL_INCOMPATIBLE_QUERY_PARAMS = [ + "policy_id", + "policy_response", + "software_id", + "software_version_id", + "software_title_id", + "bootstrap_package", + "macos_settings", + HOSTS_QUERY_PARAMS.OS_SETTINGS, + HOSTS_QUERY_PARAMS.DISK_ENCRYPTION, +] as const; + // TODO: refactor to use this type as the location.query prop of the page export type ManageHostsPageQueryParams = Record< | "page" diff --git a/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx b/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx index b6012cb0de..5789c557a9 100644 --- a/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx +++ b/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx @@ -23,7 +23,6 @@ import hostsAPI, { ILoadHostsResponse, ISortOption, MacSettingsStatusQueryParam, - HOSTS_QUERY_PARAMS, } from "services/entities/hosts"; import hostCountAPI, { IHostsCountQueryKey, @@ -91,6 +90,7 @@ import { DEFAULT_PAGE_INDEX, getHostSelectStatuses, MANAGE_HOSTS_PAGE_FILTER_KEYS, + MANAGE_HOSTS_PAGE_LABEL_INCOMPATIBLE_QUERY_PARAMS, } from "./HostsPageConfig"; import { isAcceptableStatus } from "./helpers"; @@ -535,21 +535,13 @@ const ManageHostsPage = ({ const isDeselectingLabel = newLabelId && newLabelId === selectedLabel?.id; - // Non-status labels are not compatible with policies or software filters - // so omit policies and software params from next location let newQueryParams = queryParams; if (slug) { - newQueryParams = omit(newQueryParams, [ - "policy_id", - "policy_response", - "software_id", - "software_version_id", - "software_title_id", - "bootstrap_package", - "macos_settings", - HOSTS_QUERY_PARAMS.OS_SETTINGS, - HOSTS_QUERY_PARAMS.DISK_ENCRYPTION, - ]); + // some filters are incompatible with non-status labels so omit those params from next location + newQueryParams = omit( + newQueryParams, + MANAGE_HOSTS_PAGE_LABEL_INCOMPATIBLE_QUERY_PARAMS + ); } router.replace( diff --git a/frontend/services/entities/hosts.ts b/frontend/services/entities/hosts.ts index ee5379e507..54a6fc4088 100644 --- a/frontend/services/entities/hosts.ts +++ b/frontend/services/entities/hosts.ts @@ -204,6 +204,7 @@ export default { order_direction: sortBy[0].direction, query: globalFilter, ...reconcileMutuallyInclusiveHostParams({ + label, teamId, macSettingsStatus, osSettings, @@ -272,6 +273,7 @@ export default { order_direction: sortParams.order_direction, status, ...reconcileMutuallyInclusiveHostParams({ + label, teamId, macSettingsStatus, osSettings, diff --git a/frontend/utilities/url/index.ts b/frontend/utilities/url/index.ts index 341900101f..953abd2981 100644 --- a/frontend/utilities/url/index.ts +++ b/frontend/utilities/url/index.ts @@ -16,6 +16,7 @@ type FilteredQueryValues = string | number | boolean; type FilteredQueryParams = Record; interface IMutuallyInclusiveHostParams { + label?: string; teamId?: number; macSettingsStatus?: MacSettingsStatusQueryParam; osSettings?: MdmProfileStatus; @@ -76,18 +77,27 @@ export const buildQueryStringFromParams = (queryParams: QueryParams) => { }; export const reconcileMutuallyInclusiveHostParams = ({ + label, teamId, macSettingsStatus, osSettings, }: IMutuallyInclusiveHostParams) => { - // ensure macos_settings filter is always applied in - // conjuction with a team_id, 0 (no teams) by default const reconciled: Record = { team_id: teamId }; + + if (label) { + // if label is present, include team_id in the query but exclude others + return reconciled; + } + if (macSettingsStatus) { + // ensure macos_settings filter is always applied in + // conjuction with a team_id, 0 (no teams) by default reconciled.macos_settings = macSettingsStatus; reconciled.team_id = teamId ?? 0; } if (osSettings) { + // ensure os_settings filter is always applied in + // conjuction with a team_id, 0 (no teams) by default reconciled[HOSTS_QUERY_PARAMS.OS_SETTINGS] = osSettings; reconciled.team_id = teamId ?? 0; } diff --git a/frontend/utilities/url/url.tests.ts b/frontend/utilities/url/url.tests.ts index f6b88c9347..cdf84fa5f8 100644 --- a/frontend/utilities/url/url.tests.ts +++ b/frontend/utilities/url/url.tests.ts @@ -52,6 +52,28 @@ describe("url utilities > reconcileMutuallyInclusiveHostParams", () => { reconcileMutuallyInclusiveHostParams({ macSettingsStatus, teamId }) ).toEqual({}); }); + + it("leaves teamId unchanged and excludes others if label is present", () => { + expect( + reconcileMutuallyInclusiveHostParams({ + teamId: 1, + label: "labels/7", + macSettingsStatus: "pending", + osSettings: "pending", + }) + ).toEqual({ + team_id: 1, + }); + expect( + reconcileMutuallyInclusiveHostParams({ + label: "labels/7", + macSettingsStatus: "pending", + osSettings: "pending", + }) + ).toEqual({ + team_id: undefined, + }); + }); }); describe("url utilites > buildQueryStringFromParams", () => {