pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[PERF] crm: rewrite duplicates computation

Browse Source 
RATIONALE

Duplicates computation on leads is slow when having a lot of leads. We can
simplify the heuristic to keep only relevant terms and use tailored fields
to find duplicates based on email and phone, to speedup the computation while
loosing few duplicates or avoiding false duplicates.

SPECIFICATIONS

We remove
  * an ilike on email_normalized (slow and exact matches are what really
    matters);
  * an ilike based on partner_name and contact_name (as lead is a contact
    oriented record, better optimize email and phone / mobile than names that
    are weak criterions);
  * an ilike on phone_mobile_search, which was searching on both fields
    phone and mobile;

Criterions are now

  * email domain exact match;
  * phone_sanitized exact match;
  * same commercial entity;

Main change is that only phone_sanitized is used for exact match. Partial
matching based on phone and mobile number is lost. That should not be an
issue as real duplicates should come with a complete phone or mobile number
and not partial numbers. First found in phone / mobile populates the phone
sanitized field, meaning if we have two different numbers, only one is used
for duplicate finding. This is considered acceptable.

Other main change is the removal of partner name / contact name search. Those
are weak criterions and did not return any explicit result when used internally.

Task-3142659 (Crm: Fix duplicate leads computation performances)

Part-of: odoo/odoo#112535
This commit is contained in:
Thibault Delavallée
2023-03-09 10:51:37 +01:00
parent 4ad44d1c45
commit b1695201b8
2 changed files with 244 additions and 227 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/crm/models/crm_lead.py
+36 -26
View File
@@ -188,6 +188,13 @@ class Lead(models.Model):
'Email', tracking=40, index='trigram',
compute='_compute_email_from', inverse='_inverse_email_from', readonly=False, store=True)
email_normalized = fields.Char(index='trigram') # inherited via mail.thread.blacklist
email_domain_criterion = fields.Char(
string='Email Domain Criterion',
compute="_compute_email_domain_criterion",
index='btree_not_null', # used for exact match, void value do not matter
store=True,
unaccent=False, # normalized, exact matching
)
phone = fields.Char(
'Phone', tracking=50,
compute='_compute_phone', inverse='_inverse_phone', readonly=False, store=True)
@@ -448,6 +455,14 @@ class Lead(models.Model):
if lead._get_partner_email_update():
lead.partner_id.email = lead.email_from
@api.depends('email_normalized')
def _compute_email_domain_criterion(self):
self.email_domain_criterion = False
for lead in self.filtered('email_normalized'):
lead.email_domain_criterion = iap_tools.mail_prepare_for_domain_search(
lead.email_normalized
)
@api.depends('partner_id.phone')
def _compute_phone(self):
for lead in self:
@@ -532,11 +547,16 @@ class Lead(models.Model):
for lead in self:
lead.calendar_event_count = mapped_data.get(lead.id, 0)
@api.depends('email_from', 'partner_id', 'contact_name', 'partner_name')
@api.depends('email_domain_criterion', 'email_normalized', 'partner_id',
'phone_sanitized')
def _compute_potential_lead_duplicates(self):
MIN_EMAIL_LENGTH = 7
MIN_NAME_LENGTH = 6
MIN_PHONE_LENGTH = 8
""" Override potential lead duplicates computation to be more efficient
with high lead volume.
Criterions:
* email domain exact match;
* phone_sanitized exact match;
* same commercial entity;
"""
SEARCH_RESULT_LIMIT = 21
def return_if_relevant(model_name, domain):
@@ -545,14 +565,12 @@ class Lead(models.Model):
below a given threshold (i.e: `SEARCH_RESULT_LIMIT`). Otherwise, returns
an empty recordset of the provided model as it indicates search term
was not relevant.
Note: The function will use the administrator privileges to guarantee
that a maximum amount of leads will be included in the search results
and transcend multi-company record rules. It also includes archived records.
Idea is that counter indicates duplicates are present and that lead
could be escalated to managers.
and transcend multi-company record rules. It also includes archived
records. Idea is that counter indicates duplicates are present and
the lead could be escalated to managers.
"""
# Includes archived records and transcend multi-company record rules
model = self.env[model_name].sudo().with_context(active_test=False)
res = model.search(domain, limit=SEARCH_RESULT_LIMIT)
return res if len(res) < SEARCH_RESULT_LIMIT else model
@@ -564,31 +582,23 @@ class Lead(models.Model):
]
duplicate_lead_ids = self.env['crm.lead']
email_search = iap_tools.mail_prepare_for_domain_search(lead.email_from, min_email_length=MIN_EMAIL_LENGTH)
if email_search:
# check the "company" email domain duplicates
if lead.email_domain_criterion:
duplicate_lead_ids |= return_if_relevant('crm.lead', common_lead_domain + [
('email_normalized', 'ilike', email_search)
])
if lead.partner_name and len(lead.partner_name) >= MIN_NAME_LENGTH:
duplicate_lead_ids |= return_if_relevant('crm.lead', common_lead_domain + [
('partner_name', 'ilike', lead.partner_name)
])
if lead.contact_name and len(lead.contact_name) >= MIN_NAME_LENGTH:
duplicate_lead_ids |= return_if_relevant('crm.lead', common_lead_domain + [
('contact_name', 'ilike', lead.contact_name)
('email_domain_criterion', '=', lead.email_domain_criterion)
])
# check for "same commercial entity" duplicates
if lead.partner_id and lead.partner_id.commercial_partner_id:
duplicate_lead_ids |= lead.with_context(active_test=False).search(common_lead_domain + [
("partner_id", "child_of", lead.partner_id.commercial_partner_id.id)
])
if lead.phone and len(lead.phone) >= MIN_PHONE_LENGTH:
# check the phone number duplicates, based on phone_sanitized. Only
# exact matches are found, and the single one stored in phone_sanitized
# in case phone and mobile are both set.
if lead.phone_sanitized:
duplicate_lead_ids |= return_if_relevant('crm.lead', common_lead_domain + [
('phone_mobile_search', 'ilike', lead.phone)
])
if lead.mobile and len(lead.mobile) >= MIN_PHONE_LENGTH:
duplicate_lead_ids |= return_if_relevant('crm.lead', common_lead_domain + [
('phone_mobile_search', 'ilike', lead.mobile)
('phone_sanitized', '=', lead.phone_sanitized)
])
lead.duplicate_lead_ids = duplicate_lead_ids + lead
addons/crm/tests/test_crm_lead_duplicates.py
+208 -201
View File
@@ -2,220 +2,227 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from odoo.addons.crm.tests.common import TestCrmCommon
from odoo.addons.iap.tools import iap_tools
from odoo.tests.common import tagged, users
@tagged('lead_manage')
class TestLeadConvert(TestCrmCommon):
@tagged('lead_internals')
class TestCRMLead(TestCrmCommon):
@users('user_sales_manager')
def test_potential_duplicates(self):
company = self.env['res.partner'].create({
'name': 'My company',
'email': 'mycompany@company.com',
'is_company': True,
'street': '57th Street',
@classmethod
def setUpClass(cls):
super().setUpClass()
cls.emails_provider_generic = {
('robert.poilvert@gmail.com', 'robert.poilvert@gmail.com'),
('fp@odoo.com', 'fp@odoo.com'),
('fp.alias@mail.odoo.com', 'fp.alias@mail.odoo.com'),
}
cls.emails_provider_company = {
('robert.poilvert@mycompany.com', 'mycompany.com'),
('fp@subdomain.odoo.com', 'subdomain.odoo.com'),
}
# customer data
country_us_id = cls.env.ref('base.us').id
cls.test_company = cls.env['res.partner'].create({
'city': 'New New York',
'country_id': self.env.ref('base.us').id,
'country_id': country_us_id,
'email': 'test.company@another.email.company.com',
'is_company': True,
'name': 'My company',
'street': '57th Street',
'zip': '12345',
})
cls.test_partners = cls.env['res.partner'].create([
{
'city': 'New York',
'country_id': country_us_id,
'email': 'dave@another.email.company.com',
'is_company': False,
'mobile': '+1 202 000 0123',
'name': 'Dave',
'phone': False,
'parent_id': cls.test_company.id,
'street': 'Pearl street',
'zip': '12345',
},
{
'city': 'New York',
'country_id': country_us_id,
'email': 'eve@another.email.company.com',
'is_company': False,
'mobile': '+1 202 000 3210',
'name': 'Eve',
'parent_id': cls.test_company.id,
'phone': False,
'street': 'Wall street',
'zip': '12345',
}
])
partner_1 = self.env['res.partner'].create({
'name': 'Dave',
'email': 'dave@odoo.com',
'mobile': '+1 202 555 0123',
'phone': False,
'parent_id': company.id,
'is_company': False,
'street': 'Pearl street',
'city': 'California',
'country_id': self.env.ref('base.us').id,
'zip': '95826',
})
partner_2 = self.env['res.partner'].create({
'name': 'Eve',
'email': 'eve@odoo.com',
'mobile': '+1 202 555 3210',
'phone': False,
'parent_id': company.id,
'is_company': False,
'street': 'Wall street',
'city': 'New York',
'country_id': self.env.ref('base.us').id,
'zip': '54321',
})
lead_1 = self.env['crm.lead'].create({
'name': 'Lead 1',
'type': 'lead',
'partner_name': 'Alice',
'email_from': 'alice@odoo.com',
})
lead_2 = self.env['crm.lead'].create({
'name': 'Opportunity 1',
'type': 'opportunity',
'email_from': 'alice@odoo.com',
})
lead_3 = self.env['crm.lead'].create({
'name': 'Opportunity 2',
'type': 'opportunity',
'email_from': 'alice@odoo.com',
})
lead_4 = self.env['crm.lead'].create({
'name': 'Lead 2',
'type': 'lead',
'partner_name': 'Alice Doe'
})
lead_5 = self.env['crm.lead'].create({
'name': 'Opportunity 3',
'type': 'opportunity',
'partner_name': 'Alice Doe'
})
lead_6 = self.env['crm.lead'].create({
'name': 'Opportunity 4',
'type': 'opportunity',
'partner_name': 'Bob Doe'
})
lead_7 = self.env['crm.lead'].create({
'name': 'Opportunity 5',
'type': 'opportunity',
'partner_name': 'Bob Doe',
'email_from': 'bob@odoo.com',
})
lead_8 = self.env['crm.lead'].create({
'name': 'Opportunity 6',
'type': 'opportunity',
'email_from': 'bob@mymail.com',
})
lead_9 = self.env['crm.lead'].create({
'name': 'Opportunity 7',
'type': 'opportunity',
'email_from': 'alice@mymail.com',
})
lead_10 = self.env['crm.lead'].create({
'name': 'Opportunity 8',
'type': 'opportunity',
'probability': 0,
'active': False,
'email_from': 'alice@mymail.com',
})
lead_11 = self.env['crm.lead'].create({
'name': 'Opportunity 9',
'type': 'opportunity',
'contact_name': 'charlie'
})
lead_12 = self.env['crm.lead'].create({
'name': 'Opportunity 10',
'type': 'opportunity',
'contact_name': 'Charlie Chapelin',
})
lead_13 = self.env['crm.lead'].create({
'name': 'Opportunity 8',
'type': 'opportunity',
'partner_id': partner_1.id
})
lead_14 = self.env['crm.lead'].create({
'name': 'Lead 3',
'type': 'lead',
'partner_id': partner_2.id
})
self.assertEqual(lead_1 + lead_2 + lead_3, lead_1.duplicate_lead_ids)
self.assertEqual(lead_1 + lead_2 + lead_3, lead_2.duplicate_lead_ids)
self.assertEqual(lead_1 + lead_2 + lead_3, lead_3.duplicate_lead_ids)
self.assertEqual(lead_4 + lead_5, lead_4.duplicate_lead_ids)
self.assertEqual(lead_4 + lead_5, lead_5.duplicate_lead_ids)
self.assertEqual(lead_6 + lead_7, lead_6.duplicate_lead_ids)
self.assertEqual(lead_6 + lead_7, lead_7.duplicate_lead_ids)
self.assertEqual(lead_8 + lead_9 + lead_10, lead_8.duplicate_lead_ids)
self.assertEqual(lead_8 + lead_9 + lead_10, lead_9.duplicate_lead_ids)
self.assertEqual(lead_8 + lead_9 + lead_10, lead_10.duplicate_lead_ids)
self.assertEqual(lead_11 + lead_12, lead_11.duplicate_lead_ids)
self.assertEqual(lead_12, lead_12.duplicate_lead_ids)
self.assertEqual(lead_13 + lead_14, lead_13.duplicate_lead_ids)
self.assertEqual(lead_13 + lead_14, lead_14.duplicate_lead_ids)
@users('user_sales_manager')
def test_potential_duplicates_with_phone(self):
customer = self.env['res.partner'].create({
'email': 'customer1@duplicate.example.com',
'mobile': '+32485001122',
'name': 'Customer1',
'phone': '(803)-456-6126',
})
base_lead = self.env['crm.lead'].create({
'name': 'Base Lead',
'partner_id': customer.id,
# base leads on which duplicate detection is performed
cls.lead_generic = cls.env['crm.lead'].create({
'country_id': country_us_id,
'email_from': 'FP@odoo.com',
'name': 'Generic 1',
'mobile': False,
'partner_id': cls.test_partners[0].id,
'phone': '+1 202 555 0123',
'type': 'lead',
})
self.assertEqual(base_lead.contact_name, customer.name)
self.assertEqual(base_lead.mobile, customer.mobile)
self.assertFalse(base_lead.partner_name)
self.assertEqual(base_lead.phone, customer.phone)
dup1_1 = self.env['crm.lead'].create({
'name': 'Base Lead Dup1',
'type': 'lead',
'phone': '456-6126', # shorter version of base_lead
'partner_name': 'Partner Name 1',
})
dup1_2 = self.env['crm.lead'].create({
'name': 'Base Lead Dup2',
'mobile': '8034566126',
'partner_name': 'Partner Name 2',
'type': 'lead',
})
dup1_3 = self.env['crm.lead'].create({
'name': 'Base Lead Dup3',
'partner_name': 'Partner Name 3',
'phone': '(803)-456-6126',
'type': 'lead',
})
dup1_4 = self.env['crm.lead'].create({
'mobile': '0032485001122',
# 'mobile': '0485001122', # note: does not work
'name': 'Base Lead Dup4',
'partner_name': 'Partner Name 4',
cls.lead_company = cls.env['crm.lead'].create({
'country_id': country_us_id,
'email_from': 'floppy@MYCOMPANY.com',
'mobile': '+1 202 666 4567',
'partner_id': False,
'name': 'CompanyMail 1',
'phone': False,
'type': 'lead',
})
expected = base_lead + dup1_2 + dup1_3 + dup1_4 # dup1_1 is shorter than lead -> not a dupe
self.assertEqual(
base_lead.duplicate_lead_ids, expected,
'CRM: missing %s, extra %s' % ((expected - base_lead.duplicate_lead_ids).mapped('name'), (base_lead.duplicate_lead_ids - expected).mapped('name'))
)
expected = base_lead + dup1_1 + dup1_2 + dup1_3 # dup1_4 has mobile of customer, but no link with dup1_1
self.assertEqual(
dup1_1.duplicate_lead_ids, expected,
'CRM: missing %s, extra %s' % ((expected - dup1_1.duplicate_lead_ids).mapped('name'), (dup1_1.duplicate_lead_ids - expected).mapped('name'))
)
# duplicates
cls.lead_generic_email_dupes = cls.env['crm.lead'].create([
# email based: normalized version used for email domain criterion
{
'email_from': '"Fabulous Fab" <fp@ODOO.COM>',
'name': 'Dupe1 of fp@odoo.com (same email)',
'type': 'lead',
},
{
'email_from': 'FP@odoo.com',
'name': 'Dupe2 of fp@odoo.com (same email)',
'type': 'lead',
},
# phone_sanitized based
{
'email_from': 'not.fp@not.odoo.com',
'name': 'Dupe3 of fp@odoo.com (same phone sanitized)',
'phone': '+1 202 555 0123',
'type': 'lead',
},
{
'email_from': 'not.fp@not.odoo.com',
'mobile': '+1 202 555 0123',
'name': 'Dupe4 of fp@odoo.com (same phone sanitized)',
'type': 'lead',
},
# same commercial entity
{
'name': 'Dupe5 of fp@odoo.com (same commercial entity)',
'partner_id': cls.test_partners[1].id,
},
{
'name': 'Dupe6 of fp@odoo.com (same commercial entity)',
'partner_id': cls.test_company.id,
}
])
cls.lead_generic_email_notdupes = cls.env['crm.lead'].create([
# email: check for exact match
{
'email_from': 'not.fp@odoo.com',
'name': 'NotADupe1',
'type': 'lead',
},
])
cls.lead_company_email_dupes = cls.env['crm.lead'].create([
# email based: normalized version used for email domain criterion
{
'email_from': '"The Other Fabulous Fab" <fp@mycompany.COM>',
'name': 'Dupe1 of mycompany@mycompany.com (same company)',
'type': 'lead',
},
{
'email_from': '"Same Email" <floppy@mycompany.com>',
'name': 'Dupe2 of mycompany@mycompany.com (same company)',
'type': 'lead',
},
# phone_sanitized based
{
'email_from': 'not.floppy@not.mycompany.com',
'name': 'Dupe3 of fp@odoo.com (same phone sanitized)',
'phone': '+1 202 666 4567',
'type': 'lead',
},
{
'email_from': 'not.floppy@not.mycompany.com',
'mobile': '+1 202 666 4567',
'name': 'Dupe4 of fp@odoo.com (same phone sanitized)',
'type': 'lead',
},
])
cls.lead_company_email_notdupes = cls.env['crm.lead'].create([
# email: check same company
{
'email_from': 'floppy@zboing.MYCOMPANY.com',
'name': 'NotADupe2',
'type': 'lead',
},
])
@users('user_sales_manager')
def test_potential_duplicates_with_invalid_email(self):
lead_1 = self.env['crm.lead'].create({
'name': 'Lead 1',
'type': 'lead',
'email_from': 'mail"1@mymail.com'
})
lead_2 = self.env['crm.lead'].create({
'name': 'Opportunity 1',
'type': 'opportunity',
'email_from': 'mail2@mymail.com'
})
lead_3 = self.env['crm.lead'].create({
'name': 'Opportunity 2',
'type': 'lead',
'email_from': 'odoo.com'
})
lead_4 = self.env['crm.lead'].create({
'name': 'Opportunity 3',
'type': 'opportunity',
'email_from': 'odoo.com'
})
def test_assert_initial_values(self):
""" Just be sure of initial value for those tests """
lead_generic = self.lead_generic.with_env(self.env)
self.assertEqual(lead_generic.phone_sanitized, '+12025550123')
self.assertEqual(lead_generic.email_domain_criterion, 'fp@odoo.com')
self.assertEqual(lead_generic.email_normalized, 'fp@odoo.com')
self.assertEqual(lead_1 + lead_2, lead_1.duplicate_lead_ids)
self.assertEqual(lead_2, lead_2.duplicate_lead_ids, 'Using email_normalized: does not found invalid lead_1, not that annoying')
self.assertEqual(lead_3, lead_3.duplicate_lead_ids, 'Using email_normalized: does not found invalid lead_4, not that annoying')
self.assertEqual(lead_4, lead_4.duplicate_lead_ids, 'Using email_normalized: does not found invalid lead_3, not that annoying')
lead_company = self.lead_company.with_env(self.env)
self.assertEqual(lead_company.phone_sanitized, '+12026664567')
self.assertEqual(lead_company.email_domain_criterion, '@mycompany.com')
self.assertEqual(lead_company.email_normalized, 'floppy@mycompany.com')
@users('user_sales_leads')
def test_crm_lead_duplicates_fetch(self):
""" Test heuristic to find duplicates of a given lead. """
# generic provider-based email
lead_generic = self.lead_generic.with_env(self.env)
self.assertEqual(lead_generic.duplicate_lead_ids,
lead_generic + self.lead_generic_email_dupes,
'Duplicates: exact email matching (+ self)')
# company-based email
lead_company = self.lead_company.with_env(self.env)
self.assertEqual(lead_company.duplicate_lead_ids,
lead_company + self.lead_company_email_dupes,
'Duplicates: exact email matching (+ self)')
@users('user_sales_leads')
def test_crm_lead_email_domain_criterion(self):
""" Test computed field 'email_domain_criterion' used notably to fetch
duplicates. """
for test_email, provider in self.emails_provider_generic:
with self.subTest(test_email=test_email, provider=provider):
lead = self.env['crm.lead'].create({
'email_from': test_email,
'name': test_email,
})
self.assertEqual(lead.email_domain_criterion, provider)
for test_email, provider in self.emails_provider_company:
with self.subTest(test_email=test_email, provider=provider):
lead = self.env['crm.lead'].create({
'email_from': test_email,
'name': test_email,
})
self.assertEqual(lead.email_domain_criterion, f'@{provider}',)
@users('user_sales_leads')
def test_iap_tools(self):
""" Test iap tools specifically """
for test_email, provider in self.emails_provider_generic:
with self.subTest(test_email=test_email, provider=provider):
self.assertEqual(
iap_tools.mail_prepare_for_domain_search(test_email),
test_email,
'As provider is a generic one, complete email should be returned for a company-based mail search'
)
for test_email, provider in self.emails_provider_company:
with self.subTest(test_email=test_email, provider=provider):
self.assertEqual(
iap_tools.mail_prepare_for_domain_search(test_email),
f'@{provider}',
'As provider is a company one, only the domain part should be returned for a company-based mail search'
)