From 3306a0f41492ee51e93ce9cedcd42296e09df6a0 Mon Sep 17 00:00:00 2001 From: Mathieu Duckerts-Antoine Date: Fri, 8 May 2020 08:57:44 +0000 Subject: [PATCH] [IMP] *: limit in search panel This commit introduces a new attribute 'limit' for search panel fields that allows to avoid performance issues. That integer attribute (with default 200) allows to fix a maximal number of values to display for the fields. When the number of field values to display reaches the limit, no values will be displayed. Instead, a warning message will be shown in the corresponding search panel section. Note it is possible to have no limit using limit="0" on a field. This commit reintroduces in a better way the principle brought by the fix 8d57153b34c04952a85f6642951cb2697016da84. Task ID: 2154749 --- .../account/views/account_account_views.xml | 2 +- addons/account/views/account_move_views.xml | 2 +- addons/web/models/models.py | 83 +++++++++----- .../web/static/src/js/views/search_panel.js | 73 ++++++++----- addons/web/static/src/xml/kanban.xml | 10 +- .../web/static/tests/helpers/mock_server.js | 88 +++++++++++---- .../static/tests/views/search_panel_tests.js | 77 +++++++++++++ doc/reference/views.rst | 5 + odoo/addons/base/rng/common.rng | 1 + .../test_search_panel_select_multi_range.py | 102 +++++++++++------- .../tests/test_search_panel_select_range.py | 54 ++++++++++ 11 files changed, 381 insertions(+), 116 deletions(-) diff --git a/addons/account/views/account_account_views.xml b/addons/account/views/account_account_views.xml index 7bd0635f47a..257e0ee169e 100644 --- a/addons/account/views/account_account_views.xml +++ b/addons/account/views/account_account_views.xml @@ -119,7 +119,7 @@ diff --git a/addons/account/views/account_move_views.xml b/addons/account/views/account_move_views.xml index aa5fefbfc44..503fbbdf346 100644 --- a/addons/account/views/account_move_views.xml +++ b/addons/account/views/account_move_views.xml @@ -368,7 +368,7 @@ diff --git a/addons/web/models/models.py b/addons/web/models/models.py index 99ea40c2caf..ddb99b2ef72 100644 --- a/addons/web/models/models.py +++ b/addons/web/models/models.py @@ -12,6 +12,7 @@ from odoo.tools.misc import get_lang from odoo.exceptions import UserError from collections import defaultdict +SEARCH_PANEL_ERROR_MESSAGE = "Too many items to display." class lazymapping(defaultdict): def __missing__(self, key): @@ -223,13 +224,14 @@ class Base(models.AbstractModel): @api.model - def _search_panel_domain_image(self, field_name, domain, set_count=False): + def _search_panel_domain_image(self, field_name, domain, set_count=False, limit=False): """ Return the values in the image of the provided domain by field_name. :param domain: domain whose image is returned :param field_name: the name of a field (type many2one or selection) :param set_count: whether to set the key '__count' in image values. Default is False. + :param limit: integer, maximal number of values to fetch. Default is False. :return: a dict of the form { id: { 'id': id, 'display_name': display_name, ('__count': c,) }, @@ -253,7 +255,8 @@ class Base(models.AbstractModel): domain, [(field_name, '!=', False)], ]) - groups = self.read_group(domain, [field_name], [field_name]) + groups = self.read_group(domain, [field_name], [field_name], limit=limit) + domain_image = {} for group in groups: id, display_name = group_id_name(group[field_name]) @@ -312,7 +315,7 @@ class Base(models.AbstractModel): { 'id': id, parent_name: False or (id, display_name),... } :param parent_name, string, indicates which key determines the parent :param ids: list of record ids - :return: the sublist (as a filter object) of records with the above properties + :return: the sublist of records with the above properties } """ def get_parent_id(record): @@ -342,7 +345,7 @@ class Base(models.AbstractModel): records_to_keep[id] = chain_is_fully_included # we keep initial order - return filter(lambda rec: records_to_keep.get(rec['id']), records) + return [rec for rec in records if records_to_keep.get(rec['id'])] @api.model @@ -412,7 +415,9 @@ class Base(models.AbstractModel): (if possible). If set to true and _parent_name is set on the comodel field, the information necessary for the hierarchization will be returned. Default is True. + :param limit: integer, maximal number of values to fetch. Default is None. :param search_domain: base domain of search. Default is []. + with parents if hierarchize is set) :return: { 'parent_field': parent field on the comodel of field, or False 'values': array of dictionaries containing some info on the records @@ -420,6 +425,7 @@ class Base(models.AbstractModel): The display name, the __count (how many records with that value) and possibly parent_field are fetched. } + or an object with an error message when limit is defined and is reached. """ field = self._fields[field_name] supported_types = ['many2one', 'selection'] @@ -439,12 +445,6 @@ class Base(models.AbstractModel): 'values': self._search_panel_selection_range(field_name, model_domain=model_domain,**kwargs), } - enable_counters = kwargs.get('enable_counters') - expand = kwargs.get('expand') - domain_image = {} - if enable_counters or not expand: - domain_image = self._search_panel_domain_image(field_name, model_domain, enable_counters) - Comodel = self.env[field.comodel_name].with_context(hierarchical_naming=False) field_names = ['display_name'] hierarchize = kwargs.get('hierarchize', True) @@ -460,10 +460,21 @@ class Base(models.AbstractModel): hierarchize = False comodel_domain = kwargs.get('comodel_domain', []) + enable_counters = kwargs.get('enable_counters') + expand = kwargs.get('expand') + limit = kwargs.get('limit') + domain_image = {} + if enable_counters or not expand: + domain_image = self._search_panel_domain_image(field_name, model_domain, enable_counters, + limit if not (expand or hierarchize or comodel_domain) else False + ) if not (expand or hierarchize or comodel_domain): + values = list(domain_image.values()) + if limit and len(values) == limit: + return {'error_msg': _(SEARCH_PANEL_ERROR_MESSAGE)} return { 'parent_field': parent_name, - 'values': list(domain_image.values()), + 'values': values, } if not expand: @@ -473,12 +484,15 @@ class Base(models.AbstractModel): else: condition = [('id', 'in', image_element_ids)] comodel_domain = AND([comodel_domain, condition]) - comodel_records = Comodel.search_read(comodel_domain, field_names) + comodel_records = Comodel.search_read(comodel_domain, field_names, limit=limit) if hierarchize: ids = [rec['id'] for rec in comodel_records] if expand else image_element_ids comodel_records = self._search_panel_sanitized_parent_hierarchy(comodel_records, parent_name, ids) + if limit and len(comodel_records) == limit: + return {'error_msg': _(SEARCH_PANEL_ERROR_MESSAGE)} + field_range = {} for record in comodel_records: record_id = record['id'] @@ -522,13 +536,17 @@ class Base(models.AbstractModel): for the group_by (if any). Those domains are used to fech accurate counters for values in each group. Default is [] (many2one case) or None. + :param limit: integer, maximal number of values to fetch. Default is None. :param search_domain: base domain of search. Default is []. - :return: a list of possible values, each being a dict with keys - 'id' (value), - 'name' (value label), - '__count' (how many records with that value), - 'group_id' (value of group), set if a group_by has been provided, - 'group_name' (label of group), set if a group_by has been provided + :return: { + 'values': a list of possible values, each being a dict with keys + 'id' (value), + 'name' (value label), + '__count' (how many records with that value), + 'group_id' (value of group), set if a group_by has been provided, + 'group_name' (label of group), set if a group_by has been provided + } + or an object with an error message when limit is defined and reached. """ field = self._fields[field_name] supported_types = ['many2one', 'many2many', 'selection'] @@ -544,11 +562,14 @@ class Base(models.AbstractModel): ]) if field.type == 'selection': - return self._search_panel_selection_range(field_name, model_domain=model_domain, **kwargs) + return { + 'values': self._search_panel_selection_range(field_name, model_domain=model_domain, **kwargs) + } Comodel = self.env.get(field.comodel_name).with_context(hierarchical_naming=False) field_names = ['display_name'] group_by = kwargs.get('group_by') + limit = kwargs.get('limit') if group_by: group_by_field = Comodel._fields[group_by] @@ -575,7 +596,9 @@ class Base(models.AbstractModel): expand = kwargs.get('expand') if field.type == 'many2many': - comodel_records = Comodel.search_read(comodel_domain, field_names) + comodel_records = Comodel.search_read(comodel_domain, field_names, limit=limit) + if expand and limit and len(comodel_records) == limit: + return {'error_msg': _(SEARCH_PANEL_ERROR_MESSAGE)} group_domain = kwargs.get('group_domain') field_range = [] @@ -608,7 +631,10 @@ class Base(models.AbstractModel): values['__count'] = count field_range.append(values) - return field_range + if not expand and limit and len(field_range) == limit: + return {'error_msg': _(SEARCH_PANEL_ERROR_MESSAGE)} + + return { 'values': field_range, } if field.type == 'many2one': domain_image = {} @@ -617,10 +643,15 @@ class Base(models.AbstractModel): model_domain, kwargs.get('group_domain', []), ]) - domain_image = self._search_panel_domain_image(field_name, model_domain, enable_counters) + domain_image = self._search_panel_domain_image(field_name, model_domain, enable_counters, + limit if not (expand or group_by or comodel_domain) else False + ) if not (expand or group_by or comodel_domain): - return list(domain_image.values()) + values = list(domain_image.values()) + if limit and len(values) == limit: + return {'error_msg': _(SEARCH_PANEL_ERROR_MESSAGE)} + return {'values': values, } if not expand: image_element_ids = list(domain_image.keys()) @@ -628,7 +659,9 @@ class Base(models.AbstractModel): comodel_domain, [('id', 'in', image_element_ids)], ]) - comodel_records = Comodel.search_read(comodel_domain, field_names) + comodel_records = Comodel.search_read(comodel_domain, field_names, limit=limit) + if limit and len(comodel_records) == limit: + return {'error_msg': _(SEARCH_PANEL_ERROR_MESSAGE)} field_range = [] for record in comodel_records: @@ -649,7 +682,7 @@ class Base(models.AbstractModel): field_range.append(values) - return field_range + return { 'values': field_range, } class ResCompany(models.Model): diff --git a/addons/web/static/src/js/views/search_panel.js b/addons/web/static/src/js/views/search_panel.js index 7aaeb99b648..81698a46c56 100644 --- a/addons/web/static/src/js/views/search_panel.js +++ b/addons/web/static/src/js/views/search_panel.js @@ -21,6 +21,8 @@ const qweb = core.qweb; const defaultViewTypes = ['kanban', 'tree']; let nextSectionId = 1; +const SEARCH_PANEL_DEFAULT_LIMIT = 200; + /** * Given a arch node, iterate over its children to generate the * description of each section (being either a category or a filter). @@ -31,31 +33,34 @@ let nextSectionId = 1; */ function _processSearchPanelNode(node, fields) { const sections = {}; - node.children.forEach((childNode, index) => { - if (childNode.tag !== 'field' || childNode.attrs.invisible === "1") { + node.children.forEach(({ attrs, tag }, index) => { + if (tag !== 'field' || attrs.invisible === "1") { return; } - const fieldName = childNode.attrs.name; - const type = childNode.attrs.select === 'multi' ? 'filter' : 'category'; + const fieldName = attrs.name; + const type = attrs.select === 'multi' ? 'filter' : 'category'; const sectionId = `section_${nextSectionId++}`; const section = { - color: childNode.attrs.color, - description: childNode.attrs.string || fields[fieldName].string, - enableCounters: !!pyUtils.py_eval(childNode.attrs.enable_counters || '0'), - expand: !!pyUtils.py_eval(childNode.attrs.expand || '0'), + color: attrs.color, + description: attrs.string || fields[fieldName].string, + enableCounters: !!pyUtils.py_eval(attrs.enable_counters || '0'), + expand: !!pyUtils.py_eval(attrs.expand || '0'), fieldName, - icon: childNode.attrs.icon, + icon: attrs.icon, id: sectionId, index, + limit: attrs.limit ? + pyUtils.py_eval(attrs.limit) : + SEARCH_PANEL_DEFAULT_LIMIT, type, }; if (section.type === 'category') { section.icon = section.icon || 'fa-folder'; - section.hierarchize = !!pyUtils.py_eval(childNode.attrs.hierarchize || '1'); + section.hierarchize = !!pyUtils.py_eval(attrs.hierarchize || '1'); } else if (section.type === 'filter') { - section.domain = childNode.attrs.domain || '[]'; - section.groupBy = childNode.attrs.groupby; + section.domain = attrs.domain || '[]'; + section.groupBy = attrs.groupby; section.icon = section.icon || 'fa-filter'; } sections[sectionId] = section; @@ -267,11 +272,18 @@ const SearchPanel = Widget.extend({ /** * @private * @param {string} categoryId - * @param {Object[]} values + * @param {Object} result */ - _createCategoryTree(categoryId, values) { + _createCategoryTree(categoryId, result) { const category = this.categories[categoryId]; - const parentField = category.parentField; + let { error_msg, parent_field: parentField, values } = result; + if (error_msg) { + category.errorMsg = error_msg; + values = []; + } + if (category.hierarchize) { + category.parentField = parentField; + } const unfoldedIds = Object.values(category.values || {}) .filter(c => c.folded === false) @@ -315,10 +327,15 @@ const SearchPanel = Widget.extend({ /** * @private * @param {string} filterId - * @param {Object[]} values + * @param {Object} result */ - _createFilterTree(filterId, values) { + _createFilterTree(filterId, result) { const filter = this.filters[filterId]; + let { error_msg, values } = result; + if (error_msg) { + filter.errorMsg = error_msg; + values = []; + } // restore checked property values.forEach(value => { @@ -379,8 +396,7 @@ const SearchPanel = Widget.extend({ _fetchCategories(force) { const proms = []; for (const category of Object.values(this.categories)) { - const { enableCounters, expand, hierarchize } = category; - const field = this.fields[category.fieldName]; + const { enableCounters, expand, hierarchize, limit } = category; if (force || enableCounters || !expand) { const prom = this._rpc({ method: 'search_panel_select_range', @@ -391,14 +407,12 @@ const SearchPanel = Widget.extend({ enable_counters: enableCounters, expand, hierarchize, + limit, search_domain: this.searchDomain, }, - }).then(({ parent_field, values }) => { - if (field.type === 'many2one' && hierarchize) { - category.parentField = parent_field; - } - this._createCategoryTree(category.id, values); - }); + }).then( + (result) => this._createCategoryTree(category.id, result) + ); proms.push(prom); } } @@ -419,7 +433,7 @@ const SearchPanel = Widget.extend({ const categoryDomain = this._getCategoryDomain(); const proms = []; for (const filter of Object.values(this.filters)) { - const { enableCounters, expand, groupBy } = filter; + const { enableCounters, expand, groupBy, limit } = filter; const prom = this._rpc({ method: 'search_panel_select_multi_range', model: this.model, @@ -432,9 +446,12 @@ const SearchPanel = Widget.extend({ expand, group_by: groupBy || false, group_domain: this._getGroupDomain(filter), + limit, search_domain: [...this.searchDomain, ...this.viewDomain], }, - }).then(values => { this._createFilterTree(filter.id, values); }); + }).then( + (result) => this._createFilterTree(filter.id, result) + ); proms.push(prom); } return Promise.all(proms); @@ -646,7 +663,7 @@ const SearchPanel = Widget.extend({ const sections = categories.concat(filters).sort((s1, s2) => s1.index - s2.index); sections.forEach(section => { - if (Object.keys(section.values).length) { + if (Object.keys(section.values).length || section.errorMsg) { if (section.type === 'category') { this.$el.append(this._renderCategory(section)); } else { diff --git a/addons/web/static/src/xml/kanban.xml b/addons/web/static/src/xml/kanban.xml index 114f0ccd876..f4657d4b3a3 100644 --- a/addons/web/static/src/xml/kanban.xml +++ b/addons/web/static/src/xml/kanban.xml @@ -222,7 +222,10 @@ -
    +
    + +
    +