Skip to content

Commit 6c86004

Browse files
committed
[FIX] model: ensure chart ID unicity
We have a function `forceUnicityOfFigure` that repair the spreadsheet data at import to force the figure ID unicity. But since the introduction of carousels, we also need to ensure the chart ID unicity (after the migration, chart ID and figure ID are the same). closes #7356 Task: 5153264 X-original-commit: cb66cfd Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
1 parent 5c71d93 commit 6c86004

File tree

2 files changed

+32
-0
lines changed

2 files changed

+32
-0
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,21 @@ function forceUnicityOfFigure(data: Partial<WorkbookData>): Partial<WorkbookData
154154
return data;
155155
}
156156
const figureIds = new Set();
157+
const chartIds = new Set();
157158
const uuidGenerator = new UuidGenerator();
158159
for (const sheet of data.sheets || []) {
159160
for (const figure of sheet.figures || []) {
160161
if (figureIds.has(figure.id)) {
161162
figure.id += uuidGenerator.smallUuid();
162163
}
163164
figureIds.add(figure.id);
165+
166+
if (figure.tag === "chart") {
167+
if (chartIds.has(figure.data?.chartId)) {
168+
figure.data.chartId += uuidGenerator.smallUuid();
169+
}
170+
chartIds.add(figure.data?.chartId);
171+
}
164172
}
165173
}
166174

tests/figures/chart/chart_plugin.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,30 @@ describe("datasource tests", function () {
805805
expect(cmd3).toBeCancelledBecause(CommandResult.DuplicatedChartId);
806806
});
807807

808+
test("Cannot have duplicate chart id at model creation", () => {
809+
const figure = { id: "figureId", tag: "chart", width: 400, height: 300, x: 100, y: 100 };
810+
const model = new Model({
811+
version: 7,
812+
sheets: [
813+
{
814+
id: "sh1",
815+
figures: [
816+
{ ...figure, data: { type: "line", dataSets: [], labelRange: "A1:A2" } },
817+
{ ...figure, data: { type: "line", dataSets: [], labelRange: "A1:A2" } },
818+
],
819+
},
820+
],
821+
});
822+
823+
const figures = model.getters.getFigures("sh1");
824+
expect(figures).toHaveLength(2);
825+
expect(figures[0].id).not.toEqual(figures[1].id);
826+
827+
const chartIds = model.getters.getChartIds("sh1");
828+
expect(chartIds).toHaveLength(2);
829+
expect(chartIds[0]).not.toEqual(chartIds[1]);
830+
});
831+
808832
test("reject updates that target a inexistent chart", () => {
809833
createChart(
810834
model,

0 commit comments

Comments
 (0)