From a013e6e62f09aeb4927315ca576fe0a48490f886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 22 Oct 2012 18:46:38 +0200 Subject: [PATCH] [IMP] [WIP] mail: expandables now limited to one level (using a child_of based domain); simplified code. message_read updated to fetch the oldest possible ancestor, not all direct parents. Updated some tests. Still WIP, will be continued tomorrow. bzr revid: tde@openerp.com-20121022164638-gqe5af1uxvl5h043 --- addons/mail/mail_followers.py | 1 + addons/mail/mail_message.py | 111 +++++++++++++++++---------------- addons/mail/tests/test_mail.py | 79 +++++++++++++++++------ 3 files changed, 118 insertions(+), 73 deletions(-) diff --git a/addons/mail/mail_followers.py b/addons/mail/mail_followers.py index 35603d2d720..52d26ebb51b 100644 --- a/addons/mail/mail_followers.py +++ b/addons/mail/mail_followers.py @@ -85,6 +85,7 @@ class mail_notification(osv.Model): def set_message_read(self, cr, uid, msg_ids, read=None, context=None): """ TDE note: add a comment, verify method calls, because js seems obfuscated. """ + # TDE note: child_of to set unread ? user_pid = self.pool.get('res.users').read(cr, uid, uid, ['partner_id'], context=context)['partner_id'][0] notif_ids = self.search(cr, uid, [('partner_id', '=', user_pid), ('message_id', 'in', msg_ids)], context=context) diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py index 19178a5eab7..1a650e8f67d 100644 --- a/addons/mail/mail_message.py +++ b/addons/mail/mail_message.py @@ -198,19 +198,29 @@ class mail_message(osv.Model): :param dict message: read result of a mail.message """ + is_author = False + if message['author_id']: + is_author = message['author_id'][0] == self.pool.get('res.users').read(cr, uid, uid, ['partner_id'], context=None)['partner_id'][0] + has_voted = False - if uid in message['vote_user_ids']: + if uid in message.get('vote_user_ids'): has_voted = True is_favorite = False - if uid in message['favorite_user_ids']: + if uid in message.get('favorite_user_ids'): is_favorite = True + is_private = False + if message.get('model') and message.get('res_id'): + is_private = True + try: attachment_ids = [{'id': attach[0], 'name': attach[1]} for attach in self.pool.get('ir.attachment').name_get(cr, uid, message['attachment_ids'], context=context)] except (orm.except_orm, osv.except_osv): attachment_ids = [] + # TDE note: should we send partner_ids ? + # TDE note: shouldn't we separated followers and other partners ? costly to compute maybe , try: partner_ids = self.pool.get('res.partner').name_get(cr, uid, message['partner_ids'], context=context) except (orm.except_orm, osv.except_osv): @@ -227,15 +237,13 @@ class mail_message(osv.Model): 'subject': message['subject'], 'date': message['date'], 'author_id': message['author_id'], - 'is_author': message['author_id'] and message['author_id'][0] == uid, + 'is_author': is_author, # TDE note: is this useful ? to check 'partner_ids': partner_ids, 'parent_id': message['parent_id'] and message['parent_id'][0] or False, - # TDE note: see with CHM about votes, how they are displayed (only number, or name_get ?) - # vote: should only use number of votes 'vote_nb': len(message['vote_user_ids']), 'has_voted': has_voted, - 'is_private': message['model'] and message['res_id'], + 'is_private': is_private, 'is_favorite': is_favorite, 'to_read': message['to_read'], } @@ -250,69 +258,63 @@ class mail_message(osv.Model): :param dict read_messages: dict [id]: read result of the messages to easily have access to their values, given their ID """ - # sort for group items / TDE: move to message_read - # result = sorted(result, key=lambda k: k['id']) - tree_not = [] + def _get_expandable(domain, message_nb, parent_id, id, model): + return { + 'domain': domain, + 'nb_messages': message_nb, + 'type': 'expandable', + 'parent_id': parent_id, + 'id': id, + # TDE note: why do we need model sometimes, and sometimes not ??? + 'model': model, + } + all_not_loaded_ids = [] # expandable for not show message id_list = sorted(read_messages.keys()) + # print 'id_list', id_list for message_id in id_list: message = read_messages[message_id] + # message is not a thread header (has a parent_id) + # TDE note: parent_id is false is there is a parent we can not see -> ok + if message.get('parent_id'): + continue + # TDE note: check search is correctly implemented in mail.message not_loaded_ids = self.search(cr, uid, [ - ('parent_id', '=', message['id']), + ('id', 'child_of', message['id']), ('id', 'not in', message_loaded_ids), ], context=context, limit=self._message_read_more_limit) - # TDE note: avoid using dummy None values, maybe id_min 0, id_max max(list) - # group childs not read - id_min = None - id_max = None - nb = 0 + if not not_loaded_ids: + continue + # print 'not_loaded_ids', not_loaded_ids + all_not_loaded_ids += not_loaded_ids + # group childs not read + id_min, id_max, nb = max(not_loaded_ids), 0, 0 for not_loaded_id in not_loaded_ids: if not read_messages.get(not_loaded_id): nb += 1 - if id_min == None or id_min > not_loaded_id: + if id_min > not_loaded_id: id_min = not_loaded_id - if id_max == None or id_max < not_loaded_id: + if id_max < not_loaded_id: id_max = not_loaded_id - tree_not.append(not_loaded_id) + elif nb > 0: + exp_domain = [('id', '>=', id_min), ('id', '<=', id_max), ('id', 'child_of', message_id)] + message_list.append(_get_expandable(exp_domain, nb, message_id, id_min, message.get('model'))) + id_min, id_max, nb = max(not_loaded_ids), 0, 0 else: - if nb > 0: - message_list.append({ - 'domain': [('id', '>=', id_min), ('id', '<=', id_max), ('parent_id', '=', message_id)], - 'nb_messages': nb, - 'type': 'expandable', - 'parent_id': message_id, - 'id': id_min, - 'model': message['model'] - }) - id_min = None - id_max = None - nb = 0 + id_min, id_max, nb = max(not_loaded_ids), 0, 0 if nb > 0: - message_list.append({ - 'domain': [('id', '>=', id_min), ('id', '<=', id_max), ('parent_id', '=', message_id)], - 'nb_messages': nb, - 'type': 'expandable', - 'parent_id': message_id, - 'id': id_min, - 'model': message['model'], - }) + exp_domain = [('id', '>=', id_min), ('id', '<=', id_max), ('id', 'child_of', message_id)] + message_list.append(_get_expandable(exp_domain, nb, message_id, id_min, message.get('model'))) - for msg_id in read_messages.keys() + tree_not: - message_loaded_ids.append(msg_id) + message_loaded_ids = list(set(message_loaded_ids + read_messages.keys() + all_not_loaded_ids)) # expandable for limit max ids = self.search(cr, uid, domain + [('id', 'not in', message_loaded_ids)], context=context, limit=1) - if len(ids) > 0: - message_list.append({ - 'domain': domain, - 'nb_messages': 0, - 'type': 'expandable', - 'parent_id': parent_id, - 'id': -1, - }) + if ids: + message_list.append(_get_expandable(domain, -1, parent_id, -1, None)) return message_list @@ -371,22 +373,25 @@ class mail_message(osv.Model): # TDE FIXME: check access rights on search are implemented for mail.message # fetch messages according to the domain, add their parents if uid has access to ids = self.search(cr, uid, domain, context=context, limit=limit) + # print 'message_read ids', ids for message in self.read(cr, uid, ids, self._message_read_fields, context=context): # if not in tree and not in message_loded list if not read_messages.get(message.get('id')) and message.get('id') not in message_unload_ids: read_messages[message.get('id')] = message message_list.append(self._message_get_dict(cr, uid, message, context=context)) - # get all parented message if the user have the access + # get the older ancestor the user can read parent = self._get_parent(cr, uid, message, context=context) while parent and parent.get('id') != parent_id: - if not read_messages.get(parent.get('id')) and parent.get('id') not in message_unload_ids: - read_messages[parent.get('id')] = parent - message_list.append(self._message_get_dict(cr, uid, parent, context=context)) - parent = self._get_parent(cr, uid, parent, context=context) + message = parent + parent = self._get_parent(cr, uid, message, context=context) + if not read_messages.get(message.get('id')) and message.get('id') not in message_unload_ids: + read_messages[message.get('id')] = message + message_list.append(self._message_get_dict(cr, uid, message, context=context)) # get the child expandable messages for the tree message_list = sorted(message_list, key=lambda k: k['id']) + # print 'message_read message_list', message_list message_list = self._message_read_add_expandables(cr, uid, message_list, read_messages, message_loaded_ids=message_unload_ids, domain=domain, context=context, parent_id=parent_id, limit=limit) diff --git a/addons/mail/tests/test_mail.py b/addons/mail/tests/test_mail.py index 016a392f9dc..55f50fd720c 100644 --- a/addons/mail/tests/test_mail.py +++ b/addons/mail/tests/test_mail.py @@ -123,6 +123,7 @@ class test_mail(TestMailMockups): def setUp(self): super(test_mail, self).setUp() + cr, uid = self.cr, self.uid self.ir_model = self.registry('ir.model') self.mail_alias = self.registry('mail.alias') self.mail_thread = self.registry('mail.thread') @@ -135,22 +136,25 @@ class test_mail(TestMailMockups): self.res_users = self.registry('res.users') self.res_partner = self.registry('res.partner') + # Find Employee group + group_employee_ref = self.registry('ir.model.data').get_object_reference(cr, uid, 'base', 'group_user') + group_employee_id = group_employee_ref and group_employee_ref[1] or False # Test users - self.user_demo_id = self.registry('ir.model.data').get_object_reference(self.cr, self.uid, 'base', 'user_demo')[1] - self.user_admin = self.res_users.browse(self.cr, self.uid, self.uid) + self.user_raoul_id = self.res_users.create(cr, uid, {'name': 'Raoul Grosbedon', 'login': 'raoul', 'groups_id': [(6, 0, [group_employee_id])]}) + self.user_admin = self.res_users.browse(cr, uid, uid) # Mock send_get_mail_body to test its functionality without other addons override self._send_get_mail_body = self.registry('mail.mail').send_get_mail_body self.registry('mail.mail').send_get_mail_body = self._mock_send_get_mail_body # groups@.. will cause the creation of new mail groups - self.mail_group_model_id = self.ir_model.search(self.cr, self.uid, [('model', '=', 'mail.group')])[0] - self.mail_alias.create(self.cr, self.uid, {'alias_name': 'groups', + self.mail_group_model_id = self.ir_model.search(cr, uid, [('model', '=', 'mail.group')])[0] + self.mail_alias.create(cr, uid, {'alias_name': 'groups', 'alias_model_id': self.mail_group_model_id}) # create a 'pigs' group that will be used through the various tests - self.group_pigs_id = self.mail_group.create(self.cr, self.uid, + self.group_pigs_id = self.mail_group.create(cr, uid, {'name': 'Pigs', 'description': 'Fans of Pigs, unite !'}) - self.group_pigs = self.mail_group.browse(self.cr, self.uid, self.group_pigs_id) + self.group_pigs = self.mail_group.browse(cr, uid, self.group_pigs_id) def tearDown(self): # Remove mocks @@ -260,7 +264,7 @@ class test_mail(TestMailMockups): """ Tests designed for the subscriber API as well as message subtypes """ cr, uid, user_admin, group_pigs = self.cr, self.uid, self.user_admin, self.group_pigs # Data: user Raoul - user_raoul_id = self.res_users.create(cr, uid, {'name': 'Raoul Grosbedon', 'login': 'raoul'}) + user_raoul_id = self.user_raoul_id user_raoul = self.res_users.browse(cr, uid, user_raoul_id) # Data: message subtypes self.mail_message_subtype.create(cr, uid, {'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.group'}) @@ -523,29 +527,61 @@ class test_mail(TestMailMockups): def test_30_message_read(self): """ Tests for message_read and expandables. """ cr, uid, user_admin, group_pigs = self.cr, self.uid, self.user_admin, self.group_pigs + pigs_domain = [('model', '=', 'mail.group'), ('res_id', '=', self.group_pigs_id)] - # Data: create a discussion in Pigs - msg_ids = [] - for dummy in range(5): - msg_ids.append(self.group_pigs.message_post(body='My Body', subtype='mt_comment')) + # Data: create a discussion in Pigs (2 messages, one with 2 and one with 3 answers) + msg_id1 = self.group_pigs.message_post(body='1', subtype='mt_comment') + msg_id2 = self.group_pigs.message_post(body='2', subtype='mt_comment') + msg_id3 = self.group_pigs.message_post(body='1-1', subtype='mt_comment', parent_id=msg_id1) + msg_id4 = self.group_pigs.message_post(body='2-1', subtype='mt_comment', parent_id=msg_id2) + msg_id5 = self.group_pigs.message_post(body='1-2', subtype='mt_comment', parent_id=msg_id1) + msg_id6 = self.group_pigs.message_post(body='2-2', subtype='mt_comment', parent_id=msg_id2) + msg_id7 = self.group_pigs.message_post(body='1-1-1', subtype='mt_comment', parent_id=msg_id3) + msg_id8 = self.group_pigs.message_post(body='2-1-1', subtype='mt_comment', parent_id=msg_id4) + msg_id9 = self.group_pigs.message_post(body='1-1-1', subtype='mt_comment', parent_id=msg_id3) + msg_id10 = self.group_pigs.message_post(body='2-1-1', subtype='mt_comment', parent_id=msg_id4) + msg_ids = [msg_id1, msg_id2, msg_id3, msg_id4, msg_id5, msg_id6, msg_id7, msg_id8, msg_id9, msg_id10] # Test: read some specific ids - read_msg_list = self.mail_message.message_read(cr, uid, ids=msg_ids[1:3], domain=[('body', 'like', 'dummy')]) + read_msg_list = self.mail_message.message_read(cr, uid, ids=msg_ids[2:4], domain=[('body', 'like', 'dummy')]) read_msg_ids = [msg.get('id') for msg in read_msg_list] - self.assertEqual(msg_ids[1:3], read_msg_ids, 'message_read with direct ids should read only the requested ids') + self.assertEqual(msg_ids[2:4], read_msg_ids, 'message_read with direct ids should read only the requested ids') # Test: read messages of Pigs through a domain - read_msg_list = self.mail_message.message_read(cr, uid, domain=[('model', '=', 'mail.group'), ('res_id', '=', self.group_pigs_id)], limit=200) + read_msg_list = self.mail_message.message_read(cr, uid, domain=pigs_domain, limit=200) read_msg_ids = [msg.get('id') for msg in read_msg_list] self.assertEqual(msg_ids, read_msg_ids, 'message_read with domain on Pigs should equal all messages of Pigs') + # Test: read last message, check expandables + read_msg_list = self.mail_message.message_read(cr, uid, domain=pigs_domain, limit=1) + read_msg_ids = [msg.get('id') for msg in read_msg_list if msg.get('type') != 'expandable'] + # Test: parent is added to the read messages + self.assertEqual(set([msg_id2, msg_id10]), set(read_msg_ids), 'message_read on the last Pigs message should also get its parent') + print read_msg_ids + # Test: expandables + # 1. expandable for childs of Body2 before the last reply + # 2. expandable for messages before Body2 + for msg in read_msg_list: + if msg.get('type') == 'expandable': + print msg + # 'read more threads' expandable + if msg.get('type') == 'expandable' and msg.get('nb_messages') == -1 and msg.get('id') == -1: + domain = msg.get('domain', []) + self.assertEqual(set(domain), set(pigs_domain), 'general expandable domain should equal the message_read domain parameter') + # 'read more messages' expandables + elif msg.get('type') == 'expandable': + domain = msg.get('domain', []) + self.assertIn(('id', 'child_of', msg_id2), domain, 'thread expandable domain should contain a child_of condition') + self.assertIn(('id', '>=', msg_id4), domain, 'thread expandable domain should contain a child_of condition') + self.assertIn(('id', '<=', msg_id8), domain, 'thread expandable domain should contain a child_of condition') + def test_40_needaction(self): """ Tests for mail.message needaction. """ cr, uid, user_admin, group_pigs = self.cr, self.uid, self.user_admin, self.group_pigs - user_demo = self.res_users.browse(cr, uid, self.user_demo_id) - group_pigs_demo = self.mail_group.browse(cr, self.user_demo_id, self.group_pigs_id) + user_raoul = self.res_users.browse(cr, uid, self.user_raoul_id) + group_pigs_demo = self.mail_group.browse(cr, self.user_raoul_id, self.group_pigs_id) na_admin_base = self.mail_message._needaction_count(cr, uid, domain=[]) - na_demo_base = self.mail_message._needaction_count(cr, user_demo.id, domain=[]) + na_demo_base = self.mail_message._needaction_count(cr, user_raoul.id, domain=[]) # Test: number of unread notification = needaction on mail.message notif_ids = self.mail_notification.search(cr, uid, [ @@ -573,12 +609,12 @@ class test_mail(TestMailMockups): self.assertEqual(na_admin_group, 3, 'Admin should have 3 needaction related to Pigs') # Test: demo has 0 new notifications (not a follower, not receiving its own messages), and 0 new needaction notif_ids = self.mail_notification.search(cr, uid, [ - ('partner_id', '=', user_demo.partner_id.id), + ('partner_id', '=', user_raoul.partner_id.id), ('read', '=', False) ]) self.assertEqual(len(notif_ids), na_demo_base + 0, 'Demo should have 0 new unread notifications') - na_demo = self.mail_message._needaction_count(cr, user_demo.id, domain=[]) - na_demo_group = self.mail_message._needaction_count(cr, user_demo.id, domain=[('model', '=', 'mail.group'), ('res_id', '=', self.group_pigs_id)]) + na_demo = self.mail_message._needaction_count(cr, user_raoul.id, domain=[]) + na_demo_group = self.mail_message._needaction_count(cr, user_raoul.id, domain=[('model', '=', 'mail.group'), ('res_id', '=', self.group_pigs_id)]) self.assertEqual(na_demo, na_demo_base + 0, 'Demo should have 0 new needaction') self.assertEqual(na_demo_group, 0, 'Demo should have 0 needaction related to Pigs') @@ -599,6 +635,9 @@ class test_mail(TestMailMockups): # TDE note: temp various asserts because of the random bug about msg1.child_ids msg_ids = self.mail_message.search(cr, uid, [('model', '=', 'mail.group'), ('res_id', '=', self.group_pigs_id)], limit=1) new_msg = self.mail_message.browse(cr, uid, msg_ids[0]) + if new_msg.parent_id.id != msg1.id: + import pdb + pdb.set_trace() self.assertEqual(new_msg.parent_id, msg1, 'Newly processed mail_message (%d) should have msg1 as parent (msg2 is %d)' % (new_msg.id, msg2.id)) # 2. References header