Skip to content

Commit

Permalink
[FIX] chart: preserve label order when aggregate
Browse files Browse the repository at this point in the history
Previously, the labels were not preserving their order when data was
aggregated. This was due to the labels being stored in an object, whose
order is not guaranteed. This commit addresses the issue by returning a
labelSet and mapping the data accordingly.

closes #3948

Taskid: 3834222
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
  • Loading branch information
dhrp-odoo committed Apr 2, 2024
1 parent 190bd20 commit 9961402
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/helpers/figures/charts/chart_ui_common.ts
Expand Up @@ -73,10 +73,10 @@ export function aggregateDataForLabels(
}

return {
labels: Object.keys(labelMap),
labels: Array.from(labelSet),
dataSetsValues: datasets.map((dataset, indexOfDataset) => ({
...dataset,
data: Object.values(labelMap).map((dataOfLabel: any[]) => dataOfLabel[indexOfDataset]),
data: Array.from(labelSet).map((label) => labelMap[label][indexOfDataset]),
})),
};
}
Expand Down
22 changes: 21 additions & 1 deletion tests/plugins/chart/basic_chart.test.ts
Expand Up @@ -26,7 +26,7 @@ import {
updateChart,
} from "../../test_helpers/commands_helpers";
import { getPlugin, nextTick, target } from "../../test_helpers/helpers";
import { ChartDefinition } from "./../../../src/types/chart/chart";
import { ChartDefinition, ChartJSRuntime } from "./../../../src/types/chart/chart";
jest.mock("../../../src/helpers/uuid", () => require("../../__mocks__/uuid"));

let model: Model;
Expand Down Expand Up @@ -1751,6 +1751,26 @@ describe("Chart aggregate labels", () => {
expect(chart.data!.datasets![1].data).toEqual([52, 54, 56, 58]);
expect(chart.data!.labels).toEqual(["P1", "P2", "P3", "P4"]);
});

test.each(["bar", "line", "pie"] as const)(
"Labels will not be sorted when aggregated in %s chart",
(type) => {
createChart(aggregatedModel, aggregatedChart, "42");
updateChart(aggregatedModel, "42", { type });

setCellContent(aggregatedModel, "A3", "2023");
setCellContent(aggregatedModel, "A7", "2024");

let chart = (aggregatedModel.getters.getChartRuntime("42") as ChartJSRuntime).chartJsConfig;
expect(chart.data!.datasets![0].data).toEqual([10, 11, 12, 13, 14, 15, 16, 17]);
expect(chart.data!.labels).toEqual(["P1", "2023", "P3", "P4", "P1", "2024", "P3", "P4"]);

updateChart(aggregatedModel, "42", { aggregated: true });
chart = (aggregatedModel.getters.getChartRuntime("42") as ChartJSRuntime).chartJsConfig;
expect(chart.data!.datasets![0].data).toEqual([24, 11, 28, 30, 15]);
expect(chart.data!.labels).toEqual(["P1", "2023", "P3", "P4", "2024"]);
}
);
});

describe("Linear/Time charts", () => {
Expand Down

0 comments on commit 9961402

Please sign in to comment.