Merge pull request #1200 from alphagov/ken-refactor-inbound_numbers

Refactor inbound numbers

app/dao/services_dao.py

@@ -1,7 +1,7 @@
 import uuid
 from datetime import date, datetime, timedelta
 
-from sqlalchemy import asc, func, or_
+from sqlalchemy import asc, func
 from sqlalchemy.orm import joinedload
 from flask import current_app
 
@@ -66,24 +66,18 @@ def dao_fetch_service_by_id(service_id, only_active=False):
     return query.one()
 
 
-############
-# refactor this when API only uses inbound_numbers and not sms_sender
-############
-def dao_fetch_services_by_sms_sender(sms_sender):
+def dao_fetch_service_by_inbound_number(number):
     inbound_number = InboundNumber.query.filter(
-        InboundNumber.number == sms_sender,
+        InboundNumber.number == number,
         InboundNumber.active
     ).first()
 
     if not inbound_number:
-        return []
+        return None
 
     return Service.query.filter(
-        or_(
-            Service.sms_sender == sms_sender,
-            Service.id == inbound_number.service_id
-        )
-    ).all()
+        Service.id == inbound_number.service_id
+    ).first()
 
 
 def dao_fetch_service_by_id_with_api_keys(service_id, only_active=False):

app/notifications/receive_notifications.py

@@ -7,7 +7,7 @@ from notifications_utils.recipients import validate_and_format_phone_number
 from app import statsd_client, firetext_client, mmg_client
 from app.celery import tasks
 from app.config import QueueNames
-from app.dao.services_dao import dao_fetch_services_by_sms_sender
+from app.dao.services_dao import dao_fetch_service_by_inbound_number
 from app.dao.inbound_sms_dao import dao_create_inbound_sms
 from app.models import InboundSms, INBOUND_SMS_TYPE, SMS_TYPE
 from app.errors import register_errors
@@ -32,16 +32,14 @@ def receive_mmg_sms():
 
     inbound_number = strip_leading_forty_four(post_data['Number'])
 
-    potential_services = fetch_potential_services(inbound_number, 'mmg')
-    if not potential_services:
+    service = fetch_potential_service(inbound_number, 'mmg')
+    if not service:
         # since this is an issue with our service <-> number mapping, or no inbound_sms service permission
         # we should still tell MMG that we received it successfully
         return 'RECEIVED', 200
 
     statsd_client.incr('inbound.mmg.successful')
 
-    service = potential_services[0]
-
     inbound = create_inbound_sms_object(service,
                                         content=format_mmg_message(post_data["Message"]),
                                         from_number=post_data['MSISDN'],
@@ -60,13 +58,12 @@ def receive_firetext_sms():
 
     inbound_number = strip_leading_forty_four(post_data['destination'])
 
-    potential_services = fetch_potential_services(inbound_number, 'firetext')
-    if not potential_services:
+    service = fetch_potential_service(inbound_number, 'firetext')
+    if not service:
         return jsonify({
             "status": "ok"
         }), 200
 
-    service = potential_services[0]
     inbound = create_inbound_sms_object(service=service,
                                         content=post_data["message"],
                                         from_number=post_data['source'],
@@ -118,22 +115,22 @@ def create_inbound_sms_object(service, content, from_number, provider_ref, date_
     return inbound
 
 
-def fetch_potential_services(inbound_number, provider_name):
-    potential_services = dao_fetch_services_by_sms_sender(inbound_number)
+def fetch_potential_service(inbound_number, provider_name):
+    service = dao_fetch_service_by_inbound_number(inbound_number)
 
-    if len(potential_services) != 1:
-        current_app.logger.error('Inbound number "{}" from {} not associated with exactly one service'.format(
+    if not service:
+        current_app.logger.error('Inbound number "{}" from {} not associated with a service'.format(
             inbound_number, provider_name
         ))
         statsd_client.incr('inbound.{}.failed'.format(provider_name))
         return False
 
-    if not has_inbound_sms_permissions(potential_services[0].permissions):
+    if not has_inbound_sms_permissions(service.permissions):
         current_app.logger.error(
-            'Service "{}" does not allow inbound SMS'.format(potential_services[0].id))
+            'Service "{}" does not allow inbound SMS'.format(service.id))
         return False
 
-    return potential_services
+    return service
 
 
 def has_inbound_sms_permissions(permissions):