From 39ecfcff49cdcd1a246c0668f7e7bedbb4a2a703 Mon Sep 17 00:00:00 2001 From: FrancoisGe Date: Tue, 4 Oct 2022 06:34:18 +0000 Subject: [PATCH] [FIX] web: sort an open group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit solves two bugs: 1. In a grouped empty list view, if you click on a sortable column, then a crash is displayed. How to reproduce: - Go to a grouped empty list view with at least one sortable column - Click on the sortable column Before this commit : A crash is displayed After this commit: Nothing happens. 2. In a grouped view with at least one open group, columns that do not have an aggregates value cannot be sorted. How to reproduce: - Go to a grouped list view with at least one open group. - Click on a sortable column that does not have an aggregates value Before this commit: Nothing happens After this commit: The records are sorted by the clicked column. closes odoo/odoo#102143 X-original-commit: 537bbc79fe0ce34fe8b4746bf4cc86f9ebfc8a12 Signed-off-by: Mathieu Duckerts-Antoine Signed-off-by: Georis François (fge) --- .../static/src/views/list/list_renderer.js | 10 +- .../web/static/src/views/relational_model.js | 20 ++ .../web/static/tests/views/list_view_tests.js | 212 +++++++++++++++--- .../select_create_dialog_tests.js | 1 + 4 files changed, 197 insertions(+), 46 deletions(-) diff --git a/addons/web/static/src/views/list/list_renderer.js b/addons/web/static/src/views/list/list_renderer.js index a64533673d4..abd7ba100b2 100644 --- a/addons/web/static/src/views/list/list_renderer.js +++ b/addons/web/static/src/views/list/list_renderer.js @@ -834,15 +834,7 @@ export class ListRenderer extends Component { const fieldName = column.name; const list = this.props.list; if (this.isSortable(column)) { - if (list.isGrouped) { - const isSortable = - list.groups[0].getAggregates(fieldName) || list.groupBy.includes(fieldName); - if (isSortable) { - list.sortBy(fieldName); - } - } else { - list.sortBy(fieldName); - } + list.sortBy(fieldName); } } diff --git a/addons/web/static/src/views/relational_model.js b/addons/web/static/src/views/relational_model.js index 988886c9afa..af31a45ce4c 100644 --- a/addons/web/static/src/views/relational_model.js +++ b/addons/web/static/src/views/relational_model.js @@ -2216,6 +2216,11 @@ export class DynamicGroupList extends DynamicList { return false; } + hasAggregate(fieldName) { + const group = this.groups[0]; + return group && fieldName in group.aggregates; + } + async load(params = {}) { this.limit = params.limit === undefined ? this.limit : params.limit; this.offset = params.offset === undefined ? this.offset : params.offset; @@ -2290,6 +2295,20 @@ export class DynamicGroupList extends DynamicList { this.model.notify(); } + async sortBy(fieldName) { + if (!this.groups.length) { + return; + } + const everyGroupIsClosed = this.groups.every((group) => group.isFolded); + if ( + everyGroupIsClosed && + !(this.groupBy.includes(fieldName) || this.hasAggregate(fieldName)) + ) { + return; + } + super.sortBy(fieldName); + } + // ------------------------------------------------------------------------ // Protected // ------------------------------------------------------------------------ @@ -2361,6 +2380,7 @@ export class DynamicGroupList extends DynamicList { orderby, lazy: true, expand: this.expand, + expand_orderby: this.expand ? orderByToString(this.orderBy) : null, offset: this.offset, limit: this.limit, context: this.context, diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js index 3f95e530fdf..88b274f571f 100644 --- a/addons/web/static/tests/views/list_view_tests.js +++ b/addons/web/static/tests/views/list_view_tests.js @@ -3082,6 +3082,33 @@ QUnit.module("Views", (hooks) => { ); }); + QUnit.test( + "groups can't be sorted on aggregates if there is no record", + async function (assert) { + serverData.models.foo.records = []; + + await makeView({ + type: "list", + resModel: "foo", + serverData, + groupBy: ["foo"], + arch: ` + + + + `, + mockRPC(route, args) { + if (args.method === "web_read_group") { + assert.step(args.kwargs.orderby || "default order"); + } + }, + }); + + await click(target, ".o_column_sortable"); + assert.verifySteps(["default order"]); + } + ); + QUnit.test("groups can be sorted on aggregates", async function (assert) { await makeView({ type: "list", @@ -3134,48 +3161,159 @@ QUnit.module("Views", (hooks) => { assert.verifySteps(["default order", "int_field ASC", "int_field DESC"]); }); - QUnit.test("groups cannot be sorted on non-aggregable fields", async function (assert) { - serverData.models.foo.fields.sort_field = { - string: "sortable_field", - type: "sting", - sortable: true, - default: "value", - }; - _.each(serverData.models.records, function (elem) { - elem.sort_field = "value" + elem.id; - }); - serverData.models.foo.fields.foo.sortable = true; - await makeView({ - type: "list", - resModel: "foo", - serverData, - groupBy: ["foo"], - arch: ` + QUnit.test( + "groups cannot be sorted on non-aggregable fields if every group is folded", + async function (assert) { + serverData.models.foo.fields.sort_field = { + string: "sortable_field", + type: "sting", + sortable: true, + default: "value", + }; + serverData.models.foo.records.forEach((elem) => { + elem.sort_field = "value" + elem.id; + }); + serverData.models.foo.fields.foo.sortable = true; + await makeView({ + type: "list", + resModel: "foo", + serverData, + groupBy: ["foo"], + arch: ` `, - mockRPC(route, args) { - if (args.method === "web_read_group") { - assert.step(args.kwargs.orderby || "default order"); - } - }, - }); - assert.verifySteps(["default order"]); - //we cannot sort by sort_field since it doesn't have a group_operator - await click(target.querySelectorAll(".o_column_sortable")[2]); - assert.verifySteps([]); - //we can sort by int_field since it has a group_operator - await click(target.querySelectorAll(".o_column_sortable")[1]); - assert.verifySteps(["int_field ASC"]); - //we keep previous order - await click(target.querySelectorAll(".o_column_sortable")[2]); - assert.verifySteps([]); - //we can sort on foo since we are groupped by foo + previous order - await click(target.querySelectorAll(".o_column_sortable")[0]); - assert.verifySteps(["foo ASC, int_field ASC"]); - }); + mockRPC(route, args) { + if (args.method === "web_read_group") { + assert.step(args.kwargs.orderby || "default order"); + } + }, + }); + assert.verifySteps(["default order"]); + + // we cannot sort by sort_field since it doesn't have a group_operator + await click(target.querySelector(".o_column_sortable[data-name='sort_field']")); + assert.verifySteps([]); + + // we can sort by int_field since it has a group_operator + await click(target.querySelector(".o_column_sortable[data-name='int_field']")); + assert.verifySteps(["int_field ASC"]); + + // we keep previous order + await click(target.querySelector(".o_column_sortable[data-name='sort_field']")); + assert.verifySteps([]); + + // we can sort on foo since we are groupped by foo + previous order + await click(target.querySelector(".o_column_sortable[data-name='foo']")); + assert.verifySteps(["foo ASC, int_field ASC"]); + } + ); + + QUnit.test( + "groups can be sorted on non-aggregable fields if a group isn't folded", + async function (assert) { + serverData.models.foo.fields.foo.sortable = true; + await makeView({ + type: "list", + resModel: "foo", + serverData, + groupBy: ["bar"], + arch: ` + + + `, + mockRPC(route, args) { + const { method } = args; + if (method === "web_read_group") { + assert.step( + `web_read_group.orderby: ${args.kwargs.orderby || "default order"}` + ); + assert.step( + `web_read_group.expand_orderby: ${ + args.kwargs.expand_orderby || "default order" + }` + ); + } + if (method === "web_search_read") { + assert.step( + `web_search_read.order: ${args.kwargs.order || "default order"}` + ); + } + }, + }); + await click(target.querySelectorAll(".o_group_header")[1]); + assert.deepEqual( + getNodesTextContent(target.querySelectorAll(".o_data_cell[name='foo']")), + ["yop", "blip", "gnap"] + ); + assert.verifySteps([ + "web_read_group.orderby: default order", + "web_read_group.expand_orderby: default order", + "web_search_read.order: default order", + ]); + + await click(target.querySelector(".o_column_sortable[data-name='foo']")); + assert.deepEqual( + getNodesTextContent(target.querySelectorAll(".o_data_cell[name='foo']")), + ["blip", "gnap", "yop"] + ); + assert.verifySteps([ + "web_read_group.orderby: default order", + "web_read_group.expand_orderby: default order", + "web_search_read.order: foo ASC", + ]); + } + ); + + QUnit.test( + "groups can be sorted on non-aggregable fields if a group isn't folded with expand='1'", + async function (assert) { + serverData.models.foo.fields.foo.sortable = true; + await makeView({ + type: "list", + resModel: "foo", + serverData, + groupBy: ["bar"], + arch: ` + + + `, + mockRPC(route, args) { + const { method } = args; + if (method === "web_read_group") { + assert.step( + `web_read_group.orderby: ${args.kwargs.orderby || "default order"}` + ); + assert.step( + `web_read_group.expand_orderby: ${ + args.kwargs.expand_orderby || "default order" + }` + ); + } + }, + }); + assert.deepEqual( + getNodesTextContent(target.querySelectorAll(".o_data_cell[name='foo']")), + ["blip", "yop", "blip", "gnap"] + ); + assert.verifySteps([ + "web_read_group.orderby: default order", + "web_read_group.expand_orderby: default order", + ]); + + await click(target.querySelector(".o_column_sortable[data-name='foo']")); + assert.deepEqual( + getNodesTextContent(target.querySelectorAll(".o_data_cell[name='foo']")), + ["blip", "blip", "gnap", "yop"] + ); + assert.verifySteps([ + "web_read_group.orderby: default order", + "web_read_group.expand_orderby: foo ASC", + ]); + } + ); QUnit.test("properly apply onchange in simple case", async function (assert) { serverData.models.foo.onchanges = { diff --git a/addons/web/static/tests/views/view_dialogs/select_create_dialog_tests.js b/addons/web/static/tests/views/view_dialogs/select_create_dialog_tests.js index 1445e94dd04..514719a486e 100644 --- a/addons/web/static/tests/views/view_dialogs/select_create_dialog_tests.js +++ b/addons/web/static/tests/views/view_dialogs/select_create_dialog_tests.js @@ -126,6 +126,7 @@ QUnit.module("ViewDialogs", (hooks) => { groupby: ["bar"], orderby: "", expand: false, + expand_orderby: null, lazy: true, limit: 80, offset: 0,