From 623567eba0884aba1213edacab767cc4c2732053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 20 Apr 2020 11:46:09 +0000 Subject: [PATCH 1/6] [LNT] crm: lint calendar file Just passing by. Task ID 2196182 PR #44662 --- addons/crm/models/calendar.py | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/addons/crm/models/calendar.py b/addons/crm/models/calendar.py index 99d69dbf76c..00133a9f310 100644 --- a/addons/crm/models/calendar.py +++ b/addons/crm/models/calendar.py @@ -7,17 +7,6 @@ from odoo import api, fields, models class CalendarEvent(models.Model): _inherit = 'calendar.event' - def _is_crm_lead(self, defaults, ctx=None): - """ - This method checks if the concerned model is a CRM lead. - The information is not always in the defaults values, - this is why it is necessary to check the context too. - """ - res_model = defaults.get('res_model', False) or ctx and ctx.get('default_res_model') - res_model_id = defaults.get('res_model_id', False) or ctx and ctx.get('default_res_model_id') - - return res_model and res_model == 'crm.lead' or res_model_id and self.env['ir.model'].sudo().browse(res_model_id).model == 'crm.lead' - @api.model def default_get(self, fields): if self.env.context.get('default_opportunity_id'): @@ -28,13 +17,14 @@ class CalendarEvent(models.Model): defaults = super(CalendarEvent, self).default_get(fields) # sync res_model / res_id to opportunity id (aka creating meeting from lead chatter) - ctx = self.env.context if 'opportunity_id' not in defaults: - if self._is_crm_lead(defaults, ctx): - defaults['opportunity_id'] = defaults.get('res_id', False) or ctx.get('default_res_id', False) + if self._is_crm_lead(defaults, self.env.context): + defaults['opportunity_id'] = defaults.get('res_id', False) or self.env.context.get('default_res_id', False) return defaults + opportunity_id = fields.Many2one('crm.lead', 'Opportunity', domain="[('type', '=', 'opportunity')]") + def _compute_is_highlighted(self): super(CalendarEvent, self)._compute_is_highlighted() if self.env.context.get('active_model') == 'crm.lead': @@ -43,8 +33,6 @@ class CalendarEvent(models.Model): if event.opportunity_id.id == opportunity_id: event.is_highlighted = True - opportunity_id = fields.Many2one('crm.lead', 'Opportunity', domain="[('type', '=', 'opportunity')]") - @api.model_create_multi def create(self, vals): events = super(CalendarEvent, self).create(vals) @@ -52,3 +40,14 @@ class CalendarEvent(models.Model): if event.opportunity_id and not event.activity_ids: event.opportunity_id.log_meeting(event.name, event.start, event.duration) return events + + def _is_crm_lead(self, defaults, ctx=None): + """ + This method checks if the concerned model is a CRM lead. + The information is not always in the defaults values, + this is why it is necessary to check the context too. + """ + res_model = defaults.get('res_model', False) or ctx and ctx.get('default_res_model') + res_model_id = defaults.get('res_model_id', False) or ctx and ctx.get('default_res_model_id') + + return res_model and res_model == 'crm.lead' or res_model_id and self.env['ir.model'].sudo().browse(res_model_id).model == 'crm.lead' From b415533dca2af9b84725b0c48ee5d26359760300 Mon Sep 17 00:00:00 2001 From: Victor Feyens Date: Tue, 11 Feb 2020 12:18:21 +0000 Subject: [PATCH 2/6] [REF] crm(_iap_lead): use read_group instead of computing len of one2many fields PURPOSE Several counter fields are performed inefficiently. They use compute methods using length of 2many fields instead of making use of more efficient ORM methods like read_group. SPECIFICATIONS No functional change, only technical performance improvements. Purpose is to improve lead_count performance. Some people claim to gain a lot of queries: from 90 to 30 SQL queries to read 39 partners. Hard to reproduce. LINKS Task ID 2196182 PR odoo/odoo#44662 Co-Authored-By: Fabien Pinckaers Co-Authored-By: Victor Feyens Co-Authored-By: Thibault Delavallee --- addons/crm/models/calendar.py | 4 ++- addons/crm/models/crm_lead.py | 9 +++-- addons/crm/models/res_partner.py | 35 +++++++++++++++++-- .../models/crm_iap_lead_mining_request.py | 13 +++++-- .../tests/test_performance.py | 4 +-- 5 files changed, 55 insertions(+), 10 deletions(-) diff --git a/addons/crm/models/calendar.py b/addons/crm/models/calendar.py index 00133a9f310..0c55f665e4e 100644 --- a/addons/crm/models/calendar.py +++ b/addons/crm/models/calendar.py @@ -23,7 +23,9 @@ class CalendarEvent(models.Model): return defaults - opportunity_id = fields.Many2one('crm.lead', 'Opportunity', domain="[('type', '=', 'opportunity')]") + opportunity_id = fields.Many2one( + 'crm.lead', 'Opportunity', domain="[('type', '=', 'opportunity')]", + index=True, ondelete='set null') def _compute_is_highlighted(self): super(CalendarEvent, self)._compute_is_highlighted() diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index fb5c891e377..2a9a9d79b9d 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -293,8 +293,13 @@ class Lead(models.Model): lead.expected_revenue = round((lead.planned_revenue or 0.0) * (lead.probability or 0) / 100.0, 2) def _compute_meeting_count(self): - meeting_data = self.env['calendar.event'].sudo().read_group([('opportunity_id', 'in', self.ids)], ['opportunity_id'], ['opportunity_id']) - mapped_data = {m['opportunity_id'][0]: m['opportunity_id_count'] for m in meeting_data} + if self.ids: + meeting_data = self.env['calendar.event'].sudo().read_group([ + ('opportunity_id', 'in', self.ids) + ], ['opportunity_id'], ['opportunity_id']) + mapped_data = {m['opportunity_id'][0]: m['opportunity_id_count'] for m in meeting_data} + else: + mapped_data = dict() for lead in self: lead.meeting_count = mapped_data.get(lead.id, 0) diff --git a/addons/crm/models/res_partner.py b/addons/crm/models/res_partner.py index 8129b6f9536..f214c7d5549 100644 --- a/addons/crm/models/res_partner.py +++ b/addons/crm/models/res_partner.py @@ -37,13 +37,42 @@ class Partner(models.Model): return rec def _compute_opportunity_count(self): + if self.ids: + lead_group_data = self.env['crm.lead'].read_group( + [('partner_id.commercial_partner_id', 'in', self.ids)], + ['partner_id'], ['partner_id'] + ) + else: + lead_group_data = [] + partners = dict( + (m['partner_id'][0], m['partner_id_count']) + for m in lead_group_data) + commercial_partners = {} + for partner in self.browse(partners.keys()): + commercial_partners.setdefault(partner.commercial_partner_id.id, 0) + commercial_partners[partner.commercial_partner_id.id] += partners[partner.id] for partner in self: - operator = 'child_of' if partner.is_company else '=' # the opportunity count should counts the opportunities of this company and all its contacts - partner.opportunity_count = self.env['crm.lead'].search_count([('partner_id', operator, partner.id), ('type', '=', 'opportunity')]) + if partner.is_company: + partner.opportunity_count = commercial_partners.get(partner.id, 0) + else: + partner.opportunity_count = partners.get(partner.id, 0) def _compute_meeting_count(self): + if self.ids: + self.env.cr.execute(""" + SELECT res_partner_id, calendar_event_id, count(1) + FROM calendar_event_res_partner_rel + WHERE res_partner_id IN %s + GROUP BY res_partner_id, calendar_event_id + """, [tuple(self.ids)]) + meeting_data = self.env.cr.fetchall() + events = [row[1] for row in meeting_data] + valid_events = self.env['calendar.event'].search([('id', 'in', events)]) # filter for ACLs + meetings = dict((m[0], m[2]) for m in meeting_data if m[1] in valid_events.ids) + else: + meetings = dict() for partner in self: - partner.meeting_count = len(partner.meeting_ids) + partner.meeting_count = meetings.get(partner.id, 0) def schedule_meeting(self): partner_ids = self.ids diff --git a/addons/crm_iap_lead/models/crm_iap_lead_mining_request.py b/addons/crm_iap_lead/models/crm_iap_lead_mining_request.py index b62e454ccbb..df7b9fa531a 100644 --- a/addons/crm_iap_lead/models/crm_iap_lead_mining_request.py +++ b/addons/crm_iap_lead/models/crm_iap_lead_mining_request.py @@ -76,8 +76,17 @@ class CRMLeadMiningRequest(models.Model): @api.depends('lead_ids') def _compute_leads_count(self): - for req in self: - req.leads_count = len(req.lead_ids) + if self.ids: + leads_data = self.env['crm.lead'].read_group( + [('lead_mining_request_id', 'in', self.ids)], + ['lead_mining_request_id'], ['lead_mining_request_id']) + else: + leads_data = [] + mapped_data = dict( + (m['lead_mining_request_id'][0], m['lead_mining_request_id_count']) + for m in leads_data) + for request in self: + request.leads_count = mapped_data.get(request.id, 0) @api.onchange('lead_number') def _onchange_lead_number(self): diff --git a/addons/test_mass_mailing/tests/test_performance.py b/addons/test_mass_mailing/tests/test_performance.py index 276b2701731..44a5545e876 100644 --- a/addons/test_mass_mailing/tests/test_performance.py +++ b/addons/test_mass_mailing/tests/test_performance.py @@ -51,7 +51,7 @@ class TestMassMailPerformance(TestMassMailPerformanceBase): }) # runbot needs +50 compared to local - with self.assertQueryCount(__system__=1915, marketing=1916): + with self.assertQueryCount(__system__=1916, marketing=1917): mailing.action_send_mail() self.assertEqual(mailing.sent, 50) @@ -91,7 +91,7 @@ class TestMassMailBlPerformance(TestMassMailPerformanceBase): }) # runbot needs +62 compared to local - with self.assertQueryCount(__system__=2239, marketing=2240): + with self.assertQueryCount(__system__=2240, marketing=2241): mailing.action_send_mail() self.assertEqual(mailing.sent, 50) From efafadc3ee2740407d6d6ff8082ab9e1f7d8e87f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 23 Mar 2020 15:48:09 +0000 Subject: [PATCH 3/6] [IMP] crm_iap_lead(_website): improve field naming Purpose is to somehow clean naming of fields. Task ID 2196182 PR #44662 Upgrade PR odoo/upgrade#973 --- addons/crm_iap_lead/models/crm_iap_lead_mining_request.py | 8 ++++---- addons/crm_iap_lead/views/crm_iap_lead_views.xml | 4 ++-- addons/crm_iap_lead_website/models/crm_reveal_rule.py | 4 ++-- addons/crm_iap_lead_website/views/crm_reveal_views.xml | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/addons/crm_iap_lead/models/crm_iap_lead_mining_request.py b/addons/crm_iap_lead/models/crm_iap_lead_mining_request.py index df7b9fa531a..598fe68d8d6 100644 --- a/addons/crm_iap_lead/models/crm_iap_lead_mining_request.py +++ b/addons/crm_iap_lead/models/crm_iap_lead_mining_request.py @@ -42,7 +42,7 @@ class CRMLeadMiningRequest(models.Model): user_id = fields.Many2one('res.users', string='Salesperson') tag_ids = fields.Many2many('crm.tag', string='Tags') lead_ids = fields.One2many('crm.lead', 'lead_mining_request_id', string='Generated Lead / Opportunity') - leads_count = fields.Integer(compute='_compute_leads_count', string='Number of Generated Leads') + lead_count = fields.Integer(compute='_compute_lead_count', string='Number of Generated Leads') # Company Criteria Filter filter_on_size = fields.Boolean(string='Filter on Size', default=False) @@ -74,8 +74,8 @@ class CRMLeadMiningRequest(models.Model): record.lead_credits = _('%d credits will be consumed to find %d companies.') % (company_credits, record.lead_number) record.lead_total_credits = _("This makes a total of %d credits for this request.") % (total_contact_credits + company_credits) - @api.depends('lead_ids') - def _compute_leads_count(self): + @api.depends('lead_ids.lead_mining_request_id') + def _compute_lead_count(self): if self.ids: leads_data = self.env['crm.lead'].read_group( [('lead_mining_request_id', 'in', self.ids)], @@ -86,7 +86,7 @@ class CRMLeadMiningRequest(models.Model): (m['lead_mining_request_id'][0], m['lead_mining_request_id_count']) for m in leads_data) for request in self: - request.leads_count = mapped_data.get(request.id, 0) + request.lead_count = mapped_data.get(request.id, 0) @api.onchange('lead_number') def _onchange_lead_number(self): diff --git a/addons/crm_iap_lead/views/crm_iap_lead_views.xml b/addons/crm_iap_lead/views/crm_iap_lead_views.xml index 41bf1b858a6..f76fb55013c 100644 --- a/addons/crm_iap_lead/views/crm_iap_lead_views.xml +++ b/addons/crm_iap_lead/views/crm_iap_lead_views.xml @@ -19,14 +19,14 @@ diff --git a/addons/crm_iap_lead_website/models/crm_reveal_rule.py b/addons/crm_iap_lead_website/models/crm_reveal_rule.py index 4da34077042..e34af7463af 100644 --- a/addons/crm_iap_lead_website/models/crm_reveal_rule.py +++ b/addons/crm_iap_lead_website/models/crm_reveal_rule.py @@ -57,7 +57,7 @@ class CRMRevealRule(models.Model): user_id = fields.Many2one('res.users', string='Salesperson') priority = fields.Selection(crm_stage.AVAILABLE_PRIORITIES, string='Priority') lead_ids = fields.One2many('crm.lead', 'reveal_rule_id', string='Generated Lead / Opportunity') - leads_count = fields.Integer(compute='_compute_leads_count', string='Number of Generated Leads') + lead_count = fields.Integer(compute='_compute_lead_count', string='Number of Generated Leads') opportunity_count = fields.Integer(compute='_compute_leads_count', string='Number of Generated Opportunity') # This limits the number of extra contact. @@ -101,7 +101,7 @@ class CRMRevealRule(models.Model): self.clear_caches() # Clear the cache in order to recompute _get_active_rules return super(CRMRevealRule, self).unlink() - def _compute_leads_count(self): + def _compute_lead_count(self): leads = self.env['crm.lead'].read_group([ ('reveal_rule_id', 'in', self.ids) ], fields=['reveal_rule_id', 'type'], groupby=['reveal_rule_id', 'type'], lazy=False) diff --git a/addons/crm_iap_lead_website/views/crm_reveal_views.xml b/addons/crm_iap_lead_website/views/crm_reveal_views.xml index 80d34fbbea9..cda1577498c 100644 --- a/addons/crm_iap_lead_website/views/crm_reveal_views.xml +++ b/addons/crm_iap_lead_website/views/crm_reveal_views.xml @@ -13,9 +13,9 @@ Opportunities - From 2457e7f6f45b23118ce73c47112d60802411cc4b Mon Sep 17 00:00:00 2001 From: Victor Feyens Date: Tue, 11 Feb 2020 12:19:13 +0000 Subject: [PATCH 4/6] [IMP] hr_recruitment: use read_group instead of computing len of one2many fields PURPOSE Several counter fields are performed inefficiently. They use compute methods using length of 2many fields instead of making use of more efficient ORM methods like read_group. SPECIFICATIONS No functional change, only technical performance improvements. Purpose is to improve meeting count performances. An index on applicant is also added to speedup queries. LINKS Task ID 2196182 PR #44662 Co-Authored-By: Victor Feyens Co-Authored-By: Thibault Delavallee --- addons/hr_recruitment/models/calendar.py | 2 +- addons/hr_recruitment/models/hr_recruitment.py | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/addons/hr_recruitment/models/calendar.py b/addons/hr_recruitment/models/calendar.py index 976b5019b73..554ab1cd763 100644 --- a/addons/hr_recruitment/models/calendar.py +++ b/addons/hr_recruitment/models/calendar.py @@ -33,4 +33,4 @@ class CalendarEvent(models.Model): if event.applicant_id.id == applicant_id: event.is_highlighted = True - applicant_id = fields.Many2one('hr.applicant', string="Applicant") + applicant_id = fields.Many2one('hr.applicant', string="Applicant", index=True, ondelete='set null') diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py index a07340a57e6..e18d4564948 100644 --- a/addons/hr_recruitment/models/hr_recruitment.py +++ b/addons/hr_recruitment/models/hr_recruitment.py @@ -184,8 +184,17 @@ class Applicant(models.Model): (self - applicants).application_count = False def _compute_meeting_count(self): + if self.ids: + meeting_data = self.env['calendar.event'].sudo().read_group( + [('applicant_id', 'in', self.ids)], + ['applicant_id'], + ['applicant_id'] + ) + mapped_data = {m['applicant_id'][0]: m['applicant_id_count'] for m in meeting_data} + else: + mapped_data = dict() for applicant in self: - applicant.meeting_count = self.env['calendar.event'].search_count([('applicant_id', '=', applicant.id)]) + applicant.meeting_count = mapped_data.get(applicant.id, 0) def _get_attachment_number(self): read_group_res = self.env['ir.attachment'].read_group( From 3f6625ef134c35ad8fab00ca99b451a7dfabee08 Mon Sep 17 00:00:00 2001 From: Victor Feyens Date: Tue, 11 Feb 2020 12:20:39 +0000 Subject: [PATCH 5/6] [IMP] link_tracker, mass_mailing: uuse read_group instead of computing len of one2many fields PURPOSE Several counter fields are performed inefficiently. They use compute methods using length of 2many fields instead of making use of more efficient ORM methods like read_group. SPECIFICATIONS There may be a lot of clicks linked to a given link tracker. Compute the count of clicks in batch with a read_group to improve performance. Same for utm campaign mailing count performances. Some missing indexes are added to speedup queries. LINKS Task ID 2196182 PR #44662 Co-Authored-By: Victor Feyens Co-Authored-By: Thibault Delavallee --- addons/link_tracker/models/link_tracker.py | 19 ++++++++++++++++--- addons/mass_mailing/models/mailing.py | 2 +- addons/mass_mailing/models/utm.py | 11 ++++++++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/addons/link_tracker/models/link_tracker.py b/addons/link_tracker/models/link_tracker.py index ec73046e1be..8ea1a04f50e 100644 --- a/addons/link_tracker/models/link_tracker.py +++ b/addons/link_tracker/models/link_tracker.py @@ -38,8 +38,17 @@ class LinkTracker(models.Model): @api.depends('link_click_ids.link_id') def _compute_count(self): + if self.ids: + clicks_data = self.env['link.tracker.click'].read_group( + [('link_id', 'in', self.ids)], + ['link_id'], + ['link_id'] + ) + mapped_data = {m['link_id'][0]: m['link_id_count'] for m in clicks_data} + else: + mapped_data = dict() for tracker in self: - tracker.count = len(tracker.link_click_ids) + tracker.count = mapped_data.get(tracker.id, 0) @api.depends('code') def _compute_short_url(self): @@ -187,8 +196,12 @@ class LinkTrackerClick(models.Model): _rec_name = "link_id" _description = "Link Tracker Click" - campaign_id = fields.Many2one(string='UTM Campaign', comodel_name="utm.campaign", related="link_id.campaign_id", store=True) - link_id = fields.Many2one('link.tracker', 'Link', required=True, ondelete='cascade') + campaign_id = fields.Many2one( + 'utm.campaign', 'UTM Campaign', + related="link_id.campaign_id", store=True) + link_id = fields.Many2one( + 'link.tracker', 'Link', + index=True, required=True, ondelete='cascade') ip = fields.Char(string='Internet Protocol') country_id = fields.Many2one('res.country', 'Country') diff --git a/addons/mass_mailing/models/mailing.py b/addons/mass_mailing/models/mailing.py index 2446c03dd0a..e644f4b0d7c 100644 --- a/addons/mass_mailing/models/mailing.py +++ b/addons/mass_mailing/models/mailing.py @@ -65,7 +65,7 @@ class MassMailing(models.Model): attachment_ids = fields.Many2many('ir.attachment', 'mass_mailing_ir_attachments_rel', 'mass_mailing_id', 'attachment_id', string='Attachments') keep_archives = fields.Boolean(string='Keep Archives') - campaign_id = fields.Many2one('utm.campaign', string='UTM Campaign') + campaign_id = fields.Many2one('utm.campaign', string='UTM Campaign', index=True) source_id = fields.Many2one('utm.source', string='Source', required=True, ondelete='cascade', help="This is the link source, e.g. Search Engine, another domain, or name of email list") medium_id = fields.Many2one( diff --git a/addons/mass_mailing/models/utm.py b/addons/mass_mailing/models/utm.py index 783703d3961..54ea93bf62c 100644 --- a/addons/mass_mailing/models/utm.py +++ b/addons/mass_mailing/models/utm.py @@ -20,8 +20,17 @@ class UtmCampaign(models.Model): @api.depends('mailing_mail_ids') def _compute_mailing_mail_count(self): + if self.ids: + mailing_data = self.env['mailing.mailing'].read_group( + [('campaign_id', 'in', self.ids)], + ['campaign_id'], + ['campaign_id'] + ) + mapped_data = {m['campaign_id'][0]: m['campaign_id_count'] for m in mailing_data} + else: + mapped_data = dict() for campaign in self: - campaign.mailing_mail_count = len(campaign.mailing_mail_ids) + campaign.mailing_mail_count = mapped_data.get(campaign.id, 0) def _compute_statistics(self): """ Compute statistics of the mass mailing campaign """ From 9ff94fec74731f3df5241e1eec5019e1b111854c Mon Sep 17 00:00:00 2001 From: Victor Feyens Date: Tue, 11 Feb 2020 12:28:07 +0000 Subject: [PATCH 6/6] [IMP] website_forum: improve forum.post child_count performance. PURPOSE Several counter fields are performed inefficiently. They use compute methods using length of 2many fields instead of making use of more efficient ORM methods like read_group. SPECIFICATIONS No functional change, only technical performance improvements. Purpose is to improve forum.post child_count performance. LINKS Task ID 2196182 PR #44662 --- addons/website_forum/models/forum.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index 5aba126395d..55137df5148 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -366,14 +366,15 @@ class Post(models.Model): @api.depends('child_ids.create_uid', 'website_message_ids') def _get_child_count(self): - def process(node): - total = len(node.website_message_ids) + len(node.child_ids) - for child in node.child_ids: - total += process(child) - return total - for post in self: - post.child_count = process(post) + children = self.search([('id', 'child_of', post.id)]) + children_count = len(children) + message_count = self.env['mail.message'].search_count([ + ('model', '=', post._name), + ('message_type', 'in', ['email', 'comment']), + ('res_id', 'in', children.ids + [post.id]), + ]) + post.child_count = children_count + message_count def _get_uid_has_answered(self): for post in self: