diff --git a/addons/mail/models/mail_tracking_value.py b/addons/mail/models/mail_tracking_value.py index 2b664e16f87..17dbc569363 100644 --- a/addons/mail/models/mail_tracking_value.py +++ b/addons/mail/models/mail_tracking_value.py @@ -10,7 +10,7 @@ class MailTracking(models.Model): _name = 'mail.tracking.value' _description = 'Mail Tracking Value' _rec_name = 'field' - _order = 'tracking_sequence asc' + _order = 'id DESC' field = fields.Many2one('ir.model.fields', required=True, readonly=True, index=True, ondelete='cascade') field_desc = fields.Char('Field Description', required=True, readonly=True) @@ -36,8 +36,6 @@ class MailTracking(models.Model): mail_message_id = fields.Many2one('mail.message', 'Message ID', required=True, index=True, ondelete='cascade') - tracking_sequence = fields.Integer('Tracking field sequence', readonly=True, default=100) - @api.depends('mail_message_id', 'field') def _compute_field_groups(self): for tracking in self: @@ -46,7 +44,7 @@ class MailTracking(models.Model): tracking.field_groups = field.groups if field else 'base.group_system' @api.model - def _create_tracking_values(self, initial_value, new_value, col_name, col_info, tracking_sequence, record): + def _create_tracking_values(self, initial_value, new_value, col_name, col_info, record): """ Prepare values to create a mail.tracking.value. It prepares old and new value according to the field type. @@ -56,8 +54,6 @@ class MailTracking(models.Model): date, datetime, ...; :param str col_name: technical field name, column name (e.g. 'user_id); :param dict col_info: result of fields_get(col_name); - :param int tracking_sequence: sequence used for ordering tracking - value display; :param record: record on which tracking is performed, used for related computation e.g. finding currency of monetary fields; @@ -67,7 +63,7 @@ class MailTracking(models.Model): if not field: raise ValueError(f'Unknown field {col_name} on model {record._name}') - values = {'field': field.id, 'field_desc': col_info['string'], 'field_type': col_info['type'], 'tracking_sequence': tracking_sequence} + values = {'field': field.id, 'field_desc': col_info['string'], 'field_type': col_info['type']} if col_info['type'] in {'integer', 'float', 'char', 'text', 'datetime', 'monetary'}: values.update({ @@ -110,11 +106,21 @@ class MailTracking(models.Model): def _tracking_value_format(self): """ Return structure and formatted data structure to be used by chatter - to display tracking values. + to display tracking values. Order it according to asked display, aka + ascending sequence (and field name). :return list: for each tracking value in self, their formatted display values given as a dict; """ + if not self: + return [] + field_models = self.field.mapped('model') + if len(set(field_models)) != 1: + raise ValueError('All tracking value should belong to the same model.') + TrackedModel = self.env[field_models[0]] + tracked_fields = TrackedModel.fields_get(self.field.mapped('name'), attributes={'string', 'type'}) + fields_sequence_map = dict(TrackedModel._mail_track_order_fields(tracked_fields)) + formatted = [] for tracking in self: formatted.append({ @@ -131,6 +137,10 @@ class MailTracking(models.Model): 'value': tracking._format_display_value(new=False)[0], }, }) + formatted.sort( + key=lambda info: (fields_sequence_map[info['fieldName']], info['fieldName']), + reverse=False, + ) return formatted def _format_display_value(self, new=True): diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index 1b38fcc2693..fe6864fa3a6 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -81,39 +81,60 @@ class BaseModel(models.AbstractModel): fields; :return: a tuple (changes, tracking_value_ids) where - changes: set of updated column names; + changes: set of updated column names; contains onchange tracked fields + that changed; tracking_value_ids: a list of ORM (0, 0, values) commands to create ``mail.tracking.value`` records; Override this method on a specific model to implement model-specific behavior. Also consider inheriting from ``mail.thread``. """ self.ensure_one() - changes = set() # contains onchange tracked fields that changed + updated = set() tracking_value_ids = [] - # generate tracked_values data structure: {'col_name': {col_info, new_value, old_value}} - for col_name, col_info in tracked_fields.items(): + fields_track_info = self._mail_track_order_fields(tracked_fields) + for col_name, _sequence in fields_track_info: if col_name not in initial_values: continue - initial_value = initial_values[col_name] - new_value = self[col_name] + initial_value, new_value = initial_values[col_name], self[col_name] + if new_value == initial_value or (not new_value and not initial_value): # because browse null != False + continue - if new_value != initial_value and (new_value or initial_value): # because browse null != False - tracking_sequence = getattr(self._fields[col_name], 'tracking', - getattr(self._fields[col_name], 'track_sequence', 100)) # backward compatibility with old parameter name - if tracking_sequence is True: - tracking_sequence = 100 - tracking = self.env['mail.tracking.value']._create_tracking_values( + updated.add(col_name) + tracking_value_ids.append( + [0, 0, self.env['mail.tracking.value']._create_tracking_values( initial_value, new_value, - col_name, col_info, - tracking_sequence, + col_name, tracked_fields[col_name], self - ) - if tracking: - tracking_value_ids.append([0, 0, tracking]) - changes.add(col_name) + )]) - return changes, tracking_value_ids + return updated, tracking_value_ids + + def _mail_track_order_fields(self, tracked_fields): + """ Order tracking, based on sequence found on field definition. When + having several identical sequences, field name is used. """ + fields_track_info = [ + (col_name, self._mail_track_get_field_sequence(col_name)) + for col_name in tracked_fields.keys() + ] + # sorting: sequence ASC, name ASC (higher sequence -> displayed last, then + # order by name). Model order being id DESC (aka: first insert -> last + # displayed) insert should be done by descending sequence then descending + # name. + fields_track_info.sort(key=lambda item: (item[1], item[0]), reverse=True) + return fields_track_info + + def _mail_track_get_field_sequence(self, fname): + """ Find tracking sequence of a given field, given their name. Current + parameter 'tracking' should be an integer, but attributes with True + are still supported; old naming 'track_sequence' also. """ + sequence = getattr( + self._fields[fname], 'tracking', + getattr(self._fields[fname], 'track_sequence', 100) + ) + if sequence is True: + sequence = 100 + return sequence def _message_get_default_recipients(self): """ Generic implementation for finding default recipient to mail on diff --git a/addons/mail/views/mail_tracking_value_views.xml b/addons/mail/views/mail_tracking_value_views.xml index 793023d1092..1d6cb2769cc 100644 --- a/addons/mail/views/mail_tracking_value_views.xml +++ b/addons/mail/views/mail_tracking_value_views.xml @@ -34,7 +34,6 @@ - diff --git a/addons/test_mail/tests/test_message_track.py b/addons/test_mail/tests/test_message_track.py index c017f735df8..783d70b4497 100644 --- a/addons/test_mail/tests/test_message_track.py +++ b/addons/test_mail/tests/test_message_track.py @@ -647,7 +647,6 @@ class TestTrackingInternals(MailCommon): self.env['mail.tracking.value']._create_tracking_values( '', 'Test', 'not_existing_field', {'string': 'Test', 'type': 'char'}, - 0, test_record, ) @@ -656,30 +655,70 @@ class TestTrackingInternals(MailCommon): self.env['mail.tracking.value']._create_tracking_values( '', '

Html

', 'html_field', {'string': 'HTML', 'type': 'html'}, - 0, test_record, ) @users('employee') def test_track_sequence(self): - """ Update some tracked fields and check that the mail.tracking.value are ordered according to their tracking_sequence""" + """ Update some tracked fields and check that the mail.tracking.value + are ordered according to their tracking_sequence """ record = self.record.with_env(self.env) self.assertEqual(len(record.message_ids), 1) + # order: user_id -> 1, customer_id -> 2, container_id -> True -> 100, email_from -> True -> 100 + ordered_fnames = ['user_id', 'customer_id', 'container_id', 'email_from'] + + # Update tracked fields, should generate tracking values correctly ordered record.write({ - 'name': 'Zboub', + 'container_id': self.env['mail.test.container'].with_context(mail_create_nosubscribe=True).create({'name': 'Container'}).id, 'customer_id': self.user_admin.partner_id.id, + 'email_from': 'new.from@test.example.com', + 'name': 'Zboub', 'user_id': self.user_admin.id, - 'container_id': self.env['mail.test.container'].with_context(mail_create_nosubscribe=True).create({'name': 'Container'}).id }) self.flush_tracking() self.assertEqual(len(record.message_ids), 2, 'should have 1 new tracking message') - tracking_values = self.env['mail.tracking.value'].sudo().search( [('mail_message_id', '=', record.message_ids[0].id)] ) - self.assertEqual(tracking_values[0].tracking_sequence, 1) - self.assertEqual(tracking_values[1].tracking_sequence, 2) - self.assertEqual(tracking_values[2].tracking_sequence, 100) + self.assertEqual( + tracking_values.field.mapped('name'), + ordered_fnames, + 'Track: order, based on ID DESC, should follow tracking sequence (or name) on field' + ) + + # Manually create trackings, format should be the fallback to reorder them + new_msg = record.message_post( + body='Manual Hack of tracking', + subtype_xmlid='mail.mt_note', + ) + custom_order_fnames = ['container_id', 'customer_id', 'email_from', 'user_id'] + field_ids = [ + self.env['ir.model.fields']._get(record._name, fname).id + for fname in custom_order_fnames + ] + self.env['mail.tracking.value'].sudo().create([ + { + 'field': field_id, + 'mail_message_id': new_msg.id, + 'old_value_char': 'unimportant', + 'new_value_char': 'unimportant', + } + for field_id in field_ids + ]) + tracking_values = self.env['mail.tracking.value'].sudo().search( + [('mail_message_id', '=', record.message_ids[0].id)] + ) + self.assertEqual( + tracking_values.field.mapped('name'), + list(reversed(custom_order_fnames)), + 'Tracking model: order, based on ID DESC, following reverted insertion' + ) + tracking_formatted = tracking_values._tracking_value_format() + self.assertEqual( + [t['fieldName'] for t in tracking_formatted], + ordered_fnames, + 'Track: formatted order is correctly based on field sequence definition' + ) @users('employee') def test_unlinked_field(self):