diff --git a/addons/account/wizard/account_invoice_send.py b/addons/account/wizard/account_invoice_send.py index 6c9468b9723..ce951d44a2e 100644 --- a/addons/account/wizard/account_invoice_send.py +++ b/addons/account/wizard/account_invoice_send.py @@ -78,7 +78,7 @@ class AccountInvoiceSend(models.TransientModel): if wizard.composer_id: wizard.composer_id.template_id = wizard.template_id.id wizard._compute_composition_mode() - wizard.composer_id.onchange_template_id_wrapper() + wizard.composer_id._onchange_template_id_wrapper() @api.onchange('is_email') def onchange_is_email(self): @@ -94,7 +94,7 @@ class AccountInvoiceSend(models.TransientModel): self.composer_id.template_id = self.template_id.id self._compute_composition_mode() if not self.env.context.get("wizard_opened"): - self.composer_id.onchange_template_id_wrapper() + self.composer_id._onchange_template_id_wrapper() @api.onchange('is_email') def _compute_invoice_without_email(self): @@ -117,7 +117,7 @@ class AccountInvoiceSend(models.TransientModel): def _send_email(self): if self.is_email: # with_context : we don't want to reimport the file we just exported. - self.composer_id.with_context(no_new_invoice=True, mail_notify_author=self.env.user.partner_id in self.composer_id.partner_ids).send_mail() + self.composer_id.with_context(no_new_invoice=True, mail_notify_author=self.env.user.partner_id in self.composer_id.partner_ids)._action_send_mail() if self.env.context.get('mark_invoice_as_sent'): #Salesman send posted invoice, without the right to write #but they should have the right to change this flag @@ -154,7 +154,7 @@ class AccountInvoiceSend(models.TransientModel): def save_as_template(self): self.ensure_one() - self.composer_id.save_as_template() + self.composer_id.action_save_as_template() self.template_id = self.composer_id.template_id.id action = _reopen(self, self.id, self.model, context=self._context) action.update({'name': _('Send Invoice')}) diff --git a/addons/coupon/models/mail_compose_message.py b/addons/coupon/models/mail_compose_message.py index a819777a75f..d4b50a79464 100644 --- a/addons/coupon/models/mail_compose_message.py +++ b/addons/coupon/models/mail_compose_message.py @@ -7,9 +7,9 @@ from odoo import models class MailComposeMessage(models.TransientModel): _inherit = 'mail.compose.message' - def send_mail(self, **kwargs): + def _action_send_mail(self, **kwargs): for wizard in self: if self._context.get('mark_coupon_as_sent') and wizard.model == 'coupon.coupon' and wizard.partner_ids: # Mark coupon as sent in sudo, as helpdesk users don't have the right to write on coupons self.env[wizard.model].sudo().browse(wizard.res_id).state = 'sent' - return super().send_mail(**kwargs) + return super()._action_send_mail(**kwargs) diff --git a/addons/crm/tests/test_crm_lead_convert_mass.py b/addons/crm/tests/test_crm_lead_convert_mass.py index 91f3dced4c5..b3671aa027f 100644 --- a/addons/crm/tests/test_crm_lead_convert_mass.py +++ b/addons/crm/tests/test_crm_lead_convert_mass.py @@ -24,7 +24,7 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon): with self.assertQueryCount(user_sales_manager=0): test_leads = self.env['crm.lead'].browse(test_leads.ids) - with self.assertQueryCount(user_sales_manager=255): # often 252, sometimes +3 on runbot + with self.assertQueryCount(user_sales_manager=254): # crm only: 251 - often 251, sometimes +3 on runbot test_leads._handle_salesmen_assignment(user_ids=user_ids, team_id=False) self.assertEqual(test_leads.team_id, self.sales_team_convert | self.sales_team_1) @@ -42,7 +42,7 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon): with self.assertQueryCount(user_sales_manager=0): test_leads = self.env['crm.lead'].browse(test_leads.ids) - with self.assertQueryCount(user_sales_manager=223): # crm only: 218 - generally 220, sometimes +2/+3 on runbot + with self.assertQueryCount(user_sales_manager=222): # crm only: 217 - generally 219, sometimes +2/+3 on runbot test_leads._handle_salesmen_assignment(user_ids=user_ids, team_id=team_id) self.assertEqual(test_leads.team_id, self.sales_team_convert) @@ -166,7 +166,7 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon): test_leads = self._create_leads_batch(count=50, user_ids=[False]) user_ids = self.assign_users.ids - with self.assertQueryCount(user_sales_manager=1369): # still some randomness (1366 spotted) - crm only: ?? + with self.assertQueryCount(user_sales_manager=1336): # still some randomness (1266 generally on runbot) - crm only: 1257 mass_convert = self.env['crm.lead2opportunity.partner.mass'].with_context({ 'active_model': 'crm.lead', 'active_ids': test_leads.ids, diff --git a/addons/google_calendar/tests/test_sync_odoo2google.py b/addons/google_calendar/tests/test_sync_odoo2google.py index 90e979a4601..607d9d911ba 100644 --- a/addons/google_calendar/tests/test_sync_odoo2google.py +++ b/addons/google_calendar/tests/test_sync_odoo2google.py @@ -102,7 +102,7 @@ class TestSyncOdoo2Google(TestSyncGoogle): }) partner_model = self.env.ref('base.model_res_partner') partner = self.env['res.partner'].search([], limit=1) - with self.assertQueryCount(__system__=3637): + with self.assertQueryCount(__system__=3635): event = self.env['calendar.event'].create({ 'name': "Event", 'start': datetime(2020, 1, 15, 8, 0), diff --git a/addons/hr_work_entry_holidays/tests/test_performance.py b/addons/hr_work_entry_holidays/tests/test_performance.py index f83e76487eb..b3c29b540ec 100644 --- a/addons/hr_work_entry_holidays/tests/test_performance.py +++ b/addons/hr_work_entry_holidays/tests/test_performance.py @@ -31,7 +31,7 @@ class TestWorkEntryHolidaysPerformance(TestWorkEntryHolidaysBase): self.richard_emp.generate_work_entries(date(2018, 1, 1), date(2018, 1, 2)) leave = self.create_leave(datetime(2018, 1, 1, 7, 0), datetime(2018, 1, 1, 18, 0)) - with self.assertQueryCount(__system__=96, admin=100): + with self.assertQueryCount(__system__=96, admin=97): leave.action_validate() leave.action_refuse() @@ -40,7 +40,7 @@ class TestWorkEntryHolidaysPerformance(TestWorkEntryHolidaysBase): def test_performance_leave_write(self): leave = self.create_leave(datetime(2018, 1, 1, 7, 0), datetime(2018, 1, 1, 18, 0)) - with self.assertQueryCount(__system__=24, admin=33): + with self.assertQueryCount(__system__=21, admin=29): leave.date_to = datetime(2018, 1, 1, 19, 0) leave.action_refuse() diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index 1eb110a4abf..64a56c4403d 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -35,7 +35,7 @@ class MailMail(models.Model): headers = fields.Text('Headers', copy=False) # Auto-detected based on create() - if 'mail_message_id' was passed then this mail is a notification # and during unlink() we will not cascade delete the parent and its attachments - notification = fields.Boolean('Is Notification', help='Mail has been created to notify people of an existing mail.message') + is_notification = fields.Boolean('Notification Email', help='Mail has been created to notify people of an existing mail.message') # recipients: include inactive partners (they may have been archived after # the message was sent, but they should remain visible in the relation) email_to = fields.Text('To', help='Message recipients (emails)') @@ -50,12 +50,24 @@ class MailMail(models.Model): ('exception', 'Delivery Failed'), ('cancel', 'Cancelled'), ], 'Status', readonly=True, copy=False, default='outgoing') - auto_delete = fields.Boolean( - 'Auto Delete', - help="This option permanently removes any track of email after it's been sent, including from the Technical menu in the Settings, in order to preserve storage space of your Odoo database.") + failure_type = fields.Selection(selection=[ + # generic + ("unknown", "Unknown error"), + # mail + ("mail_email_invalid", "Invalid email address"), + ("mail_email_missing", "Missing email"), + ("mail_smtp", "Connection failed (outgoing mail server problem)"), + # mass mode + ("mail_bl", "Blacklisted Address"), + ("mail_optout", "Opted Out"), + ("mail_dup", "Duplicated Email"), + ], string='Failure type') failure_reason = fields.Text( 'Failure Reason', readonly=1, copy=False, help="Failure reason. This is usually the exception thrown by the email server, stored to ease the debugging of mailing issues.") + auto_delete = fields.Boolean( + 'Auto Delete', + help="This option permanently removes any track of email after it's been sent, including from the Technical menu in the Settings, in order to preserve storage space of your Odoo database.") scheduled_date = fields.Char('Scheduled Send Date', help="If set, the queue manager will send the email after the date. If not set, the email will be send as soon as possible.") @@ -63,8 +75,8 @@ class MailMail(models.Model): def create(self, values_list): # notification field: if not set, set if mail comes from an existing mail.message for values in values_list: - if 'notification' not in values and values.get('mail_message_id'): - values['notification'] = True + if 'is_notification' not in values and values.get('mail_message_id'): + values['is_notification'] = True new_mails = super(MailMail, self).create(values_list) @@ -86,7 +98,7 @@ class MailMail(models.Model): def unlink(self): # cascade-delete the parent message for all mails that are not created for a notification - mail_msg_cascade_ids = [mail.mail_message_id.id for mail in self if not mail.notification] + mail_msg_cascade_ids = [mail.mail_message_id.id for mail in self if not mail.is_notification] res = super(MailMail, self).unlink() if mail_msg_cascade_ids: self.env['mail.message'].browse(mail_msg_cascade_ids).unlink() @@ -156,11 +168,11 @@ class MailMail(models.Model): :return: True """ - notif_mails_ids = [mail.id for mail in self if mail.notification] + notif_mails_ids = [mail.id for mail in self if mail.is_notification] if notif_mails_ids: notifications = self.env['mail.notification'].search([ ('notification_type', '=', 'email'), - ('mail_id', 'in', notif_mails_ids), + ('mail_mail_id', 'in', notif_mails_ids), ('notification_status', 'not in', ('sent', 'canceled')) ]) if notifications: @@ -182,7 +194,7 @@ class MailMail(models.Model): messages = notifications.mapped('mail_message_id').filtered(lambda m: m.is_thread_message()) # TDE TODO: could be great to notify message-based, not notifications-based, to lessen number of notifs messages._notify_message_notification_update() # notify user that we have a failure - if not failure_type or failure_type == 'RECIPIENT': # if we have another error, we want to keep the mail. + if not failure_type or failure_type in ['mail_email_invalid', 'mail_email_missing']: # if we have another error, we want to keep the mail. mail_to_delete_ids = [mail.id for mail in self if mail.auto_delete] self.browse(mail_to_delete_ids).sudo().unlink() return True @@ -286,7 +298,7 @@ class MailMail(models.Model): else: batch = self.browse(batch_ids) batch.write({'state': 'exception', 'failure_reason': exc}) - batch._postprocess_sent_message(success_pids=[], failure_type="SMTP") + batch._postprocess_sent_message(success_pids=[], failure_type="mail_smtp") else: self.browse(batch_ids)._send( auto_commit=auto_commit, @@ -360,14 +372,14 @@ class MailMail(models.Model): # mail record. notifs = self.env['mail.notification'].search([ ('notification_type', '=', 'email'), - ('mail_id', 'in', mail.ids), + ('mail_mail_id', 'in', mail.ids), ('notification_status', 'not in', ('sent', 'canceled')) ]) if notifs: notif_msg = _('Error without exception. Probably due do concurrent access update of notification records. Please see with an administrator.') notifs.sudo().write({ 'notification_status': 'exception', - 'failure_type': 'UNKNOWN', + 'failure_type': 'unknown', 'failure_reason': notif_msg, }) # `test_mail_bounce_during_send`, force immediate update to obtain the lock. @@ -376,6 +388,8 @@ class MailMail(models.Model): # build an RFC2822 email.message.Message object and send it without queuing res = None + # TDE note: could be great to pre-detect missing to/cc and skip sending it + # to go directly to failed state update for email in email_list: msg = IrMailServer.build_email( email_from=mail.email_from, @@ -401,7 +415,11 @@ class MailMail(models.Model): processing_pid = None except AssertionError as error: if str(error) == IrMailServer.NO_VALID_RECIPIENT: - failure_type = "RECIPIENT" + # if we have a list of void emails for email_list -> email missing, otherwise generic email failure + if not email.get('email_to') and failure_type != "RECIPIENT": + failure_type = "mail_email_missing" + else: + failure_type = "mail_email_invalid" # No valid recipient found for this particular # mail item -> ignore error to avoid blocking # delivery to next recipients, if any. If this is @@ -435,7 +453,7 @@ class MailMail(models.Model): failure_reason = tools.ustr(e) _logger.exception('failed sending mail (id: %s) due to %s', mail.id, failure_reason) mail.write({'state': 'exception', 'failure_reason': failure_reason}) - mail._postprocess_sent_message(success_pids=success_pids, failure_reason=failure_reason, failure_type='UNKNOWN') + mail._postprocess_sent_message(success_pids=success_pids, failure_reason=failure_reason, failure_type='unknown') if raise_exception: if isinstance(e, (AssertionError, UnicodeEncodeError)): if isinstance(e, UnicodeEncodeError): diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 88da42103d9..0f6cc22c419 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -27,37 +27,39 @@ class Message(models.Model): * notification_status 'ready', 'sent', 'bounce', 'exception', 'canceled' * notification_type - 'inbox', 'email', 'sms' (SMS addon), 'snail' (snailmail addon) + 'inbox', 'email', 'sms' (SMS addon), 'snail' (snailmail addon) * failure_type - # mail - "SMTP", "RECIPIENT", "BOUNCE", "UNKNOWN" - # sms (SMS addon) - 'sms_number_missing', 'sms_number_format', 'sms_credit', - 'sms_server', 'sms_acc' - # snailmail (snailmail addon) - 'sn_credit', 'sn_trial', 'sn_price', 'sn_fields', - 'sn_format', 'sn_error' + # generic + unknown, + # mail + "mail_email_invalid", "mail_smtp", "mail_email_missing" + # sms (SMS addon) + 'sms_number_missing', 'sms_number_format', 'sms_credit', + 'sms_server', 'sms_acc' + # snailmail (snailmail addon) + 'sn_credit', 'sn_trial', 'sn_price', 'sn_fields', + 'sn_format', 'sn_error' * mail.mail * state - 'outgoing', 'sent', 'received', 'exception', 'cancel' + 'outgoing', 'sent', 'received', 'exception', 'cancel' * failure_reason: text * sms.sms (SMS addon) * state - 'outgoing', 'sent', 'error', 'canceled' + 'outgoing', 'sent', 'error', 'canceled' * error_code - 'sms_number_missing', 'sms_number_format', 'sms_credit', - 'sms_server', 'sms_acc', - # mass mode specific codes - 'sms_blacklist', 'sms_duplicate' + 'sms_number_missing', 'sms_number_format', 'sms_credit', + 'sms_server', 'sms_acc', + # mass mode specific codes + 'sms_blacklist', 'sms_duplicate' * snailmail.letter (snailmail addon) * state - 'pending', 'sent', 'error', 'canceled' + 'pending', 'sent', 'error', 'canceled' * error_code - 'CREDIT_ERROR', 'TRIAL_ERROR', 'NO_PRICE_AVAILABLE', 'FORMAT_ERROR', - 'UNKNOWN_ERROR', + 'CREDIT_ERROR', 'TRIAL_ERROR', 'NO_PRICE_AVAILABLE', 'FORMAT_ERROR', + 'UNKNOWN_ERROR', See ``mailing.trace`` model in mass_mailing application for mailing trace information. diff --git a/addons/mail/models/mail_notification.py b/addons/mail/models/mail_notification.py index daa88004c6a..6626432b3cf 100644 --- a/addons/mail/models/mail_notification.py +++ b/addons/mail/models/mail_notification.py @@ -17,7 +17,7 @@ class MailNotification(models.Model): # origin mail_message_id = fields.Many2one('mail.message', 'Message', index=True, ondelete='cascade', required=True) - mail_id = fields.Many2one('mail.mail', 'Mail', index=True, help='Optional mail_mail ID. Used mainly to optimize searches.') + mail_mail_id = fields.Many2one('mail.mail', 'Mail', index=True, help='Optional mail_mail ID. Used mainly to optimize searches.') # recipient res_partner_id = fields.Many2one('res.partner', 'Recipient', index=True, ondelete='cascade') # status @@ -34,10 +34,12 @@ class MailNotification(models.Model): is_read = fields.Boolean('Is Read', index=True) read_date = fields.Datetime('Read Date', copy=False) failure_type = fields.Selection(selection=[ - ("SMTP", "Connection failed (outgoing mail server problem)"), - ("RECIPIENT", "Invalid email address"), - ("BOUNCE", "Email address rejected by destination"), - ("UNKNOWN", "Unknown error"), + # generic + ("unknown", "Unknown error"), + # mail + ("mail_email_invalid", "Invalid email address"), + ("mail_email_missing", "Missing email addresss"), + ("mail_smtp", "Connection failed (outgoing mail server problem)"), ], string='Failure type') failure_reason = fields.Text('Failure reason', copy=False) @@ -73,7 +75,7 @@ class MailNotification(models.Model): def format_failure_reason(self): self.ensure_one() - if self.failure_type != 'UNKNOWN': + if self.failure_type != 'unknown': return dict(type(self).failure_type.selection).get(self.failure_type, _('No Error')) else: return _("Unknown error") + ": %s" % (self.failure_reason or '') diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 003d63535ae..7028beaaf4e 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1942,9 +1942,9 @@ class MailThread(models.AbstractModel): # Simulate the onchange (like trigger in form the view) only # when having a template in single-email mode if template_id: - update_values = composer.onchange_template_id(template_id, kwargs['composition_mode'], self._name, res_id)['value'] + update_values = composer._onchange_template_id(template_id, kwargs['composition_mode'], self._name, res_id)['value'] composer.write(update_values) - return composer.send_mail(auto_commit=auto_commit) + return composer._action_send_mail(auto_commit=auto_commit) def message_notify(self, *, partner_ids=False, parent_id=False, model=False, res_id=False, @@ -2254,13 +2254,13 @@ class MailThread(models.AbstractModel): tocreate_recipient_ids = [rid for rid in recipient_ids if rid not in existing_notifications.mapped('res_partner_id.id')] existing_notifications.write({ 'notification_status': 'ready', - 'mail_id': email.id, + 'mail_mail_id': email.id, }) notif_create_values += [{ 'mail_message_id': message.id, 'res_partner_id': recipient_id, 'notification_type': 'email', - 'mail_id': email.id, + 'mail_mail_id': email.id, 'is_read': True, # discard Inbox notification 'notification_status': 'ready', } for recipient_id in tocreate_recipient_ids] diff --git a/addons/mail/tests/__init__.py b/addons/mail/tests/__init__.py index 4a99ea63e4a..7be5bb6f46f 100644 --- a/addons/mail/tests/__init__.py +++ b/addons/mail/tests/__init__.py @@ -5,5 +5,6 @@ from . import test_mail_channel from . import test_mail_channel_partner from . import test_mail_full_composer from . import test_mail_render +from . import test_mail_tools from . import test_res_partner from . import test_update_notification diff --git a/addons/mail/tests/test_mail_tools.py b/addons/mail/tests/test_mail_tools.py new file mode 100644 index 00000000000..f66a586a93e --- /dev/null +++ b/addons/mail/tests/test_mail_tools.py @@ -0,0 +1,193 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.addons.mail.tests.common import MailCommon +from odoo.tests import tagged, users +from odoo import tools + + +@tagged('mail_tools') +class TestMailTools(MailCommon): + + @classmethod + def setUpClass(cls): + super(TestMailTools, cls).setUpClass() + + cls._test_email = 'alfredoastaire@test.example.com' + cls.test_partner = cls.env['res.partner'].create({ + 'country_id': cls.env.ref('base.be').id, + 'email': cls._test_email, + 'mobile': '0456001122', + 'name': 'Alfred Astaire', + 'phone': '0456334455', + }) + + cls.sources = [ + # single email + 'alfred.astaire@test.example.com', + ' alfred.astaire@test.example.com ', + 'Fredo The Great ', + '"Fredo The Great" ', + 'Fredo "The Great" ', + # multiple emails + 'alfred.astaire@test.example.com, evelyne.gargouillis@test.example.com', + 'Fredo The Great , Evelyne The Goat ', + '"Fredo The Great" , evelyne.gargouillis@test.example.com', + '"Fredo The Great" , ', + # text containing email + 'Hello alfred.astaire@test.example.com how are you ?', + '

Hello alfred.astaire@test.example.com

', + # text containing emails + 'Hello "Fredo" , evelyne.gargouillis@test.example.com', + 'Hello "Fredo" and evelyne.gargouillis@test.example.com', + # falsy + '

Hello Fredo

', + 'j\'adore écrire des @gmail.com ou "@gmail.com" a bit randomly', + '', + ] + + @users('employee') + def test_find_partner_from_emails(self): + Partner = self.env['res.partner'] + test_partner = Partner.browse(self.test_partner.ids) + self.assertEqual(test_partner.email, self._test_email) + + # test direct match + found = Partner._mail_find_partner_from_emails([self._test_email]) + self.assertEqual(found, [test_partner]) + + # test encapsulated email + found = Partner._mail_find_partner_from_emails(['"Norbert Poiluchette" <%s>' % self._test_email]) + self.assertEqual(found, [test_partner]) + + # test with wildcard "_" + found = Partner._mail_find_partner_from_emails(['alfred_astaire@test.example.com']) + self.assertEqual(found, [self.env['res.partner']]) + + # sub-check: this search does not consider _ as a wildcard + found = Partner._mail_search_on_partner(['alfred_astaire@test.example.com']) + self.assertEqual(found, self.env['res.partner']) + + # test partners with encapsulated emails + # ------------------------------------------------------------ + test_partner.sudo().write({'email': '"Alfred Mighty Power Astaire" <%s>' % self._test_email}) + + # test direct match + found = Partner._mail_find_partner_from_emails([self._test_email]) + self.assertEqual(found, [test_partner]) + + # test encapsulated email + found = Partner._mail_find_partner_from_emails(['"Norbert Poiluchette" <%s>' % self._test_email]) + self.assertEqual(found, [test_partner]) + + # test with wildcard "_" + found = Partner._mail_find_partner_from_emails(['alfred_astaire@test.example.com']) + self.assertEqual(found, [self.env['res.partner']]) + + # sub-check: this search does not consider _ as a wildcard + found = Partner._mail_search_on_partner(['alfred_astaire@test.example.com']) + self.assertEqual(found, self.env['res.partner']) + + # test partners with look-alike emails + # ------------------------------------------------------------ + for email_lookalike in [ + 'alfred.astaire@test.example.com', + 'alfredoastaire@example.com', + 'aalfredoastaire@test.example.com', + 'alfredoastaire@test.example.comm']: + test_partner.sudo().write({'email': '"Alfred Astaire" <%s>' % email_lookalike}) + + # test direct match + found = Partner._mail_find_partner_from_emails([self._test_email]) + self.assertEqual(found, [self.env['res.partner']]) + # test encapsulated email + found = Partner._mail_find_partner_from_emails(['"Norbert Poiluchette" <%s>' % self._test_email]) + self.assertEqual(found, [self.env['res.partner']]) + # test with wildcard "_" + found = Partner._mail_find_partner_from_emails(['alfred_astaire@test.example.com']) + self.assertEqual(found, [self.env['res.partner']]) + + @users('employee') + def test_tools_email_re(self): + expected = [ + # single email + ['alfred.astaire@test.example.com'], + ['alfred.astaire@test.example.com'], + ['alfred.astaire@test.example.com'], + ['alfred.astaire@test.example.com'], + ['alfred.astaire@test.example.com'], + # multiple emails + ['alfred.astaire@test.example.com', 'evelyne.gargouillis@test.example.com'], + ['alfred.astaire@test.example.com', 'evelyne.gargouillis@test.example.com'], + ['alfred.astaire@test.example.com', 'evelyne.gargouillis@test.example.com'], + ['alfred.astaire@test.example.com', 'evelyne.gargouillis@test.example.com'], + # text containing email + ['alfred.astaire@test.example.com'], + ['alfred.astaire@test.example.com'], + # text containing emails + ['alfred.astaire@test.example.com', 'evelyne.gargouillis@test.example.com'], + ['alfred.astaire@test.example.com', 'evelyne.gargouillis@test.example.com'], + # falsy + [], [], [], + ] + + for src, exp in zip(self.sources, expected): + res = tools.email_re.findall(src) + self.assertEqual( + res, exp, + 'Seems email_re is broken with %s (expected %r, received %r)' % (src, exp, res) + ) + + @users('employee') + def test_tools_email_split_tuples(self): + expected = [ + # single email + [('', 'alfred.astaire@test.example.com')], + [('', 'alfred.astaire@test.example.com')], + [('Fredo The Great', 'alfred.astaire@test.example.com')], + [('Fredo The Great', 'alfred.astaire@test.example.com')], + [('Fredo The Great', 'alfred.astaire@test.example.com')], + # multiple emails + [('', 'alfred.astaire@test.example.com'), ('', 'evelyne.gargouillis@test.example.com')], + [('Fredo The Great', 'alfred.astaire@test.example.com'), ('Evelyne The Goat', 'evelyne.gargouillis@test.example.com')], + [('Fredo The Great', 'alfred.astaire@test.example.com'), ('', 'evelyne.gargouillis@test.example.com')], + [('Fredo The Great', 'alfred.astaire@test.example.com'), ('', 'evelyne.gargouillis@test.example.com')], + # text containing email -> probably not designed for that + [('', 'Hello alfred.astaire@test.example.comhowareyou?')], + [('', 'Hello alfred.astaire@test.example.com')], + # text containing emails -> probably not designed for that + [('Hello Fredo', 'alfred.astaire@test.example.com'), ('', 'evelyne.gargouillis@test.example.com')], + [('Hello Fredo', 'alfred.astaire@test.example.com'), ('', 'and evelyne.gargouillis@test.example.com')], + # falsy -> probably not designed for that + [], [('', "j'adore écrire des@gmail.comou"), ('', '@gmail.com')], [], + ] + + for src, exp in zip(self.sources, expected): + res = tools.email_split_tuples(src) + self.assertEqual( + res, exp, + 'Seems email_split_tuples is broken with %s (expected %r, received %r)' % (src, exp, res) + ) + + @users('employee') + def test_tools_single_email_re(self): + expected = [ + # single email + ['alfred.astaire@test.example.com'], + [], [], [], [], # formatting issue for single email re + # multiple emails -> couic + [], [], [], [], + # text containing email -> couic + [], [], + # text containing emails -> couic + [], [], + # falsy + [], [], [], + ] + + for src, exp in zip(self.sources, expected): + res = tools.single_email_re.findall(src) + self.assertEqual( + res, exp, + 'Seems single_email_re is broken with %s (expected %r, received %r)' % (src, exp, res) + ) diff --git a/addons/mail/views/mail_mail_views.xml b/addons/mail/views/mail_mail_views.xml index 420d5522d06..6a6a1121b72 100644 --- a/addons/mail/views/mail_mail_views.xml +++ b/addons/mail/views/mail_mail_views.xml @@ -37,7 +37,7 @@ - + diff --git a/addons/mail/views/mail_notification_views.xml b/addons/mail/views/mail_notification_views.xml index b96b7f70b71..1b2920a01c3 100644 --- a/addons/mail/views/mail_notification_views.xml +++ b/addons/mail/views/mail_notification_views.xml @@ -27,7 +27,7 @@ - + diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 388f08e5b87..8f51f4e83ba 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -7,10 +7,7 @@ import re from odoo import _, api, fields, models, tools, Command from odoo.exceptions import UserError - - -# main mako-like expression pattern -EXPRESSION_PATTERN = re.compile('(\$\{.+?\})') +from odoo.tools import email_re def _reopen(self, res_id, model, context=None): @@ -161,6 +158,15 @@ class MailComposer(models.TransientModel): for composer in self: composer.render_model = composer.model + # Onchanges + + @api.onchange('template_id') + def _onchange_template_id_wrapper(self): + self.ensure_one() + values = self._onchange_template_id(self.template_id.id, self.composition_mode, self.model, self.res_id)['value'] + for fname, value in values.items(): + setattr(self, fname, value) + @api.model def get_record_data(self, values): """ Returns a defaults-like dict with initial values for the composition @@ -190,16 +196,37 @@ class MailComposer(models.TransientModel): return result + # ------------------------------------------------------------ + # CRUD / ORM + # ------------------------------------------------------------ + + @api.autovacuum + def _gc_lost_attachments(self): + """ Garbage collect lost mail attachments. Those are attachments + - linked to res_model 'mail.compose.message', the composer wizard + - with res_id 0, because they were created outside of an existing + wizard (typically user input through Chatter or reports + created on-the-fly by the templates) + - unused since at least one day (create_date and write_date) + """ + limit_date = fields.Datetime.subtract(fields.Datetime.now(), days=1) + self.env['ir.attachment'].search([ + ('res_model', '=', self._name), + ('res_id', '=', 0), + ('create_date', '<', limit_date), + ('write_date', '<', limit_date)] + ).unlink() + # ------------------------------------------------------------ # ACTIONS # ------------------------------------------------------------ - # action buttons call with positionnal arguments only, so we need an intermediary function - # to ensure the context is passed correctly + def action_send_mail(self): - self.send_mail() + """ Used for action button that do not accept arguments. """ + self._action_send_mail(auto_commit=False) return {'type': 'ir.actions.act_window_close'} - def send_mail(self, auto_commit=False): + def _action_send_mail(self, auto_commit=False): """ Process the wizard content and proceed with sending the related email(s), rendering any template patterns on the fly if needed. """ notif_layout = self._context.get('custom_layout') @@ -283,6 +310,30 @@ class MailComposer(models.TransientModel): if wizard.composition_mode == 'mass_mail': batch_mails_sudo.send(auto_commit=auto_commit) + def action_save_as_template(self): + """ hit save as template button: current form value will be a new + template attached to the current document. """ + for record in self: + model = self.env['ir.model']._get(record.model or 'mail.message') + model_name = model.name or '' + template_name = "%s: %s" % (model_name, tools.ustr(record.subject)) + values = { + 'name': template_name, + 'subject': record.subject or False, + 'body_html': record.body or False, + 'model_id': model.id or False, + 'attachment_ids': [Command.set(record.attachment_ids.ids)], + } + template = self.env['mail.template'].create(values) + # generate the saved template + record.write({'template_id': template.id}) + record._onchange_template_id_wrapper() + return _reopen(self, record.id, record.model, context=self._context) + + # ------------------------------------------------------------ + # RENDERING / VALUES GENERATION + # ------------------------------------------------------------ + def get_mail_values(self, res_ids): """Generate the values that will be used by send_mail to create mail_messages or mail_mails. """ @@ -300,17 +351,6 @@ class MailComposer(models.TransientModel): records = self.env[self.model].browse(res_ids) reply_to_value = records._notify_get_reply_to(default=self.email_from) - blacklisted_rec_ids = set() - if mass_mail_mode and issubclass(type(self.env[self.model]), self.pool['mail.thread.blacklist']): - self.env['mail.blacklist'].flush(['email']) - self._cr.execute("SELECT email FROM mail_blacklist WHERE active=true") - blacklist = {x[0] for x in self._cr.fetchall()} - if blacklist: - targets = self.env[self.model].browse(res_ids).read(['email_normalized']) - # First extract email from recipient before comparing with blacklist - blacklisted_rec_ids.update(target['id'] for target in targets - if target['email_normalized'] in blacklist) - for res_id in res_ids: # static wizard (mail.message) values mail_values = { @@ -332,7 +372,7 @@ class MailComposer(models.TransientModel): record = self.env[self.model].browse(res_id) mail_values['headers'] = record._notify_email_headers() # keep a copy unless specifically requested, reset record name (avoid browsing records) - mail_values.update(notification=not self.auto_delete_message, model=self.model, res_id=res_id, record_name=False) + mail_values.update(is_notification=not self.auto_delete_message, model=self.model, res_id=res_id, record_name=False) # auto deletion of mail_mail if self.auto_delete or self.template_id.auto_delete: mail_values['auto_delete'] = True @@ -362,27 +402,114 @@ class MailComposer(models.TransientModel): attachment_ids, {'model': 'mail.message', 'res_id': 0} )['attachment_ids'] - # Filter out the blacklisted records by setting the mail state to cancel -> Used for Mass Mailing stats - if res_id in blacklisted_rec_ids: - mail_values['state'] = 'cancel' - # Do not post the mail into the recipient's chatter - mail_values['notification'] = False results[res_id] = mail_values + + results = self._process_state(results) return results - # ------------------------------------------------------------ - # TEMPLATES - # ------------------------------------------------------------ + def _process_recipient_values(self, mail_values_dict): + # Preprocess res.partners to batch-fetch from db if recipient_ids is present + # it means they are partners (the only object to fill get_default_recipient this way) + recipient_pids = [ + recipient_command[1] + for mail_values in mail_values_dict.values() + # recipient_ids is a list of x2m command tuples at this point + for recipient_command in mail_values.get('recipient_ids') or [] + if recipient_command[1] + ] + recipient_emails = { + p.id: p.email + for p in self.env['res.partner'].browse(set(recipient_pids)) + } if recipient_pids else {} - @api.onchange('template_id') - def onchange_template_id_wrapper(self): - self.ensure_one() - values = self.onchange_template_id(self.template_id.id, self.composition_mode, self.model, self.res_id)['value'] - for fname, value in values.items(): - setattr(self, fname, value) + recipients_info = {} + for record_id, mail_values in mail_values_dict.items(): + mail_to = [] + if mail_values.get('email_to'): + mail_to += email_re.findall(mail_values['email_to']) + # if unrecognized email in email_to -> keep it as used for further processing + if not mail_to: + mail_to.append(mail_values['email_to']) + # add email from recipients (res.partner) + mail_to += [ + recipient_emails[recipient_command[1]] + for recipient_command in mail_values.get('recipient_ids') or [] + if recipient_command[1] + ] + mail_to = list(set(mail_to)) + recipients_info[record_id] = { + 'mail_to': mail_to, + 'mail_to_normalized': [ + tools.email_normalize(mail) + for mail in mail_to + if tools.email_normalize(mail) + ] + } + return recipients_info - def onchange_template_id(self, template_id, composition_mode, model, res_id): + def _process_state(self, mail_values_dict): + recipients_info = self._process_recipient_values(mail_values_dict) + blacklist_ids = self._get_blacklist_record_ids(mail_values_dict) + optout_emails = self._get_optout_emails(mail_values_dict) + done_emails = self._get_done_emails(mail_values_dict) + + for record_id, mail_values in mail_values_dict.items(): + recipients = recipients_info[record_id] + # when having more than 1 recipient: we cannot really decide when a single + # email is linked to several to -> skip that part. Mass mailing should + # anyway always have a single recipient per record as this is default behavior. + if len(recipients['mail_to']) > 1: + continue + + mail_to = recipients['mail_to'][0] if recipients['mail_to'] else '' + mail_to_normalized = recipients['mail_to_normalized'][0] if recipients['mail_to_normalized'] else '' + + # prevent sending to blocked addresses that were included by mistake + # blacklisted or optout or duplicate -> cancel + if record_id in blacklist_ids: + mail_values['state'] = 'cancel' + mail_values['failure_type'] = 'mail_bl' + # Do not post the mail into the recipient's chatter + mail_values['is_notification'] = False + elif optout_emails and mail_to in optout_emails: + mail_values['state'] = 'cancel' + mail_values['failure_type'] = 'mail_optout' + elif done_emails and mail_to in done_emails: + mail_values['state'] = 'cancel' + mail_values['failure_type'] = 'mail_dup' + # void of falsy values -> error + elif not mail_to: + mail_values['state'] = 'cancel' + mail_values['failure_type'] = 'mail_email_missing' + elif not mail_to_normalized or not email_re.findall(mail_to): + mail_values['state'] = 'cancel' + mail_values['failure_type'] = 'mail_email_invalid' + elif done_emails is not None: + done_emails.append(mail_to) + + return mail_values_dict + + def _get_blacklist_record_ids(self, mail_values_dict): + blacklisted_rec_ids = set() + if self.composition_mode == 'mass_mail' and issubclass(type(self.env[self.model]), self.pool['mail.thread.blacklist']): + self.env['mail.blacklist'].flush(['email']) + self._cr.execute("SELECT email FROM mail_blacklist WHERE active=true") + blacklist = {x[0] for x in self._cr.fetchall()} + if blacklist: + targets = self.env[self.model].browse(mail_values_dict.keys()).read(['email_normalized']) + # First extract email from recipient before comparing with blacklist + blacklisted_rec_ids.update(target['id'] for target in targets + if target['email_normalized'] in blacklist) + return blacklisted_rec_ids + + def _get_done_emails(self, mail_values_dict): + return [] + + def _get_optout_emails(self, mail_values_dict): + return [] + + def _onchange_template_id(self, template_id, composition_mode, model, res_id): """ - mass_mailing: we cannot render, so return the template values - normal mode: return rendered values /!\ for x2many field, this onchange return command instead of ids @@ -427,30 +554,6 @@ class MailComposer(models.TransientModel): return {'value': values} - def save_as_template(self): - """ hit save as template button: current form value will be a new - template attached to the current document. """ - for record in self: - model = self.env['ir.model']._get(record.model or 'mail.message') - model_name = model.name or '' - template_name = "%s: %s" % (model_name, tools.ustr(record.subject)) - values = { - 'name': template_name, - 'subject': record.subject or False, - 'body_html': record.body or False, - 'model_id': model.id or False, - 'attachment_ids': [Command.set([att.id for att in record.attachment_ids])], - } - template = self.env['mail.template'].create(values) - # generate the saved template - record.write({'template_id': template.id}) - record.onchange_template_id_wrapper() - return _reopen(self, record.id, record.model, context=self._context) - - # ------------------------------------------------------------ - # RENDERING - # ------------------------------------------------------------ - def render_message(self, res_ids): """Generate template-based values of wizard, for the document records given by res_ids. This method is meant to be inherited by email_template that @@ -537,20 +640,3 @@ class MailComposer(models.TransientModel): values[res_id] = res_id_values return multi_mode and values or values[res_ids[0]] - - @api.autovacuum - def _gc_lost_attachments(self): - """ Garbage collect lost mail attachments. Those are attachments - - linked to res_model 'mail.compose.message', the composer wizard - - with res_id 0, because they were created outside of an existing - wizard (typically user input through Chatter or reports - created on-the-fly by the templates) - - unused since at least one day (create_date and write_date) - """ - limit_date = fields.Datetime.subtract(fields.Datetime.now(), days=1) - self.env['ir.attachment'].search([ - ('res_model', '=', self._name), - ('res_id', '=', 0), - ('create_date', '<', limit_date), - ('write_date', '<', limit_date)] - ).unlink() diff --git a/addons/mail/wizard/mail_compose_message_view.xml b/addons/mail/wizard/mail_compose_message_view.xml index cb3a11bb3fe..259e0d584fe 100644 --- a/addons/mail/wizard/mail_compose_message_view.xml +++ b/addons/mail/wizard/mail_compose_message_view.xml @@ -79,7 +79,8 @@ @@ -339,7 +339,7 @@ {'invisible': ['|',('state', '=', 'draft'),'&','&',('state', '!=', 'in_queue'),('scheduled', '=', 0),('failed', '=', 0)]} - + email(s) not sent. @@ -374,10 +374,10 @@ type="object" class="oe_stat_button" icon="fa-paper-plane"> - diff --git a/addons/mass_mailing/views/mailing_trace_views.xml b/addons/mass_mailing/views/mailing_trace_views.xml index b2205d374bb..3b9da452140 100644 --- a/addons/mass_mailing/views/mailing_trace_views.xml +++ b/addons/mass_mailing/views/mailing_trace_views.xml @@ -2,7 +2,7 @@ - mailing.trace.search + mailing.trace.view.search mailing.trace @@ -10,56 +10,72 @@ - - - - - - - - - - + + + + + + + + + - - - - - + + + + + - - mailing.trace.tree + + mailing.trace.view.tree mailing.trace - + - - - - - - - - - + + + + + + + - + {'invisible': [('mailing_type', '!=', 'mail')]} - - SMS Text Message have been ignored and will not be sent. + + SMS Text Message have been canceled and will not be sent. {'invisible': [('mailing_type', '!=', 'mail')]} diff --git a/addons/mass_mailing_sms/views/mailing_trace_views.xml b/addons/mass_mailing_sms/views/mailing_trace_views.xml index 32e1fc65b6f..4661f0736f8 100644 --- a/addons/mass_mailing_sms/views/mailing_trace_views.xml +++ b/addons/mass_mailing_sms/views/mailing_trace_views.xml @@ -33,15 +33,16 @@ 20 + - - - - - - - - + + + + + + + + + -