From 6e7b50708f0c400ecbde861554636f29248be2db Mon Sep 17 00:00:00 2001 From: std-odoo Date: Wed, 15 Sep 2021 11:36:26 +0000 Subject: [PATCH 1/3] [IMP] crm: remove the groups attributes on the recurring revenue fields Purpose ======= This commit remove the "groups" attribute on the recurring revenue fields. Instead we hide them directly in the views. This group was used only to turn on / off the feature, not for security reason. So it's fine to move the group in the views. It will help for the lead merge so we will not be forced to use SUDO during the merge. Task-2447721 PR odoo/odoo#75742 --- addons/crm/models/crm_lead.py | 10 ++++------ addons/crm/views/crm_lead_views.xml | 24 +++++++++++++----------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index f0c34f52554..13b17bc9bdc 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -146,14 +146,12 @@ class Lead(models.Model): # Revenues expected_revenue = fields.Monetary('Expected Revenue', currency_field='company_currency', tracking=True) prorated_revenue = fields.Monetary('Prorated Revenue', currency_field='company_currency', store=True, compute="_compute_prorated_revenue") - recurring_revenue = fields.Monetary('Recurring Revenues', currency_field='company_currency', groups="crm.group_use_recurring_revenues", tracking=True) - recurring_plan = fields.Many2one('crm.recurring.plan', string="Recurring Plan", groups="crm.group_use_recurring_revenues") + recurring_revenue = fields.Monetary('Recurring Revenues', currency_field='company_currency', tracking=True) + recurring_plan = fields.Many2one('crm.recurring.plan', string="Recurring Plan") recurring_revenue_monthly = fields.Monetary('Expected MRR', currency_field='company_currency', store=True, - compute="_compute_recurring_revenue_monthly", - groups="crm.group_use_recurring_revenues") + compute="_compute_recurring_revenue_monthly") recurring_revenue_monthly_prorated = fields.Monetary('Prorated MRR', currency_field='company_currency', store=True, - compute="_compute_recurring_revenue_monthly_prorated", - groups="crm.group_use_recurring_revenues") + compute="_compute_recurring_revenue_monthly_prorated") company_currency = fields.Many2one("res.currency", string='Currency', related='company_id.currency_id', readonly=True) # Dates date_closed = fields.Datetime('Closed Date', readonly=True, copy=False) diff --git a/addons/crm/views/crm_lead_views.xml b/addons/crm/views/crm_lead_views.xml index ca0237a9125..3ae7b9d5a47 100644 --- a/addons/crm/views/crm_lead_views.xml +++ b/addons/crm/views/crm_lead_views.xml @@ -462,7 +462,7 @@ -
+
@@ -559,11 +559,11 @@
- + + + - - + +
@@ -634,11 +634,11 @@
- + + + - - + +
@@ -772,9 +772,11 @@ - - - + + + @@ -896,7 +898,7 @@ - + From be996fdc00eacf394b59489136091f012e3b2da8 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Fri, 27 Aug 2021 14:15:52 +0000 Subject: [PATCH 2/3] [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 +
+ From ef04bec628b53349fa2a6a419f011b866f05f89b Mon Sep 17 00:00:00 2001 From: std-odoo Date: Thu, 23 Sep 2021 10:27:25 +0000 Subject: [PATCH 3/3] [IMP] crm: merge the followers of the leads if they are active Purpose ======= When merging multiple leads, we want to move the followers to the destination lead if they posted a message in the last 30 days. A message will be added in the merge note to know which followers have been added. Task-2447721 PR odoo/odoo#75742 --- addons/crm/data/crm_lead_merge_template.xml | 18 +++- addons/crm/models/crm_lead.py | 49 ++++++++++- addons/crm/tests/test_crm_lead_merge.py | 95 +++++++++++++++++++++ 3 files changed, 160 insertions(+), 2 deletions(-) diff --git a/addons/crm/data/crm_lead_merge_template.xml b/addons/crm/data/crm_lead_merge_template.xml index 3255e81123f..a46087183db 100644 --- a/addons/crm/data/crm_lead_merge_template.xml +++ b/addons/crm/data/crm_lead_merge_template.xml @@ -122,7 +122,7 @@
-
Marketing: @@ -137,6 +137,22 @@ Source:
+ +
+
+ The contacts below have been added as followers of this lead + because they have been contacted less than 30 days ago on + . +
+
    +
  • + + + () + +
    • +
+
diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index e58c95e5b6c..0e5ce514c78 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -15,7 +15,7 @@ from odoo.addons.phone_validation.tools import phone_validation from odoo.exceptions import UserError, AccessError from odoo.osv import expression from odoo.tools.translate import _ -from odoo.tools import date_utils, email_re, email_split, is_html_empty +from odoo.tools import date_utils, email_re, email_split, is_html_empty, groupby from . import crm_stage @@ -1296,10 +1296,13 @@ class Lead(models.Model): if team_id: merged_data['team_id'] = team_id + merged_followers = opportunities_head._merge_followers(opportunities_tail) + # log merge message opportunities_head.message_post_with_view( "crm.crm_lead_merge_summary", values={ + "merged_followers": merged_followers, "opportunities": opportunities_tail, "is_html_empty": is_html_empty }, @@ -1434,6 +1437,50 @@ class Lead(models.Model): 'opportunity_id': self.id, }) + def _merge_followers(self, opportunities): + """Add the followers into the destination lead if they post a message in the last 30 days. + + :param opportunities : Record of opportunities to transfer + :return: {old_lead_id: Record} Followers which have been added in + the destination lead grouped by source lead ID. + """ + self.ensure_one() + + self.env['mail.message'].flush() + self.env['mail.followers'].flush() + + # Get the active followers (followers whose partner post a message on the + # leads in the last 30 days) which should be moved on the destination lead + self.env.cr.execute( + ''' + SELECT MAX(mf.id) AS id + FROM mail_followers AS mf + JOIN mail_message AS mm + ON mm.author_id = mf.partner_id + AND mm.res_id = mf.res_id + AND mm.model = 'crm.lead' + AND mm.date > NOW() - INTERVAL '30 DAY' + /* Check if the partner is already + following the destination lead */ + LEFT JOIN mail_followers AS destf + ON destf.res_model = 'crm.lead' + AND destf.res_id = %(lead_id)s + AND destf.partner_id = mf.partner_id + /* Select only once each partner + to not create duplicated followers */ + WHERE mf.res_model = 'crm.lead' + AND mf.res_id IN %(lead_ids)s + AND destf IS NULL + GROUP BY mf.partner_id + ''', + {'lead_ids': tuple(opportunities.ids), 'lead_id': self.id}, + ) + followers_to_update = [r[0] for r in self.env.cr.fetchall()] + followers_to_update = self.env['mail.followers'].browse(followers_to_update).sudo() + followers_by_old_lead = dict(groupby(followers_to_update, lambda f: f.res_id)) + followers_to_update.write({'res_id': self.id}) + return followers_by_old_lead + # CONVERT # ---------------------------------------------------------------------- diff --git a/addons/crm/tests/test_crm_lead_merge.py b/addons/crm/tests/test_crm_lead_merge.py index f936904d174..bbf73b70155 100644 --- a/addons/crm/tests/test_crm_lead_merge.py +++ b/addons/crm/tests/test_crm_lead_merge.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import base64 +from datetime import timedelta from odoo.addons.crm.tests.common import TestLeadConvertMassCommon from odoo.fields import Datetime @@ -391,3 +392,97 @@ class TestLeadMerge(TestLeadMergeCommon): # 2many accessors updated self.assertEqual(master_lead.activity_ids, activity) self.assertEqual(master_lead.calendar_event_ids, calendar_event) + + @users('user_sales_manager') + @mute_logger('odoo.models.unlink') + def test_merge_method_followers(self): + """ Test that the followers of the leads are added in the destination lead. + + They should be added if: + - The related partner was active on the lead (posted a message in the last 30 days) + - The related partner is not already following the destination lead + + Leads Followers Info + --------------------------------------------------------------------------------- + lead_w_contact contact_1 OK (destination lead) + lead_w_email contact_1 KO (already following the destination lead) + contact_2 OK (active on lead_w_email) + contact_company KO (most recent message on lead_w_email is 35 days ago, message + on lead_w_partner is not counted as he doesn't follow it) + lead_w_partner contact_2 KO (already added with lead_w_email) + lead_w_partner_company + """ + self.leads.message_follower_ids.unlink() + self.leads.message_ids.unlink() + + self.lead_w_contact.message_subscribe([self.contact_1.id]) + self.lead_w_email.message_subscribe([self.contact_1.id, self.contact_2.id, self.contact_company.id]) + self.lead_w_partner.message_subscribe([self.contact_2.id]) + + self.env['mail.message'].create([{ + 'author_id': self.contact_1.id, + 'model': 'crm.lead', + 'res_id': self.lead_w_contact.id, + 'date': Datetime.now() - timedelta(days=1), + 'subtype_id': self.ref('mail.mt_comment'), + 'reply_to': False, + 'body': 'Test follower', + }, { + 'author_id': self.contact_1.id, + 'model': 'crm.lead', + 'res_id': self.lead_w_email.id, + 'date': Datetime.now() - timedelta(days=20), + 'subtype_id': self.ref('mail.mt_comment'), + 'reply_to': False, + 'body': 'Test follower', + }, { + 'author_id': self.contact_2.id, + 'model': 'crm.lead', + 'res_id': self.lead_w_email.id, + 'date': Datetime.now() - timedelta(days=15), + 'subtype_id': self.ref('mail.mt_comment'), + 'reply_to': False, + 'body': 'Test follower', + }, { + 'author_id': self.contact_2.id, + 'model': 'crm.lead', + 'res_id': self.lead_w_partner.id, + 'date': Datetime.now() - timedelta(days=29), + 'subtype_id': self.ref('mail.mt_comment'), + 'reply_to': False, + 'body': 'Test follower', + }, { + 'author_id': self.contact_company.id, + 'model': 'crm.lead', + 'res_id': self.lead_w_email.id, + 'date': Datetime.now() - timedelta(days=35), + 'subtype_id': self.ref('mail.mt_comment'), + 'reply_to': False, + 'body': 'Test follower', + }, { + 'author_id': self.contact_company.id, + 'model': 'crm.lead', + 'res_id': self.lead_w_partner.id, + 'date': Datetime.now(), + 'subtype_id': self.ref('mail.mt_comment'), + 'reply_to': False, + 'body': 'Test follower', + }]) + initial_followers = self.lead_w_contact.message_follower_ids + + leads = self.env['crm.lead'].browse(self.leads.ids)._sort_by_confidence_level(reverse=True) + master_lead = leads._merge_opportunity(max_length=None) + + self.assertEqual(master_lead, self.lead_w_contact) + + # Check followers of the destination lead + new_partner_followers = (master_lead.message_follower_ids - initial_followers).partner_id + self.assertIn(self.contact_2, new_partner_followers, + 'The partner must follow the destination lead') + # "contact_company" posted a message 35 days ago on lead_2, so it's considered as inactive + # "contact_company" posted a message now on lead_3, but he doesn't follow lead_3 + # so this message is just ignored + self.assertNotIn(self.contact_company, new_partner_followers, + 'The partner was not active on the lead') + self.assertIn(self.contact_1, master_lead.message_follower_ids.partner_id, + 'Should not have removed follower of the destination lead')