From 74b59bab26d6fb0d40669952765572d7cc685505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 18 Jan 2023 08:23:16 +0000 Subject: [PATCH 01/32] [FIX] mail: improve 'subtype_is_log' composer computation As subtype has no usage in mail mode, let us consider subtype_is_log as being True in mail mode, as it eases display of buttons. This is its main usage aka display "Log" instead of "Send" on composer. In mail mode it should always be send. Followup of odoo/odoo#99482 Task-2710804 (Mail: Clean MailThread API) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 7 +++++-- addons/test_mail/tests/test_mail_composer.py | 6 +++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 4f9f3ca0139..7db56eb0961 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -228,9 +228,12 @@ class MailComposer(models.TransientModel): @api.depends('subtype_id') def _compute_subtype_is_log(self): + """ In comment mode, tells whether the subtype is a note. Subtype has + no use in email mode, and this field will be False. """ note_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note') - for composer in self: - composer.subtype_is_log = not composer.subtype_id or composer.subtype_id.id == note_id + self.subtype_is_log = False + for composer in self.filtered('subtype_id'): + composer.subtype_is_log = composer.subtype_id.id == note_id @api.depends('reply_to_force_new') def _compute_reply_to_mode(self): diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index fa12cd92da8..a446d7b7957 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -310,7 +310,7 @@ class TestComposerForm(TestMailComposer): self.assertFalse(composer_form.scheduled_date) self.assertFalse(composer_form.subject, 'MailComposer: mass mode should have void default subject if no template') self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) - self.assertFalse(composer_form.subtype_is_log) + self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') @users('employee') def test_mail_composer_mass_wtpl(self): @@ -337,7 +337,7 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.subject, self.template.subject, 'MailComposer: mass mode should have template raw subject if template') self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) - self.assertFalse(composer_form.subtype_is_log) + self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') @users('employee') def test_mail_composer_mass_wtpl_norecords(self): @@ -368,7 +368,7 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.subject, self.template.subject, 'MailComposer: mass mode should have template raw subject if template') self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) - self.assertFalse(composer_form.subtype_is_log) + self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') @tagged('mail_composer') From b77af1760bc9db3c9e92f5d436338365f2c517ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 18 Jan 2023 09:54:33 +0000 Subject: [PATCH 02/32] [FIX] mail: fix 'res_domain' usage in composer Using 'res_domain' should always activate the batch mode, either in mailing or comment mode. In this commit we fix some glitches linked to the usage of this new mode recently introduced. Followup of odoo/odoo#99482 Task-3035101 (Mail: Support batch-posting from composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 7db56eb0961..9ffbfd42276 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -108,7 +108,8 @@ class MailComposer(models.TransientModel): if 'force_send' in fields_list and 'force_send' not in result: result['force_send'] = ( result.get('composition_mode') != 'comment' or - len(self._parse_res_ids(result.get('res_ids') or [])) <= 1 + (not result.get('res_domain') and + len(self._parse_res_ids(result.get('res_ids') or [])) <= 1) ) return { @@ -263,11 +264,17 @@ class MailComposer(models.TransientModel): @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._evaluate_res_ids())['value'] + values = self._onchange_template_id( + self.template_id.id, + self.composition_mode, + self.model, + self._evaluate_res_ids(), + self.res_domain, + )['value'] for fname, value in values.items(): setattr(self, fname, value) - def _onchange_template_id(self, template_id, composition_mode, model, res_ids): + def _onchange_template_id(self, template_id, composition_mode, model, res_ids, res_domain=None): """ Perform the onchange. * mass_mailing or comment in batch: we cannot render, so return the @@ -275,7 +282,7 @@ class MailComposer(models.TransientModel): * normal mode: return rendered values -> for x2many field, this onchange return command instead of ids """ - if template_id and (composition_mode == 'mass_mail' or len(res_ids) > 1): + if template_id and (composition_mode == 'mass_mail' or res_domain or len(res_ids) > 1): # copy raw template values (not rendered due to mass mode) template = self.env['mail.template'].browse(template_id) values = dict( From c712998f00fa686f09d1d39eeeb9a2683b33e8cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 24 Jan 2023 16:06:27 +0000 Subject: [PATCH 03/32] [FIX] mail: be more defensive when previewing fields Sometimes due to new ids / not existing records for previewing crash may happen when rendering the preview of templates. In this commit we rewrite a bit the computation to try to be more resistant. Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_template_preview.py | 22 ++++++++------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/addons/mail/wizard/mail_template_preview.py b/addons/mail/wizard/mail_template_preview.py index 6eb21551de0..42f49ff8c84 100644 --- a/addons/mail/wizard/mail_template_preview.py +++ b/addons/mail/wizard/mail_template_preview.py @@ -71,26 +71,20 @@ 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. """ - copy_depends_values = {'lang': self.lang} mail_template = self.mail_template_id.with_context(lang=self.lang) - try: - if not self.resource_ref.id: - self._set_mail_attributes() - else: - copy_depends_values['resource_ref'] = '%s,%s' % (self.resource_ref._name, self.resource_ref.id) + if not self.resource_ref or not self.resource_ref.id: + self._set_mail_attributes() + else: + try: mail_values = mail_template.with_context(template_preview_lang=self.lang)._generate_template( [self.resource_ref.id], 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] - finally: - # Avoid to be change by a cache invalidation (in generate_mail), e.g. Quotation / Order report - for key, value in copy_depends_values.items(): - self[key] = value + self.error_msg = False + except (ValueError, UserError) as user_error: + self._set_mail_attributes() + self.error_msg = user_error.args[0] def _set_mail_attributes(self, values=None): for field in self._MAIL_TEMPLATE_FIELDS: From a2e946277b17eb84a879aef074e2e63666319d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 24 Jan 2023 16:09:31 +0000 Subject: [PATCH 04/32] [FIX] mail: improve wording of 'report_template_ids' template field Improve 'report_template_ids' label. Followup of odoo/odoo#99482 Task-2868153 (Mail: Allow multi-reports mail templates) Part-of: odoo/odoo#107356 --- addons/mail/models/mail_template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index bccd11d813c..9101525f99f 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -68,7 +68,7 @@ class MailTemplate(models.Model): 'ir.actions.report', relation='mail_template_ir_actions_report_rel', column1='mail_template_id', column2='ir_actions_report_id', - string='Reports to print and attach', + string='Dynamic Reports', domain="[('model', '=', model)]") # options mail_server_id = fields.Many2one('ir.mail_server', 'Outgoing Mail Server', readonly=False, From 587555d55868696998635a2c2de5a82f21639225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 18 Jan 2023 10:30:22 +0000 Subject: [PATCH 05/32] [IMP] test_mail: add composer tests for res_domain usage Purpose of this commit is to add some tests about 'res_domain' usage in the composer. As it takes over 'res_ids' when being set, being sure all use cases are covered is better, especially in batch comment mode that has been recently added. Followup of odoo/odoo#99482 Task-3035101 (Mail: Support batch-posting from composer) Part-of: odoo/odoo#107356 --- addons/test_mail/tests/test_mail_composer.py | 193 ++++++++++++++----- 1 file changed, 146 insertions(+), 47 deletions(-) diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index a446d7b7957..39802f9bd0d 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -257,6 +257,38 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) self.assertFalse(composer_form.subtype_is_log) + @users('employee') + def test_mail_composer_comment_wtpl_domain(self): + """ Batch mode of composer in comment mode, using a domain. """ + composer_form = Form(self.env['mail.compose.message'].with_context( + default_composition_mode='comment', + default_model=self.test_records._name, + default_res_domain=[('id', 'in', self.test_records.ids)], + default_template_id=self.template.id, + )) + self.assertTrue(composer_form.auto_delete, 'Should take composer value') + self.assertTrue(composer_form.auto_delete_keep_log) + self.assertEqual(composer_form.author_id, self.env.user.partner_id) + self.assertEqual(composer_form.body, self.template.body_html, + 'MailComposer: comment in batch mode should have template raw body if template') + self.assertTrue(composer_form.composition_batch) + self.assertEqual(composer_form.composition_mode, 'comment') + self.assertEqual(composer_form.email_from, self.template.email_from, + 'MailComposer: comment in batch mode should have template raw email_from if template') + self.assertFalse(composer_form.force_send, 'MailComposer: batch record post use email queue for notifications') + self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) + self.assertEqual(composer_form.model, self.test_record._name) + self.assertFalse(composer_form.record_name, 'MailComposer: comment in batch mode should have void record name') + self.assertEqual(composer_form.reply_to, self.template.reply_to) + self.assertFalse(composer_form.reply_to_force_new) + self.assertFalse(composer_form.res_ids) + self.assertEqual(literal_eval(composer_form.res_domain), [('id', 'in', self.test_records.ids)]) + self.assertEqual(composer_form.scheduled_date, self.template.scheduled_date) + self.assertEqual(composer_form.subject, self.template.subject, + 'MailComposer: comment in batch mode should have template raw subject if template') + self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) + self.assertFalse(composer_form.subtype_is_log) + @users('employee') def test_mail_composer_comment_wtpl_norecords(self): """ Test specific case when running without records, to see the rendering @@ -290,6 +322,7 @@ class TestComposerForm(TestMailComposer): @users('employee') def test_mail_composer_mass(self): + """ Test composer called in mass mailing mode. """ composer_form = Form(self.env['mail.compose.message'].with_context( self._get_web_context(self.test_records, add_web=True) )) @@ -314,6 +347,8 @@ class TestComposerForm(TestMailComposer): @users('employee') def test_mail_composer_mass_wtpl(self): + """ Test composer called in mass mailing mode with a template. It globally + takes the template value raw (aka not rendered). """ composer_form = Form(self.env['mail.compose.message'].with_context( self._get_web_context(self.test_records, add_web=True, default_template_id=self.template.id) )) @@ -339,6 +374,39 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') + @users('employee') + def test_mail_composer_mass_wtpl_domain(self): + """ Same as test_mail_composer_mass_wtpl, but using a domain instead + of res_ids, to check support of domain. """ + composer_form = Form(self.env['mail.compose.message'].with_context( + default_composition_mode='mass_mail', + default_model=self.test_records._name, + default_res_domain=[('id', 'in', self.test_records.ids)], + default_template_id=self.template.id, + )) + self.assertTrue(composer_form.auto_delete, 'Should take composer value') + self.assertTrue(composer_form.auto_delete_keep_log) + self.assertEqual(composer_form.author_id, self.env.user.partner_id) + self.assertEqual(composer_form.body, self.template.body_html, + 'MailComposer: mass mode should have template raw body if template') + self.assertTrue(composer_form.composition_batch) + self.assertEqual(composer_form.composition_mode, 'mass_mail') + self.assertEqual(composer_form.email_from, self.template.email_from, + 'MailComposer: mass mode should have template raw email_from if template') + self.assertTrue(composer_form.force_send, 'MailComposer: mass mode sends emails right away') + self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) + self.assertEqual(composer_form.model, self.test_records._name) + self.assertFalse(composer_form.record_name, 'MailComposer: mass mode should have void record name') + self.assertEqual(composer_form.reply_to, self.template.reply_to) + self.assertFalse(composer_form.reply_to_force_new) + self.assertFalse(composer_form.res_ids) + self.assertEqual(literal_eval(composer_form.res_domain), [('id', 'in', self.test_records.ids)]) + self.assertEqual(composer_form.scheduled_date, self.template.scheduled_date) + self.assertEqual(composer_form.subject, self.template.subject, + 'MailComposer: mass mode should have template raw subject if template') + self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) + self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') + @users('employee') def test_mail_composer_mass_wtpl_norecords(self): """ Test specific case when running without records, to see the rendering @@ -397,15 +465,20 @@ class TestComposerInternals(TestMailComposer): 'res_id': False, }) - for composition_mode, batch in (('comment', False), ('comment', True), - ('mass_mail', False), ('mass_mail', True)): - with self.subTest(composition_mode=composition_mode, batch=batch): + for composition_mode, batch_mode in product(('comment', 'mass_mail'), + (False, True, 'domain')): + with self.subTest(composition_mode=composition_mode, batch_mode=batch_mode): + batch = bool(batch_mode) test_records = self.test_records if batch else self.test_record - ctx = self._get_web_context( - test_records, add_web=False, - default_composition_mode=composition_mode, - default_template_id=self.template.id - ) + ctx = { + 'default_model': test_records._name, + 'default_composition_mode': composition_mode, + 'default_template_id': self.template.id, + } + if batch_mode == 'domain': + ctx['default_res_domain'] = [('id', 'in', test_records.ids)] + else: + ctx['default_res_ids'] = test_records.ids composer = self.env['mail.compose.message'].with_context(ctx).create({ 'body': '

Test Body

', @@ -474,14 +547,19 @@ class TestComposerInternals(TestMailComposer): 'email_from': False, }) - for composition_mode, batch in (('comment', False), ('comment', True), - ('mass_mail', False), ('mass_mail', True)): - with self.subTest(composition_mode=composition_mode, batch=batch): + for composition_mode, batch_mode in product(('comment', 'mass_mail'), + (False, True, 'domain')): + with self.subTest(composition_mode=composition_mode, batch_mode=batch_mode): + batch = bool(batch_mode) test_records = self.test_records if batch else self.test_record - ctx = self._get_web_context( - test_records, add_web=False, - default_composition_mode=composition_mode - ) + ctx = { + 'default_model': test_records._name, + 'default_composition_mode': composition_mode, + } + if batch_mode == 'domain': + ctx['default_res_domain'] = [('id', 'in', test_records.ids)] + else: + ctx['default_res_ids'] = test_records.ids composer = self.env['mail.compose.message'].with_context(ctx).create({ 'body': '

Test Body

', @@ -548,14 +626,19 @@ class TestComposerInternals(TestMailComposer): 'auto_delete': False, }) - for composition_mode, batch in (('comment', False), ('comment', True), - ('mass_mail', False), ('mass_mail', True)): - with self.subTest(composition_mode=composition_mode, batch=batch): + for composition_mode, batch_mode in product(('comment', 'mass_mail'), + (False, True, 'domain')): + with self.subTest(composition_mode=composition_mode, batch_mode=batch_mode): + batch = bool(batch_mode) test_records = self.test_records if batch else self.test_record - ctx = self._get_web_context( - test_records, add_web=False, - default_composition_mode=composition_mode - ) + ctx = { + 'default_model': test_records._name, + 'default_composition_mode': composition_mode, + } + if batch_mode == 'domain': + ctx['default_res_domain'] = [('id', 'in', test_records.ids)] + else: + ctx['default_res_ids'] = test_records.ids # 1. check without template (default values) + template update composer = self.env['mail.compose.message'].with_context(ctx).create({ @@ -634,14 +717,19 @@ class TestComposerInternals(TestMailComposer): 'subject': False, }) - for composition_mode, batch in (('comment', False), ('comment', True), - ('mass_mail', False), ('mass_mail', True)): - with self.subTest(composition_mode=composition_mode, batch=batch): + for composition_mode, batch_mode in product(('comment', 'mass_mail'), + (False, True, 'domain')): + with self.subTest(composition_mode=composition_mode, batch_mode=batch_mode): + batch = bool(batch_mode) test_records = self.test_records if batch else self.test_record - ctx = self._get_web_context( - test_records, add_web=False, - default_composition_mode=composition_mode - ) + ctx = { + 'default_model': test_records._name, + 'default_composition_mode': composition_mode, + } + if batch_mode == 'domain': + ctx['default_res_domain'] = [('id', 'in', test_records.ids)] + else: + ctx['default_res_ids'] = test_records.ids # 1. check without template + template update composer = self.env['mail.compose.message'].with_context(ctx).create({ @@ -829,9 +917,9 @@ class TestComposerInternals(TestMailComposer): 'reply_to': False, }) - for composition_mode, batch in (('comment', False), ('comment', True), - ('mass_mail', False), ('mass_mail', True)): - with self.subTest(composition_mode=composition_mode, batch=batch): + for composition_mode, batch_mode in product(('comment', 'mass_mail'), + (False, True, 'domain')): + with self.subTest(composition_mode=composition_mode, batch_mode=batch_mode): self.assertFalse( self.env['res.partner'].search([ ('email_normalized', 'in', ['test.cc.1@test.example.com', @@ -839,11 +927,16 @@ class TestComposerInternals(TestMailComposer): ]) ) + batch = bool(batch_mode) test_records = self.test_records if batch else self.test_record - ctx = self._get_web_context( - test_records, add_web=False, - default_composition_mode=composition_mode - ) + ctx = { + 'default_model': test_records._name, + 'default_composition_mode': composition_mode, + } + if batch_mode == 'domain': + ctx['default_res_domain'] = [('id', 'in', test_records.ids)] + else: + ctx['default_res_ids'] = test_records.ids # 1. check without template + template update composer = self.env['mail.compose.message'].with_context(ctx).create({ @@ -1275,11 +1368,12 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): attachs = self.env['ir.attachment'].search([('name', 'in', [a['name'] for a in attachment_data])]) self.assertEqual(len(attachs), 2) - for batch, scheduled_date in product( - (False, True), + for batch_mode, scheduled_date in product( + (False, True, 'domain'), (False, '{{ (object.create_date or datetime.datetime(2022, 12, 26, 18, 0, 0)) + datetime.timedelta(days=2) }}') ): - with self.subTest(batch=batch, scheduled_date=scheduled_date): + with self.subTest(batch_mode=batch_mode, scheduled_date=scheduled_date): + batch = bool(batch_mode) self.template.write({'scheduled_date': scheduled_date}) test_records = self.test_records if batch else self.test_record @@ -1288,15 +1382,20 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): self.assertEqual(test_records.user_id, self.user_employee_2) self.assertEqual(test_records.message_partner_ids, self.partner_employee_2) + ctx = { + 'default_model': test_records._name, + 'default_composition_mode': 'comment', + 'default_template_id': self.template.id, + # avoid successive tests issues with followers + 'mail_create_nosubscribe': True, + } + if batch_mode == 'domain': + ctx['default_res_domain'] = [('id', 'in', test_records.ids)] + else: + ctx['default_res_ids'] = test_records.ids + # open a composer and run it in comment mode - composer_form = Form(self.env['mail.compose.message'].with_context( - self._get_web_context(test_records, add_web=True, - default_composition_mode='comment', - default_template_id=self.template.id, - # avoid successive tests issues with followers - mail_create_nosubscribe=True, - ) - )) + composer_form = Form(self.env['mail.compose.message'].with_context(ctx)) composer = composer_form.save() self.assertFalse(composer.reply_to_force_new, 'Mail: thread-enabled models should use auto thread by default') From 2c87910f2c729b79a8dcbd649f1fc5f8d1f2421a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 18 Jan 2023 16:00:19 +0000 Subject: [PATCH 06/32] [IMP] test_mail: add some additional asserts about authorship As this matter is a bit complicated due to synchronization / desynchronization of author and email, some asserts are added to help understanding the changes. To ease converting field into an editable stored field, some additional checks are added in tests to ease debugging and avoid regressions. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/test_mail/tests/test_ir_actions.py | 3 + addons/test_mail/tests/test_mail_composer.py | 59 +++++++++++++++----- 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/addons/test_mail/tests/test_ir_actions.py b/addons/test_mail/tests/test_ir_actions.py index 933886377f4..b450ec6d881 100644 --- a/addons/test_mail/tests/test_ir_actions.py +++ b/addons/test_mail/tests/test_ir_actions.py @@ -78,6 +78,9 @@ class TestServerActionsEmail(TestMailCommon, TestServerActionsBase): with self.assertSinglePostNotifications( [{'partner': self.test_partner, 'type': 'email', 'status': 'ready'}], message_info={'content': 'Hello %s' % self.test_partner.name, + 'fields_values': { + 'author_id': self.env.user.partner_id, + }, 'message_type': 'notification', 'subtype': 'mail.mt_comment', } diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 39802f9bd0d..0382c47fa59 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -570,51 +570,62 @@ class TestComposerInternals(TestMailComposer): self.assertEqual(composer.composition_mode, composition_mode) self.assertEqual(composer.email_from, self.env.user.email_formatted) - # author values reset email (FIXME: currently not synchronized) + # author update should reset email (FIXME: currently not synchronized) composer.write({'author_id': self.partner_1}) self.assertEqual(composer.author_id, self.partner_1) - self.assertEqual(composer.email_from, self.env.user.email_formatted) + self.assertEqual(composer.email_from, self.env.user.email_formatted, + 'MailComposer: TODO: author / email_from are not synchronized') # self.assertEqual(composer.email_from, self.partner_1.email_formatted) # changing template should update its email_from - composer.write({'template_id': self.template.id, 'author_id': self.env.user.partner_id}) + composer.write({'template_id': self.template.id}) # currently onchange necessary 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' and not batch: + self.assertEqual(composer.author_id, self.partner_1, + 'MailComposer: TODO: should try to synchronize with email_from') self.assertEqual(composer.email_from, self.test_record.user_id.email_formatted, f'MailComposer: should take email_from rendered from template ({composition_mode}-{batch})') else: + self.assertEqual(composer.author_id, self.partner_1, + 'MailComposer: TODO: email_from is raw, what to do with it ?') self.assertEqual(composer.email_from, self.template.email_from, f'MailComposer: should take email_from raw from template ({composition_mode}-{batch})') # manual values are kept over template values composer.write({'email_from': self.test_from}) - self.assertEqual(composer.author_id, self.env.user.partner_id) - self.assertEqual(composer.email_from, self.test_from) + self.assertEqual(composer.author_id, self.partner_1) + self.assertEqual(composer.email_from, self.test_from, + 'MailComposer: TODO: author / email_from are not synchronized') # update with template with void values: void value is not forced in # rendering mode as well as when copying template values composer.write({'template_id': template_void.id}) # currently onchange necessary composer._onchange_template_id_wrapper() + if composition_mode == 'comment' and not batch: - self.assertEqual(composer.author_id, self.env.user.partner_id) + self.assertEqual(composer.author_id, self.partner_1, + 'MailComposer: TODO: author / email_from are not synchronized') self.assertEqual(composer.email_from, self.test_from) else: - self.assertEqual(composer.author_id, self.env.user.partner_id) + self.assertEqual(composer.author_id, self.partner_1, + 'MailComposer: TODO: author / email_from are not synchronized') self.assertEqual(composer.email_from, self.test_from) # reset template: values are reset due to call to default_get composer.write({'template_id': False}) # currently onchange necessary composer._onchange_template_id_wrapper() + if composition_mode == 'comment' and not batch: - self.assertEqual(composer.author_id, self.env.user.partner_id) + self.assertEqual(composer.author_id, self.partner_1, + 'MailComposer: TODO: author / email_from are not synchronized') self.assertEqual(composer.email_from, self.env.user.email_formatted) else: - self.assertEqual(composer.author_id, self.env.user.partner_id) + self.assertEqual(composer.author_id, self.partner_1, + 'MailComposer: TODO: author / email_from are not synchronized') self.assertEqual(composer.email_from, self.env.user.email_formatted) @users('employee') @@ -1397,6 +1408,18 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): # open a composer and run it in comment mode composer_form = Form(self.env['mail.compose.message'].with_context(ctx)) composer = composer_form.save() + + # ensure some parameters used afterwards + if batch: + author = self.env.user.partner_id + self.assertEqual(composer.author_id, author, + 'Author cannot be synchronized with a raw email_from') + self.assertEqual(composer.email_from, self.template.email_from) + else: + author = self.env.user.partner_id + self.assertEqual(composer.author_id, author, + 'Author cannot be synchronized with a raw email_from') + self.assertEqual(composer.email_from, self.partner_employee_2.email_formatted) self.assertFalse(composer.reply_to_force_new, 'Mail: thread-enabled models should use auto thread by default') # due to scheduled_date, cron for sending notification will be used @@ -1469,7 +1492,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): message = test_record.message_ids[0] self.assertMailMail(self.partner_employee_2, 'sent', mail_message=message, - author=self.partner_employee, # author != email_from (template sets only email_from) + author=author, # author != email_from (template sets only email_from) email_values={ 'body_content': f'TemplateBody {test_record.name}', 'email_from': test_record.user_id.email_formatted, # set by template @@ -1487,7 +1510,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): ) self.assertMailMail(test_record.customer_id + new_partners, 'sent', mail_message=message, - author=self.partner_employee, # author != email_from (template sets only email_from) + author=author, # author != email_from (template sets only email_from) email_values={ 'body_content': f'TemplateBody {test_record.name}', 'email_from': test_record.user_id.email_formatted, # set by template @@ -1762,6 +1785,12 @@ class TestComposerResultsMass(TestMailComposer): default_template_id=self.template.id) )) composer = composer_form.save() + # ensure some parameters used afterwards + author = self.env.user.partner_id + self.assertEqual(composer.author_id, author, + 'Author cannot be synchronized with a raw email_from') + self.assertEqual(composer.email_from, self.template.email_from) + with self.mock_mail_gateway(mail_unlink_sent=False), \ freeze_time(self.reference_now): composer._action_send_mail() @@ -1798,7 +1827,7 @@ class TestComposerResultsMass(TestMailComposer): self.assertMailMail(record.customer_id + new_partners + self.partner_admin, 'sent', mail_message=message, - author=self.partner_employee, + author=author, email_values={ 'attachments_info': [ {'name': 'AttFileName_00.txt', 'raw': b'AttContent_00', 'type': 'text/plain'}, @@ -1843,7 +1872,7 @@ class TestComposerResultsMass(TestMailComposer): self.assertMailMail(record.customer_id + new_partners + self.partner_admin, 'sent', mail_message=record.message_ids[0], - author=self.partner_employee, + author=author, email_values={ 'email_from': self.partner_employee_2.email_formatted, 'reply_to': self.partner_employee_2.email_formatted, From 5b8f9341a1377f2d54d9990bc62b284161103d46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 27 Jan 2023 16:04:20 +0000 Subject: [PATCH 07/32] [IMP] test_mail: remove __system__ performance tests Replace them by admin, which should globally be the same as employee. Testing with system is a remaining of old times, when bichons were running freely in the court. Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#107356 --- addons/test_mail/tests/test_performance.py | 222 ++++++++++----------- 1 file changed, 111 insertions(+), 111 deletions(-) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 191f078b116..84df227843f 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -147,14 +147,14 @@ class TestBaseMailPerformance(BaseMailPerformance): } ]) - @users('__system__', 'demo') + @users('admin', 'demo') @warmup def test_read_mail(self): """ Read records inheriting from 'mail.thread'. """ records = self.env['mail.performance.thread'].search([]) self.assertEqual(len(records), 5) - with self.assertQueryCount(__system__=2, demo=2): + with self.assertQueryCount(admin=2, demo=2): # without cache for record in records: record.partner_id.country_id.name @@ -169,27 +169,27 @@ class TestBaseMailPerformance(BaseMailPerformance): for record in records: record.value_pc - @users('__system__', 'demo') + @users('admin', 'demo') @warmup def test_write_mail(self): """ Write records inheriting from 'mail.thread' (no recomputation). """ records = self.env['mail.performance.thread'].search([]) self.assertEqual(len(records), 5) - with self.assertQueryCount(__system__=2, demo=2): + with self.assertQueryCount(admin=2, demo=2): records.write({'name': 'X'}) - @users('__system__', 'demo') + @users('admin', 'demo') @warmup def test_write_mail_with_recomputation(self): """ Write records inheriting from 'mail.thread' (with recomputation). """ records = self.env['mail.performance.thread'].search([]) self.assertEqual(len(records), 5) - with self.assertQueryCount(__system__=2, demo=2): + with self.assertQueryCount(admin=2, demo=2): records.write({'value': 42}) - @users('__system__', 'demo') + @users('admin', 'demo') @warmup def test_write_mail_with_tracking(self): """ Write records inheriting from 'mail.thread' (with field tracking). """ @@ -200,42 +200,42 @@ class TestBaseMailPerformance(BaseMailPerformance): 'partner_id': self.res_partner_12.id, }) - with self.assertQueryCount(__system__=3, demo=3): + with self.assertQueryCount(admin=3, demo=3): record.track = 'X' - @users('__system__', 'demo') + @users('admin', 'demo') @warmup def test_create_mail(self): """ Create records inheriting from 'mail.thread' (without field tracking). """ model = self.env['mail.performance.thread'] - with self.assertQueryCount(__system__=2, demo=2): + with self.assertQueryCount(admin=2, demo=2): model.with_context(tracking_disable=True).create({'name': 'X'}) - @users('__system__', 'demo') + @users('admin', 'demo') @warmup def test_create_mail_with_tracking(self): """ Create records inheriting from 'mail.thread' (with field tracking). """ - with self.assertQueryCount(__system__=8, demo=8): + with self.assertQueryCount(admin=8, demo=8): self.env['mail.performance.thread'].create({'name': 'X'}) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_create_mail_simple(self): - with self.assertQueryCount(__system__=7, employee=7): + with self.assertQueryCount(admin=7, employee=7): self.env['mail.test.simple'].create({'name': 'Test'}) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_create_mail_simple_multi(self): - with self.assertQueryCount(__system__=7, employee=7): + with self.assertQueryCount(admin=7, employee=7): self.env['mail.test.simple'].create([{'name': 'Test'}] * 5) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_write_mail_simple(self): rec = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=1, employee=1): + with self.assertQueryCount(admin=1, employee=1): rec.write({ 'name': 'Test2', 'email_from': 'test@test.com', @@ -251,15 +251,15 @@ class TestMailAPIPerformance(BaseMailPerformance): # automatically follow activities, for backward compatibility concerning query count self.env.ref('mail.mt_activities').write({'default': True}) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_adv_activity(self): model = self.env['mail.test.activity'] - with self.assertQueryCount(__system__=7, employee=7): + with self.assertQueryCount(admin=7, employee=7): model.create({'name': 'Test'}) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.models.unlink') def test_adv_activity_full(self): @@ -268,7 +268,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_res_model': 'mail.test.activity', }) - with self.assertQueryCount(__system__=6, employee=6): + with self.assertQueryCount(admin=6, employee=6): activity = MailActivity.create({ 'summary': 'Test Activity', 'res_id': record.id, @@ -278,16 +278,16 @@ class TestMailAPIPerformance(BaseMailPerformance): # voip module read activity_type during create leading to one less query in enterprise on action_feedback _category = activity.activity_type_id.category - with self.assertQueryCount(__system__=12, employee=13): + with self.assertQueryCount(admin=13, employee=13): activity.action_feedback(feedback='Zizisse Done !') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.models.unlink') def test_adv_activity_mixin(self): record = self.env['mail.test.activity'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=6, employee=6): + with self.assertQueryCount(admin=6, employee=6): activity = record.action_start('Test Start') # read activity_type to normalize cache between enterprise and community # voip module read activity_type during create leading to one less query in enterprise on action_close @@ -295,12 +295,12 @@ class TestMailAPIPerformance(BaseMailPerformance): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(__system__=13, employee=15): + with self.assertQueryCount(admin=15, employee=15): record.action_close('Dupe feedback') self.assertEqual(record.activity_ids, self.env['mail.activity']) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.models.unlink') def test_adv_activity_mixin_w_attachments(self): @@ -313,7 +313,7 @@ class TestMailAPIPerformance(BaseMailPerformance): for values in self.test_attachments_vals ]) - with self.assertQueryCount(__system__=6, employee=6): + with self.assertQueryCount(admin=6, employee=6): activity = record.action_start('Test Start') #read activity_type to normalize cache between enterprise and community #voip module read activity_type during create leading to one less query in enterprise on action_close @@ -321,7 +321,7 @@ class TestMailAPIPerformance(BaseMailPerformance): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(__system__=16, employee=19): # com+tm 15/18 + with self.assertQueryCount(admin=19, employee=19): # com+tm 18/18 record.action_close('Dupe feedback', attachment_ids=attachments.ids) # notifications @@ -331,13 +331,13 @@ class TestMailAPIPerformance(BaseMailPerformance): set(['AttFileName_00.txt', 'AttFileName_01.txt', 'AttFileName_02.txt']) ) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer(self): test_record, _test_template = self._create_test_records() customer_id = self.customer.id - with self.assertQueryCount(__system__=2, employee=2): + with self.assertQueryCount(admin=2, employee=2): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -347,17 +347,17 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer_id)], }) - with self.assertQueryCount(__system__=26, employee=32): + with self.assertQueryCount(admin=32, employee=32): composer._action_send_mail() - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer_attachments(self): test_record, _test_template = self._create_test_records() customer = self.env['res.partner'].browse(self.customer.ids) attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals) - with self.assertQueryCount(__system__=3, employee=3): + with self.assertQueryCount(admin=3, employee=3): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -368,17 +368,17 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer.id)], }) - with self.assertQueryCount(__system__=29, employee=37): + with self.assertQueryCount(admin=37, employee=37): composer._action_send_mail() - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer_form_attachments(self): test_record, _test_template = self._create_test_records() customer = self.env['res.partner'].browse(self.customer.ids) attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals) - with self.assertQueryCount(__system__=13, employee=13): # tm 12/12 + with self.assertQueryCount(admin=13, employee=13): # tm 12/12 composer_form = Form( self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', @@ -392,7 +392,7 @@ class TestMailAPIPerformance(BaseMailPerformance): composer_form.attachment_ids.add(attachment) composer = composer_form.save() - with self.assertQueryCount(__system__=39, employee=47): # tm+com 38/46 + with self.assertQueryCount(admin=47, employee=47): # tm+com 46/46 composer._action_send_mail() # notifications @@ -400,13 +400,13 @@ class TestMailAPIPerformance(BaseMailPerformance): self.assertEqual(message.attachment_ids, attachments) self.assertEqual(message.notified_partner_ids, customer + self.user_test.partner_id) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer_mass_w_template(self): _partners, test_records, test_template = self._create_test_records_for_batch() - with self.assertQueryCount(__system__=3, employee=3): + with self.assertQueryCount(admin=3, employee=3): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'mass_mail', 'default_model': test_records._name, @@ -415,18 +415,18 @@ class TestMailAPIPerformance(BaseMailPerformance): }).create({}) composer._onchange_template_id_wrapper() - with self.assertQueryCount(__system__=108, employee=141), self.mock_mail_gateway(): + with self.assertQueryCount(admin=141, employee=141), self.mock_mail_gateway(): composer._action_send_mail() self.assertEqual(len(self._new_mails), 10) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer_nodelete(self): test_record, _test_template = self._create_test_records() customer_id = self.customer.id - with self.assertQueryCount(__system__=2, employee=2): + with self.assertQueryCount(admin=2, employee=2): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -437,17 +437,17 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer_id)], }) - with self.assertQueryCount(__system__=26, employee=32): + with self.assertQueryCount(admin=32, employee=32): composer._action_send_mail() - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer_w_template(self): test_record, test_template = self._create_test_records() test_template.write({'attachment_ids': [(5, 0)]}) - with self.assertQueryCount(__system__=23, employee=24): # tm 13/14 / com 22/23 + with self.assertQueryCount(admin=24, employee=24): # tm 14/14 / com 23/23 composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -456,7 +456,7 @@ class TestMailAPIPerformance(BaseMailPerformance): }).create({}) composer._onchange_template_id_wrapper() - with self.assertQueryCount(__system__=25, employee=31): + with self.assertQueryCount(admin=31, employee=31): composer._action_send_mail() # notifications @@ -466,13 +466,13 @@ class TestMailAPIPerformance(BaseMailPerformance): # remove created partner to ensure tests are the same each run self.env['res.partner'].sudo().search([('email', '=', 'nopartner.test@example.com')]).unlink() - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer_w_template_attachments(self): test_record, test_template = self._create_test_records() - with self.assertQueryCount(__system__=24, employee=25): # tm 14/15 / com 23/24 + with self.assertQueryCount(admin=25, employee=25): # tm 15/15 / com 24/24 composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -481,7 +481,7 @@ class TestMailAPIPerformance(BaseMailPerformance): }).create({}) composer._onchange_template_id_wrapper() - with self.assertQueryCount(__system__=33, employee=44): + with self.assertQueryCount(admin=44, employee=44): composer._action_send_mail() # notifications @@ -494,7 +494,7 @@ class TestMailAPIPerformance(BaseMailPerformance): # remove created partner to ensure tests are the same each run self.env['res.partner'].sudo().search([('email', '=', 'nopartner.test@example.com')]).unlink() - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer_w_template_form(self): @@ -502,7 +502,7 @@ class TestMailAPIPerformance(BaseMailPerformance): test_template.write({'attachment_ids': [(5, 0)]}) customer = self.env['res.partner'].browse(self.customer.ids) - with self.assertQueryCount(__system__=35, employee=37): # tm 24/26 / com 34/36 + with self.assertQueryCount(admin=37, employee=37): # tm 26/26 / com 36/36 composer_form = Form( self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', @@ -513,7 +513,7 @@ class TestMailAPIPerformance(BaseMailPerformance): ) composer = composer_form.save() - with self.assertQueryCount(__system__=35, employee=41): + with self.assertQueryCount(admin=41, employee=41): composer._action_send_mail() # notifications @@ -525,14 +525,14 @@ class TestMailAPIPerformance(BaseMailPerformance): # remove created partner to ensure tests are the same each run new_partner.unlink() - @users('__system__', 'employee') + @users('admin', 'employee') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') def test_mail_composer_w_template_form_attachments(self): test_record, test_template = self._create_test_records() customer = self.env['res.partner'].browse(self.customer.ids) - with self.assertQueryCount(__system__=36, employee=38): # tm 25/27 / com 35/37 + with self.assertQueryCount(admin=38, employee=38): # tm 27/27 / com 37/37 composer_form = Form( self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', @@ -543,7 +543,7 @@ class TestMailAPIPerformance(BaseMailPerformance): ) composer = composer_form.save() - with self.assertQueryCount(__system__=47, employee=64): + with self.assertQueryCount(admin=64, employee=64): composer._action_send_mail() # notifications @@ -559,7 +559,7 @@ class TestMailAPIPerformance(BaseMailPerformance): new_partner.unlink() @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_assignation_email(self): # Changing the `notification_type` of a user adds or removes him a group @@ -570,31 +570,31 @@ class TestMailAPIPerformance(BaseMailPerformance): # use another user already pre-defined with the email notification type, # so the ormcache is preserved. record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=27, employee=28): + with self.assertQueryCount(admin=28, employee=28): record.write({ 'user_id': self.user_test_email.id, }) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=18, employee=20): + with self.assertQueryCount(admin=20, employee=20): record.write({ 'user_id': self.user_test_inbox.id, }) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_log(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=1, employee=1): + with self.assertQueryCount(admin=1, employee=1): record._message_log( body='

Test _message_log

', message_type='comment') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_log_batch(self): records = self.env['mail.test.simple'].create([ @@ -602,7 +602,7 @@ class TestMailAPIPerformance(BaseMailPerformance): for idx in range(10) ]) - with self.assertQueryCount(__system__=1, employee=1): + with self.assertQueryCount(admin=1, employee=1): records._message_log_batch( bodies=dict( (record.id, '

Test _message_log

') @@ -610,7 +610,7 @@ class TestMailAPIPerformance(BaseMailPerformance): ), message_type='comment') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_log_with_view(self): records = self.env['mail.test.simple'].create([ @@ -618,29 +618,29 @@ class TestMailAPIPerformance(BaseMailPerformance): for idx in range(10) ]) - with self.assertQueryCount(__system__=2, employee=2): + with self.assertQueryCount(admin=2, employee=2): records._message_log_with_view( 'test_mail.mail_template_simple_test', render_values={'partner': self.customer.with_env(self.env)} ) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_log_with_post(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=4, employee=7): + with self.assertQueryCount(admin=7, employee=7): record.message_post( body='

Test message_post as log

', subtype_xmlid='mail.mt_note', message_type='comment') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_post_no_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=4, employee=7): + with self.assertQueryCount(admin=7, employee=7): record.message_post( body='

Test Post Performances basic

', partner_ids=[], @@ -648,24 +648,24 @@ class TestMailAPIPerformance(BaseMailPerformance): subtype_xmlid='mail.mt_comment') @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=23, employee=26): + with self.assertQueryCount(admin=26, employee=26): record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, message_type='comment', subtype_xmlid='mail.mt_comment') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_post_one_inbox_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=14, employee=18): + with self.assertQueryCount(admin=18, employee=18): record.message_post( body='

Test Post Performances with an inbox ping

', partner_ids=self.user_test.partner_id.ids, @@ -673,47 +673,47 @@ class TestMailAPIPerformance(BaseMailPerformance): subtype_xmlid='mail.mt_comment') @mute_logger('odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_subscribe_default(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=6, employee=6): + with self.assertQueryCount(admin=6, employee=6): record.message_subscribe(partner_ids=self.user_test.partner_id.ids) - with self.assertQueryCount(__system__=3, employee=3): + with self.assertQueryCount(admin=3, employee=3): record.message_subscribe(partner_ids=self.user_test.partner_id.ids) @mute_logger('odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_subscribe_subtypes(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) subtype_ids = (self.env.ref('test_mail.st_mail_test_simple_external') | self.env.ref('mail.mt_comment')).ids - with self.assertQueryCount(__system__=5, employee=5): + with self.assertQueryCount(admin=5, employee=5): record.message_subscribe(partner_ids=self.user_test.partner_id.ids, subtype_ids=subtype_ids) - with self.assertQueryCount(__system__=2, employee=2): + with self.assertQueryCount(admin=2, employee=2): record.message_subscribe(partner_ids=self.user_test.partner_id.ids, subtype_ids=subtype_ids) @mute_logger('odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_message_track(self): record = self.env['mail.performance.tracking'].create({'name': 'Zizizatestname'}) - with self.assertQueryCount(__system__=3, employee=3): + with self.assertQueryCount(admin=3, employee=3): record.write({'name': 'Zizizanewtestname'}) - with self.assertQueryCount(__system__=3, employee=3): + with self.assertQueryCount(admin=3, employee=3): record.write({'field_%s' % (i): 'Tracked Char Fields %s' % (i) for i in range(3)}) - with self.assertQueryCount(__system__=4, employee=4): + with self.assertQueryCount(admin=4, employee=4): record.write({'field_%s' % (i): 'Field Without Cache %s' % (i) for i in range(3)}) record.flush_recordset() record.write({'field_%s' % (i): 'Field With Cache %s' % (i) for i in range(3)}) - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_notification_reply_to_batch(self): test_records_sudo = self.env['mail.test.container'].sudo().create([ @@ -723,7 +723,7 @@ class TestMailAPIPerformance(BaseMailPerformance): } for index in range(10) ]) - with self.assertQueryCount(__system__=1, employee=1): + with self.assertQueryCount(admin=1, employee=1): test_records = self.env['mail.test.container'].browse(test_records_sudo.ids) reply_to = test_records._notify_get_reply_to( default=self.env.user.email_formatted @@ -772,7 +772,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.env.flush_all() @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_mail_mail_send(self): message = self.env['mail.message'].sudo().create({ @@ -795,11 +795,11 @@ class TestMailComplexPerformance(BaseMailPerformance): 'mail_message_id': message.id, 'recipient_ids': [(4, pid) for pid in self.partners.ids], }) - with self.assertQueryCount(__system__=9, employee=9): + with self.assertQueryCount(admin=9, employee=9): self.env['mail.mail'].sudo().browse(mail.ids).send() @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_mail_mail_send_batch_complete(self): """ A more complete use case: 10 mails, attachments, servers, ... And @@ -828,7 +828,7 @@ class TestMailComplexPerformance(BaseMailPerformance): } for idx in range(12)]) mails[-2].write({'email_cc': False, 'email_to': 'strange@example¢¡.com', 'recipient_ids': [(5, 0)]}) mails[-1].write({'email_cc': False, 'email_to': 'void', 'recipient_ids': [(5, 0)]}) - with self.assertQueryCount(__system__=43, employee=43): + with self.assertQueryCount(admin=43, employee=43): self.env['mail.mail'].sudo().browse(mails.ids).send() for mail in mails[:-2]: @@ -840,14 +840,14 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertTrue(mails[-1].to_delete, 'Mail: mails with invalid recipient are also to be unlinked') @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_message_post(self): self.container.message_subscribe(self.user_portal.partner_id.ids) record = self.container.with_user(self.env.user) # about 20 (19?) queries per additional customer group - with self.assertQueryCount(__system__=35, employee=36): + with self.assertQueryCount(admin=36, employee=36): record.message_post( body='

Test Post Performances

', message_type='comment', @@ -857,7 +857,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(record.message_ids[0].notified_partner_ids, self.partners | self.user_portal.partner_id) @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_message_post_template(self): self.container.message_subscribe(self.user_portal.partner_id.ids) @@ -865,7 +865,7 @@ class TestMailComplexPerformance(BaseMailPerformance): template = self.env.ref('test_mail.mail_test_container_tpl') # about 20 (19 ?) queries per additional customer group - with self.assertQueryCount(__system__=43, employee=44): + with self.assertQueryCount(admin=44, employee=44): record.message_post_with_source( template, message_type='comment', @@ -876,12 +876,12 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(record.message_ids[0].notified_partner_ids, self.partners | self.user_portal.partner_id | self.customer) @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_message_post_view(self): _partners, test_records, test_template = self._create_test_records_for_batch() - with self.assertQueryCount(__system__=3, employee=3): + with self.assertQueryCount(admin=3, employee=3): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'mass_mail', 'default_model': test_records._name, @@ -890,7 +890,7 @@ class TestMailComplexPerformance(BaseMailPerformance): }).create({}) composer._onchange_template_id_wrapper() - with self.assertQueryCount(__system__=111, employee=131): + with self.assertQueryCount(admin=131, employee=131): messages_as_sudo = test_records.message_post_with_source( 'test_mail.mail_template_simple_test', render_values={'partner': self.user_test.partner_id}, @@ -900,7 +900,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(len(messages_as_sudo), 10) @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_message_subscribe(self): pids = self.partners.ids @@ -917,7 +917,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(rec1.message_partner_ids, self.env.user.partner_id | self.user_portal.partner_id) # subscribe new followers with forced given subtypes - with self.assertQueryCount(__system__=4, employee=4): + with self.assertQueryCount(admin=4, employee=4): rec.message_subscribe( partner_ids=pids[:4], subtype_ids=subtype_ids @@ -926,7 +926,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(rec1.message_partner_ids, self.env.user.partner_id | self.user_portal.partner_id | self.partners[:4]) # subscribe existing and new followers with force=False, meaning only some new followers will be added - with self.assertQueryCount(__system__=5, employee=5): + with self.assertQueryCount(admin=5, employee=5): rec.message_subscribe( partner_ids=pids[:6], subtype_ids=None @@ -935,7 +935,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(rec1.message_partner_ids, self.env.user.partner_id | self.user_portal.partner_id | self.partners[:6]) # subscribe existing and new followers with force=True, meaning all will have the same subtypes - with self.assertQueryCount(__system__=4, employee=4): + with self.assertQueryCount(admin=4, employee=4): rec.message_subscribe( partner_ids=pids, subtype_ids=subtype_ids @@ -944,7 +944,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(rec1.message_partner_ids, self.env.user.partner_id | self.user_portal.partner_id | self.partners) @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_tracking_assignation(self): """ Assignation performance test on already-created record """ @@ -956,7 +956,7 @@ class TestMailComplexPerformance(BaseMailPerformance): }) rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(__system__=25, employee=26): + with self.assertQueryCount(admin=26, employee=26): rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) # write tracking message @@ -968,7 +968,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(len(rec1.message_ids), 2) @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_tracking_subscription_create(self): """ Creation performance test involving auto subscription, assignation, tracking with subtype and template send. """ @@ -976,7 +976,7 @@ class TestMailComplexPerformance(BaseMailPerformance): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(__system__=57, employee=58): # tm 57/58 + with self.assertQueryCount(admin=58, employee=58): # tm 58/58 rec = self.env['mail.test.ticket'].create({ 'name': 'Test', 'container_id': container_id, @@ -992,7 +992,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(len(rec1.message_ids), 1) @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_tracking_subscription_subtype(self): """ Write performance test involving auto subscription, tracking with subtype """ @@ -1005,7 +1005,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) self.assertEqual(len(rec1.message_ids), 1) - with self.assertQueryCount(__system__=36, employee=36): + with self.assertQueryCount(admin=36, employee=36): rec.write({ 'name': 'Test2', 'container_id': self.container.id, @@ -1021,7 +1021,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(len(rec1.message_ids), 2) @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_tracking_subscription_write(self): """ Write performance test involving auto subscription, tracking with subtype and template send """ @@ -1042,7 +1042,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(__system__=36, employee=36): + with self.assertQueryCount(admin=36, employee=36): rec.write({ 'name': 'Test2', 'container_id': container_id, @@ -1059,7 +1059,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(len(rec1.message_ids), 2) @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - @users('__system__', 'employee') + @users('admin', 'employee') @warmup def test_complex_tracking_template(self): """ Write performance test involving assignation, tracking with template """ @@ -1075,7 +1075,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(__system__=20, employee=21): + with self.assertQueryCount(admin=21, employee=21): rec.write({ 'name': 'Test2', 'customer_id': customer_id, From e2c659eb0c52914c91eac25b26aa3722ae0747b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 19 Jan 2023 16:34:41 +0000 Subject: [PATCH 08/32] [IMP] mail: make message_notify process attachments Method 'message_notify' is a simplified version of 'message_post' to use to notify people of something, without posting on a document. It creates a "user notification", a specific kind of message. In this commit we make 'message_notify' process attachments like posting does. That way the API is more similar between those two methods. Moreover it allows to give 'attachments' to notify. Those are then transformed into real 'attachment_ids' and converted if they come from the composer. That way the mail composer can be used in notification mode with attachments. This happens notably when using a template on a record that does not inherit from MailThread. It is a classic usage of 'message_notify' which allows notifying on documents that have no chatter, notably technical or admin oriented models. Followup of odoo/odoo#99482 Task-2710804 (Mail: Clean MailThread API) Part-of: odoo/odoo#107356 --- addons/calendar/models/calendar_attendee.py | 19 +++++++----- addons/mail/models/mail_thread.py | 34 +++++++++++++++++++-- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/addons/calendar/models/calendar_attendee.py b/addons/calendar/models/calendar_attendee.py index 0506fad587a..0f9a98c100f 100644 --- a/addons/calendar/models/calendar_attendee.py +++ b/addons/calendar/models/calendar_attendee.py @@ -113,13 +113,15 @@ class Attendee(models.Model): event_id = attendee.event_id.id ics_file = ics_files.get(event_id) - attachment_values = [] + attachment_ids = None if ics_file: - attachment_values = [ - (0, 0, {'name': 'invitation.ics', - 'mimetype': 'text/calendar', - 'datas': base64.b64encode(ics_file)}) - ] + attachment_ids = self.env['ir.attachment'].create({ + 'datas': base64.b64encode(ics_file), + 'description': 'invitation.ics', + 'mimetype': 'text/calendar', + 'name': 'invitation.ics', + }).ids + body = mail_template._render_field( 'body_html', attendee.ids, @@ -135,8 +137,9 @@ class Attendee(models.Model): subject=subject, partner_ids=attendee.partner_id.ids, email_layout_xmlid='mail.mail_notification_light', - attachment_ids=attachment_values, - force_send=force_send) + attachment_ids=attachment_ids, + force_send=force_send, + ) def do_tentative(self): """ Makes event invitation as Tentative. """ diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index f6f0351d8b5..74a4ae4a879 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2269,8 +2269,8 @@ class MailThread(models.AbstractModel): record.ids, )['value'] composer.write(update_values) - _mails_as_sudo, messages_as_sudo = composer._action_send_mail() - messages += messages_as_sudo + _mails_as_sudo, messages = composer._action_send_mail() + messages += messages else: messages += record.message_post( body=bodies[record.id], @@ -2286,6 +2286,7 @@ class MailThread(models.AbstractModel): author_id=None, email_from=None, model=False, res_id=False, subtype_xmlid=None, subtype_id=False, partner_ids=False, + attachments=None, attachment_ids=None, **kwargs): """ Shortcut allowing to notify partners of messages that should not be displayed on a document. It pushes notifications on inbox or by email @@ -2311,7 +2312,13 @@ class MailThread(models.AbstractModel): :param int subtype_id: subtype_id of the message, used mainly for followers notification mechanism; :param list(int) partner_ids: partner_ids to notify in addition to partners - computed based on subtype / followers matching; + computed based on subtype / followers matching; + :param list(tuple(str,str), tuple(str,str, dict)) attachments : list of attachment + tuples in the form ``(name,content)`` or ``(name,content, info)`` where content + is NOT base64 encoded; + :param list attachment_ids: list of existing attachments to link to this message + Should not be a list of commands. Attachment records attached to mail + composer will be attached to the related document. Extra keyword arguments will be used either * as default column values for the new mail.message record if they match @@ -2331,6 +2338,23 @@ class MailThread(models.AbstractModel): set(kwargs.keys()), forbidden_names={'message_id', 'message_type', 'parent_id'} ) + if attachments: + # attachments should be a list (or tuples) of 3-elements list (or tuple) + format_error = not tools.is_list_of(attachments, list) and not tools.is_list_of(attachments, tuple) + if not format_error: + format_error = not all(len(attachment) in {2, 3} for attachment in attachments) + if format_error: + raise ValueError( + _('Notification should receive attachments as a list of list or tuples (received %(aids)s)', + aids=repr(attachment_ids), + ) + ) + if attachment_ids and not tools.is_list_of(attachment_ids, int): + raise ValueError( + _('Notification should receive attachments records as a list of IDs (received %(aids)s)', + aids=repr(attachment_ids), + ) + ) if not tools.is_list_of(partner_ids, int): raise ValueError( _('Notification should receive partners given as a list of IDs (received %(pids)s)', @@ -2379,6 +2403,10 @@ class MailThread(models.AbstractModel): if 'reply_to' not in msg_values: msg_values['reply_to'] = self._notify_get_reply_to(default=email_from)[self.id if self else False] + msg_values.update( + self._process_attachments_for_post(attachments, attachment_ids, msg_values) + ) # attachement_ids, body + new_message = self._message_create([msg_values]) self._fallback_lang()._notify_thread(new_message, msg_values, **notif_kwargs) return new_message From 17e6cd1e14d9be24fcabdc8da3ccfde7831132e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 27 Jan 2023 08:05:02 +0000 Subject: [PATCH 09/32] [IMP] mail: add a notification parameter to allow notifying the author Currently notification process called notably by message_{post,notify} does not notify message's author by default. Rationale is that as he typed the message he is already aware of it. However in some cases we want to skip this step, notably when messages are generated in the name of the author and when we want him to be notified if he should. This is currently controllable through a context key 'mail_notify_author'. In this commit we add an explicit parameter propagated through the notify process, allowing to remove some context key usage and instead use real parameters. Note that all context keys usage cannot be removed as it is not always possible to propagate this parameter. Followup of odoo#99482 Task-2710804 (Mail: Clean MailThread API) Part-of: odoo/odoo#107356 --- .../data_recycle/models/data_recycle_model.py | 3 +- addons/mail/models/mail_thread.py | 9 +++- addons/mail_group/models/mail_group.py | 3 +- addons/purchase/models/purchase.py | 4 +- addons/sale/models/sale_order.py | 4 +- addons/test_mail/tests/test_message_post.py | 43 +++++++++++++++++++ 6 files changed, 59 insertions(+), 7 deletions(-) diff --git a/addons/data_recycle/models/data_recycle_model.py b/addons/data_recycle/models/data_recycle_model.py index bfcc1630999..6c91da62f27 100644 --- a/addons/data_recycle/models/data_recycle_model.py +++ b/addons/data_recycle/models/data_recycle_model.py @@ -175,7 +175,7 @@ class DataRecycleModel(models.Model): partner_ids = self.notify_user_ids.partner_id.ids if records_count else [] if partner_ids: menu_id = self.env.ref('data_recycle.menu_data_cleaning_root').id - self.env['mail.thread'].with_context(mail_notify_author=True).message_notify( + self.env['mail.thread'].message_notify( body=self.env['ir.qweb']._render( 'data_recycle.notification', { @@ -186,6 +186,7 @@ class DataRecycleModel(models.Model): } ), model=self._name, + notify_author=True, partner_ids=partner_ids, res_id=self.id, subject=_('Data to Recycle'), diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 74a4ae4a879..4ea545361f7 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2699,6 +2699,7 @@ class MailThread(models.AbstractModel): 'force_send', 'mail_auto_delete', 'model_description', + 'notify_author', 'resend_existing', 'scheduled_date', 'send_after_commit', @@ -3187,6 +3188,10 @@ class MailThread(models.AbstractModel): methods. See those methods for more details about supported parameters. Specific kwargs used in this method: + * ``notify_author``: allows to notify the author, which is False by + default as we don't want people to receive their own content. It is + used notably when impersonating partners or having automated + notifications send by current user, targeting current user; * ``skip_existing``: check existing notifications and skip them in order to avoid having several notifications / partner as it would make constraints crash. This is disabled by default to optimize speed; @@ -3213,9 +3218,11 @@ class MailThread(models.AbstractModel): if not res: return recipients_data + # notify author of its own messages, False by default + notify_author = kwargs.get('notify_author') or self.env.context.get('mail_notify_author') author_id = msg_vals.get('author_id') or message.author_id.id for pid, pdata in res.items(): - if pid and pid == author_id and not self.env.context.get('mail_notify_author'): # do not notify the author of its own messages + if pid and not notify_author and pid == author_id: continue if pdata['active'] is False: continue diff --git a/addons/mail_group/models/mail_group.py b/addons/mail_group/models/mail_group.py index 5f3bbde3901..d6bd7bd6345 100644 --- a/addons/mail_group/models/mail_group.py +++ b/addons/mail_group/models/mail_group.py @@ -495,7 +495,7 @@ class MailGroup(models.Model): for group in groups: moderators_to_notify = group.moderator_ids - MailThread = self.env['mail.thread'].with_context(mail_notify_author=True) + MailThread = self.env['mail.thread'] for moderator in moderators_to_notify: body = self.env['ir.qweb']._render('mail_group.mail_group_notify_moderation', { 'moderator': moderator, @@ -508,6 +508,7 @@ class MailGroup(models.Model): body=body, email_from=email_from, model='mail.group', + notify_author=True, res_id=group.id, ) diff --git a/addons/purchase/models/purchase.py b/addons/purchase/models/purchase.py index 62703c13c68..602c50fa92a 100644 --- a/addons/purchase/models/purchase.py +++ b/addons/purchase/models/purchase.py @@ -334,8 +334,8 @@ class PurchaseOrder(models.Model): if self.env.context.get('mark_rfq_as_sent'): self.filtered(lambda o: o.state == 'draft').write({'state': 'sent'}) po_ctx = {'mail_post_autofollow': self.env.context.get('mail_post_autofollow', True)} - if self.env.context.get('mark_rfq_as_sent'): - po_ctx['mail_notify_author'] = self.env.user.partner_id.id in (kwargs.get('partner_ids') or []) + if self.env.context.get('mark_rfq_as_sent') and 'notify_author' not in kwargs: + kwargs['notify_author'] = self.env.user.partner_id.id in (kwargs.get('partner_ids') or []) return super(PurchaseOrder, self.with_context(**po_ctx)).message_post(**kwargs) def _notify_get_recipients_groups(self, msg_vals=None): diff --git a/addons/sale/models/sale_order.py b/addons/sale/models/sale_order.py index d8e2b3ca313..9587540c24e 100644 --- a/addons/sale/models/sale_order.py +++ b/addons/sale/models/sale_order.py @@ -1232,8 +1232,8 @@ class SaleOrder(models.Model): if self.env.context.get('mark_so_as_sent'): self.filtered(lambda o: o.state == 'draft').with_context(tracking_disable=True).write({'state': 'sent'}) so_ctx = {'mail_post_autofollow': self.env.context.get('mail_post_autofollow', True)} - if self.env.context.get('mark_so_as_sent'): - so_ctx['mail_notify_author'] = self.env.user.partner_id.id in (kwargs.get('partner_ids') or []) + if self.env.context.get('mark_so_as_sent') and 'mail_notify_author' not in kwargs: + kwargs['notify_author'] = self.env.user.partner_id.id in (kwargs.get('partner_ids') or []) return super(SaleOrder, self.with_context(**so_ctx)).message_post(**kwargs) def _notify_get_recipients_groups(self, msg_vals=None): diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index ec75ffb11fa..10d7fd9905f 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -313,6 +313,49 @@ class TestMessageNotify(TestMessagePostCommon): partner_mail_body = partner_mails[0].get('body') self.assertNotIn('/mail/view?model=', partner_mail_body, 'The email sent to customer should not contain an access link') + @users('employee') + @mute_logger('odoo.addons.mail.models.mail_mail') + def test_notify_author(self): + """ Author is not added in notified people by default, unless asked to + using the 'notify_author' parameter or context key. """ + test_record = self.env['mail.test.simple'].browse(self.test_record.ids) + + with self.mock_mail_gateway(): + new_notification = test_record.message_notify( + body='

You have received a notification

', + partner_ids=(self.partner_1 + self.partner_employee).ids, + subject='This should be a subject', + ) + + self.assertEqual(new_notification.notified_partner_ids, self.partner_1) + + with self.mock_mail_gateway(): + new_notification = test_record.message_notify( + body='

You have received a notification

', + notify_author=True, + partner_ids=(self.partner_1 + self.partner_employee).ids, + subject='This should be a subject', + ) + + self.assertEqual( + new_notification.notified_partner_ids, + self.partner_1 + self.partner_employee, + 'Notify: notify_author parameter skips the author restriction' + ) + + with self.mock_mail_gateway(): + new_notification = test_record.with_context(mail_notify_author=True).message_notify( + body='

You have received a notification

', + partner_ids=(self.partner_1 + self.partner_employee).ids, + subject='This should be a subject', + ) + + self.assertEqual( + new_notification.notified_partner_ids, + self.partner_1 + self.partner_employee, + 'Notify: mail_notify_author context key skips the author restriction' + ) + @users('employee') def test_notify_batch(self): """ Test notify in batch. Currently not supported. """ From 8fb59b55d9ffa3cdcd81508b5c522cef19da0027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 27 Jan 2023 10:52:50 +0000 Subject: [PATCH 10/32] [IMP] mail: improve 'real author' detection when posting a message When posting a message, author is added in followers in order to receive answers. They also do not receive its own messages, to avoid being notified of something they just posted. However the real author is not always the message author. In this commit we better define this author * when current user is active, real author is the current user. It means posting on behalf of someone will send answers to you, not to the spoofed partner; * when current user is inactive, real author is the message author. An inactive current user means the post was probably done as sudo, aka in the mailgateway or frontend. In that case we don't want to add odoobot in followers, but the actual message author; Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#107356 --- addons/mail/models/mail_thread.py | 34 ++++++++++-- addons/test_mail/tests/test_message_post.py | 55 +++++++++++++++++++ .../tests/test_website_blog_flow.py | 2 +- 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 4ea545361f7..21a5f9d77c3 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1927,9 +1927,25 @@ class MailThread(models.AbstractModel): ) # attachement_ids, body new_message = self._message_create([msg_values]) - if msg_values['author_id'] and msg_values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): - if self.env['res.partner'].browse(msg_values['author_id']).active: # we dont want to add odoobot/inactive as a follower - self._message_subscribe(partner_ids=[msg_values['author_id']]) + # subscribe author(s) so that they receive answers; do it only when it is + # a manual post by the author (aka not a system notification, not a message + # posted 'in behalf of', and if still active). + author_subscribe = (not self._context.get('mail_create_nosubscribe') and + msg_values['message_type'] != 'notification') + if author_subscribe: + real_author_id = False + # if current user is active, they are the one doing the action and should + # be notified of answers. If they are inactive they are posting on behalf + # of someone else (a custom, mailgateway, ...) and the real author is the + # message author + if self.env.user.active: + real_author_id = self.env.user.partner_id.id + elif msg_values['author_id']: + author = self.env['res.partner'].browse(msg_values['author_id']) + if author.active: + real_author_id = author.id + if real_author_id: + self._message_subscribe(partner_ids=[real_author_id]) self._message_post_after_hook(new_message, msg_values) self._notify_thread(new_message, msg_values, **notif_kwargs) @@ -3220,9 +3236,17 @@ class MailThread(models.AbstractModel): # notify author of its own messages, False by default notify_author = kwargs.get('notify_author') or self.env.context.get('mail_notify_author') - author_id = msg_vals.get('author_id') or message.author_id.id + real_author_id = False + if not notify_author: + if self.env.user.active: + real_author_id = self.env.user.partner_id.id + elif msg_vals.get('author_id'): + real_author_id = msg_vals['author_id'] + else: + real_author_id = message.author_id.id + for pid, pdata in res.items(): - if pid and not notify_author and pid == author_id: + if pid and pid == real_author_id: continue if pdata['active'] is False: continue diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 10d7fd9905f..d4e7994173b 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -659,6 +659,61 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): partner_ids=self.partner_portal.ids, ) + @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') + @users('employee') + def test_message_post_author(self): + """ Test author recognition """ + test_record = self.test_record.with_env(self.env) + + # when a user spoofs the author: the actual author is the current user + # and not the message author + with self.assertSinglePostNotifications( + [{'partner': self.partner_admin, 'type': 'email'}], + {'content': 'Body'} + ): + new_message = test_record.message_post( + author_id=self.partner_employee_2.id, + body='Body', + message_type='comment', + subtype_xmlid='mail.mt_comment', + partner_ids=[self.partner_admin.id], + ) + + self.assertMessageFields( + new_message, + {'author_id': self.partner_employee_2, + 'email_from': formataddr((self.partner_employee_2.name, self.partner_employee_2.email_normalized)), + 'message_type': 'comment', + 'notified_partner_ids': self.partner_admin, + 'subtype_id': self.env.ref('mail.mt_comment'), + } + ) + self.assertEqual(test_record.message_partner_ids, self.partner_employee, + 'Real author is added in followers, not message author') + + # should be skipped with notifications + test_record.message_unsubscribe(partner_ids=self.partner_employee.ids) + _new_message = test_record.message_post( + author_id=self.partner_employee_2.id, + body='Body', + message_type='notification', + subtype_xmlid='mail.mt_comment', + partner_ids=[self.partner_admin.id], + ) + self.assertFalse(test_record.message_partner_ids, 'Notification should not add author in followers') + + # inactive users are not considered as authors + self.env.user.with_user(self.user_admin).active = False + _new_message = test_record.message_post( + author_id=self.partner_employee_2.id, + body='Body', + message_type='comment', + subtype_xmlid='mail.mt_comment', + partner_ids=[self.partner_admin.id], + ) + self.assertEqual(test_record.message_partner_ids, self.partner_employee_2, + 'Author is the message author when user is inactive, and shoud be added in followers') + @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') @users('employee') def test_message_post_defaults(self): diff --git a/addons/website_blog/tests/test_website_blog_flow.py b/addons/website_blog/tests/test_website_blog_flow.py index 29276e935cd..0bf8859fade 100644 --- a/addons/website_blog/tests/test_website_blog_flow.py +++ b/addons/website_blog/tests/test_website_blog_flow.py @@ -56,7 +56,7 @@ class TestWebsiteBlogFlow(TestWebsiteBlogCommon): 'website_blog: peuple following a blog should be notified of a published post') # Armand posts a message -> becomes follower - self.test_blog_post.sudo().message_post( + self.test_blog_post.with_user(self.user_employee).message_post( body='Armande BlogUser Commented', message_type='comment', author_id=self.user_employee.partner_id.id, From 5883cd9c1608052e42ecb28c4bb40525a13b7f81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 27 Jan 2023 15:36:02 +0000 Subject: [PATCH 11/32] [IMP] mail: allow to read message user created In some cases, a given user may create a message in behalf of other people. This happens for example * when calling message_notify to send notifications in a given flow, while setting author_id to someone else (e.g. appointment flow who send invite as notification, using the event responsible as author); * when using templates with email_from set which may update the author_id to match the email_from; Notably with message_notify the current user may create a message he cannot read afterwards, as he is not the author (as in 'author_id' field value) and cannot read the message even with access on related record (as it is a 'user_notification'). In this commit we relax a bit the 'read' rule on mail.message to allow reading a message the user created. Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#107356 --- addons/mail/models/mail_message.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index e7c97a9d361..03456375469 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -346,6 +346,7 @@ class Message(models.Model): """ Access rules of mail.message: - read: if - author_id == pid, uid is the author OR + - create_uid == uid, uid is the creator OR - uid is in the recipients (partner_ids) OR - uid has been notified (needaction) OR - uid have read access to the related document if model, res_id @@ -399,12 +400,13 @@ class Message(models.Model): # Read mail_message.ids to have their values message_values = dict((message_id, {}) for message_id in self.ids) - self.flush_recordset(['model', 'res_id', 'author_id', 'parent_id', 'message_type', 'partner_ids']) + self.flush_recordset(['model', 'res_id', 'author_id', 'create_uid', 'parent_id', 'message_type', 'partner_ids']) self.env['mail.notification'].flush_model(['mail_message_id', 'res_partner_id']) if operation == 'read': self._cr.execute(""" - SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, m.parent_id, + SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, m.create_uid, + m.parent_id, COALESCE(partner_rel.res_partner_id, needaction_rel.res_partner_id), m.message_type as message_type FROM "%s" m @@ -413,11 +415,12 @@ class Message(models.Model): LEFT JOIN "mail_notification" needaction_rel ON needaction_rel.mail_message_id = m.id AND needaction_rel.res_partner_id = %%(pid)s WHERE m.id = ANY (%%(ids)s)""" % self._table, dict(pid=self.env.user.partner_id.id, ids=self.ids)) - for mid, rmod, rid, author_id, parent_id, partner_id, message_type in self._cr.fetchall(): + for mid, rmod, rid, author_id, create_uid, parent_id, partner_id, message_type in self._cr.fetchall(): message_values[mid] = { 'model': rmod, 'res_id': rid, 'author_id': author_id, + 'create_uid': create_uid, 'parent_id': parent_id, 'notified': any((message_values[mid].get('notified'), partner_id)), 'message_type': message_type, @@ -459,7 +462,14 @@ class Message(models.Model): author_ids = [] if operation == 'read': author_ids = [mid for mid, message in message_values.items() - if message.get('author_id') and message.get('author_id') == self.env.user.partner_id.id] + if ( + message.get('author_id') and + message.get('author_id') == self.env.user.partner_id.id + ) or ( + message.get('create_uid') and + message.get('create_uid') == self.env.uid + ) + ] elif operation == 'write': author_ids = [mid for mid, message in message_values.items() if message.get('author_id') == self.env.user.partner_id.id] elif operation == 'create': From 69209ae4bf22dbe55c3ec14ea734963d1d4b6d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 18 Jan 2023 14:42:16 +0000 Subject: [PATCH 12/32] [MOV] (mass_)mail(_ing): move 'model_is_thread' field to mail This field will be used to remove low-level asserts using existence of 'message_post' on a given model instead of correctly checking the inheritance on mail.thread. This is going to increase a bit query counters but those queries should be fast and have no real impact on performances. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 8 ++++++++ addons/mail/wizard/mail_compose_message_views.xml | 1 + addons/mass_mailing/wizard/mail_compose_message.py | 9 +-------- .../mass_mailing/wizard/mail_compose_message_views.xml | 1 - 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 9ffbfd42276..b32c39ed554 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -149,6 +149,7 @@ class MailComposer(models.TransientModel): composition_batch = fields.Boolean( 'Batch composition', compute='_compute_composition_batch') # more than 1 record (raw source) model = fields.Char('Related Document Model') + model_is_thread = fields.Boolean('Thread-Enabled', compute='_compute_model_is_thread') res_ids = fields.Text('Related Document IDs') res_domain = fields.Text('Active domain') res_domain_user_id = fields.Many2one( @@ -227,6 +228,13 @@ class MailComposer(models.TransientModel): res_ids = composer._evaluate_res_ids() composer.composition_batch = len(res_ids) > 1 if res_ids else False + @api.depends('model') + def _compute_model_is_thread(self): + """ Determine if model is thread enabled. """ + for composer in self: + model = self.env['ir.model']._get(composer.model) + composer.model_is_thread = model.is_mail_thread + @api.depends('subtype_id') def _compute_subtype_is_log(self): """ In comment mode, tells whether the subtype is a note. Subtype has diff --git a/addons/mail/wizard/mail_compose_message_views.xml b/addons/mail/wizard/mail_compose_message_views.xml index 0419c1ab393..2ab4ea3cc49 100644 --- a/addons/mail/wizard/mail_compose_message_views.xml +++ b/addons/mail/wizard/mail_compose_message_views.xml @@ -19,6 +19,7 @@ + diff --git a/addons/mass_mailing/wizard/mail_compose_message.py b/addons/mass_mailing/wizard/mail_compose_message.py index c522ee8d07a..70bc9d72c40 100644 --- a/addons/mass_mailing/wizard/mail_compose_message.py +++ b/addons/mass_mailing/wizard/mail_compose_message.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import api, fields, models +from odoo import fields, models class MailComposeMessage(models.TransientModel): @@ -11,13 +11,6 @@ class MailComposeMessage(models.TransientModel): campaign_id = fields.Many2one('utm.campaign', string='Mass Mailing Campaign', ondelete='set null') mass_mailing_name = fields.Char(string='Mass Mailing Name', help='If set, a mass mailing will be created so that you can track its results in the Email Marketing app.') mailing_list_ids = fields.Many2many('mailing.list', string='Mailing List') - model_is_thread = fields.Boolean(compute='_compute_model_is_thread') - - @api.depends('model') - def _compute_model_is_thread(self): - for composer in self: - model = self.env['ir.model']._get(composer.model) - composer.model_is_thread = model.is_mail_thread def _action_send_mail(self, auto_commit=False): """ Override to generate the mass mailing in case only the name was diff --git a/addons/mass_mailing/wizard/mail_compose_message_views.xml b/addons/mass_mailing/wizard/mail_compose_message_views.xml index 3f42753d1a9..b4111a23f66 100644 --- a/addons/mass_mailing/wizard/mail_compose_message_views.xml +++ b/addons/mass_mailing/wizard/mail_compose_message_views.xml @@ -8,7 +8,6 @@ - Date: Thu, 19 Jan 2023 16:35:05 +0000 Subject: [PATCH 13/32] [FIX] mail: better support using composer on no thread record When using the mail composer in comment mode on models that do not inherit from mail.thread, the post is transformed into notification process. Instead of calling 'message_post' on the records (which would crash) 'message_notify' is called, using MailThread as abstract class (which creates notifications without having to inherit from mail.thread). However some parameters from the composer are not supported when calling 'message_notify'. This commit fixes it and adds tests accordingly. Followup of odoo/odoo#99482 Task-2710804 (Mail: Clean MailThread API) Part-of: odoo/odoo#107356 --- addons/mail/tests/common.py | 2 +- addons/mail/wizard/mail_compose_message.py | 2 + .../tests/test_mail_thread_internals.py | 63 ++++++++++++++++++- 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index eaa5d21bb26..dca9d428702 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -1072,7 +1072,7 @@ class MailCommon(common.TransactionCase, MailCase): return cls.user_portal @classmethod - def _create_records_for_batch(cls, model, count, additional_values=None, prefix=None): + def _create_records_for_batch(cls, model, count, additional_values=None, prefix=''): additional_values = additional_values or {} records = cls.env[model] partners = cls.env['res.partner'] diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index b32c39ed554..31532d97386 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -498,6 +498,8 @@ class MailComposer(models.TransientModel): messages = self.env['mail.message'] for res_id, post_values in post_values_all.items(): if ActiveModel._name == 'mail.thread': + post_values.pop('message_type') # forced to user_notification + post_values.pop('parent_id', False) # not supported in notify if self.model: post_values['model'] = self.model post_values['res_id'] = res_id diff --git a/addons/test_mail/tests/test_mail_thread_internals.py b/addons/test_mail/tests/test_mail_thread_internals.py index 56122681bac..36d254e3868 100644 --- a/addons/test_mail/tests/test_mail_thread_internals.py +++ b/addons/test_mail/tests/test_mail_thread_internals.py @@ -8,7 +8,7 @@ from werkzeug.urls import url_parse, url_decode from odoo import exceptions from odoo.addons.test_mail.models.test_mail_models import MailTestSimple from odoo.addons.test_mail.tests.common import TestMailCommon, TestRecipients -from odoo.tests.common import tagged, HttpCase, users +from odoo.tests.common import tagged, Form, HttpCase, users from odoo.tools import mute_logger @@ -473,6 +473,67 @@ class TestNoThread(TestMailCommon, TestRecipients): subject='Test Notify', ) + @users('employee') + def test_message_notify_composer(self): + """ Test comment mode on composer which triggers a notify when model + does not inherit from mail thread. """ + test_records, _test_partners = self._create_records_for_batch('mail.test.nothread', 2) + + test_reports = self.env['ir.actions.report'].sudo().create([ + { + 'name': 'Test Report on Mail Test Ticket', + 'model': test_records._name, + 'print_report_name': "'TestReport for %s' % object.name", + 'report_type': 'qweb-pdf', + 'report_name': 'test_mail.mail_test_ticket_test_template', + }, { + 'name': 'Test Report 2 on Mail Test Ticket', + 'model': test_records._name, + 'print_report_name': "'TestReport2 for %s' % object.name", + 'report_type': 'qweb-pdf', + 'report_name': 'test_mail.mail_test_ticket_test_template_2', + } + ]) + test_template = self.env['mail.template'].create({ + 'auto_delete': True, + 'body_html': '

TemplateBody

', + 'email_from': '{{ (user.email_formatted) }}', + 'email_to': '', + 'mail_server_id': self.mail_server_domain.id, + 'partner_to': '{{ object.customer_id.id if object.customer_id else "" }}', + 'name': 'TestTemplate', + 'model_id': self.env['ir.model']._get(test_records._name).id, + 'reply_to': '{{ ctx.get("custom_reply_to") or "info@test.example.com" }}', + 'report_template_ids': [(6, 0, test_reports.ids)], + 'scheduled_date': '{{ (object.create_date or datetime.datetime(2022, 12, 26, 18, 0, 0)) + datetime.timedelta(days=2) }}', + 'subject': 'TemplateSubject {{ object.name }}', + }) + attachment_data = self._generate_attachments_data(2, test_template._name, test_template.id) + test_template.write({'attachment_ids': [(0, 0, a) for a in attachment_data]}) + + ctx = { + 'default_composition_mode': 'comment', + 'default_model': test_records._name, + 'default_res_domain': [('id', 'in', test_records.ids)], + 'default_template_id': test_template.id, + } + # open a composer and run it in comment mode + composer_form = Form(self.env['mail.compose.message'].with_context(ctx)) + composer = composer_form.save() + + with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app(): + _, messages = composer._action_send_mail() + + self.assertEqual(len(messages), 2) + for record, message in zip(test_records, messages): + self.assertEqual( + sorted(message.mapped('attachment_ids.name')), + sorted(['AttFileName_00.txt', 'AttFileName_01.txt', + f'TestReport2 for {record.name}.html', + f'TestReport for {record.name}.html']) + ) + self.assertEqual(len(messages.attachment_ids), 8, 'No attachments should be shared') + @users('employee') def test_message_notify_norecord(self): """ Test notifying on no record, just using the abstract model itself. """ From c349e8d929d249981251e41af50b0141648011e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 7 Dec 2022 17:53:32 +0000 Subject: [PATCH 14/32] [REF] mail: move composer 'record_name' field from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute field ``record_name`` in compute method. Computation is coming either from parent message, either from the record's display name in monorecord comment mode. In batch mode it makes no sense to compute a single record name. In email mode it is not used anyway. Computing it in composer allows to benefit from its computation to give it to the message created afterwards during message_post. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 44 +++++++++++++++----- addons/test_mail/tests/test_mail_composer.py | 9 ++-- addons/test_mail/tests/test_performance.py | 18 ++++---- 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 31532d97386..07433cad0d0 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -89,7 +89,7 @@ class MailComposer(models.TransientModel): else: result['res_ids'] = False # record / parent based computation - if result.get('composition_mode') == 'comment' and (set(fields_list) & {'model', 'res_ids', 'partner_ids', 'record_name', 'subject'}): + if result.get('composition_mode') == 'comment' and (set(fields_list) & {'model', 'res_ids', 'partner_ids', 'subject'}): result.update(self.get_record_data(result)) # threading support check: 'update' requires to use message_update/post @@ -155,7 +155,9 @@ class MailComposer(models.TransientModel): res_domain_user_id = fields.Many2one( 'res.users', string='Responsible', help='Used as context used to evaluate composer domain') - record_name = fields.Char('Message Record Name') + record_name = fields.Char( + 'Record Name', + compute='_compute_record_name', readonly=False, store=True) # useful only in monorecord comment mode # characteristics message_type = fields.Selection([ ('comment', 'Comment'), @@ -235,6 +237,33 @@ class MailComposer(models.TransientModel): model = self.env['ir.model']._get(composer.model) composer.model_is_thread = model.is_mail_thread + @api.depends('composition_mode', 'model', 'parent_id', 'res_domain', 'res_ids') + def _compute_record_name(self): + """ Computation is coming either from parent message, either from the + record's display name in monorecord comment mode. + + In batch mode it makes no sense to compute a single record name. In + email mode it is not used anyway. """ + toreset = self.filtered( + lambda comp: comp.record_name + and (comp.composition_mode != 'comment' or comp.composition_batch) + ) + if toreset: + toreset.record_name = False + + toupdate = self.filtered( + lambda comp: not comp.record_name + and comp.composition_mode == 'comment' + and not comp.composition_batch + ) + for composer in toupdate: + if composer.parent_id.record_name: + composer.record_name = composer.parent_id.record_name + continue + res_ids = composer._evaluate_res_ids() + if composer.model and len(res_ids) == 1: + composer.record_name = self.env[composer.model].browse(res_ids).display_name + @api.depends('subtype_id') def _compute_subtype_is_log(self): """ In comment mode, tells whether the subtype is a note. Subtype has @@ -390,7 +419,7 @@ class MailComposer(models.TransientModel): wizard when sending an email related a previous email (parent_id) or a document (model, res_id). This is based on previously computed default values. """ - result, record_name, subject = {}, False, False + result, subject = {}, False model = values.get('model') res_ids = self._parse_res_ids(values['res_ids']) if values.get('res_ids') else [] if values.get('parent_id'): @@ -402,21 +431,16 @@ class MailComposer(models.TransientModel): res_ids = [parent.res_id] result['res_ids'] = res_ids result['partner_ids'] = values.get('partner_ids', list()) + parent.partner_ids.ids - record_name = parent.record_name subject = tools.ustr(parent.subject or '') - if (not subject or not record_name) and model and res_ids and len(res_ids) == 1: + if not subject and model and res_ids and len(res_ids) == 1: record = self.env[model].browse(res_ids[0]) - if not record_name: - record_name = record.display_name if not subject: subject = record._message_compute_subject() elif model and len(res_ids) == 1: record = self.env[model].browse(res_ids[0]) - record_name = record.display_name or '' subject = record._message_compute_subject() if values.get('parent_id') or len(res_ids) == 1: # to be cleanup when moving to computed fields - result['record_name'] = record_name result['subject'] = subject return result @@ -612,7 +636,7 @@ class MailComposer(models.TransientModel): STA - 'message_type', STA - 'parent_id', DYN - 'partner_ids', - STA - 'record_name', (to check, single post or False in email) + STA - 'record_name', DYN - 'reply_to', STA - 'reply_to_force_new', DYN - 'scheduled_date', diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 0382c47fa59..b21b05714e0 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -819,16 +819,14 @@ class TestComposerInternals(TestMailComposer): composer._onchange_template_id_wrapper() # values are reset with default_get call, if it returns value - # (aka subject for comment mode), and not record_name because - # it was forgotten probably + # (aka subject for comment mode) if composition_mode == 'comment' and not batch: self.assertFalse(composer.body) # self.assertFalse(composer.mail_server_id.id) self.assertEqual(composer.mail_server_id, self.mail_server_global, 'TODO: Values are kept (should be reset ?)') - # self.assertEqual(composer.record_name, self.test_record.name) self.assertEqual(composer.record_name, 'Manual update', - 'TODO: Reset not called') + 'MailComposer: record name does not depend on template') # self.assertFalse(composer.scheduled_date) self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}', 'TODO: Values are kept (should be reset ?)') @@ -840,9 +838,8 @@ class TestComposerInternals(TestMailComposer): # self.assertFalse(composer.mail_server_id.id) self.assertEqual(composer.mail_server_id, self.mail_server_global, 'TODO: Values are kept (should be reset ?)') - # self.assertFalse(composer.record_name) self.assertEqual(composer.record_name, 'Manual update', - 'TODO: Reset not called') + 'MailComposer: record name does not depend on template') # self.assertFalse(composer.scheduled_date) self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}', 'TODO: Values are kept (should be reset ?)') diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 84df227843f..e159c0de891 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -337,7 +337,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_mail_composer(self): test_record, _test_template = self._create_test_records() customer_id = self.customer.id - with self.assertQueryCount(admin=2, employee=2): + with self.assertQueryCount(admin=4, employee=4): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -357,7 +357,7 @@ class TestMailAPIPerformance(BaseMailPerformance): test_record, _test_template = self._create_test_records() customer = self.env['res.partner'].browse(self.customer.ids) attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals) - with self.assertQueryCount(admin=3, employee=3): + with self.assertQueryCount(admin=5, employee=5): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -406,7 +406,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_mail_composer_mass_w_template(self): _partners, test_records, test_template = self._create_test_records_for_batch() - with self.assertQueryCount(admin=3, employee=3): + with self.assertQueryCount(admin=4, employee=4): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'mass_mail', 'default_model': test_records._name, @@ -426,7 +426,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_mail_composer_nodelete(self): test_record, _test_template = self._create_test_records() customer_id = self.customer.id - with self.assertQueryCount(admin=2, employee=2): + with self.assertQueryCount(admin=4, employee=4): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -447,7 +447,7 @@ class TestMailAPIPerformance(BaseMailPerformance): test_record, test_template = self._create_test_records() test_template.write({'attachment_ids': [(5, 0)]}) - with self.assertQueryCount(admin=24, employee=24): # tm 14/14 / com 23/23 + with self.assertQueryCount(admin=25, employee=25): # tm 15/15 / com 24/24 composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -472,7 +472,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_mail_composer_w_template_attachments(self): test_record, test_template = self._create_test_records() - with self.assertQueryCount(admin=25, employee=25): # tm 15/15 / com 24/24 + with self.assertQueryCount(admin=26, employee=26): # tm 16/16 / com 25/25 composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -865,7 +865,7 @@ class TestMailComplexPerformance(BaseMailPerformance): template = self.env.ref('test_mail.mail_test_container_tpl') # about 20 (19 ?) queries per additional customer group - with self.assertQueryCount(admin=44, employee=44): + with self.assertQueryCount(admin=45, employee=45): record.message_post_with_source( template, message_type='comment', @@ -881,7 +881,7 @@ class TestMailComplexPerformance(BaseMailPerformance): def test_complex_message_post_view(self): _partners, test_records, test_template = self._create_test_records_for_batch() - with self.assertQueryCount(admin=3, employee=3): + with self.assertQueryCount(admin=4, employee=4): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'mass_mail', 'default_model': test_records._name, @@ -1075,7 +1075,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(admin=21, employee=21): + with self.assertQueryCount(admin=22, employee=22): rec.write({ 'name': 'Test2', 'customer_id': customer_id, From 7ee2e2d5d3bb186042b54ee16bae6cd3cf1816ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 19 Dec 2022 14:45:12 +0000 Subject: [PATCH 15/32] [REF] mail: move composer 'auto_delete_*' fields from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute field ``auto_delete(_keep_log)`` in compute methods. For auto_delete, computation is coming either from template, either from composition mode. When having a template, its value copied. Without template it is True in comment mode to remove notification emails by default. In email mode we keep emails (backward compatibility mode). For auto_delete_keep_log, it is used only in email mode. It is used to keep the core message when unlinking sent emails. It allows to keep the message as a trace in the record's chatter. In other modes it has no use and can be set to False. When auto_delete is turned off it has no usage. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- .../tests/test_mail_performance.py | 4 +- addons/mail/wizard/mail_compose_message.py | 48 ++++++++++++------- addons/test_mail/tests/test_mail_composer.py | 30 ++++++------ addons/test_mail/tests/test_message_post.py | 2 +- 4 files changed, 48 insertions(+), 36 deletions(-) diff --git a/addons/l10n_generic_coa/tests/test_mail_performance.py b/addons/l10n_generic_coa/tests/test_mail_performance.py index 716e3d8d491..7a4d19b58b3 100644 --- a/addons/l10n_generic_coa/tests/test_mail_performance.py +++ b/addons/l10n_generic_coa/tests/test_mail_performance.py @@ -302,7 +302,7 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance): )), }, fields_values={ - 'auto_delete': False, + 'auto_delete': True, 'email_from': self.user_account_other.email_formatted, 'is_notification': True, # should keep logs by default 'mail_server_id': self.mail_server_global, @@ -406,7 +406,7 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance): )), }, fields_values={ - 'auto_delete': False, + 'auto_delete': True, 'email_from': self.user_account_other.email_formatted, 'is_notification': True, # should keep logs by default 'mail_server_id': self.mail_server_global, diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 07433cad0d0..bda09c5cc3d 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -101,9 +101,6 @@ class MailComposer(models.TransientModel): if 'create_uid' in fields_list and 'create_uid' not in result: result['create_uid'] = self.env.uid - # comment mode by default removes emails - if 'auto_delete' in fields_list and 'auto_delete' not in result and result.get('composition_mode') == 'comment': - result['auto_delete'] = True # batch post mode by default use queues for notifications if 'force_send' in fields_list and 'force_send' not in result: result['force_send'] = ( @@ -186,10 +183,13 @@ class MailComposer(models.TransientModel): 'wizard_id', 'partner_id', 'Additional Contacts', domain=_partner_ids_domain) # sending - auto_delete = fields.Boolean('Delete Emails', + auto_delete = fields.Boolean( + 'Delete Emails', + compute="_compute_auto_delete", readonly=False, store=True, 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.') auto_delete_keep_log = fields.Boolean( - 'Keep Message Copy', default=True, + 'Keep Message Copy', + compute="_compute_auto_delete_keep_log", readonly=False, store=True, help='Keep a copy of the email content if emails are removed (mass mailing only)') force_send = fields.Boolean( 'Send mailing or notifications directly') @@ -282,6 +282,31 @@ class MailComposer(models.TransientModel): for composer in self: composer.reply_to_force_new = composer.reply_to_mode == 'new' + @api.depends('composition_mode', 'template_id') + def _compute_auto_delete(self): + """ Computation is coming either from template, either from composition + mode. When having a template, its value is copied. Without template it + is True in comment mode to remove notification emails by default. In + email mode we keep emails (backward compatibility mode). """ + for composer in self: + if composer.template_id: + composer.auto_delete = composer.template_id.auto_delete + else: + composer.auto_delete = composer.composition_mode == 'comment' + + @api.depends('composition_mode', 'auto_delete') + def _compute_auto_delete_keep_log(self): + """ Keep logs is used only in email mode. It is used to keep the core + message when unlinking sent emails. It allows to keep the message as + a trace in the record's chatter. In other modes it has no use and + can be set to False. When auto_delete is turned off it has no usage. """ + toreset = self.filtered( + lambda comp: comp.composition_mode != 'mass_mail' or + not comp.auto_delete + ) + toreset.auto_delete_keep_log = False + (self - toreset).auto_delete_keep_log = True + # Overrides of mail.render.mixin @api.depends('model') def _compute_render_model(self): @@ -324,8 +349,7 @@ class MailComposer(models.TransientModel): template = self.env['mail.template'].browse(template_id) values = dict( (field, template[field]) - for field in ('auto_delete', - 'email_from', + for field in ('email_from', 'reply_to', 'scheduled_date', 'subject', @@ -346,7 +370,6 @@ class MailComposer(models.TransientModel): self.env['mail.template'].browse(template_id), template_res_ids, ('attachment_ids', - 'auto_delete', 'body_html', 'email_cc', 'email_from', @@ -380,8 +403,6 @@ class MailComposer(models.TransientModel): default_model=model, default_res_ids=res_ids ).default_get(['attachment_ids', - 'auto_delete', - 'auto_delete_keep_log', 'body', 'composition_mode', 'email_from', @@ -397,8 +418,6 @@ class MailComposer(models.TransientModel): values = dict( (key, default_values[key]) for key in ('attachment_ids', - 'auto_delete', - 'auto_delete_keep_log', 'body', 'email_from', 'mail_server_id', @@ -761,7 +780,6 @@ class MailComposer(models.TransientModel): # some fields are specific to mail or message **( { - 'auto_delete': self.auto_delete, 'body_html': bodies[res_id], 'res_id': res_id, } if email_mode else {}), @@ -775,7 +793,6 @@ class MailComposer(models.TransientModel): self.template_id, res_ids, ('attachment_ids', - 'auto_delete', 'email_to', 'email_cc', 'mail_server_id', @@ -787,9 +804,6 @@ class MailComposer(models.TransientModel): for res_id in res_ids: # remove attachments from template values as they should not be rendered template_values[res_id].pop('attachment_ids', None) - # auto_delete is not supported in batch post, only in mass mail - if not email_mode: - template_values[res_id].pop('auto_delete', False) mail_values_all[res_id].update(template_values[res_id]) # Handle recipients. Without template, if no partner_ids is given, update diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index b21b05714e0..92025741e3e 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -128,7 +128,7 @@ class TestComposerForm(TestMailComposer): self._get_web_context(self.test_record, add_web=True) )) self.assertTrue(composer_form.auto_delete, 'MailComposer: comment mode should remove notification emails by default') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertFalse(composer_form.body) self.assertFalse(composer_form.composition_batch) @@ -206,7 +206,7 @@ class TestComposerForm(TestMailComposer): self._get_web_context(self.test_record, add_web=True, default_template_id=self.template.id) )) self.assertTrue(composer_form.auto_delete, 'Should take template value') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertEqual(composer_form.body, f'

TemplateBody {self.test_record.name}

') self.assertFalse(composer_form.composition_batch) @@ -236,7 +236,7 @@ class TestComposerForm(TestMailComposer): default_template_id=self.template.id), )) self.assertTrue(composer_form.auto_delete, 'Should take composer value') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertEqual(composer_form.body, self.template.body_html, 'MailComposer: comment in batch mode should have template raw body if template') @@ -267,7 +267,7 @@ class TestComposerForm(TestMailComposer): default_template_id=self.template.id, )) self.assertTrue(composer_form.auto_delete, 'Should take composer value') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertEqual(composer_form.body, self.template.body_html, 'MailComposer: comment in batch mode should have template raw body if template') @@ -299,7 +299,7 @@ class TestComposerForm(TestMailComposer): default_template_id=self.template.id, )) self.assertTrue(composer_form.auto_delete, 'Should take composer value') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertEqual(composer_form.body, '

TemplateBody

') self.assertFalse(composer_form.composition_batch) @@ -327,7 +327,7 @@ class TestComposerForm(TestMailComposer): self._get_web_context(self.test_records, add_web=True) )) self.assertFalse(composer_form.auto_delete) - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: if emails are kept, logs are automatically kept') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertFalse(composer_form.body) self.assertTrue(composer_form.composition_batch) @@ -659,9 +659,10 @@ class TestComposerInternals(TestMailComposer): # default creation values if composition_mode == 'comment': self.assertTrue(composer.auto_delete, 'By default, remove notification emails') + self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') else: self.assertFalse(composer.auto_delete, 'By default, keep mailing emails') - self.assertTrue(composer.auto_delete_keep_log) + self.assertFalse(composer.auto_delete_keep_log, 'Emails are not unlinked, logs are already kept') self.assertTrue(composer.email_add_signature) self.assertEqual(composer.email_layout_xmlid, 'mail.test_layout') self.assertEqual(composer.message_type, 'comment') @@ -675,7 +676,7 @@ class TestComposerInternals(TestMailComposer): # values come from template if composition_mode == 'comment': self.assertTrue(composer.auto_delete) - self.assertTrue(composer.auto_delete_keep_log) + self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') self.assertTrue(composer.email_add_signature, 'TODO: should be False as template negates this config') self.assertEqual(composer.email_layout_xmlid, 'mail.test_layout') self.assertEqual(composer.message_type, 'comment') @@ -704,14 +705,12 @@ class TestComposerInternals(TestMailComposer): composer._onchange_template_id_wrapper() if composition_mode == 'comment': - # self.assertFalse(composer.auto_delete, 'TODO: should be updated') - self.assertTrue(composer.auto_delete) + self.assertFalse(composer.auto_delete) self.assertEqual(composer.message_type, 'notification') self.assertEqual(composer.subtype_id, self.env.ref('mail.mt_note')) self.assertTrue(composer.subtype_is_log) else: - # self.assertFalse(composer.auto_delete, 'TODO: should be updated') - self.assertTrue(composer.auto_delete) + self.assertFalse(composer.auto_delete) self.assertEqual(composer.message_type, 'notification') self.assertEqual(composer.subtype_id, self.env.ref('mail.mt_note')) self.assertTrue(composer.subtype_is_log) @@ -1238,7 +1237,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): 'partner_ids': [(4, self.partner_1.id), (4, self.partner_2.id)] }) self.assertTrue(composer.auto_delete, 'Comment mode removes notification emails by default') - self.assertTrue(composer.auto_delete_keep_log) + self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') with self.mock_mail_gateway(mail_unlink_sent=True): composer._action_send_mail() @@ -1260,7 +1259,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): 'partner_ids': [(4, self.partner_1.id), (4, self.partner_2.id)] }) self.assertFalse(composer.auto_delete) - self.assertTrue(composer.auto_delete_keep_log) + self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') with self.mock_mail_gateway(mail_unlink_sent=True): composer._action_send_mail() @@ -1684,8 +1683,7 @@ class TestComposerResultsMass(TestMailComposer): self.assertEqual(len(self._mails), 2, 'Should have sent 1 email per record') self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record') - # self.assertEqual(self._new_mails.exists(), self._new_mails, 'Should not have deleted mail.mail records') - self.assertFalse(self._new_mails.exists(), 'TODO: Template is forced over composer value, which is not correct') + self.assertEqual(self._new_mails.exists(), self._new_mails, 'Should not have deleted mail.mail records') self.assertEqual(len(self._new_msgs), 2, 'Should have created 1 mail.mail per record') self.assertEqual(self._new_msgs.exists(), self._new_msgs, 'Should not have deleted mail.message records') diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index d4e7994173b..9555980e5cb 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -1231,7 +1231,7 @@ class TestMessagePostHelpers(TestMessagePostCommon): fields_values={ 'auto_delete': False, 'is_internal': False, - 'is_notification': True, # auto_delete_keep_log -> keep underlying mail.message + 'is_notification': False, # no to_delete -> no keep_log 'message_type': 'email', 'model': test_record._name, 'notified_partner_ids': self.env['res.partner'], From 4924df5743822ff0d71c914d6ed5196c8f310c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 20 Oct 2022 14:11:23 +0000 Subject: [PATCH 16/32] [REF] mail: move composer 'author_id' / 'email_from' fields from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute fields ``email_from`` and ``author_id`` in compute methods. Email_from computation is coming either from template, either from context. When having a template with a value set, copy it (in batch mode) or render it (in monorecord comment mode) on the composer. Otherwise try to take current user's email. When removing the template, fallback on default thread behavior (which is current user's email). Author computation is not controllable from the template currently. We therefore try to synchronize it with the given email_from (in rendered mode to avoid trying to find partner based on qweb expressions), or fallback on current user. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- .../tests/test_mail_performance.py | 16 ++- addons/mail/wizard/mail_compose_message.py | 100 ++++++++++++++---- addons/test_mail/tests/test_mail_composer.py | 55 +++++----- 3 files changed, 117 insertions(+), 54 deletions(-) diff --git a/addons/l10n_generic_coa/tests/test_mail_performance.py b/addons/l10n_generic_coa/tests/test_mail_performance.py index 7a4d19b58b3..eb32053b6f4 100644 --- a/addons/l10n_generic_coa/tests/test_mail_performance.py +++ b/addons/l10n_generic_coa/tests/test_mail_performance.py @@ -269,10 +269,8 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance): self.assertEqual(len(print_msg.attachment_ids), 3) self.assertNotIn(self.attachments, print_msg.attachment_ids, 'Attachments should be duplicated, not just linked') - self.assertEqual(print_msg.author_id, self.env.user.partner_id, - 'TODO: not synchronized with email_from choice') - # self.assertEqual(print_msg.author_id, self.user_account_other.partner_id, - # 'Should take invoice_user_id partner') + self.assertEqual(print_msg.author_id, self.user_account_other.partner_id, + 'Should take invoice_user_id partner') self.assertEqual(print_msg.email_from, self.user_account_other.email_formatted, 'Should take invoice_user_id email') self.assertEqual(print_msg.notified_partner_ids, test_customer + self.user_accountman.partner_id) @@ -285,7 +283,7 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance): self.assertMailMail( test_customer, 'sent', - author=self.user_account.partner_id, # author: current user, not synchronized with email_from of template + author=self.user_account_other.partner_id, # author: synchronized with email_from of template content=f'TemplateBody for {test_move.name}', email_values={ 'attachments_info': [ @@ -373,10 +371,8 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance): self.assertEqual(len(print_msg.attachment_ids), 3) self.assertNotIn(self.attachments, print_msg.attachment_ids, 'Attachments should be duplicated, not just linked') - self.assertEqual(print_msg.author_id, self.env.user.partner_id, - 'TODO: not synchronized with email_from choice') - # self.assertEqual(print_msg.author_id, self.user_account_other.partner_id, - # 'Should take invoice_user_id partner') + self.assertEqual(print_msg.author_id, self.user_account_other.partner_id, + 'Should take invoice_user_id partner') self.assertEqual(print_msg.email_from, self.user_account_other.email_formatted, 'Should take invoice_user_id email') self.assertEqual(print_msg.notified_partner_ids, test_customer + self.user_accountman.partner_id) @@ -389,7 +385,7 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance): self.assertMailMail( test_customer, 'sent', - author=self.user_account.partner_id, # author: current user, not synchronized with email_from of template + author=self.user_account_other.partner_id, # author: synchronized with email_from of template content=f'SpanishBody for {test_move.name}', # translated version email_values={ 'attachments_info': [ diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index bda09c5cc3d..0ac29ba80b2 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -9,7 +9,7 @@ import logging from odoo import _, api, fields, models, tools, Command from odoo.exceptions import UserError, ValidationError from odoo.osv import expression -from odoo.tools import email_re +from odoo.tools import email_re, is_html_empty _logger = logging.getLogger(__name__) @@ -65,16 +65,6 @@ class MailComposer(models.TransientModel): result = super().default_get(fields_list) - # author - missing_author = 'author_id' in fields_list and 'author_id' not in result - missing_email_from = 'email_from' in fields_list and 'email_from' not in result - if missing_author or missing_email_from: - author_id, email_from = self.env['mail.thread']._message_compute_author(result.get('author_id'), result.get('email_from'), raise_on_email=False) - if missing_email_from: - result['email_from'] = email_from - if missing_author: - result['author_id'] = author_id - # record context management if 'model' in fields_list and 'model' not in result: result['model'] = self.env.context.get('active_model') @@ -134,9 +124,12 @@ class MailComposer(models.TransientModel): email_layout_xmlid = fields.Char('Email Notification Layout', copy=False) email_add_signature = fields.Boolean(default=True) # origin - email_from = fields.Char('From', help="Email address of the sender. This field is set when no matching partner is found and replaces the author_id field in the chatter.") + email_from = fields.Char( + 'From', compute='_compute_authorship', readonly=False, store=True, + help="Email address of the sender. This field is set when no matching partner is found and replaces the author_id field in the chatter.") author_id = fields.Many2one( - 'res.partner', 'Author', + 'res.partner', string='Author', + compute='_compute_authorship', readonly=False, store=True, help="Author of the message. If not set, email_from may hold an email address that did not match any partner.") # composition composition_mode = fields.Selection( @@ -214,6 +207,47 @@ class MailComposer(models.TransientModel): for composer in self: composer._evaluate_res_domain() + @api.depends('composition_mode', 'email_from', 'model', + 'res_domain', 'res_ids', 'template_id') + def _compute_authorship(self): + """ Computation is coming either from template, either from context. + When having a template with a value set, copy it (in batch mode) or + render it (in monorecord comment mode) on the composer. Otherwise + try to take current user's email. When removing the template, fallback + on default thread behavior (which is current user's email). + + Author is not controllable from the template currently. We therefore + try to synchronize it with the given email_from (in rendered mode to + avoid trying to find partner based on qweb expressions), or fallback + on current user. """ + Thread = self.env['mail.thread'].with_context(active_test=False) + for composer in self: + rendering_mode = composer.composition_mode == 'comment' and not composer.composition_batch + updated_author_id = None + + # update email_from first as it is the main used field currently + if composer.template_id.email_from: + composer._set_value_from_template('email_from') + # removing template or void from -> fallback on current user as default + elif not composer.template_id or not composer.email_from: + composer.email_from = self.env.user.email_formatted + updated_author_id = self.env.user.partner_id.id + + # Update author. When being in rendered mode: link with rendered + # email_from or fallback on current user if email does not match. + # When changing template in raw mode or resetting also fallback. + if composer.email_from and rendering_mode and not updated_author_id: + updated_author_id, _ = Thread._message_compute_author( + None, composer.email_from, raise_on_email=False, + ) + if not updated_author_id: + updated_author_id = self.env.user.partner_id.id + if not rendering_mode or not composer.template_id: + updated_author_id = self.env.user.partner_id.id + + if updated_author_id: + composer.author_id = updated_author_id + @api.depends('res_domain', 'res_ids') def _compute_composition_batch(self): """ Determine if batch mode is activated: @@ -349,8 +383,7 @@ class MailComposer(models.TransientModel): template = self.env['mail.template'].browse(template_id) values = dict( (field, template[field]) - for field in ('email_from', - 'reply_to', + for field in ('reply_to', 'scheduled_date', 'subject', ) @@ -372,7 +405,6 @@ class MailComposer(models.TransientModel): ('attachment_ids', 'body_html', 'email_cc', - 'email_from', 'email_to', 'mail_server_id', 'partner_ids', @@ -405,7 +437,6 @@ class MailComposer(models.TransientModel): ).default_get(['attachment_ids', 'body', 'composition_mode', - 'email_from', 'mail_server_id', 'model', 'parent_id', @@ -419,7 +450,6 @@ class MailComposer(models.TransientModel): (key, default_values[key]) for key in ('attachment_ids', 'body', - 'email_from', 'mail_server_id', 'partner_ids', 'reply_to', @@ -1144,3 +1174,37 @@ class MailComposer(models.TransientModel): if not tools.is_list_of(res_ids, int): raise ValidationError(error_msg) return res_ids + + def _set_value_from_template(self, template_fname, composer_fname=False): + """ Set composer value from its template counterpart. In monorecord + comment mode, we get directly the rendered value, giving the real + value to the user. Otherwise we get the raw (unrendered) value from + template, as it will be rendered at send time (for mass mail, whatever + the number of contextual records to mail) or before posting on records + (for comment in batch). + + :param str template_fname: name of field on template model, used to + fetch the value (and maybe render it); + :param str composer_fname: name of field on composer model, when field + names do not match (e.g. body_html on template used to populate body + on composer); + """ + self.ensure_one() + composer_fname = composer_fname or template_fname + + # fetch template value, check if void + template_value = self.template_id[template_fname] if self.template_id else False + if template_value and template_fname == 'body_html': + template_value = template_value if not is_html_empty(template_value) else False + + if template_value: + if self.composition_mode == 'comment' and not self.composition_batch: + res_ids = self._evaluate_res_ids() + rendering_res_ids = res_ids or [0] + self[composer_fname] = self.template_id._generate_template( + rendering_res_ids, + {template_fname}, + )[rendering_res_ids[0]][template_fname] + else: + self[composer_fname] = self.template_id[template_fname] + return self[composer_fname] diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 92025741e3e..3cf2d6e2646 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -79,7 +79,7 @@ class TestMailComposer(TestMailCommon, TestRecipients): } ]) - cls.test_from = '"John Doe" ' + cls.test_from = '"John Doe" ' cls.template = cls.env['mail.template'].create({ 'auto_delete': True, @@ -207,7 +207,8 @@ class TestComposerForm(TestMailComposer): )) self.assertTrue(composer_form.auto_delete, 'Should take template value') self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') - self.assertEqual(composer_form.author_id, self.env.user.partner_id) + self.assertEqual(composer_form.author_id, self.user_employee_2.partner_id, + 'MailComposer: author is synchronized with email_from when possible') self.assertEqual(composer_form.body, f'

TemplateBody {self.test_record.name}

') self.assertFalse(composer_form.composition_batch) self.assertEqual(composer_form.composition_mode, 'comment') @@ -579,52 +580,54 @@ class TestComposerInternals(TestMailComposer): # changing template should update its email_from composer.write({'template_id': self.template.id}) - # currently onchange necessary - composer._onchange_template_id_wrapper() if composition_mode == 'comment' and not batch: - self.assertEqual(composer.author_id, self.partner_1, - 'MailComposer: TODO: should try to synchronize with email_from') + self.assertEqual(composer.author_id, self.test_record.user_id.partner_id, + f'MailComposer: should try to link in rendered mode: {composer.author_id.name}, expected {self.env.user.name}') self.assertEqual(composer.email_from, self.test_record.user_id.email_formatted, - f'MailComposer: should take email_from rendered from template ({composition_mode}-{batch})') + 'MailComposer: should take email_from rendered from template') else: - self.assertEqual(composer.author_id, self.partner_1, - 'MailComposer: TODO: email_from is raw, what to do with it ?') + self.assertEqual(composer.author_id, self.env.user.partner_id, + f'MailComposer: should reset to current user in raw mode: {composer.author_id.name}, expected {self.env.user.name}') self.assertEqual(composer.email_from, self.template.email_from, - f'MailComposer: should take email_from raw from template ({composition_mode}-{batch})') + 'MailComposer: should take email_from raw from template') - # manual values are kept over template values + # manual values are kept over template values; if email does not + # match any author, reset author composer.write({'email_from': self.test_from}) - self.assertEqual(composer.author_id, self.partner_1) - self.assertEqual(composer.email_from, self.test_from, - 'MailComposer: TODO: author / email_from are not synchronized') + if composition_mode == 'comment' and not batch: + self.assertEqual(composer.author_id, self.test_record.user_id.partner_id, + 'MailComposer: TODO: compute not called') + self.assertEqual(composer.email_from, self.test_from, + 'MailComposer: manual values should be kept') + else: + self.assertEqual(composer.author_id, self.env.user.partner_id, + 'MailComposer: TODO: compute not called') + self.assertEqual(composer.email_from, self.test_from, + 'MailComposer: manual values should be kept') # update with template with void values: void value is not forced in # rendering mode as well as when copying template values composer.write({'template_id': template_void.id}) - # currently onchange necessary - composer._onchange_template_id_wrapper() if composition_mode == 'comment' and not batch: - self.assertEqual(composer.author_id, self.partner_1, + self.assertEqual(composer.author_id, self.env.user.partner_id, 'MailComposer: TODO: author / email_from are not synchronized') self.assertEqual(composer.email_from, self.test_from) else: - self.assertEqual(composer.author_id, self.partner_1, + self.assertEqual(composer.author_id, self.env.user.partner_id, 'MailComposer: TODO: author / email_from are not synchronized') self.assertEqual(composer.email_from, self.test_from) # reset template: values are reset due to call to default_get composer.write({'template_id': False}) - # currently onchange necessary - composer._onchange_template_id_wrapper() if composition_mode == 'comment' and not batch: - self.assertEqual(composer.author_id, self.partner_1, + self.assertEqual(composer.author_id, self.env.user.partner_id, 'MailComposer: TODO: author / email_from are not synchronized') self.assertEqual(composer.email_from, self.env.user.email_formatted) else: - self.assertEqual(composer.author_id, self.partner_1, + self.assertEqual(composer.author_id, self.env.user.partner_id, 'MailComposer: TODO: author / email_from are not synchronized') self.assertEqual(composer.email_from, self.env.user.email_formatted) @@ -1412,7 +1415,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): 'Author cannot be synchronized with a raw email_from') self.assertEqual(composer.email_from, self.template.email_from) else: - author = self.env.user.partner_id + author = self.partner_employee_2 self.assertEqual(composer.author_id, author, 'Author cannot be synchronized with a raw email_from') self.assertEqual(composer.email_from, self.partner_employee_2.email_formatted) @@ -1488,7 +1491,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): message = test_record.message_ids[0] self.assertMailMail(self.partner_employee_2, 'sent', mail_message=message, - author=author, # author != email_from (template sets only email_from) + author=author, # author is different in batch and monorecord mode (raw or rendered email_from) email_values={ 'body_content': f'TemplateBody {test_record.name}', 'email_from': test_record.user_id.email_formatted, # set by template @@ -1506,7 +1509,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): ) self.assertMailMail(test_record.customer_id + new_partners, 'sent', mail_message=message, - author=author, # author != email_from (template sets only email_from) + author=author, # author is different in batch and monorecord mode (raw or rendered email_from) email_values={ 'body_content': f'TemplateBody {test_record.name}', 'email_from': test_record.user_id.email_formatted, # set by template @@ -1627,7 +1630,7 @@ class TestComposerResultsCommentStatus(TestMailComposer): self.assertMailMail( recipient, 'sent', mail_message=message, - author=self.partner_employee, # author != email_from (template sets only email_from) + author=self.partner_employee_2, # author synchronized with email_from email_values={ 'email_from': self.user_employee_2.email_formatted, # set by template }, From 2436ddf14113bc7cac9144aedd86b167e03597e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 20 Oct 2022 14:12:49 +0000 Subject: [PATCH 17/32] [REF] mail: move composer 'reply_to_*' fields from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONSS Improve composer to correctly compute fields ``reply_to(_force_new)`` in compute methods. Reply_to computation is coming either from template, either reset. When having a template with a value set, copy it (in batch mode) or render it (in monorecord comment mode) on the composer. When removing the template reset it. Reply_to_force_new computation depends on model. If it does not inherit from MailThread, avoid replies to be considered as thread updates, they will instead follow the routing rules (alias, ...). Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 38 ++++++++++++++------ addons/test_mail/tests/test_mail_composer.py | 6 ++-- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 0ac29ba80b2..dd0508ebff8 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -82,11 +82,6 @@ class MailComposer(models.TransientModel): if result.get('composition_mode') == 'comment' and (set(fields_list) & {'model', 'res_ids', 'partner_ids', 'subject'}): result.update(self.get_record_data(result)) - # threading support check: 'update' requires to use message_update/post - if 'reply_to_mode' in fields_list and 'reply_to_mode' not in result and result.get('model'): - if result['model'] not in self.env or not hasattr(self.env[result['model']], 'message_post'): - result['reply_to_mode'] = 'new' - # when being in new mode, create_uid is not granted -> ACLs issue may arise if 'create_uid' in fields_list and 'create_uid' not in result: result['create_uid'] = self.env.uid @@ -161,9 +156,12 @@ class MailComposer(models.TransientModel): subtype_is_log = fields.Boolean('Is a log', compute='_compute_subtype_is_log') mail_activity_type_id = fields.Many2one('mail.activity.type', 'Mail Activity Type', ondelete='set null') # destination - reply_to = fields.Char('Reply To', help='Reply email address. Setting the reply_to bypasses the automatic thread creation.') + reply_to = fields.Char( + 'Reply To', compute='_compute_reply_to', readonly=False, store=True, + help='Reply email address. Setting the reply_to bypasses the automatic thread creation.') reply_to_force_new = fields.Boolean( string='Considers answers as new thread', + compute='_compute_reply_to_force_new', readonly=False, store=True, help='Manage answers as new incoming emails instead of replies going to the same thread.') reply_to_mode = fields.Selection([ ('update', 'Store email and replies in the chatter of each record'), @@ -307,6 +305,28 @@ class MailComposer(models.TransientModel): for composer in self.filtered('subtype_id'): composer.subtype_is_log = composer.subtype_id.id == note_id + @api.depends('composition_mode', 'model', 'res_domain', 'res_ids', + 'template_id') + def _compute_reply_to(self): + """ Computation is coming either from template, either reset. When + having a template with a value set, copy it (in batch mode) or render + it (in monorecord comment mode) on the composer. When removing the + template, reset it. """ + for composer in self: + if composer.template_id: + composer._set_value_from_template('reply_to') + else: + composer.reply_to = False + + @api.depends('model') + def _compute_reply_to_force_new(self): + """ If model does not inherit from MailThread, avoid replies to be + considered as thread updates, they will instead follow the routing + rules (alias, ...). """ + self.filtered( + lambda composer: not composer.model or not composer.model_is_thread + ).reply_to_force_new = True + @api.depends('reply_to_force_new') def _compute_reply_to_mode(self): for composer in self: @@ -383,8 +403,7 @@ class MailComposer(models.TransientModel): template = self.env['mail.template'].browse(template_id) values = dict( (field, template[field]) - for field in ('reply_to', - 'scheduled_date', + for field in ('scheduled_date', 'subject', ) if template[field] @@ -408,7 +427,6 @@ class MailComposer(models.TransientModel): 'email_to', 'mail_server_id', 'partner_ids', - 'reply_to', 'report_template_ids', 'scheduled_date', 'subject', @@ -441,7 +459,6 @@ class MailComposer(models.TransientModel): 'model', 'parent_id', 'partner_ids', - 'reply_to', 'res_ids', 'scheduled_date', 'subject', @@ -452,7 +469,6 @@ class MailComposer(models.TransientModel): 'body', 'mail_server_id', 'partner_ids', - 'reply_to', 'scheduled_date', 'subject', ) if key in default_values) diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 3cf2d6e2646..814df78e8c9 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -1013,14 +1013,12 @@ class TestComposerInternals(TestMailComposer): if composition_mode == 'comment' and not batch: self.assertEqual(composer.partner_ids, self.partner_admin, 'TODO: Values are kept (should be reset ?)') - self.assertEqual(composer.reply_to, 'info@test.example.com', - 'TODO: Values are kept (should be reset ?)') + self.assertFalse(composer.reply_to) self.assertFalse(composer.reply_to_force_new) else: self.assertEqual(composer.partner_ids, self.partner_admin, 'TODO: Values are kept (should be reset ?)') - self.assertEqual(composer.reply_to, self.template.reply_to, - 'TODO: Values are kept (should be reset ?)') + self.assertFalse(composer.reply_to) self.assertFalse(composer.reply_to_force_new) # 2. check with default From 5cfe750a83fcf703500f99e38d0c904a9a110027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 20 Oct 2022 14:29:44 +0000 Subject: [PATCH 18/32] [REF] mail: move composer 'body' and 'subject' field from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute fields ``body`` and ``subject`` in compute methods. Subject computation is coming either form template, either from context. When having a template with a value set, copy it (in batch mode) or render it (in monorecord comment mode) on the composer. Otherwise it comes from the parent (if set), or computed based on the generic '_message_compute_ subject' method in monorecord comment mode, or set to False. When removing the template, reset it. Body computation is coming either from template, either reset. When having a template with a value set, copy it (in batch mode) or render it (in monorecord comment mode) on the composer. When removing the template, reset it. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 64 ++++++++++++-------- addons/test_mail/tests/test_mail_composer.py | 7 +-- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index dd0508ebff8..9054cd07f50 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -79,7 +79,7 @@ class MailComposer(models.TransientModel): else: result['res_ids'] = False # record / parent based computation - if result.get('composition_mode') == 'comment' and (set(fields_list) & {'model', 'res_ids', 'partner_ids', 'subject'}): + if result.get('composition_mode') == 'comment' and (set(fields_list) & {'model', 'res_ids', 'partner_ids'}): result.update(self.get_record_data(result)) # when being in new mode, create_uid is not granted -> ACLs issue may arise @@ -106,10 +106,10 @@ class MailComposer(models.TransientModel): ]) # content - subject = fields.Char('Subject', compute=False) + subject = fields.Char('Subject', compute='_compute_subject', readonly=False, store=True) body = fields.Html( - 'Contents', render_engine='qweb', render_options={'post_process': True}, - compute=False, default='', sanitize_style=True) + 'Contents', render_engine='qweb', render_options={'post_process': True}, sanitize_style=True, + compute='_compute_body', readonly=False, store=True) parent_id = fields.Many2one( 'mail.message', 'Parent Message', ondelete='set null') template_id = fields.Many2one('mail.template', 'Use template', domain="[('model', '=', model)]") @@ -205,6 +205,40 @@ class MailComposer(models.TransientModel): for composer in self: composer._evaluate_res_domain() + @api.depends('composition_mode', 'model', 'parent_id', 'res_domain', + 'res_ids', 'template_id') + def _compute_subject(self): + """ Computation is coming either form template, either from context. + When having a template with a value set, copy it (in batch mode) or + render it (in monorecord comment mode) on the composer. Otherwise + it comes from the parent (if set), or computed based on the generic + '_message_compute_subject' method in monorecord comment mode, or + set to False. When removing the template, reset it. """ + for composer in self: + if composer.template_id: + composer._set_value_from_template('subject') + if not composer.template_id or not composer.subject: + subject = composer.parent_id.subject + if (not subject and composer.model and + composer.composition_mode == 'comment' and + not composer.composition_batch): + res_ids = composer._evaluate_res_ids() + subject = self.env[composer.model].browse(res_ids)._message_compute_subject() + composer.subject = subject + + @api.depends('composition_mode', 'model', 'res_domain', 'res_ids', + 'template_id') + def _compute_body(self): + """ Computation is coming either from template, either reset. When + having a template with a value set, copy it (in batch mode) or render + it (in monorecord comment mode) on the composer. When removing the + template, reset it. """ + for composer in self: + if composer.template_id: + composer._set_value_from_template('body_html', 'body') + if not composer.template_id: + composer.body = False + @api.depends('composition_mode', 'email_from', 'model', 'res_domain', 'res_ids', 'template_id') def _compute_authorship(self): @@ -404,7 +438,6 @@ class MailComposer(models.TransientModel): values = dict( (field, template[field]) for field in ('scheduled_date', - 'subject', ) if template[field] ) @@ -412,8 +445,6 @@ class MailComposer(models.TransientModel): values['attachment_ids'] = [att.id for att in template.attachment_ids] if template.mail_server_id: values['mail_server_id'] = template.mail_server_id.id - if not tools.is_html_empty(template.body_html): - values['body'] = template.body_html elif template_id and len(res_ids) <= 1: # render template (mono record, comment mode) and set it as composer values # trick to evaluate qweb even when having no records @@ -422,14 +453,12 @@ class MailComposer(models.TransientModel): self.env['mail.template'].browse(template_id), template_res_ids, ('attachment_ids', - 'body_html', 'email_cc', 'email_to', 'mail_server_id', 'partner_ids', 'report_template_ids', 'scheduled_date', - 'subject', ) )[template_res_ids[0]] # transform attachments into attachment_ids; not attached to the document because this will @@ -453,7 +482,6 @@ class MailComposer(models.TransientModel): default_model=model, default_res_ids=res_ids ).default_get(['attachment_ids', - 'body', 'composition_mode', 'mail_server_id', 'model', @@ -461,16 +489,13 @@ class MailComposer(models.TransientModel): 'partner_ids', 'res_ids', 'scheduled_date', - 'subject', ]) values = dict( (key, default_values[key]) for key in ('attachment_ids', - 'body', 'mail_server_id', 'partner_ids', 'scheduled_date', - 'subject', ) if key in default_values) # This onchange should return command instead of ids for x2many field. @@ -484,7 +509,7 @@ class MailComposer(models.TransientModel): wizard when sending an email related a previous email (parent_id) or a document (model, res_id). This is based on previously computed default values. """ - result, subject = {}, False + result = {} model = values.get('model') res_ids = self._parse_res_ids(values['res_ids']) if values.get('res_ids') else [] if values.get('parent_id'): @@ -496,17 +521,6 @@ class MailComposer(models.TransientModel): res_ids = [parent.res_id] result['res_ids'] = res_ids result['partner_ids'] = values.get('partner_ids', list()) + parent.partner_ids.ids - subject = tools.ustr(parent.subject or '') - if not subject and model and res_ids and len(res_ids) == 1: - record = self.env[model].browse(res_ids[0]) - if not subject: - subject = record._message_compute_subject() - elif model and len(res_ids) == 1: - record = self.env[model].browse(res_ids[0]) - subject = record._message_compute_subject() - - if values.get('parent_id') or len(res_ids) == 1: # to be cleanup when moving to computed fields - result['subject'] = subject return result diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 814df78e8c9..86f8905e2c7 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -820,8 +820,7 @@ class TestComposerInternals(TestMailComposer): # currently onchange necessary composer._onchange_template_id_wrapper() - # values are reset with default_get call, if it returns value - # (aka subject for comment mode) + # values are reset with default_get call / compute field if composition_mode == 'comment' and not batch: self.assertFalse(composer.body) # self.assertFalse(composer.mail_server_id.id) @@ -845,9 +844,7 @@ class TestComposerInternals(TestMailComposer): # self.assertFalse(composer.scheduled_date) self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}', 'TODO: Values are kept (should be reset ?)') - # self.assertFalse(composer.subject) - self.assertEqual(composer.subject, 'Back to my amazing subject for {{ record.name }}', - 'TODO: Values are kept (should be reset ?)') + self.assertFalse(composer.subject) # 2. check with default ctx['default_template_id'] = self.template.id From 05f45043da44ad4284dfc4cf36665600068485c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 20 Oct 2022 14:30:24 +0000 Subject: [PATCH 19/32] [REF] mail: move composer 'mail_server_id' field from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute field ``mail_server_id`` in compute method. Copy value from template when updating it, if set on template. When removing the template, reset it. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 20 +++++++++++++------- addons/test_mail/tests/test_mail_composer.py | 8 ++------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 9054cd07f50..b86fd2c7d11 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -184,7 +184,9 @@ class MailComposer(models.TransientModel): help='Keep a copy of the email content if emails are removed (mass mailing only)') force_send = fields.Boolean( 'Send mailing or notifications directly') - mail_server_id = fields.Many2one('ir.mail_server', 'Outgoing mail server') + mail_server_id = fields.Many2one( + 'ir.mail_server', string='Outgoing mail server', + compute='_compute_mail_server_id', readonly=False, store=True) scheduled_date = fields.Char( 'Scheduled Date', help="In comment mode: if set, postpone notifications sending. " @@ -395,6 +397,16 @@ class MailComposer(models.TransientModel): toreset.auto_delete_keep_log = False (self - toreset).auto_delete_keep_log = True + @api.depends('template_id') + def _compute_mail_server_id(self): + """ Copy value from template when updating it, if set on template. When + removing the template, reset it. """ + for composer in self: + if composer.template_id.mail_server_id: + composer.mail_server_id = composer.template_id.mail_server_id + if not composer.template_id: + composer.mail_server_id = False + # Overrides of mail.render.mixin @api.depends('model') def _compute_render_model(self): @@ -443,8 +455,6 @@ class MailComposer(models.TransientModel): ) if template.attachment_ids: values['attachment_ids'] = [att.id for att in template.attachment_ids] - if template.mail_server_id: - values['mail_server_id'] = template.mail_server_id.id elif template_id and len(res_ids) <= 1: # render template (mono record, comment mode) and set it as composer values # trick to evaluate qweb even when having no records @@ -455,7 +465,6 @@ class MailComposer(models.TransientModel): ('attachment_ids', 'email_cc', 'email_to', - 'mail_server_id', 'partner_ids', 'report_template_ids', 'scheduled_date', @@ -483,7 +492,6 @@ class MailComposer(models.TransientModel): default_res_ids=res_ids ).default_get(['attachment_ids', 'composition_mode', - 'mail_server_id', 'model', 'parent_id', 'partner_ids', @@ -493,7 +501,6 @@ class MailComposer(models.TransientModel): values = dict( (key, default_values[key]) for key in ('attachment_ids', - 'mail_server_id', 'partner_ids', 'scheduled_date', ) if key in default_values) @@ -855,7 +862,6 @@ class MailComposer(models.TransientModel): ('attachment_ids', 'email_to', 'email_cc', - 'mail_server_id', 'partner_ids', 'report_template_ids', 'scheduled_date', diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 86f8905e2c7..89cd1115bd7 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -823,9 +823,7 @@ class TestComposerInternals(TestMailComposer): # values are reset with default_get call / compute field if composition_mode == 'comment' and not batch: self.assertFalse(composer.body) - # self.assertFalse(composer.mail_server_id.id) - self.assertEqual(composer.mail_server_id, self.mail_server_global, - 'TODO: Values are kept (should be reset ?)') + self.assertFalse(composer.mail_server_id.id) self.assertEqual(composer.record_name, 'Manual update', 'MailComposer: record name does not depend on template') # self.assertFalse(composer.scheduled_date) @@ -836,9 +834,7 @@ class TestComposerInternals(TestMailComposer): 'Check effective content') else: self.assertFalse(composer.body) - # self.assertFalse(composer.mail_server_id.id) - self.assertEqual(composer.mail_server_id, self.mail_server_global, - 'TODO: Values are kept (should be reset ?)') + self.assertFalse(composer.mail_server_id.id) self.assertEqual(composer.record_name, 'Manual update', 'MailComposer: record name does not depend on template') # self.assertFalse(composer.scheduled_date) From 2c1bb88644d9b89772cbfaf21f51693113325404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 29 Oct 2021 14:17:56 +0000 Subject: [PATCH 20/32] [REF] mail: move composer 'model' and 'res_ids' fields from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute fields ``model`` and ``res_ids`` in compute methods. Model can be set from parent or using 'active_model' context key frequently used as the composer is most invoked from list or form views. Res Ids computation may come from parent in comment mode, if set. It takes the parent message's res_id. Otherwise the composer uses the 'active_ids' context key, unless it is too big to be stored in database. Indeed when invoked for big mailings, 'active_ids' may be a very big list. Support of 'active_ids' when sending is granted in order to not always rely on 'res_ids' field. When 'active_ids' is not present, fallback on 'active_id'. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 60 +++++++++++++--------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index b86fd2c7d11..48067377d53 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -32,7 +32,7 @@ class MailComposer(models.TransientModel): at model and view levels to provide specific features. The behavior of the wizard depends on the composition_mode field: - - 'comment': post on a record. The wizard is pre-populated via ``get_record_data`` + - 'comment': post on a record. - 'mass_mail': wizard in mass mailing mode where the mail details can contain template placeholders that will be merged with actual data before being sent to each recipient. @@ -65,21 +65,8 @@ class MailComposer(models.TransientModel): result = super().default_get(fields_list) - # record context management - if 'model' in fields_list and 'model' not in result: - result['model'] = self.env.context.get('active_model') - if 'res_ids' in fields_list and 'res_ids' not in result: - if self.env.context.get('active_ids'): - active_res_ids = self._parse_res_ids(self.env.context['active_ids']) - # beware, field is limited in storage, usage of active_ids in context still required - if active_res_ids and len(active_res_ids) <= self._batch_size: - result['res_ids'] = self.env.context['active_ids'] - elif self.env.context.get('active_id'): - result['res_ids'] = [self.env.context['active_id']] - else: - result['res_ids'] = False # record / parent based computation - if result.get('composition_mode') == 'comment' and (set(fields_list) & {'model', 'res_ids', 'partner_ids'}): + if result.get('composition_mode') == 'comment' and (set(fields_list) & {'partner_ids'}): result.update(self.get_record_data(result)) # when being in new mode, create_uid is not granted -> ACLs issue may arise @@ -133,9 +120,9 @@ class MailComposer(models.TransientModel): string='Composition mode', default='comment') composition_batch = fields.Boolean( 'Batch composition', compute='_compute_composition_batch') # more than 1 record (raw source) - model = fields.Char('Related Document Model') + model = fields.Char('Related Document Model', compute='_compute_model', readonly=False, store=True) model_is_thread = fields.Boolean('Thread-Enabled', compute='_compute_model_is_thread') - res_ids = fields.Text('Related Document IDs') + res_ids = fields.Text('Related Document IDs', compute='_compute_res_ids', readonly=False, store=True) res_domain = fields.Text('Active domain') res_domain_user_id = fields.Many2one( 'res.users', string='Responsible', @@ -298,6 +285,17 @@ class MailComposer(models.TransientModel): res_ids = composer._evaluate_res_ids() composer.composition_batch = len(res_ids) > 1 if res_ids else False + @api.depends('composition_mode', 'parent_id') + def _compute_model(self): + """ Model can be set from parent or using 'active_model' context key + frequently used as the composer is most invoked from list or form + views. """ + for composer in self: + if composer.parent_id and composer.composition_mode == 'comment': + composer.model = composer.parent_id.model + elif not composer.model: + composer.model = self.env.context.get('active_model') + @api.depends('model') def _compute_model_is_thread(self): """ Determine if model is thread enabled. """ @@ -305,6 +303,26 @@ class MailComposer(models.TransientModel): model = self.env['ir.model']._get(composer.model) composer.model_is_thread = model.is_mail_thread + @api.depends('composition_mode', 'parent_id') + def _compute_res_ids(self): + """ Computation may come from parent in comment mode, if set. It takes + the parent message's res_id. Otherwise the composer uses the 'active_ids' + context key, unless it is too big to be stored in database. Indeed + when invoked for big mailings, 'active_ids' may be a very big list. + Support of 'active_ids' when sending is granted in order to not always + rely on 'res_ids' field. When 'active_ids' is not present, fallback + on 'active_id'. """ + for composer in self.filtered(lambda composer: not composer.res_ids): + if composer.parent_id and composer.composition_mode == 'comment': + composer.res_ids = f"{[composer.parent_id.res_id]}" + else: + active_res_ids = self._parse_res_ids(self.env.context.get('active_ids')) + # beware, field is limited in storage, usage of active_ids in context still required + if active_res_ids and len(active_res_ids) <= self._batch_size: + composer.res_ids = f"{self.env.context['active_ids']}" + elif not active_res_ids and self.env.context.get('active_id'): + composer.res_ids = f"{[self.env.context['active_id']]}" + @api.depends('composition_mode', 'model', 'parent_id', 'res_domain', 'res_ids') def _compute_record_name(self): """ Computation is coming either from parent message, either from the @@ -517,16 +535,8 @@ class MailComposer(models.TransientModel): a document (model, res_id). This is based on previously computed default values. """ result = {} - model = values.get('model') - res_ids = self._parse_res_ids(values['res_ids']) if values.get('res_ids') else [] if values.get('parent_id'): parent = self.env['mail.message'].browse(values.get('parent_id')) - if not model: - model = parent.model - result['model'] = model - if not res_ids: - res_ids = [parent.res_id] - result['res_ids'] = res_ids result['partner_ids'] = values.get('partner_ids', list()) + parent.partner_ids.ids return result From c6567038748544f055021d813e944b419f9b9c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 24 Oct 2022 14:35:56 +0000 Subject: [PATCH 21/32] [REF] mail: move composer 'scheduled_date' field from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute fields ``scheduled_date`` in compute method. Scheduled_date computation is coming either from template, either reset. When having a template with a value set, copy it (in batch mode) or render it (in monorecord comment mode) on the composer. When removing the template, reset it. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 19 ++++++++++++++----- addons/test_mail/tests/test_mail_composer.py | 20 +++----------------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 48067377d53..402ec5ed83b 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -176,6 +176,7 @@ class MailComposer(models.TransientModel): compute='_compute_mail_server_id', readonly=False, store=True) scheduled_date = fields.Char( 'Scheduled Date', + compute='_compute_scheduled_date', readonly=False, store=True, help="In comment mode: if set, postpone notifications sending. " "In mass mail mode: if sent, send emails after that date. " "This date is considered as being in UTC timezone.") @@ -425,6 +426,18 @@ class MailComposer(models.TransientModel): if not composer.template_id: composer.mail_server_id = False + @api.depends('composition_mode', 'model', 'res_ids', 'template_id') + def _compute_scheduled_date(self): + """ Computation is coming either from template, either reset. When + having a template with a value set, copy it (in batch mode) or render + it (in monorecord comment mode) on the composer. When removing the + template, reset it. """ + for composer in self: + if composer.template_id: + composer._set_value_from_template('scheduled_date') + if not composer.template_id: + composer.scheduled_date = False + # Overrides of mail.render.mixin @api.depends('model') def _compute_render_model(self): @@ -467,8 +480,7 @@ class MailComposer(models.TransientModel): template = self.env['mail.template'].browse(template_id) values = dict( (field, template[field]) - for field in ('scheduled_date', - ) + for field in () if template[field] ) if template.attachment_ids: @@ -485,7 +497,6 @@ class MailComposer(models.TransientModel): 'email_to', 'partner_ids', 'report_template_ids', - 'scheduled_date', ) )[template_res_ids[0]] # transform attachments into attachment_ids; not attached to the document because this will @@ -514,13 +525,11 @@ class MailComposer(models.TransientModel): 'parent_id', 'partner_ids', 'res_ids', - 'scheduled_date', ]) values = dict( (key, default_values[key]) for key in ('attachment_ids', 'partner_ids', - 'scheduled_date', ) if key in default_values) # This onchange should return command instead of ids for x2many field. diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 89cd1115bd7..818ebdf7345 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -764,8 +764,6 @@ class TestComposerInternals(TestMailComposer): # changing template should update its content composer.write({'template_id': self.template.id}) - # currently onchange necessary - composer._onchange_template_id_wrapper() # values come from template if composition_mode == 'comment' and not batch: @@ -799,8 +797,6 @@ class TestComposerInternals(TestMailComposer): # update with template with void values: void value is not forced in # rendering mode as well as in raw mode composer.write({'template_id': template_void.id}) - # currently onchange necessary - composer._onchange_template_id_wrapper() if composition_mode == 'comment' and not batch: self.assertEqual(composer.body, '

Back to my amazing body Date: Thu, 8 Dec 2022 13:21:14 +0000 Subject: [PATCH 23/32] [REF] mail: move composer 'attachment_ids' field from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute fields ``attachment_ids`` and reports (``report_template_ids`` on template that generates reports) in compute method. Computation is based on template and composition mode. In monorecord comment mode, template is used to generate attachments based on both attachment_ids of template, and reports coming from report_template_ids. Those are generated based on the current record to display. As template generation returns a list of tuples, new attachments are created on the fly during the compute. In batch or email mode, only attachment_ids from template are used on the composer. Reports will be generated at sending time. When template is removed, attachments are reset. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 72 +++++++++++++------- addons/test_mail/tests/test_mail_composer.py | 27 +++----- 2 files changed, 55 insertions(+), 44 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index fe4a3fdb836..d336f1fe790 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -98,7 +98,8 @@ class MailComposer(models.TransientModel): template_id = fields.Many2one('mail.template', 'Use template', domain="[('model', '=', model)]") attachment_ids = fields.Many2many( 'ir.attachment', 'mail_compose_message_ir_attachments_rel', - 'wizard_id', 'attachment_id', 'Attachments') + 'wizard_id', 'attachment_id', string='Attachments', + compute='_compute_attachment_ids', readonly=False, store=True) email_layout_xmlid = fields.Char('Email Notification Layout', copy=False) email_add_signature = fields.Boolean(default=True) # origin @@ -226,6 +227,48 @@ class MailComposer(models.TransientModel): if not composer.template_id: composer.body = False + @api.depends('composition_mode', 'model', 'res_domain', 'res_ids', 'template_id') + def _compute_attachment_ids(self): + """ Computation is based on template and composition mode. In monorecord + comment mode, template is used to generate attachments based on both + attachment_ids of template, and reports coming from report_template_ids. + Those are generated based on the current record to display. As template + generation returns a list of tuples, new attachments are created on + the fly during the compute. + + In batch or email mode, only attachment_ids from template are used on + the composer. Reports will be generated at sending time. + + When template is removed, attachments are reset. """ + for composer in self: + res_ids = composer._evaluate_res_ids() or [0] + if (composer.template_id.attachment_ids and + (composer.composition_mode == 'mass_mail' or composer.composition_batch)): + composer.attachment_ids = composer.template_id.attachment_ids + elif composer.template_id and composer.composition_mode == 'comment' and len(res_ids) == 1: + rendered_values = composer._generate_template_for_composer( + composer.template_id, + res_ids, + ('attachment_ids', 'attachments'), + )[res_ids[0]] + attachment_ids = rendered_values.get('attachment_ids') or [] + # transform attachments into attachment_ids; not attached to the + # document because this will be done further in the posting + # process, allowing to clean database if email not send + if rendered_values.get('attachments'): + attachment_ids += self.env['ir.attachment'].create([ + {'name': attach_fname, + 'datas': attach_datas, + 'res_model': 'mail.compose.message', + 'res_id': 0, + 'type': 'binary', # override default_type from context, possibly meant for another model! + } for attach_fname, attach_datas in rendered_values.pop('attachments') + ]).ids + if attachment_ids: + composer.attachment_ids = attachment_ids + elif not composer.template_id: + composer.attachment_ids = False + @api.depends('composition_mode', 'email_from', 'model', 'res_domain', 'res_ids', 'template_id') def _compute_authorship(self): @@ -510,8 +553,6 @@ class MailComposer(models.TransientModel): for field in () if template[field] ) - if template.attachment_ids: - values['attachment_ids'] = [att.id for att in template.attachment_ids] elif template_id and len(res_ids) <= 1: # render template (mono record, comment mode) and set it as composer values # trick to evaluate qweb even when having no records @@ -519,40 +560,21 @@ class MailComposer(models.TransientModel): values = self._generate_template_for_composer( self.env['mail.template'].browse(template_id), template_res_ids, - ('attachment_ids', - 'report_template_ids', - ) + () )[template_res_ids[0]] - # transform attachments into attachment_ids; not attached to the document because this will - # be done further in the posting process, allowing to clean database if email not send - attachment_ids = [] - Attachment = self.env['ir.attachment'] - for attach_fname, attach_datas in values.pop('attachments', []): - data_attach = { - 'name': attach_fname, - 'datas': attach_datas, - 'res_model': 'mail.compose.message', - 'res_id': 0, - 'type': 'binary', # override default_type from context, possibly meant for another model! - } - attachment_ids.append(Attachment.create(data_attach).id) - if values.get('attachment_ids', []) or attachment_ids: - values['attachment_ids'] = [Command.set(values.get('attachment_ids', []) + attachment_ids)] else: default_values = self.with_context( default_composition_mode=composition_mode, default_model=model, default_res_ids=res_ids - ).default_get(['attachment_ids', - 'composition_mode', + ).default_get(['composition_mode', 'model', 'parent_id', 'res_ids', ]) values = dict( (key, default_values[key]) - for key in ('attachment_ids', - ) if key in default_values) + for key in () if key in default_values) # This onchange should return command instead of ids for x2many field. values = self._convert_to_write(values) diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index c0c37ed58c8..1c0581c95b2 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -484,8 +484,6 @@ class TestComposerInternals(TestMailComposer): composer = self.env['mail.compose.message'].with_context(ctx).create({ 'body': '

Test Body

', }) - # currently onchange necessary - composer._onchange_template_id_wrapper() # values coming from template: attachment_ids + report in comment if composition_mode == 'comment' and not batch: @@ -515,29 +513,22 @@ class TestComposerInternals(TestMailComposer): else: self.assertEqual(composer.attachment_ids, attachs + extra_attach) - # update with template with void values: values are kept + # update with template with void values: values are kept, void + # value is not forced in rendering mode as well as when copying + # template values composer.write({'template_id': template_void.id}) - # currently onchange necessary - composer._onchange_template_id_wrapper() if composition_mode == 'comment' and not batch: - self.assertEqual(composer.attachment_ids, attachs + extra_attach + generated, - 'TODO: Values are kept (should be reset ?)') + self.assertEqual(composer.attachment_ids, attachs + extra_attach + generated) else: - self.assertEqual(composer.attachment_ids, attachs + extra_attach, - 'TODO: Values are kept (should be reset ?)') + self.assertEqual(composer.attachment_ids, attachs + extra_attach) - # reset template: values are kept + # reset template: values are reset composer.write({'template_id': False}) - # currently onchange necessary - composer._onchange_template_id_wrapper() - if composition_mode == 'comment' and not batch: - self.assertEqual(composer.attachment_ids, attachs + extra_attach + generated, - 'TODO: Values are kept (should be reset ?)') + self.assertFalse(composer.attachment_ids) else: - self.assertEqual(composer.attachment_ids, attachs + extra_attach, - 'TODO: Values are kept (should be reset ?)') + self.assertFalse(composer.attachment_ids) @users('employee') @mute_logger('odoo.addons.mail.models.mail_mail') @@ -1098,10 +1089,8 @@ class TestComposerInternals(TestMailComposer): self._get_web_context(self.test_record) ).create({ 'body': '

Template Body

', - 'partner_ids': [self.partner_employee_2.id], 'template_id': template_1.id, }) - composer._onchange_template_id_wrapper() composer._action_send_mail() self.assertEqual( From 65470cf180279972099b8b81cd086b26e7abfc2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 12 Jan 2023 16:37:30 +0000 Subject: [PATCH 24/32] [REF] mail: move composer 'force_send' field to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute field ``force_send`` in compute method. It is currently computed in a default_get override. It is set to True in mass mailing mode (send directly by default) and in monorecord comment mode (send notifications right now). Batch comment delays notification to use the email queue. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index d336f1fe790..492d37d2d52 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -69,14 +69,6 @@ class MailComposer(models.TransientModel): if 'create_uid' in fields_list and 'create_uid' not in result: result['create_uid'] = self.env.uid - # batch post mode by default use queues for notifications - if 'force_send' in fields_list and 'force_send' not in result: - result['force_send'] = ( - result.get('composition_mode') != 'comment' or - (not result.get('res_domain') and - len(self._parse_res_ids(result.get('res_ids') or [])) <= 1) - ) - return { fname: result[fname] for fname in result if fname in fields_list @@ -168,7 +160,8 @@ class MailComposer(models.TransientModel): compute="_compute_auto_delete_keep_log", readonly=False, store=True, help='Keep a copy of the email content if emails are removed (mass mailing only)') force_send = fields.Boolean( - 'Send mailing or notifications directly') + 'Send mailing or notifications directly', + compute='_compute_force_send', readonly=False, store=True) mail_server_id = fields.Many2one( 'ir.mail_server', string='Outgoing mail server', compute='_compute_mail_server_id', readonly=False, store=True) @@ -486,6 +479,15 @@ class MailComposer(models.TransientModel): toreset.auto_delete_keep_log = False (self - toreset).auto_delete_keep_log = True + @api.depends('composition_mode', 'model', 'res_domain', 'res_ids') + def _compute_force_send(self): + """ It is set to True in mass mailing mode (send directly by default) + and in monorecord comment mode (send notifications right now). Batch + comment delays notification to use the email queue. """ + for composer in self: + composer.force_send = (composer.composition_mode == 'mass_mail' or + not composer.composition_batch) + @api.depends('template_id') def _compute_mail_server_id(self): """ Copy value from template when updating it, if set on template. When From 29f14327cdefa35db80fbe18462cf889629441cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 12 Jan 2023 16:39:30 +0000 Subject: [PATCH 25/32] [REF] mail: move composer 'subtype_id' field to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute field ``subtype_id`` in compute method. It is currently always set to be a 'comment' subtype. However in mass mailing mode it is not used, as we do not use the value. Better void it directly. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/mail/wizard/mail_compose_message.py | 11 ++++++++++- addons/test_mail/tests/test_mail_composer.py | 17 +++++++---------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 492d37d2d52..5160f0ca6d1 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -128,7 +128,7 @@ class MailComposer(models.TransientModel): "message, comment for other messages such as user replies") subtype_id = fields.Many2one( 'mail.message.subtype', 'Subtype', ondelete='set null', - default=lambda self: self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment')) + compute="_compute_subtype_id", readonly=False, store=True) subtype_is_log = fields.Boolean('Is a log', compute='_compute_subtype_is_log') mail_activity_type_id = fields.Many2one('mail.activity.type', 'Mail Activity Type', ondelete='set null') # destination @@ -384,6 +384,15 @@ class MailComposer(models.TransientModel): if composer.model and len(res_ids) == 1: composer.record_name = self.env[composer.model].browse(res_ids).display_name + @api.depends('composition_mode') + def _compute_subtype_id(self): + """ Computation defaults from composition mode. Subtype is not used in + mass mail mode, and is comment for comment mode. """ + comment_composers = self.filtered(lambda comp: comp.composition_mode == 'comment') + if comment_composers: + comment_composers.subtype_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment') + (self - comment_composers).subtype_id = False + @api.depends('subtype_id') def _compute_subtype_is_log(self): """ In comment mode, tells whether the subtype is a note. Subtype has diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 1c0581c95b2..77d892f61a9 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -343,7 +343,7 @@ class TestComposerForm(TestMailComposer): self.assertEqual(sorted(literal_eval(composer_form.res_ids)), sorted(self.test_records.ids)) self.assertFalse(composer_form.scheduled_date) self.assertFalse(composer_form.subject, 'MailComposer: mass mode should have void default subject if no template') - self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) + self.assertFalse(composer_form.subtype_id, 'MailComposer: subtype is not used in mail mode') self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') @users('employee') @@ -372,7 +372,7 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.scheduled_date, self.template.scheduled_date) self.assertEqual(composer_form.subject, self.template.subject, 'MailComposer: mass mode should have template raw subject if template') - self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) + self.assertFalse(composer_form.subtype_id, 'MailComposer: subtype is not used in mail mode') self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') @users('employee') @@ -405,7 +405,7 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.scheduled_date, self.template.scheduled_date) self.assertEqual(composer_form.subject, self.template.subject, 'MailComposer: mass mode should have template raw subject if template') - self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) + self.assertFalse(composer_form.subtype_id, 'MailComposer: subtype is not used in mail mode') self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') @users('employee') @@ -436,7 +436,7 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.scheduled_date, self.template.scheduled_date) self.assertEqual(composer_form.subject, self.template.subject, 'MailComposer: mass mode should have template raw subject if template') - self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) + self.assertFalse(composer_form.subtype_id, 'MailComposer: subtype is not used in mail mode') self.assertFalse(composer_form.subtype_is_log, 'MailComposer: subtype is log has no meaning in mail mode') @@ -654,18 +654,17 @@ class TestComposerInternals(TestMailComposer): if composition_mode == 'comment': self.assertTrue(composer.auto_delete, 'By default, remove notification emails') self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') + self.assertEqual(composer.subtype_id, self.env.ref('mail.mt_comment')) else: self.assertFalse(composer.auto_delete, 'By default, keep mailing emails') self.assertFalse(composer.auto_delete_keep_log, 'Emails are not unlinked, logs are already kept') + self.assertFalse(composer.subtype_id) self.assertTrue(composer.email_add_signature) self.assertEqual(composer.email_layout_xmlid, 'mail.test_layout') self.assertEqual(composer.message_type, 'comment') - self.assertEqual(composer.subtype_id, self.env.ref('mail.mt_comment')) # changing template should update its content composer.write({'template_id': self.template.id}) - # currently onchange necessary - composer._onchange_template_id_wrapper() # values come from template if composition_mode == 'comment': @@ -681,7 +680,7 @@ class TestComposerInternals(TestMailComposer): self.assertTrue(composer.email_add_signature, 'TODO: should be False as template negates this config') self.assertEqual(composer.email_layout_xmlid, 'mail.test_layout') self.assertEqual(composer.message_type, 'comment') - self.assertEqual(composer.subtype_id, self.env.ref('mail.mt_comment')) + self.assertFalse(composer.subtype_id) # manual update composer.write({ @@ -695,8 +694,6 @@ class TestComposerInternals(TestMailComposer): # update with template with void values: void value is forced for # booleans, cannot distinguish composer.write({'template_id': template_falsy.id}) - # currently onchange necessary - composer._onchange_template_id_wrapper() if composition_mode == 'comment': self.assertFalse(composer.auto_delete) From 300ae603d110fc3c7fb5a69b4c662f49e3c85975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 3 Nov 2021 14:20:51 +0000 Subject: [PATCH 26/32] [REF] mail: remove composer onchange, now unnecessary PURPOSE Purpose of this task is to remove the _onchange_template_id method on composer model. Split it into editable computed stored fields. It gives a better control of value generation and avoid having to call the onchange when composer is invoked in code. SPECIFICATIONS Remove onchange as it is not used anymore. All fields have been converted. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/account/wizard/account_invoice_send.py | 2 - .../models/account_edi_document.py | 2 +- addons/mail/models/mail_thread.py | 31 ++------- addons/mail/wizard/mail_compose_message.py | 69 +------------------ addons/sale/models/res_company.py | 12 +--- addons/test_mail/tests/test_performance.py | 4 -- addons/test_mail_full/tests/test_portal.py | 2 - 7 files changed, 10 insertions(+), 112 deletions(-) diff --git a/addons/account/wizard/account_invoice_send.py b/addons/account/wizard/account_invoice_send.py index ccf4863cdcb..dc42d1541d6 100644 --- a/addons/account/wizard/account_invoice_send.py +++ b/addons/account/wizard/account_invoice_send.py @@ -78,7 +78,6 @@ 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() @api.onchange('is_email') def onchange_is_email(self): @@ -94,7 +93,6 @@ class AccountInvoiceSend(models.TransientModel): self.composer_id.composition_mode = 'comment' if len(res_ids) == 1 else 'mass_mail' self.composer_id.template_id = self.template_id.id self._compute_composition_mode() - self.composer_id._onchange_template_id_wrapper() @api.onchange('is_email') def _compute_invoice_without_email(self): diff --git a/addons/account_edi/models/account_edi_document.py b/addons/account_edi/models/account_edi_document.py index 2a7de3fc4b9..bf807d36b16 100644 --- a/addons/account_edi/models/account_edi_document.py +++ b/addons/account_edi/models/account_edi_document.py @@ -262,7 +262,7 @@ class AccountEdiDocument(models.Model): Can be overridden where e.g. a zip-file needs to be sent with the individual files instead of the entire zip IMPORTANT: * If the attachment's id is returned, no new attachment will be created, the existing one on the move is linked - to the wizard (see _onchange_template_id in mail.compose.message). + to the wizard (see computed attachment_ids field in mail.compose.message). * If the attachment's content is returned, a new one is created and linked to the wizard. Thus, when sending the mail (clicking on 'send & print' in the wizard), a new attachment is added to the move (see _action_send_mail in mail.compose.message). diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 21a5f9d77c3..f65807673d2 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2193,18 +2193,7 @@ class MailThread(models.AbstractModel): composer = self.env['mail.compose.message'].with_context( **composer_ctx ).create(composer_values) - # Simulate the onchange (like trigger in form the view) only - # when having a template in single-email mode - if template: - update_values = composer._onchange_template_id( - template.id, - 'mass_mail', - self._name, - subset.ids, - )['value'] - composer.write(update_values) - - mails_as_sudo, _messages_as_sudo = composer._action_send_mail(auto_commit=auto_commit) + mails_as_sudo, _messages = composer._action_send_mail(auto_commit=auto_commit) mails_su += mails_as_sudo return mails_su @@ -2262,7 +2251,7 @@ class MailThread(models.AbstractModel): if not subtype_id: subtype_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note') - messages = self.env['mail.message'] + messages_all = self.env['mail.message'] for record in self: if template: composer = self.env['mail.compose.message'].with_context( @@ -2275,26 +2264,16 @@ class MailThread(models.AbstractModel): 'subtype_id': subtype_id, **kwargs, }) - - # Simulate the onchange (like trigger in form the view) to do the - # rendering in mono-record mode - update_values = composer._onchange_template_id( - template.id, - 'comment', - self._name, - record.ids, - )['value'] - composer.write(update_values) _mails_as_sudo, messages = composer._action_send_mail() - messages += messages + messages_all += messages else: - messages += record.message_post( + messages_all += record.message_post( body=bodies[record.id], message_type=message_type, subtype_id=subtype_id, **kwargs ) - return messages + return messages_all @api.returns('mail.message', lambda value: value.id) def message_notify(self, *, diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 5160f0ca6d1..3c419c50da0 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -240,7 +240,6 @@ class MailComposer(models.TransientModel): composer.attachment_ids = composer.template_id.attachment_ids elif composer.template_id and composer.composition_mode == 'comment' and len(res_ids) == 1: rendered_values = composer._generate_template_for_composer( - composer.template_id, res_ids, ('attachment_ids', 'attachments'), )[res_ids[0]] @@ -451,7 +450,6 @@ class MailComposer(models.TransientModel): and not composer.composition_batch): res_ids = composer._evaluate_res_ids() or [0] rendered_values = composer._generate_template_for_composer( - composer.template_id, res_ids, {'email_cc', 'email_to', 'partner_ids'}, find_or_create_partners=True, @@ -533,65 +531,6 @@ class MailComposer(models.TransientModel): non_mass_mail.can_edit_body = True super(MailComposer, self - non_mass_mail)._compute_can_edit_body() - # 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._evaluate_res_ids(), - self.res_domain, - )['value'] - for fname, value in values.items(): - setattr(self, fname, value) - - def _onchange_template_id(self, template_id, composition_mode, model, res_ids, res_domain=None): - """ Perform the onchange. - - * mass_mailing or comment in batch: we cannot render, so return the - template values - * normal mode: return rendered values - -> for x2many field, this onchange return command instead of ids - """ - if template_id and (composition_mode == 'mass_mail' or res_domain or len(res_ids) > 1): - # copy raw template values (not rendered due to mass mode) - template = self.env['mail.template'].browse(template_id) - values = dict( - (field, template[field]) - for field in () - if template[field] - ) - elif template_id and len(res_ids) <= 1: - # render template (mono record, comment mode) and set it as composer values - # trick to evaluate qweb even when having no records - template_res_ids = res_ids if res_ids else [0] - values = self._generate_template_for_composer( - self.env['mail.template'].browse(template_id), - template_res_ids, - () - )[template_res_ids[0]] - else: - default_values = self.with_context( - default_composition_mode=composition_mode, - default_model=model, - default_res_ids=res_ids - ).default_get(['composition_mode', - 'model', - 'parent_id', - 'res_ids', - ]) - values = dict( - (key, default_values[key]) - for key in () if key in default_values) - - # This onchange should return command instead of ids for x2many field. - values = self._convert_to_write(values) - - return {'value': values} - # ------------------------------------------------------------ # CRUD / ORM # ------------------------------------------------------------ @@ -746,7 +685,6 @@ class MailComposer(models.TransientModel): # generate the saved template record.write({'template_id': template.id}) - record._onchange_template_id_wrapper() return _reopen(self, record.id, record.model, context=self.env.context) # ------------------------------------------------------------ @@ -918,7 +856,6 @@ class MailComposer(models.TransientModel): # generate template-based values if self.template_id: template_values = self._generate_template_for_composer( - self.template_id, res_ids, ('attachment_ids', 'email_to', @@ -1092,13 +1029,11 @@ class MailComposer(models.TransientModel): return mail_values_dict - def _generate_template_for_composer(self, template, res_ids, render_fields, + def _generate_template_for_composer(self, res_ids, render_fields, find_or_create_partners=True): """ Generate values based on template and relevant values for the mail.compose.message wizard. - :param record template: a mail template, as during onchange mode it - may not be set on self (to remove when removing the onchange); :param list res_ids: list of record IDs on which template is rendered; :param list render_fields: list of fields to render on template; :param boolean find_or_create_partners: transform emails into partners @@ -1124,7 +1059,7 @@ class MailComposer(models.TransientModel): 'partner_ids': 'partner_to', } template_fields = {mapping.get(fname, fname) for fname in render_fields} - template_values = template._generate_template( + template_values = self.template_id._generate_template( res_ids, template_fields, find_or_create_partners=True diff --git a/addons/sale/models/res_company.py b/addons/sale/models/res_company.py index 3380057638c..3e0f2008753 100644 --- a/addons/sale/models/res_company.py +++ b/addons/sale/models/res_company.py @@ -110,7 +110,7 @@ class ResCompany(models.Model): sample_sales_order = self._get_sample_sales_order() template = self.env.ref('sale.email_template_edi_sale', False) - message_composer = self.env['mail.compose.message'].with_context( + self.env['mail.compose.message'].with_context( mark_so_as_sent=True, default_email_layout_xmlid='mail.mail_notification_layout_with_responsible_signature', proforma=self.env.context.get('proforma', False), @@ -120,15 +120,7 @@ class ResCompany(models.Model): 'template_id': template.id if template else False, 'model': sample_sales_order._name, 'composition_mode': 'comment', - }) - - # Simulate the onchange (like trigger in form the view) - update_values = message_composer._onchange_template_id( - template.id, 'comment', sample_sales_order._name, sample_sales_order.ids - )['value'] - message_composer.write(update_values) - - message_composer._action_send_mail() + })._action_send_mail() self.set_onboarding_step_done('sale_onboarding_sample_quotation_state') diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index e159c0de891..26ae19e3ef8 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -413,7 +413,6 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_res_ids': test_records.ids, 'default_template_id': test_template.id, }).create({}) - composer._onchange_template_id_wrapper() with self.assertQueryCount(admin=141, employee=141), self.mock_mail_gateway(): composer._action_send_mail() @@ -454,7 +453,6 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_res_ids': test_record.ids, 'default_template_id': test_template.id, }).create({}) - composer._onchange_template_id_wrapper() with self.assertQueryCount(admin=31, employee=31): composer._action_send_mail() @@ -479,7 +477,6 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_res_ids': test_record.ids, 'default_template_id': test_template.id, }).create({}) - composer._onchange_template_id_wrapper() with self.assertQueryCount(admin=44, employee=44): composer._action_send_mail() @@ -888,7 +885,6 @@ class TestMailComplexPerformance(BaseMailPerformance): 'default_res_ids': test_records.ids, 'default_template_id': test_template.id, }).create({}) - composer._onchange_template_id_wrapper() with self.assertQueryCount(admin=131, employee=131): messages_as_sudo = test_records.message_post_with_source( diff --git a/addons/test_mail_full/tests/test_portal.py b/addons/test_mail_full/tests/test_portal.py index a0f72c3d78a..08e7503ebe0 100644 --- a/addons/test_mail_full/tests/test_portal.py +++ b/addons/test_mail_full/tests/test_portal.py @@ -320,8 +320,6 @@ class TestPortalFlow(MailCommon, HttpCase): Other tests below check that that same link has the correct behavior. This test follows the common use case by using a template while the next send the mail without a template.""" composer = self._get_composer_with_context(self.mail_template.id).create({}) - # currently onchange necessary - composer._onchange_template_id_wrapper() with self.mock_mail_gateway(mail_unlink_sent=True): composer._action_send_mail() From 6218d84f04b51b4afe72cb98bdbb603b46af27f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 9 Dec 2022 17:10:10 +0000 Subject: [PATCH 27/32] [IMP] various: update query counters Update query counters now that fields are editable computed fields. Some counters are higher as the usage of composer in code is now closer to the usage in view, with fields being correctly computed. Some other counters are a bit lessened, but it globally stays the same. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- addons/test_mail/tests/test_performance.py | 82 +++++++++---------- .../tests/test_performance.py | 4 +- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 26ae19e3ef8..914f0542f45 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -216,19 +216,19 @@ class TestBaseMailPerformance(BaseMailPerformance): @warmup def test_create_mail_with_tracking(self): """ Create records inheriting from 'mail.thread' (with field tracking). """ - with self.assertQueryCount(admin=8, demo=8): + with self.assertQueryCount(admin=7, demo=7): self.env['mail.performance.thread'].create({'name': 'X'}) @users('admin', 'employee') @warmup def test_create_mail_simple(self): - with self.assertQueryCount(admin=7, employee=7): + with self.assertQueryCount(admin=6, employee=6): self.env['mail.test.simple'].create({'name': 'Test'}) @users('admin', 'employee') @warmup def test_create_mail_simple_multi(self): - with self.assertQueryCount(admin=7, employee=7): + with self.assertQueryCount(admin=6, employee=6): self.env['mail.test.simple'].create([{'name': 'Test'}] * 5) @users('admin', 'employee') @@ -256,7 +256,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_adv_activity(self): model = self.env['mail.test.activity'] - with self.assertQueryCount(admin=7, employee=7): + with self.assertQueryCount(admin=6, employee=6): model.create({'name': 'Test'}) @users('admin', 'employee') @@ -321,7 +321,7 @@ class TestMailAPIPerformance(BaseMailPerformance): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(admin=19, employee=19): # com+tm 18/18 + with self.assertQueryCount(admin=18, employee=18): # com+tm 17/17 record.action_close('Dupe feedback', attachment_ids=attachments.ids) # notifications @@ -337,7 +337,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_mail_composer(self): test_record, _test_template = self._create_test_records() customer_id = self.customer.id - with self.assertQueryCount(admin=4, employee=4): + with self.assertQueryCount(admin=3, employee=3): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -347,7 +347,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer_id)], }) - with self.assertQueryCount(admin=32, employee=32): + with self.assertQueryCount(admin=29, employee=29): composer._action_send_mail() @users('admin', 'employee') @@ -357,7 +357,7 @@ class TestMailAPIPerformance(BaseMailPerformance): test_record, _test_template = self._create_test_records() customer = self.env['res.partner'].browse(self.customer.ids) attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals) - with self.assertQueryCount(admin=5, employee=5): + with self.assertQueryCount(admin=3, employee=3): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -368,7 +368,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer.id)], }) - with self.assertQueryCount(admin=37, employee=37): + with self.assertQueryCount(admin=33, employee=33): composer._action_send_mail() @users('admin', 'employee') @@ -378,7 +378,7 @@ class TestMailAPIPerformance(BaseMailPerformance): test_record, _test_template = self._create_test_records() customer = self.env['res.partner'].browse(self.customer.ids) attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals) - with self.assertQueryCount(admin=13, employee=13): # tm 12/12 + with self.assertQueryCount(admin=11, employee=11): composer_form = Form( self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', @@ -392,7 +392,7 @@ class TestMailAPIPerformance(BaseMailPerformance): composer_form.attachment_ids.add(attachment) composer = composer_form.save() - with self.assertQueryCount(admin=47, employee=47): # tm+com 46/46 + with self.assertQueryCount(admin=43, employee=43): # tm+com 42/42 composer._action_send_mail() # notifications @@ -406,7 +406,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_mail_composer_mass_w_template(self): _partners, test_records, test_template = self._create_test_records_for_batch() - with self.assertQueryCount(admin=4, employee=4): + with self.assertQueryCount(admin=3, employee=3): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'mass_mail', 'default_model': test_records._name, @@ -414,7 +414,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_template_id': test_template.id, }).create({}) - with self.assertQueryCount(admin=141, employee=141), self.mock_mail_gateway(): + with self.assertQueryCount(admin=118, employee=121), self.mock_mail_gateway(): composer._action_send_mail() self.assertEqual(len(self._new_mails), 10) @@ -425,7 +425,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_mail_composer_nodelete(self): test_record, _test_template = self._create_test_records() customer_id = self.customer.id - with self.assertQueryCount(admin=4, employee=4): + with self.assertQueryCount(admin=3, employee=3): composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -436,7 +436,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer_id)], }) - with self.assertQueryCount(admin=32, employee=32): + with self.assertQueryCount(admin=29, employee=29): composer._action_send_mail() @users('admin', 'employee') @@ -454,7 +454,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_template_id': test_template.id, }).create({}) - with self.assertQueryCount(admin=31, employee=31): + with self.assertQueryCount(admin=28, employee=28): composer._action_send_mail() # notifications @@ -470,7 +470,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_mail_composer_w_template_attachments(self): test_record, test_template = self._create_test_records() - with self.assertQueryCount(admin=26, employee=26): # tm 16/16 / com 25/25 + with self.assertQueryCount(admin=25, employee=25): # tm 15/15 / com 24/24 composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -478,7 +478,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_template_id': test_template.id, }).create({}) - with self.assertQueryCount(admin=44, employee=44): + with self.assertQueryCount(admin=39, employee=39): composer._action_send_mail() # notifications @@ -499,7 +499,7 @@ class TestMailAPIPerformance(BaseMailPerformance): test_template.write({'attachment_ids': [(5, 0)]}) customer = self.env['res.partner'].browse(self.customer.ids) - with self.assertQueryCount(admin=37, employee=37): # tm 26/26 / com 36/36 + with self.assertQueryCount(admin=37, employee=37): # tm 27/27 / com 36/36 composer_form = Form( self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', @@ -510,7 +510,7 @@ class TestMailAPIPerformance(BaseMailPerformance): ) composer = composer_form.save() - with self.assertQueryCount(admin=41, employee=41): + with self.assertQueryCount(admin=38, employee=38): composer._action_send_mail() # notifications @@ -529,7 +529,7 @@ class TestMailAPIPerformance(BaseMailPerformance): test_record, test_template = self._create_test_records() customer = self.env['res.partner'].browse(self.customer.ids) - with self.assertQueryCount(admin=38, employee=38): # tm 27/27 / com 37/37 + with self.assertQueryCount(admin=37, employee=37): # tm 27/27 / com 36/36 composer_form = Form( self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', @@ -540,7 +540,7 @@ class TestMailAPIPerformance(BaseMailPerformance): ) composer = composer_form.save() - with self.assertQueryCount(admin=64, employee=64): + with self.assertQueryCount(admin=59, employee=59): composer._action_send_mail() # notifications @@ -567,7 +567,7 @@ class TestMailAPIPerformance(BaseMailPerformance): # use another user already pre-defined with the email notification type, # so the ormcache is preserved. record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=28, employee=28): + with self.assertQueryCount(admin=25, employee=25): record.write({ 'user_id': self.user_test_email.id, }) @@ -576,7 +576,7 @@ class TestMailAPIPerformance(BaseMailPerformance): @warmup def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=20, employee=20): + with self.assertQueryCount(admin=18, employee=18): record.write({ 'user_id': self.user_test_inbox.id, }) @@ -650,7 +650,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=26, employee=26): + with self.assertQueryCount(admin=24, employee=24): record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -662,7 +662,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_message_post_one_inbox_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=18, employee=18): + with self.assertQueryCount(admin=17, employee=17): record.message_post( body='

Test Post Performances with an inbox ping

', partner_ids=self.user_test.partner_id.ids, @@ -675,7 +675,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_message_subscribe_default(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=6, employee=6): + with self.assertQueryCount(admin=5, employee=5): record.message_subscribe(partner_ids=self.user_test.partner_id.ids) with self.assertQueryCount(admin=3, employee=3): @@ -688,7 +688,7 @@ class TestMailAPIPerformance(BaseMailPerformance): record = self.env['mail.test.simple'].create({'name': 'Test'}) subtype_ids = (self.env.ref('test_mail.st_mail_test_simple_external') | self.env.ref('mail.mt_comment')).ids - with self.assertQueryCount(admin=5, employee=5): + with self.assertQueryCount(admin=4, employee=4): record.message_subscribe(partner_ids=self.user_test.partner_id.ids, subtype_ids=subtype_ids) with self.assertQueryCount(admin=2, employee=2): @@ -844,7 +844,7 @@ class TestMailComplexPerformance(BaseMailPerformance): record = self.container.with_user(self.env.user) # about 20 (19?) queries per additional customer group - with self.assertQueryCount(admin=36, employee=36): + with self.assertQueryCount(admin=35, employee=34): record.message_post( body='

Test Post Performances

', message_type='comment', @@ -862,7 +862,7 @@ class TestMailComplexPerformance(BaseMailPerformance): template = self.env.ref('test_mail.mail_test_container_tpl') # about 20 (19 ?) queries per additional customer group - with self.assertQueryCount(admin=45, employee=45): + with self.assertQueryCount(admin=42, employee=41): record.message_post_with_source( template, message_type='comment', @@ -878,8 +878,8 @@ class TestMailComplexPerformance(BaseMailPerformance): def test_complex_message_post_view(self): _partners, test_records, test_template = self._create_test_records_for_batch() - with self.assertQueryCount(admin=4, employee=4): - composer = self.env['mail.compose.message'].with_context({ + with self.assertQueryCount(admin=3, employee=3): + _composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'mass_mail', 'default_model': test_records._name, 'default_res_ids': test_records.ids, @@ -913,7 +913,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(rec1.message_partner_ids, self.env.user.partner_id | self.user_portal.partner_id) # subscribe new followers with forced given subtypes - with self.assertQueryCount(admin=4, employee=4): + with self.assertQueryCount(admin=3, employee=3): rec.message_subscribe( partner_ids=pids[:4], subtype_ids=subtype_ids @@ -922,7 +922,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(rec1.message_partner_ids, self.env.user.partner_id | self.user_portal.partner_id | self.partners[:4]) # subscribe existing and new followers with force=False, meaning only some new followers will be added - with self.assertQueryCount(admin=5, employee=5): + with self.assertQueryCount(admin=4, employee=4): rec.message_subscribe( partner_ids=pids[:6], subtype_ids=None @@ -931,7 +931,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.assertEqual(rec1.message_partner_ids, self.env.user.partner_id | self.user_portal.partner_id | self.partners[:6]) # subscribe existing and new followers with force=True, meaning all will have the same subtypes - with self.assertQueryCount(admin=4, employee=4): + with self.assertQueryCount(admin=3, employee=3): rec.message_subscribe( partner_ids=pids, subtype_ids=subtype_ids @@ -952,7 +952,7 @@ class TestMailComplexPerformance(BaseMailPerformance): }) rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=26, employee=26): + with self.assertQueryCount(admin=23, employee=23): rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) # write tracking message @@ -972,7 +972,7 @@ class TestMailComplexPerformance(BaseMailPerformance): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(admin=58, employee=58): # tm 58/58 + with self.assertQueryCount(admin=52, employee=52): rec = self.env['mail.test.ticket'].create({ 'name': 'Test', 'container_id': container_id, @@ -1001,7 +1001,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) self.assertEqual(len(rec1.message_ids), 1) - with self.assertQueryCount(admin=36, employee=36): + with self.assertQueryCount(admin=33, employee=33): rec.write({ 'name': 'Test2', 'container_id': self.container.id, @@ -1038,7 +1038,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=36, employee=36): + with self.assertQueryCount(admin=33, employee=33): rec.write({ 'name': 'Test2', 'container_id': container_id, @@ -1071,7 +1071,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(admin=22, employee=22): + with self.assertQueryCount(admin=21, employee=21): rec.write({ 'name': 'Test2', 'customer_id': customer_id, @@ -1301,7 +1301,7 @@ class TestMailHeavyPerformancePost(BaseMailPerformance): attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals) # enable_logging = self.cr._enable_logging() if self.warm else nullcontext() # with self.assertQueryCount(employee=49), enable_logging: - with self.assertQueryCount(employee=49): + with self.assertQueryCount(employee=45): record_container.with_context({}).message_post( body='

Test body

', subject='Test Subject', diff --git a/addons/test_mass_mailing/tests/test_performance.py b/addons/test_mass_mailing/tests/test_performance.py index 1c8fd0cc755..cb1e5c9b8ae 100644 --- a/addons/test_mass_mailing/tests/test_performance.py +++ b/addons/test_mass_mailing/tests/test_performance.py @@ -47,7 +47,7 @@ class TestMassMailPerformance(TestMassMailPerformanceBase): }) # runbot needs +2 compared to local - with self.assertQueryCount(__system__=426, marketing=427): # tm 424/425 + with self.assertQueryCount(__system__=427, marketing=428): # tm 425/426 mailing.action_send_mail() self.assertEqual(mailing.sent, 50) @@ -94,7 +94,7 @@ class TestMassMailBlPerformance(TestMassMailPerformanceBase): }) # runbot needs +2 compared to local - with self.assertQueryCount(__system__=488, marketing=489): # tm 486/487 + with self.assertQueryCount(__system__=489, marketing=490): # tm 487/488 mailing.action_send_mail() self.assertEqual(mailing.sent, 50) From c60612068e31a4cd18a70112ae25cd6e5a27a86e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 24 Jan 2023 16:02:11 +0000 Subject: [PATCH 28/32] [REF] hr_recruitment_survey: rename 'applicant_id' o2m field 'applicant_id' name is misleading as one could think it is a m2o. Better name it 'applicant_ids' to better highlight it is a o2m. Moreover it avoid conflicts with default value 'default_applicant_id' propagated from some views (notably jobs kanban view). Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#107356 --- .../hr_recruitment_survey/models/survey_invite.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/addons/hr_recruitment_survey/models/survey_invite.py b/addons/hr_recruitment_survey/models/survey_invite.py index 61d7b9a064d..0cd315dea9c 100644 --- a/addons/hr_recruitment_survey/models/survey_invite.py +++ b/addons/hr_recruitment_survey/models/survey_invite.py @@ -31,18 +31,12 @@ class SurveyInvite(models.TransientModel): class SurveyUserInput(models.Model): _inherit = "survey.user_input" - applicant_id = fields.One2many('hr.applicant', 'response_id', string='Applicant') + applicant_ids = fields.One2many('hr.applicant', 'response_id', string='Applicant') def _mark_done(self): odoobot = self.env.ref('base.partner_root') for user_input in self: - if user_input.applicant_id: - body = _('The applicant "%s" has finished the survey.', user_input.applicant_id.partner_name) - user_input.applicant_id.message_post(body=body, author_id=odoobot.id) + if user_input.applicant_ids: + body = _('The applicant "%s" has finished the survey.', user_input.applicant_ids[:1].partner_name) + user_input.applicant_ids.message_post(body=body, author_id=odoobot.id) return super()._mark_done() - - @api.model_create_multi - def create(self, values_list): - if 'default_applicant_id' in self.env.context: - self = self.with_context(default_applicant_id=False) - return super().create(values_list) From a166aafb36b43242e82a5b0a08704684c4ff2c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 25 Jan 2023 15:15:18 +0000 Subject: [PATCH 29/32] [MOV] hr_recruitment_survey: split survey models into separate files Just reordering a bit code while passing by. Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#107356 --- addons/hr_recruitment_survey/__init__.py | 1 + addons/hr_recruitment_survey/models/__init__.py | 2 +- .../models/survey_user_input.py | 17 +++++++++++++++++ addons/hr_recruitment_survey/wizard/__init__.py | 3 +++ .../{models => wizard}/survey_invite.py | 16 +--------------- 5 files changed, 23 insertions(+), 16 deletions(-) create mode 100644 addons/hr_recruitment_survey/models/survey_user_input.py create mode 100644 addons/hr_recruitment_survey/wizard/__init__.py rename addons/hr_recruitment_survey/{models => wizard}/survey_invite.py (64%) diff --git a/addons/hr_recruitment_survey/__init__.py b/addons/hr_recruitment_survey/__init__.py index cde864bae21..35e7c9600c5 100644 --- a/addons/hr_recruitment_survey/__init__.py +++ b/addons/hr_recruitment_survey/__init__.py @@ -1,3 +1,4 @@ # -*- coding: utf-8 -*- from . import models +from . import wizard diff --git a/addons/hr_recruitment_survey/models/__init__.py b/addons/hr_recruitment_survey/models/__init__.py index c1e428c5a64..275bb0515ea 100644 --- a/addons/hr_recruitment_survey/models/__init__.py +++ b/addons/hr_recruitment_survey/models/__init__.py @@ -3,4 +3,4 @@ from . import hr_job from . import hr_applicant -from . import survey_invite +from . import survey_user_input diff --git a/addons/hr_recruitment_survey/models/survey_user_input.py b/addons/hr_recruitment_survey/models/survey_user_input.py new file mode 100644 index 00000000000..743f8a0b6fd --- /dev/null +++ b/addons/hr_recruitment_survey/models/survey_user_input.py @@ -0,0 +1,17 @@ +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import fields, models, _ + + +class SurveyUserInput(models.Model): + _inherit = "survey.user_input" + + applicant_ids = fields.One2many('hr.applicant', 'response_id', string='Applicant') + + def _mark_done(self): + odoobot = self.env.ref('base.partner_root') + for user_input in self: + if user_input.applicant_ids: + body = _('The applicant "%s" has finished the survey.', user_input.applicant_ids[:1].partner_name) + user_input.applicant_ids.message_post(body=body, author_id=odoobot.id) + return super()._mark_done() diff --git a/addons/hr_recruitment_survey/wizard/__init__.py b/addons/hr_recruitment_survey/wizard/__init__.py new file mode 100644 index 00000000000..7d07eb5367d --- /dev/null +++ b/addons/hr_recruitment_survey/wizard/__init__.py @@ -0,0 +1,3 @@ +# -*- coding: utf-8 -*- + +from . import survey_invite diff --git a/addons/hr_recruitment_survey/models/survey_invite.py b/addons/hr_recruitment_survey/wizard/survey_invite.py similarity index 64% rename from addons/hr_recruitment_survey/models/survey_invite.py rename to addons/hr_recruitment_survey/wizard/survey_invite.py index 0cd315dea9c..aba0ada21fe 100644 --- a/addons/hr_recruitment_survey/models/survey_invite.py +++ b/addons/hr_recruitment_survey/wizard/survey_invite.py @@ -1,6 +1,6 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import api, fields, models, _ +from odoo import fields, models, _ from odoo.tools.misc import clean_context @@ -26,17 +26,3 @@ class SurveyInvite(models.TransientModel): body = '

%s

' % content self.applicant_id.message_post(body=body) return super().action_invite() - - -class SurveyUserInput(models.Model): - _inherit = "survey.user_input" - - applicant_ids = fields.One2many('hr.applicant', 'response_id', string='Applicant') - - def _mark_done(self): - odoobot = self.env.ref('base.partner_root') - for user_input in self: - if user_input.applicant_ids: - body = _('The applicant "%s" has finished the survey.', user_input.applicant_ids[:1].partner_name) - user_input.applicant_ids.message_post(body=body, author_id=odoobot.id) - return super()._mark_done() From fb0359e750a73519eb3699ea4ff726f37cc69892 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 12 Jan 2023 17:44:12 +0000 Subject: [PATCH 30/32] [IMP] account: update account.invoice.send Due to new composer, it is a bit less necessary to create them on the fly randomly. Some quick code cleaning is performed to better understand code and variables. Notably 'default_get' override was a bit messy and should not return unwanted values. Task-3093257 (Mail: The Composer Update) Task-3149286 (Account: Cleanup account.invoice.send wizard) Part-of: odoo/odoo#107356 --- addons/account/wizard/account_invoice_send.py | 24 +++++++++---------- .../wizard/account_invoice_send_views.xml | 9 ++++++- .../tests/test_mail_performance.py | 17 +++++++++++-- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/addons/account/wizard/account_invoice_send.py b/addons/account/wizard/account_invoice_send.py index dc42d1541d6..b1d64c971e9 100644 --- a/addons/account/wizard/account_invoice_send.py +++ b/addons/account/wizard/account_invoice_send.py @@ -31,23 +31,23 @@ class AccountInvoiceSend(models.TransientModel): readonly=True) @api.model - def default_get(self, fields): - res = super(AccountInvoiceSend, self).default_get(fields) - res_ids = self._context.get('active_ids') + def default_get(self, fields_list): + result = super().default_get(fields_list) + res_ids = self.env.context.get('active_ids') invoices = self.env['account.move'].browse(res_ids).filtered(lambda move: move.is_invoice(include_receipts=True)) if not invoices: raise UserError(_("You can only send invoices.")) - composer = self.env['mail.compose.message'].create({ - 'composition_mode': 'comment' if len(res_ids) == 1 else 'mass_mail', - 'subtype_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment'), - }) - res.update({ - 'invoice_ids': res_ids, - 'composer_id': composer.id, - }) - return res + if 'invoice_ids' in fields_list and not result.get('invoice_ids'): + result['invoice_ids'] = res_ids + if 'composer_id' in fields_list and not result.get('composer_id'): + composer = self.env['mail.compose.message'].create({ + 'composition_mode': 'comment' if len(res_ids) == 1 else 'mass_mail', + 'subtype_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment'), + }) + result['composer_id'] = composer.id + return result @api.onchange('invoice_ids') def _compute_composition_mode(self): diff --git a/addons/account/wizard/account_invoice_send_views.xml b/addons/account/wizard/account_invoice_send_views.xml index f37c5006cf4..46de03f03ed 100644 --- a/addons/account/wizard/account_invoice_send_views.xml +++ b/addons/account/wizard/account_invoice_send_views.xml @@ -14,10 +14,17 @@ + - + + + + + + +
diff --git a/addons/l10n_generic_coa/tests/test_mail_performance.py b/addons/l10n_generic_coa/tests/test_mail_performance.py index eb32053b6f4..b52914c6597 100644 --- a/addons/l10n_generic_coa/tests/test_mail_performance.py +++ b/addons/l10n_generic_coa/tests/test_mail_performance.py @@ -61,7 +61,7 @@ class BaseMailAccountPerformance(AccountTestInvoicingCommon, MailCommon): ]) ], 'login': 'user_account', - 'name': 'Ernest Employee', + 'name': 'Ernest Employee Account', 'notification_type': 'inbox', 'signature': '--\nErnest', }) @@ -77,7 +77,7 @@ class BaseMailAccountPerformance(AccountTestInvoicingCommon, MailCommon): ]) ], 'login': 'user_account_other', - 'name': 'Eglantine Employee', + 'name': 'Eglantine Employee AccountOther', 'notification_type': 'inbox', 'signature': '--\nEglantine', }) @@ -134,6 +134,19 @@ class BaseMailAccountPerformance(AccountTestInvoicingCommon, MailCommon): class TestAccountComposerPerformance(BaseMailAccountPerformance): """ Test performance of custom composer for moves. """ + def test_assert_initial_values(self): + """ Test initial values to ease understanding of results and notifications """ + for move in self.test_account_moves: + with self.subTest(move=move): + self.assertEqual( + move.invoice_user_id, + self.user_account_other, + ) + self.assertEqual( + move.message_partner_ids, + self.user_accountman.partner_id, + ) + @users('user_account') @warmup @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') From 40456d7445d473b1c9fad8a6de39870387bd8103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 25 Jan 2023 13:47:12 +0000 Subject: [PATCH 31/32] [FIX] account: fix some layouting in account invoice send wizard It is still using some outdated classes for margin. Layout was a bit broken, especially the "Preview as PDF" helper-like line. Task-3093257 (Mail: The Composer Update) Task-3149286 (Account: Cleanup account.invoice.send wizard) Part-of: odoo/odoo#107356 --- addons/account/wizard/account_invoice_send_views.xml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/addons/account/wizard/account_invoice_send_views.xml b/addons/account/wizard/account_invoice_send_views.xml index 46de03f03ed..002b85482af 100644 --- a/addons/account/wizard/account_invoice_send_views.xml +++ b/addons/account/wizard/account_invoice_send_views.xml @@ -28,7 +28,9 @@
-
+
Preview as a PDF
@@ -36,8 +38,8 @@
-
- +
+
From 8a66cc7db0c4e682f4c3b804f87858075c4ed46a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 25 Jan 2023 14:40:35 +0000 Subject: [PATCH 32/32] [IMP] mail: display scheduled_datetime of delayed notify in chatter Followup of odoo/odoo#95623 (introduce scheduled message notification) as well as odoo/odoo#99482 (support scheduled datetime from template to composer in both comment and mailing modes). Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#107356 --- addons/contacts/__manifest__.py | 3 ++ addons/contacts/data/mail_demo.xml | 19 ++++++++++++ addons/im_livechat/tests/test_message.py | 1 + addons/mail/models/mail_message.py | 29 ++++++++++++++++--- addons/mail/models/mail_thread.py | 2 +- .../static/src/components/message/message.xml | 5 ++++ addons/mail/static/src/models/message.js | 20 +++++++++++++ addons/mail/static/src/models/message_view.js | 17 +++++++++++ addons/rating/models/mail_message.py | 4 +-- addons/sms/models/mail_message.py | 4 +-- addons/test_mail/tests/test_performance.py | 8 ++--- 11 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 addons/contacts/data/mail_demo.xml diff --git a/addons/contacts/__manifest__.py b/addons/contacts/__manifest__.py index 0a910c80538..0e7702f34ad 100644 --- a/addons/contacts/__manifest__.py +++ b/addons/contacts/__manifest__.py @@ -15,6 +15,9 @@ You can track your vendors, customers and other contacts. 'data': [ 'views/contact_views.xml', ], + 'demo': [ + 'data/mail_demo.xml', + ], 'application': True, 'license': 'LGPL-3', 'assets': { diff --git a/addons/contacts/data/mail_demo.xml b/addons/contacts/data/mail_demo.xml new file mode 100644 index 00000000000..12eb254cbd0 --- /dev/null +++ b/addons/contacts/data/mail_demo.xml @@ -0,0 +1,19 @@ + + + + + + +

Hello! Could you send us your address ?

 + + res.partner + + email + + + + + + + + diff --git a/addons/im_livechat/tests/test_message.py b/addons/im_livechat/tests/test_message.py index 3957660a45c..79a8a2c4b7d 100644 --- a/addons/im_livechat/tests/test_message.py +++ b/addons/im_livechat/tests/test_message.py @@ -73,6 +73,7 @@ class TestImLivechatMessage(TransactionCase): 'recipients': [], 'record_name': "test1 Ernest Employee", 'res_id': channel_livechat_1.id, + 'scheduledDatetime': False, 'sms_ids': [], 'starred_partner_ids': [], 'subject': False, diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 03456375469..4ef8133eaef 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -904,10 +904,17 @@ class Message(models.Model): domain = expression.AND([domain, [('id', '>', min_id)]]) return self.search(domain, limit=limit) - def message_format(self, format_reply=True): - """ Get the message values in the format for web client. Since message values can be broadcasted, - computed fields MUST NOT BE READ and broadcasted. - :returns list(dict). + def message_format(self, format_reply=True, msg_vals=None): + """ Get the message values in the format for web client. Since message + values can be broadcasted, computed fields MUST NOT BE READ and + broadcasted. + + :param msg_vals: dictionary of values used to create the message. If + given it may be used to access values related to ``message`` without + accessing it directly. It lessens query count in some optimized use + cases by avoiding access message content in db; + + :returns list(dict). Example : { 'body': HTML content of the message @@ -958,6 +965,19 @@ class Message(models.Model): com_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment') note_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note') + + # fetch scheduled notifications once, only if msg_vals is not given to + # avoid useless queries when notifying Inbox right after a message_post + scheduled_dt_by_msg_id = {} + if msg_vals: + scheduled_dt_by_msg_id = {msg.id: msg_vals.get('scheduled_date') for msg in self} + elif self: + schedulers = self.env['mail.message.schedule'].sudo().search([ + ('mail_message_id', 'in', self.ids) + ]) + for scheduler in schedulers: + scheduled_dt_by_msg_id[scheduler.mail_message_id.id] = scheduler.scheduled_datetime + for vals in vals_list: message_sudo = self.browse(vals['id']).sudo().with_prefetch(self.ids) notifs = message_sudo.notification_ids.filtered(lambda n: n.res_partner_id) @@ -969,6 +989,7 @@ class Message(models.Model): 'subtype_description': message_sudo.subtype_id.description, 'is_notification': vals['message_type'] == 'user_notification', 'recipients': [{'id': p.id, 'name': p.name} for p in message_sudo.partner_ids], + 'scheduledDatetime': scheduled_dt_by_msg_id.get(vals['id'], False), }) if vals['model'] and self.env[vals['model']]._original_module: vals['module_icon'] = modules.module.get_module_icon(self.env[vals['model']]._original_module) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index f65807673d2..dd60edc395a 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2857,7 +2857,7 @@ class MailThread(models.AbstractModel): } for pid in inbox_pids] self.env['mail.notification'].sudo().create(notif_create_values) - message_format_values = message.message_format()[0] + message_format_values = message.message_format(msg_vals=msg_vals)[0] for partner_id in inbox_pids: bus_notifications.append((self.env['res.partner'].browse(partner_id), 'mail.message/inbox', dict(message_format_values))) self.env['bus.bus'].sudo()._sendmany(bus_notifications) diff --git a/addons/mail/static/src/components/message/message.xml b/addons/mail/static/src/components/message/message.xml index ed5c98dfcfd..e96eef8fa04 100644 --- a/addons/mail/static/src/components/message/message.xml +++ b/addons/mail/static/src/components/message/message.xml @@ -76,6 +76,11 @@ on document +
+ + + +
diff --git a/addons/mail/static/src/models/message.js b/addons/mail/static/src/models/message.js index 17bae319e1b..70d2e56d8f1 100644 --- a/addons/mail/static/src/models/message.js +++ b/addons/mail/static/src/models/message.js @@ -99,6 +99,9 @@ Model({ if ("recipients" in data) { data2.recipients = data.recipients; } + if ("scheduledDatetime" in data) { + data2.scheduledDatetime = data.scheduledDatetime; + } if ("starred_partner_ids" in data && this.messaging.currentPartner) { data2.isStarred = data.starred_partner_ids.includes( this.messaging.currentPartner.id @@ -698,6 +701,23 @@ Model({ }, }), recipients: many("Partner"), + /** + * In case of delayed notification, a scheduled datetime linked to the + * message scheduler is given in order to display it new to the post + * datetime. + */ + scheduledDatetime: attr(), + scheduledMomentDate: attr({ + compute() { + if (!this.scheduledDatetime) { + return clear(); + } + if (!moment.isMoment(this.scheduledDatetime)) { + return moment(str_to_datetime(this.scheduledDatetime)); + } + return this.scheduledDatetime; + }, + }), shortTime: attr({ compute() { if (!this.momentDate) { diff --git a/addons/mail/static/src/models/message_view.js b/addons/mail/static/src/models/message_view.js index d488451344c..b685547f642 100644 --- a/addons/mail/static/src/models/message_view.js +++ b/addons/mail/static/src/models/message_view.js @@ -713,5 +713,22 @@ Model({ return this.env._t("Read More"); }, }), + /** + * Scheduled for sending + */ + scheduledFromNow: attr({ + compute() { + if (!this.message) { + return clear(); + } + if (!this.message.scheduledMomentDate) { + return clear(); + } + if (!this.clockWatcher.clock.date) { + return clear(); + } + return this.message.scheduledMomentDate.fromNow(); + }, + }), }, }); diff --git a/addons/rating/models/mail_message.py b/addons/rating/models/mail_message.py index dcd72234c3b..5a8419af34a 100644 --- a/addons/rating/models/mail_message.py +++ b/addons/rating/models/mail_message.py @@ -26,8 +26,8 @@ class MailMessage(models.Model): ]) return [('id', 'in', ratings.mapped('message_id').ids)] - def message_format(self, format_reply=True): - message_values = super().message_format(format_reply=format_reply) + def message_format(self, format_reply=True, msg_vals=None): + message_values = super().message_format(format_reply=format_reply, msg_vals=msg_vals) rating_mixin_messages = self.filtered(lambda message: message.model and message.res_id diff --git a/addons/sms/models/mail_message.py b/addons/sms/models/mail_message.py index e3a4c67c5df..65906097a84 100644 --- a/addons/sms/models/mail_message.py +++ b/addons/sms/models/mail_message.py @@ -33,13 +33,13 @@ class MailMessage(models.Model): return ['&', ('notification_ids.notification_status', '=', 'exception'), ('notification_ids.notification_type', '=', 'sms')] raise NotImplementedError() - def message_format(self, format_reply=True): + def message_format(self, format_reply=True, msg_vals=None): """ Override in order to retrieves data about SMS (recipient name and SMS status) TDE FIXME: clean the overall message_format thingy """ - message_values = super(MailMessage, self).message_format(format_reply=format_reply) + message_values = super(MailMessage, self).message_format(format_reply=format_reply, msg_vals=msg_vals) all_sms_notifications = self.env['mail.notification'].sudo().search([ ('mail_message_id', 'in', [r['id'] for r in message_values]), ('notification_type', '=', 'sms') diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 914f0542f45..ba1b99a8ecd 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -1187,7 +1187,7 @@ class TestMailComplexPerformance(BaseMailPerformance): ] }]) - with self.assertQueryCount(employee=6): + with self.assertQueryCount(employee=7): res = messages.message_format() self.assertEqual(len(res), 2) for message in res: @@ -1196,7 +1196,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.env.flush_all() self.env.invalidate_all() - with self.assertQueryCount(employee=19): + with self.assertQueryCount(employee=20): res = messages.message_format() self.assertEqual(len(res), 2) for message in res: @@ -1217,14 +1217,14 @@ class TestMailComplexPerformance(BaseMailPerformance): 'res_id': record.id } for record in records]) - with self.assertQueryCount(employee=5): + with self.assertQueryCount(employee=6): res = messages.message_format() self.assertEqual(len(res), 6) self.env.flush_all() self.env.invalidate_all() - with self.assertQueryCount(employee=14): + with self.assertQueryCount(employee=15): res = messages.message_format() self.assertEqual(len(res), 6)