From dc3143b4377a56258f9da65ba0b0f5f8a4629d67 Mon Sep 17 00:00:00 2001 From: RachelElysia <71795832+RachelElysia@users.noreply.github.com> Date: Thu, 16 Dec 2021 15:35:19 -0800 Subject: [PATCH] Pack flow: Refactor ManagePacksPage (#3376) --- changes/issue-3281-packs-loading-state | 1 + cypress/integration/free/admin.spec.ts | 2 + .../packs/ManagePacksPage/ManagePacksPage.tsx | 103 ++++++++++-------- .../PacksListWrapper/PacksListWrapper.tsx | 32 +++--- .../PacksListWrapper/PacksTableConfig.tsx | 39 ++++--- 5 files changed, 95 insertions(+), 82 deletions(-) create mode 100644 changes/issue-3281-packs-loading-state diff --git a/changes/issue-3281-packs-loading-state b/changes/issue-3281-packs-loading-state new file mode 100644 index 0000000000..84687b86de --- /dev/null +++ b/changes/issue-3281-packs-loading-state @@ -0,0 +1 @@ +* ManagePacksPage loading state updated along with use of Context API \ No newline at end of file diff --git a/cypress/integration/free/admin.spec.ts b/cypress/integration/free/admin.spec.ts index ebe38ec7e0..b7d8f30003 100644 --- a/cypress/integration/free/admin.spec.ts +++ b/cypress/integration/free/admin.spec.ts @@ -116,6 +116,8 @@ describe( // On the Settings pages, they should… // See everything except for the “Teams” pages cy.visit("/settings/organization"); + cy.wait(1000); // eslint-disable-line cypress/no-unnecessary-waiting + cy.findByText(/teams/i).should("not.exist"); cy.get(".react-tabs").within(() => { cy.findByText(/organization settings/i).should("exist"); diff --git a/frontend/pages/packs/ManagePacksPage/ManagePacksPage.tsx b/frontend/pages/packs/ManagePacksPage/ManagePacksPage.tsx index c21fbf254c..097de0f24f 100644 --- a/frontend/pages/packs/ManagePacksPage/ManagePacksPage.tsx +++ b/frontend/pages/packs/ManagePacksPage/ManagePacksPage.tsx @@ -1,17 +1,16 @@ -import React, { useState, useCallback, useEffect } from "react"; +import React, { useState, useCallback, useContext } from "react"; import { useDispatch, useSelector } from "react-redux"; +import { useQuery } from "react-query"; -import { push } from "react-router-redux"; -import pack, { IPack } from "interfaces/pack"; -import { IUser } from "interfaces/user"; +import { IPack } from "interfaces/pack"; +import { IError } from "interfaces/errors"; -// @ts-ignore -import packActions from "redux/nodes/entities/packs/actions"; +import { AppContext } from "context/app"; +import packsAPI from "services/entities/packs"; // @ts-ignore import { renderFlash } from "redux/nodes/notifications/actions"; -import paths from "router/paths"; -import permissionUtils from "utilities/permissions"; +import PATHS from "router/paths"; // @ts-ignore import deepDifference from "utilities/deep_difference"; @@ -21,17 +20,13 @@ import PacksListWrapper from "./components/PacksListWrapper"; import RemovePackModal from "./components/RemovePackModal"; const baseClass = "manage-packs-page"; -interface IRootState { - auth: { - user: IUser; - }; - entities: { - packs: { - isLoading: boolean; - data: IPack[]; - errors: { name: string; reason: string }[]; - }; - }; + +interface IManagePacksPageProps { + router: any; +} + +interface IPacksResponse { + packs: IPack[]; } const renderTable = ( @@ -39,45 +34,56 @@ const renderTable = ( onEnablePackClick: React.MouseEventHandler, onDisablePackClick: React.MouseEventHandler, onCreatePackClick: React.MouseEventHandler, - packsList: IPack[], - packsErrors: { name: string; reason: string }[] + packs: IPack[] | undefined, + packsError: IError | null, + isLoadingPacks: boolean ): JSX.Element => { - if (Object.keys(packsErrors).length > 0) { + if (packsError) { return ; } + const isTableDataLoading = isLoadingPacks || packs === null; + return ( ); }; -const ManagePacksPage = (): JSX.Element => { - const currentUser = useSelector((state: IRootState) => state.auth.user); - const isOnlyObserver = permissionUtils.isOnlyObserver(currentUser); +const ManagePacksPage = ({ router }: IManagePacksPageProps): JSX.Element => { + const { isOnlyObserver } = useContext(AppContext); const dispatch = useDispatch(); - const { NEW_PACK } = paths; - const onCreatePackClick = () => dispatch(push(NEW_PACK)); - useEffect(() => { - dispatch(packActions.loadAll()); - }, [dispatch]); - - const packs = useSelector((state: IRootState) => state.entities.packs); - const packsList = Object.values(packs.data); - const packsErrors = packs.errors; + const onCreatePackClick = () => router.push(PATHS.NEW_PACK); const [selectedPackIds, setSelectedPackIds] = useState([]); const [showRemovePackModal, setShowRemovePackModal] = useState( false ); + const { + data: packs, + error: packsError, + isLoading: isLoadingPacks, + refetch: refetchPacks, + } = useQuery( + "packs", + () => packsAPI.loadAll(), + { + // refetchOnMount: false, + // refetchOnReconnect: false, + refetchOnWindowFocus: false, + select: (data: IPacksResponse) => data.packs, + } + ); + const toggleRemovePackModal = useCallback(() => { setShowRemovePackModal(!showRemovePackModal); }, [showRemovePackModal, setShowRemovePackModal]); @@ -91,7 +97,8 @@ const ManagePacksPage = (): JSX.Element => { const packOrPacks = selectedPackIds.length === 1 ? "pack" : "packs"; const promises = selectedPackIds.map((id: number) => { - return dispatch(packActions.destroy({ id })); + packsAPI.destroy(id); + return null; }); return Promise.all(promises) @@ -100,7 +107,6 @@ const ManagePacksPage = (): JSX.Element => { renderFlash("success", `Successfully deleted ${packOrPacks}.`) ); toggleRemovePackModal(); - dispatch(packActions.loadAll()); }) .catch(() => { dispatch( @@ -109,9 +115,12 @@ const ManagePacksPage = (): JSX.Element => { `Unable to remove ${packOrPacks}. Please try again.` ) ); + }) + .finally(() => { + refetchPacks(); toggleRemovePackModal(); }); - }, [dispatch, selectedPackIds, toggleRemovePackModal]); + }, [dispatch, refetchPacks, selectedPackIds, toggleRemovePackModal]); const onEnableDisablePackSubmit = useCallback( (selectedTablePackIds: any, disablePack: boolean) => { @@ -119,7 +128,8 @@ const ManagePacksPage = (): JSX.Element => { const enableOrDisable = disablePack ? "disabled" : "enabled"; const promises = selectedTablePackIds.map((id: number) => { - return dispatch(packActions.update({ id }, { disabled: disablePack })); + packsAPI.update(id, { disabled: disablePack }); + return null; }); return Promise.all(promises) @@ -130,7 +140,6 @@ const ManagePacksPage = (): JSX.Element => { `Successfully ${enableOrDisable} selected ${packOrPacks}.` ) ); - dispatch(packActions.loadAll()); }) .catch(() => { dispatch( @@ -139,9 +148,12 @@ const ManagePacksPage = (): JSX.Element => { `Unable to ${enableOrDisable} selected ${packOrPacks}. Please try again.` ) ); + }) + .finally(() => { + refetchPacks(); }); }, - [dispatch, selectedPackIds] + [dispatch, refetchPacks, selectedPackIds] ); const onEnablePackClick = (selectedTablePackIds: any) => { @@ -171,7 +183,7 @@ const ManagePacksPage = (): JSX.Element => { - {!isOnlyObserver && packsList.length > 0 && ( + {!isOnlyObserver && packs && packs.length > 0 && (
- {!packs.isLoading && + {!isLoadingPacks && renderTable( onRemovePackClick, onEnablePackClick, onDisablePackClick, onCreatePackClick, - packsList, - packsErrors + packs, + packsError, + isLoadingPacks )}
{showRemovePackModal && ( diff --git a/frontend/pages/packs/ManagePacksPage/components/PacksListWrapper/PacksListWrapper.tsx b/frontend/pages/packs/ManagePacksPage/components/PacksListWrapper/PacksListWrapper.tsx index ecdfb863c4..526709c1e3 100644 --- a/frontend/pages/packs/ManagePacksPage/components/PacksListWrapper/PacksListWrapper.tsx +++ b/frontend/pages/packs/ManagePacksPage/components/PacksListWrapper/PacksListWrapper.tsx @@ -18,19 +18,14 @@ interface IPacksListWrapperProps { onEnablePackClick: any; onDisablePackClick: any; onCreatePackClick: any; - packsList: IPack[]; + packs?: IPack[]; + isLoading: boolean; } interface IRootState { auth: { user: IUser; }; - entities: { - packs: { - isLoading: boolean; - data: IPack[]; - }; - }; } const PacksListWrapper = ({ @@ -38,29 +33,28 @@ const PacksListWrapper = ({ onEnablePackClick, onDisablePackClick, onCreatePackClick, - packsList, + packs, + isLoading, }: IPacksListWrapperProps): JSX.Element => { - const loadingTableData = useSelector( - (state: IRootState) => state.entities.packs.isLoading - ); - const currentUser = useSelector((state: IRootState) => state.auth.user); const isOnlyObserver = permissionUtils.isOnlyObserver(currentUser); - const [filteredPacks, setFilteredPacks] = useState(packsList); + const [filteredPacks, setFilteredPacks] = useState( + packs + ); const [searchString, setSearchString] = useState(""); useEffect(() => { - setFilteredPacks(packsList); - }, [packsList]); + setFilteredPacks(packs); + }, [packs]); useEffect(() => { setFilteredPacks(() => { - return packsList.filter((pack) => { + return packs?.filter((pack) => { return pack.name.toLowerCase().includes(searchString.toLowerCase()); }); }); - }, [packsList, searchString, setFilteredPacks]); + }, [packs, searchString, setFilteredPacks]); const onQueryChange = useCallback( (queryData) => { @@ -129,14 +123,14 @@ const PacksListWrapper = ({ resultsTitle={"packs"} columns={tableHeaders} data={generateDataSet(filteredPacks)} - isLoading={loadingTableData} + isLoading={isLoading} defaultSortHeader={"pack"} defaultSortDirection={"desc"} showMarkAllPages={false} isAllPagesSelected={false} onQueryChange={onQueryChange} inputPlaceHolder="Search by name" - searchable={packsList.length > 0} + searchable={packs && packs.length > 0} disablePagination onPrimarySelectActionClick={onRemovePackClick} primarySelectActionButtonVariant="text-icon" diff --git a/frontend/pages/packs/ManagePacksPage/components/PacksListWrapper/PacksTableConfig.tsx b/frontend/pages/packs/ManagePacksPage/components/PacksListWrapper/PacksTableConfig.tsx index 929ea0f55c..ac11257529 100644 --- a/frontend/pages/packs/ManagePacksPage/components/PacksListWrapper/PacksTableConfig.tsx +++ b/frontend/pages/packs/ManagePacksPage/components/PacksListWrapper/PacksTableConfig.tsx @@ -47,12 +47,12 @@ interface IDataColumn { } interface IPackTableData { - id: number; - name: string; - query_count: number; - status: string; - total_hosts_count: number; - updated_at: string; + id?: number; + name?: string; + query_count?: number; + status?: string; + total_hosts_count?: number; + updated_at?: string; } // NOTE: cellProps come from react-table @@ -146,20 +146,23 @@ const generateTableHeaders = (isOnlyObserver = true): IDataColumn[] => { return tableHeaders; }; -const enhancePackData = (packs: IPack[]): IPackTableData[] => { - return packs.map((pack: IPack) => { - return { - id: pack.id, - name: pack.name, - query_count: pack.query_count, - status: pack.disabled ? "disabled" : "enabled", - total_hosts_count: pack.total_hosts_count, - updated_at: pack.updated_at, - }; - }); +const enhancePackData = (packs: IPack[] | undefined): IPackTableData[] => { + if (packs) { + return packs.map((pack: IPack) => { + return { + id: pack.id, + name: pack.name, + query_count: pack.query_count, + status: pack.disabled ? "disabled" : "enabled", + total_hosts_count: pack.total_hosts_count, + updated_at: pack.updated_at, + }; + }); + } + return []; }; -const generateDataSet = (packs: IPack[]): IPackTableData[] => { +const generateDataSet = (packs: IPack[] | undefined): IPackTableData[] => { return [...enhancePackData(packs)]; };