Merge pull request #9 from 18F/jim/062522/smsclientfixes

initial sms provider cleanup

.gitignore

@@ -77,5 +77,6 @@ celerybeat-schedule
 
 # CloudFoundry
 .cf
+varsfile*
 
 /scripts/run_my_tests.sh

app/clients/__init__.py

@@ -1,3 +1,5 @@
+from celery import current_app
+
 class ClientException(Exception):
     '''
     Base Exceptions for sending notifications that fail
@@ -36,7 +38,7 @@ class NotificationProviderClients(object):
 
     def get_client_by_name_and_type(self, name, notification_type):
         assert notification_type in ['email', 'sms']
-
+        
         if notification_type == 'email':
             return self.get_email_client(name)
 

app/clients/sms/aws_sns.py

@@ -20,11 +20,14 @@ class AwsSnsClient(SmsClient):
         self.current_app = current_app
         self.statsd_client = statsd_client
         self.long_code_regex = re.compile(r"^\+1\d{10}$")
-
+        
     @property
     def name(self):
         return 'sns'
 
+    def get_name(self):
+        return 'sns'
+
     def send_sms(self, to, content, reference, sender=None, international=False):
         matched = False
 

app/config.py

@@ -92,7 +92,7 @@ class Config(object):
     GOVUK_ALERTS_CLIENT_ID = 'govuk-alerts'
 
     INTERNAL_CLIENT_API_KEYS = json.loads(
-        os.environ.get('INTERNAL_CLIENT_API_KEYS', '{}')
+        os.environ.get('INTERNAL_CLIENT_API_KEYS', '{"notify-admin":["dev-notify-secret-key"]}')
     )
 
     # encyption secret/salt
@@ -374,8 +374,8 @@ class Config(object):
     FIRETEXT_INBOUND_SMS_AUTH = json.loads(os.environ.get('FIRETEXT_INBOUND_SMS_AUTH', '[]'))
     MMG_INBOUND_SMS_AUTH = json.loads(os.environ.get('MMG_INBOUND_SMS_AUTH', '[]'))
     MMG_INBOUND_SMS_USERNAME = json.loads(os.environ.get('MMG_INBOUND_SMS_USERNAME', '[]'))
-    ROUTE_SECRET_KEY_1 = os.environ.get('ROUTE_SECRET_KEY_1', '')
+    ROUTE_SECRET_KEY_1 = os.environ.get('ROUTE_SECRET_KEY_1', 'dev-route-secret-key-1')
-    ROUTE_SECRET_KEY_2 = os.environ.get('ROUTE_SECRET_KEY_2', '')
+    ROUTE_SECRET_KEY_2 = os.environ.get('ROUTE_SECRET_KEY_2', 'dev-route-secret-key-2')
 
     HIGH_VOLUME_SERVICE = json.loads(os.environ.get('HIGH_VOLUME_SERVICE', '[]'))
 
@@ -421,10 +421,10 @@ class Development(Config):
     TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters'
     LETTER_SANITISE_BUCKET_NAME = 'development-letters-sanitise'
 
-    INTERNAL_CLIENT_API_KEYS = {
+    # INTERNAL_CLIENT_API_KEYS = {
-        Config.ADMIN_CLIENT_ID: ['dev-notify-secret-key'],
+    #     Config.ADMIN_CLIENT_ID: ['dev-notify-secret-key'],
-        Config.GOVUK_ALERTS_CLIENT_ID: ['govuk-alerts-secret-key']
+    #     Config.GOVUK_ALERTS_CLIENT_ID: ['govuk-alerts-secret-key']
-    }
+    # }
 
     SECRET_KEY = 'dev-notify-secret-key'
     DANGEROUS_SALT = 'dev-notify-salt'

app/delivery/send_to_providers.py

@@ -45,6 +45,9 @@ def send_sms_to_provider(notification):
 
     if notification.status == 'created':
         provider = provider_to_use(SMS_TYPE, notification.international)
+        if not provider:
+            technical_failure(notification=notification)
+            return
 
         template_model = SerialisedTemplate.from_id_and_service_id(
             template_id=notification.template_id, service_id=service.id, version=notification.template_version
@@ -169,9 +172,10 @@ provider_cache = TTLCache(maxsize=8, ttl=10)
 
 @cached(cache=provider_cache)
 def provider_to_use(notification_type, international=True):
-    international = True # TODO: remove or resolve the functionality of this
+    international = False # TODO: remove or resolve the functionality of this flag
+    # TODO rip firetext and mmg out of early migrations and clean up the expression below
     active_providers = [
-        p for p in get_provider_details_by_notification_type(notification_type, international) if p.active
+        p for p in get_provider_details_by_notification_type(notification_type, international) if p.active and p.identifier not in ['firetext','mmg']
     ]
 
     if not active_providers:
@@ -181,11 +185,10 @@ def provider_to_use(notification_type, international=True):
         raise Exception("No active {} providers".format(notification_type))
 
     if len(active_providers) == 1:
-        weights = [100]
+        chosen_provider = active_providers[0]
     else:
         weights = [p.priority for p in active_providers]
-
+        chosen_provider = random.choices(active_providers, weights=weights)[0]
-    chosen_provider = random.choices(active_providers, weights=weights)[0]
 
     return notification_provider_clients.get_client_by_name_and_type(chosen_provider.identifier, notification_type)
 

migrations/versions/0076_add_intl_flag_to_provider.py

@@ -18,8 +18,8 @@ def upgrade():
     op.add_column('provider_details', sa.Column('supports_international', sa.Boolean(), nullable=False, server_default=sa.false()))
     op.add_column('provider_details_history', sa.Column('supports_international', sa.Boolean(), nullable=False, server_default=sa.false()))
 
-    op.execute("UPDATE provider_details SET supports_international=True WHERE identifier='mmg'")
+    op.execute("UPDATE provider_details SET supports_international=True WHERE identifier='sns'")
-    op.execute("UPDATE provider_details_history SET supports_international=True WHERE identifier='mmg'")
+    op.execute("UPDATE provider_details_history SET supports_international=True WHERE identifier='sns'")
 
 
 def downgrade():