diff --git a/app/main/forms.py b/app/main/forms.py index 93a327c0b..0b92466dd 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -504,10 +504,6 @@ class ServiceReplyToEmailFrom(Form): email_address = email_address(label='Email reply to address') -class InboudnSmsConfirm(Form): - inbound_number = '' - - class ServiceSmsSender(Form): sms_sender = StringField( 'Text message sender', diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index becb6f7ce..7e0ece827 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -32,8 +32,7 @@ from app.main.forms import ( ServiceLetterContactBlock, ServiceBrandingOrg, LetterBranding, - ServiceInboundApiForm, - InboudnSmsConfirm) + ServiceInboundApiForm) from app import user_api_client, current_service, organisations_client, inbound_number_client @@ -364,8 +363,7 @@ def service_set_inbound_number(service_id): inbound_number = inbound_number_client.get_available_inbound_number(service_id) new_number = True - form = InboudnSmsConfirm() - if form.validate_on_submit(): + if request.method == 'POST': switch_service_permissions(current_service['id'], 'inbound_sms') if new_number: inbound_number_client.activate_inbound_sms_service(service_id, inbound_number['data']['id']) @@ -376,8 +374,7 @@ def service_set_inbound_number(service_id): return render_template( 'views/service-settings/confirm-inbound-number.html', - inbound_number=inbound_number['data']['number'], - form=form + inbound_number=inbound_number['data']['number'] ) diff --git a/app/notify_client/inbound_number_client.py b/app/notify_client/inbound_number_client.py index db95d9927..d3fd6cd02 100644 --- a/app/notify_client/inbound_number_client.py +++ b/app/notify_client/inbound_number_client.py @@ -7,8 +7,6 @@ class InboundNumberClient(NotifyAdminAPIClient): super().__init__("a" * 73, "b") def init_app(self, app): - import pdb - pdb.set_trace() self.base_url = app.config['API_HOST_NAME'] self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] self.api_key = app.config['ADMIN_CLIENT_SECRET'] @@ -29,5 +27,5 @@ class InboundNumberClient(NotifyAdminAPIClient): def deactivate_inbound_sms_permission(self, inbound_number_id): return self.post(url='/inbound_number/{}/off'.format(inbound_number_id), data={}) - def get_available_inbound_number(self, service_id): - return self.get(url='/inbound_number/{}/available'.format(service_id)) + def get_available_inbound_number(self): + return self.get(url='/inbound_number/available'.format()) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 606d4c2c8..637a0ef82 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -50,6 +50,7 @@ def test_should_show_overview( mock_get_letter_organisations, user, expected_rows, + mock_get_inbound_number_for_service ): service_one['permissions'] = ['sms', 'email'] @@ -73,7 +74,7 @@ def test_should_show_overview( 'Send emails On Change', 'Email reply to address test@example.com Change', 'Send text messages On Change', - 'Text message sender elevenchars', + 'Text message sender 0781239871', 'International text messages On Change', 'Receive text messages On Change', 'API endpoint for received text messages None Change', @@ -97,6 +98,7 @@ def test_should_show_overview_for_service_with_more_things_set( service_with_reply_to_addresses, mock_get_organisation, mock_get_letter_organisations, + mock_get_inbound_number_for_service, permissions, expected_rows ): @@ -122,7 +124,8 @@ def test_service_settings_show_elided_api_url_if_needed( mocker, fake_uuid, url, - elided_url + elided_url, + mock_get_inbound_number_for_service ): service_one['permissions'] = ['sms', 'email', 'inbound_sms'] service_one['inbound_api'] = [fake_uuid] @@ -150,6 +153,7 @@ def test_if_cant_send_letters_then_cant_see_letter_contact_block( logged_in_client, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): response = logged_in_client.get(url_for( 'main.service_settings', service_id=service_one['id'] @@ -161,18 +165,19 @@ def test_if_can_receive_inbound_then_cant_change_sms_sender( logged_in_client, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): service_one['permissions'] = ['email', 'sms', 'inbound_sms'] - service_one['sms_sender'] = 'SomeNumber' response = logged_in_client.get(url_for( 'main.service_settings', service_id=service_one['id'] )) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') rows_as_text = [" ".join(row.text.split()) for row in page.find_all('tr')] - assert 'Text message sender SomeNumber Change' not in rows_as_text + assert 'Text message sender 0781239871 Change' not in rows_as_text assert url_for('main.service_request_to_go_live', service_id=service_one['id'], set_inbound_sms=False) not in response.get_data(as_text=True) - assert 'SomeNumber' in response.get_data(as_text=True) + assert '0781239871' in response.get_data(as_text=True) + def test_letter_contact_block_shows_none_if_not_set( @@ -180,6 +185,7 @@ def test_letter_contact_block_shows_none_if_not_set( service_one, mocker, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): service_one['permissions'] = ['letter'] response = logged_in_client.get(url_for( @@ -197,6 +203,7 @@ def test_escapes_letter_contact_block( service_one, mocker, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): service_one['permissions'] = ['letter'] service_one['letter_contact_block'] = 'foo\nbar' @@ -244,6 +251,7 @@ def test_show_restricted_service( logged_in_client, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -255,6 +263,7 @@ def test_switch_service_to_live( logged_in_platform_admin_client, service_one, mock_update_service, + mock_get_inbound_number_for_service ): response = logged_in_platform_admin_client.get( url_for('main.service_switch_live', service_id=service_one['id'])) @@ -274,6 +283,7 @@ def test_show_live_service( service_one, mock_get_live_service, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -285,7 +295,8 @@ def test_switch_service_to_restricted( logged_in_platform_admin_client, service_one, mock_get_live_service, - mock_update_service + mock_update_service, + mock_get_inbound_number_for_service ): response = logged_in_platform_admin_client.get( url_for('main.service_switch_live', service_id=service_one['id'])) @@ -335,7 +346,8 @@ def test_should_redirect_after_service_name_confirmation( logged_in_client, service_one, mock_update_service, - mock_verify_password + mock_verify_password, + mock_get_inbound_number_for_service ): service_id = service_one['id'] service_new_name = 'New Name' @@ -394,6 +406,7 @@ def test_should_redirect_after_request_to_go_live( service_one, mocker, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): mock_post = mocker.patch( 'app.main.views.feedback.requests.post', @@ -487,6 +500,7 @@ def test_route_permissions( service_one, mock_get_letter_organisations, route, + mock_get_inbound_number_for_service ): validate_route_permission( mocker, @@ -543,6 +557,7 @@ def test_route_for_platform_admin( service_one, mock_get_letter_organisations, route, + mock_get_inbound_number_for_service ): validate_route_permission(mocker, app_, @@ -593,6 +608,7 @@ def test_enabling_and_disabling_email_and_sms( notification_type, permissions_before_switch, permissions_after_switch, + mock_get_inbound_number_for_service ): service_one['permissions'] = permissions_before_switch mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) @@ -611,6 +627,7 @@ def test_set_reply_to_email_address( mock_update_service, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): service_one['permissions'] = ['email'] data = {"email_address": "test@someservice.gov.uk"} @@ -627,6 +644,7 @@ def test_set_reply_to_email_address( def test_if_reply_to_email_address_set_then_form_populated( logged_in_client, service_one, + mock_get_inbound_number_for_service ): service_one['permissions'] = ['email'] service_one['reply_to_email_address'] = 'test@service.gov.uk' @@ -683,6 +701,7 @@ def test_shows_research_mode_indicator( service_one, mocker, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): service_one['research_mode'] = True mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) @@ -699,6 +718,7 @@ def test_does_not_show_research_mode_indicator( logged_in_client, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) assert response.status_code == 200 @@ -713,6 +733,7 @@ def test_set_text_message_sender( 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']), @@ -731,6 +752,7 @@ def test_set_text_message_sender_and_inbound_sms( 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', @@ -756,6 +778,7 @@ def test_turn_inbound_sms_off( 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', @@ -781,6 +804,7 @@ def test_set_text_message_sender_and_not_inbound_sms( 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', @@ -810,6 +834,7 @@ def test_set_text_message_sender_validation( mock_update_service, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service, content, expected_error, ): @@ -857,6 +882,7 @@ def test_set_inbound_api_validation( def test_if_sms_sender_set_then_form_populated( logged_in_client, service_one, + mock_get_inbound_number_for_service ): service_one['sms_sender'] = 'elevenchars' response = logged_in_client.get(url_for('main.service_set_sms_sender', service_id=service_one['id'])) @@ -1237,6 +1263,7 @@ def test_archive_service_after_confirm( logged_in_platform_admin_client, service_one, mocker, + mock_get_inbound_number_for_service ): mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) @@ -1252,6 +1279,7 @@ def test_archive_service_prompts_user( service_one, mocker, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): mocked_fn = mocker.patch('app.service_api_client.post') @@ -1267,6 +1295,7 @@ def test_cant_archive_inactive_service( logged_in_platform_admin_client, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): service_one['active'] = False @@ -1281,6 +1310,7 @@ def test_suspend_service_after_confirm( logged_in_platform_admin_client, service_one, mocker, + mock_get_inbound_number_for_service ): mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) @@ -1296,6 +1326,7 @@ def test_suspend_service_prompts_user( service_one, mocker, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): mocked_fn = mocker.patch('app.service_api_client.post') @@ -1312,6 +1343,7 @@ def test_cant_suspend_inactive_service( logged_in_platform_admin_client, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): service_one['active'] = False @@ -1326,6 +1358,7 @@ def test_resume_service_after_confirm( logged_in_platform_admin_client, service_one, mocker, + mock_get_inbound_number_for_service ): service_one['active'] = False mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) @@ -1342,6 +1375,7 @@ def test_resume_service_prompts_user( service_one, mocker, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): service_one['active'] = False mocked_fn = mocker.patch('app.service_api_client.post') @@ -1359,6 +1393,7 @@ def test_cant_resume_active_service( logged_in_platform_admin_client, service_one, mock_get_letter_organisations, + mock_get_inbound_number_for_service ): response = logged_in_platform_admin_client.get(url_for('main.service_settings', service_id=service_one['id'])) diff --git a/tests/conftest.py b/tests/conftest.py index 955440e97..dc1039091 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,7 +14,6 @@ from app.notify_client.models import ( InvitedUser ) - from . import ( service_json, TestClient, @@ -199,8 +198,8 @@ def mock_update_service(mocker): @pytest.fixture(scope='function') def mock_update_service_raise_httperror_duplicate_name(mocker): def _update( - service_id, - **kwargs + service_id, + **kwargs ): json_mock = Mock(return_value={'message': {'name': ["Duplicate service name '{}'".format(kwargs.get('name'))]}}) resp_mock = Mock(status_code=400, json=json_mock) @@ -275,7 +274,6 @@ def mock_get_service_template(mocker): @pytest.fixture(scope='function') def mock_get_service_template_with_priority(mocker): def _get(service_id, template_id, version=None): - template = template_json( service_id, template_id, "Two week reminder", "sms", "Template content with & entity", process_type='priority') @@ -499,7 +497,6 @@ def mock_get_service_templates(mocker): @pytest.fixture(scope='function') def mock_get_service_templates_when_no_templates_exist(mocker): - def _create(service_id): return {'data': []} @@ -510,7 +507,6 @@ def mock_get_service_templates_when_no_templates_exist(mocker): @pytest.fixture(scope='function') def mock_get_service_templates_with_only_one_template(mocker): - def _get(service_id): return {'data': [ template_json( @@ -1100,24 +1096,24 @@ def mock_get_jobs(mocker, api_user_active): @pytest.fixture(scope='function') def mock_get_notifications( - mocker, - api_user_active, - template_content=None, - personalisation=None, - redact_personalisation=False, + mocker, + api_user_active, + template_content=None, + personalisation=None, + redact_personalisation=False, ): def _get_notifications( - service_id, - job_id=None, - page=1, - page_size=50, - template_type=None, - status=None, - limit_days=None, - rows=5, - include_jobs=None, - include_from_test_key=None, - to=None, + service_id, + job_id=None, + page=1, + page_size=50, + template_type=None, + status=None, + limit_days=None, + rows=5, + include_jobs=None, + include_from_test_key=None, + to=None, ): job = None if job_id is not None: @@ -1195,8 +1191,8 @@ def mock_get_notifications_with_no_notifications(mocker): @pytest.fixture(scope='function') def mock_get_inbound_sms(mocker): def _get_inbound_sms( - service_id, - user_number=None, + service_id, + user_number=None, ): return [{ 'user_number': '0790090000' + str(i), @@ -1214,7 +1210,7 @@ def mock_get_inbound_sms(mocker): @pytest.fixture(scope='function') def mock_get_inbound_sms_with_no_messages(mocker): def _get_inbound_sms( - service_id, + service_id, ): return [] @@ -1227,7 +1223,7 @@ def mock_get_inbound_sms_with_no_messages(mocker): @pytest.fixture(scope='function') def mock_get_inbound_sms_summary(mocker): def _get_inbound_sms_summary( - service_id, + service_id, ): return { 'count': 99, @@ -1243,7 +1239,7 @@ def mock_get_inbound_sms_summary(mocker): @pytest.fixture(scope='function') def mock_get_inbound_sms_summary_with_no_messages(mocker): def _get_inbound_sms_summary( - service_id, + service_id, ): return { 'count': 0, @@ -1256,6 +1252,13 @@ def mock_get_inbound_sms_summary_with_no_messages(mocker): ) +@pytest.fixture(scope='function') +def mock_get_inbound_number_for_service(mocker): + return mocker.patch( + 'app.inbound_number_client.get_inbound_sms_number_for_service', + return_value={'data': {'number': '0781239871'}}) + + @pytest.fixture(scope='function') def mock_has_permissions(mocker): def _has_permission(permissions=None, any_=False, admin_override=False): @@ -1309,6 +1312,7 @@ def mock_s3_download(mocker, content=None): def _download(service_id, upload_id): return content + return mocker.patch('app.main.views.send.s3download', side_effect=_download) @@ -1423,6 +1427,7 @@ def mock_get_monthly_template_statistics(mocker, service_one, fake_uuid): } } } + return mocker.patch( 'app.template_statistics_client.get_monthly_template_statistics_for_service', side_effect=_stats @@ -1444,6 +1449,7 @@ def mock_get_monthly_notification_stats(mocker, service_one, fake_uuid): }, } }} + return mocker.patch( 'app.service_api_client.get_monthly_notification_stats', side_effect=_stats @@ -1695,15 +1701,15 @@ def mock_reset_failed_login_count(mocker): @pytest.fixture def mock_get_notification( - mocker, - fake_uuid, - notification_status='delivered', - redact_personalisation=False, - template_type=None, + mocker, + fake_uuid, + notification_status='delivered', + redact_personalisation=False, + template_type=None, ): def _get_notification( - service_id, - notification_id, + service_id, + notification_id, ): noti = notification_json( service_id, @@ -1738,7 +1744,7 @@ def mock_get_notification( @pytest.fixture def mock_send_notification(mocker, fake_uuid): def _send_notification( - service_id, *, template_id, recipient, personalisation + service_id, *, template_id, recipient, personalisation ): return {'id': fake_uuid} @@ -1756,11 +1762,11 @@ def client(app_): @pytest.fixture(scope='function') def logged_in_client( - client, - active_user_with_permissions, - mocker, - service_one, - mock_login + client, + active_user_with_permissions, + mocker, + service_one, + mock_login ): client.login(active_user_with_permissions, mocker, service_one) yield client @@ -1768,11 +1774,11 @@ def logged_in_client( @pytest.fixture(scope='function') def logged_in_platform_admin_client( - client, - platform_admin_user, - mocker, - service_one, - mock_login, + client, + platform_admin_user, + mocker, + service_one, + mock_login, ): mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user, mocker, service_one) @@ -1803,11 +1809,11 @@ def client_request(logged_in_client): @staticmethod def get( - endpoint, - _expected_status=200, - _follow_redirects=False, - _test_page_title=True, - **endpoint_kwargs + endpoint, + _expected_status=200, + _follow_redirects=False, + _test_page_title=True, + **endpoint_kwargs ): resp = logged_in_client.get( url_for(endpoint, **(endpoint_kwargs or {})),