Skip to content

Commit 497c818

Browse files
committed
[FIX] GaugeChart: Update config upon sheet renaming
How to reproduce: - Start with a spreadsheet with two sheets - In Sheet 1, create a gauge chart - Add an inflection point as a formula which points towards Sheet 2 - Rename Sheet 2 to something else The inflection point formula was not updated to match the new sheet name. Transforming the name of a range following a `RENAME_SHEET` command cannot work currently as we rely solely on the getters to convert an XC to a range, adapt it and then regenerate an XC based on the updated range. During this process, the name of the sheet does not change inside the plugins which means that if we try to generate an xc, we end up with the previous sheetName. The fix suggested here stops relying on the getters (and therefore the plugin states) in favor of the sheetName change information contained in the original `RENAME_SHEET` command. closes #7030 Task: 4868971 X-original-commit: 5ab44cd Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com> Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
1 parent 4123c85 commit 497c818

File tree

4 files changed

+51
-7
lines changed

4 files changed

+51
-7
lines changed

src/helpers/figures/charts/abstract_chart.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
import { ApplyRangeChange, CommandResult, CoreGetters, RangeAdapter, UID } from "../../../types";
1+
import {
2+
AdaptSheetName,
3+
ApplyRangeChange,
4+
CommandResult,
5+
CoreGetters,
6+
RangeAdapter,
7+
UID,
8+
} from "../../../types";
29
import {
310
ChartCreationContext,
411
ChartDefinition,
@@ -69,7 +76,11 @@ export abstract class AbstractChart {
6976
* This function should be used to update all the ranges of the chart after
7077
* a grid change (add/remove col/row, rename sheet, ...)
7178
*/
72-
abstract updateRanges(applyChange: ApplyRangeChange): AbstractChart;
79+
abstract updateRanges(
80+
applyChange: ApplyRangeChange,
81+
sheetId: UID,
82+
adaptSheetName: AdaptSheetName
83+
): AbstractChart;
7384

7485
/**
7586
* Duplicate the chart when a sheet is duplicated.

src/helpers/figures/charts/gauge_chart.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { tryToNumber } from "../../../functions/helpers";
88
import { BasePlugin } from "../../../plugins/base_plugin";
99
import { _t } from "../../../translation";
1010
import {
11+
AdaptSheetName,
1112
ApplyRangeChange,
1213
CellValueType,
1314
Color,
@@ -30,7 +31,7 @@ import {
3031
} from "../../../types/chart/gauge_chart";
3132
import { CellErrorType } from "../../../types/errors";
3233
import { Validator } from "../../../types/validator";
33-
import { adaptStringRange } from "../../formulas";
34+
import { adaptFormulaStringRanges, adaptStringRange } from "../../formulas";
3435
import { clip, formatValue } from "../../index";
3536
import { createValidRange } from "../../range";
3637
import { rangeReference } from "../../references";
@@ -261,11 +262,19 @@ export class GaugeChart extends AbstractChart {
261262
};
262263
}
263264

264-
updateRanges(applyChange: ApplyRangeChange): GaugeChart {
265+
updateRanges(
266+
applyChange: ApplyRangeChange,
267+
sheetId: UID,
268+
adaptSheetName: AdaptSheetName
269+
): GaugeChart {
265270
const dataRange = adaptChartRange(this.dataRange, applyChange);
266271

267272
const adaptFormula = (formula: string) =>
268-
this.getters.adaptFormulaStringDependencies(this.sheetId, formula, applyChange);
273+
adaptFormulaStringRanges(this.sheetId, formula, {
274+
applyChange,
275+
sheetId,
276+
sheetName: adaptSheetName,
277+
});
269278
const sectionRule = adaptSectionRuleFormulas(this.sectionRule, adaptFormula);
270279
const definition = this.getDefinitionWithSpecificRanges(dataRange, sectionRule);
271280
return new GaugeChart(definition, this.sheetId, this.getters);

src/plugins/core/chart.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { AbstractChart } from "../../helpers/figures/charts/abstract_chart";
44
import { chartFactory, validateChartDefinition } from "../../helpers/figures/charts/chart_factory";
55
import { ChartCreationContext, ChartDefinition, ChartType } from "../../types/chart/chart";
66
import {
7+
AdaptSheetName,
78
ApplyRangeChange,
89
Command,
910
CommandResult,
@@ -45,9 +46,13 @@ export class ChartPlugin extends CorePlugin<ChartState> implements ChartState {
4546
private validateChartDefinition = (cmd: CreateChartCommand | UpdateChartCommand) =>
4647
validateChartDefinition(this, cmd.definition);
4748

48-
adaptRanges(applyChange: ApplyRangeChange) {
49+
adaptRanges(applyChange: ApplyRangeChange, sheetId: UID, adaptSheetName: AdaptSheetName) {
4950
for (const [chartId, chart] of Object.entries(this.charts)) {
50-
this.history.update("charts", chartId, chart?.updateRanges(applyChange));
51+
this.history.update(
52+
"charts",
53+
chartId,
54+
chart?.updateRanges(applyChange, sheetId, adaptSheetName)
55+
);
5156
}
5257
}
5358

tests/figures/chart/gauge/gauge_chart_plugin.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
duplicateSheet,
1818
paste,
1919
redo,
20+
renameSheet,
2021
setCellContent,
2122
setFormat,
2223
undo,
@@ -176,6 +177,24 @@ describe("datasource tests", function () {
176177
upperInflectionPoint: { type: "number", value: "=SUM('Copy of Sheet1'!B1:C4)" },
177178
});
178179
});
180+
181+
test("gauge range are adapted when renaming the sheet", () => {
182+
renameSheet(model, "Sheet2", "Boom");
183+
let chart = model.getters.getChartDefinition("chartId") as GaugeChartDefinition;
184+
expect(chart.sectionRule).toMatchObject({
185+
lowerInflectionPoint: { operator: "<", type: "percentage", value: "=Boom!A2" },
186+
});
187+
188+
renameSheet(model, "Sheet1", "Magic");
189+
chart = model.getters.getChartDefinition("chartId") as GaugeChartDefinition;
190+
expect(chart.dataRange).toStrictEqual("Magic!B1:B4");
191+
expect(chart.sectionRule).toMatchObject({
192+
rangeMin: "=A1+5",
193+
rangeMax: "=C8",
194+
lowerInflectionPoint: { operator: "<", type: "percentage", value: "=Boom!A2" },
195+
upperInflectionPoint: { operator: "<", type: "number", value: "=SUM(Magic!B1:C4)" },
196+
});
197+
});
179198
});
180199

181200
test("can delete an imported gauge chart", () => {

0 commit comments

Comments
 (0)