From a8adf4d7d71337f793dc86149eb734ac648ead76 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 25 Oct 2017 11:58:54 +0100 Subject: [PATCH] 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 | 8 +++++ app/service/rest.py | 18 ++++++++-- tests/app/dao/test_service_sms_sender_dao.py | 31 ++++++++++++++-- tests/app/service/test_rest.py | 38 +++++++++++++++++--- 4 files changed, 86 insertions(+), 9 deletions(-) diff --git a/app/dao/service_sms_sender_dao.py b/app/dao/service_sms_sender_dao.py index 69397f441..4b89b7e8d 100644 --- a/app/dao/service_sms_sender_dao.py +++ b/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: diff --git a/app/service/rest.py b/app/service/rest.py index 3d2dc2827..005dbd8e9 100644 --- a/app/service/rest.py +++ b/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'], diff --git a/tests/app/dao/test_service_sms_sender_dao.py b/tests/app/dao/test_service_sms_sender_dao.py index 3dcec1469..c38ebe2bc 100644 --- a/tests/app/dao/test_service_sms_sender_dao.py +++ b/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()) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 3e1483d1a..0ced56ec3 100644 --- a/tests/app/service/test_rest.py +++ b/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): - service = create_service() +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') 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):