From f4a9b0de025a602ea7ad20816867d538cf0eeeef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Gr=C3=A4=C3=9Fl?= Date: Thu, 23 Nov 2023 17:10:45 +0100 Subject: [PATCH] feat(TitleColumn): RHINENG-3112 - Allow passing in a link via href prop (#2087) * feat(TitleColumn): RHINENG-3112 - Allow passing in a link via href prop * fix(TitleColumn): Use react router Link InsightsLink is not fully working in scenarios where the Link is in another applications. --- .../ImmutableDevices/ImmutableDevices.cy.js | 4 +- .../ImmutableDevices/ImmutableDevices.js | 7 - src/components/InventoryTable/EntityTable.js | 15 +- .../InventoryTable/EntityTable.test.js | 37 ++-- .../InventoryTable/EntityTableToolbar.test.js | 12 +- .../InventoryTable/InventoryTable.js | 3 - src/components/InventoryTable/TitleColumn.js | 38 ++-- .../InventoryTable/TitleColumn.test.js | 130 +++++++----- .../__snapshots__/EntityTable.test.js.snap | 185 +++++++++--------- .../__snapshots__/TitleColumn.test.js.snap | 100 +++++++--- src/components/InventoryTable/helpers.js | 10 +- .../ConventionalSystemsTab.js | 1 - src/store/entities.js | 4 +- 13 files changed, 308 insertions(+), 238 deletions(-) diff --git a/src/components/ImmutableDevices/ImmutableDevices.cy.js b/src/components/ImmutableDevices/ImmutableDevices.cy.js index c69926310..48721ad12 100644 --- a/src/components/ImmutableDevices/ImmutableDevices.cy.js +++ b/src/components/ImmutableDevices/ImmutableDevices.cy.js @@ -174,8 +174,8 @@ describe('ImmutableDevices', () => { cy.get('td[data-label="Name"]') .first() - .find('.ins-composed-col > :nth-child(2) > a') - .trigger('click'); + .find('.ins-composed-col > div > a') + .click(); cy.get('#mock-detail-page'); }); diff --git a/src/components/ImmutableDevices/ImmutableDevices.js b/src/components/ImmutableDevices/ImmutableDevices.js index 928c91e15..15f062e38 100644 --- a/src/components/ImmutableDevices/ImmutableDevices.js +++ b/src/components/ImmutableDevices/ImmutableDevices.js @@ -4,7 +4,6 @@ import React, { useEffect } from 'react'; import { TableVariant } from '@patternfly/react-table'; import { InventoryTable } from '../InventoryTable'; import useFeatureFlag from '../../Utilities/useFeatureFlag'; -import { useNavigate } from 'react-router-dom'; import { edgeColumns } from './columns'; const ImmutableDevices = ({ @@ -22,7 +21,6 @@ const ImmutableDevices = ({ ...props }) => { const dispatch = useDispatch(); - const navigate = useNavigate(); const inventoryGroupsEnabled = useFeatureFlag('hbi.ui.inventory-groups'); const totalItems = useSelector(({ entities }) => entities?.total); @@ -44,10 +42,6 @@ const ImmutableDevices = ({ return [...mergeAppColumns(filteredColumns), ...edgeColumns]; }; - const onRowClick = (_key, systemId) => { - navigate(`/insights/inventory/${systemId}?appName=vulnerabilities`); - }; - return ( { const dispatch = useDispatch(); - const location = useLocation(); const columns = useColumns( columnsProp, disableDefaultColumns, @@ -51,7 +49,6 @@ const EntityTable = ({ columnsCounter ); const rows = useSelector(({ entities: { rows } }) => rows); - const navigate = useInsightsNavigate(); const onItemSelect = (_event, checked, rowId) => { const row = isExpandable ? rows[rowId / 2] : rows[rowId]; dispatch(selectEntity(rowId === -1 ? 0 : row.id, checked)); @@ -70,14 +67,6 @@ const EntityTable = ({ [loaded, columns, hasItems, rows, isExpandable] ); - const defaultRowClick = (_event, key) => { - navigate( - `${location.pathname}${ - location.pathname.slice(-1) === '/' ? '' : '/' - }${key}` - ); - }; - const tableSortBy = { index: columns?.findIndex( @@ -110,7 +99,7 @@ const EntityTable = ({ actions, expandable, loaded, - onRowClick: onRowClick || defaultRowClick, + onRowClick, noDetail, sortBy, noSystemsTable, diff --git a/src/components/InventoryTable/EntityTable.test.js b/src/components/InventoryTable/EntityTable.test.js index 84de3de25..c6360cc50 100644 --- a/src/components/InventoryTable/EntityTable.test.js +++ b/src/components/InventoryTable/EntityTable.test.js @@ -2,6 +2,10 @@ /* eslint-disable camelcase */ /* eslint-disable react/display-name */ import React from 'react'; +import { + fireEvent, + render as rederWithTestingLibrary, +} from '@testing-library/react'; import { act } from 'react-dom/test-utils'; import EntityTable from './EntityTable'; import { mount, render } from 'enzyme'; @@ -34,7 +38,16 @@ describe('EntityTable', () => { }, ], columns: [ - { key: 'one', sortKey: 'one', title: 'One', renderFunc: TitleColumn }, + { + key: 'one', + sortKey: 'one', + title: 'One', + renderFunc: (display_name, id, item, props) => ( + + {display_name} + + ), + }, ], page: 1, perPage: 50, @@ -636,31 +649,23 @@ describe('EntityTable', () => { describe('API', () => { jest.mock('../../Utilities/useFeatureFlag'); - it('should call default onRowClick', () => { - // const history = createMemoryHistory(); - // const store = mockStore(initialState); - // const wrapper = mount( - // - // - // - // - // - // ); - // wrapper.find('table tbody tr a[widget="col"]').first().simulate('click'); - // expect(history.location.pathname).toBe('/testing-id'); - }); it('should call onRowClick', () => { const onRowClick = jest.fn(); const store = mockStore(initialState); - const wrapper = mount( + const { container } = rederWithTestingLibrary( ); - wrapper.find('table tbody tr a[widget="col"]').first().simulate('click'); + + const link = container.querySelector('table tbody tr a'); + act(() => { + fireEvent.click(link); + }); + expect(onRowClick).toHaveBeenCalled(); }); diff --git a/src/components/InventoryTable/EntityTableToolbar.test.js b/src/components/InventoryTable/EntityTableToolbar.test.js index a0d071c24..6e54dd993 100644 --- a/src/components/InventoryTable/EntityTableToolbar.test.js +++ b/src/components/InventoryTable/EntityTableToolbar.test.js @@ -42,7 +42,17 @@ describe('EntityTableToolbar', () => { one: 'data', }, ], - columns: [{ key: 'one', title: 'One', renderFunc: TitleColumn }], + columns: [ + { + key: 'one', + title: 'One', + renderFunc: (display_name, id, item, props) => ( + + {display_name} + + ), + }, + ], page: 1, perPage: 50, total: 500, diff --git a/src/components/InventoryTable/InventoryTable.js b/src/components/InventoryTable/InventoryTable.js index 3c01c31b1..3635b7039 100644 --- a/src/components/InventoryTable/InventoryTable.js +++ b/src/components/InventoryTable/InventoryTable.js @@ -82,7 +82,6 @@ const InventoryTable = forwardRef( tableProps, isRbacEnabled, hasCheckbox, - onRowClick, abortOnUnmount = true, showCentosVersions = false, ...props @@ -281,7 +280,6 @@ const InventoryTable = forwardRef( { +const onRowClick = (event, id, { loaded, onRowClick: rowClick, noDetail }) => { if (loaded && !noDetail) { const isMetaKey = event.ctrlKey || event.metaKey || event.which === 2; if (isMetaKey) { return; } else if (rowClick) { - rowClick(event, key, isMetaKey); + rowClick(event, id, isMetaKey); } } @@ -24,32 +24,34 @@ const onRowClick = (event, key, { loaded, onRowClick: rowClick, noDetail }) => { }; /** - * Helper function to generate first cell in plain inventory either with clickable detail or just data from attribut. + * Helper component to generate first cell in plain inventory either with clickable detail or just data from attribut. * This is later on used in redux in `renderFunc`. - * @param {React.node} data React node with information that will be shown to user as column title. + * @param {React.node} children React node with information that will be shown to user as column title. * @param {string} id inventory UUID, used to navigate to correct URL. * @param {*} item row data, holds every information from redux store for currecnt row. * @param {*} props additional props passed from `EntityTable` - holds any props passed to inventory table. */ -const TitleColumn = (data, id, item, props) => ( +const TitleColumn = ({ children, id, item, ...props }) => (
-
{item?.os_release}
+ {item?.os_release &&
{item?.os_release}
}
{props?.noDetail ? ( - data + children ) : ( - { - onRowClick(event, id, props); + { + onRowClick(event, id, props); + }, + } + : {}), }} > - {data} - + {children} + )}
diff --git a/src/components/InventoryTable/TitleColumn.test.js b/src/components/InventoryTable/TitleColumn.test.js index 313b8e1f5..87983264c 100644 --- a/src/components/InventoryTable/TitleColumn.test.js +++ b/src/components/InventoryTable/TitleColumn.test.js @@ -1,71 +1,107 @@ -/* eslint-disable new-cap */ -/* eslint-disable camelcase */ import React from 'react'; import TitleColumn from './TitleColumn'; -import { mount } from 'enzyme'; -import toJson from 'enzyme-to-json'; +import { act, fireEvent, render } from '@testing-library/react'; + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + Link: ({ children, ...props }) => {children}, // eslint-disable-line +})); describe('TitleColumn', () => { it('should render correctly with NO data', () => { - const Cmp = () => TitleColumn(); - const wrapper = mount(); - expect(toJson(wrapper)).toMatchSnapshot(); + const { asFragment } = render(); + expect(asFragment()).toMatchSnapshot(); }); it('should render correctly with data', () => { - const Cmp = () => - TitleColumn('something', 'some-id', { os_release: 'os_release' }); - const wrapper = mount(); - expect(toJson(wrapper)).toMatchSnapshot(); + const { asFragment } = render( + something + ); + expect(asFragment()).toMatchSnapshot(); + }); + + it('should render correctly with os_release', () => { + const { asFragment } = render( + + something + + ); + + expect(asFragment()).toMatchSnapshot(); + }); + + it('should render correctly with href', () => { + const { asFragment } = render( + + something + + ); + + expect(asFragment()).toMatchSnapshot(); + }); + + it('should render correctly with to', () => { + const { asFragment } = render( + + something + + ); + + expect(asFragment()).toMatchSnapshot(); }); it('should render correctly no detail with data', () => { - const Cmp = () => - TitleColumn( - 'something', - 'some-id', - { os_release: 'os_release' }, - { noDetail: true } - ); - const wrapper = mount(); - expect(toJson(wrapper)).toMatchSnapshot(); + const { asFragment } = render( + + something + + ); + expect(asFragment()).toMatchSnapshot(); }); describe('API', () => { - const open = jest.fn(); + it('should call onClick', () => { + const onClick = jest.fn(); + const { container } = render( + + something + + ); - beforeEach(() => { - Object.defineProperty(window, 'open', { - writable: true, - value: open, + const link = container.querySelector('a'); + act(() => { + fireEvent.click(link); }); - }); - it('should call onClick', () => { - const onClick = jest.fn(); - const Cmp = () => - TitleColumn( - 'something', - 'some-id', - { os_release: 'os_release' }, - { onRowClick: onClick, loaded: true } - ); - const wrapper = mount(); - wrapper.find('a').first().simulate('click'); expect(onClick).toHaveBeenCalled(); }); - it('should NOT call onClick', () => { + it('should not call onClick if not loaded', () => { const onClick = jest.fn(); - const Cmp = () => - TitleColumn( - 'something', - 'some-id', - { os_release: 'os_release' }, - { onRowClick: onClick } - ); - const wrapper = mount(); - wrapper.find('a').first().simulate('click'); + const { container } = render( + + something + + ); + + const link = container.querySelector('a'); + act(() => { + fireEvent.click(link); + }); + expect(onClick).not.toHaveBeenCalled(); }); }); diff --git a/src/components/InventoryTable/__snapshots__/EntityTable.test.js.snap b/src/components/InventoryTable/__snapshots__/EntityTable.test.js.snap index 9eedda7bc..a7699a7e5 100644 --- a/src/components/InventoryTable/__snapshots__/EntityTable.test.js.snap +++ b/src/components/InventoryTable/__snapshots__/EntityTable.test.js.snap @@ -104,13 +104,11 @@ exports[`EntityTable DOM should render correctly - compact 1`] = `
-
data @@ -284,22 +282,19 @@ exports[`EntityTable DOM should render correctly - is expandable 1`] = ` }, "cells": Array [ -
-
- -
+ data + , ], "id": "testing-id", @@ -470,22 +465,19 @@ exports[`EntityTable DOM should render correctly - with actions 1`] = ` }, "cells": Array [ -
-
- -
+ data + , ], "id": "testing-id", @@ -604,22 +596,19 @@ exports[`EntityTable DOM should render correctly - without checkbox 1`] = ` }, "cells": Array [ -
-
- -
+ data + , ], "id": "testing-id", @@ -743,13 +732,11 @@ exports[`EntityTable DOM should render correctly 1`] = `
-
data @@ -812,22 +799,25 @@ exports[`EntityTable DOM sort by should render correctly - is expandable 1`] = ` }, "cells": Array [ -
-
- -
+ data + , ], "id": "testing-id", @@ -892,22 +882,25 @@ exports[`EntityTable DOM sort by should render correctly - without checkbox 1`] }, "cells": Array [ -
-
- -
+ data + , ], "id": "testing-id", @@ -973,22 +966,26 @@ exports[`EntityTable DOM sort by should render correctly 1`] = ` }, "cells": Array [ -
-
- -
+ data + , ], "id": "testing-id", diff --git a/src/components/InventoryTable/__snapshots__/TitleColumn.test.js.snap b/src/components/InventoryTable/__snapshots__/TitleColumn.test.js.snap index 930d3903c..9356f9657 100644 --- a/src/components/InventoryTable/__snapshots__/TitleColumn.test.js.snap +++ b/src/components/InventoryTable/__snapshots__/TitleColumn.test.js.snap @@ -1,69 +1,113 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`TitleColumn should render correctly no detail with data 1`] = ` - +
-
+
os_release
something
- + `; exports[`TitleColumn should render correctly with NO data 1`] = ` - +
- + `; exports[`TitleColumn should render correctly with data 1`] = ` - +
+
+ +`; + +exports[`TitleColumn should render correctly with href 1`] = ` + +
+ +
+ +`; + +exports[`TitleColumn should render correctly with os_release 1`] = ` + +
+
os_release
+
+ +`; + +exports[`TitleColumn should render correctly with to 1`] = ` + +
+
- + `; diff --git a/src/components/InventoryTable/helpers.js b/src/components/InventoryTable/helpers.js index 48920928f..d8eacd8d6 100644 --- a/src/components/InventoryTable/helpers.js +++ b/src/components/InventoryTable/helpers.js @@ -8,15 +8,11 @@ import { Skeleton } from '@patternfly/react-core'; export const buildCells = (item, columns, extra) => { return columns.map(({ key, composed, renderFunc }) => { - // eslint-disable-next-line new-cap const data = composed ? ( - {TitleColumn( - composed.map((key) => get(item, key, ' ')), - item.id, - item, - extra - )} + + {composed.map((key) => get(item, key, ' '))} + ) : ( get(item, key, ' ') diff --git a/src/components/InventoryTabs/ConventionalSystems/ConventionalSystemsTab.js b/src/components/InventoryTabs/ConventionalSystems/ConventionalSystemsTab.js index 591cab042..24e8fa7c2 100644 --- a/src/components/InventoryTabs/ConventionalSystems/ConventionalSystemsTab.js +++ b/src/components/InventoryTabs/ConventionalSystems/ConventionalSystemsTab.js @@ -295,7 +295,6 @@ const ConventionalSystemsTab = ({ ], }} bulkSelect={bulkSelectConfig} - onRowClick={(_e, id, app) => navigate(`/${id}${app ? `/${app}` : ''}`)} showCentosVersions={true} /> diff --git a/src/store/entities.js b/src/store/entities.js index 23894fb3d..6823d2f76 100644 --- a/src/store/entities.js +++ b/src/store/entities.js @@ -46,7 +46,9 @@ export const defaultColumns = (groupsEnabled = false) => [ key: 'display_name', sortKey: 'display_name', title: 'Name', - renderFunc: TitleColumn, + renderFunc: (display_name, id, item, props) => ( + {display_name} + ), }, ...(groupsEnabled ? [