[FW][MERGE] mail, various: improve follower subscription in gateway
SPECIFICATIONS: MAIL GATEWAY
Document creation or update is still done without auto subscribe. Indeed user
running mailgateway or owning alias is not necessarily linked to the email
author. That way we avoid auto subscription of irrelevant people.
Posting message based on incoming email is now allowing auto subscription if
there is an author found during email parsing. We also ensure this author is
not root, to be sure he is not added in followers of documents.
SPECIFICATIONS: INACTIVE PARTNERS
Tested flows
* posting a message through an inactive partner (like automated actions
posting a message on behalf of an inactive partner);
* automatic subscription based on parent record (like an archived user and
partner following a project that should not be added as follower of sub
tasks);
* automatic subscription based on responsible field: this is already fixed
as user has to be active to receive a notification and be added in
followers;
LINKS
Task ID-2326281
PR odoo/odoo#56560
Closes to odoo/odoo#38383
closes odoo/odoo#56743
Forward-port-of: odoo/odoo#56560
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
Odoo's Mergebot
@@ -193,7 +193,7 @@ FROM mail_channel channel WHERE channel.id IN %s """
|
||||
res = []
|
||||
return res
|
||||
|
||||
def _get_subscription_data(self, doc_data, pids, cids, include_pshare=False):
|
||||
def _get_subscription_data(self, doc_data, pids, cids, include_pshare=False, include_active=False):
|
||||
""" Private method allowing to fetch follower data from several documents of a given model.
|
||||
Followers can be filtered given partner IDs and channel IDs.
|
||||
|
||||
@@ -202,6 +202,7 @@ FROM mail_channel channel WHERE channel.id IN %s """
|
||||
:param pids: optional partner to filter; if None take all, otherwise limitate to pids
|
||||
:param cids: optional channel to filter; if None take all, otherwise limitate to cids
|
||||
:param include_pshare: optional join in partner to fetch their share status
|
||||
:param include_active: optional join in partner to fetch their active flag
|
||||
|
||||
:return: list of followers data which is a list of tuples containing
|
||||
follower ID,
|
||||
@@ -210,6 +211,7 @@ FROM mail_channel channel WHERE channel.id IN %s """
|
||||
channel ID (void if partner_id),
|
||||
followed subtype IDs,
|
||||
share status of partner (void id channel_id, returned only if include_pshare is True)
|
||||
active flag status of partner (void id channel_id, returned only if include_active is True)
|
||||
"""
|
||||
# base query: fetch followers of given documents
|
||||
where_clause = ' OR '.join(['fol.res_model = %s AND fol.res_id IN %s'] * len(doc_data))
|
||||
@@ -231,17 +233,20 @@ FROM mail_channel channel WHERE channel.id IN %s """
|
||||
where_clause += "AND (%s)" % " OR ".join(sub_where)
|
||||
|
||||
query = """
|
||||
SELECT fol.id, fol.res_id, fol.partner_id, fol.channel_id, array_agg(subtype.id)%s
|
||||
SELECT fol.id, fol.res_id, fol.partner_id, fol.channel_id, array_agg(subtype.id)%s%s
|
||||
FROM mail_followers fol
|
||||
%s
|
||||
LEFT JOIN mail_followers_mail_message_subtype_rel fol_rel ON fol_rel.mail_followers_id = fol.id
|
||||
LEFT JOIN mail_message_subtype subtype ON subtype.id = fol_rel.mail_message_subtype_id
|
||||
WHERE %s
|
||||
GROUP BY fol.id%s""" % (
|
||||
GROUP BY fol.id%s%s""" % (
|
||||
', partner.partner_share' if include_pshare else '',
|
||||
'LEFT JOIN res_partner partner ON partner.id = fol.partner_id' if include_pshare else '',
|
||||
', partner.active' if include_active else '',
|
||||
'LEFT JOIN res_partner partner ON partner.id = fol.partner_id' if (include_pshare or include_active) else '',
|
||||
where_clause,
|
||||
', partner.partner_share' if include_pshare else '')
|
||||
', partner.partner_share' if include_pshare else '',
|
||||
', partner.active' if include_active else ''
|
||||
)
|
||||
self.env.cr.execute(query, tuple(where_params))
|
||||
return self.env.cr.fetchall()
|
||||
|
||||
|
||||
@@ -1006,6 +1006,7 @@ class MailThread(models.AbstractModel):
|
||||
thread_id = False
|
||||
for model, thread_id, custom_values, user_id, alias in routes or ():
|
||||
subtype_id = False
|
||||
related_user = self.env['res.users'].browse(user_id)
|
||||
Model = self.env[model].with_context(mail_create_nosubscribe=True, mail_create_nolog=True)
|
||||
if not (thread_id and hasattr(Model, 'message_update') or hasattr(Model, 'message_new')):
|
||||
raise ValueError(
|
||||
@@ -1015,7 +1016,7 @@ class MailThread(models.AbstractModel):
|
||||
|
||||
# disabled subscriptions during message_new/update to avoid having the system user running the
|
||||
# email gateway become a follower of all inbound messages
|
||||
ModelCtx = Model.with_user(user_id).sudo()
|
||||
ModelCtx = Model.with_user(related_user).sudo()
|
||||
if thread_id and hasattr(ModelCtx, 'message_update'):
|
||||
thread = ModelCtx.browse(thread_id)
|
||||
thread.message_update(message_dict)
|
||||
@@ -1048,6 +1049,9 @@ class MailThread(models.AbstractModel):
|
||||
if thread._name == 'mail.thread': # message with parent_id not linked to record
|
||||
new_msg = thread.message_notify(**post_params)
|
||||
else:
|
||||
# parsing should find an author independently of user running mail gateway, and ensure it is not odoobot
|
||||
partner_from_found = message_dict.get('author_id') and message_dict['author_id'] != self.env['ir.model.data'].xmlid_to_res_id('base.partner_root')
|
||||
thread = thread.with_context(mail_create_nosubscribe=not partner_from_found)
|
||||
new_msg = thread.message_post(**post_params)
|
||||
|
||||
if new_msg and original_partner_ids:
|
||||
@@ -1844,8 +1848,8 @@ class MailThread(models.AbstractModel):
|
||||
self._message_set_main_attachment_id(values['attachment_ids'])
|
||||
|
||||
if values['author_id'] and values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'):
|
||||
# if self.env['res.partner'].browse(values['author_id']).active: # we dont want to add odoobot/inactive as a follower
|
||||
self._message_subscribe([values['author_id']])
|
||||
if self.env['res.partner'].browse(values['author_id']).active: # we dont want to add odoobot/inactive as a follower
|
||||
self._message_subscribe([values['author_id']])
|
||||
|
||||
self._message_post_after_hook(new_message, values)
|
||||
self._notify_thread(new_message, values, **notif_kwargs)
|
||||
@@ -2769,11 +2773,11 @@ class MailThread(models.AbstractModel):
|
||||
|
||||
if udpated_fields:
|
||||
doc_data = [(model, [updated_values[fname] for fname in fnames]) for model, fnames in updated_relation.items()]
|
||||
res = self.env['mail.followers']._get_subscription_data(doc_data, None, None, include_pshare=True)
|
||||
for fid, rid, pid, cid, subtype_ids, pshare in res:
|
||||
res = self.env['mail.followers']._get_subscription_data(doc_data, None, None, include_pshare=True, include_active=True)
|
||||
for fid, rid, pid, cid, subtype_ids, pshare, active in res:
|
||||
sids = [parent[sid] for sid in subtype_ids if parent.get(sid)]
|
||||
sids += [sid for sid in subtype_ids if sid not in parent and sid in def_ids or sid in int_ids]
|
||||
if pid:
|
||||
if pid and active: # auto subscribe only active partners
|
||||
new_partners[pid] = (set(sids) & set(all_ids)) - set(int_ids) if pshare else set(sids) & set(all_ids)
|
||||
if cid:
|
||||
new_channels[cid] = (set(sids) & set(all_ids)) - set(int_ids)
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
|
||||
from psycopg2 import IntegrityError
|
||||
|
||||
from odoo.tests import tagged
|
||||
from odoo.tests import tagged, users
|
||||
from odoo.addons.test_mail.tests.common import TestMailCommon
|
||||
from odoo.tools.misc import mute_logger
|
||||
|
||||
@@ -17,13 +17,20 @@ class BaseFollowersTest(TestMailCommon):
|
||||
cls._create_portal_user()
|
||||
cls._create_channel_listener()
|
||||
|
||||
# allow employee to update partners
|
||||
cls.user_employee.write({'groups_id': [(4, cls.env.ref('base.group_partner_manager').id)]})
|
||||
|
||||
Subtype = cls.env['mail.message.subtype']
|
||||
cls.mt_mg_def = Subtype.create({'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.test.simple'})
|
||||
cls.mt_cl_def = Subtype.create({'name': 'mt_cl_def', 'default': True, 'res_model': 'mail.test.container'})
|
||||
# global
|
||||
cls.mt_al_def = Subtype.create({'name': 'mt_al_def', 'default': True, 'res_model': False})
|
||||
cls.mt_mg_nodef = Subtype.create({'name': 'mt_mg_nodef', 'default': False, 'res_model': 'mail.test.simple'})
|
||||
cls.mt_al_nodef = Subtype.create({'name': 'mt_al_nodef', 'default': False, 'res_model': False})
|
||||
cls.mt_mg_def_int = cls.env['mail.message.subtype'].create({'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.test.simple', 'internal': True})
|
||||
# mail.test.simple
|
||||
cls.mt_mg_def = Subtype.create({'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.test.simple'})
|
||||
cls.mt_mg_nodef = Subtype.create({'name': 'mt_mg_nodef', 'default': False, 'res_model': 'mail.test.simple'})
|
||||
cls.mt_mg_def_int = Subtype.create({'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.test.simple', 'internal': True})
|
||||
# mail.test.container
|
||||
cls.mt_cl_def = Subtype.create({'name': 'mt_cl_def', 'default': True, 'res_model': 'mail.test.container'})
|
||||
|
||||
cls.default_group_subtypes = Subtype.search([('default', '=', True), '|', ('res_model', '=', 'mail.test.simple'), ('res_model', '=', False)])
|
||||
cls.default_group_subtypes_portal = Subtype.search([('internal', '=', False), ('default', '=', True), '|', ('res_model', '=', 'mail.test.simple'), ('res_model', '=', False)])
|
||||
|
||||
@@ -127,6 +134,27 @@ class BaseFollowersTest(TestMailCommon):
|
||||
channel_ids=[self.channel_listen.id]
|
||||
)
|
||||
|
||||
@users('employee')
|
||||
def test_followers_inactive(self):
|
||||
""" Test standard API does not subscribe inactive partners """
|
||||
customer = self.env['res.partner'].create({
|
||||
'name': 'Valid Lelitre',
|
||||
'email': 'valid.lelitre@agrolait.com',
|
||||
'country_id': self.env.ref('base.be').id,
|
||||
'mobile': '0456001122',
|
||||
'active': False,
|
||||
})
|
||||
document = self.env['mail.test.simple'].browse(self.test_record.id)
|
||||
self.assertEqual(document.message_partner_ids, self.env['res.partner'])
|
||||
document.message_subscribe(partner_ids=(self.partner_portal | customer).ids)
|
||||
self.assertEqual(document.message_partner_ids, self.partner_portal)
|
||||
self.assertEqual(document.message_follower_ids.partner_id, self.partner_portal)
|
||||
|
||||
# works through low-level API
|
||||
document._message_subscribe(partner_ids=(self.partner_portal | customer).ids)
|
||||
self.assertEqual(document.message_partner_ids, self.partner_portal, 'No active test: customer not visible')
|
||||
self.assertEqual(document.message_follower_ids.partner_id, self.partner_portal | customer)
|
||||
|
||||
|
||||
class AdvancedFollowersTest(TestMailCommon):
|
||||
@classmethod
|
||||
@@ -159,6 +187,22 @@ class AdvancedFollowersTest(TestMailCommon):
|
||||
""" Creator of records are automatically added as followers """
|
||||
self.assertEqual(self.test_track.message_partner_ids, self.user_employee.partner_id)
|
||||
|
||||
def test_auto_subscribe_inactive(self):
|
||||
""" Test inactive are not added as followers in automated subscription """
|
||||
self.test_track.user_id = False
|
||||
self.user_admin.active = False
|
||||
self.user_admin.flush()
|
||||
self.partner_admin.active = False
|
||||
self.partner_admin.flush()
|
||||
|
||||
self.test_track.with_user(self.user_admin).message_post(body='Coucou hibou', message_type='comment')
|
||||
self.assertEqual(self.test_track.message_partner_ids, self.user_employee.partner_id)
|
||||
self.assertEqual(self.test_track.message_follower_ids.partner_id, self.user_employee.partner_id)
|
||||
|
||||
self.test_track.write({'user_id': self.user_admin.id})
|
||||
self.assertEqual(self.test_track.message_partner_ids, self.user_employee.partner_id)
|
||||
self.assertEqual(self.test_track.message_follower_ids.partner_id, self.user_employee.partner_id)
|
||||
|
||||
def test_auto_subscribe_post(self):
|
||||
""" People posting a message are automatically added as followers """
|
||||
self.test_track.with_user(self.user_admin).message_post(body='Coucou hibou', message_type='comment')
|
||||
@@ -193,13 +237,20 @@ class AdvancedFollowersTest(TestMailCommon):
|
||||
automatically create subscription with matching subtypes
|
||||
* subscribing to a sub-record as creator applies default subtype values
|
||||
* portal user should not have access to internal subtypes
|
||||
|
||||
Inactive partners should not be auto subscribed.
|
||||
"""
|
||||
container = self.env['mail.test.container'].with_context(self._test_context).create({
|
||||
'name': 'Project-Like',
|
||||
})
|
||||
|
||||
container.message_subscribe(partner_ids=[self.partner_portal.id])
|
||||
self.assertEqual(container.message_partner_ids, self.partner_portal)
|
||||
container.message_subscribe(partner_ids=(self.partner_portal | self.partner_admin).ids)
|
||||
self.assertEqual(container.message_partner_ids, self.partner_portal | self.partner_admin)
|
||||
|
||||
self.user_admin.active = False
|
||||
self.user_admin.flush()
|
||||
self.partner_admin.active = False
|
||||
self.partner_admin.flush()
|
||||
|
||||
sub1 = self.env['mail.test.track'].with_user(self.user_employee).create({
|
||||
'name': 'Task-Like Test',
|
||||
@@ -210,6 +261,7 @@ class AdvancedFollowersTest(TestMailCommon):
|
||||
external_defaults = all_defaults.filtered(lambda subtype: not subtype.internal)
|
||||
|
||||
self.assertEqual(sub1.message_partner_ids, self.partner_portal | self.user_employee.partner_id)
|
||||
self.assertEqual(sub1.message_follower_ids.partner_id, self.partner_portal | self.user_employee.partner_id)
|
||||
self.assertEqual(
|
||||
sub1.message_follower_ids.filtered(lambda fol: fol.partner_id == self.partner_portal).subtype_ids,
|
||||
external_defaults | self.sub_umb1)
|
||||
|
||||
@@ -223,19 +223,62 @@ class TestMailgateway(TestMailCommon):
|
||||
set(['rosaçée.gif', 'verte!µ.gif', 'orangée.gif']))
|
||||
|
||||
def test_message_process_followers(self):
|
||||
pass
|
||||
# TODO : the author of a message post should be added as follower
|
||||
# currently it is not the case as otherwise Administrator would be follower of a lot of stuff
|
||||
# this is a bug with mail_create_nosubscribe -> should be changed in master
|
||||
# self.assertEqual(record.message_partner_ids, self.partner_1,
|
||||
# 'message_process: recognized email -> added as follower')
|
||||
""" Incoming email: recognized author not archived and not odoobot: added as follower """
|
||||
with self.mock_mail_gateway():
|
||||
record = self.format_and_process(MAIL_TEMPLATE, self.partner_1.email_formatted, 'groups@test.com')
|
||||
|
||||
# TODO : the author of a message post on mail.test should not be added as follower
|
||||
# Test: author (and not recipient) added as follower
|
||||
# self.assertEqual(self.test_public.message_partner_ids, self.partner_1 | self.partner_2,
|
||||
# 'message_process: after reply, group should have 2 followers')
|
||||
# self.assertEqual(self.test_public.message_channel_ids, self.env['mail.test.container'],
|
||||
# 'message_process: after reply, group should have 2 followers (0 channels)')
|
||||
self.assertEqual(record.message_ids[0].author_id, self.partner_1,
|
||||
'message_process: recognized email -> author_id')
|
||||
self.assertEqual(record.message_ids[0].email_from, self.partner_1.email_formatted)
|
||||
self.assertEqual(record.message_follower_ids.partner_id, self.partner_1,
|
||||
'message_process: recognized email -> added as follower')
|
||||
self.assertEqual(record.message_partner_ids, self.partner_1,
|
||||
'message_process: recognized email -> added as follower')
|
||||
|
||||
# just an email -> no follower
|
||||
with self.mock_mail_gateway():
|
||||
record2 = self.format_and_process(
|
||||
MAIL_TEMPLATE, self.email_from, 'groups@test.com',
|
||||
subject='Another Email')
|
||||
|
||||
self.assertEqual(record2.message_ids[0].author_id, self.env['res.partner'])
|
||||
self.assertEqual(record2.message_ids[0].email_from, self.email_from)
|
||||
self.assertEqual(record2.message_follower_ids.partner_id, self.env['res.partner'],
|
||||
'message_process: unrecognized email -> no follower')
|
||||
self.assertEqual(record2.message_partner_ids, self.env['res.partner'],
|
||||
'message_process: unrecognized email -> no follower')
|
||||
|
||||
# archived partner -> no follower
|
||||
self.partner_1.active = False
|
||||
self.partner_1.flush()
|
||||
with self.mock_mail_gateway():
|
||||
record3 = self.format_and_process(
|
||||
MAIL_TEMPLATE, self.partner_1.email_formatted, 'groups@test.com',
|
||||
subject='Yet Another Email')
|
||||
|
||||
self.assertEqual(record3.message_ids[0].author_id, self.env['res.partner'])
|
||||
self.assertEqual(record3.message_ids[0].email_from, self.partner_1.email_formatted)
|
||||
self.assertEqual(record3.message_follower_ids.partner_id, self.env['res.partner'],
|
||||
'message_process: unrecognized email -> no follower')
|
||||
self.assertEqual(record3.message_partner_ids, self.env['res.partner'],
|
||||
'message_process: unrecognized email -> no follower')
|
||||
|
||||
|
||||
# partner_root -> never again
|
||||
odoobot = self.env.ref('base.partner_root')
|
||||
odoobot.active = True
|
||||
odoobot.email = 'odoobot@example.com'
|
||||
with self.mock_mail_gateway():
|
||||
record4 = self.format_and_process(
|
||||
MAIL_TEMPLATE, odoobot.email_formatted, 'groups@test.com',
|
||||
subject='Odoobot Automatic Answer')
|
||||
|
||||
self.assertEqual(record4.message_ids[0].author_id, odoobot)
|
||||
self.assertEqual(record4.message_ids[0].email_from, odoobot.email_formatted)
|
||||
self.assertEqual(record4.message_follower_ids.partner_id, self.env['res.partner'],
|
||||
'message_process: unrecognized email -> no follower')
|
||||
self.assertEqual(record4.message_partner_ids, self.env['res.partner'],
|
||||
'message_process: unrecognized email -> no follower')
|
||||
|
||||
# --------------------------------------------------
|
||||
# Author recognition
|
||||
|
||||
Reference in New Issue
Block a user