From be996fdc00eacf394b59489136091f012e3b2da8 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Fri, 27 Aug 2021 14:15:52 +0000 Subject: [PATCH] [IMP] crm_*: add more fields when we merge multiple leads Purpose ======= Add more fields when we merge multiple leads to be sure not lose valuable information. Specifications ============== Those fields are propagated to the destination if the value on the destination is Falsy - referred - color - recurring_revenue - recurring_plan - function - lang_id - date_deadline - reveal_id - lead_mining_request_id - reveal_ip - reveal_iap_credits - reveal_rule_id - event_lead_rule_id - event_id The "lost_reason" field is propagated to the destination only if it's lost (otherwise, it makes no sense to have a lost reason on a non-lost lead). The field "iap_enrich_done" is set to True if at least one lead has been enriched. We also keep the sum of all the lead tags (and remove any potential duplicates). The address is taken from the lead with the most non-empty address fields (sorted by highest rank if multiple lead have the same amount of non-empty fields). Task-2447721 PR odoo/odoo#75742 --- addons/crm/data/crm_lead_merge_template.xml | 25 ++++- addons/crm/models/crm_lead.py | 40 ++++++-- addons/crm/static/src/scss/crm.scss | 32 +++++- addons/crm/tests/common.py | 14 ++- addons/crm/tests/test_crm_lead_merge.py | 99 ++++++++++++++++++- addons/crm_iap_enrich/models/crm_lead.py | 6 ++ addons/crm_iap_enrich/tests/__init__.py | 1 + .../tests/test_crm_lead_merge.py | 26 +++++ addons/crm_iap_mine/models/crm_lead.py | 3 + addons/event_crm/__manifest__.py | 1 + .../data/crm_lead_merge_template.xml | 20 ++++ addons/event_crm/models/crm_lead.py | 3 + addons/iap_crm/models/crm_lead.py | 3 + addons/sale_crm/__manifest__.py | 1 + .../sale_crm/data/crm_lead_merge_template.xml | 18 ++++ .../website_crm_iap_reveal/models/crm_lead.py | 3 + .../data/crm_lead_merge_template.xml | 11 +++ 17 files changed, 293 insertions(+), 13 deletions(-) create mode 100644 addons/crm_iap_enrich/tests/test_crm_lead_merge.py create mode 100644 addons/event_crm/data/crm_lead_merge_template.xml create mode 100644 addons/sale_crm/data/crm_lead_merge_template.xml diff --git a/addons/crm/data/crm_lead_merge_template.xml b/addons/crm/data/crm_lead_merge_template.xml index c081ada9ae9..3255e81123f 100644 --- a/addons/crm/data/crm_lead_merge_template.xml +++ b/addons/crm/data/crm_lead_merge_template.xml @@ -40,15 +40,34 @@
Priority:
+
+ Lost Reason: +
Created on:
Last Action:
+
+ Expected Closing: +
Notes:
+
+ Language: +
+
+ Referred By: +
+
+ Tags: +
+
+
+
Salesperson:
@@ -85,10 +104,14 @@
Website:
+
+ Job Position: +
+ t-if="lead.street or lead.street2 or lead.zip or lead.city or lead.state_id or lead.country_id" + name="address">
Address:
diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index 13b17bc9bdc..e58c95e5b6c 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -33,31 +33,31 @@ CRM_LEAD_FIELDS_TO_MERGE = [ # description 'name', 'user_id', + 'color', 'company_id', + 'lang_id', 'team_id', + 'referred', # pipeline 'stage_id', # revenues 'expected_revenue', + 'recurring_plan', + 'recurring_revenue', # dates 'create_date', 'date_action_last', + 'date_deadline', # partner / contact 'partner_id', 'title', 'partner_name', 'contact_name', 'email_from', + 'function', 'mobile', 'phone', 'website', - # address - 'street', - 'street2', - 'zip', - 'city', - 'state_id', - 'country_id', ] # Subset of partner fields: sync any of those @@ -1224,6 +1224,7 @@ class Lead(models.Model): if fnames is None: fnames = self._merge_get_fields() fcallables = self._merge_get_fields_specific() + address_values = self._merge_get_fields_address() # helpers def _get_first_not_null(attr, opportunities): @@ -1244,6 +1245,8 @@ class Lead(models.Model): fcallable = fcallables.get(field_name) if fcallable and callable(fcallable): data[field_name] = fcallable(field_name, self) + elif field_name in address_values: + data[field_name] = address_values[field_name] elif not fcallable and field.type in ('many2many', 'one2many'): continue else: @@ -1321,15 +1324,36 @@ class Lead(models.Model): return opportunities_head + def _merge_get_fields_address(self): + """The address fields are propagated as a whole. + + The address is taken from the lead with the most non-empty address field + (sorted by highest rank if multiple lead have the same amount of non-empty + fields). + """ + source_lead = max(self, key=lambda lead: len(list( + lead[field] for field in PARTNER_ADDRESS_FIELDS_TO_SYNC + if lead[field] + ))) + return {fname: source_lead[fname] for fname in PARTNER_ADDRESS_FIELDS_TO_SYNC} + def _merge_get_fields_specific(self): return { 'description': lambda fname, leads: '

'.join(desc for desc in leads.mapped('description') if not is_html_empty(desc)), 'type': lambda fname, leads: 'opportunity' if any(lead.type == 'opportunity' for lead in leads) else 'lead', 'priority': lambda fname, leads: max(leads.mapped('priority')) if leads else False, + 'tag_ids': lambda fname, leads: leads.mapped('tag_ids'), + 'lost_reason': lambda fname, leads: + False if leads and leads[0].probability + else next((lead.lost_reason for lead in leads if lead.lost_reason), False), } def _merge_get_fields(self): - return list(CRM_LEAD_FIELDS_TO_MERGE) + list(self._merge_get_fields_specific().keys()) + return ( + CRM_LEAD_FIELDS_TO_MERGE + + list(self._merge_get_fields_specific().keys()) + + PARTNER_ADDRESS_FIELDS_TO_SYNC + ) def _merge_dependences(self, opportunities): """ Merge dependences (messages, attachments,activities, calendar events, diff --git a/addons/crm/static/src/scss/crm.scss b/addons/crm/static/src/scss/crm.scss index c33a456d679..e7bdc20d092 100644 --- a/addons/crm/static/src/scss/crm.scss +++ b/addons/crm/static/src/scss/crm.scss @@ -36,4 +36,34 @@ blockquote { font-style: normal; } -} \ No newline at end of file + + // Tag colors + @for $size from 1 through length($o-colors) { + .o_tag_color_#{$size - 1} { + border: transparent; + line-height: normal; + $background-color: white; + // no color selected + @if $size == 1 { + & { + color: black; + background-color: $background-color; + box-shadow: inset 0 0 0 1px nth($o-colors, $size); + } + } @else { + $background-color: nth($o-colors, $size); + & { + color: white; + background-color: $background-color; + } + } + @at-root a#{&} { + &:hover { + color: color-yiq($background-color); + background-color: darken($background-color, 10%); + } + } + } + } +} + diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py index 9527089281d..c1a1088d0df 100644 --- a/addons/crm/tests/common.py +++ b/addons/crm/tests/common.py @@ -5,7 +5,7 @@ from ast import literal_eval from contextlib import contextmanager from unittest.mock import patch -from odoo.addons.crm.models.crm_lead import CRM_LEAD_FIELDS_TO_MERGE +from odoo.addons.crm.models.crm_lead import PARTNER_ADDRESS_FIELDS_TO_SYNC from odoo.addons.mail.tests.common import MailCase, mail_new_test_user from odoo.addons.phone_validation.tools import phone_validation from odoo.addons.sales_team.tests.common import TestSalesCommon @@ -423,7 +423,10 @@ class TestCrmCommon(TestSalesCommon, MailCase): finally: # support specific values caller may want to check in addition to generic tests for fname, expected in expected.items(): - self.assertEqual(opportunity[fname], expected) + if expected is False: + self.assertFalse(opportunity[fname], "%s must be False" % fname) + else: + self.assertEqual(opportunity[fname], expected, "%s must be equal to %s" % (fname, expected)) # classic fields: first not void wins or specific computation for fname in fields_all: @@ -438,6 +441,9 @@ class TestCrmCommon(TestSalesCommon, MailCase): self.assertEqual(opp_value, _get_priority()) elif fname in ('order_ids', 'visitor_ids'): self.assertEqual(opp_value, _aggregate(fname)) + elif fname in PARTNER_ADDRESS_FIELDS_TO_SYNC: + # Specific computation, has its own test + continue else: self.assertEqual( opp_value if opp_value or not isinstance(opp_value, models.BaseModel) else False, @@ -607,6 +613,9 @@ class TestLeadConvertMassCommon(TestLeadConvertCommon): 'partner_id': cls.contact_1.id, }) cls.lead_w_partner.write({'stage_id': False}) + + cls.tags = cls.env['crm.tag'].create([{'name': 'Tag %i' % i} for i in range(4)]) + cls.lead_1.tag_ids = cls.tags[:3] cls.lead_w_partner_company = cls.env['crm.lead'].create({ 'name': 'New1', 'type': 'lead', @@ -616,6 +625,7 @@ class TestLeadConvertMassCommon(TestLeadConvertCommon): 'partner_id': cls.contact_company_1.id, 'contact_name': 'Hermes Conrad', 'email_from': 'hermes.conrad@test.example.com', + 'tag_ids': (cls.tags[:2] | cls.tags[3]), }) cls.lead_w_contact = cls.env['crm.lead'].create({ 'name': 'LeadContact', diff --git a/addons/crm/tests/test_crm_lead_merge.py b/addons/crm/tests/test_crm_lead_merge.py index 8f580585058..f936904d174 100644 --- a/addons/crm/tests/test_crm_lead_merge.py +++ b/addons/crm/tests/test_crm_lead_merge.py @@ -84,6 +84,73 @@ class TestLeadMerge(TestLeadMergeCommon): self.assertEqual(self.lead_w_email_lost.user_id, self.env['res.users']) self.assertEqual(self.lead_w_email_lost.team_id, self.sales_team_1) + @users('user_sales_manager') + @mute_logger('odoo.models.unlink') + def test_lead_merge_address_not_propagated(self): + """All addresses have the same number of non-empty address fields, take the first one (lead_w_contact) + because it's the lead that has the best confidence level after being sorted with '_sort_by_confidence_level'""" + initial_address = { + 'street': 'Test street', + 'street2': 'Test street2', + 'city': 'Test City', + 'zip': '5000', + 'state_id': False, + 'country_id': self.env.ref('base.be'), + } + self.lead_w_contact.write(initial_address) + + (self.leads - self.lead_w_contact).write({ + 'street': 'Other street', + 'street2': 'Other street2', + 'city': 'Other City', + 'zip': '6666', + 'state_id': self.env.ref('base.state_us_1'), + 'country_id': False, + }) + + leads = self.env['crm.lead'].browse(self.leads.ids)._sort_by_confidence_level(reverse=True) + with self.assertLeadMerged(self.lead_w_contact, leads, **initial_address): + leads._merge_opportunity(auto_unlink=False, max_length=None) + + @users('user_sales_manager') + @mute_logger('odoo.models.unlink') + def test_lead_merge_address_propagated(self): + """Test that the address with the most non-empty fields is propagated. + + Should take the address of "lead_w_partner" (maximum number of non-empty address + fields and with an highest rank than "lead_w_email_lost") + """ + self.leads.write({ + 'street': 'Original street', + 'street2': False, + 'city': False, + 'zip': False, + 'state_id': False, + 'country_id': False, + }) + new_address = { + 'street': 'New street', + 'street2': False, + 'city': 'New City', + 'zip': False, + 'state_id': False, + 'country_id': False, + } + self.lead_w_partner.write(new_address) + self.lead_w_email_lost.write({ + 'street': 'Other street', + 'street2': False, + 'city': 'Other City', + 'zip': False, + 'state_id': False, + 'country_id': False, + }) + + leads = self.env['crm.lead'].browse(self.leads.ids)._sort_by_confidence_level(reverse=True) + + with self.assertLeadMerged(self.lead_w_contact, leads, **new_address): + leads._merge_opportunity(auto_unlink=False, max_length=None) + @users('user_sales_manager') @mute_logger('odoo.models.unlink') def test_lead_merge_internals(self): @@ -234,10 +301,40 @@ class TestLeadMerge(TestLeadMergeCommon): # ensure initial data (self.lead_w_partner_company | self.lead_1).write({'type': 'opportunity', 'probability': 50}) leads = self.env['crm.lead'].browse(self.leads.ids)._sort_by_confidence_level(reverse=True) + + # lead_w_partner is lost, check that the "lost_reason" is not propagated + # because "lead_1" is not lost + lost_reason = self.env['crm.lost.reason'].create({'name': 'Test Reason'}) + self.lead_w_partner.write({ + 'lost_reason': lost_reason, + 'probability': 0, + }) + + all_tags = self.leads.mapped('tag_ids') + with self.assertLeadMerged(self.lead_1, leads, name='Nibbler Spacecraft Request', partner_id=self.contact_company_1, - priority='2'): + priority='2', + lost_reason=False, + tag_ids=all_tags): + leads._merge_opportunity(auto_unlink=False, max_length=None) + + @users('user_sales_manager') + @mute_logger('odoo.models.unlink') + def test_merge_method_propagate_lost_reason(self): + """Check that the lost reason is propagated to the final lead if it's lost.""" + self.leads.write({ + 'probability': 0, + 'automated_probability': 50, # Do not automatically update the probability + }) + + lost_reason = self.env['crm.lost.reason'].create({'name': 'Test Reason'}) + self.lead_w_partner.lost_reason = lost_reason + + leads = self.env['crm.lead'].browse(self.leads.ids)._sort_by_confidence_level(reverse=True) + + with self.assertLeadMerged(leads[0], leads, lost_reason=lost_reason): leads._merge_opportunity(auto_unlink=False, max_length=None) @users('user_sales_manager') diff --git a/addons/crm_iap_enrich/models/crm_lead.py b/addons/crm_iap_enrich/models/crm_lead.py index 869d2704bd4..215e7090577 100644 --- a/addons/crm_iap_enrich/models/crm_lead.py +++ b/addons/crm_iap_enrich/models/crm_lead.py @@ -159,3 +159,9 @@ class Lead(models.Model): values=template_values, subtype_id=self.env.ref('mail.mt_note').id ) + + def _merge_get_fields_specific(self): + return { + ** super(Lead, self)._merge_get_fields_specific(), + 'iap_enrich_done': lambda fname, leads: any(lead.iap_enrich_done for lead in leads), + } diff --git a/addons/crm_iap_enrich/tests/__init__.py b/addons/crm_iap_enrich/tests/__init__.py index 33ab21b0da9..76341f51790 100644 --- a/addons/crm_iap_enrich/tests/__init__.py +++ b/addons/crm_iap_enrich/tests/__init__.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from . import test_crm_lead_merge from . import test_lead_enrich diff --git a/addons/crm_iap_enrich/tests/test_crm_lead_merge.py b/addons/crm_iap_enrich/tests/test_crm_lead_merge.py new file mode 100644 index 00000000000..054a51b38e7 --- /dev/null +++ b/addons/crm_iap_enrich/tests/test_crm_lead_merge.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + + +from odoo.addons.crm.tests.test_crm_lead_merge import TestLeadMergeCommon +from odoo.tests.common import tagged, users +from odoo.tools import mute_logger + + +@tagged('lead_manage') +class TestLeadMerge(TestLeadMergeCommon): + + @users('user_sales_manager') + @mute_logger('odoo.models.unlink') + def test_merge_method_iap_enrich_done(self): + """Test that the "iap_enrich_done" is set to True if at least one lead have this value True""" + self.leads.iap_enrich_done = False + self.lead_w_contact.write({ + 'reveal_id': 'test_reveal_id', + 'iap_enrich_done': True, + }) + + leads = self.env['crm.lead'].browse(self.leads.ids)._sort_by_confidence_level(reverse=True) + + with self.assertLeadMerged(leads[0], leads, iap_enrich_done=True, reveal_id='test_reveal_id'): + leads._merge_opportunity(auto_unlink=False, max_length=None) diff --git a/addons/crm_iap_mine/models/crm_lead.py b/addons/crm_iap_mine/models/crm_lead.py index eaed1d92bd7..35107394157 100644 --- a/addons/crm_iap_mine/models/crm_lead.py +++ b/addons/crm_iap_mine/models/crm_lead.py @@ -8,3 +8,6 @@ class Lead(models.Model): _inherit = 'crm.lead' lead_mining_request_id = fields.Many2one('crm.iap.lead.mining.request', string='Lead Mining Request', index=True) + + def _merge_get_fields(self): + return super(Lead, self)._merge_get_fields() + ['lead_mining_request_id'] diff --git a/addons/event_crm/__manifest__.py b/addons/event_crm/__manifest__.py index a9a5aeb3dbe..dba6b325056 100644 --- a/addons/event_crm/__manifest__.py +++ b/addons/event_crm/__manifest__.py @@ -11,6 +11,7 @@ 'data': [ 'security/event_crm_security.xml', 'security/ir.model.access.csv', + 'data/crm_lead_merge_template.xml', 'views/crm_lead_views.xml', 'views/event_registration_views.xml', 'views/event_lead_rule_views.xml', diff --git a/addons/event_crm/data/crm_lead_merge_template.xml b/addons/event_crm/data/crm_lead_merge_template.xml new file mode 100644 index 00000000000..3dfecd8d128 --- /dev/null +++ b/addons/event_crm/data/crm_lead_merge_template.xml @@ -0,0 +1,20 @@ + + + + + + diff --git a/addons/event_crm/models/crm_lead.py b/addons/event_crm/models/crm_lead.py index de709404e95..07a67e90f4d 100644 --- a/addons/event_crm/models/crm_lead.py +++ b/addons/event_crm/models/crm_lead.py @@ -30,3 +30,6 @@ class Lead(models.Model): self.sudo().write({ 'registration_ids': [(4, registration.id) for registration in opportunities.sudo().registration_ids] }) + + def _merge_get_fields(self): + return super(Lead, self)._merge_get_fields() + ['event_lead_rule_id', 'event_id'] diff --git a/addons/iap_crm/models/crm_lead.py b/addons/iap_crm/models/crm_lead.py index c45d9da847d..43d3da90902 100644 --- a/addons/iap_crm/models/crm_lead.py +++ b/addons/iap_crm/models/crm_lead.py @@ -8,3 +8,6 @@ class Lead(models.Model): _inherit = 'crm.lead' reveal_id = fields.Char(string='Reveal ID', help="Technical ID of reveal request done by IAP.") + + def _merge_get_fields(self): + return super(Lead, self)._merge_get_fields() + ['reveal_id'] diff --git a/addons/sale_crm/__manifest__.py b/addons/sale_crm/__manifest__.py index c8c5d551283..3fc3bbc7d7e 100644 --- a/addons/sale_crm/__manifest__.py +++ b/addons/sale_crm/__manifest__.py @@ -19,6 +19,7 @@ modules. 'depends': ['sale_management', 'crm'], 'data': [ 'security/ir.model.access.csv', + 'data/crm_lead_merge_template.xml', 'views/partner_views.xml', 'views/sale_order_views.xml', 'views/crm_lead_views.xml', diff --git a/addons/sale_crm/data/crm_lead_merge_template.xml b/addons/sale_crm/data/crm_lead_merge_template.xml new file mode 100644 index 00000000000..dc21ebb7b4c --- /dev/null +++ b/addons/sale_crm/data/crm_lead_merge_template.xml @@ -0,0 +1,18 @@ + + + + + + diff --git a/addons/website_crm_iap_reveal/models/crm_lead.py b/addons/website_crm_iap_reveal/models/crm_lead.py index 97cf26914e3..90b8ca89d69 100644 --- a/addons/website_crm_iap_reveal/models/crm_lead.py +++ b/addons/website_crm_iap_reveal/models/crm_lead.py @@ -10,3 +10,6 @@ class Lead(models.Model): reveal_ip = fields.Char(string='IP Address') reveal_iap_credits = fields.Integer(string='IAP Credits') reveal_rule_id = fields.Many2one('crm.reveal.rule', string='Lead Generation Rule', index=True) + + def _merge_get_fields(self): + return super(Lead, self)._merge_get_fields() + ['reveal_ip', 'reveal_iap_credits', 'reveal_rule_id'] diff --git a/addons/website_crm_partner_assign/data/crm_lead_merge_template.xml b/addons/website_crm_partner_assign/data/crm_lead_merge_template.xml index 5381daccace..f624710faa9 100644 --- a/addons/website_crm_partner_assign/data/crm_lead_merge_template.xml +++ b/addons/website_crm_partner_assign/data/crm_lead_merge_template.xml @@ -11,6 +11,17 @@ Partner Assignment Date:
+ + + lead.street or lead.street2 or lead.zip or lead.city or lead.state_id or lead.country_id + or lead.partner_latitude or lead.partner_longitude + + + +
+ Geolocation: latitude, longitude +
+