pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[REF] mail: move composer 'author_id' / 'email_from' fields from onchange to computed

Browse Source 
RATIONALE

Remove _onchange_template_id / defaults usage for composer fields. It
lacks control and make value computation hard to understand and predict.

PURPOSE

Split _onchange_template_id + default_get + get_record_data usage into editable
computed stored fields. Each field should have its own method with its triggers
(template, composition mode, ...).

SPECIFICATIONS

Improve composer to correctly compute fields ``email_from`` and ``author_id``
in compute methods.

Email_from computation is coming either from template, either from context.
When having a template with a value set, copy it (in batch mode) or render it
(in monorecord comment mode) on the composer. Otherwise try to take current
user's email. When removing the template, fallback on default thread behavior
(which is current user's email).

Author computation is not controllable from the template currently. We therefore
try to synchronize it with the given email_from (in rendered mode to avoid
trying to find partner based on qweb expressions), or fallback on current user.

Task-2088884 (Mail: Use editable computed stored fields in composer)

Part-of: odoo/odoo#107356
This commit is contained in:
Thibault Delavallée
2023-01-27 19:56:05 +01:00
parent 7ee2e2d5d3
commit 4924df5743
3 changed files with 117 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/l10n_generic_coa/tests/test_mail_performance.py
+6 -10
View File
@@ -269,10 +269,8 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance):
self.assertEqual(len(print_msg.attachment_ids), 3)
self.assertNotIn(self.attachments, print_msg.attachment_ids,
'Attachments should be duplicated, not just linked')
self.assertEqual(print_msg.author_id, self.env.user.partner_id,
'TODO: not synchronized with email_from choice')
# self.assertEqual(print_msg.author_id, self.user_account_other.partner_id,
# 'Should take invoice_user_id partner')
self.assertEqual(print_msg.author_id, self.user_account_other.partner_id,
'Should take invoice_user_id partner')
self.assertEqual(print_msg.email_from, self.user_account_other.email_formatted,
'Should take invoice_user_id email')
self.assertEqual(print_msg.notified_partner_ids, test_customer + self.user_accountman.partner_id)
@@ -285,7 +283,7 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance):
self.assertMailMail(
test_customer,
'sent',
author=self.user_account.partner_id, # author: current user, not synchronized with email_from of template
author=self.user_account_other.partner_id, # author: synchronized with email_from of template
content=f'TemplateBody for {test_move.name}',
email_values={
'attachments_info': [
@@ -373,10 +371,8 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance):
self.assertEqual(len(print_msg.attachment_ids), 3)
self.assertNotIn(self.attachments, print_msg.attachment_ids,
'Attachments should be duplicated, not just linked')
self.assertEqual(print_msg.author_id, self.env.user.partner_id,
'TODO: not synchronized with email_from choice')
# self.assertEqual(print_msg.author_id, self.user_account_other.partner_id,
# 'Should take invoice_user_id partner')
self.assertEqual(print_msg.author_id, self.user_account_other.partner_id,
'Should take invoice_user_id partner')
self.assertEqual(print_msg.email_from, self.user_account_other.email_formatted,
'Should take invoice_user_id email')
self.assertEqual(print_msg.notified_partner_ids, test_customer + self.user_accountman.partner_id)
@@ -389,7 +385,7 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance):
self.assertMailMail(
test_customer,
'sent',
author=self.user_account.partner_id, # author: current user, not synchronized with email_from of template
author=self.user_account_other.partner_id, # author: synchronized with email_from of template
content=f'SpanishBody for {test_move.name}', # translated version
email_values={
'attachments_info': [
addons/mail/wizard/mail_compose_message.py
+82 -18
View File
@@ -9,7 +9,7 @@ import logging
from odoo import _, api, fields, models, tools, Command
from odoo.exceptions import UserError, ValidationError
from odoo.osv import expression
from odoo.tools import email_re
from odoo.tools import email_re, is_html_empty
_logger = logging.getLogger(__name__)
@@ -65,16 +65,6 @@ class MailComposer(models.TransientModel):
result = super().default_get(fields_list)
# author
missing_author = 'author_id' in fields_list and 'author_id' not in result
missing_email_from = 'email_from' in fields_list and 'email_from' not in result
if missing_author or missing_email_from:
author_id, email_from = self.env['mail.thread']._message_compute_author(result.get('author_id'), result.get('email_from'), raise_on_email=False)
if missing_email_from:
result['email_from'] = email_from
if missing_author:
result['author_id'] = author_id
# record context management
if 'model' in fields_list and 'model' not in result:
result['model'] = self.env.context.get('active_model')
@@ -134,9 +124,12 @@ class MailComposer(models.TransientModel):
email_layout_xmlid = fields.Char('Email Notification Layout', copy=False)
email_add_signature = fields.Boolean(default=True)
# origin
email_from = fields.Char('From', help="Email address of the sender. This field is set when no matching partner is found and replaces the author_id field in the chatter.")
email_from = fields.Char(
'From', compute='_compute_authorship', readonly=False, store=True,
help="Email address of the sender. This field is set when no matching partner is found and replaces the author_id field in the chatter.")
author_id = fields.Many2one(
'res.partner', 'Author',
'res.partner', string='Author',
compute='_compute_authorship', readonly=False, store=True,
help="Author of the message. If not set, email_from may hold an email address that did not match any partner.")
# composition
composition_mode = fields.Selection(
@@ -214,6 +207,47 @@ class MailComposer(models.TransientModel):
for composer in self:
composer._evaluate_res_domain()
@api.depends('composition_mode', 'email_from', 'model',
'res_domain', 'res_ids', 'template_id')
def _compute_authorship(self):
""" Computation is coming either from template, either from context.
When having a template with a value set, copy it (in batch mode) or
render it (in monorecord comment mode) on the composer. Otherwise
try to take current user's email. When removing the template, fallback
on default thread behavior (which is current user's email).
Author is not controllable from the template currently. We therefore
try to synchronize it with the given email_from (in rendered mode to
avoid trying to find partner based on qweb expressions), or fallback
on current user. """
Thread = self.env['mail.thread'].with_context(active_test=False)
for composer in self:
rendering_mode = composer.composition_mode == 'comment' and not composer.composition_batch
updated_author_id = None
# update email_from first as it is the main used field currently
if composer.template_id.email_from:
composer._set_value_from_template('email_from')
# removing template or void from -> fallback on current user as default
elif not composer.template_id or not composer.email_from:
composer.email_from = self.env.user.email_formatted
updated_author_id = self.env.user.partner_id.id
# Update author. When being in rendered mode: link with rendered
# email_from or fallback on current user if email does not match.
# When changing template in raw mode or resetting also fallback.
if composer.email_from and rendering_mode and not updated_author_id:
updated_author_id, _ = Thread._message_compute_author(
None, composer.email_from, raise_on_email=False,
)
if not updated_author_id:
updated_author_id = self.env.user.partner_id.id
if not rendering_mode or not composer.template_id:
updated_author_id = self.env.user.partner_id.id
if updated_author_id:
composer.author_id = updated_author_id
@api.depends('res_domain', 'res_ids')
def _compute_composition_batch(self):
""" Determine if batch mode is activated:
@@ -349,8 +383,7 @@ class MailComposer(models.TransientModel):
template = self.env['mail.template'].browse(template_id)
values = dict(
(field, template[field])
for field in ('email_from',
'reply_to',
for field in ('reply_to',
'scheduled_date',
'subject',
)
@@ -372,7 +405,6 @@ class MailComposer(models.TransientModel):
('attachment_ids',
'body_html',
'email_cc',
'email_from',
'email_to',
'mail_server_id',
'partner_ids',
@@ -405,7 +437,6 @@ class MailComposer(models.TransientModel):
).default_get(['attachment_ids',
'body',
'composition_mode',
'email_from',
'mail_server_id',
'model',
'parent_id',
@@ -419,7 +450,6 @@ class MailComposer(models.TransientModel):
(key, default_values[key])
for key in ('attachment_ids',
'body',
'email_from',
'mail_server_id',
'partner_ids',
'reply_to',
@@ -1144,3 +1174,37 @@ class MailComposer(models.TransientModel):
if not tools.is_list_of(res_ids, int):
raise ValidationError(error_msg)
return res_ids
def _set_value_from_template(self, template_fname, composer_fname=False):
""" Set composer value from its template counterpart. In monorecord
comment mode, we get directly the rendered value, giving the real
value to the user. Otherwise we get the raw (unrendered) value from
template, as it will be rendered at send time (for mass mail, whatever
the number of contextual records to mail) or before posting on records
(for comment in batch).
:param str template_fname: name of field on template model, used to
fetch the value (and maybe render it);
:param str composer_fname: name of field on composer model, when field
names do not match (e.g. body_html on template used to populate body
on composer);
"""
self.ensure_one()
composer_fname = composer_fname or template_fname
# fetch template value, check if void
template_value = self.template_id[template_fname] if self.template_id else False
if template_value and template_fname == 'body_html':
template_value = template_value if not is_html_empty(template_value) else False
if template_value:
if self.composition_mode == 'comment' and not self.composition_batch:
res_ids = self._evaluate_res_ids()
rendering_res_ids = res_ids or [0]
self[composer_fname] = self.template_id._generate_template(
rendering_res_ids,
{template_fname},
)[rendering_res_ids[0]][template_fname]
else:
self[composer_fname] = self.template_id[template_fname]
return self[composer_fname]
addons/test_mail/tests/test_mail_composer.py
+29 -26
View File
@@ -79,7 +79,7 @@ class TestMailComposer(TestMailCommon, TestRecipients):
}
])
cls.test_from = '"John Doe" <john@example.com>'
cls.test_from = '"John Doe" <john.doe@test.example.com>'
cls.template = cls.env['mail.template'].create({
'auto_delete': True,
@@ -207,7 +207,8 @@ class TestComposerForm(TestMailComposer):
))
self.assertTrue(composer_form.auto_delete, 'Should take template value')
self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete')
self.assertEqual(composer_form.author_id, self.env.user.partner_id)
self.assertEqual(composer_form.author_id, self.user_employee_2.partner_id,
'MailComposer: author is synchronized with email_from when possible')
self.assertEqual(composer_form.body, f'<p>TemplateBody {self.test_record.name}</p>')
self.assertFalse(composer_form.composition_batch)
self.assertEqual(composer_form.composition_mode, 'comment')
@@ -579,52 +580,54 @@ class TestComposerInternals(TestMailComposer):
# changing template should update its email_from
composer.write({'template_id': self.template.id})
# currently onchange necessary
composer._onchange_template_id_wrapper()
if composition_mode == 'comment' and not batch:
self.assertEqual(composer.author_id, self.partner_1,
'MailComposer: TODO: should try to synchronize with email_from')
self.assertEqual(composer.author_id, self.test_record.user_id.partner_id,
f'MailComposer: should try to link in rendered mode: {composer.author_id.name}, expected {self.env.user.name}')
self.assertEqual(composer.email_from, self.test_record.user_id.email_formatted,
f'MailComposer: should take email_from rendered from template ({composition_mode}-{batch})')
'MailComposer: should take email_from rendered from template')
else:
self.assertEqual(composer.author_id, self.partner_1,
'MailComposer: TODO: email_from is raw, what to do with it ?')
self.assertEqual(composer.author_id, self.env.user.partner_id,
f'MailComposer: should reset to current user in raw mode: {composer.author_id.name}, expected {self.env.user.name}')
self.assertEqual(composer.email_from, self.template.email_from,
f'MailComposer: should take email_from raw from template ({composition_mode}-{batch})')
'MailComposer: should take email_from raw from template')
# manual values are kept over template values
# manual values are kept over template values; if email does not
# match any author, reset author
composer.write({'email_from': self.test_from})
self.assertEqual(composer.author_id, self.partner_1)
self.assertEqual(composer.email_from, self.test_from,
'MailComposer: TODO: author / email_from are not synchronized')
if composition_mode == 'comment' and not batch:
self.assertEqual(composer.author_id, self.test_record.user_id.partner_id,
'MailComposer: TODO: compute not called')
self.assertEqual(composer.email_from, self.test_from,
'MailComposer: manual values should be kept')
else:
self.assertEqual(composer.author_id, self.env.user.partner_id,
'MailComposer: TODO: compute not called')
self.assertEqual(composer.email_from, self.test_from,
'MailComposer: manual values should be kept')
# update with template with void values: void value is not forced in
# rendering mode as well as when copying template values
composer.write({'template_id': template_void.id})
# currently onchange necessary
composer._onchange_template_id_wrapper()
if composition_mode == 'comment' and not batch:
self.assertEqual(composer.author_id, self.partner_1,
self.assertEqual(composer.author_id, self.env.user.partner_id,
'MailComposer: TODO: author / email_from are not synchronized')
self.assertEqual(composer.email_from, self.test_from)
else:
self.assertEqual(composer.author_id, self.partner_1,
self.assertEqual(composer.author_id, self.env.user.partner_id,
'MailComposer: TODO: author / email_from are not synchronized')
self.assertEqual(composer.email_from, self.test_from)
# reset template: values are reset due to call to default_get
composer.write({'template_id': False})
# currently onchange necessary
composer._onchange_template_id_wrapper()
if composition_mode == 'comment' and not batch:
self.assertEqual(composer.author_id, self.partner_1,
self.assertEqual(composer.author_id, self.env.user.partner_id,
'MailComposer: TODO: author / email_from are not synchronized')
self.assertEqual(composer.email_from, self.env.user.email_formatted)
else:
self.assertEqual(composer.author_id, self.partner_1,
self.assertEqual(composer.author_id, self.env.user.partner_id,
'MailComposer: TODO: author / email_from are not synchronized')
self.assertEqual(composer.email_from, self.env.user.email_formatted)
@@ -1412,7 +1415,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
'Author cannot be synchronized with a raw email_from')
self.assertEqual(composer.email_from, self.template.email_from)
else:
author = self.env.user.partner_id
author = self.partner_employee_2
self.assertEqual(composer.author_id, author,
'Author cannot be synchronized with a raw email_from')
self.assertEqual(composer.email_from, self.partner_employee_2.email_formatted)
@@ -1488,7 +1491,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
message = test_record.message_ids[0]
self.assertMailMail(self.partner_employee_2, 'sent',
mail_message=message,
author=author, # author != email_from (template sets only email_from)
author=author, # author is different in batch and monorecord mode (raw or rendered email_from)
email_values={
'body_content': f'TemplateBody {test_record.name}',
'email_from': test_record.user_id.email_formatted, # set by template
@@ -1506,7 +1509,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
)
self.assertMailMail(test_record.customer_id + new_partners, 'sent',
mail_message=message,
author=author, # author != email_from (template sets only email_from)
author=author, # author is different in batch and monorecord mode (raw or rendered email_from)
email_values={
'body_content': f'TemplateBody {test_record.name}',
'email_from': test_record.user_id.email_formatted, # set by template
@@ -1627,7 +1630,7 @@ class TestComposerResultsCommentStatus(TestMailComposer):
self.assertMailMail(
recipient, 'sent',
mail_message=message,
author=self.partner_employee, # author != email_from (template sets only email_from)
author=self.partner_employee_2, # author synchronized with email_from
email_values={
'email_from': self.user_employee_2.email_formatted, # set by template
},