[FW][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#129269 Forward-port-of: odoo/odoo#129111 Forward-port-of: odoo/odoo#128823 Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
@@ -2,8 +2,9 @@
|
||||
# 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, _
|
||||
from odoo import api, fields, models, tools, _
|
||||
from odoo.addons.phone_validation.tools import phone_validation
|
||||
|
||||
|
||||
@@ -195,11 +196,17 @@ 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 and phone
|
||||
# formatting have to 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:
|
||||
valid_partner = self.env['res.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,
|
||||
@@ -220,7 +227,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 = {
|
||||
@@ -249,17 +256,21 @@ class EventRegistration(models.Model):
|
||||
line_suffix=line_suffix
|
||||
) for registration in self
|
||||
]
|
||||
return ("%s<br/>" % prefix if prefix else "") + (
|
||||
"<ol>" if line_counter else "<ul>") + ("".join(reg_lines)) + ("</ol>" if line_counter else "</ul>")
|
||||
description = (prefix if prefix else '') + Markup("<br/>")
|
||||
if line_counter:
|
||||
description += Markup("<ol>") + Markup('').join(reg_lines) + Markup("</ol>")
|
||||
else:
|
||||
description += Markup("<ul>") + Markup('').join(reg_lines) + Markup("</ul>")
|
||||
return description
|
||||
|
||||
def _get_lead_description_registration(self, line_suffix=''):
|
||||
""" Build the description line specific to a given registration. """
|
||||
self.ensure_one()
|
||||
return "<li>%s (%s)%s</li>" % (
|
||||
return Markup("<li>") + "%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("</li>")
|
||||
|
||||
def _get_lead_tracked_values(self):
|
||||
""" Tracked values are based on two subset of fields to track in order
|
||||
@@ -340,8 +351,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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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_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" <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 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" <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_eventregistrationdesk')
|
||||
def test_event_crm_trigger_done(self):
|
||||
"""Test the case when the "crm.lead.rule" is executed when we write on the
|
||||
|
||||
Reference in New Issue
Block a user