From 4aa85d304a936cfc62e61b915186f1cb2a32042c Mon Sep 17 00:00:00 2001 From: Mathieu Duckerts-Antoine Date: Tue, 9 Nov 2021 15:55:49 +0000 Subject: [PATCH] [FIX] web: pivot: keep subTrees in comparison mode Before this commit, when comparing a period P of reference to another one, some rows with only data comming from P would not always be presented in the table. The problem used to manifest only when loading at least to level at the same time (starting from Total or any header via expand all). The reason is the following: - the data comming from P are processed first: starting from somewhere in the trees of headers, some nodes linked to the data comming from P are progressively created with no children at first, then the children (if any) are added later and so on progressively. - the data comming from the other period is processed and nodes are created with no children and so on in the same way, clearing possibly all the node children comming from P. For example, with a row groupby = [field_a, field_b], a group [v, v1] from P a group [v, v2] from the other period, a node corresponding to v would be added, then a node child for v1. Then the node for v would be recreated with no children, then a node child for v2 would be created. leading to a tree of row headers of the form Total > v > v2. Note that it was not affecting the measure values for the displayed rows since those measures are stored separately from the tree and are linked to the nodes via some ids. We fix the problem by avoiding the recreation of a node if it already exists. opw-2635113 closes odoo/odoo#80468 X-original-commit: 364475b5da3d4aaa87db715f0c0bbaa9dd3e05c8 Signed-off-by: Aaron Bohy (aab) Signed-off-by: Mathieu Duckerts-Antoine Co-authored-by: Mathieu Duckerts-Antoine Co-authored-by: Andrea Grazioso --- .../src/legacy/js/views/pivot/pivot_model.js | 6 +- .../web/static/src/views/pivot/pivot_model.js | 6 +- .../static/tests/legacy/views/pivot_tests.js | 64 +++++++++++++++++++ .../static/tests/views/pivot_view_tests.js | 56 ++++++++++++++++ 4 files changed, 130 insertions(+), 2 deletions(-) diff --git a/addons/web/static/src/legacy/js/views/pivot/pivot_model.js b/addons/web/static/src/legacy/js/views/pivot/pivot_model.js index 72296444aa4..6fc3f6bc136 100644 --- a/addons/web/static/src/legacy/js/views/pivot/pivot_model.js +++ b/addons/web/static/src/legacy/js/views/pivot/pivot_model.js @@ -719,7 +719,11 @@ var PivotModel = AbstractModel.extend({ values.slice(0, values.length - 1).forEach(function (value) { tree = tree.directSubTrees.get(value); }); - tree.directSubTrees.set(values[values.length - 1], { + const value = values[values.length - 1]; + if (tree.directSubTrees.has(value)) { + return; + } + tree.directSubTrees.set(value, { root: { labels: labels, values: values, diff --git a/addons/web/static/src/views/pivot/pivot_model.js b/addons/web/static/src/views/pivot/pivot_model.js index fe30557458b..fb3b353773d 100644 --- a/addons/web/static/src/views/pivot/pivot_model.js +++ b/addons/web/static/src/views/pivot/pivot_model.js @@ -779,7 +779,11 @@ export class PivotModel extends Model { values.slice(0, values.length - 1).forEach(function (value) { tree = tree.directSubTrees.get(value); }); - tree.directSubTrees.set(values[values.length - 1], { + const value = values[values.length - 1]; + if (tree.directSubTrees.has(value)) { + return; + } + tree.directSubTrees.set(value, { root: { labels: labels, values: values, diff --git a/addons/web/static/tests/legacy/views/pivot_tests.js b/addons/web/static/tests/legacy/views/pivot_tests.js index 8a85ea42361..36ecf32b5b8 100644 --- a/addons/web/static/tests/legacy/views/pivot_tests.js +++ b/addons/web/static/tests/legacy/views/pivot_tests.js @@ -3434,6 +3434,70 @@ QUnit.module('Views', { pivot.destroy(); }); + QUnit.test('comparison with two groupbys: rows from reference period should be displayed', async function (assert) { + assert.expect(3); + + this.data.partner.records = [ + { id: 1, date: "2021-10-10", product_id: 1, customer: 1 }, + { id: 2, date: "2020-10-10", product_id: 2, customer: 1 }, + ] + this.data.product.records = [ + { id: 1, display_name: "A" }, + { id: 2, display_name: "B" }, + ] + this.data.customer.records = [ + { id: 1, display_name: "P" }, + ] + + const pivot = await createView({ + View: PivotView, + model: "partner", + data: this.data, + arch: '', + archs: { + "partner,false,search": "" + }, + }); + + // compare 2021 to 2020 + await cpHelpers.toggleFilterMenu(pivot.el); + await cpHelpers.toggleMenuItem(pivot.el, "Date"); + await cpHelpers.toggleMenuItemOption(pivot.el, "Date", "2021"); + await cpHelpers.toggleComparisonMenu(pivot.el); + await cpHelpers.toggleMenuItem(pivot.el, 0); + + assert.strictEqual( + pivot.$('th').slice(0, 6).text(), + [ + "Total", + "Count", + "2020", "2021", "Variation" + ].join(''), + "The col headers should be as expected" + ); + + assert.strictEqual( + pivot.$('th').slice(6).text(), + [ + 'Total', + 'P', + 'B', + 'A', + ].join(''), + "The row headers should be as expected" + ); + + const values = [ + "1", "1", "0%", + "1", "1", "0%", + "1", "0", "-100%", + "0", "1", "100%", + ]; + assert.strictEqual(getCurrentValues(pivot), values.join()); + + pivot.destroy(); + }); + QUnit.test('pivot rendering with boolean field', async function (assert) { assert.expect(4); diff --git a/addons/web/static/tests/views/pivot_view_tests.js b/addons/web/static/tests/views/pivot_view_tests.js index f08ce150816..2ca8c08afa5 100644 --- a/addons/web/static/tests/views/pivot_view_tests.js +++ b/addons/web/static/tests/views/pivot_view_tests.js @@ -5335,4 +5335,60 @@ QUnit.module("Views", (hooks) => { // No separator should be displayed in the menu "Measures" assert.containsNone(pivot, ".o_cp_bottom_left .dropdown-menu div.dropdown-divider"); }); + + QUnit.test( + "comparison with two groupbys: rows from reference period should be displayed", + async function (assert) { + assert.expect(3); + + serverData.models.partner.records = [ + { id: 1, date: "2021-10-10", product_id: 1, customer: 1 }, + { id: 2, date: "2020-10-10", product_id: 2, customer: 1 }, + ]; + serverData.models.product.records = [ + { id: 1, display_name: "A" }, + { id: 2, display_name: "B" }, + ]; + serverData.models.customer.records = [{ id: 1, display_name: "P" }]; + + const pivot = await makeView({ + type: "pivot", + resModel: "partner", + serverData, + arch: ` + + + + + `, + searchViewArch: ` + + + + `, + }); + + // compare 2021 to 2020 + await toggleFilterMenu(pivot); + await toggleMenuItem(pivot, "Date"); + await toggleMenuItemOption(pivot, "Date", "2021"); + await toggleComparisonMenu(pivot); + await toggleMenuItem(pivot, 0); + + assert.deepEqual( + [...pivot.el.querySelectorAll("th")].slice(0, 6).map((el) => el.innerText), + ["", "Total", "Count", "2020", "2021", "Variation"], + "The col headers should be as expected" + ); + + assert.deepEqual( + [...pivot.el.querySelectorAll("th")].slice(6).map((el) => el.innerText), + ["Total", "P", "B", "A"], + "The row headers should be as expected" + ); + + const values = ["1", "1", "0%", "1", "1", "0%", "1", "0", "-100%", "0", "1", "100%"]; + assert.strictEqual(getCurrentValues(pivot), values.join()); + } + ); });