[FIX] Charts: fix color scale migration

in PR #6773, we changed the color scale definition from a theme string
tot an object that allows custom scales. However, the migration assumed
the new scale to be a triplet of strings (probably the remnants of a
previous implementation idea).

Furthermore, the migration did not account for charts sitting inside a
carousel and not a standard figure.

Task: 5363581

Author: rrahir

packages/o-spreadsheet-engine/src/migrations/migration_steps.ts

@@ -1,13 +1,12 @@
 import { BACKGROUND_CHART_COLOR, FORMULA_REF_IDENTIFIER } from "../constants";
-import { COLORSCHEMES } from "../helpers/color";
 import { toXC } from "../helpers/coordinates";
 import { getItemId } from "../helpers/data_normalization";
 import { getUniqueText, sanitizeSheetName } from "../helpers/misc";
 import { getMaxObjectId } from "../helpers/pivot/pivot_helpers";
 import { DEFAULT_TABLE_CONFIG } from "../helpers/table_presets";
 import { overlap, toZone, zoneToXc } from "../helpers/zones";
 import { Registry } from "../registry";
-import { CustomizedDataSet } from "../types/chart";
+import { CustomizedDataSet, schemeToColorScale } from "../types/chart";
 import { Format } from "../types/format";
 import { DEFAULT_LOCALE } from "../types/locale";
 import { Zone } from "../types/misc";
@@ -561,7 +560,16 @@ migrationStepRegistry
         for (const figure of sheet.figures || []) {
           if (figure.tag === "chart" && figure.data.type === "geo") {
             if ("colorScale" in figure.data && typeof figure.data.colorScale === "string") {
-              figure.data.colorScale = COLORSCHEMES[figure.data.colorScale];
+              figure.data.colorScale = schemeToColorScale(figure.data.colorScale);
+            }
+          }
+          if (figure.tag === "carousel") {
+            for (const definition of Object.values<any>(figure.data.chartDefinitions) || []) {
+              if (definition.type === "geo") {
+                if ("colorScale" in definition && typeof definition.colorScale === "string") {
+                  definition.colorScale = schemeToColorScale(figure.data.colorScale);
+                }
+              }
             }
           }
         }

src/index.ts

@@ -215,7 +215,7 @@ import { HighlightStore } from "./stores/highlight_store";
 import { ModelStore } from "./stores/model_store";
 import { NotificationStore } from "./stores/notification_store";
 import { RendererStore } from "./stores/renderer_store";
-import { AddFunctionDescription, isMatrix } from "./types";
+import { AddFunctionDescription, isMatrix, schemeToColorScale } from "./types";
 
 /**
  * We export here all entities that needs to be accessed publicly by Odoo.
@@ -400,6 +400,7 @@ export const helpers = {
   isNumber,
   isDateTime,
   createCustomFields,
+  schemeToColorScale,
 };
 
 export const links = {

tests/model/model_import_export.test.ts

@@ -8,13 +8,14 @@ import {
 import { DEFAULT_TABLE_CONFIG } from "@odoo/o-spreadsheet-engine/helpers/table_presets";
 import { getCurrentVersion } from "@odoo/o-spreadsheet-engine/migrations/data";
 import { CellIsRule, Model } from "../../src";
-import { COLORSCHEMES, toCartesian, toZone } from "../../src/helpers";
+import { toCartesian, toZone } from "../../src/helpers";
 import {
   BorderDescr,
   ColorScaleRule,
   DEFAULT_LOCALE,
   DEFAULT_LOCALES,
   IconSetRule,
+  schemeToColorScale,
 } from "../../src/types";
 import {
   activateSheet,
@@ -32,6 +33,7 @@ import {
   getEvaluatedCell,
   getMerges,
 } from "../test_helpers/getters_helpers";
+import { mockGeoJsonService } from "../test_helpers/helpers";
 
 describe("data", () => {
   test("give default col size if not specified", () => {
@@ -781,24 +783,48 @@ test("migrate version 18.5.1: chartId is added to figure data", () => {
   expect(model.exportData().sheets[0].figures[0].data.chartId).toBe("someuuid");
 });
 
-test("migrate version 19.1.0: colorScale is changed to a trio of color", () => {
+test("migrate version 19.1.0: colorScale is changed to a colorScale", () => {
+  const getChartDefinition = (chartId) => ({
+    chartId,
+    type: "geo",
+    colorScale: "reds",
+    labelRange: "",
+    dataSets: [],
+    legendPosition: "top",
+    title: { text: "Demo Geo Chart" },
+  });
+  const sheetId = "sh1";
   const data = {
     version: "18.5.1",
     sheets: [
       {
-        id: "sh1",
+        id: sheetId,
         figures: [
           {
             id: "someuuid",
             tag: "chart",
-            data: { type: "geo", colorScale: "reds", labelRange: "", dataSets: [] },
+            data: getChartDefinition("chartId1"),
+          },
+          {
+            id: "someuuid",
+            tag: "carousel",
+            data: {
+              chartDefinitions: { chartId2: getChartDefinition("chartId2") },
+            },
           },
         ],
       },
     ],
   };
-  const model = new Model(data);
-  expect(model.exportData().sheets[0].figures[0].data.colorScale).toEqual(COLORSCHEMES.reds);
+  const model = new Model(data, { external: { geoJsonService: mockGeoJsonService } });
+  expect(model.exportData().sheets[0].figures[0].data.colorScale).toEqual(
+    schemeToColorScale("reds")
+  );
+
+  // check runtime as well for safety as geo charts depend on geo service to
+  // build their runtime and use their color scale
+  const chartIds = model.getters.getChartIds(sheetId);
+  expect(model.getters.getChartRuntime(chartIds[0])).toBeDefined();
 });
 
 describe("Import", () => {