Skip to content

[REF] mail, various: have email-like explicit external recipients #185240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

[REF] mail, various: have email-like explicit external recipients #185240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4cd205a
[REF] mail, various: improve suggested recipients
tde-banana-odoo Jan 13, 2025
4a6d16b
[REF] mail, various: have email-like explicit external recipients
tde-banana-odoo Jan 13, 2025
8cb62bd
[IMP] mail, various: limit auto subscription
tde-banana-odoo Jan 13, 2025
2f1b17b
[IMP] mail: fetch last discussion recipients by default
tde-banana-odoo Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions addons/crm/tests/test_crm_lead_notification.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def test_lead_message_get_suggested_recipients(self):
'name': 'New Customer',
'email': 'new.customer.format@test.example.com',
'partner_id': False,
'reason': 'Customer Email',
'create_values': {
'company_name': 'Format Name',
'is_company': False,
Expand All @@ -97,20 +96,23 @@ def test_lead_message_get_suggested_recipients(self):
'name': 'Multi Name',
'email': 'new.customer.multi.1@test.example.com', # only first found normalized email is kept
'partner_id': False,
'reason': 'Customer Email',
'create_values': {
'is_company': True,
'type': 'contact',
'user_id': self.user_sales_leads.id,
},
}, {
'name': '',
'email': 'new.customer.2@test.example.com', # second found creates another contact
'partner_id': False,
'create_values': {}, # not targeted as primary lead customer hence no values
},
], [
# here contact name -> individual
{
'name': 'Std Name',
'email': 'new.customer.simple@test.example.com',
'partner_id': False,
'reason': 'Customer Email',
'create_values': {
'is_company': False,
'type': 'contact',
Expand All @@ -123,7 +125,6 @@ def test_lead_message_get_suggested_recipients(self):
'name': 'test.lang@test.example.com',
'email': 'test.lang@test.example.com',
'partner_id': False,
'reason': 'Customer Email',
'create_values': {
'is_company': False,
'lang': 'en_US',
Expand All @@ -135,35 +136,31 @@ def test_lead_message_get_suggested_recipients(self):
'partner_id': self.contact_1.id,
'name': 'Philip J Fry',
'email': 'philip.j.fry@test.example.com',
'reason': 'Customer',
'create_values': {},
},
], [
{
'partner_id': partner_no_email.id,
'name': 'Test Partner',
'email': False,
'reason': 'Customer',
'create_values': {},
},
], [
{
'partner_id': partner_no_email.id,
'email': False,
'name': 'Test Partner',
'reason': 'Customer',
'create_values': {},
}, {
'name': '',
'email': 'test_cc@odoo.com',
'partner_id': False,
'reason': 'CC Email',
'create_values': {},
},
],
]):
with self.subTest(lead_name=lead.name, email_from=lead.email_from):
res = lead._message_get_suggested_recipients()
res = lead._message_get_suggested_recipients(no_create=True)
self.assertEqual(len(res), len(expected_suggested))
for received, expected in zip(res, expected_suggested):
self.assertDictEqual(received, expected)
Expand Down Expand Up @@ -215,11 +212,10 @@ def test_lead_message_get_suggested_recipients_values_for_create(self):
'partner_name': partner_name,
**lead_details_for_contact,
})
suggestion = lead1._message_get_suggested_recipients()[0]
suggestion = lead1._message_get_suggested_recipients(no_create=True)[0]
self.assertFalse(suggestion.get('partner_id'))
self.assertEqual(suggestion['email'], suggested_email)
self.assertEqual(suggestion['name'], suggested_name)
self.assertEqual(suggestion['reason'], 'Customer Email')

create_values = suggestion['create_values']
customer_information = lead1._get_customer_information().get(email_normalize(email), {})
Expand Down
195 changes: 130 additions & 65 deletions addons/mail/models/mail_thread.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import lxml
import logging
import pytz
import re
import time
import threading

Expand All @@ -27,7 +26,7 @@
from requests import Session
from werkzeug import urls

from odoo import _, api, exceptions, fields, models, Command
from odoo import _, api, exceptions, fields, models, Command, tools
from odoo.addons.mail.tools.discuss import Store
from odoo.addons.mail.tools.web_push import push_to_end_point, DeviceUnreachableError
from odoo.exceptions import MissingError, AccessError
Expand All @@ -38,8 +37,8 @@
ormcache, is_list_of,
)
from odoo.tools.mail import (
append_content_to_html, decode_message_header, email_normalize,
email_normalize_all, email_split,
append_content_to_html, decode_message_header,
email_normalize, email_normalize_all, email_split,
email_split_and_format, formataddr, html_sanitize,
generate_tracking_message_id,
unfold_references,
Expand Down Expand Up @@ -2072,74 +2071,118 @@ def sort_key(p):
found_results[res_id] |= partner
return found_results

def _message_add_suggested_recipient(self, recipients, partner=None, email=None, reason=''):
""" Called by _message_get_suggested_recipients, to add a suggested
recipient as a dictionary in the result list """
def _message_add_suggested_recipients(self):
self.ensure_one()
recipient_data = {'create_values': {}, 'partner_id': False, 'reason': reason}
if email and not partner:
partner = self._partner_find_from_emails_single([email], no_create=True)
if partner:
recipient_data['name'] = partner.name

# check if already suggested
parse_name, email_normalized = parse_contact_from_email(email)
if email_normalized and email_normalized in {val['email'] for val in recipients}: # already existing email -> skip
return recipients
if partner and partner in self.message_partner_ids: # recipient already in the followers -> skip
return recipients
if partner and partner.id in {val['partner_id'] for val in recipients}: # already existing partner ID -> skip
return recipients

if partner:
recipient_data.update({
'partner_id': partner.id, 'name': partner.name,
'email': ','.join(email_normalize_all(partner.email)) or partner.email,
})
else: # unknown partner, we are probably managing an email address
customer_values = self._get_customer_information().get(email_normalized) or {}
name = recipient_data.get('name') or customer_values.pop('name', False) or parse_name
recipient_data.update({
'name': name,
'email': email_normalized,
'create_values': customer_values,
})
recipients.append(recipient_data)
return recipients

def _message_get_suggested_recipients(self):
""" Get suggested recipients to be managed by Chatter

:returns: list of dictionaries (per suggested recipient) containing:
* partner_id: int: recipient partner id
* name: str: name of the recipient
* email: str: email of recipient
* reason: str
* new_partner_values: dict: data for unknown partner
"""
self.ensure_one()
recipients = []
email_to_lst, partners = [], self.env['res.partner']

# add responsible
user_field = self._fields.get('user_id')
if user_field and user_field.type == 'many2one' and user_field.comodel_name == 'res.users':
thread = self.sudo() # SUPERUSER because of a read on res.users that would crash otherwise
if thread.user_id and thread.user_id.partner_id:
thread._message_add_suggested_recipient(
recipients, partner=thread.user_id.partner_id, reason=self._fields['user_id'].string,
)
# SUPERUSER because of a read on res.users that would crash otherwise
partners += self.sudo().user_id.partner_id

# add customers
customers = self._mail_get_partners()[self.id]
for customer in customers.filtered(lambda p: not p.is_public):
self._message_add_suggested_recipient(recipients, partner=customer, reason=_('Customer'))
partners += self._mail_get_partners()[self.id].filtered(lambda p: not p.is_public)

# add email
email_fname = self._mail_get_primary_email_field()
email_normalized = email_fname and email_normalize(self[email_fname], strict=False)
if email_normalized and email_normalized not in customers.mapped('email_normalized'):
self._message_add_suggested_recipient(recipients, email=self[email_fname], reason=_('Customer Email'))
if email_fname and self[email_fname]:
email_to_lst.append(self[email_fname])

return email_to_lst, partners

def _message_get_suggested_recipients(self, reply_discussion=False, reply_message=None,
no_create=True):
""" Get suggested recipients, contextualized depending on discussion.

:param bool reply_discussion: consider user replies to the discussion.
Last relevant message is fetched and used to search for additional
'To' and 'Cc' to propose;
:param bool reply_message: specific message user is replying-to. Bypasses
'reply_discussion';
:param bool no_create: do not create partners when emails are not linked
to existing partners, see '_partner_find_from_emails';

:returns: list of dictionaries (per suggested recipient) containing:
* create_values: dict: data to populate new partner, if not found
* email: str: email of recipient
* name: str: name of the recipient
* partner_id: int: recipient partner id
"""
def email_key(email):
return email_normalize(email, strict=False) or email.strip()

self.ensure_one()
email_to_lst, partners = self._message_add_suggested_recipients()

# find last relevant message
if reply_discussion:
messages = self.message_ids.sorted(
lambda msg: (
msg.message_type == 'email', # incoming email = probably customer
msg.message_type == 'comment', # user input > other input
msg.date, msg.id, # newer first
), reverse=True,
)
reply_message = next(
(msg for msg in messages if msg.message_type in ('comment', 'email')),
self.env['mail.message']
)
# fetch answer-based recipients as well as author
if reply_message:
# direct recipients, and author if not archived / root
partners += (reply_message.partner_ids | reply_message.author_id).filtered(lambda p: p.active)
# To and Cc emails (mainly for incoming email), and email_from if not linked to hereabove author
email_to_lst += [reply_message.incoming_email_to or '', reply_message.incoming_email_cc or '', reply_message.email_from or '']
from_normalized = email_normalize(reply_message.email_from)
if from_normalized and from_normalized != reply_message.author_id.email_normalized:
email_to_lst.append(reply_message.email_from)
# flatten emails, as some inputs are stringified list of emails (e.g. Cc, To)
email_to_lst = [
e for email_input in email_to_lst
for e in email_split_and_format(email_input)
if e and e.strip()
]

# organize and deduplicate partners, exclude followers, keep ordering
followers = self.message_partner_ids
# sanitize email inputs, exclude followers and aliases, add some banned emails, keep ordering, then link to partners
skip_emails_normalized = (followers | partners).mapped('email_normalized') + (followers | partners).mapped('email')
skip_emails_normalized += [self.env.ref('base.partner_root').email_normalized] # never propose odoobot
if email_to_lst:
skip_emails_normalized.extend(
self.env['mail.alias'].sudo().search(
[('alias_full_name', 'in', [email_key(e) for e in email_to_lst])]
).mapped('alias_full_name')
)
email_to_lst = [e for e in email_to_lst if email_key(e) not in skip_emails_normalized]
partners += self._partner_find_from_emails_single(email_to_lst, no_create=no_create)

# final filtering
partners = self.env['res.partner'].browse(tools.misc.unique(
p.id for p in partners if p not in followers
))
email_to_lst = list(tools.misc.unique(
e for e in email_to_lst
if email_key(e) not in (partners.mapped('email_normalized') + partners.mapped('email'))
))
Comment on lines +2165 to +2168
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't that already the same idea that

skip_emails_normalized = (followers + partners).mapped('email_normalized')
...
email_to_lst = [e for e in email_to_lst if email_key(e) not in skip_emails_normalized]

(except that we take the email, and that we remove duplicate, but I feel like both filtering can be merged)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that one filtres after creating finding partners, so we have to keep the "final list without partners". The first one is mainly to avoid finding duplicates, the second one is about "what remains". Don't think I can remove that one :) .

# fetch model-related additional information
emails_normalized_info = self._get_customer_information() if email_to_lst else {}

recipients = [{
'email': partner.email_normalized,
'name': partner.name,
'partner_id': partner.id,
'create_values': {},
} for partner in partners]
for email_input in email_to_lst:
name, email_normalized = parse_contact_from_email(email_input)
recipients.append({
'email': email_normalized,
'name': emails_normalized_info.get(email_normalized, {}).pop('name', False) or name,
'partner_id': False,
'create_values': emails_normalized_info.get(email_normalized, {}),
})
return recipients

def _mail_find_user_for_gateway(self, email_value, alias=None):
Expand Down Expand Up @@ -2377,13 +2420,15 @@ def message_post(self, *,

# subscribe author(s) so that they receive answers; do it only when it is
# a manual post by the author (aka not a system notification, not a message
# posted 'in behalf of', and if still active).
# posted 'in behalf of'). Limit to active and internal partners, as external
# customers should be proposed through suggested recipients.
author_subscribe = (
not self._context.get('mail_post_autofollow_author_skip')
and msg_values['message_type'] not in ('notification', 'user_notification', 'auto_comment')
)
if author_subscribe:
if real_author := self._message_compute_real_author(msg_values['author_id']):
real_author = self._message_compute_real_author(msg_values['author_id'])
if real_author and not real_author.partner_share:
self._message_subscribe(partner_ids=[real_author.id])

self._message_post_after_hook(new_message, msg_values)
Expand Down Expand Up @@ -3415,6 +3460,7 @@ def _notify_thread_by_email(self, message, recipients_data, msg_vals=False,

base_mail_values = self._notify_by_email_get_base_mail_values(
message,
partners_data,
additional_values={'auto_delete': mail_auto_delete}
)

Expand Down Expand Up @@ -3747,12 +3793,14 @@ def _notify_by_email_render_layout(self, message, recipients_group,
mail_body = message.body
return mail_body

def _notify_by_email_get_base_mail_values(self, message, additional_values=None):
def _notify_by_email_get_base_mail_values(self, message, recipients_data, additional_values=None):
""" Return model-specific and message-related values to be used when
creating notification emails. It serves as a common basis for all
notification emails based on a given message.

:param record message: <mail.message> record being notified;
:param list recipients_data: list of email recipients data based on <res.partner>
records formatted using a list of dicts. See ``MailThread._notify_get_recipients()``;
:param dict additional_values: optional additional values to add (ease
custom calls and inheritance);

Expand Down Expand Up @@ -3787,6 +3835,13 @@ def _notify_by_email_get_base_mail_values(self, message, additional_values=None)

# prepare headers (as sudo as accessing mail.alias.domain, restricted)
headers = {}
# prepare external emails to modify Msg[To] and enable Reply-All including external people
external = ','.join(
formataddr((r['name'], r['email_normalized']))
for r in recipients_data if r['id'] and r['active'] and r['email_normalized'] and r['share']
)
if external:
headers['X-Msg-To-Add'] = external
if message_sudo.record_alias_domain_id.bounce_email:
headers['Return-Path'] = message_sudo.record_alias_domain_id.bounce_email
headers = self._notify_by_email_get_headers(headers=headers)
Expand Down Expand Up @@ -3990,11 +4045,19 @@ def _notify_get_recipients(self, message, msg_vals=False, **kwargs):
if notify_author_mention and skip_author_id in pids:
skip_author_id = False

# avoid double email notification if already emailed in original email
emailed_normalized = [email for email in email_normalize_all(
f"{msg_vals.get('incoming_email_to', msg_sudo.incoming_email_to) or ''}, "
f"{msg_vals.get('incoming_email_cc', msg_sudo.incoming_email_cc) or ''}"
)]

for pid, pdata in res.items():
if pid and pid == skip_author_id:
continue
if pdata['active'] is False:
continue
if pdata['email_normalized'] in emailed_normalized:
continue
recipients_data.append(pdata)

# avoid double notification (on demand due to additional queries)
Expand Down Expand Up @@ -4727,7 +4790,9 @@ def _thread_to_store(self, store: Store, fields, *, request_list=None):
['model', '=', self._name], ['res_id', '=', thread.id]
]))
if request_list and "suggestedRecipients" in request_list:
res["suggestedRecipients"] = thread._message_get_suggested_recipients()
res["suggestedRecipients"] = thread._message_get_suggested_recipients(
reply_discussion=True, no_create=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no_create=True is already the default

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same reasoning, I prefer to make it explicit as it is going to change, so I note what is really expected currently to be sure it was thought like that.

)
if res:
store.add(thread, res, as_thread=True)

Expand Down
9 changes: 4 additions & 5 deletions addons/mail/models/mail_thread_cc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ def message_update(self, msg_dict, update_vals=None):
cc_values.update(update_vals)
return super().message_update(msg_dict, cc_values)

def _message_get_suggested_recipients(self):
recipients = super()._message_get_suggested_recipients()
def _message_add_suggested_recipients(self):
email_to_lst, partners = super()._message_add_suggested_recipients()
if self.email_cc:
for email in tools.mail.email_split_and_format(self.email_cc):
self._message_add_suggested_recipient(recipients, email=email, reason=_('CC Email'))
return recipients
email_to_lst.append(self.email_cc)
return email_to_lst, partners
Loading