Refactor the sms sender code to remove any unused methods.

Refactor tests/db/create_service() to behave more like the real world.
Created new create_service_with_inbound_number and create_service_with_defined_sms_sender() test/db methods.

tests/app/service/test_rest.py

@@ -8,6 +8,7 @@ import pytest
 from flask import url_for, current_app
 from freezegun import freeze_time
 
+from app.dao.service_sms_sender_dao import dao_add_sms_sender_for_service
 from app.dao.services_dao import dao_remove_user_from_service
 from app.dao.templates_dao import dao_redact_template
 from app.dao.users_dao import save_model_user
@@ -307,7 +308,6 @@ def test_create_service(client, sample_user):
 
     service_db = Service.query.get(json_resp['data']['id'])
     assert service_db.name == 'created service'
-    assert service_db.sms_sender == current_app.config['FROM_NUMBER']
 
     auth_header_fetch = create_authorization_header()
 
@@ -322,7 +322,7 @@ def test_create_service(client, sample_user):
 
     service_sms_senders = ServiceSmsSender.query.filter_by(service_id=service_db.id).all()
     assert len(service_sms_senders) == 1
-    assert service_sms_senders[0].sms_sender == service_db.sms_sender
+    assert service_sms_senders[0].sms_sender == service_db.get_default_sms_sender()
 
 
 def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_uuid):
@@ -1505,63 +1505,6 @@ def test_get_only_api_created_notifications_for_service(
     assert resp['notifications'][0]['id'] == str(without_job.id)
 
 
-def test_set_sms_sender_for_service(client, sample_service):
-    data = {
-        'sms_sender': 'elevenchars',
-    }
-
-    auth_header = create_authorization_header()
-
-    resp = client.post(
-        '/service/{}'.format(sample_service.id),
-        data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), auth_header]
-    )
-    result = json.loads(resp.get_data(as_text=True))
-    assert resp.status_code == 200
-    assert result['data']['sms_sender'] == 'elevenchars'
-    service_sms_senders = ServiceSmsSender.query.filter_by(service_id=sample_service.id).all()
-    assert len(service_sms_senders) == 1
-    assert service_sms_senders[0].sms_sender == 'elevenchars'
-    assert service_sms_senders[0].is_default
-
-
-def test_set_sms_sender_for_service_rejects_invalid_characters(client, sample_service):
-    data = {
-        'sms_sender': 'invalid####',
-    }
-
-    auth_header = create_authorization_header()
-
-    resp = client.post(
-        '/service/{}'.format(sample_service.id),
-        data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), auth_header]
-    )
-    result = json.loads(resp.get_data(as_text=True))
-    assert resp.status_code == 400
-    assert result['result'] == 'error'
-    assert result['message'] == {'sms_sender': ['Only alphanumeric characters allowed']}
-
-
-def test_set_sms_sender_for_service_rejects_null(client, sample_service):
-    data = {
-        'sms_sender': None,
-    }
-
-    auth_header = create_authorization_header()
-
-    resp = client.post(
-        '/service/{}'.format(sample_service.id),
-        data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), auth_header]
-    )
-    result = json.loads(resp.get_data(as_text=True))
-    assert resp.status_code == 400
-    assert result['result'] == 'error'
-    assert result['message'] == {'sms_sender': ['Field may not be null.']}
-
-
 @pytest.mark.parametrize('default_sms_sender, should_prefix', [
     (None, True),  # None means use default
     ('Foo', False),
@@ -1572,9 +1515,10 @@ def test_prefixing_messages_based_on_sms_sender(
     default_sms_sender,
     should_prefix,
 ):
-    service = create_service(
-        sms_sender=default_sms_sender or current_app.config['FROM_NUMBER']
-    )
+    service = create_service()
+    if default_sms_sender:
+        # add another sms sender that is the default.
+        dao_add_sms_sender_for_service(service_id=service.id, sms_sender=default_sms_sender, is_default=True)
     create_service_sms_sender(
         service=service,
         sms_sender='ignored',
@@ -2688,7 +2632,7 @@ def test_add_service_sms_sender_can_add_multiple_senders(client, notify_db_sessi
 
 def test_add_service_sms_sender_when_it_is_an_inbound_number_updates_the_only_existing_sms_sender(
         client, notify_db_session):
-    service = create_service(sms_sender='GOVUK')
+    service = create_service()
     inbound_number = create_inbound_number(number='12345')
     data = {
         "sms_sender": str(inbound_number.id),
@@ -2713,7 +2657,7 @@ def test_add_service_sms_sender_when_it_is_an_inbound_number_updates_the_only_ex
 
 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')
+    service = create_service()
     create_service_sms_sender(service=service, sms_sender="second", is_default=False)
     inbound_number = create_inbound_number(number='12345')
     data = {
@@ -2738,7 +2682,7 @@ def test_add_service_sms_sender_when_it_is_an_inbound_number_inserts_new_sms_sen
 
 
 def test_add_service_sms_sender_switches_default(client, notify_db_session):
-    service = create_service(sms_sender='first')
+    service = create_service()
     data = {
         "sms_sender": 'second',
         "is_default": True,
@@ -2752,7 +2696,7 @@ def test_add_service_sms_sender_switches_default(client, notify_db_session):
     assert resp_json['sms_sender'] == 'second'
     assert not resp_json['inbound_number_id']
     assert resp_json['is_default']
-    sms_senders = ServiceSmsSender.query.filter_by(sms_sender='first').first()
+    sms_senders = ServiceSmsSender.query.filter_by(sms_sender='testing').first()
     assert not sms_senders.is_default
 
 
@@ -2790,7 +2734,7 @@ def test_update_service_sms_sender(client, notify_db_session):
 
 
 def test_update_service_sms_sender_switches_default(client, notify_db_session):
-    service = create_service(sms_sender='first')
+    service = create_service()
     service_sms_sender = create_service_sms_sender(service=service, sms_sender='1235', is_default=False)
     data = {
         "sms_sender": 'second',
@@ -2805,7 +2749,7 @@ def test_update_service_sms_sender_switches_default(client, notify_db_session):
     assert resp_json['sms_sender'] == 'second'
     assert not resp_json['inbound_number_id']
     assert resp_json['is_default']
-    sms_senders = ServiceSmsSender.query.filter_by(sms_sender='first').first()
+    sms_senders = ServiceSmsSender.query.filter_by(sms_sender='testing').first()
     assert not sms_senders.is_default
 
 
@@ -2875,7 +2819,7 @@ def test_get_service_sms_sender_by_id_returns_404_when_sms_sender_does_not_exist
 
 
 def test_get_service_sms_senders_for_service(client, notify_db_session):
-    service_sms_sender = create_service_sms_sender(service=create_service(sms_sender='first'),
+    service_sms_sender = create_service_sms_sender(service=create_service(),
                                                    sms_sender='second',
                                                    is_default=False)
     response = client.get('/service/{}/sms-sender'.format(service_sms_sender.service_id),
@@ -2885,7 +2829,7 @@ def test_get_service_sms_senders_for_service(client, notify_db_session):
     json_resp = json.loads(response.get_data(as_text=True))
     assert len(json_resp) == 2
     assert json_resp[0]['is_default']
-    assert json_resp[0]['sms_sender'] == 'first'
+    assert json_resp[0]['sms_sender'] == current_app.config['FROM_NUMBER']
     assert not json_resp[1]['is_default']
     assert json_resp[1]['sms_sender'] == 'second'