Skip to content

Commit b2089fa

Browse files
authored
fix: Prevent dashboard error when metricName is defined for non-metric source (#1649)
Closes HDX-3236 # Summary This PR fixes an error that occurs when a metricName/metricType is set for a dashboard tile configuration, despite the queried source not being a metric source. 1. Updates in DBEditTimeChartForm prevent us from saving configurations with metricName/metricType for non metric sources 2. Updates in DBDashboardPage ensure that metricName/metricType is ignored for any saved configurations for non-metric sources. ## Demo A new tile would be saved with a metricName/Type incorrectly when 1. Create the tile 2. Select a metric source 3. Select a metric name 4. Switch back to a non-metric source 5. Save And the Dashboard tile would then error: <img width="1288" height="1012" alt="Screenshot 2026-01-23 at 2 39 38 PM" src="https://github.com/user-attachments/assets/4fa4b0bf-355e-47bb-a504-cd03e0dca2d0" /> Now, the configuration is not saved with metricName/Type, and the dashboard does not error for a saved configuration that has a metricName/Type: <img width="769" height="423" alt="Screenshot 2026-01-23 at 2 43 04 PM" src="https://github.com/user-attachments/assets/92af36aa-dd46-47b8-ae59-d0e4bfcb28af" />
1 parent cf3ebb4 commit b2089fa

3 files changed

Lines changed: 57 additions & 25 deletions

File tree

‎.changeset/short-camels-hug.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hyperdx/app": patch
3+
---
4+
5+
fix: Prevent dashboard error when metricName is defined for non-metric source

‎packages/app/src/DBDashboardPage.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { convertToDashboardTemplate } from '@hyperdx/common-utils/dist/core/util
2020
import {
2121
AlertState,
2222
DashboardFilter,
23+
SourceKind,
2324
TSourceUnion,
2425
} from '@hyperdx/common-utils/dist/types';
2526
import {
@@ -182,10 +183,14 @@ const Tile = forwardRef(
182183

183184
useEffect(() => {
184185
if (source != null) {
186+
const isMetricSource = source.kind === SourceKind.Metric;
187+
185188
// TODO: will need to update this when we allow for multiple metrics per chart
186189
const firstSelect = chart.config.select[0];
187190
const metricType =
188-
typeof firstSelect !== 'string' ? firstSelect?.metricType : undefined;
191+
isMetricSource && typeof firstSelect !== 'string'
192+
? firstSelect?.metricType
193+
: undefined;
189194
const tableName = getMetricTableName(source, metricType);
190195
if (source.connection) {
191196
setQueriedConfig({
@@ -200,7 +205,7 @@ const Tile = forwardRef(
200205
},
201206
implicitColumnExpression: source.implicitColumnExpression,
202207
filters,
203-
metricTables: source.metricTables,
208+
metricTables: isMetricSource ? source.metricTables : undefined,
204209
});
205210
}
206211
}

‎packages/app/src/components/DBEditTimeChartForm.tsx‎

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,29 @@ const getSeriesFieldPath = (
135135
return `${namePrefix}${fieldName}` as FieldPath<SavedChartConfigWithSeries>;
136136
};
137137

138+
export function normalizeChartConfig<
139+
C extends Pick<
140+
SavedChartConfig,
141+
'select' | 'having' | 'orderBy' | 'displayType' | 'metricTables'
142+
>,
143+
>(config: C, source: TSource): C {
144+
const isMetricSource = source.kind === SourceKind.Metric;
145+
return {
146+
...config,
147+
// Strip out metric-specific fields for non-metric sources
148+
select:
149+
!isMetricSource && Array.isArray(config.select)
150+
? config.select.map(s => omit(s, ['metricName', 'metricType']))
151+
: config.select,
152+
metricTables: isMetricSource ? config.metricTables : undefined,
153+
// Order By and Having can only be set by the user for table charts
154+
having:
155+
config.displayType === DisplayType.Table ? config.having : undefined,
156+
orderBy:
157+
config.displayType === DisplayType.Table ? config.orderBy : undefined,
158+
};
159+
}
160+
138161
// Helper function to validate metric names for metric sources
139162
const validateMetricNames = (
140163
tableSource: TSource | undefined,
@@ -684,19 +707,13 @@ export default function EditTimeChartForm({
684707
select: isSelectEmpty
685708
? tableSource.defaultTableSelectExpression || ''
686709
: config.select,
687-
// Order By and Having can only be set by the user for table charts
688-
having: config.displayType === DisplayType.Table ? config.having : '',
689-
orderBy:
690-
config.displayType === DisplayType.Table
691-
? config.orderBy
692-
: undefined,
693710
};
694711
setQueriedConfigAndSource(
695712
// WARNING: DON'T JUST ASSIGN OBJECTS OR DO SPREAD OPERATOR STUFF WHEN
696713
// YOUR STATE IS AN OBJECT. YOU'RE COPYING BY REFERENCE WHICH MIGHT
697714
// ACCIDENTALLY CAUSE A useQuery SOMEWHERE TO FIRE A REQUEST EVERY TIME
698715
// AN INPUT CHANGES. USE structuredClone TO PERFORM A DEEP COPY INSTEAD
699-
structuredClone(newConfig),
716+
structuredClone(normalizeChartConfig(newConfig, tableSource)),
700717
tableSource,
701718
);
702719
}
@@ -734,25 +751,30 @@ export default function EditTimeChartForm({
734751

735752
const handleSave = useCallback(
736753
(v: SavedChartConfigWithSeries) => {
737-
// Validate metric sources have metric names selected
738-
if (validateMetricNames(tableSource, v.series, setError)) {
739-
return;
740-
}
754+
if (tableSource != null) {
755+
// Validate metric sources have metric names selected
756+
if (validateMetricNames(tableSource, v.series, setError)) {
757+
return;
758+
}
741759

742-
// If the chart type is search, we need to ensure the select is a string
743-
if (displayType === DisplayType.Search && typeof v.select !== 'string') {
744-
v.select = '';
745-
} else if (displayType !== DisplayType.Search) {
746-
v.select = v.series;
747-
}
760+
// If the chart type is search, we need to ensure the select is a string
761+
if (
762+
displayType === DisplayType.Search &&
763+
typeof v.select !== 'string'
764+
) {
765+
v.select = '';
766+
} else if (displayType !== DisplayType.Search) {
767+
v.select = v.series;
768+
}
748769

749-
// If the chart is not a table chart, we should clear the having clause
750-
if (displayType !== DisplayType.Table) {
751-
v.having = undefined;
752-
}
770+
const normalizedChartConfig = normalizeChartConfig(
771+
// Avoid saving the series field. Series should be persisted in the select field.
772+
omit(v, ['series']),
773+
tableSource,
774+
);
753775

754-
// Avoid saving the series field. Series should be persisted in the select field.
755-
onSave?.(omit(v, ['series']));
776+
onSave?.(normalizedChartConfig);
777+
}
756778
},
757779
[onSave, displayType, tableSource, setError],
758780
);

0 commit comments

Comments
 (0)