diff --git a/.changeset/bright-knives-notice.md b/.changeset/bright-knives-notice.md new file mode 100644 index 0000000000..5c0d552c84 --- /dev/null +++ b/.changeset/bright-knives-notice.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/app': patch +--- + +Add metric column name validation when saving dashboard tiles diff --git a/packages/app/src/components/DBEditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm.tsx index cd80dd0f9f..c2b93f4459 100644 --- a/packages/app/src/components/DBEditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm.tsx @@ -134,6 +134,31 @@ const getSeriesFieldPath = ( return `${namePrefix}${fieldName}` as FieldPath; }; +// Helper function to validate metric names for metric sources +const validateMetricNames = ( + tableSource: TSource | undefined, + series: SavedChartConfigWithSelectArray['select'] | undefined, + setError: ( + name: FieldPath, + error: { type: string; message: string }, + ) => void, +): boolean => { + if (tableSource?.kind === SourceKind.Metric && Array.isArray(series)) { + let hasValidationError = false; + series.forEach((s, index) => { + if (s.metricType && !s.metricName) { + setError(getSeriesFieldPath(`series.${index}.`, 'metricName'), { + type: 'manual', + message: 'Please select a metric name', + }); + hasValidationError = true; + } + }); + return hasValidationError; + } + return false; +}; + const NumberFormatInputControlled = ({ control, onSubmit, @@ -594,22 +619,8 @@ export default function EditTimeChartForm({ const onSubmit = useCallback(() => { handleSubmit(form => { // Validate metric sources have metric names selected - if ( - tableSource?.kind === SourceKind.Metric && - Array.isArray(form.series) - ) { - let hasValidationError = false; - form.series.forEach((series, index) => { - if (series.metricType && !series.metricName) { - setError(getSeriesFieldPath(`series.${index}.`, 'metricName'), { - type: 'manual', - message: 'Please select a metric name', - }); - hasValidationError = true; - } - }); - - if (hasValidationError) return; + if (validateMetricNames(tableSource, form.series, setError)) { + return; } // Merge the series and select fields back together, and prevent the series field from being submitted @@ -682,6 +693,11 @@ export default function EditTimeChartForm({ const handleSave = useCallback( (v: SavedChartConfigWithSeries) => { + // 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 = ''; @@ -691,7 +707,7 @@ export default function EditTimeChartForm({ // Avoid saving the series field. Series should be persisted in the select field. onSave?.(omit(v, ['series'])); }, - [onSave, displayType], + [onSave, displayType, tableSource, setError], ); // Track previous values for detecting changes diff --git a/packages/app/src/components/__tests__/DBEditTimeChartForm.test.tsx b/packages/app/src/components/__tests__/DBEditTimeChartForm.test.tsx index 003b9d36a8..072943a52b 100644 --- a/packages/app/src/components/__tests__/DBEditTimeChartForm.test.tsx +++ b/packages/app/src/components/__tests__/DBEditTimeChartForm.test.tsx @@ -368,3 +368,44 @@ describe('DBEditTimeChartForm - Metric Name Validation', () => { }); }); }); + +describe('DBEditTimeChartForm - Save Button Metric Name Validation', () => { + const renderComponent = (props = {}) => { + return renderWithMantine( + + + , + ); + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should show validation error when clicking save without selecting a metric name', async () => { + const onSave = jest.fn(); + renderComponent({ onSave }); + + // Find and click the save button + const saveButton = screen.getByTestId('chart-save-button'); + await userEvent.click(saveButton); + + // Verify that the validation error is displayed + await waitFor(() => { + const errorMessage = screen.getByTestId('metric-name-error'); + expect(errorMessage).toBeInTheDocument(); + expect(errorMessage).toHaveTextContent('Please select a metric name'); + }); + + // Verify that onSave was not called + expect(onSave).not.toHaveBeenCalled(); + + // Verify that the metric name select has aria-invalid attribute + const metricSelect = screen.getByTestId('metric-name-selector'); + expect(metricSelect).toHaveAttribute('aria-invalid', 'true'); + }); +});