From 89ca89ca9269aea9ce2850db28ef6a7477e1ef18 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Fri, 17 Apr 2020 08:18:40 +0000 Subject: [PATCH] [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 --- addons/crm/__manifest__.py | 2 +- addons/crm/models/crm_lead.py | 52 +++++++++++++++---- addons/crm/tests/common.py | 3 +- addons/crm/tests/test_crm_lead.py | 30 +++++++++++ addons/crm/tests/test_crm_lead_convert.py | 10 ++-- addons/crm/views/crm_lead_views.xml | 27 +++------- addons/crm_iap_lead_enrich/models/crm_lead.py | 2 +- 7 files changed, 89 insertions(+), 37 deletions(-) diff --git a/addons/crm/__manifest__.py b/addons/crm/__manifest__.py index 839a1e70ef3..105717e16d0 100644 --- a/addons/crm/__manifest__.py +++ b/addons/crm/__manifest__.py @@ -3,7 +3,7 @@ { 'name': 'CRM', - 'version': '1.0', + 'version': '1.1', 'category': 'Sales/CRM', 'sequence': 5, 'summary': 'Track leads and close opportunities', diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index cf08fe7ac31..1bdc0ba34c7 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -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, diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py index ce936bb2e3b..449a5a50ce8 100644 --- a/addons/crm/tests/common.py +++ b/addons/crm/tests/common.py @@ -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({ diff --git a/addons/crm/tests/test_crm_lead.py b/addons/crm/tests/test_crm_lead.py index 3ec5fde38f4..8473ed4a555 100644 --- a/addons/crm/tests/test_crm_lead.py +++ b/addons/crm/tests/test_crm_lead.py @@ -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" ' + lead.phone = '+1 202 555 7799' + self.assertEqual(partner.email, '"John Zoidberg" ') + 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) diff --git a/addons/crm/tests/test_crm_lead_convert.py b/addons/crm/tests/test_crm_lead_convert.py index 75801e4a39f..33115d34cdd 100644 --- a/addons/crm/tests/test_crm_lead_convert.py +++ b/addons/crm/tests/test_crm_lead_convert.py @@ -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) diff --git a/addons/crm/views/crm_lead_views.xml b/addons/crm/views/crm_lead_views.xml index 9e5d4ae732c..94e0c49f339 100644 --- a/addons/crm/views/crm_lead_views.xml +++ b/addons/crm/views/crm_lead_views.xml @@ -22,6 +22,9 @@ domain="['|', ('team_id', '=', team_id), ('team_id', '=', False)]" attrs="{'invisible': ['|', ('active', '=', False), ('type', '=', 'lead')]}"/> +
@@ -139,15 +142,8 @@