From 011d48f6ad3d3a489fab6fcb897ad7f680c952ea Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Mon, 30 Sep 2024 15:10:34 -0400 Subject: [PATCH] fix(RHINENG-11501): Fix status and biz risk modal save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On CVE List page if I edit status of one or multiple cves in stage preview I will get an error modal of `Couldn’t update status Select items and update status to try again.` Then the default filters are removed, If I click reset filters then the filters are reapplied but cves are not sorted by publish date The cve(s) status is updated though There are 3 issues: - Edit status causes error modal - Default filters are removed after editting status - Clicking reset filters does not properly sort cves after This PR fixes an issue where the CVETableContext was not being properly passed to the useShowModal hook. --- .../SmartComponents/CVEs/CVEsAssets.js | 20 +---- .../Modals/BusinessRiskModal.js | 22 +++++- .../Modals/BusinessRiskModal.test.js | 54 ++++++++++++- .../SmartComponents/Modals/CveStatusModal.js | 26 +++++-- .../Modals/CveStatusModal.test.js | 78 ++++++++++++++++++- 5 files changed, 169 insertions(+), 31 deletions(-) diff --git a/src/Components/SmartComponents/CVEs/CVEsAssets.js b/src/Components/SmartComponents/CVEs/CVEsAssets.js index 5b7b1ec39..642012d1e 100644 --- a/src/Components/SmartComponents/CVEs/CVEsAssets.js +++ b/src/Components/SmartComponents/CVEs/CVEsAssets.js @@ -1,5 +1,5 @@ -import React, { useState, useCallback, useContext } from 'react'; +import React, { useState, useCallback } from 'react'; import { classNames, expandable, sortable, nowrap, wrappable } from '@patternfly/react-table'; import messages from '../../../Messages'; import { intl } from '../../../Utilities/IntlProvider'; @@ -9,9 +9,7 @@ import { clearNotifications } from '@redhat-cloud-services/frontend-components-notifications/redux'; import { useDispatch } from 'react-redux'; -import { fetchCveListByAccount, clearCVEsStore } from '../../../Store/Actions/Actions'; -import { updateRef } from '../../../Helpers/MiscHelper'; -import { CVETableContext } from './CVEs'; +import { fetchCveListByAccount } from '../../../Store/Actions/Actions'; export const VULNERABILITIES_HEADER = [ { @@ -87,26 +85,16 @@ export const useDownloadReport = (parameters, shouldUseHybridSystemFilter) => { export const useShowModal = (Modal, modalProps = {}) => { const [ModalComponent, setModalComponent] = useState(() => () => null); - const { - cves: { meta } = {}, - params, - methods: { apply } = {} - } = useContext(CVETableContext); - - const dispatch = useDispatch(); const showModal = useCallback((cvesList, goToFirstPage) => { setModalComponent(() => () => { - dispatch(clearCVEsStore()); - updateRef(goToFirstPage ? { ...meta, page: 1 } : meta, params, apply); - }} + goToFirstPage={goToFirstPage} {...modalProps} /> ); - }, [params, apply, meta]); + }, [modalProps, Modal]); return [ModalComponent, showModal]; }; diff --git a/src/Components/SmartComponents/Modals/BusinessRiskModal.js b/src/Components/SmartComponents/Modals/BusinessRiskModal.js index fea4dee88..09053ebea 100644 --- a/src/Components/SmartComponents/Modals/BusinessRiskModal.js +++ b/src/Components/SmartComponents/Modals/BusinessRiskModal.js @@ -1,13 +1,29 @@ import { Form, FormGroup, Radio, Stack, StackItem, TextArea } from '@patternfly/react-core'; import propTypes from 'prop-types'; -import React, { useEffect, useState, useCallback } from 'react'; +import React, { useContext, useEffect, useState, useCallback } from 'react'; import { setBusinessRisk } from '../../../Helpers/APIHelper'; import { BUSINESS_RISK_OPTIONS } from '../../../Helpers/constants'; import BaseModal from './BaseModal'; import { injectIntl } from 'react-intl'; import messages from '../../../Messages'; +import { useDispatch } from 'react-redux'; +import { CVETableContext } from '../CVEs/CVEs'; +import { clearCVEsStore } from '../../../Store/Actions/Actions'; +import { updateRef as updateRefHelper } from '../../../Helpers/MiscHelper'; + +export const BusinessRiskModal = ({ cves, goToFirstPage, intl }) => { + const { + cves: { meta } = {}, + params, + methods: { apply } = {} + } = useContext(CVETableContext); + const dispatch = useDispatch(); + + const updateRef = () => { + dispatch(clearCVEsStore()); + updateRefHelper(goToFirstPage ? { ...meta, page: 1 } : meta, params, apply); + }; -export const BusinessRiskModal = ({ cves, updateRef, intl }) => { const [cveList] = useState(cves); const [businessRiskId, setBusinessRiskId] = useState('0'); const [label, setLabel] = useState(); @@ -108,7 +124,7 @@ export const BusinessRiskModal = ({ cves, updateRef, intl }) => { BusinessRiskModal.propTypes = { cves: propTypes.array, - updateRef: propTypes.func, + goToFirstPage: propTypes.bool, intl: propTypes.any }; diff --git a/src/Components/SmartComponents/Modals/BusinessRiskModal.test.js b/src/Components/SmartComponents/Modals/BusinessRiskModal.test.js index b9b8bca32..0f7a149aa 100644 --- a/src/Components/SmartComponents/Modals/BusinessRiskModal.test.js +++ b/src/Components/SmartComponents/Modals/BusinessRiskModal.test.js @@ -1,13 +1,36 @@ import React from 'react'; import * as deps from '../../../Helpers/APIHelper'; +import { updateRef } from '../../../Helpers/MiscHelper'; import { BusinessRiskModal } from './BusinessRiskModal'; import ReducerRegistry from '../../../Utilities/ReducerRegistry'; import { intl } from '../../../Utilities/IntlProvider'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import TestWrapper from '../../../Utilities/TestWrapper'; import '@testing-library/jest-dom'; +import { CVETableContext } from '../CVEs/CVEs'; const store = ReducerRegistry.getStore(); +jest.mock('../../../Helpers/MiscHelper', () => ({ + ...jest.requireActual('../../../Helpers/MiscHelper'), + updateRef: jest.fn() +})); + +const mockContext = { + cves: { + isLoading: false, + meta: { + page: 1 + }, + }, + params: { + advisory_available: 'true', + affecting: 'system1', + sort: '-public_date' + }, + methods: { + apply: jest.fn() + } +}; describe('BusinessRiskModal', () => { const setBusinessRiskMock = jest.fn(parameters => new Promise(resolve => resolve(parameters))); @@ -43,7 +66,8 @@ describe('BusinessRiskModal', () => { expect(screen.getByRole('radio', { name: 'Low' })).not.toHaveAttribute('checked'); expect(screen.getByRole('radio', { name: 'Not defined' })).toHaveAttribute('checked'); }); - it('Should save a list of CVEs', () => { + it('Should save a list of CVEs', async () => { + const testContext = { ...mockContext, cves: { meta: { page: 2 }}}; const cves = [ { id: 'SOME CVE1', business_risk_id: '3' }, { id: 'SOME CVE2', business_risk_id: '2' }, @@ -52,7 +76,9 @@ describe('BusinessRiskModal', () => { ]; render( - + + + ); @@ -64,5 +90,27 @@ describe('BusinessRiskModal', () => { business_risk_text: 'Test', cve: ['SOME CVE1', 'SOME CVE2', 'SOME CVE3', 'SOME CVE4'] }); + await waitFor(() => expect(updateRef).toHaveBeenCalledWith({ page: 2 }, mockContext.params, mockContext.methods.apply)); + }); + it('Should call updateRef with goToFirstPage', async () => { + const testContext = { ...mockContext, cves: { meta: { page: 2 }}}; + const cves = [ + { id: 'SOME CVE1', business_risk_id: '3' }, + { id: 'SOME CVE2', business_risk_id: '2' }, + { id: 'SOME CVE3', business_risk_id: '1' }, + { id: 'SOME CVE4', business_risk_id: '0' } + ]; + render( + + + + + + ); + + fireEvent.click(screen.getByRole('radio', { name: 'Critical' })); + fireEvent.change(screen.getByRole('textbox', { name: /justification/i }), { target: { value: 'Test' } }); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => expect(updateRef).toHaveBeenCalledWith({ page: 1 }, mockContext.params, mockContext.methods.apply)); }); }); diff --git a/src/Components/SmartComponents/Modals/CveStatusModal.js b/src/Components/SmartComponents/Modals/CveStatusModal.js index d41dc2149..d76b1ce3a 100644 --- a/src/Components/SmartComponents/Modals/CveStatusModal.js +++ b/src/Components/SmartComponents/Modals/CveStatusModal.js @@ -1,13 +1,29 @@ import { Checkbox, Form, FormGroup, Icon, Split, SplitItem, Stack, StackItem, Tooltip } from '@patternfly/react-core'; import { InfoCircleIcon, OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; import propTypes from 'prop-types'; -import React, { useState, useEffect } from 'react'; +import React, { useContext, useState, useEffect } from 'react'; import { setCveStatus, setSystemCveStatus } from '../../../Helpers/APIHelper'; import BaseModal, { useJustificationInput, useStatusSelect } from './BaseModal'; import { injectIntl } from 'react-intl'; import messages from '../../../Messages'; +import { CVETableContext } from '../CVEs/CVEs'; +import { clearCVEsStore } from '../../../Store/Actions/Actions'; +import { useDispatch } from 'react-redux'; +import { updateRef as updateRefHelper } from '../../../Helpers/MiscHelper'; + +export const CveStatusModal = ({ cves, intl, canEditPairStatus, goToFirstPage }) => { + const { + cves: { meta } = {}, + params, + methods: { apply } = {} + } = useContext(CVETableContext); + const dispatch = useDispatch(); + + const updateRef = () => { + dispatch(clearCVEsStore()); + updateRefHelper(goToFirstPage ? { ...meta, page: 1 } : meta, params, apply); + }; -export const CveStatusModal = ({ cves, updateRef, intl, canEditPairStatus }) => { const [cveList] = useState(cves); const { StatusSelect, statusId, setProps: setSelectProps } = useStatusSelect(getDefaultStatus()); const { JustificationInput, justification } = useJustificationInput(getDefaultJustification()); @@ -19,7 +35,7 @@ export const CveStatusModal = ({ cves, updateRef, intl, canEditPairStatus }) => setSelectProps({ ouiaId: 'status-select' }); }, [setSelectProps]); - const handleSave = () => { + const handleSave = async () => { return Promise.all([ setCveStatus({ cve: cveList.map(item => item.id), @@ -138,9 +154,9 @@ export const CveStatusModal = ({ cves, updateRef, intl, canEditPairStatus }) => CveStatusModal.propTypes = { cves: propTypes.array, - updateRef: propTypes.func, intl: propTypes.any, - canEditPairStatus: propTypes.bool.isRequired + canEditPairStatus: propTypes.bool.isRequired, + goToFirstPage: propTypes.bool }; export default injectIntl(CveStatusModal); diff --git a/src/Components/SmartComponents/Modals/CveStatusModal.test.js b/src/Components/SmartComponents/Modals/CveStatusModal.test.js index a243295bd..bd8b59b4f 100644 --- a/src/Components/SmartComponents/Modals/CveStatusModal.test.js +++ b/src/Components/SmartComponents/Modals/CveStatusModal.test.js @@ -1,12 +1,35 @@ import React from 'react'; import { Provider } from 'react-redux'; import * as deps from '../../../Helpers/APIHelper'; +import { updateRef } from '../../../Helpers/MiscHelper'; import CVEStatusModal from './CveStatusModal'; import ReducerRegistry from '../../../Utilities/ReducerRegistry'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import TestWrapper from '../../../Utilities/TestWrapper'; +import { CVETableContext } from '../CVEs/CVEs'; const MockStore = ({ children }) => {children}; +jest.mock('../../../Helpers/MiscHelper', () => ({ + ...jest.requireActual('../../../Helpers/MiscHelper'), + updateRef: jest.fn() +})); + +const mockContext = { + cves: { + isLoading: false, + meta: { + page: 1 + }, + }, + params: { + advisory_available: 'true', + affecting: 'system1', + sort: '-public_date' + }, + methods: { + apply: jest.fn() + } +}; describe('CveStatusModal component', () => { it('should render with one CVE selected', () => { @@ -25,7 +48,7 @@ describe('CveStatusModal component', () => { render( - + ); @@ -46,13 +69,14 @@ describe('CveStatusModal component', () => { }); }); - it('should render with multiple CVEs selected', () => { + it('should render with multiple CVEs selected', async () => { const setCveStatusMock = jest.fn(parameters => new Promise(resolve => resolve(parameters))); const setSystemCveStatusMock = jest.fn(parameters => new Promise(resolve => resolve(parameters))); deps.setCveStatus = setCveStatusMock; // CVE status deps.setSystemCveStatus = setSystemCveStatusMock; // CVE-system pair status, only call when checkbox is unchecked + const testContext = { ...mockContext, cves: { meta: { page: 2 }}}; const cveList2 = [ { id: "CVE-2020-0001", @@ -69,7 +93,9 @@ describe('CveStatusModal component', () => { render( - + + + ); @@ -88,5 +114,49 @@ describe('CveStatusModal component', () => { status_id: 0, status_text: 'new justification' }); + + await waitFor(() => expect(updateRef).toHaveBeenCalledWith({ page: 2 }, mockContext.params, mockContext.methods.apply)); + }); + it('should call updateRef with goToFirstPage', async () => { + const setCveStatusMock = jest.fn(parameters => new Promise(resolve => resolve(parameters))); + const setSystemCveStatusMock = jest.fn(parameters => new Promise(resolve => resolve(parameters))); + + deps.setCveStatus = setCveStatusMock; // CVE status + deps.setSystemCveStatus = setSystemCveStatusMock; // CVE-system pair status, only call when checkbox is unchecked + + const testContext = { ...mockContext, cves: { meta: { page: 2 }}}; + const cveList2 = [ + { + id: "CVE-2020-0001", + status_id: "3", + justification: "test" + }, + { + id: "CVE-2020-0002", + status_id: "4", + justification: "test2" + } + ]; + + render( + + + + + + + + ); + + const statusDropdown = screen.getByRole('combobox', { name: /select input/i }); + const justificationNote = screen.getByRole('textbox', { name: /justification note/i }); + const saveButton = screen.getByRole('button', { name: /save/i }); + + fireEvent.click(statusDropdown); + screen.getByRole('option', { name: 'On-hold' }); + fireEvent.change(justificationNote, { target: { value: 'new justification' } }); + fireEvent.click(saveButton); + + await waitFor(() => expect(updateRef).toHaveBeenCalledWith({ page: 1 }, mockContext.params, mockContext.methods.apply)); }); });