From 0fa19bf9e993787f49066ea238cf8e1e2ef56fc9 Mon Sep 17 00:00:00 2001 From: Pavel Makarichev Date: Sat, 3 Feb 2024 13:37:42 +0300 Subject: [PATCH 1/3] FE: Fix broker list when disk usage is unknown --- .../Brokers/BrokersList/BrokersList.tsx | 44 +++++++------ .../BrokersList/__test__/BrokersList.spec.tsx | 63 +++++++++++-------- 2 files changed, 57 insertions(+), 50 deletions(-) diff --git a/frontend/src/components/Brokers/BrokersList/BrokersList.tsx b/frontend/src/components/Brokers/BrokersList/BrokersList.tsx index ede570c65..75e063516 100644 --- a/frontend/src/components/Brokers/BrokersList/BrokersList.tsx +++ b/frontend/src/components/Brokers/BrokersList/BrokersList.tsx @@ -12,11 +12,16 @@ 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 keyBy from 'lodash/keyBy'; import SkewHeader from './SkewHeader/SkewHeader'; import * as S from './BrokersList.styled'; const NA = 'N/A'; +const NA_DISK_USAGE = { + segmentCount: NA, + segmentSize: NA, +}; const BrokersList: React.FC = () => { const navigate = useNavigate(); @@ -37,32 +42,25 @@ const BrokersList: React.FC = () => { } = clusterStats; const rows = React.useMemo(() => { - let brokersResource; - if (!diskUsage || !diskUsage?.length) { - brokersResource = - brokers?.map((broker) => { - return { - brokerId: broker.id, - segmentSize: NA, - segmentCount: NA, - }; - }) || []; - } else { - brokersResource = diskUsage; + if (!brokers || brokers.length === 0) { + return []; } - return brokersResource.map(({ brokerId, segmentSize, segmentCount }) => { - const broker = brokers?.find(({ id }) => id === brokerId); + const diskUsageByBroker = keyBy(diskUsage, 'brokerId'); + + return brokers.map((broker) => { + const diskUse = diskUsageByBroker[broker.id] || NA_DISK_USAGE; + 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, + brokerId: broker.id, + size: diskUse.segmentSize, + count: diskUse.segmentCount, + port: broker.port, + host: broker.host, + partitionsLeader: broker.partitionsLeader, + partitionsSkew: broker.partitionsSkew, + leadersSkew: broker.leadersSkew, + inSyncPartitions: broker.inSyncPartitions, }; }); }, [diskUsage, brokers]); diff --git a/frontend/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx b/frontend/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx index 3e88569a3..e98f28e52 100644 --- a/frontend/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx +++ b/frontend/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx @@ -154,24 +154,49 @@ describe('BrokersList Component', () => { expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() ); }); - }); - }); - describe('when diskUsage is empty', () => { - beforeEach(() => { - (useBrokers as jest.Mock).mockImplementation(() => ({ - data: brokersPayload, - })); - (useClusterStats as jest.Mock).mockImplementation(() => ({ - data: { ...clusterStatsPayload, diskUsage: undefined }, - })); + describe('when diskUsage', () => { + describe('is empty', () => { + beforeEach(() => { + (useClusterStats as jest.Mock).mockImplementation(() => ({ + data: { ...clusterStatsPayload, diskUsage: undefined }, + })); + }); + + it('renders list of all brokers', async () => { + renderComponent(); + expect(screen.getByRole('table')).toBeInTheDocument(); + expect(screen.getAllByRole('row').length).toEqual(3); + }); + }); + + describe('was NOT set for second broker', () => { + beforeEach(() => { + (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); + }); + }); + }); }); - describe('when it has no brokers', () => { + describe('when the brokers list is empty', () => { beforeEach(() => { (useBrokers as jest.Mock).mockImplementation(() => ({ data: [], })); + (useClusterStats as jest.Mock).mockImplementation(() => ({ + data: clusterStatsPayload, + })); }); it('renders empty table', async () => { @@ -182,22 +207,6 @@ describe('BrokersList Component', () => { ).toBeInTheDocument(); }); }); - - it('renders list of all brokers', async () => { - renderComponent(); - expect(screen.getByRole('table')).toBeInTheDocument(); - expect(screen.getAllByRole('row').length).toEqual(3); - }); - it('opens broker when row clicked', async () => { - renderComponent(); - await userEvent.click(screen.getByRole('cell', { name: '100' })); - - await waitFor(() => - expect(mockedUsedNavigate).toBeCalledWith( - clusterBrokerPath(clusterName, '100') - ) - ); - }); }); }); }); From 8c17623a345903208745a81a57168a1c27dd0348 Mon Sep 17 00:00:00 2001 From: Pavel Makarichev Date: Mon, 5 Feb 2024 22:41:16 +0300 Subject: [PATCH 2/3] FE: Get rid of lodash --- .../Brokers/BrokersList/BrokersList.tsx | 2 +- .../src/lib/functions/__tests__/keyBy.spec.ts | 17 +++++++++++++++++ frontend/src/lib/functions/keyBy.ts | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 frontend/src/lib/functions/__tests__/keyBy.spec.ts create mode 100644 frontend/src/lib/functions/keyBy.ts diff --git a/frontend/src/components/Brokers/BrokersList/BrokersList.tsx b/frontend/src/components/Brokers/BrokersList/BrokersList.tsx index 75e063516..f869269fa 100644 --- a/frontend/src/components/Brokers/BrokersList/BrokersList.tsx +++ b/frontend/src/components/Brokers/BrokersList/BrokersList.tsx @@ -10,9 +10,9 @@ import Table, { LinkCell, SizeCell } from 'components/common/NewTable'; import CheckMarkRoundIcon from 'components/common/Icons/CheckMarkRoundIcon'; import { ColumnDef } from '@tanstack/react-table'; import { clusterBrokerPath } from 'lib/paths'; +import { keyBy } from 'lib/functions/keyBy'; import Tooltip from 'components/common/Tooltip/Tooltip'; import ColoredCell from 'components/common/NewTable/ColoredCell'; -import keyBy from 'lodash/keyBy'; import SkewHeader from './SkewHeader/SkewHeader'; import * as S from './BrokersList.styled'; diff --git a/frontend/src/lib/functions/__tests__/keyBy.spec.ts b/frontend/src/lib/functions/__tests__/keyBy.spec.ts new file mode 100644 index 000000000..9c3455de3 --- /dev/null +++ b/frontend/src/lib/functions/__tests__/keyBy.spec.ts @@ -0,0 +1,17 @@ +import { keyBy } from 'lib/functions/keyBy'; + +describe('keyBy', () => { + it('returns grouped object', () => { + const original = [ + { id: 100, host: 'b-1.test.kafka.amazonaws.com', port: 9092 }, + { id: 200, host: 'b-2.test.kafka.amazonaws.com', port: 9092 }, + ]; + const expected = { + 100: { id: 100, host: 'b-1.test.kafka.amazonaws.com', port: 9092 }, + 200: { id: 200, host: 'b-2.test.kafka.amazonaws.com', port: 9092 }, + }; + const result = keyBy(original, 'id'); + + expect(result).toEqual(expected); + }); +}); diff --git a/frontend/src/lib/functions/keyBy.ts b/frontend/src/lib/functions/keyBy.ts new file mode 100644 index 000000000..c04a671c0 --- /dev/null +++ b/frontend/src/lib/functions/keyBy.ts @@ -0,0 +1,18 @@ +type PropertyExtractorByKey = { + [P in A[K] as A[K] extends PropertyKey ? A[K] : never]: A; +}; + +export function keyBy< + A extends object, + K extends keyof { + [P in keyof A as A[P] extends PropertyKey ? P : never]: unknown; + } +>(collection: A[] | undefined | null, property: K) { + if (collection === undefined || collection === null) { + return {} as PropertyExtractorByKey; + } + + return collection.reduce((acc, cur) => { + return { ...acc, [cur[property] as unknown as PropertyKey]: cur }; + }, {} as PropertyExtractorByKey); +} From ed78da881a9fc9d872ae0031e57a2c17114a9160 Mon Sep 17 00:00:00 2001 From: Pavel Makarichev Date: Tue, 6 Feb 2024 16:34:41 +0300 Subject: [PATCH 3/3] FE: Simplify signature, mutate object --- frontend/src/lib/functions/keyBy.ts | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/frontend/src/lib/functions/keyBy.ts b/frontend/src/lib/functions/keyBy.ts index c04a671c0..8a5b84ed0 100644 --- a/frontend/src/lib/functions/keyBy.ts +++ b/frontend/src/lib/functions/keyBy.ts @@ -1,18 +1,21 @@ -type PropertyExtractorByKey = { - [P in A[K] as A[K] extends PropertyKey ? A[K] : never]: A; +type AvailableKeys = keyof { + [P in keyof A as A[P] extends PropertyKey ? P : never]: unknown; }; -export function keyBy< - A extends object, - K extends keyof { - [P in keyof A as A[P] extends PropertyKey ? P : never]: unknown; - } ->(collection: A[] | undefined | null, property: K) { +export function keyBy>( + collection: A[] | undefined | null, + property: K +) { if (collection === undefined || collection === null) { - return {} as PropertyExtractorByKey; + return {} as Record; } - return collection.reduce((acc, cur) => { - return { ...acc, [cur[property] as unknown as PropertyKey]: cur }; - }, {} as PropertyExtractorByKey); + return collection.reduce>((acc, cur) => { + const key = cur[property] as unknown as PropertyKey; + + // eslint-disable-next-line no-param-reassign + acc[key] = cur; + + return acc; + }, {}); }