[MERGE] sms: better actions for send / cancel / set outgoing

Improve cancel / set outgoing actions on sms model. This notably allows to
synchronize notifications when hitting action buttons on sms form view.

Also add an action to set as error. This will be used when having IAP
feedback on SMS sending.

Improve `send` API, to allow fine control of what is unlinked after sending:
sms in errors and/or sms sent. This allows notably to keep all outgoing sms
notably when IAP feedback will be added.

See sub commits for more details.

Task-2634957
Prepares Task-2535005 (SMS view pimp) and Task-2560666 (IAP feedback)

closes odoo/odoo#75798

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

addons/mass_mailing_sms/models/sms_sms.py

@@ -28,7 +28,7 @@ class SmsSms(models.Model):
             res[sms.id] = body
         return res
 
-    def _postprocess_iap_sent_sms(self, iap_results, failure_reason=None, delete_all=False):
+    def _postprocess_iap_sent_sms(self, iap_results, failure_reason=None, unlink_failed=False, unlink_sent=True):
         all_sms_ids = [item['res_id'] for item in iap_results]
         if any(sms.mailing_id for sms in self.env['sms.sms'].sudo().browse(all_sms_ids)):
             for state in self.IAP_TO_SMS_STATE.keys():
@@ -40,4 +40,7 @@ class SmsSms(models.Model):
                     traces.set_sent()
                 elif traces:
                     traces.set_failed(failure_type=self.IAP_TO_SMS_STATE[state])
-        return super(SmsSms, self)._postprocess_iap_sent_sms(iap_results, failure_reason=failure_reason, delete_all=delete_all)
+        return super(SmsSms, self)._postprocess_iap_sent_sms(
+            iap_results, failure_reason=failure_reason,
+            unlink_failed=unlink_failed, unlink_sent=unlink_sent
+        )

addons/sms/models/sms_sms.py

@@ -45,23 +45,57 @@ class SmsSms(models.Model):
         ('sms_optout', 'Opted Out'),
     ], copy=False)
 
-    def send(self, delete_all=False, auto_commit=False, raise_exception=False):
+    def action_set_canceled(self):
+        self.state = 'canceled'
+        notifications = self.env['mail.notification'].sudo().search([
+            ('sms_id', 'in', self.ids),
+            # sent is sent -> cannot reset
+            ('notification_status', 'not in', ['canceled', 'sent']),
+        ])
+        if notifications:
+            notifications.write({'notification_status': 'canceled'})
+            if not self._context.get('sms_skip_msg_notification', False):
+                notifications.mail_message_id._notify_message_notification_update()
+
+    def action_set_error(self, failure_type):
+        self.state = 'error'
+        self.failure_type = failure_type
+        notifications = self.env['mail.notification'].sudo().search([
+            ('sms_id', 'in', self.ids),
+            # sent can be set to error due to IAP feedback
+            ('notification_status', '!=', 'exception'),
+        ])
+        if notifications:
+            notifications.write({'notification_status': 'exception', 'failure_type': failure_type})
+            if not self._context.get('sms_skip_msg_notification', False):
+                notifications.mail_message_id._notify_message_notification_update()
+
+    def action_set_outgoing(self):
+        self.state = 'outgoing'
+        notifications = self.env['mail.notification'].sudo().search([
+            ('sms_id', 'in', self.ids),
+            # sent is sent -> cannot reset
+            ('notification_status', 'not in', ['ready', 'sent']),
+        ])
+        if notifications:
+            notifications.write({'notification_status': 'ready', 'failure_type': False})
+            if not self._context.get('sms_skip_msg_notification', False):
+                notifications.mail_message_id._notify_message_notification_update()
+
+    def send(self, unlink_failed=False, unlink_sent=True, auto_commit=False, raise_exception=False):
         """ Main API method to send SMS.
 
-          :param delete_all: delete all SMS (sent or not); otherwise delete only
-            sent SMS;
+          :param unlink_failed: unlink failed SMS after IAP feedback;
+          :param unlink_sent: unlink sent SMS after IAP feedback;
           :param auto_commit: commit after each batch of SMS;
           :param raise_exception: raise if there is an issue contacting IAP;
         """
         for batch_ids in self._split_batch():
-            self.browse(batch_ids)._send(delete_all=delete_all, raise_exception=raise_exception)
+            self.browse(batch_ids)._send(unlink_failed=unlink_failed, unlink_sent=unlink_sent, raise_exception=raise_exception)
             # auto-commit if asked except in testing mode
             if auto_commit is True and not getattr(threading.currentThread(), 'testing', False):
                 self._cr.commit()
 
-    def set_outgoing(self):
-        self.state = 'outgoing'
-
     def resend_failed(self):
         sms_to_send = self.filtered(lambda sms: sms.state == 'error')
         notification_title = _('Warning')
@@ -88,9 +122,6 @@ class SmsSms(models.Model):
             }
         }
 
-    def cancel(self):
-        self.state = 'canceled'
-
     @api.model
     def _process_queue(self, ids=None):
         """ Send immediately queued messages, committing after each message is sent.
@@ -112,7 +143,7 @@ class SmsSms(models.Model):
         try:
             # auto-commit except in testing mode
             auto_commit = not getattr(threading.currentThread(), 'testing', False)
-            res = self.browse(ids).send(delete_all=False, auto_commit=auto_commit, raise_exception=False)
+            res = self.browse(ids).send(unlink_failed=False, unlink_sent=True, auto_commit=auto_commit, raise_exception=False)
         except Exception:
             _logger.exception("Failed processing SMS queue")
         return res
@@ -122,7 +153,7 @@ class SmsSms(models.Model):
         for sms_batch in tools.split_every(batch_size, self.ids):
             yield sms_batch
 
-    def _send(self, delete_all=False, raise_exception=False):
+    def _send(self, unlink_failed=False, unlink_sent=True, raise_exception=False):
         """ This method tries to send SMS after checking the number (presence and
         formatting). """
         iap_data = [{
@@ -137,25 +168,33 @@ class SmsSms(models.Model):
             _logger.info('Sent batch %s SMS: %s: failed with exception %s', len(self.ids), self.ids, e)
             if raise_exception:
                 raise
-            self._postprocess_iap_sent_sms([{'res_id': sms.id, 'state': 'server_error'} for sms in self], delete_all=delete_all)
+            self._postprocess_iap_sent_sms(
+                [{'res_id': sms.id, 'state': 'server_error'} for sms in self],
+                unlink_failed=unlink_failed, unlink_sent=unlink_sent)
         else:
             _logger.info('Send batch %s SMS: %s: gave %s', len(self.ids), self.ids, iap_results)
-            self._postprocess_iap_sent_sms(iap_results, delete_all=delete_all)
+            self._postprocess_iap_sent_sms(iap_results, unlink_failed=unlink_failed, unlink_sent=unlink_sent)
 
-    def _postprocess_iap_sent_sms(self, iap_results, failure_reason=None, delete_all=False):
-        if delete_all:
-            todelete_sms_ids = [item['res_id'] for item in iap_results]
-        else:
-            todelete_sms_ids = [item['res_id'] for item in iap_results if item['state'] == 'success']
+    def _postprocess_iap_sent_sms(self, iap_results, failure_reason=None, unlink_failed=False, unlink_sent=True):
+        todelete_sms_ids = []
+        if unlink_failed:
+            todelete_sms_ids += [item['res_id'] for item in iap_results if item['state'] != 'success']
+        if unlink_sent:
+            todelete_sms_ids += [item['res_id'] for item in iap_results if item['state'] == 'success']
 
         for state in self.IAP_TO_SMS_STATE.keys():
             sms_ids = [item['res_id'] for item in iap_results if item['state'] == state]
             if sms_ids:
-                if state != 'success' and not delete_all:
+                if state != 'success' and not unlink_failed:
                     self.env['sms.sms'].sudo().browse(sms_ids).write({
                         'state': 'error',
                         'failure_type': self.IAP_TO_SMS_STATE[state],
                     })
+                if state == 'success' and not unlink_sent:
+                    self.env['sms.sms'].sudo().browse(sms_ids).write({
+                        'state': 'sent',
+                        'failure_type': False,
+                    })
                 notifications = self.env['mail.notification'].sudo().search([
                     ('notification_type', '=', 'sms'),
                     ('sms_id', 'in', sms_ids),

addons/sms/tests/common.py

@@ -21,7 +21,7 @@ class MockSMS(common.BaseCase):
     def mockSMSGateway(self, sms_allow_unlink=False, sim_error=None, nbr_t_error=None):
         self._clear_sms_sent()
         sms_create_origin = SmsSms.create
-        sms_unlink_origin = SmsSms.unlink
+        sms_send_origin = SmsSms._send
 
         def _contact_iap(local_endpoint, params):
             # mock single sms sending
@@ -60,18 +60,17 @@ class MockSMS(common.BaseCase):
             self._new_sms += res.sudo()
             return res
 
-        def _sms_sms_unlink(records, *args, **kwargs):
+        def _sms_sms_send(records, unlink_failed=False, unlink_sent=True, raise_exception=False):
             if sms_allow_unlink:
-                return sms_unlink_origin(records, *args, **kwargs)
-            # hack: instead of unlink, update state to sent for tests
+                return sms_send_origin(records, unlink_failed=unlink_failed, unlink_sent=unlink_sent, raise_exception=raise_exception)
             else:
-                records.filtered(lambda sms: sms.id in self._new_sms.ids).state = 'sent'
+                return sms_send_origin(records, unlink_failed=False, unlink_sent=False, raise_exception=raise_exception)
             return True
 
         try:
             with patch.object(SmsApi, '_contact_iap', side_effect=_contact_iap), \
                     patch.object(SmsSms, 'create', autospec=True, wraps=SmsSms, side_effect=_sms_sms_create), \
-                    patch.object(SmsSms, 'unlink', autospec=True, wraps=SmsSms, side_effect=_sms_sms_unlink):
+                    patch.object(SmsSms, '_send', autospec=True, wraps=SmsSms, side_effect=_sms_sms_send):
                 yield
         finally:
             pass

addons/sms/views/sms_sms_views.xml

@@ -7,8 +7,8 @@
             <form string="SMS">
                 <header>
                     <button name="send" string="Send Now" type="object" states='outgoing' class="oe_highlight"/>
-                    <button name="set_outgoing" string="Retry" type="object" states='error,canceled'/>
-                    <button name="cancel" string="Cancel" type="object" states='error,outgoing'/>
+                    <button name="action_set_outgoing" string="Retry" type="object" states='error,canceled'/>
+                    <button name="action_set_canceled" string="Cancel" type="object" states='error,outgoing'/>
                     <field name="state" widget="statusbar"/>
                 </header>
                 <sheet>

addons/test_mail_full/tests/test_sms_management.py

@@ -2,36 +2,116 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo.addons.test_mail_full.tests.common import TestMailFullCommon, TestMailFullRecipients
+from odoo.tests import tagged
+from odoo.tools import mute_logger
 
 
-class TestSMSWizards(TestMailFullCommon, TestMailFullRecipients):
+class TestSMSActionsCommon(TestMailFullCommon, TestMailFullRecipients):
 
     @classmethod
     def setUpClass(cls):
-        super(TestSMSWizards, cls).setUpClass()
+        super(TestSMSActionsCommon, cls).setUpClass()
         cls.test_record = cls.env['mail.test.sms'].with_context(**cls._test_context).create({
             'name': 'Test',
             'customer_id': cls.partner_1.id,
         })
         cls.test_record = cls._reset_mail_context(cls.test_record)
         cls.msg = cls.test_record.message_post(body='TEST BODY', author_id=cls.partner_employee.id)
+        cls.sms_p1 = cls.env['sms.sms'].create({
+            'body': 'TEST BODY',
+            'failure_type': 'sms_number_format',
+            'mail_message_id': cls.msg.id,
+            'number': cls.partner_1.mobile,
+            'partner_id': cls.partner_1.id,
+            'state': 'error',
+        })
         cls.notif_p1 = cls.env['mail.notification'].create({
             'mail_message_id': cls.msg.id,
             'res_partner_id': cls.partner_1.id,
+            'sms_id': cls.sms_p1.id,
             'sms_number': cls.partner_1.mobile,
             'notification_type': 'sms',
             'notification_status': 'exception',
             'failure_type': 'sms_number_format',
         })
+        cls.sms_p2 = cls.env['sms.sms'].create({
+            'body': 'TEST BODY',
+            'failure_type': 'sms_credit',
+            'mail_message_id': cls.msg.id,
+            'number': cls.partner_2.mobile,
+            'partner_id': cls.partner_2.id,
+            'state': 'error',
+        })
         cls.notif_p2 = cls.env['mail.notification'].create({
             'mail_message_id': cls.msg.id,
             'res_partner_id': cls.partner_2.id,
+            'sms_id': cls.sms_p2.id,
             'sms_number': cls.partner_2.mobile,
             'notification_type': 'sms',
             'notification_status': 'exception',
             'failure_type': 'sms_credit',
         })
 
+
+@tagged('sms_management')
+class TestSMSActions(TestSMSActionsCommon):
+
+    def test_sms_set_cancel(self):
+        self._reset_bus()
+        self.sms_p1.action_set_canceled()
+        self.assertEqual(self.sms_p1.state, 'canceled')
+
+        self.assertMessageBusNotifications(self.msg)
+        self.assertSMSNotification([
+            {'partner': self.partner_1, 'number': self.notif_p1.sms_number, 'state': 'canceled', 'failure_type': 'sms_number_format'},
+            {'partner': self.partner_2, 'number': self.notif_p2.sms_number, 'state': 'exception', 'failure_type': 'sms_credit'}
+        ], 'TEST BODY', self.msg, check_sms=False)    # do not check new sms as they already exist
+
+        self._reset_bus()
+        self.sms_p2.with_context(sms_skip_msg_notification=True).action_set_canceled()
+        self.assertEqual(self.sms_p2.state, 'canceled')
+
+        self.assertEqual(self.env['bus.bus'].search([]), self.env['bus.bus'], 'SMS: no bus notifications unless asked')
+        self.assertSMSNotification([
+            {'partner': self.partner_1, 'number': self.notif_p1.sms_number, 'state': 'canceled', 'failure_type': 'sms_number_format'},
+            {'partner': self.partner_2, 'number': self.notif_p2.sms_number, 'state': 'canceled', 'failure_type': 'sms_credit'}
+        ], 'TEST BODY', self.msg, check_sms=False)    # do not check new sms as they already exist
+
+
+    def test_sms_set_error(self):
+        self._reset_bus()
+        (self.sms_p1 + self.sms_p2).with_context(sms_skip_msg_notification=True).action_set_canceled()
+        self.assertEqual(self.sms_p1.state, 'canceled')
+        self.assertEqual(self.sms_p2.state, 'canceled')
+        self.assertEqual(self.env['bus.bus'].search([]), self.env['bus.bus'], 'SMS: no bus notifications unless asked')
+
+        (self.sms_p1 + self.sms_p2).action_set_error('sms_server')
+        self.assertEqual(self.sms_p1.state, 'error')
+        self.assertEqual(self.sms_p2.state, 'error')
+
+        self.assertMessageBusNotifications(self.msg)
+        self.assertSMSNotification([
+            {'partner': self.partner_1, 'number': self.notif_p1.sms_number, 'state': 'exception', 'failure_type': 'sms_server'},
+            {'partner': self.partner_2, 'number': self.notif_p2.sms_number, 'state': 'exception', 'failure_type': 'sms_server'}
+        ], 'TEST BODY', self.msg, check_sms=False)    # do not check new sms as they already exist
+
+    def test_sms_set_outgoing(self):
+        self._reset_bus()
+        (self.sms_p1 + self.sms_p2).action_set_outgoing()
+        self.assertEqual(self.sms_p1.state, 'outgoing')
+        self.assertEqual(self.sms_p2.state, 'outgoing')
+
+        self.assertMessageBusNotifications(self.msg)
+        self.assertSMSNotification([
+            {'partner': self.partner_1, 'number': self.notif_p1.sms_number, 'state': 'ready'},
+            {'partner': self.partner_2, 'number': self.notif_p2.sms_number, 'state': 'ready'}
+        ], 'TEST BODY', self.msg, check_sms=False)    # do not check new sms as they already exist
+
+
+@tagged('sms_management')
+class TestSMSWizards(TestSMSActionsCommon):
+
+    @mute_logger('odoo.addons.sms.models.sms_sms')
     def test_sms_resend(self):
         self._reset_bus()
 
@@ -47,6 +127,7 @@ class TestSMSWizards(TestMailFullCommon, TestMailFullRecipients):
         ], 'TEST BODY', self.msg, check_sms=True)
         self.assertMessageBusNotifications(self.msg)
 
+    @mute_logger('odoo.addons.sms.models.sms_sms')
     def test_sms_resend_update_number(self):
         self._reset_bus()
 
@@ -76,6 +157,7 @@ class TestSMSWizards(TestMailFullCommon, TestMailFullRecipients):
         ], 'TEST BODY', self.msg, check_sms=False)
         self.assertMessageBusNotifications(self.msg)
 
+    @mute_logger('odoo.addons.sms.models.sms_sms')
     def test_sms_resend_internals(self):
         self._reset_bus()
         self.assertSMSNotification([
@@ -91,6 +173,7 @@ class TestSMSWizards(TestMailFullCommon, TestMailFullRecipients):
             with self.mockSMSGateway():
                 wizard.action_resend()
 
+    @mute_logger('odoo.addons.sms.models.sms_sms')
     def test_sms_resend_w_cancel(self):
         self._reset_bus()
 

addons/test_mail_full/tests/test_sms_sms.py

@@ -131,7 +131,7 @@ class TestSMSPost(TestMailFullCommon, LinkTrackerMock):
     def test_sms_send_batch_size(self):
         self.count = 0
 
-        def _send(sms_self, delete_all=False, raise_exception=False):
+        def _send(sms_self, unlink_failed=False, unlink_sent=True, raise_exception=False):
             self.count += 1
             return DEFAULT
 
@@ -147,7 +147,7 @@ class TestSMSPost(TestMailFullCommon, LinkTrackerMock):
 
     def test_sms_send_delete_all(self):
         with self.mockSMSGateway(sms_allow_unlink=True, sim_error='jsonrpc_exception'):
-            self.env['sms.sms'].browse(self.sms_all.ids).send(delete_all=True, raise_exception=False)
+            self.env['sms.sms'].browse(self.sms_all.ids).send(unlink_failed=True, unlink_sent=True, raise_exception=False)
         self.assertFalse(len(self.sms_all.exists()))
 
     def test_sms_send_raise(self):