[IMP] crm: allow editing phone and email of the lead

PURPOSE

Get rid of email / phone behavior on lead that is hard to understand: if
and contact is set fields are readonly, if not they are editable.

SPECIFICATIONS

We want to be able to edit them if a partner is set. If we write on the
email/phone of the lead, it should write on the partner (and vice-versa).

Even a Falsy value is propagated to the customer. Reason is that if you update
a contact information, it should be available for all other records. Setting
a void value probably means you want to stop contacting this contact. This is
now also propagated.

SIDE NOTE: MOBILE FIELD

Mobile field is a bit different. Main phone contact field is phone, and is
placed in the main contact section of the lead view. Mobile is under a more
technical / detailled tab and is not completely synchronized with the partner.
Like zip, city or country, changing partner updates lead information but
changing lead values does not propagate those to the customer. Those fields
can be used for deal-specific information if needed.

Statistics: 5k crm.lead use mobile for 80k active records which means it is
a "lesser field".

LINKS

Task ID 2207636
PR odoo/odoo#48395
Upgrade PR odoo/upgrade#1025
This commit is contained in:
std-odoo
2020-05-18 15:03:29 +00:00
committed by Thibault Delavallée
parent df92415f21
commit 89ca89ca92
7 changed files with 89 additions and 37 deletions
+1 -1
View File
@@ -3,7 +3,7 @@
{
'name': 'CRM',
'version': '1.0',
'version': '1.1',
'category': 'Sales/CRM',
'sequence': 5,
'summary': 'Track leads and close opportunities',
+43 -9
View File
@@ -3,9 +3,8 @@
import logging
import threading
from datetime import date, datetime
from psycopg2 import sql
from datetime import datetime, timedelta, date
from dateutil.relativedelta import relativedelta
from odoo import api, fields, models, tools, SUPERUSER_ID
from odoo.tools.translate import _
@@ -120,7 +119,6 @@ class Lead(models.Model):
'res.partner', string='Customer', index=True, tracking=10,
domain="['|', ('company_id', '=', False), ('company_id', '=', company_id)]",
help="Linked partner (optional). Usually created when converting the lead. You can find a partner by its Name, TIN, Email or Internal Reference.")
partner_address_email = fields.Char('Partner Contact Email', related='partner_id.email', readonly=True)
partner_is_blacklisted = fields.Boolean('Partner is blacklisted', related='partner_id.is_blacklisted', readonly=True)
contact_name = fields.Char(
'Contact Name', tracking=30,
@@ -133,10 +131,10 @@ class Lead(models.Model):
title = fields.Many2one('res.partner.title', string='Title',compute='_compute_partner_id_values', readonly=False, store=True)
email_from = fields.Char(
'Email', tracking=40, index=True,
compute='_compute_partner_id_values', readonly=False, store=True)
compute='_compute_email_from', inverse='_inverse_email_from', readonly=False, store=True)
phone = fields.Char(
'Phone', tracking=50,
compute='_compute_partner_id_values', readonly=False, store=True)
compute='_compute_phone', inverse='_inverse_phone', readonly=False, store=True)
mobile = fields.Char('Mobile', compute='_compute_partner_id_values', readonly=False, store=True)
phone_mobile_search = fields.Char('Phone/Mobile', store=False, search='_search_phone_mobile_search')
phone_state = fields.Selection([
@@ -170,6 +168,7 @@ class Lead(models.Model):
lost_reason = fields.Many2one(
'crm.lost.reason', string='Lost Reason',
index=True, ondelete='restrict', tracking=True)
ribbon_message = fields.Char('Ribbon message', compute='_compute_ribbon_message')
_sql_constraints = [
('check_probability', 'check(probability >= 0 and probability <= 100)', 'The probability of closing the deal should be between 0% and 100%!')
@@ -244,7 +243,29 @@ class Lead(models.Model):
def _compute_partner_id_values(self):
""" compute the new values when partner_id has changed """
for lead in self:
lead.update(lead._preare_values_from_partner(lead.partner_id))
lead.update(lead._prepare_values_from_partner(lead.partner_id))
@api.depends('partner_id.email')
def _compute_email_from(self):
for lead in self:
if lead.partner_id and lead.partner_id.email != lead.email_from:
lead.email_from = lead.partner_id.email
def _inverse_email_from(self):
for lead in self:
if lead.partner_id and lead.email_from != lead.partner_id.email:
lead.partner_id.email = lead.email_from
@api.depends('partner_id.phone')
def _compute_phone(self):
for lead in self:
if lead.partner_id and lead.phone != lead.partner_id.phone:
lead.phone = lead.partner_id.phone
def _inverse_phone(self):
for lead in self:
if lead.partner_id and lead.phone != lead.partner_id.phone:
lead.partner_id.phone = lead.phone
@api.depends('phone', 'country_id.code')
def _compute_phone_state(self):
@@ -309,6 +330,21 @@ class Lead(models.Model):
for lead in self:
lead.meeting_count = mapped_data.get(lead.id, 0)
@api.depends('email_from', 'phone', 'partner_id')
def _compute_ribbon_message(self):
for lead in self:
will_write_email = lead.partner_id and lead.email_from != lead.partner_id.email
will_write_phone = lead.partner_id and lead.phone != lead.partner_id.phone
if will_write_email and will_write_phone:
lead.ribbon_message = _('By saving this change, the customer email and phone number will also be updated.')
elif will_write_email:
lead.ribbon_message = _('By saving this change, the customer email will also be updated.')
elif will_write_phone:
lead.ribbon_message = _('By saving this change, the customer phone number will also be updated.')
else:
lead.ribbon_message = False
def _search_phone_mobile_search(self, operator, value):
if len(value) <= 2:
raise UserError(_('Please enter at least 3 digits when searching on phone / mobile.'))
@@ -344,7 +380,7 @@ class Lead(models.Model):
if self.mobile:
self.mobile = self.phone_format(self.mobile)
def _preare_values_from_partner(self, partner):
def _prepare_values_from_partner(self, partner):
""" Get a dictionary with values coming from customer information to
copy on a lead. Email_from and phone fields get the current lead
values to avoid being reset if customer has no value for them. """
@@ -360,8 +396,6 @@ class Lead(models.Model):
'city': partner.city,
'state_id': partner.state_id.id,
'country_id': partner.country_id.id,
'email_from': partner.email or self.email_from,
'phone': partner.phone or self.phone,
'mobile': partner.mobile,
'zip': partner.zip,
'function': partner.function,
+2 -1
View File
@@ -93,6 +93,7 @@ class TestCrmCommon(TestSalesCommon, MailCase):
'partner_id': False,
'contact_name': 'Amy Wong',
'email_from': 'amy.wong@test.example.com',
'country_id': cls.env.ref('base.us').id,
})
# update lead_1: stage_id is not computed anymore by default for leads
cls.lead_1.write({
@@ -145,6 +146,7 @@ class TestCrmCommon(TestSalesCommon, MailCase):
'street': 'Cookieville Minimum-Security Orphanarium',
'city': 'New New York',
'country_id': cls.env.ref('base.us').id,
'mobile': '+1 202 555 0999',
'zip': '97648',
})
@@ -180,7 +182,6 @@ class TestCrmCommon(TestSalesCommon, MailCase):
'type': 'lead',
'team_id': lead.team_id.id,
'partner_id': self.customer.id,
'email_from': 'another.email@test.example.com',
})
if create_opp:
self.opp_lost = self.env['crm.lead'].create({
+30
View File
@@ -31,6 +31,36 @@ class TestCRMLead(TestCrmCommon):
# self.assertEqual(lead.zip, self.contact_1.zip)
# self.assertEqual(lead.country_id, self.contact_1.country_id)
@users('user_sales_manager')
def test_crm_lead_partner_sync(self):
lead, partner = self.lead_1.with_user(self.env.user), self.contact_2
partner_email, partner_phone = self.contact_2.email, self.contact_2.phone
lead.partner_id = partner
# email & phone must be automatically set on the lead
lead.partner_id = partner
self.assertEqual(lead.email_from, partner_email)
self.assertEqual(lead.phone, partner_phone)
# writing on the lead field must change the partner field
lead.email_from = '"John Zoidberg" <john.zoidberg@test.example.com>'
lead.phone = '+1 202 555 7799'
self.assertEqual(partner.email, '"John Zoidberg" <john.zoidberg@test.example.com>')
self.assertEqual(partner.email_normalized, 'john.zoidberg@test.example.com')
self.assertEqual(partner.phone, '+1 202 555 7799')
# writing on the partner must change the lead values
partner.email = partner_email
partner.phone = '+1 202 555 6666'
self.assertEqual(lead.email_from, partner_email)
self.assertEqual(lead.phone, '+1 202 555 6666')
# resetting lead values also resets partner
lead.email_from, lead.phone = False, False
self.assertFalse(partner.email)
self.assertFalse(partner.email_normalized)
self.assertFalse(partner.phone)
@users('user_sales_manager')
def test_crm_lead_stages(self):
lead = self.lead_1.with_user(self.env.user)
+4 -6
View File
@@ -43,7 +43,7 @@ class TestLeadConvert(crm_common.TestLeadConvertCommon):
self.assertEqual(lead.partner_id, self.contact_2)
self.assertEqual(lead.email_from, self.contact_2.email)
self.assertEqual(lead.mobile, self.contact_2.mobile)
self.assertEqual(lead.phone, '123456789')
self.assertEqual(lead.phone, self.contact_2.phone)
self.assertEqual(lead.team_id, self.sales_team_1)
self.assertEqual(lead.stage_id, self.stage_team1_1)
@@ -250,7 +250,6 @@ class TestLeadConvert(crm_common.TestLeadConvertCommon):
self.lead_1.write({
'partner_id': self.customer.id,
})
self.customer.write({'email': False})
convert = self.env['crm.lead2opportunity.partner'].with_context({
'active_model': 'crm.lead',
'active_id': self.lead_1.id,
@@ -457,11 +456,10 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon):
duplicates if deduplicate is set to True. """
lead_1_dups = self._create_duplicates(self.lead_1, create_opp=False)
lead_1_final = self.lead_1 # after merge: same but with lower ID
lead_1_dups_partner = lead_1_dups[1] # copy with a partner_id set but another email -> not correctly taken into account
lead_w_partner_dups = self._create_duplicates(self.lead_w_partner, create_opp=False)
lead_w_partner_final = lead_w_partner_dups[0] # lead_w_partner has no stage -> lower in sort by confidence
lead_w_partner_dups_partner = lead_w_partner_dups[1] # copy with a partner_id set but another email -> not correctly taken into account
lead_w_partner_dups_partner = lead_w_partner_dups[1] # copy with a partner_id (with the same email)
mass_convert = self.env['crm.lead2opportunity.partner.mass'].with_context({
'active_model': 'crm.lead',
@@ -477,8 +475,8 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon):
mass_convert.action_mass_convert()
self.assertEqual(
(lead_1_dups | lead_w_partner_dups).exists(),
lead_1_dups_partner | lead_w_partner_final | lead_w_partner_dups_partner
(lead_1_dups | lead_w_partner_dups | lead_w_partner_dups_partner).exists(),
lead_w_partner_final
)
for lead in lead_1_final | lead_w_partner_final:
self.assertTrue(lead.active)
+8 -19
View File
@@ -22,6 +22,9 @@
domain="['|', ('team_id', '=', team_id), ('team_id', '=', False)]"
attrs="{'invisible': ['|', ('active', '=', False), ('type', '=', 'lead')]}"/>
</header>
<div class="text-center alert alert-primary" role="alert" attrs="{'invisible': ['|', ('ribbon_message', '=', False), ('ribbon_message', '=', '')]}">
<field name="ribbon_message"/>
</div>
<sheet>
<field name="active" invisible="1"/>
<div class="oe_button_box" name="button_box">
@@ -139,15 +142,8 @@
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': email_from}" groups="base.group_user"
attrs="{'invisible': ['|', ('is_blacklisted', '=', False), ('partner_address_email', '!=', False)]}"/>
<field name="email_from"
attrs="{'invisible': [('partner_address_email', '!=', False)]}" string="Email" widget="email"/>
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': partner_address_email}" groups="base.group_user"
attrs="{'invisible': ['|', ('partner_is_blacklisted', '=', False), ('partner_address_email', '=', False)]}"/>
<field name="partner_address_email"
attrs="{'invisible': [('partner_address_email', '=', False)]}" widget="email" string="Email"/>
attrs="{'invisible': [('is_blacklisted', '=', False)]}"/>
<field name="email_from" string="Email" widget="email"/>
</div>
<label for="phone" class="oe_inline"/>
<div class="o_row o_row_readonly">
@@ -165,6 +161,7 @@
<field name="title" placeholder="Title" domain="[]" options='{"no_open": True}'/>
</div>
<field name="is_blacklisted" invisible="1"/>
<field name="phone_blacklisted" invisible="1"/>
<field name="email_state" invisible="1"/>
<field name="phone_state" invisible="1"/>
<label for="email_from" class="oe_inline"/>
@@ -172,15 +169,8 @@
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': email_from}" groups="base.group_user"
attrs="{'invisible': ['|', ('is_blacklisted', '=', False), ('partner_address_email', '!=', False)]}"/>
<field name="email_from"
attrs="{'invisible': [('partner_address_email', '!=', False)]}" string="Email" widget="email"/>
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': partner_address_email}" groups="base.group_user"
attrs="{'invisible': ['|', ('partner_is_blacklisted', '=', False), ('partner_address_email', '=', False)]}"/>
<field name="partner_address_email"
attrs="{'invisible': [('partner_address_email', '=', False)]}" widget="email" string="Email"/>
attrs="{'invisible': [('is_blacklisted', '=', False)]}"/>
<field name="email_from" string="Email" widget="email"/>
</div>
<field name="email_cc" groups="base.group_no_one"/>
<field name="function"/>
@@ -470,7 +460,6 @@
<field name="activity_date_deadline"/>
<field name="user_email"/>
<field name="user_id"/>
<field name="partner_address_email"/>
<field name="partner_id"/>
<field name="activity_summary"/>
<field name="active"/>
@@ -47,7 +47,7 @@ class Lead(models.Model):
# If lead is lost, active == False, but is anyway removed from the search in the cron.
if lead.probability == 100 or lead.iap_enrich_done:
continue
normalized_email = tools.email_normalize(lead.partner_address_email) or tools.email_normalize(lead.email_from)
normalized_email = tools.email_normalize(lead.email_from)
if normalized_email:
lead_emails[lead.id] = normalized_email.split('@')[1]
else: