From 92b4e2e866363ecb1b62c2448d40a5fcfc481348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 15 Nov 2018 11:11:34 +0000 Subject: [PATCH 1/3] [FIX] web: fix performance issue with read_progress_bar The read_progress_bar method is incredibly not efficient. It is also wrong in other ways (for example, if 2 groups have same labels, there is a collision). But let us talk about efficiency: it iterates on each records on the domain, then, it computes some information. It is really worse for selection fields: it performs a fields_get for each iteration. This commit improves the situation by doing the fields_get only once. Note that this is not the end of the story: this only has a small effect on the performance of this method. It looks like there is a deeper issue in some cases (kanban view for subscription, field 'activity_state') --- addons/web/models/models.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/addons/web/models/models.py b/addons/web/models/models.py index 572f07333d2..2e3bd54788d 100644 --- a/addons/web/models/models.py +++ b/addons/web/models/models.py @@ -40,11 +40,14 @@ class Base(models.AbstractModel): records_values = self.search_read(domain or [], [progress_bar['field'], group_by]) data = {} + field_type = self._fields[group_by].type + if field_type == 'selection': + selection_labels = dict(self.fields_get()[group_by]['selection']) + for record_values in records_values: group_by_value = record_values[group_by] # Again, imitating what _read_group_format_result and _read_group_prepare_data do - field_type = self._fields[group_by].type if group_by_value and field_type in ['date', 'datetime']: locale = self._context.get('lang') or 'en_US' group_by_value = fields.Datetime.to_datetime(group_by_value) @@ -61,7 +64,7 @@ class Base(models.AbstractModel): locale=locale) if field_type == 'selection': - group_by_value = dict(self.fields_get()[group_by]['selection'])[group_by_value] + group_by_value = selection_labels[group_by_value] if type(group_by_value) == tuple: group_by_value = group_by_value[1] # FIXME should use technical value (0) From 1bb15e0559aaf15626d5e91e04ebf78cc10cbd44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 15 Nov 2018 12:11:02 +0000 Subject: [PATCH 2/3] [FIX] mail: fix performance issue for activity view Fun fact: for the subscription model in odoo production database, the get_activity_data rpc takes 36s, and transfer 420mb (non gzipped) of data (when looking at all subscriptions). The main reason for that is that the get_activity_data method create a domain with all the res_ids (so, if you have 20k subscriptions, the domain will look like (res_id, in, [...20k ids]). This domain is given to read group, and the resulting groups (each with a domain key which includes those res_ids) will be sent back to the web client. As far as I can tell, the domain key is not used by the web client, so we send (number of groups)*(number of records) of useless data. This commit improves the situation in two different ways: - we only generate the domain if there is a domain at the beginning. This should greatly improve the performance when there is no current domain - we do not transfer the data to the web client --- addons/mail/models/mail_activity.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index 4a29e788d95..9838b96a5cd 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -422,8 +422,10 @@ class MailActivity(models.Model): @api.model def get_activity_data(self, res_model, domain): - res = self.env[res_model].search(domain) - activity_domain = [('res_id', 'in', res.ids), ('res_model', '=', res_model)] + activity_domain = [('res_model', '=', res_model)] + if domain: + res = self.env[res_model].search(domain) + activity_domain.append(('res_id', 'in', res.ids)) grouped_activities = self.env['mail.activity'].read_group( activity_domain, ['res_id', 'activity_type_id', 'res_name:max(res_name)', 'ids:array_agg(id)', 'date_deadline:min(date_deadline)'], @@ -443,7 +445,6 @@ class MailActivity(models.Model): state = self._compute_state_from_date(group['date_deadline'], self.user_id.sudo().tz) activity_data[res_id][activity_type_id] = { 'count': group['__count'], - 'domain': group['__domain'], 'ids': group['ids'], 'state': state, 'o_closest_deadline': group['date_deadline'], From 990dce6e2a2ed4ef614c0e9efb2371076397bac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 15 Nov 2018 13:51:45 +0000 Subject: [PATCH 3/3] [FIX] web: prevent crash in ungrouped kanban views with progressbar The progressbar widget is not supposed to be displayed in non grouped mode. Before this commit, this situation resulted in a traceback. For example, if the user has an activity on the subscription model, and tries to open it, it will open an ungrouped kanban view. With this commit, we check first that the kanban view is in grouped mode, before loading the progress bar data. --- .../src/js/views/kanban/kanban_model.js | 2 +- addons/web/static/tests/views/kanban_tests.js | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/addons/web/static/src/js/views/kanban/kanban_model.js b/addons/web/static/src/js/views/kanban/kanban_model.js index fd21ed4df86..bfb6f5e0cc9 100644 --- a/addons/web/static/src/js/views/kanban/kanban_model.js +++ b/addons/web/static/src/js/views/kanban/kanban_model.js @@ -305,7 +305,7 @@ var KanbanModel = BasicModel.extend({ * @override */ _load: function (dataPoint, options) { - if (dataPoint.progressBar) { + if (dataPoint.groupedBy.length && dataPoint.progressBar) { return this._readProgressBarGroup(dataPoint, options); } return this._super.apply(this, arguments); diff --git a/addons/web/static/tests/views/kanban_tests.js b/addons/web/static/tests/views/kanban_tests.js index f8f2e232794..84d63fa4f67 100644 --- a/addons/web/static/tests/views/kanban_tests.js +++ b/addons/web/static/tests/views/kanban_tests.js @@ -4232,6 +4232,37 @@ QUnit.module('Views', { kanban.destroy(); }); + QUnit.test('column progressbars should not crash in non grouped views', function (assert) { + assert.expect(3); + + var kanban = createView({ + View: KanbanView, + model: 'partner', + data: this.data, + arch: + '' + + '' + + '' + + '' + + '' + + '
' + + '' + + '
' + + '' + + '', + mockRPC: function (route, args) { + assert.step(route) + return this._super(route, args); + }, + }); + + assert.strictEqual(kanban.$('.o_kanban_record').text(), 'namenamenamename', + "should have renderer 4 records"); + + assert.verifySteps(['/web/dataset/search_read'], "no read on progress bar data is done"); + kanban.destroy(); + }); + QUnit.test('column progressbars: creating a new column should create a new progressbar', function (assert) { assert.expect(1);