Make send a test teach you how placeholders work

This commit does two main things:

- adds textboxes to the send yourself a test page, so you can replace
  ((name)) with ‘Chris’, or whatever your name is
- rejigs the send page a bit to make it clearer what the CSV upload is
  for and how to use it

The idea being that, since most users go into ‘send yourself a test’
first, it teaches them about how placeholders work by making them do the
replacing themselves.

app/assets/stylesheets/components/file-upload.scss

@@ -50,6 +50,11 @@
       display: none;
     }
 
+    &-alternate-link {
+      display: inline-block;
+      line-height: 35px;
+    }
+
   }
 
 }

app/assets/stylesheets/components/page-footer.scss

@@ -5,7 +5,6 @@
   &-back-link {
     @include button($grey-1);
     display: inline-block;
-    margin-left: 10px;
   }
 
   &-delete-link {
@@ -36,6 +35,11 @@
     margin-top: $gutter;
   }
 
+  .button,
+  .button-destructive {
+    margin-right: 10px;
+  }
+
   .button-destructive {
     @include button($error-colour);
     padding: 0.52632em 0.78947em 0.26316em 0.78947em;

app/main/views/send.py

@@ -50,20 +50,26 @@ def get_page_headings(template_type):
     }[template_type]
 
 
-def get_example_csv_rows(template, number_of_rows=2):
+def get_example_csv_fields(column_headers, use_example_as_example, submitted_fields):
+    print("="*40)
+    print(list(column_headers))
+    if use_example_as_example:
+        return ["example" for header in column_headers]
+    elif submitted_fields:
+        return [submitted_fields.get(header) for header in column_headers]
+    else:
+        return list(column_headers)
+
+
+def get_example_csv_rows(template, use_example_as_example=True, submitted_fields=False):
     return [
-        [
-            {
-                'email': current_user.email_address,
-                'sms': validate_and_format_phone_number(
-                    current_user.mobile_number, human_readable=True
-                )
-            }[template.template_type]
-        ] + [
-            "{} {}".format(header, i) for header in template.placeholders
-        ]
-        for i in range(1, number_of_rows + 1)
-    ]
+        {
+            '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
+            )
+        }[template.template_type]
+    ] + get_example_csv_fields(template.placeholders, use_example_as_example, submitted_fields)
 
 
 @main.route("/services/<service_id>/send/<template_type>", methods=['GET'])
@@ -131,7 +137,7 @@ def send_messages(service_id, template_id):
         'views/send.html',
         template=template,
         recipient_column=first_column_heading[template.template_type],
-        example=get_example_csv_rows(template),
+        example=[get_example_csv_rows(template, use_example_as_example=True)],
         form=form
     )
 
@@ -139,43 +145,58 @@ def send_messages(service_id, template_id):
 @main.route("/services/<service_id>/send/<template_id>.csv", methods=['GET'])
 @login_required
 @user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', any_=True)
-def get_example_csv(service_id, template_id, number_of_rows=2):
+def get_example_csv(service_id, template_id):
     template = Template(service_api_client.get_service_template(service_id, template_id)['data'])
     with io.StringIO() as output:
         writer = csv.writer(output)
-        writer.writerows(
-            [
-                [first_column_heading[template.template_type]] +
-                list(template.placeholders)
-            ] +
-            get_example_csv_rows(template, number_of_rows=number_of_rows)
-        )
+        writer.writerows([
+            [first_column_heading[template.template_type]] + list(template.placeholders),
+            get_example_csv_rows(template)
+        ])
         return output.getvalue(), 200, {
             'Content-Type': 'text/csv; charset=utf-8',
             'Content-Disposition': 'inline; filename="{}.csv"'.format(template.name)
         }
 
 
-@main.route("/services/<service_id>/send/<template_id>/to-self", methods=['GET'])
+@main.route("/services/<service_id>/send/<template_id>/test", methods=['GET', 'POST'])
 @login_required
 @user_has_permissions('send_texts', 'send_emails', 'send_letters')
-def send_message_to_self(service_id, template_id):
-    template = Template(service_api_client.get_service_template(service_id, template_id)['data'])
+def send_test(service_id, template_id):
 
-    filedata = {
-        'file_name': 'Test run',
-        'data': get_example_csv(service_id, template_id, number_of_rows=1)[0]
-    }
+    template = Template(
+        service_api_client.get_service_template(service_id, template_id)['data'],
+        prefix=current_service['name']
+    )
 
-    upload_id = str(uuid.uuid4())
+    if len(template.placeholders) == 0 or request.method == 'POST':
+        with io.StringIO() as output:
+            writer = csv.writer(output)
+            writer.writerows([
+                [first_column_heading[template.template_type]] + list(template.placeholders),
+                get_example_csv_rows(template, use_example_as_example=False, submitted_fields=request.form)
+            ])
+            filedata = {
+                'file_name': 'Test run',
+                'data': output.getvalue()
+            }
+            upload_id = str(uuid.uuid4())
+            s3upload(upload_id, service_id, filedata, current_app.config['AWS_REGION'])
+            session['upload_data'] = {"template_id": template_id, "original_file_name": filedata['file_name']}
+            return redirect(url_for(
+                '.check_messages',
+                upload_id=upload_id,
+                service_id=service_id,
+                template_type=template.template_type,
+                from_test=True
+            ))
 
-    s3upload(upload_id, service_id, filedata, current_app.config['AWS_REGION'])
-    session['upload_data'] = {"template_id": template_id, "original_file_name": filedata['file_name']}
-
-    return redirect(url_for('.check_messages',
-                            upload_id=upload_id,
-                            service_id=service_id,
-                            template_type=template.template_type))
+    return render_template(
+        'views/send-test.html',
+        template=template,
+        recipient_column=first_column_heading[template.template_type],
+        example=[get_example_csv_rows(template, use_example_as_example=False)]
+    )
 
 
 @main.route("/services/<service_id>/send/<template_id>/from-api", methods=['GET'])
@@ -232,6 +253,11 @@ def check_messages(service_id, template_type, upload_id):
         ) if current_service['restricted'] else None
     )
 
+    if request.args.get('from_test'):
+        back_link = url_for('.send_test', service_id=service_id, template_id=template.id)
+    else:
+        back_link = url_for('.send_messages', service_id=service_id, template_id=template.id)
+
     with suppress(StopIteration):
         template.values = next(recipients.rows)
         first_recipient = template.values.get(recipients.recipient_column_header, '')
@@ -256,7 +282,8 @@ def check_messages(service_id, template_type, upload_id):
         send_button_text=get_send_button_text(template.template_type, session['upload_data']['notification_count']),
         upload_id=upload_id,
         form=CsvUploadForm(),
-        statistics=statistics
+        statistics=statistics,
+        back_link=back_link
     )
 
 

app/templates/components/file-upload.html

@@ -1,4 +1,4 @@
-{% macro file_upload(field, button_text="Choose file") %}
+{% macro file_upload(field, button_text="Choose file", alternate_link=None, alternate_link_text=None) %}
   <form method="post" enctype="multipart/form-data" class="{% if field.errors %}error{% endif %}" data-module="file-upload">
     <label class="file-upload-label" for="{{ field.name }}">
       <span class="visually-hidden">{{ field.label }}</span>
@@ -19,6 +19,11 @@
     <label class="file-upload-button" for="{{ field.name }}">
       {{ button_text }}
     </label>
+    {% if alternate_link and alternate_link_text %}
+      <span class="file-upload-alternate-link">
+        or <a href="{{ alternate_link }}">{{ alternate_link_text }}</a>
+      </span>
+    {% endif %}
     <label class="file-upload-filename" for="{{ field.name }}"></label>
     <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
     <input type="submit" class="file-upload-submit" value="Submit" />

app/templates/views/check.html

@@ -97,12 +97,16 @@
     errors or
     count_of_recipients > (current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0)) 
   ) %}
-    {{file_upload(form.file, button_text='Re-upload your file')}}
+    {% if request.args.from_test %}
+      <a href="{{ back_link }}" class="page-footer-back-link">Back</a>
+    {% else %}
+      {{file_upload(form.file, button_text='Re-upload your file')}}
+    {% endif %}
   {% else %}
-    <form method="post" enctype="multipart/form-data" action="{{url_for('main.start_job', service_id=current_service.id, upload_id=upload_id)}}">
+    <form method="post" enctype="multipart/form-data" action="{{url_for('main.start_job', service_id=current_service.id, upload_id=upload_id)}}" class='page-footer'>
       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
       <input type="submit" class="button" value="{{ send_button_text }}" />
-      <a href="{{url_for('.send_messages', service_id=current_service.id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
+      <a href="{{ back_link }}" class="page-footer-back-link">Back</a>
     </form>
   {% endif %}
 

app/templates/views/send-test.html

@@ -0,0 +1,59 @@
+{% 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, field, text_field, index_field, index_field_heading %}
+
+{% block page_title %}
+  Send text messages – GOV.UK Notify
+{% endblock %}
+
+{% block maincolumn_content %}
+
+
+  <h1 class="heading-large">Send a test</h1>
+
+  {% if 'sms' == template.template_type %}
+    <div class="grid-row">
+      <div class="column-two-thirds">
+        {{ sms_message(
+          template.formatted_as_markup,
+          recipient='',
+          show_placeholder_for_recipient=True
+        ) }}
+      </div>
+    </div>
+  {% elif 'email' == template.template_type %}
+    {{ email_message(
+      template.formatted_subject_as_markup,
+      template.formatted_as_markup,
+      recipient='',
+      show_placeholder_for_recipient=True
+    ) }}
+  {% endif %}
+
+  <form method="post">
+    {% call(item, row_number) list_table(
+      example,
+      caption="Fill in your {}".format('field' if template.placeholders|length == 1 else 'fields'),
+      field_headings=[
+        '<span class="placeholder">{}</span>'.format(recipient_column)|safe
+      ] + template.placeholders_as_markup|list
+    ) %}
+      {% for column in item %}
+        {% call field() %}
+          {% if loop.index > 1 %}
+            <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 %}
+            {{ column }}
+          {% endif %}
+        {% endcall %}
+      {% endfor %}
+    {% endcall %}
+
+    {{ page_footer("Check and confirm", back_link=url_for('.send_messages', service_id=current_service.id, template_id=template.id)) }}
+  </form>
+
+{% endblock %}

app/templates/views/send.html

@@ -11,41 +11,47 @@
 
 {% block maincolumn_content %}
 
-
   <h1 class="heading-large">Send {{ 'text messages' if 'sms' == template.template_type else 'emails' }}</h1>
 
-  <div class="grid-row">
-    <div class="column-two-thirds">
-      {% if 'sms' == template.template_type %}
-        {{ sms_message(template.formatted_as_markup) }}
-      {% elif 'email' == template.template_type %}
-        {{ email_message(
-          template.formatted_subject_as_markup,
-          template.formatted_as_markup
+  {% if 'sms' == template.template_type %}
+    <div class="grid-row">
+      <div class="column-two-thirds">
+        {{ sms_message(
+          template.formatted_as_markup,
+          recipient='',
+          show_placeholder_for_recipient=True
         ) }}
-      {% endif %}
-    </div>
-    <div class="column-one-third">
-      <a href="{{ url_for(".send_message_to_self", service_id=current_service.id, template_id=template.id) }}" class='secondary-button'>Send yourself a test</a>
+      </div>
     </div>
+  {% elif 'email' == template.template_type %}
+    {{ email_message(
+      template.formatted_subject_as_markup,
+      template.formatted_as_markup,
+      recipient='',
+      show_placeholder_for_recipient=True
+    ) }}
+  {% endif %}
+
+  <div class="page-footer bottom-gutter">
+    {{file_upload(
+      form.file,
+      button_text='Upload a .csv file',
+      alternate_link=url_for(".send_test", service_id=current_service.id, template_id=template.id),
+      alternate_link_text='send yourself a test'
+    )}}
   </div>
 
-  <h2 class="heading-medium">
-    Make a CSV file with
-    {{ template.placeholders|length + 1 }}
-    {% if template.placeholders %}
-      columns,
-    {% else %}
-      column,
-    {% endif %}
-    like this:
+  <h2 class="heading-medium" style="margin: 45px 0 15px 0">
+    Example file
   </h2>
 
   {% call(item, row_number) list_table(
     example,
     caption="Example",
     caption_visible=False,
-    field_headings=['1'] + [recipient_column] + template.placeholders_as_markup|list
+    field_headings=['1'] + [
+      '<span class="placeholder">{}</span>'.format(recipient_column)|safe
+    ] + template.placeholders_as_markup|list
   ) %}
     {{ index_field(row_number) }}
     {% for column in item %}
@@ -57,6 +63,16 @@
     <a href="{{ url_for('.get_example_csv', service_id=current_service.id, template_id=template.id) }}">Download this example</a>
   </p>
 
-  {{file_upload(form.file, button_text='Upload your CSV file')}}
+  <h2 class="heading-medium">
+    Formatting
+  </h2>
+  <ul class="list list-bullet">
+    <li>
+      put one recipient per row
+    </li>
+    <li>
+      save or export your data as a .csv (comma separated values) file
+    </li>
+  </ul>
 
 {% endblock %}

tests/app/main/views/test_send.py

@@ -77,7 +77,7 @@ def test_upload_csv_invalid_extension(app_,
         assert "{} is not a CSV file".format(filename) in resp.get_data(as_text=True)
 
 
-def test_send_test_sms_message_to_self(
+def test_send_test_sms_message(
     app_,
     mocker,
     api_user_active,
@@ -98,14 +98,14 @@ def test_send_test_sms_message_to_self(
         with app_.test_client() as client:
             client.login(api_user_active)
             response = client.get(
-                url_for('main.send_message_to_self', service_id=fake_uuid, template_id=fake_uuid),
+                url_for('main.send_test', service_id=fake_uuid, template_id=fake_uuid),
                 follow_redirects=True
             )
         assert response.status_code == 200
         mock_s3_upload.assert_called_with(ANY, fake_uuid, expected_data, 'eu-west-1')
 
 
-def test_send_test_email_message_to_self(
+def test_send_test_email_message(
     app_,
     mocker,
     api_user_active,
@@ -126,14 +126,50 @@ def test_send_test_email_message_to_self(
         with app_.test_client() as client:
             client.login(api_user_active)
             response = client.get(
-                url_for('main.send_message_to_self', service_id=fake_uuid, template_id=fake_uuid),
+                url_for('main.send_test', service_id=fake_uuid, template_id=fake_uuid),
                 follow_redirects=True
             )
         assert response.status_code == 200
         mock_s3_upload.assert_called_with(ANY, fake_uuid, expected_data, 'eu-west-1')
 
 
-def test_send_test_message_from_api_page(
+def test_send_test_sms_message_with_placeholders(
+    app_,
+    mocker,
+    api_user_active,
+    mock_login,
+    mock_get_service,
+    mock_get_service_template_with_placeholders,
+    mock_s3_upload,
+    mock_has_permissions,
+    mock_get_users_by_service,
+    mock_get_service_statistics,
+    fake_uuid
+):
+
+    expected_data = {
+        'data': 'phone number,name\r\n07700 900 762,Jo\r\n',
+        'file_name': 'Test run'
+    }
+    mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
+
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            client.login(api_user_active)
+            response = client.post(
+                url_for(
+                    'main.send_test',
+                    service_id=fake_uuid,
+                    template_id=fake_uuid
+                ),
+                data={'name': 'Jo'},
+                follow_redirects=True
+            )
+        assert response.status_code == 200
+        mock_s3_upload.assert_called_with(ANY, fake_uuid, expected_data, 'eu-west-1')
+
+
+def test_api_info_page(
     app_,
     mocker,
     api_user_active,
@@ -174,7 +210,7 @@ def test_download_example_csv(
                 follow_redirects=True
             )
         assert response.status_code == 200
-        assert response.get_data(as_text=True) == 'phone number\r\n07700 900 762\r\n07700 900 762\r\n'
+        assert response.get_data(as_text=True) == 'phone number\r\n07700 900321\r\n'
         assert 'text/csv' in response.headers['Content-Type']
 
 
@@ -339,7 +375,7 @@ def test_route_permissions(mocker,
             "GET",
             302,
             url_for(
-                'main.send_message_to_self',
+                'main.send_test',
                 service_id=service_one['id'],
                 template_type='sms',
                 template_id=fake_uuid),
@@ -362,7 +398,7 @@ def test_route_invalid_permissions(mocker,
         'main.choose_template',
         'main.send_messages',
         'main.get_example_csv',
-        'main.send_message_to_self']
+        'main.send_test']
     with app_.test_request_context():
         for route in routes:
             validate_route_permission(
@@ -410,7 +446,7 @@ def test_route_choose_template_manage_service_permissions(mocker,
             service_id=service_one['id'],
             template_id=template_id) not in page
         assert url_for(
-            "main.send_message_to_self",
+            "main.send_test",
             service_id=service_one['id'],
             template_id=template_id) not in page
         assert url_for(
@@ -485,7 +521,7 @@ def test_route_choose_template_manage_api_keys_permissions(mocker,
             service_one)
         page = resp.get_data(as_text=True)
         assert url_for(
-            "main.send_message_to_self",
+            "main.send_test",
             service_id=service_one['id'],
             template_id=template_id) not in page
         assert url_for(

tests/conftest.py

@@ -202,6 +202,18 @@ def mock_get_service_template(mocker):
         'app.service_api_client.get_service_template', side_effect=_get)
 
 
+@pytest.fixture(scope='function')
+def mock_get_service_template_with_placeholders(mocker):
+    def _get(service_id, template_id):
+        template = template_json(
+            service_id, template_id, "Two week reminder", "sms", "((name)), your vehicle tax is about to expire"
+        )
+        return {'data': template}
+
+    return mocker.patch(
+        'app.service_api_client.get_service_template', side_effect=_get)
+
+
 @pytest.fixture(scope='function')
 def mock_get_service_email_template(mocker):
     def _create(service_id, template_id):