This PR is to retain current behaviour when we allocate an inbound number for a service.

When a service is allocated an inbound number and they only have one SMS sender, then update that SMS sender to the inbound number.
That way they will not have more than one SMS sender and will not have to choose to use either one.

app/dao/service_sms_sender_dao.py

@@ -83,6 +83,14 @@ def dao_update_service_sms_sender(service_id, service_sms_sender_id, is_default,
     return sms_sender_to_update
 
 
+@transactional
+def update_existing_sms_sender_with_inbound_number(service_sms_sender, sms_sender, inbound_number_id):
+    service_sms_sender.sms_sender = sms_sender
+    service_sms_sender.inbound_number_id = inbound_number_id
+    db.session.add(service_sms_sender)
+    return service_sms_sender
+
+
 def _get_existing_default(service_id):
     sms_senders = dao_get_sms_senders_by_service_id(service_id=service_id)
     if sms_senders:

app/service/rest.py

@@ -28,8 +28,8 @@ from app.dao.service_sms_sender_dao import (
     dao_add_sms_sender_for_service,
     dao_update_service_sms_sender,
     dao_get_service_sms_senders_by_id,
-    dao_get_sms_senders_by_service_id
+    dao_get_sms_senders_by_service_id,
-)
+    update_existing_sms_sender_with_inbound_number)
 from app.dao.services_dao import (
     dao_fetch_service_by_id,
     dao_fetch_all_services,
@@ -654,10 +654,22 @@ def add_service_sms_sender(service_id):
     form = validate(request.get_json(), add_service_sms_sender_request)
     inbound_number_id = form.get('inbound_number_id', None)
     sms_sender = form.get('sms_sender')
+
     if inbound_number_id:
         updated_number = dao_allocate_number_for_service(service_id=service_id, inbound_number_id=inbound_number_id)
-        # the sms_sender in the form is the inbound_number_id from client, use number from table.
+        # the sms_sender in the form is not set, use the inbound number
         sms_sender = updated_number.number
+        existing_sms_sender = dao_get_sms_senders_by_service_id(service_id)
+        # we don't want to create a new sms sender for the service if we are allocating an inbound number.
+        if len(existing_sms_sender) == 1:
+            update_existing_sms_sender = existing_sms_sender[0]
+            new_sms_sender = update_existing_sms_sender_with_inbound_number(
+                service_sms_sender=update_existing_sms_sender,
+                sms_sender=sms_sender,
+                inbound_number_id=inbound_number_id)
+
+            return jsonify(new_sms_sender.serialize()), 201
+
     new_sms_sender = dao_add_sms_sender_for_service(service_id=service_id,
                                                     sms_sender=sms_sender,
                                                     is_default=form['is_default'],

tests/app/dao/test_service_sms_sender_dao.py

@@ -6,9 +6,12 @@ from sqlalchemy.exc import SQLAlchemyError
 from app.dao.service_sms_sender_dao import (
     insert_or_update_service_sms_sender,
     dao_add_sms_sender_for_service,
-    dao_update_service_sms_sender, dao_get_service_sms_senders_by_id, dao_get_sms_senders_by_service_id)
+    dao_update_service_sms_sender,
+    dao_get_service_sms_senders_by_id,
+    dao_get_sms_senders_by_service_id,
+    update_existing_sms_sender_with_inbound_number)
 from app.models import ServiceSmsSender
-from tests.app.db import create_service
+from tests.app.db import create_service, create_inbound_number
 
 
 def test_update_service_sms_sender_updates_existing_row(notify_db_session):
@@ -145,3 +148,27 @@ def test_dao_update_service_sms_sender_raises_exception_when_no_default_after_up
                                       service_sms_sender_id=sms_sender.id,
                                       is_default=False,
                                       sms_sender="updated")
+
+
+def test_update_existing_sms_sender_with_inbound_number(notify_db_session):
+    service = create_service(sms_sender='testing')
+    inbound_number = create_inbound_number(number='12345', service_id=service.id)
+
+    existing_sms_sender = ServiceSmsSender.query.filter_by(service_id=service.id).one()
+    sms_sender = update_existing_sms_sender_with_inbound_number(
+        service_sms_sender=existing_sms_sender, sms_sender=inbound_number.number, inbound_number_id=inbound_number.id)
+
+    assert sms_sender.inbound_number_id == inbound_number.id
+    assert sms_sender.sms_sender == inbound_number.number
+    assert sms_sender.is_default
+
+
+def test_update_existing_sms_sender_with_inbound_number_raises_exception_if_inbound_number_does_not_exist(
+        notify_db_session
+):
+    service = create_service(sms_sender='testing')
+    existing_sms_sender = ServiceSmsSender.query.filter_by(service_id=service.id).one()
+    with pytest.raises(expected_exception=SQLAlchemyError):
+        update_existing_sms_sender_with_inbound_number(service_sms_sender=existing_sms_sender,
+                                                       sms_sender='blah',
+                                                       inbound_number_id=uuid.uuid4())

tests/app/service/test_rest.py

@@ -2602,12 +2602,13 @@ def test_add_service_sms_sender_can_add_multiple_senders(client, notify_db_sessi
     assert len(senders) == 2
 
 
-def test_add_service_sms_sender_when_it_is_an_inbound_number(client, notify_db_session):
+def test_add_service_sms_sender_when_it_is_an_inbound_number_updates_the_only_existing_sms_sender(
-    service = create_service()
+        client, notify_db_session):
+    service = create_service(sms_sender='GOVUK')
     inbound_number = create_inbound_number(number='12345')
     data = {
         "sms_sender": str(inbound_number.id),
-        "is_default": False,
+        "is_default": True,
         "inbound_number_id": str(inbound_number.id)
     }
     response = client.post('/service/{}/sms-sender'.format(service.id),
@@ -2620,7 +2621,36 @@ def test_add_service_sms_sender_when_it_is_an_inbound_number(client, notify_db_s
     resp_json = json.loads(response.get_data(as_text=True))
     assert resp_json['sms_sender'] == inbound_number.number
     assert resp_json['inbound_number_id'] == str(inbound_number.id)
-    assert not resp_json['is_default']
+    assert resp_json['is_default']
+
+    senders = ServiceSmsSender.query.all()
+    assert len(senders) == 1
+
+
+def test_add_service_sms_sender_when_it_is_an_inbound_number_inserts_new_sms_sender_when_more_than_one(
+        client, notify_db_session):
+    service = create_service(sms_sender='GOVUK')
+    create_service_sms_sender(service=service, sms_sender="second", is_default=False)
+    inbound_number = create_inbound_number(number='12345')
+    data = {
+        "sms_sender": str(inbound_number.id),
+        "is_default": True,
+        "inbound_number_id": str(inbound_number.id)
+    }
+    response = client.post('/service/{}/sms-sender'.format(service.id),
+                           data=json.dumps(data),
+                           headers=[('Content-Type', 'application/json'), create_authorization_header()]
+                           )
+    assert response.status_code == 201
+    updated_number = InboundNumber.query.get(inbound_number.id)
+    assert updated_number.service_id == service.id
+    resp_json = json.loads(response.get_data(as_text=True))
+    assert resp_json['sms_sender'] == inbound_number.number
+    assert resp_json['inbound_number_id'] == str(inbound_number.id)
+    assert resp_json['is_default']
+
+    senders = ServiceSmsSender.query.filter_by(service_id=service.id).all()
+    assert len(senders) == 3
 
 
 def test_add_service_sms_sender_switches_default(client, notify_db_session):