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

[IMP] Add more detailed default email subjects from chatter

Browse Source 
All emails sent from the chatter start with "Re:" followed
by the name of the record. The name of the record alone is sometimes not enough for
the followers to understand what the mail is about.
Additionally, "Re:" does not make sense when starting a conversation.

This commit gives better default subject
for event registrations and allows thread models
to override the default subject of messages.

This also removes "Re:" from default mail subjects.

Task-2833215

closes odoo/odoo#95817

Signed-off-by: Stéphane Debauche (std) <std@odoo.com>
This commit is contained in:
Renaud Thiry
2023-01-05 15:36:53 +01:00
parent 86e2cc1167
commit f039f57eb8
15 changed files with 196 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/event/models/event_registration.py
+15 -18
View File
@@ -152,25 +152,9 @@ class EventRegistration(models.Model):
return ret
def name_get(self):
""" Custom name_get implementation to better differentiate registrations
linked to a given partner but with different name (one partner buying
several registrations)
* name, partner_id has no name -> take name
* partner_id has name, name void or same -> take partner name
* both have name: partner + name
""" Custom name_get in case a registration is nott linked to an attendee
"""
ret_list = []
for registration in self:
if registration.partner_id.name:
if registration.name and registration.name != registration.partner_id.name:
name = '%s, %s' % (registration.partner_id.name, registration.name)
else:
name = registration.partner_id.name
else:
name = registration.name
ret_list.append((registration.id, name))
return ret_list
return [(registration.id, registration.name or f"#{registration.id}") for registration in self]
def toggle_active(self):
pre_inactive = self - self.filtered(self._active_name)
@@ -252,6 +236,19 @@ class EventRegistration(models.Model):
# MAILING / GATEWAY
# ------------------------------------------------------------
def _message_compute_subject(self):
if self.name:
return _(
"%(event_name)s - Registration for %(attendee_name)s",
event_name=self.event_id.name,
attendee_name=self.name,
)
return _(
"%(event_name)s - Registration #%(registration_id)s",
event_name=self.event_id.name,
registration_id=self.id,
)
def _message_get_suggested_recipients(self):
recipients = super(EventRegistration, self)._message_get_suggested_recipients()
public_users = self.env['res.users'].sudo()
addons/im_livechat/tests/test_message.py
+1
View File
@@ -54,6 +54,7 @@ class TestImLivechatMessage(TransactionCase):
'guestAuthor': [('clear',)],
'history_partner_ids': [],
'id': message.id,
'default_subject': channel_livechat_1.name,
'is_discussion': False,
'is_note': True,
'is_notification': False,
addons/mail/models/mail_message.py
+5 -5
View File
@@ -846,13 +846,12 @@ class Message(models.Model):
'name': message_sudo.author_guest_id.name,
} if message_sudo.author_guest_id else [('clear',)]
if message_sudo.model and message_sudo.res_id:
record_name = self.env[message_sudo.model] \
.browse(message_sudo.res_id) \
.sudo() \
.with_prefetch(thread_ids_by_model_name[message_sudo.model]) \
.display_name
record_sudo = self.env[message_sudo.model].browse(message_sudo.res_id).sudo()
record_name = record_sudo.with_prefetch(thread_ids_by_model_name[message_sudo.model]).display_name
default_subject = record_sudo._message_compute_subject()
else:
record_name = False
default_subject = False
reactions_per_content = defaultdict(lambda: self.env['mail.message.reaction'])
for reaction in message_sudo.reaction_ids:
reactions_per_content[reaction.content] |= reaction
@@ -868,6 +867,7 @@ class Message(models.Model):
allowed_tracking_ids = message_sudo.tracking_value_ids.filtered(lambda tracking: not tracking.field_groups or self.env.is_superuser() or self.user_has_groups(tracking.field_groups))
vals.update({
'author': author,
'default_subject': default_subject,
'guestAuthor': guestAuthor,
'notifications': message_sudo.notification_ids._filtered_for_web_client()._notification_format(),
'attachment_ids': message_sudo.attachment_ids._attachment_format() if not legacy else message_sudo.attachment_ids._attachment_format(legacy=True),
addons/mail/models/mail_thread.py
+18 -1
View File
@@ -2320,6 +2320,14 @@ class MailThread(models.AbstractModel):
parent_id = current_ancestor.parent_id.id if current_ancestor.parent_id else parent_id
return parent_id
def _message_compute_subject(self):
"""Overridable method to get the default subject for a message related to this record.
:return str:
"""
self.ensure_one()
return self.name_get()[0][1]
def _message_create(self, values_list):
if not isinstance(values_list, (list)):
values_list = [values_list]
@@ -2739,7 +2747,16 @@ class MailThread(models.AbstractModel):
:param dict base_mail_values: base mail.mail values, holding message
to notify (mail_message_id and its fields), server, references, subject.
"""
mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) # in cache, no queries
mail_subject = message.subject
if not message.subject:
thread = self
if not thread and message.model:
thread = self.env[message.model].browse(message.res_id)
if not thread:
mail_subject = message.record_name
else:
mail_subject = thread._message_compute_subject()
# Replace new lines by spaces to conform to email headers requirements
mail_subject = ' '.join((mail_subject or '').splitlines())
# compute references: set references to the parent and add current message just to
addons/mail/static/src/components/message/message.xml
+1 -1
View File
@@ -119,7 +119,7 @@
'border-warning bg-warning-light opacity-50': message.isHighlighted and (message.isDiscussionOrNotification or message.message_type === 'sms'),
}"
/>
<em t-if="message.subject and !message.isSubjectSimilarToOriginThreadName" class="o_MessageView_subject position-relative mb-1 me-2">Subject: <t t-esc="message.subject"/></em>
<em t-if="message.subject and !message.isSubjectSimilarToOriginThreadNameOrDefaultSubject" class="o_MessageView_subject position-relative mb-1 me-2">Subject: <t t-esc="message.subject"/></em>
<div class="o_MessageView_content position-relative text-break" t-ref="content">
<div t-if="!composerViewInEditing" class="o_MessageView_prettyBody" t-ref="prettyBody"/><!-- message.prettyBody is inserted here from _update() -->
<ComposerView t-if="composerViewInEditing"
addons/mail/static/src/models/message.js
+18 -7
View File
@@ -42,6 +42,9 @@ Model({
if ("id" in data) {
data2.id = data.id;
}
if ("default_subject" in data) {
data2.defaultSubject = data.default_subject;
}
if ("is_discussion" in data) {
data2.is_discussion = data.is_discussion;
}
@@ -363,6 +366,10 @@ Model({
return !this.messaging.isCurrentUserGuest && !this.isTemporary && !this.isTransient;
},
}),
/**
* The subject the message would have received by default
*/
defaultSubject: attr(),
date: attr(),
/**
* Determines the date of the message as a moment object.
@@ -534,25 +541,29 @@ Model({
},
}),
/**
* States whether `originThread.name` and `subject` contain similar
* values except it contains the extra prefix at the start
* of the subject.
* States whether `originThread.name` or `defaultSubject` contain
* similar values to `subject` with prefixes.
*
* This is necessary to avoid displaying the subject, if
* the subject is same as threadname.
* the subject is same as threadname or the default subject for that thread.
*/
isSubjectSimilarToOriginThreadName: attr({
isSubjectSimilarToOriginThreadNameOrDefaultSubject: attr({
compute() {
if (!this.subject || !this.originThread || !this.originThread.name) {
if (!this.subject || ((!this.originThread || !this.originThread.name) && !this.defaultSubject)) {
return false;
}
const threadName = this.originThread.name.toLowerCase().trim();
const defaultSubject = this.defaultSubject.toLowerCase();
const hiddenSubjects = [defaultSubject];
if (defaultSubject !== threadName) {
hiddenSubjects.push(threadName);
}
const prefixList = ["re:", "fw:", "fwd:"];
let cleanedSubject = this.subject.toLowerCase();
let wasSubjectCleaned = true;
while (wasSubjectCleaned) {
wasSubjectCleaned = false;
if (threadName === cleanedSubject) {
if (hiddenSubjects.includes(cleanedSubject)) {
return true;
}
for (const prefix of prefixList) {
addons/mail/static/tests/helpers/mock_server/models/mail_message.js
+2
View File
@@ -166,6 +166,8 @@ patch(MockServer.prototype, "mail/models/mail_message", {
attachment_ids: formattedAttachments,
author: formattedAuthor,
history_partner_ids: historyPartnerIds,
default_subject: message.model && message.res_id &&
this.mockMailThread_MessageComputeSubject(message.model, [message.res_id]).get(message.res_id),
linkPreviews: linkPreviewsFormatted,
needaction_partner_ids: needactionPartnerIds,
notifications,
addons/mail/static/tests/helpers/mock_server/models/mail_thread.js
+11
View File
@@ -78,6 +78,17 @@ patch(MockServer.prototype, "mail/models/mail_thread", {
return [author_id, email_from];
},
/**
* @param {string} model
* @param {integer[]} ids
*
* @returns {Map<integer:string>}
* Simulate `_message_compute_subject` on `mail.thread`
*/
mockMailThread_MessageComputeSubject(model, ids) {
const records = this.getRecords(model, [["id", "in", ids]]);
return new Map(records.map(record => [record.id, record.name || '']));
},
/**
* Simulates `_message_add_suggested_recipient` on `mail.thread`.
*
addons/mail/static/tests/helpers/mock_server/models/res_fake.js
+9
View File
@@ -35,4 +35,13 @@ patch(MockServer.prototype, "mail/models/res_fake", {
}
return result;
},
/**
* @override
*/
mockMailThread_MessageComputeSubject(model, ids) {
if (model === 'res.fake') {
return new Map(ids.map(id => [id, "Custom Default Subject"]));
}
return this._super(model, ids);
},
});
addons/mail/static/tests/qunit_suite_tests/components/chatter_tests.js
+61 -2
View File
@@ -371,7 +371,7 @@ QUnit.module("mail", {}, function () {
);
QUnit.test(
"should display subject when subject is not the same as the thread name",
"should display subject when subject isn't infered from the record",
async function (assert) {
assert.expect(2);
@@ -428,11 +428,70 @@ QUnit.module("mail", {}, function () {
assert.containsNone(
document.body,
".o_MessageView_subject",
"should not display subject of the message"
"should not display subject of the message",
);
}
);
QUnit.test(
"should not display subject when subject is the same as the default subject",
async function (assert) {
assert.expect(1);
const pyEnv = await startServer();
const fakeRecordId = pyEnv["res.fake"].create({
name: "Salutations, voyageur",
});
pyEnv["mail.message"].create({
body: "not empty",
model: "res.fake",
res_id: fakeRecordId,
subject: "Custom Default Subject", // default subject for res.fake, set on the model
});
const { openView } = await start();
await openView({
res_id: fakeRecordId,
res_model: "res.fake",
views: [[false, "form"]],
});
assert.containsNone(
document.body,
".o_MessageView_subject",
"should not display subject of the message",
);
}
);
QUnit.test(
"should not display subject when subject is the same as the thread name with custom default subject",
async function (assert) {
assert.expect(1);
const pyEnv = await startServer();
const fakeRecordId = pyEnv["res.fake"].create({
name: "Salutations, voyageur",
});
pyEnv["mail.message"].create({
body: "not empty",
model: "res.fake",
res_id: fakeRecordId,
subject: "Salutations, voyageur",
});
const { openView } = await start();
await openView({
res_id: fakeRecordId,
res_model: "res.fake",
views: [[false, "form"]],
});
assert.containsNone(
document.body,
".o_MessageView_subject",
"should not display subject of the message",
);
}
);
QUnit.test(
"should not display user notification messages in chatter",
async function (assert) {
addons/mail/static/tests/tours/mail_full_composer_test_tour.js
+2 -2
View File
@@ -54,9 +54,9 @@ tour.register(
trigger: '[name="subject"] input',
run() {
const subjectValue = document.querySelector('[name="subject"] input').value;
if (subjectValue !== "Re: Test User") {
if (subjectValue !== "Test User") {
console.error(
`Full composer should have "Re: Test User" in subject input (actual: ${subjectValue})`
`Full composer should have "Test User" in subject input (actual: ${subjectValue})`
);
}
},
addons/mail/wizard/mail_compose_message.py
+7 -7
View File
@@ -191,21 +191,21 @@ class MailComposer(models.TransientModel):
if values.get('parent_id'):
parent = self.env['mail.message'].browse(values.get('parent_id'))
result['record_name'] = parent.record_name
subject = tools.ustr(parent.subject or parent.record_name or '')
if not values.get('model'):
result['model'] = parent.model
if not values.get('res_id'):
result['res_id'] = parent.res_id
partner_ids = values.get('partner_ids', list()) + parent.partner_ids.ids
result['partner_ids'] = partner_ids
record = self.env[values.get('model') or result['model']].browse(values.get('res_id') or result['res_id'])
parent_subject = tools.ustr(parent.subject or '')
subject = parent_subject or record._message_compute_subject()
elif values.get('model') and values.get('res_id'):
doc_name_get = self.env[values.get('model')].browse(values.get('res_id')).name_get()
result['record_name'] = doc_name_get and doc_name_get[0][1] or ''
subject = tools.ustr(result['record_name'])
record = self.env[values['model']].browse(values['res_id'])
doc_name = record.display_name
result['record_name'] = doc_name or ''
subject = record._message_compute_subject()
re_prefix = _('Re:')
if subject and not (subject.startswith('Re:') or subject.startswith(re_prefix)):
subject = "%s %s" % (re_prefix, subject)
result['subject'] = subject
return result
addons/test_mail/models/test_mail_models.py
+4
View File
@@ -115,6 +115,10 @@ class MailTestTicket(models.Model):
user_id = fields.Many2one('res.users', 'Responsible', tracking=1)
container_id = fields.Many2one('mail.test.container', tracking=True)
def _message_compute_subject(self):
self.ensure_one()
return f"Ticket for {self.name} on {self.datetime.strftime('%m/%d/%Y, %H:%M:%S')}"
def _message_get_default_recipients(self):
return dict(
(record.id, {
addons/test_mail/tests/test_mail_composer.py
+27 -7
View File
@@ -120,9 +120,7 @@ class TestComposerForm(TestMailComposer):
self.assertFalse(composer_form.reply_to)
self.assertFalse(composer_form.reply_to_force_new)
self.assertEqual(composer_form.res_id, self.test_record.id)
self.assertEqual(
composer_form.subject, 'Re: %s' % self.test_record.name,
'MailComposer: comment mode should have default subject Re: record_name')
self.assertEqual(composer_form.subject, self.test_record._message_compute_subject())
self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment'))
@users('employee')
@@ -503,7 +501,7 @@ class TestComposerInternals(TestMailComposer):
# (aka subject for comment mode)
if composition_mode == 'comment':
self.assertFalse(composer.body)
self.assertEqual(composer.subject, 'Re: %s' % self.test_record.name)
self.assertEqual(composer.subject, self.test_record._message_compute_subject())
# TDE FIXME: server id is kept, not sure why
# self.assertFalse(composer.mail_server_id.id)
self.assertEqual(composer.mail_server_id, self.template.mail_server_id)
@@ -572,6 +570,25 @@ class TestComposerInternals(TestMailComposer):
self.assertEqual(composer.mail_server_id.id, False)
self.assertEqual(composer.record_name, 'CustomName')
@users('employee')
def test_mail_composer_default_subject(self):
"""Make sure the default subject is applied in the composer."""
simple_record = self.env['mail.test.simple'].create({'name': 'TestA'})
ticket_record = self.env['mail.test.ticket'].create({'name': 'Test1'})
# default behaviour, use the record name
ctx = self._get_web_context(simple_record, add_web=False, composition_mode='comment')
_, message = self.env['mail.compose.message'].with_context(ctx).create({
'body': '<p>Test Body</p>',
})._action_send_mail()
self.assertEqual(message.subject, simple_record.name)
# custom subject
ctx = self._get_web_context(ticket_record, add_web=False, composition_mode='comment')
_, messages = self.env['mail.compose.message'].with_context(ctx).create({
'body': '<p>Test Body</p>',
})._action_send_mail()
self.assertEqual(messages.subject, ticket_record._message_compute_subject())
@users('employee')
@mute_logger('odoo.models.unlink')
def test_mail_composer_recipients(self):
@@ -736,7 +753,10 @@ class TestComposerInternals(TestMailComposer):
def test_mail_composer_parent(self):
""" Test specific management in comment mode when having parent_id set:
record_name, subject, parent's partners. """
parent = self.test_record.message_post(body='Test', partner_ids=(self.partner_1 + self.partner_2).ids)
subject = "Parent Subject"
parent = self.test_record.message_post(subject=subject,
body='Test',
partner_ids=(self.partner_1 + self.partner_2).ids)
composer = self.env['mail.compose.message'].with_context(
self._get_web_context(self.test_record, add_web=False, default_parent_id=parent.id)
@@ -749,7 +769,7 @@ class TestComposerInternals(TestMailComposer):
self.assertEqual(composer.parent_id, parent)
self.assertEqual(composer.partner_ids, self.partner_1 + self.partner_2)
self.assertEqual(composer.record_name, self.test_record.name)
self.assertEqual(composer.subject, 'Re: %s' % self.test_record.name)
self.assertEqual(composer.subject, subject)
@users('user_rendering_restricted')
@mute_logger('odoo.tests', 'odoo.addons.base.models.ir_rule', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
@@ -970,7 +990,7 @@ class TestComposerResultsComment(TestMailComposer):
message = self.test_record.message_ids[0]
self.assertEqual(message.author_id, self.user_employee.partner_id)
self.assertEqual(message.body, '<p>Test Body</p>')
self.assertEqual(message.subject, 'Re: %s' % self.test_record.name)
self.assertEqual(message.subject, self.test_record._message_compute_subject())
self.assertEqual(message.subtype_id, self.env.ref('mail.mt_comment'))
self.assertEqual(message.partner_ids, self.partner_1 | self.partner_2)
addons/test_mail/tests/test_message_post.py
+15 -1
View File
@@ -325,6 +325,20 @@ class TestMessageNotify(TestMessagePostCommon):
subject='Notify Subject',
)
@users('employee')
def test_notify_default_subject(self):
""" Test notify in batch. Currently not supported. """
test_partner = self.env['res.partner'].sudo().create([{'name': 'John', 'email': 'john@test.lan'}])
test_record = self.env['mail.test.ticket'].create({'name': 'Test'})
mt_comment = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment')
test_record.message_subscribe([test_partner.id], subtype_ids=[mt_comment])
with self.mock_mail_gateway(), self.mock_mail_app():
test_record.message_post(body="Test Message", message_type="comment", subtype_id=mt_comment)
self.assertEqual(len(self._new_msgs), 1)
self.assertEqual(self._new_msgs.subject, test_record._message_compute_subject())
@users('employee')
@mute_logger('odoo.addons.mail.models.mail_mail')
def test_notify_from_user_id(self):
@@ -911,7 +925,7 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase):
[self.partner_2],
body_content='<p>Test Answer Bis</p>',
reply_to=msg.reply_to,
subject='Re: %s' % self.test_record.name,
subject=self.test_record.name,
references_content='openerp-%d-mail.test.simple' % self.test_record.id,
references='%s %s' % (parent_msg.message_id, new_msg.message_id),
)