From 3dac7dd5c09a822c6cc50f94fa7163c95f744a15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Thu, 3 Nov 2022 10:44:56 +0000
Subject: [PATCH] [IMP] mail: support rendering on void or falsy record sets

In this commit we support calling rendering methods on void or falsy record
sets. Currently some rendering methods prevent from having 'None', some do
nothing specific. As Qweb or inline template support rendering on void records
we can safely support rendering on [], (), [False], [None]. Returned result
will either be void, either contain the rendered on void record for the
False or None key.

In this commit we also add a protection against trying to render a non
existing field. Instead of raising a KeyError, a ValueError is raised with
a clearer error message.

Task-2710804 (Mail: Clean MailThread Posting API)

Part-of: odoo/odoo#106072
---
 addons/mail/models/mail_render_mixin.py | 20 +++++------
 addons/mail/tests/test_mail_render.py   | 44 +++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/addons/mail/models/mail_render_mixin.py b/addons/mail/models/mail_render_mixin.py
index 8ad6fe4a913..75929065faa 100644
--- a/addons/mail/models/mail_render_mixin.py
+++ b/addons/mail/models/mail_render_mixin.py
@@ -289,7 +289,7 @@ class MailRenderMixin(models.AbstractModel):
         :return dict: {res_id: string of rendered template based on record}
         """
         results = dict.fromkeys(res_ids, u"")
-        if not template_src:
+        if not template_src or not res_ids:
             return results
 
         # prepare template variables
@@ -341,11 +341,9 @@ class MailRenderMixin(models.AbstractModel):
 
         :return dict: {res_id: string of rendered template based on record}
         """
-        # prevent wrong values (rendering on a void record set, ...)
-        if any(r is None for r in res_ids):
-            raise ValueError(_('Template rendering should be called on a valid record IDs.'))
-
         results = {}
+        if not res_ids:
+            return results
 
         # prepare template variables
         variables = self._render_eval_context()
@@ -384,12 +382,8 @@ class MailRenderMixin(models.AbstractModel):
 
         :return dict: {res_id: string of rendered template based on record}
         """
-        # prevent wrong values (rendering on a void record set, ...)
-        if any(r is None for r in res_ids):
-            raise ValueError(_('Template rendering should be called on a valid record IDs.'))
-
         results = dict.fromkeys(res_ids, u"")
-        if not template_txt:
+        if not template_txt or not res_ids:
             return results
 
         template_instructions = parse_inline_template(str(template_txt))
@@ -554,6 +548,12 @@ class MailRenderMixin(models.AbstractModel):
 
         :return dict: {res_id: string of rendered template based on record}
         """
+        if field not in self:
+            raise ValueError(
+                _('Rendering of %(field_name)s is not possible as not defined on template.',
+                  field_name=field
+                 )
+            )
         if options is None:
             options = {}
 
diff --git a/addons/mail/tests/test_mail_render.py b/addons/mail/tests/test_mail_render.py
index b196063043c..b97f9a0305f 100644
--- a/addons/mail/tests/test_mail_render.py
+++ b/addons/mail/tests/test_mail_render.py
@@ -97,6 +97,13 @@ class TestMailRenderCommon(common.MailCommon):
     <span>Autre Narrateur</span>
 </p>"""
         ]
+        cls.base_rendered_void = [
+            '<p>Hello</p>',
+            '<p>Hello </p>',
+            """<p>
+    <span>English Speaker</span>
+</p>"""
+        ]
 
         # link to mail template
         cls.test_template = cls.env['mail.template'].create({
@@ -198,6 +205,43 @@ class TestMailRender(TestMailRenderCommon):
             )[partner.id]
             self.assertEqual(rendered, expected)
 
+    @users('employee')
+    def test_render_field_no_records(self):
+        """ Test rendering on void IDs, or a list with dummy / falsy ID """
+        template = self.test_template.with_env(self.env)
+        partner = self.render_object.with_env(self.env)
+        for res_ids in ([], (), [False], [''], [None], [False, partner.id]):  # various corner cases
+            for fname, expected_obj, expected_void in zip(['subject', 'body_html'], self.base_rendered, self.base_rendered_void):
+                with self.subTest():
+                    rendered_all = template._render_field(
+                        fname,
+                        res_ids,
+                        compute_lang=True
+                    )
+                    if res_ids:
+                        self.assertTrue(res_ids[0] in rendered_all,
+                                        f'Rendering: key {repr(res_ids[0])} is considered as valid and should have an entry')
+                        self.assertEqual(rendered_all[res_ids[0]], expected_void)
+                    if len(res_ids) == 2:  # second is partner
+                        self.assertTrue(res_ids[1] in rendered_all)
+                        self.assertEqual(rendered_all[res_ids[1]], expected_obj)
+                    if not res_ids:
+                        self.assertFalse(rendered_all,
+                                         'Rendering: void input -> void output')
+
+    @users('employee')
+    def test_render_field_not_existing(self):
+        """ Test trying to render a not-existing field: raise a proper ValueError
+        instead of crashing / raising a KeyError """
+        template = self.env['mail.template'].browse(self.test_template.ids)
+        partner = self.env['res.partner'].browse(self.render_object_fr.ids)
+        with self.assertRaises(ValueError):
+            _rendered = template._render_field(
+                'not_existing',
+                partner.ids,
+                compute_lang=True
+            )[partner.id]
+
     @users('employee')
     def test_render_template_inline_template(self):
         partner = self.env['res.partner'].browse(self.render_object.ids)