[FW][FIX] sms: compose SMS on non-thread models

Nothing prevents from creating templates and composers on non thread
models. However most necessary code relies on mail.thread.

Here we move code from mail.thread directly to BaseModel. For most
business models it does not change anything. For some side models
it allows / fixes usage of SMS composer, allowing to dynamically
find numbers / partners, ...

One notable use case that triggered this PR is sending of SMS to
users using their work phone number, or through their partner. Either
we hack user model, either we provide a generic fix.

Note that in current master (18.2+) most 'mail' generic features are
supported on non-thread models (template usage and sending, mailing
using composer, ...). SMS is still thread-specific while phone
capabilities have already been moved to generic code.

We therefore continue towards generic simple mail / sms usage on
business records, while advanced features (followers, logs, direct
post using SMS type) require thread-enabled models.

Task-4113190
sentry-4681535519

closes odoo#202802

Forward-port-of: odoo#202454
Forward-port-of: odoo#200650
Signed-off-by: Thibault Delavallee (tde) <[email protected]>

Author: robodoo

addons/mail/tests/common.py

@@ -1519,6 +1519,7 @@ def setUpClass(cls):
                 'email': '[email protected]',
                 "name": "Mitchell Admin",
                 'notification_type': 'inbox',
+                'phone': '0455135790',
             })
         # have root available at hand, just in case
         cls.user_root = cls.env.ref('base.user_root')

addons/sms/models/__init__.py

@@ -8,6 +8,7 @@
 from . import mail_message
 from . import mail_notification
 from . import mail_thread
+from . import models
 from . import res_partner
 from . import sms_sms
 from . import sms_template

addons/sms/models/mail_thread.py

@@ -42,89 +42,6 @@ def _compute_message_has_sms_error(self):
     def _search_message_has_sms_error(self, operator, operand):
         return ['&', ('message_ids.has_sms_error', operator, operand), ('message_ids.author_id', '=', self.env.user.partner_id.id)]
 
-    def _sms_get_recipients_info(self, force_field=False, partner_fallback=True):
-        """" Get SMS recipient information on current record set. This method
-        checks for numbers and sanitation in order to centralize computation.
-
-        Example of use cases
-
-          * click on a field -> number is actually forced from field, find customer
-            linked to record, force its number to field or fallback on customer fields;
-          * contact -> find numbers from all possible phone fields on record, find
-            customer, force its number to found field number or fallback on customer fields;
-
-        :param force_field: either give a specific field to find phone number, either
-            generic heuristic is used to find one based on ``_phone_get_number_fields``;
-        :param partner_fallback: if no value found in the record, check its customer
-            values based on ``_mail_get_partners``;
-
-        :return dict: record.id: {
-            'partner': a res.partner recordset that is the customer (void or singleton)
-                linked to the recipient. See ``_mail_get_partners``;
-            'sanitized': sanitized number to use (coming from record's field or partner's
-                phone fields). Set to False is number impossible to parse and format;
-            'number': original number before sanitation;
-            'partner_store': whether the number comes from the customer phone fields. If
-                False it means number comes from the record itself, even if linked to a
-                customer;
-            'field_store': field in which the number has been found (generally mobile or
-                phone, see ``_phone_get_number_fields``);
-        } for each record in self
-        """
-        result = dict.fromkeys(self.ids, False)
-        tocheck_fields = [force_field] if force_field else self._phone_get_number_fields()
-        for record in self:
-            all_numbers = [record[fname] for fname in tocheck_fields if fname in record]
-            all_partners = record._mail_get_partners()[record.id]
-
-            valid_number, fname = False, False
-            for fname in [f for f in tocheck_fields if f in record]:
-                valid_number = record._phone_format(fname=fname)
-                if valid_number:
-                    break
-
-            if valid_number:
-                result[record.id] = {
-                    'partner': all_partners[0] if all_partners else self.env['res.partner'],
-                    'sanitized': valid_number,
-                    'number': record[fname],
-                    'partner_store': False,
-                    'field_store': fname,
-                }
-            elif all_partners and partner_fallback:
-                partner = self.env['res.partner']
-                for partner in all_partners:
-                    for fname in self.env['res.partner']._phone_get_number_fields():
-                        valid_number = partner._phone_format(fname=fname)
-                        if valid_number:
-                            break
-
-                if not valid_number:
-                    fname = 'mobile' if partner.mobile else ('phone' if partner.phone else 'mobile')
-
-                result[record.id] = {
-                    'partner': partner,
-                    'sanitized': valid_number if valid_number else False,
-                    'number': partner[fname],
-                    'partner_store': True,
-                    'field_store': fname,
-                }
-            else:
-                # did not find any sanitized number -> take first set value as fallback;
-                # if none, just assign False to the first available number field
-                value, fname = next(
-                    ((value, fname) for value, fname in zip(all_numbers, tocheck_fields) if value),
-                    (False, tocheck_fields[0] if tocheck_fields else False)
-                )
-                result[record.id] = {
-                    'partner': self.env['res.partner'],
-                    'sanitized': False,
-                    'number': value,
-                    'partner_store': False,
-                    'field_store': fname
-                }
-        return result
-
     @api.returns('mail.message', lambda value: value.id)
     def message_post(self, *args, body='', message_type='notification', **kwargs):
         # When posting an 'SMS' `message_type`, make sure that the body is used as-is in the sms,

addons/sms/models/models.py

@@ -0,0 +1,89 @@
+from odoo import models
+from odoo.addons.phone_validation.tools import phone_validation
+
+
+class BaseModel(models.AbstractModel):
+    _inherit = 'base'
+
+    def _sms_get_recipients_info(self, force_field=False, partner_fallback=True):
+        """" Get SMS recipient information on current record set. This method
+        checks for numbers and sanitation in order to centralize computation.
+
+        Example of use cases
+
+          * click on a field -> number is actually forced from field, find customer
+            linked to record, force its number to field or fallback on customer fields;
+          * contact -> find numbers from all possible phone fields on record, find
+            customer, force its number to found field number or fallback on customer fields;
+
+        :param force_field: either give a specific field to find phone number, either
+            generic heuristic is used to find one based on ``_phone_get_number_fields``;
+        :param partner_fallback: if no value found in the record, check its customer
+            values based on ``_mail_get_partners``;
+
+        :return dict: record.id: {
+            'partner': a res.partner recordset that is the customer (void or singleton)
+                linked to the recipient. See ``_mail_get_partners``;
+            'sanitized': sanitized number to use (coming from record's field or partner's
+                phone fields). Set to False is number impossible to parse and format;
+            'number': original number before sanitation;
+            'partner_store': whether the number comes from the customer phone fields. If
+                False it means number comes from the record itself, even if linked to a
+                customer;
+            'field_store': field in which the number has been found (generally mobile or
+                phone, see ``_phone_get_number_fields``);
+        } for each record in self
+        """
+        result = dict.fromkeys(self.ids, False)
+        tocheck_fields = [force_field] if force_field else self._phone_get_number_fields()
+        for record in self:
+            all_numbers = [record[fname] for fname in tocheck_fields if fname in record]
+            all_partners = record._mail_get_partners()[record.id]
+
+            valid_number, fname = False, False
+            for fname in [f for f in tocheck_fields if f in record]:
+                valid_number = record._phone_format(fname=fname)
+                if valid_number:
+                    break
+
+            if valid_number:
+                result[record.id] = {
+                    'partner': all_partners[0] if all_partners else self.env['res.partner'],
+                    'sanitized': valid_number,
+                    'number': record[fname],
+                    'partner_store': False,
+                    'field_store': fname,
+                }
+            elif all_partners and partner_fallback:
+                partner = self.env['res.partner']
+                for partner in all_partners:
+                    for fname in self.env['res.partner']._phone_get_number_fields():
+                        valid_number = partner._phone_format(fname=fname)
+                        if valid_number:
+                            break
+
+                if not valid_number:
+                    fname = 'mobile' if partner.mobile else ('phone' if partner.phone else 'mobile')
+
+                result[record.id] = {
+                    'partner': partner,
+                    'sanitized': valid_number if valid_number else False,
+                    'number': partner[fname],
+                    'partner_store': True,
+                    'field_store': fname,
+                }
+            else:
+                # did not find any sanitized number -> take first set value as fallback;
+                # if none, just assign False to the first available number field
+                value, fname = next(
+                    ((value, fname) for value, fname in zip(all_numbers, tocheck_fields) if value),
+                    (False, tocheck_fields[0] if tocheck_fields else False)
+                )
+                result[record.id] = {
+                    'partner': self.env['res.partner'],
+                    'sanitized': False,
+                    'number': value,
+                    'partner_store': False,
+                    'field_store': fname
+                }
+        return result

addons/sms/wizard/sms_composer.py

@@ -103,7 +103,7 @@ def _compute_recipients(self):
                 continue
 
             records = composer._get_records()
-            if records and isinstance(records, self.pool['mail.thread']):
+            if records:
                 res = records._sms_get_recipients_info(force_field=composer.number_field_name, partner_fallback=not composer.comment_single_recipient)
                 composer.recipient_valid_count = len([rid for rid, rvalues in res.items() if rvalues['sanitized']])
                 composer.recipient_invalid_count = len([rid for rid, rvalues in res.items() if not rvalues['sanitized']])
@@ -116,28 +116,28 @@ def _compute_recipients(self):
     def _compute_recipient_single_stored(self):
         for composer in self:
             records = composer._get_records()
-            if not records or not isinstance(records, self.pool['mail.thread']) or not composer.comment_single_recipient:
+            if not records or not composer.comment_single_recipient:
                 composer.recipient_single_number_itf = ''
                 continue
             records.ensure_one()
             res = records._sms_get_recipients_info(force_field=composer.number_field_name, partner_fallback=True)
             if not composer.recipient_single_number_itf:
-                composer.recipient_single_number_itf = res[records.id]['number'] or ''
+                composer.recipient_single_number_itf = res[records.id]['sanitized'] or res[records.id]['number'] or ''
             if not composer.number_field_name:
                 composer.number_field_name = res[records.id]['field_store']
 
     @api.depends('res_model', 'number_field_name')
     def _compute_recipient_single_non_stored(self):
         for composer in self:
             records = composer._get_records()
-            if not records or not isinstance(records, self.pool['mail.thread']) or not composer.comment_single_recipient:
+            if not records or not composer.comment_single_recipient:
                 composer.recipient_single_description = False
                 composer.recipient_single_number = ''
                 continue
             records.ensure_one()
             res = records._sms_get_recipients_info(force_field=composer.number_field_name, partner_fallback=True)
             composer.recipient_single_description = res[records.id]['partner'].name or records._mail_get_partners()[records[0].id].display_name
-            composer.recipient_single_number = res[records.id]['number'] or ''
+            composer.recipient_single_number = res[records.id]['sanitized'] or res[records.id]['number'] or ''
 
     @api.depends('recipient_single_number', 'recipient_single_number_itf')
     def _compute_recipient_single_valid(self):
@@ -204,7 +204,14 @@ def _action_send_sms(self):
             return self._action_send_sms_mass(records)
 
     def _action_send_sms_numbers(self):
-        sms_values = [{'body': self.body, 'number': number} for number in self.sanitized_numbers.split(',')]
+        sms_values = [
+            {
+                'body': self.body,
+                'number': number
+            } for number in (
+                self.sanitized_numbers.split(',') if self.sanitized_numbers else [self.recipient_single_number_itf or self.recipient_single_number or '']
+            )
+        ]
         self.env['sms.sms'].sudo().create(sms_values).send()
         return True
 

addons/test_mail_sms/models/test_mail_sms_models.py

@@ -150,3 +150,20 @@ def _mailing_get_opt_out_list_sms(self, mailing):
             ('id', 'in', res_ids),
             ('opt_out', '=', True)
         ]).ids
+
+# ------------------------------------------------------------
+# OTHER
+# ------------------------------------------------------------
+
+class SMSTestNotMailThread(models.Model):
+    """ Models not inheriting from mail.thread but using some cross models
+    capabilities of mail. """
+    _name = 'sms.test.nothread'
+    _description = "NoThread Model"
+
+    name = fields.Char()
+    company_id = fields.Many2one('res.company')
+    customer_id = fields.Many2one('res.partner')
+
+    def _mail_get_partner_fields(self, introspect_fields=False):
+        return ['customer_id']

addons/test_mail_sms/security/ir.model.access.csv

@@ -11,3 +11,4 @@ access_mail_test_sms_partner_all,mail.test.sms.partner.all,model_mail_test_sms_p
 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
+access_sms_test_nothread_user,sms.test.nothread.user,model_sms_test_nothread,base.group_user,1,1,1,1

addons/test_mail_sms/tests/__init__.py

@@ -7,6 +7,7 @@
 from . import test_sms_composer
 from . import test_sms_controller
 from . import test_sms_management
+from . import test_sms_mixin
 from . import test_sms_performance
 from . import test_sms_post
 from . import test_sms_server_actions

addons/test_mail_sms/tests/test_sms_composer.py

@@ -184,8 +184,8 @@ def test_composer_nofield_w_customer(self):
                 })
         self.assertFalse(composer.number_field_name)
         self.assertTrue(composer.recipient_single_valid)
-        self.assertEqual(composer.recipient_single_number, self.partner_1.mobile)
-        self.assertEqual(composer.recipient_single_number_itf, self.partner_1.mobile)
+        self.assertEqual(composer.recipient_single_number, self.partner_numbers[0])
+        self.assertEqual(composer.recipient_single_number_itf, self.partner_numbers[0])
 
     def test_composer_internals(self):
         with self.with_user('employee'):
@@ -201,8 +201,8 @@ def test_composer_internals(self):
         self.assertEqual(composer.number_field_name, 'phone_nbr')
         self.assertTrue(composer.comment_single_recipient)
         self.assertEqual(composer.recipient_single_description, self.test_record.customer_id.display_name)
-        self.assertEqual(composer.recipient_single_number, self.test_numbers[1])
-        self.assertEqual(composer.recipient_single_number_itf, self.test_numbers[1])
+        self.assertEqual(composer.recipient_single_number, self.test_numbers_san[1])
+        self.assertEqual(composer.recipient_single_number_itf, self.test_numbers_san[1])
         self.assertTrue(composer.recipient_single_valid)
         self.assertEqual(composer.recipient_valid_count, 1)
         self.assertEqual(composer.recipient_invalid_count, 0)
@@ -289,6 +289,7 @@ def test_composer_sending_with_no_number_field(self):
         self.assertSMSNotification([{'number': self.random_numbers_san[0]}], self._test_body)
 
 
+@tagged('sms_composer')
 class TestSMSComposerBatch(SMSCommon):
     @classmethod
     def setUpClass(cls):
@@ -339,6 +340,7 @@ def test_composer_batch_res_ids(self):
             )
 
 
+@tagged('sms_composer')
 class TestSMSComposerMass(SMSCommon):
 
     @classmethod