[FIX] event_crm: fix lead/registration values propagation, improve tests

Fix various issues that appeared when working with registrations. When converting
registrations to leads using rules some convert flows have issues or can crash. This
PR fixes several issues, see sub commits for more explanations.

Improve test suite to cover more use cases.

Task-3431124

closes odoo/odoo#128823

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

addons/event_crm/models/event_registration.py

@@ -3,7 +3,8 @@
 
 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
 
 
 class EventRegistration(models.Model):
@@ -190,16 +191,42 @@ class EventRegistration(models.Model):
 
         :return dict: values used for create / write on a lead
         """
-        valid_partner = related_partner = next(
+        valid_partner = next(
             (reg.partner_id for reg in self if reg.partner_id != self.env.ref('base.public_partner')),
             self.env['res.partner']
         )  # CHECKME: broader than just public partner
 
-        # mono registration mode: keep partner only if email and phone matches, otherwise registration > partner
-        if len(self) == 1:
-            if (related_partner.phone and self.phone and related_partner.phone != self.phone) or \
-                (related_partner.email and self.email and related_partner.email != self.email):
-                valid_partner = self.env['res.partner']
+        # mono registration mode: keep partner only if email and phone matches;
+        # 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:
+                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, taking into account formatting
+            if valid_partner and self.phone and valid_partner.phone:
+                phone_formatted = phone_validation.phone_format(
+                    self.phone,
+                    valid_partner.country_id.code or None,
+                    valid_partner.country_id.phone_code or None,
+                    force_format='E164',
+                    raise_exception=False
+                )
+                partner_phone_formatted = phone_validation.phone_format(
+                    valid_partner.phone,
+                    valid_partner.country_id.code or None,
+                    valid_partner.country_id.phone_code or None,
+                    force_format='E164',
+                    raise_exception=False
+                )
+                if phone_formatted and partner_phone_formatted and phone_formatted != partner_phone_formatted:
+                    valid_partner = self.env['res.partner']
+                if (not phone_formatted or not partner_phone_formatted) and self.phone != valid_partner.phone:
+                    valid_partner = self.env['res.partner']
 
         if valid_partner:
             contact_vals = self.env['crm.lead']._prepare_values_from_partner(valid_partner)
@@ -207,7 +234,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 = {
@@ -327,8 +354,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

addons/event_crm/tests/common.py

 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
+from odoo import tools
 from odoo.addons.crm.tests.common import TestCrmCommon
 from odoo.addons.event.tests.common import TestEventCommon
 
@@ -99,8 +100,8 @@ class TestEventCrmCommon(TestCrmCommon, TestEventCommon):
 
         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 +112,10 @@ class TestEventCrmCommon(TestCrmCommon, TestEventCommon):
             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(registration.email, lead.description)
             if registration.phone:
                 self.assertIn(registration.phone, lead.description)
 

addons/event_crm/tests/test_event_crm_flow.py

@@ -18,11 +18,14 @@ class TestEventCrmFlow(TestEventCrmCommon):
         ]
         cls.registration_values[-1]['email'] = '"John Doe" <invalid@not.example.com>'
 
-    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_eventmanager')
     def test_event_crm_flow_batch_create(self):
@@ -97,54 +100,62 @@ class TestEventCrmFlow(TestEventCrmCommon):
         self.assertNotIn('invalid@not.example.com', lead.description)
 
     @users('user_eventmanager')
-    def test_event_crm_flow_per_attendee_single(self):
-        self.assert_initial_data()
+    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" <my.name@test.example.com', 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-based contact information, everything synchonized
+        # test: partner but with other contact information -> registration prior
         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',
+            'email': 'other.email@test.example.com',
             'phone': False,
-            'mobile': '0456332211',
+            'mobile': '0456112233',
             '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',
+    @users('user_eventmanager')
+    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,
-            '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)
-
-        # test: partner but with other contact information -> registration prior
-        registration = self.env['event.registration'].create({
-            'partner_id': self.event_customer.id,
-            'email': 'other.email@test.example.com',
-            'phone': False,
-            'mobile': '0456112233',
-            'event_id': self.event_0.id,
+        self.test_rule_attendee.write({
+            'event_registration_filter': '[]',  # try various email combinations
         })
-        self.assertEqual(len(self.event_0.registration_ids), 4)
-        self.assertLeadConvertion(self.test_rule_attendee, registration, partner=None)
+        for email, phone, base_partner, expected_partner in [
+            (False, False, self.event_customer, self.event_customer),  # should take partner info
+            ('"Other Name" <constantin@test.example.com>', 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.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
+        ]:
+            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_eventmanager')
     def test_order_rule_duplicate_lead(self):