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",
},
];