From 957c99abd53b42fa82903ad7ad03300797a706cf Mon Sep 17 00:00:00 2001 From: David Beguin Date: Wed, 12 Dec 2018 15:29:51 +0000 Subject: [PATCH 1/6] [REF] mail, various : make message_partner_info_from_emails from mail.thread private Purpose: make some methods from mail.thread mixin private and update their naming if necessary. Some methods prepare data for business flows and deal with internal information. Public methods should either use of give some access to those methods. Computation itself should be keps internal. In this commit we put message_partner_info_from_emails as private. As it is used in discuss JS we define a controller that controls access to this method. It explicitly checks for related document access rule and rights before calling the private method. Some addons are updated accordingly to the method change. Related to task ID 1911679 Linked to PR #29483 --- addons/account/models/account_invoice.py | 10 +- addons/crm/models/crm_lead.py | 4 +- addons/mail/controllers/main.py | 10 ++ addons/mail/models/mail_thread.py | 170 ++++++++---------- .../src/js/composers/chatter_composer.js | 21 ++- addons/project/models/project.py | 4 +- addons/test_mail/tests/test_mail_gateway.py | 15 +- addons/website_mail/controllers/main.py | 2 +- .../website_mail_channel/controllers/main.py | 2 +- 9 files changed, 127 insertions(+), 111 deletions(-) diff --git a/addons/account/models/account_invoice.py b/addons/account/models/account_invoice.py index 3eabee15eab..bfdd08315c0 100644 --- a/addons/account/models/account_invoice.py +++ b/addons/account/models/account_invoice.py @@ -754,24 +754,26 @@ class AccountInvoice(models.Model): """ # Split `From` and `CC` email address from received email to look for related partners to subscribe on the invoice subscribed_emails = email_split((msg_dict.get('from') or '') + ',' + (msg_dict.get('cc') or '')) - seen_partner_ids = [pid for pid in self._find_partner_from_emails(subscribed_emails) if pid] + seen_partner_ids = [partner.id for partner in self.env['mail.thread']._mail_find_partner_from_emails(subscribed_emails, records=self) if partner] # Detection of the partner_id of the invoice: # 1) check if the email_from correspond to a supplier email_from = msg_dict.get('from') or '' email_from = email_escape_char(email_split(email_from)[0]) - partner_id = self._search_on_partner(email_from, extra_domain=[('supplier', '=', True)]) + partners = self.env['mail.thread'].sudo()._mail_search_on_partner(email_from, extra_domain=[('supplier', '=', True)]).id + partner_id = partners.ids[0] if partners else False # 2) otherwise, if the email sender is from odoo internal users then it is likely that the vendor sent the bill # by mail to the internal user who, inturn, forwarded that email to the alias to automatically generate the bill # on behalf of the vendor. if not partner_id: - user_partner_id = self._search_on_user(email_from) + user_partners = self.env['mail.thread'].sudo()._mail_search_on_user(email_from).id + user_partner_id = user_partners.ids[0] if user_partners else False if user_partner_id and user_partner_id in self.env.ref('base.group_user').users.mapped('partner_id').ids: # In this case, we will look for the vendor's email address in email's body and assume if will come first email_addresses = email_re.findall(msg_dict.get('body')) if email_addresses: - partner_ids = [pid for pid in self._find_partner_from_emails([email_addresses[0]], force_create=False) if pid] + partner_ids = [partner.id for partner in self.env['mail.thread']._mail_find_partner_from_emails([email_addresses[0]], records=self, force_create=False) if partner] partner_id = partner_ids and partner_ids[0] # otherwise, there's no fallback on the partner_id found for the regular author of the mail.message as we want # the partner_id to stay empty diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index cd6f402e579..074b18396da 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -1229,8 +1229,8 @@ class Lead(models.Model): return super(Lead, self)._message_post_after_hook(message, *args, **kwargs) @api.multi - def message_partner_info_from_emails(self, emails, link_mail=False): - result = super(Lead, self).message_partner_info_from_emails(emails, link_mail=link_mail) + def _message_partner_info_from_emails(self, emails, link_mail=False): + result = super(Lead, self)._message_partner_info_from_emails(emails, link_mail=link_mail) for partner_info in result: if not partner_info.get('partner_id') and (self.partner_name or self.contact_name): emails = email_re.findall(partner_info['full_name'] or '') diff --git a/addons/mail/controllers/main.py b/addons/mail/controllers/main.py index 611db76e980..f1a233b1e5c 100644 --- a/addons/mail/controllers/main.py +++ b/addons/mail/controllers/main.py @@ -279,3 +279,13 @@ class MailController(http.Controller): 'moderation_channel_ids': request.env.user.moderation_channel_ids.ids, } return values + + @http.route('/mail/get_partner_info', type='json', auth='user') + def message_partner_info_from_emails(self, model, res_ids, emails, link_mail=False): + records = request.env[model].browse(res_ids) + try: + records.check_access_rule('read') + records.check_access_rights('read') + except: + return [] + return records._message_partner_info_from_emails(emails, link_mail=link_mail) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index dc8e2db4a62..57f8197e65e 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -26,6 +26,7 @@ from werkzeug import url_encode from werkzeug import urls from odoo import _, api, exceptions, fields, models, tools +from odoo.osv import expression from odoo.tools import pycompat, ustr from odoo.tools.misc import clean_context from odoo.tools.safe_eval import safe_eval @@ -937,13 +938,6 @@ class MailThread(models.AbstractModel): # Mail gateway # ------------------------------------------------------ - def _message_find_partners(self, message, header_fields=['From']): - """ Find partners related to some header fields of the message. - - :param string message: an email.message instance """ - s = ', '.join([tools.decode_smtp_header(message.get(h)) for h in header_fields if message.get(h)]) - return [x for x in self._find_partner_from_emails(tools.email_split(s)) if x] - 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 """ short_message = _("Mailbox unavailable - %s") % error_message @@ -1063,9 +1057,9 @@ class MailThread(models.AbstractModel): # 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: - message_dict['author_id'] = author_ids[0] + authors = self._mail_find_partner_from_emails([email_from], records=record_set) + if authors: + message_dict['author_id'] = authors[0].id # Alias: check alias_contact settings if alias: @@ -1652,7 +1646,8 @@ class MailThread(models.AbstractModel): 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']) + recipient_emails = ', '.join([tools.decode_smtp_header(message.get(h)) for h in ['To', 'Cc'] if message.get(h)]) + partner_ids = [x.id for x in self._mail_find_partner_from_emails(tools.email_split(recipient_emails), records=self) if x] msg_dict['partner_ids'] = [(4, partner_id) for partner_id in partner_ids] if message.get('Date'): @@ -1725,7 +1720,7 @@ class MailThread(models.AbstractModel): self.ensure_one() if email and not partner: # get partner info from email - partner_info = self.message_partner_info_from_emails([email])[0] + partner_info = self._message_partner_info_from_emails([email])[0] if partner_info.get('partner_id'): partner = self.env['res.partner'].sudo().browse([partner_info['partner_id']])[0] if email and email in [val[1] for val in result[self.ids[0]]]: # already existing email -> skip @@ -1754,84 +1749,79 @@ class MailThread(models.AbstractModel): obj._message_add_suggested_recipient(result, partner=obj.user_id.partner_id, reason=self._fields['user_id'].string) return result - def _search_on_user(self, email_address, extra_domain=[]): + def _mail_search_on_user(self, normalized_emails, extra_domain=False): + """ Find partners linked to users, given an email address that will + be normalized. Search is done as sudo on res.users model to avoid domain + on partner like ('user_ids', '!=', False) that would not be efficient. """ + domain = [('email_normalized', 'in', normalized_emails)] + if extra_domain: + domain = expression.AND(domain, extra_domain) Users = self.env['res.users'].sudo() - # exact, case-insensitive match - partners = Users.search([('email', '=ilike', email_address)], limit=1).mapped('partner_id') - if not partners: - # if no match with addr-spec, attempt substring match within name-addr pair - email_brackets = "<%s>" % email_address - partners = Users.search([('email', 'ilike', email_brackets)], limit=1).mapped('partner_id') - return partners.id + partners = Users.search(domain).mapped('partner_id') + # return a search on partner to filter results current user should not see (multi company for example) + return self.env['res.partner'].search([('id', 'in', partners.ids)]) - def _search_on_partner(self, email_address, extra_domain=[]): - Partner = self.env['res.partner'].sudo() - # exact, case-insensitive match - partners = Partner.search([('email', '=ilike', email_address)] + extra_domain, limit=1) - if not partners: - # if no match with addr-spec, attempt substring match within name-addr pair - email_brackets = "<%s>" % email_address - partners = Partner.search([('email', 'ilike', email_brackets)] + extra_domain, limit=1) - return partners.id + def _mail_search_on_partner(self, normalized_emails, extra_domain=False): + domain = [('email_normalized', 'in', normalized_emails)] + if extra_domain: + domain = expression.AND(domain, extra_domain) + return self.env['res.partner'].search(domain) - @api.multi - def _find_partner_from_emails(self, emails, res_model=None, res_id=None, check_followers=True, force_create=False, exclude_aliases=True): - """ Utility method to find partners from email addresses. The rules are : - 1 - check in document (model | self, id) followers - 2 - try to find a matching partner that is also an user - 3 - try to find a matching partner - 4 - create a new one if force_create = True + @api.model + def _mail_find_partner_from_emails(self, emails, records=None, force_create=False): + """ Utility method to find partners from email addresses. If no partner is + found, create new partners if force_create is enabled. Search heuristics - :param list emails: list of email addresses - :param string model: model to fetch related record; by default self - is used. - :param boolean check_followers: check in document followers - :param boolean force_create: create a new partner if not found - :param boolean exclude_aliases: do not try to find a partner that could match an alias. Normally aliases - should not be used as partner emails but it could be the case due to some - strange manipulation + * 1: check in records (record set) followers if records is mail.thread + enabled and if check_followers parameter is enabled; + * 2: search for partners with user; + * 3: search for partners; + + :param records: record set on which to check followers; + :param list emails: list of email addresses for finding partner; + :param boolean force_create: create a new partner if not found + + :return list partners: a list of partner records ordered as given emails. + If no partner has been found and/or created for a given emails its + matching partner is an empty record. """ - if res_model is None: - res_model = self._name - if res_id is None and self.ids: - res_id = self.ids[0] - followers = self.env['res.partner'] - if res_model and res_id: - record = self.env[res_model].browse(res_id) - if hasattr(record, 'message_partner_ids'): - followers = record.message_partner_ids + if records and issubclass(type(records), self.pool['mail.thread']): + followers = records.mapped('message_partner_ids') + else: + followers = self.env['res.partner'] + catchall_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain") - partner_ids = [] + # first, build a normalized email list and remove those linked to aliases to avoid adding aliases as partners + normalized_emails = [tools.email_normalize(contact) for contact in emails if tools.email_normalize(contact)] + if catchall_domain: + domain_left_parts = [email.split('@')[0] for email in normalized_emails if email and email.split('@')[1] == catchall_domain.lower()] + if domain_left_parts: + found_alias_names = self.env['mail.alias'].sudo().search([('alias_name', 'in', domain_left_parts)]).mapped('alias_name') + normalized_emails = [email for email in normalized_emails if email.split('@')[0] not in found_alias_names] + done_partners = [follower for follower in followers if follower.email_normalized in normalized_emails] + remaining = [email for email in normalized_emails if email not in [partner.email_normalized for partner in done_partners]] + + user_partners = self._mail_search_on_user(remaining) + done_partners += [user_partner for user_partner in user_partners] + remaining = [email for email in normalized_emails if email not in [partner.email_normalized for partner in done_partners]] + + partners = self._mail_search_on_partner(remaining) + done_partners += [partner for partner in partners] + remaining = [email for email in normalized_emails if email not in [partner.email_normalized for partner in done_partners]] + + # iterate and keep ordering + partners = [] for contact in emails: - partner_id = False - email_address = tools.email_split(contact) - if not email_address: - partner_ids.append(partner_id) - continue - if exclude_aliases and self.env['mail.alias'].search([('alias_name', 'ilike', email_address)], limit=1): - partner_ids.append(partner_id) - continue - - email_address = email_address[0] - # Escape special SQL characters in email_address to avoid invalid matches - email_address = tools.email_escape_char(email_address) - - # first try: check in document's followers - partner_id = next((partner.id for partner in followers if partner.email == email_address), False) - # second try: check in partners that are also users - if not partner_id: - partner_id = self._search_on_user(email_address) - # third try: check in partners - if not partner_id: - partner_id = self._search_on_partner(email_address) - if not partner_id and force_create: - partner_id = self.env['res.partner'].name_create(contact)[0] - partner_ids.append(partner_id) - return partner_ids + normalized_email = tools.email_normalize(contact) + partner = next((partner for partner in done_partners if partner.email_normalized == normalized_email), self.env['res.partner']) + if not partner and force_create and normalized_email in normalized_emails: + partner = self.env['res.partner'].name_create(contact)[0] + partners.append(partner) + return partners @api.multi - def message_partner_info_from_emails(self, emails, link_mail=False): + def _message_partner_info_from_emails(self, emails, link_mail=False): """ Convert a list of emails into a list partner_ids and a list new_partner_ids. The return value is non conventional because it is meant to be used by the mail widget. @@ -1839,24 +1829,18 @@ class MailThread(models.AbstractModel): :return dict: partner_ids and new_partner_ids """ self.ensure_one() MailMessage = self.env['mail.message'].sudo() - partner_ids = self._find_partner_from_emails(emails) + partners = self._mail_find_partner_from_emails(emails, records=self) result = list() - for idx in range(len(emails)): - email_address = emails[idx] - partner_id = partner_ids[idx] - partner_info = {'full_name': email_address, 'partner_id': partner_id} + for idx, contact in enumerate(emails): + partner = partners[idx] + partner_info = {'full_name': partner.email_formatted if partner else contact, 'partner_id': partner.id} result.append(partner_info) # link mail with this from mail to the new partner id - if link_mail and partner_info['partner_id']: - # Escape special SQL characters in email_address to avoid invalid matches - email_address = (email_address.replace('\\', '\\\\').replace('%', '\\%').replace('_', '\\_')) - email_brackets = "<%s>" % email_address + if link_mail and partner: MailMessage.search([ - '|', - ('email_from', '=ilike', email_address), - ('email_from', 'ilike', email_brackets), + ('email_from', '=ilike', partner.email_normalized), ('author_id', '=', False) - ]).write({'author_id': partner_info['partner_id']}) + ]).write({'author_id': partner.id}) return result # ------------------------------------------------------ diff --git a/addons/mail/static/src/js/composers/chatter_composer.js b/addons/mail/static/src/js/composers/chatter_composer.js index 2dc81d095fa..33c727a236d 100644 --- a/addons/mail/static/src/js/composers/chatter_composer.js +++ b/addons/mail/static/src/js/composers/chatter_composer.js @@ -88,11 +88,14 @@ var ChatterComposer = BasicComposer.extend({ // partner_id var def; if (namesToFind.length > 0) { - def = self._rpc({ + def = this._rpc({ + route: '/mail/get_partner_info', + params: { model: self._model, - method: 'message_partner_info_from_emails', - args: [[self.context.default_res_id], namesToFind], - }); + res_ids: [self.context.default_res_id], + emails: namesToFind, + }, + }); } // for unknown names + incomplete partners @@ -143,10 +146,14 @@ var ChatterComposer = BasicComposer.extend({ var def; if (newNamesToFind.length > 0) { def = self._rpc({ + route: '/mail/get_partner_info', + params: { model: self._model, - method: 'message_partner_info_from_emails', - args: [[self.context.default_res_id], newNamesToFind, true], - }); + res_ids: [self.context.default_res_id], + emails: namesToFind, + link_mail: true, + }, + }); } Promise.resolve(def).then(function (result) { result = result || []; diff --git a/addons/project/models/project.py b/addons/project/models/project.py index 61918344199..9a6c177ff7d 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -841,7 +841,7 @@ class Task(models.Model): task = super(Task, self.with_context(create_context)).message_new(msg, custom_values=defaults) email_list = task.email_split(msg) - partner_ids = [p for p in task._find_partner_from_emails(email_list, force_create=False) if p] + partner_ids = [p.id for p in self.env['mail.thread']._mail_find_partner_from_emails(email_list, records=task, force_create=False) if p] task.message_subscribe(partner_ids) return task @@ -849,7 +849,7 @@ class Task(models.Model): def message_update(self, msg, update_vals=None): """ Override to update the task according to the email. """ email_list = self.email_split(msg) - partner_ids = [p for p in self._find_partner_from_emails(email_list, force_create=False) if p] + partner_ids = [p.id for p in self.env['mail.thread']._mail_find_partner_from_emails(email_list, records=self, force_create=False) if p] self.message_subscribe(partner_ids) return super(Task, self).message_update(msg, update_vals=update_vals) diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index d81dd38d66d..03373e585ee 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -184,7 +184,7 @@ class TestMailgateway(BaseFunctionalTest, MockEmails): self.assertEqual(len(self._mails), 0, 'No notification / bounce should be sent') @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models') - def test_message_process_partner_find(self): + def test_message_process_email_partner_find(self): """ Finding the partner based on email, based on partner / user / follower """ self.alias.write({'alias_force_thread_id': self.test_record.id}) from_1 = self.env['res.partner'].create({'name': 'Brice Denisse', 'email': 'from.test@example.com'}) @@ -205,6 +205,19 @@ class TestMailgateway(BaseFunctionalTest, MockEmails): self.format_and_process(MAIL_TEMPLATE, from_1.email_formatted, 'groups@test.com') self.assertEqual(self.test_record.message_ids[0].author_id, from_3) + @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models') + def test_message_process_email_author_exclude_alias(self): + """ Do not set alias as author to avoid including aliases in discussions """ + from_1 = self.env['res.partner'].create({'name': 'Brice Denisse', 'email': 'from.test@test.com'}) + self.env['mail.alias'].create({ + 'alias_name': 'from.test', + 'alias_model_id': self.env['ir.model']._get('mail.test.gateway').id + }) + + record = self.format_and_process(MAIL_TEMPLATE, from_1.email_formatted, 'groups@test.com') + self.assertFalse(record.message_ids[0].author_id) + self.assertEqual(record.message_ids[0].email_from, from_1.email_formatted) + # -------------------------------------------------- # Alias configuration # -------------------------------------------------- diff --git a/addons/website_mail/controllers/main.py b/addons/website_mail/controllers/main.py index 62533f08a91..1e42e47b40e 100644 --- a/addons/website_mail/controllers/main.py +++ b/addons/website_mail/controllers/main.py @@ -18,7 +18,7 @@ class WebsiteMail(http.Controller): partner_ids = request.env.user.partner_id.ids else: # mail_thread method - partner_ids = record.sudo()._find_partner_from_emails([email], check_followers=True) + partner_ids = [p.id for p in self.env['mail.thread'].sudo()._mail_find_partner_from_emails([email], records=record.sudo(), check_followers=True) if p] if not partner_ids or not partner_ids[0]: name = email.split('@')[0] partner_ids = request.env['res.partner'].sudo().create({'name': name, 'email': email}).ids diff --git a/addons/website_mail_channel/controllers/main.py b/addons/website_mail_channel/controllers/main.py index b2c688c62f7..8865fd517b1 100644 --- a/addons/website_mail_channel/controllers/main.py +++ b/addons/website_mail_channel/controllers/main.py @@ -111,7 +111,7 @@ class MailGroup(http.Controller): else: # public users will recieve confirmation email - partner_ids = channel.sudo()._find_partner_from_emails([email], check_followers=True) + partner_ids = [p.id for p in self.env['mail.thread'].sudo()._mail_find_partner_from_emails([email], records=channel.sudo(), check_followers=True) if p] if not partner_ids or not partner_ids[0]: name = email.split('@')[0] partner_ids = [request.env['res.partner'].sudo().create({'name': name, 'email': email}).id] From 5723ee1f21c6182494621a128204f988813fa408 Mon Sep 17 00:00:00 2001 From: David Beguin Date: Wed, 12 Dec 2018 15:43:09 +0000 Subject: [PATCH 2/6] [REF] mail, various : make message_get_default_recipients from mail.thread private Purpose: make some methods from mail.thread mixin private and update their naming if necessary. Some methods prepare data for business flows and deal with internal information. Public methods should either use of give some access to those methods. Computation itself should be keps internal. In this commit we put message_get_default_recipients as private. Its API is improved to clearly separate model-inherited method (overriden in some addons) and general tool method given by the mixin. Finally all sudo hidden in the computation are removed. Caller now give a sudo-ed record set in order to be more explicit. Low level method just computes the result and does not care about sudo-ing or ACLs. Note a model approach for this method is necessary as mass mailing using mail.template records does not ensure records to check inherit from mail thread. Some addons are updated accordingly to the method change. Related to task ID 1911679 Linked to PR #29483 --- addons/crm/models/crm_lead.py | 13 ++++----- addons/event/models/event.py | 13 ++++----- addons/mail/models/mail_template.py | 3 +- addons/mail/models/mail_thread.py | 32 +++++++++++----------- addons/mail/models/res_partner.py | 8 ++++-- addons/mail/wizard/mail_compose_message.py | 5 ++-- addons/mass_mailing/models/mass_mailing.py | 8 ++++-- 7 files changed, 45 insertions(+), 37 deletions(-) diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index 074b18396da..dba52b75a48 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -1163,13 +1163,12 @@ class Lead(models.Model): return view_id @api.multi - def message_get_default_recipients(self): - return { - r.id : {'partner_ids': [], - 'email_to': r.email_normalized, - 'email_cc': False} - for r in self.sudo() - } + def _message_get_default_recipients(self): + return {r.id: { + 'partner_ids': [], + 'email_to': r.email_normalized, + 'email_cc': False} + for r in self} @api.multi def message_get_suggested_recipients(self): diff --git a/addons/event/models/event.py b/addons/event/models/event.py index 0095452a69d..3b338adc6ea 100644 --- a/addons/event/models/event.py +++ b/addons/event/models/event.py @@ -492,15 +492,14 @@ class EventRegistration(models.Model): return recipients @api.multi - def message_get_default_recipients(self): + def _message_get_default_recipients(self): # Prioritize registration email over partner_id, which may be shared when a single # partner booked multiple seats - return { - r.id: {'partner_ids': [], - 'email_to': r.email, - 'email_cc': False} - for r in self - } + return {r.id: { + 'partner_ids': [], + 'email_to': r.email, + 'email_cc': False} + for r in self} def _message_post_after_hook(self, message, *args, **kwargs): if self.email and not self.partner_id: diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index 33790cd8e12..376fb469d66 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -384,7 +384,8 @@ class MailTemplate(models.Model): self.ensure_one() if self.use_default_to or self._context.get('tpl_force_default_to'): - default_recipients = self.env['mail.thread'].message_get_default_recipients(res_model=self.model, res_ids=res_ids) + records = self.env[self.model].browse(res_ids).sudo() + default_recipients = self.env['mail.thread']._message_get_default_recipients_on_records(records) for res_id, recipients in default_recipients.items(): results[res_id].pop('partner_to', None) results[res_id].update(recipients) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 57f8197e65e..dfff9ff94ed 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1688,28 +1688,28 @@ class MailThread(models.AbstractModel): # Recipient management # ------------------------------------------------------ - @api.multi - def message_get_default_recipients(self, res_model=None, res_ids=None): - if res_model and res_ids: - if hasattr(self.env[res_model], 'message_get_default_recipients'): - return self.env[res_model].browse(res_ids).message_get_default_recipients() - records = self.env[res_model].sudo().browse(res_ids) - else: - records = self.sudo() + @api.model + def _message_get_default_recipients_on_records(self, records): + """ Generic implementation for finding default recipient to mail on + a recordset. ``_message_get_default_recipients`` may be defined to + implement custom behavior. """ + if hasattr(records, '_message_get_default_recipients'): + return records._message_get_default_recipients() + res = {} for record in records: - recipient_ids, email_to, email_cc = set(), False, False - if 'partner_id' in self._fields and record.partner_id: - recipient_ids.add(record.partner_id.id) - elif 'email_normalized' in self._fields and record.email_normalized: + recipient_ids, email_to, email_cc = [], False, False + if 'partner_id' in record and record.partner_id: + recipient_ids.append(record.partner_id.id) + elif 'email_normalized' in record and record.email_normalized: email_to = record.email_normalized - elif 'email_from' in self._fields and record.email_from: + elif 'email_from' in record and record.email_from: email_to = record.email_from - elif 'partner_email' in self._fields and record.partner_email: + elif 'partner_email' in record and record.partner_email: email_to = record.partner_email - elif 'email' in self._fields: + elif 'email' in record and record.email: email_to = record.email - res[record.id] = {'partner_ids': list(recipient_ids), 'email_to': email_to, 'email_cc': email_cc} + res[record.id] = {'partner_ids': recipient_ids, 'email_to': email_to, 'email_cc': email_cc} return res @api.multi diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index beee5608d92..ba099700d09 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -34,8 +34,12 @@ class Partner(models.Model): return recipients @api.multi - def message_get_default_recipients(self): - return dict((res_id, {'partner_ids': [res_id], 'email_to': False, 'email_cc': False}) for res_id in self.ids) + def _message_get_default_recipients(self): + return {r.id: { + 'partner_ids': [r.id], + 'email_to': False, + 'email_cc': False} + for r in self} @api.model def _notify_prepare_template_context(self, message, record, model_description=False, mail_auto_delete=True): diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index ba43532c81e..0fdc996a600 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -467,7 +467,7 @@ class MailComposer(models.TransientModel): template rendering represent a significant part of the process. Default recipients are also computed, based on mail_thread method - message_get_default_recipients. This allows to ensure a mass mailing has + _message_get_default_recipients. This allows to ensure a mass mailing has always some recipients specified. :param browse wizard: current mail.compose.message browse record @@ -488,7 +488,8 @@ class MailComposer(models.TransientModel): replies_to = self.env['mail.template']._render_template(self.reply_to, self.model, res_ids) default_recipients = {} if not self.partner_ids: - default_recipients = self.env['mail.thread'].message_get_default_recipients(res_model=self.model, res_ids=res_ids) + records = self.env[self.model].browse(res_ids).sudo() + default_recipients = self.env['mail.thread']._message_get_default_recipients_on_records(records) results = dict.fromkeys(res_ids, False) for res_id in res_ids: diff --git a/addons/mass_mailing/models/mass_mailing.py b/addons/mass_mailing/models/mass_mailing.py index 98faafd5da8..b43b6e8a428 100644 --- a/addons/mass_mailing/models/mass_mailing.py +++ b/addons/mass_mailing/models/mass_mailing.py @@ -287,8 +287,12 @@ class MassMailingContact(models.Model): return contact.name_get()[0] @api.multi - def message_get_default_recipients(self): - return dict((record.id, {'partner_ids': [], 'email_to': record.email_normalized, 'email_cc': False}) for record in self) + def _message_get_default_recipients(self): + return {r.id: { + 'partner_ids': [], + 'email_to': r.email_normalized, + 'email_cc': False} + for r in self} class MassMailingStage(models.Model): From 3f707ba000a51011b5bc3f049632e051b5b7ef41 Mon Sep 17 00:00:00 2001 From: David Beguin Date: Wed, 12 Dec 2018 15:44:49 +0000 Subject: [PATCH 3/6] [REF] mail, various : make message_receive_bounce from mail.thread private Purpose: make some methods from mail.thread mixin private and update their naming if necessary. Some methods prepare data for business flows and deal with internal information. Public methods should either use of give some access to those methods. Computation itself should be keps internal. In this commit we put message_receive_bounce as private. Future commit(s) will probably rewrite part of the bounce logic. Some addons are updated accordingly to the method change. Related to task ID 1911679 Linked to PR #29483 --- addons/mail/models/mail_channel.py | 4 ++-- addons/mail/models/mail_thread.py | 8 ++++---- addons/mass_mailing/models/mail_thread.py | 4 ++-- .../tests/test_mail_auto_blacklist.py | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 60b2fd28954..7c76f1162a1 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -338,12 +338,12 @@ class Channel(models.Model): return res @api.multi - def message_receive_bounce(self, email, partner, mail_id=None): + def _message_receive_bounce(self, email, partner, mail_id=None): """ Override bounce management to unsubscribe bouncing addresses """ for p in partner: if p.message_bounce >= self.MAX_BOUNCE_LIMIT: self._action_unfollow(p) - return super(Channel, self).message_receive_bounce(email, partner, mail_id=mail_id) + return super(Channel, self)._message_receive_bounce(email, partner, mail_id=mail_id) @api.multi def _notify_email_recipients(self, message, recipient_ids): diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index dfff9ff94ed..28b3b38f980 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1178,7 +1178,7 @@ class MailThread(models.AbstractModel): if partner_address: partners = partners.sudo().search([('email', '=', partner_address)]) for partner in partners: - partner.message_receive_bounce(partner_address, partner, mail_id=bounced_mail_id) + partner._message_receive_bounce(partner_address, partner, mail_id=bounced_mail_id) mail_message = self.env['mail.message'] if email_part: @@ -1194,8 +1194,8 @@ class MailThread(models.AbstractModel): '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(int(bounced_thread_id)).message_receive_bounce(partner_address, partners, mail_id=bounced_mail_id) + if bounced_model in self.env and hasattr(self.env[bounced_model], '_message_receive_bounce') and bounced_thread_id: + self.env[bounced_model].browse(int(bounced_thread_id))._message_receive_bounce(partner_address, partners, 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, partners) @@ -1469,7 +1469,7 @@ class MailThread(models.AbstractModel): return True @api.multi - def message_receive_bounce(self, email, partner, mail_id=None): + 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. diff --git a/addons/mass_mailing/models/mail_thread.py b/addons/mass_mailing/models/mail_thread.py index 85d42827617..9411d7300c3 100644 --- a/addons/mass_mailing/models/mail_thread.py +++ b/addons/mass_mailing/models/mail_thread.py @@ -54,7 +54,7 @@ class MailThread(models.AbstractModel): return super(MailThread, no_massmail).message_post_with_template(template_id, **kwargs) @api.multi - def message_receive_bounce(self, email, partner, mail_id=None): + def _message_receive_bounce(self, email, partner, mail_id=None): """In addition, an auto blacklist rule check if the email can be blacklisted to avoid sending mails indefinitely to this email address. This rule checks if the email bounced too much. If this is the case, @@ -63,7 +63,7 @@ class MailThread(models.AbstractModel): in the last month and the bounced are at least separated by one week, to avoid blacklist someone because of a temporary mail server error, then the email is considered as invalid and is blacklisted.""" - super(MailThread, self).message_receive_bounce(email, partner, mail_id=None) + super(MailThread, self)._message_receive_bounce(email, partner, mail_id=None) three_months_ago = fields.Datetime.to_string(datetime.datetime.now() - datetime.timedelta(weeks=13)) stats = self.env['mail.mail.statistics'] \ diff --git a/addons/test_mass_mailing/tests/test_mail_auto_blacklist.py b/addons/test_mass_mailing/tests/test_mail_auto_blacklist.py index b4a679d907e..ac04a835847 100644 --- a/addons/test_mass_mailing/tests/test_mail_auto_blacklist.py +++ b/addons/test_mass_mailing/tests/test_mail_auto_blacklist.py @@ -22,28 +22,28 @@ class TestAutoBlacklist(common.TransactionCase): 'bounced': datetime.datetime.now() - datetime.timedelta(weeks=2), 'email': self.mailing_contact_1.email }) - self.mailing_contact_1.message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) + self.mailing_contact_1._message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) mail_statistics.create({ 'model': 'mail.mass_mailing.contact', 'res_id': self.mailing_contact_1.id, 'bounced': datetime.datetime.now() - datetime.timedelta(weeks=3), 'email': self.mailing_contact_1.email }) - self.mailing_contact_1.message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) + self.mailing_contact_1._message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) mail_statistics.create({ 'model': 'mail.mass_mailing.contact', 'res_id': self.mailing_contact_1.id, 'bounced': datetime.datetime.now() - datetime.timedelta(weeks=4), 'email': self.mailing_contact_1.email }) - self.mailing_contact_1.message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) + self.mailing_contact_1._message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) mail_statistics.create({ 'model': 'mail.mass_mailing.contact', 'res_id': self.mailing_contact_1.id, 'bounced': datetime.datetime.now() - datetime.timedelta(weeks=5), 'email': self.mailing_contact_1.email }) - self.mailing_contact_1.message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) + self.mailing_contact_1._message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) # create mass mailing record @@ -77,7 +77,7 @@ class TestAutoBlacklist(common.TransactionCase): 'email': self.mailing_contact_1.email }) # call bounced - self.mailing_contact_1.message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) + self.mailing_contact_1._message_receive_bounce(self.mailing_contact_1.email, self.mailing_contact_1) # check blacklist blacklist_record = mail_blacklist.search([('email', '=', self.mailing_contact_1.email)]) From 70d44ed787a8edc9d7c40b223dd628a4755f9614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 13 May 2019 09:31:04 +0000 Subject: [PATCH 4/6] [REF] mail, various : make message_get_suggested_recipients from mail.thread private Purpose: make some methods from mail.thread mixin private and update their naming if necessary. Some methods prepare data for business flows and deal with internal information. Public methods should either use of give some access to those methods. Computation itself should be keps internal. In this commit we put message_get_suggested_recipients as private. As it is used in discuss JS we define a controller that controls access to this method. It explicitly checks for related document access rule and rights before calling the private method. Some addons are updated accordingly to the method change. Related to task ID 1911679 Linked to PR #29483 --- addons/crm/models/crm_lead.py | 4 +-- addons/event/models/event.py | 4 +-- .../hr_recruitment/models/hr_recruitment.py | 4 +-- addons/mail/controllers/main.py | 10 +++++++ addons/mail/models/mail_cc_mixin.py | 4 +-- addons/mail/models/mail_thread.py | 4 +-- addons/mail/models/res_partner.py | 4 +-- addons/mail/static/src/js/chatter.js | 9 ++++--- addons/mail/static/tests/chatter_tests.js | 26 +++++++++---------- addons/project/models/project.py | 4 +-- addons/test_mail/tests/test_mail_cc.py | 2 +- .../website_event_track/models/event_track.py | 4 +-- 12 files changed, 45 insertions(+), 34 deletions(-) diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index dba52b75a48..ff91389cbce 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -1171,8 +1171,8 @@ class Lead(models.Model): for r in self} @api.multi - def message_get_suggested_recipients(self): - recipients = super(Lead, self).message_get_suggested_recipients() + def _message_get_suggested_recipients(self): + recipients = super(Lead, self)._message_get_suggested_recipients() try: for lead in self: if lead.partner_id: diff --git a/addons/event/models/event.py b/addons/event/models/event.py index 3b338adc6ea..576b573d5c6 100644 --- a/addons/event/models/event.py +++ b/addons/event/models/event.py @@ -474,8 +474,8 @@ class EventRegistration(models.Model): self.phone = contact.phone or self.phone @api.multi - def message_get_suggested_recipients(self): - recipients = super(EventRegistration, self).message_get_suggested_recipients() + def _message_get_suggested_recipients(self): + recipients = super(EventRegistration, self)._message_get_suggested_recipients() public_users = self.env['res.users'].sudo() public_groups = self.env.ref("base.group_public", raise_if_not_found=False) if public_groups: diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py index 4c9c6f63f5b..88185cace9f 100644 --- a/addons/hr_recruitment/models/hr_recruitment.py +++ b/addons/hr_recruitment/models/hr_recruitment.py @@ -381,8 +381,8 @@ class Applicant(models.Model): return res @api.multi - def message_get_suggested_recipients(self): - recipients = super(Applicant, self).message_get_suggested_recipients() + def _message_get_suggested_recipients(self): + recipients = super(Applicant, self)._message_get_suggested_recipients() for applicant in self: if applicant.partner_id: applicant._message_add_suggested_recipient(recipients, partner=applicant.partner_id, reason=_('Contact')) diff --git a/addons/mail/controllers/main.py b/addons/mail/controllers/main.py index f1a233b1e5c..40ef7f9fe62 100644 --- a/addons/mail/controllers/main.py +++ b/addons/mail/controllers/main.py @@ -289,3 +289,13 @@ class MailController(http.Controller): except: return [] return records._message_partner_info_from_emails(emails, link_mail=link_mail) + + @http.route('/mail/get_suggested_recipients', type='json', auth='user') + def message_get_suggested_recipients(self, model, res_ids): + records = request.env[model].browse(res_ids) + try: + records.check_access_rule('read') + records.check_access_rights('read') + except: + return {} + return records._message_get_suggested_recipients() diff --git a/addons/mail/models/mail_cc_mixin.py b/addons/mail/models/mail_cc_mixin.py index bf682c7d4dd..88e16f7d6b8 100644 --- a/addons/mail/models/mail_cc_mixin.py +++ b/addons/mail/models/mail_cc_mixin.py @@ -44,8 +44,8 @@ class MailCCMixin(models.AbstractModel): return super(MailCCMixin, self).message_update(msg_dict, cc_values) @api.multi - def message_get_suggested_recipients(self): - recipients = super(MailCCMixin, self).message_get_suggested_recipients() + def _message_get_suggested_recipients(self): + recipients = super(MailCCMixin, self)._message_get_suggested_recipients() for record in self: if record.email_cc: for email in tools.email_split_and_format(record.email_cc): diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 28b3b38f980..df59d76692c 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1714,7 +1714,7 @@ class MailThread(models.AbstractModel): @api.multi def _message_add_suggested_recipient(self, result, partner=None, email=None, reason=''): - """ Called by message_get_suggested_recipients, to add a suggested + """ Called by _message_get_suggested_recipients, to add a suggested recipient in the result dictionary. The form is : partner_id, partner_name or partner_name, reason """ self.ensure_one() @@ -1738,7 +1738,7 @@ class MailThread(models.AbstractModel): return result @api.multi - def message_get_suggested_recipients(self): + def _message_get_suggested_recipients(self): """ Returns suggested recipients for ids. Those are a list of tuple (partner_id, partner_name, reason), to be managed by Chatter. """ result = dict((res_id, []) for res_id in self.ids) diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index ba099700d09..e5877a2f201 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -27,8 +27,8 @@ class Partner(models.Model): user_id = fields.Many2one(tracking=True) @api.multi - def message_get_suggested_recipients(self): - recipients = super(Partner, self).message_get_suggested_recipients() + def _message_get_suggested_recipients(self): + recipients = super(Partner, self)._message_get_suggested_recipients() for partner in self: partner._message_add_suggested_recipient(recipients, partner=partner, reason=_('Partner Profile')) return recipients diff --git a/addons/mail/static/src/js/chatter.js b/addons/mail/static/src/js/chatter.js index 296cf092efb..6cd159c5ad3 100644 --- a/addons/mail/static/src/js/chatter.js +++ b/addons/mail/static/src/js/chatter.js @@ -539,10 +539,11 @@ var Chatter = Widget.extend({ if (!this.suggested_partners_def) { this.suggested_partners_def = new Promise(function (resolve, reject) { self._rpc({ - model: self.record.model, - method: 'message_get_suggested_recipients', - args: [[self.context.default_res_id]], - context: self.context, + route: '/mail/get_suggested_recipients', + params: { + model: self.record.model, + res_ids: [self.context.default_res_id], + }, }).then(function (result) { if (!self.suggested_partners_def) { return; // widget has been reset (e.g. we just switched to another record) diff --git a/addons/mail/static/tests/chatter_tests.js b/addons/mail/static/tests/chatter_tests.js index bcf30f2c434..f9fda076dae 100644 --- a/addons/mail/static/tests/chatter_tests.js +++ b/addons/mail/static/tests/chatter_tests.js @@ -473,7 +473,7 @@ QUnit.test('chatter in create mode', async function (assert) { '', res_id: 2, mockRPC: function (route, args) { - if (route === "/web/dataset/call_kw/partner/message_get_suggested_recipients") { + if (route === "/mail/get_suggested_recipients") { return Promise.resolve({2: []}); } return this._super(route, args); @@ -540,7 +540,7 @@ QUnit.test('chatter rendering inside the sheet', async function (assert) { '', res_id: 2, mockRPC: function (route, args) { - if (route === "/web/dataset/call_kw/partner/message_get_suggested_recipients") { + if (route === "/mail/get_suggested_recipients") { return Promise.resolve({2: []}); } return this._super(route, args); @@ -773,7 +773,7 @@ QUnit.test('chatter: post, receive and star messages', async function (assert) { '', res_id: 2, mockRPC: function (route, args) { - if (args.method === 'message_get_suggested_recipients') { + if (route === '/mail/get_suggested_recipients') { return Promise.resolve({2: []}); } if (args.method === 'get_mention_suggestions') { @@ -916,7 +916,7 @@ QUnit.test('chatter: post a message disable the send button', async function(ass res_id: 2, session: {}, mockRPC: function (route, args) { - if (args.method === 'message_get_suggested_recipients') { + if (route === '/mail/get_suggested_recipients') { return Promise.resolve({2: []}); } if (args.method === 'message_post') { @@ -963,7 +963,7 @@ QUnit.test('chatter: post message failure keep message', async function(assert) res_id: 2, session: {}, mockRPC: function (route, args) { - if (args.method === 'message_get_suggested_recipients') { + if (route === '/mail/get_suggested_recipients') { return Promise.resolve({2: []}); } if (args.method === 'message_post') { @@ -1109,7 +1109,7 @@ QUnit.test('chatter: post a message and switch in edit mode', async function (as res_id: 2, session: {}, mockRPC: function (route, args) { - if (args.method === 'message_get_suggested_recipients') { + if (route === '/mail/get_suggested_recipients') { return Promise.resolve({2: []}); } if (args.method === 'message_post') { @@ -1174,7 +1174,7 @@ QUnit.test('chatter: discard changes on message post with post_refresh "always"' res_id: 2, session: {}, mockRPC: function (route, args) { - if (route === "/web/dataset/call_kw/partner/message_get_suggested_recipients") { + if (route === "/mail/get_suggested_recipients") { return Promise.resolve({2: []}); } return this._super(route, args); @@ -1225,7 +1225,7 @@ QUnit.test('chatter: discard changes on message post without post_refresh', asyn res_id: 2, session: {}, mockRPC: function (route, args) { - if (route === "/web/dataset/call_kw/partner/message_get_suggested_recipients") { + if (route === "/mail/get_suggested_recipients") { return Promise.resolve({2: []}); } if (args.method === 'message_format') { @@ -1303,7 +1303,7 @@ QUnit.test('chatter: discard changes on message post with post_refresh "recipien res_id: 2, session: {}, mockRPC: function (route, args) { - if (route === "/web/dataset/call_kw/partner/message_get_suggested_recipients") { + if (route === "/mail/get_suggested_recipients") { return Promise.resolve({2: [[42, "Me"]]}); } if (args.method === 'get_mention_suggestions') { @@ -1399,7 +1399,7 @@ QUnit.test('chatter: discard changes on opening full-composer', async function ( res_id: 2, session: {}, mockRPC: function (route, args) { - if (route === "/web/dataset/call_kw/partner/message_get_suggested_recipients") { + if (route === "/mail/get_suggested_recipients") { return Promise.resolve({2: []}); } return this._super(route, args); @@ -1580,7 +1580,7 @@ QUnit.test('chatter: keep context when sending a message', async function(assert user_context: {lang: 'en_US'}, }, mockRPC: function (route, args) { - if (args.method === 'message_get_suggested_recipients') { + if (route === '/mail/get_suggested_recipients') { return Promise.resolve({2: []}); } if (args.method === 'message_post') { @@ -2569,7 +2569,7 @@ QUnit.test('chatter: suggested partner auto-follow on message post', async funct '', res_id: 2, mockRPC: function (route, args) { - if (args.method === 'message_get_suggested_recipients') { + if (route === '/mail/get_suggested_recipients') { return Promise.resolve({2: [ [ 8, @@ -2743,7 +2743,7 @@ QUnit.test('chatter: mention prefetched partners (followers & employees)', async subtypes: [], }); } - if (args.method === 'message_get_suggested_recipients') { + if (route === '/mail/get_suggested_recipients') { return Promise.resolve({2: []}); } if (args.method === 'get_mention_suggestions') { diff --git a/addons/project/models/project.py b/addons/project/models/project.py index 9a6c177ff7d..8d4aa49ec26 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -854,8 +854,8 @@ class Task(models.Model): return super(Task, self).message_update(msg, update_vals=update_vals) @api.multi - def message_get_suggested_recipients(self): - recipients = super(Task, self).message_get_suggested_recipients() + def _message_get_suggested_recipients(self): + recipients = super(Task, self)._message_get_suggested_recipients() for task in self: if task.partner_id: reason = _('Customer Email') if task.partner_id.email else _('Customer') diff --git a/addons/test_mail/tests/test_mail_cc.py b/addons/test_mail/tests/test_mail_cc.py index 3e512c8798f..45503d3b649 100644 --- a/addons/test_mail/tests/test_mail_cc.py +++ b/addons/test_mail/tests/test_mail_cc.py @@ -55,7 +55,7 @@ class TestMailCc(common.BaseFunctionalTest, common.MockEmails): @mute_logger('odoo.addons.mail.models.mail_mail') def test_mail_cc_recipient_suggestion(self): record = self.env['mail.test.cc'].create({'email_cc': 'cc1@example.com, cc2@example.com, cc3 '}) - suggestions = record.message_get_suggested_recipients()[record.id] + suggestions = record._message_get_suggested_recipients()[record.id] self.assertEqual(sorted(suggestions), [ (False, 'cc1@example.com', 'CC Email'), (False, 'cc2@example.com', 'CC Email'), diff --git a/addons/website_event_track/models/event_track.py b/addons/website_event_track/models/event_track.py index 4627d68b5a9..6be7d86e75c 100644 --- a/addons/website_event_track/models/event_track.py +++ b/addons/website_event_track/models/event_track.py @@ -156,8 +156,8 @@ class Track(models.Model): return super(Track, self)._track_subtype(init_values) @api.multi - def message_get_suggested_recipients(self): - recipients = super(Track, self).message_get_suggested_recipients() + def _message_get_suggested_recipients(self): + recipients = super(Track, self)._message_get_suggested_recipients() for track in self: if track.partner_email != track.partner_id.email: track._message_add_suggested_recipient(recipients, email=track.partner_email, reason=_('Speaker Email')) From 0f141607b8214887d0eff4c561a2b17399a43613 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 4 Jan 2019 10:23:26 +0000 Subject: [PATCH 5/6] [IMP] mail: reflect blacklist inheritance on model Like already done for some mail mixins * mail.thread reflected on ir.model: see 1119fdd8a3c7fb0a18b348c4724d71eebf53a28e; * mail.activity.mixin reflected on ir.model: see f48021846a2b54d84d4317b40fa7128f76c28d10; We allow to reflect the mail.blacklist.mixin on ir.model in this commit. This inheritance imply the mail.address.mixin to allow normalization of emails on models using the blacklist mixin. It is useful notably to clearly store this inheritance and also allow future tweaking through studio to enable more advanced mail capabilities to custom models. Blacklist is useful when dealing with mass mailing and marketing automation. It will also be used in mailgateway to find all blacklist-related models in Odoo and update the blacklist accordingly to the bounce and error management in the mailgateway. Those improvements will come soon. Related to task ID 1911679 Linked to PR #29483 --- addons/mail/models/ir_model.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/addons/mail/models/ir_model.py b/addons/mail/models/ir_model.py index da91018b14a..4ac1e8d7fc4 100644 --- a/addons/mail/models/ir_model.py +++ b/addons/mail/models/ir_model.py @@ -17,6 +17,10 @@ class IrModel(models.Model): string="Mail Activity", default=False, help="Whether this model supports activities.", ) + is_mail_blacklist = fields.Boolean( + string="Mail Blacklist", default=False, + help="Whether this model supports blacklist.", + ) def unlink(self): # Delete followers, messages and attachments for models that will be unlinked. @@ -51,13 +55,15 @@ class IrModel(models.Model): @api.multi def write(self, vals): - if self and ('is_mail_thread' in vals or 'is_mail_activity' in vals): + if self and ('is_mail_thread' in vals or 'is_mail_activity' in vals or 'is_mail_blacklist' in vals): if not all(rec.state == 'manual' for rec in self): raise UserError(_('Only custom models can be modified.')) if 'is_mail_thread' in vals and not all(rec.is_mail_thread <= vals['is_mail_thread'] for rec in self): raise UserError(_('Field "Mail Thread" cannot be changed to "False".')) if 'is_mail_activity' in vals and not all(rec.is_mail_activity <= vals['is_mail_activity'] for rec in self): raise UserError(_('Field "Mail Activity" cannot be changed to "False".')) + if 'is_mail_blacklist' in vals and not all(rec.is_mail_blacklist <= vals['is_mail_blacklist'] for rec in self): + raise UserError(_('Field "Mail Blacklist" cannot be changed to "False".')) res = super(IrModel, self).write(vals) # setup models; this reloads custom models in registry self.pool.setup_models(self._cr) @@ -72,6 +78,7 @@ class IrModel(models.Model): vals = super(IrModel, self)._reflect_model_params(model) vals['is_mail_thread'] = issubclass(type(model), self.pool['mail.thread']) vals['is_mail_activity'] = issubclass(type(model), self.pool['mail.activity.mixin']) + vals['is_mail_blacklist'] = issubclass(type(model), self.pool['mail.blacklist.mixin']) return vals @api.model @@ -85,4 +92,8 @@ class IrModel(models.Model): parents = model_class._inherit or [] parents = [parents] if isinstance(parents, str) else parents model_class._inherit = parents + ['mail.activity.mixin'] + if model_data.get('is_mail_blacklist') and model_class._name != 'mail.blacklist.mixin': + parents = model_class._inherit or [] + parents = [parents] if isinstance(parents, str) else parents + model_class._inherit = parents + ['mail.blacklist.mixin'] return model_class From dbd4aab6c3172688d98cef700265795527d63cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 18 Apr 2019 13:25:09 +0000 Subject: [PATCH 6/6] [REF] mail: make blacklist a mixin built upon mail.thread and add message_bounce * mail.blacklist.mixin now clearly make inheritance on mail.thread. Indeed we consider models using the blacklist mechanism as being used in mailing features, meaning they will anyway inherit form mail.thread. In standard Odoo it is the case for the 3 models using it (Lead/Opportunity, Contact and Mailing Contact); * define message_bounce directly in mail.blacklist.mixin. Bounce counter is indeed linked to the blacklist and mailing mechanism; * rename mail.blacklist.mixin to mail.thread.blacklist to ensure coherency with mail.thread and mail.thread.cc (another mixin build on mailL.thread); Inherit declarations in various addons are updated accordingly. Related to task ID 1911679 Linked to PR #29483 --- addons/crm/models/crm_lead.py | 5 +-- addons/mail/models/ir_model.py | 6 ++-- addons/mail/models/mail_address_mixin.py | 32 +++++++++---------- addons/mail/models/mail_blacklist.py | 27 +++++++++++----- addons/mail/models/mail_thread.py | 17 +++++----- addons/mail/models/res_partner.py | 3 +- addons/mail/wizard/mail_compose_message.py | 2 +- addons/mass_mailing/models/mass_mailing.py | 3 +- addons/test_mail/models/test_mail_models.py | 2 +- .../models/mass_mail_test.py | 2 +- 10 files changed, 53 insertions(+), 46 deletions(-) diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index ff91389cbce..96a4048d891 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -50,7 +50,7 @@ class Lead(models.Model): _name = "crm.lead" _description = "Lead/Opportunity" _order = "priority desc,activity_date_deadline,id desc" - _inherit = ['mail.thread.cc', 'mail.activity.mixin', 'utm.mixin', 'format.address.mixin', 'mail.blacklist.mixin'] + _inherit = ['mail.thread.cc', 'mail.thread.blacklist', 'mail.activity.mixin', 'utm.mixin', 'format.address.mixin'] _primary_email = 'email_from' def _default_probability(self): @@ -96,9 +96,6 @@ class Lead(models.Model): date_last_stage_update = fields.Datetime(string='Last Stage Update', index=True, default=fields.Datetime.now) date_conversion = fields.Datetime('Conversion Date', readonly=True) - # Messaging and marketing - message_bounce = fields.Integer('Bounce', help="Counter of the number of bounced emails for this contact", default=0) - # Only used for type opportunity probability = fields.Float('Probability', group_operator="avg", default=lambda self: self._default_probability()) planned_revenue = fields.Monetary('Expected Revenue', currency_field='company_currency', tracking=True) diff --git a/addons/mail/models/ir_model.py b/addons/mail/models/ir_model.py index 4ac1e8d7fc4..b4950d73ed4 100644 --- a/addons/mail/models/ir_model.py +++ b/addons/mail/models/ir_model.py @@ -78,7 +78,7 @@ class IrModel(models.Model): vals = super(IrModel, self)._reflect_model_params(model) vals['is_mail_thread'] = issubclass(type(model), self.pool['mail.thread']) vals['is_mail_activity'] = issubclass(type(model), self.pool['mail.activity.mixin']) - vals['is_mail_blacklist'] = issubclass(type(model), self.pool['mail.blacklist.mixin']) + vals['is_mail_blacklist'] = issubclass(type(model), self.pool['mail.thread.blacklist']) return vals @api.model @@ -92,8 +92,8 @@ class IrModel(models.Model): parents = model_class._inherit or [] parents = [parents] if isinstance(parents, str) else parents model_class._inherit = parents + ['mail.activity.mixin'] - if model_data.get('is_mail_blacklist') and model_class._name != 'mail.blacklist.mixin': + if model_data.get('is_mail_blacklist') and model_class._name != 'mail.thread.blacklist': parents = model_class._inherit or [] parents = [parents] if isinstance(parents, str) else parents - model_class._inherit = parents + ['mail.blacklist.mixin'] + model_class._inherit = parents + ['mail.thread.blacklist'] return model_class diff --git a/addons/mail/models/mail_address_mixin.py b/addons/mail/models/mail_address_mixin.py index 49dff2f86c5..c2e5fdb9111 100644 --- a/addons/mail/models/mail_address_mixin.py +++ b/addons/mail/models/mail_address_mixin.py @@ -6,25 +6,25 @@ from odoo.exceptions import UserError class MailAddressMixin(models.AbstractModel): - """ Purpose of this mixing is to store a normalized email based on the primary email field. - A normalized email is considered as : - - having a left part + @ + a right part (the domain can be without '.something') - - being lower case - - having no name before the address. Typically, having no 'Name <>' - Ex: - - Formatted Email : 'Name ' - - Normalized Email : 'name@domain.com' - The primary email field can be specified on the parent model, if it differs from the default one ('email') - The email_normalized field can than be used on that model to search quickly on emails (by simple comparison - and not using time consuming regex anymore). - """ + """ Purpose of this mixin is to store a normalized email based on the primary email field. + A normalized email is considered as : + - having a left part + @ + a right part (the domain can be without '.something') + - being lower case + - having no name before the address. Typically, having no 'Name <>' + Ex: + - Formatted Email : 'Name ' + - Normalized Email : 'name@domain.com' + The primary email field can be specified on the parent model, if it differs from the default one ('email') + The email_normalized field can than be used on that model to search quickly on emails (by simple comparison + and not using time consuming regex anymore). """ _name = 'mail.address.mixin' - _description = 'Email address mixin' + _description = 'Email Address Mixin' _primary_email = 'email' - email_normalized = fields.Char(string='Normalized email address', compute="_compute_email_normalized", invisible=True, - compute_sudo=True, store=True, help="""This field is used to search on email address, - as the primary email field can contain more than strictly an email address.""") + email_normalized = fields.Char( + string='Normalized Email', compute="_compute_email_normalized", compute_sudo=True, + store=True, invisible=True, + help="This field is used to search on email address as the primary email field can contain more than strictly an email address.") @api.depends(lambda self: [self._primary_email]) def _compute_email_normalized(self): diff --git a/addons/mail/models/mail_blacklist.py b/addons/mail/models/mail_blacklist.py index 4636a19dcbe..4311aa54b6e 100644 --- a/addons/mail/models/mail_blacklist.py +++ b/addons/mail/models/mail_blacklist.py @@ -94,19 +94,22 @@ class MailBlackList(models.Model): class MailBlackListMixin(models.AbstractModel): - """ Mixin that is inherited by all model with opt out. - This mixin is inheriting of another mixin, mail.address.mixin which defines the _primary_email variable - and the email_normalized field that are mandatory to use the blacklist mixin. - """ - _name = 'mail.blacklist.mixin' + """ Mixin that is inherited by all model with opt out. This mixin inherits from + mail.address.mixin which defines the _primary_email variable and the email_normalized + field that are mandatory to use the blacklist mixin. Mail Thread capabilities + are required for this mixin. """ + _name = 'mail.thread.blacklist' _description = 'Mail Blacklist mixin' - _inherit = ['mail.address.mixin'] + _inherit = ['mail.thread', 'mail.address.mixin'] # Note : is_blacklisted sould only be used for display. As the compute is not depending on the blacklist, # once read, it won't be re-computed again if the blacklist is modified in the same request. - is_blacklisted = fields.Boolean(string='Blacklist', compute="_compute_is_blacklisted", compute_sudo=True, - store=False, search="_search_is_blacklisted", groups="base.group_user", + is_blacklisted = fields.Boolean( + string='Blacklist', compute="_compute_is_blacklisted", compute_sudo=True, store=False, + search="_search_is_blacklisted", groups="base.group_user", help="If the email address is on the blacklist, the contact won't receive mass mailing anymore, from any list") + # messaging + message_bounce = fields.Integer('Bounce', help="Counter of the number of bounced emails for this contact", default=0) @api.model def _search_is_blacklisted(self, operator, value): @@ -147,3 +150,11 @@ class MailBlackListMixin(models.AbstractModel): ('email', 'in', self.mapped('email_normalized'))]).mapped('email')) for record in self: record.is_blacklisted = record.email_normalized in blacklist + + @api.multi + def _message_receive_bounce(self, email, partner, mail_id=None): + """ Override of mail.thread generic method. Purpose is to increment the + bounce counter of the record. """ + super(MailBlackListMixin, self)._message_receive_bounce(email, partner, mail_id=mail_id) + for record in self: + record.message_bounce = record.message_bounce + 1 diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index df59d76692c..3d645bca794 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1472,17 +1472,18 @@ class MailThread(models.AbstractModel): 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. + behavior is to do nothing. This method is meant to be overridden in various + modules to add some specific behavior like blacklist management or mass + mailing statistics update. check is an integer ``message_bounce`` column exists. If it is the case, its content is incremented. + :param record partner: partner matching the bounced email address, if any; + :param string email: email that caused the bounce; :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 + but it could be useful if the information was kept. This is + used notably in mass mailing; + """ + pass def _message_extract_payload_postprocess(self, message, body, attachments): """ Perform some cleaning / postprocess in the body and attachments diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index e5877a2f201..966d0bb2dbd 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -18,10 +18,9 @@ class Partner(models.Model): """ Update partner to add a field about notification preferences. Add a generic opt-out field that can be used to restrict usage of automatic email templates. """ _name = "res.partner" - _inherit = ['res.partner', 'mail.thread', 'mail.activity.mixin', 'mail.blacklist.mixin'] + _inherit = ['res.partner', 'mail.activity.mixin', 'mail.thread.blacklist'] _mail_flat_thread = False - message_bounce = fields.Integer('Bounce', help="Counter of the number of bounced emails for this contact", default=0) channel_ids = fields.Many2many('mail.channel', 'mail_channel_partner', 'partner_id', 'channel_id', string='Channels', copy=False) # override the field to track the visibility of user user_id = fields.Many2one(tracking=True) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 0fdc996a600..72076826085 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -305,7 +305,7 @@ class MailComposer(models.TransientModel): reply_to_value = self.env['mail.thread']._notify_get_reply_to_on_records(default=self.email_from, records=records) blacklisted_rec_ids = [] - if mass_mail_mode and issubclass(type(self.env[self.model]), self.pool['mail.blacklist.mixin']): + if mass_mail_mode and issubclass(type(self.env[self.model]), self.pool['mail.thread.blacklist']): BL_sudo = self.env['mail.blacklist'].sudo() blacklist = set(BL_sudo.search([]).mapped('email')) if blacklist: diff --git a/addons/mass_mailing/models/mass_mailing.py b/addons/mass_mailing/models/mass_mailing.py index b43b6e8a428..6d47d33e356 100644 --- a/addons/mass_mailing/models/mass_mailing.py +++ b/addons/mass_mailing/models/mass_mailing.py @@ -211,7 +211,7 @@ class MassMailingContact(models.Model): be able to deal with large contact list to email without bloating the partner base.""" _name = 'mail.mass_mailing.contact' - _inherit = ['mail.thread', 'mail.blacklist.mixin'] + _inherit = ['mail.thread.blacklist'] _description = 'Mass Mailing Contact' _order = 'email' _rec_name = 'email' @@ -226,7 +226,6 @@ class MassMailingContact(models.Model): 'contact_id', 'list_id', string='Mailing Lists') subscription_list_ids = fields.One2many('mail.mass_mailing.list_contact_rel', 'contact_id', string='Subscription Information') - message_bounce = fields.Integer(string='Bounced', help='Counter of the number of bounced emails for this contact.', default=0) country_id = fields.Many2one('res.country', string='Country') tag_ids = fields.Many2many('res.partner.category', string='Tags') opt_out = fields.Boolean('Opt Out', compute='_compute_opt_out', search='_search_opt_out', diff --git a/addons/test_mail/models/test_mail_models.py b/addons/test_mail/models/test_mail_models.py index bdf9f164da8..b9ff40ebc7a 100644 --- a/addons/test_mail/models/test_mail_models.py +++ b/addons/test_mail/models/test_mail_models.py @@ -20,7 +20,7 @@ class MailTestGateway(models.Model): mailing features and base performances. """ _description = 'Simple Chatter Model for Mail Gateway' _name = 'mail.test.gateway' - _inherit = ['mail.blacklist.mixin', 'mail.thread'] + _inherit = ['mail.thread.blacklist'] _primary_email = 'email_from' name = fields.Char() diff --git a/addons/test_mass_mailing/models/mass_mail_test.py b/addons/test_mass_mailing/models/mass_mail_test.py index 1abe766c412..3da40c424ce 100644 --- a/addons/test_mass_mailing/models/mass_mail_test.py +++ b/addons/test_mass_mailing/models/mass_mail_test.py @@ -20,7 +20,7 @@ class MassMailTestBlacklist(models.Model): """ Model using blacklist mechanism for mass mailing. """ _description = 'Mass Mailing Model w Blacklist' _name = 'mass.mail.test.bl' - _inherit = ['mail.thread', 'mail.blacklist.mixin'] + _inherit = ['mail.thread.blacklist'] _primary_email = 'email_from' # blacklist field to check