From baeb1a1938eee92ed810a6d6a7b7feed4f84acc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Mon, 5 Nov 2018 13:16:41 +0000 Subject: [PATCH 1/2] [FIX] web: accept field name as a groupby descriptor In some cases, and for historical reasons, the groupby property can be expressed as a string (for example, 'stage_id') instead of a list of strings (for example, ['stage_id']). However, the support was not complete. A crash could happen in some cases. For example, if there is a group_by: 'stage_id' property in the context of an action, it works in list view, but adding that view to the dashboard would result in a crash, without this commit. --- .../web/static/src/js/views/abstract_view.js | 7 ++++- .../static/tests/views/abstract_view_tests.js | 30 +++++++++++++++++++ addons/web/static/tests/views/list_tests.js | 1 + 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/addons/web/static/src/js/views/abstract_view.js b/addons/web/static/src/js/views/abstract_view.js index f3277d394a3..7fca3b83ff3 100644 --- a/addons/web/static/src/js/views/abstract_view.js +++ b/addons/web/static/src/js/views/abstract_view.js @@ -162,6 +162,11 @@ var AbstractView = Class.extend({ this.controllerParams.withControlPanel = params.withControlPanel; } + var groupBy = params.groupBy; + if (typeof groupBy === 'string') { + groupBy = [groupBy]; + } + this.loadParams = { context: params.context, count: params.count || ((this.controllerParams.ids !== undefined) && @@ -172,7 +177,7 @@ var AbstractView = Class.extend({ comparisonTimeRange: comparisonTimeRange, comparisonTimeRangeDescription: comparisonTimeRangeDescription, compare: compare, - groupedBy: params.groupBy, + groupedBy: groupBy, modelName: params.modelName, res_id: params.currentId, res_ids: params.ids, diff --git a/addons/web/static/tests/views/abstract_view_tests.js b/addons/web/static/tests/views/abstract_view_tests.js index b958550a8e1..01f1daafeb4 100644 --- a/addons/web/static/tests/views/abstract_view_tests.js +++ b/addons/web/static/tests/views/abstract_view_tests.js @@ -3,6 +3,7 @@ odoo.define('web.abstract_view_tests', function (require) { var AbstractView = require('web.AbstractView'); var ajax = require('web.ajax'); +var ListView = require('web.ListView'); var testUtils = require('web.test_utils'); var createAsyncView = testUtils.createAsyncView; @@ -14,6 +15,16 @@ QUnit.module('Views', { fields: {}, record: [], }, + foo: { + fields: { + foo: {string: "Foo", type: "char"}, + bar: {string: "Bar", type: "boolean"}, + }, + records: [ + {id: 1, bar: true, foo: "yop"}, + {id: 2, bar: true, foo: "blip"}, + ] + }, }; }, }, function () { @@ -100,5 +111,24 @@ QUnit.module('Views', { "should load 'c' when 'a' and 'b' are loaded"); defs.c.resolve(); }); + + QUnit.test('groupBy attribute can be a string, instead of a list of strings', function (assert) { + assert.expect(2); + + var list = testUtils.createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '', + groupBy: 'bar', + mockRPC: function (route, args) { + assert.strictEqual(args.method, 'read_group'); + assert.deepEqual(args.kwargs.groupby, ['bar']); + return this._super.apply(this, arguments); + }, + }); + list.destroy(); + }); + }); }); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 5a0d00b6f6d..4cd4b58ea9e 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -307,6 +307,7 @@ QUnit.module('Views', { assert.strictEqual(list.$('th:contains(Bar)').length, 1, "should contain Bar"); assert.strictEqual(list.$('tr.o_group_header').length, 2, "should have 2 .o_group_header"); assert.strictEqual(list.$('th.o_group_name').length, 2, "should have 2 .o_group_name"); + list.destroy(); }); QUnit.test('basic grouped list rendering 1 col without selector', function (assert) { From 6e9c1d92d7484c66735d9abc58e09b0e88fac57d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Tue, 6 Nov 2018 08:36:15 +0000 Subject: [PATCH 2/2] [FIX] board: use correct context in sub board views Before commit 60f143c8e7b7ea2387031c968dfa0f31a288afb2, the dashboard parameters did not use the parameters from the action, so some important information could be lost. However, in this same commit, the action context overrides the context from the user defined views, which means that some information is lost. For example, customized group bys are ignored. This commit fixes the issue with some careful context manipulation. Some special care was done to remove the groupby from the action context if it is not empty, because that same logic is used in views. --- addons/board/static/src/js/board_view.js | 12 ++++--- addons/board/static/tests/dashboard_tests.js | 38 ++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/addons/board/static/src/js/board_view.js b/addons/board/static/src/js/board_view.js index 1cf27c79fe8..946cdff859f 100644 --- a/addons/board/static/src/js/board_view.js +++ b/addons/board/static/src/js/board_view.js @@ -247,10 +247,14 @@ var BoardRenderer = FormRenderer.extend({ // the action does not exist anymore return $.when(); } + var evalContext = new Context(params.context).eval(); + if (evalContext.group_by && evalContext.group_by.length === 0) { + delete evalContext.group_by; + } // tz and lang are saved in the custom view - // override the language to take the current one - var rawContext = new Context(params.context, action.context, {lang: session.user_context.lang}); - var context = pyUtils.eval('context', rawContext); + // override the https://github.com/odoo/odoo/pull/28408language to take the current one + var rawContext = new Context(action.context, evalContext, {lang: session.user_context.lang}); + var context = pyUtils.eval('context', rawContext, evalContext); var domain = params.domain || pyUtils.eval('domain', action.domain || '[]', action.context); var viewType = params.viewType || action.views[0][1]; var view = _.find(action.views, function (descr) { @@ -332,7 +336,7 @@ var BoardRenderer = FormRenderer.extend({ self.defs.push(self._createController({ $node: $html.find('.oe_action[data-id=' + action.id + '] .oe_content'), actionID: _.str.toNumber(action.name), - context: new Context(action.context), + context: action.context, domain: Domain.prototype.stringToArray(action.domain, {}), viewType: action.view_mode, })); diff --git a/addons/board/static/tests/dashboard_tests.js b/addons/board/static/tests/dashboard_tests.js index 42fb870458f..1e17ca71db1 100644 --- a/addons/board/static/tests/dashboard_tests.js +++ b/addons/board/static/tests/dashboard_tests.js @@ -24,6 +24,7 @@ QUnit.module('Dashboard', { fields: { display_name: {string: "Displayed name", type: "char", searchable: true}, foo: {string: "Foo", type: "char", default: "My little Foo Value", searchable: true}, + bar: {string: "Bar", type: "boolean"}, }, records: [{ id: 1, @@ -787,4 +788,41 @@ QUnit.test("Views should be loaded in the user's language", function (assert) { form.destroy(); }); +QUnit.test("Dashboard should use correct groupby", function (assert) { + assert.expect(1); + var form = createView({ + View: BoardView, + model: 'board', + data: this.data, + arch: '
' + + '' + + '' + + '' + + '' + + '' + + '
', + mockRPC: function (route, args) { + if (args.method === 'read_group') { + assert.deepEqual(args.kwargs.groupby, ['bar'], + 'user defined groupby should have precedence on action groupby'); + } + if (route === '/web/action/load') { + return $.when({ + res_model: 'partner', + context: { + group_by: 'some_field', + }, + views: [[4, 'list']], + }); + } + return this._super.apply(this, arguments); + }, + archs: { + 'partner,4,list': + '', + }, + }); + + form.destroy(); +}); });