From 9a3581d925af1a00849418608fd5db6d3cae203b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Thu, 30 Apr 2020 13:11:26 +0000
Subject: [PATCH] [FIX] sms: fix crash when using composer on a record without
valid phone number
When no valid phone number was found on a record, and when using the composer
in single recipient mode, you had a crash at sending as composer tried to
write on a field called False.
Instead we just take the first available phone field of the record. As they
are all void we can update them safely.
Task ID 2244192
X-original-commit: 2306d7d79d8e64597912f15365cede7473589731
---
addons/sms/models/mail_thread.py | 7 +++-
addons/sms/tests/common.py | 7 ++++
.../test_mail_full/tests/test_sms_composer.py | 32 +++++++++++++++++++
addons/test_mail_full/tests/test_sms_post.py | 14 ++++++++
4 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py
index 820c5230cfe9..e39eb5c029df 100644
--- a/addons/sms/models/mail_thread.py
+++ b/addons/sms/models/mail_thread.py
@@ -129,7 +129,12 @@ class MailThread(models.AbstractModel):
'field_store': fname,
}
else:
- value, fname = next(((value, fname) for value, fname in zip(all_numbers, tocheck_fields) if value), (0, False))
+ # 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),
+ (0, tocheck_fields[0] if tocheck_fields else False)
+ )
result[record.id] = {
'partner': self.env['res.partner'],
'sanitized': False,
diff --git a/addons/sms/tests/common.py b/addons/sms/tests/common.py
index 620351c7c4ba..a5f4e511cfd3 100644
--- a/addons/sms/tests/common.py
+++ b/addons/sms/tests/common.py
@@ -111,6 +111,13 @@ class MockSMS(common.BaseCase):
if content is not None:
self.assertEqual(sms.body, content)
+ def assertNoSMSNotification(self, messages=None):
+ base_domain = [('notification_type', '=', 'sms')]
+ if messages is not None:
+ base_domain += [('mail_message_id', 'in', messages.ids)]
+ self.assertEqual(self.env['mail.notification'].search(base_domain), self.env['mail.notification'])
+ self.assertEqual(self._sms, [])
+
def assertSMSNotification(self, recipients_info, content, messages=None, check_sms=True):
""" Check content of notifications.
diff --git a/addons/test_mail_full/tests/test_sms_composer.py b/addons/test_mail_full/tests/test_sms_composer.py
index 4fc596c19858..44df9efd9512 100644
--- a/addons/test_mail_full/tests/test_sms_composer.py
+++ b/addons/test_mail_full/tests/test_sms_composer.py
@@ -155,6 +155,38 @@ class TestSMSComposerComment(test_mail_full_common.TestSMSCommon, test_mail_full
self.test_record.flush()
self.assertEqual(self.test_record.phone_nbr, self.random_numbers[0])
+ def test_composer_comment_wo_partner_wo_value_update(self):
+ """ Test record without partner and without phone values: should allow updating first found phone field """
+ self.test_record.write({
+ 'customer_id': False,
+ 'phone_nbr': False,
+ 'mobile_nbr': False,
+ })
+ default_field_name = self.env['mail.test.sms']._sms_get_number_fields()[0]
+
+ with self.with_user('employee'):
+ composer = self.env['sms.composer'].with_context(
+ active_model='mail.test.sms', active_id=self.test_record.id,
+ default_composition_mode='comment',
+ ).create({
+ 'body': self._test_body,
+ })
+ self.assertFalse(composer.recipient_single_number_itf)
+ self.assertFalse(composer.recipient_single_number)
+ self.assertEqual(composer.number_field_name, default_field_name)
+
+ composer.write({
+ 'recipient_single_number_itf': self.random_numbers_san[0],
+ })
+ self.assertEqual(composer.recipient_single_number_itf, self.random_numbers_san[0])
+ self.assertFalse(composer.recipient_single_number)
+
+ with self.mockSMSGateway():
+ messages = composer._action_send_sms()
+
+ self.assertEqual(self.test_record[default_field_name], self.random_numbers_san[0])
+ self.assertSMSNotification([{'partner': self.env['res.partner'], 'number': self.random_numbers_san[0]}], self._test_body, messages)
+
def test_composer_numbers_no_model(self):
with self.with_user('employee'):
composer = self.env['sms.composer'].with_context(
diff --git a/addons/test_mail_full/tests/test_sms_post.py b/addons/test_mail_full/tests/test_sms_post.py
index b28ac3a0a37f..26b5c5268b55 100644
--- a/addons/test_mail_full/tests/test_sms_post.py
+++ b/addons/test_mail_full/tests/test_sms_post.py
@@ -125,6 +125,20 @@ class TestSMSPost(test_mail_full_common.TestSMSCommon, test_mail_full_common.Tes
self.assertSMSNotification([{'number': self.test_record.mobile_nbr}], self._test_body, messages)
+ def test_message_sms_on_field_wo_partner_wo_value(self):
+ """ Test record without a partner and without phone values. """
+ self.test_record.write({
+ 'customer_id': False,
+ 'phone_nbr': False,
+ 'mobile_nbr': False,
+ })
+
+ with self.with_user('employee'), self.mockSMSGateway():
+ test_record = self.env['mail.test.sms'].browse(self.test_record.id)
+ messages = test_record._message_sms(self._test_body)
+
+ self.assertNoSMSNotification(messages)
+
def test_message_sms_on_field_wo_partner_default_field(self):
self.test_record.write({'customer_id': False})
--
GitLab