From d7b4c8b92b8784d54ace3162fc157c1c9a4a1502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 8 Jul 2016 11:10:21 +0200 Subject: [PATCH 01/17] [IMP] base: partner: add notion of customer or shared-user partner We already know that a user is a shared user. This means a partner created for a customer for the purpose of sharing data. However sometimes it is interesting to know that a given partner is a customer or a partner linked to a shared user. This will be used notably in the Discuss app. --- openerp/addons/base/res/res_partner.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/openerp/addons/base/res/res_partner.py b/openerp/addons/base/res/res_partner.py index 07b604c0897..4357e4f71b9 100644 --- a/openerp/addons/base/res/res_partner.py +++ b/openerp/addons/base/res/res_partner.py @@ -200,6 +200,10 @@ class Partner(models.Model, FormatAddress): company_id = fields.Many2one('res.company', 'Company', index=True, default=_default_company) color = fields.Integer(string='Color Index', default=0) user_ids = fields.One2many('res.users', 'partner_id', string='Users', auto_join=True) + partner_share = fields.Boolean( + 'Share Partner', compute='_compute_partner_share', store=True, + help="Either customer (no user), either shared user. Indicated the current partner is a customer without " + "access or with a limited access created for sharing data.") contact_address = fields.Char(compute='_compute_contact_address', string='Complete Address') # technical field used for managing commercial fields @@ -239,6 +243,11 @@ class Partner(models.Model, FormatAddress): for partner in self: partner.tz_offset = datetime.datetime.now(pytz.timezone(partner.tz or 'GMT')).strftime('%z') + @api.depends('user_ids.share') + def _compute_partner_share(self): + for partner in self: + partner.partner_share = not partner.user_ids or any(user.share for user in partner.user_ids) + @api.depends(lambda self: self._display_address_depends()) def _compute_contact_address(self): for partner in self: From ea4c4a22c41ec780818ec03ff6801791bda46065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 18 Aug 2016 13:12:35 +0200 Subject: [PATCH 02/17] [IMP] mail: clean test about forwarded emails and parent_id Previous version of the test was actually not correct. Mailgateway was not considering references as in-reply-to when trying to find answers that have to be considered as a new thread instead of an answer to an existing thread. Purpose of the test is actually to be sure that answers considered as a new thread have no parent_id set. Otherwise you may have threads linked to messages belonging to other records, leading to access right issues. Moreover if this email begins a new discussion thread it makes no sense to have a parent message set. A new test is written to better simulate the test case. It sets a message_id containing reply_to which is the way odoo uses to indicated that answers to a message have to be considered as new threads. This is used notably in mass mailing for example. --- addons/mail/tests/test_mail_gateway.py | 57 ++++++++++++++++++-------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/addons/mail/tests/test_mail_gateway.py b/addons/mail/tests/test_mail_gateway.py index 147d0ed6c81..f5492dc22e9 100644 --- a/addons/mail/tests/test_mail_gateway.py +++ b/addons/mail/tests/test_mail_gateway.py @@ -512,7 +512,7 @@ class TestMailgateway(TestMail): @mute_logger('openerp.addons.mail.models.mail_thread', 'openerp.models') def test_message_process_in_reply_to(self): - """ Incoming email using in-rely-to should go into the right destination event with a wrong destination """ + """ Incoming email using in-rely-to should go into the right destination even with a wrong destination """ self.format_and_process( MAIL_TEMPLATE, email_from='valid.other@gmail.com', msg_id='<1198923581.41972151344608186800.JavaMail.diff1@agrolait.com>', @@ -524,7 +524,7 @@ class TestMailgateway(TestMail): @mute_logger('openerp.addons.mail.models.mail_thread', 'openerp.models') def test_message_process_references(self): - """ Incoming email using in-rely-to should go into the right destination event with a wrong destination """ + """ Incoming email using references should go into the right destination even with a wrong destination """ self.format_and_process( MAIL_TEMPLATE, to='erroneous@example.com', extra='References: <2233@a.com>\r\n\t<3edss_dsa@b.com> %s' % self.fake_email.message_id, @@ -700,21 +700,42 @@ class TestMailgateway(TestMail): self.assertEqual(msg.needaction_partner_ids, self.user_employee.partner_id | self.env.user.partner_id, 'message_post: private discussion: incorrect notified recipients when replying') - # Do bert forward it to an alias - msg = self.env['mail.message'].browse(msg1.id) + @mute_logger('openerp.addons.mail.models.mail_thread', 'openerp.models', 'openerp.addons.mail.models.mail_mail') + def test_forward_parent_id(self): + msg = self.group_pigs.sudo(self.user_employee).message_post(no_auto_thread=True, subtype='mail.mt_comment') + self.assertNotIn(msg.model, msg.message_id) + self.assertNotIn('-%d-' % msg.res_id, msg.message_id) + self.assertIn('reply_to', msg.message_id) + # forward it to a new thread AND an existing thread - for i, to in enumerate(['groups', 'public']): - fw_msg_id = '' % (i,) - fw_message = MAIL_TEMPLATE.format(to='%s@example.com' % (to,), - cc='', - subject='FW: Re: 1', - email_from='b.t@example.com', - extra='References: %s' % msg.message_id, - msg_id=fw_msg_id) - self.env['mail.thread'].message_process(None, fw_message) + fw_msg_id = '' + fw_message = MAIL_TEMPLATE.format(to='groups@example.com', + cc='', + subject='FW: Re: 1', + email_from='b.t@example.com', + extra='In-Reply-To: %s' % msg.message_id, + msg_id=fw_msg_id) + self.env['mail.thread'].message_process(None, fw_message) + msg_fw = self.env['mail.message'].search([('message_id', '=', fw_msg_id)]) + self.assertEqual(len(msg_fw), 1) + channel = self.env['mail.channel'].search([('name', "=", msg_fw.subject)]) + self.assertEqual(len(channel), 1) + self.assertEqual(msg_fw.model, 'mail.channel') + self.assertFalse(msg_fw.parent_id) + self.assertTrue(msg_fw.res_id == channel.id) - msg_fw = self.env['mail.message'].search([('message_id', '=', fw_msg_id)]) - self.assertEqual(len(msg_fw), 1) - - self.assertEqual(msg_fw.model, 'mail.channel') - self.assertFalse(msg_fw.parent_id) + fw_msg_id = '' + fw_message = MAIL_TEMPLATE.format(to='public@example.com', + cc='', + subject='FW: Re: 2', + email_from='b.t@example.com', + extra='In-Reply-To: %s' % msg.message_id, + msg_id=fw_msg_id) + self.env['mail.thread'].message_process(None, fw_message) + msg_fw = self.env['mail.message'].search([('message_id', '=', fw_msg_id)]) + self.assertEqual(len(msg_fw), 1) + channel = self.env['mail.channel'].search([('name', "=", msg_fw.subject)]) + self.assertEqual(len(channel), 0) + self.assertEqual(msg_fw.model, 'mail.channel') + self.assertFalse(msg_fw.parent_id) + self.assertTrue(msg_fw.res_id == self.group_public.id) From b259eb69d2085a961c1ad00d8bba3cb8a92123c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 3 Dec 2015 12:59:19 +0100 Subject: [PATCH 03/17] [CLEAN] mail, tools: factorize decode email tools mail.mail and mail.thread have methods to decode headers. This commit move them in mail tools. It is indeed simplier to have all email tools in the same file. Moreover they have been renamed to avoid confusion with standard library methods: decode and decode_header already exist. * decode in mail_message is now decode_smtp_header * decode_header in mail_thread is now decode_message_header A method to data from references is also moved into tools. A regex was already defined in tools. Now the method using it is already put in tools. Mailgateway will now simply call this method. --- addons/hr/models/mail_thread.py | 8 +-- addons/mail/models/mail_message.py | 11 ----- addons/mail/models/mail_thread.py | 60 +++++++++-------------- addons/mass_mailing/models/mail_thread.py | 11 ++--- openerp/tools/mail.py | 38 ++++++++++++-- 5 files changed, 67 insertions(+), 61 deletions(-) diff --git a/addons/hr/models/mail_thread.py b/addons/hr/models/mail_thread.py index d27cbbd2afc..2e756883856 100644 --- a/addons/hr/models/mail_thread.py +++ b/addons/hr/models/mail_thread.py @@ -4,11 +4,11 @@ import logging from odoo import api, models -from odoo.addons.mail.models.mail_thread import decode_header -from odoo.tools import email_split +from odoo.tools import decode_message_header, email_split _logger = logging.getLogger(__name__) + class MailThread(models.AbstractModel): _inherit = "mail.thread" @@ -18,13 +18,13 @@ class MailThread(models.AbstractModel): if res: alias = route[4] - email_from = decode_header(message, 'From') + email_from = decode_message_header(message, 'From') message_id = message.get('Message-Id') # Identically equal to the definition in mail module because sub methods are local # variables and cannot be called with super def _create_bounce_email(body_html): - bounce_to = decode_header(message, 'Return-Path') or email_from + bounce_to = decode_message_header(message, 'Return-Path') or email_from bounce_mail_values = { 'body_html': body_html, 'subject': 'Re: %s' % message.get('subject'), diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 5aa52d755c8..8d6a933221b 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -3,7 +3,6 @@ import logging -from email.header import decode_header from email.utils import formataddr from odoo import _, api, fields, models, SUPERUSER_ID, tools @@ -14,16 +13,6 @@ from odoo.osv import expression _logger = logging.getLogger(__name__) -def decode(text): - """Returns unicode() string conversion of the the given encoded smtp header text""" - # TDE proposal: move to tools ? - if text: - text = decode_header(text.replace('\r', '')) - # The joining space will not be needed as of Python 3.3 - # See https://hg.python.org/cpython/rev/8c03fe231877 - return ' '.join([tools.ustr(x[0], x[1]) for x in text]) - - class Message(models.Model): """ Messages model: system notification (replacing res.log notifications), comments (OpenChatter discussion) and incoming emails. """ diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 3980ebd4b3f..194d7dc25e4 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -9,7 +9,6 @@ import json import lxml import logging import pytz -import re import socket import time import xmlrpclib @@ -21,20 +20,12 @@ from lxml import etree from werkzeug import url_encode from odoo import _, api, exceptions, fields, models, tools -from odoo.addons.mail.models.mail_message import decode from odoo.tools.safe_eval import safe_eval _logger = logging.getLogger(__name__) -mail_header_msgid_re = re.compile('<[^<>]+>') - - -def decode_header(message, header, separator=' '): - return separator.join(map(decode, filter(None, message.get_all(header, [])))) - - class MailThread(models.AbstractModel): ''' mail_thread model is meant to be inherited by any model that needs to act as a discussion topic on which messages can be attached. Public @@ -795,7 +786,7 @@ class MailThread(models.AbstractModel): """ Find partners related to some header fields of the message. :param string message: an email.message instance """ - s = ', '.join([decode(message.get(h)) for h in header_fields if message.get(h)]) + s = ', '.join([tools.decode_smtp_header(message.get(h)) for h in header_fields if message.get(h)]) return filter(lambda x: x, self._find_partner_from_emails(tools.email_split(s))) @api.model @@ -819,13 +810,13 @@ class MailThread(models.AbstractModel): assert len(route) == 5, 'A route should contain 5 elements: model, thread_id, custom_values, uid, alias record' message_id = message.get('Message-Id') - email_from = decode_header(message, 'From') + email_from = tools.decode_message_header(message, 'From') author_id = message_dict.get('author_id') model, thread_id, alias = route[0], route[1], route[4] record_set = None def _create_bounce_email(): - bounce_to = decode_header(message, 'Return-Path') or email_from + bounce_to = tools.decode_smtp_header(message.get('Return-Path')) or email_from bounce_mail_values = { 'body_html': '

Hello,

' '

The following email sent to %s cannot be accepted because this is ' @@ -979,10 +970,10 @@ class MailThread(models.AbstractModel): # Get email.message.Message variables for future processing message_id = message.get('Message-Id') - email_from = decode_header(message, 'From') - email_to = decode_header(message, 'To') - references = decode_header(message, 'References') - in_reply_to = decode_header(message, 'In-Reply-To').strip() + email_from = tools.decode_message_header(message, 'From') + email_to = tools.decode_message_header(message, 'To') + references = tools.decode_message_header(message, 'References') + in_reply_to = tools.decode_message_header(message, 'In-Reply-To').strip() thread_references = references or in_reply_to # 0. First check if this is a bounce message or not. @@ -996,8 +987,8 @@ class MailThread(models.AbstractModel): return [] # 1. message is a reply to an existing message (exact match of message_id) - ref_match = thread_references and tools.reference_re.search(thread_references) - msg_references = mail_header_msgid_re.findall(thread_references) + ref_match, reply_model, reply_thread_id, reply_hostname = tools.email_references(thread_references) + msg_references = tools.mail_header_msgid_re.findall(thread_references) mail_messages = MailMessage.sudo().search([('message_id', 'in', msg_references)], limit=1) if ref_match and mail_messages: model, thread_id = mail_messages.model, mail_messages.res_id @@ -1017,9 +1008,6 @@ class MailThread(models.AbstractModel): # 2. message is a reply to an existign thread (6.1 compatibility) if ref_match: - reply_thread_id = int(ref_match.group(1)) - reply_model = ref_match.group(2) or fallback_model - reply_hostname = ref_match.group(3) local_hostname = socket.gethostname() # do not match forwarded emails from another OpenERP system (thread_id collision!) if local_hostname == reply_hostname: @@ -1071,11 +1059,11 @@ class MailThread(models.AbstractModel): # Delivered-To is a safe bet in most modern MTAs, but we have to fallback on To + Cc values # for all the odd MTAs out there, as there is no standard header for the envelope's `rcpt_to` value. rcpt_tos = \ - ','.join([decode_header(message, 'Delivered-To'), - decode_header(message, 'To'), - decode_header(message, 'Cc'), - decode_header(message, 'Resent-To'), - decode_header(message, 'Resent-Cc')]) + ','.join([tools.decode_message_header(message, 'Delivered-To'), + tools.decode_message_header(message, 'To'), + tools.decode_message_header(message, 'Cc'), + tools.decode_message_header(message, 'Resent-To'), + tools.decode_message_header(message, 'Resent-Cc')]) local_parts = [e.split('@')[0] for e in tools.email_split(rcpt_tos)] if local_parts: aliases = Alias.search([('alias_name', 'in', local_parts)]) @@ -1105,7 +1093,7 @@ class MailThread(models.AbstractModel): # 5. Fallback to the provided parameters, if they work if not thread_id: # Legacy: fallback to matching [ID] in the Subject - match = tools.res_re.search(decode_header(message, 'Subject')) + match = tools.res_re.search(tools.decode_message_header(message, 'Subject')) thread_id = match and match.group(1) # Convert into int (bug spotted in 7.0 because of str) try: @@ -1355,7 +1343,7 @@ class MailThread(models.AbstractModel): # RFC2231 filename = email.utils.collapse_rfc2231_value(filename).strip() else: - filename = decode(filename) + filename = tools.decode_smtp_header(filename) encoding = part.get_content_charset() # None if attachment # 0) Inline Attachments -> attachments, with a third part in the tuple to match cid / attachment @@ -1432,19 +1420,19 @@ class MailThread(models.AbstractModel): msg_dict['message_id'] = message_id if message.get('Subject'): - msg_dict['subject'] = decode(message.get('Subject')) + msg_dict['subject'] = tools.decode_smtp_header(message.get('Subject')) # Envelope fields not stored in mail.message but made available for message_new() - msg_dict['from'] = decode(message.get('from')) - msg_dict['to'] = decode(message.get('to')) - msg_dict['cc'] = decode(message.get('cc')) - msg_dict['email_from'] = decode(message.get('from')) + msg_dict['from'] = tools.decode_smtp_header(message.get('from')) + msg_dict['to'] = tools.decode_smtp_header(message.get('to')) + msg_dict['cc'] = tools.decode_smtp_header(message.get('cc')) + msg_dict['email_from'] = tools.decode_smtp_header(message.get('from')) partner_ids = self._message_find_partners(message, ['To', 'Cc']) msg_dict['partner_ids'] = [(4, partner_id) for partner_id in partner_ids] if message.get('Date'): try: - date_hdr = decode(message.get('Date')) + date_hdr = tools.decode_smtp_header(message.get('Date')) parsed_date = dateutil.parser.parse(date_hdr, fuzzy=True) if parsed_date.utcoffset() is None: # naive datetime, so we arbitrarily decide to make it @@ -1461,13 +1449,13 @@ class MailThread(models.AbstractModel): msg_dict['date'] = stored_date.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT) if message.get('In-Reply-To'): - parent_ids = self.env['mail.message'].search([('message_id', '=', decode(message['In-Reply-To'].strip()))], limit=1) + parent_ids = self.env['mail.message'].search([('message_id', '=', tools.decode_smtp_header(message['In-Reply-To'].strip()))], limit=1) if parent_ids: msg_dict['parent_id'] = parent_ids.id msg_dict['internal'] = parent_ids.subtype_id and parent_ids.subtype_id.internal or False if message.get('References') and 'parent_id' not in msg_dict: - msg_list = mail_header_msgid_re.findall(decode(message['References'])) + msg_list = tools.mail_header_msgid_re.findall(tools.decode_smtp_header(message['References'])) parent_ids = self.env['mail.message'].search([('message_id', 'in', [x.strip() for x in msg_list])], limit=1) if parent_ids: msg_dict['parent_id'] = parent_ids.id diff --git a/addons/mass_mailing/models/mail_thread.py b/addons/mass_mailing/models/mail_thread.py index 1c0c8a38478..0931271509d 100644 --- a/addons/mass_mailing/models/mail_thread.py +++ b/addons/mass_mailing/models/mail_thread.py @@ -5,10 +5,7 @@ import logging import re from odoo import api, models - -from odoo.addons.mail.models.mail_message import decode -from odoo.addons.mail.models.mail_thread import decode_header - +from odoo.tools import decode_smtp_header, decode_message_header _logger = logging.getLogger(__name__) @@ -24,8 +21,8 @@ class MailThread(models.AbstractModel): return False to end the routing process. """ bounce_alias = self.env['ir.config_parameter'].get_param("mail.bounce.alias") message_id = message.get('Message-Id') - email_from = decode_header(message, 'From') - email_to = decode_header(message, 'To') + email_from = decode_message_header(message, 'From') + email_to = decode_message_header(message, 'To') # 0. Verify whether this is a bounced email (wrong destination,...) -> use it to collect data, such as dead leads if bounce_alias and bounce_alias in email_to: @@ -70,6 +67,6 @@ class MailThread(models.AbstractModel): by using the References header of the incoming message and looking for matching message_id in mail.mail.statistics. """ if message.get('References'): - message_ids = [x.strip() for x in decode(message['References']).split()] + message_ids = [x.strip() for x in decode_smtp_header(message['References']).split()] self.env['mail.mail.statistics'].set_replied(mail_message_ids=message_ids) return super(MailThread, self).message_route_process(message, message_dict, routes) diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index 1f8e8d6f67f..3c9953677fd 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -1,21 +1,21 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from lxml import etree import cgi import logging -import lxml.html import lxml.html.clean as clean import random import re import socket import threading import time + +from email.header import decode_header from email.utils import getaddresses, formataddr +from lxml import etree import openerp from openerp.loglevels import ustr -from openerp.tools.translate import _ _logger = logging.getLogger(__name__) @@ -403,7 +403,10 @@ email_re = re.compile(r"""([a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,63})""", # matches a string containing only one email single_email_re = re.compile(r"""^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,63}$""", re.VERBOSE) +# legacy behavior: match [ID] in the Subject, used with the fallback model of the mail gateway res_re = re.compile(r"\[([0-9]+)\]", re.UNICODE) + +# update command in emails body command_re = re.compile("^Set-([a-z]+) *: *(.+)$", re.I + re.UNICODE) # Updated in 7.0 to match the model name as well @@ -411,6 +414,8 @@ command_re = re.compile("^Set-([a-z]+) *: *(.+)$", re.I + re.UNICODE) # group(1) = the record ID ; group(2) = the model (if any) ; group(3) = the domain reference_re = re.compile("<.*-open(?:object|erp)-(\\d+)(?:-([\w.]+))?[^>]*@([^>]*)>", re.UNICODE) +mail_header_msgid_re = re.compile('<[^<>]+>') + def generate_tracking_message_id(res_id): """Returns a string that can be used in the Message-ID RFC822 header field @@ -487,3 +492,30 @@ def email_split_and_format(text): # is strictly required in RFC2822's `addr-spec`. if addr[1] if '@' in addr[1]] + +def email_references(references): + ref_match, model, thread_id, hostname = False, False, False, False + if references: + ref_match = reference_re.search(references) + if ref_match: + model = ref_match.group(2) + thread_id = int(ref_match.group(1)) + hostname = ref_match.group(3) + return (ref_match, model, thread_id, hostname) + +# was mail_message.decode() +def decode_smtp_header(smtp_header): + """Returns unicode() string conversion of the given encoded smtp header + text. email.header decode_header method return a decoded string and its + charset for each decoded par of the header. This method unicodes the + decoded header and join them in a complete string. """ + if smtp_header: + text = decode_header(smtp_header.replace('\r', '')) + # The joining space will not be needed as of Python 3.3 + # See https://hg.python.org/cpython/rev/8c03fe231877 + return ' '.join([ustr(x[0], x[1]) for x in text]) + return u'' + +# was mail_thread.decode_header() +def decode_message_header(message, header, separator=' '): + return separator.join(map(decode_smtp_header, filter(None, message.get_all(header, [])))) From 0028c42136f02fcb7a9a81ed5763396290ffdcb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 17 Aug 2016 13:45:57 +0200 Subject: [PATCH 04/17] [CLEAN] mail: clean mail gateway code The purpose of this commit is to clean a bit the code related to email routing in the mailgateway: message_route and message_route_verify. message_route is now clearly separeted into two categories. First one handles emails considered as answers to existing threads. Second one handles emails that have to create new threads, either through alias or default behavior of the mailgateway. message_route_verify has been a bit facotrized to simplify the various overrides that appeared lately in the codebase. This commit only contains code cleaning. No functional change should occur due to this commit. --- addons/hr/models/mail_thread.py | 39 ++-- addons/mail/models/mail_thread.py | 349 +++++++++++++++--------------- openerp/tools/mail.py | 9 +- 3 files changed, 191 insertions(+), 206 deletions(-) diff --git a/addons/hr/models/mail_thread.py b/addons/hr/models/mail_thread.py index 2e756883856..06eec55240d 100644 --- a/addons/hr/models/mail_thread.py +++ b/addons/hr/models/mail_thread.py @@ -3,7 +3,7 @@ import logging -from odoo import api, models +from odoo import api, models, _ from odoo.tools import decode_message_header, email_split _logger = logging.getLogger(__name__) @@ -13,35 +13,24 @@ class MailThread(models.AbstractModel): _inherit = "mail.thread" @api.model - def message_route_verify(self, message, message_dict, route, update_author=True, assert_model=True, create_fallback=True, allow_private=False): - res = super(MailThread, self).message_route_verify(message, message_dict, route, update_author, assert_model, create_fallback, allow_private) + def message_route_verify(self, message, message_dict, route, + update_author=True, assert_model=True, + create_fallback=True, allow_private=False, + drop_alias=False): + res = super(MailThread, self).message_route_verify( + message, message_dict, route, + update_author=update_author, + assert_model=assert_model, + create_fallback=create_fallback, + allow_private=allow_private, + drop_alias=drop_alias) if res: alias = route[4] email_from = decode_message_header(message, 'From') message_id = message.get('Message-Id') - # Identically equal to the definition in mail module because sub methods are local - # variables and cannot be called with super - def _create_bounce_email(body_html): - bounce_to = decode_message_header(message, 'Return-Path') or email_from - bounce_mail_values = { - 'body_html': body_html, - 'subject': 'Re: %s' % message.get('subject'), - 'email_to': bounce_to, - 'auto_delete': True, - } - bounce_from = self.env['ir.mail_server']._get_default_bounce_address() - if bounce_from: - bounce_mail_values['email_from'] = 'MAILER-DAEMON <%s>' % bounce_from - self.env['mail.mail'].create(bounce_mail_values).send() - - def _warn(message): - _logger.info('Routing mail with Message-Id %s: route %s: %s', - message_id, route, message) - # Alias: check alias_contact settings for employees - if alias and alias.alias_contact == 'employees': email_address = email_split(email_from)[0] employee = self.env['hr.employee'].search([('work_email', 'ilike', email_address)], limit=1) @@ -49,7 +38,7 @@ class MailThread(models.AbstractModel): employee = self.env['hr.employee'].search([('user_id.email', 'ilike', email_address)], limit=1) if not employee: mail_template = self.env.ref('hr.mail_template_data_unknown_employee_email_address') - _warn('alias %s does not accept unknown employees, skipping' % alias.alias_name) - _create_bounce_email(mail_template.body_html) + self._routing_warn(_('alias %s does not accept unknown employees') % alias.alias_name, _('skipping'), message_id, route, False) + self._routing_create_bounce_email(email_from, mail_template.body_html, message) return False return res diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 194d7dc25e4..fe15cc9c466 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -789,8 +789,32 @@ class MailThread(models.AbstractModel): s = ', '.join([tools.decode_smtp_header(message.get(h)) for h in header_fields if message.get(h)]) return filter(lambda x: x, self._find_partner_from_emails(tools.email_split(s))) + def _routing_warn(self, error_message, warn_suffix, message_id, route, raise_exception): + """ Tools method used in message_route_verify: whether to log a warning or raise an error """ + full_message = _('Routing mail with Message-Id %s: route %s: %s') % (message_id, route, error_message) + if raise_exception: + raise ValueError(full_message) + else: + _logger.info(full_message + warn_suffix and '; %s' % warn_suffix or '') + + def _routing_create_bounce_email(self, email_from, body_html, message): + bounce_to = tools.decode_message_header(message, 'Return-Path') or email_from + bounce_mail_values = { + 'body_html': body_html, + 'subject': 'Re: %s' % message.get('subject'), + 'email_to': bounce_to, + 'auto_delete': True, + } + bounce_from = self.env['ir.mail_server']._get_default_bounce_address() + if bounce_from: + bounce_mail_values['email_from'] = 'MAILER-DAEMON <%s>' % bounce_from + self.env['mail.mail'].create(bounce_mail_values).send() + @api.model - def message_route_verify(self, message, message_dict, route, update_author=True, assert_model=True, create_fallback=True, allow_private=False): + def message_route_verify(self, message, message_dict, route, + update_author=True, assert_model=True, + create_fallback=True, allow_private=False, + drop_alias=False): """ Verify route validity. Check and rules: 1 - if thread_id -> check that document effectively exists; otherwise fallback on a message_new by resetting thread_id @@ -804,6 +828,22 @@ class MailThread(models.AbstractModel): check on alias parent document that the author is in the followers 'partners': check that author_id id set + + :param message: an email.message instance + :param message_dict: dictionary of values that will be given to + mail_message.create() + :param route: route to check which is a tuple (model, thread_id, + custom_values, uid, alias) + :param update_author: update message_dict['author_id']. TDE TODO: move me + :param assert_model: if an error occurs, tell whether to raise an error + or just log a warning and try other processing or + invalidate route + :param create_fallback: if the route aims at updating a record and that + record does not exists or does not support update + either fallback on creating a new record in the + same model or raise / warn + :param allow_private: allow void model / thread_id routes, aka private + discussions """ assert isinstance(route, (list, tuple)), 'A route should be a list or a tuple' @@ -815,88 +855,55 @@ class MailThread(models.AbstractModel): model, thread_id, alias = route[0], route[1], route[4] record_set = None - def _create_bounce_email(): - bounce_to = tools.decode_smtp_header(message.get('Return-Path')) or email_from - bounce_mail_values = { - 'body_html': '

Hello,

' - '

The following email sent to %s cannot be accepted because this is ' - 'a private email address. Only allowed people can contact us at this address.

' - '
%s
' % (message.get('to'), message_dict.get('body')), - 'subject': 'Re: %s' % message.get('subject'), - 'email_to': bounce_to, - 'auto_delete': True, - } - bounce_from = self.env['ir.mail_server']._get_default_bounce_address() - if bounce_from: - bounce_mail_values['email_from'] = 'MAILER-DAEMON <%s>' % bounce_from - self.env['mail.mail'].create(bounce_mail_values).send() - - def _warn(message): - _logger.info('Routing mail with Message-Id %s: route %s: %s', - message_id, route, message) + _generic_bounce_body_html = """
+

Hello,

+

The following email sent to %s cannot be accepted because this is a private email address. + Only allowed people can contact us at this address.

+
%s
""" % (message.get('to'), message_dict.get('body')), # Wrong model if model and model not in self.env: - if assert_model: - assert model in self.env, 'Routing: unknown target model %s' % model - _warn('unknown target model %s' % model) + self._routing_warn(_('unknown target model %s') % model, '', message_id, route, assert_model) return () - # Private message: should not contain any thread_id - if not model and thread_id: - if assert_model: - if thread_id: - raise ValueError('Routing: posting a message without model should be with a null res_id (private message), received %s.' % thread_id) - _warn('posting a message without model should be with a null res_id (private message), received %s resetting thread_id' % thread_id) - thread_id = 0 - # Private message: should have a parent_id (only answers) - if not model and not message_dict.get('parent_id'): - if assert_model: - if not message_dict.get('parent_id'): - raise ValueError('Routing: posting a message without model should be with a parent_id (private mesage).') - _warn('posting a message without model should be with a parent_id (private mesage), skipping') - return False + # Private message + if not model: + # should not contain any thread_id + if thread_id: + self._routing_warn(_('posting a message without model should be with a null res_id (private message), received %s') % thread_id, _('resetting thread_id'), message_id, route, assert_model) + thread_id = 0 + # should have a parent_id (only answers) + if not message_dict.get('parent_id'): + self._routing_warn(_('posting a message without model should be with a parent_id (private message)'), _('skipping'), message_id, route, assert_model) + return False if model and thread_id: record_set = self.env[model].browse(thread_id) elif model: record_set = self.env[model] - # Existing Document: check if exists; if not, fallback on create if allowed - if thread_id and not record_set.exists(): - if create_fallback: - _warn('reply to missing document (%s,%s), fall back on new document creation' % (model, thread_id)) + # Existing Document: check if exists and model accepts the mailgateway; if not, fallback on create if allowed + if thread_id: + if not record_set.exists() and create_fallback: + self._routing_warn(_('reply to missing document (%s,%s), fall back on new document creation') % (model, thread_id), '', message_id, route, False) + thread_id = None + elif not hasattr(record_set, 'message_update') and create_fallback: + self._routing_warn(_('model %s does not accept document update, fall back on document creation') % model, '', message_id, route, False) thread_id = None - elif assert_model: - # TDE FIXME: change assert to some real error - assert record_set.exists(), 'Routing: reply to missing document (%s,%s)' % (model, thread_id) - else: - _warn('reply to missing document (%s,%s), skipping' % (model, thread_id)) - return False - # Existing Document: check model accepts the mailgateway - if thread_id and model and not hasattr(record_set, 'message_update'): - if create_fallback: - _warn('model %s does not accept document update, fall back on document creation' % model) - thread_id = None - elif assert_model: - assert hasattr(record_set, 'message_update'), 'Routing: model %s does not accept document update, crashing' % model - else: - _warn('model %s does not accept document update, skipping' % model) + if not record_set.exists(): + self._routing_warn(_('reply to missing document (%s,%s)') % (model, thread_id), _('skipping'), message_id, route, assert_model) + return False + elif not hasattr(record_set, 'message_update'): + self._routing_warn(_('model %s does not accept document update') % model, _('skipping'), message_id, route, assert_model) return False # New Document: check model accepts the mailgateway if not thread_id and model and not hasattr(record_set, 'message_new'): - if assert_model: - if not hasattr(record_set, 'message_new'): - raise ValueError( - 'Model %s does not accept document creation, crashing' % model - ) - _warn('model %s does not accept document creation, skipping' % model) + self._routing_warn(_('model %s does not accept document creation') % model, _('skipping'), message_id, route, assert_model) return False - # Update message author if asked - # We do it now because we need it for aliases (contact settings) + # Update message author if asked. We do it now because we need it for aliases (contact settings) if not author_id and update_author: author_ids = self.env['mail.thread']._find_partner_from_emails([email_from], res_model=model, res_id=thread_id) if author_ids: @@ -914,140 +921,132 @@ class MailThread(models.AbstractModel): set(partner.id for channel in obj.message_channel_ids for partner in channel.channel_partner_ids) ) if not author_id or author_id not in accepted_partner_ids: - _warn('alias %s restricted to internal followers, skipping' % alias.alias_name) - _create_bounce_email() + self._routing_warn(_('alias %s restricted to internal followers') % alias.alias_name, _('skipping'), message_id, route, False) + self._routing_create_bounce_email(email_from, _generic_bounce_body_html, message) return False elif alias and alias.alias_contact == 'partners' and not author_id: - _warn('alias %s does not accept unknown author, skipping' % alias.alias_name) - _create_bounce_email() + self._routing_warn(_('alias %s does not accept unknown author') % alias.alias_name, _('skipping'), message_id, route, False) + self._routing_create_bounce_email(email_from, _generic_bounce_body_html, message) return False if not model and not thread_id and not alias and not allow_private: - return () + return False - return (model, thread_id, route[2], route[3], None if self._context.get('drop_alias', False) else route[4]) + return (model, thread_id, route[2], route[3], None if drop_alias else route[4]) @api.model def message_route(self, message, message_dict, model=None, thread_id=None, custom_values=None): - """Attempt to figure out the correct target model, thread_id, + """ Attempt to figure out the correct target model, thread_id, custom_values and user_id to use for an incoming message. Multiple values may be returned, if a message had multiple recipients matching existing mail.aliases, for example. The following heuristics are used, in this order: - 1. If the message replies to an existing thread_id, and - properly contains the thread model in the 'In-Reply-To' - header, use this model/thread_id pair, and ignore - custom_value (not needed as no creation will take place) - 2. Look for a mail.alias entry matching the message - recipient, and use the corresponding model, thread_id, - custom_values and user_id. - 3. Fallback to the ``model``, ``thread_id`` and ``custom_values`` - provided. - 4. If all the above fails, raise an exception. - :param string message: an email.message instance - :param dict message_dict: dictionary holding message variables - :param string model: the fallback model to use if the message - does not match any of the currently configured mail aliases - (may be None if a matching alias is supposed to be present) - :type dict custom_values: optional dictionary of default field values - to pass to ``message_new`` if a new record needs to be created. - Ignored if the thread record already exists, and also if a - matching mail.alias was found (aliases define their own defaults) - :param int thread_id: optional ID of the record/thread from ``model`` - to which this mail should be attached. Only used if the message - does not reply to an existing thread and does not match any mail alias. - :return: list of [model, thread_id, custom_values, user_id, alias] + * if the message replies to an existing thread by having a Message-Id + that matches an existing mail_message.message_id, we take the original + message model/thread_id pair and ignore custom_value as no creation will + take place + * if the message replies to an existing thread by having In-Reply-To or + References matching odoo model/thread_id Message-Id and if this thread + has messages without message_id, take this model/thread_id pair and + ignore custom_value as no creation will take place (6.1 compatibility) + * look for a mail.alias entry matching the message recipients and use the + corresponding model, thread_id, custom_values and user_id. This could + lead to a thread update or creation depending on the alias + * fallback on provided ``model``, ``thread_id`` and ``custom_values`` + * raise an exception as no route has been found + + :param string message: an email.message instance + :param dict message_dict: dictionary holding parsed message variables + :param string model: the fallback model to use if the message does not match + any of the currently configured mail aliases (may be None if a matching + alias is supposed to be present) + :type dict custom_values: optional dictionary of default field values + to pass to ``message_new`` if a new record needs to be created. + Ignored if the thread record already exists, and also if a matching + mail.alias was found (aliases define their own defaults) + :param int thread_id: optional ID of the record/thread from ``model`` to + which this mail should be attached. Only used if the message does not + reply to an existing thread and does not match any mail alias. + :return: list of routes [(model, thread_id, custom_values, user_id, alias)] :raises: ValueError, TypeError """ if not isinstance(message, Message): raise TypeError('message must be an email.message.Message at this point') MailMessage = self.env['mail.message'] - Alias = self.env['mail.alias'] + Alias, dest_aliases = self.env['mail.alias'], self.env['mail.alias'] fallback_model = model - # Get email.message.Message variables for future processing + # get email.message.Message variables for future processing + local_hostname = socket.gethostname() message_id = message.get('Message-Id') - email_from = tools.decode_message_header(message, 'From') - email_to = tools.decode_message_header(message, 'To') + + # compute references to find if message is a reply to an existing thread references = tools.decode_message_header(message, 'References') in_reply_to = tools.decode_message_header(message, 'In-Reply-To').strip() thread_references = references or in_reply_to + reply_match, reply_model, reply_thread_id, reply_hostname, reply_private = tools.email_references(thread_references) + + # author and recipients + email_from = tools.decode_message_header(message, 'From') + email_from_localpart = (tools.email_split(email_from) or [''])[0].split('@', 1)[0].lower() + email_to = tools.decode_message_header(message, 'To') + email_to_localpart = (tools.email_split(email_to) or [''])[0].split('@', 1)[0].lower() + + # Delivered-To is a safe bet in most modern MTAs, but we have to fallback on To + Cc values + # for all the odd MTAs out there, as there is no standard header for the envelope's `rcpt_to` value. + rcpt_tos = ','.join([ + tools.decode_message_header(message, 'Delivered-To'), + tools.decode_message_header(message, 'To'), + tools.decode_message_header(message, 'Cc'), + tools.decode_message_header(message, 'Resent-To'), + tools.decode_message_header(message, 'Resent-Cc')]) + rcpt_tos_localparts = [e.split('@')[0] for e in tools.email_split(rcpt_tos)] # 0. First check if this is a bounce message or not. # See http://datatracker.ietf.org/doc/rfc3462/?include_text=1 # As all MTA does not respect this RFC (googlemail is one of them), # we also need to verify if the message come from "mailer-daemon" - localpart = (tools.email_split(email_from) or [''])[0].split('@', 1)[0].lower() - if message.get_content_type() == 'multipart/report' or localpart == 'mailer-daemon': - _logger.info("Not routing bounce email from %s to %s with Message-Id %s", - email_from, email_to, message_id) + if message.get_content_type() == 'multipart/report' or email_from_localpart == 'mailer-daemon': + _logger.info('Routing mail with Message-Id %s: not routing bounce email from %s to %s', + message_id, email_from, email_to) return [] - # 1. message is a reply to an existing message (exact match of message_id) - ref_match, reply_model, reply_thread_id, reply_hostname = tools.email_references(thread_references) - msg_references = tools.mail_header_msgid_re.findall(thread_references) - mail_messages = MailMessage.sudo().search([('message_id', 'in', msg_references)], limit=1) - if ref_match and mail_messages: - model, thread_id = mail_messages.model, mail_messages.res_id - alias = Alias.search([('alias_name', '=', (tools.email_split(email_to) or [''])[0].split('@', 1)[0].lower())]) - alias = alias[0] if alias else None - route = self.with_context(drop_alias=True).message_route_verify( - message, message_dict, - (model, thread_id, custom_values, self._uid, alias), - update_author=True, assert_model=False, create_fallback=True) - if route: - _logger.info( - 'Routing mail from %s to %s with Message-Id %s: direct reply to msg: model: %s, thread_id: %s, custom_values: %s, uid: %s', - email_from, email_to, message_id, model, thread_id, custom_values, self._uid) - return [route] - elif route is False: - return [] + # 1. message is a reply to an existing message (exact match of message_id or compat-mode) + if reply_match: + compat_mode = False + msg_references = tools.mail_header_msgid_re.findall(thread_references) + mail_messages = MailMessage.sudo().search([('message_id', 'in', msg_references)], limit=1) - # 2. message is a reply to an existign thread (6.1 compatibility) - if ref_match: - local_hostname = socket.gethostname() + # message is a reply to an existing thread (6.1 compatibility) # do not match forwarded emails from another OpenERP system (thread_id collision!) - if local_hostname == reply_hostname: - thread_id, model = reply_thread_id, reply_model - if thread_id and model in self.env: - record = self.env[model].browse(thread_id) - compat_mail_msg_ids = MailMessage.search([ - ('message_id', '=', False), - ('model', '=', model), - ('res_id', '=', thread_id)]) - if compat_mail_msg_ids and record.exists() and hasattr(record, 'message_update'): - route = self.message_route_verify( - message, message_dict, - (model, thread_id, custom_values, self._uid, None), - update_author=True, assert_model=True, create_fallback=True) - if route: - # parent is invalid for a compat-reply - message_dict.pop('parent_id', None) - _logger.info( - 'Routing mail from %s to %s with Message-Id %s: direct thread reply (compat-mode) to model: %s, thread_id: %s, custom_values: %s, uid: %s', - email_from, email_to, message_id, model, thread_id, custom_values, self._uid) - return [route] - elif route is False: - return [] + if not mail_messages and local_hostname == reply_hostname and reply_thread_id and reply_model in self.env: + mail_messages = MailMessage.search([ + ('message_id', '=', False), + ('model', '=', reply_model), + ('res_id', '=', reply_thread_id)]) + compat_mode = True - # 3. Reply to a private message - if in_reply_to: - mail_message_ids = MailMessage.search([ - ('message_id', '=', in_reply_to), - '!', ('message_id', 'ilike', 'reply_to') - ], limit=1) - if mail_message_ids: + if mail_messages: + model, thread_id = mail_messages.model, mail_messages.res_id + if not reply_private: # TDE note: not sure why private mode as no alias search, copying existing behavior + dest_aliases = Alias.search([('alias_name', '=', email_to_localpart)], limit=1) + + # TDE Note: compat mode = without context key, why ? because route = self.message_route_verify( message, message_dict, - (mail_message_ids.model, mail_message_ids.res_id, custom_values, self._uid, None), - update_author=True, assert_model=True, create_fallback=True, allow_private=True) + (model, thread_id, custom_values, self._uid, dest_aliases), + update_author=True, assert_model=reply_private, create_fallback=True, + allow_private=reply_private, drop_alias=True) if route: + # TDE Note: compat mode: parent is invalid for a compat-reply + # message_dict.pop('parent_id', None) + # TDE note: add compat mode for compat mode in debug _logger.info( - 'Routing mail from %s to %s with Message-Id %s: direct reply to a private message: %s, custom_values: %s, uid: %s', - email_from, email_to, message_id, mail_message_ids.id, custom_values, self._uid) + 'Routing mail from %s to %s with Message-Id %s%s: direct reply to msg: model: %s, thread_id: %s, custom_values: %s, uid: %s', + email_from, email_to, message_id, compat_mode and ' (compat mode)' or '', model, thread_id, custom_values, self._uid) return [route] elif route is False: return [] @@ -1056,20 +1055,11 @@ class MailThread(models.AbstractModel): message_dict.pop('parent_id', None) # 4. Look for a matching mail.alias entry - # Delivered-To is a safe bet in most modern MTAs, but we have to fallback on To + Cc values - # for all the odd MTAs out there, as there is no standard header for the envelope's `rcpt_to` value. - rcpt_tos = \ - ','.join([tools.decode_message_header(message, 'Delivered-To'), - tools.decode_message_header(message, 'To'), - tools.decode_message_header(message, 'Cc'), - tools.decode_message_header(message, 'Resent-To'), - tools.decode_message_header(message, 'Resent-Cc')]) - local_parts = [e.split('@')[0] for e in tools.email_split(rcpt_tos)] - if local_parts: - aliases = Alias.search([('alias_name', 'in', local_parts)]) - if aliases: + if rcpt_tos_localparts: + dest_aliases = Alias.search([('alias_name', 'in', rcpt_tos_localparts)]) + if dest_aliases: routes = [] - for alias in aliases: + for alias in dest_aliases: user_id = alias.alias_user_id.id if not user_id: # TDE note: this could cause crashes, because no clue that the user @@ -1100,15 +1090,16 @@ class MailThread(models.AbstractModel): thread_id = int(thread_id) except: thread_id = False - route = self.message_route_verify( - message, message_dict, - (fallback_model, thread_id, custom_values, self._uid, None), - update_author=True, assert_model=True) - if route: - _logger.info( - 'Routing mail from %s to %s with Message-Id %s: fallback to model:%s, thread_id:%s, custom_values:%s, uid:%s', - email_from, email_to, message_id, fallback_model, thread_id, custom_values, self._uid) - return [route] + if fallback_model: + route = self.message_route_verify( + message, message_dict, + (fallback_model, thread_id, custom_values, self._uid, None), + update_author=True, assert_model=True) + if route: + _logger.info( + 'Routing mail from %s to %s with Message-Id %s: fallback to model:%s, thread_id:%s, custom_values:%s, uid:%s', + email_from, email_to, message_id, fallback_model, thread_id, custom_values, self._uid) + return [route] # ValueError if no routes found and if no bounce occured raise ValueError( diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index 3c9953677fd..9646e4fde74 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -413,6 +413,7 @@ command_re = re.compile("^Set-([a-z]+) *: *(.+)$", re.I + re.UNICODE) # Typical form of references is # group(1) = the record ID ; group(2) = the model (if any) ; group(3) = the domain reference_re = re.compile("<.*-open(?:object|erp)-(\\d+)(?:-([\w.]+))?[^>]*@([^>]*)>", re.UNICODE) +discussion_re = re.compile("<.*-open(?:object|erp)-private[^>]*@([^>]*)>", re.UNICODE) mail_header_msgid_re = re.compile('<[^<>]+>') @@ -494,14 +495,18 @@ def email_split_and_format(text): if '@' in addr[1]] def email_references(references): - ref_match, model, thread_id, hostname = False, False, False, False + ref_match, model, thread_id, hostname, is_private = False, False, False, False, False if references: ref_match = reference_re.search(references) if ref_match: model = ref_match.group(2) thread_id = int(ref_match.group(1)) hostname = ref_match.group(3) - return (ref_match, model, thread_id, hostname) + else: + ref_match = discussion_re.search(references) + if ref_match: + is_private = True + return (ref_match, model, thread_id, hostname, is_private) # was mail_message.decode() def decode_smtp_header(smtp_header): From 6aebdc2ab6ff2dec95aae9b2c04f99e70b41fdbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 17 Aug 2016 17:18:17 +0200 Subject: [PATCH 05/17] [FIX][IMP] mail: check for rcptto when finding destination aliases When a new answer comes in a thread the mailgateway finds if the recipients are linked to existing aliases. This allows notably to check if the author can effectively contact this alias. Previously only the To was checked. However we cannot ensure the recipient is contained in To as it could be in Delivered-To for example. We now check for all possible recipients. --- addons/mail/models/mail_thread.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index fe15cc9c466..d59ed690445 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1032,7 +1032,7 @@ class MailThread(models.AbstractModel): if mail_messages: model, thread_id = mail_messages.model, mail_messages.res_id if not reply_private: # TDE note: not sure why private mode as no alias search, copying existing behavior - dest_aliases = Alias.search([('alias_name', '=', email_to_localpart)], limit=1) + dest_aliases = Alias.search([('alias_name', 'in', rcpt_tos_localparts)], limit=1) # TDE Note: compat mode = without context key, why ? because route = self.message_route_verify( From 0843df213c808df2291f180d44ff9efc3c1c879e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 24 Aug 2016 12:47:45 +0200 Subject: [PATCH 06/17] [FIX][IMP] mail, mass_mailing: improve making of Return-Path alias Previously Return-Path were composed using bounce_alias--- . However this may lead to return adresses not supported by the incoming mail server. We now form the bounce alias using bounce_alis+ --. Using a + indicates that the right part of the address is for information and used to manage the bounce. Only the left part is used for the email routing, allowing the return email to effectively land in the alias mailbox. --- addons/mail/models/mail_mail.py | 4 ++-- addons/mass_mailing/models/mail_thread.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index aecb32cb5ac..5d90c54c0af 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -222,9 +222,9 @@ class MailMail(models.Model): catchall_domain = self.env['ir.config_parameter'].get_param("mail.catchall.domain") if bounce_alias and catchall_domain: if mail.model and mail.res_id: - headers['Return-Path'] = '%s-%d-%s-%d@%s' % (bounce_alias, mail.id, mail.model, mail.res_id, catchall_domain) + headers['Return-Path'] = '%s+%d-%s-%d@%s' % (bounce_alias, mail.id, mail.model, mail.res_id, catchall_domain) else: - headers['Return-Path'] = '%s-%d@%s' % (bounce_alias, mail.id, catchall_domain) + headers['Return-Path'] = '%s+%d@%s' % (bounce_alias, mail.id, catchall_domain) if mail.headers: try: headers.update(safe_eval(mail.headers)) diff --git a/addons/mass_mailing/models/mail_thread.py b/addons/mass_mailing/models/mail_thread.py index 0931271509d..af5782b9360 100644 --- a/addons/mass_mailing/models/mail_thread.py +++ b/addons/mass_mailing/models/mail_thread.py @@ -29,7 +29,7 @@ class MailThread(models.AbstractModel): # Bounce regex # Typical form of bounce is bounce_alias-128-crm.lead-34@domain # group(1) = the mail ID; group(2) = the model (if any); group(3) = the record ID - bounce_re = re.compile("%s-(\d+)-?([\w.]+)?-?(\d+)?" % re.escape(bounce_alias), re.UNICODE) + bounce_re = re.compile("%s\+(\d+)-?([\w.]+)?-?(\d+)?" % re.escape(bounce_alias), re.UNICODE) bounce_match = bounce_re.search(email_to) if bounce_match: bounced_model, bounced_thread_id = None, False From 21949937636a2b0fc5b71a2667eede1750e05175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 11 Dec 2015 18:14:58 +0100 Subject: [PATCH 07/17] [REM] mail: remove support of [ID] matching in subject in mail gateway This is an old compatibility code. It is time to remove it as it is not used sinces ages. --- addons/mail/models/mail_thread.py | 9 --------- addons/mail/tests/test_mail_gateway.py | 9 +-------- openerp/tools/mail.py | 3 --- 3 files changed, 1 insertion(+), 20 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index d59ed690445..9a84e8546a4 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1081,15 +1081,6 @@ class MailThread(models.AbstractModel): return routes # 5. Fallback to the provided parameters, if they work - if not thread_id: - # Legacy: fallback to matching [ID] in the Subject - match = tools.res_re.search(tools.decode_message_header(message, 'Subject')) - thread_id = match and match.group(1) - # Convert into int (bug spotted in 7.0 because of str) - try: - thread_id = int(thread_id) - except: - thread_id = False if fallback_model: route = self.message_route_verify( message, message_dict, diff --git a/addons/mail/tests/test_mail_gateway.py b/addons/mail/tests/test_mail_gateway.py index f5492dc22e9..5720dab2459 100644 --- a/addons/mail/tests/test_mail_gateway.py +++ b/addons/mail/tests/test_mail_gateway.py @@ -560,15 +560,8 @@ class TestMailgateway(TestMail): to='noone@example.com>', subject='spam', extra='In-Reply-To: <12321321-openerp-%d-mail.channel@neighbor.com>' % frog_groups.id) - # 4. Subject contains [] + model passed to message+process -> only attached to group, but not to mail (not in msg1.child_ids) - self.format_and_process( - MAIL_TEMPLATE, - to='erroneous@example.com', email_from='other6@gmail.com', - extra='', subject='Re: [%s] 1' % self.group_public.id, model='mail.channel', - msg_id='<1198923581.41972151344608186803.JavaMail.5@agrolait.com>') - # Test created messages - self.assertEqual(len(self.group_public.message_ids), 3, 'message_process: group should contain 6 messages') + self.assertEqual(len(self.group_public.message_ids), 2, 'message_process: group should contain 6 messages') self.assertEqual(len(self.group_public.message_ids[0].child_ids), 0, 'message_process: msg1 should not have children') @mute_logger('openerp.addons.mail.models.mail_thread', 'openerp.models') diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index 9646e4fde74..da4ec6fb716 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -403,9 +403,6 @@ email_re = re.compile(r"""([a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,63})""", # matches a string containing only one email single_email_re = re.compile(r"""^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,63}$""", re.VERBOSE) -# legacy behavior: match [ID] in the Subject, used with the fallback model of the mail gateway -res_re = re.compile(r"\[([0-9]+)\]", re.UNICODE) - # update command in emails body command_re = re.compile("^Set-([a-z]+) *: *(.+)$", re.I + re.UNICODE) From 72dfcae2a4b03623e780b857abf37c7aec5b8456 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 7 Jul 2016 16:03:05 +0200 Subject: [PATCH 08/17] [IMP] mail: improve notification management for customers Currently it is impossible in Discuss to know whether an email has been sent to a customer and whether it failed or bounced. A notified partner has an entry in the needaction m2m table. In this commit we decorate this table to add fields about the email notification: is an email sent, did it failed, did it bounce. This information is kept only for customers. Internal users does not use this information. Moreover their notification is deleted once the message is read in the Chatter. This avoids having a notification table that grows quickly. Chatter now holds a new icon for email details. It allows to know on a thread status of emails sent to customers. --- addons/mail/models/__init__.py | 1 + addons/mail/models/mail_mail.py | 13 ++++++ addons/mail/models/mail_message.py | 53 ++++++++++++++++++---- addons/mail/models/mail_notification.py | 30 ++++++++++++ addons/mail/models/mail_thread.py | 2 +- addons/mail/models/res_partner.py | 20 +++++++- addons/mail/security/ir.model.access.csv | 3 ++ addons/mail/security/mail_security.xml | 10 ++++ addons/mail/static/src/js/chat_manager.js | 2 + addons/mail/static/src/js/chat_window.js | 1 + addons/mail/static/src/js/client_action.js | 1 + addons/mail/static/src/js/thread.js | 1 + addons/mail/static/src/less/thread.less | 46 +++++++++++++++++-- addons/mail/static/src/xml/thread.xml | 15 ++++++ addons/mail/views/mail_message_views.xml | 22 +++++++-- 15 files changed, 202 insertions(+), 18 deletions(-) create mode 100644 addons/mail/models/mail_notification.py diff --git a/addons/mail/models/__init__.py b/addons/mail/models/__init__.py index 1ba9746eb4e..2426a6dc9e6 100644 --- a/addons/mail/models/__init__.py +++ b/addons/mail/models/__init__.py @@ -4,6 +4,7 @@ import mail_message_subtype import mail_tracking_value import mail_alias import mail_followers +import mail_notification import mail_message import mail_mail import mail_thread diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index 5d90c54c0af..7297c763953 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -129,6 +129,19 @@ class MailMail(models.Model): :param browse_record mail: the mail that was just sent :return: True """ + notif_emails = self.filtered(lambda email: email.notification) + if notif_emails: + notifications = self.env['mail.notification'].search([ + ('mail_message_id', 'in', notif_emails.mapped('mail_message_id').ids), + ('is_email', '=', True)]) + if mail_sent: + notifications.write({ + 'email_status': 'sent', + }) + else: + notifications.write({ + 'email_status': 'exception', + }) if mail_sent: self.sudo().filtered(lambda self: self.auto_delete).unlink() return True diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 8d6a933221b..0f8054d958a 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -82,6 +82,10 @@ class Message(models.Model): help='Need Action') channel_ids = fields.Many2many( 'mail.channel', 'mail_message_mail_channel_rel', string='Channels') + # notifications + notification_ids = fields.One2many( + 'mail.notification', 'mail_message_id', 'Notifications', + auto_join=True, copy=False) # user interface starred_partner_ids = fields.Many2many( 'res.partner', 'mail_message_res_partner_starred_rel', string='Favorited By') @@ -105,7 +109,10 @@ class Message(models.Model): @api.multi def _get_needaction(self): """ Need action on a mail.message = notified on my channel """ - my_messages = self.sudo().filtered(lambda msg: self.env.user.partner_id in msg.needaction_partner_ids) + my_messages = self.env['mail.notification'].sudo().search([ + ('mail_message_id', 'in', self.ids), + ('res_partner_id', '=', self.env.user.partner_id.id), + ('is_read', '=', False)]).mapped('mail_message_id') for message in self: message.needaction = message in my_messages @@ -117,8 +124,8 @@ class Message(models.Model): @api.model def _search_needaction(self, operator, operand): if operator == '=' and operand: - return [('needaction_partner_ids', 'in', self.env.user.partner_id.id)] - return [('needaction_partner_ids', 'not in', self.env.user.partner_id.id)] + return ['&', ('notification_ids.res_partner_id', '=', self.env.user.partner_id.id), ('notification_ids.is_read', '=', False)] + return ['&', ('notification_ids.res_partner_id', '=', self.env.user.partner_id.id), ('notification_ids.is_read', '=', True)] @api.depends('starred_partner_ids') def _get_starred(self): @@ -147,7 +154,8 @@ class Message(models.Model): """ Remove all needactions of the current partner. If channel_ids is given, restrict to messages written in one of those channels. """ partner_id = self.env.user.partner_id.id - if domain is None: + delete_mode = not self.env.user.share # delete employee notifs, keep customer ones + if domain is None and delete_mode: query = "DELETE FROM mail_message_res_partner_needaction_rel WHERE res_partner_id IN %s" args = [(partner_id,)] if channel_ids: @@ -170,7 +178,14 @@ class Message(models.Model): if channel_ids: msg_domain += [('channel_ids', 'in', channel_ids)] unread_messages = self.search(expression.AND([msg_domain, domain])) - unread_messages.sudo().write({'needaction_partner_ids': [(3, partner_id)]}) + notifications = self.env['mail.notification'].sudo().search([ + ('mail_message_id', 'in', unread_messages.ids), + ('res_partner_id', '=', self.env.user.partner_id.id), + ('is_read', '=', False)]) + if delete_mode: + notifications.unlink() + else: + notifications.write({'is_read': True}) ids = unread_messages.mapped('id') notification = {'type': 'mark_as_read', 'message_ids': ids, 'channel_ids': channel_ids} @@ -193,14 +208,20 @@ class Message(models.Model): def set_message_done(self): """ Remove the needaction from messages for the current partner. """ partner_id = self.env.user.partner_id - messages = self.filtered(lambda msg: partner_id in msg.needaction_partner_ids) - if not len(messages): + delete_mode = not self.env.user.share # delete employee notifs, keep customer ones + + notifications = self.env['mail.notification'].sudo().search([ + ('mail_message_id', 'in', self.ids), + ('res_partner_id', '=', partner_id.id), + ('is_read', '=', False)]) + + if not notifications: return - messages.sudo().write({'needaction_partner_ids': [(3, partner_id.id)]}) # notifies changes in messages through the bus. To minimize the number of # notifications, we need to group the messages depending on their channel_ids groups = [] + messages = notifications.mapped('mail_message_id') current_channel_ids = messages[0].channel_ids current_group = [] for record in messages: @@ -215,6 +236,11 @@ class Message(models.Model): current_group = [record.id] current_channel_ids = record.channel_ids + if delete_mode: + notifications.unlink() + else: + notifications.write({'is_read': True}) + for (msg_ids, channel_ids) in groups: notification = {'type': 'mark_as_read', 'message_ids': msg_ids, 'channel_ids': [c.id for c in channel_ids]} self.env['bus.bus'].sendone((self._cr.dbname, 'res.partner', partner_id.id), notification) @@ -270,6 +296,8 @@ class Message(models.Model): partners |= message.partner_ids elif not message.subtype_id and message.partner_ids: # take specified people of message without a subtype (log) partners |= message.partner_ids + if message.needaction_partner_ids: # notified + partners |= message.needaction_partner_ids if message.attachment_ids: attachments |= message.attachment_ids if message.tracking_value_ids: @@ -311,6 +339,11 @@ class Message(models.Model): else: partner_ids = [partner_tree[partner.id] for partner in message.partner_ids if partner.id in partner_tree] + + customer_email_data = [] + for notification in message.notification_ids.filtered(lambda notif: notif.res_partner_id.partner_share): + customer_email_data.append((partner_tree[notification.res_partner_id.id][0], partner_tree[notification.res_partner_id.id][1], notification.email_status)) + attachment_ids = [] for attachment in message.attachment_ids: if attachment.id in attachments_tree: @@ -323,6 +356,10 @@ class Message(models.Model): message_dict.update({ 'author_id': author, 'partner_ids': partner_ids, + 'customer_email_status': (all(d[2] == 'sent' for d in customer_email_data) and 'sent') or + (any(d[2] == 'exception' for d in customer_email_data) and 'exception') or + (any(d[2] == 'bounce' for d in customer_email_data) and 'bounce') or 'ready', + 'customer_email_data': customer_email_data, 'attachment_ids': attachment_ids, 'tracking_value_ids': tracking_value_ids, }) diff --git a/addons/mail/models/mail_notification.py b/addons/mail/models/mail_notification.py new file mode 100644 index 00000000000..e14754b7e8f --- /dev/null +++ b/addons/mail/models/mail_notification.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- + +from odoo import api, fields, models + + +class Notification(models.Model): + _name = 'mail.notification' + _table = 'mail_message_res_partner_needaction_rel' + _rec_name = 'res_partner_id' + _log_access = False + _description = 'Message Notifications' + + mail_message_id = fields.Many2one( + 'mail.message', 'Message', index=True, ondelete='cascade', required=True) + res_partner_id = fields.Many2one( + 'res.partner', 'Needaction Recipient', index=True, ondelete='cascade', required=True) + is_read = fields.Boolean('Is Read', index=True) + is_email = fields.Boolean('Sent by Email', index=True) + email_status = fields.Selection([ + ('ready', 'Ready to Send'), + ('sent', 'Sent'), + ('bounce', 'Bounced'), + ('exception', 'Exception')], 'Email Status', + default='ready', index=True) + + @api.model_cr + def init(self): + self._cr.execute('SELECT indexname FROM pg_indexes WHERE indexname = %s', ('mail_notification_res_partner_id_is_read_email_status_mail_message_id',)) + if not self._cr.fetchone(): + self._cr.execute('CREATE INDEX mail_notification_res_partner_id_is_read_email_status_mail_message_id ON mail_message_res_partner_needaction_rel (res_partner_id, is_read, email_status, mail_message_id)') diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 9a84e8546a4..cea21f5c668 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -190,7 +190,7 @@ class MailThread(models.AbstractModel): # search for unread messages, directly in SQL to improve performances self._cr.execute(""" SELECT msg.res_id FROM mail_message msg RIGHT JOIN mail_message_res_partner_needaction_rel rel - ON rel.mail_message_id = msg.id AND rel.res_partner_id = %s + ON rel.mail_message_id = msg.id AND rel.res_partner_id = %s AND (rel.is_read = false OR rel.is_read IS NULL) WHERE msg.model = %s AND msg.res_id in %s""", (self.env.user.partner_id.id, self._name, tuple(self.ids),)) for result in self._cr.fetchall(): diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 2e47dacc1dc..7c3a96ba36b 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -133,6 +133,17 @@ class Partner(models.Model): emails |= self.env['mail.mail'].create(create_values) return emails, recipients_nbr + @api.model + def _notify_udpate_notifications(self, emails): + for email in emails: + notifications = self.env['mail.notification'].sudo().search([ + ('mail_message_id', '=', email.mail_message_id.id), + ('res_partner_id', 'in', email.recipient_ids.ids)]) + notifications.write({ + 'is_email': True, + 'email_status': 'ready', + }) + @api.multi def _notify(self, message, force_send=False, user_signature=True): # TDE TODO: model-dependant ? (like customer -> always email ?) @@ -166,7 +177,6 @@ class Partner(models.Model): base_template_ctx['signature'] = False base_mail_values = self._notify_prepare_email_values(message) - # classify recipients: actions / no action if message.model and message.res_id and hasattr(self.env[message.model], '_message_notification_recipients'): recipients = self.env[message.model].browse(message.res_id)._message_notification_recipients(message, self) @@ -185,6 +195,9 @@ class Partner(models.Model): fol_values = template_fol.generate_email(message.id, fields=['body_html', 'subject']) # send email new_emails, new_recipients_nbr = self._notify_send(fol_values['body'], fol_values['subject'], recipient_template_values['followers'], **base_mail_values) + # update notifications + self._notify_udpate_notifications(new_emails) + emails |= new_emails recipients_nbr += new_recipients_nbr if recipient_template_values['not_followers']: @@ -196,6 +209,9 @@ class Partner(models.Model): not_values = template_not.generate_email(message.id, fields=['body_html', 'subject']) # send email new_emails, new_recipients_nbr = self._notify_send(not_values['body'], not_values['subject'], recipient_template_values['not_followers'], **base_mail_values) + # update notifications + self._notify_udpate_notifications(new_emails) + emails |= new_emails recipients_nbr += new_recipients_nbr @@ -226,7 +242,7 @@ class Partner(models.Model): self.env.cr.execute(""" SELECT count(*) as needaction_count FROM mail_message_res_partner_needaction_rel R - WHERE R.res_partner_id = %s """, (self.env.user.partner_id.id,)) + WHERE R.res_partner_id = %s AND (R.is_read = false OR R.is_read IS NULL)""", (self.env.user.partner_id.id,)) return self.env.cr.dictfetchall()[0].get('needaction_count') _logger.error('Call to needaction_count without partner_id') return 0 diff --git a/addons/mail/security/ir.model.access.csv b/addons/mail/security/ir.model.access.csv index 384e99d91fa..59a37569bfe 100644 --- a/addons/mail/security/ir.model.access.csv +++ b/addons/mail/security/ir.model.access.csv @@ -10,6 +10,9 @@ access_mail_followers_all,mail.followers.all,model_mail_followers,,1,0,0,0 access_mail_followers_portal,mail.followers.portal,model_mail_followers,base.group_portal,1,1,1,0 access_mail_followers_user,mail.followers.user,model_mail_followers,base.group_user,1,1,1,0 access_mail_followers_system,mail.followers.system,model_mail_followers,base.group_system,1,1,1,1 +access_mail_notification_portal,mail.notification.portal,model_mail_notification,base.group_portal,1,0,0,0 +access_mail_notification_user,mail.notification.user,model_mail_notification,base.group_user,1,1,1,0 +access_mail_notification_system, mail.notification.system,model_mail_notification,base.group_system,1,1,1,1 access_mail_channel_all,mail.group.all,model_mail_channel,,1,0,0,0 access_mail_channel_user,mail.group.user,model_mail_channel,base.group_user,1,1,1,1 access_mail_channel_partner_public,mail.channel.partner.public,model_mail_channel_partner,base.group_public,1,0,0,0 diff --git a/addons/mail/security/mail_security.xml b/addons/mail/security/mail_security.xml index deab4c51438..65495db6925 100644 --- a/addons/mail/security/mail_security.xml +++ b/addons/mail/security/mail_security.xml @@ -23,6 +23,16 @@ + + mail.notifications: group_user: write its own entries + + + [('res_partner_id', '=', user.partner_id.id)] + + + + + mail.message.subtype: portal/public: read public subtypes diff --git a/addons/mail/static/src/js/chat_manager.js b/addons/mail/static/src/js/chat_manager.js index d37e9dcf282..2cf6090871f 100644 --- a/addons/mail/static/src/js/chat_manager.js +++ b/addons/mail/static/src/js/chat_manager.js @@ -132,6 +132,8 @@ function make_message (data) { attachment_ids: data.attachment_ids || [], subject: data.subject, email_from: data.email_from, + customer_email_status: data.customer_email_status, + customer_email_data: data.customer_email_data, record_name: data.record_name, tracking_value_ids: data.tracking_value_ids, channel_ids: data.channel_ids, diff --git a/addons/mail/static/src/js/chat_window.js b/addons/mail/static/src/js/chat_window.js index da8900b8230..7bdece26fb3 100644 --- a/addons/mail/static/src/js/chat_window.js +++ b/addons/mail/static/src/js/chat_window.js @@ -32,6 +32,7 @@ return Widget.extend({ autofocus: true, display_stars: true, display_reply_icon: false, + display_email_icon: false, placeholder: _t("Say something"), input_less: false, }); diff --git a/addons/mail/static/src/js/client_action.js b/addons/mail/static/src/js/client_action.js index f94997c946c..7de0b1ac83b 100644 --- a/addons/mail/static/src/js/client_action.js +++ b/addons/mail/static/src/js/client_action.js @@ -491,6 +491,7 @@ var ChatAction = Widget.extend(ControlPanelMixin, { display_empty_channel: !messages.length && !this.domain.length, display_no_match: !messages.length && this.domain.length, display_subject: this.channel.mass_mailing || this.channel.id === "channel_inbox", + display_email_icon: false, display_reply_icon: true, }; }, diff --git a/addons/mail/static/src/js/thread.js b/addons/mail/static/src/js/thread.js index ea593d21dfa..2176eff55fa 100644 --- a/addons/mail/static/src/js/thread.js +++ b/addons/mail/static/src/js/thread.js @@ -72,6 +72,7 @@ var Thread = Widget.extend({ display_document_link: true, display_avatar: true, squash_close_messages: true, + display_email_icon: true, display_reply_icon: false, }); this.expanded_msg_ids = []; diff --git a/addons/mail/static/src/less/thread.less b/addons/mail/static/src/less/thread.less index 5da7d2b4963..7a9efa45731 100644 --- a/addons/mail/static/src/less/thread.less +++ b/addons/mail/static/src/less/thread.less @@ -70,7 +70,8 @@ } } - i.o_thread_message_star, i.o_thread_message_needaction, i.o_thread_message_reply { + + i.o_thread_message_star, i.o_thread_message_needaction, i.o_thread_message_reply, i.o_thread_message_email { cursor: pointer; opacity: 0; &.fa-star { @@ -84,7 +85,7 @@ opacity: @mail-thread-side-date-opacity; } - i.o_thread_message_star, i.o_thread_message_needaction, i.o_thread_message_reply { + i.o_thread_message_star, i.o_thread_message_needaction, i.o_thread_message_reply, i.o_thread_message_email { opacity: @mail-thread-icon-opacity; &:hover { opacity: 1; @@ -146,9 +147,48 @@ } } - .o_thread_message_star, .o_thread_message_needaction, .o_thread_message_reply { + .o_thread_message_star, .o_thread_message_needaction, .o_thread_message_reply, .o_thread_message_email { padding: 4px; } + + i.o_thread_message_email { + &.o_thread_message_email_ready { + color: grey; + } + &.o_thread_message_email_exception { + color: red; + opacity: 1; + } + &.o_thread_message_email_bounce { + color: red; + opacity: 1; + } + } + + .o_thread_tooltip_container { + display: inline; + position: relative; + + .o_thread_tooltip_content { + top: 10px; + background: @gray-lighter; + padding: 4px; + border: solid @gray-lighter 1px; + border-radius: 5px; + color: gray; + width: 300px; + opacity: 0; + position: absolute; + transition: opacity 0.5s; + i.fa-exclamation { + color: red; + } + } + + .o_thread_tooltip:hover + .o_thread_tooltip_content { + opacity: 1; + } + } } } .o_thread_title { diff --git a/addons/mail/static/src/xml/thread.xml b/addons/mail/static/src/xml/thread.xml index 9896cb0ce6e..47e84e38f60 100644 --- a/addons/mail/static/src/xml/thread.xml +++ b/addons/mail/static/src/xml/thread.xml @@ -94,6 +94,21 @@ (from #) + + + + + + + + + + + +
+
+
+
- - - - + + + + + + + + + + + + + + + + + + From 9c1af690d58068f3831bc7e22878d9b0065f8a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 8 Jul 2016 14:18:08 +0200 Subject: [PATCH 09/17] [IMP] mail: bounce management Handle bounces using bounce alias directly in mailgateway. Previously bounces were used only in mass mailing to update campaign statistics. Now the mailgateway tries to find data from standard delivery status emails. This data is used to update state of emails sent to customers, to display it in the Chatter. It is also used to increment the message_bounce counter on the bounced partner and bounced record, if any and if the field exists. Previous more generic code that detect bounces is kept. This code is more specific to standard delivery failure notifications by parsing its content. --- addons/mail/data/mail_data.xml | 6 +++ addons/mail/models/mail_thread.py | 61 ++++++++++++++++++++++ addons/mass_mailing/__openerp__.py | 1 - addons/mass_mailing/data/mail_data.xml | 12 ----- addons/mass_mailing/models/mail_thread.py | 59 +++++---------------- addons/mass_mailing/models/mass_mailing.py | 8 --- 6 files changed, 81 insertions(+), 66 deletions(-) delete mode 100644 addons/mass_mailing/data/mail_data.xml diff --git a/addons/mail/data/mail_data.xml b/addons/mail/data/mail_data.xml index 874f83d9af1..29a11948c6a 100644 --- a/addons/mail/data/mail_data.xml +++ b/addons/mail/data/mail_data.xml @@ -39,6 +39,12 @@ catchall
+ + + mail.bounce.alias + bounce + + Discussions diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index cea21f5c668..502831183c7 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -9,6 +9,7 @@ import json import lxml import logging import pytz +import re import socket import time import xmlrpclib @@ -977,6 +978,7 @@ class MailThread(models.AbstractModel): raise TypeError('message must be an email.message.Message at this point') MailMessage = self.env['mail.message'] Alias, dest_aliases = self.env['mail.alias'], self.env['mail.alias'] + bounce_alias = self.env['ir.config_parameter'].get_param("mail.bounce.alias") fallback_model = model # get email.message.Message variables for future processing @@ -1005,6 +1007,49 @@ class MailThread(models.AbstractModel): tools.decode_message_header(message, 'Resent-Cc')]) rcpt_tos_localparts = [e.split('@')[0] for e in tools.email_split(rcpt_tos)] + # 0. Verify whether this is a bounced email and use it to collect bounce data and update notifications for customers + if bounce_alias and bounce_alias in email_to_localpart: + # Bounce regex: typical form of bounce is bounce_alias+128-crm.lead-34@domain + # group(1) = the mail ID; group(2) = the model (if any); group(3) = the record ID + bounce_re = re.compile("%s\+(\d+)-?([\w.]+)?-?(\d+)?" % re.escape(bounce_alias), re.UNICODE) + bounce_match = bounce_re.search(email_to) + + if bounce_match: + bounced_mail_id, bounced_model, bounced_thread_id = bounce_match.group(1), bounce_match.group(2), int(bounce_match.group(3)) + + email_part = next((part for part in message.walk() if part.get_content_type() == 'message/rfc822'), None) + dsn_part = next((part for part in message.walk() if part.get_content_type() == 'message/delivery-status'), None) + + partner, partner_address = self.env['res.partner'], False + if dsn_part and len(dsn_part.get_payload()) > 1: + dsn = dsn_part.get_payload()[1] + final_recipient_data = tools.decode_message_header(dsn, 'Final-Recipient') + partner_address = final_recipient_data.split(';', 1)[1].strip() + if partner_address: + partner = partner.sudo().search([('email', 'like', partner_address)]) + partner.message_receive_bounce(partner_address, partner, mail_id=bounced_mail_id) + + mail_message = self.env['mail.message'] + if email_part: + email = email_part.get_payload()[0] + bounced_message_id = tools.mail_header_msgid_re.findall(tools.decode_message_header(email, 'Message-Id')) + mail_message = MailMessage.sudo().search([('message_id', 'in', bounced_message_id)]) + + if partner and mail_message: + notifications = self.env['mail.notification'].sudo().search([ + ('mail_message_id', '=', mail_message.id), + ('res_partner_id', '=', partner.id)]) + notifications.write({ + 'email_status': 'bounce' + }) + + if bounced_model in self.env and hasattr(self.env[bounced_model], 'message_receive_bounce') and bounced_thread_id: + self.env[bounced_model].browse(bounced_thread_id).message_receive_bounce(partner_address, partner, mail_id=bounced_mail_id) + + _logger.info('Routing mail from %s to %s with Message-Id %s: bounced mail from mail %s, model: %s, thread_id: %s: dest %s (partner %s)', + email_from, email_to, message_id, bounced_mail_id, bounced_model, bounced_thread_id, partner_address, partner.id) + return [] + # 0. First check if this is a bounce message or not. # See http://datatracker.ietf.org/doc/rfc3462/?include_text=1 # As all MTA does not respect this RFC (googlemail is one of them), @@ -1255,6 +1300,22 @@ class MailThread(models.AbstractModel): self.write(update_vals) return True + @api.multi + def message_receive_bounce(self, email, partner, mail_id=None): + """Called by ``message_process`` when a bounce email (such as Undelivered + Mail Returned to Sender) is received for an existing thread. The default + behavior is to check is an integer ``message_bounce`` column exists. + If it is the case, its content is incremented. + + :param mail_id: ID of the sent email that bounced. It may not exist anymore + but it could be usefull if the information was kept. This is + used notably in mass mailing. + :param RecordSet partner: partner matching the bounced email address, if any + :param string email: email that caused the bounce """ + if 'message_bounce' in self._fields: + for record in self: + record.message_bounce = record.message_bounce + 1 + def _message_extract_payload_postprocess(self, message, body, attachments): """ Perform some cleaning / postprocess in the body and attachments extracted from the email. Note that this processing is specific to the diff --git a/addons/mass_mailing/__openerp__.py b/addons/mass_mailing/__openerp__.py index 403ab0efd4a..b2baefeeed7 100644 --- a/addons/mass_mailing/__openerp__.py +++ b/addons/mass_mailing/__openerp__.py @@ -22,7 +22,6 @@ professional emails and reuse templates in a few clicks. ], 'data': [ 'security/mass_mailing_security.xml', - 'data/mail_data.xml', 'data/mass_mailing_data.xml', 'wizard/mail_compose_message_views.xml', 'wizard/test_mailing_views.xml', diff --git a/addons/mass_mailing/data/mail_data.xml b/addons/mass_mailing/data/mail_data.xml deleted file mode 100644 index f8ebe1dbce4..00000000000 --- a/addons/mass_mailing/data/mail_data.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - mail.bounce.alias - bounce - - - - diff --git a/addons/mass_mailing/models/mail_thread.py b/addons/mass_mailing/models/mail_thread.py index af5782b9360..066dd285c79 100644 --- a/addons/mass_mailing/models/mail_thread.py +++ b/addons/mass_mailing/models/mail_thread.py @@ -4,62 +4,31 @@ import logging import re -from odoo import api, models +from odoo import api, models, tools from odoo.tools import decode_smtp_header, decode_message_header _logger = logging.getLogger(__name__) class MailThread(models.AbstractModel): - """ Update MailThread to add the feature of bounced emails and replied emails - in message_process. """ + """ Update MailThread to add the support of bounce management in mass mailing statistics. """ _inherit = 'mail.thread' - def message_route_check_bounce(self, message): - """ Override to verify that the email_to is the bounce alias. If it is the - case, log the bounce, set the parent and related document as bounced and - return False to end the routing process. """ - bounce_alias = self.env['ir.config_parameter'].get_param("mail.bounce.alias") - message_id = message.get('Message-Id') - email_from = decode_message_header(message, 'From') - email_to = decode_message_header(message, 'To') - - # 0. Verify whether this is a bounced email (wrong destination,...) -> use it to collect data, such as dead leads - if bounce_alias and bounce_alias in email_to: - # Bounce regex - # Typical form of bounce is bounce_alias-128-crm.lead-34@domain - # group(1) = the mail ID; group(2) = the model (if any); group(3) = the record ID - bounce_re = re.compile("%s\+(\d+)-?([\w.]+)?-?(\d+)?" % re.escape(bounce_alias), re.UNICODE) - bounce_match = bounce_re.search(email_to) - if bounce_match: - bounced_model, bounced_thread_id = None, False - bounced_mail_id = bounce_match.group(1) - statistics = self.env['mail.mail.statistics'].set_bounced(mail_mail_ids=[bounced_mail_id]) - for stat in statistics: - bounced_model = stat.model - bounced_thread_id = stat.res_id - _logger.info('Routing mail from %s to %s with Message-Id %s: bounced mail from mail %s, model: %s, thread_id: %s', - email_from, email_to, message_id, bounced_mail_id, bounced_model, bounced_thread_id) - if bounced_model and bounced_model in self.env and hasattr(self.env[bounced_model], 'message_receive_bounce') and bounced_thread_id: - self.env[bounced_model].browse(bounced_thread_id).message_receive_bounce(mail_id=bounced_mail_id) - return False - - return True - @api.model def message_route(self, message, message_dict, model=None, thread_id=None, custom_values=None): - if not self.message_route_check_bounce(message): - return [] - return super(MailThread, self).message_route(message, message_dict, model, thread_id, custom_values) + """ Override to udpate mass mailing statistics based on bounce emails """ + bounce_alias = self.env['ir.config_parameter'].get_param("mail.bounce.alias") + email_to = decode_message_header(message, 'To') + email_to_localpart = (tools.email_split(email_to) or [''])[0].split('@', 1)[0].lower() - def message_receive_bounce(self, mail_id=None): - """Called by ``message_process`` when a bounce email (such as Undelivered - Mail Returned to Sender) is received for an existing thread. The default - behavior is to check is an integer ``message_bounce`` column exists. - If it is the case, its content is incremented. """ - if 'message_bounce' in self._fields: - for mail_thread in self: - mail_thread.message_bounce = mail_thread.message_bounce + 1 + if bounce_alias and bounce_alias in email_to_localpart: + bounce_re = re.compile("%s\+(\d+)-?([\w.]+)?-?(\d+)?" % re.escape(bounce_alias), re.UNICODE) + bounce_match = bounce_re.search(email_to) + if bounce_match: + bounced_mail_id = bounce_match.group(1) + self.env['mail.mail.statistics'].set_bounced(mail_mail_ids=[bounced_mail_id]) + + return super(MailThread, self).message_route(message, message_dict, model, thread_id, custom_values) @api.model def message_route_process(self, message, message_dict, routes): diff --git a/addons/mass_mailing/models/mass_mailing.py b/addons/mass_mailing/models/mass_mailing.py index de197a48b89..93a5a01530e 100644 --- a/addons/mass_mailing/models/mass_mailing.py +++ b/addons/mass_mailing/models/mass_mailing.py @@ -98,14 +98,6 @@ class MassMailingContact(models.Model): def message_get_default_recipients(self): return dict((record.id, {'partner_ids': [], 'email_to': record.email, 'email_cc': False}) for record in self) - def message_receive_bounce(self, mail_id=None): - """Called by ``message_process`` when a bounce email (such as Undelivered - Mail Returned to Sender) is received for an existing thread. As contacts - do not inherit form mail.thread, we have to define this method to be able - to track bounces (see mail.thread for more details). """ - for contact in self: - contact.message_bounce = contact.message_bounce + 1 - class MassMailingStage(models.Model): From 029d1baf359eabb7ed531287f3cb2b3b8d586d39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 17 Aug 2016 10:20:27 +0200 Subject: [PATCH 10/17] [REF] mail: remove required alias on users This commit remove required alias on res.users model. It also removes the inheritance of mail.alias.mixin. The field alias_id is kept and allow people to use aliases on users if they want. However there is no default void alias created for each user anymore. Default from of mail.message does not use user alias anymore. Indeed this creates issues with aliases not being correctly configured and can be confusing compared to private channels. Using user aliases is therefore now done manually instead of being a default behavior. --- addons/auth_signup/data/auth_signup_data.xml | 2 -- addons/crm/tests/common.py | 1 - addons/event/tests/common.py | 2 -- addons/hr_holidays/tests/common.py | 3 --- addons/mail/models/mail_message.py | 6 ++--- addons/mail/models/res_users.py | 25 ++++++-------------- addons/mail/tests/common.py | 3 --- addons/mail/tests/test_mail_features.py | 22 ++++------------- addons/mail/tests/test_mail_message.py | 10 ++++---- addons/mail/views/res_users_views.xml | 13 ++-------- addons/project/tests/test_access_rights.py | 1 - addons/project/tests/test_project_base.py | 2 -- addons/sale/tests/test_sale_common.py | 2 -- addons/survey/tests/test_survey.py | 6 ++--- addons/website_blog/tests/common.py | 3 --- addons/website_forum/tests/common.py | 3 --- 16 files changed, 23 insertions(+), 81 deletions(-) diff --git a/addons/auth_signup/data/auth_signup_data.xml b/addons/auth_signup/data/auth_signup_data.xml index c8ded45916e..4d16b5e4951 100644 --- a/addons/auth_signup/data/auth_signup_data.xml +++ b/addons/auth_signup/data/auth_signup_data.xml @@ -9,8 +9,6 @@ - - _usertemplate diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py index 7a04e27ddb9..49b664e8b60 100644 --- a/addons/crm/tests/common.py +++ b/addons/crm/tests/common.py @@ -18,7 +18,6 @@ class TestCrm(TestMail): cls.user_salesman_all = Users.create({ 'name': 'Riton La Chignole', 'login': 'riton', - 'alias_name': 'riton', 'email': 'riton.salesman_all@example.com', 'groups_id': [(6, 0, [user_group_employee.id, user_group_salesman_all.id])] }) diff --git a/addons/event/tests/common.py b/addons/event/tests/common.py index 6e2f0406357..dbd142a08be 100644 --- a/addons/event/tests/common.py +++ b/addons/event/tests/common.py @@ -27,7 +27,6 @@ class TestEventCommon(common.TransactionCase): self.user_eventuser = self.Users.with_context({'no_reset_password': True}).create({ 'name': 'Armande EventUser', 'login': 'Armande', - 'alias_name': 'armande', 'email': 'armande.eventuser@example.com', 'tz': 'Europe/Brussels', 'groups_id': [(6, 0, [self.group_employee_id, self.group_event_user_id])] @@ -35,7 +34,6 @@ class TestEventCommon(common.TransactionCase): self.user_eventmanager = self.Users.with_context({'no_reset_password': True}).create({ 'name': 'Bastien EventManager', 'login': 'bastien', - 'alias_name': 'bastien', 'email': 'bastien.eventmanager@example.com', 'tz': 'Europe/Brussels', 'groups_id': [(6, 0, [ diff --git a/addons/hr_holidays/tests/common.py b/addons/hr_holidays/tests/common.py index 2d9f518b340..8134675266a 100644 --- a/addons/hr_holidays/tests/common.py +++ b/addons/hr_holidays/tests/common.py @@ -18,21 +18,18 @@ class TestHrHolidaysBase(common.TransactionCase): self.user_hruser_id = Users.create({ 'name': 'Armande HrUser', 'login': 'Armande', - 'alias_name': 'armande', 'email': 'armande.hruser@example.com', 'groups_id': [(6, 0, [group_employee_id, self.ref('base.group_hr_user')])] }).id self.user_hrmanager_id = Users.create({ 'name': 'Bastien HrManager', 'login': 'bastien', - 'alias_name': 'bastien', 'email': 'bastien.hrmanager@example.com', 'groups_id': [(6, 0, [group_employee_id, self.ref('base.group_hr_manager')])] }).id self.user_employee_id = Users.create({ 'name': 'David Employee', 'login': 'david', - 'alias_name': 'david', 'email': 'david.employee@example.com', 'groups_id': [(6, 0, [group_employee_id])] }).id diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 0f8054d958a..a2f1c516560 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -26,11 +26,9 @@ class Message(models.Model): @api.model def _get_default_from(self): - if self.env.user.alias_name and self.env.user.alias_domain: - return formataddr((self.env.user.name, '%s@%s' % (self.env.user.alias_name, self.env.user.alias_domain))) - elif self.env.user.email: + if self.env.user.email: return formataddr((self.env.user.name, self.env.user.email)) - raise UserError(_("Unable to send email, please configure the sender's email address or alias.")) + raise UserError(_("Unable to send email, please configure the sender's email address.")) @api.model def _get_default_author(self): diff --git a/addons/mail/models/res_users.py b/addons/mail/models/res_users.py index 9e4294df3cd..1f95c02f078 100644 --- a/addons/mail/models/res_users.py +++ b/addons/mail/models/res_users.py @@ -14,11 +14,15 @@ class Users(models.Model): group, and the user. This is done by overriding the write method. """ _name = 'res.users' - _inherit = ['mail.alias.mixin', 'res.users'] + _inherit = ['res.users'] - alias_id = fields.Many2one('mail.alias', 'Alias', ondelete="restrict", required=True, + alias_id = fields.Many2one('mail.alias', 'Alias', ondelete="set null", required=False, help="Email address internally associated with this user. Incoming "\ "emails will appear in the user's notifications.", copy=False, auto_join=True) + alias_contact = fields.Selection([ + ('everyone', 'Everyone'), + ('partners', 'Authenticated Partners'), + ('followers', 'Followers only')], string='Alias Contact Security', related='alias_id.alias_contact') def __init__(self, pool, cr): """ Override of __init__ to add access rights on notification_email_send @@ -31,17 +35,9 @@ class Users(models.Model): type(self).SELF_WRITEABLE_FIELDS.extend(['notify_email']) # duplicate list to avoid modifying the original reference type(self).SELF_READABLE_FIELDS = list(self.SELF_READABLE_FIELDS) - type(self).SELF_READABLE_FIELDS.extend(['notify_email', 'alias_domain', 'alias_name']) + type(self).SELF_READABLE_FIELDS.extend(['notify_email']) return init_res - def get_alias_model_name(self, vals): - return self._name - - def get_alias_values(self): - values = super(Users, self).get_alias_values() - values['alias_force_thread_id'] = self.id - return values - @api.model def create(self, values): if not values.get('login', False): @@ -65,13 +61,6 @@ class Users(models.Model): self.env['mail.channel'].search([('group_ids', 'in', user_group_ids)])._subscribe_users() return write_res - @api.multi - def copy_data(self, default=None): - data = super(Users, self).copy_data(default)[0] - if data and data.get('alias_name'): - data['alias_name'] = data['login'] - return [data] - def _create_welcome_message(self): self.ensure_one() if not self.has_group('base.group_user'): diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index f40a6d6a83e..cf87cd200ac 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -56,7 +56,6 @@ class TestMail(common.SavepointCase): cls.user_employee = Users.create({ 'name': 'Ernest Employee', 'login': 'ernest', - 'alias_name': 'ernest', 'email': 'e.e@example.com', 'signature': '--\nErnest', 'notify_email': 'always', @@ -64,7 +63,6 @@ class TestMail(common.SavepointCase): cls.user_public = Users.create({ 'name': 'Bert Tartignole', 'login': 'bert', - 'alias_name': 'bert', 'email': 'b.t@example.com', 'signature': 'SignBert', 'notify_email': 'always', @@ -72,7 +70,6 @@ class TestMail(common.SavepointCase): cls.user_portal = Users.create({ 'name': 'Chell Gladys', 'login': 'chell', - 'alias_name': 'chell', 'email': 'chell@gladys.portal', 'signature': 'SignChell', 'notify_email': 'always', diff --git a/addons/mail/tests/test_mail_features.py b/addons/mail/tests/test_mail_features.py index 2a003a747ed..7dc8fb2f7f1 100644 --- a/addons/mail/tests/test_mail_features.py +++ b/addons/mail/tests/test_mail_features.py @@ -9,22 +9,8 @@ class TestMailFeatures(TestMail): # TDE TODO: tests on the redirection controller def test_alias_setup(self): - Users = self.env['res.users'].with_context({'no_reset_password': True}) - - user_valentin = Users.create({ - 'name': 'Valentin Cognito', 'email': 'valentin.cognito@gmail.com', - 'login': 'valentin.cognito', 'alias_name': 'valentin.cognito'}) - self.assertEqual(user_valentin.alias_name, user_valentin.login, "Login should be used as alias") - - user_pagan = Users.create({ - 'name': 'Pagan Le Marchant', 'email': 'plmarchant@gmail.com', - 'login': 'plmarchant@gmail.com', 'alias_name': 'plmarchant@gmail.com'}) - self.assertEqual(user_pagan.alias_name, 'plmarchant', "If login is an email, the alias should keep only the local part") - - user_barty = Users.create({ - 'name': 'Bartholomew Ironside', 'email': 'barty@gmail.com', - 'login': 'b4r+_#_R3wl$$', 'alias_name': 'b4r+_#_R3wl$$'}) - self.assertEqual(user_barty.alias_name, 'b4r+_-_r3wl-', 'Disallowed chars should be replaced by hyphens') + alias = self.env['mail.alias'].with_context(alias_model_name='mail.channel').create({'alias_name': 'b4r+_#_R3wl$$'}) + self.assertEqual(alias.alias_name, 'b4r+_-_r3wl-', 'Disallowed chars should be replaced by hyphens') @mute_logger('openerp.addons.mail.models.mail_mail') def test_needaction(self): @@ -138,8 +124,8 @@ class TestMessagePost(TestMail): # notification emails: followers + recipients - notify_email=none (partner_2) - author (user_employee) self.assertEqual(set(m['email_from'] for m in self._mails), - set(['%s <%s@%s>' % (self.user_employee.name, self.user_employee.alias_name, _domain)]), - 'message_post: notification email wrong email_from: should use alias of sender') + set(['%s <%s>' % (self.user_employee.name, self.user_employee.email)]), + 'message_post: notification email wrong email_from: should use sender email') self.assertEqual(set(m['email_to'][0] for m in self._mails), set(['%s <%s>' % (self.partner_1.name, self.partner_1.email), '%s <%s>' % (self.env.user.name, self.env.user.email)])) diff --git a/addons/mail/tests/test_mail_message.py b/addons/mail/tests/test_mail_message.py index 3df8b40a9b9..ab095f5f3ca 100644 --- a/addons/mail/tests/test_mail_message.py +++ b/addons/mail/tests/test_mail_message.py @@ -49,8 +49,8 @@ class TestMailMessage(TestMail): msg = self.env['mail.message'].sudo(self.user_employee).create({}) self.assertIn('-private', msg.message_id, 'mail_message: message_id for a void message should be a "private" one') - self.assertEqual(msg.reply_to, '%s <%s@%s>' % (self.user_employee.name, self.user_employee.alias_name, alias_domain)) - self.assertEqual(msg.email_from, '%s <%s@%s>' % (self.user_employee.name, self.user_employee.alias_name, alias_domain)) + self.assertEqual(msg.reply_to, '%s <%s>' % (self.user_employee.name, self.user_employee.email)) + self.assertEqual(msg.email_from, '%s <%s>' % (self.user_employee.name, self.user_employee.email)) def test_mail_message_values_alias_catchall(self): alias_domain = 'example.com' @@ -61,7 +61,7 @@ class TestMailMessage(TestMail): msg = self.env['mail.message'].sudo(self.user_employee).create({}) self.assertIn('-private', msg.message_id, 'mail_message: message_id for a void message should be a "private" one') self.assertEqual(msg.reply_to, '%s <%s@%s>' % (self.env.user.company_id.name, alias_catchall, alias_domain)) - self.assertEqual(msg.email_from, '%s <%s@%s>' % (self.user_employee.name, self.user_employee.alias_name, alias_domain)) + self.assertEqual(msg.email_from, '%s <%s>' % (self.user_employee.name, self.user_employee.email)) def test_mail_message_values_document_no_alias(self): self.env['ir.config_parameter'].search([('key', '=', 'mail.catchall.domain')]).unlink() @@ -85,7 +85,7 @@ class TestMailMessage(TestMail): }) self.assertIn('-openerp-%d-mail.channel' % self.group_pigs.id, msg.message_id, 'mail_message: message_id for a void message should be a "private" one') self.assertEqual(msg.reply_to, '%s %s <%s@%s>' % (self.env.user.company_id.name, self.group_pigs.name, self.group_pigs.alias_name, alias_domain)) - self.assertEqual(msg.email_from, '%s <%s@%s>' % (self.user_employee.name, self.user_employee.alias_name, alias_domain)) + self.assertEqual(msg.email_from, '%s <%s>' % (self.user_employee.name, self.user_employee.email)) def test_mail_message_values_document_alias_catchall(self): alias_domain = 'example.com' @@ -99,7 +99,7 @@ class TestMailMessage(TestMail): }) self.assertIn('-openerp-%d-mail.channel' % self.group_pigs.id, msg.message_id, 'mail_message: message_id for a void message should be a "private" one') self.assertEqual(msg.reply_to, '%s %s <%s@%s>' % (self.env.user.company_id.name, self.group_pigs.name, self.group_pigs.alias_name, alias_domain)) - self.assertEqual(msg.email_from, '%s <%s@%s>' % (self.user_employee.name, self.user_employee.alias_name, alias_domain)) + self.assertEqual(msg.email_from, '%s <%s>' % (self.user_employee.name, self.user_employee.email)) def test_mail_message_values_no_auto_thread(self): msg = self.env['mail.message'].sudo(self.user_employee).create({ diff --git a/addons/mail/views/res_users_views.xml b/addons/mail/views/res_users_views.xml index 7cd4b25689b..c8b18ba4f07 100644 --- a/addons/mail/views/res_users_views.xml +++ b/addons/mail/views/res_users_views.xml @@ -27,17 +27,8 @@ -
% endif -% if not ctx['is_discussion'] or not len(ctx['actions']) == 0 or ctx.get('button_access') or ctx.get('is_accessible'): +% if not ctx['is_discussion'] or not len(ctx['actions']) == 0 or ctx.get('button_access'):
diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 53794d4038a..d551c97d418 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -114,11 +114,6 @@ class Message(models.Model): for message in self: message.needaction = message in my_messages - @api.multi - def _is_accessible(self): - self.ensure_one() - return False - @api.model def _search_needaction(self, operator, operand): if operator == '=' and operand: diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 125acce8010..78404ed8ec5 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -86,7 +86,6 @@ class Partner(models.Model): 'tracking': tracking, 'is_discussion': is_discussion, 'subtype': message.subtype_id, - 'is_accessible': message._is_accessible() } @api.model diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index 3ef6636af73..d36126de7f8 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -911,12 +911,3 @@ class Tags(models.Model): if self.env.user.karma < forum.karma_tag_create: raise KarmaError(_('Not enough karma to create a new Tag')) return super(Tags, self.with_context(mail_create_nolog=True, mail_create_nosubscribe=True)).create(vals) - -class Message(models.Model): - _name = 'mail.message' - _inherit = ['mail.message'] - - @api.multi - def _is_accessible(self): - res = super(Message, self)._is_accessible() - return self.model == 'forum.post' or res From af703b8275cc0164f5db87d1bef15b41f6d5c48f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 31 Aug 2016 14:28:26 +0200 Subject: [PATCH 16/17] [IMP] mail, website_forum, website_blog: avoid storing message needaction partners Forum and blog are public models that could have a lot of followers. After notifying followers of a new comment, discard the needaction information. For those models email is considered as sufficient. This way we avoid storing a lot of unnecessary entries in the m2m table between message and partner. --- addons/mail/models/mail_message.py | 10 ++++++++-- addons/website_blog/models/website_blog.py | 11 +++++++++++ addons/website_forum/models/forum.py | 11 +++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index d551c97d418..7dbe4ad70d9 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -777,8 +777,14 @@ class Message(models.Model): if not self._context.get('mail_notify_author', False) and self_sudo.author_id: partners = partners - self_sudo.author_id - # update message - self.write({'channel_ids': [(6, 0, channels.ids)], 'needaction_partner_ids': [(6, 0, partners.ids)]}) + # update message, with maybe custom values + message_values = { + 'channel_ids': [(6, 0, channels.ids)], + 'needaction_partner_ids': [(6, 0, partners.ids)] + } + if self.model and self.res_id and hasattr(self.env[self.model], 'message_get_message_notify_values'): + message_values.update(self.env[self.model].browse(self.res_id).message_get_message_notify_values(self, message_values)) + self.write(message_values) # notify partners and channels partners._notify(self, force_send=force_send, send_after_commit=send_after_commit, user_signature=user_signature) diff --git a/addons/website_blog/models/website_blog.py b/addons/website_blog/models/website_blog.py index 1ffa0c4d900..a0e78f1aaa9 100644 --- a/addons/website_blog/models/website_blog.py +++ b/addons/website_blog/models/website_blog.py @@ -217,3 +217,14 @@ class BlogPost(models.Model): for category, data in res.iteritems(): res[category]['button_access'] = {'url': access_action, 'title': _('View Blog Post')} return res + + @api.multi + def message_get_message_notify_values(self, message, message_values): + """ Override to avoid keeping all notified recipients of a comment. + We avoid tracking needaction on post comments. Only emails should be + sufficient. """ + if message.message_type == 'comment': + return { + 'needaction_partner_ids': [], + } + return {} diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index d36126de7f8..cf8f38df001 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -810,6 +810,17 @@ class Post(models.Model): kwargs['record_name'] = kwargs.get('record_name') or self.parent_id and self.parent_id.name return super(Post, self).message_post(message_type=message_type, subtype=subtype, **kwargs) + @api.multi + def message_get_message_notify_values(self, message, message_values): + """ Override to avoid keeping all notified recipients of a comment. + We avoid tracking needaction on post comments. Only emails should be + sufficient. """ + if message.message_type == 'comment': + return { + 'needaction_partner_ids': [], + } + return {} + class PostReason(models.Model): _name = "forum.post.reason" From 6404d50c89a8af682de5aa81e87a9dd89a600852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 31 Aug 2016 17:07:28 +0200 Subject: [PATCH 17/17] [IMP] mail: unsubscribe partners that bounce on channels If a partner bounces more than 10 times on a channel, unsubscribe him from the channel. --- addons/mail/models/mail_channel.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 6b3ad2b5ebf..a8bed5820f4 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -37,6 +37,8 @@ class Channel(models.Model): _inherit = ['mail.thread'] _inherits = {'mail.alias': 'alias_id'} + MAX_BOUNCE_LIMIT = 10 + def _get_default_image(self): image_path = modules.get_module_resource('mail', 'static/src/img', 'groupdefault.png') return tools.image_resize_image_big(open(image_path, 'rb').read().encode('base64')) @@ -156,14 +158,17 @@ class Channel(models.Model): @api.multi def action_unfollow(self): - partner_id = self.env.user.partner_id.id + return self._action_unfollow(self.env.user.partner_id) + + @api.multi + def _action_unfollow(self, partner): channel_info = self.channel_info('unsubscribe')[0] # must be computed before leaving the channel (access rights) - result = self.write({'channel_partner_ids': [(3, partner_id)]}) - self.env['bus.bus'].sendone((self._cr.dbname, 'res.partner', partner_id), channel_info) + result = self.write({'channel_partner_ids': [(3, partner.id)]}) + self.env['bus.bus'].sendone((self._cr.dbname, 'res.partner', partner.id), channel_info) if not self.email_send: notification = _('
left #%s
') % (self.id, self.name,) # post 'channel left' message as root since the partner just unsubscribed from the channel - self.sudo().message_post(body=notification, message_type="notification", subtype="mail.mt_comment", author_id=partner_id) + self.sudo().message_post(body=notification, message_type="notification", subtype="mail.mt_comment", author_id=partner.id) return result @api.multi @@ -201,6 +206,13 @@ class Channel(models.Model): res['headers'] = repr(headers) return res + @api.multi + def message_receive_bounce(self, email, partner, mail_id=None): + """ Override bounce management to unsubscribe bouncing addresses """ + if partner.message_bounce >= self.MAX_BOUNCE_LIMIT: + self._action_unfollow(partner) + return super(Channel, self).message_receive_bounce(email, partner, mail_id=mail_id) + @api.multi def message_get_recipient_values(self, notif_message=None, recipient_ids=None): # real mailing list: multiple recipients (hidden by X-Forge-To)