From 883fc97046cbc75c3aa3a23a78403a0188fad005 Mon Sep 17 00:00:00 2001 From: Abhay Padamani <141122205+ktx-abhay@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:34:23 +0530 Subject: [PATCH] fix(dashboard): deduplicate panel and layout IDs on import and dashboard load (#13760) https://github.com/openobserve/openobserve/issues/13104 --- web/src/utils/commons.spec.ts | 82 +++++++++++++++++++ web/src/utils/commons.ts | 48 ++++++++++- .../views/Dashboards/ImportDashboard.spec.ts | 46 ++++++++++- web/src/views/Dashboards/ImportDashboard.vue | 22 +++-- 4 files changed, 184 insertions(+), 14 deletions(-) diff --git a/web/src/utils/commons.spec.ts b/web/src/utils/commons.spec.ts index 032fedb699..dcd705bf62 100644 --- a/web/src/utils/commons.spec.ts +++ b/web/src/utils/commons.spec.ts @@ -935,6 +935,88 @@ describe("Commons Utility Functions", () => { }); }); + describe("getDashboard — duplicate layout ids", () => { + it("reassigns duplicate layout.i per tab, then saves and re-fetches", async () => { + const dashboardId = "dashboard-dup-layout"; + const mockDashboard = { + tabs: [ + { + tabId: "tab-1", + panels: [ + { id: "panel-1", layout: { x: 0, y: 0, w: 24, h: 9, i: 1 } }, + { id: "panel-2", layout: { x: 0, y: 0, w: 24, h: 9, i: 1 } }, + { id: "panel-3", layout: { x: 0, y: 0, w: 24, h: 9, i: 2 } }, + ], + }, + ], + }; + + mockStore.state.organizationData.allDashboardData[dashboardId] = mockDashboard; + (dashboardService.save as any).mockResolvedValue({ data: { success: true } }); + (dashboardService.get_Dashboard as any).mockResolvedValue({ + data: { version: 1, v1: mockDashboard, hash: 123 }, + }); + + await getDashboard(mockStore, dashboardId, "test-folder"); + + const ids = mockDashboard.tabs[0].panels.map((p: any) => p.layout.i); + // first occurrence kept, collisions reassigned from max i (2) upward + expect(ids[0]).toBe(1); + expect(ids[2]).toBe(2); + expect(new Set(ids).size).toBe(ids.length); // all unique + expect(dashboardService.save).toHaveBeenCalled(); + expect(dashboardService.get_Dashboard).toHaveBeenCalled(); + }); + + it("dedupes layout.i independently within each tab", async () => { + const dashboardId = "dashboard-dup-layout-tabs"; + const mockDashboard = { + tabs: [ + { + tabId: "tab-1", + panels: [{ id: "panel-1", layout: { x: 0, y: 0, w: 24, h: 9, i: 1 } }], + }, + { + tabId: "tab-2", + panels: [{ id: "panel-2", layout: { x: 0, y: 0, w: 24, h: 9, i: 1 } }], + }, + ], + }; + + mockStore.state.organizationData.allDashboardData[dashboardId] = mockDashboard; + (dashboardService.save as any).mockResolvedValue({ data: { success: true } }); + + await getDashboard(mockStore, dashboardId, "test-folder"); + + // same i across different tabs is fine (grid is per-tab) → no save + expect(mockDashboard.tabs[0].panels[0].layout.i).toBe(1); + expect(mockDashboard.tabs[1].panels[0].layout.i).toBe(1); + expect(dashboardService.save).not.toHaveBeenCalled(); + }); + + it("does not save when all layout ids are already unique", async () => { + const dashboardId = "dashboard-unique-layout"; + const mockDashboard = { + tabs: [ + { + tabId: "tab-1", + panels: [ + { id: "panel-1", layout: { x: 0, y: 0, w: 24, h: 9, i: 1 } }, + { id: "panel-2", layout: { x: 0, y: 0, w: 24, h: 9, i: 2 } }, + ], + }, + ], + }; + + mockStore.state.organizationData.allDashboardData[dashboardId] = mockDashboard; + (dashboardService.save as any).mockResolvedValue({ data: { success: true } }); + + await getDashboard(mockStore, dashboardId, "test-folder"); + + expect(dashboardService.save).not.toHaveBeenCalled(); + }); + }); + describe("addVariable", () => { it("should add variable to dashboard with empty variables", async () => { const dashboardId = "dashboard-1"; diff --git a/web/src/utils/commons.ts b/web/src/utils/commons.ts index 5688050d0b..68e850d1f4 100644 --- a/web/src/utils/commons.ts +++ b/web/src/utils/commons.ts @@ -556,8 +556,8 @@ export const getDashboard = async (store: any, dashboardId: any, folderId: any) // Ensure variables structure always exists (fix for dashboards with no variables) ensureVariablesStructure(dashboardJson); - // Fix duplicate panel IDs and check if any were found - const hasDuplicates = fixDuplicatePanelIds(dashboardJson); + // Fix duplicate panel IDs and layout IDs (layout.i) and check if any were found + const hasDuplicateIds = dedupeDashboardIds(dashboardJson); // The reserved timestamp column is not allowed as a SQL output alias — // normalize it to `ts`. Returns true when anything was rewritten, so we @@ -565,8 +565,8 @@ export const getDashboard = async (store: any, dashboardId: any, folderId: any) const timestampColumn = store.state.zoConfig?.timestamp_column ?? "_timestamp"; const hasReservedAlias = normalizeReservedTimestampAlias(dashboardJson, timestampColumn); - // If either fix changed the dashboard, save it and retrieve the updated version - if (hasDuplicates || hasReservedAlias) { + // If any fix changed the dashboard, save it and retrieve the updated version + if (hasDuplicateIds || hasReservedAlias) { // Save the dashboard with the applied fixes await updateDashboard( store, @@ -627,6 +627,46 @@ const generateUniquePanelId = (existingIds: Set): string => { return id; }; +// Fix duplicate panel layout ids (layout.i). layout.i is a numeric grid-slot +// id scoped to a tab, so dedupe per-tab and continue from the tab's max i. +const fixDuplicateLayoutIds = (dashboardJson: any): boolean => { + let hasDuplicates = false; + + for (const tab of dashboardJson?.tabs || []) { + if (!Array.isArray(tab?.panels)) continue; + + const usedIds = new Set(); + let maxI = 0; + for (const panel of tab.panels) { + if (typeof panel?.layout?.i === "number") { + maxI = Math.max(maxI, panel.layout.i); + } + } + + for (const panel of tab.panels) { + if (!panel?.layout) continue; + + const currentId = panel.layout.i; + if (typeof currentId !== "number" || usedIds.has(currentId)) { + panel.layout.i = ++maxI; + hasDuplicates = true; + } + + usedIds.add(panel.layout.i); + } + } + + return hasDuplicates; +}; + +// Dedupe both panel ids and layout ids in place. Used on dashboard load and at +// import time (post schema migration). Returns true if anything was changed. +export const dedupeDashboardIds = (dashboardJson: any): boolean => { + const fixedPanelIds = fixDuplicatePanelIds(dashboardJson); + const fixedLayoutIds = fixDuplicateLayoutIds(dashboardJson); + return fixedPanelIds || fixedLayoutIds; +}; + export const deleteDashboardById = async (store: any, dashboardId: any, folderId: any) => { // Delete the dashboard using the dashboardService await dashboardService.delete(store.state.selectedOrganization.identifier, dashboardId, folderId); diff --git a/web/src/views/Dashboards/ImportDashboard.spec.ts b/web/src/views/Dashboards/ImportDashboard.spec.ts index b7170ae9fc..e0f7f8316d 100644 --- a/web/src/views/Dashboards/ImportDashboard.spec.ts +++ b/web/src/views/Dashboards/ImportDashboard.spec.ts @@ -8,10 +8,14 @@ import { createI18n } from "vue-i18n"; import enLocaleFull from "@/locales/languages/en-US.json"; // Mock external dependencies -vi.mock("@/utils/commons", () => ({ - getAllDashboards: vi.fn(), - getFoldersList: vi.fn(), -})); +vi.mock("@/utils/commons", async () => { + const actual: any = await vi.importActual("@/utils/commons"); + return { + getAllDashboards: vi.fn(), + getFoldersList: vi.fn(), + dedupeDashboardIds: actual.dedupeDashboardIds, + }; +}); vi.mock("@/services/dashboards", () => ({ default: { @@ -901,4 +905,38 @@ describe("ImportDashboard.vue", () => { expect(wrapper.find("#import-dashboard-form").exists()).toBe(true); }); }); + + describe("import id uniqueness", () => { + it("dedupes duplicate panel ids and layout ids on import (before create)", async () => { + const dashboardService = (await import("@/services/dashboards")).default; + (dashboardService.create as any).mockResolvedValue({ data: { dashboardId: "d1" } }); + + wrapper = mountComponent(); + await nextTick(); + + wrapper.vm.jsonStr = JSON.stringify({ + title: "Dup Dashboard", + tabs: [ + { + tabId: "tab-1", + panels: [ + { id: "panel-1", layout: { x: 0, y: 0, w: 24, h: 9, i: 1 } }, + { id: "panel-1", layout: { x: 0, y: 0, w: 24, h: 9, i: 1 } }, + ], + }, + ], + }); + + await wrapper.vm.importFromJsonStr(); + await flushPromises(); + + // migrateSchema dedupes after schema migration and before validation, + // so the dashboard reaching create has unique panel ids and layout ids. + expect(dashboardService.create).toHaveBeenCalledTimes(1); + const sentDashboard = (dashboardService.create as any).mock.calls[0][1]; + const panels = sentDashboard.tabs[0].panels; + expect(new Set(panels.map((p: any) => p.id)).size).toBe(panels.length); + expect(new Set(panels.map((p: any) => p.layout.i)).size).toBe(panels.length); + }); + }); }); diff --git a/web/src/views/Dashboards/ImportDashboard.vue b/web/src/views/Dashboards/ImportDashboard.vue index 65cea26e14..0112e0c019 100644 --- a/web/src/views/Dashboards/ImportDashboard.vue +++ b/web/src/views/Dashboards/ImportDashboard.vue @@ -250,7 +250,7 @@ along with this program. If not, see . // @ts-nocheck import { defineComponent, ref, onMounted, reactive, watch } from "vue"; import { useI18nTyped } from "@/types/i18n"; -import { getAllDashboards, getFoldersList } from "../../utils/commons.js"; +import { getAllDashboards, getFoldersList, dedupeDashboardIds } from "../../utils/commons.js"; import { useStore } from "vuex"; import { useRouter, useRoute } from "vue-router"; import dashboardService from "../../services/dashboards.js"; @@ -442,6 +442,16 @@ export default defineComponent({ }); //import dashboard from the json + const migrateSchema = (dashboard: any) => { + const converted = convertDashboardSchemaVersion(dashboard); + try { + dedupeDashboardIds(converted); + } catch (e) { + console.error("Failed to dedupe dashboard ids on import", e); + } + return converted; + }; + const importDashboardFromJSON = async (jsonObj: any, selectedFolder: any) => { const data = typeof jsonObj == "string" @@ -507,7 +517,7 @@ export default defineComponent({ //it will convert the dashboard schema version to the latest version try { - const convertedSchema = convertDashboardSchemaVersion(dashboard); + const convertedSchema = migrateSchema(dashboard); // Validate the converted schema before importing const validationErrors = validateDashboardJson(convertedSchema); @@ -623,7 +633,7 @@ export default defineComponent({ const importPromises = dashboards.map((dashboard, index) => { try { - const converted = convertDashboardSchemaVersion(dashboard); + const converted = migrateSchema(dashboard); // Validate the converted schema before importing const validationErrors = validateDashboardJson(converted); @@ -674,7 +684,7 @@ export default defineComponent({ // get the dashboard const oldImportedSchema = JSON.parse(jsonStr.value); - const convertedSchema = convertDashboardSchemaVersion(oldImportedSchema); + const convertedSchema = migrateSchema(oldImportedSchema); // Validate the converted schema before importing const validationErrors = validateDashboardJson(convertedSchema); @@ -729,12 +739,12 @@ export default defineComponent({ if (Array.isArray(jsonObj)) { jsonObj.forEach((input, index) => { // migrate to new schema - const convertedSchema = convertDashboardSchemaVersion(input); + const convertedSchema = migrateSchema(input); validateBasicInputs(convertedSchema, index); }); } else { // migrate to new schema - const convertedSchema = convertDashboardSchemaVersion(jsonObj); + const convertedSchema = migrateSchema(jsonObj); validateBasicInputs(convertedSchema); } if (dashboardErrorsToDisplay.value.length > 0) {