Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/beige-parents-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@hyperdx/app': patch
'@hyperdx/common-utils': patch
---

fix: prevent false "data source not set" error on markdown dashboard tiles
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
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> = {},
): 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);
// 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', () => {
// 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
});
});
});
2 changes: 2 additions & 0 deletions packages/app/src/DBDashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
displayTypeSupportsRawSqlAlerts,
} from '@hyperdx/common-utils/dist/core/utils';
import {
displayTypeRequiresSource,
isBuilderChartConfig,
isBuilderSavedChartConfig,
isRawSqlChartConfig,
Expand Down Expand Up @@ -397,6 +398,7 @@ const Tile = forwardRef(
const isSourceUnset =
!!chart.config &&
isBuilderSavedChartConfig(chart.config) &&
displayTypeRequiresSource(chart.config.displayType) &&
!chart.config.source;

useEffect(() => {
Expand Down
170 changes: 170 additions & 0 deletions packages/common-utils/src/__tests__/guards.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
13 changes: 13 additions & 0 deletions packages/common-utils/src/guards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
BuilderSavedChartConfig,
ChartConfig,
ChartConfigWithOptDateRange,
DisplayType,
RawSqlChartConfig,
RawSqlSavedChartConfig,
SavedChartConfig,
Expand Down Expand Up @@ -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;
}
Loading