Merge pull request #1024 from alphagov/add-letters-templates-2

Let users upload a CSV file of addresses against a letter template
2026-07-13 09:50:08 -04:00 · 2016-11-15 17:57:28 +01:00
parent 3e779675a7 a108165e47
commit 48496eefde
24 changed files with 252 additions and 76 deletions
--- a/app/assets/stylesheets/components/letter.scss
+++ b/app/assets/stylesheets/components/letter.scss
@@ -0,0 +1,13 @@
+$outline-width: 5px;
+
+.letter {
+  font-family: Helvetica, Arial, sans-serif;
+  box-shadow:
+  1px 1px 0 0 $panel-colour,
+  2px 2px 0 0 rgba($panel-colour, 0.5),
+  -1px 1px 0 0 $panel-colour,
+  -2px 2px 0 0 rgba($panel-colour, 0.5);
+  outline: $outline-width solid rgba($text-colour, 0.1);
+  padding: 20px;
+  margin: $outline-width $outline-width $gutter;
+}
--- a/app/assets/stylesheets/main.scss
+++ b/app/assets/stylesheets/main.scss
@@ -58,6 +58,7 @@ $path: '/static/images/';
@import 'components/research-mode';
@import 'components/tick-cross';
@import 'components/list-entry';
+@import 'components/letter';

@import 'views/job';
@import 'views/edit-template';
--- a/app/main/init.py
+++ b/app/main/init.py
@@ -26,6 +26,5 @@ from app.main.views import (
    invites,
    feedback,
    providers,
-    platform_admin,
-    letters
+    platform_admin
 )
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -276,6 +276,10 @@ class EmailTemplateForm(SMSTemplateForm):
        validators=[DataRequired(message="Can’t be empty")])


+class LetterTemplateForm(EmailTemplateForm):
+    pass
+
+
 class ForgotPasswordForm(Form):
    email_address = email_address(gov_user=False)

--- a/app/main/views/letters.py
+++ b/app/main/views/letters.py
@@ -1,14 +0,0 @@
-from flask import render_template, abort
-from flask_login import login_required
-
-from app import current_service
-from app.main import main
-from app.utils import user_has_permissions
-
-
-@main.route("/services/<service_id>/letters")
-@login_required
-def letters(service_id):
-    if not current_service['can_send_letters']:
-        abort(403)
-    return render_template('views/letters.html')
--- a/app/main/views/send.py
+++ b/app/main/views/send.py
@@ -17,8 +17,9 @@ from flask import (
 )

 from flask_login import login_required, current_user
+from notifications_utils.columns import Columns
 from notifications_utils.template import Template
-from notifications_utils.recipients import RecipientCSV, first_column_heading, validate_and_format_phone_number
+from notifications_utils.recipients import RecipientCSV, first_column_headings, validate_and_format_phone_number

 from app.main import main
 from app.main.forms import CsvUploadForm, ChooseTimeForm, get_next_days_until, get_furthest_possible_scheduled_time
@@ -33,7 +34,8 @@ from app.utils import user_has_permissions, get_errors_for_csv, Spreadsheet, get
 def get_page_headings(template_type):
    return {
        'email': 'Email templates',
-        'sms': 'Text message templates'
+        'sms': 'Text message templates',
+        'letter': 'Letter templates'
    }[template_type]


@@ -47,14 +49,27 @@ def get_example_csv_fields(column_headers, use_example_as_example, submitted_fie


 def get_example_csv_rows(template, use_example_as_example=True, submitted_fields=False):
-    return [
-        {
-            'email': 'test@example.com' if use_example_as_example else current_user.email_address,
-            'sms': '07700 900321' if use_example_as_example else validate_and_format_phone_number(
-                current_user.mobile_number, human_readable=True
+    return {
+        'email': ['test@example.com'] if use_example_as_example else [current_user.email_address],
+        'sms': ['07700 900321'] if use_example_as_example else [validate_and_format_phone_number(
+            current_user.mobile_number, human_readable=True
+        )],
+        'letter': [
+            (submitted_fields or {}).get(
+                key, get_example_letter_address(key) if use_example_as_example else key
            )
-        }[template.template_type]
-    ] + get_example_csv_fields(template.placeholders, use_example_as_example, submitted_fields)
+            for key in first_column_headings['letter']
+        ]
+    }[template.template_type] + get_example_csv_fields(template.placeholders, use_example_as_example, submitted_fields)
+
+
+def get_example_letter_address(key):
+    return {
+        'address line 1': 'A. Name',
+        'address line 2': '123 Example Street',
+        'address line 3': 'Example town',
+        'postcode': 'XM4 5HQ'
+    }.get(key, '')


@main.route("/services/<service_id>/send/<template_type>", methods=['GET'])
@@ -66,8 +81,10 @@ def get_example_csv_rows(template, use_example_as_example=True, submitted_fields
                      'manage_api_keys',
                      admin_override=True, any_=True)
 def choose_template(service_id, template_type):
-    if template_type not in ['email', 'sms']:
+    if template_type not in ['email', 'sms', 'letter']:
        abort(404)
+    if not current_service['can_send_letters'] and template_type == 'letter':
+        abort(403)
    return render_template(
        'views/templates/choose.html',
        templates=[
@@ -114,14 +131,13 @@ def send_messages(service_id, template_id):
                form.file.data.filename
            ))

+    column_headings = first_column_headings[template.template_type] + list(template.placeholders)
+
    return render_template(
        'views/send.html',
        template=template,
-        column_headings=list(ascii_uppercase[:len(template.placeholders) + 1]),
-        example=[
-            [first_column_heading[template.template_type]] + list(template.placeholders),
-            get_example_csv_rows(template)
-        ],
+        column_headings=list(ascii_uppercase[:len(column_headings)]),
+        example=[column_headings, get_example_csv_rows(template)],
        form=form
    )

@@ -132,7 +148,7 @@ def send_messages(service_id, template_id):
 def get_example_csv(service_id, template_id):
    template = Template(service_api_client.get_service_template(service_id, template_id)['data'])
    return Spreadsheet.from_rows([
-        [first_column_heading[template.template_type]] + list(template.placeholders),
+        first_column_headings[template.template_type] + list(template.placeholders),
        get_example_csv_rows(template)
    ]).as_csv_data, 200, {
        'Content-Type': 'text/csv; charset=utf-8',
@@ -159,7 +175,7 @@ def send_test(service_id, template_id):
            {
                'file_name': file_name,
                'data': Spreadsheet.from_rows([
-                    [first_column_heading[template.template_type]] + list(template.placeholders),
+                    first_column_headings[template.template_type] + list(template.placeholders),
                    get_example_csv_rows(template, use_example_as_example=False, submitted_fields=request.form)
                ]).as_csv_data
            },
@@ -181,7 +197,7 @@ def send_test(service_id, template_id):
    return render_template(
        'views/send-test.html',
        template=template,
-        recipient_column=first_column_heading[template.template_type],
+        recipient_columns=first_column_headings[template.template_type],
        example=[get_example_csv_rows(template, use_example_as_example=False)],
        help=get_help_argument()
    )
@@ -233,7 +249,7 @@ def check_messages(service_id, template_type, upload_id):
        max_initial_rows_shown=50,
        max_errors_shown=50,
        whitelist=itertools.chain.from_iterable(
-            [user.mobile_number, user.email_address] for user in users
+            [user.name, user.mobile_number, user.email_address] for user in users
        ) if current_service['restricted'] else None,
        remaining_messages=remaining_messages
    )
@@ -255,7 +271,10 @@ def check_messages(service_id, template_type, upload_id):

    with suppress(StopIteration):
        template.values = next(recipients.rows)
-        first_recipient = template.values.get(recipients.recipient_column_header, '')
+        first_recipient = template.values.get(
+            Columns.make_key(recipients.recipient_column_headers[0]),
+            ''
+        )

    session['upload_data']['notification_count'] = len(list(recipients.rows))
    session['upload_data']['valid'] = not recipients.has_errors
@@ -306,6 +325,14 @@ def start_job(service_id, upload_id):

    session.pop('upload_data')

+    template = service_api_client.get_service_template(
+        service_id,
+        upload_data.get('template_id')
+    )['data']
+
+    if template['template_type'] == 'letter':
+        abort(403)
+
    job_api_client.create_job(
        upload_id,
        service_id,
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -6,19 +6,20 @@ from flask_login import login_required
 from dateutil.parser import parse

 from notifications_utils.template import Template
-from notifications_utils.recipients import first_column_heading
+from notifications_utils.recipients import first_column_headings
 from notifications_python_client.errors import HTTPError

 from app.main import main
 from app.utils import user_has_permissions
-from app.main.forms import SMSTemplateForm, EmailTemplateForm
+from app.main.forms import SMSTemplateForm, EmailTemplateForm, LetterTemplateForm
 from app.main.views.send import get_example_csv_rows
 from app import service_api_client, current_service, template_statistics_client


 form_objects = {
    'email': EmailTemplateForm,
-    'sms': SMSTemplateForm
+    'sms': SMSTemplateForm,
+    'letter': LetterTemplateForm
 }

 page_headings = {
@@ -74,8 +75,10 @@ def view_template_version(service_id, template_id, version):
@login_required
@user_has_permissions('manage_templates', admin_override=True)
 def add_service_template(service_id, template_type):
-    if template_type not in ['sms', 'email']:
+    if template_type not in ['sms', 'email', 'letter']:
        abort(404)
+    if not current_service['can_send_letters'] and template_type == 'letter':
+        abort(403)

    form = form_objects[template_type]()
    if form.validate_on_submit():
@@ -133,7 +136,7 @@ def edit_service_template(service_id, template_id):
                new_template=new_template,
                column_headings=list(ascii_uppercase[:len(new_template.placeholders) + 1]),
                example_rows=[
-                    [first_column_heading[new_template.template_type]] + list(new_template.placeholders),
+                    first_column_headings[new_template.template_type] + list(new_template.placeholders),
                    get_example_csv_rows(new_template),
                    get_example_csv_rows(new_template)
                ],
--- a/app/templates/components/letter.html
+++ b/app/templates/components/letter.html
@@ -0,0 +1,7 @@
+{% macro letter(
+  body
+) %}
+  <div class="letter">
+    {{ body }}
+  </div>
+{% endmacro %}
--- a/app/templates/components/message-count-label.html
+++ b/app/templates/components/message-count-label.html
@@ -11,5 +11,11 @@
    {%- else -%}
      emails
    {%- endif -%}
+  {%- elif template_type == 'letter' -%}
+    {%- if count == 1 -%}
+      letter
+    {%- else -%}
+      letters
+    {%- endif -%}
  {%- endif %} {{ suffix }}
 {%- endmacro %}
--- a/app/templates/main_nav.html
+++ b/app/templates/main_nav.html
@@ -51,7 +51,7 @@
    <li><a href="{{ url_for('.choose_template', service_id=current_service.id, template_type='email') }}">Email templates</a></li>
    <li><a href="{{ url_for('.choose_template', service_id=current_service.id, template_type='sms') }}">Text message templates</a></li>
    {% if current_service.can_send_letters %}
-      <li><a href="{{ url_for('.letters', service_id=current_service.id) }}">Letter templates</a></li>
+      <li><a href="{{ url_for('.choose_template', service_id=current_service.id, template_type='letter') }}">Letter templates</a></li>
    {% endif %}
  {% endif %}
  {% if current_user.has_permissions(['manage_users', 'manage_settings'], admin_override=True) %}
--- a/app/templates/partials/check/too-many-messages.html
+++ b/app/templates/partials/check/too-many-messages.html
@@ -15,10 +15,24 @@
        You can still send {{ remaining_messages }} messages today, but
      {% endif %}
      ‘{{ original_file_name }}’ contains
-      {{ count_of_recipients }} {{ recipients.recipient_column_header }}
-      {%- if count_of_recipients != 1 -%}
-        {{ 'es' if 'email address' == recipients.recipient_column_header else 's' }}
-      {%- endif %}.
+      {{ count_of_recipients }}
+      {% if count_of_recipients == 1 -%}
+        {%- if template.template_type == 'email' -%}
+          email address
+        {%- elif template.template_type == 'sms' -%}
+          phone number
+        {%- elif template.template_type == 'letter' -%}
+          address
+        {%- endif -%}
+      {%- else -%}
+        {%- if template.template_type == 'email' -%}
+          email addresses
+        {%- elif template.template_type == 'sms' -%}
+          phone numbers
+        {%- elif template.template_type == 'letter' -%}
+          addresses
+        {%- endif -%}
+      {%- endif -%}.
    </p>
  {% endcall %}
 </div>
--- a/app/templates/partials/templates/guidance-formatting-letters.html
+++ b/app/templates/partials/templates/guidance-formatting-letters.html
@@ -0,0 +1,19 @@
+<h2 class="heading-medium">Formatting</h2>
+<p>
+  To put a title in your template, use a hash:
+</p>
+<div class="panel panel-border-wide">
+  <p>
+    # This is a title
+  </p>
+</div>
+<p>
+  To make bullet points, use asterisks:
+</p>
+<div class="panel panel-border-wide">
+  <p>
+    * point 1<br/>
+    * point 2<br/>
+    * point 3<br/>
+  </p>
+</div>
--- a/app/templates/views/check.html
+++ b/app/templates/views/check.html
@@ -3,6 +3,7 @@
 {% from "components/email-message.html" import email_message %}
 {% from "components/radios.html" import radio_select %}
 {% from "components/sms-message.html" import sms_message %}
+{% from "components/letter.html" import letter %}
 {% from "components/table.html" import list_table, field, text_field, index_field, hidden_field_heading %}
 {% from "components/file-upload.html" import file_upload %}
 {% from "components/page-footer.html" import page_footer %}
@@ -30,7 +31,7 @@
      {% endcall %}
    </div>

-  {% elif not recipients.has_recipient_column %}
+  {% elif not recipients.has_recipient_columns %}

    <div class="bottom-gutter">
      {% call banner_wrapper(type='dangerous') %}
@@ -148,7 +149,9 @@
        )}}
      </div>
    </div>
-  {% endif %}
+    {% elif 'letter' == template.template_type %}
+      {{ letter(template.formatted_as_markup if errors else template.replaced|safe) }}
+    {% endif %}

  {% if errors %}
    {% if request.args.from_test %}
@@ -190,7 +193,9 @@
          {{ item['columns'][column].data if item['columns'][column].data != None }}
        {% endcall %}
      {% else %}
-        {{ text_field(item['columns'][column].data) }}
+        {% call field() %}
+          {{ item['columns'][column].data if item['columns'][column].data != None }}
+        {% endcall %}
      {% endif %}
    {% endfor %}
    {% if item['columns'].get(None) %}
--- a/app/templates/views/edit-letter-template.html
+++ b/app/templates/views/edit-letter-template.html
@@ -0,0 +1,36 @@
+{% extends "withnav_template.html" %}
+{% from "components/textbox.html" import textbox %}
+{% from "components/page-footer.html" import page_footer %}
+
+{% block page_title %}
+  {{ heading_action }} letter template – GOV.UK Notify
+{% endblock %}
+
+{% block maincolumn_content %}
+
+    <h1 class="heading-large">
+      {{ heading_action }} letter template
+    </h1>
+
+    <form method="post">
+      <div class="grid-row">
+        <div class="column-three-quarters">
+          {{ textbox(form.name, width='1-1', hint='Your recipients won’t see this', rows=10) }}
+          {{ textbox(form.subject, width='1-1', highlight_tags=True, rows=2) }}
+        </div>
+        <div class="column-three-quarters">
+          {{ textbox(form.template_content, highlight_tags=True, width='1-1', rows=8) }}
+          {{ page_footer(
+            'Save',
+            delete_link=url_for('.delete_service_template', service_id=current_service.id, template_id=template_id) if template_id or None,
+            delete_link_text='Delete this template'
+          ) }}
+        </div>
+        <aside class="column-whole">
+          {% include "partials/templates/guidance-formatting-letters.html" %}
+          {% include "partials/templates/guidance-personalisation.html" %}
+        </aside>
+      </div>
+    </form>
+
+{% endblock %}
--- a/app/templates/views/letters.html
+++ b/app/templates/views/letters.html
@@ -1,19 +0,0 @@
-{% extends "withnav_template.html" %}
-{% from "components/sms-message.html" import sms_message %}
-{% from "components/email-message.html" import email_message %}
-{% from "components/page-footer.html" import page_footer %}
-{% from "components/file-upload.html" import file_upload %}
-{% from "components/table.html" import list_table, text_field, index_field, index_field_heading %}
-
-{% block page_title %}
-  Upload recipients – GOV.UK Notify
-{% endblock %}
-
-{% block maincolumn_content %}
-
-  <h1 class="heading-large">Letters 📩📨💌📮📜📑👌👌👌</h1>
-  <p>
-    [insert content here]
-  </p>
-
-{% endblock %}
--- a/app/templates/views/send-test.html
+++ b/app/templates/views/send-test.html
@@ -1,6 +1,7 @@
 {% extends "withnav_template.html" %}
 {% from "components/sms-message.html" import sms_message %}
 {% from "components/email-message.html" import email_message %}
+{% from "components/letter.html" import letter %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/file-upload.html" import file_upload %}
 {% from "components/table.html" import list_table, field, text_field, index_field, index_field_heading %}
@@ -38,17 +39,19 @@
      recipient='',
      show_placeholder_for_recipient=True
    ) }}
+  {% elif 'letter' == template.template_type %}
+    {{ letter(template.formatted_as_markup) }}
  {% endif %}

  <form method="post">
    {% call(item, row_number) list_table(
      example,
-      caption="Fill in the {}".format('field' if template.placeholders|length == 1 else 'fields'),
-      field_headings=[recipient_column] + template.placeholders|list
+      caption="Fill in the {}".format('field' if (recipient_columns + template.placeholders|list)|length == 2 else 'fields'),
+      field_headings=recipient_columns + template.placeholders|list
    ) %}
      {% for column in item %}
        {% call field() %}
-          {% if loop.index > 1 %}
+          {% if loop.index > 1 or template.template_type == 'letter' %}
            <label class="visuallyhidden" for="placeholder-field-{{ loop.index }}">{{ column }}</label>
            <input class="form-control form-control-1-1 " data-module="" name="{{ column }}" rows="8" type="text" value="" id="placeholder-field-{{ loop.index }}">
          {% else %}
--- a/app/templates/views/send.html
+++ b/app/templates/views/send.html
@@ -1,6 +1,7 @@
 {% extends "withnav_template.html" %}
 {% from "components/sms-message.html" import sms_message %}
 {% from "components/email-message.html" import email_message %}
+{% from "components/letter.html" import letter %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/file-upload.html" import file_upload %}
 {% from "components/table.html" import list_table, text_field, index_field, index_field_heading %}
@@ -30,6 +31,8 @@
      recipient='',
      show_placeholder_for_recipient=True
    ) }}
+  {% elif 'letter' == template.template_type %}
+    {{ letter(template.formatted_as_markup) }}
  {% endif %}

  <div class="page-footer bottom-gutter">
--- a/app/templates/views/templates/_template.html
+++ b/app/templates/views/templates/_template.html
@@ -1,5 +1,6 @@
 {% from "components/email-message.html" import email_message %}
 {% from "components/sms-message.html" import sms_message %}
+{% from "components/letter.html" import letter %}

 <div class="column-two-thirds">
  {% if 'email' == template.template_type %}
@@ -14,6 +15,8 @@
      template.formatted_as_markup,
      id=template.id
    ) }}
+  {% elif 'letter' == template.template_type %}
+    {{ letter(template.formatted_as_markup) }}
  {% endif %}
 </div>
 <div class="column-one-third">
--- a/app/templates/views/templates/template.html
+++ b/app/templates/views/templates/template.html
@@ -21,7 +21,9 @@

  {{ page_footer(
    secondary_link=url_for('.choose_template', service_id=current_service.id, template_type=template.template_type),
-    secondary_link_text='All {} templates'.format('email' if 'email' == template.template_type else 'text message')
+    secondary_link_text='All {} templates'.format(
+      'email' if 'email' == template.template_type else 'text message' if 'sms' == template.template_type else 'letter'
+    )
  ) }}

 {% endblock %}
--- a/app/utils.py
+++ b/app/utils.py
@@ -86,6 +86,11 @@ def get_errors_for_csv(recipients, template_type):
                errors.append("fix 1 email address")
            else:
                errors.append("fix {} email addresses".format(number_of_bad_recipients))
+        elif 'letter' == template_type:
+            if 1 == number_of_bad_recipients:
+                errors.append("fix 1 address")
+            else:
+                errors.append("fix {} addresses".format(number_of_bad_recipients))

    if recipients.rows_with_missing_data:
        number_of_rows_with_missing_data = len(list(recipients.rows_with_missing_data))
--- a/requirements.txt
+++ b/requirements.txt
@@ -18,4 +18,4 @@ pytz==2016.4

 git+https://github.com/alphagov/notifications-python-client.git@1.3.0#egg=notifications-python-client==1.3.0

-git+https://github.com/alphagov/notifications-utils.git@9.1.1#egg=notifications-utils==9.1.1
+git+https://github.com/alphagov/notifications-utils.git@10.0.0#egg=notifications-utils==10.0.0
--- a/tests/app/main/views/test_letters.py
+++ b/tests/app/main/views/test_letters.py
@@ -1,34 +1,51 @@
 import pytest
 from flask import url_for
+from functools import partial

 from tests import service_json

+letters_urls = [
+    partial(url_for, 'main.choose_template', template_type='letter'),
+    partial(url_for, 'main.add_service_template', template_type='letter'),
+]

+
+@pytest.mark.parametrize('url', letters_urls)
@pytest.mark.parametrize('can_send_letters, response_code', [
    (True, 200),
    (False, 403)
 ])
-def test_letters_access_restricted(logged_in_client, mocker, can_send_letters, response_code):
+def test_letters_access_restricted(
+    logged_in_client,
+    mocker,
+    can_send_letters,
+    response_code,
+    mock_get_service_templates,
+    url
+):
    service = service_json(can_send_letters=can_send_letters)
    mocker.patch('app.service_api_client.get_service', return_value={"data": service})

-    response = logged_in_client.get(url_for('main.letters', service_id=service['id']))
+    response = logged_in_client.get(url(service_id=service['id']))

    assert response.status_code == response_code


+@pytest.mark.parametrize('url', letters_urls)
 def test_letters_lets_in_without_permission(
    client,
    mocker,
    mock_login,
    mock_has_permissions,
    api_user_active,
+    mock_get_service_templates,
+    url
 ):
    service = service_json(can_send_letters=True)
    mocker.patch('app.service_api_client.get_service', return_value={"data": service})

    client.login(api_user_active)
-    response = client.get(url_for('main.letters', service_id=service['id']))
+    response = client.get(url(service_id=service['id']))

    assert api_user_active.permissions == {}
    assert response.status_code == 200
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -405,6 +405,33 @@ def test_create_job_should_call_api(
    )


+def test_cant_start_letters_job(
+    app_,
+    client,
+    service_one,
+    mock_get_service,
+    active_user_with_permissions,
+    mock_create_job,
+    mock_get_service_letter_template,
+    mocker,
+    fake_uuid
+):
+    client.login(active_user_with_permissions, mocker, service_one)
+    with client.session_transaction() as session:
+        session['upload_data'] = {
+            'original_file_name': 'example.csv',
+            'template_id': fake_uuid,
+            'notification_count': 123,
+            'valid': True
+        }
+    response = client.post(
+        url_for('main.start_job', service_id=fake_uuid, upload_id=fake_uuid),
+        data={}
+    )
+    assert response.status_code == 403
+    mock_create_job.assert_not_called()
+
+
 def test_check_messages_should_revalidate_file_when_uploading_file(
    app_,
    service_one,
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -335,6 +335,21 @@ def mock_get_service_email_template(mocker):
        'app.service_api_client.get_service_template', side_effect=_create)


+@pytest.fixture(scope='function')
+def mock_get_service_letter_template(mocker):
+    def _create(service_id, template_id):
+        template = template_json(
+            service_id,
+            template_id,
+            "Two week reminder",
+            "letter",
+            "Your vehicle tax is about to expire", "Subject")
+        return {'data': template}
+
+    return mocker.patch(
+        'app.service_api_client.get_service_template', side_effect=_create)
+
+
@pytest.fixture(scope='function')
 def mock_create_service_template(mocker, fake_uuid):
    def _create(name, type_, content, service, subject=None):