From c173b504489d866060ce7af1b78488793946ea24 Mon Sep 17 00:00:00 2001 From: denysrachynskyi Date: Tue, 27 Jun 2023 14:11:42 +0200 Subject: [PATCH 1/5] Fixed brokers not shown if disk usage unknown --- .../Brokers/BrokersList/BrokersList.tsx | 49 ++++++++++++------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx b/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx index e59c006b0c6..fe7a0a8683c 100644 --- a/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx +++ b/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx @@ -12,6 +12,7 @@ import { ColumnDef } from '@tanstack/react-table'; import { clusterBrokerPath } from 'lib/paths'; import Tooltip from 'components/common/Tooltip/Tooltip'; import ColoredCell from 'components/common/NewTable/ColoredCell'; +import { BrokerDiskUsage } from 'generated-sources'; import SkewHeader from './SkewHeader/SkewHeader'; import * as S from './BrokersList.styled'; @@ -22,7 +23,7 @@ const BrokersList: React.FC = () => { const navigate = useNavigate(); const { clusterName } = useAppParams<{ clusterName: ClusterName }>(); const { data: clusterStats = {} } = useClusterStats(clusterName); - const { data: brokers } = useBrokers(clusterName); + const { data: brokers = [] } = useBrokers(clusterName); const { brokerCount, @@ -37,34 +38,46 @@ const BrokersList: React.FC = () => { } = clusterStats; const rows = React.useMemo(() => { - let brokersResource; + let brokersResource: BrokerDiskUsage[]; if (!diskUsage || !diskUsage?.length) { brokersResource = brokers?.map((broker) => { return { brokerId: broker.id, - segmentSize: NA, - segmentCount: NA, + segmentSize: NA as unknown as number, + segmentCount: NA as unknown as number, }; }) || []; } else { brokersResource = diskUsage; } - return brokersResource.map(({ brokerId, segmentSize, segmentCount }) => { - const broker = brokers?.find(({ id }) => id === brokerId); - return { - brokerId, - size: segmentSize || NA, - count: segmentCount || NA, - port: broker?.port, - host: broker?.host, - partitionsLeader: broker?.partitionsLeader, - partitionsSkew: broker?.partitionsSkew, - leadersSkew: broker?.leadersSkew, - inSyncPartitions: broker?.inSyncPartitions, - }; - }); + return brokers?.map( + ({ + id, + port, + host, + partitionsLeader, + partitionsSkew, + leadersSkew, + inSyncPartitions, + }) => { + const brokerResource = brokersResource.find( + ({ brokerId }) => id === brokerId + ); + return { + brokerId: id, + size: brokerResource?.segmentSize || NA, + count: brokerResource?.segmentCount || NA, + port, + host, + partitionsLeader, + partitionsSkew, + leadersSkew, + inSyncPartitions, + }; + } + ); }, [diskUsage, brokers]); const columns = React.useMemo[]>( From 13652ac46cadd7f2cd7bd6bc945eb7ab5027bed8 Mon Sep 17 00:00:00 2001 From: denysrachynskyi Date: Tue, 27 Jun 2023 14:51:31 +0200 Subject: [PATCH 2/5] unit tests update --- .../BrokersList/__test__/BrokersList.spec.tsx | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx b/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx index 0c60cf4749c..132b7bba6a5 100644 --- a/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx +++ b/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx @@ -56,11 +56,11 @@ describe('BrokersList Component', () => { }); it('opens broker when row clicked', async () => { renderComponent(); - await userEvent.click(screen.getByRole('cell', { name: '0' })); + await userEvent.click(screen.getByRole('cell', { name: '1' })); await waitFor(() => expect(mockedUsedNavigate).toBeCalledWith( - clusterBrokerPath(clusterName, '0') + clusterBrokerPath(clusterName, '1') ) ); }); @@ -166,5 +166,24 @@ describe('BrokersList Component', () => { ); }); }); + + describe('when some brokers have no diskUsage', () => { + beforeEach(() => { + (useBrokers as jest.Mock).mockImplementation(() => ({ + data: brokersPayload, + })); + (useClusterStats as jest.Mock).mockImplementation(() => ({ + data: { + ...clusterStatsPayload, + diskUsage: [clusterStatsPayload.diskUsage[0]], + }, + })); + }); + it('renders list of all brokers', async () => { + renderComponent(); + expect(screen.getByRole('table')).toBeInTheDocument(); + expect(screen.getAllByRole('row').length).toEqual(3); + }); + }); }); }); From b794266daba6a51e7dd8fe7b6d29c405a0366e9a Mon Sep 17 00:00:00 2001 From: denysrachynskyi Date: Tue, 27 Jun 2023 15:03:48 +0200 Subject: [PATCH 3/5] N/A as unknown to assign type easier --- .../src/components/Brokers/BrokersList/BrokersList.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx b/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx index fe7a0a8683c..361e5c91d5e 100644 --- a/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx +++ b/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx @@ -17,7 +17,7 @@ import { BrokerDiskUsage } from 'generated-sources'; import SkewHeader from './SkewHeader/SkewHeader'; import * as S from './BrokersList.styled'; -const NA = 'N/A'; +const NA = 'N/A' as unknown; const BrokersList: React.FC = () => { const navigate = useNavigate(); @@ -44,8 +44,8 @@ const BrokersList: React.FC = () => { brokers?.map((broker) => { return { brokerId: broker.id, - segmentSize: NA as unknown as number, - segmentCount: NA as unknown as number, + segmentSize: NA as number, + segmentCount: NA as number, }; }) || []; } else { From 4440a615eba11234d42d5a7a2a20bd4cea96ad01 Mon Sep 17 00:00:00 2001 From: denysrachynskyi Date: Fri, 30 Jun 2023 11:16:16 +0200 Subject: [PATCH 4/5] N/A type assignment in one place --- .../src/components/Brokers/BrokersList/BrokersList.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx b/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx index 361e5c91d5e..5627f619e14 100644 --- a/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx +++ b/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx @@ -17,7 +17,7 @@ import { BrokerDiskUsage } from 'generated-sources'; import SkewHeader from './SkewHeader/SkewHeader'; import * as S from './BrokersList.styled'; -const NA = 'N/A' as unknown; +const NA = 'N/A' as unknown as number; const BrokersList: React.FC = () => { const navigate = useNavigate(); @@ -44,8 +44,8 @@ const BrokersList: React.FC = () => { brokers?.map((broker) => { return { brokerId: broker.id, - segmentSize: NA as number, - segmentCount: NA as number, + segmentSize: NA, + segmentCount: NA, }; }) || []; } else { From ac5c5a3ce25679f30378a4325ac2faace2c59cdf Mon Sep 17 00:00:00 2001 From: denysrachynskyi Date: Wed, 5 Jul 2023 12:56:38 +0200 Subject: [PATCH 5/5] BrokersList tests to use dynamic value from mock --- .../BrokersList/__test__/BrokersList.spec.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx b/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx index 132b7bba6a5..db870456868 100644 --- a/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx +++ b/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx @@ -52,7 +52,9 @@ describe('BrokersList Component', () => { it('renders', async () => { renderComponent(); expect(screen.getByRole('table')).toBeInTheDocument(); - expect(screen.getAllByRole('row').length).toEqual(3); + expect(screen.getAllByRole('row').length).toEqual( + brokersPayload.length + 1 + ); }); it('opens broker when row clicked', async () => { renderComponent(); @@ -153,7 +155,9 @@ describe('BrokersList Component', () => { it('renders list of all brokers', async () => { renderComponent(); expect(screen.getByRole('table')).toBeInTheDocument(); - expect(screen.getAllByRole('row').length).toEqual(3); + expect(screen.getAllByRole('row').length).toEqual( + brokersPayload.length + 1 + ); }); it('opens broker when row clicked', async () => { renderComponent(); @@ -182,7 +186,9 @@ describe('BrokersList Component', () => { it('renders list of all brokers', async () => { renderComponent(); expect(screen.getByRole('table')).toBeInTheDocument(); - expect(screen.getAllByRole('row').length).toEqual(3); + expect(screen.getAllByRole('row').length).toEqual( + brokersPayload.length + 1 + ); }); }); });