fix(machines): repeated machine.count API calls (#4497)

src/app/base/components/node/MachinesHeader/MachinesHeader.test.tsx

@@ -77,7 +77,7 @@ describe("MachinesHeader", () => {
           initialEntries={[{ pathname: "/machines", key: "testKey" }]}
         >
           <CompatRouter>
-            <MachinesHeader />
+            <MachinesHeader machineCount={2} />
           </CompatRouter>
         </MemoryRouter>
       </Provider>

src/app/base/components/node/MachinesHeader/MachinesHeader.tsx

@@ -8,20 +8,21 @@ import { matchPath, Link } from "react-router-dom-v5-compat";
 import type { SectionHeaderProps } from "app/base/components/SectionHeader";
 import SectionHeader from "app/base/components/SectionHeader";
 import urls from "app/base/urls";
-import { useFetchMachineCount } from "app/store/machine/utils/hooks";
 import { actions as resourcePoolActions } from "app/store/resourcepool";
 import resourcePoolSelectors from "app/store/resourcepool/selectors";
 import { actions as tagActions } from "app/store/tag";
 import tagSelectors from "app/store/tag/selectors";
 
-type Props = SectionHeaderProps;
+type Props = SectionHeaderProps & { machineCount: number };
 
-export const MachinesHeader = (props: Props): JSX.Element => {
+export const MachinesHeader = ({
+  machineCount,
+  ...props
+}: Props): JSX.Element => {
   const dispatch = useDispatch();
   const location = useLocation();
   const poolCount = useSelector(resourcePoolSelectors.count);
   const tagCount = useSelector(tagSelectors.count);
-  const { machineCount } = useFetchMachineCount();
 
   useEffect(() => {
     dispatch(resourcePoolActions.fetch());

src/app/machines/views/MachineList/MachineListHeader/MachineListHeader.test.tsx

@@ -65,7 +65,11 @@ describe("MachineListHeader", () => {
   });
 
   it("displays a loader if machines have not loaded", () => {
-    state.machine.counts["mocked-nanoid-3"] = machineStateCountFactory({
+    state.machine.selectedMachines = {
+      groups: ["admin"],
+      grouping: FetchGroupKey.Owner,
+    };
+    state.machine.counts["mocked-nanoid-2"] = machineStateCountFactory({
       loading: true,
     });
     const store = mockStore(state);
@@ -89,7 +93,7 @@ describe("MachineListHeader", () => {
   });
 
   it("displays a machine count if machines have loaded", () => {
-    state.machine.counts["mocked-nanoid-2"] = machineStateCountFactory({
+    state.machine.counts["mocked-nanoid-1"] = machineStateCountFactory({
       count: 2,
       loaded: true,
     });
@@ -121,10 +125,6 @@ describe("MachineListHeader", () => {
       grouping: FetchGroupKey.Owner,
     };
     state.machine.counts["mocked-nanoid-2"] = machineStateCountFactory({
-      count: 10,
-      loaded: true,
-    });
-    state.machine.counts["mocked-nanoid-3"] = machineStateCountFactory({
       loading: true,
     });
     renderWithBrowserRouter(
@@ -188,10 +188,6 @@ describe("MachineListHeader", () => {
       grouping: FetchGroupKey.Owner,
     };
     state.machine.counts["mocked-nanoid-2"] = machineStateCountFactory({
-      count: 10,
-      loaded: true,
-    });
-    state.machine.counts["mocked-nanoid-3"] = machineStateCountFactory({
       count: 2,
       loaded: true,
     });
@@ -214,10 +210,6 @@ describe("MachineListHeader", () => {
       grouping: FetchGroupKey.Owner,
     };
     state.machine.counts["mocked-nanoid-2"] = machineStateCountFactory({
-      count: 10,
-      loaded: true,
-    });
-    state.machine.counts["mocked-nanoid-3"] = machineStateCountFactory({
       count: 2,
       loaded: true,
     });
@@ -235,11 +227,11 @@ describe("MachineListHeader", () => {
 
   it("displays a message when all machines have been selected", () => {
     state.machine.selectedMachines = { filter: {} };
-    state.machine.counts["mocked-nanoid-2"] = machineStateCountFactory({
+    state.machine.counts["mocked-nanoid-1"] = machineStateCountFactory({
       count: 10,
       loaded: true,
     });
-    state.machine.counts["mocked-nanoid-3"] = machineStateCountFactory({
+    state.machine.counts["mocked-nanoid-2"] = machineStateCountFactory({
       count: 10,
       loaded: true,
     });

src/app/machines/views/MachineList/MachineListHeader/MachineListHeader.tsx

@@ -49,15 +49,17 @@ export const MachineListHeader = ({
     "machineViewTagsSeen",
     false
   );
-  // Get the count of all machines that match the current filters.
-  const { machineCount } = useFetchMachineCount(
-    FilterMachineItems.parseFetchFilters(searchFilter)
-  );
-  const { selectedCount, selectedCountLoading } = useMachineSelectedCount(
-    FilterMachineItems.parseFetchFilters(searchFilter)
-  );
+  const filter = FilterMachineItems.parseFetchFilters(searchFilter);
+  // Get the count of all machines
+  const { machineCount: allMachineCount } = useFetchMachineCount();
+  // Get the count of all machines that match the current filter
+  const { machineCount: availableMachineCount } = useFetchMachineCount(filter, {
+    isEnabled: FilterMachineItems.isNonEmptyFilter(searchFilter),
+  });
+  // Get the count of selected machines that match the current filter
+  const { selectedCount, selectedCountLoading } =
+    useMachineSelectedCount(filter);
   const previousSelectedCount = usePrevious(selectedCount);
-
   const selectedMachines = useSelector(machineSelectors.selectedMachines);
 
   useEffect(() => {
@@ -133,9 +135,10 @@ export const MachineListHeader = ({
           />
         )
       }
+      machineCount={allMachineCount}
       subtitle={
         <ModelListSubtitle
-          available={machineCount}
+          available={availableMachineCount || allMachineCount}
           modelName="machine"
           selected={selectedCount}
         />

src/app/pools/views/Pools.tsx

@@ -9,10 +9,12 @@ import urls from "app/base/urls";
 import NotFound from "app/base/views/NotFound";
 import PoolAdd from "app/pools/views/PoolAdd";
 import PoolEdit from "app/pools/views/PoolEdit";
+import { useFetchMachineCount } from "app/store/machine/utils/hooks";
 import { getRelativeRoute } from "app/utils";
 
 const Pools = (): JSX.Element => {
   const base = urls.pools.index;
+  const { machineCount } = useFetchMachineCount();
   return (
     <Section
       header={
@@ -22,6 +24,7 @@ const Pools = (): JSX.Element => {
               Add pool
             </Button>,
           ]}
+          machineCount={machineCount}
         />
       }
     >

src/app/store/machine/utils/hooks.test.tsx

@@ -149,6 +149,28 @@ describe("machine hook utils", () => {
       expect(getDispatches()).toHaveLength(1);
     });
 
+    it("fetches if isEnabled changes back to true", async () => {
+      const store = mockStore(state);
+      const { rerender } = renderHook(
+        (queryOptions: UseFetchQueryOptions) =>
+          useFetchMachineCount({ hostname: "spotted-quoll" }, queryOptions),
+        {
+          initialProps: { isEnabled: true },
+          wrapper: generateWrapper(store),
+        }
+      );
+      const expectedActionType = machineActions.count("mocked-nanoid-1").type;
+      const getDispatches = () =>
+        store
+          .getActions()
+          .filter((action) => action.type === expectedActionType);
+      expect(getDispatches()).toHaveLength(1);
+      rerender({ isEnabled: false });
+      expect(getDispatches()).toHaveLength(1);
+      rerender({ isEnabled: true });
+      expect(getDispatches()).toHaveLength(2);
+    });
+
     it("returns the machine count", async () => {
       jest.restoreAllMocks();
       jest.spyOn(reduxToolkit, "nanoid").mockReturnValueOnce("mocked-nanoid");
@@ -345,6 +367,28 @@ describe("machine hook utils", () => {
       expect(getDispatches()).toHaveLength(1);
     });
 
+    it("fetches again if isEnabled changes back to true", async () => {
+      const store = mockStore(state);
+      const { rerender } = renderHook(
+        (queryOptions: UseFetchQueryOptions) =>
+          useFetchMachines(undefined, queryOptions),
+        {
+          initialProps: { isEnabled: true },
+          wrapper: generateWrapper(store),
+        }
+      );
+      const expectedActionType = machineActions.fetch("mocked-nanoid-1").type;
+      const getDispatches = () =>
+        store
+          .getActions()
+          .filter((action) => action.type === expectedActionType);
+      expect(getDispatches()).toHaveLength(1);
+      rerender({ isEnabled: false });
+      expect(getDispatches()).toHaveLength(1);
+      rerender({ isEnabled: true });
+      expect(getDispatches()).toHaveLength(2);
+    });
+
     it("does not fetch again if the options haven't changed including empty objects", () => {
       const store = mockStore(state);
       const { rerender } = renderHook(

src/app/store/machine/utils/hooks.ts

@@ -255,11 +255,13 @@ export const useSelectedMachinesActionsDispatch = ({
 };
 
 export const useMachineSelectedCount = (
-  filters?: FetchFilters | null
+  filters?: FetchFilters | null,
+  queryOptions?: UseFetchQueryOptions
 ): {
   selectedCount: number;
   selectedCountLoading: boolean;
 } => {
+  const { isEnabled } = queryOptions || { isEnabled: true };
   let selectedState = useSelector(machineSelectors.selectedMachines);
   let selectedCount = 0;
   // Shallow clone the selected state so that object can be modified.
@@ -274,11 +276,13 @@ export const useMachineSelectedCount = (
   }
   // Get the count of machines in selected groups or filters.
   const filtersFromSelected = selectedToFilters(selectedMachines);
-  // refactor to use a separate count for groups and separate for items
   const {
     machineCount: fetchedSelectedCount,
     machineCountLoading: selectedLoading,
-  } = useFetchMachineCount({ ...filters, ...filtersFromSelected });
+  } = useFetchMachineCount(
+    { ...filters, ...filtersFromSelected },
+    { isEnabled: isEnabled && filtersFromSelected !== null }
+  );
   // Only add the count if there are filters as sending `null` filters
   // to the count API will return a count of all machines.
   if (filtersFromSelected) {
@@ -388,13 +392,23 @@ export const useFetchMachineCount = (
   useEffect(() => {
     // undefined, null and {} are all equivalent i.e. no filters so compare the
     // current and previous filters using an empty object if the filters are falsy.
-    if (
-      (isEnabled && !fastDeepEqual(filters || {}, previousFilters || {})) ||
-      (isEnabled && !callId)
-    ) {
-      setCallId(nanoid());
+    if (isEnabled) {
+      if (
+        !fastDeepEqual(filters || {}, previousFilters || {}) ||
+        !callId ||
+        isEnabled !== previousIsEnabled
+      ) {
+        setCallId(nanoid());
+      }
     }
-  }, [callId, dispatch, filters, previousFilters, isEnabled]);
+  }, [
+    callId,
+    dispatch,
+    filters,
+    previousFilters,
+    isEnabled,
+    previousIsEnabled,
+  ]);
 
   useEffect(() => {
     return () => {
@@ -554,19 +568,26 @@ export const useFetchMachines = (
   useEffect(() => {
     // undefined, null and {} are all equivalent i.e. no filters so compare the
     // current and previous filters using an empty object if the filters are falsy.
-    if (
-      (isEnabled && !fastDeepEqual(options || {}, previousOptions || {})) ||
-      !callId
-    ) {
-      setCallId(nanoid());
+    if (isEnabled) {
+      if (
+        !fastDeepEqual(options || {}, previousOptions || {}) ||
+        !callId ||
+        isEnabled !== previousIsEnabled
+      ) {
+        setCallId(nanoid());
+      }
     }
-  }, [callId, options, previousOptions, isEnabled]);
+  }, [
+    callId,
+    dispatch,
+    options,
+    previousOptions,
+    isEnabled,
+    previousIsEnabled,
+  ]);
 
   useEffect(() => {
-    if (
-      (isEnabled && callId && callId !== previousCallId) ||
-      (isEnabled !== previousIsEnabled && callId)
-    ) {
+    if (isEnabled && callId && callId !== previousCallId) {
       dispatch(
         machineActions.fetch(
           callId,

src/app/store/machine/utils/search.test.ts

@@ -218,3 +218,11 @@ describe("client side search", () => {
     });
   });
 });
+
+it("isNonEmptyFilter returns false for empty search string", () => {
+  expect(FilterMachineItems.isNonEmptyFilter("")).toBe(false);
+});
+
+it("isNonEmptyFilter returns true for defined search string", () => {
+  expect(FilterMachineItems.isNonEmptyFilter("status:(=broken)")).toBe(true);
+});

src/app/store/machine/utils/search.ts

@@ -1,3 +1,5 @@
+import fastDeepEqual from "fast-deep-equal";
+
 import { MachineMeta } from "app/store/machine/types";
 import type { Machine, FetchFilters } from "app/store/machine/types";
 import type { Tag } from "app/store/tag/types";
@@ -242,6 +244,10 @@ class FilterMachineHandlers extends FilterHandlers {
     );
   };
 
+  isNonEmptyFilter = (filter: string) => {
+    return !fastDeepEqual(this.parseFetchFilters(filter), {});
+  };
+
   /**
    * Merge a new filter into the existing list of filters. This is required
    * because multiple DSL filter names can be mapped to the same API key name.

src/app/tags/components/TagsHeader/TagsHeader.tsx

@@ -1,6 +1,7 @@
 import { Button } from "@canonical/react-components";
 
 import MachinesHeader from "app/base/components/node/MachinesHeader";
+import { useFetchMachineCount } from "app/store/machine/utils/hooks";
 import TagHeaderForms from "app/tags/components/TagsHeader/TagHeaderForms";
 import { TagHeaderViews } from "app/tags/constants";
 import type { TagHeaderContent, TagSetHeaderContent } from "app/tags/types";
@@ -37,6 +38,7 @@ export const TagsHeader = ({
   setHeaderContent,
   tagViewState,
 }: Props): JSX.Element => {
+  const { machineCount } = useFetchMachineCount();
   return (
     <MachinesHeader
       aria-label={Label.Header}
@@ -62,6 +64,7 @@ export const TagsHeader = ({
           />
         )
       }
+      machineCount={machineCount}
       title={getHeaderTitle(headerContent)}
     />
   );