Merge pull request #1168 from alphagov/contact-letter-block

add letter_contact_block edit fields

app/__init__.py

@@ -27,7 +27,7 @@ from flask_wtf import CsrfProtect
 from functools import partial
 
 from notifications_python_client.errors import HTTPError
-from notifications_utils import logging, request_id
+from notifications_utils import logging, request_id, formatters
 from notifications_utils.clients.statsd.statsd_client import StatsdClient
 from notifications_utils.recipients import validate_phone_number, InvalidPhoneError
 from pygments import highlight
@@ -134,6 +134,7 @@ def create_app():
     application.add_template_filter(format_notification_status_as_field_status)
     application.add_template_filter(format_notification_status_as_url)
     application.add_template_filter(formatted_list)
+    application.add_template_filter(nl2br)
 
     application.after_request(useful_headers_after_request)
     application.after_request(save_service_after_request)
@@ -375,6 +376,10 @@ def formatted_list(
         ).format(**locals())
 
 
+def nl2br(value):
+    return formatters.nl2br(value) if value else ''
+
+
 @login_manager.user_loader
 def load_user(user_id):
     return user_api_client.get_user(user_id)

app/main/forms.py

@@ -486,10 +486,23 @@ class ServiceSmsSender(Form):
     )
 
     def validate_sms_sender(form, field):
-        if field.data and not re.match('^[a-zA-Z0-9\s]+$', field.data):
+        if field.data and not re.match(r'^[a-zA-Z0-9\s]+$', field.data):
             raise ValidationError('Use letters and numbers only')
 
 
+class ServiceLetterContactBlock(Form):
+    letter_contact_block = TextAreaField(
+        'How should users contact you?'
+    )
+
+    def validate_letter_contact_block(form, field):
+        line_count = field.data.strip().count('\n')
+        if line_count >= 10:
+            raise ValidationError(
+                'Contains {} lines, maximum is 10'.format(line_count + 1)
+            )
+
+
 class ServiceBrandingOrg(Form):
 
     def __init__(self, organisations=[], *args, **kwargs):

app/main/views/service_settings.py

@@ -25,6 +25,7 @@ from app.main.forms import (
     RequestToGoLiveForm,
     ServiceReplyToEmailFrom,
     ServiceSmsSender,
+    ServiceLetterContactBlock,
     ServiceBrandingOrg
 )
 from app import user_api_client, current_service, organisations_client
@@ -236,6 +237,7 @@ def service_set_reply_to_email(service_id):
             reply_to_email_address=form.email_address.data
         )
         return redirect(url_for('.service_settings', service_id=service_id))
+
     return render_template(
         'views/service-settings/set-reply-to-email.html',
         form=form)
@@ -259,6 +261,27 @@ def service_set_sms_sender(service_id):
         form=form)
 
 
+@main.route("/services/<service_id>/service-settings/set-letter-contact-block", methods=['GET', 'POST'])
+@login_required
+@user_has_permissions('manage_settings', admin_override=True)
+def service_set_letter_contact_block(service_id):
+
+    if not current_service['can_send_letters']:
+        abort(403)
+
+    form = ServiceLetterContactBlock(letter_contact_block=current_service['letter_contact_block'])
+    if form.validate_on_submit():
+        service_api_client.update_service(
+            current_service['id'],
+            letter_contact_block=form.letter_contact_block.data.replace('\r', '') or None
+        )
+        return redirect(url_for('.service_settings', service_id=service_id))
+    return render_template(
+        'views/service-settings/set-letter-contact-block.html',
+        form=form
+    )
+
+
 @main.route("/services/<service_id>/service-settings/set-branding-and-org", methods=['GET', 'POST'])
 @login_required
 @user_has_permissions(admin_override=True)

app/main/views/templates.py

@@ -84,6 +84,7 @@ def view_letter_template_as_pdf(service_id, template_id):
     return render_pdf(HTML(string=str(
         LetterPreviewTemplate(
             service_api_client.get_service_template(service_id, template_id)['data'],
+            contact_block=current_service['letter_contact_block'],
         )
     )))
 
@@ -144,6 +145,7 @@ def view_template_version_as_pdf(service_id, template_id, version):
     return render_pdf(HTML(string=str(
         LetterPreviewTemplate(
             service_api_client.get_service_template(service_id, template_id, version=version)['data'],
+            contact_block=current_service['letter_contact_block'],
         )
     )))
 

app/notify_client/service_api_client.py

@@ -90,7 +90,8 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             'sms_sender',
             'created_by',
             'branding',
-            'organisation'
+            'organisation',
+            'letter_contact_block'
         }
         if disallowed_attributes:
             raise TypeError('Not allowed to update service attributes: {}'.format(

app/templates/components/textbox.html

@@ -30,7 +30,9 @@
       {% endif %}
     </label>
     {{ field(**{
-      'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''),
+      'class':
+        'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else
+        'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''),
       'data-module': 'highlight-tags' if highlight_tags else '',
       'rows': rows|string
     }) }}

app/templates/partials/templates/guidance-contact-block.html

@@ -0,0 +1,8 @@
+<h2 class="heading-medium">Contact details</h2>
+<p>
+  Add contact details for your service in
+  <a href="{{ url_for('.service_set_letter_contact_block', service_id=current_service.id) }}">settings</a>.
+</p>
+<p>
+  The text will appear in the top right of all letters your service sends.
+</p>

app/templates/views/edit-letter-template.html

@@ -26,9 +26,10 @@
             delete_link_text='Delete this template'
           ) }}
         </div>
-        <aside class="column-whole">
+        <aside class="column-three-quarters">
           {% include "partials/templates/guidance-formatting-letters.html" %}
           {% include "partials/templates/guidance-personalisation.html" %}
+          {% include "partials/templates/guidance-contact-block.html" %}
         </aside>
       </div>
     </form>

app/templates/views/service-settings.html

@@ -20,23 +20,35 @@
         caption_visible=False
       ) %}
         {% call row() %}
-          {{ text_field('Service name' )}}
+          {{ text_field('Service name') }}
           {{ text_field(current_service.name) }}
           {{ edit_field('Change', url_for('.service_name_change', service_id=current_service.id)) }}
         {% endcall %}
+
         {% call row() %}
-          {{ text_field('Email reply to address')}}
+          {{ text_field('Email reply to address') }}
           {{ text_field(
             current_service.reply_to_email_address,
             status='' if current_service.reply_to_email_address else 'default'
           ) }}
           {{ edit_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }}
         {% endcall %}
+
         {% call row() %}
-          {{ text_field('Text message sender')}}
+          {{ text_field('Text message sender') }}
           {{ text_field(current_service.sms_sender or '40604') }}
           {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }}
         {% endcall %}
+
+        {% if current_service.can_send_letters %}
+          {% call row() %}
+            {{ text_field('Letter contact details') }}
+            {% call field(status='' if current_service.letter_contact_block else 'default') %}
+              {{ current_service.letter_contact_block | escape | nl2br | safe }}
+            {% endcall %}
+            {{ edit_field('Change', url_for('.service_set_letter_contact_block', service_id=current_service.id)) }}
+          {% endcall %}
+        {% endif %}
       {% endcall %}
     </div>
 

app/templates/views/service-settings/set-letter-contact-block.html

@@ -0,0 +1,29 @@
+{% extends "withnav_template.html" %}
+{% from "components/textbox.html" import textbox %}
+{% from "components/page-footer.html" import page_footer %}
+
+{% block service_page_title %}
+  Letter contact block
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-large">
+    Letter contact details
+  </h1>
+  <div class="grid-row">
+    <form method="post" class="column-half">
+      {{ textbox(
+        form.letter_contact_block,
+        width='1-1',
+        rows=10
+      ) }}
+      {{ page_footer(
+        'Save',
+        back_link=url_for('.service_settings', service_id=current_service.id),
+        back_link_text='Back to settings'
+      ) }}
+    </form>
+  </div>
+
+{% endblock %}

app/utils.py

@@ -312,7 +312,8 @@ def get_template(
             )
         else:
             return LetterPreviewTemplate(
-                template
+                template,
+                contact_block=service['letter_contact_block'],
             )
 
 

tests/__init__.py

@@ -52,7 +52,8 @@ def service_json(
     can_send_letters=False,
     organisation=None,
     branding='govuk',
-    created_at=None
+    created_at=None,
+    letter_contact_block=None
 ):
     if users is None:
         users = []
@@ -70,7 +71,8 @@ def service_json(
         'can_send_letters': can_send_letters,
         'organisation': organisation,
         'branding': branding,
-        'created_at': created_at or str(datetime.utcnow())
+        'created_at': created_at or str(datetime.utcnow()),
+        'letter_contact_block': letter_contact_block
     }
 
 

tests/app/main/views/test_service_settings.py

@@ -49,6 +49,49 @@ def test_should_show_overview_for_service_with_more_things_set(
         assert row == " ".join(page.find_all('tr')[index + 1].text.split())
 
 
+def test_if_cant_send_letters_then_cant_see_letter_contact_block(
+    logged_in_client,
+    service_one,
+):
+    response = logged_in_client.get(url_for(
+        'main.service_settings', service_id=service_one['id']
+    ))
+    assert 'Letter contact block' not in response.get_data(as_text=True)
+
+
+def test_letter_contact_block_shows_None_if_not_set(
+    logged_in_client,
+    service_one,
+    mocker
+):
+    service_one['can_send_letters'] = True
+    response = logged_in_client.get(url_for(
+        'main.service_settings', service_id=service_one['id']
+    ))
+
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    div = page.find_all('tr')[4].find_all('td')[1].div
+    assert div.text.strip() == 'None'
+    assert 'default' in div.attrs['class'][0]
+
+
+def test_escapes_letter_contact_block(
+    logged_in_client,
+    service_one,
+    mocker,
+):
+    service_one['can_send_letters'] = True
+    service_one['letter_contact_block'] = 'foo\nbar<script>alert(1);</script>'
+    response = logged_in_client.get(url_for(
+        'main.service_settings', service_id=service_one['id']
+    ))
+
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    div = str(page.find_all('tr')[4].find_all('td')[1].div)
+    assert 'foo<br>bar' in div
+    assert '<script>' not in div
+
+
 def test_should_show_service_name(
     logged_in_client,
     service_one,
@@ -486,7 +529,6 @@ def test_shows_research_mode_indicator(
     mocker,
 ):
     service_one['research_mode'] = True
-    mocker.patch('app.service_api_client.get_service', return_value={"data": service_one})
     mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one)
 
     response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id']))
@@ -538,6 +580,61 @@ def test_if_sms_sender_set_then_form_populated(
     assert page.find(id='sms_sender')['value'] == 'elevenchars'
 
 
+@pytest.mark.parametrize('method', ['get', 'post'])
+def test_cant_set_letter_contact_block_if_service_cant_send_letters(
+    logged_in_client,
+    service_one,
+    method
+):
+    assert not service_one['can_send_letters']
+    response = getattr(logged_in_client, method)(
+        url_for('main.service_set_letter_contact_block', service_id=service_one['id'])
+    )
+    assert response.status_code == 403
+
+
+def test_set_letter_contact_block_prepopulates(
+    logged_in_client,
+    service_one
+):
+    service_one['can_send_letters'] = True
+    service_one['letter_contact_block'] = 'foo bar baz waz'
+    response = logged_in_client.get(url_for('main.service_set_letter_contact_block', service_id=service_one['id']))
+    assert response.status_code == 200
+    assert 'foo bar baz waz' in response.get_data(as_text=True)
+
+
+def test_set_letter_contact_block_saves(
+    logged_in_client,
+    service_one,
+    mock_update_service,
+):
+    service_one['can_send_letters'] = True
+    response = logged_in_client.post(
+        url_for('main.service_set_letter_contact_block', service_id=service_one['id']),
+        data={'letter_contact_block': 'foo bar baz waz'}
+    )
+    assert response.status_code == 302
+    assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
+    mock_update_service.assert_called_once_with(service_one['id'], letter_contact_block='foo bar baz waz')
+
+
+def test_set_letter_contact_block_has_max_10_lines(
+    logged_in_client,
+    service_one,
+    mock_update_service,
+):
+    service_one['can_send_letters'] = True
+    response = logged_in_client.post(
+        url_for('main.service_set_letter_contact_block', service_id=service_one['id']),
+        data={'letter_contact_block': '\n'.join(map(str, range(0, 11)))}
+    )
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    error_message = page.find('span', class_='error-message').text.strip()
+    assert error_message == 'Contains 11 lines, maximum is 10'
+
+
 def test_should_show_branding(
     logged_in_platform_admin_client,
     service_one,
@@ -632,7 +729,6 @@ def test_switch_service_disable_letters(
     mocker,
 ):
     service_one['can_send_letters'] = True
-    mocker.patch('app.service_api_client.get_service', return_value={"data": service_one})
     mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one)
 
     response = logged_in_platform_admin_client.get(