From a5ef9a42748871652cd820861e6277f51a6e322d Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Tue, 5 May 2020 11:26:48 +0200
Subject: [PATCH] sms: move default country_code/trunk_prefix to base class
 (#42427)

---
 passerelle/apps/choosit/models.py | 10 +---------
 passerelle/apps/mobyt/models.py   | 10 +---------
 passerelle/apps/orange/models.py  |  8 +-------
 passerelle/apps/ovh/models.py     |  8 --------
 passerelle/apps/oxyd/models.py    | 10 +---------
 passerelle/base/models.py         | 16 ++++++++++------
 tests/test_sms.py                 | 19 +++++++++++++------
 7 files changed, 27 insertions(+), 54 deletions(-)

diff --git a/passerelle/apps/choosit/models.py b/passerelle/apps/choosit/models.py
index a4678dcd..afb72b53 100644
--- a/passerelle/apps/choosit/models.py
+++ b/passerelle/apps/choosit/models.py
@@ -7,21 +7,16 @@ from django.utils.translation import ugettext_lazy as _
 from django.db import models
 
 from passerelle.utils.jsonresponse import APIError
 from passerelle.base.models import SMSResource
 
 
 class ChoositSMSGateway(SMSResource):
     key = models.CharField(verbose_name=_('Key'), max_length=64)
-    default_country_code = models.CharField(verbose_name=_('Default country code'), max_length=3,
-                                            default=u'33')
-    default_trunk_prefix = models.CharField(verbose_name=_('Default trunk prefix'), max_length=2,
-                                            default=u'0')
-    # FIXME: add regexp field, to check destination and from format
 
     TEST_DEFAULTS = {
         'create_kwargs': {
             'key': '1234',
         },
         'test_vectors': [
             {
                 'response': '',
@@ -71,20 +66,17 @@ class ChoositSMSGateway(SMSResource):
 
     @classmethod
     def get_verbose_name(cls):
         return cls._meta.verbose_name
 
     def send_msg(self, text, sender, destinations, **kwargs):
         """Send a SMS using the Choosit provider"""
         # from http://sms.choosit.com/documentation_technique.html
-        # unfortunately it lacks a batch API...
-        destinations = self.clean_numbers(destinations,
-                                          self.default_country_code,
-                                          self.default_trunk_prefix)
+
         results = []
         for dest in destinations:
             params = {
                 'key': self.key,
                 'recipient': dest,
                 'content': text[:160],
             }
             data = {'data': json.dumps(params)}
diff --git a/passerelle/apps/mobyt/models.py b/passerelle/apps/mobyt/models.py
index 4983b9d7..1dcb1899 100644
--- a/passerelle/apps/mobyt/models.py
+++ b/passerelle/apps/mobyt/models.py
@@ -13,21 +13,16 @@ class MobytSMSGateway(SMSResource):
         ('l', _('sms direct')),
         ('ll', _('sms low-cost')),
         ('n', _('sms top')),
     )
     username = models.CharField(verbose_name=_('Username'), max_length=64)
     password = models.CharField(verbose_name=_('Password'), max_length=64)
     quality = models.CharField(max_length=4, choices=MESSAGES_QUALITIES, default='l',
                                verbose_name=_('Message quality'))
-    default_country_code = models.CharField(verbose_name=_('Default country code'), max_length=3,
-                                            default=u'33')
-    default_trunk_prefix = models.CharField(verbose_name=_('Default trunk prefix'), max_length=2,
-                                            default=u'0')
-    # FIXME: add regexp field, to check destination and from format
 
     manager_view_template_name = 'passerelle/manage/messages_service_view.html'
 
     TEST_DEFAULTS = {
         'create_kwargs': {
             'username': 'john',
             'password': 'doe',
         },
@@ -45,20 +40,17 @@ class MobytSMSGateway(SMSResource):
     }
 
     class Meta:
         verbose_name = 'Mobyt'
         db_table = 'sms_mobyt'
 
     def send_msg(self, text, sender, destinations, **kwargs):
         """Send a SMS using the Mobyt provider"""
-        # unfortunately it lacks a batch API...
-        destinations = self.clean_numbers(destinations,
-                                          self.default_country_code,
-                                          self.default_trunk_prefix)
+
         rcpt = ','.join(destinations)
         params = {
             'user': self.username,
             'pass': self.password,
             'rcpt': rcpt,
             'data': text,
             'sender': sender,
             'qty': self.quality,
diff --git a/passerelle/apps/orange/models.py b/passerelle/apps/orange/models.py
index e1b17613..f0fe6a4b 100644
--- a/passerelle/apps/orange/models.py
+++ b/passerelle/apps/orange/models.py
@@ -42,21 +42,16 @@ def get_json(response):
         raise OrangeError('Orange returned Invalid JSON content: %s' % response.text)
 
 
 class OrangeSMSGateway(SMSResource):
     username = models.CharField(verbose_name=_('Identifier'), max_length=64)
     password = models.CharField(verbose_name=_('Password'), max_length=64)
     groupname = models.CharField(verbose_name=_('Group'), max_length=64)
 
-    default_country_code = models.CharField(
-        verbose_name=_('Default country code'), max_length=3, default='33')
-    default_trunk_prefix = models.CharField(
-        verbose_name=_('Default trunk prefix'), max_length=2, default='0')
-
     manager_view_template_name = 'passerelle/manage/messages_service_view.html'
 
     class Meta:
         verbose_name = _('Orange')
         db_table = 'sms_orange'
 
     def get_access_token(self):
         headers = {'content-type': 'application/x-www-form-urlencoded'}
@@ -103,14 +98,13 @@ class OrangeSMSGateway(SMSResource):
             URL_DIFFUSION % group_id, json=payload, headers=headers)
         if response.status_code != 201:
             raise OrangeError('Orange fails to send SMS: %s, %s' % (
                 response.status_code, response.text))
         return get_json(response)
 
     def send_msg(self, text, sender, destinations, **kwargs):
         '''Send a SMS using the Orange provider'''
-        destinations = self.clean_numbers(
-            destinations, self.default_country_code, self.default_trunk_prefix)
+
         access_token = self.get_access_token()
         group_id = self.group_id_from_name(access_token)
         response = self.diffusion(access_token, group_id, destinations, text)
         return response
diff --git a/passerelle/apps/ovh/models.py b/passerelle/apps/ovh/models.py
index d9d96135..529d56c5 100644
--- a/passerelle/apps/ovh/models.py
+++ b/passerelle/apps/ovh/models.py
@@ -23,22 +23,17 @@ class OVHSMSGateway(SMSResource):
     )
     account = models.CharField(verbose_name=_('Account'), max_length=64)
     username = models.CharField(verbose_name=_('Username'), max_length=64)
     password = models.CharField(verbose_name=_('Password'), max_length=64)
     msg_class = models.IntegerField(choices=MESSAGES_CLASSES, default=1,
                                     verbose_name=_('Message class'))
     credit_threshold_alert = models.PositiveIntegerField(verbose_name=_('Credit alert threshold'),
                                                          default=100)
-    default_country_code = models.CharField(verbose_name=_('Default country code'), max_length=3,
-                                            default=u'33')
-    default_trunk_prefix = models.CharField(verbose_name=_('Default trunk prefix'), max_length=2,
-                                            default=u'0')
     credit_left = models.PositiveIntegerField(verbose_name=_('Credit left'), default=0)
-    # FIXME: add regexp field, to check destination and from format
 
     manager_view_template_name = 'passerelle/manage/messages_service_view.html'
 
     TEST_DEFAULTS = {
         'create_kwargs': {
             'account': '1234',
             'username': 'john',
             'password': 'doe',
@@ -77,19 +72,16 @@ class OVHSMSGateway(SMSResource):
     }
 
     class Meta:
         verbose_name = 'OVH'
         db_table = 'sms_ovh'
 
     def send_msg(self, text, sender, destinations, **kwargs):
         """Send a SMS using the OVH provider"""
-        destinations = self.clean_numbers(destinations,
-                                          self.default_country_code,
-                                          self.default_trunk_prefix)
 
         text = force_text(text).encode('utf-8')
         to = ','.join(destinations)
         params = {
             'account': self.account.encode('utf-8'),
             'login': self.username.encode('utf-8'),
             'password': self.password.encode('utf-8'),
             'from': sender.encode('utf-8'),
diff --git a/passerelle/apps/oxyd/models.py b/passerelle/apps/oxyd/models.py
index 5e945b16..f1482cae 100644
--- a/passerelle/apps/oxyd/models.py
+++ b/passerelle/apps/oxyd/models.py
@@ -6,21 +6,16 @@ from django.utils.encoding import force_text
 from passerelle.utils.jsonresponse import APIError
 from passerelle.base.models import SMSResource
 from django.utils.translation import ugettext_lazy as _
 
 
 class OxydSMSGateway(SMSResource):
     username = models.CharField(verbose_name=_('Username'), max_length=64)
     password = models.CharField(verbose_name=_('Password'), max_length=64)
-    default_country_code = models.CharField(verbose_name=_('Default country code'), max_length=3,
-                                            default=u'33')
-    default_trunk_prefix = models.CharField(verbose_name=_('Default trunk prefix'), max_length=2,
-                                            default=u'0')
-    # FIXME: add regexp field, to check destination and from format
 
     manager_view_template_name = 'passerelle/manage/messages_service_view.html'
 
     TEST_DEFAULTS = {
         'create_kwargs': {
             'username': 'john',
             'password': 'doe',
         },
@@ -48,20 +43,17 @@ class OxydSMSGateway(SMSResource):
     URL = 'http://sms.oxyd.fr/send.php'
 
     class Meta:
         verbose_name = 'Oxyd'
         db_table = 'sms_oxyd'
 
     def send_msg(self, text, sender, destinations, **kwargs):
         """Send a SMS using the Oxyd provider"""
-        # unfortunately it lacks a batch API...
-        destinations = self.clean_numbers(destinations,
-                                          self.default_country_code,
-                                          self.default_trunk_prefix)
+
         results = []
         for dest in destinations:
             params = {
                 'id': self.username,
                 'pass': self.password,
                 'num': dest,
                 'sms': text.encode('utf-8'),
                 'flash': '0',
diff --git a/passerelle/base/models.py b/passerelle/base/models.py
index cdf322ff..ed331865 100644
--- a/passerelle/base/models.py
+++ b/passerelle/base/models.py
@@ -916,34 +916,37 @@ 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:')
 
+    default_country_code = models.CharField(verbose_name=_('Default country code'), max_length=3,
+                                            default=u'33')
+    default_trunk_prefix = models.CharField(verbose_name=_('Default trunk prefix'), max_length=2,
+                                            default=u'0')
     max_message_length = models.IntegerField(_('Maximum message length'), default=160)
 
-    @classmethod
-    def clean_numbers(cls, destinations, default_country_code='33',
-                      default_trunk_prefix='0'):  # Yeah France first !
+    def clean_numbers(self, destinations):
+
         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):]
+            elif number.startswith(self.default_trunk_prefix):
+                number = '00' + self.default_country_code + number[len(self.default_trunk_prefix):]
             else:
                 raise APIError('phone number %r is unsupported (no international prefix, '
                                'no local trunk prefix)' % number)
             numbers.append(number)
         return numbers
 
     @endpoint(perm='can_send_messages', methods=['post'])
     def send(self, request, *args, **kwargs):
@@ -955,19 +958,20 @@ class SMSResource(BaseResource):
             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]
+        data['to'] = self.clean_numbers(data['to'])
+        stop = not bool('nostop' in request.GET)
         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)}
         SMSLog.objects.create(appname=self.get_connector_slug(), slug=self.slug)
         return result
 
     class Meta:
         abstract = True
 
 
diff --git a/tests/test_sms.py b/tests/test_sms.py
index c4b9d5c9..d8f74197 100644
--- a/tests/test_sms.py
+++ b/tests/test_sms.py
@@ -1,32 +1,39 @@
 import mock
 import pytest
 
 from django.contrib.contenttypes.models import ContentType
 
 from passerelle.apps.ovh.models import OVHSMSGateway
 from passerelle.base.models import ApiUser, AccessRight, SMSResource, SMSLog
+from passerelle.utils.jsonresponse import APIError
 
 from test_manager import login, admin_user
 
 import utils
 
 pytestmark = pytest.mark.django_db
 
 klasses = SMSResource.__subclasses__()
 
 
 def test_clean_numbers():
-    assert SMSResource.clean_numbers(['+ 33 12']) == ['003312']
-    assert SMSResource.clean_numbers(['0 0 33 12']) == ['003312']
-    assert SMSResource.clean_numbers(['0 12']) == ['003312']
-    assert SMSResource.clean_numbers(['+ 33 12'], '32', '1') == ['003312']
-    assert SMSResource.clean_numbers(['0 0 33 12'], '32', '1') == ['003312']
-    assert SMSResource.clean_numbers(['1 12'], '32', '1') == ['003212']
+    connector = OVHSMSGateway()
+    assert connector.clean_numbers(['+ 33 12']) == ['003312']
+    assert connector.clean_numbers(['0 0 33 12']) == ['003312']
+    assert connector.clean_numbers(['0 12']) == ['003312']
+    connector.default_country_code = '32'
+    connector.default_trunk_prefix = '1'
+    connector.save()
+    assert connector.clean_numbers(['+ 33 12']) == ['003312']
+    assert connector.clean_numbers(['0 0 33 12']) == ['003312']
+    assert connector.clean_numbers(['1 12']) == ['003212']
+    with pytest.raises(APIError, match='phone number %r is unsupported' % '0123'):
+        connector.clean_numbers(['0123'])
 
 
 @pytest.fixture(params=klasses)
 def connector(request, db):
     klass = request.param
     kwargs = getattr(klass, 'TEST_DEFAULTS', {}).get('create_kwargs', {})
     kwargs.update({
         'title': klass.__name__,
-- 
2.26.2

