Skip to content

Commit

Permalink
fix(machines): repeated machine.count API calls
Browse files Browse the repository at this point in the history
  • Loading branch information
petermakowski committed Oct 19, 2022
1 parent 0c1d41e commit dbb0a62
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe("MachinesHeader", () => {
initialEntries={[{ pathname: "/machines", key: "testKey" }]}
>
<CompatRouter>
<MachinesHeader />
<MachinesHeader machineCount={2} />
</CompatRouter>
</MemoryRouter>
</Provider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,18 @@ 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 => {
const dispatch = useDispatch();
const location = useLocation();
const poolCount = useSelector(resourcePoolSelectors.count);
const tagCount = useSelector(tagSelectors.count);
const { machineCount } = useFetchMachineCount();

useEffect(() => {
dispatch(resourcePoolActions.fetch());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
});
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
});
Expand All @@ -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,
});
Expand All @@ -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,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -133,9 +135,10 @@ export const MachineListHeader = ({
/>
)
}
machineCount={allMachineCount}
subtitle={
<ModelListSubtitle
available={machineCount}
available={availableMachineCount || allMachineCount}
modelName="machine"
selected={selectedCount}
/>
Expand Down
3 changes: 3 additions & 0 deletions src/app/pools/views/Pools.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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={
Expand All @@ -22,6 +24,7 @@ const Pools = (): JSX.Element => {
Add pool
</Button>,
]}
machineCount={machineCount}
/>
}
>
Expand Down
10 changes: 7 additions & 3 deletions src/app/store/machine/utils/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) {
Expand Down
8 changes: 8 additions & 0 deletions src/app/store/machine/utils/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
6 changes: 6 additions & 0 deletions src/app/store/machine/utils/search.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions src/app/tags/components/TagsHeader/TagsHeader.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -37,6 +38,7 @@ export const TagsHeader = ({
setHeaderContent,
tagViewState,
}: Props): JSX.Element => {
const { machineCount } = useFetchMachineCount();
return (
<MachinesHeader
aria-label={Label.Header}
Expand All @@ -62,6 +64,7 @@ export const TagsHeader = ({
/>
)
}
machineCount={machineCount}
title={getHeaderTitle(headerContent)}
/>
);
Expand Down

0 comments on commit dbb0a62

Please sign in to comment.