From a62927e7e2d7df4047f8606b41e70ef61ac29b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 14 Jan 2022 12:49:44 +0000 Subject: [PATCH] [REF] mail: improve recipients computation Purpose of this commit is to improve notification recipients computation. Code cleaning * make it batch enabled. Currently it works only for a single record. It should evolve towards real multi computation, allowing to prepare post and notification in batch; * better manage multi users. Now when several users are linked to the same partner, we try to find an internal user or fallback on share user. This is cleaned compared to a distinct done previously; * support recipients without subtype (pids + records) as a new case to include follower computation; Improvements * include follower status if linked to records (is_follower can be used notably in link computation to redirect to the portal); * fetch partner lang directly in the SQL query. While fetching partner data let us add its lang. That way it will be easier to improve lang support in layouts; Task-2739294 (Mail: Batch recipients fetch and improve its usage) Part-of: odoo/odoo#82167 --- addons/mail/models/mail_followers.py | 233 ++++++++++++++---- addons/mail/models/mail_thread.py | 28 +-- addons/mail/tests/common.py | 8 +- addons/mail/wizard/mail_resend_message.py | 13 +- addons/mail/wizard/mail_wizard_invite.py | 23 +- addons/sms/models/mail_followers.py | 33 +-- addons/sms/wizard/sms_resend.py | 14 +- .../models/test_mail_corner_case_models.py | 13 +- addons/test_mail/tests/test_mail_followers.py | 99 +++++--- addons/test_mail/tests/test_performance.py | 2 +- 10 files changed, 307 insertions(+), 159 deletions(-) diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index 6f3e661373c..1f9f1473bea 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -83,78 +83,225 @@ class Followers(models.Model): subtype if records and subtype are set; * partners if pids is given; - :param records: fetch data from followers of records that follow subtype_id; - :param message_type: mail.message.message_type in order to allow custom behavior depending on it (SMS for example); + :param records: fetch data from followers of ``records`` that follow + ``subtype_id``; + :param message_type: mail.message.message_type in order to allow custom + behavior depending on it (SMS for example); :param subtype_id: mail.message.subtype to check against followers; - :param pids: additional set of partner IDs from which to fetch recipient data; + :param pids: additional set of partner IDs from which to fetch recipient + data independently from following status; - :return: list of recipient data which is a tuple containing - partner ID , - active value (always True for channels), - share status of partner, - notification status of partner or channel (email or inbox), - user groups of partner, + :return dict: recipients data based on record.ids if given, else a generic + '0' key to keep a dict-like return format. Each item is a dict based on + recipients partner ids formatted like + {'active': whether partner is active; + 'id': res.partner ID; + 'is_follower': True if linked to a record and if partner is a follower; + 'lang': lang of the partner; + 'groups': groups of the partner's user. If several users exist preference + is given to internal user, then share users. In case of multiples + users of same kind groups are unioned; + 'notif': notification type ('inbox' or 'email'). Overrides may change + this value (e.g. 'sms' in sms module); + 'share': if partner is a customer (no user or share user); + 'ushare': if partner has users, whether all are shared (public or portal); + 'type': summary of partner 'usage' (portal, customer, internal user); + } """ self.env['mail.followers'].flush(['partner_id', 'subtype_ids']) self.env['mail.message.subtype'].flush(['internal']) self.env['res.users'].flush(['notification_type', 'active', 'partner_id', 'groups_id']) self.env['res.partner'].flush(['active', 'partner_share']) self.env['res.groups'].flush(['users']) + # if we have records and a subtype: we have to fetch followers if records and subtype_id: query = """ -SELECT DISTINCT ON (pid) * FROM ( WITH sub_followers AS ( - SELECT fol.partner_id, - coalesce(subtype.internal, false) as internal + SELECT fol.partner_id AS pid, + fol.id AS fid, + fol.res_id AS res_id, + TRUE as is_follower, + COALESCE(subrel.follow, FALSE) AS subtype_follower, + COALESCE(subrel.internal, FALSE) AS internal FROM mail_followers fol - JOIN mail_followers_mail_message_subtype_rel subrel ON subrel.mail_followers_id = fol.id - 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 + LEFT JOIN LATERAL ( + SELECT TRUE AS follow, + subtype.internal AS internal + FROM mail_followers_mail_message_subtype_rel m + LEFT JOIN mail_message_subtype subtype ON subtype.id = m.mail_message_subtype_id + WHERE m.mail_followers_id = fol.id AND m.mail_message_subtype_id = %s + ) subrel ON TRUE + WHERE fol.res_model = %s + AND fol.res_id IN %s UNION ALL - SELECT id, - FALSE + SELECT res_partner.id AS pid, + 0 AS fid, + 0 AS res_id, + FALSE as is_follower, + FALSE as subtype_follower, + FALSE as internal FROM res_partner - WHERE id=ANY(%s) + WHERE res_partner.id = ANY(%s) ) SELECT partner.id as pid, partner.active as active, + partner.lang as lang, partner.partner_share as pshare, - users.notification_type AS notif, - array_agg(groups_rel.gid) AS groups + sub_user.uid as uid, + COALESCE(sub_user.share, FALSE) as ushare, + COALESCE(sub_user.notification_type, 'email') as notif, + sub_user.groups as groups, + sub_followers.res_id as res_id, + sub_followers.is_follower as _insert_followerslower 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 - JOIN sub_followers ON sub_followers.partner_id = partner.id - AND NOT (sub_followers.internal AND partner.partner_share) - GROUP BY partner.id, - users.notification_type -) AS x -ORDER BY pid, notif + JOIN sub_followers ON sub_followers.pid = partner.id + AND (NOT sub_followers.internal OR NOT partner.partner_share) + LEFT JOIN LATERAL ( + SELECT users.id AS uid, + users.share AS share, + users.notification_type AS notification_type, + ARRAY_AGG(groups_rel.gid) FILTER (WHERE groups_rel.gid IS NOT NULL) AS groups + FROM res_users users + LEFT JOIN res_groups_users_rel groups_rel ON groups_rel.uid = users.id + WHERE users.partner_id = partner.id AND users.active + GROUP BY users.id, + users.share, + users.notification_type + ORDER BY users.share ASC NULLS FIRST, users.id ASC + FETCH FIRST ROW ONLY + ) sub_user ON TRUE + + WHERE sub_followers.subtype_follower OR partner.id = ANY(%s) """ - params = [subtype_id, records._name, tuple(records.ids), list(pids or [])] + params = [subtype_id, records._name, tuple(records.ids), list(pids or []), list(pids or [])] self.env.cr.execute(query, tuple(params)) res = self.env.cr.fetchall() - elif pids: + # partner_ids and records: no sub query for followers but check for follower status + elif pids and records: params = [] - query_pid = """ -SELECT partner.id as pid, -partner.active as active, partner.partner_share as pshare, -users.notification_type AS notif, NULL AS groups -FROM res_partner partner -LEFT JOIN res_users users ON users.partner_id = partner.id AND users.active -WHERE partner.id IN %s""" - params.append(tuple(pids)) - query = 'SELECT DISTINCT ON (pid) * FROM (%s) AS x ORDER BY pid, notif' % query_pid + query = """ + SELECT partner.id as pid, + partner.active as active, + partner.lang as lang, + partner.partner_share as pshare, + sub_user.uid as uid, + COALESCE(sub_user.share, FALSE) as ushare, + COALESCE(sub_user.notification_type, 'email') as notif, + sub_user.groups as groups, + ARRAY_AGG(fol.res_id) FILTER (WHERE fol.res_id IS NOT NULL) AS res_ids + FROM res_partner partner + LEFT JOIN mail_followers fol ON fol.partner_id = partner.id + AND fol.res_model = %s + AND fol.res_id IN %s + LEFT JOIN LATERAL ( + SELECT users.id AS uid, + users.share AS share, + users.notification_type AS notification_type, + ARRAY_AGG(groups_rel.gid) FILTER (WHERE groups_rel.gid IS NOT NULL) AS groups + FROM res_users users + LEFT JOIN res_groups_users_rel groups_rel ON groups_rel.uid = users.id + WHERE users.partner_id = partner.id AND users.active + GROUP BY users.id, + users.share, + users.notification_type + ORDER BY users.share ASC NULLS FIRST, users.id ASC + FETCH FIRST ROW ONLY + ) sub_user ON TRUE + + WHERE partner.id IN %s + GROUP BY partner.id, + sub_user.uid, + sub_user.share, + sub_user.notification_type, + sub_user.groups +""" + params = [records._name, tuple(records.ids), tuple(pids)] + self.env.cr.execute(query, tuple(params)) + simplified_res = self.env.cr.fetchall() + # simplified query contains res_ids -> flatten it by making it a list + # with res_id and add follower status + res = [] + for item in simplified_res: + res_ids = item[-1] + if not res_ids: # keep res_ids Falsy (global), set as not follower + flattened = [list(item) + [False]] + else: # generate an entry for each res_id with partner being follower + flattened = [list(item[:-1]) + [res_id, True] + for res_id in res_ids] + res += flattened + # only partner ids: no follower status involved, fetch only direct recipients information + elif pids: + query = """ + SELECT partner.id as pid, + partner.active as active, + partner.lang as lang, + partner.partner_share as pshare, + sub_user.uid as uid, + COALESCE(sub_user.share, FALSE) as ushare, + COALESCE(sub_user.notification_type, 'email') as notif, + sub_user.groups as groups, + 0 as res_id, + FALSE as is_follower + FROM res_partner partner + LEFT JOIN LATERAL ( + SELECT users.id AS uid, + users.share AS share, + users.notification_type AS notification_type, + ARRAY_AGG(groups_rel.gid) FILTER (WHERE groups_rel.gid IS NOT NULL) AS groups + FROM res_users users + LEFT JOIN res_groups_users_rel groups_rel ON groups_rel.uid = users.id + WHERE users.partner_id = partner.id AND users.active + GROUP BY users.id, + users.share, + users.notification_type + ORDER BY users.share ASC NULLS FIRST, users.id ASC + FETCH FIRST ROW ONLY + ) sub_user ON TRUE + + WHERE partner.id IN %s + GROUP BY partner.id, + sub_user.uid, + sub_user.share, + sub_user.notification_type, + sub_user.groups +""" + params = [tuple(pids)] self.env.cr.execute(query, tuple(params)) res = self.env.cr.fetchall() else: res = [] - return res + + res_ids = records.ids if records else [0] + doc_infos = dict((res_id, {}) for res_id in res_ids) + for (partner_id, is_active, lang, pshare, uid, ushare, notif, groups, res_id, is_follower) in res: + to_update = [res_id] if res_id else res_ids + for res_id_to_update in to_update: + # avoid updating already existing information, unnecessary dict update + if not res_id and partner_id in doc_infos[res_id_to_update]: + continue + follower_data = { + 'active': is_active, + 'id': partner_id, + 'is_follower': is_follower, + 'lang': lang, + 'groups': set(groups or []), + 'notif': notif, + 'share': pshare, + 'uid': uid, + 'ushare': ushare, + } + # additional information + if follower_data['ushare']: # any type of share user + follower_data['type'] = 'portal' + elif follower_data['share']: # no user, is share -> customer (partner only) + follower_data['type'] = 'customer' + else: # has a user not share -> internal user + follower_data['type'] = 'user' + doc_infos[res_id_to_update][partner_id] = follower_data + + return doc_infos def _get_subscription_data(self, doc_data, pids, include_pshare=False, include_active=False): """ Private method allowing to fetch follower data from several documents of a given model. diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 28cc003f4e5..bc7178e8a39 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2503,15 +2503,15 @@ class MailThread(models.AbstractModel): TDE/XDO TODO: flag rdata directly, with for example r['notif'] = 'ocn_client' and r['needaction']=False and correctly override _notify_get_recipients - :return list recipients_data: this is a list of recipients information - composed of a dictionary { - 'active': partner.active; + :return list recipients_data: this is a list of recipients information (see + ``MailFollowers._get_recipient_data()`` for more details) formatted like + [{'active': partner.active; 'id': id of the res.partner; 'groups': res.group IDs if linked to a user; 'notif': 'inbox', 'email', 'sms' (SMS App); 'share': partner.partner_share; 'type': 'customer', 'portal', 'user;' - } + }, {...}] """ msg_sudo = message.sudo() # get values from msg_vals or from message if msg_vals doen't exists @@ -2521,27 +2521,17 @@ class MailThread(models.AbstractModel): # is it possible to have record but no subtype_id ? recipients_data = [] - res = self.env['mail.followers']._get_recipient_data(self, message_type, subtype_id, pids) + res = self.env['mail.followers']._get_recipient_data(self, message_type, subtype_id, pids)[self.id if self else 0] if not res: return recipients_data author_id = msg_vals.get('author_id') or message.author_id.id - for pid, active, pshare, notif, groups in res: + for pid, pdata in res.items(): if pid and pid == author_id and not self.env.context.get('mail_notify_author'): # do not notify the author of its own messages continue - if pid: - if active is False: - continue - pdata = {'id': pid, 'active': active, 'share': pshare, 'groups': groups or []} - if notif == 'inbox': - recipients_data.append(dict(pdata, notif=notif, type='user')) - elif not pshare and notif: # has an user and is not shared, is therefore user - recipients_data.append(dict(pdata, notif=notif, type='user')) - elif pshare and notif: # has an user but is shared, is therefore portal - recipients_data.append(dict(pdata, notif=notif, type='portal')) - else: # has no user, is therefore customer - recipients_data.append(dict(pdata, notif=notif if notif else 'email', type='customer')) - + if pdata['active'] is False: + continue + recipients_data.append(pdata) return recipients_data def _notify_get_recipients_groups(self, msg_vals=None): diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 9478fd0fb4f..d8331d8de05 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -602,7 +602,7 @@ class MailCase(MockEmail): return cls.email_template - def _generate_notify_recipients(self, partners): + def _generate_notify_recipients(self, partners, record=None): """ Tool method to generate recipients data according to structure used in notification methods. Purpose is to allow testing of internals of some notification methods, notably testing links or group-based notification @@ -612,11 +612,13 @@ class MailCase(MockEmail): """ return [ {'id': partner.id, - 'active': True, - 'share': partner.partner_share, + 'active': partner.active, + 'is_follower': partner in record.message_partner_ids if record else False, 'groups': partner.user_ids.groups_id.ids, 'notif': partner.user_ids.notification_type or 'email', + 'share': partner.partner_share, 'type': 'user' if partner.user_ids and not partner.partner_share else partner.user_ids and 'portal' or 'customer', + 'ushare': all(user.share for user in partner.user_ids) if partner.user_ids else False, } for partner in partners ] diff --git a/addons/mail/wizard/mail_resend_message.py b/addons/mail/wizard/mail_resend_message.py index f7763eb0272..d2b197048f5 100644 --- a/addons/mail/wizard/mail_resend_message.py +++ b/addons/mail/wizard/mail_resend_message.py @@ -68,15 +68,10 @@ class MailResendMessage(models.TransientModel): record = self.env[message.model].browse(message.res_id) if message.is_thread_message() else self.env['mail.thread'] email_partners_data = [] - for pid, active, pshare, notif, groups in self.env['mail.followers']._get_recipient_data(None, 'comment', False, pids=to_send.ids): - if pid and notif == 'email' or not notif: - pdata = {'id': pid, 'share': pshare, 'active': active, 'notif': 'email', 'groups': groups or []} - if not pshare and notif: # has an user and is not shared, is therefore user - email_partners_data.append(dict(pdata, type='user')) - elif pshare and notif: # has an user and is shared, is therefore portal - email_partners_data.append(dict(pdata, type='portal')) - else: # has no user, is therefore customer - email_partners_data.append(dict(pdata, type='customer')) + recipients_data = self.env['mail.followers']._get_recipient_data(None, 'comment', False, pids=to_send.ids)[0] + for pid, pdata in recipients_data.items(): + if pid and pdata.get('notif', 'email') == 'email': + email_partners_data.append(pdata) record._notify_thread_by_email( message, email_partners_data, diff --git a/addons/mail/wizard/mail_wizard_invite.py b/addons/mail/wizard/mail_wizard_invite.py index b859e1aabd4..a39da2284bc 100644 --- a/addons/mail/wizard/mail_wizard_invite.py +++ b/addons/mail/wizard/mail_wizard_invite.py @@ -70,25 +70,14 @@ class Invite(models.TransientModel): 'reply_to_force_new': True, 'email_add_signature': True, }) - partners_data = [] - recipients_data = self.env['mail.followers']._get_recipient_data(document, 'comment', False, pids=new_partners.ids) - for pid, active, pshare, notif, groups in recipients_data: - pdata = { - 'active': active, - 'id': pid, - 'groups': groups or [], - 'notif': 'email', - 'share': pshare, - } - if not pshare and notif: # has an user and is not shared, is therefore user - partners_data.append(dict(pdata, type='user')) - elif pshare and notif: # has an user and is shared, is therefore portal - partners_data.append(dict(pdata, type='portal')) - else: # has no user, is therefore customer - partners_data.append(dict(pdata, type='customer')) + email_partners_data = [] + recipients_data = self.env['mail.followers']._get_recipient_data(document, 'comment', False, pids=new_partners.ids)[document.id] + for _pid, pdata in recipients_data.items(): + pdata['notif'] = 'email' + email_partners_data.append(pdata) document._notify_thread_by_email( - message, partners_data, + message, email_partners_data, send_after_commit=False ) # in case of failure, the web client must know the message was diff --git a/addons/sms/models/mail_followers.py b/addons/sms/models/mail_followers.py index d45a9556f74..248da8afd1d 100644 --- a/addons/sms/models/mail_followers.py +++ b/addons/sms/models/mail_followers.py @@ -1,24 +1,29 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import api, fields, models +from odoo import models class Followers(models.Model): _inherit = ['mail.followers'] def _get_recipient_data(self, records, message_type, subtype_id, pids=None): - if message_type == 'sms': - if pids is None: - sms_pids = records._sms_get_default_partners().ids - else: - sms_pids = pids - res = super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids) - new_res = [] - for pid, active, pshare, notif, groups in res: - if pid and pid in sms_pids: - notif = 'sms' - new_res.append((pid, active, pshare, notif, groups)) - return new_res - else: + if message_type != 'sms' or not (pids or records): return super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids) + + if pids is None and records: + records_pids = dict( + (record.id, record._sms_get_default_partners().ids) + for record in records + ) + elif pids and records: + records_pids = dict((record.id, pids) for record in records) + else: + records_pids = {0: pids if pids else []} + recipients_data = super(Followers, self)._get_recipient_data(records, message_type, subtype_id, pids=pids) + for rid, rdata in recipients_data.items(): + sms_pids = records_pids.get(rid) or [] + for pid, pdata in rdata.items(): + if pid in sms_pids: + pdata['notif'] = 'sms' + return recipients_data diff --git a/addons/sms/wizard/sms_resend.py b/addons/sms/wizard/sms_resend.py index 5cb2ab995f3..4a262eb933d 100644 --- a/addons/sms/wizard/sms_resend.py +++ b/addons/sms/wizard/sms_resend.py @@ -91,16 +91,10 @@ class SMSResend(models.TransientModel): numbers = [r.sms_number for r in self.recipient_ids if r.resend and not r.partner_id] recipients_data = [] - for pid, active, pshare, notif, groups in self.env['mail.followers']._get_recipient_data(record, 'sms', False, pids=pids): - if pid and notif == 'sms': - recipients_data.append({ - 'active': active, - 'groups': groups or [], - 'id': pid, - 'notif': notif, - 'share': pshare, - 'type': 'customer' if pshare else 'user', - }) + all_recipients_data = self.env['mail.followers']._get_recipient_data(record, 'sms', False, pids=pids)[record.id] + for pid, pdata in all_recipients_data.items(): + if pid and pdata['notif'] == 'sms': + recipients_data.append(pdata) if recipients_data or numbers: record._notify_thread_by_sms( self.mail_message_id, recipients_data, diff --git a/addons/test_mail/models/test_mail_corner_case_models.py b/addons/test_mail/models/test_mail_corner_case_models.py index a348cbf49d6..95e0ad9ae7f 100644 --- a/addons/test_mail/models/test_mail_corner_case_models.py +++ b/addons/test_mail/models/test_mail_corner_case_models.py @@ -70,12 +70,13 @@ class MailTestLang(models.Model): groups = super(MailTestLang, self)._notify_get_recipients_groups(msg_vals=msg_vals) local_msg_vals = dict(msg_vals or {}) - customer_group_opts = next(group for group in groups if group[0] == 'customer')[2] - customer_group_opts['has_button_access'] = True - customer_group_opts['actions'] = [ - {'url': self._notify_get_action_link('controller', controller='/test_mail/do_stuff', **local_msg_vals), - 'title': _('TestStuff')} - ] + for group in [g for g in groups if g[0] in('follower', 'customer')]: + group_options = group[2] + group_options['has_button_access'] = True + group_options['actions'] = [ + {'url': self._notify_get_action_link('controller', controller='/test_mail/do_stuff', **local_msg_vals), + 'title': _('TestStuff')} + ] return groups diff --git a/addons/test_mail/tests/test_mail_followers.py b/addons/test_mail/tests/test_mail_followers.py index f7f6003119d..5b358c6bc29 100644 --- a/addons/test_mail/tests/test_mail_followers.py +++ b/addons/test_mail/tests/test_mail_followers.py @@ -453,27 +453,35 @@ class RecipientsNotificationTest(TestMailCommon): 'partner_id': cls.common_partner.id, } ]) + (cls.user_1 + cls.user_2).flush() def assertRecipientsData(self, recipients_data, records, partners, partner_to_users=None): """ Custom assert as recipients structure is custom and may change due - to some implementation choice. Currently supporting only single-record - computation. """ - self.assertEqual(len(recipients_data), len(partners.ids)) - for partner in partners: - partner_data = next(r for r in recipients_data if r[0] == partner.id) - (pid, active, pshare, notif, groups) = partner_data - if partner_to_users and partner_to_users.get(pid): #helps making test explicit - user = partner_to_users[pid] - else: - user = partner.user_ids[-1] if partner.user_ids else self.env['res.users'] - self.assertEqual(active, partner.active) - if user: - self.assertEqual(set(groups), set(user.groups_id.ids)) - self.assertEqual(notif, user.notification_type) - else: - self.assertEqual(set(groups), {None}) - self.assertEqual(notif, None) - self.assertEqual(pshare, partner.partner_share) + to some implementation choice. """ + self.assertEqual(set(recipients_data.keys()), set(records.ids)) + for record in records: + record_data = recipients_data[record.id] + self.assertEqual(set(record_data.keys()), set(partners.ids)) + for partner in partners: + partner_data = record_data[partner.id] + if partner_to_users and partner_to_users.get(partner.id): #helps making test explicit + user = partner_to_users[partner.id] + else: + user = next((user for user in partner.user_ids if not user.share), self.env['res.users']) + if not user: + user = next((user for user in partner.user_ids), self.env['res.users']) + self.assertEqual(partner_data['active'], partner.active) + if user: + self.assertEqual(partner_data['groups'], set(user.groups_id.ids)) + self.assertEqual(partner_data['notif'], user.notification_type) + self.assertEqual(partner_data['uid'], user.id) + else: + self.assertEqual(partner_data['groups'], set()) + self.assertEqual(partner_data['notif'], 'email') + self.assertFalse(partner_data['uid']) + self.assertEqual(partner_data['is_follower'], partner in record.message_partner_ids) + self.assertEqual(partner_data['share'], partner.partner_share) + self.assertEqual(partner_data['ushare'], user.share) @users('employee') def test_notification_nodupe(self): @@ -490,13 +498,13 @@ class RecipientsNotificationTest(TestMailCommon): ('res_partner_id', '=', self.common_partner.id) ]) self.assertEqual(len(notif), 1) - self.assertEqual(notif.notification_type, 'email') + self.assertEqual(notif.notification_type, 'inbox', 'Multi users should take internal users if possible') - res = self.env['mail.followers']._get_recipient_data( + recipients_data = self.env['mail.followers']._get_recipient_data( test, 'comment', self.env.ref('mail.mt_comment').id, pids=self.common_partner.ids) - self.assertRecipientsData(res, test, self.common_partner + self.partner_employee, - partner_to_users={self.common_partner.id: self.user_1}) + self.assertRecipientsData(recipients_data, test, self.common_partner + self.partner_employee, + partner_to_users={self.common_partner.id: self.user_2}) @users('employee') def test_notification_unlink(self): @@ -556,6 +564,8 @@ class RecipientsNotificationTest(TestMailCommon): 'partner_id': shared_partner.id, } ]) + (user_2_1 + user_2_2 + user_2_3).flush() + # just ensure current share status self.assertFalse(shared_partner.partner_share) self.assertTrue(user_2_1.share) @@ -566,7 +576,7 @@ class RecipientsNotificationTest(TestMailCommon): with self.assertSinglePostNotifications( [{'group': 'customer', 'partner': shared_partner, - 'status': 'sent', 'type': 'email'}], + 'status': 'sent', 'type': 'inbox'}], message_info={'content': 'User Choice Notification'}): test.message_post( body='

User Choice Notification

', @@ -579,13 +589,10 @@ class RecipientsNotificationTest(TestMailCommon): test, 'comment', self.env.ref('mail.mt_comment').id, pids=shared_partner.ids) self.assertRecipientsData(recipients_data, test, self.partner_employee + shared_partner, - partner_to_users={shared_partner.id: user_2_1}) + partner_to_users={shared_partner.id: user_2_2}) @users('employee') def test_recipients_fetch(self): - """ Test internals of ``_get_recipient_data`` to ease its maintenance. - Even if it is low-level knowing what it produces allows to write short - tests. """ test_records = self.env['mail.test.simple'].create([ {'email_from': 'ignasse@example.com', 'name': 'Test %s' % idx, @@ -640,13 +647,31 @@ class RecipientsNotificationTest(TestMailCommon): test_records[0], 'comment', False, pids=(self.common_partner + self.partner_admin).ids ) - # TDE FIXME: currently crashes as no user infomation is fetched for PIDs - # self.assertRecipientsData(recipients_data, test_records[0], self.common_partner + self.partner_admin) - self.assertEqual(len(recipients_data), 2) - for partner in self.common_partner + self.partner_admin: - partner_data = next(r for r in recipients_data if r[0] == partner.id) - (_pid, active, pshare, notif, groups) = partner_data - self.assertEqual(active, partner.active) - self.assertEqual(groups, None, 'FIXME: currently not fetching any group information') - self.assertEqual(notif, 'inbox' if partner == self.partner_admin else 'email') - self.assertEqual(pshare, partner.partner_share) + self.assertRecipientsData(recipients_data, test_records[0], self.common_partner + self.partner_admin) + + # multi mode + test_records[1].message_subscribe(self.partner_portal.ids) + test_records[0:4].message_subscribe(self.common_partner.ids) + recipients_data = self.env['mail.followers']._get_recipient_data( + test_records, 'comment', self.env.ref('mail.mt_comment').id, + pids=self.partner_admin.ids + ) + # 0: portal is follower but does not follow comment + common partner (+ admin as pid) + recipients_data_1 = dict((r, recipients_data[r]) for r in recipients_data if r in test_records[0:1].ids) + self.assertRecipientsData(recipients_data_1, test_records[0:1], self.env.user.partner_id + self.common_partner + self.partner_admin) + # 1: portal is follower with comment + common partner (+ admin as pid) + recipients_data_1 = dict((r, recipients_data[r]) for r in recipients_data if r in test_records[1:2].ids) + self.assertRecipientsData(recipients_data_1, test_records[1:2], self.env.user.partner_id + self.common_partner + self.partner_portal + self.partner_admin) + # 2-3: common partner (+ admin as pid) + recipients_data_2 = dict((r, recipients_data[r]) for r in recipients_data if r in test_records[2:4].ids) + self.assertRecipientsData(recipients_data_2, test_records[2:4], self.env.user.partner_id + self.common_partner + self.partner_admin) + # 4+: env user partner (+ admin as pid) + recipients_data_3 = dict((r, recipients_data[r]) for r in recipients_data if r in test_records[4:].ids) + self.assertRecipientsData(recipients_data_3, test_records[4:], self.env.user.partner_id + self.partner_admin) + + # multi mode, pids only + recipients_data = self.env['mail.followers']._get_recipient_data( + test_records, 'comment', False, + pids=(self.env.user.partner_id + self.partner_admin).ids + ) + self.assertRecipientsData(recipients_data, test_records, self.env.user.partner_id + self.partner_admin) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 39cda308659..1ccbf127981 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -587,7 +587,7 @@ class TestMailComplexPerformance(BaseMailPerformance): record = self.container.with_user(self.env.user) template_id = self.env.ref('test_mail.mail_test_container_tpl').id - with self.assertQueryCount(__system__=72, employee=73): + with self.assertQueryCount(__system__=72, employee=73): # about 20 queries per additional customer group record.message_post_with_template(template_id, message_type='comment', composition_mode='comment') self.assertEqual(record.message_ids[0].body, '

Adding stuff on %s

' % record.name)