Skip to content

Commit

Permalink
[FW][FIX] event_crm: fix lead/registration values propagation, improv…
Browse files Browse the repository at this point in the history 
…e 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#129027

Forward-port-of: odoo#128823
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
  • Loading branch information
@robodoo
robodoo committed Jul 19, 2023
2 parents dd5b18e + 7eee007 commit 62c4599
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 57 deletions.
62 changes: 47 additions & 15 deletions addons/event_crm/models/event_registration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
# 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


class EventRegistration(models.Model):
Expand Down Expand Up @@ -190,24 +192,50 @@ def _get_lead_contact_values(self):
: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)
# force email_from / phone only if not set on partner because those fields are now synchronized automatically
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 = {
Expand Down Expand Up @@ -235,17 +263,21 @@ def _get_lead_description(self, prefix='', line_counter=True, line_suffix=''):
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
Expand Down Expand Up @@ -326,8 +358,8 @@ def _find_first_notnull(self, field_name):

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
10 changes: 7 additions & 3 deletions addons/event_crm/tests/common.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- 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

Expand Down Expand Up @@ -99,8 +100,8 @@ def assertLeadConvertion(self, rule, registrations, partner=None, **expected):

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
Expand All @@ -111,7 +112,10 @@ def assertLeadConvertion(self, rule, registrations, partner=None, **expected):
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)

Expand Down
89 changes: 50 additions & 39 deletions addons/event_crm/tests/test_event_crm_flow.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ def setUpClass(cls):
]
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):
Expand Down Expand Up @@ -97,54 +100,62 @@ def test_event_crm_flow_batch_update(self):
self.assertNotIn('invalid@not.example.com', lead.description)

@users('user_eventregistrationdesk')
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_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,
'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_eventregistrationdesk')
def test_event_crm_trigger_done(self):
Expand Down

0 comments on commit 62c4599

Please sign in to comment.