From c6a8ef43ec7239d239d7475ee553abbc10ef8f24 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 Aug 2017 15:02:20 +0100 Subject: [PATCH] When setting the permisssion to allow inbound numbers the client will call an api endpoint that will set the next avaiable inbound number for the service. As before the service manager will not be able to change the Text message sender once they have the inbound sms permission. If the inbound sms permission is turned off the Text message sender setting is once more configurable by the service manager. The inbound number remains bound to the service, but has "inactive", so that the number can not be used again. --- app/main/views/service_settings.py | 2 +- app/notify_client/inbound_number_client.py | 7 ---- .../test_inbound_sms_setting.py | 33 +++++++++---------- tests/app/main/views/test_service_settings.py | 1 - 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 56dfed5d8..84b43ef1b 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -359,7 +359,7 @@ def service_set_inbound_number(service_id): switch_service_permissions(current_service['id'], 'inbound_sms') set_inbound_sms = request.args.get('set_inbound_sms', False) try: - if set_inbound_sms: + if set_inbound_sms == 'True': inbound_number_client.activate_inbound_sms_service(service_id) return redirect(url_for('.service_settings', service_id=service_id)) else: diff --git a/app/notify_client/inbound_number_client.py b/app/notify_client/inbound_number_client.py index ea60d2fe1..97f77259a 100644 --- a/app/notify_client/inbound_number_client.py +++ b/app/notify_client/inbound_number_client.py @@ -12,7 +12,6 @@ class InboundNumberClient(NotifyAdminAPIClient): self.api_key = app.config['ADMIN_CLIENT_SECRET'] def get_all_inbound_sms_number_service(self): - return self.get('/inbound_number') def get_inbound_sms_number_for_service(self, service_id): @@ -21,11 +20,5 @@ class InboundNumberClient(NotifyAdminAPIClient): def activate_inbound_sms_service(self, service_id): return self.post(url='/inbound-number/service/{}'.format(service_id), data={}) - def reactivate_inbound_sms_service(self, inbound_number_id): - return self.post(url='/inbound_number/{}/on'.format(inbound_number_id), data={}) - def deactivate_inbound_sms_permission(self, service_id): return self.post(url='/inbound-number/service/{}/off'.format(service_id), data={}) - - def get_available_inbound_number(self): - return self.get(url='/inbound_number/available'.format()) diff --git a/tests/app/main/views/service_settings/test_inbound_sms_setting.py b/tests/app/main/views/service_settings/test_inbound_sms_setting.py index 4e0b35492..c4c072057 100644 --- a/tests/app/main/views/service_settings/test_inbound_sms_setting.py +++ b/tests/app/main/views/service_settings/test_inbound_sms_setting.py @@ -8,15 +8,13 @@ from notifications_python_client.errors import HTTPError def test_set_text_message_sender( logged_in_client, mock_update_service, - service_one, - mock_get_letter_organisations, - mock_get_inbound_number_for_service + service_one ): data = {"sms_sender": "elevenchars"} response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id']), - data=data, - follow_redirects=True) - assert response.status_code == 200 + data=data) + assert response.status_code == 302 + assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) mock_update_service.assert_called_with( service_one['id'], @@ -69,7 +67,8 @@ def test_allow_inbound_sms_returns_400_if_no_numbers_available( mock_switch_service = mocker.patch('app.service_api_client.update_service_with_properties') mock_activate_inbound = mocker.patch('app.inbound_number_client.activate_inbound_sms_service', side_effect=HTTPError) - logged_in_client.get(url_for('main.service_set_inbound_number', service_id=service_one['id'])) + logged_in_client.get( + url_for('main.service_set_inbound_number', service_id=service_one['id'], set_inbound_sms='True')) mock_activate_inbound.assert_called_once_with(service_one['id']) assert mock_switch_service.call_count == 2 @@ -80,12 +79,13 @@ def test_set_text_message_sender_and_inbound_sms_permission_exists_return_403( mocker, ): service_one['permissions'] = ['inbound_sms'] - update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties', - return_value=service_one) + mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) + update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties') data = {"sms_sender": "elevenchars"} response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id']), data=data) + assert response.status_code == 403 assert not update_service_mock.called @@ -105,18 +105,17 @@ def test_turn_inbound_sms_off( response = logged_in_client.get(url_for('main.service_set_inbound_number', service_id=service_one['id'], set_inbound_sms=False)) assert response.status_code == 302 - assert response.location == url_for('main.service_set_sms_sender', service_id=service_one['id']) + assert response.location == url_for('main.service_set_sms_sender', service_id=service_one['id'], _external=True) assert app.current_service['permissions'] == [] mock_deactivate_inbound.assert_called_once_with(service_id=service_one['id']) + assert update_service_mock.called def test_set_text_message_sender_and_not_inbound_sms( logged_in_client, service_one, - mock_get_letter_organisations, - mocker, - mock_get_inbound_number_for_service + mocker ): service_one['permissions'] = [] update_service_mock = mocker.patch('app.service_api_client.update_service', @@ -125,9 +124,9 @@ def test_set_text_message_sender_and_not_inbound_sms( data = {"sms_sender": "elevenchars"} response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id'], set_inbound_sms=False), - data=data, - follow_redirects=True) - assert response.status_code == 200 + data=data) + assert response.status_code == 302 + assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) update_service_mock.assert_called_with( service_one['id'], @@ -145,8 +144,6 @@ def test_set_text_message_sender_validation( logged_in_client, mock_update_service, service_one, - mock_get_letter_organisations, - mock_get_inbound_number_for_service, content, expected_error, ): diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 542150fb7..c9c20293c 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -724,7 +724,6 @@ def test_does_not_show_research_mode_indicator( assert not element - @pytest.mark.parametrize('url, bearer_token, expected_errors', [ ("", "", "Can’t be empty Can’t be empty"), ("http://not_https.com", "1234567890", "Must be a valid https url"),