[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

@@ -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')})

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)

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,

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()

addons/mail/wizard/mail_compose_message_view.xml

@@ -79,7 +79,8 @@
                         <button string="Log" attrs="{'invisible': [('is_log', '=', False)]}" name="action_send_mail" type="object" class="btn-primary" data-hotkey="q"/>
                         <button string="Cancel" class="btn-secondary" special="cancel" data-hotkey="z" />
 
-                        <button icon="fa-lg fa-save" type="object" name="save_as_template" string="Save as new template"
+                        <button icon="fa-lg fa-save" type="object"
+                                name="action_save_as_template" string="Save as new template"
                                 class="float-right btn-secondary" help="Save as a new template"/>
                     </footer>
                 </form>

addons/mass_mailing/models/mailing.py

@@ -687,7 +687,7 @@ class MassMailing(models.Model):
             composer = composer.with_context(active_ids=res_ids, **extra_context)
             # auto-commit except in testing mode
             auto_commit = not getattr(threading.currentThread(), 'testing', False)
-            composer.send_mail(auto_commit=auto_commit)
+            composer._action_send_mail(auto_commit=auto_commit)
             mailing.write({
                 'state': 'done',
                 'sent_date': fields.Datetime.now(),

addons/purchase/models/mail_compose_message.py

@@ -7,7 +7,7 @@ from odoo import models
 class MailComposeMessage(models.TransientModel):
     _inherit = 'mail.compose.message'
 
-    def send_mail(self, auto_commit=False):
+    def _action_send_mail(self, auto_commit=False):
         if self.env.context.get('mark_rfq_as_sent') and self.model == 'purchase.order':
             self = self.with_context(mail_notify_author=self.env.user.partner_id in self.partner_ids)
-        return super(MailComposeMessage, self).send_mail(auto_commit=auto_commit)
+        return super(MailComposeMessage, self)._action_send_mail(auto_commit=auto_commit)

addons/sale/models/res_company.py

@@ -91,10 +91,10 @@ class ResCompany(models.Model):
             'composition_mode': 'comment'})
 
         # Simulate the onchange (like trigger in form the view)
-        update_values = message_composer.onchange_template_id(template.id, 'comment', 'sale.order', sample_sales_order.id)['value']
+        update_values = message_composer._onchange_template_id(template.id, 'comment', 'sale.order', sample_sales_order.id)['value']
         message_composer.write(update_values)
 
-        message_composer.send_mail()
+        message_composer._action_send_mail()
 
         self.set_onboarding_step_done('sale_onboarding_sample_quotation_state')
 

addons/sale/wizard/mail_compose_message.py

@@ -7,7 +7,7 @@ from odoo import models
 class MailComposeMessage(models.TransientModel):
     _inherit = 'mail.compose.message'
 
-    def send_mail(self, auto_commit=False):
+    def _action_send_mail(self, auto_commit=False):
         if self.env.context.get('mark_so_as_sent') and self.model == 'sale.order':
             self = self.with_context(mail_notify_author=self.env.user.partner_id in self.partner_ids)
-        return super(MailComposeMessage, self).send_mail(auto_commit=auto_commit)
+        return super(MailComposeMessage, self)._action_send_mail(auto_commit=auto_commit)

addons/test_mail/tests/test_mail_composer.py

@@ -216,7 +216,7 @@ class TestComposerInternals(TestMailComposer):
             'body': '<p>Test Body</p>',
         })
         # currently onchange necessary
-        composer.onchange_template_id_wrapper()
+        composer._onchange_template_id_wrapper()
 
         # values coming from template
         self.assertEqual(len(composer.attachment_ids), 4)
@@ -255,7 +255,7 @@ class TestComposerInternals(TestMailComposer):
             # changing template should update its email_from
             composer.write({'template_id': self.template.id, 'author_id': self.env.user.partner_id})
             # currently onchange necessary
-            composer.onchange_template_id_wrapper()
+            composer._onchange_template_id_wrapper()
             self.assertEqual(composer.author_id, self.env.user.partner_id,
                              'MailComposer: should take value given by user')
             if composition_mode == 'comment':
@@ -294,7 +294,7 @@ class TestComposerInternals(TestMailComposer):
             # changing template should update its content
             composer.write({'template_id': self.template.id})
             # currently onchange necessary
-            composer.onchange_template_id_wrapper()
+            composer._onchange_template_id_wrapper()
 
             # values come from template
             if composition_mode == 'comment':
@@ -313,7 +313,7 @@ class TestComposerInternals(TestMailComposer):
             # reset template should reset values
             composer.write({'template_id': False})
             # currently onchange necessary
-            composer.onchange_template_id_wrapper()
+            composer._onchange_template_id_wrapper()
 
             # values are reset
             if composition_mode == 'comment':
@@ -336,7 +336,7 @@ class TestComposerInternals(TestMailComposer):
                 'template_id': self.template.id,
             })
             # currently onchange necessary
-            composer.onchange_template_id_wrapper()
+            composer._onchange_template_id_wrapper()
 
             # values come from template
             if composition_mode == 'comment':
@@ -354,7 +354,7 @@ class TestComposerInternals(TestMailComposer):
                 'template_id': self.template.id,
             })
             # currently onchange necessary
-            composer.onchange_template_id_wrapper()
+            composer._onchange_template_id_wrapper()
 
             # values come from template
             if composition_mode == 'comment':
@@ -407,7 +407,7 @@ class TestComposerInternals(TestMailComposer):
                 'subject': 'Subject',
                 'body': '<p>Body text</p>',
                 'partner_ids': []
-            }).send_mail()
+            })._action_send_mail()
 
             self.assertEqual(self.test_record.message_ids[0].body, '<p>Body text</p>')
             self.assertEqual(self.test_record.message_ids[0].author_id, portal_user.partner_id)
@@ -418,7 +418,7 @@ class TestComposerInternals(TestMailComposer):
             }).create({
                 'subject': 'Subject',
                 'body': '<p>Body text 2</p>'
-            }).send_mail()
+            })._action_send_mail()
 
             self.assertEqual(self.test_record.message_ids[0].body, '<p>Body text 2</p>')
             self.assertEqual(self.test_record.message_ids[0].author_id, portal_user.partner_id)
@@ -430,7 +430,7 @@ class TestComposerInternals(TestMailComposer):
         ).create({
             'subject': 'Template Subject',
             'body': '<p>Template Body</p>',
-        }).save_as_template()
+        }).action_save_as_template()
 
         # Test: email_template subject, body_html, model
         template = self.env['mail.template'].search([
@@ -457,7 +457,7 @@ class TestComposerResultsComment(TestMailComposer):
             'partner_ids': [(4, self.partner_1.id), (4, self.partner_2.id)]
         })
         with self.mock_mail_gateway(mail_unlink_sent=True):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # notifications
         message = self.test_record.message_ids[0]
@@ -477,7 +477,7 @@ class TestComposerResultsComment(TestMailComposer):
             'partner_ids': [(4, self.partner_1.id), (4, self.partner_2.id)]
         })
         with self.mock_mail_gateway(mail_unlink_sent=True):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # notifications
         message = self.test_record.message_ids[0]
@@ -498,7 +498,7 @@ class TestComposerResultsComment(TestMailComposer):
             'body': '<p>Test Body</p>',
             'partner_ids': [(4, self.partner_1.id), (4, self.partner_2.id)]
         })
-        composer.send_mail()
+        composer._action_send_mail()
 
         message = self.test_record.message_ids[0]
         self.assertEqual(message.body, '<p>Test Body</p>')
@@ -543,7 +543,7 @@ class TestComposerResultsComment(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app():
-            composer.send_mail()
+            composer._action_send_mail()
 
         # check new partners have been created based on emails given
         new_partners = self.env['res.partner'].search([
@@ -626,7 +626,7 @@ class TestComposerResultsMass(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=True):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # global outgoing
         self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record')
@@ -682,7 +682,7 @@ class TestComposerResultsMass(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=False):
-            composer.send_mail()
+            composer._action_send_mail()
 
         new_partners = self.env['res.partner'].search([
             ('email', 'in', [email_to_1, email_to_2, email_to_3, email_cc_1])
@@ -731,7 +731,7 @@ class TestComposerResultsMass(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=True):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # global outgoing
         self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record')
@@ -764,7 +764,7 @@ class TestComposerResultsMass(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=True):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # global outgoing
         self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record')
@@ -793,7 +793,7 @@ class TestComposerResultsMass(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=True):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # global outgoing
         self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record')
@@ -812,7 +812,7 @@ class TestComposerResultsMass(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=True):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # global outgoing
         self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record')
@@ -826,7 +826,7 @@ class TestComposerResultsMass(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=False):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # global outgoing
         self.assertEqual(len(self._new_mails), 1, 'Should have created 1 mail.mail per record')
@@ -839,4 +839,4 @@ class TestComposerResultsMass(TestMailComposer):
         ))
         composer = composer_form.save()
         with self.mock_mail_gateway(mail_unlink_sent=False), self.assertRaises(ValueError):
-            composer.send_mail()
+            composer._action_send_mail()

addons/test_mail/tests/test_performance.py

@@ -272,7 +272,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
             })
 
         with self.assertQueryCount(__system__=32, emp=38):
-            composer.send_mail()
+            composer._action_send_mail()
 
     @users('__system__', 'emp')
     @warmup
@@ -293,7 +293,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
             })
 
         with self.assertQueryCount(__system__=25, emp=31):
-            composer.send_mail()
+            composer._action_send_mail()
 
     @users('__system__', 'emp')
     @warmup
@@ -309,10 +309,10 @@ class TestMailAPIPerformance(BaseMailPerformance):
                 'default_res_id': test_record.id,
                 'default_template_id': test_template.id,
             }).create({})
-            composer.onchange_template_id_wrapper()
+            composer._onchange_template_id_wrapper()
 
         with self.assertQueryCount(__system__=33, emp=39):
-            composer.send_mail()
+            composer._action_send_mail()
 
         # remove created partner to ensure tests are the same each run
         self.env['res.partner'].sudo().search([('email', '=', 'nopartner.test@example.com')]).unlink()

addons/website_sale/models/mail_compose_message.py

@@ -7,7 +7,7 @@ from odoo import api, models
 class MailComposeMessage(models.TransientModel):
     _inherit = 'mail.compose.message'
 
-    def send_mail(self, auto_commit=False):
+    def _action_send_mail(self, auto_commit=False):
         context = self._context
         # TODO TDE: clean that brole one day
         if context.get('website_sale_send_recovery_email') and self.model == 'sale.order' and context.get('active_ids'):
@@ -16,4 +16,4 @@ class MailComposeMessage(models.TransientModel):
                 ('cart_recovery_email_sent', '=', False),
                 ('is_abandoned_cart', '=', True)
             ]).write({'cart_recovery_email_sent': True})
-        return super(MailComposeMessage, self).send_mail(auto_commit=auto_commit)
+        return super(MailComposeMessage, self)._action_send_mail(auto_commit=auto_commit)