From bcfb9b4695142aeecc1c32966dbf1bb8ec176414 Mon Sep 17 00:00:00 2001 From: vinzee Date: Tue, 5 May 2026 13:39:10 -0700 Subject: [PATCH 1/2] fix: prevent false \"data source not set\" error on markdown dashboard tiles Markdown tiles created via the external API or MCP always showed \"The data source for this tile is not set. Edit the tile to select a data source.\" in the dashboard UI, despite not requiring a source. Root cause: convertToInternalTileConfig stores source: '' on the internal config to satisfy BuilderSavedChartConfig's required source field. The frontend's isSourceUnset check used !chart.config.source, which evaluates to true for an empty string, incorrectly triggering the error banner for every API-created markdown tile. Fix: add chart.config.displayType !== DisplayType.Markdown to the isSourceUnset guard in DBDashboardPage so markdown tiles are never considered to have an unset source. Tests added: - packages/common-utils: unit tests for isBuilderSavedChartConfig and the isSourceUnset pattern, verifying markdown tiles are excluded - packages/api: unit tests for convertToInternalTileConfig covering the markdown tile conversion and layout field preservation Co-authored-by: Cursor --- .changeset/beige-parents-lick.md | 6 + .../v2/utils/__tests__/dashboards.test.ts | 75 ++++++++ packages/app/src/DBDashboardPage.tsx | 2 + .../common-utils/src/__tests__/guards.test.ts | 170 ++++++++++++++++++ packages/common-utils/src/guards.ts | 13 ++ 5 files changed, 266 insertions(+) create mode 100644 .changeset/beige-parents-lick.md create mode 100644 packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts create mode 100644 packages/common-utils/src/__tests__/guards.test.ts diff --git a/.changeset/beige-parents-lick.md b/.changeset/beige-parents-lick.md new file mode 100644 index 0000000000..e2fd26d144 --- /dev/null +++ b/.changeset/beige-parents-lick.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/app': patch +'@hyperdx/common-utils': patch +--- + +fix: prevent false "data source not set" error on markdown dashboard tiles diff --git a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts new file mode 100644 index 0000000000..aff2119059 --- /dev/null +++ b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts @@ -0,0 +1,75 @@ +import { isBuilderSavedChartConfig } from '@hyperdx/common-utils/dist/guards'; +import { DisplayType } from '@hyperdx/common-utils/dist/types'; + +import { ConfigTile, convertToInternalTileConfig } from '../dashboards'; + +function makeMarkdownTile( + markdown?: string, + overrides: Partial = {}, +): ConfigTile { + return { + id: 'test-id', + x: 0, + y: 0, + w: 12, + h: 4, + name: 'Text Tile', + config: { displayType: 'markdown', markdown }, + ...overrides, + }; +} + +describe('convertToInternalTileConfig', () => { + describe('markdown tiles', () => { + it('sets displayType to Markdown', () => { + const result = convertToInternalTileConfig(makeMarkdownTile('# Hello')); + expect(result.config).toMatchObject({ + displayType: DisplayType.Markdown, + }); + }); + + it('preserves markdown content', () => { + const content = '## Guide\n\nSome instructions here.'; + const result = convertToInternalTileConfig(makeMarkdownTile(content)); + if (!isBuilderSavedChartConfig(result.config)) { + throw new Error('Expected BuilderSavedChartConfig for markdown tile'); + } + expect(result.config.markdown).toBe(content); + }); + + it('handles missing markdown content', () => { + const result = convertToInternalTileConfig(makeMarkdownTile(undefined)); + expect(result.config).toMatchObject({ + displayType: DisplayType.Markdown, + }); + }); + + it('preserves tile layout fields', () => { + const tile = makeMarkdownTile('Some text', { + x: 4, + y: 8, + w: 24, + h: 3, + name: 'How to Use', + }); + const result = convertToInternalTileConfig(tile); + expect(result.x).toBe(4); + expect(result.y).toBe(8); + expect(result.w).toBe(24); + expect(result.h).toBe(3); + expect(result.name).toBe('How to Use'); + }); + + it('does not set a real sourceId on the internal config', () => { + // Markdown tiles have source: '' in the internal format to satisfy the + // BuilderSavedChartConfig type. The frontend uses displayTypeRequiresSource() + // to exclude markdown tiles from "source unset" checks. + // See DBDashboardPage isSourceUnset and common-utils/src/guards.ts. + const result = convertToInternalTileConfig(makeMarkdownTile('# Hello')); + if (!isBuilderSavedChartConfig(result.config)) { + throw new Error('Expected BuilderSavedChartConfig for markdown tile'); + } + expect(result.config.source).not.toMatch(/^[a-f0-9]{24}$/); // not a real ObjectId + }); + }); +}); diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index e955dba243..286a51e133 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -25,6 +25,7 @@ import { displayTypeSupportsRawSqlAlerts, } from '@hyperdx/common-utils/dist/core/utils'; import { + displayTypeRequiresSource, isBuilderChartConfig, isBuilderSavedChartConfig, isRawSqlChartConfig, @@ -397,6 +398,7 @@ const Tile = forwardRef( const isSourceUnset = !!chart.config && isBuilderSavedChartConfig(chart.config) && + displayTypeRequiresSource(chart.config.displayType) && !chart.config.source; useEffect(() => { diff --git a/packages/common-utils/src/__tests__/guards.test.ts b/packages/common-utils/src/__tests__/guards.test.ts new file mode 100644 index 0000000000..166465aee1 --- /dev/null +++ b/packages/common-utils/src/__tests__/guards.test.ts @@ -0,0 +1,170 @@ +import { + displayTypeRequiresSource, + isBuilderSavedChartConfig, + isRawSqlSavedChartConfig, +} from '@/guards'; +import { DisplayType } from '@/types'; + +describe('isRawSqlSavedChartConfig', () => { + it('returns true when configType is "sql"', () => { + expect( + isRawSqlSavedChartConfig({ + configType: 'sql', + displayType: DisplayType.Table, + connection: 'conn-id', + sqlTemplate: 'SELECT 1', + } as any), + ).toBe(true); + }); + + it('returns false for a builder config with no configType', () => { + expect( + isRawSqlSavedChartConfig({ + displayType: DisplayType.Line, + source: 'src-id', + select: [], + where: '', + } as any), + ).toBe(false); + }); + + it('returns false for a markdown config', () => { + expect( + isRawSqlSavedChartConfig({ + displayType: DisplayType.Markdown, + markdown: '# Hello', + source: '', + where: '', + select: [], + } as any), + ).toBe(false); + }); +}); + +describe('isBuilderSavedChartConfig', () => { + it('returns true for a standard builder config', () => { + expect( + isBuilderSavedChartConfig({ + displayType: DisplayType.Line, + source: 'src-id', + select: [], + where: '', + } as any), + ).toBe(true); + }); + + it('returns false for a raw SQL config', () => { + expect( + isBuilderSavedChartConfig({ + configType: 'sql', + displayType: DisplayType.Table, + connection: 'conn-id', + sqlTemplate: 'SELECT 1', + } as any), + ).toBe(false); + }); + + it('returns true for a markdown tile config', () => { + // Markdown tiles are stored as BuilderSavedChartConfig with source: '' + // because the type requires a source field. displayTypeRequiresSource() + // is the canonical guard for excluding sourceless display types from the + // "source unset" check -- see DBDashboardPage isSourceUnset. + expect( + isBuilderSavedChartConfig({ + displayType: DisplayType.Markdown, + markdown: '# Hello', + source: '', + where: '', + select: [], + } as any), + ).toBe(true); + }); +}); + +describe('displayTypeRequiresSource', () => { + it('returns false for Markdown', () => { + expect(displayTypeRequiresSource(DisplayType.Markdown)).toBe(false); + }); + + it('returns true for Line', () => { + expect(displayTypeRequiresSource(DisplayType.Line)).toBe(true); + }); + + it('returns true for Table', () => { + expect(displayTypeRequiresSource(DisplayType.Table)).toBe(true); + }); + + it('returns true for Search', () => { + expect(displayTypeRequiresSource(DisplayType.Search)).toBe(true); + }); + + it('returns true for StackedBar', () => { + expect(displayTypeRequiresSource(DisplayType.StackedBar)).toBe(true); + }); + + it('returns true for Pie', () => { + expect(displayTypeRequiresSource(DisplayType.Pie)).toBe(true); + }); + + it('returns true for undefined', () => { + expect(displayTypeRequiresSource(undefined)).toBe(true); + }); + + describe('isSourceUnset pattern', () => { + // These tests exercise the production isSourceUnset condition used in + // DBDashboardPage via the real displayTypeRequiresSource export. + function isSourceUnset(config: any): boolean { + return ( + !!config && + isBuilderSavedChartConfig(config) && + displayTypeRequiresSource(config.displayType) && + !config.source + ); + } + + it('does not fire for markdown tiles with empty source', () => { + expect( + isSourceUnset({ + displayType: DisplayType.Markdown, + markdown: '# Hello', + source: '', // stored as '' by convertToInternalTileConfig + where: '', + select: [], + }), + ).toBe(false); + }); + + it('fires for builder tiles with empty source', () => { + expect( + isSourceUnset({ + displayType: DisplayType.Line, + source: '', + select: [], + where: '', + }), + ).toBe(true); + }); + + it('does not fire for builder tiles with a real source', () => { + expect( + isSourceUnset({ + displayType: DisplayType.Line, + source: 'abc123', + select: [], + where: '', + }), + ).toBe(false); + }); + + it('does not fire for raw SQL tiles', () => { + expect( + isSourceUnset({ + configType: 'sql', + displayType: DisplayType.Table, + connection: 'conn-id', + sqlTemplate: 'SELECT 1', + }), + ).toBe(false); + }); + }); +}); diff --git a/packages/common-utils/src/guards.ts b/packages/common-utils/src/guards.ts index 62621db5d0..8efc810923 100644 --- a/packages/common-utils/src/guards.ts +++ b/packages/common-utils/src/guards.ts @@ -3,6 +3,7 @@ import { BuilderSavedChartConfig, ChartConfig, ChartConfigWithOptDateRange, + DisplayType, RawSqlChartConfig, RawSqlSavedChartConfig, SavedChartConfig, @@ -31,3 +32,15 @@ export function isBuilderSavedChartConfig( ): chartConfig is BuilderSavedChartConfig { return !isRawSqlSavedChartConfig(chartConfig); } + +/** + * Returns true when a display type semantically requires a data source to be + * configured. Currently Markdown is the only display type that does not need a + * source (it renders static content). Add any future sourceless display types + * here rather than scattering per-type checks across the codebase. + */ +export function displayTypeRequiresSource( + displayType: DisplayType | undefined, +): boolean { + return displayType !== DisplayType.Markdown; +} From ed420bc16d42001735e206c997b4cb1bd1cff611 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 6 May 2026 16:40:07 -0600 Subject: [PATCH 2/2] fix: use toHaveProperty for untyped tile name assertion in test --- .../external-api/v2/utils/__tests__/dashboards.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts index aff2119059..46e40ee481 100644 --- a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts +++ b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts @@ -57,7 +57,9 @@ describe('convertToInternalTileConfig', () => { expect(result.y).toBe(8); expect(result.w).toBe(24); expect(result.h).toBe(3); - expect(result.name).toBe('How to Use'); + // name is picked by convertToInternalTileConfig but not part of the + // Tile type — verify it round-trips via the runtime object. + expect(result).toHaveProperty('name', 'How to Use'); }); it('does not set a real sourceId on the internal config', () => {