From 3046c3b517b284cd4fd83a078835b9c43da81f7e Mon Sep 17 00:00:00 2001 From: "Adrien Minne (adrm)" Date: Wed, 9 Apr 2025 07:09:05 +0000 Subject: [PATCH] [FIX] xlsx: correctly export aggregated charts The feature to aggregate the chart data based on the labels does not exist in xlsx files. But rather than exporting the chart as normal, ignoring the aggregation, we exported the chart as an image. Task: 4714410 X-original-commit: 2ed572676d003be9b3b13a672948656deb5483d3 --- src/helpers/figures/charts/bar_chart.ts | 2 - src/helpers/figures/charts/line_chart.ts | 2 - src/helpers/figures/charts/pie_chart.ts | 2 - tests/xlsx/xlsx_export.test.ts | 47 +++++------------------- 4 files changed, 9 insertions(+), 44 deletions(-) diff --git a/src/helpers/figures/charts/bar_chart.ts b/src/helpers/figures/charts/bar_chart.ts index e1ec6520f6..80d597022c 100644 --- a/src/helpers/figures/charts/bar_chart.ts +++ b/src/helpers/figures/charts/bar_chart.ts @@ -189,8 +189,6 @@ export class BarChart extends AbstractChart { } getDefinitionForExcel(): ExcelChartDefinition | undefined { - // Excel does not support aggregating labels - if (this.aggregated) return undefined; const dataSets: ExcelChartDataset[] = this.dataSets .map((ds: DataSet) => toExcelDataset(this.getters, ds)) .filter((ds) => ds.range !== "" && ds.range !== CellErrorType.InvalidReference); diff --git a/src/helpers/figures/charts/line_chart.ts b/src/helpers/figures/charts/line_chart.ts index fb45a238b1..4fcd204455 100644 --- a/src/helpers/figures/charts/line_chart.ts +++ b/src/helpers/figures/charts/line_chart.ts @@ -193,8 +193,6 @@ export class LineChart extends AbstractChart { } getDefinitionForExcel(): ExcelChartDefinition | undefined { - // Excel does not support aggregating labels - if (this.aggregated) return undefined; const dataSets: ExcelChartDataset[] = this.dataSets .map((ds: DataSet) => toExcelDataset(this.getters, ds)) .filter((ds) => ds.range !== "" && ds.range !== CellErrorType.InvalidReference); diff --git a/src/helpers/figures/charts/pie_chart.ts b/src/helpers/figures/charts/pie_chart.ts index d42cb11ec4..3850c18e94 100644 --- a/src/helpers/figures/charts/pie_chart.ts +++ b/src/helpers/figures/charts/pie_chart.ts @@ -165,8 +165,6 @@ export class PieChart extends AbstractChart { } getDefinitionForExcel(): ExcelChartDefinition | undefined { - // Excel does not support aggregating labels - if (this.aggregated) return undefined; const dataSets: ExcelChartDataset[] = this.dataSets .map((ds: DataSet) => toExcelDataset(this.getters, ds)) .filter((ds) => ds.range !== "" && ds.range !== CellErrorType.InvalidReference); diff --git a/tests/xlsx/xlsx_export.test.ts b/tests/xlsx/xlsx_export.test.ts index c0f5c8b6e0..fec9bda34d 100644 --- a/tests/xlsx/xlsx_export.test.ts +++ b/tests/xlsx/xlsx_export.test.ts @@ -4,7 +4,7 @@ import { RAND, RANDARRAY, RANDBETWEEN } from "../../src/functions/module_math"; import { buildSheetLink, toXC } from "../../src/helpers"; import { DEFAULT_TABLE_CONFIG } from "../../src/helpers/table_presets"; import { Model } from "../../src/model"; -import { CustomizedDataSet, Dimension, ExcelChartType } from "../../src/types"; +import { CustomizedDataSet, Dimension } from "../../src/types"; import { XLSXExportXMLFile, XMLString } from "../../src/types/xlsx"; import { hexaToInt } from "../../src/xlsx/conversion"; import { adaptFormulaToExcel } from "../../src/xlsx/functions/cells"; @@ -1270,43 +1270,14 @@ describe("Test XLSX export", () => { expect(await exportPrettifiedXlsx(model)).toMatchSnapshot(); }); - test.each(["bar", "line", "pie", "scatter", "radar"] as const)( - "%s chart that aggregate labels is exported as image", - async (type: ExcelChartType) => { - const model = new Model({ - sheets: [ - { - ...chartData.sheets, - cells: { - ...chartData.sheets[0].cells, - A6: "P1", - A7: "P2", - A8: "P3", - A9: "P4", - B6: "17", - B7: "26", - B8: "13", - B9: "31", - C6: "31", - C7: "18", - C8: "9", - C9: "27", - }, - }, - ], - }); - createChart( - model, - { - dataSets: [{ dataRange: "Sheet1!B1:B9" }], - labelRange: "Sheet1!A2:A9", - aggregated: true, - type, - }, - "1" - ); - expect(getExportedExcelData(model).sheets[0].charts.length).toBe(0); - expect(getExportedExcelData(model).sheets[0].images.length).toBe(1); + test.each(["bar", "line", "pie"] as const)( + "%s chart that aggregate labels is exported as normal chart, ignoring the aggregation", + async (type: "bar" | "line" | "pie") => { + const model = new Model(); + createChart(model, { aggregated: true, type }, "1"); + const exportedData = getExportedExcelData(model); + expect(exportedData.sheets[0].charts.length).toBe(1); + expect(exportedData.sheets[0].images.length).toBe(0); } );