[IMP] mail: add tracking on computed stored fields

Also delay the tracking until pre-commit, so that several updates on a
record generate a single tracking message.

7 files changed, 179 insertions, 27 deletions

diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index ae8d6ffb8d2..82309efe62d 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -27,6 +27,7 @@ from lxml import etree
 from werkzeug import urls
 
 from odoo import _, api, exceptions, fields, models, tools, registry, SUPERUSER_ID
+from odoo.exceptions import MissingError
 from odoo.osv import expression
 
 from odoo.tools import pycompat, ustr
@@ -257,7 +258,9 @@ class MailThread(models.AbstractModel):
             - log a creation message
         """
         if self._context.get('tracking_disable'):
-            return super(MailThread, self).create(vals_list)
+            threads = super(MailThread, self).create(vals_list)
+            threads._discard_tracking()
+            return threads
 
         threads = super(MailThread, self).create(vals_list)
         # subscribe uid unless asked not to
@@ -294,6 +297,7 @@ class MailThread(models.AbstractModel):
                 threads_no_subtype._message_log_batch(bodies={t.id: body for t in threads_no_subtype})
 
         # post track template if a tracked field changed
+        threads._discard_tracking()
         if not self._context.get('mail_notrack'):
             fnames = self._get_tracked_fields()
             for thread in threads:
@@ -310,14 +314,8 @@ class MailThread(models.AbstractModel):
         if self._context.get('tracking_disable'):
             return super(MailThread, self).write(values)
 
-        tracked_fields = None
         if not self._context.get('mail_notrack'):
-            tracked_fields = self._get_tracked_fields()
-        if tracked_fields:
-            initial_values = {
-                record.id: {name: record[name] for name in tracked_fields}
-                for record in self
-            }
+            self._prepare_tracking(self._fields)
 
         # Perform write
         result = super(MailThread, self).write(values)
@@ -325,20 +323,21 @@ class MailThread(models.AbstractModel):
         # update followers
         self._message_auto_subscribe(values)
 
-        # Perform the tracking
-        if tracked_fields:
-            context = clean_context(self._context)
-            tracking = self.with_context(context).message_track(tracked_fields, initial_values)
-            if any(change for rec_id, (change, tracking_value_ids) in tracking.items()):
-                (changes, tracking_value_ids) = tracking[self[0].id]
-                self._message_track_post_template(changes)
         return result
 
+    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._field_computed[field] if f.store)
+
+        return super()._compute_field_value(field)
+
     def unlink(self):
         """ Override unlink to delete messages and followers. This cannot be
         cascaded, because link is done through (res_model, res_id). """
         if not self:
             return True
+        # discard pending tracking
+        self._discard_tracking()
         self.env['mail.message'].search([('model', '=', self._name), ('res_id', 'in', self.ids), ('message_type', '!=', 'user_notification')]).unlink()
         res = super(MailThread, self).unlink()
         self.env['mail.followers'].sudo().search(
@@ -426,6 +425,49 @@ class MailThread(models.AbstractModel):
     # Technical methods / wrappers / tools
     # ------------------------------------------------------
 
+    def _prepare_tracking(self, fields):
+        """ Prepare the tracking of ``fields`` for ``self``.
+
+        :param fields: iterable of fields names to potentially track
+        """
+        fnames = self._get_tracked_fields().intersection(fields)
+        if not fnames:
+            return
+        func = self.browse()._finalize_tracking
+        [initial_values] = self.env.cr.precommit.add(func, dict)
+        for record in self:
+            if not record.id:
+                continue
+            values = initial_values.setdefault(record.id, {})
+            if values is not None:
+                for fname in fnames:
+                    values.setdefault(fname, record[fname])
+
+    def _discard_tracking(self):
+        """ Prevent any tracking of fields on ``self``. """
+        if not self._get_tracked_fields():
+            return
+        func = self.browse()._finalize_tracking
+        [initial_values] = self.env.cr.precommit.add(func, dict)
+        # disable tracking by setting initial values to None
+        for id_ in self.ids:
+            initial_values[id_] = None
+
+    def _finalize_tracking(self, initial_values):
+        """ Generate the tracking messages for the records that have been
+        prepared with ``_prepare_tracking``.
+        """
+        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()
+        context = clean_context(self._context)
+        tracking = records.with_context(context).message_track(fnames, initial_values)
+        for record in records:
+            changes, tracking_value_ids = tracking.get(record.id, (None, None))
+            record._message_track_post_template(changes)
+
     def with_lang(self):
         if not self._context.get("lang"):
             return self.with_context(lang=self.env.user.lang)
@@ -542,12 +584,12 @@ class MailThread(models.AbstractModel):
 
     @tools.ormcache()
     def _get_tracked_fields(self):
-        """ Return the name of the tracked fields for the current model. """
-        return [
+        """ Return the set of tracked fields names for the current model. """
+        return {
             name
             for name, field in self._fields.items()
             if getattr(field, 'tracking', None) or getattr(field, 'track_visibility', None)
-        ]
+        }
 
     def _creation_subtype(self):
         """ Give the subtypes triggered by the creation of a record
@@ -616,6 +658,8 @@ class MailThread(models.AbstractModel):
 
         # generate tracked_values data structure: {'col_name': {col_info, new_value, old_value}}
         for col_name, col_info in tracked_fields.items():
+            if col_name not in initial:
+                continue
             initial_value = initial[col_name]
             new_value = record[col_name]
 
@@ -648,6 +692,7 @@ class MailThread(models.AbstractModel):
         :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
@@ -655,10 +700,13 @@ class MailThread(models.AbstractModel):
         tracked_fields = self.fields_get(tracked_fields)
         tracking = dict()
         for record in self:
-            tracking[record.id] = record._message_track(tracked_fields, initial_values[record.id])
+            try:
+                tracking[record.id] = record._message_track(tracked_fields, initial_values[record.id])
+            except MissingError:
+                continue
 
         for record in self:
-            changes, tracking_value_ids = tracking[record.id]
+            changes, tracking_value_ids = tracking.get(record.id, (None, None))
             if not changes:
                 continue
 
diff --git a/addons/test_mail/models/test_mail_models.py b/addons/test_mail/models/test_mail_models.py
index e84c6d3ee5e..340bcbcb9c5 100644
--- a/addons/test_mail/models/test_mail_models.py
+++ b/addons/test_mail/models/test_mail_models.py
@@ -170,3 +170,14 @@ class MailTrackingModel(models.Model):
     field_0 = fields.Char(tracking=True)
     field_1 = fields.Char(tracking=True)
     field_2 = fields.Char(tracking=True)
+
+
+class MailCompute(models.Model):
+    _name = 'mail.test.compute'
+    _description = "Test model with several tracked computed fields"
+    _inherit = ['mail.thread']
+
+    partner_id = fields.Many2one('res.partner', tracking=True)
+    partner_name = fields.Char(related='partner_id.name', store=True, tracking=True)
+    partner_email = fields.Char(related='partner_id.email', store=True, tracking=True)
+    partner_phone = fields.Char(related='partner_id.phone', tracking=True)
diff --git a/addons/test_mail/security/ir.model.access.csv b/addons/test_mail/security/ir.model.access.csv
index 2753e2fbd72..95a4e84e98e 100644
--- a/addons/test_mail/security/ir.model.access.csv
+++ b/addons/test_mail/security/ir.model.access.csv
@@ -17,3 +17,4 @@ access_mail_test_cc_user,mail.test.cc.user,model_mail_test_cc,base.group_user,1,
 access_mail_test_multi_company_user,mail.test.multi.company.user,model_mail_test_multi_company,base.group_user,1,1,1,1
 access_mail_test_multi_company_portal,mail.test.multi.company.portal,model_mail_test_multi_company,base.group_portal,1,0,0,0
 access_mail_test_tracking_user,mail.test.tracking,model_mail_test_tracking,base.group_user,1,1,1,1
+access_mail_test_compute,mail.test.compute,model_mail_test_compute,base.group_user,1,1,1,1
diff --git a/addons/test_mail/tests/common.py b/addons/test_mail/tests/common.py
index d7e63584814..77cef2ce183 100644
--- a/addons/test_mail/tests/common.py
+++ b/addons/test_mail/tests/common.py
@@ -52,6 +52,11 @@ class TestMailCommon(common.SavepointCase, mail_common.MailCase):
         cls.email_template = cls.env['mail.template'].create(create_values)
         return cls.email_template
 
+    def flush_tracking(self):
+        """ Force the creation of tracking values. """
+        self.env['base'].flush()
+        self.cr.precommit()
+
 
 class TestMailMultiCompanyCommon(TestMailCommon):
 
diff --git a/addons/test_mail/tests/test_discuss.py b/addons/test_mail/tests/test_discuss.py
index e2bda7dc8d9..86b9a7de0cb 100644
--- a/addons/test_mail/tests/test_discuss.py
+++ b/addons/test_mail/tests/test_discuss.py
@@ -38,24 +38,29 @@ class TestChatterTweaks(TestMailCommon, TestRecipients):
     def test_chatter_mail_create_nolog(self):
         """ Test disable of automatic chatter message at create """
         rec = self.env['mail.test.simple'].with_user(self.user_employee).with_context({'mail_create_nolog': True}).create({'name': 'Test'})
+        self.flush_tracking()
         self.assertEqual(rec.message_ids, self.env['mail.message'])
 
         rec = self.env['mail.test.simple'].with_user(self.user_employee).with_context({'mail_create_nolog': False}).create({'name': 'Test'})
+        self.flush_tracking()
         self.assertEqual(len(rec.message_ids), 1)
 
     def test_chatter_mail_notrack(self):
         """ Test disable of automatic value tracking at create and write """
         rec = self.env['mail.test.track'].with_user(self.user_employee).create({'name': 'Test', 'user_id': self.user_employee.id})
+        self.flush_tracking()
         self.assertEqual(len(rec.message_ids), 1,
                          "A creation message without tracking values should have been posted")
         self.assertEqual(len(rec.message_ids.sudo().tracking_value_ids), 0,
                          "A creation message without tracking values should have been posted")
 
         rec.with_context({'mail_notrack': True}).write({'user_id': self.user_admin.id})
+        self.flush_tracking()
         self.assertEqual(len(rec.message_ids), 1,
                          "No new message should have been posted with mail_notrack key")
 
         rec.with_context({'mail_notrack': False}).write({'user_id': self.user_employee.id})
+        self.flush_tracking()
         self.assertEqual(len(rec.message_ids), 2,
                          "A tracking message should have been posted")
         self.assertEqual(len(rec.message_ids.sudo().mapped('tracking_value_ids')), 1,
@@ -64,16 +69,20 @@ class TestChatterTweaks(TestMailCommon, TestRecipients):
     def test_chatter_tracking_disable(self):
         """ Test disable of all chatter features at create and write """
         rec = self.env['mail.test.track'].with_user(self.user_employee).with_context({'tracking_disable': True}).create({'name': 'Test', 'user_id': self.user_employee.id})
+        self.flush_tracking()
         self.assertEqual(rec.sudo().message_ids, self.env['mail.message'])
         self.assertEqual(rec.sudo().mapped('message_ids.tracking_value_ids'), self.env['mail.tracking.value'])
 
         rec.write({'user_id': self.user_admin.id})
+        self.flush_tracking()
         self.assertEqual(rec.sudo().mapped('message_ids.tracking_value_ids'), self.env['mail.tracking.value'])
 
         rec.with_context({'tracking_disable': False}).write({'user_id': self.user_employee.id})
+        self.flush_tracking()
         self.assertEqual(len(rec.sudo().mapped('message_ids.tracking_value_ids')), 1)
 
         rec = self.env['mail.test.track'].with_user(self.user_employee).with_context({'tracking_disable': False}).create({'name': 'Test', 'user_id': self.user_employee.id})
+        self.flush_tracking()
         self.assertEqual(len(rec.sudo().message_ids), 1,
                          "Creation message without tracking values should have been posted")
         self.assertEqual(len(rec.sudo().mapped('message_ids.tracking_value_ids')), 0,
diff --git a/addons/test_mail/tests/test_message_track.py b/addons/test_mail/tests/test_message_track.py
index 1c5d19aded4..20ee3d74c4b 100644
--- a/addons/test_mail/tests/test_message_track.py
+++ b/addons/test_mail/tests/test_message_track.py
@@ -14,6 +14,7 @@ class TestTracking(TestMailCommon):
         record = self.env['mail.test.full'].with_user(self.user_employee).with_context(self._test_context).create({
             'name': 'Test',
         })
+        self.flush_tracking()
         self.record = record.with_context(mail_notrack=False)
 
     def test_message_track_no_tracking(self):
@@ -22,6 +23,7 @@ class TestTracking(TestMailCommon):
             'name': 'Tracking or not',
             'count': 32,
         })
+        self.flush_tracking()
         self.assertEqual(self.record.message_ids, self.env['mail.message'])
 
     def test_message_track_no_subtype(self):
@@ -32,6 +34,7 @@ class TestTracking(TestMailCommon):
                 'name': 'Test2',
                 'customer_id': customer.id,
             })
+            self.flush_tracking()
 
         # one new message containing tracking; without subtype linked to tracking, a note is generated
         self.assertEqual(len(self.record.message_ids), 1)
@@ -61,6 +64,7 @@ class TestTracking(TestMailCommon):
             'email_from': 'noone@example.com',
             'umbrella_id': umbrella.id,
         })
+        self.flush_tracking()
         # one new message containing tracking; subtype linked to tracking
         self.assertEqual(len(self.record.message_ids), 1)
         self.assertEqual(self.record.message_ids.subtype_id, self.env.ref('test_mail.st_mail_test_full_umbrella_upd'))
@@ -85,6 +89,7 @@ class TestTracking(TestMailCommon):
                 'name': 'Test2',
                 'customer_id': self.user_admin.partner_id.id,
             })
+            self.flush_tracking()
 
         self.assertEqual(len(self.record.message_ids), 2, 'should have 2 new messages: one for tracking, one for template')
 
@@ -113,6 +118,7 @@ class TestTracking(TestMailCommon):
                 'customer_id': self.user_admin.partner_id.id,
                 'mail_template': self.env.ref('test_mail.mail_test_full_tracking_tpl').id,
             })
+            self.flush_tracking()
 
         self.assertEqual(len(record.message_ids), 1, 'should have 1 new messages for template')
         # one new message containing the template linked to tracking
@@ -129,6 +135,7 @@ class TestTracking(TestMailCommon):
             'user_id': self.user_admin.id,
             'umbrella_id': self.env['mail.test'].with_context(mail_create_nosubscribe=True).create({'name': 'Umbrella'}).id
         })
+        self.flush_tracking()
         self.assertEqual(len(self.record.message_ids), 1, 'should have 1 tracking message')
 
         tracking_values = self.env['mail.tracking.value'].search([('mail_message_id', '=', self.record.message_ids.id)])
@@ -143,6 +150,7 @@ class TestTracking(TestMailCommon):
         field.groups = 'base.group_erp_manager'
 
         self.record.sudo().write({'email_from': 'X'})
+        self.flush_tracking()
 
         msg_emp = self.record.message_ids.message_format()
         msg_sudo = self.record.sudo().message_ids.message_format()
@@ -156,6 +164,7 @@ class TestTracking(TestMailCommon):
         field.groups = 'base.group_erp_manager'
 
         self.record.sudo().write({'email_from': 'X'})
+        self.flush_tracking()
 
         msg_emp = self.record._notify_prepare_template_context(self.record.message_ids, {})
         msg_sudo = self.record.sudo()._notify_prepare_template_context(self.record.message_ids, {})
@@ -165,7 +174,7 @@ class TestTracking(TestMailCommon):
     def test_unlinked_field(self):
         record_sudo = self.record.sudo()
         record_sudo.write({'email_from': 'X'})  # create a tracking value
-        record_sudo.flush()
+        self.flush_tracking()
         self.assertEqual(len(record_sudo.message_ids.tracking_value_ids), 1)
         ir_model_field = self.env['ir.model.fields'].search([
             ('model', '=', 'mail.test.full'),
@@ -202,6 +211,7 @@ class TestTracking(TestMailCommon):
                 'company_id': company1.id,
                 'user_id': self.env.user.id, # trigger track template
             })
+            self.flush_tracking()
 
         new_partner = Partner.search([('email', '=', email_new_partner)])
         self.assertTrue(new_partner)
@@ -255,3 +265,72 @@ class TestTracking(TestMailCommon):
             'description': 'Zizizatestmaildescription',
         })
         test_mail_record.with_context(default_parent_id=2147483647).write({'name': magic_code})
+
+    def test_message_track_multiple(self):
+        """ check that multiple updates generate a single tracking message """
+        umbrella = self.env['mail.test'].with_context(mail_create_nosubscribe=True).create({'name': 'Umbrella'})
+        self.record.name = 'Zboub'
+        self.record.customer_id = self.user_admin.partner_id
+        self.record.user_id = self.user_admin
+        self.record.umbrella_id = umbrella
+        self.flush_tracking()
+
+        # should have a single message with all tracked fields
+        self.assertEqual(len(self.record.message_ids), 1, 'should have 1 tracking message')
+        self.assertTracking(self.record.message_ids[0], [
+            ('customer_id', 'many2one', False, self.user_admin.partner_id),
+            ('user_id', 'many2one', False, self.user_admin),
+            ('umbrella_id', 'many2one', False, umbrella),
+        ])
+
+    def test_tracked_compute(self):
+        # no tracking at creation
+        record = self.env['mail.test.compute'].create({})
+        self.flush_tracking()
+        self.assertEqual(len(record.message_ids), 1)
+        self.assertEqual(len(record.message_ids[0].tracking_value_ids), 0)
+
+        # assign partner_id: one tracking message for the modified field and all
+        # the stored and non-stored computed fields on the record
+        partner = self.env['res.partner'].create({
+            'name': 'Foo',
+            'email': 'foo@example.com',
+            'phone': '1234567890',
+        })
+        record.partner_id = partner
+        self.flush_tracking()
+        self.assertEqual(len(record.message_ids), 2)
+        self.assertEqual(len(record.message_ids[0].tracking_value_ids), 4)
+        self.assertTracking(record.message_ids[0], [
+            ('partner_id', 'many2one', False, partner),
+            ('partner_name', 'char', False, 'Foo'),
+            ('partner_email', 'char', False, 'foo@example.com'),
+            ('partner_phone', 'char', False, '1234567890'),
+        ])
+
+        # modify partner: one tracking message for the only recomputed field
+        partner.write({'name': 'Fool'})
+        self.flush_tracking()
+        self.assertEqual(len(record.message_ids), 3)
+        self.assertEqual(len(record.message_ids[0].tracking_value_ids), 1)
+        self.assertTracking(record.message_ids[0], [
+            ('partner_name', 'char', 'Foo', 'Fool'),
+        ])
+
+        # modify partner: one tracking message for both stored computed fields;
+        # the non-stored computed fields have no tracking
+        partner.write({
+            'name': 'Bar',
+            'email': 'bar@example.com',
+            'phone': '0987654321',
+        })
+        # force recomputation of 'partner_phone' to make sure it does not
+        # generate tracking values
+        self.assertEqual(record.partner_phone, '0987654321')
+        self.flush_tracking()
+        self.assertEqual(len(record.message_ids), 4)
+        self.assertEqual(len(record.message_ids[0].tracking_value_ids), 2)
+        self.assertTracking(record.message_ids[0], [
+            ('partner_name', 'char', 'Fool', 'Bar'),
+            ('partner_email', 'char', 'foo@example.com', 'bar@example.com'),
+        ])
diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py
index 79058b69b6a..d9a722ed63b 100644
--- a/addons/test_mail/tests/test_performance.py
+++ b/addons/test_mail/tests/test_performance.py
@@ -560,7 +560,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
         })
         rec1 = rec.with_context(active_test=False)      # to see inactive records
         self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id)
-        with self.assertQueryCount(__system__=39, emp=40):
+        with self.assertQueryCount(__system__=40, emp=41):
             rec.write({'user_id': self.user_portal.id})
         self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id)
         # write tracking message
@@ -580,7 +580,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
         customer_id = self.customer.id
         user_id = self.user_portal.id
 
-        with self.assertQueryCount(__system__=123, emp=124):
+        with self.assertQueryCount(__system__=121, emp=122):
             rec = self.env['mail.test.full'].create({
                 'name': 'Test',
                 'umbrella_id': umbrella_id,
@@ -609,7 +609,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
         rec1 = rec.with_context(active_test=False)      # to see inactive records
         self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id)
         self.assertEqual(len(rec1.message_ids), 1)
-        with self.assertQueryCount(__system__=87, emp=88):
+        with self.assertQueryCount(__system__=87, emp=87):
             rec.write({
                 'name': 'Test2',
                 'umbrella_id': self.umbrella.id,
@@ -646,14 +646,13 @@ class TestMailComplexPerformance(BaseMailPerformance):
         rec1 = rec.with_context(active_test=False)      # to see inactive records
         self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id)
 
-        with self.assertQueryCount(__system__=95, emp=96):
+        with self.assertQueryCount(__system__=96, emp=96):
             rec.write({
                 'name': 'Test2',
                 'umbrella_id': umbrella_id,
                 'customer_id': customer_id,
                 })
 
-
         self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id)
         # write tracking message
         self.assertEqual(rec1.message_ids[0].subtype_id, self.env.ref('test_mail.st_mail_test_full_umbrella_upd'))
@@ -680,7 +679,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
         rec1 = rec.with_context(active_test=False)      # to see inactive records
         self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id)
 
-        with self.assertQueryCount(__system__=34, emp=36):
+        with self.assertQueryCount(__system__=35, emp=36):
             rec.write({
                 'name': 'Test2',
                 'customer_id': customer_id,