From 7352973043d8d954a6d3e2c90024e21e47479c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 17 Jul 2023 09:47:49 +0000 Subject: [PATCH 1/5] [FIX] event_crm: correctly update lead phone when created from registration When having rules creating leads from registrations, email and phone are copied from the registration if the linked partner has no value for those fields. However an error in the code lead to setting phone into email which is not really correct. This commit also add tests for event_crm who allowed to spot other issues which are fixed in the next commits. Task-3431124 X-original-commit: 9bbf621e8e9cda90a0d29fd44e21cd4b61a54d73 Part-of: odoo/odoo#129269 --- addons/event_crm/models/event_registration.py | 2 +- addons/event_crm/tests/common.py | 11 ++- addons/event_crm/tests/test_event_crm_flow.py | 91 +++++++++++-------- 3 files changed, 59 insertions(+), 45 deletions(-) diff --git a/addons/event_crm/models/event_registration.py b/addons/event_crm/models/event_registration.py index 2fd8e9e4ac1..994500f26ed 100644 --- a/addons/event_crm/models/event_registration.py +++ b/addons/event_crm/models/event_registration.py @@ -220,7 +220,7 @@ class EventRegistration(models.Model): if not valid_partner.email: contact_vals['email_from'] = self._find_first_notnull('email') if not valid_partner.phone: - contact_vals['email_from'] = self._find_first_notnull('phone') + contact_vals['phone'] = self._find_first_notnull('phone') else: # don't force email_from + partner_id because those fields are now synchronized automatically contact_vals = { diff --git a/addons/event_crm/tests/common.py b/addons/event_crm/tests/common.py index 6d53c0fb41f..8197aca73ea 100644 --- a/addons/event_crm/tests/common.py +++ b/addons/event_crm/tests/common.py @@ -3,7 +3,7 @@ from datetime import datetime, timedelta -from odoo import fields +from odoo import fields, tools from odoo.addons.crm.tests.common import TestCrmCommon from odoo.addons.event.tests.common import EventCase @@ -99,8 +99,8 @@ class EventCrmCase(TestCrmCommon, EventCase): self.assertNotIn('False', lead.name) # avoid a "Dear False" like construct ^^ (this assert is serious and intended) self.assertEqual(lead.contact_name, expected_contact_name) self.assertEqual(lead.partner_name, expected_partner_name) - self.assertEqual(lead.email_from, partner.email if partner else registrations._find_first_notnull('email')) - self.assertEqual(lead.phone, partner.phone if partner else registrations._find_first_notnull('phone')) + self.assertEqual(lead.email_from, partner.email if partner and partner.email else registrations._find_first_notnull('email')) + self.assertEqual(lead.phone, partner.phone if partner and partner.phone else registrations._find_first_notnull('phone')) self.assertEqual(lead.mobile, partner.mobile if partner and partner.mobile else registrations._find_first_notnull('mobile')) # description: to improve @@ -111,7 +111,10 @@ class EventCrmCase(TestCrmCommon, EventCase): elif registration.partner_id.name: self.assertIn(registration.partner_id.name, lead.description) if registration.email: - self.assertIn(registration.email, lead.description) + if tools.email_normalize(registration.email) == registration.partner_id.email_normalized: + self.assertIn(registration.partner_id.email, lead.description) + else: + self.assertIn(tools.email_normalize(registration.email), lead.description) if registration.phone: self.assertIn(registration.phone, lead.description) diff --git a/addons/event_crm/tests/test_event_crm_flow.py b/addons/event_crm/tests/test_event_crm_flow.py index 74352c775db..3a32ea87e7d 100644 --- a/addons/event_crm/tests/test_event_crm_flow.py +++ b/addons/event_crm/tests/test_event_crm_flow.py @@ -18,11 +18,14 @@ class TestEventCrmFlow(TestEventCrmCommon): ] cls.registration_values[-1]['email'] = '"John Doe" ' - def assert_initial_data(self): + def test_assert_initial_data(self): + """ Ensure base test values to ease test understanding and maintenance """ self.assertEqual(len(self.registration_values), 5) + + self.assertEqual(self.event_customer.country_id, self.env.ref('base.be')) self.assertEqual(self.event_customer.email_normalized, 'constantin@test.example.com') - self.assertEqual(self.event_customer.phone, '0485112233') self.assertFalse(self.event_customer.mobile) + self.assertEqual(self.event_customer.phone, '0485112233') @users('user_eventregistrationdesk') def test_event_crm_flow_batch_create(self): @@ -97,43 +100,24 @@ class TestEventCrmFlow(TestEventCrmCommon): self.assertNotIn('invalid@not.example.com', lead.description) @users('user_eventregistrationdesk') - def test_event_crm_flow_per_attendee_single(self): - self.assert_initial_data() - - # test: partner-based contact information, everything synchonized - registration = self.env['event.registration'].create({ - 'partner_id': self.event_customer.id, - 'event_id': self.event_0.id, - }) - self.assertEqual(registration.email, self.event_customer.email) - self.assertEqual(registration.phone, self.event_customer.phone) - - # per-attendee rule - self.assertLeadConvertion(self.test_rule_attendee, registration, partner=registration.partner_id) - - # test: no partner and contact information - registration = self.env['event.registration'].create({ - 'name': 'My Registration', - 'partner_id': False, - 'email': 'super.email@test.example.com', - 'phone': False, - 'mobile': '0456332211', - 'event_id': self.event_0.id, - }) - self.assertEqual(len(self.event_0.registration_ids), 2) - self.assertLeadConvertion(self.test_rule_attendee, registration, partner=None) - - # test: no partner and few information - registration = self.env['event.registration'].create({ - 'name': False, - 'partner_id': False, - 'email': 'giga.email@test.example.com', - 'phone': False, - 'mobile': False, - 'event_id': self.event_0.id, - }) - self.assertEqual(len(self.event_0.registration_ids), 3) - self.assertLeadConvertion(self.test_rule_attendee, registration, partner=None) + def test_event_crm_flow_per_attendee_single_wo_partner(self): + """ Single registration, attendee based, no partner involved, check + contact info propagation """ + for name, email, mobile, phone in [ + ('My Name', 'super.email@test.example.com', '0456442211', '0456332211'), + (False, 'super.email@test.example.com', False, '0456442211'), + ('"My Name"', '"My Name" ', False, False), + ]: + with self.subTest(name=name, email=email, mobile=mobile, phone=phone): + registration = self.env['event.registration'].create({ + 'name': name, + 'partner_id': False, + 'email': email, + 'mobile': mobile, + 'phone': phone, + 'event_id': self.event_0.id, + }) + self.assertLeadConvertion(self.test_rule_attendee, registration, partner=None) # test: partner but with other contact information -> registration prior registration = self.env['event.registration'].create({ @@ -143,9 +127,36 @@ class TestEventCrmFlow(TestEventCrmCommon): 'mobile': '0456112233', 'event_id': self.event_0.id, }) - self.assertEqual(len(self.event_0.registration_ids), 4) self.assertLeadConvertion(self.test_rule_attendee, registration, partner=None) + @users('user_eventregistrationdesk') + def test_event_crm_flow_per_attendee_single_wpartner(self): + """ Single registration, attendee based, with partner involved, check + contact information, check synchronization and update """ + self.event_customer2.write({ + 'email': False, + 'phone': False, + }) + self.test_rule_attendee.write({ + 'event_registration_filter': '[]', # try various email combinations + }) + for email, phone, base_partner, expected_partner in [ + (False, False, self.event_customer, self.event_customer), # should take partner info + ('"Other Name" ', False, self.event_customer, self.env['res.partner']), # same email normalized -> currently not supported + ('other.email@test.example.com', False, self.event_customer, self.env['res.partner']), # not same email -> no partner on lead + (False, '+32485112233', self.event_customer, self.env['res.partner']), # same phone but differently formatted -> currently not supported + (False, '0485112244', self.event_customer, self.env['res.partner']), # other phone -> no partner on lead + ('other.email@test.example.com', '0485112244', self.event_customer2, self.event_customer2), # mail / phone update from registration as void on partner + ]: + with self.subTest(email=email, phone=phone, base_partner=base_partner): + registration = self.env['event.registration'].create({ + 'partner_id': base_partner.id, + 'email': email, + 'phone': phone, + 'event_id': self.event_0.id, + }) + self.assertLeadConvertion(self.test_rule_attendee, registration, partner=expected_partner) + @users('user_eventregistrationdesk') def test_event_crm_trigger_done(self): """Test the case when the "crm.lead.rule" is executed when we write on the From 19d583095d62e6dd3f0177e07d46db0296b37711 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 17 Jul 2023 09:49:26 +0000 Subject: [PATCH 2/5] [FIX] event_crm: compare email based on normalized version when possible When checking if partner and registration have the same email, better compare normalized versions if possible as it filters out formatting differences e.g. if one input has a formatted email with another name. Task-3431124 X-original-commit: 1f2a08e6b0880d20f877a73badd3b071939bcc13 Part-of: odoo/odoo#129269 --- addons/event_crm/models/event_registration.py | 16 ++++++++++++---- addons/event_crm/tests/test_event_crm_flow.py | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/addons/event_crm/models/event_registration.py b/addons/event_crm/models/event_registration.py index 994500f26ed..a4940115e01 100644 --- a/addons/event_crm/models/event_registration.py +++ b/addons/event_crm/models/event_registration.py @@ -3,7 +3,7 @@ from collections import defaultdict -from odoo import api, fields, models, _ +from odoo import api, fields, models, tools, _ from odoo.addons.phone_validation.tools import phone_validation @@ -195,10 +195,18 @@ class EventRegistration(models.Model): ) # CHECKME: broader than just public partner # mono registration mode: keep partner only if email and phone matches; - # otherwise registration > partner. Note that _phone_format has to be - # taken into account as it may have side effects on phone formatting + # otherwise registration > partner. Note that email format has to be + # taken into account in comparison if len(self) == 1 and valid_partner: - if self.email and valid_partner.email and valid_partner.email != self.email: + # compare emails: email_normalized or raw + if self.email and valid_partner.email: + if valid_partner.email_normalized and tools.email_normalize(self.email) != valid_partner.email_normalized: + valid_partner = self.env['res.partner'] + elif not valid_partner.email_normalized and valid_partner.email != self.email: + valid_partner = self.env['res.partner'] + + # compare phone + if valid_partner and valid_partner.phone and self.phone and valid_partner.phone != self.phone: valid_partner = self.env['res.partner'] if valid_partner and self.phone and valid_partner.phone: phone_formatted = phone_validation.phone_format( diff --git a/addons/event_crm/tests/test_event_crm_flow.py b/addons/event_crm/tests/test_event_crm_flow.py index 3a32ea87e7d..c9a74e6e809 100644 --- a/addons/event_crm/tests/test_event_crm_flow.py +++ b/addons/event_crm/tests/test_event_crm_flow.py @@ -142,7 +142,7 @@ class TestEventCrmFlow(TestEventCrmCommon): }) for email, phone, base_partner, expected_partner in [ (False, False, self.event_customer, self.event_customer), # should take partner info - ('"Other Name" ', False, self.event_customer, self.env['res.partner']), # same email normalized -> currently not supported + ('"Other Name" ', False, self.event_customer, self.event_customer), # same email normalized ('other.email@test.example.com', False, self.event_customer, self.env['res.partner']), # not same email -> no partner on lead (False, '+32485112233', self.event_customer, self.env['res.partner']), # same phone but differently formatted -> currently not supported (False, '0485112244', self.event_customer, self.env['res.partner']), # other phone -> no partner on lead From 44e022e27a53507924ab9cb83bc0c241e01f3459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 17 Jul 2023 10:25:53 +0000 Subject: [PATCH 3/5] [FIX] event_crm: correctly compare IDs to check registration changes When checking updated values of a registration to propagate to its linked lead we may compare a recordset to IDs. This commit fixes that by always comparing IDs. Task-3431124 X-original-commit: 874c2cf44c9f42d928b646c86c974251dd95cfc0 Part-of: odoo/odoo#129269 Co-authored-by: Jeremy Hennecart --- addons/event_crm/models/event_registration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addons/event_crm/models/event_registration.py b/addons/event_crm/models/event_registration.py index a4940115e01..adf6e4d5287 100644 --- a/addons/event_crm/models/event_registration.py +++ b/addons/event_crm/models/event_registration.py @@ -348,8 +348,8 @@ class EventRegistration(models.Model): def _convert_value(self, value, field_name): """ Small tool because convert_to_write is touchy """ - if value and self._fields[field_name].type in ['many2many', 'one2many']: + if isinstance(value, models.BaseModel) and self._fields[field_name].type in ['many2many', 'one2many']: return value.ids - if value and self._fields[field_name].type == 'many2one': + if isinstance(value, models.BaseModel) and self._fields[field_name].type == 'many2one': return value.id return value From d43afeacb6f0716411626c861380d56505c3f53b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 5 Jun 2023 08:28:06 +0000 Subject: [PATCH 4/5] [FIX] event_crm: compare formatted numbers when possible Partial backport of odoo/odoo@94fa8d962535a754c9fb26997a38cc8b51b8974e keeping only event_crm part as other changes were done only for Odoo16. In this commit we try to compare formatted phones of registration and partner, before checking the actual phone numbers. If only formatting differs then it is not a different number. Task-3431124 X-original-commit: 4589eebe4978ac0746fa4bc5c8664cb30d912cc1 Part-of: odoo/odoo#129269 --- addons/event_crm/models/event_registration.py | 8 +++----- addons/event_crm/tests/test_event_crm_flow.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/addons/event_crm/models/event_registration.py b/addons/event_crm/models/event_registration.py index adf6e4d5287..b71a2e42906 100644 --- a/addons/event_crm/models/event_registration.py +++ b/addons/event_crm/models/event_registration.py @@ -195,8 +195,8 @@ class EventRegistration(models.Model): ) # CHECKME: broader than just public partner # mono registration mode: keep partner only if email and phone matches; - # otherwise registration > partner. Note that email format has to be - # taken into account in comparison + # otherwise registration > partner. Note that email format and phone + # formatting have to taken into account in comparison if len(self) == 1 and valid_partner: # compare emails: email_normalized or raw if self.email and valid_partner.email: @@ -205,9 +205,7 @@ class EventRegistration(models.Model): elif not valid_partner.email_normalized and valid_partner.email != self.email: valid_partner = self.env['res.partner'] - # compare phone - if valid_partner and valid_partner.phone and self.phone and valid_partner.phone != self.phone: - valid_partner = self.env['res.partner'] + # compare phone, taking into account formatting if valid_partner and self.phone and valid_partner.phone: phone_formatted = phone_validation.phone_format( self.phone, diff --git a/addons/event_crm/tests/test_event_crm_flow.py b/addons/event_crm/tests/test_event_crm_flow.py index c9a74e6e809..b0fe62e5f76 100644 --- a/addons/event_crm/tests/test_event_crm_flow.py +++ b/addons/event_crm/tests/test_event_crm_flow.py @@ -144,7 +144,7 @@ class TestEventCrmFlow(TestEventCrmCommon): (False, False, self.event_customer, self.event_customer), # should take partner info ('"Other Name" ', False, self.event_customer, self.event_customer), # same email normalized ('other.email@test.example.com', False, self.event_customer, self.env['res.partner']), # not same email -> no partner on lead - (False, '+32485112233', self.event_customer, self.env['res.partner']), # same phone but differently formatted -> currently not supported + (False, '+32485112233', self.event_customer, self.event_customer), # same phone but differently formatted (False, '0485112244', self.event_customer, self.env['res.partner']), # other phone -> no partner on lead ('other.email@test.example.com', '0485112244', self.event_customer2, self.event_customer2), # mail / phone update from registration as void on partner ]: From 9b313b28e71b6704855e869940b31f2b08fccb60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 17 Jul 2023 09:48:45 +0000 Subject: [PATCH 5/5] [FIX] event_crm: avoid formatting issues in generated description Avoid issues when generating lead description based on linked registrations notably with formatted emails. Task-3431124 X-original-commit: 471025a7bd227d334ac5b450c93a2ee99c22517a Part-of: odoo/odoo#129269 --- addons/event_crm/models/event_registration.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/addons/event_crm/models/event_registration.py b/addons/event_crm/models/event_registration.py index b71a2e42906..1abca8f3602 100644 --- a/addons/event_crm/models/event_registration.py +++ b/addons/event_crm/models/event_registration.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from collections import defaultdict +from markupsafe import Markup from odoo import api, fields, models, tools, _ from odoo.addons.phone_validation.tools import phone_validation @@ -255,17 +256,21 @@ class EventRegistration(models.Model): line_suffix=line_suffix ) for registration in self ] - return ("%s
" % prefix if prefix else "") + ( - "
    " if line_counter else "
      ") + ("".join(reg_lines)) + ("
" if line_counter else "") + description = (prefix if prefix else '') + Markup("
") + if line_counter: + description += Markup("
    ") + Markup('').join(reg_lines) + Markup("
") + else: + description += Markup("
    ") + Markup('').join(reg_lines) + Markup("
") + return description def _get_lead_description_registration(self, line_suffix=''): """ Build the description line specific to a given registration. """ self.ensure_one() - return "
  • %s (%s)%s
    • " % ( + return Markup("
  • ") + "%s (%s)%s" % ( self.name or self.partner_id.name or self.email, " - ".join(self[field] for field in ('email', 'phone') if self[field]), - " %s" % line_suffix if line_suffix else "", - ) + f" {line_suffix}" if line_suffix else "", + ) + Markup("
    • ") def _get_lead_tracked_values(self): """ Tracked values are based on two subset of fields to track in order