From 6344fc7d50a0184047be6caeacb2e09fb4bf768e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 8 Jun 2023 12:21:54 +0200 Subject: [PATCH] [REF] mail, various: fix and test 2many fields tracking PURPOSE Simplify 'mail.tracking.value' model and code. Remove unnecessary fields and computation. Make code easier to handle and more batch-enabled. SPECIFICATIONS In old times, 'account' and 'project' supported 2many fields tracking. Then it was moved directly into 'mail'. Thanks to precommit hooks and complete record access for relational fields, it is now possible to track 2many fields. In this commit we cleanup odoo/odoo@167944cc93a653b6b4aa53c888fbeffe3042463a that landed during work on this task and add some tests to be sure it is effectively covered. Task-3345979 (Mail: Simplify tracking model) Part-of: odoo/odoo#124182 --- addons/mail/models/mail_tracking_value.py | 16 ++--- addons/mail/tests/common.py | 2 + .../models/test_mail_corner_case_models.py | 34 +++++++++-- addons/test_mail/security/ir.model.access.csv | 2 + addons/test_mail/tests/test_message_track.py | 58 +++++++++++++++++++ 5 files changed, 100 insertions(+), 12 deletions(-) diff --git a/addons/mail/models/mail_tracking_value.py b/addons/mail/models/mail_tracking_value.py index 37f7341dd32..4c5632ad381 100644 --- a/addons/mail/models/mail_tracking_value.py +++ b/addons/mail/models/mail_tracking_value.py @@ -71,7 +71,7 @@ class MailTracking(models.Model): values = {'field': field.id, 'field_desc': col_info['string'], 'field_type': col_info['type'], 'tracking_sequence': tracking_sequence} - if col_info['type'] in ['integer', 'float', 'char', 'text', 'datetime', 'monetary']: + if col_info['type'] in {'integer', 'float', 'char', 'text', 'datetime', 'monetary'}: values.update({ f'old_value_{col_info["type"]}': initial_value, f'new_value_{col_info["type"]}': new_value @@ -95,15 +95,15 @@ class MailTracking(models.Model): }) elif col_info['type'] == 'many2one': values.update({ - 'old_value_integer': initial_value and initial_value.id or 0, - 'new_value_integer': new_value and new_value.id or 0, - 'old_value_char': initial_value and initial_value.sudo().display_name or '', - 'new_value_char': new_value and new_value.sudo().display_name or '' + 'old_value_integer': initial_value.id if initial_value else 0, + 'new_value_integer': new_value.id if new_value else 0, + 'old_value_char': initial_value.display_name if initial_value else '', + 'new_value_char': new_value.display_name if new_value else '' }) - elif col_info['type'] in ['many2many', 'one2many']: + elif col_info['type'] in {'one2many', 'many2many'}: values.update({ - 'old_value_char': initial_value and ', '.join(initial_value.mapped('display_name')) or '', - 'new_value_char': new_value and ', '.join(new_value.mapped('display_name')) or '' + 'old_value_char': ', '.join(initial_value.mapped('display_name')) if initial_value else '', + 'new_value_char': ', '.join(new_value.mapped('display_name')) if new_value else '', }) else: tracked = False diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index d8a39fad0b4..fbc0beac7f3 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -1144,6 +1144,8 @@ class MailCase(MockEmail): 'datetime': 'datetime', 'integer': 'integer', 'float': 'float', + 'many2many': 'char', + 'one2many': 'char', 'selection': 'char', 'text': 'text', } diff --git a/addons/test_mail/models/test_mail_corner_case_models.py b/addons/test_mail/models/test_mail_corner_case_models.py index e088f90d4df..4ed07eaa72a 100644 --- a/addons/test_mail/models/test_mail_corner_case_models.py +++ b/addons/test_mail/models/test_mail_corner_case_models.py @@ -92,6 +92,23 @@ class MailTestLang(models.Model): # TRACKING MODELS # ------------------------------------------------------------ +class MailTestTrackAllM2M(models.Model): + _name = 'mail.test.track.all.m2m' + _description = 'Sub-model: pseudo tags for tracking' + _inherit = ['mail.thread'] + + name = fields.Char('Name') + + +class MailTestTrackAllO2M(models.Model): + _name = 'mail.test.track.all.o2m' + _description = 'Sub-model: pseudo tags for tracking' + _inherit = ['mail.thread'] + + name = fields.Char('Name') + mail_track_all_id = fields.Many2one('mail.test.track.all') + + class MailTestTrackAll(models.Model): _name = 'mail.test.track.all' _description = 'Test tracking on all field types' @@ -106,13 +123,22 @@ class MailTestTrackAll(models.Model): float_field = fields.Float('Float', tracking=5) html_field = fields.Html('Html', tracking=6) integer_field = fields.Integer('Integer', tracking=7) - many2one_field_id = fields.Many2one('res.partner', string='Many2one', tracking=8) - monetary_field = fields.Monetary('Monetary', tracking=9) + many2many_field = fields.Many2many( + 'mail.test.track.all.m2m', string='Many2Many', + tracking=8) + many2one_field_id = fields.Many2one('res.partner', string='Many2one', tracking=9) + monetary_field = fields.Monetary('Monetary', tracking=10) + one2many_field = fields.One2many( + 'mail.test.track.all.o2m', 'mail_track_all_id', + string='One2Many', + tracking=11) selection_field = fields.Selection( string='Selection', selection=[('first', 'FIRST'), ('second', 'SECOND')], - tracking=10) - text_field = fields.Text('Text', tracking=11) + tracking=12) + text_field = fields.Text('Text', tracking=13) + + name = fields.Char('Name') class MailTestTrackCompute(models.Model): diff --git a/addons/test_mail/security/ir.model.access.csv b/addons/test_mail/security/ir.model.access.csv index 24fa71f25f9..28a9b72143d 100644 --- a/addons/test_mail/security/ir.model.access.csv +++ b/addons/test_mail/security/ir.model.access.csv @@ -46,6 +46,8 @@ access_mail_test_multi_company_with_activity_portal,mail.test.multi.company.with access_mail_test_nothread_user,mail.test.nothread.user,model_mail_test_nothread,base.group_user,1,1,1,1 access_mail_test_nothread_portal,mail.test.nothread.portal,model_mail_test_nothread,base.group_portal,1,0,0,0 access_mail_test_track_all,mail.test.track.all,model_mail_test_track_all,base.group_user,1,1,1,1 +access_mail_test_track_all_m2m,mail.test.track.all.m2m,model_mail_test_track_all_m2m,base.group_user,1,1,1,1 +access_mail_test_track_all_o2m,mail.test.track.all.o2m,model_mail_test_track_all_o2m,base.group_user,1,1,1,1 access_mail_test_track_compute,mail.test.track.compute,model_mail_test_track_compute,base.group_user,1,1,1,1 access_mail_test_track_monetary,mail.test.track.monetary,model_mail_test_track_monetary,base.group_user,1,1,1,1 access_mail_test_track_selection_portal,mail.test.track.selection.portal,model_mail_test_track_selection,base.group_portal,0,0,0,0 diff --git a/addons/test_mail/tests/test_message_track.py b/addons/test_mail/tests/test_message_track.py index 3e2e250c6be..3496bd64bd7 100644 --- a/addons/test_mail/tests/test_message_track.py +++ b/addons/test_mail/tests/test_message_track.py @@ -370,6 +370,64 @@ class TestTrackingInternals(MailCommon): 'phone': '0456001122', }) + @users('employee') + def test_mail_track_2many(self): + """ Check result of tracking one2many and many2many fields. Current + usage is to aggregate names into value_char fields. """ + # Create a record with an initially invalid selection value + test_tags = self.env['mail.test.track.all.m2m'].create([ + {'name': 'Tag1',}, + {'name': 'Tag2',}, + {'name': 'Tag3',}, + ]) + test_record = self.env['mail.test.track.all'].create({ + 'name': 'Test 2Many fields tracking', + }) + self.flush_tracking() + + # no tracked field, no tracking at create + last_message = test_record.message_ids[0] + self.assertFalse(last_message.tracking_value_ids) + + # update m2m + test_record.write({ + 'many2many_field': [(4, test_tags[0].id), (4, test_tags[1].id)], + }) + self.flush_tracking() + last_message = test_record.message_ids[0] + self.assertTracking( + last_message, + [('many2many_field', 'many2many', '', ', '.join(test_tags[:2].mapped('name')))] + ) + + # update m2m + o2m + test_record.write({ + 'many2many_field': [(3, test_tags[0].id), (4, test_tags[2].id)], + 'one2many_field': [ + (0, 0, {'name': 'Child1'}), + (0, 0, {'name': 'Child2'}), + (0, 0, {'name': 'Child3'}), + ], + }) + self.flush_tracking() + last_message = test_record.message_ids[0] + self.assertTracking( + last_message, + [ + ('many2many_field', 'many2many', ', '.join(test_tags[:2].mapped('name')), ', '.join((test_tags[1] + test_tags[2]).mapped('name'))), + ('one2many_field', 'one2many', '', ', '.join(('Child1', 'Child2', 'Child3'))), + ] + ) + + # remove from o2m + test_record.write({'one2many_field': [(3, test_record.one2many_field[0].id)]}) + self.flush_tracking() + last_message = test_record.message_ids[0] + self.assertTracking( + last_message, + [('one2many_field', 'one2many', ', '.join(('Child1', 'Child2', 'Child3')), ', '.join(('Child2', 'Child3')))] + ) + @users('employee') def test_mail_track_all_no2many(self): test_record = self.env['mail.test.track.all'].create({