From 2d55bb7ae28bd5468bcacddf6f068483d831fd58 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 8 Feb 2016 16:36:53 +0000
Subject: [PATCH 1/5] Use `Template` to replace/highlight placeholders
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit brings in the `Template` util, added here:
https://github.com/alphagov/notifications-utils/pull/1

It also does a fair bit of tidying up, which I’ve unfortunately squashed into
this one massive commit. The main change is moving 404 handling into the
templates dao, so that every view isn’t littered with `try: … except(HTTPError)`.

It also adds new features, in a prototypy sort of way, which are:
- download a prefilled example CSV
- show all the columns for your template on the 'check' page
---
 app/__init__.py                             | 22 -----
 app/main/dao/templates_dao.py               | 23 +++++-
 app/main/views/jobs.py                      |  6 +-
 app/main/views/sms.py                       | 89 +++++++++++++--------
 app/main/views/templates.py                 | 33 +++-----
 app/templates/components/email-message.html |  4 +-
 app/templates/components/sms-message.html   |  2 +-
 app/templates/views/check-sms.html          |  8 +-
 app/templates/views/job.html                |  2 +-
 app/templates/views/manage-templates.html   | 18 ++---
 app/templates/views/send-sms.html           | 16 ++++
 requirements.txt                            |  2 +-
 tests/app/main/views/test_sms.py            |  3 +-
 13 files changed, 127 insertions(+), 101 deletions(-)

diff --git a/app/__init__.py b/app/__init__.py
index e85140659..c0c431c0b 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -62,8 +62,6 @@ def create_app(config_name, config_overrides=None):
 
     application.session_interface = ItsdangerousSessionInterface()
 
-    application.add_template_filter(placeholders)
-    application.add_template_filter(replace_placeholders)
     application.add_template_filter(nl2br)
     application.add_template_filter(format_datetime)
     application.add_template_filter(syntax_highlight_json)
@@ -123,16 +121,6 @@ def convert_to_boolean(value):
     return value
 
 
-def placeholders(value):
-    if not value:
-        return value
-    return Markup(re.sub(
-        r"\(\(([^\)]+)\)\)",  # anything that looks like ((registration number))
-        lambda match: "<span class='placeholder'>{}</span>".format(match.group(1)),
-        value
-    ))
-
-
 def nl2br(value):
     _paragraph_re = re.compile(r'(?:\r\n|\r|\n){2,}')
 
@@ -141,16 +129,6 @@ def nl2br(value):
     return Markup(result)
 
 
-def replace_placeholders(template, values):
-    if not template:
-        return template
-    return Markup(re.sub(
-        r"\(\(([^\)]+)\)\)",  # anything that looks like ((registration number))
-        lambda match: values.get(match.group(1), ''),
-        template
-    ))
-
-
 def syntax_highlight_json(code):
     return Markup(highlight(code, JavascriptLexer(), HtmlFormatter(noclasses=True)))
 
diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py
index 72d47bcd2..4eb05ed9d 100644
--- a/app/main/dao/templates_dao.py
+++ b/app/main/dao/templates_dao.py
@@ -1,4 +1,4 @@
-from flask import url_for
+from flask import url_for, abort
 from app import notifications_api_client
 from app.main.utils import BrowsableItem
 
@@ -17,9 +17,24 @@ def get_service_templates(service_id):
     return notifications_api_client.get_service_templates(service_id)
 
 
-def get_service_template(service_id, template_id):
-    return notifications_api_client.get_service_template(
-        service_id, template_id)
+def get_service_templates_or_404(service_id):
+    try:
+        get_service_templates(service_id)
+    except HTTPError as e:
+        if e.status_code == 404:
+            abort(404)
+        else:
+            raise e
+
+
+def get_service_template_or_404(service_id, template_id):
+    try:
+        return notifications_api_client.get_service_template(service_id, template_id)
+    except HTTPError as e:
+        if e.status_code == 404:
+            abort(404)
+        else:
+            raise e
 
 
 def delete_service_template(service_id, template_id):
diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py
index d19e29cd3..dd5e45e04 100644
--- a/app/main/views/jobs.py
+++ b/app/main/views/jobs.py
@@ -8,6 +8,7 @@ from flask import (
 )
 from flask_login import login_required
 from notifications_python_client.errors import HTTPError
+from utils.template import Template
 
 from app import job_api_client
 from app.main import main
@@ -38,7 +39,6 @@ def view_jobs(service_id):
 def view_job(service_id, job_id):
     try:
         job = job_api_client.get_job(service_id, job_id)['data']
-        template = templates_dao.get_service_template(service_id, job['template'])['data']
         messages = []
         return render_template(
             'views/job.html',
@@ -55,7 +55,9 @@ def view_job(service_id, job_id):
             cost=u'£0.00',
             uploaded_file_name=job['original_file_name'],
             uploaded_file_time=job['created_at'],
-            template=template,
+            template=Template(
+                templates_dao.get_service_template_or_404(service_id, job['template'])['data']
+            ),
             service_id=service_id
         )
     except HTTPError as e:
diff --git a/app/main/views/sms.py b/app/main/views/sms.py
index c7c7f75c1..94a5b19f6 100644
--- a/app/main/views/sms.py
+++ b/app/main/views/sms.py
@@ -1,6 +1,8 @@
 import csv
 import uuid
 import botocore
+import re
+import io
 
 from datetime import date
 
@@ -12,12 +14,14 @@ from flask import (
     flash,
     abort,
     session,
-    current_app
+    current_app,
+    Markup
 )
 
 from flask_login import login_required
 from werkzeug import secure_filename
 from notifications_python_client.errors import HTTPError
+from utils.template import Template
 
 from app.main import main
 from app.main.forms import CsvUploadForm
@@ -35,22 +39,19 @@ from app.main.utils import (
 
 @main.route("/services/<service_id>/sms/send", methods=['GET'])
 def choose_sms_template(service_id):
-    try:
-        templates = templates_dao.get_service_templates(service_id)['data']
-    except HTTPError as e:
-        if e.status_code == 404:
-            abort(404)
-        else:
-            raise e
-
-    return render_template('views/choose-sms-template.html',
-                           templates=templates,
-                           service_id=service_id)
+    return render_template(
+        'views/choose-sms-template.html',
+        templates=[
+            Template(template) for template in templates_dao.get_service_templates(service_id)['data']
+        ],
+        service_id=service_id
+    )
 
 
 @main.route("/services/<service_id>/sms/send/<template_id>", methods=['GET', 'POST'])
 @login_required
 def send_sms(service_id, template_id):
+
     form = CsvUploadForm()
     if form.validate_on_submit():
         try:
@@ -63,24 +64,33 @@ def send_sms(service_id, template_id):
                                     service_id=service_id,
                                     upload_id=upload_id))
         except ValueError as e:
-            message = 'There was a problem uploading: {}'.format(
-                      csv_file.filename)
-            flash(message)
+            flash('There was a problem uploading: {}'.format(csv_file.filename))
             flash(str(e))
             return redirect(url_for('.send_sms', service_id=service_id, template_id=template_id))
 
-    try:
-        template = templates_dao.get_service_template(service_id, template_id)['data']
-    except HTTPError as e:
-        if e.status_code == 404:
-            abort(404)
-        else:
-            raise e
+    template = Template(
+       templates_dao.get_service_template_or_404(service_id, template_id)['data']
+    )
 
-    return render_template('views/send-sms.html',
-                           template=template,
-                           form=form,
-                           service_id=service_id)
+    return render_template(
+        'views/send-sms.html',
+        template=template,
+        column_headers=['phone number'] + template.placeholders_as_markup,
+        form=form,
+        service_id=service_id
+    )
+
+
+@main.route("/services/<service_id>/sms/send/<template_id>.csv", methods=['GET'])
+@login_required
+def get_example_csv(service_id, template_id):
+    template = templates_dao.get_service_template_or_404(service_id, template_id)['data']
+    output = io.StringIO()
+    csv.writer(output).writerow(
+        ['phone number'] + Template(template).list_placeholders
+    )
+
+    return(output.getvalue(), 200, {'Content-Type': 'text/csv; charset=utf-8'})
 
 
 @main.route("/services/<service_id>/sms/check/<upload_id>",
@@ -89,21 +99,23 @@ def send_sms(service_id, template_id):
 def check_sms(service_id, upload_id):
 
     if request.method == 'GET':
-
         contents = s3download(service_id, upload_id)
         if not contents:
             flash('There was a problem reading your upload file')
         upload_result = _get_numbers(contents)
         upload_data = session['upload_data']
-        original_file_name = upload_data.get('original_file_name')
         template_id = upload_data.get('template_id')
-        template = templates_dao.get_service_template(service_id, template_id)['data']
+        template = Template(
+            templates_dao.get_service_template_or_404(service_id, template_id)['data'],
+            values=upload_result['valid'][0] if upload_result['valid'] else {},
+            drop_values={'phone'}
+        )
         return render_template(
             'views/check-sms.html',
             upload_result=upload_result,
-            message_template=template['content'],
-            original_file_name=original_file_name,
-            template_id=template_id,
+            template=template,
+            column_headers=['phone number'] + template.placeholders_as_markup,
+            original_file_name=upload_data.get('original_file_name'),
             service_id=service_id
         )
     elif request.method == 'POST':
@@ -153,3 +165,16 @@ def _get_numbers(contents):
         except InvalidPhoneError:
             rejects.append({"line_number": i+2, "phone": row['phone']})
     return {"valid": valid, "rejects": rejects}
+
+
+def _get_column_headers(template_content, marked_up=False):
+    headers = re.findall(
+        r"\(\(([^\)]+)\)\)",  # anything that looks like ((registration number))
+        template_content
+    )
+    if marked_up:
+        return [
+            Markup("<span class='placeholder'>{}</span>".format(header))
+            for header in headers
+        ]
+    return headers
diff --git a/app/main/views/templates.py b/app/main/views/templates.py
index 33d93e93a..0b3f492ff 100644
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -1,13 +1,15 @@
 from flask import request, render_template, redirect, url_for, flash, abort
 from flask_login import login_required
 
+from notifications_python_client.errors import HTTPError
+from utils.template import Template
+
 from app.main import main
 from app.main.forms import TemplateForm
 from app import job_api_client
 from app.main.dao.services_dao import get_service_by_id
 from app.main.dao import templates_dao as tdao
 from app.main.dao import services_dao as sdao
-from notifications_python_client.errors import HTTPError
 
 
 @main.route("/services/<service_id>/templates")
@@ -15,7 +17,6 @@ from notifications_python_client.errors import HTTPError
 def manage_service_templates(service_id):
     try:
         jobs = job_api_client.get_job(service_id)['data']
-        templates = tdao.get_service_templates(service_id)['data']
     except HTTPError as e:
         if e.status_code == 404:
             abort(404)
@@ -25,7 +26,11 @@ def manage_service_templates(service_id):
         'views/manage-templates.html',
         service_id=service_id,
         has_jobs=bool(jobs),
-        templates=[tdao.TemplatesBrowsableItem(x) for x in templates])
+        templates=[
+            Template(template)
+            for template in tdao.get_service_templates(service_id)['data']
+        ]
+    )
 
 
 @main.route("/services/<service_id>/templates/add", methods=['GET', 'POST'])
@@ -56,14 +61,7 @@ def add_service_template(service_id):
 @main.route("/services/<service_id>/templates/<int:template_id>", methods=['GET', 'POST'])
 @login_required
 def edit_service_template(service_id, template_id):
-    try:
-        template = tdao.get_service_template(service_id, template_id)['data']
-    except HTTPError as e:
-        if e.status_code == 404:
-            abort(404)
-        else:
-            raise e
-
+    template = tdao.get_service_template_or_404(service_id, template_id)['data']
     template['template_content'] = template['content']
     form = TemplateForm(**template)
 
@@ -84,21 +82,14 @@ def edit_service_template(service_id, template_id):
 @main.route("/services/<service_id>/templates/<int:template_id>/delete", methods=['GET', 'POST'])
 @login_required
 def delete_service_template(service_id, template_id):
-    try:
-        template = tdao.get_service_template(service_id, template_id)['data']
-    except HTTPError as e:
-        if e.status_code == 404:
-            abort(404)
-        else:
-            raise e
-
-    template['template_content'] = template['content']
-    form = TemplateForm(**template)
+    template = tdao.get_service_template_or_404(service_id, template_id)['data']
 
     if request.method == 'POST':
         tdao.delete_service_template(service_id, template_id)
         return redirect(url_for('.manage_service_templates', service_id=service_id))
 
+    template['template_content'] = template['content']
+    form = TemplateForm(**template)
     flash('Are you sure you want to delete ‘{}’?'.format(form.name.data), 'delete')
     return render_template(
         'views/edit-template.html',
diff --git a/app/templates/components/email-message.html b/app/templates/components/email-message.html
index 2a8f65eb8..6d9920121 100644
--- a/app/templates/components/email-message.html
+++ b/app/templates/components/email-message.html
@@ -10,10 +10,10 @@
   {% endif %}
   <div class="email-message">
     <div class="email-message-subject">
-      {{ subject|placeholders }}
+      {{ subject }}
     </div>
     <div class="email-message-body">
-      {{ body|nl2br|placeholders }}
+      {{ body|nl2br }}
     </div>
   </div>
 {% endmacro %}
diff --git a/app/templates/components/sms-message.html b/app/templates/components/sms-message.html
index 044f3f5c7..490254485 100644
--- a/app/templates/components/sms-message.html
+++ b/app/templates/components/sms-message.html
@@ -11,7 +11,7 @@
     </h3>
   {% endif %}
   <div class="sms-message-wrapper{% if input_name %}-with-radio{% endif %}">
-    {{ body|placeholders }}
+    {{ body }}
   </div>
   {% if recipient %}
     <p class="sms-message-recipient">
diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html
index 2a61a83f2..d4b761075 100644
--- a/app/templates/views/check-sms.html
+++ b/app/templates/views/check-sms.html
@@ -17,14 +17,14 @@
     {% for rejected in upload_result.rejects %}
       <p>Line {{rejected.line_number}}: {{rejected.phone }}</a>
     {% endfor %}
-    <p><a href="{{url_for('.send_sms', service_id=service_id, template_id=template_id)}}" class="button">Go back and resolve errors</a></p>
+    <p><a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="button">Go back and resolve errors</a></p>
 
   {% else %}
 
     <div class="grid-row">
       <div class="column-two-thirds">
         {{ sms_message(
-          message_template|replace_placeholders(upload_result.valid[0])
+          template.replaced
         )}}
       </div>
     </div>
@@ -32,13 +32,13 @@
     <form method="POST" enctype="multipart/form-data">
       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
       <input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.valid|count, '' if upload_result.valid|count == 1 else 's') }}" />
-      <a href="{{url_for('.send_sms', service_id=service_id, template_id=template_id)}}" class="page-footer-back-link">Back</a>
+      <a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
     </form>
 
     {% call(item) list_table(
       upload_result.valid,
       caption=original_file_name,
-      field_headings=['Phone number']
+      field_headings=column_headers
     ) %}
       {% call field() %}
         {{ item.phone }}
diff --git a/app/templates/views/job.html b/app/templates/views/job.html
index c72e6d34f..40af13526 100644
--- a/app/templates/views/job.html
+++ b/app/templates/views/job.html
@@ -17,7 +17,7 @@
     <div class="grid-row">
       <div class="column-two-thirds">
         {{ sms_message(
-          template['content'],
+          template.formatted_as_markup,
         )}}
       </div>
     </div>
diff --git a/app/templates/views/manage-templates.html b/app/templates/views/manage-templates.html
index 2d3da827e..346c87739 100644
--- a/app/templates/views/manage-templates.html
+++ b/app/templates/views/manage-templates.html
@@ -26,19 +26,19 @@ Manage templates – GOV.UK Notify
       <div class="column-two-thirds">
 
         {% for template in templates %}
-          {% if template.get_field('template_type') == 'sms' %}
-            {{ sms_message(
-              template.get_field('content'),
-              name=template.title,
-              id=template.get_field('id'),
-              edit_link=url_for('.edit_service_template', service_id=template.get_field('service'), template_id=template.get_field('id'))
-            ) }}
-          {% elif template.get_field('template_type') == 'email' %}
+          {% if template.template_type == 'email' %}
             {{ email_message(
               template.get_field('subject'),
               template.get_field('content'),
               name=template.get_field('name'),
-              edit_link=url_for('.edit_service_template', service_id=template.get_field('service'), template_id=template.get_field('id'))
+              edit_link=url_for('.edit_service_template', service_id=service_id, template_id=template.id)
+            ) }}
+          {% else %}
+            {{ sms_message(
+              template.formatted_as_markup,
+              name=template.name,
+              id=template.id,
+              edit_link=url_for('.edit_service_template', service_id=service_id, template_id=template.id)
             ) }}
           {% endif %}
         {% endfor %}
diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html
index 66f7491ee..c43c52265 100644
--- a/app/templates/views/send-sms.html
+++ b/app/templates/views/send-sms.html
@@ -2,6 +2,7 @@
 {% from "components/sms-message.html" import sms_message %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/file-upload.html" import file_upload %}
+{% from "components/table.html" import list_table %}
 
 {% block page_title %}
   Send text messages – GOV.UK Notify
@@ -32,5 +33,20 @@
       "Continue to preview"
     ) }}
 
+    {% if column_headers %}
+      {% call(item) list_table(
+        [],
+        caption='Preview',
+        field_headings=column_headers,
+        field_headings_visible=True,
+        caption_visible=False,
+        empty_message="Your data here"
+      ) %}
+      {% endcall %}
+    {% endif %}
+    <p class="table-show-more-link">
+      <a href="{{ url_for('.get_example_csv', service_id=service_id, template_id=template.id) }}">Download this CSV file</a>
+    </p>
+
   </form>
 {% endblock %}
diff --git a/requirements.txt b/requirements.txt
index 2c5fed09d..631ea6965 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -14,4 +14,4 @@ Pygments==2.0.2
 
 git+https://github.com/alphagov/notifications-python-client.git@0.2.5#egg=notifications-python-client==0.2.5
 
-git+https://github.com/alphagov/notifications-utils.git@0.0.3#egg=notifications-utils==0.0.3
+git+https://github.com/alphagov/notifications-utils.git@0.1.0#egg=notifications-utils==0.1.0
diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py
index 9f4c4c158..57af2f99e 100644
--- a/tests/app/main/views/test_sms.py
+++ b/tests/app/main/views/test_sms.py
@@ -8,9 +8,8 @@ def test_choose_sms_template(app_,
                              api_user_active,
                              mock_login,
                              mock_get_user,
-                             mock_get_service_templates,
                              mock_check_verify_code,
-                             mock_get_service_template):
+                             mock_get_service_templates):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)

From efb2140bbbd862b6ace38b3220a5eafd94e478e8 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 17 Feb 2016 14:20:55 +0000
Subject: [PATCH 2/5] Check CSV files match the template
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit adds a first stab at checking whether a CSV file has the right
data to fill the placeholders.

The UI is very much first bash, but I’d like to get this merged and see how it
feels. The main thing is that we’ve got all the bit in place now to do this
logic.
---
 app/assets/stylesheets/components/table.scss |  7 +++
 app/main/views/sms.py                        | 26 +++++++---
 app/templates/views/check-sms.html           | 51 +++++++++++++++++---
 app/templates/views/choose-sms-template.html |  2 +-
 app/templates/views/send-sms.html            | 14 ++++--
 5 files changed, 80 insertions(+), 20 deletions(-)

diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss
index 650509f24..1b40c5fa8 100644
--- a/app/assets/stylesheets/components/table.scss
+++ b/app/assets/stylesheets/components/table.scss
@@ -36,6 +36,13 @@
 
     }
 
+    &-missing {
+      color: $error-colour;
+      font-weight: bold;
+      border-left: 5px solid $error-colour;
+      padding-left: 7px;
+    }
+
   }
 
 }
diff --git a/app/main/views/sms.py b/app/main/views/sms.py
index 94a5b19f6..d191063ff 100644
--- a/app/main/views/sms.py
+++ b/app/main/views/sms.py
@@ -14,11 +14,10 @@ from flask import (
     flash,
     abort,
     session,
-    current_app,
-    Markup
+    current_app
 )
 
-from flask_login import login_required
+from flask_login import login_required, current_user
 from werkzeug import secure_filename
 from notifications_python_client.errors import HTTPError
 from utils.template import Template
@@ -72,10 +71,19 @@ def send_sms(service_id, template_id):
        templates_dao.get_service_template_or_404(service_id, template_id)['data']
     )
 
+    example_data = [dict(
+        phone=current_user.mobile_number,
+        **{
+            header: "test {}".format(header) for header in template.placeholders
+        }
+    )]
+
     return render_template(
         'views/send-sms.html',
         template=template,
-        column_headers=['phone number'] + template.placeholders_as_markup,
+        column_headers=['phone'] + template.placeholders_as_markup,
+        placeholders=template.placeholders,
+        example_data=example_data,
         form=form,
         service_id=service_id
     )
@@ -85,10 +93,11 @@ def send_sms(service_id, template_id):
 @login_required
 def get_example_csv(service_id, template_id):
     template = templates_dao.get_service_template_or_404(service_id, template_id)['data']
+    placeholders = list(Template(template).placeholders)
     output = io.StringIO()
-    csv.writer(output).writerow(
-        ['phone number'] + Template(template).list_placeholders
-    )
+    writer = csv.writer(output)
+    writer.writerow(['phone'] + placeholders)
+    writer.writerow([current_user.mobile_number] + ["test {}".format(header) for header in placeholders])
 
     return(output.getvalue(), 200, {'Content-Type': 'text/csv; charset=utf-8'})
 
@@ -116,7 +125,8 @@ def check_sms(service_id, upload_id):
             template=template,
             column_headers=['phone number'] + template.placeholders_as_markup,
             original_file_name=upload_data.get('original_file_name'),
-            service_id=service_id
+            service_id=service_id,
+            form=CsvUploadForm()
         )
     elif request.method == 'POST':
         upload_data = session['upload_data']
diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html
index d4b761075..87656b2af 100644
--- a/app/templates/views/check-sms.html
+++ b/app/templates/views/check-sms.html
@@ -2,6 +2,7 @@
 {% from "components/sms-message.html" import sms_message %}
 {% from "components/table.html" import list_table, field %}
 {% from "components/placeholder.html" import placeholder %}
+{% from "components/file-upload.html" import file_upload %}
 {% from "components/page-footer.html" import page_footer %}
 
 {% block page_title %}
@@ -23,17 +24,40 @@
 
     <div class="grid-row">
       <div class="column-two-thirds">
-        {{ sms_message(
-          template.replaced
-        )}}
+        {% if template.missing_data or template.additional_data %}
+          {{ sms_message(template.formatted_as_markup)}}
+        {% else %}
+          {{ sms_message(template.replaced)}}
+        {% endif %}
       </div>
     </div>
 
-    <form method="POST" enctype="multipart/form-data">
-      <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
-      <input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.valid|count, '' if upload_result.valid|count == 1 else 's') }}" />
-      <a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
-    </form>
+    {% if template.missing_data %}
+      {{ banner(
+        "Add these columns to your CSV file: " + ", ".join(template.missing_data),
+        type="dangerous"
+      ) }}
+    {% elif template.additional_data %}
+      {{ banner(
+        "Remove these columns from your CSV file:" + ", ".join(template.missing_data),
+        type="dangerous"
+      ) }}
+    {% else %}
+      <form method="POST" enctype="multipart/form-data">
+        <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
+        <input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.valid|count, '' if upload_result.valid|count == 1 else 's') }}" />
+        <a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
+      </form>
+    {% endif %}
+
+    {% if template.missing_data or template.additional_data %}
+      <form method="post" action="{{ url_for('.send_sms', service_id=service_id, template_id=template.id) }}" enctype="multipart/form-data">
+        {{file_upload(form.file, button_text='Choose a CSV file')}}
+        {{ page_footer(
+          "Upload"
+        ) }}
+      </form>
+    {% endif %}
 
     {% call(item) list_table(
       upload_result.valid,
@@ -43,6 +67,17 @@
       {% call field() %}
         {{ item.phone }}
       {% endcall %}
+      {% for column in template.placeholders %}
+        {% if item.get(column) %}
+          {% call field() %}
+            {{ item.get(column) }}
+          {% endcall %}
+        {% else %}
+          {% call field(status='missing') %}
+            missing
+          {% endcall %}
+        {% endif %}
+      {% endfor %}
     {% endcall %}
 
   {% endif %}
diff --git a/app/templates/views/choose-sms-template.html b/app/templates/views/choose-sms-template.html
index e12ed204f..383d34834 100644
--- a/app/templates/views/choose-sms-template.html
+++ b/app/templates/views/choose-sms-template.html
@@ -17,7 +17,7 @@
       <div class="grid-row">
         {% for template in templates %}
           <div class="column-two-thirds">
-            {{ sms_message(template.content, name=template.name) }}
+            {{ sms_message(template.formatted_as_markup, name=template.name) }}
           </div>
           <div class="column-one-third">
             <div class="sms-message-use-link">
diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html
index c43c52265..74c01d724 100644
--- a/app/templates/views/send-sms.html
+++ b/app/templates/views/send-sms.html
@@ -2,7 +2,7 @@
 {% from "components/sms-message.html" import sms_message %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/file-upload.html" import file_upload %}
-{% from "components/table.html" import list_table %}
+{% from "components/table.html" import list_table, field %}
 
 {% block page_title %}
   Send text messages – GOV.UK Notify
@@ -16,7 +16,7 @@
 
     <div class="grid-row">
       <div class="column-two-thirds">
-        {{ sms_message(template.content) }}
+        {{ sms_message(template.formatted_as_markup) }}
       </div>
     </div>
 
@@ -35,13 +35,21 @@
 
     {% if column_headers %}
       {% call(item) list_table(
-        [],
+        example_data,
         caption='Preview',
         field_headings=column_headers,
         field_headings_visible=True,
         caption_visible=False,
         empty_message="Your data here"
       ) %}
+        {% call field() %}
+          {{ item.phone }}
+        {% endcall %}
+        {% for column in template.placeholders %}
+          {% call field() %}
+            {{ item.get(column) }}
+          {% endcall %}
+        {% endfor %}
       {% endcall %}
     {% endif %}
     <p class="table-show-more-link">

From 45cacd82d3a941ec33c3e360310bb35ac2bac8d4 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 17 Feb 2016 15:49:07 +0000
Subject: [PATCH 3/5] Validate CSVs fully
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit extends the existing function to validate each row’s phone number
to also validate that all the required data is present.

It does this using the checking that the `Template` class can do when given
a template and a `dict` of values.
---
 app/__init__.py                               |  10 ++
 .../stylesheets/components/page-footer.scss   |   2 +-
 app/main/dao/services_dao.py                  |   2 +-
 app/main/dao/templates_dao.py                 |   2 +-
 app/main/forms.py                             |   2 +-
 app/main/views/sms.py                         |  56 ++++-----
 app/templates/views/check-sms.html            | 117 +++++++++---------
 app/templates/views/send-sms.html             |  11 +-
 app/{main => }/utils.py                       |   3 -
 tests/app/main/test_phone_validation.py       |   2 +-
 tests/app/main/views/test_sms.py              |   4 +-
 11 files changed, 98 insertions(+), 113 deletions(-)
 rename app/{main => }/utils.py (98%)

diff --git a/app/__init__.py b/app/__init__.py
index c0c431c0b..b28fc9adc 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -19,6 +19,7 @@ from app.notify_client.job_api_client import JobApiClient
 from app.notify_client.status_api_client import StatusApiClient
 from app.its_dangerous_session import ItsdangerousSessionInterface
 from app.asset_fingerprinter import AssetFingerprinter
+from app.utils import validate_phone_number, InvalidPhoneError
 import app.proxy_fix
 from config import configs
 from utils import logging
@@ -65,6 +66,7 @@ def create_app(config_name, config_overrides=None):
     application.add_template_filter(nl2br)
     application.add_template_filter(format_datetime)
     application.add_template_filter(syntax_highlight_json)
+    application.add_template_filter(valid_phone_number)
 
     application.after_request(useful_headers_after_request)
     register_errorhandlers(application)
@@ -139,6 +141,14 @@ def format_datetime(date):
     return native.strftime('%A %d %B %Y at %H:%M')
 
 
+def valid_phone_number(phone_number):
+    try:
+        validate_phone_number(phone_number)
+        return True
+    except InvalidPhoneError:
+        return False
+
+
 # https://www.owasp.org/index.php/List_of_useful_HTTP_headers
 def useful_headers_after_request(response):
     response.headers.add('X-Frame-Options', 'deny')
diff --git a/app/assets/stylesheets/components/page-footer.scss b/app/assets/stylesheets/components/page-footer.scss
index 1df6405f1..7dfe8ece0 100644
--- a/app/assets/stylesheets/components/page-footer.scss
+++ b/app/assets/stylesheets/components/page-footer.scss
@@ -1,6 +1,6 @@
 .page-footer {
 
-  margin-bottom: 50px;
+  margin-bottom: 30px;
 
   &-back-link {
     @include button($grey-1);
diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py
index 5c6fd25dc..52c217679 100644
--- a/app/main/dao/services_dao.py
+++ b/app/main/dao/services_dao.py
@@ -1,6 +1,6 @@
 from flask import url_for
 from app import notifications_api_client
-from app.main.utils import BrowsableItem
+from app.utils import BrowsableItem
 
 
 def insert_new_service(service_name, user_id):
diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py
index 4eb05ed9d..032c45fbf 100644
--- a/app/main/dao/templates_dao.py
+++ b/app/main/dao/templates_dao.py
@@ -1,6 +1,6 @@
 from flask import url_for, abort
 from app import notifications_api_client
-from app.main.utils import BrowsableItem
+from app.utils import BrowsableItem
 
 
 def insert_service_template(name, type_, content, service_id):
diff --git a/app/main/forms.py b/app/main/forms.py
index d97784863..cf1aa76c8 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -14,7 +14,7 @@ from wtforms.validators import DataRequired, Email, Length, Regexp
 
 from app.main.validators import Blacklist, CsvFileValidator
 
-from app.main.utils import (
+from app.utils import (
     validate_phone_number,
     format_phone_number,
     InvalidPhoneError
diff --git a/app/main/views/sms.py b/app/main/views/sms.py
index d191063ff..2e4154af3 100644
--- a/app/main/views/sms.py
+++ b/app/main/views/sms.py
@@ -20,7 +20,7 @@ from flask import (
 from flask_login import login_required, current_user
 from werkzeug import secure_filename
 from notifications_python_client.errors import HTTPError
-from utils.template import Template
+from utils.template import Template, NeededByTemplateError, NoPlaceholderForDataError
 
 from app.main import main
 from app.main.forms import CsvUploadForm
@@ -30,7 +30,7 @@ from app.main.uploader import (
 )
 from app.main.dao import templates_dao
 from app import job_api_client
-from app.main.utils import (
+from app.utils import (
     validate_phone_number,
     InvalidPhoneError
 )
@@ -68,7 +68,7 @@ def send_sms(service_id, template_id):
             return redirect(url_for('.send_sms', service_id=service_id, template_id=template_id))
 
     template = Template(
-       templates_dao.get_service_template_or_404(service_id, template_id)['data']
+        templates_dao.get_service_template_or_404(service_id, template_id)['data']
     )
 
     example_data = [dict(
@@ -111,19 +111,22 @@ def check_sms(service_id, upload_id):
         contents = s3download(service_id, upload_id)
         if not contents:
             flash('There was a problem reading your upload file')
-        upload_result = _get_numbers(contents)
         upload_data = session['upload_data']
         template_id = upload_data.get('template_id')
+        raw_template = templates_dao.get_service_template_or_404(service_id, template_id)['data']
+        upload_result = _get_rows(contents, raw_template)
         template = Template(
-            templates_dao.get_service_template_or_404(service_id, template_id)['data'],
-            values=upload_result['valid'][0] if upload_result['valid'] else {},
+            raw_template,
+            values=upload_result['rows'][0] if upload_result['valid'] else {},
             drop_values={'phone'}
         )
         return render_template(
             'views/check-sms.html',
             upload_result=upload_result,
             template=template,
-            column_headers=['phone number'] + template.placeholders_as_markup,
+            column_headers=['phone number'] + list(
+                template.placeholders if upload_result['valid'] else template.placeholders_as_markup
+            ),
             original_file_name=upload_data.get('original_file_name'),
             service_id=service_id,
             form=CsvUploadForm()
@@ -155,36 +158,19 @@ def _get_filedata(file):
     return {'file_name': file.filename, 'data': lines}
 
 
-def _format_filename(filename):
-    d = date.today()
-    basename, extenstion = filename.split('.')
-    formatted_name = '{}_{}.csv'.format(basename, d.strftime('%Y%m%d'))
-    return secure_filename(formatted_name)
-
-
-def _get_numbers(contents):
+def _get_rows(contents, raw_template):
     reader = csv.DictReader(
         contents.split('\n'),
         lineterminator='\n',
-        quoting=csv.QUOTE_NONE)
-    valid, rejects = [], []
-    for i, row in enumerate(reader):
+        quoting=csv.QUOTE_NONE
+    )
+    valid = True
+    rows = []
+    for row in reader:
+        rows.append(row)
         try:
             validate_phone_number(row['phone'])
-            valid.append(row)
-        except InvalidPhoneError:
-            rejects.append({"line_number": i+2, "phone": row['phone']})
-    return {"valid": valid, "rejects": rejects}
-
-
-def _get_column_headers(template_content, marked_up=False):
-    headers = re.findall(
-        r"\(\(([^\)]+)\)\)",  # anything that looks like ((registration number))
-        template_content
-    )
-    if marked_up:
-        return [
-            Markup("<span class='placeholder'>{}</span>".format(header))
-            for header in headers
-        ]
-    return headers
+            Template(raw_template, values=row, drop_values={'phone'}).replaced
+        except (InvalidPhoneError, NeededByTemplateError, NoPlaceholderForDataError):
+            valid = False
+    return {"valid": valid, "rows": rows}
diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html
index 87656b2af..c41c4759f 100644
--- a/app/templates/views/check-sms.html
+++ b/app/templates/views/check-sms.html
@@ -11,74 +11,73 @@
 
 {% block maincolumn_content %}
 
-  <h1 class="heading-large">Check and confirm</h1>
 
-  {% if upload_result.rejects %}
-    <h3 class="heading-small">The following numbers are invalid</h3>
-    {% for rejected in upload_result.rejects %}
-      <p>Line {{rejected.line_number}}: {{rejected.phone }}</a>
-    {% endfor %}
-    <p><a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="button">Go back and resolve errors</a></p>
+  {% if template.additional_data %}
+    {{ banner(
+      "Remove these columns from your CSV file:" + ", ".join(template.missing_data),
+      type="dangerous"
+    ) }}
+  {% elif not upload_result.valid %}
+    {{ banner(
+      "Your CSV file contained missing or invalid data",
+      type="dangerous"
+    ) }}
+  {% endif %}
 
-  {% else %}
+  <h1 class="heading-large">
+    {{ "Check and confirm" if upload_result.valid else "Send text messages" }}
+  </h1>
 
-    <div class="grid-row">
-      <div class="column-two-thirds">
-        {% if template.missing_data or template.additional_data %}
-          {{ sms_message(template.formatted_as_markup)}}
-        {% else %}
-          {{ sms_message(template.replaced)}}
-        {% endif %}
-      </div>
+  <div class="grid-row">
+    <div class="column-two-thirds">
+      {% if template.missing_data or template.additional_data %}
+        {{ sms_message(template.formatted_as_markup)}}
+      {% else %}
+        {{ sms_message(template.replaced)}}
+      {% endif %}
     </div>
+  </div>
 
-    {% if template.missing_data %}
-      {{ banner(
-        "Add these columns to your CSV file: " + ", ".join(template.missing_data),
-        type="dangerous"
+  {% if upload_result.valid %}
+    <form method="post" enctype="multipart/form-data">
+      <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
+      <input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.rows|count, '' if upload_result.rows|count == 1 else 's') }}" />
+      <a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
+    </form>
+  {% else %}
+    <form method="post" action="{{ url_for('.send_sms', service_id=service_id, template_id=template.id) }}" enctype="multipart/form-data">
+      {{file_upload(form.file, button_text='Choose a CSV file')}}
+      {{ page_footer(
+        "Upload"
       ) }}
-    {% elif template.additional_data %}
-      {{ banner(
-        "Remove these columns from your CSV file:" + ", ".join(template.missing_data),
-        type="dangerous"
-      ) }}
-    {% else %}
-      <form method="POST" enctype="multipart/form-data">
-        <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
-        <input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.valid|count, '' if upload_result.valid|count == 1 else 's') }}" />
-        <a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
-      </form>
-    {% endif %}
+    </form>
+  {% endif %}
 
-    {% if template.missing_data or template.additional_data %}
-      <form method="post" action="{{ url_for('.send_sms', service_id=service_id, template_id=template.id) }}" enctype="multipart/form-data">
-        {{file_upload(form.file, button_text='Choose a CSV file')}}
-        {{ page_footer(
-          "Upload"
-        ) }}
-      </form>
-    {% endif %}
-
-    {% call(item) list_table(
-      upload_result.valid,
-      caption=original_file_name,
-      field_headings=column_headers
-    ) %}
+  {% call(item) list_table(
+    upload_result.rows,
+    caption=original_file_name,
+    field_headings=column_headers
+  ) %}
+    {% if item.phone|valid_phone_number %}
       {% call field() %}
         {{ item.phone }}
       {% endcall %}
-      {% for column in template.placeholders %}
-        {% if item.get(column) %}
-          {% call field() %}
-            {{ item.get(column) }}
-          {% endcall %}
-        {% else %}
-          {% call field(status='missing') %}
-            missing
-          {% endcall %}
-        {% endif %}
-      {% endfor %}
-    {% endcall %}
+    {% else %}
+      {% call field(status='missing') %}
+        {{ item.phone }}
+      {% endcall %}
+    {% endif %}
+    {% for column in template.placeholders %}
+      {% if item.get(column) %}
+        {% call field() %}
+          {{ item.get(column) }}
+        {% endcall %}
+      {% else %}
+        {% call field(status='missing') %}
+          missing
+        {% endcall %}
+      {% endif %}
+    {% endfor %}
+  {% endcall %}
 
-  {% endif %}
 {% endblock %}
diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html
index 74c01d724..15671cc60 100644
--- a/app/templates/views/send-sms.html
+++ b/app/templates/views/send-sms.html
@@ -22,13 +22,6 @@
 
     {{file_upload(form.file, button_text='Choose a CSV file')}}
 
-    {{ banner(
-      'You can only send messages to yourself until you <a href="{}">request to go live</a>'.format(
-        url_for('.service_request_to_go_live', service_id=service_id)
-      )|safe,
-      type='important'
-    ) }}
-
     {{ page_footer(
       "Continue to preview"
     ) }}
@@ -36,10 +29,10 @@
     {% if column_headers %}
       {% call(item) list_table(
         example_data,
-        caption='Preview',
+        caption='Example',
         field_headings=column_headers,
         field_headings_visible=True,
-        caption_visible=False,
+        caption_visible=True,
         empty_message="Your data here"
       ) %}
         {% call field() %}
diff --git a/app/main/utils.py b/app/utils.py
similarity index 98%
rename from app/main/utils.py
rename to app/utils.py
index 9b023c91d..88a127a23 100644
--- a/app/main/utils.py
+++ b/app/utils.py
@@ -1,6 +1,3 @@
-from flask import current_app
-
-
 class BrowsableItem(object):
     """
     Maps for the template browse-list.
diff --git a/tests/app/main/test_phone_validation.py b/tests/app/main/test_phone_validation.py
index f1b54c5fb..62595d5da 100644
--- a/tests/app/main/test_phone_validation.py
+++ b/tests/app/main/test_phone_validation.py
@@ -1,4 +1,4 @@
-from app.main.utils import (
+from app.utils import (
     validate_phone_number,
     InvalidPhoneError
 )
diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py
index 57af2f99e..d3609afb4 100644
--- a/tests/app/main/views/test_sms.py
+++ b/tests/app/main/views/test_sms.py
@@ -63,10 +63,10 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(app_,
                                    follow_redirects=True)
         assert response.status_code == 200
         content = response.get_data(as_text=True)
-        assert 'The following numbers are invalid' in content
+        assert 'Your CSV file contained missing or invalid data' in content
         assert '+44 123' in content
         assert '+44 456' in content
-        assert 'Go back and resolve errors' in content
+        assert 'Choose a CSV file' in content
 
 
 @moto.mock_s3

From 426a23decd725cef7f909cc7a7ceb8f77fce9da4 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 18 Feb 2016 07:44:50 +0000
Subject: [PATCH 4/5] Add a hint about how to use placeholders
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since placeholders (almost) work now, it’s worth telling people what the syntax
is.

This commit also removes the ‘template type’ picker, since you can only create
SMS templates at the moment. This will be revisited when we start looking at how
you add an email template.
---
 app/assets/stylesheets/app.scss               |  4 +++
 app/assets/stylesheets/main.scss              |  1 +
 .../stylesheets/views/edit-template.scss      |  9 +++++++
 app/main/dao/templates_dao.py                 | 19 ++++---------
 app/main/forms.py                             |  1 -
 app/main/views/templates.py                   |  4 +--
 app/templates/views/edit-template.html        | 27 +++++++++++--------
 tests/app/main/views/test_templates.py        |  4 +--
 8 files changed, 38 insertions(+), 31 deletions(-)
 create mode 100644 app/assets/stylesheets/views/edit-template.scss

diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss
index dcb2a9f2b..e4a629217 100644
--- a/app/assets/stylesheets/app.scss
+++ b/app/assets/stylesheets/app.scss
@@ -40,6 +40,10 @@ a {
   }
 }
 
+.form-control-1-1 {
+  width: 100%;
+}
+
 .form-control-5em {
 
   width: 100%;
diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss
index f04d61fce..eb420be44 100644
--- a/app/assets/stylesheets/main.scss
+++ b/app/assets/stylesheets/main.scss
@@ -49,6 +49,7 @@ $path: '/static/images/';
 @import 'components/api-key';
 
 @import 'views/job';
+@import 'views/edit-template';
 
 // TODO: break this up
 @import 'app';
diff --git a/app/assets/stylesheets/views/edit-template.scss b/app/assets/stylesheets/views/edit-template.scss
new file mode 100644
index 000000000..3635b067f
--- /dev/null
+++ b/app/assets/stylesheets/views/edit-template.scss
@@ -0,0 +1,9 @@
+.edit-template {
+
+  &-placeholder-hint {
+    display: block;
+    padding-top: 20px;
+    color: $secondary-text-colour;
+  }
+
+}
diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py
index 032c45fbf..d34f2b187 100644
--- a/app/main/dao/templates_dao.py
+++ b/app/main/dao/templates_dao.py
@@ -1,32 +1,23 @@
 from flask import url_for, abort
 from app import notifications_api_client
 from app.utils import BrowsableItem
+from notifications_python_client.errors import HTTPError
 
 
-def insert_service_template(name, type_, content, service_id):
+def insert_service_template(name, content, service_id):
     return notifications_api_client.create_service_template(
-        name, type_, content, service_id)
+        name, 'sms', content, service_id)
 
 
-def update_service_template(id_, name, type_, content, service_id):
+def update_service_template(id_, name, content, service_id):
     return notifications_api_client.update_service_template(
-        id_, name, type_, content, service_id)
+        id_, name, 'sms', content, service_id)
 
 
 def get_service_templates(service_id):
     return notifications_api_client.get_service_templates(service_id)
 
 
-def get_service_templates_or_404(service_id):
-    try:
-        get_service_templates(service_id)
-    except HTTPError as e:
-        if e.status_code == 404:
-            abort(404)
-        else:
-            raise e
-
-
 def get_service_template_or_404(service_id, template_id):
     try:
         return notifications_api_client.get_service_template(service_id, template_id)
diff --git a/app/main/forms.py b/app/main/forms.py
index cf1aa76c8..ddab3fe01 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -188,7 +188,6 @@ class TemplateForm(Form):
     name = StringField(
         u'Template name',
         validators=[DataRequired(message="Template name cannot be empty")])
-    template_type = RadioField(u'Template type', choices=[('sms', 'SMS')])
 
     template_content = TextAreaField(
         u'Message',
diff --git a/app/main/views/templates.py b/app/main/views/templates.py
index 0b3f492ff..a8453f9e5 100644
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -48,7 +48,7 @@ def add_service_template(service_id):
 
     if form.validate_on_submit():
         tdao.insert_service_template(
-            form.name.data, form.template_type.data, form.template_content.data, service_id)
+            form.name.data, form.template_content.data, service_id)
         return redirect(url_for(
             '.manage_service_templates', service_id=service_id))
     return render_template(
@@ -67,7 +67,7 @@ def edit_service_template(service_id, template_id):
 
     if form.validate_on_submit():
         tdao.update_service_template(
-            template_id, form.name.data, form.template_type.data,
+            template_id, form.name.data,
             form.template_content.data, service_id)
         return redirect(url_for('.manage_service_templates', service_id=service_id))
 
diff --git a/app/templates/views/edit-template.html b/app/templates/views/edit-template.html
index ae3d5005e..2563dd81e 100644
--- a/app/templates/views/edit-template.html
+++ b/app/templates/views/edit-template.html
@@ -11,17 +11,22 @@
     <h1 class="heading-large">{{ h1 }}</h1>
 
     <form method="post">
-      {{ textbox(form.name) }}
-      <fieldset class="form-group">
-        <legend class="form-label">
-          Template type
-        </legend>
-        <label class="block-label" for="template_type">
-          <input type="radio" name="template_type" id="template_type" checked="checked" value="sms" />
-          Text message
-        </label>
-      </fieldset>
-      {{ textbox(form.template_content, highlight_tags=True) }}
+      <div class="grid-row">
+        <div class="column-two-thirds">
+          {{ textbox(form.name, width='1-1') }}
+        </div>
+      </div>
+      <div class="grid-row">
+        <div class="column-two-thirds">
+          {{ textbox(form.template_content, highlight_tags=True, width='1-1') }}
+        </div>
+        <div class="column-one-third">
+          <label for='template_content' class='edit-template-placeholder-hint'>
+            Add placeholders using double brackets, eg Your thing
+            is due on ((date))
+          </label>
+        </div>
+      </div>
       {{ page_footer(
         'Save',
         delete_link=url_for('.delete_service_template', service_id=service_id, template_id=template_id) if template_id or None,
diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py
index 47af74f9f..3319f1468 100644
--- a/tests/app/main/views/test_templates.py
+++ b/tests/app/main/views/test_templates.py
@@ -58,12 +58,10 @@ def test_should_redirect_when_saving_a_template(app_,
             service_id = str(uuid.uuid4())
             template_id = 456
             name = "new name"
-            type_ = "sms"
             content = "template content"
             data = {
                 'id': template_id,
                 'name': name,
-                'template_type': type_,
                 "template_content": content,
                 "service": service_id
             }
@@ -76,7 +74,7 @@ def test_should_redirect_when_saving_a_template(app_,
             assert response.location == url_for(
                 '.manage_service_templates', service_id=service_id, _external=True)
             mock_update_service_template.assert_called_with(
-                template_id, name, type_, content, service_id)
+                template_id, name, 'sms', content, service_id)
 
 
 def test_should_show_delete_template_page(app_,

From 29dc039786534c59336bcf50045624726c93df15 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 18 Feb 2016 15:50:28 +0000
Subject: [PATCH 5/5] Skip initial space when parsing CSVs

---
 app/main/views/sms.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/main/views/sms.py b/app/main/views/sms.py
index 2e4154af3..958a549e0 100644
--- a/app/main/views/sms.py
+++ b/app/main/views/sms.py
@@ -162,7 +162,8 @@ def _get_rows(contents, raw_template):
     reader = csv.DictReader(
         contents.split('\n'),
         lineterminator='\n',
-        quoting=csv.QUOTE_NONE
+        quoting=csv.QUOTE_NONE,
+        skipinitialspace=True
     )
     valid = True
     rows = []