diff options
| author | Thibault Francois <tfr@openerp.com> | 2020-01-23 15:39:57 +0000 |
|---|---|---|
| committer | Thibault Francois <tfr@openerp.com> | 2020-02-14 10:40:57 +0000 |
| commit | 8c1c7f26876eac8a431ef12fe373298fd01ce241 (patch) | |
| tree | ad12a749705e5c8b912b0cd577b96935835c2b41 /addons | |
| parent | 7639f7fbcb7578ca77509c656737747904b1b927 (diff) | |
[FIX] mail: Don't create duplicate mail.notification
When more than one user are linked to the same res.partner and this user is assigned to a task or a sale.order
as many email notifications will be created as there are users linked to the same res.partner.
This won't send more than 1 email, but multiple mail.notification
records will be created.
This causes duplicate entries in the mail.notification table, and can
crash when an old unique constraint is still present in that table (from
earlier Odoo versions)
To prevent this, we deduplicate the notifications and order by notification
type to get 'email' first in case the user have different notification
type ('email', 'inbox').
How to reproduce:
- Create a first user test1
- Create a second user test2
- Merge the contact test1 and test2
- Create a customer customer
- Set the user test1 as salesman
- Create a new sale.order with customer as partner
Add a test to reproduce the pathological scenario
Solution
add distinct on partner.id and order by partner.id,
users.notification_type
closes odoo/odoo#45386
X-original-commit: 00f36c60e4790c8f5341cbc4f2c67e22e6d68f87
Signed-off-by: Olivier Dony (odo) <odo@openerp.com>
Diffstat (limited to 'addons')
| -rw-r--r-- | addons/mail/models/mail_followers.py | 66 | ||||
| -rw-r--r-- | addons/test_mail/tests/test_mail_followers.py | 36 |
2 files changed, 71 insertions, 31 deletions
diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index 2cd2d48b38d..f2c5bd56fa4 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -125,37 +125,40 @@ class Followers(models.Model): self.env['mail.channel'].flush(['email_send', 'channel_type']) if records and subtype_id: query = """ -WITH sub_followers AS ( - SELECT fol.id, fol.partner_id, fol.channel_id, subtype.internal - FROM mail_followers fol - RIGHT JOIN mail_followers_mail_message_subtype_rel subrel - ON subrel.mail_followers_id = fol.id - RIGHT JOIN mail_message_subtype subtype - ON subtype.id = subrel.mail_message_subtype_id - WHERE subrel.mail_message_subtype_id = %%s AND fol.res_model = %%s AND fol.res_id IN %%s -) -SELECT partner.id as pid, NULL AS cid, - partner.active as active, partner.partner_share as pshare, NULL as ctype, - users.notification_type AS notif, array_agg(groups.id) AS groups - FROM res_partner partner - LEFT JOIN res_users users ON users.partner_id = partner.id AND users.active - LEFT JOIN res_groups_users_rel groups_rel ON groups_rel.uid = users.id - LEFT JOIN res_groups groups ON groups.id = groups_rel.gid - WHERE EXISTS ( - SELECT partner_id FROM sub_followers - WHERE sub_followers.channel_id IS NULL - AND sub_followers.partner_id = partner.id - AND (coalesce(sub_followers.internal, false) <> TRUE OR coalesce(partner.partner_share, false) <> TRUE) - ) %s - GROUP BY partner.id, users.notification_type -UNION -SELECT NULL AS pid, channel.id AS cid, - TRUE as active, NULL AS pshare, channel.channel_type AS ctype, - CASE WHEN channel.email_send = TRUE THEN 'email' ELSE 'inbox' END AS notif, NULL AS groups - FROM mail_channel channel - WHERE EXISTS ( - SELECT channel_id FROM sub_followers WHERE partner_id IS NULL AND sub_followers.channel_id = channel.id - ) %s +SELECT DISTINCT ON(pid, cid) * FROM ( + WITH sub_followers AS ( + SELECT fol.id, fol.partner_id, fol.channel_id, subtype.internal + FROM mail_followers fol + RIGHT JOIN mail_followers_mail_message_subtype_rel subrel + ON subrel.mail_followers_id = fol.id + RIGHT JOIN mail_message_subtype subtype + ON subtype.id = subrel.mail_message_subtype_id + WHERE subrel.mail_message_subtype_id = %%s AND fol.res_model = %%s AND fol.res_id IN %%s + ) + SELECT partner.id as pid, NULL AS cid, + partner.active as active, partner.partner_share as pshare, NULL as ctype, + users.notification_type AS notif, array_agg(groups.id) AS groups + FROM res_partner partner + LEFT JOIN res_users users ON users.partner_id = partner.id AND users.active + LEFT JOIN res_groups_users_rel groups_rel ON groups_rel.uid = users.id + LEFT JOIN res_groups groups ON groups.id = groups_rel.gid + WHERE EXISTS ( + SELECT partner_id FROM sub_followers + WHERE sub_followers.channel_id IS NULL + AND sub_followers.partner_id = partner.id + AND (coalesce(sub_followers.internal, false) <> TRUE OR coalesce(partner.partner_share, false) <> TRUE) + ) %s + GROUP BY partner.id, users.notification_type + UNION + SELECT NULL AS pid, channel.id AS cid, + TRUE as active, NULL AS pshare, channel.channel_type AS ctype, + CASE WHEN channel.email_send = TRUE THEN 'email' ELSE 'inbox' END AS notif, NULL AS groups + FROM mail_channel channel + WHERE EXISTS ( + SELECT channel_id FROM sub_followers WHERE partner_id IS NULL AND sub_followers.channel_id = channel.id + ) %s +) AS x +ORDER BY pid, cid, notif """ % ('OR partner.id IN %s' if pids else '', 'OR channel.id IN %s' if cids else '') params = [subtype_id, records._name, tuple(records.ids)] if pids: @@ -183,6 +186,7 @@ SELECT NULL AS pid, channel.id AS cid, FROM mail_channel channel WHERE channel.id IN %s """ params.append(tuple(cids)) query = ' UNION'.join(x for x in [query_pid, query_cid] if x) + query = 'SELECT DISTINCT ON(pid, cid) * FROM (%s) AS x ORDER BY pid, cid, notif' % query self.env.cr.execute(query, tuple(params)) res = self.env.cr.fetchall() else: diff --git a/addons/test_mail/tests/test_mail_followers.py b/addons/test_mail/tests/test_mail_followers.py index f3b1957d6c6..1e1597334d5 100644 --- a/addons/test_mail/tests/test_mail_followers.py +++ b/addons/test_mail/tests/test_mail_followers.py @@ -3,6 +3,7 @@ from psycopg2 import IntegrityError +from odoo.tests import tagged from odoo.addons.test_mail.tests.common import TestMailCommon from odoo.tools.misc import mute_logger @@ -205,3 +206,38 @@ class AdvancedFollowersTest(TestMailCommon): self.assertEqual( sub1.message_follower_ids.filtered(lambda fol: fol.partner_id == self.user_employee.partner_id).subtype_ids, all_defaults) + + +@tagged('post_install', '-at_install') +class DuplicateNotificationTest(TestMailCommon): + def test_no_duplicate_notification(self): + """ + Check that we only create one mail.notification per partner + + Post install because we need the registery to be ready to send notification + """ + #Simulate case of 2 users that got their partner merged + common_partner = self.env['res.partner'].create({"name": "demo1", "email": "demo1@test.com"}) + user_1 = self.env['res.users'].create({'login': 'demo1', 'partner_id': common_partner.id, 'notification_type': 'email'}) + user_2 = self.env['res.users'].create({'login': 'demo2', 'partner_id': common_partner.id, 'notification_type': 'inbox'}) + + #Trigger auto subscribe notification + test = self.env['mail.test.track'].create({"name": "Test Track", "user_id": user_2.id}) + mail_message = self.env['mail.message'].search([ + ('res_id', '=', test.id), + ('model', '=', 'mail.test.track'), + ('message_type', '=', 'user_notification') + ]) + notif = self.env['mail.notification'].search([ + ('mail_message_id', '=', mail_message.id), + ('res_partner_id', '=', common_partner.id) + ]) + self.assertEqual(len(notif), 1) + self.assertEqual(notif.notification_type, 'email') + + subtype = self.env.ref('mail.mt_comment') + res = self.env['mail.followers']._get_recipient_data(test, 'comment', subtype.id, pids=common_partner.ids) + partner_notif = [r for r in res if r[0] == common_partner.id] + self.assertEqual(len(partner_notif), 1) + self.assertEqual(partner_notif[0][5], 'email') + |
