From 905d8565ad2fae3f7ff96606352ee3ab86ee78cb Mon Sep 17 00:00:00 2001
From: Khushi <khpa@odoo.com>
Date: Thu, 2 Nov 2023 15:15:34 +0530
Subject: [PATCH] [FIX] spreadsheet: traceback while inserting chart from other
 model
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Steps to reproduce:
- Insert a list/pivot in a blank spreadsheet from a module (say Sales).
- Apply a global filter on it.
- Insert an odoo chart in the same sheet but from a different module (eg. CRM)

This works just fine when chart is inserted from the same model because it matches
the existing filter (from the pivot/list), and directly returns that field matching
without check the field matching from charts.

However in case of different model, the issue is that when there are no charts in the
sheet, the existing code of `getOdooChartIds` retrieves the incorrect chart ids
(`getChartIds` getter returns all chart ids, including the id of chart being inserted).
This leads to a traceback as the code tries to fetch fieldMatchings for a non-existent
chart within the sheet.

This commit resolves the issue by modifying the `getOdooChartIds` method to now utilize
`this.charts` instead of `getChartIds` getter, which correctly provides the ids of
charts already present in the sheet.

Task ID: 3573402

closes odoo/odoo#140700

Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
---
 .../src/chart/plugins/odoo_chart_core_plugin.js     | 10 +---------
 .../tests/charts/model/odoo_chart_plugin_test.js    | 13 +++++++++++++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/addons/spreadsheet/static/src/chart/plugins/odoo_chart_core_plugin.js b/addons/spreadsheet/static/src/chart/plugins/odoo_chart_core_plugin.js
index 2af4ef138f3..78b5ab9b702 100644
--- a/addons/spreadsheet/static/src/chart/plugins/odoo_chart_core_plugin.js
+++ b/addons/spreadsheet/static/src/chart/plugins/odoo_chart_core_plugin.js
@@ -86,15 +86,7 @@ export class OdooChartCorePlugin extends CorePlugin {
      * @returns {Array<string>}
      */
     getOdooChartIds() {
-        const ids = [];
-        for (const sheetId of this.getters.getSheetIds()) {
-            ids.push(
-                ...this.getters
-                    .getChartIds(sheetId)
-                    .filter((id) => this.getters.getChartType(id).startsWith("odoo_"))
-            );
-        }
-        return ids;
+        return Object.keys(this.charts);
     }
 
     /**
diff --git a/addons/spreadsheet/static/tests/charts/model/odoo_chart_plugin_test.js b/addons/spreadsheet/static/tests/charts/model/odoo_chart_plugin_test.js
index 3529032e24e..0899a8eafa3 100644
--- a/addons/spreadsheet/static/tests/charts/model/odoo_chart_plugin_test.js
+++ b/addons/spreadsheet/static/tests/charts/model/odoo_chart_plugin_test.js
@@ -5,7 +5,10 @@ import { OdooChart } from "@spreadsheet/chart/odoo_chart/odoo_chart";
 import { OdooLineChart } from "@spreadsheet/chart/odoo_chart/odoo_line_chart";
 import { nextTick } from "@web/../tests/helpers/utils";
 import { createSpreadsheetWithChart, insertChartInSpreadsheet } from "../../utils/chart";
+import { insertListInSpreadsheet } from "../../utils/list";
 import { createModelWithDataSource, waitForDataSourcesLoaded } from "../../utils/model";
+import { addGlobalFilter } from "../../utils/commands";
+import { THIS_YEAR_GLOBAL_FILTER } from "../../utils/global_filter";
 import * as spreadsheet from "@odoo/o-spreadsheet";
 import { makeServerError } from "@web/../tests/helpers/mock_server";
 import { session } from "@web/session";
@@ -519,4 +522,14 @@ QUnit.module("spreadsheet > odoo chart plugin", {}, () => {
             [1, 3]
         );
     });
+
+    QUnit.test("Can insert odoo chart from a different model", async (assert) => {
+        const model = await createModelWithDataSource();
+        insertListInSpreadsheet(model, { model: "product", columns: ["name"] });
+        await addGlobalFilter(model, THIS_YEAR_GLOBAL_FILTER);
+        const sheetId = model.getters.getActiveSheetId();
+        assert.strictEqual(model.getters.getChartIds(sheetId).length, 0);
+        insertChartInSpreadsheet(model);
+        assert.strictEqual(model.getters.getChartIds(sheetId).length, 1);
+    });
 });