From 976712db7f43366544f28f3b950797dff22bc04e Mon Sep 17 00:00:00 2001 From: Lior Keren Date: Tue, 19 Mar 2024 10:40:58 +0200 Subject: [PATCH 1/4] refactor(CVEsTableToolbar): Remove unnecessary func def in onSelect --- src/Components/SmartComponents/CVEs/CVEsTableToolbar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/SmartComponents/CVEs/CVEsTableToolbar.js b/src/Components/SmartComponents/CVEs/CVEsTableToolbar.js index 33331c7cb..0e5386a66 100644 --- a/src/Components/SmartComponents/CVEs/CVEsTableToolbar.js +++ b/src/Components/SmartComponents/CVEs/CVEsTableToolbar.js @@ -159,7 +159,7 @@ const CVEsTableToolbarWithContext = ({ context, canEditStatusOrBusinessRisk, can isDisabled: cves.meta.total_items === 0 && selectedCvesCount === 0, checked: Boolean(selectedCvesCount), ouiaId: 'bulk-select', - onSelect: () => selectOptions.handleOnCheckboxChange() + onSelect: selectOptions.handleOnCheckboxChange } : undefined} filterConfig={{ items: [ From ed4370513b17a535dd38602a756823c1175ca02d Mon Sep 17 00:00:00 2001 From: Lior Keren Date: Tue, 19 Mar 2024 13:54:50 +0200 Subject: [PATCH 2/4] refactor(CVEs): Replace isFirstLoad with causeARerender Reduce unnecessary re-renders --- src/Components/SmartComponents/CVEs/CVEs.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Components/SmartComponents/CVEs/CVEs.js b/src/Components/SmartComponents/CVEs/CVEs.js index 567f9402f..d09b4975a 100644 --- a/src/Components/SmartComponents/CVEs/CVEs.js +++ b/src/Components/SmartComponents/CVEs/CVEs.js @@ -35,7 +35,10 @@ export const CVEs = ({ rbac }) => { const dispatch = useDispatch(); const [CveStatusModal, setStatusModal] = useState(() => () => null); const [CveBusinessRiskModal, setBusinessRiskModal] = useState(() => () => null); - const [isFirstLoad, setFirstLoad] = useState(true); + + // This state causes a re-render when updateRef is called in and + // to fetch the updated status of the CVEs + const [causeARerender, setCauseARerender] = useState(false); const [[ canEditStatusOrBusinessRisk, @@ -81,14 +84,9 @@ export const CVEs = ({ rbac }) => { }, [shouldUseHybridSystemFilter]); useEffect(() => { - if (isFirstLoad) { - setFirstLoad(false); - } - else { - dispatch(fetchCveListByAccount(parameters, shouldUseHybridSystemFilter)); - setUrlParam({ ...parameters }); - } - }, [parameters, isFirstLoad, shouldUseHybridSystemFilter]); + dispatch(fetchCveListByAccount(parameters, shouldUseHybridSystemFilter)); + setUrlParam({ ...parameters }); + }, [parameters, causeARerender, shouldUseHybridSystemFilter]); useEffect(() => { return () => { @@ -119,7 +117,7 @@ export const CVEs = ({ rbac }) => { { - setFirstLoad(true); + setCauseARerender(!causeARerender); dispatch(clearCVEsStore()); updateRef(goToFirstPage ? { ...meta, page: 1 } : meta, parameters, apply); }} @@ -134,7 +132,7 @@ export const CVEs = ({ rbac }) => { cves={cvesList} canEditPairStatus={canEditPairStatus} updateRef={() => { - setFirstLoad(true); + setCauseARerender(!causeARerender); dispatch(clearCVEsStore()); updateRef(goToFirstPage ? { ...meta, page: 1 } : meta, parameters, apply); }} From ca971585ff22504660a37093560e02c4f34730b4 Mon Sep 17 00:00:00 2001 From: Lior Keren Date: Tue, 19 Mar 2024 13:56:26 +0200 Subject: [PATCH 3/4] refactor(CVEs): Add updateRefFuncBuilder helper function Remove code duplication --- src/Components/SmartComponents/CVEs/CVEs.js | 25 +++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Components/SmartComponents/CVEs/CVEs.js b/src/Components/SmartComponents/CVEs/CVEs.js index d09b4975a..d5ac2c542 100644 --- a/src/Components/SmartComponents/CVEs/CVEs.js +++ b/src/Components/SmartComponents/CVEs/CVEs.js @@ -111,31 +111,32 @@ export const CVEs = ({ rbac }) => { ); }; - const showBusinessRiskModal = (cvesList, goToFirstPage) => { + // helper function for showBusinessRiskModal and showStatusModal: + const updateRefFuncBuilder = (goToFirstPage) => { const { meta } = cves; + + return (() => { + setCauseARerender(!causeARerender); + dispatch(clearCVEsStore()); + updateRef(goToFirstPage ? { ...meta, page: 1 } : meta, parameters, apply); + }); + }; + + const showBusinessRiskModal = (cvesList, goToFirstPage) => { setBusinessRiskModal(() => () => { - setCauseARerender(!causeARerender); - dispatch(clearCVEsStore()); - updateRef(goToFirstPage ? { ...meta, page: 1 } : meta, parameters, apply); - }} + updateRef={updateRefFuncBuilder(goToFirstPage)} /> ); }; const showStatusModal = (cvesList, goToFirstPage) => { - const { meta } = cves; setStatusModal(() => () => { - setCauseARerender(!causeARerender); - dispatch(clearCVEsStore()); - updateRef(goToFirstPage ? { ...meta, page: 1 } : meta, parameters, apply); - }} + updateRef={updateRefFuncBuilder(goToFirstPage)} /> ); }; From eda8a99559b7737305b95eb93b793835aec609fa Mon Sep 17 00:00:00 2001 From: Lior Keren Date: Thu, 21 Mar 2024 18:10:57 +0200 Subject: [PATCH 4/4] temp - Creating subcomponents for the modals & removing them from state --- src/Components/SmartComponents/CVEs/CVEs.js | 76 ++++++++++++++----- .../CVEs/components/CveBusinessRiskModal.js | 22 ++++++ .../CVEs/components/CveStatusModal.js | 24 ++++++ .../SmartComponents/Modals/BaseModal.js | 19 ++++- .../Modals/BusinessRiskModal.js | 6 +- .../SmartComponents/Modals/CveStatusModal.js | 6 +- 6 files changed, 127 insertions(+), 26 deletions(-) create mode 100644 src/Components/SmartComponents/CVEs/components/CveBusinessRiskModal.js create mode 100644 src/Components/SmartComponents/CVEs/components/CveStatusModal.js diff --git a/src/Components/SmartComponents/CVEs/CVEs.js b/src/Components/SmartComponents/CVEs/CVEs.js index d5ac2c542..7cd3be221 100644 --- a/src/Components/SmartComponents/CVEs/CVEs.js +++ b/src/Components/SmartComponents/CVEs/CVEs.js @@ -5,8 +5,8 @@ import PropTypes from 'prop-types'; import { CVES_ALLOWED_PARAMS, PATCHMAN_ADVISORY_DOCS_PATH, PERMISSIONS, SERVICE_NAME } from '../../../Helpers/constants'; import { createCveListByAccount } from '../../../Helpers/VulnerabilityHelper'; import { constructFilterParameters, updateRef, useUrlParams } from '../../../Helpers/MiscHelper'; -import BusinessRiskModal from '../Modals/BusinessRiskModal'; -import StatusModal from '../Modals/CveStatusModal'; +import { CveStatusModal } from './components/CveStatusModal'; +import { CveBusinessRiskModal } from './components/CveBusinessRiskModal'; import CVEsTable from './CVEsTable'; import CVEsTableToolbar from './CVEsTableToolbar'; import DownloadReport from '../../../Helpers/DownloadReport'; @@ -33,8 +33,37 @@ export const CVETableContext = React.createContext({}); export const CVEs = ({ rbac }) => { const dispatch = useDispatch(); - const [CveStatusModal, setStatusModal] = useState(() => () => null); - const [CveBusinessRiskModal, setBusinessRiskModal] = useState(() => () => null); + + // These states will contain data only when the modals are open + // This is necessary because the showModal functions get these props down in the component tree + const [cvesList, setCvesList] = useState([]); + const [goToFirstPage, setGoToFirstPage] = useState(false); + + const [isStatusModalOpen, setIsStatusModalOpen] = useState(false); + const [isBusinessRiskModalOpen, setIsBusinessRiskModalOpen] = useState(false); + + const closeModalHandler = () => { + setCvesList([]); + setGoToFirstPage(false); + }; + + const closeStatusModalHandler = () => { + closeModalHandler(); + setIsStatusModalOpen(false); + }; + + const closeBusinessRiskModalHandler = () => { + closeModalHandler(); + setIsBusinessRiskModalOpen(false); + }; + + // useEffect(() => { + // closeModalHandler(isStatusModalOpen); + // }, [isStatusModalOpen]); + + // useEffect(() => { + // closeModalHandler(isBusinessRiskModalOpen); + // }, [isBusinessRiskModalOpen]); // This state causes a re-render when updateRef is called in and // to fetch the updated status of the CVEs @@ -122,23 +151,18 @@ export const CVEs = ({ rbac }) => { }); }; - const showBusinessRiskModal = (cvesList, goToFirstPage) => { - setBusinessRiskModal(() => () => - - ); + const showModal = (setIsModalOpen, cvesList, goToFirstPage) => { + setCvesList(cvesList); + setGoToFirstPage(goToFirstPage); + setIsModalOpen(true); }; const showStatusModal = (cvesList, goToFirstPage) => { - setStatusModal(() => () => - - ); + showModal(setIsStatusModalOpen, cvesList, goToFirstPage); + }; + + const showBusinessRiskModal = (cvesList, goToFirstPage) => { + showModal(setIsBusinessRiskModalOpen, cvesList, goToFirstPage); }; const openCves = (cves) => { @@ -167,8 +191,20 @@ export const CVEs = ({ rbac }) => { } }} > - - + { isBusinessRiskModalOpen && + closeBusinessRiskModalHandler()} + />} + + { isStatusModalOpen && + closeStatusModalHandler()} + />} {ColumnManagementModal} diff --git a/src/Components/SmartComponents/CVEs/components/CveBusinessRiskModal.js b/src/Components/SmartComponents/CVEs/components/CveBusinessRiskModal.js new file mode 100644 index 000000000..7d01e4f45 --- /dev/null +++ b/src/Components/SmartComponents/CVEs/components/CveBusinessRiskModal.js @@ -0,0 +1,22 @@ +import React from 'react'; +import propTypes from 'prop-types'; + +import BusinessRiskModal from '../..//Modals/BusinessRiskModal'; + +export const CveBusinessRiskModal = ({ cvesList, updateRefFunc, closeModalHandler }) => ( + +); + +CveBusinessRiskModal.propTypes = { + cvesList: propTypes.array, + updateRefFunc: propTypes.func, + closeModalHandler: propTypes.func +}; + +CveBusinessRiskModal.defaultProps = { + cvesList: [] +}; diff --git a/src/Components/SmartComponents/CVEs/components/CveStatusModal.js b/src/Components/SmartComponents/CVEs/components/CveStatusModal.js new file mode 100644 index 000000000..f7d4136f8 --- /dev/null +++ b/src/Components/SmartComponents/CVEs/components/CveStatusModal.js @@ -0,0 +1,24 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import StatusModal from '../../Modals/CveStatusModal'; + +export const CveStatusModal = ({ cvesList, canEditPairStatus, updateRefFunc, closeModalHandler }) => ( + +); + +CveStatusModal.propTypes = { + cvesList: PropTypes.array, + canEditPairStatus: PropTypes.bool, + updateRefFunc: PropTypes.func, + closeModalHandler: PropTypes.func +}; + +CveStatusModal.defaultProps = { + cvesList: [] +}; diff --git a/src/Components/SmartComponents/Modals/BaseModal.js b/src/Components/SmartComponents/Modals/BaseModal.js index 84480f697..8eac038d3 100644 --- a/src/Components/SmartComponents/Modals/BaseModal.js +++ b/src/Components/SmartComponents/Modals/BaseModal.js @@ -47,12 +47,22 @@ export function useJustificationInput(initialValue) { return { JustificationInput, justification, setJustification, setProps }; } -export const BaseModal = ({ items, title, onSave, onSuccessNotification, onFailureNotification, ouiaId, children }) => { +export const BaseModal = ({ + items, + title, + onSave, + onSuccessNotification, + onFailureNotification, + ouiaId, + children, + closeModalHandler +}) => { const [targetItems, setTargetItems] = useState(items); const dispatch = useDispatch(); const handleClose = () => { + closeModalHandler(); setTargetItems(undefined); }; @@ -96,7 +106,12 @@ BaseModal.propTypes = { onSave: propTypes.func, onSuccessNotification: propTypes.object, onFailureNotification: propTypes.object, - ouiaId: propTypes.string + ouiaId: propTypes.string, + closeModalHandler: propTypes.func +}; + +BaseModal.defaultProps = { + closeModalHandler: () => {} }; export default BaseModal; diff --git a/src/Components/SmartComponents/Modals/BusinessRiskModal.js b/src/Components/SmartComponents/Modals/BusinessRiskModal.js index e2d42ad65..b6485ba6b 100644 --- a/src/Components/SmartComponents/Modals/BusinessRiskModal.js +++ b/src/Components/SmartComponents/Modals/BusinessRiskModal.js @@ -7,7 +7,7 @@ import BaseModal from './BaseModal'; import { injectIntl } from 'react-intl'; import messages from '../../../Messages'; -export const BusinessRiskModal = ({ cves, updateRef, intl }) => { +export const BusinessRiskModal = ({ cves, updateRef, intl, closeModalHandler }) => { const [cveList] = useState(cves); const [businessRiskId, setBusinessRiskId] = useState('0'); const [label, setLabel] = useState(); @@ -66,6 +66,7 @@ export const BusinessRiskModal = ({ cves, updateRef, intl }) => { onFailureNotification={onFailureNotification} title={intl.formatMessage(messages.businessRiskModalTitle)} ouiaId="business-risk-modal" + closeModalHandler={closeModalHandler} > @@ -109,7 +110,8 @@ export const BusinessRiskModal = ({ cves, updateRef, intl }) => { BusinessRiskModal.propTypes = { cves: propTypes.array, updateRef: propTypes.func, - intl: propTypes.any + intl: propTypes.any, + closeModalHandler: propTypes.func }; export default injectIntl(BusinessRiskModal); diff --git a/src/Components/SmartComponents/Modals/CveStatusModal.js b/src/Components/SmartComponents/Modals/CveStatusModal.js index 637998a6d..e963e6e21 100644 --- a/src/Components/SmartComponents/Modals/CveStatusModal.js +++ b/src/Components/SmartComponents/Modals/CveStatusModal.js @@ -7,7 +7,7 @@ import BaseModal, { useJustificationInput, useStatusSelect } from './BaseModal'; import { injectIntl } from 'react-intl'; import messages from '../../../Messages'; -export const CveStatusModal = ({ cves, updateRef, intl, canEditPairStatus }) => { +export const CveStatusModal = ({ cves, updateRef, intl, canEditPairStatus, closeModalHandler }) => { const [cveList] = useState(cves); const { StatusSelect, statusId, setProps: setSelectProps } = useStatusSelect(getDefaultStatus()); const { JustificationInput, justification } = useJustificationInput(getDefaultJustification()); @@ -70,6 +70,7 @@ export const CveStatusModal = ({ cves, updateRef, intl, canEditPairStatus }) => onFailureNotification={onFailureNotification} title={title} ouiaId="status-modal" + closeModalHandler={closeModalHandler} > @@ -139,7 +140,8 @@ CveStatusModal.propTypes = { cves: propTypes.array, updateRef: propTypes.func, intl: propTypes.any, - canEditPairStatus: propTypes.bool.isRequired + canEditPairStatus: propTypes.bool.isRequired, + closeModalHandler: propTypes.func }; export default injectIntl(CveStatusModal);