fix(RHINENG-11501): Fix status and biz risk modal save

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.
RedHatInsights · Oct 2, 2024 · 011d48f · 011d48f
1 parent 0c9ebdf
commit 011d48f
Show file tree

Hide file tree

Showing 5 changed files with 169 additions and 31 deletions.
diff --git 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(() => () =>
             <Modal
                 cves={cvesList}
-                updateRef={() => {
-                    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
@@ -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
@@ -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(
             <TestWrapper store={ store }>
-                <BusinessRiskModal open cves={cves} updateRef={jest.fn()} intl={intl}/>
+                <CVETableContext.Provider value={testContext}>
+                    <BusinessRiskModal open cves={cves} intl={intl} />
+                </CVETableContext.Provider>
             </TestWrapper>
         );
 
@@ -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(
+            <TestWrapper store={ store }>
+                <CVETableContext.Provider value={testContext}>
+                    <BusinessRiskModal open cves={cves} intl={intl} goToFirstPage={true} />
+                </CVETableContext.Provider>
+            </TestWrapper>
+        );
+
+        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
@@ -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
@@ -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 }) => <Provider store={ReducerRegistry.getStore()}>{children}</Provider>;
+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(
             <MockStore>
                 <TestWrapper>
-                    <CVEStatusModal open cves={cveList} />
+                    <CVEStatusModal open cves={cveList} canEditPairStatus={true} />
                 </TestWrapper>
             </MockStore>
         );
@@ -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(
             <MockStore>
                 <TestWrapper>
-                    <CVEStatusModal open cves={cveList2} />
+                    <CVETableContext.Provider value={testContext}>
+                        <CVEStatusModal open cves={cveList2} canEditPairStatus={true} />
+                    </CVETableContext.Provider>
                 </TestWrapper>
             </MockStore>
         );
@@ -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(
+            <MockStore>
+                <TestWrapper>
+                    <CVETableContext.Provider value={testContext}>
+                        <CVEStatusModal open cves={cveList2} goToFirstPage={true} canEditPairStatus={true} />
+                    </CVETableContext.Provider>
+                </TestWrapper>
+            </MockStore>
+        );
+
+        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));
     });
 });