From d348c5e8801a13a153d84da651aa7b6f696f7c25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Mon, 22 Nov 2021 16:57:56 +0000
Subject: [PATCH 1/5] [IMP] crm, test_mail: flush tracking in tests involving
 tracking

Tracking is generated using commit hooks, meaning they are really sent when
the commit ends. This is done to ease values aggregation and accumulation
through various record updates. In some tests we therefore have to manually
flush the tracking, otherwise it is not created and posted. Notably tests about
lead lost wizard were not flushing and therefore not generated the tracking
messages. Those tests will be improved in future commits, cleaning them is
therefore a necessary step.

We also specifically add some tracking flush in mail performance tests. This
is to be sure we measure impact of a write and tracking separately from
previous transactions. Seems everything is working as intended as warmup and
query counters already perform flushes. Here we simply flush at the end
of the setup to be sure all base is clean before starting tests.

Task-2671709

Part-of: odoo/odoo#78648
---
 addons/crm/tests/common.py                 | 4 ++++
 addons/crm/tests/test_crm_lead_lost.py     | 7 ++++++-
 addons/test_mail/tests/test_performance.py | 7 +++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py
index 71633308398..02b35af3a3f 100644
--- a/addons/crm/tests/common.py
+++ b/addons/crm/tests/common.py
@@ -242,6 +242,10 @@ class TestCrmCommon(TestSalesCommon, MailCase):
             'res_id': cls.activity_type_1.id,
         })
 
+    def setUp(self):
+        super(TestCrmCommon, self).setUp()
+        self.flush_tracking()
+
     @classmethod
     def _activate_multi_company(cls):
         cls.company_2 = cls.env['res.company'].create({
diff --git a/addons/crm/tests/test_crm_lead_lost.py b/addons/crm/tests/test_crm_lead_lost.py
index 7f2819f7825..a4fe7216240 100644
--- a/addons/crm/tests/test_crm_lead_lost.py
+++ b/addons/crm/tests/test_crm_lead_lost.py
@@ -6,7 +6,7 @@ from odoo.exceptions import AccessError
 from odoo.tests.common import tagged, users
 
 
-@tagged('lead_manage')
+@tagged('lead_manage', 'lead_lost')
 class TestLeadConvert(crm_common.TestCrmCommon):
 
     @classmethod
@@ -22,9 +22,11 @@ class TestLeadConvert(crm_common.TestCrmCommon):
             'user_id': self.user_sales_salesman.id,
             'probability': 32,
         })
+        self.flush_tracking()
 
         lead = self.lead_1.with_user(self.env.user)
         self.assertEqual(lead.probability, 32)
+        self.assertEqual(len(lead.message_ids), 2, 'Should have tracked new responsible')
 
         lost_wizard = self.env['crm.lead.lost'].with_context({
             'active_ids': lead.ids,
@@ -33,11 +35,14 @@ class TestLeadConvert(crm_common.TestCrmCommon):
         })
 
         lost_wizard.action_lost_reason_apply()
+        self.flush_tracking()
 
         self.assertEqual(lead.probability, 0)
         self.assertEqual(lead.automated_probability, 0)
         self.assertFalse(lead.active)
         self.assertEqual(lead.lost_reason, self.lost_reason)  # TDE FIXME: should be called lost_reason_id non didjou
+        # check messages
+        self.assertEqual(len(lead.message_ids), 3, 'Should have logged a tracking message for lost lead with reason')
 
     @users('user_sales_salesman')
     def test_lead_lost_crm_rights(self):
diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py
index 95df796b177..34d5bf0298d 100644
--- a/addons/test_mail/tests/test_performance.py
+++ b/addons/test_mail/tests/test_performance.py
@@ -47,6 +47,7 @@ class BaseMailPerformance(TransactionCaseWithUserDemo):
         super(BaseMailPerformance, self).setUp()
         # patch registry to simulate a ready environment
         self.patch(self.env.registry, 'ready', True)
+        self._flush_tracking()
 
     def _init_mail_gateway(self):
         # setup mail gateway
@@ -59,6 +60,12 @@ class BaseMailPerformance(TransactionCaseWithUserDemo):
         self.env['ir.config_parameter'].set_param('mail.catchall.alias', self.alias_catchall)
         self.env['ir.config_parameter'].set_param('mail.default.from', self.default_from)
 
+    def _flush_tracking(self):
+        """ Force the creation of tracking values notably, and ensure tests are
+        reproducible. """
+        self.env['base'].flush()
+        self.cr.flush()
+
 
 @tagged('mail_performance')
 class TestBaseMailPerformance(BaseMailPerformance):

From a830ceaaf9ef72002027afa25cbb5b1575a28ee3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Wed, 20 Oct 2021 07:14:08 +0000
Subject: [PATCH 2/5] [IMP] crm: allow to log a feedback when marking a lead as
 lost

PURPOSE

Allow sales reps to add a closing note to their lead while it is being marked
as lost. This is currently already done by a lot of sales reps but manually
with the "Log a Note" button.

SPECIFICATIONS

In ``Lost Reason`` model: add a new html field allowing to log a note on the
lost leads. Below the m2o, add an Extra Comment field where users can add a
"closing note". When the wizard is submitted, log this message as a note on
selected records.

Add tests, allowing to test both the wizard and this new feature.

Task-2671709

Part-of: odoo/odoo#78648
---
 addons/crm/tests/test_crm_lead_lost.py    | 71 +++++++++++++++++++++--
 addons/crm/wizard/crm_lead_lost.py        | 19 +++++-
 addons/crm/wizard/crm_lead_lost_views.xml |  1 +
 addons/mail/tests/common.py               |  9 ++-
 4 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/addons/crm/tests/test_crm_lead_lost.py b/addons/crm/tests/test_crm_lead_lost.py
index a4fe7216240..735d3ec2ed3 100644
--- a/addons/crm/tests/test_crm_lead_lost.py
+++ b/addons/crm/tests/test_crm_lead_lost.py
@@ -4,6 +4,7 @@
 from odoo.addons.crm.tests import common as crm_common
 from odoo.exceptions import AccessError
 from odoo.tests.common import tagged, users
+from odoo.tools import mute_logger
 
 
 @tagged('lead_manage', 'lead_lost')
@@ -18,34 +19,96 @@ class TestLeadConvert(crm_common.TestCrmCommon):
 
     @users('user_sales_salesman')
     def test_lead_lost(self):
+        """ Test setting a lead as lost using the wizard. Also check that an
+        'html editor' void content used as feedback is not logged on the lead. """
+        # Initial data
+        self.assertEqual(len(self.lead_1.message_ids), 1, 'Should contain creation message')
+        creation_message = self.lead_1.message_ids[0]
+        self.assertEqual(creation_message.subtype_id, self.env.ref('crm.mt_lead_create'))
+
+        # Update responsible as ACLs is "own only" for user_sales_salesman
         self.lead_1.with_user(self.user_sales_manager).write({
             'user_id': self.user_sales_salesman.id,
             'probability': 32,
         })
         self.flush_tracking()
 
-        lead = self.lead_1.with_user(self.env.user)
+        lead = self.env['crm.lead'].browse(self.lead_1.ids)
+        self.assertFalse(lead.lost_reason)
         self.assertEqual(lead.probability, 32)
         self.assertEqual(len(lead.message_ids), 2, 'Should have tracked new responsible')
+        update_message = lead.message_ids[0]
+        self.assertEqual(update_message.subtype_id, self.env.ref('mail.mt_note'))
 
+        # mark as lost using the wizard
         lost_wizard = self.env['crm.lead.lost'].with_context({
             'active_ids': lead.ids,
         }).create({
-            'lost_reason_id': self.lost_reason.id
+            'lost_reason_id': self.lost_reason.id,
+            'lost_feedback': '<p></p>',  # void content
         })
 
         lost_wizard.action_lost_reason_apply()
         self.flush_tracking()
 
-        self.assertEqual(lead.probability, 0)
-        self.assertEqual(lead.automated_probability, 0)
+        # check lead update
         self.assertFalse(lead.active)
+        self.assertEqual(lead.automated_probability, 0)
         self.assertEqual(lead.lost_reason, self.lost_reason)  # TDE FIXME: should be called lost_reason_id non didjou
+        self.assertEqual(lead.probability, 0)
         # check messages
         self.assertEqual(len(lead.message_ids), 3, 'Should have logged a tracking message for lost lead with reason')
+        update_message = lead.message_ids[0]
+        self.assertEqual(update_message.subtype_id, self.env.ref('crm.mt_lead_lost'))
+        self.assertEqual(len(update_message.tracking_value_ids), 2, 'Tracking: active, lost reason')
+        self.assertTracking(
+            update_message,
+            [('active', 'boolean', True, False),
+             ('lost_reason', 'many2one', False, self.lost_reason)
+            ]
+        )
+
+    @users('user_sales_leads')
+    def test_lead_lost_batch_wfeedback(self):
+        """ Test setting leads as lost in batch using the wizard, including a log
+        message. """
+        leads = self._create_leads_batch(lead_type='lead', count=10, probabilities=[10, 20, 30])
+        self.assertEqual(len(leads), 10)
+        self.flush_tracking()
+
+        lost_wizard = self.env['crm.lead.lost'].with_context({
+            'active_ids': leads.ids,
+        }).create({
+            'lost_reason_id': self.lost_reason.id,
+            'lost_feedback': '<p>I cannot find it. It was in my closet and pouf, disappeared.</p>',
+        })
+        lost_wizard.action_lost_reason_apply()
+        self.flush_tracking()
+
+        for lead in leads:
+            # check content
+            self.assertFalse(lead.active)
+            self.assertEqual(lead.automated_probability, 0)
+            self.assertEqual(lead.probability, 0)
+            self.assertEqual(lead.lost_reason, self.lost_reason)
+            # check messages
+            self.assertEqual(len(lead.message_ids), 3, 'Should have 3 messages: creation, lost, and log')
+            lost_message = lead.message_ids.filtered(lambda msg: msg.subtype_id == self.env.ref('crm.mt_lead_lost'))
+            self.assertTrue(lost_message)
+            self.assertTracking(
+                lost_message,
+                [('active', 'boolean', True, False),
+                 ('lost_reason', 'many2one', False, self.lost_reason)
+                ]
+            )
+            note_message = lead.message_ids.filtered(lambda msg: msg.subtype_id == self.env.ref('mail.mt_note'))
+            self.assertTrue(note_message)
+            self.assertEqual(note_message.body, '<p>I cannot find it. It was in my closet and pouf, disappeared.</p>')
 
     @users('user_sales_salesman')
+    @mute_logger('odoo.addons.base.models')
     def test_lead_lost_crm_rights(self):
+        """ Test ACLs of lost reasons management and usage """
         lead = self.lead_1.with_user(self.env.user)
 
         # nice try little salesman but only managers can create lost reason to avoid bloating the DB
diff --git a/addons/crm/wizard/crm_lead_lost.py b/addons/crm/wizard/crm_lead_lost.py
index 8f64a40579b..f2ca5958544 100644
--- a/addons/crm/wizard/crm_lead_lost.py
+++ b/addons/crm/wizard/crm_lead_lost.py
@@ -1,6 +1,8 @@
 # -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo import api, fields, models
+from odoo import fields, models, _
+from odoo.tools.mail import is_html_empty
 
 
 class CrmLeadLost(models.TransientModel):
@@ -8,7 +10,20 @@ class CrmLeadLost(models.TransientModel):
     _description = 'Get Lost Reason'
 
     lost_reason_id = fields.Many2one('crm.lost.reason', 'Lost Reason')
+    lost_feedback = fields.Html(
+        'Closing Note', sanitize=True,
+        help="Closing note logged in leads discussion history."
+    )
 
     def action_lost_reason_apply(self):
+        self.ensure_one()
         leads = self.env['crm.lead'].browse(self.env.context.get('active_ids'))
-        return leads.action_set_lost(lost_reason=self.lost_reason_id.id)
+        res = leads.action_set_lost(lost_reason=self.lost_reason_id.id)
+        if not is_html_empty(self.lost_feedback):
+            leads._message_log_batch(
+                bodies=dict(
+                    (lead.id, self.lost_feedback)
+                    for lead in leads),
+                subject=False,
+            )
+        return res
diff --git a/addons/crm/wizard/crm_lead_lost_views.xml b/addons/crm/wizard/crm_lead_lost_views.xml
index 79611a98e7c..c6ca6b23aca 100644
--- a/addons/crm/wizard/crm_lead_lost_views.xml
+++ b/addons/crm/wizard/crm_lead_lost_views.xml
@@ -8,6 +8,7 @@
                     <group class="oe_title">
                         <field name="lost_reason_id" options="{'no_create_edit': True}" />
                     </group>
+                    <field name="lost_feedback" class="oe_title" placeholder="What went wrong ?"/>
                     <footer>
                         <button name="action_lost_reason_apply" string="Submit" type="object" class="btn-primary" data-hotkey="q"/>
                         <button string="Cancel" class="btn-secondary" special="cancel" data-hotkey="z"/>
diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py
index d3dfc0b117b..78311e2b6b8 100644
--- a/addons/mail/tests/common.py
+++ b/addons/mail/tests/common.py
@@ -860,15 +860,18 @@ class MailCase(MockEmail):
         for field_name, value_type, old_value, new_value in data:
             tracking = tracking_values.filtered(lambda track: track.field.name == field_name)
             self.assertEqual(len(tracking), 1)
-            if value_type in ('char', 'integer'):
+            if value_type == 'char':
                 self.assertEqual(tracking.old_value_char, old_value)
                 self.assertEqual(tracking.new_value_char, new_value)
-            elif value_type in ('many2one'):
+            elif value_type in ('boolean', 'integer'):
+                self.assertEqual(tracking.old_value_integer, old_value)
+                self.assertEqual(tracking.new_value_integer, new_value)
+            elif value_type == 'many2one':
                 self.assertEqual(tracking.old_value_integer, old_value and old_value.id or False)
                 self.assertEqual(tracking.new_value_integer, new_value and new_value.id or False)
                 self.assertEqual(tracking.old_value_char, old_value and old_value.display_name or '')
                 self.assertEqual(tracking.new_value_char, new_value and new_value.display_name or '')
-            elif value_type in ('monetary'):
+            elif value_type == 'monetary':
                 self.assertEqual(tracking.old_value_monetary, old_value)
                 self.assertEqual(tracking.new_value_monetary, new_value)
             else:

From d1edc9e6a7f086eb0350bc501cfb4f2c3c309029 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Wed, 20 Oct 2021 07:28:24 +0000
Subject: [PATCH 3/5] [REF] crm: correctly name lost_reason field on lead

As this is a many2one, it should end with an ``_id`` suffix. Otherwise we
may think this is a char field, which was probably the case at one point.

Task-2671709

Part-of: odoo/odoo#78648
---
 addons/crm/__manifest__.py                         |  2 +-
 addons/crm/data/crm_lead_merge_template.xml        |  4 ++--
 addons/crm/models/crm_lead.py                      | 10 +++++-----
 addons/crm/models/crm_lost_reason.py               | 10 +++++++---
 addons/crm/report/crm_opportunity_report_views.xml |  2 +-
 addons/crm/tests/test_crm_lead_lost.py             | 10 +++++-----
 addons/crm/tests/test_crm_lead_merge.py            |  8 ++++----
 addons/crm/views/crm_lead_views.xml                |  4 ++--
 addons/crm/wizard/crm_lead_lost.py                 |  4 ++--
 9 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/addons/crm/__manifest__.py b/addons/crm/__manifest__.py
index 55e0bcd8780..722950735fd 100644
--- a/addons/crm/__manifest__.py
+++ b/addons/crm/__manifest__.py
@@ -4,7 +4,7 @@
 
 {
     'name': 'CRM',
-    'version': '1.6',
+    'version': '1.7',
     'category': 'Sales/CRM',
     'sequence': 15,
     'summary': 'Track leads and close opportunities',
diff --git a/addons/crm/data/crm_lead_merge_template.xml b/addons/crm/data/crm_lead_merge_template.xml
index a46087183db..af9e485123b 100644
--- a/addons/crm/data/crm_lead_merge_template.xml
+++ b/addons/crm/data/crm_lead_merge_template.xml
@@ -40,8 +40,8 @@
                 <div t-if="lead.priority">
                     Priority: <span t-field="lead.priority"/>
                 </div>
-                <div t-if="lead.lost_reason">
-                    Lost Reason: <span t-field="lead.lost_reason"/>
+                <div t-if="lead.lost_reason_id">
+                    Lost Reason: <span t-field="lead.lost_reason_id"/>
                 </div>
                 <div>
                     Created on: <span t-field="lead.create_date"/>
diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py
index 1f3daa6d1c9..61c8adfba25 100644
--- a/addons/crm/models/crm_lead.py
+++ b/addons/crm/models/crm_lead.py
@@ -213,7 +213,7 @@ class Lead(models.Model):
     automated_probability = fields.Float('Automated Probability', compute='_compute_probabilities', readonly=True, store=True)
     is_automated_probability = fields.Boolean('Is automated probability?', compute="_compute_is_automated_probability")
     # Won/Lost
-    lost_reason = fields.Many2one(
+    lost_reason_id = fields.Many2one(
         'crm.lost.reason', string='Lost Reason',
         index=True, ondelete='restrict', tracking=True)
     # Statistics
@@ -901,7 +901,7 @@ class Lead(models.Model):
         activated = self.filtered(lambda lead: lead.active)
         archived = self.filtered(lambda lead: not lead.active)
         if activated:
-            activated.write({'lost_reason': False})
+            activated.write({'lost_reason_id': False})
             activated._compute_probabilities()
         if archived:
             archived.write({'probability': 0, 'automated_probability': 0})
@@ -1355,9 +1355,9 @@ class Lead(models.Model):
             '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:
+            'lost_reason_id': lambda fname, leads:
                 False if leads and leads[0].probability
-                else next((lead.lost_reason for lead in leads if lead.lost_reason), False),
+                else next((lead.lost_reason_id for lead in leads if lead.lost_reason_id), False),
         }
 
     def _merge_get_fields(self):
@@ -1723,7 +1723,7 @@ class Lead(models.Model):
         self.ensure_one()
         if 'stage_id' in init_values and self.probability == 100 and self.stage_id:
             return self.env.ref('crm.mt_lead_won')
-        elif 'lost_reason' in init_values and self.lost_reason:
+        elif 'lost_reason_id' in init_values and self.lost_reason_id:
             return self.env.ref('crm.mt_lead_lost')
         elif 'stage_id' in init_values:
             return self.env.ref('crm.mt_lead_stage')
diff --git a/addons/crm/models/crm_lost_reason.py b/addons/crm/models/crm_lost_reason.py
index 4b848acbc5f..714a984db68 100644
--- a/addons/crm/models/crm_lost_reason.py
+++ b/addons/crm/models/crm_lost_reason.py
@@ -13,8 +13,12 @@ class LostReason(models.Model):
     leads_count = fields.Integer('Leads Count', compute='_compute_leads_count')
 
     def _compute_leads_count(self):
-        lead_data = self.env['crm.lead'].with_context(active_test=False).read_group([('lost_reason', 'in', self.ids)], ['lost_reason'], ['lost_reason'])
-        mapped_data = dict((data['lost_reason'][0], data['lost_reason_count']) for data in lead_data)
+        lead_data = self.env['crm.lead'].with_context(active_test=False).read_group(
+            [('lost_reason_id', 'in', self.ids)],
+            ['lost_reason_id'],
+            ['lost_reason_id']
+        )
+        mapped_data = dict((data['lost_reason_id'][0], data['lost_reason_id_count']) for data in lead_data)
         for reason in self:
             reason.leads_count = mapped_data.get(reason.id, 0)
 
@@ -22,7 +26,7 @@ class LostReason(models.Model):
         return {
             'name': _('Leads'),
             'view_mode': 'tree,form',
-            'domain': [('lost_reason', 'in', self.ids)],
+            'domain': [('lost_reason_id', 'in', self.ids)],
             'res_model': 'crm.lead',
             'type': 'ir.actions.act_window',
             'context': {'create': False, 'active_test': False},
diff --git a/addons/crm/report/crm_opportunity_report_views.xml b/addons/crm/report/crm_opportunity_report_views.xml
index 42347272642..8f8a7432ea6 100644
--- a/addons/crm/report/crm_opportunity_report_views.xml
+++ b/addons/crm/report/crm_opportunity_report_views.xml
@@ -105,7 +105,7 @@
                         <filter string="Conversion Date" context="{'group_by':'date_conversion:month'}" name="conversion_date" help="Conversion Date from Lead to Opportunity" groups="crm.group_use_lead"/>
                         <filter string="Expected Closing" context="{'group_by':'date_deadline:month'}" name="date_deadline"/>
                         <filter string="Closed Date" context="{'group_by':'date_closed'}" name="date_closed_groupby"/>
-                        <filter string="Lost Reason" name="lostreason" context="{'group_by':'lost_reason'}"/>
+                        <filter string="Lost Reason" name="lostreason" context="{'group_by':'lost_reason_id'}"/>
                     </group>
                 </search>
             </field>
diff --git a/addons/crm/tests/test_crm_lead_lost.py b/addons/crm/tests/test_crm_lead_lost.py
index 735d3ec2ed3..e7dd92669fc 100644
--- a/addons/crm/tests/test_crm_lead_lost.py
+++ b/addons/crm/tests/test_crm_lead_lost.py
@@ -34,7 +34,7 @@ class TestLeadConvert(crm_common.TestCrmCommon):
         self.flush_tracking()
 
         lead = self.env['crm.lead'].browse(self.lead_1.ids)
-        self.assertFalse(lead.lost_reason)
+        self.assertFalse(lead.lost_reason_id)
         self.assertEqual(lead.probability, 32)
         self.assertEqual(len(lead.message_ids), 2, 'Should have tracked new responsible')
         update_message = lead.message_ids[0]
@@ -54,7 +54,7 @@ class TestLeadConvert(crm_common.TestCrmCommon):
         # check lead update
         self.assertFalse(lead.active)
         self.assertEqual(lead.automated_probability, 0)
-        self.assertEqual(lead.lost_reason, self.lost_reason)  # TDE FIXME: should be called lost_reason_id non didjou
+        self.assertEqual(lead.lost_reason_id, self.lost_reason)  # TDE FIXME: should be called lost_reason_id non didjou
         self.assertEqual(lead.probability, 0)
         # check messages
         self.assertEqual(len(lead.message_ids), 3, 'Should have logged a tracking message for lost lead with reason')
@@ -64,7 +64,7 @@ class TestLeadConvert(crm_common.TestCrmCommon):
         self.assertTracking(
             update_message,
             [('active', 'boolean', True, False),
-             ('lost_reason', 'many2one', False, self.lost_reason)
+             ('lost_reason_id', 'many2one', False, self.lost_reason)
             ]
         )
 
@@ -90,7 +90,7 @@ class TestLeadConvert(crm_common.TestCrmCommon):
             self.assertFalse(lead.active)
             self.assertEqual(lead.automated_probability, 0)
             self.assertEqual(lead.probability, 0)
-            self.assertEqual(lead.lost_reason, self.lost_reason)
+            self.assertEqual(lead.lost_reason_id, self.lost_reason)
             # check messages
             self.assertEqual(len(lead.message_ids), 3, 'Should have 3 messages: creation, lost, and log')
             lost_message = lead.message_ids.filtered(lambda msg: msg.subtype_id == self.env.ref('crm.mt_lead_lost'))
@@ -98,7 +98,7 @@ class TestLeadConvert(crm_common.TestCrmCommon):
             self.assertTracking(
                 lost_message,
                 [('active', 'boolean', True, False),
-                 ('lost_reason', 'many2one', False, self.lost_reason)
+                 ('lost_reason_id', 'many2one', False, self.lost_reason)
                 ]
             )
             note_message = lead.message_ids.filtered(lambda msg: msg.subtype_id == self.env.ref('mail.mt_note'))
diff --git a/addons/crm/tests/test_crm_lead_merge.py b/addons/crm/tests/test_crm_lead_merge.py
index bbf73b70155..9f5b8bb2dfa 100644
--- a/addons/crm/tests/test_crm_lead_merge.py
+++ b/addons/crm/tests/test_crm_lead_merge.py
@@ -307,7 +307,7 @@ class TestLeadMerge(TestLeadMergeCommon):
         # 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,
+            'lost_reason_id': lost_reason,
             'probability': 0,
         })
 
@@ -317,7 +317,7 @@ class TestLeadMerge(TestLeadMergeCommon):
                                    name='Nibbler Spacecraft Request',
                                    partner_id=self.contact_company_1,
                                    priority='2',
-                                   lost_reason=False,
+                                   lost_reason_id=False,
                                    tag_ids=all_tags):
             leads._merge_opportunity(auto_unlink=False, max_length=None)
 
@@ -331,11 +331,11 @@ class TestLeadMerge(TestLeadMergeCommon):
         })
 
         lost_reason = self.env['crm.lost.reason'].create({'name': 'Test Reason'})
-        self.lead_w_partner.lost_reason = lost_reason
+        self.lead_w_partner.lost_reason_id = 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):
+        with self.assertLeadMerged(leads[0], leads, lost_reason_id=lost_reason):
             leads._merge_opportunity(auto_unlink=False, max_length=None)
 
     @users('user_sales_manager')
diff --git a/addons/crm/views/crm_lead_views.xml b/addons/crm/views/crm_lead_views.xml
index 277d07edb80..44127d2899d 100644
--- a/addons/crm/views/crm_lead_views.xml
+++ b/addons/crm/views/crm_lead_views.xml
@@ -246,7 +246,7 @@
                                 <field name="tag_ids" widget="many2many_tags" options="{'color_field': 'color', 'no_create_edit': True}"/>
                             </group>
                             <group name="opportunity_info" attrs="{'invisible': [('type', '=', 'lead')]}">
-                                <field name="lost_reason" attrs="{'invisible': [('active', '=', True)]}"/>
+                                <field name="lost_reason_id" attrs="{'invisible': [('active', '=', True)]}"/>
                                 <field name="date_conversion" invisible="1"/>
                                 <field name="company_id" groups="base.group_multi_company"/>
                                 <field name="user_company_ids" invisible="1"/>
@@ -985,7 +985,7 @@
                         <filter name="stage" string="Stage" context="{'group_by':'stage_id'}"/>
                         <filter name="city" string="City" context="{'group_by': 'city'}"/>
                         <filter string="Country" name="country" context="{'group_by':'country_id'}" />
-                        <filter string="Lost Reason" name="lostreason" context="{'group_by':'lost_reason'}"/>
+                        <filter string="Lost Reason" name="lostreason" context="{'group_by':'lost_reason_id'}"/>
                         <filter string="Company" name="company" context="{'group_by':'company_id'}" groups="base.group_multi_company"/>
                         <filter string="Campaign" name="compaign" domain="[]" context="{'group_by':'campaign_id'}"/>
                         <filter string="Medium" name="medium" domain="[]" context="{'group_by':'medium_id'}"/>
diff --git a/addons/crm/wizard/crm_lead_lost.py b/addons/crm/wizard/crm_lead_lost.py
index f2ca5958544..66b36754c6f 100644
--- a/addons/crm/wizard/crm_lead_lost.py
+++ b/addons/crm/wizard/crm_lead_lost.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo import fields, models, _
+from odoo import fields, models
 from odoo.tools.mail import is_html_empty
 
 
@@ -18,7 +18,7 @@ class CrmLeadLost(models.TransientModel):
     def action_lost_reason_apply(self):
         self.ensure_one()
         leads = self.env['crm.lead'].browse(self.env.context.get('active_ids'))
-        res = leads.action_set_lost(lost_reason=self.lost_reason_id.id)
+        res = leads.action_set_lost(lost_reason_id=self.lost_reason_id.id)
         if not is_html_empty(self.lost_feedback):
             leads._message_log_batch(
                 bodies=dict(

From 869ada08a00786d9efa140a7ec1ebf3d34dadc8b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Tue, 16 Nov 2021 16:05:43 +0000
Subject: [PATCH 4/5] [IMP] mail: rename tracking methods and clean dead code

Purpose is to have tracking methods beginning with ``_track``, indicating
those are tool methods used for tracking. It helps organizing the mail thread
file and having short but precise method names.

We also remove the usage of ``mail_track_log_only`` context key that is not
used anymore in the code. It allows to shorten a bit the code and make it
easier to read.

Task-2671709

Part-of: odoo/odoo#78648
---
 addons/account/models/account_move.py |   2 +-
 addons/mail/models/mail_thread.py     | 157 ++++++++++++++------------
 addons/project/models/project.py      |   2 +-
 3 files changed, 85 insertions(+), 76 deletions(-)

diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py
index 04d80bb138c..42d526d7d08 100644
--- a/addons/account/models/account_move.py
+++ b/addons/account/models/account_move.py
@@ -2319,7 +2319,7 @@ class AccountMove(models.Model):
 
         if not self.is_invoice(include_receipts=True):
             if self.payment_id and 'state' in init_values:
-                self.payment_id.message_track(['state'], {self.payment_id.id: init_values})
+                self.payment_id._message_track(['state'], {self.payment_id.id: init_values})
             return super(AccountMove, self)._track_subtype(init_values)
 
         if 'payment_state' in init_values and self.payment_state == 'paid':
diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index 60b7c1b53de..00d9bfd98ce 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -258,7 +258,7 @@ class MailThread(models.AbstractModel):
         """
         if self._context.get('tracking_disable'):
             threads = super(MailThread, self).create(vals_list)
-            threads._discard_tracking()
+            threads._track_discard()
             return threads
 
         threads = super(MailThread, self).create(vals_list)
@@ -298,9 +298,9 @@ class MailThread(models.AbstractModel):
                 threads_no_subtype._message_log_batch(bodies=bodies)
 
         # post track template if a tracked field changed
-        threads._discard_tracking()
+        threads._track_discard()
         if not self._context.get('mail_notrack'):
-            fnames = self._get_tracked_fields()
+            fnames = self._track_get_fields()
             for thread in threads:
                 create_values = create_values_list[thread.id]
                 changes = [fname for fname in fnames if create_values.get(fname)]
@@ -316,7 +316,7 @@ class MailThread(models.AbstractModel):
             return super(MailThread, self).write(values)
 
         if not self._context.get('mail_notrack'):
-            self._prepare_tracking(self._fields)
+            self._track_prepare(self._fields)
 
         # Perform write
         result = super(MailThread, self).write(values)
@@ -332,7 +332,7 @@ class MailThread(models.AbstractModel):
         if not self:
             return True
         # discard pending tracking
-        self._discard_tracking()
+        self._track_discard()
         self.env['mail.message'].search([('model', '=', self._name), ('res_id', 'in', self.ids)]).sudo().unlink()
         res = super(MailThread, self).unlink()
         self.env['mail.followers'].sudo().search(
@@ -405,7 +405,7 @@ class MailThread(models.AbstractModel):
 
     def _compute_field_value(self, field):
         if not self._context.get('tracking_disable') and not self._context.get('mail_notrack'):
-            self._prepare_tracking(f.name for f in self.pool.field_computed[field] if f.store)
+            self._track_prepare(f.name for f in self.pool.field_computed[field] if f.store)
 
         return super()._compute_field_value(field)
 
@@ -465,15 +465,15 @@ class MailThread(models.AbstractModel):
     # TRACKING / LOG
     # ------------------------------------------------------
 
-    def _prepare_tracking(self, fields):
-        """ Prepare the tracking of ``fields`` for ``self``.
+    def _track_prepare(self, fields_iter):
+        """ Prepare the tracking of ``fields_iter`` for ``self``.
 
-        :param fields: iterable of fields names to potentially track
+        :param iter fields_iter: iterable of fields names to potentially track
         """
-        fnames = self._get_tracked_fields().intersection(fields)
+        fnames = self._track_get_fields().intersection(fields_iter)
         if not fnames:
             return
-        self.env.cr.precommit.add(self._finalize_tracking)
+        self.env.cr.precommit.add(self._track_finalize)
         initial_values = self.env.cr.precommit.data.setdefault(f'mail.tracking.{self._name}', {})
         for record in self:
             if not record.id:
@@ -483,37 +483,37 @@ class MailThread(models.AbstractModel):
                 for fname in fnames:
                     values.setdefault(fname, record[fname])
 
-    def _discard_tracking(self):
+    def _track_discard(self):
         """ Prevent any tracking of fields on ``self``. """
-        if not self._get_tracked_fields():
+        if not self._track_get_fields():
             return
-        self.env.cr.precommit.add(self._finalize_tracking)
+        self.env.cr.precommit.add(self._track_finalize)
         initial_values = self.env.cr.precommit.data.setdefault(f'mail.tracking.{self._name}', {})
         # disable tracking by setting initial values to None
         for id_ in self.ids:
             initial_values[id_] = None
 
-    def _finalize_tracking(self):
+    def _track_finalize(self):
         """ Generate the tracking messages for the records that have been
-        prepared with ``_prepare_tracking``.
+        prepared with ``_tracking_prepare``.
         """
         initial_values = self.env.cr.precommit.data.pop(f'mail.tracking.{self._name}', {})
         ids = [id_ for id_, vals in initial_values.items() if vals]
         if not ids:
             return
         records = self.browse(ids).sudo()
-        fnames = self._get_tracked_fields()
+        fnames = self._track_get_fields()
         context = clean_context(self._context)
-        tracking = records.with_context(context).message_track(fnames, initial_values)
+        tracking = records.with_context(context)._message_track(fnames, initial_values)
         for record in records:
-            changes, tracking_value_ids = tracking.get(record.id, (None, None))
+            changes, _tracking_value_ids = tracking.get(record.id, (None, None))
             record._message_track_post_template(changes)
         # this method is called after the main flush() and just before commit();
         # we have to flush() again in case we triggered some recomputations
         self.flush()
 
     @tools.ormcache('self.env.uid', 'self.env.su')
-    def _get_tracked_fields(self):
+    def _track_get_fields(self):
         """ Return the set of tracked fields names for the current model. """
         model_fields = {
             name
@@ -523,7 +523,69 @@ class MailThread(models.AbstractModel):
 
         return model_fields and set(self.fields_get(model_fields))
 
+    def _track_subtype(self, initial_values):
+        """ Give the subtypes triggered by the changes on the record according
+        to values that have been updated.
+
+        :param dict initial_values: original values of the record; only modified
+          fields are present in the dict
+
+        :returns: a subtype browse record or False if no subtype is triggered
+        """
+        self.ensure_one()
+        return False
+
+    def _message_track(self, fields_iter, initial_values_dict):
+        """ Track updated values. Comparing the initial and current values of
+        the fields given in tracked_fields, it generates a message containing
+        the updated values. This message can be linked to a mail.message.subtype
+        given by the ``_track_subtype`` method.
+
+        :param iter fields_iter: iterable of field names to track
+        :param dict initial_values_dict: mapping {record_id: initial_values}
+          where initial_values is a dict {field_name: value, ... }
+        :return: mapping {record_id: (changed_field_names, tracking_value_ids)}
+            containing existing records only
+        """
+        if not fields_iter:
+            return {}
+
+        tracked_fields = self.fields_get(fields_iter)
+        tracking = dict()
+        for record in self:
+            try:
+                tracking[record.id] = record._mail_track(tracked_fields, initial_values_dict[record.id])
+            except MissingError:
+                continue
+
+        for record in self:
+            changes, tracking_value_ids = tracking.get(record.id, (None, None))
+            if not changes:
+                continue
+
+            # find subtypes and post messages or log if no subtype found
+            subtype = record._track_subtype(
+                dict((col_name, initial_values_dict[record.id][col_name])
+                     for col_name in changes)
+            )
+            if subtype:
+                if not subtype.exists():
+                    _logger.debug('subtype "%s" not found' % subtype.name)
+                    continue
+                record.message_post(subtype_id=subtype.id, tracking_value_ids=tracking_value_ids)
+            elif tracking_value_ids:
+                record._message_log(tracking_value_ids=tracking_value_ids)
+
+        return tracking
+
     def _message_track_post_template(self, changes):
+        """ Based on a tracking, post a message defined by ``_track_template``
+        parameters. It allows to implement automatic post of messages based
+        on templates (e.g. stage change triggering automatic email).
+
+        :param dict changes: mapping {record_id: (changed_field_names, tracking_value_ids)}
+            containing existing records only
+        """
         if not changes:
             return True
         # Clean the context to get rid of residual default_* keys
@@ -535,7 +597,7 @@ class MailThread(models.AbstractModel):
         # doesn't exist
         self = self.with_context(clean_context(self._context))
         templates = self._track_template(changes)
-        for field_name, (template, post_kwargs) in templates.items():
+        for _field_name, (template, post_kwargs) in templates.items():
             if not template:
                 continue
             if isinstance(template, str):
@@ -547,59 +609,6 @@ class MailThread(models.AbstractModel):
     def _track_template(self, changes):
         return dict()
 
-    def message_track(self, tracked_fields, initial_values):
-        """ Track updated values. Comparing the initial and current values of
-        the fields given in tracked_fields, it generates a message containing
-        the updated values. This message can be linked to a mail.message.subtype
-        given by the ``_track_subtype`` method.
-
-        :param tracked_fields: iterable of field names to track
-        :param initial_values: mapping {record_id: {field_name: value}}
-        :return: mapping {record_id: (changed_field_names, tracking_value_ids)}
-            containing existing records only
-        """
-        if not tracked_fields:
-            return True
-
-        tracked_fields = self.fields_get(tracked_fields)
-        tracking = dict()
-        for record in self:
-            try:
-                tracking[record.id] = record._mail_track(tracked_fields, initial_values[record.id])
-            except MissingError:
-                continue
-
-        for record in self:
-            changes, tracking_value_ids = tracking.get(record.id, (None, None))
-            if not changes:
-                continue
-
-            # find subtypes and post messages or log if no subtype found
-            subtype = False
-            # By passing this key, that allows to let the subtype empty and so don't sent email because partners_to_notify from mail_message._notify will be empty
-            if not self._context.get('mail_track_log_only'):
-                subtype = record._track_subtype(dict((col_name, initial_values[record.id][col_name]) for col_name in changes))
-            if subtype:
-                if not subtype.exists():
-                    _logger.debug('subtype "%s" not found' % subtype.name)
-                    continue
-                record.message_post(subtype_id=subtype.id, tracking_value_ids=tracking_value_ids)
-            elif tracking_value_ids:
-                record._message_log(tracking_value_ids=tracking_value_ids)
-
-        return tracking
-
-    def _track_subtype(self, init_values):
-        """ Give the subtypes triggered by the changes on the record according
-        to values that have been updated.
-
-        :param init_values: the original values of the record; only modified fields
-                            are present in the dict
-        :type init_values: dict
-        :returns: a subtype browse record or False if no subtype is trigerred
-        """
-        return False
-
     # ------------------------------------------------------
     # MAIL GATEWAY
     # ------------------------------------------------------
diff --git a/addons/project/models/project.py b/addons/project/models/project.py
index e1c4eb52cdc..b6c8a1b12a6 100644
--- a/addons/project/models/project.py
+++ b/addons/project/models/project.py
@@ -1457,7 +1457,7 @@ class Task(models.Model):
         """ Returns the set of tracked field names for the current model.
         Those fields are the ones tracked in the parent task when using task dependencies.
 
-        See :meth:`mail.models.MailThread._get_tracked_fields`"""
+        See :meth:`mail.models.MailThread._track_get_fields`"""
         fields = {name for name, field in self._fields.items() if getattr(field, 'task_dependency_tracking', None)}
         return fields and set(self.fields_get(fields))
 

From 1341d52540d7bd30ef1c98adbd6d188db4cafe98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Tue, 16 Nov 2021 16:45:18 +0000
Subject: [PATCH 5/5] [IMP] mail, crm: allow to give a body to tracking

PURPOSE

Concatenate feedback message and tracking when marking a lead as lost. This
currently generates 2 consecutive message about the same change, having a
single one is better from an UI point of view.

SPECIFICATIONS

Allow to link a body to a value change tracking. Tracking is currently done
by accumulating changes in a structure (see ``env.cr.precommit.data`` usage
with ``mail.tracking.<name>`` key). In the end those values are used to
generate a message with tracking value and a subtype.

In this commit we allow to manually set a body used as a message for the
tracking message, adding a new ``mail.tracking.message.<name>`` key. It is
used when posting or logging the tracking message, simply propagated as body
to ``message_post`` or ``message_log``.

In crm we use this when losting a lead through the dedicated wizard. A bit
of custom html allows to have a nice display.

Task-2671709

Part-of: odoo/odoo#78648
---
 addons/crm/tests/test_crm_lead_lost.py    |  7 +++----
 addons/crm/wizard/crm_lead_lost.py        | 14 +++++++-------
 addons/crm/wizard/crm_lead_lost_views.xml |  4 ++--
 addons/mail/models/mail_thread.py         | 23 +++++++++++++++++++++--
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/addons/crm/tests/test_crm_lead_lost.py b/addons/crm/tests/test_crm_lead_lost.py
index e7dd92669fc..33d5f38ea41 100644
--- a/addons/crm/tests/test_crm_lead_lost.py
+++ b/addons/crm/tests/test_crm_lead_lost.py
@@ -92,7 +92,7 @@ class TestLeadConvert(crm_common.TestCrmCommon):
             self.assertEqual(lead.probability, 0)
             self.assertEqual(lead.lost_reason_id, self.lost_reason)
             # check messages
-            self.assertEqual(len(lead.message_ids), 3, 'Should have 3 messages: creation, lost, and log')
+            self.assertEqual(len(lead.message_ids), 2, 'Should have 2 messages: creation, lost with log')
             lost_message = lead.message_ids.filtered(lambda msg: msg.subtype_id == self.env.ref('crm.mt_lead_lost'))
             self.assertTrue(lost_message)
             self.assertTracking(
@@ -101,9 +101,8 @@ class TestLeadConvert(crm_common.TestCrmCommon):
                  ('lost_reason_id', 'many2one', False, self.lost_reason)
                 ]
             )
-            note_message = lead.message_ids.filtered(lambda msg: msg.subtype_id == self.env.ref('mail.mt_note'))
-            self.assertTrue(note_message)
-            self.assertEqual(note_message.body, '<p>I cannot find it. It was in my closet and pouf, disappeared.</p>')
+            self.assertIn('<p>I cannot find it. It was in my closet and pouf, disappeared.</p>', lost_message.body,
+                          'Feedback should be included directly within tracking message')
 
     @users('user_sales_salesman')
     @mute_logger('odoo.addons.base.models')
diff --git a/addons/crm/wizard/crm_lead_lost.py b/addons/crm/wizard/crm_lead_lost.py
index 66b36754c6f..0db92f5a739 100644
--- a/addons/crm/wizard/crm_lead_lost.py
+++ b/addons/crm/wizard/crm_lead_lost.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo import fields, models
+from odoo import fields, models, _
 from odoo.tools.mail import is_html_empty
 
 
@@ -18,12 +18,12 @@ class CrmLeadLost(models.TransientModel):
     def action_lost_reason_apply(self):
         self.ensure_one()
         leads = self.env['crm.lead'].browse(self.env.context.get('active_ids'))
-        res = leads.action_set_lost(lost_reason_id=self.lost_reason_id.id)
         if not is_html_empty(self.lost_feedback):
-            leads._message_log_batch(
-                bodies=dict(
-                    (lead.id, self.lost_feedback)
-                    for lead in leads),
-                subject=False,
+            leads._track_set_log_message(
+                '<div style="margin-bottom: 4px;"><p>%s:</p>%s<br /></div>' % (
+                    _('Lost Comment'),
+                    self.lost_feedback
+                )
             )
+        res = leads.action_set_lost(lost_reason_id=self.lost_reason_id.id)
         return res
diff --git a/addons/crm/wizard/crm_lead_lost_views.xml b/addons/crm/wizard/crm_lead_lost_views.xml
index c6ca6b23aca..b046443d5e8 100644
--- a/addons/crm/wizard/crm_lead_lost_views.xml
+++ b/addons/crm/wizard/crm_lead_lost_views.xml
@@ -5,10 +5,10 @@
             <field name="model">crm.lead.lost</field>
             <field name="arch" type="xml">
                 <form string="Lost Reason">
-                    <group class="oe_title">
+                    <group>
                         <field name="lost_reason_id" options="{'no_create_edit': True}" />
                     </group>
-                    <field name="lost_feedback" class="oe_title" placeholder="What went wrong ?"/>
+                    <field name="lost_feedback" placeholder="What went wrong ?"/>
                     <footer>
                         <button name="action_lost_reason_apply" string="Submit" type="object" class="btn-primary" data-hotkey="q"/>
                         <button string="Cancel" class="btn-secondary" special="cancel" data-hotkey="z"/>
diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index 00d9bfd98ce..18b851e1b50 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -512,6 +512,16 @@ class MailThread(models.AbstractModel):
         # we have to flush() again in case we triggered some recomputations
         self.flush()
 
+    def _track_set_log_message(self, message):
+        """ Link tracking to a message logged as body, in addition to subtype
+        description (if set) and tracking values that make the core content of
+        tracking message. """
+        if not self._track_get_fields():
+            return
+        body_values = self.env.cr.precommit.data.setdefault(f'mail.tracking.message.{self._name}', {})
+        for id_ in self.ids:
+            body_values[id_] = message
+
     @tools.ormcache('self.env.uid', 'self.env.su')
     def _track_get_fields(self):
         """ Return the set of tracked fields names for the current model. """
@@ -558,6 +568,8 @@ class MailThread(models.AbstractModel):
             except MissingError:
                 continue
 
+        # find content to log as body
+        bodies = self.env.cr.precommit.data.pop(f'mail.tracking.message.{self._name}', {})
         for record in self:
             changes, tracking_value_ids = tracking.get(record.id, (None, None))
             if not changes:
@@ -572,9 +584,16 @@ class MailThread(models.AbstractModel):
                 if not subtype.exists():
                     _logger.debug('subtype "%s" not found' % subtype.name)
                     continue
-                record.message_post(subtype_id=subtype.id, tracking_value_ids=tracking_value_ids)
+                record.message_post(
+                    body=bodies.get(record.id) or '',
+                    subtype_id=subtype.id,
+                    tracking_value_ids=tracking_value_ids
+                )
             elif tracking_value_ids:
-                record._message_log(tracking_value_ids=tracking_value_ids)
+                record._message_log(
+                    body=bodies.get(record.id) or '',
+                    tracking_value_ids=tracking_value_ids
+                )
 
         return tracking