From 023bc673274d1ebc0003d3bdae8faa30950a4851 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 30 Sep 2022 13:39:32 +0000 Subject: [PATCH] [IMP] mail: introduce a batch version of Partner._find_or_create () Purpose is to be able to create find or create partners in batch based on a list of emails. When sending email in batch, notably using templates, doing sequentially a search or create for each potential email to contact is not efficient from computation point of view. This commit introduces a new batch method for that purpose. If no valid email is found for a given email, it is used as it to try to find partners with same invalid email and/or keep the value in order to allow updating it afterwards. Notably with notifications resend it is possible to update emails, if only a typo prevents from having a real email for example. Falsy emails do not create partners as those have no real interest. Task-3034875 (Mail: Speedup and batch partners find or create with templates) Prepares Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#99482 --- addons/mail/models/res_partner.py | 91 ++++++++++ addons/mail/tests/test_res_partner.py | 230 +++++++++++++++++++++++-- odoo/addons/base/models/res_partner.py | 6 +- 3 files changed, 314 insertions(+), 13 deletions(-) diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 39751f708cd..cf821a0dbcb 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -102,6 +102,97 @@ class Partner(models.Model): create_values['email'] = parsed_email return self.create(create_values) + @api.model + def _find_or_create_from_emails(self, emails, additional_values=None): + """ Based on a list of emails, find or create partners. Additional values + can be given to newly created partners. If an email is not unique (e.g. + multi-email input), only the first found email is considered. + + If no valid email is found for a given item, the given value is used to + find partners with same invalid email or create a new one with the wrong + value. It allows updating it afterwards. Notably with notifications + resend it is possible to update emails, if only a typo prevents from + having a real email for example. + + :param list emails: list of emails that may be formatted (each input + will be parsed and normalized); + :param dict additional_values: additional values given to create if + the partner is not found. Typically used to propagate a company_id; + + :return: res.partner records in a list, following order of emails. It + is not a recordset, to keep Falsy values. + """ + partners, tocreate_vals_list = self.env['res.partner'], [] + name_emails = [self._parse_partner_name(email) for email in emails] + + # find valid emails_normalized, filtering out false / void values, and search + # for existing partners based on those emails + emails_normalized = {email_normalized + for _name, email_normalized in name_emails + if email_normalized} + # find partners for invalid (but not void) emails, aka either invalid email + # either no email and a name that will be used as email + names = { + name.strip() + for name, email_normalized in name_emails + if not email_normalized and name.strip() + } + if emails_normalized or names: + domains = [] + if emails_normalized: + domains.append([('email_normalized', 'in', list(emails_normalized))]) + if names: + domains.append([('email', 'in', list(names))]) + partners += self.search(expression.OR(domains)) + + # create partners for valid email without any existing partner. Keep + # only first found occurrence of each normalized email, aka: ('Norbert', + # 'norbert@gmail.com'), ('Norbert With Surname', 'norbert@gmail.com')' + # -> a single partner is created for email 'norbert@gmail.com' + seen = set() + notfound_emails = (emails_normalized - set(partners.mapped('email_normalized'))) if partners else emails_normalized + notfound_name_emails = [ + name_email + for name_email in name_emails + if name_email[1] in notfound_emails and name_email[1] not in seen + and not seen.add(name_email[1]) + ] + tocreate_vals_list += [ + { + self._rec_name: name or email_normalized, + 'email': email_normalized, + **(additional_values or {}) + } + for name, email_normalized in notfound_name_emails + ] + + # create partners for invalid emails (aka name and not email_normalized) + # without any existing partner + tocreate_vals_list += [ + { + self._rec_name: name, + 'email': name, + **(additional_values or {}) + } + for name in names if name not in partners.mapped('email') + ] + + # create partners once + if tocreate_vals_list: + partners += self.create(tocreate_vals_list) + + return [ + next( + (partner for partner in partners + if (email_normalized and partner.email_normalized == email_normalized) + or (not email_normalized and email and partner.email == email) + or (not email_normalized and name and partner.name == name) + ), + self.env['res.partner'] + ) + for (name, email_normalized), email in zip(name_emails, emails) + ] + # ------------------------------------------------------------ # DISCUSS # ------------------------------------------------------------ diff --git a/addons/mail/tests/test_res_partner.py b/addons/mail/tests/test_res_partner.py index 8354c7c32f9..061840cacb7 100644 --- a/addons/mail/tests/test_res_partner.py +++ b/addons/mail/tests/test_res_partner.py @@ -1,29 +1,57 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from contextlib import contextmanager +from unittest.mock import patch from uuid import uuid4 +from odoo import tools from odoo.addons.mail.tests.common import MailCommon, mail_new_test_user -from odoo.tests.common import Form, users - +from odoo.tests.common import Form, tagged, users +from odoo.addons.base.models.res_partner import Partner # samples use effective TLDs from the Mozilla public suffix # list at http://publicsuffix.org SAMPLES = [ ('"Raoul Grosbedon" ', 'Raoul Grosbedon', 'raoul@chirurgiens-dentistes.fr'), - ('ryu+giga-Sushi@aizubange.fukushima.jp', '', 'ryu+giga-Sushi@aizubange.fukushima.jp'), + ('ryu+giga-Sushi@aizubange.fukushima.jp', 'ryu+giga-sushi@aizubange.fukushima.jp', 'ryu+giga-Sushi@aizubange.fukushima.jp'), ('Raoul chirurgiens-dentistes.fr', 'Raoul chirurgiens-dentistes.fr', ''), (" Raoul O'hara ", "Raoul O'hara", '!@historicalsociety.museum'), ('Raoul Grosbedon ', 'Raoul Grosbedon', 'raoul@CHIRURGIENS-dentistes.fr'), ('Raoul megaraoul@chirurgiens-dentistes.fr', 'Raoul', 'megaraoul@chirurgiens-dentistes.fr'), - ('"Patrick Da Beast Poilvache" ', 'Patrick Poilvache', 'patrick@example.com'), - ('Patrick Caché ', 'Patrick Poilvache', 'patrick@example.com'), - ('Patrick Caché <2patrick@EXAMPLE.COM>', 'Patrick Caché', '2patrick@example.com'), - + ('"Patrick Da Beast Poilvache" ', 'Patrick Da Beast Poilvache', 'patrick@example.com'), + ('Patrick Caché ', 'Patrick Da Beast Poilvache', 'patrick@example.com'), + ('Patrick Caché ', 'Patrick Caché', 'patrick.2@example.com'), ] + +@tagged('res_partner') class TestPartner(MailCommon): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls._activate_multi_company() + + @contextmanager + def mockPartnerCalls(self): + _original_create = Partner.create + _original_search = Partner.search + self._new_partners = self.env['res.partner'] + + def _res_partner_create(model, *args, **kwargs): + records = _original_create(model, *args, **kwargs) + self._new_partners += records.sudo() + return records + + with patch.object(Partner, 'create', + autospec=True, side_effect=_res_partner_create) as mock_partner_create, \ + patch.object(Partner, 'search', + autospec=True, side_effect=_original_search) as mock_partner_search: + self._mock_partner_create = mock_partner_create + self._mock_partner_search = mock_partner_search + yield + def _check_find_or_create(self, test_string, expected_name, expected_email, expected_email_normalized=False, check_partner=False, should_create=False): expected_email_normalized = expected_email_normalized or expected_email partner = self.env['res.partner'].find_or_create(test_string) @@ -36,16 +64,42 @@ class TestPartner(MailCommon): self.assertEqual(partner.email_normalized or '', expected_email_normalized) return partner - def test_res_partner_find_or_create(self): - Partner = self.env['res.partner'] + def test_parse_partner_name(self): + samples = [ + 'Raoul raoul@grosbedon.fr', + 'Raoul chirurgiens-dentistes.fr', + 'invalid', + 'False', + '', + ' ', + False, + None, + ] + expected = [ + ('Raoul', 'raoul@grosbedon.fr'), + ('Raoul chirurgiens-dentistes.fr', ''), + ('invalid', ''), + ('False', ''), + ('', ''), + ('', ''), + ('', ''), + ('', ''), + ] + for (expected_name, expected_email), sample in zip(expected, samples): + parsed = self.env['res.partner']._parse_partner_name(sample) + self.assertEqual(parsed[0], expected_name) + self.assertEqual(parsed[1], expected_email) - partner = Partner.browse(Partner.name_create(SAMPLES[0][0])[0]) + def test_res_partner_find_or_create(self): + PartnerModel = self.env['res.partner'] + + partner = PartnerModel.browse(PartnerModel.name_create(SAMPLES[0][0])[0]) self._check_find_or_create( SAMPLES[0][0], SAMPLES[0][1], SAMPLES[0][2], check_partner=partner, should_create=False ) - partner_2 = Partner.browse(Partner.name_create('sarah.john@connor.com')[0]) + partner_2 = PartnerModel.browse(PartnerModel.name_create('sarah.john@connor.com')[0]) found_2 = self._check_find_or_create( 'john@connor.com', 'john@connor.com', 'john@connor.com', check_partner=partner_2, should_create=True @@ -76,7 +130,7 @@ class TestPartner(MailCommon): check_partner=new4, should_create=True ) - existing = Partner.create({ + existing = PartnerModel.create({ 'name': SAMPLES[6][1], 'email': SAMPLES[6][0], }) @@ -98,6 +152,158 @@ class TestPartner(MailCommon): with self.assertRaises(ValueError): self.env['res.partner'].find_or_create("Raoul chirurgiens-dentistes.fr", assert_valid_email=True) + @users('employee_c2') + def test_res_partner_find_or_create_from_emails(self): + """ Test for _find_or_create_from_emails allowing to find or create + partner based on emails in a batch-enabled and optimized fashion. """ + self.user_employee_c2.write({ + 'groups_id': [(4, self.env.ref('base.group_partner_manager').id)], + }) + + samples_emails = [item[0] for item in SAMPLES] + partners = self.env['res.partner'].with_context(lang='en_US')._find_or_create_from_emails( + samples_emails, + additional_values=None, + ) + self.assertEqual(len(partners), len(SAMPLES)) + for (sample, exp_name, exp_email), partner in zip(SAMPLES, partners): + exp_email_normalized = tools.email_normalize(exp_email) + self.assertFalse(partner.company_id) + self.assertEqual(partner.email_normalized, exp_email_normalized) + self.assertTrue(partner.id) + self.assertEqual(partner.lang, 'en_US') + self.assertEqual(partner.name, exp_name) + + new_samples = SAMPLES + [ + ('"New Customer" ', 'New Customer', 'new.customer@test.example.com'), + ('"Duplicated Raoul" ', 'Raoul Grosbedon', 'raoul@chirurgiens-dentistes.fr'), + ('Invalid', 'Invalid', ''), + (False, False, False), + (None, False, False), + (' ', False, False), + ('', False, False), + ] + samples_emails = [item[0] for item in new_samples] + partners = self.env['res.partner'].with_context(lang='en_US')._find_or_create_from_emails( + samples_emails, + additional_values={'company_id': self.env.company.id}, + ) + self.assertEqual(len(partners), len(new_samples)) + for (sample, exp_name, exp_email), partner in zip(new_samples, partners): + with self.subTest(sample=sample, exp_name=exp_name, exp_email=exp_email, partner=partner): + exp_company = self.env.company if sample in [ + '"New Customer" ', # valid email, not known -> new customer + 'Invalid' # invalid email, not known -> create a new partner + ] else self.env['res.company'] + if sample in [False, None, ' ', '']: + self.assertFalse(partner) + else: + exp_email_normalized = tools.email_normalize(exp_email) + self.assertEqual(partner.company_id, exp_company) + self.assertEqual(partner.email_normalized, exp_email_normalized) + self.assertEqual(partner.name, exp_name) + + @users('employee_c2') + def test_res_partner_find_or_create_from_emails_dupes(self): + """ Specific test for duplicates management: based on email to avoid + creating similar partners. """ + self.user_employee_c2.write({ + 'groups_id': [(4, self.env.ref('base.group_partner_manager').id)], + }) + + # all same partner, same email 'test.customer@test.dupe.example.com' + email_dupes_samples = [ + '"Formatted Customer" ', + 'test.customer@test.dupe.example.com', + '"Another Name" ', + '"Mix of both" ', + 'falsy', + ' ', + ] + with self.mockPartnerCalls(): + other_partners = self.env['res.partner'].with_context(lang='en_US')._find_or_create_from_emails( + other_samples, + additional_values=None, + ) + # calls + self.assertEqual(self._mock_partner_create.call_count, 1) + self.assertEqual(self._mock_partner_search.call_count, 1) + self.assertEqual(len(self._new_partners), 3) + # results + self.assertEqual(len(other_partners), len(other_samples)) + self.assertEqual(other_partners[0], new_partners[0], 'Should take already existing partner') + self.assertEqual(other_partners[1], partners[0], 'Should take already existing partner') + self.assertEqual(other_partners[2].email, "test.customer3@test.dupe.example.com") + self.assertEqual(other_partners[2].name, "Third Customer") + self.assertEqual(other_partners[3].email, '"Falsy" ') + self.assertEqual(other_partners[3].name, '"Falsy" ') + self.assertEqual(other_partners[4].email, "falsy") + self.assertEqual(other_partners[4].name, "falsy") + def test_res_partner_log_portal_group(self): Users = self.env['res.users'] subtype_note = self.env.ref('mail.mt_note') diff --git a/odoo/addons/base/models/res_partner.py b/odoo/addons/base/models/res_partner.py index f3ce9d1870e..8e4c6ced2e8 100644 --- a/odoo/addons/base/models/res_partner.py +++ b/odoo/addons/base/models/res_partner.py @@ -820,7 +820,11 @@ class Partner(models.Model): Otherwise: default, everything is set as the name. Starting from 13.3 returned email will be normalized to have a coherent encoding. - """ + + :return: name, email (normalized if possible) + """ + if not text or not text.strip(): + return '', '' name, email = '', '' split_results = tools.email_split_tuples(text) if split_results: