Skip to content
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.

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

[IMP] mail, test_mail, *: add new email.cc.mixin on main models #31275

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+168 −23
Diff settings

Always

Just for now

Copy path View file
@@ -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', 'mail.activity.mixin', 'utm.mixin', 'format.address.mixin', 'mail.blacklist.mixin']
_inherit = ['mail.thread.cc', 'mail.activity.mixin', 'utm.mixin', 'format.address.mixin', 'mail.blacklist.mixin']
_primary_email = 'email_from'

def _default_probability(self):
@@ -74,7 +74,6 @@ def _default_stage_id(self):
index=True, tracking=True, help='When sending mails, the default email address is taken from the Sales Team.')
kanban_state = fields.Selection([('grey', 'No next activity planned'), ('red', 'Next activity late'), ('green', 'Next activity is planned')],
string='Kanban State', compute='_compute_kanban_state')
email_cc = fields.Text('Global CC', help="These email addresses will be added to the CC field of all inbound and outbound emails for this record before being sent. Separate multiple email addresses with a comma")
description = fields.Text('Notes')
tag_ids = fields.Many2many('crm.lead.tag', 'crm_lead_tag_rel', 'lead_id', 'tag_id', string='Tags', help="Classify and analyze your lead/opportunity categories like: Training, Service")
contact_name = fields.Char('Contact Name', tracking=30)
@@ -1204,7 +1203,6 @@ def message_new(self, msg_dict, custom_values=None):
defaults = {
'name': msg_dict.get('subject') or _("No Subject"),
'email_from': msg_dict.get('from'),
'email_cc': msg_dict.get('cc'),
'partner_id': msg_dict.get('author_id', False),
}
if msg_dict.get('author_id'):
@@ -107,6 +107,7 @@
aria-label="Blacklisted" attrs="{'invisible': [('is_blacklisted', '=', False)]}" groups="base.group_user"></i>
<field name="email_from" widget="email"/>
</div>
<field name="email_cc" groups="base.group_no_one"/>
<field name="function"/>
<field name="phone" widget="phone"/>
<field name="mobile"/>
@@ -92,7 +92,7 @@ class Applicant(models.Model):
_name = "hr.applicant"
_description = "Applicant"
_order = "priority desc, id desc"
_inherit = ['mail.thread', 'mail.activity.mixin', 'utm.mixin']
_inherit = ['mail.thread.cc', 'mail.activity.mixin', 'utm.mixin']

def _default_stage_id(self):
if self._context.get('default_job_id'):
@@ -117,8 +117,6 @@ def _default_company_id(self):
active = fields.Boolean("Active", default=True, help="If the active field is set to false, it will allow you to hide the case without removing it.")
description = fields.Text("Description")
email_from = fields.Char("Email", size=128, help="Applicant email")
email_cc = fields.Text("Watchers Emails", size=252,
help="These email addresses will be added to the CC field of all inbound and outbound emails for this record before being sent. Separate multiple email addresses with a comma")
probability = fields.Float("Probability")
partner_id = fields.Many2one('res.partner', "Contact")
create_date = fields.Datetime("Creation Date", readonly=True, index=True)
@@ -407,7 +405,6 @@ def message_new(self, msg, custom_values=None):
'name': msg.get('subject') or _("No Subject"),
'partner_name': val,
'email_from': msg.get('from'),
'email_cc': msg.get('cc'),
'partner_id': msg.get('author_id', False),
}
if msg.get('priority'):
@@ -131,6 +131,7 @@
<group>
<field name="partner_id"/>
<field name="email_from" widget="email"/>
<field name="email_cc" groups="base.group_no_one"/>
<field name="partner_phone"/>
<field name="partner_mobile"/>
<field name="type_id" placeholder="Degree"/>
@@ -10,6 +10,7 @@
from . import mail_activity
from . import mail_mail
from . import mail_thread
from . import mail_cc_mixin
from . import mail_address_mixin
from . import mail_blacklist
from . import mail_channel
@@ -0,0 +1,53 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.

from odoo import _, api, fields, models, tools
from email.utils import formataddr


class MailCCMixin(models.AbstractModel):
_name = 'mail.thread.cc'
_inherit = 'mail.thread'

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Mar 27, 2019

Contributor

It is weird to inherit from mail.thread in this small mixin. I suppose it comes from having to call super on some mail thread methods. In that case naming a bit weird as well as having models inheriting from mail.cc.mixin then mail.thread which is not necessary anymore.

What about naming this mixin mail.thread.cc (as it is build mainly upon mail.thread), and just updating other models with inherit from mail.thread.cc (and not both as this is not necessary) ?

This comment has been minimized.

Copy link
@Xavier-Do

Xavier-Do Mar 27, 2019

Author Contributor

I didn't want to remove inherit to mail.thread in updated models in order to be keep things clear but i guess that if we rename the 'mixin' to mail.thread.cc it is ok. Good idea.

_description = 'Email CC management'

email_cc = fields.Char('Email cc', help='List of cc from incoming emails.')

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Mar 27, 2019

Contributor

I would ensure content is always sanitized at maximum like

"Name correctly escaped" <sanitized email>

I suppose _email_dict purpose is to update CC field only with unknown email adresses and update existing email adresses with new formatted address. This is a good idea. Maybe we could improve a bit your implementation by rebuilding manually what is stored. For example _email_dict could return a dict {'sanitized_email': 'formatted_email'} and we store formatted emails in database, not the raw-one received from incoming emails.

For sanitized emails: use email_normalize in tools so that we always use the same sanitize methodology (basically it lowers everything currently but I think we have plans somewhere in mass mailing to improve it a bit)
For formatted emails: I would manually use a formataddr ("Name with escaping because we could have quotation marks or strange stuff" <sanitized_email>). Indeed default lib formataddr does not put name between brackets and fails with email adresses like "Raoul, the Great raoul@example.com".

This comment has been minimized.

Copy link
@Xavier-Do

Xavier-Do Mar 27, 2019

Author Contributor

"update existing email addresses with new formatted address."
No actually I want to keep the very first one. If some user exchange mails using same cc with different format, I want to update this field only once. So the email_dict ensure that if we have an existing cc, we dont update it (we update old_dict with new_dict)

Last time we talked (i think it was with you) we discussed about keeping email as raw as possible in database exept if we need to search them (it is not the case here). We can sanitize it before using it, but we cannot find the initial format. We may also want to keep cc if they are in a non sanitize able incorrect format.


def _mail_cc_sanitized_raw_dict(self, cc_string):
'''return a dict of sanitize_email:raw_email from a string of cc'''
if not cc_string:
return {}
return {tools.email_normalize(email): formataddr((name, tools.email_normalize(email)))
for (name, email) in tools.email_split_tuples(cc_string)}

@api.model
def message_new(self, msg_dict, custom_values=None):
if custom_values is None:
custom_values = {}
cc_values = {
'email_cc': ", ".join(self._mail_cc_sanitized_raw_dict(msg_dict.get('cc')).values()),
}
cc_values.update(custom_values)
This conversation was marked as resolved by Xavier-Do

This comment has been minimized.

Copy link
@alexkuhn

alexkuhn Feb 20, 2019

Contributor

Just personal preference, but it's easier to understand code with custom_values.update(cc_values) instead

This comment has been minimized.

Copy link
@alexkuhn

alexkuhn Feb 20, 2019

Contributor

Maybe you do that because you do not want to change in place the passed dict custom_values? Would name variable cc_values differently then, something like new_custom_values?

return super(MailCCMixin, self).message_new(msg_dict, cc_values)

@api.multi
def message_update(self, msg_dict, update_vals=None):
'''Adds cc email to self.email_cc while trying to keep email as raw as possible but unique'''
if update_vals is None:
update_vals = {}
cc_values = {}
new_cc = self._mail_cc_sanitized_raw_dict(msg_dict.get('cc'))
if new_cc:
old_cc = self._mail_cc_sanitized_raw_dict(self.email_cc)
new_cc.update(old_cc)
cc_values['email_cc'] = ", ".join(new_cc.values())
cc_values.update(update_vals)
This conversation was marked as resolved by Xavier-Do

This comment has been minimized.

Copy link
@alexkuhn

alexkuhn Feb 20, 2019

Contributor

same thing as with message_new (easier to read that update_vals is updated)

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()
for record in self:
if record.email_cc:
for email in tools.email_split_and_format(record.email_cc):
record._message_add_suggested_recipient(recipients, email=email, reason=_('CC Email'))
return recipients
@@ -21,6 +21,7 @@ var ChatterComposer = BasicComposer.extend({
template: 'mail.chatter.Composer',
events: _.extend({}, BasicComposer.prototype.events, {
'click .o_composer_button_full_composer': '_onOpenFullComposer',
'click .o_suggested_show_more_less': '_onClickPartnerSuggestionsReadMoreLess',
}),
init: function (parent, model, suggestedPartners, options) {
this._super(parent, options);
@@ -237,6 +238,14 @@ var ChatterComposer = BasicComposer.extend({
// Handlers
//--------------------------------------------------------------------------

/**
* @private
* @param {MouseEvent} ev
*/
_onClickPartnerSuggestionsReadMoreLess: function (ev) {
ev.preventDefault();
this.$('.o_suggested_toggle').toggleClass('o_hidden');
},
/**
* @private
*/
@@ -60,7 +60,7 @@
<!-- List of followers -->
<div class="o_composer_suggested_partners">
<t t-foreach='widget.suggestedPartners' t-as='recipient'>
<div t-attf-title="Add as recipient and follower (reason: #{recipient.reason})">
<div t-attf-title="Add as recipient and follower (reason: #{recipient.reason})" t-att-class="recipient_index > 2 ? 'o_suggested_toggle o_hidden' : ''">
<div class="custom-control custom-checkbox">
<input type="checkbox"
class="custom-control-input"
@@ -74,6 +74,14 @@
</div>
</div>
</t>
<t t-if='widget.suggestedPartners.length > 3'>
<a href="#" class='o_suggested_show_more_less o_suggested_toggle'>
Show <t t-esc="widget.suggestedPartners.length - 3"/> more
</a>
<a href="#" class='o_suggested_show_more_less o_suggested_toggle o_hidden'>
Show <t t-esc="widget.suggestedPartners.length - 3"/> less
</a>
</t>
</div>
</t>
</t>
@@ -81,7 +81,7 @@ def unlink(self):
return res

def get_alias_model_name(self, vals):
return vals.get('alias_model', 'maintenance.equipment')
return vals.get('alias_model', 'maintenance.request')

def get_alias_values(self):
values = super(MaintenanceEquipmentCategory, self).get_alias_values()
@@ -253,9 +253,10 @@ def _cron_generate_requests(self):
if not next_requests:
equipment._create_new_request(equipment.next_action_date)


class MaintenanceRequest(models.Model):
_name = 'maintenance.request'
_inherit = ['mail.thread', 'mail.activity.mixin']
_inherit = ['mail.thread.cc', 'mail.activity.mixin']
_description = 'Maintenance Request'
_order = "id desc"

@@ -94,6 +94,7 @@
class="oe_inline"/> <span class="ml8">hours</span>
</div>
<field name="priority" widget="priority"/>
<field name="email_cc" string="Email cc" groups="base.group_no_one"/>
</group>
</group>
<field name='description' placeholder="Internal Note ......."/>
@@ -399,7 +399,7 @@ class Task(models.Model):
_name = "project.task"
_description = "Task"
_date_name = "date_assign"
_inherit = ['portal.mixin', 'mail.thread', 'mail.activity.mixin', 'rating.mixin']
_inherit = ['portal.mixin', 'mail.thread.cc', 'mail.activity.mixin', 'rating.mixin']
_mail_post_access = 'read'
_order = "priority desc, sequence, id desc"

@@ -496,8 +496,6 @@ def _read_group_stage_ids(self, stages, domain, order):
subtask_project_id = fields.Many2one('project.project', related="project_id.subtask_project_id", string='Sub-task Project', readonly=True)
subtask_count = fields.Integer("Sub-task count", compute='_compute_subtask_count')
email_from = fields.Char(string='Email', help="These people will receive email.", index=True)
email_cc = fields.Char(string='Watchers Emails', help="""These email addresses will be added to the CC field of all inbound
and outbound emails for this record before being sent. Separate multiple email addresses with a comma""")
# Computed field about working time elapsed between record creation and assignation/closing.
working_hours_open = fields.Float(compute='_compute_elapsed', string='Working hours to assign', store=True, group_operator="avg")
working_hours_close = fields.Float(compute='_compute_elapsed', string='Working hours to close', store=True, group_operator="avg")
@@ -835,7 +833,6 @@ def message_new(self, msg, custom_values=None):
defaults = {
'name': msg.get('subject') or _("No Subject"),
'email_from': msg.get('from'),
'email_cc': msg.get('cc'),
'planned_hours': 0.0,
'partner_id': msg.get('author_id')
}
@@ -124,3 +124,10 @@ class MailModel(models.Model):
def _value_pc(self):
for record in self:
record.value_pc = float(record.value) / 100


class MailCC(models.Model):
_name = 'mail.test.cc'
_inherit = ['mail.thread.cc']

name = fields.Char()
@@ -10,3 +10,6 @@ access_mail_test_full_portal,mail.test.full.portal,model_mail_test_full,base.gro
access_mail_test_full_user,mail.test.full.user,model_mail_test_full,base.group_user,1,1,1,1
access_mail_test_portal,mail.test.portal,model_mail_test,base.group_portal,1,1,0,0
access_mail_test_user,mail.test.user,model_mail_test,base.group_user,1,1,1,1
access_mail_test_cc_portal,mail.test.cc.portal,model_mail_test_cc,base.group_portal,1,0,0,0
access_mail_test_cc_user,mail.test.cc.user,model_mail_test_cc,base.group_user,1,1,1,1

@@ -6,6 +6,7 @@
from . import test_mail_mail
from . import test_mail_race
from . import test_mail_resend
from . import test_mail_cc
from . import test_mail_channel
from . import test_mail_gateway
from . import test_mail_template
@@ -0,0 +1,66 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.

from odoo.addons.test_mail.tests import common
from odoo.tests import tagged
from odoo.tools import mute_logger, email_split_and_format
from odoo.addons.test_mail.data.test_mail_data import MAIL_TEMPLATE


@tagged('cc_test')
class TestMailResend(common.BaseFunctionalTest, common.MockEmails):

@mute_logger('odoo.addons.mail.models.mail_thread')
def test_message_cc_new(self):
alias = self.env['mail.alias'].create({
'alias_name': 'cc_record',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.cc').id,
'alias_contact': 'everyone'})
record = self.format_and_process(MAIL_TEMPLATE, target_model='mail.test.cc', to='cc_record@example.com',
cc='cc1@example.com, cc2@example.com')
cc = email_split_and_format(record.email_cc)
self.assertEqual(sorted(cc), ['cc1@example.com', 'cc2@example.com'], 'cc should contains exactly 2 cc')

@mute_logger('odoo.addons.mail.models.mail_thread')
def test_message_cc_update_with_old(self):
record = self.env['mail.test.cc'].create({'email_cc': 'cc1 <cc1@example.com>, cc2@example.com'})
alias = self.env['mail.alias'].create({
'alias_name': 'cc_record',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.cc').id,
'alias_contact': 'everyone',
'alias_force_thread_id': record.id})

self.format_and_process(MAIL_TEMPLATE, subject='Re: Frogs', target_model='mail.test.cc',
msg_id='1198923581.41972151344608186799.JavaMail.diff1@agrolait.com',
to='cc_record@example.com',
cc='cc2 <cc2@example.com>, cc3@example.com')
cc = email_split_and_format(record.email_cc)
self.assertEqual(sorted(cc), ['cc1 <cc1@example.com>', 'cc2@example.com', 'cc3@example.com'], 'new cc should have been added on record (unique)')

@mute_logger('odoo.addons.mail.models.mail_thread')
def test_message_cc_update_no_old(self):
record = self.env['mail.test.cc'].create({})
alias = self.env['mail.alias'].create({
'alias_name': 'cc_record',
'alias_user_id': False,
'alias_model_id': self.env['ir.model']._get('mail.test.cc').id,
'alias_contact': 'everyone',
'alias_force_thread_id': record.id})
self.format_and_process(MAIL_TEMPLATE, subject='Re: Frogs', target_model='mail.test.cc',
msg_id='1198923581.41972151344608186799.JavaMail.diff1@agrolait.com',
to='cc_record@example.com',
cc='cc2 <cc2@example.com>, cc3@example.com')
cc = email_split_and_format(record.email_cc)
self.assertEqual(sorted(cc), ['cc2 <cc2@example.com>', 'cc3@example.com'], 'new cc should have been added on record (unique)')

@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 <cc3@example.com>'})
suggestions = record.message_get_suggested_recipients()[record.id]
self.assertEqual(sorted(suggestions), [
(False, 'cc1@example.com', 'CC Email'),
(False, 'cc2@example.com', 'CC Email'),
(False, 'cc3 <cc3@example.com>', 'CC Email')
], 'cc should be in suggestions')
Copy path View file
@@ -474,28 +474,29 @@ def email_send(email_from, email_to, subject, body, email_cc=None, email_bcc=Non
cr.close()
return res

def email_split(text):
""" Return a list of the email addresses found in ``text`` """
def email_split_tuples(text):
""" Return a list of (name, email) addresse tuples found in ``text``"""
if not text:
return []
return [addr[1] for addr in getaddresses([text])
return [(addr[0], addr[1]) for addr in getaddresses([text])
# getaddresses() returns '' when email parsing fails, and
# sometimes returns emails without at least '@'. The '@'
# is strictly required in RFC2822's `addr-spec`.
if addr[1]
if '@' in addr[1]]

def email_split(text):
""" Return a list of the email addresses found in ``text`` """
if not text:
return []
return [email for (name, email) in email_split_tuples(text)]

def email_split_and_format(text):
""" Return a list of email addresses found in ``text``, formatted using
formataddr. """
if not text:
return []
return [formataddr((addr[0], addr[1])) for addr in getaddresses([text])
# getaddresses() returns '' when email parsing fails, and
# sometimes returns emails without at least '@'. The '@'
# is strictly required in RFC2822's `addr-spec`.
if addr[1]
if '@' in addr[1]]
return [formataddr((name, email)) for (name, email) in email_split_tuples(text)]

def email_normalize(text):
""" Sanitize and standardize email address entries.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.