Merge pull request #1018 from alphagov/fix-firetest-inbound

Strip leading 44 from inbound SMS numbers to normalise to how we stor…

app/notifications/receive_notifications.py

@@ -27,10 +27,13 @@ def receive_mmg_sms():
     }
     """
     post_data = request.get_json()
-    potential_services = dao_fetch_services_by_sms_sender(post_data['Number'])
+
+    inbound_number = strip_leading_forty_four(post_data['Number'])
+
+    potential_services = dao_fetch_services_by_sms_sender(inbound_number)
 
     if len(potential_services) != 1:
-        current_app.logger.error('Inbound number "{}" not associated with exactly one service'.format(
+        current_app.logger.error('Inbound number "{}" from MMG not associated with exactly one service'.format(
             post_data['Number']
         ))
         statsd_client.incr('inbound.mmg.failed')
@@ -88,9 +91,11 @@ def create_inbound_mmg_sms_object(service, json):
 def receive_firetext_sms():
     post_data = request.form
 
-    potential_services = dao_fetch_services_by_sms_sender(post_data['destination'])
+    inbound_number = strip_leading_forty_four(post_data['destination'])
+
+    potential_services = dao_fetch_services_by_sms_sender(inbound_number)
     if len(potential_services) != 1:
-        current_app.logger.error('Inbound number "{}" not associated with exactly one service'.format(
+        current_app.logger.error('Inbound number "{}" from firetext not associated with exactly one service'.format(
             post_data['destination']
         ))
         statsd_client.incr('inbound.firetext.failed')
@@ -120,3 +125,9 @@ def receive_firetext_sms():
     return jsonify({
         "status": "ok"
     }), 200
+
+
+def strip_leading_forty_four(number):
+    if number.startswith('44'):
+        return number.replace('44', '0', 1)
+    return number

tests/app/notifications/test_receive_notification.py

@@ -7,11 +7,11 @@ from flask import json
 from app.notifications.receive_notifications import (
     format_mmg_message,
     format_mmg_datetime,
-    create_inbound_mmg_sms_object
+    create_inbound_mmg_sms_object,
+    strip_leading_forty_four
 )
 
 from app.models import InboundSms
-from tests.app.conftest import sample_service
 from tests.app.db import create_service
 
 
@@ -163,7 +163,6 @@ def test_receive_notification_from_firetext_persists_message_with_normalized_pho
 
 
 def test_returns_ok_to_firetext_if_mismatched_sms_sender(notify_db_session, client, mocker):
-
     mock = mocker.patch('app.notifications.receive_notifications.statsd_client.incr')
 
     create_service(service_name='b', sms_sender='07111111199')
@@ -181,3 +180,16 @@ def test_returns_ok_to_firetext_if_mismatched_sms_sender(notify_db_session, clie
     assert not InboundSms.query.all()
     assert result['status'] == 'ok'
     mock.assert_has_calls([call('inbound.firetext.failed')])
+
+
+@pytest.mark.parametrize(
+    'number, expected',
+    [
+        ('447123123123', '07123123123'),
+        ('447123123144', '07123123144'),
+        ('07123123123', '07123123123'),
+        ('447444444444', '07444444444')
+    ]
+)
+def test_strip_leading_country_code(number, expected):
+    assert strip_leading_forty_four(number) == expected