Fleet UI: Sort (and bookmark) Manage Policies tables by failing host count (#11735)

changes/issue-7166-order-policies-by-failing-count

@@ -0,0 +1 @@
+- Can reorder (and bookmark) policy tables by failing count

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: {

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);
 };
 

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}
                 />
               ))}
           </div>

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}
       />
     );
 

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 = ({
         <Spinner />
       ) : (
         <TableContainer
-          resultsTitle={"policies"}
+          resultsTitle="policies"
           columns={generateTableHeaders(
             {
               selectedTeamId: currentTeam?.id,
@@ -141,11 +156,10 @@ const PoliciesTable = ({
           )}
           filters={{ global: searchQuery }}
           isLoading={isLoading}
-          defaultSortHeader={"name"}
-          defaultSortDirection={"asc"}
+          defaultSortHeader={sortHeader || DEFAULT_SORT_HEADER}
+          defaultSortDirection={sortDirection || DEFAULT_SORT_DIRECTION}
           defaultSearchQuery={searchQuery}
           defaultPageIndex={page}
-          manualSortBy
           showMarkAllPages={false}
           isAllPagesSelected={false}
           primarySelectAction={{
@@ -169,7 +183,7 @@ const PoliciesTable = ({
           onClientSidePaginationChange={onClientSidePaginationChange}
           isClientSideFilter
           searchQueryColumn="name"
-          onQueryChange={onQueryChange}
+          onQueryChange={onTableQueryChange}
           inputPlaceHolder="Search by name"
           searchable={searchable}
         />

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) => (
+        <HeaderCell
+          value={cellProps.column.title}
+          isSortedDesc={cellProps.column.isSortedDesc}
+        />
+      ),
       accessor: "name",
       Cell: (cellProps: ICellProps): JSX.Element => (
         <LinkCell
@@ -150,6 +155,7 @@ const generateTableHeaders = (
           path={PATHS.EDIT_POLICY(cellProps.row.original)}
         />
       ),
+      sortType: "caseInsensitive",
     },
     {
       title: "Yes",
@@ -195,8 +201,12 @@ const generateTableHeaders = (
     },
     {
       title: "No",
-      Header: () => <PassingColumnHeader isPassing={false} />,
-      disableSortBy: true,
+      Header: (cellProps) => (
+        <HeaderCell
+          value={<PassingColumnHeader isPassing={false} />}
+          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",
     },
   ];