From c9e6e7326c09d82d272fda8ba17bfe6cb704b515 Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Fri, 24 Apr 2020 18:04:36 +0200
Subject: [PATCH 2/3] sms: factorise sms code on parameters (#21465)

---
 passerelle/base/models.py | 59 ++++++++++++++++++++++++---------------
 tests/test_sms.py         |  2 +-
 2 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/passerelle/base/models.py b/passerelle/base/models.py
index 4ab5efcb..39b77a27 100644
--- a/passerelle/base/models.py
+++ b/passerelle/base/models.py
@@ -917,57 +917,72 @@ class HTTPResource(models.Model):
 class SMSResource(BaseResource):
     category = _('SMS Providers')
     documentation_url = 'https://doc-publik.entrouvert.com/admin-fonctionnel/les-tutos/configuration-envoi-sms/'
 
     _can_send_messages_description = _('Sending messages is limited to the following API users:')
 
     max_message_length = models.IntegerField(_('Maximum message length'), default=160)
 
+    @classmethod
+    def clean_number(cls, destination, default_country_code='33',
+                     default_trunk_prefix='0'):  # Yeah France first !
+        # most gateways needs the number prefixed by the country code, this is
+        # really unfortunate.
+        dest = destination.strip()
+        number = ''.join(re.findall('[0-9]', dest))
+        if dest.startswith('+'):
+            number = '00' + number
+        elif number.startswith('00'):
+            # assumes 00 is international access code, remove it
+            pass
+        elif number.startswith(default_trunk_prefix):
+            number = '00' + default_country_code + number[len(default_trunk_prefix):]
+        else:
+            raise APIError('phone number %r is unsupported (no international prefix, '
+                           'no local trunk prefix)' % number)
+        return number
+
     @classmethod
     def clean_numbers(cls, destinations, default_country_code='33',
                       default_trunk_prefix='0'):  # Yeah France first !
         numbers = []
         for dest in destinations:
-            # most gateways needs the number prefixed by the country code, this is
-            # really unfortunate.
-            dest = dest.strip()
-            number = ''.join(re.findall('[0-9]', dest))
-            if dest.startswith('+'):
-                number = '00' + number
-            elif number.startswith('00'):
-                # assumes 00 is international access code, remove it
-                pass
-            elif number.startswith(default_trunk_prefix):
-                number = '00' + default_country_code + number[len(default_trunk_prefix):]
-            else:
-                raise APIError('phone number %r is unsupported (no international prefix, '
-                               'no local trunk prefix)' % number)
-            numbers.append(number)
+            numbers.append(cls.clean_number(
+                dest, default_country_code, default_trunk_prefix))
         return numbers
 
-    @endpoint(perm='can_send_messages', methods=['post'])
-    def send(self, request, *args, **kwargs):
+    def get_parameters(self, request, *args, **kwargs):
         try:
             data = json_loads(request.body)
             assert isinstance(data, dict), 'JSON payload is not a dict'
             assert 'message' in data, 'missing "message" in JSON payload'
             assert 'from' in data, 'missing "from" in JSON payload'
             assert 'to' in data, 'missing "to" in JSON payload'
             assert isinstance(data['message'], six.text_type), 'message is not a string'
             assert isinstance(data['from'], six.text_type), 'from is not a string'
             assert all(map(lambda x: isinstance(x, six.text_type), data['to'])), \
                 'to is not a list of strings'
         except (ValueError, AssertionError) as e:
             raise APIError('Payload error: %s' % e)
-        data['message'] = data['message'][:self.max_message_length]
-        logging.info('sending message %r to %r with sending number %r',
-                     data['message'], data['to'], data['from'])
-        stop = not bool('nostop' in request.GET)
-        result = {'data': self.send_msg(data['message'], data['from'], data['to'], stop=stop)}
+        parameters = {
+            'text': data['message'][:self.max_message_length],
+            'sender': data['from'],
+            'destinations': data['to'],
+            'stop': not bool('nostop' in request.GET)
+        }
+        logging.info(
+            'sending message %r to %r with sending number %r',
+            parameters['text'], parameters['destinations'], parameters['sender'])
+        return parameters
+
+    @endpoint(perm='can_send_messages', methods=['post'])
+    def send(self, request, *args, **kwargs):
+        parameters = self.get_parameters(request, *args, **kwargs)
+        result = {'data': self.send_msg(**parameters)}
         SMSLog.objects.create(appname=self.get_connector_slug(), slug=self.slug)
         return result
 
     class Meta:
         abstract = True
 
 
 class SMS(models.Model):
diff --git a/tests/test_sms.py b/tests/test_sms.py
index 53d6f2ca..a8224e88 100644
--- a/tests/test_sms.py
+++ b/tests/test_sms.py
@@ -86,17 +86,17 @@ def test_sms_max_message_length(app, connector):
     payload = {
         'message': message_above_limit,
         'from': '+33699999999',
         'to': ['+33688888888'],
     }
     with mock.patch.object(OVHSMSGateway, 'send_msg') as send_function:
         send_function.return_value = {}
         result = app.post_json(path, params=payload)
-        assert send_function.call_args[0][0] == 'a' * connector.max_message_length
+        assert send_function.call_args[1]['text'] == 'a' * connector.max_message_length
 
 
 @pytest.mark.parametrize('connector', [OVHSMSGateway], indirect=True)
 def test_sms_log(app, connector):
     path = '/%s/%s/send/' % (connector.get_connector_slug(), connector.slug)
     assert not SMSLog.objects.filter(appname=connector.get_connector_slug(), slug=connector.slug).exists()
 
     payload = {
-- 
2.26.2

