From 452b385c12bdfb36caf5751dcaf0989e5caefe63 Mon Sep 17 00:00:00 2001 From: RachelElysia <71795832+RachelElysia@users.noreply.github.com> Date: Wed, 17 May 2023 17:25:57 -0400 Subject: [PATCH] Fleet UI: Sort (and bookmark) Manage Policies tables by failing host count (#11735) --- ...issue-7166-order-policies-by-failing-count | 1 + .../TableContainer/TableContainer.tsx | 5 +- frontend/hooks/useTeamIdParam.ts | 18 +++ .../ManagePoliciesPage/ManagePoliciesPage.tsx | 132 ++++++++++++++---- .../PoliciesTable/PoliciesTable.tests.tsx | 3 + .../PoliciesTable/PoliciesTable.tsx | 28 +++- .../PoliciesTable/PoliciesTableConfig.tsx | 19 ++- 7 files changed, 164 insertions(+), 42 deletions(-) create mode 100644 changes/issue-7166-order-policies-by-failing-count diff --git a/changes/issue-7166-order-policies-by-failing-count b/changes/issue-7166-order-policies-by-failing-count new file mode 100644 index 0000000000..4204366f4a --- /dev/null +++ b/changes/issue-7166-order-policies-by-failing-count @@ -0,0 +1 @@ +- Can reorder (and bookmark) policy tables by failing count diff --git a/frontend/components/TableContainer/TableContainer.tsx b/frontend/components/TableContainer/TableContainer.tsx index 9164342d9c..c0fab7d5c4 100644 --- a/frontend/components/TableContainer/TableContainer.tsx +++ b/frontend/components/TableContainer/TableContainer.tsx @@ -20,7 +20,10 @@ export interface ITableQueryData { searchQuery: string; sortHeader: string; sortDirection: string; - showInheritedTable?: boolean; // Only used for policies tables + /** Only used for showing inherited policies table */ + showInheritedTable?: boolean; + /** Only used for sort/query changes to inherited policies table */ + editingInheritedTable?: boolean; } interface IRowProps extends Row { original: { diff --git a/frontend/hooks/useTeamIdParam.ts b/frontend/hooks/useTeamIdParam.ts index 1aa3fca42a..fdbb55751c 100644 --- a/frontend/hooks/useTeamIdParam.ts +++ b/frontend/hooks/useTeamIdParam.ts @@ -30,6 +30,23 @@ const rebuildQueryStringWithTeamId = ( newTeamId: number ) => { const parts = splitQueryStringParts(queryString); + + // Reset page to 0 + const pageIndex = parts.findIndex((p) => p.startsWith("page=")); + const inheritedPageIndex = parts.findIndex((p) => + p.startsWith("inherited_page=") + ); + + const newPagePart = "page=0"; + const newInheritedPagePart = "inherited_page=0"; + + if (pageIndex) { + parts.splice(pageIndex, 1, newPagePart); + } + if (inheritedPageIndex) { + parts.splice(inheritedPageIndex, 1, newInheritedPagePart); + } + const teamIndex = parts.findIndex((p) => p.startsWith("team_id=")); // URLs for the app represent "All teams" by the absence of the team id param @@ -54,6 +71,7 @@ const rebuildQueryStringWithTeamId = ( } else { parts.splice(teamIndex, 1); // just remove the old team part } + return joinQueryStringParts(parts); }; diff --git a/frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx b/frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx index c5d3cbef06..a47110318d 100644 --- a/frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx +++ b/frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx @@ -47,10 +47,14 @@ interface IManagePoliciesPageProps { pathname: string; query: { team_id?: string; - page?: string; query?: string; - inherited_page?: string; + order_key?: string; + order_direction?: "asc" | "desc"; + page?: string; inherited_table?: "true"; + inherited_order_key?: string; + inherited_order_direction?: "asc" | "desc"; + inherited_page?: string; }; search: string; }; @@ -124,28 +128,50 @@ const ManagePolicyPage = ({ // Functions to avoid race conditions const initialSearchQuery = (() => queryParams.query ?? "")(); + const initialSortHeader = (() => + (queryParams?.order_key as "name" | "failing_host_count") ?? "name")(); + const initialSortDirection = (() => + (queryParams?.order_direction as "asc" | "desc") ?? "asc")(); const initialPage = (() => queryParams && queryParams.page ? parseInt(queryParams?.page, 10) : 0)(); const initialShowInheritedTable = (() => queryParams && queryParams.inherited_table === "true")(); + const initialInheritedSortHeader = (() => + (queryParams?.inherited_order_key as "name" | "failing_host_count") ?? + "name")(); + const initialInheritedSortDirection = (() => + (queryParams?.inherited_order_direction as "asc" | "desc") ?? "asc")(); const initialInheritedPage = (() => queryParams && queryParams.inherited_page ? parseInt(queryParams?.inherited_page, 10) : 0)(); - // Never set as state as URL is source of truth - // State only used for handling team change - const [searchQuery, setSearchQuery] = useState(initialSearchQuery); - const [page, setPage] = useState(initialPage); - const [showInheritedTable, setShowInheritedTable] = useState( - initialShowInheritedTable + const page = initialPage; + const showInheritedTable = initialShowInheritedTable; + const inheritedPage = initialInheritedPage; + const searchQuery = initialSearchQuery; + + // Needs update on location change or table state might not match URL + const [sortHeader, setSortHeader] = useState(initialSortHeader); + const [sortDirection, setSortDirection] = useState(initialSortDirection); + const [inheritedSortDirection, setInheritedSortDirection] = useState( + initialInheritedSortDirection + ); + const [inheritedSortHeader, setInheritedSortHeader] = useState( + initialInheritedSortHeader ); - const [inheritedPage, setInheritedPage] = useState(initialInheritedPage); useEffect(() => { setLastEditedQueryPlatform(null); }, []); + useEffect(() => { + setSortHeader(initialSortHeader); + setSortDirection(initialSortDirection); + setInheritedSortHeader(initialInheritedSortHeader); + setInheritedSortDirection(initialInheritedSortDirection); + }, [location]); + useEffect(() => { const path = location.pathname + location.search; if (location.search && filteredPoliciesPath !== path) { @@ -153,14 +179,6 @@ const ManagePolicyPage = ({ } }, [location, filteredPoliciesPath, setFilteredPoliciesPath]); - useEffect(() => { - setShowInheritedTable(initialShowInheritedTable); - setInheritedPage(initialInheritedPage); - setPage(initialPage); - setSearchQuery(initialSearchQuery); - // TODO: handle invalid values for params - }, [location]); - const { data: globalPolicies, error: globalPoliciesError, @@ -244,20 +262,21 @@ const ManagePolicyPage = ({ (teamId: number) => { setSelectedPolicyIds([]); handleTeamChange(teamId); - setPage(0); - setSearchQuery(""); }, [handleTeamChange] ); // TODO: Look into useDebounceCallback with dependencies + // Inherited table uses the same onQueryChange function but routes to different URL params const onQueryChange = useCallback( async (newTableQuery: ITableQueryData) => { const { pageIndex: newPageIndex, searchQuery: newSearchQuery, + sortDirection: newSortDirection, + sortHeader: newSortHeader, + inheritedTable, } = newTableQuery; - // Rebuild queryParams to dispatch new browser location to react-router const newQueryParams: { [key: string]: string | number | undefined } = {}; @@ -265,10 +284,25 @@ const ManagePolicyPage = ({ newQueryParams.query = newSearchQuery; } - newQueryParams.page = newPageIndex; - // Reset page number to 0 for new filters - if (newSearchQuery !== searchQuery) { - newQueryParams.page = 0; + // Updates main policy table URL params + // No change to inherited policy table URL params + if (!inheritedTable) { + newQueryParams.order_key = newSortHeader; + newQueryParams.order_direction = newSortDirection; + newQueryParams.page = newPageIndex.toString(); + if (showInheritedTable) { + newQueryParams.inherited_order_key = inheritedSortHeader; + newQueryParams.inherited_order_direction = inheritedSortDirection; + newQueryParams.inherited_page = inheritedPage.toString(); + } + // Reset page number to 0 for new filters + if ( + newSortDirection !== sortDirection || + newSortHeader !== sortHeader || + newSearchQuery !== searchQuery + ) { + newQueryParams.page = "0"; + } } if (showInheritedTable) { @@ -276,8 +310,23 @@ const ManagePolicyPage = ({ showInheritedTable && showInheritedTable.toString(); } - if (showInheritedTable && inheritedPage !== 0) { - newQueryParams.inherited_page = inheritedPage; + // Updates inherited policy table URL params + // No change to main policy table URL params + if (showInheritedTable && inheritedTable) { + newQueryParams.inherited_order_key = newSortHeader; + newQueryParams.inherited_order_direction = newSortDirection; + newQueryParams.inherited_page = newPageIndex.toString(); + newQueryParams.order_key = sortHeader; + newQueryParams.order_direction = sortDirection; + newQueryParams.page = page.toString(); + newQueryParams.query = searchQuery; + // Reset page number to 0 for new filters + if ( + newSortDirection !== inheritedSortDirection || + newSortHeader !== inheritedSortHeader + ) { + newQueryParams.inherited_page = "0"; + } } if (teamIdForApi !== undefined) { @@ -286,12 +335,18 @@ const ManagePolicyPage = ({ const locationPath = getNextLocationPath({ pathPrefix: PATHS.MANAGE_POLICIES, - queryParams: newQueryParams, + queryParams: { ...queryParams, ...newQueryParams }, }); router?.replace(locationPath); }, - [teamIdForApi, searchQuery, showInheritedTable, inheritedPage, router, page] + [ + teamIdForApi, + searchQuery, + showInheritedTable, + inheritedSortDirection, + sortDirection, + ] // Other dependencies can cause infinite re-renders as URL is source of truth ); const onClientSidePaginationChange = useCallback( @@ -302,12 +357,17 @@ const ManagePolicyPage = ({ ...queryParams, page: pageIndex, query: searchQuery, + order_direction: sortDirection, + order_key: sortHeader, + inherited_order_direction: inheritedSortDirection, + inherited_order_key: inheritedSortHeader, + inherited_page: inheritedPage, }, }); router?.replace(locationPath); }, - [searchQuery] // Dependencies required for correct variable state + [searchQuery, queryParams, sortHeader, sortDirection] // Dependencies required for correct variable state ); const onClientSideInheritedPaginationChange = useCallback( @@ -319,11 +379,16 @@ const ManagePolicyPage = ({ inherited_table: "true", inherited_page: pageIndex, query: searchQuery, + page, + order_direction: sortDirection, + order_key: sortHeader, + inherited_order_direction: inheritedSortDirection, + inherited_order_key: inheritedSortHeader, }, }); router?.replace(locationPath); }, - [] // Dependencies required for correct variable state + [queryParams, inheritedSortHeader, inheritedSortDirection] // Dependencies required for correct variable state ); const toggleManageAutomationsModal = () => @@ -556,6 +621,8 @@ const ManagePolicyPage = ({ isPremiumTier={isPremiumTier} isSandboxMode={isSandboxMode} searchQuery={searchQuery} + sortHeader={sortHeader} + sortDirection={sortDirection} page={page} onQueryChange={onQueryChange} /> @@ -578,6 +645,8 @@ const ManagePolicyPage = ({ isSandboxMode={isSandboxMode} onClientSidePaginationChange={onClientSidePaginationChange} searchQuery={searchQuery} + sortHeader={sortHeader} + sortDirection={sortDirection} page={page} onQueryChange={onQueryChange} /> @@ -620,7 +689,10 @@ const ManagePolicyPage = ({ onClientSidePaginationChange={ onClientSideInheritedPaginationChange } + sortHeader={inheritedSortHeader} + sortDirection={inheritedSortDirection} page={inheritedPage} + onQueryChange={onQueryChange} /> ))} diff --git a/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTable.tests.tsx b/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTable.tests.tsx index b489376990..6619d69a2c 100644 --- a/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTable.tests.tsx +++ b/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTable.tests.tsx @@ -1,5 +1,6 @@ import React from "react"; import { render, screen } from "@testing-library/react"; +import { noop } from "lodash"; import createMockPolicy from "__mocks__/policyMock"; import PoliciesTable from "./PoliciesTable"; @@ -19,6 +20,7 @@ describe("Policies table", () => { isSandboxMode searchQuery="" page={0} + onQueryChange={noop} /> ); @@ -44,6 +46,7 @@ describe("Policies table", () => { isSandboxMode={false} searchQuery="" page={0} + onQueryChange={noop} /> ); diff --git a/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTable.tsx b/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTable.tsx index 130703e4f8..d06b231e1f 100644 --- a/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTable.tsx +++ b/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTable.tsx @@ -21,20 +21,25 @@ const TAGGED_TEMPLATES = { }, }; +const DEFAULT_SORT_DIRECTION = "asc"; +const DEFAULT_SORT_HEADER = "updated_at"; + interface IPoliciesTableProps { policiesList: IPolicyStats[]; isLoading: boolean; onAddPolicyClick?: () => void; onDeletePolicyClick: (selectedTableIds: number[]) => void; canAddOrDeletePolicy?: boolean; - tableType?: string; + tableType?: "inheritedPolicies"; currentTeam: ITeamSummary | undefined; currentAutomatedPolicies?: number[]; isPremiumTier?: boolean; isSandboxMode?: boolean; onClientSidePaginationChange?: (pageIndex: number) => void; - onQueryChange?: (newTableQuery: ITableQueryData) => void; + onQueryChange: (newTableQuery: ITableQueryData) => void; searchQuery: string; + sortHeader?: "name" | "failing_host_count"; + sortDirection?: "asc" | "desc"; page: number; } @@ -52,10 +57,20 @@ const PoliciesTable = ({ onQueryChange, onClientSidePaginationChange, searchQuery, + sortHeader, + sortDirection, page, }: IPoliciesTableProps): JSX.Element => { const { config } = useContext(AppContext); + // Inherited table uses the same onQueryChange but require different URL params + const onTableQueryChange = (newTableQuery: ITableQueryData) => { + onQueryChange({ + ...newTableQuery, + editingInheritedTable: tableType === "inheritedPolicies", + }); + }; + const emptyState = () => { const emptyPolicies: IEmptyTableProps = { iconName: "empty-policies", @@ -124,7 +139,7 @@ const PoliciesTable = ({ ) : ( diff --git a/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTableConfig.tsx b/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTableConfig.tsx index f86e75afc4..30b08fb234 100644 --- a/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTableConfig.tsx +++ b/frontend/pages/policies/ManagePoliciesPage/components/PoliciesTable/PoliciesTableConfig.tsx @@ -6,6 +6,7 @@ import { millisecondsToHours, millisecondsToMinutes, isAfter } from "date-fns"; import ReactTooltip from "react-tooltip"; // @ts-ignore import Checkbox from "components/forms/fields/Checkbox"; +import HeaderCell from "components/TableContainer/DataTable/HeaderCell"; import LinkCell from "components/TableContainer/DataTable/LinkCell/LinkCell"; import StatusIndicator from "components/StatusIndicator"; import Icon from "components/Icon"; @@ -106,8 +107,12 @@ const generateTableHeaders = ( const tableHeaders: IDataColumn[] = [ { title: "Name", - Header: "Name", - disableSortBy: true, + Header: (cellProps) => ( + + ), accessor: "name", Cell: (cellProps: ICellProps): JSX.Element => ( ), + sortType: "caseInsensitive", }, { title: "Yes", @@ -195,8 +201,12 @@ const generateTableHeaders = ( }, { title: "No", - Header: () => , - disableSortBy: true, + Header: (cellProps) => ( + } + isSortedDesc={cellProps.column.isSortedDesc} + /> + ), accessor: "failing_host_count", Cell: (cellProps: ICellProps): JSX.Element => { if (cellProps.row.original.has_run) { @@ -234,6 +244,7 @@ const generateTableHeaders = ( ); }, + sortType: "caseInsensitive", }, ];