From 6837b76d44c55c3068e7952f16ee7443d70f5253 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 15 Feb 2021 20:59:26 +0000 Subject: [PATCH] Remove existing broadcast permission form This will be replaced by a new form that has it's own template, route etc as it will vary quite a lot from the existing service permission form. --- app/main/views/service_settings.py | 27 ----------- app/models/service.py | 11 ----- app/navigation.py | 4 -- .../test_service_setting_permissions.py | 48 ++++++------------- tests/app/main/views/test_service_settings.py | 1 - 5 files changed, 15 insertions(+), 76 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 03fcd9c92..60307f847 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -71,7 +71,6 @@ PLATFORM_ADMIN_SERVICE_PERMISSIONS = OrderedDict([ ('inbound_sms', {'title': 'Receive inbound SMS', 'requires': 'sms', 'endpoint': '.service_set_inbound_number'}), ('email_auth', {'title': 'Email authentication'}), ('international_letters', {'title': 'Send international letters', 'requires': 'letter'}), - ('broadcast', {'title': 'Send cell broadcasts'}), ]) @@ -315,32 +314,6 @@ def service_set_permission(service_id, permission): ) -@main.route("/services//service-settings/permissions/broadcast", methods=["GET", "POST"]) -@user_is_platform_admin -def service_set_broadcast_permission(service_id): - - title = PLATFORM_ADMIN_SERVICE_PERMISSIONS['broadcast']['title'] - form = ServiceOnOffSettingForm( - name=title, - enabled=current_service.has_permission('broadcast') - ) - - if form.validate_on_submit(): - - if form.enabled.data: - current_service.force_broadcast_permission_on() - else: - current_service.force_permission('broadcast', on=False) - - return redirect(url_for(".service_settings", service_id=service_id)) - - return render_template( - 'views/service-settings/set-service-setting.html', - title=title, - form=form, - ) - - @main.route("/services//service-settings/archive", methods=['GET', 'POST']) @user_has_permissions('manage_service') def archive_service(service_id): diff --git a/app/models/service.py b/app/models/service.py index 44c5cd3c5..4448dcd1c 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -116,17 +116,6 @@ class Service(JSONModel): permissions | permission if on else permissions - permission, ) - def force_broadcast_permission_on(self): - ret = self.update_permissions( - set(self.permissions) - {'email', 'sms', 'letter'} | {'broadcast'} - ) - broadcast_org_id = current_app.config['BROADCAST_ORGANISATION_ID'] - organisations_client.update_service_organisation( - service_id=self.id, - org_id=broadcast_org_id - ) - return ret - def update_permissions(self, permissions): return self.update(permissions=list(permissions)) diff --git a/app/navigation.py b/app/navigation.py index 99efe22ce..4985bd680 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -316,7 +316,6 @@ class HeaderNavigation(Navigation): 'service_switch_count_as_live', 'service_switch_live', 'service_set_permission', - 'service_set_broadcast_permission', 'service_verify_reply_to_address', 'service_verify_reply_to_address_updates', 'services_or_dashboard', @@ -681,7 +680,6 @@ class MainNavigation(Navigation): 'service_switch_count_as_live', 'service_switch_live', 'service_set_permission', - 'service_set_broadcast_permission', 'services_or_dashboard', 'show_accounts_or_dashboard', 'sign_in', @@ -991,7 +989,6 @@ class CaseworkNavigation(Navigation): 'service_switch_count_as_live', 'service_switch_live', 'service_set_permission', - 'service_set_broadcast_permission', 'service_verify_reply_to_address', 'service_verify_reply_to_address_updates', 'services_or_dashboard', @@ -1313,7 +1310,6 @@ class OrgNavigation(Navigation): 'service_switch_count_as_live', 'service_switch_live', 'service_set_permission', - 'service_set_broadcast_permission', 'service_verify_reply_to_address', 'service_verify_reply_to_address_updates', 'services_or_dashboard', diff --git a/tests/app/main/views/service_settings/test_service_setting_permissions.py b/tests/app/main/views/service_settings/test_service_setting_permissions.py index 1ca27fe54..9ce561106 100644 --- a/tests/app/main/views/service_settings/test_service_setting_permissions.py +++ b/tests/app/main/views/service_settings/test_service_setting_permissions.py @@ -4,7 +4,7 @@ import pytest from flask import url_for from app.main.views.service_settings import PLATFORM_ADMIN_SERVICE_PERMISSIONS -from tests.conftest import SERVICE_ONE_ID, normalize_spaces, set_config +from tests.conftest import normalize_spaces @pytest.fixture @@ -38,6 +38,20 @@ def test_service_set_permission_requires_platform_admin( ) +def test_service_set_permission_does_not_exist_for_broadcast_permission( + mocker, + client_request, + platform_admin_user, + service_one, + mock_get_inbound_number_for_service, +): + client_request.login(platform_admin_user) + client_request.get( + 'main.service_set_permission', service_id=service_one['id'], permission='broadcast', + _expected_status=404 + ) + + @pytest.mark.parametrize('initial_permissions, permission, form_data, expected_update', [ ( [], @@ -75,18 +89,6 @@ def test_service_set_permission_requires_platform_admin( 'False', [], ), - ( - ['email', 'sms', 'letter', 'international_sms', 'international_letters'], - 'broadcast', - 'True', - ['international_sms', 'international_letters', 'broadcast'], - ), - ( - ['broadcast', 'international_sms', 'international_letters'], - 'broadcast', - 'False', - ['international_sms', 'international_letters'], - ), ]) def test_service_set_permission( mocker, @@ -213,23 +215,3 @@ def test_normal_user_doesnt_see_any_platform_admin_settings( for permission in platform_admin_settings: assert permission not in page - - -def test_setting_broadcast_sets_organisation_if_config_value_set( - mock_update_service_organisation, - mock_update_service, - platform_admin_client, - fake_uuid, -): - with set_config(platform_admin_client.application, 'BROADCAST_ORGANISATION_ID', fake_uuid): - response = platform_admin_client.post( - url_for('main.service_set_permission', service_id=SERVICE_ONE_ID, permission='broadcast'), - data={'enabled': True} - ) - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=SERVICE_ONE_ID, _external=True) - - mock_update_service_organisation.assert_called_once_with( - service_id=SERVICE_ONE_ID, - org_id=fake_uuid - ) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index f8a254258..c596f6949 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -185,7 +185,6 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( 'Label Value Action', 'Notes None 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)