Merge pull request #541 from alphagov/populate-test

Introduce replacement of placeholders to send yourself a test
2026-02-06 11:23:48 -05:00 · 2016-05-05 11:00:36 +01:00
parent afc8fb08f3 6917e11207
commit d93eb4f30e
13 changed files with 303 additions and 89 deletions
--- a/app/assets/stylesheets/components/file-upload.scss
+++ b/app/assets/stylesheets/components/file-upload.scss
@@ -50,6 +50,11 @@
      display: none;
    }

+    &-alternate-link {
+      display: inline-block;
+      line-height: 35px;
+    }
+
  }

 }
--- a/app/assets/stylesheets/components/page-footer.scss
+++ b/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;
--- a/app/assets/stylesheets/components/sms-message.scss
+++ b/app/assets/stylesheets/components/sms-message.scss
@@ -31,7 +31,7 @@
 .sms-message-recipient {
  @include copy-19;
  color: $secondary-text-colour;
-  margin: -20px 0 $gutter 0;
+  margin: 10px 0 5px 0;
 }

 .sms-message-name {
@@ -65,6 +65,10 @@
    
  }

+  .primary {
+    @include bold-19;
+  }
+
 }

 .sms-message-use-links-with-title {
--- a/app/main/views/send.py
+++ b/app/main/views/send.py
@@ -50,20 +50,24 @@ 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):
+    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 +135,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)],
        form=form
    )

@@ -139,43 +143,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,14 +251,21 @@ def check_messages(service_id, template_type, upload_id):
        ) if current_service['restricted'] else None
    )

+    if request.args.get('from_test') and len(template.placeholders):
+        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, '')

    session['upload_data']['notification_count'] = len(list(recipients.rows))
    session['upload_data']['valid'] = not recipients.has_errors
    return render_template(
        'views/check.html',
        recipients=recipients,
+        first_recipient=first_recipient,
        template=template,
        page_heading=get_page_headings(template.template_type),
        errors=get_errors_for_csv(recipients, template.template_type),
@@ -254,7 +280,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
    )


--- a/app/templates/components/email-message.html
+++ b/app/templates/components/email-message.html
@@ -1,4 +1,13 @@
-{% macro email_message(subject, body, name=None, edit_link=None, from_name=None, from_address=None) %}
+{% macro email_message(
+  subject,
+  body,
+  name=None,
+  edit_link=None,
+  from_name=None,
+  from_address=None,
+  recipient=None,
+  show_placeholder_for_recipient=False
+) %}
  {% if name %}
    <h3 class="email-message-name">
      {% if edit_link %}
@@ -20,6 +29,20 @@
              </td>
            </tr>
          {% endif %}
+          {% if recipient is not none %}
+            <tr>
+              <th>To</th>
+              <td>
+                {% if show_placeholder_for_recipient %}
+                  <span class="placeholder">
+                    email address
+                  </span>
+                {% else %}
+                  {{ recipient }}
+                {% endif %}
+              </td>
+            </tr>
+          {% endif %}
          {% if subject %}
            <tr class="email-message-meta">
              <th>Subject</th>
--- a/app/templates/components/file-upload.html
+++ b/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" />
--- a/app/templates/components/sms-message.html
+++ b/app/templates/components/sms-message.html
@@ -1,4 +1,12 @@
-{% macro sms_message(body, recipient=None, name=None, id=None, edit_link=None, from=None) %}
+{% macro sms_message(
+  body,
+  recipient=None,
+  name=None,
+  id=None,
+  edit_link=None,
+  from=None,
+  show_placeholder_for_recipient=False
+) %}
  {% if name %}
    <h3 class="sms-message-name">
      {% if edit_link %}
@@ -8,6 +16,18 @@
      {% endif %}
    </h3>
  {% endif %}
+  {% if recipient is not none %}
+    <p class="sms-message-recipient">
+      To: 
+      {% if show_placeholder_for_recipient %}
+        <span class="placeholder">
+          phone number
+        </span>
+      {% else %}
+        {{ recipient }}
+      {% endif %}
+    </p>
+  {% endif %}
  <div class="sms-message-wrapper{% if input_name %}-with-radio{% endif %}">
    {% if from %}
      <span class="sms-message-from">
@@ -16,11 +36,6 @@
    {% endif %}
    {{ body|nl2br }}
  </div>
-  {% if recipient %}
-    <p class="sms-message-recipient">
-      {{ recipient }}
-    </p>
-  {% endif %}
  {% if id %}
    <p class="sms-message-recipient">
      Template ID: {{ id }}
--- a/app/templates/views/check.html
+++ b/app/templates/views/check.html
@@ -77,13 +77,17 @@
      template.formatted_subject_as_markup if errors else template.replaced_subject,
      template.formatted_as_markup if errors else template.replaced,
      from_address='{}@notifications.service.gov.uk'.format(current_service.email_from),
-      from_name=current_service.name
+      from_name=current_service.name,
+      recipient=first_recipient,
+      show_placeholder_for_recipient=errors
    )}}
  {% elif 'sms' == template.template_type %}
    <div class="grid-row">
      <div class="column-two-thirds">
        {{ sms_message(
-          template.formatted_as_markup if errors else template.replaced
+          template.formatted_as_markup if errors else template.replaced,
+          recipient=first_recipient,
+          show_placeholder_for_recipient=errors
        )}}
      </div>
    </div>
@@ -93,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 %}

--- a/app/templates/views/send-test.html
+++ b/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 %}
--- a/app/templates/views/send.html
+++ b/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 %}
--- a/app/templates/views/templates/_template.html
+++ b/app/templates/views/templates/_template.html
@@ -18,8 +18,8 @@
 <div class="column-one-third">
  <div class="sms-message-use-links{% if show_title %}-with-title{% endif %}">
    {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters']) %}
-      <a href="{{ url_for(".send_messages", service_id=current_service.id, template_id=template.id) }}">
-      Send {{ 'text messages' if 'sms' == template.template_type else 'emails' }}
+      <a href="{{ url_for(".send_messages", service_id=current_service.id, template_id=template.id) }}" class="primary">
+        Send {{ 'text messages' if 'sms' == template.template_type else 'emails' }}
      </a>
    {% endif %}
    {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) %}
--- a/tests/app/main/views/test_send.py
+++ b/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(
--- a/tests/conftest.py
+++ b/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):