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

@@ -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";

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>): 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<number>();
+    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);

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);
+    });
+  });
 });

web/src/views/Dashboards/ImportDashboard.vue

@@ -250,7 +250,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 // @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) {