diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index 06e38224334..ae682206165 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -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 diff --git a/addons/crm/tests/test_crm_lead_duplicates.py b/addons/crm/tests/test_crm_lead_duplicates.py index 20bf3cd6e50..b40c38e878b 100644 --- a/addons/crm/tests/test_crm_lead_duplicates.py +++ b/addons/crm/tests/test_crm_lead_duplicates.py @@ -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" ', + '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" ', + 'name': 'Dupe1 of mycompany@mycompany.com (same company)', + 'type': 'lead', + }, + { + 'email_from': '"Same Email" ', + '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' + )