From 97aca503f714f8cc249819ac34da709a0b358948 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 13 Jul 2020 08:56:57 +0100 Subject: [PATCH 1/5] No emails, texts or letters for broadcast services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To make the interface as simple as possible we don’t want to mix up sending other types of communication where services have the broadcast permission. This commit removes the other permissions once a service has been given the broadcast permission by a platform admin user. --- app/main/views/service_settings.py | 29 ++++++++++++ app/navigation.py | 4 ++ .../test_service_setting_permissions.py | 44 +++++++++++++------ 3 files changed, 64 insertions(+), 13 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 9aa2257e8..6bebd361b 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -305,6 +305,35 @@ 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_permission('broadcast', on=True) + current_service.force_permission('email', on=False) + current_service.force_permission('sms', on=False) + current_service.force_permission('letter', on=False) + 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/navigation.py b/app/navigation.py index a2a5137cd..e099d3776 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -307,6 +307,7 @@ 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', @@ -635,6 +636,7 @@ 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', @@ -933,6 +935,7 @@ 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', @@ -1237,6 +1240,7 @@ 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 ece67d2a3..8c88d630b 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 @@ -1,4 +1,5 @@ import functools +from unittest.mock import call import pytest from flask import url_for @@ -38,15 +39,34 @@ def test_service_set_permission_requires_platform_admin( ) -@pytest.mark.parametrize('permission, form_data, on', [ - ('inbound_sms', 'True', True), - ('inbound_sms', 'False', False), - ('email_auth', 'True', True), - ('email_auth', 'False', False), - ('international_letters', 'True', True), - ('international_letters', 'False', False), - ('broadcast', 'True', True), - ('broadcast', 'False', False), +@pytest.mark.parametrize('permission, form_data, expected_calls', [ + ('inbound_sms', 'True', [ + call('inbound_sms', on=True), + ]), + ('inbound_sms', 'False', [ + call('inbound_sms', on=False), + ]), + ('email_auth', 'True', [ + call('email_auth', on=True), + ]), + ('email_auth', 'False', [ + call('email_auth', on=False), + ]), + ('international_letters', 'True', [ + call('international_letters', on=True), + ]), + ('international_letters', 'False', [ + call('international_letters', on=False), + ]), + ('broadcast', 'True', [ + call('broadcast', on=True), + call('email', on=False), + call('sms', on=False), + call('letter', on=False), + ]), + ('broadcast', 'False', [ + call('broadcast', on=False), + ]), ]) def test_service_set_permission( mocker, @@ -55,7 +75,7 @@ def test_service_set_permission( mock_get_inbound_number_for_service, permission, form_data, - on + expected_calls, ): force_permission = mocker.patch('app.models.service.Service.force_permission') response = platform_admin_client.post( @@ -64,9 +84,7 @@ def test_service_set_permission( ) assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - force_permission.assert_called_with( - permission, on=on - ) + assert force_permission.call_args_list == expected_calls @pytest.mark.parametrize('service_fields, endpoint, kwargs, text', [ From c17db333d3db7e05d984dc194746267cbdc01381 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 13 Jul 2020 08:43:29 +0100 Subject: [PATCH 2/5] Hide email, text message and letter settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Services with the broadcast permission won’t be able to send emails, text messages or letters. So we should avoid confusion by removing any reference to these things. --- app/templates/views/service-settings.html | 375 +++++++++--------- tests/app/main/views/test_service_settings.py | 6 + 2 files changed, 195 insertions(+), 186 deletions(-) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 3b6a6cc7e..9899de72c 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -47,203 +47,206 @@ {% endcall %} {% endcall %} - {% call mapping_table( - caption='Email settings', - field_headings=['Label', 'Value', 'Action'], - field_headings_visible=False, - caption_visible=True - ) %} + {% if not current_service.has_permission('broadcast') %} + + {% call mapping_table( + caption='Email settings', + field_headings=['Label', 'Value', 'Action'], + field_headings_visible=False, + caption_visible=True + ) %} + + {% call row() %} + {{ text_field('Send emails') }} + {{ boolean_field('email' in current_service.permissions) }} + {{ edit_field( + 'Change', + url_for( + '.service_set_channel', + channel='email', + service_id=current_service.id + ), + permissions=['manage_service'] + )}} + {% endcall %} + + {% call settings_row(if_has_permission='email') %} + {{ text_field('Reply-to email addresses') }} + {% call field(status='default' if current_service.count_email_reply_to_addresses == 0 else '') %} + {{ current_service.default_email_reply_to_address or 'Not set' }} + {% if current_service.count_email_reply_to_addresses > 1 %} +
+ {{ '…and %d more' | format(current_service.count_email_reply_to_addresses - 1) }} +
+ {% endif %} + {% endcall %} + {{ edit_field( + 'Manage', + url_for('.service_email_reply_to', + service_id=current_service.id), + permissions=['manage_service','manage_api_keys'] + ) + }} + {% endcall %} + + {% call settings_row(if_has_permission='email') %} + {{ text_field('Email branding') }} + {{ text_field(current_service.email_branding_name) }} + {{ edit_field( + 'Change', + url_for('.branding_request', service_id=current_service.id, branding_type="email"), + permissions=['manage_service'], + )}} + {% endcall %} + + {% call settings_row(if_has_permission='email') %} + {{ text_field('Send files by email') }} + {{ text_field(current_service.contact_link if current_service.contact_link else "Not set up", truncate=true) }} + {{ edit_field( + 'Manage', + url_for('.send_files_by_email_contact_details', service_id=current_service.id), + permissions=['manage_service'], + )}} + {% endcall %} - {% call row() %} - {{ text_field('Send emails') }} - {{ boolean_field('email' in current_service.permissions) }} - {{ edit_field( - 'Change', - url_for( - '.service_set_channel', - channel='email', - service_id=current_service.id - ), - permissions=['manage_service'] - )}} {% endcall %} - {% call settings_row(if_has_permission='email') %} - {{ text_field('Reply-to email addresses') }} - {% call field(status='default' if current_service.count_email_reply_to_addresses == 0 else '') %} - {{ current_service.default_email_reply_to_address or 'Not set' }} - {% if current_service.count_email_reply_to_addresses > 1 %} -
- {{ '…and %d more' | format(current_service.count_email_reply_to_addresses - 1) }} -
- {% endif %} + {% call mapping_table( + caption='Text message settings', + field_headings=['Label', 'Value', 'Action'], + field_headings_visible=False, + caption_visible=True + ) %} + + {% call row() %} + {{ text_field('Send text messages') }} + {{ boolean_field('sms' in current_service.permissions) }} + {{ edit_field( + 'Change', + url_for( + '.service_set_channel', + service_id=current_service.id, + channel='sms' + ), + permissions=['manage_service'] + )}} {% endcall %} - {{ edit_field( - 'Manage', - url_for('.service_email_reply_to', - service_id=current_service.id), - permissions=['manage_service','manage_api_keys'] + + {% call settings_row(if_has_permission='sms') %} + {{ text_field('Text message senders') }} + {% call field(status='default' if current_service.default_sms_sender == "None" else '') %} + {{ current_service.default_sms_sender | nl2br if current_service.default_sms_sender else 'None'}} + {% if current_service.count_sms_senders > 1 %} +
+ {{ '…and %d more' | format(current_service.count_sms_senders - 1) }} +
+ {% endif %} + {% endcall %} + {{ edit_field( + 'Manage', + url_for('.service_sms_senders', + service_id=current_service.id), + permissions=['manage_service','manage_api_keys'] ) - }} - {% endcall %} - - {% call settings_row(if_has_permission='email') %} - {{ text_field('Email branding') }} - {{ text_field(current_service.email_branding_name) }} - {{ edit_field( - 'Change', - url_for('.branding_request', service_id=current_service.id, branding_type="email"), - permissions=['manage_service'], - )}} - {% endcall %} - - {% call settings_row(if_has_permission='email') %} - {{ text_field('Send files by email') }} - {{ text_field(current_service.contact_link if current_service.contact_link else "Not set up", truncate=true) }} - {{ edit_field( - 'Manage', - url_for('.send_files_by_email_contact_details', service_id=current_service.id), - permissions=['manage_service'], - )}} - {% endcall %} - - {% endcall %} - - {% call mapping_table( - caption='Text message settings', - field_headings=['Label', 'Value', 'Action'], - field_headings_visible=False, - caption_visible=True - ) %} - - {% call row() %} - {{ text_field('Send text messages') }} - {{ boolean_field('sms' in current_service.permissions) }} - {{ edit_field( - 'Change', - url_for( - '.service_set_channel', - service_id=current_service.id, - channel='sms' - ), - permissions=['manage_service'] - )}} - {% endcall %} - - {% call settings_row(if_has_permission='sms') %} - {{ text_field('Text message senders') }} - {% call field(status='default' if current_service.default_sms_sender == "None" else '') %} - {{ current_service.default_sms_sender | nl2br if current_service.default_sms_sender else 'None'}} - {% if current_service.count_sms_senders > 1 %} -
- {{ '…and %d more' | format(current_service.count_sms_senders - 1) }} -
- {% endif %} + }} {% endcall %} - {{ edit_field( - 'Manage', - url_for('.service_sms_senders', - service_id=current_service.id), - permissions=['manage_service','manage_api_keys'] - ) - }} - {% endcall %} - {% call settings_row(if_has_permission='sms') %} - {{ text_field('Start text messages with service name') }} - {{ boolean_field(current_service.prefix_sms) }} - {{ edit_field( - 'Change', - url_for('.service_set_sms_prefix', - service_id=current_service.id), - permissions=['manage_service'] - ) - }} - {% endcall %} - - {% call settings_row(if_has_permission='sms') %} - {{ text_field('Send international text messages') }} - {{ boolean_field('international_sms' in current_service.permissions) }} - {{ edit_field( - 'Change', - url_for('.service_set_international_sms', - service_id=current_service.id), - permissions=['manage_service'] - ) - }} - {% endcall %} - - {% call settings_row(if_has_permission='sms') %} - {{ text_field('Receive text messages') }} - {{ boolean_field('inbound_sms' in current_service.permissions) }} - {{ edit_field( - 'Change', - url_for('.service_set_inbound_sms', - service_id=current_service.id), - permissions=['manage_service'] - ) - }} - {% endcall %} - - {% endcall %} - - {% call mapping_table( - caption='Letter settings', - field_headings=['Label', 'Value', 'Action'], - field_headings_visible=False, - caption_visible=True - ) %} - - {% call row() %} - {{ text_field('Send letters') }} - {{ boolean_field('letter' in current_service.permissions) }} - {{ edit_field( - 'Change', - url_for( - '.service_set_channel', - channel='letter', - service_id=current_service.id - ), - permissions=['manage_service'] - )}} - {% endcall %} - - {% call settings_row(if_has_permission='letter') %} - {{ text_field('Sender addresses') }} - {% call field(status='' if current_service.count_letter_contact_details else 'default') %} - {% if current_service.default_letter_contact_block %} - {{ current_service.default_letter_contact_block_html }} - {% elif current_service.count_letter_contact_details %} - Blank - {% else %} - Not set - {% endif %} - {% if current_service.count_letter_contact_details > 1 %} -
- {{ '…and %d more' | format(current_service.count_letter_contact_details - 1) }} -
- {% endif %} + {% call settings_row(if_has_permission='sms') %} + {{ text_field('Start text messages with service name') }} + {{ boolean_field(current_service.prefix_sms) }} + {{ edit_field( + 'Change', + url_for('.service_set_sms_prefix', + service_id=current_service.id), + permissions=['manage_service'] + ) + }} {% endcall %} - {{ edit_field( - 'Manage', - url_for('.service_letter_contact_details', - service_id=current_service.id), - permissions=['manage_service','manage_api_keys'] - ) - }} + + {% call settings_row(if_has_permission='sms') %} + {{ text_field('Send international text messages') }} + {{ boolean_field('international_sms' in current_service.permissions) }} + {{ edit_field( + 'Change', + url_for('.service_set_international_sms', + service_id=current_service.id), + permissions=['manage_service'] + ) + }} + {% endcall %} + + {% call settings_row(if_has_permission='sms') %} + {{ text_field('Receive text messages') }} + {{ boolean_field('inbound_sms' in current_service.permissions) }} + {{ edit_field( + 'Change', + url_for('.service_set_inbound_sms', + service_id=current_service.id), + permissions=['manage_service'] + ) + }} + {% endcall %} + {% endcall %} - {% call settings_row(if_has_permission='letter') %} - {{ text_field('Letter branding') }} - {{ optional_text_field(current_service.letter_branding.name) }} - {{ edit_field( - 'Change', - url_for('.branding_request', service_id=current_service.id, branding_type="letter"), - permissions=['manage_service'] - )}} - {% endcall %} + {% call mapping_table( + caption='Letter settings', + field_headings=['Label', 'Value', 'Action'], + field_headings_visible=False, + caption_visible=True + ) %} - {% endcall %} + {% call row() %} + {{ text_field('Send letters') }} + {{ boolean_field('letter' in current_service.permissions) }} + {{ edit_field( + 'Change', + url_for( + '.service_set_channel', + channel='letter', + service_id=current_service.id + ), + permissions=['manage_service'] + )}} + {% endcall %} + + {% call settings_row(if_has_permission='letter') %} + {{ text_field('Sender addresses') }} + {% call field(status='' if current_service.count_letter_contact_details else 'default') %} + {% if current_service.default_letter_contact_block %} + {{ current_service.default_letter_contact_block_html }} + {% elif current_service.count_letter_contact_details %} + Blank + {% else %} + Not set + {% endif %} + {% if current_service.count_letter_contact_details > 1 %} +
+ {{ '…and %d more' | format(current_service.count_letter_contact_details - 1) }} +
+ {% endif %} + {% endcall %} + {{ edit_field( + 'Manage', + url_for('.service_letter_contact_details', + service_id=current_service.id), + permissions=['manage_service','manage_api_keys'] + ) + }} + {% endcall %} + + {% call settings_row(if_has_permission='letter') %} + {{ text_field('Letter branding') }} + {{ optional_text_field(current_service.letter_branding.name) }} + {{ edit_field( + 'Change', + url_for('.branding_request', service_id=current_service.id, branding_type="letter"), + permissions=['manage_service'] + )}} + {% endcall %} + + {% endcall %} + {% endif %} {% if current_service.trial_mode %} diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 9fc755635..c5f4bb4a2 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -289,6 +289,12 @@ def test_send_files_by_email_row_on_settings_page( 'Letter branding Not set Change', ]), + (['broadcast'], [ + + 'Service name service one Change', + 'Sign-in method Text message code Change', + + ]), ]) def test_should_show_overview_for_service_with_more_things_set( client, From 89dd64cbadf7574850673114a1c8389f93de8919 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 13 Jul 2020 09:04:53 +0100 Subject: [PATCH 3/5] =?UTF-8?q?Don=E2=80=99t=20let=20broadcast=20re-enable?= =?UTF-8?q?=20other=20channels?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a service is switched over to broadcast it has the email, text message and letter permissions removed. And the links to switch these settings back on are hidden. This commit ensures that even if the user manually goes to the URLs for these pages, they still won’t be able to switch the other channels back on. --- app/main/views/service_settings.py | 3 +++ tests/app/main/views/test_service_settings.py | 26 +++++++++++++++++++ tests/conftest.py | 1 + 3 files changed, 30 insertions(+) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 6bebd361b..c4e695a6d 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -696,6 +696,9 @@ def service_set_channel(service_id, channel): if channel not in {'email', 'sms', 'letter'}: abort(404) + if current_service.has_permission('broadcast'): + abort(403) + form = ServiceSwitchChannelForm( channel=channel, enabled=current_service.has_permission(channel) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index c5f4bb4a2..55e402004 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -3729,6 +3729,32 @@ def test_switch_service_enable_letters( assert mocked_fn.call_args[0][0] == service_one['id'] +@pytest.mark.parametrize('channel', ( + 'email', 'sms', 'letter', +)) +def test_broadcast_service_cant_post_to_set_other_channels_endpoint( + client_request, + service_one, + channel, +): + service_one['permissions'] = ['broadcast'] + + client_request.get( + 'main.service_set_channel', + service_id=SERVICE_ONE_ID, + channel=channel, + _expected_status=403, + ) + + client_request.post( + 'main.service_set_channel', + service_id=SERVICE_ONE_ID, + channel=channel, + _data={'enabled': 'True'}, + _expected_status=403, + ) + + @pytest.mark.parametrize('permissions, expected_checked', [ (['international_sms'], 'on'), ([''], 'off'), diff --git a/tests/conftest.py b/tests/conftest.py index 6cee5a58b..818b68da3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3037,6 +3037,7 @@ def client_request( _expected_status = 302 assert resp.status_code == _expected_status, resp.location + if _expected_redirect: assert resp.location == _expected_redirect page = BeautifulSoup(resp.data.decode('utf-8'), 'html.parser') From fd05bed600335a9652f82d46f36cf68dc56bf0f6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 13 Jul 2020 09:34:49 +0100 Subject: [PATCH 4/5] Remove references to email, text and letters in trial mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don’t know what trial mode for broadcast services is going to look like, but it’s not going to involve sending emails. --- app/templates/views/service-settings.html | 26 ++++++++++-------- tests/app/main/views/test_service_settings.py | 27 +++++++++++++++++++ 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 9899de72c..09f253413 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -252,13 +252,15 @@ {% if current_service.trial_mode %}

Your service is in trial mode

-

You can only:

+ {% if not current_service.has_permission('broadcast') %} +

You can only:

-
    -
  • send {{ current_service.message_limit }} text messages and emails per day
    • -
  • send messages to yourself and other people in your team
    • -
  • create letter templates, but not send them
    • -
+
    +
  • send {{ current_service.message_limit }} text messages and emails per day
    • +
  • send messages to yourself and other people in your team
    • +
  • create letter templates, but not send them
    • +
+ {% endif %}

{% if current_user.has_permissions('manage_service') %} @@ -272,11 +274,13 @@ {% else %}

Your service is live

-

- You can send up to - {{ "{:,}".format(current_service.message_limit) }} messages - per day. -

+ {% if not current_service.has_permission('broadcast') %} +

+ You can send up to + {{ "{:,}".format(current_service.message_limit) }} messages + per day. +

+ {% endif %}

Problems or comments? Give feedback. diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 55e402004..0d830ee00 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -510,6 +510,7 @@ def test_service_name_change_fails_if_new_name_has_less_than_2_alphanumeric_char ]) def test_show_restricted_service( client_request, + service_one, single_reply_to_email_address, single_letter_contact_block, mock_get_organisation, @@ -539,6 +540,32 @@ def test_show_restricted_service( assert not request_to_live_link +def test_show_restricted_broadcast_service( + client_request, + service_one, + single_reply_to_email_address, + single_letter_contact_block, + mock_get_organisation, + single_sms_sender, + mock_get_service_settings_page_common, +): + service_one['permissions'] = 'broadcast' + page = client_request.get( + 'main.service_settings', + service_id=SERVICE_ONE_ID, + ) + + assert page.select('main h2')[0].text == 'Your service is in trial mode' + + request_to_live = page.select_one('main p') + request_to_live_link = request_to_live.select_one('a') + assert normalize_spaces(page.select_one('main p').text) == ( + 'To remove these restrictions, you can send us a request to go live.' + ) + assert request_to_live_link['href'] == url_for('main.request_to_go_live', service_id=SERVICE_ONE_ID) + assert not page.select_one('main ul') + + @freeze_time("2017-04-01 11:09:00.061258") def test_switch_service_to_live( client_request, From c20958ef348d927db3c6c823467fed41c5eefdac Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 14 Jul 2020 10:43:48 +0100 Subject: [PATCH 5/5] Clean up test formatting Ensures that: - test name reflects what it now does - only one parameter per line - argument order in parameterize matches argument order in function definition --- tests/app/main/views/test_service_settings.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 0d830ee00..d7a1b3da3 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -3677,7 +3677,8 @@ def test_unknown_channel_404s( 'It costs between 35p and 81p to send a letter using Notify.', 'Send letters', ['email', 'sms'], - 'False', 'True', + 'False', + 'True', ['email', 'sms', 'letter'], ), ( @@ -3685,7 +3686,8 @@ def test_unknown_channel_404s( 'It costs between 35p and 81p to send a letter using Notify.', 'Send letters', ['email', 'sms', 'letter'], - 'True', 'False', + 'True', + 'False', ['email', 'sms'], ), ( @@ -3693,7 +3695,8 @@ def test_unknown_channel_404s( 'You have a free allowance of 250,000 text messages each financial year.', 'Send text messages', [], - 'False', 'True', + 'False', + 'True', ['sms'], ), ( @@ -3701,7 +3704,8 @@ def test_unknown_channel_404s( 'It’s free to send emails through GOV.UK Notify.', 'Send emails', [], - 'False', 'True', + 'False', + 'True', ['email'], ), ( @@ -3709,11 +3713,12 @@ def test_unknown_channel_404s( 'It’s free to send emails through GOV.UK Notify.', 'Send emails', ['email', 'sms', 'letter'], - 'True', 'True', + 'True', + 'True', ['email', 'sms', 'letter'], ), ]) -def test_switch_service_enable_letters( +def test_switch_service_channels_on_and_off( client_request, service_one, mocker, @@ -3721,9 +3726,9 @@ def test_switch_service_enable_letters( channel, expected_first_para, expected_legend, + initial_permissions, expected_initial_value, posted_value, - initial_permissions, expected_updated_permissions, ): mocked_fn = mocker.patch('app.service_api_client.update_service', return_value=service_one)