From a99d882c022229bcda78e2ab4f658206eaf5ad40 Mon Sep 17 00:00:00 2001 From: Anjali Date: Thu, 20 May 2021 12:18:37 +0000 Subject: [PATCH] [IMP] crm,*_crm: better merge leads side documents Purpose of this commit is to ensure side documents are redirected to the master opportunity when merging leads. Those side documents include * communication history (mail.message); * attachments (ir.attachment); * visitors (website.visitor); However some documents are currently not specifically handled : * meetings (calendar.event); * activities (mail.activity); * sale orders (sale.order); * attendees (event.registration); In this commit we ensure all are attached to the final master opportunity. That way we prevent loosing access to those documents and ensure we keep the complete history of all merged leads. Also tests are added to ensure the merging of leads and its contents. A new field is added to have the o2m field between leads and calendar events. In order to clarify naming, ``meeting_count`` is renamed to ``calendar_event_count`` to match naming. Task Id-2457941 COM PR odoo/odoo#68884 UPG PR odoo/odoo#2494 Related: odoo/upgrade#2494 Signed-off-by: Thibault Delavallee (tde) --- addons/crm/__manifest__.py | 2 +- addons/crm/models/crm_lead.py | 145 +++++++++++------- addons/crm/tests/common.py | 32 +++- addons/crm/tests/test_crm_lead_merge.py | 65 +++++++- addons/crm/views/crm_lead_views.xml | 6 +- addons/crm/views/res_partner_views.xml | 1 - addons/event_crm/models/crm_lead.py | 8 + addons/event_crm/tests/__init__.py | 1 + addons/event_crm/tests/test_crm_lead_merge.py | 37 +++++ addons/sale_crm/models/crm_lead.py | 6 + addons/sale_crm/tests/__init__.py | 1 + addons/sale_crm/tests/test_crm_lead_merge.py | 40 +++++ addons/website_crm/tests/__init__.py | 1 + .../website_crm/tests/test_crm_lead_merge.py | 40 +++++ 14 files changed, 316 insertions(+), 69 deletions(-) create mode 100644 addons/event_crm/tests/test_crm_lead_merge.py create mode 100644 addons/sale_crm/tests/test_crm_lead_merge.py create mode 100644 addons/website_crm/tests/test_crm_lead_merge.py diff --git a/addons/crm/__manifest__.py b/addons/crm/__manifest__.py index 75b071d3ee4..8518fa6630f 100644 --- a/addons/crm/__manifest__.py +++ b/addons/crm/__manifest__.py @@ -4,7 +4,7 @@ { 'name': 'CRM', - 'version': '1.5', + 'version': '1.6', 'category': 'Sales/CRM', 'sequence': 15, 'summary': 'Track leads and close opportunities', diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index 42a51605f3b..ba6ff745235 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -225,7 +225,8 @@ class Lead(models.Model): 'crm.lost.reason', string='Lost Reason', index=True, ondelete='restrict', tracking=True) # Statistics - meeting_count = fields.Integer('# Meetings', compute='_compute_meeting_count') + calendar_event_ids = fields.One2many('calendar.event', 'opportunity_id', string='Meetings') + calendar_event_count = fields.Integer('# Meetings', compute='_compute_calendar_event_count') duplicate_lead_ids = fields.Many2many("crm.lead", compute="_compute_potential_lead_duplicates", string="Potential Duplicate Lead", context={"active_test": False}) duplicate_lead_count = fields.Integer(compute="_compute_potential_lead_duplicates", string="Potential Duplicate Lead Count") # UX @@ -504,7 +505,7 @@ class Lead(models.Model): for lead in self: lead.recurring_revenue_monthly_prorated = (lead.recurring_revenue_monthly or 0.0) * (lead.probability or 0) / 100.0 - def _compute_meeting_count(self): + def _compute_calendar_event_count(self): if self.ids: meeting_data = self.env['calendar.event'].sudo().read_group([ ('opportunity_id', 'in', self.ids) @@ -513,7 +514,7 @@ class Lead(models.Model): else: mapped_data = dict() for lead in self: - lead.meeting_count = mapped_data.get(lead.id, 0) + lead.calendar_event_count = mapped_data.get(lead.id, 0) @api.depends('email_from', 'partner_id', 'contact_name', 'partner_name') def _compute_potential_lead_duplicates(self): @@ -1310,62 +1311,6 @@ class Lead(models.Model): message_body = "\n\n".join(message_bodies) return self.message_post(body=message_body, subject=subject) - def _merge_opportunity_history(self, opportunities): - """ Move mail.message from the given opportunities to the current one. `self` is the - crm.lead record destination for message of `opportunities`. - - :param opportunities: see ``_merge_dependences`` - """ - self.ensure_one() - for opportunity in opportunities: - for message in opportunity.message_ids: - if message.subject: - subject = _("From %(source_name)s : %(source_subject)s", source_name=opportunity.name, source_subject=message.subject) - else: - subject = _("From %(source_name)s", source_name=opportunity.name) - message.write({ - 'res_id': self.id, - 'subject': subject, - }) - return True - - def _merge_opportunity_attachments(self, opportunities): - """ Move attachments of given opportunities to the current one `self`, and rename - the attachments having same name than native ones. - - :param opportunities: see ``_merge_dependences`` - """ - self.ensure_one() - - # return attachments of opportunity - def _get_attachments(opportunity_id): - return self.env['ir.attachment'].search([('res_model', '=', self._name), ('res_id', '=', opportunity_id)]) - - first_attachments = _get_attachments(self.id) - # counter of all attachments to move. Used to make sure the name is different for all attachments - count = 1 - for opportunity in opportunities: - attachments = _get_attachments(opportunity.id) - for attachment in attachments: - values = {'res_id': self.id} - for attachment_in_first in first_attachments: - if attachment.name == attachment_in_first.name: - values['name'] = "%s (%s)" % (attachment.name, count) - count += 1 - attachment.write(values) - return True - - def _merge_dependences(self, opportunities): - """ Merge dependences (messages, attachments, ...). These dependences will be - transfered to `self`, the most important lead. - - :param opportunities : recordset of opportunities to transfer. Does not - include `self` which is the target crm.lead being the result of the merge. - """ - self.ensure_one() - self._merge_opportunity_history(opportunities) - self._merge_opportunity_attachments(opportunities) - def merge_opportunity(self, user_id=False, team_id=False, auto_unlink=True): """ Merge opportunities in one. Different cases of merge: - merge leads together = 1 new lead @@ -1439,6 +1384,88 @@ class Lead(models.Model): def _merge_get_fields(self): return list(CRM_LEAD_FIELDS_TO_MERGE) + list(self._merge_get_fields_specific().keys()) + def _merge_dependences(self, opportunities): + """ Merge dependences (messages, attachments,activities, calendar events, + ...). These dependences will be transfered to `self` considered as the + master lead. + + :param opportunities : recordset of opportunities to transfer. Does not + include `self` which is the target crm.lead being the result of the + merge; + """ + self.ensure_one() + self._merge_dependences_history(opportunities) + self._merge_dependences_attachments(opportunities) + self._merge_dependences_calendar_events(opportunities) + + def _merge_dependences_history(self, opportunities): + """ Move history from the given opportunities to the current one. `self` + is the crm.lead record destination for message of `opportunities`. + + This method moves + * messages + * activities + + :param opportunities: see ``_merge_dependences`` + """ + self.ensure_one() + for opportunity in opportunities: + for message in opportunity.message_ids: + if message.subject: + subject = _("From %(source_name)s : %(source_subject)s", source_name=opportunity.name, source_subject=message.subject) + else: + subject = _("From %(source_name)s", source_name=opportunity.name) + message.write({ + 'res_id': self.id, + 'subject': subject, + }) + + opportunities.activity_ids.write({ + 'res_id': self.id, + }) + + return True + + def _merge_dependences_attachments(self, opportunities): + """ Move attachments of given opportunities to the current one `self`, and rename + the attachments having same name than native ones. + + :param opportunities: see ``_merge_dependences`` + """ + self.ensure_one() + + all_attachments = self.env['ir.attachment'].search([ + ('res_model', '=', self._name), + ('res_id', 'in', opportunities.ids) + ]) + + for opportunity in opportunities: + attachments = all_attachments.filtered(lambda attach: attach.res_id == opportunity.id) + for attachment in attachments: + attachment.write({ + 'res_id': self.id, + 'name': _("%(attach_name)s (from %(lead_name)s)", + attach_name=attachment.name, + lead_name=opportunity.name[:20] + ) + }) + return True + + def _merge_dependences_calendar_events(self, opportunities): + """ Move calender.event from the given opportunities to the current one. `self` is the + crm.lead record destination for event of `opportunities`. + :param opportunities: see ``merge_dependences`` + """ + self.ensure_one() + meetings = self.env['calendar.event'].search([('opportunity_id', 'in', opportunities.ids)]) + return meetings.write({ + 'res_id': self.id, + 'opportunity_id': self.id, + }) + + # CONVERT + # ---------------------------------------------------------------------- + def _convert_opportunity_data(self, customer, team_id=False): """ Extract the data from a lead to create the opportunity :param customer : res.partner record diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py index 2147169e8cc..f863b577c94 100644 --- a/addons/crm/tests/common.py +++ b/addons/crm/tests/common.py @@ -53,6 +53,7 @@ class TestCrmCommon(TestSalesCommon, MailCase): 'phone', 'probability', 'expected_revenue', 'street', 'street2', 'zip', 'create_date', 'date_action_last', 'email_from', 'email_cc', 'website' ] + merge_fields = ['description', 'type', 'priority'] @classmethod def setUpClass(cls): @@ -210,6 +211,21 @@ class TestCrmCommon(TestSalesCommon, MailCase): 'zip': '87654', }) + # test activities + cls.activity_type_1 = cls.env['mail.activity.type'].create({ + 'name': 'Lead Test Activity 1', + 'summary': 'ACT 1 : Presentation, barbecue, ... ', + 'res_model_id': cls.env['ir.model']._get('crm.crm_lead').id, + 'category': 'meeting', + 'delay_count': 5, + }) + cls.env['ir.model.data'].create({ + 'name': cls.activity_type_1.name.lower().replace(' ', '_'), + 'module': 'crm', + 'model': cls.activity_type_1._name, + 'res_id': cls.activity_type_1.id, + }) + def _create_leads_batch(self, lead_type='lead', count=10, email_dup_count=0, partner_count=0, partner_ids=None, user_ids=None, country_ids=None, probabilities=None): @@ -361,12 +377,11 @@ class TestCrmCommon(TestSalesCommon, MailCase): self.assertIn(opportunity, leads) # save opportunity value before being modified by merge process - fields_all = self.FIELDS_FIRST_SET + ['description', 'type', 'priority'] - # ensure tests are synchronized with crm code - self.assertTrue(all(field in fields_all for field in CRM_LEAD_FIELDS_TO_MERGE + list(self.env['crm.lead']._merge_get_fields_specific().keys()))) + fields_all = self.FIELDS_FIRST_SET + self.merge_fields original_opp_values = dict( (fname, opportunity[fname]) for fname in fields_all + if fname in opportunity ) def _find_value(lead, fname): @@ -390,6 +405,13 @@ class TestCrmCommon(TestSalesCommon, MailCase): values = [_find_value(lead, 'priority') for lead in leads] return max(values) + def _aggregate(fname): + if isinstance(self.env['crm.lead'][fname], models.BaseModel): + values = leads.mapped(fname) + else: + values = [_find_value(lead, fname) for lead in leads] + return values + try: # merge process will modify opportunity yield @@ -400,6 +422,8 @@ class TestCrmCommon(TestSalesCommon, MailCase): # classic fields: first not void wins or specific computation for fname in fields_all: + if fname not in opportunity: # not all fields available when doing -u + continue opp_value = opportunity[fname] if fname == 'description': self.assertEqual(opp_value, _get_description()) @@ -407,6 +431,8 @@ class TestCrmCommon(TestSalesCommon, MailCase): self.assertEqual(opp_value, _get_type()) elif fname == 'priority': self.assertEqual(opp_value, _get_priority()) + elif fname in ('order_ids', 'visitor_ids'): + self.assertEqual(opp_value, _aggregate(fname)) else: self.assertEqual( opp_value if opp_value or not isinstance(opp_value, models.BaseModel) else False, diff --git a/addons/crm/tests/test_crm_lead_merge.py b/addons/crm/tests/test_crm_lead_merge.py index de4aa826b37..97206dfec73 100644 --- a/addons/crm/tests/test_crm_lead_merge.py +++ b/addons/crm/tests/test_crm_lead_merge.py @@ -1,19 +1,21 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import base64 + from odoo.addons.crm.tests.common import TestLeadConvertMassCommon from odoo.fields import Datetime from odoo.tests.common import tagged, users @tagged('lead_manage') -class TestLeadMerge(TestLeadConvertMassCommon): +class TestLeadMergeCommon(TestLeadConvertMassCommon): """ During a mixed merge (involving leads and opps), data should be handled a certain way following their type (m2o, m2m, text, ...). """ @classmethod def setUpClass(cls): - super(TestLeadMerge, cls).setUpClass() + super(TestLeadMergeCommon, cls).setUpClass() cls.leads = cls.lead_1 + cls.lead_w_partner + cls.lead_w_contact + cls.lead_w_email + cls.lead_w_partner_company + cls.lead_w_email_lost # reset some assigned users to test salesmen assign @@ -29,6 +31,10 @@ class TestLeadMerge(TestLeadConvertMassCommon): cls.assign_users = cls.user_sales_manager + cls.user_sales_leads_convert + cls.user_sales_salesman + +@tagged('lead_manage') +class TestLeadMerge(TestLeadMergeCommon): + def test_initial_data(self): """ Ensure initial data to avoid spaghetti test update afterwards """ self.assertFalse(self.lead_1.date_conversion) @@ -160,3 +166,58 @@ class TestLeadMerge(TestLeadConvertMassCommon): partner_id=self.contact_company_1, priority='2'): leads._merge_opportunity(auto_unlink=False, max_length=None) + + @users('user_sales_manager') + def test_merge_method_dependencies(self): + """ Test if dependences for leads are not lost while merging leads. In + this test leads are ordered as + + lead_w_partner_company ---opp----seq=1 (ID greater) + lead_w_contact -----------lead---seq=30 + lead_w_email -------------lead---seq=3----------------attachments + lead_1 -------------------lead---seq=1----------------activity+meeting + lead_w_partner -----------lead---seq=False + """ + self.env['crm.lead'].browse(self.lead_w_partner_company.ids).write({'type': 'opportunity'}) + + # create side documents + attachments = self.env['ir.attachment'].create([ + {'name': '%02d.txt' % idx, + 'datas': base64.b64encode(b'Att%02d' % idx), + 'res_model': 'crm.lead', + 'res_id': self.lead_w_email.id, + } for idx in range(4) + ]) + lead_1 = self.env['crm.lead'].browse(self.lead_1.ids) + activity = lead_1.activity_schedule('crm.lead_test_activity_1') + calendar_event = self.env['calendar.event'].create({ + 'name': 'Meeting with partner', + 'activity_ids': [(4, activity.id)], + 'start': '2021-06-12 21:00:00', + 'stop': '2021-06-13 00:00:00', + 'res_model_id': self.env['ir.model']._get('crm.crm_lead').id, + 'res_id': lead_1.id, + 'opportunity_id': lead_1.id, + }) + + # run merge and check documents are moved to the master record + merge = self.env['crm.merge.opportunity'].with_context({ + 'active_model': 'crm.lead', + 'active_ids': self.leads.ids, + 'active_id': False, + }).create({ + 'team_id': self.sales_team_convert.id, + 'user_id': False, + }) + result = merge.action_merge() + master_lead = self.leads.filtered(lambda lead: lead.id == result['res_id']) + + # check result of merge process + self.assertEqual(master_lead, self.lead_w_partner_company) + # records updated + self.assertEqual(calendar_event.opportunity_id, master_lead) + self.assertEqual(calendar_event.res_id, master_lead.id) + self.assertTrue(all(att.res_id == master_lead.id for att in attachments)) + # 2many accessors updated + self.assertEqual(master_lead.activity_ids, activity) + self.assertEqual(master_lead.calendar_event_ids, calendar_event) diff --git a/addons/crm/views/crm_lead_views.xml b/addons/crm/views/crm_lead_views.xml index 6e874e178cf..4a7c45ad514 100644 --- a/addons/crm/views/crm_lead_views.xml +++ b/addons/crm/views/crm_lead_views.xml @@ -30,9 +30,9 @@ context="{'partner_id': partner_id}" attrs="{'invisible': [('type', '=', 'lead')]}">
- - Meetings - Meeting + + Meetings + Meeting