From 0387b72c0048d79ffec9d3e1ddaadccb065d1b1b Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 Aug 2017 13:34:29 +0100 Subject: [PATCH] Get the inbound sms working. Created a new test file for the inbound sms service settings changes, felt like the other file was too massive --- app/main/views/service_settings.py | 62 ++--- app/notify_client/inbound_number_client.py | 20 +- app/templates/views/service-settings.html | 2 +- .../main/views/service_settings/__init__.py | 0 .../test_inbound_sms_setting.py | 163 +++++++++++ tests/app/main/views/test_service_settings.py | 253 +----------------- 6 files changed, 198 insertions(+), 302 deletions(-) create mode 100644 tests/app/main/views/service_settings/__init__.py create mode 100644 tests/app/main/views/service_settings/test_inbound_sms_setting.py diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index e886efc93..56dfed5d8 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -66,7 +66,7 @@ def service_settings(service_id): inbound_api_url = '' inbound_number = inbound_number_client.get_inbound_sms_number_for_service(service_id) - if inbound_number['data'] is None or inbound_number['data'] == '': + if inbound_number['data'] == {}: disp_inbound_number = '' else: disp_inbound_number = inbound_number['data']['number'] @@ -335,15 +335,12 @@ def service_set_reply_to_email(service_id): @user_has_permissions('manage_settings', admin_override=True) def service_set_sms_sender(service_id): form = ServiceSmsSender() - if form.validate_on_submit(): - set_inbound_sms = request.args.get('set_inbound_sms', False) - if set_inbound_sms == 'True': - switch_service_permissions(current_service['id'], 'inbound_sms', form.sms_sender.data) - else: - service_api_client.update_service( - current_service['id'], - sms_sender=form.sms_sender.data or None + if 'inbound_sms' in current_service['permissions']: + abort(403) + service_api_client.update_service( + current_service['id'], + sms_sender=form.sms_sender.data or None ) return redirect(url_for('.service_settings', service_id=service_id)) @@ -355,43 +352,22 @@ def service_set_sms_sender(service_id): form=form) -@main.route("/services//service-settings/set-inbound-number", methods=['GET', 'POST']) +@main.route("/services//service-settings/set-inbound-number", methods=['GET']) @login_required @user_has_permissions('manage_settings', admin_override=True) def service_set_inbound_number(service_id): - - new_number = False - - inbound_number = inbound_number_client.get_inbound_sms_number_for_service(service_id) - - if inbound_number['data'] is None: - - inbound_number = inbound_number_client.get_available_inbound_number() - # if (inbound_number['data'] is None): - if (inbound_number['data'] is []): - # either return 404 or 400 or return a message to the html - - - new_number = True - - if request.method == 'POST': - try: - switch_service_permissions(current_service['id'], 'inbound_sms') - if new_number: - inbound_number_client.activate_inbound_sms_service(service_id, inbound_number['data']['id']) - return redirect(url_for('.service_settings', service_id=service_id)) - else: - inbound_number_client.reactivate_inbound_sms_service(inbound_number['data']['id']) - return redirect(url_for('.service_settings', service_id=service_id)) - - except HTTPError as e: - raise e - - - return render_template( - 'views/service-settings/confirm-inbound-number.html', - inbound_number=inbound_number['data']['number'] - ) + switch_service_permissions(current_service['id'], 'inbound_sms') + set_inbound_sms = request.args.get('set_inbound_sms', False) + try: + if set_inbound_sms: + inbound_number_client.activate_inbound_sms_service(service_id) + return redirect(url_for('.service_settings', service_id=service_id)) + else: + inbound_number_client.deactivate_inbound_sms_permission(service_id=service_id) + return redirect(url_for('.service_set_sms_sender', service_id=service_id)) + except HTTPError as e: + switch_service_permissions(current_service['id'], 'inbound_sms') + raise e @main.route("/services//service-settings/set-sms", methods=['GET']) diff --git a/app/notify_client/inbound_number_client.py b/app/notify_client/inbound_number_client.py index 601bfb6f4..ea60d2fe1 100644 --- a/app/notify_client/inbound_number_client.py +++ b/app/notify_client/inbound_number_client.py @@ -1,5 +1,6 @@ from app.notify_client import NotifyAdminAPIClient + class InboundNumberClient(NotifyAdminAPIClient): def __init__(self): @@ -15,21 +16,16 @@ class InboundNumberClient(NotifyAdminAPIClient): return self.get('/inbound_number') def get_inbound_sms_number_for_service(self, service_id): - return self.get('/inbound_number/service/{}'.format(service_id)) + return self.get('/inbound-number/service/{}'.format(service_id)) - # def activate_inbound_sms_service(self, service_id, inbound_number_id): - # return self.post(url='/inbound_number/{}/service/{}'.format(inbound_number_id, service_id), data={}) + 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): - try: - - - catch: - return self.post(url='/inbound_number/{}/on'.format(inbound_number_id), data={}) - def deactivate_inbound_sms_permission(self, inbound_number_id): - return self.post(url='/inbound_number/{}/off'.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()) + def get_available_inbound_number(self): + return self.get(url='/inbound_number/available'.format()) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 1b829858f..0d03b5c2f 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -204,7 +204,7 @@
  • {% if can_receive_inbound %} - + Stop inbound sms {% else %} diff --git a/tests/app/main/views/service_settings/__init__.py b/tests/app/main/views/service_settings/__init__.py new file mode 100644 index 000000000..e69de29bb 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 new file mode 100644 index 000000000..4e0b35492 --- /dev/null +++ b/tests/app/main/views/service_settings/test_inbound_sms_setting.py @@ -0,0 +1,163 @@ +import app +import pytest +from bs4 import BeautifulSoup +from flask import url_for +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 +): + 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 + + mock_update_service.assert_called_with( + service_one['id'], + sms_sender="elevenchars" + ) + + +def test_get_inbound_number_in_service_settings( + logged_in_client, + mock_update_service, + mock_get_letter_organisations, + service_one, + mocker +): + mocker_get_inbound_number_fun = mocker.patch( + 'app.inbound_number_client.get_inbound_sms_number_for_service', + return_value={'data': {'number': '077777777', 'id': 'some_uuid'}}) + + response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) + assert response.status_code == 200 + mocker_get_inbound_number_fun.assert_called_once_with(service_one['id']) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + element = page.find('span', {"id": "077777777"}) + assert not element + + +def test_allow_inbound_sms_sets_a_number_for_service( + logged_in_client, + service_one, + mocker +): + mocker.patch('app.service_api_client.update_service_with_properties') + mock_activate_inbound_sms = mocker.patch('app.inbound_number_client.activate_inbound_sms_service') + + response = logged_in_client.get(url_for('main.service_set_inbound_number', + service_id=service_one['id'], + set_inbound_sms=True)) + + assert response.status_code == 302 + assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) + mock_activate_inbound_sms.assert_called_once_with(service_one['id']) + + +def test_allow_inbound_sms_returns_400_if_no_numbers_available( + logged_in_client, + service_one, + mocker +): + 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'])) + mock_activate_inbound.assert_called_once_with(service_one['id']) + assert mock_switch_service.call_count == 2 + + +def test_set_text_message_sender_and_inbound_sms_permission_exists_return_403( + logged_in_client, + service_one, + mocker, +): + service_one['permissions'] = ['inbound_sms'] + update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties', + return_value=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) + assert response.status_code == 403 + + assert not update_service_mock.called + assert app.current_service['permissions'] == ['inbound_sms'] + + +def test_turn_inbound_sms_off( + logged_in_client, + service_one, + mocker +): + service_one['permissions'] = ['inbound_sms'] + update_service_mock = mocker.patch('app.service_api_client.update_service', + return_value=service_one) + mock_deactivate_inbound = mocker.patch('app.inbound_number_client.deactivate_inbound_sms_permission') + + 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 app.current_service['permissions'] == [] + mock_deactivate_inbound.assert_called_once_with(service_id=service_one['id']) + + +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 +): + service_one['permissions'] = [] + update_service_mock = mocker.patch('app.service_api_client.update_service', + return_value=service_one) + + 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 + + update_service_mock.assert_called_with( + service_one['id'], + sms_sender="elevenchars" + ) + assert app.current_service['permissions'] == [] + + +@pytest.mark.parametrize('content, expected_error', [ + ("", "Can’t be empty"), + ("twelvecharss", "Enter 11 characters or fewer"), + (".", "Use letters and numbers only") +]) +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, +): + response = logged_in_client.post(url_for( + 'main.service_set_sms_sender', + service_id=service_one['id']), + data={"sms_sender": content}, + follow_redirects=True + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert response.status_code == 200 + assert page.select(".error-message")[0].text.strip() == expected_error + assert not mock_update_service.called diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index aba7172e1..542150fb7 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1,18 +1,17 @@ +import uuid from unittest.mock import call, ANY, Mock import pytest -import uuid -from flask import url_for from bs4 import BeautifulSoup +from flask import url_for from werkzeug.exceptions import InternalServerError import app from app.main.views.service_settings import dummy_bearer_token from app.utils import email_safe from tests import validate_route_permission, service_json -from tests.conftest import normalize_spaces - from tests.conftest import active_user_with_permissions, platform_admin_user +from tests.conftest import normalize_spaces @pytest.mark.parametrize('user, expected_rows', [ @@ -230,10 +229,10 @@ def test_should_show_service_name( def test_should_redirect_after_change_service_name( - logged_in_client, - service_one, - mock_update_service, - mock_service_name_is_unique + logged_in_client, + service_one, + mock_update_service, + mock_service_name_is_unique ): response = logged_in_client.post( url_for('main.service_name_change', service_id=service_one['id']), @@ -725,244 +724,6 @@ def test_does_not_show_research_mode_indicator( assert not element -def test_set_text_message_sender( - logged_in_client, - mock_update_service, - service_one, - mock_get_letter_organisations, - mock_get_inbound_number_for_service -): - 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 - - mock_update_service.assert_called_with( - service_one['id'], - sms_sender="elevenchars" - ) - - -def test_get_inbound_number_in_service_settings( - logged_in_client, - mock_update_service, - mock_get_letter_organisations, - service_one, - mocker -): - mocker_get_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.get_inbound_sms_number_for_service', - return_value={'data': {'number': '077777777', 'id': 'some_uuid'}}) - - response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) - assert response.status_code == 200 - mocker_get_inbound_number_fun.assert_called_once_with(service_one['id']) - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - element = page.find('span', {"id": "077777777"}) - assert not element - - -def test_get_inbound_number_confirm_page( - logged_in_client, - mock_update_service, - service_one, - mocker -): - mocker_get_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.get_inbound_sms_number_for_service', - return_value={'data': {'number': '077777777', 'id': 'some_uuid'}}) - - response = logged_in_client.get(url_for('main.service_set_inbound_number', service_id=service_one['id']), - ) - - assert response.status_code == 200 - assert '077777777' in response.get_data(as_text=True) - - mocker_get_inbound_number_fun.assert_called_once_with(service_one['id']) - - -def test_if_currently_inbound_number_is_none( - logged_in_client, - mock_update_service, - service_one, - mocker -): - mocker_get_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.get_inbound_sms_number_for_service', - return_value={'data': ''}) - - mocker_get_available_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.get_available_inbound_number', - return_value={'data': {'number': '0123445678', 'id': 'some_uuid'}}) - - mocker_get_activate_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.activate_inbound_sms_service') - - response = logged_in_client.post(url_for('main.service_set_inbound_number', service_id=service_one['id']), - ) - - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - - mocker_get_inbound_number_fun.assert_called_once_with(service_one['id']) - mocker_get_available_inbound_number_fun.assert_called_once_with() - mocker_get_activate_inbound_number_fun.assert_called_once_with(service_one['id'], 'some_uuid') - - -def test_if_currently_inbound_number_is_not_empty( - logged_in_client, - mock_update_service, - service_one, - mocker -): - mocker_get_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.get_inbound_sms_number_for_service', - return_value={'data': {'number': '077777777', 'id': 'some_uuid'}}) - - mocker_get_reactivate_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.reactivate_inbound_sms_service') - - response = logged_in_client.post(url_for('main.service_set_inbound_number', service_id=service_one['id']),) - - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - - mocker_get_inbound_number_fun.assert_called_once_with(service_one['id']) - mocker_get_reactivate_inbound_number_fun.assert_called_once_with('some_uuid') - - -def test_no_inbound_number_available( - logged_in_client, - mock_update_service, - service_one, - mocker -): - mocker_get_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.get_inbound_sms_number_for_service', - return_value={'data': None}) - - mocker_get_available_inbound_number_fun = mocker.patch( - 'app.inbound_number_client.get_available_inbound_number', - return_value=Mock('something', - status_code=400, - json=lambda: { - } - )) - - response = logged_in_client.get(url_for('main.service_set_inbound_number', service_id=service_one['id']), - ) - - assert response.status_code == 400 - - # mocker_get_inbound_number_fun.assert_called_once_with(service_one['id']) - # mocker_get_available_inbound_number_fun.assert_called_once_with() - - -def test_set_text_message_sender_and_inbound_sms( - logged_in_client, - service_one, - mock_get_letter_organisations, - mocker, - mock_get_inbound_number_for_service -): - service_one['permissions'] = [] - update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties', - return_value=service_one) - - data = {"sms_sender": "elevenchars"} - response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id'], - set_inbound_sms=True), - data=data, - follow_redirects=True) - assert response.status_code == 200 - - update_service_mock.assert_called_with( - service_one['id'], - {'permissions': ['inbound_sms'], - 'sms_sender': "elevenchars"} - ) - assert app.current_service['permissions'] == ['inbound_sms'] - - -def test_turn_inbound_sms_off( - logged_in_client, - service_one, - mock_get_letter_organisations, - mocker, - mock_get_inbound_number_for_service -): - service_one['permissions'] = ['inbound_sms'] - update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties', - return_value=service_one) - - data = {"sms_sender": "elevenchars"} - response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id'], - set_inbound_sms=True), - data=data, - follow_redirects=True) - assert response.status_code == 200 - - update_service_mock.assert_called_with( - service_one['id'], - {'permissions': [], - 'sms_sender': "elevenchars"} - ) - assert app.current_service['permissions'] == [] - - -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 -): - service_one['permissions'] = [] - update_service_mock = mocker.patch('app.service_api_client.update_service', - return_value=service_one) - - 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 - - update_service_mock.assert_called_with( - service_one['id'], - sms_sender="elevenchars" - ) - assert app.current_service['permissions'] == [] - - -@pytest.mark.parametrize('content, expected_error', [ - ("", "Can’t be empty"), - ("twelvecharss", "Enter 11 characters or fewer"), - (".", "Use letters and numbers only") -]) -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, -): - response = logged_in_client.post(url_for( - 'main.service_set_sms_sender', - service_id=service_one['id']), - data={"sms_sender": content}, - follow_redirects=True - ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert response.status_code == 200 - assert page.select(".error-message")[0].text.strip() == expected_error - assert not mock_update_service.called - @pytest.mark.parametrize('url, bearer_token, expected_errors', [ ("", "", "Can’t be empty Can’t be empty"),