diff --git a/addons/account/wizard/account_move_send.py b/addons/account/wizard/account_move_send.py index 203b34f3727..4649c691b7d 100644 --- a/addons/account/wizard/account_move_send.py +++ b/addons/account/wizard/account_move_send.py @@ -430,8 +430,7 @@ class AccountMoveSend(models.Model): .with_context( no_new_invoice=True, mail_notify_author=self.env.user.partner_id.id in partner_ids, - mailing_document_based=True)\ - .message_post( + ).message_post( message_type='comment', **kwargs, **{ diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 2692497a85e..cb1b7e148a9 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -339,9 +339,9 @@ class MockEmail(common.BaseCase, MockSmtplibCase): ) return mail - def _find_sent_email(self, email_from, emails_to, subject=None): - """ Find an outgoing email based on from / to and optional subject when - having a conflict. + def _find_sent_email(self, email_from, emails_to, subject=None, body=None, attachment_names=None): + """ Find an outgoing email based on from / to and optional subject, body + and attachment names when having conflicts. :return sent_email: an outgoing email generated during the mock; """ @@ -349,9 +349,14 @@ class MockEmail(common.BaseCase, MockSmtplibCase): mail for mail in self._mails if set(mail['email_to']) == set(emails_to) and mail['email_from'] == email_from ] - if len(sent_emails) > 1 and subject: + if len(sent_emails) > 1: # try to better filter - sent_email = next((mail for mail in sent_emails if mail['subject'] == subject), False) + sent_email = next((mail for mail in sent_emails + if (subject is None or mail['subject'] == subject) + and (body is None or mail['body'] == body) + and (attachment_names is None + or set(attachment_names) == set(attachment[0] for attachment in mail['attachments'])) + ), False) else: sent_email = sent_emails[0] if sent_emails else False return sent_email @@ -563,14 +568,14 @@ class MockEmail(common.BaseCase, MockSmtplibCase): """ direct_check = ['body_alternative', 'email_from', 'references', 'reply_to', 'subject'] content_check = ['body_alternative_content', 'body_content', 'references_content'] - list_check = ['email_bcc', 'email_cc', 'email_to'] + email_list_check = ['email_bcc', 'email_cc', 'email_to'] other_check = ['attachments', 'attachments_info', 'body'] expected = {} - for fname in direct_check + content_check + list_check + other_check: + for fname in direct_check + content_check + email_list_check + other_check: if fname in values: expected[fname] = values[fname] - unknown = set(values.keys()) - set(direct_check + content_check + list_check + other_check) + unknown = set(values.keys()) - set(direct_check + content_check + email_list_check + other_check) if unknown: raise NotImplementedError('Unsupported %s' % ', '.join(unknown)) @@ -591,10 +596,15 @@ class MockEmail(common.BaseCase, MockSmtplibCase): expected['email_to'] = email_to_list # fetch mail + attachments = [attachment['name'] + for attachment in values.get('attachments_info', []) + if 'name' in attachment] sent_mail = self._find_sent_email( expected['email_from'], expected['email_to'], - values.get('subject'), + subject=values.get('subject'), + body=values.get('body'), + attachment_names=attachments or None ) debug_info = '' if not sent_mail: @@ -629,7 +639,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase): if 'body' in expected: self.assertHtmlEqual(expected['body'], sent_mail['body'], 'Value for %s: expected %s, received %s' % ('body', expected['body'], sent_mail['body'])) # beware to avoid list ordering differences (but Falsy values -> compare directly) - for val in list_check: + for val in email_list_check: if expected.get(val): self.assertEqual(sorted(expected[val]), sorted(sent_mail[val]), 'Value for %s: expected %s, received %s' % (val, expected[val], sent_mail[val])) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 6f848b724a8..4657479aa09 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -1084,8 +1084,7 @@ class MailComposer(models.TransientModel): blacklist_ids = self._get_blacklist_record_ids(mail_values_dict, recipients_info) optout_emails = self._get_optout_emails(mail_values_dict) done_emails = self._get_done_emails(mail_values_dict) - # in case of an invoice e.g. - mailing_document_based = self.env.context.get('mailing_document_based') + sent_emails_mapping = {} # distinct emails sent to each address as {email_address: [mail_values]} for record_id, mail_values in mail_values_dict.items(): recipients = recipients_info[record_id] @@ -1108,7 +1107,7 @@ class MailComposer(models.TransientModel): elif optout_emails and mail_to in optout_emails: mail_values['state'] = 'cancel' mail_values['failure_type'] = 'mail_optout' - elif done_emails and mail_to in done_emails and not mailing_document_based: + elif mail_to in done_emails: mail_values['state'] = 'cancel' mail_values['failure_type'] = 'mail_dup' # void of falsy values -> error @@ -1118,8 +1117,20 @@ class MailComposer(models.TransientModel): elif not mail_to_normalized: mail_values['state'] = 'cancel' mail_values['failure_type'] = 'mail_email_invalid' - elif done_emails is not None and not mailing_document_based: - done_emails.append(mail_to) + elif mail_to in sent_emails_mapping: + # If the number of attachments on the mail exactly matches the number of attachments on the composer + # we assume the attachments are copies of the ones attached to the composer and thus are the same + if len(self.attachment_ids) == len(mail_values.get('attachment_ids', [])) and\ + any(sent_mail.get('subject') == mail_values.get('subject') and + sent_mail.get('body') == mail_values.get('body') for sent_mail in sent_emails_mapping[mail_to]): + mail_values['state'] = 'cancel' + mail_values['failure_type'] = 'mail_dup' + else: + sent_emails_mapping[mail_to].append(mail_values) + else: + sent_emails_mapping[mail_to] = [mail_values] + + done_emails += sent_emails_mapping.keys() return mail_values_dict diff --git a/addons/purchase/models/__init__.py b/addons/purchase/models/__init__.py index e9d1576b95e..f0dd03c884b 100644 --- a/addons/purchase/models/__init__.py +++ b/addons/purchase/models/__init__.py @@ -9,4 +9,3 @@ from . import product from . import res_company from . import res_config_settings from . import res_partner -from . import mail_compose_message diff --git a/addons/purchase/models/mail_compose_message.py b/addons/purchase/models/mail_compose_message.py deleted file mode 100644 index d5d529ed6f2..00000000000 --- a/addons/purchase/models/mail_compose_message.py +++ /dev/null @@ -1,13 +0,0 @@ -# -*- coding: utf-8 -*- -# purches Part of Odoo. See LICENSE file for full copyright and licensing details. - -from odoo import models - - -class MailComposeMessage(models.TransientModel): - _inherit = 'mail.compose.message' - - def _action_send_mail(self, auto_commit=False): - if self.model == 'purchase.order': - self = self.with_context(mailing_document_based=True) - return super(MailComposeMessage, self)._action_send_mail(auto_commit=auto_commit) diff --git a/addons/sale/wizard/__init__.py b/addons/sale/wizard/__init__.py index 9aa43983a08..246f28cce38 100644 --- a/addons/sale/wizard/__init__.py +++ b/addons/sale/wizard/__init__.py @@ -2,7 +2,6 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from . import account_payment_register -from . import mail_compose_message from . import payment_link_wizard from . import payment_provider_onboarding_wizard from . import res_config_settings diff --git a/addons/sale/wizard/mail_compose_message.py b/addons/sale/wizard/mail_compose_message.py deleted file mode 100644 index 6e989df230c..00000000000 --- a/addons/sale/wizard/mail_compose_message.py +++ /dev/null @@ -1,13 +0,0 @@ -# -*- coding: utf-8 -*- -# Part of Odoo. See LICENSE file for full copyright and licensing details. - -from odoo import models - - -class MailComposeMessage(models.TransientModel): - _inherit = 'mail.compose.message' - - def _action_send_mail(self, auto_commit=False): - if self.model == 'sale.order': - self = self.with_context(mailing_document_based=True) - return super(MailComposeMessage, self)._action_send_mail(auto_commit=auto_commit) diff --git a/addons/test_mail/data/mail_template_data.xml b/addons/test_mail/data/mail_template_data.xml index f5970d9e211..dcb4712c463 100644 --- a/addons/test_mail/data/mail_template_data.xml +++ b/addons/test_mail/data/mail_template_data.xml @@ -44,6 +44,19 @@ + + diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 98f92af1a15..516e6a44d19 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -6,9 +6,10 @@ import base64 from ast import literal_eval from datetime import timedelta from freezegun import freeze_time -from itertools import product +from itertools import chain, product from unittest.mock import DEFAULT, patch +from odoo import Command from odoo.addons.base.tests.test_ir_cron import CronMixinCase from odoo.addons.mail.tests.common import mail_new_test_user, MailCommon from odoo.addons.mail.wizard.mail_compose_message import MailComposer @@ -63,7 +64,7 @@ class TestMailComposer(MailCommon, TestRecipients): additional_values={'user_id': cls.user_employee_2.id}, ) - cls.test_report, cls.test_report_2 = cls.env['ir.actions.report'].create([ + cls.test_report, cls.test_report_2, cls.test_report_3 = cls.env['ir.actions.report'].create([ { 'name': 'Test Report on Mail Test Ticket', 'model': 'mail.test.ticket', @@ -76,6 +77,12 @@ class TestMailComposer(MailCommon, TestRecipients): 'print_report_name': "'TestReport2 for %s' % object.name", 'report_type': 'qweb-pdf', 'report_name': 'test_mail.mail_test_ticket_test_template_2', + }, { + 'name': 'Test Report 3 with variable data on Mail Test Ticket', + 'model': 'mail.test.ticket', + 'print_report_name': "'TestReport3 for %s' % object.name", + 'report_type': 'qweb-pdf', + 'report_name': 'test_mail.mail_test_ticket_test_variable_template', } ]) @@ -1992,6 +1999,174 @@ class TestComposerResultsMass(TestMailComposer): self.assertEqual(len(self._new_msgs), 2, 'Should have created 1 mail.mail per record') self.assertFalse(self._new_msgs.exists(), 'Should have deleted mail.message records') + @users('employee') + @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') + def test_mail_composer_duplicates(self): + """ Ensures emails sent to the same recipient multiple times + are only sent when they are not duplicates + """ + self.template.write({ + 'auto_delete': False, + 'body_html': '

Common Body

', + 'email_to': '', + 'partner_to': '{{ object.customer_id.id if object.customer_id else "" }}', + 'subject': 'Common Subject', + }) + + # Guarantee points of variation for test_report_3 + self.test_records[0].write({'count': 1, 'name': 'A'}) + self.test_records[1].write({'count': 2, 'name': 'B'}) + + template_attachment, composer_attachment = self.env['ir.attachment'].create([{ + 'datas': base64.b64encode(b'ExtraData'), + 'mimetype': 'text/plain', + 'name': f'{record._name}_Common_Attachment.txt', + 'res_id': record.id if record else False, + 'res_model': record._name, + } for record in (self.template, self.env['mail.compose.message'])]) + + customer_ids = self.partners[:2] + customer_ids.write({ + 'email': 'test@test.lan' + }) + + def _instanciate_composer(composer_attachments=False): + composer_form = Form(self.env['mail.compose.message'].with_context( + self._get_web_context(self.test_records[:2], add_web=True, + default_template_id=self.template.id) + )) + composer = composer_form.save() + if composer_attachments: + composer_attachments.res_id = composer.id + composer.attachment_ids += composer_attachments + return composer + + base_customer_values = [{'email': 'test@test.lan'}] + # different recipients should always receive emails + customer_diff_emails = [{'email': f'difftest{n}@test.lan'} for n in range(2)] + base_template_values = { + 'attachment_ids': [Command.clear()], + 'body_html': '

Common Body

', + 'report_template_ids': [Command.clear()], + 'subject': 'Common Subject', + } + same_attachments = {'attachment_ids': template_attachment.ids} + diff_body = {'body_html': '

'} + # regardless of whether they have different bodies or not, they are considered duplicates + diff_attachment_same_content = {'report_template_ids': [self.test_report_2.id]} + diff_attachment_diff_content = {'report_template_ids': [self.test_report_3.id]} + diff_subject = {'subject': '{{ object.name }}'} + + # all template variations using same recipients + diff_combinations = product( + [[diff_body], [diff_subject], + [diff_attachment_same_content], [diff_attachment_diff_content], + [diff_attachment_same_content, same_attachments], + [diff_body, diff_attachment_diff_content, diff_subject]], + [base_customer_values]) + # no template variations using different recipients + diff_combinations = chain(diff_combinations, [[[], customer_diff_emails]]) + # expect all sent + for template_changes, customer_changes in diff_combinations: + test_template_values = dict(base_template_values) + for change in template_changes: + test_template_values.update(change) + self.template.write(test_template_values) + for customer, base_vals, update_vals in zip(customer_ids, base_customer_values, customer_changes): + customer.write({**base_vals, **update_vals}) + with self.subTest(template_values=test_template_values, customer_changes=customer_changes): + composer = _instanciate_composer() + with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app(): + composer._action_send_mail() + self.assertEqual(len(self._new_mails), 2) + # check emails + for record in self.test_records[:2]: + # email_to will be normalized and formatted, even if already formatted + expected_subject = base_template_values['subject'] + if test_template_values['subject'] != base_template_values['subject']: + expected_subject = record.name + expected_body = base_template_values['body_html'] + if test_template_values['body_html'] != base_template_values['body_html']: + expected_body = f'

{record.name}

' + expected_attachment_info = [] + if self.template.attachment_ids: + expected_attachment_info.append({'name': 'mail.template_Common_Attachment.txt', 'type': 'text/plain'}) + if self.template.report_template_ids: + test_report_name = 'TestReport2' if self.template.report_template_ids == self.test_report_2 else 'TestReport3' + expected_attachment_info.append( + {'name': f'{test_report_name} for {record.name}.html', 'type': 'text/plain'}, + ) + self.assertMailMailWEmails( + [record.customer_id.email], + 'sent', + author=self.env.user.partner_id, + content=expected_body, + mail_message=record.message_ids[0], + email_to_recipients=[[record.customer_id.email_formatted]], + email_values={ + 'attachments_info': expected_attachment_info, + 'body': expected_body, + 'email_from': record.user_id.email_formatted, + 'subject': expected_subject, + }, + ) + + # expect duplicates + for composer_attachment, template_changes in zip([False, composer_attachment, [], composer_attachment], [[], [], [same_attachments], [same_attachments]]): + test_template_values = dict(base_template_values) + for change in template_changes: + test_template_values.update(change) + self.template.write(test_template_values) + # reset customers to have the same email + for customer, base_vals in zip(customer_ids, base_customer_values): + customer.write(base_vals) + with self.subTest(composer_attachments=composer_attachment, template_values=test_template_values): + composer = _instanciate_composer(composer_attachments=composer_attachment) + with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app(): + composer._action_send_mail() + self.assertEqual(len(self._new_mails), 2) + self.assertEqual(len(self._mails), 1) + # check email + expected_attachment_info = [] + if template_changes: + expected_attachment_info.append({ + 'name': f'{self.template._name}_Common_Attachment.txt', + 'raw': b'ExtraData', + 'type': 'text/plain', + }) + if composer_attachment: + expected_attachment_info.append({ + 'name': f'{composer._name}_Common_Attachment.txt', + 'raw': b'ExtraData', + 'type': 'text/plain', + }) + self.assertMailMailWRecord( + self.test_records[0], + [self.test_records[0].customer_id], + 'sent', + author=self.env.user.partner_id, + content='Common Body', + email_values={ + 'attachments_info': expected_attachment_info, + 'body_content': 'Common Body', + 'email_from': record.user_id.email_formatted, + 'subject': 'Common Subject', + }, + ) + self. assertMailMailWRecord( + self.test_records[1], + [self.test_records[1].customer_id], + 'cancel', + author=self.env.user.partner_id, + content='Common Body', + email_values={ + 'attachments_info': expected_attachment_info, + 'body_content': 'Common Body', + 'email_from': record.user_id.email_formatted, + 'subject': 'Common Subject', + }, + ) + @users('employee') @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') def test_mail_composer_wtpl(self): @@ -2727,66 +2902,3 @@ class TestComposerResultsMassStatus(TestMailComposer): } ) self.assertEqual(len(self._mails), 2, 'Should have sent 2 emails, even to excluded email.') - - @users('employee') - @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') - def test_mailing_duplicates_document_based(self): - """ Tests a document-based mass mailing with the same address mails - This should be allowed and not considered as duplicate in this context - """ - test_records = self.test_records.with_env(self.env) - composer_form = Form(self.env['mail.compose.message'].with_context( - self._get_web_context(test_records, add_web=True, - default_template_id=self.template.id) - )) - composer = composer_form.save() - - # by default duplicates are canceled - with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app(): - composer._action_send_mail() - - for record, expected_state, expected_ft in zip( - test_records, - ['cancel', 'sent', 'sent', 'sent', 'cancel', 'sent', 'cancel'], - ['mail_bl', False, False, False, 'mail_dup', False, 'mail_dup'] - ): - with self.subTest(record=record, expected_state=expected_state, expected_ft=expected_ft): - self.assertMailMailWRecord( - record, record.customer_id, expected_state, - # author is current user, email_from is coming from template (user_id of record) - author=self.user_employee.partner_id, - fields_values={ - 'email_from': self.user_employee_2.email_formatted, - 'failure_reason': False, - 'failure_type': expected_ft, - }, - email_values={ - 'email_from': self.user_employee_2.email_formatted, - } - ) - self.assertEqual(len(self._mails), 4, 'Should have sent 4 emails, and skipped an excluded and 2 duplicate emails.') - - # magic context key allowing to send duplicates when necessary - with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app(): - composer.with_context(mailing_document_based=True)._action_send_mail() - - for record, expected_state, expected_ft in zip( - test_records, - ['cancel', 'sent', 'sent', 'sent', 'sent', 'sent', 'sent'], - ['mail_bl', False, False, False, False, False, False] - ): - with self.subTest(record=record, expected_state=expected_state, expected_ft=expected_ft): - self.assertMailMailWRecord( - record, record.customer_id, expected_state, - # author is current user, email_from is coming from template (user_id of record) - author=self.user_employee.partner_id, - fields_values={ - 'email_from': self.user_employee_2.email_formatted, - 'failure_reason': False, - 'failure_type': expected_ft, - }, - email_values={ - 'email_from': self.user_employee_2.email_formatted, - } - ) - self.assertEqual(len(self._mails), 6, 'Should have sent 6 emails, and skipped an excluded email') diff --git a/addons/test_mass_mailing/tests/test_mailing.py b/addons/test_mass_mailing/tests/test_mailing.py index d4206fc1347..dc969fbf92a 100644 --- a/addons/test_mass_mailing/tests/test_mailing.py +++ b/addons/test_mass_mailing/tests/test_mailing.py @@ -538,8 +538,9 @@ class TestMassMailing(TestMassMailCommon): @mute_logger('odoo.addons.mail.models.mail_mail') def test_mailing_mailing_list_optout(self): """ Test mailing list model specific optout behavior """ - mailing_contact_1 = self.env['mailing.contact'].create({'name': 'test 1A', 'email': 'test@test.example.com'}) - mailing_contact_2 = self.env['mailing.contact'].create({'name': 'test 1B', 'email': 'test@test.example.com'}) + # as duplication checks body and subject, we create 2 exact copies to make sure only 1 is sent + mailing_contact_1 = self.env['mailing.contact'].create({'name': 'test 1', 'email': 'test@test.example.com'}) + mailing_contact_2 = self.env['mailing.contact'].create({'name': 'test 1', 'email': 'test@test.example.com'}) mailing_contact_3 = self.env['mailing.contact'].create({'name': 'test 3', 'email': 'test3@test.example.com'}) mailing_contact_4 = self.env['mailing.contact'].create({'name': 'test 4', 'email': 'test4@test.example.com'}) mailing_contact_5 = self.env['mailing.contact'].create({'name': 'test 5', 'email': 'test5@test.example.com'})