diff --git a/.changeset/short-camels-hug.md b/.changeset/short-camels-hug.md new file mode 100644 index 00000000..1157b896 --- /dev/null +++ b/.changeset/short-camels-hug.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/app": patch +--- + +fix: Prevent dashboard error when metricName is defined for non-metric source diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index 808dc0b3..b2d6fe4f 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -20,6 +20,7 @@ import { convertToDashboardTemplate } from '@hyperdx/common-utils/dist/core/util import { AlertState, DashboardFilter, + SourceKind, TSourceUnion, } from '@hyperdx/common-utils/dist/types'; import { @@ -182,10 +183,14 @@ const Tile = forwardRef( useEffect(() => { if (source != null) { + const isMetricSource = source.kind === SourceKind.Metric; + // TODO: will need to update this when we allow for multiple metrics per chart const firstSelect = chart.config.select[0]; const metricType = - typeof firstSelect !== 'string' ? firstSelect?.metricType : undefined; + isMetricSource && typeof firstSelect !== 'string' + ? firstSelect?.metricType + : undefined; const tableName = getMetricTableName(source, metricType); if (source.connection) { setQueriedConfig({ @@ -200,7 +205,7 @@ const Tile = forwardRef( }, implicitColumnExpression: source.implicitColumnExpression, filters, - metricTables: source.metricTables, + metricTables: isMetricSource ? source.metricTables : undefined, }); } } diff --git a/packages/app/src/components/DBEditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm.tsx index cdd187d9..7b7c8991 100644 --- a/packages/app/src/components/DBEditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm.tsx @@ -135,6 +135,29 @@ const getSeriesFieldPath = ( return `${namePrefix}${fieldName}` as FieldPath; }; +export function normalizeChartConfig< + C extends Pick< + SavedChartConfig, + 'select' | 'having' | 'orderBy' | 'displayType' | 'metricTables' + >, +>(config: C, source: TSource): C { + const isMetricSource = source.kind === SourceKind.Metric; + return { + ...config, + // Strip out metric-specific fields for non-metric sources + select: + !isMetricSource && Array.isArray(config.select) + ? config.select.map(s => omit(s, ['metricName', 'metricType'])) + : config.select, + metricTables: isMetricSource ? config.metricTables : undefined, + // Order By and Having can only be set by the user for table charts + having: + config.displayType === DisplayType.Table ? config.having : undefined, + orderBy: + config.displayType === DisplayType.Table ? config.orderBy : undefined, + }; +} + // Helper function to validate metric names for metric sources const validateMetricNames = ( tableSource: TSource | undefined, @@ -684,19 +707,13 @@ export default function EditTimeChartForm({ select: isSelectEmpty ? tableSource.defaultTableSelectExpression || '' : config.select, - // Order By and Having can only be set by the user for table charts - having: config.displayType === DisplayType.Table ? config.having : '', - orderBy: - config.displayType === DisplayType.Table - ? config.orderBy - : undefined, }; setQueriedConfigAndSource( // WARNING: DON'T JUST ASSIGN OBJECTS OR DO SPREAD OPERATOR STUFF WHEN // YOUR STATE IS AN OBJECT. YOU'RE COPYING BY REFERENCE WHICH MIGHT // ACCIDENTALLY CAUSE A useQuery SOMEWHERE TO FIRE A REQUEST EVERY TIME // AN INPUT CHANGES. USE structuredClone TO PERFORM A DEEP COPY INSTEAD - structuredClone(newConfig), + structuredClone(normalizeChartConfig(newConfig, tableSource)), tableSource, ); } @@ -734,25 +751,30 @@ export default function EditTimeChartForm({ const handleSave = useCallback( (v: SavedChartConfigWithSeries) => { - // Validate metric sources have metric names selected - if (validateMetricNames(tableSource, v.series, setError)) { - return; - } + if (tableSource != null) { + // Validate metric sources have metric names selected + if (validateMetricNames(tableSource, v.series, setError)) { + return; + } - // If the chart type is search, we need to ensure the select is a string - if (displayType === DisplayType.Search && typeof v.select !== 'string') { - v.select = ''; - } else if (displayType !== DisplayType.Search) { - v.select = v.series; - } + // If the chart type is search, we need to ensure the select is a string + if ( + displayType === DisplayType.Search && + typeof v.select !== 'string' + ) { + v.select = ''; + } else if (displayType !== DisplayType.Search) { + v.select = v.series; + } - // If the chart is not a table chart, we should clear the having clause - if (displayType !== DisplayType.Table) { - v.having = undefined; - } + const normalizedChartConfig = normalizeChartConfig( + // Avoid saving the series field. Series should be persisted in the select field. + omit(v, ['series']), + tableSource, + ); - // Avoid saving the series field. Series should be persisted in the select field. - onSave?.(omit(v, ['series'])); + onSave?.(normalizedChartConfig); + } }, [onSave, displayType, tableSource, setError], );