From d8ca1f34129678c017af15843511c17786cebbe9 Mon Sep 17 00:00:00 2001 From: Hubert Van De Walle Date: Mon, 12 Feb 2024 09:15:03 +0000 Subject: [PATCH] [FIX] web: missing measures in graph view Steps to reproduce ================== - Install account_accountant - Go to settings - Enable Budget Management - Go to Accounting > Reporting > Management > Budgets Analysis - Switch to the graph view - Change the measure to "Planned amount" and then back to "Practical amount" => The practical_amount measure is undefined, The theoritical_amount measure is missing. Cause of the issue ================== The view is defined as follows: ```xml ``` The theoritical_amount and practical_amount are non stored fields and thus are skipped inside `computeReportMeasures` unless they are passed in `activeMeasures | additionalMeasures`. [0] When parsing the graph view, the last field of type measure is passed to the graph model and is the one that will be used initially. [1] This is why the practical_amount is initially defined. Solution ======== We simply need to keep track of fields of type measure. This was the case in 14.0 but got lost in the conversion. --- [0]: https://github.com/odoo/odoo/blob/e7a9ebec3176c37485643fcda2381e489a1df86f/addons/web/static/src/views/helpers/utils.js#L49-L60 [1]: https://github.com/odoo/odoo/blob/0fb64bef16914937cf4a1d1618fb58ade6d16f14/addons/web/static/src/views/graph/graph_arch_parser.js#L63 opw-3713613 closes odoo/odoo#153967 X-original-commit: 76178cd61ba10ff29b24183edb550c54b319a230 Signed-off-by: Mathieu Duckerts-Antoine (dam) Signed-off-by: Hubert Van De Walle --- .../src/views/graph/graph_arch_parser.js | 3 +- .../web/static/src/views/graph/graph_model.js | 1 + .../web/static/src/views/graph/graph_view.js | 1 + .../static/tests/views/graph_view_tests.js | 48 ++++++++++++++++++- 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/addons/web/static/src/views/graph/graph_arch_parser.js b/addons/web/static/src/views/graph/graph_arch_parser.js index a393bf16668..8b7c62cf8c9 100644 --- a/addons/web/static/src/views/graph/graph_arch_parser.js +++ b/addons/web/static/src/views/graph/graph_arch_parser.js @@ -9,7 +9,7 @@ const ORDERS = ["ASC", "DESC", "asc", "desc", null]; export class GraphArchParser { parse(arch, fields = {}) { - const archInfo = { fields, fieldAttrs: {}, groupBy: [] }; + const archInfo = { fields, fieldAttrs: {}, groupBy: [], measures: [] }; visitXML(arch, (node) => { switch (node.tagName) { case "graph": { @@ -67,6 +67,7 @@ export class GraphArchParser { } const isMeasure = node.getAttribute("type") === "measure"; if (isMeasure) { + archInfo.measures.push(fieldName); // the last field with type="measure" (if any) will be used as measure else __count archInfo.measure = fieldName; } else { diff --git a/addons/web/static/src/views/graph/graph_model.js b/addons/web/static/src/views/graph/graph_model.js index 323de10aa6a..35d047c29ef 100644 --- a/addons/web/static/src/views/graph/graph_model.js +++ b/addons/web/static/src/views/graph/graph_model.js @@ -182,6 +182,7 @@ export class GraphModel extends Model { this._normalize(metaData); metaData.measures = computeReportMeasures(metaData.fields, metaData.fieldAttrs, [ + ...(metaData.viewMeasures || []), metaData.measure, ]); diff --git a/addons/web/static/src/views/graph/graph_view.js b/addons/web/static/src/views/graph/graph_view.js index 4b76b37bdc0..08ebf7e4d70 100644 --- a/addons/web/static/src/views/graph/graph_view.js +++ b/addons/web/static/src/views/graph/graph_view.js @@ -37,6 +37,7 @@ export const graphView = { fields: fields, groupBy: archInfo.groupBy, measure: archInfo.measure || "__count", + viewMeasures: archInfo.measures, mode: archInfo.mode || "bar", order: archInfo.order || null, resModel: resModel, diff --git a/addons/web/static/tests/views/graph_view_tests.js b/addons/web/static/tests/views/graph_view_tests.js index 4d088a83bcf..7ebc16ca1a6 100644 --- a/addons/web/static/tests/views/graph_view_tests.js +++ b/addons/web/static/tests/views/graph_view_tests.js @@ -2440,7 +2440,7 @@ QUnit.module("Views", (hooks) => { QUnit.test("process default view description", async function (assert) { assert.expect(1); const propsFromArch = new GraphArchParser().parse(); - assert.deepEqual(propsFromArch, { fields: {}, fieldAttrs: {}, groupBy: [] }); + assert.deepEqual(propsFromArch, { fields: {}, fieldAttrs: {}, groupBy: [], measures: [] }); }); QUnit.test("process simple arch (no field tag)", async function (assert) { @@ -2454,6 +2454,7 @@ QUnit.module("Views", (hooks) => { fields, fieldAttrs: {}, groupBy: [], + measures: [], mode: "line", order: "ASC", }); @@ -2465,6 +2466,7 @@ QUnit.module("Views", (hooks) => { fields, fieldAttrs: {}, groupBy: [], + measures: [], stacked: false, title: "Title", }); @@ -2492,11 +2494,33 @@ QUnit.module("Views", (hooks) => { fighters: { string: "FooFighters" }, }, measure: "revenue", + measures: ["revenue"], groupBy: ["date:day", "foo"], mode: "pie", }); }); + QUnit.test("process arch with non stored field tags of type measure", async function (assert) { + assert.expect(1); + const fields = serverData.models.foo.fields; + fields.revenue.store = false; + const arch = ` + + + + + + `; + const propsFromArch = new GraphArchParser().parse(arch, fields); + assert.deepEqual(propsFromArch, { + fields, + fieldAttrs: {}, + measure: "foo", + measures: ["revenue", "foo"], + groupBy: ["product_id"], + }); + }); + QUnit.test("displaying chart data with three groupbys", async function (assert) { // this test makes sure the line chart shows all data labels (X axis) when // it is grouped by several fields @@ -3078,6 +3102,28 @@ QUnit.module("Views", (hooks) => { assert.strictEqual(getYAxeLabel(graph), "Product"); }); + QUnit.test( + "non store fields defined on the arch are present in the measures", + async function (assert) { + serverData.models.foo.fields.revenue.store = false; + await makeView({ + serverData, + type: "graph", + resModel: "foo", + arch: ` + + + + `, + }); + await toggleMenu(target, "Measures"); + assert.deepEqual( + Array.from(target.querySelectorAll(".o_menu_item")).map((e) => e.innerText.trim()), + ["Foo", "Revenue", "Count"] + ); + } + ); + QUnit.test('graph view "graph_measure" field in context', async function (assert) { assert.expect(6); const graph = await makeView({