Merge pull request #1803 from alphagov/self-serve-letters

Allows services to choose if they can send letters

app/main/forms.py

@@ -584,6 +584,17 @@ class ServiceLetterContactBlockForm(StripWhitespaceForm):
             )
 
 
+class ServiceSwitchLettersForm(StripWhitespaceForm):
+
+    enabled = RadioField(
+        'Send letters',
+        choices=[
+            ('on', 'On'),
+            ('off', 'Off'),
+        ],
+    )
+
+
 class ServiceBrandingOrg(StripWhitespaceForm):
 
     def __init__(self, organisations=[], *args, **kwargs):

app/main/views/service_settings.py

@@ -35,6 +35,7 @@ from app.main.forms import (
     FreeSMSAllowance,
     ServiceEditInboundNumberForm,
     SMSPrefixForm,
+    ServiceSwitchLettersForm,
 )
 from app import user_api_client, current_service, organisations_client, inbound_number_client, billing_api_client
 from notifications_utils.formatters import formatted_list
@@ -252,14 +253,6 @@ def update_service_permissions(service_id, permissions, sms_sender=None):
     service_api_client.update_service_with_properties(service_id, data)
 
 
-@main.route("/services/<service_id>/service-settings/can-send-letters")
-@login_required
-@user_has_permissions(admin_override=True)
-def service_switch_can_send_letters(service_id):
-    switch_service_permissions(service_id, 'letter')
-    return redirect(url_for('.service_settings', service_id=service_id))
-
-
 @main.route("/services/<service_id>/service-settings/send-letters-as-pdf")
 @login_required
 @user_has_permissions(admin_override=True)
@@ -268,14 +261,6 @@ def service_switch_send_letters_as_pdf(service_id):
     return redirect(url_for('.service_settings', service_id=service_id))
 
 
-@main.route("/services/<service_id>/service-settings/can-send-international-sms")
-@login_required
-@user_has_permissions(admin_override=True)
-def service_switch_can_send_international_sms(service_id):
-    switch_service_permissions(service_id, 'international_sms')
-    return redirect(url_for('.service_settings', service_id=service_id))
-
-
 @main.route("/services/<service_id>/service-settings/can-send-email")
 @login_required
 @user_has_permissions(admin_override=True)
@@ -502,12 +487,25 @@ def service_set_inbound_sms(service_id):
     )
 
 
-@main.route("/services/<service_id>/service-settings/set-letters", methods=['GET'])
+@main.route("/services/<service_id>/service-settings/set-letters", methods=['GET', 'POST'])
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)
 def service_set_letters(service_id):
+    form = ServiceSwitchLettersForm(
+        enabled='on' if 'letter' in current_service['permissions'] else 'off'
+    )
+    if form.validate_on_submit():
+        force_service_permission(
+            service_id,
+            'letter',
+            on=(form.enabled.data == 'on'),
+        )
+        return redirect(
+            url_for(".service_settings", service_id=service_id)
+        )
     return render_template(
         'views/service-settings/set-letters.html',
+        form=form,
     )
 
 

app/templates/views/pricing.html

@@ -87,7 +87,7 @@
 
     </details>
 
-    <h2 class="heading-medium">Letters</h2>
+    <h2 class="heading-medium" id="letters">Letters</h2>
     <p>The cost of sending a letter depends on how many sheets of paper you need.</p>
     <div class="bottom-gutter-3-2">
       {% call mapping_table(

app/templates/views/service-settings.html

@@ -243,11 +243,6 @@
             {{ 'Stop sending emails' if 'email' in current_service.permissions else 'Allow to send emails' }}
           </a>
         </li>
-        <li class="bottom-gutter">
-          <a href="{{ url_for('.service_switch_can_send_letters', service_id=current_service.id) }}" class="button">
-            {{ 'Stop sending letters' if 'letter' in current_service.permissions else 'Allow to send letters' }}
-          </a>
-        </li>
         {% if 'letter' in current_service.permissions %}
           <li class="bottom-gutter">
             <a href="{{ url_for('.service_switch_send_letters_as_pdf', service_id=current_service.id) }}" class="button">

app/templates/views/service-settings/set-letters.html

@@ -1,34 +1,26 @@
 {% extends "withnav_template.html" %}
-{% from "components/textbox.html" import textbox %}
+{% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}
-  Text message sender
+  Send letters
 {% endblock %}
 
 {% block maincolumn_content %}
 
   <div class="grid-row">
     <div class="column-five-sixths">
-      <h1 class="heading-large">Letters</h1>
+      <h1 class="heading-large">Send letters</h1>
-      {% if 'letter' in current_service.permissions %} 
+      <p>
-        <p>
+        It costs between 30p and 45p to send a letter using Notify.
-          Your service can send letters.
+      </p>
-        </p>
+      <p>
-        <p>
+        See <a href="{{ url_for(".pricing", _anchor="letters") }}">pricing</a> for the list
-          If you want to stop your service from sending letters,
+        of rates.
-          <a href="{{ url_for('.support') }}">get in touch with the GOV.UK Notify team</a>.
+      </p>
-        </p>
+      {{ radios(form.enabled) }}
-      {% else %}
-        <p>
-          Using GOV.UK Notify to send letters is an invitation&#8209;only feature.
-        </p>
-        <p>
-          If you want to try it out,
-          <a href="{{ url_for('.support') }}">get in touch with the GOV.UK Notify team</a>.
-        </p>
-      {% endif %}
       {{ page_footer(
+        'Save',
         back_link=url_for('.service_settings', service_id=current_service.id),
         back_link_text='Back to settings'
       ) }}

tests/app/main/views/service_settings/test_service_setting_permissions.py

@@ -31,9 +31,6 @@ def get_service_settings_page(
     ({'permissions': ['email']}, '.service_switch_can_send_email', {}, 'Stop sending emails'),
     ({'permissions': []}, '.service_switch_can_send_email', {}, 'Allow to send emails'),
 
-    ({'permissions': ['letter']}, '.service_switch_can_send_letters', {}, 'Stop sending letters'),
-    ({'permissions': []}, '.service_switch_can_send_letters', {}, 'Allow to send letters'),
-
     ({'permissions': ['sms']}, '.service_switch_can_send_sms', {}, 'Stop sending sms'),
     ({'permissions': []}, '.service_switch_can_send_sms', {}, 'Allow to send sms'),
 

tests/app/main/views/test_service_settings.py

@@ -530,8 +530,6 @@ def test_route_permissions(
     'main.service_request_to_go_live',
     'main.service_switch_live',
     'main.service_switch_research_mode',
-    'main.service_switch_can_send_letters',
-    'main.service_switch_can_send_international_sms',
     'main.archive_service',
 ])
 def test_route_invalid_permissions(
@@ -584,8 +582,6 @@ def test_route_for_platform_admin(
 @pytest.mark.parametrize('route', [
     'main.service_switch_live',
     'main.service_switch_research_mode',
-    'main.service_switch_can_send_letters',
-    'main.service_switch_can_send_international_sms',
 ])
 def test_route_for_platform_admin_update_service(
         mocker,
@@ -1557,40 +1553,50 @@ def test_should_set_sms_allowance(
     )
 
 
+@pytest.mark.parametrize((
+    'expected_initial_value,'
+    'posted_value,'
+    'initial_permissions,'
+    'expected_updated_permissions'
+), [
+    ('off', 'on', ['email', 'sms'], ['email', 'sms', 'letter']),
+    ('on', 'off', ['email', 'sms', 'letter'], ['email', 'sms']),
+])
 def test_switch_service_enable_letters(
-    logged_in_platform_admin_client,
+    client_request,
     service_one,
     mocker,
+    expected_initial_value,
+    posted_value,
+    initial_permissions,
+    expected_updated_permissions,
 ):
     mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one)
+    service_one['permissions'] = initial_permissions
 
-    response = logged_in_platform_admin_client.get(
+    page = client_request.get(
-        url_for('main.service_switch_can_send_letters', service_id=service_one['id'])
+        'main.service_set_letters',
+        service_id=service_one['id'],
     )
 
-    assert response.status_code == 302
+    assert page.select_one('input[checked]')['value'] == expected_initial_value
-    assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
+    assert len(page.select('input[checked]')) == 1
-    assert 'letter' in mocked_fn.call_args[0][1]['permissions']
+
+    client_request.post(
+        'main.service_set_letters',
+        service_id=service_one['id'],
+        _data={'enabled': posted_value},
+        _expected_redirect=url_for(
+            'main.service_settings',
+            service_id=service_one['id'],
+            _external=True
+        )
+    )
+
+    assert set(mocked_fn.call_args[0][1]['permissions']) == set(expected_updated_permissions)
     assert mocked_fn.call_args[0][0] == service_one['id']
 
 
-def test_switch_service_disable_letters(
-    logged_in_platform_admin_client,
-    service_one,
-    mocker,
-):
-    service_one['permissions'] = ['letter']
-    mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one)
-
-    response = logged_in_platform_admin_client.get(
-        url_for('main.service_switch_can_send_letters', 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)
-    assert mocked_fn.call_args == call(service_one['id'], {"permissions": []})
-
-
 @pytest.mark.parametrize('permissions, expected_checked', [
     (['international_sms'], 'on'),
     ([''], 'off'),
@@ -1802,21 +1808,6 @@ def test_cant_resume_active_service(
 
 
 @pytest.mark.parametrize('endpoint, permissions, expected_p', [
-    (
-        'main.service_set_letters',
-        [],
-        (
-            'Using GOV.UK Notify to send letters is an invitation‑only '
-            'feature.'
-        )
-    ),
-    (
-        'main.service_set_letters',
-        ['letter'],
-        (
-            'Your service can send letters.'
-        )
-    ),
     (
         'main.service_set_inbound_sms',
         ['sms'],