From 2a0bcd103aeceb4f996d189fa86337edf40f0d3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Wed, 26 Aug 2020 09:49:03 +0000 Subject: [PATCH] [FIX] mail, test_mail: fix `channel_get` result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - flush must be done before executing queries - when the current partner is given, it shouldn't be checked twice - when a channel includes the given partners but also has more partners, it shouldn't be returned - query can be limited to 1 result, minor performance gain in the rare case where more than one canonical channel exists task-2324119 closes odoo/odoo#56680 X-original-commit: b716dd9e62991997e03556f2d3e2483921fedef9 Signed-off-by: Sébastien Theys (seb) --- addons/mail/models/mail_channel.py | 75 +++++++++++---------- addons/test_mail/tests/test_mail_channel.py | 31 +++++++++ 2 files changed, 72 insertions(+), 34 deletions(-) diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 093c94878b4..1cbf07475d7 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -625,42 +625,49 @@ class Channel(models.Model): only the given partners. :param partners_to : list of res.partner ids to add to the conversation :param pin : True if getting the channel should pin it for the current user - :returns a channel header, or False if the users_to was False - :rtype : dict + :returns: channel_info of the created or existing channel + :rtype: dict """ - if partners_to: + if self.env.user.partner_id.id not in partners_to: partners_to.append(self.env.user.partner_id.id) - # determine type according to the number of partner in the channel - self.env.cr.execute(""" - SELECT P.channel_id - FROM mail_channel C, mail_channel_partner P - WHERE P.channel_id = C.id - AND C.public LIKE 'private' - AND P.partner_id IN %s - AND C.channel_type LIKE 'chat' - GROUP BY P.channel_id - HAVING ARRAY_AGG(DISTINCT P.partner_id ORDER BY P.partner_id) = %s - """, (tuple(partners_to), sorted(list(partners_to)),)) - result = self.env.cr.dictfetchall() - if result: - # get the existing channel between the given partners - channel = self.browse(result[0].get('channel_id')) - # pin up the channel for the current partner - if pin: - self.env['mail.channel.partner'].search([('partner_id', '=', self.env.user.partner_id.id), ('channel_id', '=', channel.id)]).write({'is_pinned': True}) - else: - # create a new one - channel = self.create({ - 'channel_partner_ids': [(4, partner_id) for partner_id in partners_to], - 'public': 'private', - 'channel_type': 'chat', - 'email_send': False, - 'name': ', '.join(self.env['res.partner'].sudo().browse(partners_to).mapped('name')), - }) - # broadcast the channel header to the other partner (not me) - channel._broadcast(partners_to) - return channel.channel_info()[0] - return False + # determine type according to the number of partner in the channel + self.flush() + self.env.cr.execute(""" + SELECT P.channel_id + FROM mail_channel C, mail_channel_partner P + WHERE P.channel_id = C.id + AND C.public LIKE 'private' + AND P.partner_id IN %s + AND C.channel_type LIKE 'chat' + AND NOT EXISTS ( + SELECT * + FROM mail_channel_partner P2 + WHERE P2.channel_id = C.id + AND P2.partner_id NOT IN %s + ) + GROUP BY P.channel_id + HAVING ARRAY_AGG(DISTINCT P.partner_id ORDER BY P.partner_id) = %s + LIMIT 1 + """, (tuple(partners_to), tuple(partners_to), sorted(list(partners_to)),)) + result = self.env.cr.dictfetchall() + if result: + # get the existing channel between the given partners + channel = self.browse(result[0].get('channel_id')) + # pin up the channel for the current partner + if pin: + self.env['mail.channel.partner'].search([('partner_id', '=', self.env.user.partner_id.id), ('channel_id', '=', channel.id)]).write({'is_pinned': True}) + else: + # create a new one + channel = self.create({ + 'channel_partner_ids': [(4, partner_id) for partner_id in partners_to], + 'public': 'private', + 'channel_type': 'chat', + 'email_send': False, + 'name': ', '.join(self.env['res.partner'].sudo().browse(partners_to).mapped('name')), + }) + # broadcast the channel header to the other partner (not me) + channel._broadcast(partners_to) + return channel.channel_info()[0] @api.model def channel_get_and_minimize(self, partners_to): diff --git a/addons/test_mail/tests/test_mail_channel.py b/addons/test_mail/tests/test_mail_channel.py index e54d90ff8b7..e08d45caa69 100644 --- a/addons/test_mail/tests/test_mail_channel.py +++ b/addons/test_mail/tests/test_mail_channel.py @@ -226,6 +226,37 @@ class TestChannelFeatures(TestMailCommon): self.assertEqual(test_channel_group.channel_partner_ids, self.env['res.partner']) self.assertEqual(self.test_channel.channel_partner_ids, self.user_employee.partner_id | test_partner) + def test_channel_get(self): + current_user = self.env['res.users'].create({ + "login": "adam", + "name": "Jonas", + }) + current_user = current_user.with_user(current_user) + current_partner = current_user.partner_id + other_partner = self.test_partner + + # `channel_get` should return a new channel the first time a partner is given + initial_channel_info = current_user.env['mail.channel'].channel_get(partners_to=other_partner.ids) + self.assertEqual(set(p['id'] for p in initial_channel_info['members']), {current_partner.id, other_partner.id}) + + # `channel_get` should return the existing channel every time the same partner is given + same_channel_info = current_user.env['mail.channel'].channel_get(partners_to=other_partner.ids) + self.assertEqual(same_channel_info['id'], initial_channel_info['id']) + + # `channel_get` should return the existing channel when the current partner is given together with the other partner + together_channel_info = current_user.env['mail.channel'].channel_get(partners_to=(current_partner + other_partner).ids) + self.assertEqual(together_channel_info['id'], initial_channel_info['id']) + + # `channel_get` should return a new channel the first time just the current partner is given, + # even if a channel containing the current partner together with other partners already exists + solo_channel_info = current_user.env['mail.channel'].channel_get(partners_to=current_partner.ids) + self.assertNotEqual(solo_channel_info['id'], initial_channel_info['id']) + self.assertEqual(set(p['id'] for p in solo_channel_info['members']), {current_partner.id}) + + # `channel_get` should return the existing channel every time the current partner is given + same_solo_channel_info = current_user.env['mail.channel'].channel_get(partners_to=current_partner.ids) + self.assertEqual(same_solo_channel_info['id'], solo_channel_info['id']) + @tagged('moderation') class TestChannelModeration(TestMailCommon):