[FW][FIX] base, mail: fix various template preview issues

## Issues

- `error_msg` is not set when the template cannot find any record to use as example (16.2)
- `resource_ref` is only initialized in default_get. If the preview generates a report, the commit from that action invalidates the cache and that value disappears. (14.0, real issue since 16.2 as it was previously handled another way)
- html data using empty nodes `<t></t>` are incorrectly converted to 'self-closing' tags `<t/>` which becomes <t> thanks to standard html parsing. (14.0, real issue in 16.0 then 16.2)*

This last one was not an issue because jquery used to fix these invalid tags for us, until a recent update here: 9c41ee5091ac06ac3ca71aeac607195c70061e4a

## Fixes

- Simply add a default value and set that at the end of the compute method
- make resource_ref a write-able computed field to keep the same functionality while allowing the ORM to recompute it
- use the `method` argument of `lxml.etree.tostring` to tell lxml to print the content of the nodes as valid HTML in the data converter

See sub commits for more details.

Task-3162320

closes odoo/odoo#118710

Forward-port-of: odoo/odoo#114632
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

addons/mail/models/mail_template.py

@@ -6,7 +6,7 @@ import itertools
 import logging
 
 from odoo import _, api, fields, models, tools, Command
-from odoo.exceptions import UserError
+from odoo.exceptions import ValidationError, UserError
 from odoo.tools import is_html_empty
 from odoo.tools.safe_eval import safe_eval, time
 
@@ -146,12 +146,22 @@ class MailTemplate(models.Model):
             record.attachment_ids.write({'res_model': record._name, 'res_id': record.id})
         return self
 
+    def _check_abstract_models(self, vals_list):
+        model_names = self.sudo().env['ir.model'].browse(filter(None, (
+            vals.get('model_id') for vals in vals_list
+        ))).mapped('model')
+        for model in model_names:
+            if self.env[model]._abstract:
+                raise ValidationError(_('You may not define a template on an abstract model: %s', model))
+
     @api.model_create_multi
     def create(self, vals_list):
+        self._check_abstract_models(vals_list)
         return super().create(vals_list)\
             ._fix_attachment_ownership()
 
     def write(self, vals):
+        self._check_abstract_models([vals])
         super().write(vals)
         self._fix_attachment_ownership()
         return True

addons/mail/tests/test_mail_template.py

@@ -3,7 +3,7 @@
 from markupsafe import Markup
 
 from odoo.addons.mail.tests.common import MailCommon
-from odoo.exceptions import AccessError, UserError
+from odoo.exceptions import AccessError, ValidationError, UserError
 from odoo.modules.module import get_module_resource
 from odoo.tests import Form, tagged, users
 from odoo.tools import convert_file
@@ -50,6 +50,26 @@ class TestMailTemplate(MailCommon):
         self.assertEqual(values[self.partner_employee.id]['subject'], '6', 'We must trust mail template values')
         self.assertIn('13', values[self.partner_employee.id]['body_html'], 'We must trust mail template values')
 
+    @users('admin')
+    def test_mail_template_abstract_model(self):
+        """Check abstract models cannot be set on templates."""
+        # create
+        with self.assertRaises(ValidationError), self.cr.savepoint():
+            self.env['mail.template'].create({
+                'name': 'Test abstract template',
+                'model_id': self.env['ir.model']._get('mail.thread').id, # abstract model
+            })
+        # write
+        template = self.env['mail.template'].create({
+            'name': 'Test abstract template',
+            'model_id': self.env['ir.model']._get('res.partner').id,
+        })
+        with self.assertRaises(ValidationError), self.cr.savepoint():
+            template.write({
+                'name': 'Test abstract template',
+                'model_id': self.env['ir.model']._get('mail.thread').id,
+            })
+
     def test_mail_template_acl(self):
         # Sanity check
         self.assertTrue(self.user_admin.has_group('mail.group_mail_template_editor'))

addons/mail/wizard/mail_template_preview.py

@@ -28,21 +28,16 @@ class MailTemplatePreview(models.TransientModel):
     def _selection_languages(self):
         return self.env['res.lang'].get_installed()
 
-    @api.model
-    def default_get(self, fields):
-        result = super(MailTemplatePreview, self).default_get(fields)
-        if not result.get('mail_template_id') or 'resource_ref' not in fields:
-            return result
-        mail_template = self.env['mail.template'].browse(result['mail_template_id']).sudo()
-        model = mail_template.model
-        res = self.env[model].search([], limit=1)
-        if res:
-            result['resource_ref'] = '%s,%s' % (model, res.id)
-        return result
 
     mail_template_id = fields.Many2one('mail.template', string='Related Mail Template', required=True)
     model_id = fields.Many2one('ir.model', string='Targeted model', related="mail_template_id.model_id")
-    resource_ref = fields.Reference(string='Record', selection='_selection_target_model')
+    resource_ref = fields.Reference(
+        string='Record',
+        compute='_compute_resource_ref',
+        compute_sudo=False, readonly=False,
+        selection='_selection_target_model',
+        store=True
+    )
     lang = fields.Selection(_selection_languages, string='Template Preview Language')
     no_record = fields.Boolean('No Record', compute='_compute_no_record')
     error_msg = fields.Char('Error Message', compute='_compute_mail_template_fields')
@@ -71,6 +66,7 @@ class MailTemplatePreview(models.TransientModel):
         """ Preview the mail template (body, subject, ...) depending of the language and
         the record reference, more precisely the record id for the defined model of the mail template.
         If no record id is selectable/set, the inline_template placeholders won't be replace in the display information. """
+        error_msg = False
         mail_template = self.mail_template_id.with_context(lang=self.lang)
         if not self.resource_ref or not self.resource_ref.id:
             self._set_mail_attributes()
@@ -82,10 +78,18 @@ class MailTemplatePreview(models.TransientModel):
                     self._MAIL_TEMPLATE_FIELDS
                 )[self.resource_ref.id]
                 self._set_mail_attributes(values=mail_values)
-                self.error_msg = False
             except (ValueError, UserError) as user_error:
                 self._set_mail_attributes()
-                self.error_msg = user_error.args[0]
+                error_msg = user_error.args[0]
+        self.error_msg = error_msg
+
+    @api.depends('mail_template_id')
+    def _compute_resource_ref(self):
+        for preview in self:
+            mail_template = preview.mail_template_id.sudo()
+            model = mail_template.model
+            res = self.env[model].search([], limit=1)
+            preview.resource_ref = f'{model},{res.id}' if res else False
 
     def _set_mail_attributes(self, values=None):
         for field in self._MAIL_TEMPLATE_FIELDS:

addons/test_mail/tests/test_mail_template_preview.py

@@ -20,6 +20,45 @@ class TestMailTemplateTools(TestMailTemplateCommon):
         self.assertEqual(len(self.test_template.partner_to.split(',')), 2)
         self.assertTrue(self.test_record.email_from)
 
+    def test_mail_template_preview_empty_database(self):
+        """Check behaviour of the wizard when there is no record for the target model."""
+        self.env['mail.test.lang'].search([]).unlink()
+        test_template = self.env['mail.template'].browse(self.test_template.ids)
+        preview = self.env['mail.template.preview'].create({
+            'mail_template_id': test_template.id,
+        })
+
+        self.assertFalse(preview.error_msg)
+        for field in preview._MAIL_TEMPLATE_FIELDS:
+            if field in ['partner_to', 'report_template_ids']:
+                continue
+            self.assertEqual(test_template[field], preview[field])
+
+    def test_mail_template_preview_dynamic_attachment(self):
+        """Check behaviour with templates that use reports."""
+        test_record = self.env['mail.test.lang'].browse(self.test_record.ids)
+        test_report = self.env['ir.actions.report'].sudo().create({
+                'name': 'Test Report',
+                'model': test_record._name,
+                'print_report_name': "'TestReport for %s' % object.name",
+                'report_type': 'qweb-pdf',
+                'report_name': 'test_mail.mail_test_ticket_test_template',
+        })
+        self.test_template.write({
+            'report_template_ids': test_report.ids,
+            'attachment_ids': False,
+        })
+
+        preview = self.env['mail.template.preview'].with_context({
+            'force_report_rendering': False, # this also invalidates the test records...
+            }).create({
+            'mail_template_id': self.test_template.id,
+            'resource_ref': test_record,
+        })
+
+        self.assertEqual(preview.body_html, f'<p>EnglishBody for {test_record.name}</p>')
+        self.assertFalse(preview.attachment_ids, 'Reports should not be listed in attachments')
+
     def test_mail_template_preview_force_lang(self):
         test_record = self.env['mail.test.lang'].browse(self.test_record.ids)
         test_record.write({

odoo/addons/test_convert/tests/test_convert.py

@@ -265,6 +265,24 @@ class TestEvalXML(common.TransactionCase):
     def test_xml(self):
         pass
 
-    @unittest.skip("not tested")
     def test_html(self):
-        pass
+        self.assertEqual(
+            self.eval_xml(Field(ET.fromstring(
+            """<parent>
+                <t t-if="True">
+                    <t t-out="'text'"/>
+                </t>
+                <t t-else="">
+                    <t t-out="'text2'"></t>
+                </t>
+            </parent>"""), type="html")),
+            """<parent>
+                <t t-if="True">
+                    <t t-out="'text'"></t>
+                </t>
+                <t t-else="">
+                    <t t-out="'text2'"></t>
+                </t>
+            </parent>""",
+            "Evaluating an HTML field should give empty nodes instead of self-closing tags"
+        )

odoo/tools/convert.py

@@ -142,7 +142,7 @@ def _eval_xml(self, node, env):
             return '<?xml version="1.0"?>\n'\
                 +_process("".join(etree.tostring(n, encoding='unicode') for n in node))
         if t == 'html':
-            return _process("".join(etree.tostring(n, encoding='unicode') for n in node))
+            return _process("".join(etree.tostring(n, method='html', encoding='unicode') for n in node))
 
         data = node.text
         if node.get('file'):