From c99de4551583e801ecc6669ac456c4f7e2eef1da Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 19 Jan 2018 11:52:10 +0100 Subject: [PATCH] [IMP] chatter: improve chatter readability Only messages as comments and email template should keep a white backround since they are not internal. It is also enough to display fields that changed on subtypes (update message) since emails will be reworked soon and more information will be given there. The support of 'always' track can be removed. (is now considered as a simple tracked field) The use of self.env.ref('mail.mt_comment').id add some query and the performance tests needs to be update since message_format is called by notify_by_chat. This code will be refactor later --- addons/im_livechat/static/src/js/im_livechat.js | 1 + addons/mail/models/mail_message.py | 7 +++++-- addons/mail/models/mail_thread.py | 17 ++--------------- addons/mail/static/src/js/chat_manager.js | 1 + addons/mail/static/src/less/thread.less | 11 ++++++----- addons/mail/static/src/xml/thread.xml | 2 +- addons/mail/static/tests/chat_window_test.js | 1 + addons/mail/static/tests/chatter_tests.js | 5 +++++ addons/test_mail/tests/test_message_track.py | 12 +++++------- addons/test_mail/tests/test_performance.py | 14 +++++++------- 10 files changed, 34 insertions(+), 37 deletions(-) diff --git a/addons/im_livechat/static/src/js/im_livechat.js b/addons/im_livechat/static/src/js/im_livechat.js index 049e88cb23c..1cda8ee0cdc 100644 --- a/addons/im_livechat/static/src/js/im_livechat.js +++ b/addons/im_livechat/static/src/js/im_livechat.js @@ -228,6 +228,7 @@ var LivechatButton = Widget.extend({ date: moment(time.str_to_datetime(data.date)), is_needaction: false, is_note: data.is_note, + is_discussion: data.is_discussion, customer_email_data: [] }; diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 0f3665c4acb..8fa05876f79 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -404,6 +404,7 @@ class Message(models.Model): 'id': 59, 'subject': False 'is_note': True # only if the subtype is internal + 'is_discussion': False # only if the message is a discussion (subtype == discussion) } """ message_values = self.read([ @@ -417,13 +418,15 @@ class Message(models.Model): message_tree = dict((m.id, m) for m in self.sudo()) self._message_read_dict_postprocess(message_values, message_tree) - # add subtype data (is_note flag, subtype_description). Do it as sudo + # add subtype data (is_note flag, is_discussion flag , subtype_description). Do it as sudo # because portal / public may have to look for internal subtypes subtype_ids = [msg['subtype_id'][0] for msg in message_values if msg['subtype_id']] - subtypes = self.env['mail.message.subtype'].sudo().browse(subtype_ids).read(['internal', 'description']) + subtypes = self.env['mail.message.subtype'].sudo().browse(subtype_ids).read(['internal', 'description','id']) subtypes_dict = dict((subtype['id'], subtype) for subtype in subtypes) + xml_comment_id = self.env.ref('mail.mt_comment').id for message in message_values: message['is_note'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['internal'] + message['is_discussion'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['id'] == xml_comment_id message['subtype_description'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['description'] if message['model'] and self.env[message['model']]._original_module: message['module_icon'] = modules.module.get_module_icon(self.env[message['model']]._original_module) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index dd2c40965f0..9a2f815fe71 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -461,8 +461,7 @@ class MailThread(models.AbstractModel): def _get_tracked_fields(self, updated_fields): """ Return a structure of tracked fields for the current model. :param list updated_fields: modified field names - :return dict: a dict mapping field name to description, containing - always tracked fields and modified on_change fields + :return dict: a dict mapping field name to description, containing on_change fields """ tracked_fields = [] for name, field in self._fields.items(): @@ -521,14 +520,11 @@ class MailThread(models.AbstractModel): - a set of updated column names - a list of changes (initial value, new value, column name, column info) """ self.ensure_one() - changes = set() # contains always and onchange tracked fields that changed - displays = set() # contains always tracked field that did not change but displayed for information + changes = set() # contains onchange tracked fields that changed tracking_value_ids = [] - display_values_ids = [] # generate tracked_values data structure: {'col_name': {col_info, new_value, old_value}} for col_name, col_info in tracked_fields.items(): - track_visibility = getattr(self._fields[col_name], 'track_visibility', 'onchange') initial_value = initial[col_name] new_value = getattr(self, col_name) @@ -539,15 +535,6 @@ class MailThread(models.AbstractModel): if col_name in tracked_fields: changes.add(col_name) - # 'always' tracked fields in separate variable; added if other changes - elif new_value == initial_value and track_visibility == 'always' and col_name in tracked_fields: - tracking = self.env['mail.tracking.value'].create_tracking_values(initial_value, initial_value, col_name, col_info) - if tracking: - display_values_ids.append([0, 0, tracking]) - displays.add(col_name) - - if changes and displays: - tracking_value_ids = display_values_ids + tracking_value_ids return changes, tracking_value_ids diff --git a/addons/mail/static/src/js/chat_manager.js b/addons/mail/static/src/js/chat_manager.js index 9d1b9cddb70..35d16915341 100644 --- a/addons/mail/static/src/js/chat_manager.js +++ b/addons/mail/static/src/js/chat_manager.js @@ -130,6 +130,7 @@ function make_message (data) { subtype_description: data.subtype_description, is_author: data.author_id && data.author_id[0] === session.partner_id, is_note: data.is_note, + is_discussion: data.is_discussion, is_system_notification: (data.message_type === 'notification' && data.model === 'mail.channel') || data.info === 'transient_message', attachment_ids: data.attachment_ids || [], diff --git a/addons/mail/static/src/less/thread.less b/addons/mail/static/src/less/thread.less index 3683ecd9bbe..8d8bc761829 100644 --- a/addons/mail/static/src/less/thread.less +++ b/addons/mail/static/src/less/thread.less @@ -47,6 +47,11 @@ padding: 4px @odoo-horizontal-padding; margin-bottom: 0px; + &.o_mail_not_discussion { + background-color: @mail-thread-note; + border-bottom: 1px solid @gray-lighter-dark; + } + .o_thread_message_sidebar { .o-flex(0, 0, @mail-thread-avatar-size); margin-right: 10px; @@ -109,11 +114,7 @@ text-align: justify; } - &.o_mail_note { - background-color: @mail-thread-note; - padding-left: @grid-gutter-width*0.3; - border-bottom: 1px solid @gray-lighter-dark; - } + .o_mail_subject { font-style: italic; diff --git a/addons/mail/static/src/xml/thread.xml b/addons/mail/static/src/xml/thread.xml index 7806029557b..611b3a3997a 100644 --- a/addons/mail/static/src/xml/thread.xml +++ b/addons/mail/static/src/xml/thread.xml @@ -139,7 +139,7 @@ -
+
message with onchange + always tracked values """ + """ Update some tracked fields not linked to some subtype -> message with onchange """ customer = self.env['res.partner'].create({'name': 'Customer', 'email': 'cust@example.com'}) self.record.write({ 'name': 'Test2', @@ -59,12 +59,11 @@ class TestTracking(common.BaseFunctionalTest, common.MockEmails): # verify tracked value self.assertTracking( self.record.message_ids, - [('email_from', 'char', False, False), # always tracked field - ('customer_id', 'many2one', False, customer) # onchange tracked field + [('customer_id', 'many2one', False, customer) # onchange tracked field ]) def test_message_track_subtype(self): - """ Update some tracked fields linked to some subtype -> message with onchange + always tracked values """ + """ Update some tracked fields linked to some subtype -> message with onchange """ self.record.message_subscribe( partner_ids=[self.user_admin.partner_id.id], subtype_ids=[self.env.ref('test_mail.st_mail_test_full_umbrella_upd').id] @@ -87,12 +86,11 @@ class TestTracking(common.BaseFunctionalTest, common.MockEmails): # verify tracked value self.assertTracking( self.record.message_ids, - [('email_from', 'char', False, 'noone@example.com'), # always tracked field - ('umbrella_id', 'many2one', False, umbrella) # onchange tracked field + [('umbrella_id', 'many2one', False, umbrella) # onchange tracked field ]) def test_message_track_template(self): - """ Update some tracked fields linked to some template -> message with onchange + always tracked values """ + """ Update some tracked fields linked to some template -> message with onchange """ self.record.write({'mail_template': self.env.ref('test_mail.mail_test_full_tracking_tpl').id}) self.assertEqual(self.record.message_ids, self.env['mail.message']) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index d4ff7fca349..c14769b09c1 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -77,7 +77,7 @@ class TestMailPerformance(TransactionCase): 'partner_id': self.env.ref('base.res_partner_12').id, }) - with self.assertQueryCount(admin=32, demo=44): # test_mail only: 30 - 41 + with self.assertQueryCount(admin=33, demo=45): # test_mail only: 30 - 41 record.track = 'X' @users('admin', 'demo') @@ -93,13 +93,13 @@ class TestMailPerformance(TransactionCase): @warmup def test_create_mail_with_tracking(self): """ Create records inheriting from 'mail.thread' (with field tracking). """ - with self.assertQueryCount(admin=62, demo=84): # test_mail only: 58 - 78 + with self.assertQueryCount(admin=64, demo=86): # test_mail only: 58 - 78 self.env['test_performance.mail'].create({'name': 'X'}) @users('admin', 'emp') @warmup def test_create_mail_simple(self): - with self.assertQueryCount(admin=32, emp=42): # test_mail only: 30 - 39 + with self.assertQueryCount(admin=33, emp=43): # test_mail only: 30 - 39 self.env['mail.test.simple'].create({'name': 'Test'}) @users('admin', 'emp') @@ -137,7 +137,7 @@ class TestAdvMailPerformance(TransactionCase): def test_adv_activity(self): model = self.env['mail.test.activity'] - with self.assertQueryCount(admin=35, emp=45): # test_mail only: 33 - 42 + with self.assertQueryCount(admin=36, emp=46): # test_mail only: 33 - 42 model.create({'name': 'Test'}) @users('admin', 'emp') @@ -149,7 +149,7 @@ class TestAdvMailPerformance(TransactionCase): 'default_res_model': 'mail.test.activity', }) - with self.assertQueryCount(admin=47, emp=52): # com runbot 46 - 51 // test_mail only: 35 - 40 + with self.assertQueryCount(admin=47, emp=53): # com runbot 46 - 51 // test_mail only: 35 - 40 model.create({ 'summary': 'Test Activity', 'res_id': record.id, @@ -276,7 +276,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(admin=347, emp=420): # test_mail only: 330 - 399 + with self.assertQueryCount(admin=348, emp=421): # test_mail only: 330 - 399 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -315,7 +315,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=238, emp=278): # test_mail only: 232 - 271 + with self.assertQueryCount(admin=239, emp=279): # test_mail only: 232 - 271 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id,