From 752b685b26461844a32d412dd875fc57d8756eb2 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Thu, 4 Feb 2021 17:11:49 +0000 Subject: [PATCH] Show only relevant platform admin settings for broadcast service A few of note Count in list of live services - this should be set to no in the API (to be implemented) so we never show broadcast services in the list of live services to reduce security leaks Organisation - all broadcast services are currently going to be found in a single organisation so we keep track of them easily. Therefore there is no need to allow the changing of the organisation Email authentication - we may in time not allow these services to use email auth to log in but this hasn't been decided so let's keep it for the moment Rate limit - although a service may end up using our API to create broadcasts, there is currently no rate limit check on this endpoint and it's also extremely unlikely that any service would ever breach the default limit --- app/templates/views/service-settings.html | 6 ++- tests/app/main/views/test_service_settings.py | 46 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 9ec635c4b..437c63237 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -329,17 +329,18 @@ {% endif %} {% endcall %} + {% if not current_service.has_permission('broadcast') %} {% call row() %} {{ text_field('Count in list of live services')}} {{ text_field('Yes' if current_service.count_as_live else 'No') }} {{ edit_field('Change', url_for('.service_switch_count_as_live', service_id=current_service.id), suffix='if service is counted in list of live services') }} {% endcall %} - {% call row() %} {{ text_field('Billing details')}} {{ optional_text_field(current_service.billing_details, default="No billing details yet", wrap=True) }} {{ edit_field('Change', url_for('.edit_service_billing_details', service_id=current_service.id), suffix='billing details for service') }} {% endcall %} + {% endif %} {% call row() %} {{ text_field('Notes')}} @@ -347,6 +348,7 @@ {{ edit_field('Change', url_for('.edit_service_notes', service_id=current_service.id), suffix='the notes for the service') }} {% endcall %} + {% if not current_service.has_permission('broadcast') %} {% call row() %} {{ text_field('Organisation')}} {% call field() %} @@ -365,6 +367,7 @@ {% endcall %} {{ edit_field('Change', url_for('.link_service_to_organisation', service_id=current_service.id), suffix='organisation for service') }} {% endcall %} + {% call row() %} {{ text_field('Rate limit')}} {{ text_field('{:,} per minute'.format(current_service.rate_limit)) }} @@ -407,6 +410,7 @@ {% endcall %} {{ edit_field('Change', url_for('.data_retention', service_id=current_service.id), suffix='data retention') }} {% endcall %} + {% endif %} {% for permission in service_permissions %} {% if not service_permissions[permission].requires or current_service.has_permission(service_permissions[permission].requires) %} diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index eba95138b..e7c3f0d7c 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -149,6 +149,52 @@ def test_should_show_overview( app.service_api_client.get_service.assert_called_with(SERVICE_ONE_ID) +def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( + client, + mocker, + api_user_active, + no_reply_to_email_addresses, + no_letter_contact_blocks, + mock_get_organisation, + single_sms_sender, + mock_get_service_settings_page_common, +): + service_one = service_json( + SERVICE_ONE_ID, + users=[api_user_active['id']], + permissions=['broadcast'], + organisation_id=ORGANISATION_ID, + contact_link='contact_us@gov.uk', + ) + mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) + + client.login(create_platform_admin_user(), mocker, service_one) + response = client.get(url_for( + 'main.service_settings', service_id=SERVICE_ONE_ID + )) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.find('h1').text == 'Settings' + rows = page.select('tr') + + expected_rows = [ + 'Label Value Action', + 'Service name Test Service Change service name', + 'Sign-in method Text message code Change sign-in method', + + 'Label Value Action', + 'Live Off Change service status', + 'Notes No notes yet Change the notes for the service', + 'Email authentication Off Change your settings for Email authentication', + 'Send cell broadcasts On Change your settings for Send cell broadcasts', + ] + + assert len(rows) == len(expected_rows) + for index, row in enumerate(expected_rows): + assert row == " ".join(rows[index].text.split()) + app.service_api_client.get_service.assert_called_with(SERVICE_ONE_ID) + + def test_no_go_live_link_for_service_without_organisation( client_request, mocker,