From 82de620424e22f9ca7cb9768db638c3a2b4f9318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 28 Jul 2021 07:46:37 +0000 Subject: [PATCH 01/16] [REM] tools: remove deprecated ``email_send`` tool This tool function is deprecated. As explained in docstring: "since OpenERP 6.1 please use ir.mail_server.send_email()". Seems we are allowed to finally remove it. Task ID-2377974 Community PR odoo/odoo#61467 --- odoo/tools/mail.py | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index 34f5a792914..11b29e0ecec 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -470,46 +470,6 @@ def generate_tracking_message_id(res_id): rndstr = ("%.15f" % rnd)[2:] return "<%s.%.15f-openerp-%s@%s>" % (rndstr, time.time(), res_id, socket.gethostname()) -def email_send(email_from, email_to, subject, body, email_cc=None, email_bcc=None, reply_to=False, - attachments=None, message_id=None, references=None, openobject_id=False, debug=False, subtype='plain', headers=None, - smtp_server=None, smtp_port=None, ssl=False, smtp_user=None, smtp_password=None, cr=None, uid=None): - """Low-level function for sending an email (deprecated). - - :deprecate: since OpenERP 6.1, please use ir.mail_server.send_email() instead. - :param email_from: A string used to fill the `From` header, if falsy, - config['email_from'] is used instead. Also used for - the `Reply-To` header if `reply_to` is not provided - :param email_to: a sequence of addresses to send the mail to. - """ - - # If not cr, get cr from current thread database - local_cr = None - if not cr: - db_name = getattr(threading.currentThread(), 'dbname', None) - if db_name: - local_cr = cr = odoo.registry(db_name).cursor() - else: - raise Exception("No database cursor found, please pass one explicitly") - - # Send Email - try: - mail_server_pool = odoo.registry(cr.dbname)['ir.mail_server'] - res = False - # Pack Message into MIME Object - email_msg = mail_server_pool.build_email(email_from, email_to, subject, body, email_cc, email_bcc, reply_to, - attachments, message_id, references, openobject_id, subtype, headers=headers) - - res = mail_server_pool.send_email(cr, uid or 1, email_msg, mail_server_id=None, - smtp_server=smtp_server, smtp_port=smtp_port, smtp_user=smtp_user, smtp_password=smtp_password, - smtp_encryption=('ssl' if ssl else None), smtp_debug=debug) - except Exception: - _logger.exception("tools.email_send failed to deliver email") - return False - finally: - if local_cr: - cr.close() - return res - def email_split_tuples(text): """ Return a list of (name, email) address tuples found in ``text``""" if not text: From 5620e2f2e35410b1f4863b8ef3fb055daea49c7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 16 Aug 2021 12:08:36 +0000 Subject: [PATCH 02/16] [IMP] various: update query counters Update query counters according to last runbot state. This helps spotting query counters change linked to this PR. Task ID-2377974 Community PR odoo/odoo#61467 --- addons/crm/tests/test_crm_lead_convert_mass.py | 6 +++--- addons/google_calendar/tests/test_sync_odoo2google.py | 2 +- addons/hr_work_entry_holidays/tests/test_performance.py | 4 ++-- addons/test_mass_mailing/tests/test_performance.py | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) 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/test_mass_mailing/tests/test_performance.py b/addons/test_mass_mailing/tests/test_performance.py index 3b5a2c6bc6a..3a757d32339 100644 --- a/addons/test_mass_mailing/tests/test_performance.py +++ b/addons/test_mass_mailing/tests/test_performance.py @@ -50,8 +50,8 @@ class TestMassMailPerformance(TestMassMailPerformanceBase): 'mailing_domain': [('id', 'in', self.mm_recs.ids)], }) - # runbot needs +50 compared to local - with self.assertQueryCount(__system__=1718, marketing=1718): + # runbot needs +101 compared to local (1616) + with self.assertQueryCount(__system__=1717, marketing=1717): mailing.action_send_mail() self.assertEqual(mailing.sent, 50) @@ -90,8 +90,8 @@ class TestMassMailBlPerformance(TestMassMailPerformanceBase): 'mailing_domain': [('id', 'in', self.mm_recs.ids)], }) - # runbot needs +62 compared to local - with self.assertQueryCount(__system__=2008, marketing=2008): + # runbot needs +125 compared to local (1882) + with self.assertQueryCount(__system__=2007, marketing=2007): mailing.action_send_mail() self.assertEqual(mailing.sent, 50) From eebadb323bf82c8bc635888da9e8afbdc5d3b0e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 28 Jul 2021 07:52:02 +0000 Subject: [PATCH 03/16] [MOV] (test_)mail: reorganize mail test tools files Since a few time, test_mail holds tests linked to specific test models. Generic testing of mail features are located in mail. Followup of fix done in 12 and forward ported until odoo/odoo@b7af68590ebe1bcd4c4a91b9ea416964aca4763b Also rename test_mass_mailing_server.py unit test file to test_mailing_server to match current naming of files. Task ID-2377974 Community PR odoo/odoo#61467 --- addons/mail/tests/__init__.py | 1 + addons/{test_mail => mail}/tests/test_mail_tools.py | 4 ++-- addons/test_mail/tests/__init__.py | 1 - addons/test_mass_mailing/tests/__init__.py | 4 ++-- .../{test_mass_mailing_server.py => test_mailing_server.py} | 0 5 files changed, 5 insertions(+), 5 deletions(-) rename addons/{test_mail => mail}/tests/test_mail_tools.py (96%) rename addons/test_mass_mailing/tests/{test_mass_mailing_server.py => test_mailing_server.py} (100%) 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/test_mail/tests/test_mail_tools.py b/addons/mail/tests/test_mail_tools.py similarity index 96% rename from addons/test_mail/tests/test_mail_tools.py rename to addons/mail/tests/test_mail_tools.py index b36a1335c5d..931936ed353 100644 --- a/addons/test_mail/tests/test_mail_tools.py +++ b/addons/mail/tests/test_mail_tools.py @@ -1,12 +1,12 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo.addons.test_mail.tests.common import TestMailCommon, TestRecipients +from odoo.addons.mail.tests.common import MailCommon from odoo.tests import tagged, users @tagged('mail_tools') -class TestMailTools(TestMailCommon, TestRecipients): +class TestMailTools(MailCommon): @classmethod def setUpClass(cls): diff --git a/addons/test_mail/tests/__init__.py b/addons/test_mail/tests/__init__.py index 8958112bbf6..b6e302cb99d 100644 --- a/addons/test_mail/tests/__init__.py +++ b/addons/test_mail/tests/__init__.py @@ -11,7 +11,6 @@ from . import test_mail_mail from . import test_mail_gateway from . import test_mail_thread_internals from . import test_mail_template -from . import test_mail_tools from . import test_message_management from . import test_message_post from . import test_message_track diff --git a/addons/test_mass_mailing/tests/__init__.py b/addons/test_mass_mailing/tests/__init__.py index 180386874e0..c50bc65832e 100644 --- a/addons/test_mass_mailing/tests/__init__.py +++ b/addons/test_mass_mailing/tests/__init__.py @@ -5,7 +5,7 @@ from . import test_blacklist from . import test_blacklist_behavior from . import test_blacklist_mixin from . import test_link_tracker -from . import test_mailing_test from . import test_mailing -from . import test_mass_mailing_server +from . import test_mailing_server +from . import test_mailing_test from . import test_performance diff --git a/addons/test_mass_mailing/tests/test_mass_mailing_server.py b/addons/test_mass_mailing/tests/test_mailing_server.py similarity index 100% rename from addons/test_mass_mailing/tests/test_mass_mailing_server.py rename to addons/test_mass_mailing/tests/test_mailing_server.py From 461a1ef460175314ecef2e3bc1fc9ca20b068f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 28 Jul 2021 09:00:43 +0000 Subject: [PATCH 04/16] [IMP] mail: add tests for email finding tools Purpose is to better understand the use of email_re regex (find emails in a text) and email_split_tuples (find name and email in a string used in email headers or as email_from / email_to / email_cc). Task ID-2377974 Community PR odoo/odoo#61467 --- addons/mail/tests/test_mail_tools.py | 110 +++++++++++++++++++++++++++ odoo/tools/mail.py | 4 +- 2 files changed, 113 insertions(+), 1 deletion(-) diff --git a/addons/mail/tests/test_mail_tools.py b/addons/mail/tests/test_mail_tools.py index 931936ed353..f66a586a93e 100644 --- a/addons/mail/tests/test_mail_tools.py +++ b/addons/mail/tests/test_mail_tools.py @@ -3,6 +3,7 @@ from odoo.addons.mail.tests.common import MailCommon from odoo.tests import tagged, users +from odoo import tools @tagged('mail_tools') @@ -21,6 +22,30 @@ class TestMailTools(MailCommon): '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'] @@ -81,3 +106,88 @@ class TestMailTools(MailCommon): # 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/odoo/tools/mail.py b/odoo/tools/mail.py index 11b29e0ecec..b3a03d31e9f 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -471,7 +471,9 @@ def generate_tracking_message_id(res_id): return "<%s.%.15f-openerp-%s@%s>" % (rndstr, time.time(), res_id, socket.gethostname()) def email_split_tuples(text): - """ Return a list of (name, email) address tuples found in ``text``""" + """ Return a list of (name, email) address tuples found in ``text`` . Note + that text should be an email header or a stringified email list as it may + give broader results than expected on actual text. """ if not text: return [] return [(addr[0], addr[1]) for addr in getaddresses([text]) From 6cd10f66f7cba868ef21b361b59067cfcb061891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 2 Jun 2021 13:28:28 +0000 Subject: [PATCH 05/16] [REM] test_mail_full: remove unused file A python file is not imported anymore and contains dead code since its content is now managed elsewhere. Oversight of odoo/odoo@ed508ee1bd56847e235a4f8e487e827cc34e3a87 (odoo/odoo#67322). Task ID-2377974 Community PR odoo/odoo#61467 --- .../test_mail_full/models/mailing_mailing.py | 29 ------------------- 1 file changed, 29 deletions(-) delete mode 100644 addons/test_mail_full/models/mailing_mailing.py diff --git a/addons/test_mail_full/models/mailing_mailing.py b/addons/test_mail_full/models/mailing_mailing.py deleted file mode 100644 index 8d8a49a1ec9..00000000000 --- a/addons/test_mail_full/models/mailing_mailing.py +++ /dev/null @@ -1,29 +0,0 @@ -# -*- coding: utf-8 -*- -# Part of Odoo. See LICENSE file for full copyright and licensing details. - -import logging - -from odoo import models - -_logger = logging.getLogger(__name__) - - -class Mailing(models.Model): - _inherit = 'mailing.mailing' - - def _get_opt_out_list_sms(self): - """Returns a set of emails opted-out in target model""" - self.ensure_one() - if self.mailing_model_real in ('mail.test.sms.bl.optout', - 'mail.test.sms.partner', - 'mail.test.sms.partner.2many'): - res_ids = self._get_recipients() - opt_out_contacts = set(self.env[self.mailing_model_real].search([ - ('id', 'in', res_ids), - ('opt_out', '=', True) - ]).ids) - _logger.info( - "Mass-mailing %s targets %s, optout: %s emails", - self, self.mailing_model_real, len(opt_out_contacts)) - return opt_out_contacts - return super(Mailing, self)._get_opt_out_list_sms() From e9af6096167392c2d3605389367eba9df1b68c50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 28 Jul 2021 12:14:17 +0000 Subject: [PATCH 06/16] [IMP] mail, various: reorganize and lint composer action name In this commit some reorganization is performed within mail compose message code. Purpose is to reorder a bit methods by main usage: onchange, CRUD, actions, values generation with rendering and template management. Some renaming is performed on action methods, notably send_mail that has some impact on sub-addons. Finally we also set onchange and sub-onchange methods private. No functional change should be implied by this commit. LINKS Task ID-2377974 Community PR odoo/odoo#61467 Enterprise PR odoo/enterprise#14633 --- addons/account/wizard/account_invoice_send.py | 8 +- addons/coupon/models/mail_compose_message.py | 4 +- addons/mail/models/mail_thread.py | 4 +- addons/mail/wizard/mail_compose_message.py | 120 +++++++++--------- .../mail/wizard/mail_compose_message_view.xml | 3 +- addons/mass_mailing/models/mailing.py | 2 +- .../purchase/models/mail_compose_message.py | 4 +- addons/sale/models/res_company.py | 4 +- addons/sale/wizard/mail_compose_message.py | 4 +- addons/test_mail/tests/test_mail_composer.py | 42 +++--- addons/test_mail/tests/test_performance.py | 8 +- .../models/mail_compose_message.py | 4 +- 12 files changed, 103 insertions(+), 104 deletions(-) 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/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 003d63535ae..f3336657c64 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, diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 388f08e5b87..f0764e045aa 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -9,10 +9,6 @@ from odoo import _, api, fields, models, tools, Command from odoo.exceptions import UserError -# main mako-like expression pattern -EXPRESSION_PATTERN = re.compile('(\$\{.+?\})') - - def _reopen(self, res_id, model, context=None): # save original model in context, because selecting the list of available # templates requires a model in context @@ -161,6 +157,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 +195,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 +309,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. """ @@ -371,18 +421,7 @@ class MailComposer(models.TransientModel): results[res_id] = mail_values return results - # ------------------------------------------------------------ - # TEMPLATES - # ------------------------------------------------------------ - - @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) - - def onchange_template_id(self, template_id, composition_mode, model, res_id): + 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 +466,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 +552,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..fc8d54adc6f 100644 --- a/addons/mass_mailing/views/mailing_trace_views.xml +++ b/addons/mass_mailing/views/mailing_trace_views.xml @@ -10,22 +10,22 @@ - - - - + + + + - - - - - + + + + + - - - - - + + + + + @@ -38,16 +38,13 @@ - - - - - - - - - + + + + + + @@ -58,7 +55,7 @@
- +
@@ -71,24 +68,19 @@ - - - - - + + + - + - - - @@ -103,8 +95,8 @@ mailing.trace - - + + diff --git a/addons/mass_mailing/wizard/mail_compose_message.py b/addons/mass_mailing/wizard/mail_compose_message.py index 5d769938bd5..fb9fe8874bf 100644 --- a/addons/mass_mailing/wizard/mail_compose_message.py +++ b/addons/mass_mailing/wizard/mail_compose_message.py @@ -17,6 +17,7 @@ class MailComposeMessage(models.TransientModel): """ Override method that generated the mail content by creating the mailing.trace values in the o2m of mail_mail, when doing pure email mass mailing. """ + now = fields.Datetime.now() self.ensure_one() res = super(MailComposeMessage, self).get_mail_values(res_ids) # use only for allowed models in mass mailing @@ -32,7 +33,7 @@ class MailComposeMessage(models.TransientModel): 'state': 'done', 'reply_to_mode': self.reply_to_mode, 'reply_to': self.reply_to if self.reply_to_mode == 'new' else False, - 'sent_date': fields.Datetime.now(), + 'sent_date': now, 'body_html': self.body, 'mailing_model_id': self.env['ir.model']._get(self.model).id, 'mailing_domain': self.active_domain, @@ -98,9 +99,9 @@ class MailComposeMessage(models.TransientModel): } # propagate failed states to trace when still-born if mail_values.get('state') == 'cancel': - trace_vals['ignored'] = fields.Datetime.now() + trace_vals['trace_status'] = 'cancel' elif mail_values.get('state') == 'exception': - trace_vals['exception'] = fields.Datetime.now() + trace_vals['trace_status'] = 'error' if error_code: trace_vals['failure_type'] = error_code diff --git a/addons/mass_mailing_sms/data/mailing_demo.xml b/addons/mass_mailing_sms/data/mailing_demo.xml index 54b14b0d3e7..bcb8a008b5e 100644 --- a/addons/mass_mailing_sms/data/mailing_demo.xml +++ b/addons/mass_mailing_sms/data/mailing_demo.xml @@ -21,7 +21,9 @@ +32465000000 res.partner - + sent + + @@ -29,7 +31,9 @@ +32465000001 res.partner - + sent + + @@ -37,7 +41,9 @@ +32465000002 res.partner - + sent + + @@ -45,7 +51,9 @@ +32465000003 res.partner - + sent + + @@ -53,8 +61,10 @@ +32465000004 res.partner - - + error + sms_server + + @@ -62,8 +72,9 @@ 123123 res.partner - - + bounce + + @@ -71,8 +82,9 @@ res.partner - - + error + sms_number_missing + @@ -128,7 +140,8 @@ +32456001100 mailing.contact - + sent + @@ -136,7 +149,8 @@ +32456001111 mailing.contact - + sent + @@ -144,7 +158,8 @@ +32456001122 mailing.contact - + sent + @@ -152,7 +167,7 @@ +32456001133 mailing.contact - + error sms_credit @@ -161,7 +176,7 @@ dummy mailing.contact - + error sms_number_format diff --git a/addons/mass_mailing_sms/models/mailing_trace.py b/addons/mass_mailing_sms/models/mailing_trace.py index 99b4173643e..58e55e4119f 100644 --- a/addons/mass_mailing_sms/models/mailing_trace.py +++ b/addons/mass_mailing_sms/models/mailing_trace.py @@ -53,36 +53,3 @@ class MailingTrace(models.Model): as it serves as obfuscation when unsubscribing. A valid trio code / mailing_id / number will be requested. """ return ''.join(random.choice(string.ascii_letters + string.digits) for dummy in range(self.CODE_SIZE)) - - def _get_records_from_sms(self, sms_sms_ids=None, additional_domain=None): - if not self.ids and sms_sms_ids: - domain = [('sms_sms_id_int', 'in', sms_sms_ids)] - else: - domain = [('id', 'in', self.ids)] - if additional_domain: - domain = expression.AND([domain, additional_domain]) - return self.search(domain) - - def set_failed(self, failure_type): - for trace in self: - trace.write({'exception': fields.Datetime.now(), 'failure_type': failure_type}) - - def set_sms_sent(self, sms_sms_ids=None): - statistics = self._get_records_from_sms(sms_sms_ids, [('sent', '=', False)]) - statistics.write({'sent': fields.Datetime.now()}) - return statistics - - def set_sms_clicked(self, sms_sms_ids=None): - statistics = self._get_records_from_sms(sms_sms_ids, [('clicked', '=', False)]) - statistics.write({'clicked': fields.Datetime.now()}) - return statistics - - def set_sms_ignored(self, sms_sms_ids=None): - statistics = self._get_records_from_sms(sms_sms_ids, [('ignored', '=', False)]) - statistics.write({'ignored': fields.Datetime.now()}) - return statistics - - def set_sms_exception(self, sms_sms_ids=None): - statistics = self._get_records_from_sms(sms_sms_ids, [('exception', '=', False)]) - statistics.write({'exception': fields.Datetime.now()}) - return statistics diff --git a/addons/mass_mailing_sms/models/sms_sms.py b/addons/mass_mailing_sms/models/sms_sms.py index 840aa066824..504e4417924 100644 --- a/addons/mass_mailing_sms/models/sms_sms.py +++ b/addons/mass_mailing_sms/models/sms_sms.py @@ -37,7 +37,7 @@ class SmsSms(models.Model): ('sms_sms_id_int', 'in', sms_ids) ]) if traces and state == 'success': - traces.write({'sent': fields.Datetime.now(), 'exception': False}) + traces.set_sent() elif traces: traces.set_failed(failure_type=self.IAP_TO_SMS_STATE[state]) return super(SmsSms, self)._postprocess_iap_sent_sms(iap_results, failure_reason=failure_reason, delete_all=delete_all) diff --git a/addons/mass_mailing_sms/tests/common.py b/addons/mass_mailing_sms/tests/common.py index eb48b4b669f..ff3802d0331 100644 --- a/addons/mass_mailing_sms/tests/common.py +++ b/addons/mass_mailing_sms/tests/common.py @@ -31,7 +31,7 @@ class MassSMSCase(SMSCase): # TRACE 'partner': res.partner record (may be empty), 'number': number used for notification (may be empty, computed based on partner), - 'state': outgoing / sent / ignored / bounced / exception / opened (outgoing by default), + 'trace_status': outgoing / sent / cancel / bounce / error / opened (sent by default), 'record: linked record, # SMS.SMS 'content': optional: if set, check content of sent SMS; @@ -55,9 +55,9 @@ class MassSMSCase(SMSCase): state_mapping = { 'sent': 'sent', 'outgoing': 'outgoing', - 'exception': 'error', - 'ignored': 'canceled', - 'bounced': 'error', + 'error': 'error', + 'cancel': 'canceled', + 'bounce': 'error', } traces = self.env['mailing.trace'].search([ ('mass_mailing_id', 'in', mailing.ids), @@ -74,27 +74,27 @@ class MassSMSCase(SMSCase): for recipient_info, link_info, record in zip(recipients_info, sms_links_info, records): partner = recipient_info.get('partner', self.env['res.partner']) number = recipient_info.get('number') - state = recipient_info.get('state', 'outgoing') + status = recipient_info.get('trace_status', 'outgoing') content = recipient_info.get('content', None) if number is None and partner: number = partner._sms_get_recipients_info()[partner.id]['sanitized'] trace = traces.filtered( - lambda t: t.sms_number == number and t.state == state and (t.res_id == record.id if record else True) + lambda t: t.sms_number == number and t.trace_status == status and (t.res_id == record.id if record else True) ) self.assertTrue(len(trace) == 1, - 'SMS: found %s notification for number %s, (state: %s) (1 expected)' % (len(trace), number, state)) + 'SMS: found %s notification for number %s, (status: %s) (1 expected)' % (len(trace), number, status)) self.assertTrue(bool(trace.sms_sms_id_int)) if check_sms: - if state == 'sent': + if status == 'sent': if sent_unlink: self.assertSMSIapSent([number], content=content) else: self.assertSMS(partner, number, 'sent', content=content) - elif state in state_mapping: - sms_state = state_mapping[state] - error_code = recipient_info['failure_type'] if state in ('exception', 'ignored', 'bounced') else None + elif status in state_mapping: + sms_state = state_mapping[status] + error_code = recipient_info['failure_type'] if status in ('error', 'cancel', 'bounce') else None self.assertSMS(partner, number, sms_state, error_code=error_code, content=content) else: raise NotImplementedError() @@ -103,7 +103,7 @@ class MassSMSCase(SMSCase): # shortened links are directly included in sms.sms record as well as # in sent sms (not like mails who are post-processed) sms_sent = self._find_sms_sent(partner, number) - sms_sms = self._find_sms_sms(partner, number, state_mapping[state]) + sms_sms = self._find_sms_sms(partner, number, state_mapping[status]) for (url, is_shortened, add_link_params) in link_info: if url == 'unsubscribe': url = '%s/sms/%d/%s' % (mailing.get_base_url(), mailing.id, trace.sms_code) diff --git a/addons/mass_mailing_sms/views/mailing_mailing_views.xml b/addons/mass_mailing_sms/views/mailing_mailing_views.xml index 8735ec297bf..7ef205dc3ce 100644 --- a/addons/mass_mailing_sms/views/mailing_mailing_views.xml +++ b/addons/mass_mailing_sms/views/mailing_mailing_views.xml @@ -61,11 +61,11 @@
- + {'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..297286f05c3 100644 --- a/addons/mass_mailing_sms/views/mailing_trace_views.xml +++ b/addons/mass_mailing_sms/views/mailing_trace_views.xml @@ -33,14 +33,14 @@ 20 + - - - - - - - + + + + + + @@ -60,16 +60,6 @@ {'invisible': [('trace_type', '!=', 'mail')]} - - - {'invisible': ['|', ('opened', '=', False), ('trace_type', '=', 'sms')]} - - - - - {'invisible': ['|', ('replied', '=', False), ('trace_type', '=', 'sms')]} - - @@ -87,13 +77,13 @@
- +
- @@ -87,18 +94,26 @@ This number appears to be invalid. - + + + + + + + + + + - - - - + + - - - - + + + + + From f16dbbcaecb7d46aefd3451eab2082316e8b0eec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 17 Aug 2021 08:35:18 +0000 Subject: [PATCH 16/16] [IMP] mass_mailing: improve mailing trace report PURPOSE Improve user experience of report model build on mailing traces. SPECIFICATIONS Add scheduled count to reporting. Indeed we have number of sent and delivered traces but it lacks count of scheduled. Some traces are not sent for example when email or sms is missing or obviously wrong. Those are currently not counted in reporting, this is fixed. Views are also improved to add relevant metrics and measures. Notably add a list view on report model, allowing to see some metrics at a glance. Improve SMS related views, hiding opened / replied metrics as we do not track them currently in SMS. LINKS Task ID-2377974 Community PR odoo/odoo#61467 Enterprise PR odoo/enterprise#14633 --- .../report/mailing_trace_report.py | 6 ++- .../report/mailing_trace_report_views.xml | 54 +++++++++++++++---- .../report/mailing_trace_report_views.xml | 29 +++++++++- 3 files changed, 75 insertions(+), 14 deletions(-) diff --git a/addons/mass_mailing/report/mailing_trace_report.py b/addons/mass_mailing/report/mailing_trace_report.py index e3711151077..da9b7cf4999 100644 --- a/addons/mass_mailing/report/mailing_trace_report.py +++ b/addons/mass_mailing/report/mailing_trace_report.py @@ -19,6 +19,7 @@ class MailingTraceReport(models.Model): string='Status', readonly=True) email_from = fields.Char('From', readonly=True) # traces + scheduled = fields.Integer(readonly=True) sent = fields.Integer(readonly=True) delivered = fields.Integer(readonly=True) error = fields.Integer(readonly=True) @@ -57,11 +58,12 @@ class MailingTraceReport(models.Model): 'trace.create_date as scheduled_date', 'mailing.state', 'mailing.email_from', + "COUNT(trace.id) as scheduled", 'COUNT(trace.sent_datetime) as sent', - "(COUNT(trace.sent_datetime) - COUNT(trace.trace_status) FILTER (WHERE trace.trace_status IN ('error', 'bounce', 'cancel'))) as delivered", + "(COUNT(trace.id) - COUNT(trace.trace_status) FILTER (WHERE trace.trace_status IN ('error', 'bounce', 'cancel'))) as delivered", "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'error') as error", "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'bounce') as bounced", - "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'cancel') as cancel", + "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'cancel') as canceled", "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'open') as opened", "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'reply') as replied", "COUNT(trace.links_click_datetime) as clicked", diff --git a/addons/mass_mailing/report/mailing_trace_report_views.xml b/addons/mass_mailing/report/mailing_trace_report_views.xml index 51c70d9baee..a7e793872dc 100644 --- a/addons/mass_mailing/report/mailing_trace_report_views.xml +++ b/addons/mass_mailing/report/mailing_trace_report_views.xml @@ -1,17 +1,42 @@ + + mailing.trace.report.view.tree + mailing.trace.report + + + + + + + + + + + + + + + + + + + mailing.trace.report.view.pivot mailing.trace.report - + + - - + + + + @@ -20,10 +45,11 @@ mailing.trace.report.view.graph mailing.trace.report - + - + + @@ -35,9 +61,9 @@ - + - + - + @@ -59,8 +85,14 @@ Mass Mailing Analysis mailing.trace.report [('mailing_type', '=', 'mail')] - graph,pivot -

Mass Mailing Statistics allows you to check different mailing related information like number of bounced mails, opened mails, replied mails. You can sort out your analysis by different groups to get accurate grained analysis.

 + graph,pivot,tree + +

+ Mass Mailing Statistics allows you to check different mailing related information + like number of bounced mails, opened mails, replied mails. You can sort out + your analysis by different groups to get accurate grained analysis. +

+ + + mailing.sms.trace.report.view.tree + mailing.trace.report + + + primary + tree + + + 1 + + + 1 + + + + mailing.sms.trace.report.view.pivot mailing.trace.report @@ -8,6 +25,9 @@ primary pivot + + 1 + 1 @@ -32,7 +52,7 @@ SMS Marketing Analysis mailing.trace.report - graph,pivot + graph,pivot,tree [('mailing_type', '=', 'sms')]

@@ -56,6 +76,13 @@ + + 2 + tree + + + +