From 94c616eddc10bbbc8006f2ca90c90738b84a6aca Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Mon, 20 Apr 2026 10:09:06 -0400 Subject: [PATCH] fix: Prevent duplicate tile IDs in dashboard imports --- .changeset/khaki-shoes-cough.md | 7 ++ packages/api/src/utils/zod.ts | 20 ++---- packages/app/src/DBDashboardImportPage.tsx | 40 ++++++++--- .../dashboard-template-import.spec.ts | 67 +++++++++++++++++++ .../e2e/page-objects/DashboardImportPage.ts | 16 +++++ .../common-utils/src/__tests__/utils.test.ts | 51 ++++++++++++++ packages/common-utils/src/types.ts | 23 ++++++- 7 files changed, 198 insertions(+), 26 deletions(-) create mode 100644 .changeset/khaki-shoes-cough.md diff --git a/.changeset/khaki-shoes-cough.md b/.changeset/khaki-shoes-cough.md new file mode 100644 index 00000000..ecec853e --- /dev/null +++ b/.changeset/khaki-shoes-cough.md @@ -0,0 +1,7 @@ +--- +"@hyperdx/common-utils": patch +"@hyperdx/api": patch +"@hyperdx/app": patch +--- + +fix: Prevent duplicate tile IDs in dashboard imports diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index bd9c1813..5eba7d5e 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -1,4 +1,5 @@ import { + addDuplicateTileIdIssues, AggregateFunctionSchema, AlertThresholdType, DashboardFilterSchema, @@ -469,20 +470,11 @@ export type ExternalDashboardTileWithId = z.infer< export const externalDashboardTileListSchema = z .array(externalDashboardTileSchemaWithOptionalId) - .superRefine((tiles, ctx) => { - const seen = new Set(); - for (const tile of tiles) { - if (tile.id && seen.has(tile.id)) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `Duplicate tile ID: ${tile.id}. Omit the ID to generate a unique one.`, - }); - } - if (tile.id) { - seen.add(tile.id); - } - } - }); + .superRefine((tiles, ctx) => + addDuplicateTileIdIssues(tiles, ctx, { + messageSuffix: '. Omit the ID to generate a unique one.', + }), + ); // ============================== // Alerts diff --git a/packages/app/src/DBDashboardImportPage.tsx b/packages/app/src/DBDashboardImportPage.tsx index 0080ba29..db92795b 100644 --- a/packages/app/src/DBDashboardImportPage.tsx +++ b/packages/app/src/DBDashboardImportPage.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from 'react'; +import { type ReactNode, useEffect, useMemo, useRef, useState } from 'react'; import dynamic from 'next/dynamic'; import Head from 'next/head'; import Link from 'next/link'; @@ -59,7 +59,7 @@ function FileSelection({ const [error, setError] = useState<{ message: string; - details?: string; + details?: ReactNode; } | null>(null); const [errorDetails, { toggle: toggleErrorDetails }] = useDisclosure(false); @@ -75,18 +75,38 @@ function FileSelection({ setError(null); if (!file) return; + let data: unknown; try { const text = await file.text(); - const data = JSON.parse(text); - const parsed = DashboardTemplateSchema.parse(data); // throws if invalid - onComplete(parsed); - } catch (e: any) { + data = JSON.parse(text); + } catch (e: unknown) { + onComplete(null); + setError({ + message: 'Invalid JSON File', + details: e instanceof Error ? e.message : 'Failed to parse JSON', + }); + return; + } + + const result = DashboardTemplateSchema.safeParse(data); + if (!result.success) { onComplete(null); setError({ message: 'Failed to Import Dashboard', - details: e?.message ?? 'Failed to parse/validate JSON', + details: ( + + {result.error.issues.map(issue => ( + + {issue.message} + + ))} + + ), }); + return; } + + onComplete(result.data); }; return ( @@ -153,7 +173,7 @@ function FileSelection({ {error && (
{error.message} - {error.details && ( + {error.details != null && ( <> - - {error.details} - + {error.details} )}
diff --git a/packages/app/tests/e2e/features/dashboard-template-import.spec.ts b/packages/app/tests/e2e/features/dashboard-template-import.spec.ts index 83ec6e28..6ce9dc0e 100644 --- a/packages/app/tests/e2e/features/dashboard-template-import.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-template-import.spec.ts @@ -84,6 +84,73 @@ test.describe('Dashboard Template Import', { tag: ['@dashboard'] }, () => { }, ); + test( + 'should show a friendly error when the imported file has duplicate tile IDs', + { tag: '@full-stack' }, + async ({ page }) => { + const duplicateId = 'duplicate-tile-id'; + const dashboardFile = { + version: '0.1.0', + name: 'Duplicate Tile Dashboard', + tiles: [ + { + id: duplicateId, + x: 0, + y: 0, + w: 6, + h: 4, + config: { + name: 'Tile A', + source: 'Logs', + displayType: 'number', + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + whereLanguage: 'sql', + }, + }, + { + id: duplicateId, + x: 6, + y: 0, + w: 6, + h: 4, + config: { + name: 'Tile B', + source: 'Logs', + displayType: 'number', + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + whereLanguage: 'sql', + }, + }, + ], + }; + + await test.step('Navigate to the dashboard file import page', async () => { + await dashboardImportPage.gotoImport(); + await expect(dashboardImportPage.fileUploadDropzone).toBeVisible(); + }); + + await test.step('Upload a dashboard file with duplicate tile IDs', async () => { + await dashboardImportPage.uploadDashboardFile( + JSON.stringify(dashboardFile), + ); + }); + + await test.step('Verify the import error is shown and the mapping step is not reached', async () => { + await expect(dashboardImportPage.importErrorTitle).toBeVisible(); + await expect(dashboardImportPage.mappingStepHeading).toBeHidden(); + }); + + await test.step('Expand error details and verify the duplicate tile ID is reported', async () => { + await dashboardImportPage.showErrorDetailsButton.click(); + await expect( + page.getByText(`Duplicate tile ID: ${duplicateId}`), + ).toBeVisible(); + }); + }, + ); + test( 'should show error for invalid template name', { tag: '@full-stack' }, diff --git a/packages/app/tests/e2e/page-objects/DashboardImportPage.ts b/packages/app/tests/e2e/page-objects/DashboardImportPage.ts index 6a980adf..26b8b9af 100644 --- a/packages/app/tests/e2e/page-objects/DashboardImportPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardImportPage.ts @@ -11,8 +11,11 @@ export class DashboardImportPage { readonly dashboardNameInput: Locator; readonly finishImportButton: Locator; readonly fileUploadDropzone: Locator; + readonly fileUploadInput: Locator; readonly templateNotFoundText: Locator; readonly browseAvailableTemplatesLink: Locator; + readonly importErrorTitle: Locator; + readonly showErrorDetailsButton: Locator; constructor(page: Page) { this.page = page; @@ -25,12 +28,25 @@ export class DashboardImportPage { this.fileUploadDropzone = page.getByText('Drag and drop a JSON file here', { exact: false, }); + this.fileUploadInput = page.locator('input[type="file"]'); this.templateNotFoundText = page.getByText( "Oops! We couldn't find that template.", ); this.browseAvailableTemplatesLink = page.getByRole('link', { name: 'browsing available templates', }); + this.importErrorTitle = page.getByText('Failed to Import Dashboard'); + this.showErrorDetailsButton = page.getByRole('button', { + name: 'Show Details', + }); + } + + async uploadDashboardFile(content: string, filename = 'dashboard.json') { + await this.fileUploadInput.setInputFiles({ + name: filename, + mimeType: 'application/json', + buffer: Buffer.from(content), + }); } async gotoTemplates() { diff --git a/packages/common-utils/src/__tests__/utils.test.ts b/packages/common-utils/src/__tests__/utils.test.ts index bf20075f..ec4a583b 100644 --- a/packages/common-utils/src/__tests__/utils.test.ts +++ b/packages/common-utils/src/__tests__/utils.test.ts @@ -5,6 +5,7 @@ import { BuilderChartConfigWithDateRange, Connection, DashboardSchema, + DashboardTemplateSchema, MetricsDataType, SourceKind, TSource, @@ -933,6 +934,56 @@ describe('utils', () => { }); }); + describe('DashboardTemplateSchema duplicate tile IDs', () => { + const makeTemplateTile = (id: string) => ({ + id, + x: 0, + y: 0, + w: 6, + h: 4, + config: { + name: `Tile ${id}`, + source: 'source1', + displayType: 'number', + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + whereLanguage: 'sql', + }, + }); + + it('accepts tiles with unique IDs', () => { + const template = { + version: '0.1.0', + name: 'Unique Tiles', + tiles: [ + makeTemplateTile('tile-a'), + makeTemplateTile('tile-b'), + makeTemplateTile('tile-c'), + ], + }; + expect(() => DashboardTemplateSchema.parse(template)).not.toThrow(); + }); + + it('rejects tiles with duplicate IDs and surfaces the duplicate ID', () => { + const template = { + version: '0.1.0', + name: 'Duplicate Tiles', + tiles: [ + makeTemplateTile('dup'), + makeTemplateTile('unique'), + makeTemplateTile('dup'), + ], + }; + + const result = DashboardTemplateSchema.safeParse(template); + expect(result.success).toBe(false); + if (!result.success) { + const messages = result.error.issues.map(i => i.message); + expect(messages).toContain('Duplicate tile ID: dup'); + } + }); + }); + describe('isJsonExpression', () => { it('should return false for expressions without dots', () => { expect(isJsonExpression('col')).toBe(false); diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 5d7d4834..f0a27b8b 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -910,6 +910,27 @@ export const PresetDashboardFilterSchema = DashboardFilterSchema.extend({ export type PresetDashboardFilter = z.infer; +export function addDuplicateTileIdIssues( + tiles: { id?: string }[], + ctx: z.RefinementCtx, + options?: { messageSuffix?: string }, +) { + const suffix = options?.messageSuffix ?? ''; + const seen = new Set(); + for (let i = 0; i < tiles.length; i++) { + const id = tiles[i].id; + if (!id) continue; + if (seen.has(id)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `Duplicate tile ID: ${id}${suffix}`, + path: [i, 'id'], + }); + } + seen.add(id); + } +} + export const DashboardSchema = z.object({ id: z.string(), name: z.string().min(1), @@ -939,7 +960,7 @@ export const DashboardTemplateSchema = DashboardWithoutIdSchema.omit({ version: z.string().min(1), description: z.string().optional(), tags: z.array(z.string()).optional(), - tiles: z.array(TileTemplateSchema), + tiles: z.array(TileTemplateSchema).superRefine(addDuplicateTileIdIssues), filters: z.array(DashboardFilterSchema).optional(), }); export type DashboardTemplate = z.infer;