Skip to content

Commit 368ea66

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #730 from PeterYurkovich/OCPBUGS-72604
OCPBUGS-72604: remove top level dispatch into useEffect
2 parents 9daccd5 + 9d02b51 commit 368ea66

1 file changed

Lines changed: 158 additions & 123 deletions

File tree

‎web/src/components/MetricsPage.tsx‎

Lines changed: 158 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -710,135 +710,170 @@ export const QueryTable: FC<QueryTableProps> = ({ index, namespace, customDataso
710710
setSortBy({});
711711
}, [namespace, query]);
712712

713-
if (!isEnabled || !isExpanded || !query) {
714-
return null;
715-
}
716-
717-
if (error) {
718-
return <Error error={error} title={t('Error loading values')} />;
719-
}
720-
721-
if (!data) {
722-
return <LoadingInline />;
723-
}
724-
725-
// Add any data series from `series` (those displayed in the graph) that are not in `data.result`.
726-
// This happens for queries that exclude a series currently, but included that same series at some
727-
// point during the graph's range.
728-
const expiredSeries = _.differenceWith(series, data.result, (s, r) => _.isEqual(s, r.metric));
729-
const result = expiredSeries.length
730-
? [...data.result, ...expiredSeries.map((metric) => ({ metric }))]
731-
: data.result;
732-
733-
if (!result || result.length === 0) {
734-
return (
735-
<div data-test={DataTestIDs.MetricsPageYellowNoDatapointsFound}>
736-
<YellowExclamationTriangleIcon /> {t('No datapoints found.')}
737-
</div>
738-
);
739-
}
713+
const isUnused = !isEnabled || !isExpanded || !query;
714+
const isError = !!error;
715+
const isLoading = !data;
716+
const result = useMemo(() => {
717+
if (isUnused || isError || isLoading) {
718+
return [];
719+
}
720+
// Add any data series from `series` (those displayed in the graph) that are not
721+
// in `data.result`.This happens for queries that exclude a series currently, but
722+
// included that same series at some point during the graph's range.
723+
const expiredSeries = _.differenceWith(series, data.result, (s, r) => _.isEqual(s, r.metric));
724+
return expiredSeries.length
725+
? [...data.result, ...expiredSeries.map((metric) => ({ metric }))]
726+
: data.result;
727+
}, [data?.result, series, isUnused, isError, isLoading]);
728+
const isEmptyGraph = !result || result.length === 0;
729+
730+
const tableData = useMemo(() => {
731+
if (isUnused || isError || isLoading || isEmptyGraph) {
732+
return {};
733+
}
734+
const transforms: ITransform[] = [sortable, wrappable];
740735

741-
const transforms: ITransform[] = [sortable, wrappable];
742-
743-
const buttonCell = (labels) => ({ title: <SeriesButton index={index} labels={labels} /> });
744-
745-
let columns, rows;
746-
if (data.resultType === 'scalar') {
747-
columns = [
748-
'',
749-
{
750-
title: t('Value'),
751-
transforms,
752-
cellTransforms: [
753-
(data: IFormatterValueType) => {
754-
const val = data?.title ? data.title : data;
755-
return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
756-
},
757-
],
758-
},
759-
];
760-
rows = [[buttonCell({}), _.get(result, '[1]')]];
761-
} else if (data.resultType === 'string') {
762-
columns = [
763-
{
764-
title: t('Value'),
765-
transforms,
766-
cellTransforms: [
767-
(data: IFormatterValueType) => {
768-
const val = data?.title ? data.title : data;
769-
return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
770-
},
771-
],
772-
},
773-
];
774-
rows = [[result?.[1]]];
775-
} else {
776-
const allLabelKeys = _.uniq(_.flatMap(result, ({ metric }) => Object.keys(metric))).sort();
777-
778-
columns = [
779-
'',
780-
...allLabelKeys.map((k) => ({
781-
title: <span>{k === '__name__' ? t('Name') : k}</span>,
782-
transforms,
783-
})),
784-
{
785-
title: t('Value'),
786-
transforms,
787-
cellTransforms: [
788-
(data: IFormatterValueType) => {
789-
const val = data?.title ? data.title : data;
790-
return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
791-
},
792-
],
793-
},
794-
];
736+
const buttonCell = (labels) => ({ title: <SeriesButton index={index} labels={labels} /> });
795737

796-
let rowMapper;
797-
if (data.resultType === 'matrix') {
798-
rowMapper = ({ metric, values }) => [
738+
let columns, rows;
739+
if (data.resultType === 'scalar') {
740+
columns = [
799741
'',
800-
..._.map(allLabelKeys, (k) => metric[k]),
801742
{
802-
title: (
803-
<>
804-
{_.map(values, ([time, v]) => (
805-
<div key={time}>
806-
{v}&nbsp;@{time}
807-
</div>
808-
))}
809-
</>
810-
),
743+
title: t('Value'),
744+
transforms,
745+
cellTransforms: [
746+
(data: IFormatterValueType) => {
747+
const val = data?.title ? data.title : data;
748+
return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
749+
},
750+
],
751+
},
752+
];
753+
rows = [[buttonCell({}), _.get(result, '[1]')]];
754+
} else if (data.resultType === 'string') {
755+
columns = [
756+
{
757+
title: t('Value'),
758+
transforms,
759+
cellTransforms: [
760+
(data: IFormatterValueType) => {
761+
const val = data?.title ? data.title : data;
762+
return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
763+
},
764+
],
811765
},
812766
];
767+
rows = [[result?.[1]]];
813768
} else {
814-
rowMapper = ({ metric, value }) => [
815-
buttonCell(metric),
816-
..._.map(allLabelKeys, (k) => metric[k]),
817-
_.get(value, '[1]', { title: <span>{t('None')}</span> }),
769+
const allLabelKeys = _.uniq(_.flatMap(result, ({ metric }) => Object.keys(metric))).sort();
770+
771+
columns = [
772+
'',
773+
...allLabelKeys.map((k) => ({
774+
title: <span>{k === '__name__' ? t('Name') : k}</span>,
775+
transforms,
776+
})),
777+
{
778+
title: t('Value'),
779+
transforms,
780+
cellTransforms: [
781+
(data: IFormatterValueType) => {
782+
const val = data?.title ? data.title : data;
783+
return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
784+
},
785+
],
786+
},
818787
];
819-
}
820788

821-
rows = _.map(result, rowMapper);
822-
if (sortBy) {
823-
// Sort Values column numerically and sort all the other columns alphabetically
824-
const valuesColIndex = allLabelKeys.length + 1;
825-
const sort =
826-
sortBy.index === valuesColIndex
827-
? (cells) => {
828-
const v = Number(cells[valuesColIndex]);
829-
return Number.isNaN(v) ? 0 : v;
830-
}
831-
: `${sortBy.index}`;
832-
rows = _.orderBy(rows, [sort], [sortBy.direction]);
789+
let rowMapper;
790+
if (data.resultType === 'matrix') {
791+
rowMapper = ({ metric, values }) => [
792+
'',
793+
..._.map(allLabelKeys, (k) => metric[k]),
794+
{
795+
title: (
796+
<>
797+
{_.map(values, ([time, v]) => (
798+
<div key={time}>
799+
{v}&nbsp;@{time}
800+
</div>
801+
))}
802+
</>
803+
),
804+
},
805+
];
806+
} else {
807+
rowMapper = ({ metric, value }) => [
808+
buttonCell(metric),
809+
..._.map(allLabelKeys, (k) => metric[k]),
810+
_.get(value, '[1]', { title: <span>{t('None')}</span> }),
811+
];
812+
}
813+
814+
rows = _.map(result, rowMapper);
815+
if (sortBy) {
816+
// Sort Values column numerically and sort all the other columns alphabetically
817+
const valuesColIndex = allLabelKeys.length + 1;
818+
const sort =
819+
sortBy.index === valuesColIndex
820+
? (cells) => {
821+
const v = Number(cells[valuesColIndex]);
822+
return Number.isNaN(v) ? 0 : v;
823+
}
824+
: `${sortBy.index}`;
825+
rows = _.orderBy(rows, [sort], [sortBy.direction]);
826+
}
833827
}
834-
}
835828

836-
// Dispatch query table result so QueryKebab can access it for data export
837-
dispatch(queryBrowserPatchQuery(index, { queryTableData: { columns, rows } }));
829+
const onSort = (e, i, direction) => setSortBy({ index: i, direction });
838830

839-
const onSort = (e, i, direction) => setSortBy({ index: i, direction });
831+
const tableRows = rows.slice((page - 1) * perPage, page * perPage).map((cells) => ({ cells }));
840832

841-
const tableRows = rows.slice((page - 1) * perPage, page * perPage).map((cells) => ({ cells }));
833+
return {
834+
onSort,
835+
tableRows,
836+
columns,
837+
rows,
838+
};
839+
}, [
840+
data?.resultType,
841+
isEmptyGraph,
842+
index,
843+
isUnused,
844+
isError,
845+
isLoading,
846+
page,
847+
perPage,
848+
result,
849+
sortBy,
850+
t,
851+
valueFormat,
852+
]);
853+
854+
useEffect(() => {
855+
if (tableData.columns && tableData.rows) {
856+
dispatch(
857+
queryBrowserPatchQuery(index, {
858+
queryTableData: { columns: tableData.columns, rows: tableData.rows },
859+
}),
860+
);
861+
}
862+
}, [dispatch, index, tableData?.columns, tableData?.rows]);
863+
864+
if (isUnused) {
865+
return null;
866+
} else if (isError) {
867+
return <Error error={error} title={t('Error loading values')} />;
868+
} else if (isLoading) {
869+
return <LoadingInline />;
870+
} else if (isEmptyGraph) {
871+
return (
872+
<div data-test={DataTestIDs.MetricsPageYellowNoDatapointsFound}>
873+
<YellowExclamationTriangleIcon /> {t('No datapoints found.')}
874+
</div>
875+
);
876+
}
842877

843878
return (
844879
<>
@@ -854,19 +889,19 @@ export const QueryTable: FC<QueryTableProps> = ({ index, namespace, customDataso
854889
<Table
855890
aria-label={t('query results table')}
856891
gridBreakPoint={TableGridBreakpoint.none}
857-
rows={tableRows.length}
892+
rows={tableData?.tableRows.length}
858893
variant={TableVariant.compact}
859894
data-test={DataTestIDs.MetricsPageQueryTable}
860895
>
861896
<Thead>
862897
<Tr>
863-
{columns.map((col, columnIndex) => {
898+
{tableData?.columns.map((col, columnIndex) => {
864899
const sortParams =
865900
columnIndex !== 0
866901
? {
867902
sort: {
868903
sortBy,
869-
onSort,
904+
onSort: tableData?.onSort,
870905
columnIndex,
871906
},
872907
}
@@ -880,15 +915,15 @@ export const QueryTable: FC<QueryTableProps> = ({ index, namespace, customDataso
880915
</Tr>
881916
</Thead>
882917
<Tbody>
883-
{tableRows.map((row, rowIndex) => (
918+
{tableData?.tableRows.map((row, rowIndex) => (
884919
<Tr key={`row-${rowIndex}`}>
885920
{row.cells?.map((cell, cellIndex) => (
886921
<Td
887922
style={{ fontFamily: t_global_font_family_mono.var }}
888923
key={`cell-${rowIndex}-${cellIndex}`}
889924
>
890-
{columns[cellIndex].cellTransforms
891-
? columns[cellIndex].cellTransforms[0](
925+
{tableData?.columns[cellIndex].cellTransforms
926+
? tableData?.columns[cellIndex].cellTransforms[0](
892927
typeof cell === 'string' ? cell : cell?.title,
893928
)
894929
: typeof cell === 'string'
@@ -902,7 +937,7 @@ export const QueryTable: FC<QueryTableProps> = ({ index, namespace, customDataso
902937
</Table>
903938
</InnerScrollContainer>
904939
<TablePagination
905-
itemCount={rows.length}
940+
itemCount={tableData?.rows.length}
906941
page={page}
907942
perPage={perPage}
908943
setPage={setPage}

0 commit comments

Comments
 (0)