From 987d974ccb8c75c6eca899adb5be50fe48c8eb2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 6 May 2021 09:15:28 +0000 Subject: [FIX] sale, mass_mailing_sms, test_mail_full: correctly support partner-only records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When performing an sms marketing on a model having only a ``partner_id`` field available (aka no ``phone`` or ``mobile``) it crashes due to seen list computation (aka already contacted recipients). This is due to an SQL query not taking into account those models as it works only for those with a phone field. We fix it as done in ``mass_mailìng`` app, aka fetching information on the related partner if available. Some tests for models using a 2many relationships towards recipients are added. Note that this kind of model does not really support complete SMS notification, as only the first found partner is notified. Mass SMS on this kind of model is currently not possible as seen list is not supported. There is no standard use case of this in Odoo codebase. Correctly support sms marketing on sale model by defining the necessary methods. As sale order has only a partner_id field available we have to correctly override phone related methods for SMS. LINKS Task ID-2431217 COM PR odoo/odoo#71140 Signed-off-by: Thibault Delavallee (tde) --- addons/mass_mailing_sms/models/mailing_mailing.py | 32 +++++++-- addons/sale/models/sale.py | 8 +++ addons/sms/models/mail_thread.py | 6 +- addons/test_mail_full/models/mailing_mailing.py | 3 +- addons/test_mail_full/models/test_mail_models.py | 18 +++++ addons/test_mail_full/security/ir.model.access.csv | 2 + addons/test_mail_full/tests/test_mass_sms.py | 83 +++++++++++++++++++++- addons/test_mail_full/tests/test_sms_post.py | 18 +++++ 8 files changed, 160 insertions(+), 10 deletions(-) diff --git a/addons/mass_mailing_sms/models/mailing_mailing.py b/addons/mass_mailing_sms/models/mailing_mailing.py index a0b938dbb04..ea2e36d7cbe 100644 --- a/addons/mass_mailing_sms/models/mailing_mailing.py +++ b/addons/mass_mailing_sms/models/mailing_mailing.py @@ -180,31 +180,51 @@ class Mailing(models.Model): self.ensure_one() target = self.env[self.mailing_model_real] + partner_fields = [] if issubclass(type(target), self.pool['mail.thread.phone']): phone_fields = ['phone_sanitized'] elif issubclass(type(target), self.pool['mail.thread']): phone_fields = target._sms_get_number_fields() + partner_fields = target._sms_get_partner_fields() else: phone_fields = [] if 'mobile' in target._fields: phone_fields.append('mobile') if 'phone' in target._fields: phone_fields.append('phone') - if not phone_fields: + partner_field = next( + (fname for fname in partner_fields if target._fields[fname].type == 'many2one'), + False + ) + if not phone_fields and not partner_field: raise UserError(_("Unsupported %s for mass SMS", self.mailing_model_id.name)) query = """ SELECT %(select_query)s FROM mailing_trace trace JOIN %(target_table)s target ON (trace.res_id = target.id) + %(join_add_query)s WHERE (%(where_query)s) - AND trace.mass_mailing_id = %%(mailing_id)s - AND trace.model = %%(target_model)s + AND trace.mass_mailing_id = %%(mailing_id)s + AND trace.model = %%(target_model)s """ + if phone_fields: + # phone fields are checked on target mailed model + select_query = 'target.id, ' + ', '.join('target.%s' % fname for fname in phone_fields) + where_query = ' OR '.join('target.%s IS NOT NULL' % fname for fname in phone_fields) + join_add_query = '' + else: + # phone fields are checked on res.partner model + partner_phone_fields = ['mobile', 'phone'] + select_query = 'target.id, ' + ', '.join('partner.%s' % fname for fname in partner_phone_fields) + where_query = ' OR '.join('partner.%s IS NOT NULL' % fname for fname in partner_phone_fields) + join_add_query = 'JOIN res_partner partner ON (target.%s = partner.id)' % partner_field + query = query % { - 'select_query': 'target.id, ' + ', '.join('target.%s' % fname for fname in phone_fields), - 'where_query': ' OR '.join('target.%s IS NOT NULL' % fname for fname in phone_fields), - 'target_table': target._table + 'select_query': select_query, + 'where_query': where_query, + 'target_table': target._table, + 'join_add_query': join_add_query, } params = {'mailing_id': self.id, 'target_model': self.mailing_model_real} self._cr.execute(query, params) diff --git a/addons/sale/models/sale.py b/addons/sale/models/sale.py index 37f30458176..7c81f62bfe6 100644 --- a/addons/sale/models/sale.py +++ b/addons/sale/models/sale.py @@ -858,6 +858,14 @@ Reason(s) of this behavior could be: self.filtered(lambda o: o.state == 'draft').with_context(tracking_disable=True).write({'state': 'sent'}) return super(SaleOrder, self.with_context(mail_post_autofollow=True)).message_post(**kwargs) + def _sms_get_number_fields(self): + """ No phone or mobile field is available on sale model. Instead SMS will + fallback on partner-based computation using ``_sms_get_partner_fields``. """ + return [] + + def _sms_get_partner_fields(self): + return ['partner_id'] + def _send_order_confirmation_mail(self): if self.env.su: # sending mail in sudo was meant for it being sent from superuser diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py index c3ff8168faa..45bd8643906 100644 --- a/addons/sms/models/mail_thread.py +++ b/addons/sms/models/mail_thread.py @@ -53,13 +53,15 @@ class MailThread(models.AbstractModel): """ partners = self.env['res.partner'] for fname in self._sms_get_partner_fields(): - partners |= self.mapped(fname) + partners = partners.union(*self.mapped(fname)) # ensure ordering return partners def _sms_get_number_fields(self): """ This method returns the fields to use to find the number to use to send an SMS on a record. """ - return ['mobile'] + if 'mobile' in self: + return ['mobile'] + return [] def _sms_get_recipients_info(self, force_field=False, partner_fallback=True): """" Get SMS recipient information on current record set. This method diff --git a/addons/test_mail_full/models/mailing_mailing.py b/addons/test_mail_full/models/mailing_mailing.py index a97c41e9047..8d8a49a1ec9 100644 --- a/addons/test_mail_full/models/mailing_mailing.py +++ b/addons/test_mail_full/models/mailing_mailing.py @@ -15,7 +15,8 @@ class Mailing(models.Model): """Returns a set of emails opted-out in target model""" self.ensure_one() if self.mailing_model_real in ('mail.test.sms.bl.optout', - 'mail.test.sms.partner'): + 'mail.test.sms.partner', + 'mail.test.sms.partner.2many'): res_ids = self._get_recipients() opt_out_contacts = set(self.env[self.mailing_model_real].search([ ('id', 'in', res_ids), diff --git a/addons/test_mail_full/models/test_mail_models.py b/addons/test_mail_full/models/test_mail_models.py index 9ee84a0b3a2..a3b5eb8dc4d 100644 --- a/addons/test_mail_full/models/test_mail_models.py +++ b/addons/test_mail_full/models/test_mail_models.py @@ -89,3 +89,21 @@ class MailTestSMSPartner(models.Model): def _sms_get_number_fields(self): return [] + + +class MailTestSMSPartner2Many(models.Model): + """ A model like sale order having only a customer, not specific phone + or mobile fields. """ + _description = 'Chatter Model for SMS Gateway (M2M Partners only)' + _name = 'mail.test.sms.partner.2many' + _inherit = ['mail.thread'] + + name = fields.Char() + customer_ids = fields.Many2many('res.partner', string='Customers') + opt_out = fields.Boolean() + + def _sms_get_partner_fields(self): + return ['customer_ids'] + + def _sms_get_number_fields(self): + return [] diff --git a/addons/test_mail_full/security/ir.model.access.csv b/addons/test_mail_full/security/ir.model.access.csv index b65c78635ce..5a099a7d8b4 100644 --- a/addons/test_mail_full/security/ir.model.access.csv +++ b/addons/test_mail_full/security/ir.model.access.csv @@ -7,3 +7,5 @@ access_mail_test_sms_bl_optout_all,mail.test.sms.bl.optout.all,model_mail_test_s access_mail_test_sms_bl_optout_user,mail.test.sms.bl.optout.user,model_mail_test_sms_bl_optout,base.group_user,1,1,1,1 access_mail_test_sms_partner_all,mail.test.sms.partner.all,model_mail_test_sms_partner,,0,0,0,0 access_mail_test_sms_partner_user,mail.test.sms.partner.user,model_mail_test_sms_partner,base.group_user,1,1,1,1 +access_mail_test_sms_partner_2many_all,mail.test.sms.partner.2many.all,model_mail_test_sms_partner_2many,,0,0,0,0 +access_mail_test_sms_partner_2many_user,mail.test.sms.partner.2many.user,model_mail_test_sms_partner_2many,base.group_user,1,1,1,1 diff --git a/addons/test_mail_full/tests/test_mass_sms.py b/addons/test_mail_full/tests/test_mass_sms.py index 9dc48232ab3..dbc91b95c77 100644 --- a/addons/test_mail_full/tests/test_mass_sms.py +++ b/addons/test_mail_full/tests/test_mass_sms.py @@ -5,6 +5,7 @@ from ast import literal_eval from odoo.addons.phone_validation.tools import phone_validation from odoo.addons.test_mail_full.tests.common import TestMailFullCommon +from odoo import exceptions from odoo.tests import tagged from odoo.tests.common import users from odoo.tools import mute_logger @@ -262,7 +263,7 @@ class TestMassSMSInternals(TestMassSMSCommon): mailing_test.action_send_sms() -@tagged('mass_mailing') +@tagged('mass_mailing', 'mass_mailing_sms') class TestMassSMS(TestMassSMSCommon): @users('user_marketing') @@ -293,6 +294,86 @@ class TestMassSMS(TestMassSMSCommon): ] for record in self.records], ) + @users('user_marketing') + @mute_logger('odoo.addons.mail.models.mail_mail') + def test_mass_sms_partner_only(self): + """ Check sending SMS marketing on models having only a partner_id fields + set is working. """ + mailing = self.env['mailing.mailing'].browse(self.mailing_sms.ids) + mailing.write({ + 'mailing_model_id': self.env['ir.model']._get('mail.test.sms.partner').id, + }) + + records = self.env['mail.test.sms.partner'].create([ + {'name': 'SMSTest on %s' % partner.name, + 'customer_id': partner.id, + } for partner in self.partners + ]) + + with self.mockSMSGateway(): + mailing.action_send_sms() + + self.assertEqual(len(mailing.mailing_trace_ids), 10) + self.assertSMSTraces( + [{'partner': record.customer_id, + 'number': record.customer_id.phone_sanitized, + 'state': 'sent', + 'content': 'Dear %s this is a mass SMS with two links' % record.display_name + } for record in records], + mailing, records, + sms_links_info=[[ + ('http://www.odoo.com/smstest', True, {}), + ('http://www.odoo.com/smstest/%s' % record.id, True, {}), + # unsubscribe is not shortened and parsed at sending + ('unsubscribe', False, {}), + ] for record in records], + ) + + # add a new record, send -> sent list should not resend traces + new_record = self.env['mail.test.sms.partner'].create([ + {'name': 'Duplicate SMS on %s' % self.partners[0].name, + 'customer_id': self.partners[0].id, + } + ]) + with self.mockSMSGateway(): + mailing.action_send_sms() + + self.assertEqual(len(mailing.mailing_trace_ids), 11) + self.assertSMSTraces( + [{'partner': new_record.customer_id, + 'number': new_record.customer_id.phone_sanitized, + 'state': 'sent', + 'content': 'Dear %s this is a mass SMS with two links' % new_record.display_name + }], + mailing, new_record, + sms_links_info=[[ + ('http://www.odoo.com/smstest', True, {}), + ('http://www.odoo.com/smstest/%s' % new_record.id, True, {}), + # unsubscribe is not shortened and parsed at sending + ('unsubscribe', False, {}), + ]], + ) + + @users('user_marketing') + @mute_logger('odoo.addons.mail.models.mail_mail') + def test_mass_sms_partner_only_m2m(self): + """ Check sending SMS marketing on models having only a m2m to partners + is currently not suppored. """ + mailing = self.env['mailing.mailing'].browse(self.mailing_sms.ids) + mailing.write({ + 'mailing_model_id': self.env['ir.model']._get('mail.test.sms.partner.2many').id, + }) + + records = self.env['mail.test.sms.partner.2many'].create([ + {'name': 'SMSTest on %s' % partner.name, + 'customer_ids': [(4, partner.id)], + } for partner in self.partners + ]) + + with self.assertRaises(exceptions.UserError), self.mockSMSGateway(): + mailing.action_send_sms() + + @users('user_marketing') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mass_sms_w_opt_out(self): diff --git a/addons/test_mail_full/tests/test_sms_post.py b/addons/test_mail_full/tests/test_sms_post.py index 6851974fd88..ad39f0c2a8a 100644 --- a/addons/test_mail_full/tests/test_sms_post.py +++ b/addons/test_mail_full/tests/test_sms_post.py @@ -108,6 +108,24 @@ class TestSMSPost(TestMailFullCommon, TestRecipients): # should not crash but have a failed notification self.assertSMSNotification([{'partner': self.env['res.partner'], 'number': False, 'state': 'exception', 'failure_type': 'sms_number_missing'}], self._test_body, messages) + def test_message_sms_model_w_partner_m2m_only(self): + with self.with_user('employee'): + record = self.env['mail.test.sms.partner.2many'].create({'customer_ids': [(4, self.partner_1.id)]}) + + with self.mockSMSGateway(): + messages = record._message_sms(self._test_body) + + self.assertSMSNotification([{'partner': self.partner_1}], self._test_body, messages) + + # TDE: should take first found one according to partner ordering + with self.with_user('employee'): + record = self.env['mail.test.sms.partner.2many'].create({'customer_ids': [(4, self.partner_1.id), (4, self.partner_2.id)]}) + + with self.mockSMSGateway(): + messages = record._message_sms(self._test_body) + + self.assertSMSNotification([{'partner': self.partner_2}], self._test_body, messages) + def test_message_sms_on_field_w_partner(self): with self.with_user('employee'), self.mockSMSGateway(): test_record = self.env['mail.test.sms'].browse(self.test_record.id) -- cgit v1.2.3