From ca19b17fd3e09d2f4e53208542f4356d3a6ee25a Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Wed, 27 Mar 2019 13:38:46 +0000 Subject: [PATCH] [IMP] web: fetch records alongside read_group This rev. adds params to the 'web_read_group' method, allowing to search_read records inside each groups in a single RPC, instead of doing a 'web_read_group' RPC followed by n 'search_read' RPCs, where n is the number of opened groups. Part of task 1915702 --- addons/web/controllers/main.py | 20 +----- addons/web/models/models.py | 65 +++++++++++++++++-- addons/web/static/src/js/core/rpc.js | 7 ++ .../static/src/js/views/basic/basic_model.js | 46 +++++++++---- .../static/src/js/views/list/list_model.js | 6 +- .../web/static/tests/helpers/mock_server.js | 19 +++++- addons/web/static/tests/views/list_tests.js | 16 ++--- 7 files changed, 126 insertions(+), 53 deletions(-) diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index 98387b74106..4b78d022944 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -878,8 +878,7 @@ class DataSet(http.Controller): def search_read(self, model, fields=False, offset=0, limit=False, domain=None, sort=None): return self.do_search_read(model, fields, offset, limit, domain, sort) - def do_search_read(self, model, fields=False, offset=0, limit=False, domain=None - , sort=None): + def do_search_read(self, model, fields=False, offset=0, limit=False, domain=None, sort=None): """ Performs a search() followed by a read() (if needed) using the provided search criteria @@ -896,22 +895,7 @@ class DataSet(http.Controller): :rtype: list """ Model = request.env[model] - - records = Model.search_read(domain, fields, - offset=offset or 0, limit=limit or False, order=sort or False) - if not records: - return { - 'length': 0, - 'records': [] - } - if limit and len(records) == limit: - length = Model.search_count(domain) - else: - length = len(records) + (offset or 0) - return { - 'length': length, - 'records': records - } + return Model.web_search_read(domain, fields, offset=offset, limit=limit, order=sort) @http.route('/web/dataset/load', type='json', auth="user") def load(self, model, id, fields): diff --git a/addons/web/models/models.py b/addons/web/models/models.py index 271f8b831da..e85de4f6c45 100644 --- a/addons/web/models/models.py +++ b/addons/web/models/models.py @@ -18,11 +18,41 @@ class Base(models.AbstractModel): _inherit = 'base' @api.model - def web_read_group(self, domain, fields, groupby, limit=None, offset=0, orderby=False, - lazy=True): + def web_search_read(self, domain=None, fields=None, offset=0, limit=None, order=None): """ - Returns the result of a read_group and the total number of groups matching the search - domain. + Performs a search_read and a search_count. + + :param domain: search domain + :param fields: list of fields to read + :param limit: maximum number of records to read + :param offset: number of records to skip + :param order: columns to sort results + :return: { + 'records': array of read records (result of a call to 'search_read') + 'length': number of records matching the domain (result of a call to 'search_count') + } + """ + records = self.search_read(domain, fields, offset=offset, limit=limit, order=order) + if not records: + return { + 'length': 0, + 'records': [] + } + if limit and len(records) == limit: + length = self.search_count(domain) + else: + length = len(records) + offset + return { + 'length': length, + 'records': records + } + + @api.model + def web_read_group(self, domain, fields, groupby, limit=None, offset=0, orderby=False, + lazy=True, expand=False, expand_limit=None, expand_orderby=False): + """ + Returns the result of a read_group (and optionally search for and read records inside each + group), and the total number of groups matching the search domain. :param domain: search domain :param fields: list of fields to read (see ``fields``` param of ``read_group``) @@ -31,13 +61,16 @@ class Base(models.AbstractModel): :param offset: see ``offset`` param of ``read_group`` :param orderby: see ``orderby`` param of ``read_group`` :param lazy: see ``lazy`` param of ``read_group`` + :param expand: if true, and groupby only contains one field, read records inside each group + :param expand_limit: maximum number of records to read in each group + :param expand_orderby: order to apply when reading records in each group :return: { 'groups': array of read groups 'length': total number of groups } """ - groups = self.read_group(domain, fields, groupby, offset=offset, limit=limit, - orderby=orderby, lazy=lazy) + groups = self._web_read_group(domain, fields, groupby, limit, offset, orderby, lazy, expand, + expand_limit, expand_orderby) if not groups: length = 0 @@ -51,6 +84,26 @@ class Base(models.AbstractModel): 'length': length } + @api.model + def _web_read_group(self, domain, fields, groupby, limit=None, offset=0, orderby=False, + lazy=True, expand=False, expand_limit=None, expand_orderby=False): + """ + Performs a read_group and optionally a web_search_read for each group. + See ``web_read_group`` for params description. + + :returns: array of groups + """ + groups = self.read_group(domain, fields, groupby, offset=offset, limit=limit, + orderby=orderby, lazy=lazy) + + if expand and len(groupby) == 1: + for group in groups: + group['__data'] = self.web_search_read(domain=group['__domain'], fields=fields, + offset=0, limit=expand_limit, + order=expand_orderby) + + return groups + @api.model def read_progress_bar(self, domain, group_by, progress_bar): """ diff --git a/addons/web/static/src/js/core/rpc.js b/addons/web/static/src/js/core/rpc.js index 802f7082cad..b09f7542b5c 100644 --- a/addons/web/static/src/js/core/rpc.js +++ b/addons/web/static/src/js/core/rpc.js @@ -68,6 +68,13 @@ return { orderBy = options.orderBy || params.orderBy || params.kwargs.orderby; params.kwargs.orderby = orderBy ? this._serializeSort(orderBy) : orderBy; params.kwargs.lazy = 'lazy' in options ? options.lazy : params.lazy; + + if (options.method === 'web_read_group') { + params.kwargs.expand = options.expand || params.expand || params.kwargs.expand; + params.kwargs.expand_limit = options.expand_limit || params.expand_limit || params.kwargs.expand_limit; + var expandOrderBy = options.expand_orderby || params.expand_orderby || params.kwargs.expand_orderby; + params.kwargs.expand_orderby = expandOrderBy ? this._serializeSort(expandOrderBy) : expandOrderBy; + } } if (options.method === 'search_read') { diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index d07ec61abdd..804e6d6d1c9 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -4247,6 +4247,7 @@ var BasicModel = AbstractModel.extend({ */ _readGroup: function (list, options) { var self = this; + options = options || {}; var groupByField = list.groupedBy[0]; var rawGroupBy = groupByField.split(':')[0]; var fields = _.uniq(list.getFieldNames().concat(rawGroupBy)); @@ -4254,6 +4255,7 @@ var BasicModel = AbstractModel.extend({ return order.name === rawGroupBy || list.fields[order.name].group_operator !== undefined; }); var openGroupsLimit = list.groupsLimit || self.OPEN_GROUP_LIMIT; + var expand = list.openGroupByDefault && options.fetchRecordsWithGroups; return this._rpc({ model: list.model, method: 'web_read_group', @@ -4265,6 +4267,9 @@ var BasicModel = AbstractModel.extend({ offset: list.groupsOffset, orderBy: orderedBy, lazy: true, + expand: expand, + expand_limit: expand ? list.limit : null, + expand_orderby: expand ? list.orderedBy : null, }) .then(function (result) { var groups = result.groups; @@ -4318,7 +4323,7 @@ var BasicModel = AbstractModel.extend({ delete self.localData[newGroup.id]; // restore the internal state of the group var updatedProps = _.pick(oldGroup, 'isOpen', 'offset', 'id'); - if (options && options.onlyGroups || oldGroup.isOpen && newGroup.groupedBy.length) { + if (options.onlyGroups || oldGroup.isOpen && newGroup.groupedBy.length) { // If the group is opened and contains subgroups, // also keep its data to keep internal state of // sub-groups @@ -4343,11 +4348,16 @@ var BasicModel = AbstractModel.extend({ list.count += newGroup.count; if (newGroup.isOpen && newGroup.count > 0) { openGroupCount++; + if (group.__data) { + // bypass the search_read when the group's records have been obtained + // by the call to 'web_read_group' (see @_searchReadUngroupedList) + newGroup.__data = group.__data; + } options = _.defaults({enableRelationalFetch: false}, options); defs.push(self._load(newGroup, options)); } }); - if (options && options.keepEmptyGroups) { + if (options.keepEmptyGroups) { // Find the groups that were available in a previous // readGroup but are not there anymore. // Note that these groups are put after existing groups so @@ -4363,7 +4373,7 @@ var BasicModel = AbstractModel.extend({ } return Promise.all(defs).then(function (groups) { - if (!options || !options.onlyGroups) { + if (!options.onlyGroups) { // generate the res_ids of the main list, being the concatenation // of the fetched res_ids in each group list.res_ids = _.flatten(_.map(groups, function (group) { @@ -4552,17 +4562,25 @@ var BasicModel = AbstractModel.extend({ _searchReadUngroupedList: function (list) { var self = this; var fieldNames = list.getFieldNames(); - return this._rpc({ - route: '/web/dataset/search_read', - model: list.model, - fields: fieldNames, - context: list.getContext(), - domain: list.domain || [], - limit: list.limit, - offset: list.loadMoreOffset + list.offset, - orderBy: list.orderedBy, - }) - .then(function (result) { + var prom; + if (list.__data) { + // the data have already been fetched (alonside the groups by the + // call to 'web_read_group'), so we can bypass the search_read + prom = Promise.resolve(list.__data); + } else { + prom = this._rpc({ + route: '/web/dataset/search_read', + model: list.model, + fields: fieldNames, + context: list.getContext(), + domain: list.domain || [], + limit: list.limit, + offset: list.loadMoreOffset + list.offset, + orderBy: list.orderedBy, + }); + } + return prom.then(function (result) { + delete list.__data; list.count = result.length; var ids = _.pluck(result.records, 'id'); var data = _.map(result.records, function (record) { diff --git a/addons/web/static/src/js/views/list/list_model.js b/addons/web/static/src/js/views/list/list_model.js index 73c2d86864d..41178498c96 100644 --- a/addons/web/static/src/js/views/list/list_model.js +++ b/addons/web/static/src/js/views/list/list_model.js @@ -89,9 +89,11 @@ odoo.define('web.ListModel', function (require) { * @override * @private */ - _readGroup: function (list) { + _readGroup: function (list, options) { var self = this; - return this._super.apply(this, arguments).then(function (result) { + options = options || {}; + options.fetchRecordsWithGroups = true; + return this._super(list, options).then(function (result) { var prom; if (!list.parentID) { // groupbys buttons are only displayed on the first level of diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index 0eb54bd3e0b..b1427ecb6f4 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -1209,9 +1209,26 @@ var MockServer = Class.extend({ * @param {boolean} kwargs.lazy still mostly ignored * @param {integer} [kwargs.limit] * @param {integer} [kwargs.offset] + * @param {boolean} [kwargs.expand=false] if true, read records inside each + * group + * @param {integer} [kwargs.expand_limit] + * @param {integer} [kwargs.expand_orderby] * @returns {Object[]} */ _mockWebReadGroup: function (model, kwargs) { + var self = this; + var groups = this._mockReadGroup(model, kwargs); + if (kwargs.expand && kwargs.groupby.length === 1) { + groups.forEach(function (group) { + group.__data = self._mockSearchReadController({ + domain: group.__domain, + model: model, + fields: kwargs.fields, + limit: kwargs.expand_limit, + order: kwargs.expand_orderby, + }); + }); + } var allGroups = this._mockReadGroup(model, { domain: kwargs.domain, fields: ['display_name'], @@ -1219,7 +1236,7 @@ var MockServer = Class.extend({ lazy: kwargs.lazy, }); return { - groups: this._mockReadGroup(model, kwargs), + groups: groups, length: allGroups.length, }; }, diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index d4efb3154a4..a275f3602f7 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -4471,7 +4471,7 @@ QUnit.module('Views', { }); QUnit.test('grouped list with expand attribute', async function (assert) { - assert.expect(6); + assert.expect(5); var list = await createView({ View: ListView, @@ -4487,11 +4487,10 @@ QUnit.module('Views', { assert.containsN(list, '.o_group_header', 2); assert.containsN(list, '.o_data_row', 4); + assert.strictEqual(list.$('.o_data_cell').text(), 'yopblipgnapblip'); assert.verifySteps([ - 'web_read_group', - '/web/dataset/search_read', - '/web/dataset/search_read', + 'web_read_group', // records are fetched alongside groups ]); list.destroy(); @@ -4525,13 +4524,12 @@ QUnit.module('Views', { }); QUnit.test('grouped lists with expand attribute and a lot of groups', async function (assert) { - assert.expect(10); + assert.expect(8); for (var i = 0; i < 15; i++) { this.data.foo.records.push({foo: 'record ' + i, int_field: i}); } - var nbSearchRead = 0; var list = await createView({ View: ListView, model: 'foo', @@ -4542,24 +4540,18 @@ QUnit.module('Views', { if (args.method === 'web_read_group') { assert.step(args.method); } - if (route === '/web/dataset/search_read') { - nbSearchRead++; - } return this._super.apply(this, arguments); }, }); assert.containsN(list, '.o_group_header', 10); // page 1 assert.containsN(list, '.o_data_row', 11); // one group contains two records - assert.strictEqual(nbSearchRead, 10); assert.containsOnce(list, '.o_pager_counter'); // has a pager - nbSearchRead = 0; await testUtils.dom.click(list.$('.o_pager_next')); // switch to page 2 assert.containsN(list, '.o_group_header', 7); // page 2 assert.containsN(list, '.o_data_row', 7); - assert.strictEqual(nbSearchRead, 7); assert.verifySteps([ 'web_read_group', // read_group page 1