darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-07-18 21:44:11 -04:00
Code Issues Packages Projects Releases Wiki Activity

Show one field at a time on send yourself a test

Browse Source 
The send yourself a test feature is useful for two things:
- constructing an email/text message/letter without uploading a CSV file
- seeing what the thing your going to send will look like (either by
  getting it in your inbox or downloading the PDF)
- learning the concept of placeholders, ie understanding they’re thing
  that gets populated with _stuff_

The problem we’re seeing is that the current UI breaks when a template
has a lot of placeholders. This is especially apparent with letter
templates, which have a minimum of 7 placeholders by virtue of the
address.

The idea behind having the form fields side-by-side was to help people
understand the relationship between their spreadsheet columns and the
placeholders. But this means that the page was doing a lot of work,
trying to teach:
- replacement of placeholders
- link between placeholders and spreadsheet columns

The latter is better explained by the example spreadsheet shown on the
upload page. So it can safely be removed from the send yourself a test
page – in other words the fields don’t need to be shown side by side.

Showing them one-at-a-time works well because:
- it’s really obvious, even on first use, what the page is asking you to
  do
- as your step through each placeholder, you see the message build up
  with the data you’ve entered – you’re learning how replacement of
  placeholders works by repetition

This also means adding a matching endpoint for viewing each step of
making the test letter as a PDF/PNG because we can’t reuse the view of
the template without any placeholders filled any more.
This commit is contained in:
Chris Hill-Scott
2017-05-04 09:30:55 +01:00
parent eb7b8631b6
commit 8c03feb334
6 changed files with 466 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
app/main/forms.py
View File

@@ -1,6 +1,6 @@
import re
import pytz
from flask_login import current_user
from flask_wtf import Form
from datetime import datetime, timedelta
@@ -619,3 +619,26 @@ class ChooseTemplateType(Form):
class SearchTemplatesForm(Form):
search = SearchField('Search by name')
class PlaceholderForm(Form):
pass
def get_placeholder_form_instance(
placeholder_name,
dict_to_populate_from,
optional_placeholder=False
):
PlaceholderForm.placeholder_value = StringField(
placeholder_name,
validators=[
DataRequired(message='Can’t be empty')
] if not optional_placeholder else []
)
return PlaceholderForm(
placeholder_value=dict_to_populate_from.get(placeholder_name, '')
)

179
app/main/views/send.py
View File

@@ -21,10 +21,20 @@ from flask import (
from flask_login import login_required, current_user
from notifications_utils.columns import Columns
from notifications_utils.recipients import RecipientCSV, first_column_headings, validate_and_format_phone_number
from notifications_utils.recipients import (
RecipientCSV,
first_column_headings,
validate_and_format_phone_number,
optional_address_columns,
)
from app.main import main
from app.main.forms import CsvUploadForm, ChooseTimeForm, get_furthest_possible_scheduled_time
from app.main.forms import (
CsvUploadForm,
ChooseTimeForm,
get_furthest_possible_scheduled_time,
get_placeholder_form_instance
)
from app.main.uploader import (
s3upload,
s3download
@@ -147,12 +157,24 @@ def get_example_csv(service_id, template_id):
}
@main.route("/services/<service_id>/send/<template_id>/test", methods=['GET', 'POST'])
@main.route("/services/<service_id>/send/<template_id>/test")
@login_required
@user_has_permissions('send_texts', 'send_emails', 'send_letters')
def send_test(service_id, template_id):
session['send_test_values'] = dict()
return redirect(url_for(
'.send_test_step',
service_id=service_id,
template_id=template_id,
step_index=0,
help=get_help_argument(),
))
file_name = current_app.config['TEST_MESSAGE_FILENAME']
@main.route("/services/<service_id>/send/<template_id>/test/step-<int:step_index>", methods=['GET', 'POST'])
@login_required
@user_has_permissions('send_texts', 'send_emails', 'send_letters')
def send_test_step(service_id, template_id, step_index):
template = service_api_client.get_service_template(service_id, template_id)['data']
@@ -162,48 +184,100 @@ def send_test(service_id, template_id):
show_recipient=True,
expand_emails=True,
letter_preview_url=url_for(
'.view_letter_template_preview',
'.send_test_preview',
service_id=service_id,
template_id=template_id,
filetype='png',
page_count=get_page_count_for_letter(template),
),
page_count=get_page_count_for_letter(template),
)
if len(template.placeholders) == 0 or request.method == 'POST':
upload_id = s3upload(
service_id,
{
'file_name': file_name,
'data': Spreadsheet.from_rows([
first_column_headings[template.template_type] + list(template.placeholders),
get_example_csv_rows(template, use_example_as_example=False, submitted_fields=request.form)
]).as_csv_data
},
current_app.config['AWS_REGION']
)
session['upload_data'] = {
"template_id": template_id,
"original_file_name": file_name
}
placeholders = fields_to_fill_in(template)
if len(placeholders) == 0:
return make_and_upload_csv_file(service_id, template)
current_placeholder = placeholders[step_index]
optional_placeholder = (current_placeholder in optional_address_columns)
form = get_placeholder_form_instance(
current_placeholder,
dict_to_populate_from=get_normalised_send_test_values_from_session(),
optional_placeholder=optional_placeholder,
)
if form.validate_on_submit():
session['send_test_values'][current_placeholder] = form.placeholder_value.data
if all(
get_normalised_send_test_values_from_session().get(placeholder, False) not in (False, None)
for placeholder in placeholders
):
return make_and_upload_csv_file(service_id, template)
return redirect(url_for(
'.check_messages',
upload_id=upload_id,
'.send_test_step',
service_id=service_id,
template_type=template.template_type,
from_test=True,
help=2 if request.args.get('help') else 0
template_id=template_id,
step_index=step_index + 1,
help=get_help_argument(),
))
if get_help_argument():
back_link = None
elif step_index == 0:
back_link = url_for(
'.view_template',
service_id=service_id,
template_id=template_id,
)
else:
back_link = url_for(
'.send_test_step',
service_id=service_id,
template_id=template_id,
step_index=step_index - 1,
)
template.values = get_normalised_send_test_values_from_session()
return render_template(
'views/send-test.html',
template=template,
recipient_columns=first_column_headings[template.template_type],
example=[get_example_csv_rows(template, use_example_as_example=False)],
help=get_help_argument()
form=form,
optional_placeholder=optional_placeholder,
help=get_help_argument(),
back_link=back_link,
)
@main.route("/services/<service_id>/send/<template_id>/test.<filetype>", methods=['GET'])
@login_required
@user_has_permissions('send_texts', 'send_emails', 'send_letters')
def send_test_preview(service_id, template_id, filetype):
if filetype not in ('pdf', 'png'):
abort(404)
template = service_api_client.get_service_template(service_id, template_id)['data']
template = get_template(
template,
current_service,
letter_preview_url=url_for(
'.send_test_preview',
service_id=service_id,
template_id=template_id,
filetype='png',
),
page_count=get_page_count_for_letter(template),
)
template.values = get_normalised_send_test_values_from_session()
return TemplatePreview.from_utils_template(template, filetype, page=request.args.get('page'))
def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False):
if not session.get('upload_data'):
@@ -380,3 +454,48 @@ def get_check_messages_back_url(service_id, template_type):
return url_for('.send_test', service_id=service_id, template_id=templates[0]['id'], help=1)
return url_for('main.choose_template', service_id=service_id)
def fields_to_fill_in(template):
recipient_columns = first_column_headings[template.template_type]
if 'letter' == template.template_type:
return recipient_columns + list(template.placeholders)
session['send_test_values'][recipient_columns[0]] = {
'email': current_user.email_address,
'sms': current_user.mobile_number,
}.get(template.template_type)
return list(template.placeholders)
def get_normalised_send_test_values_from_session():
return {
key: ''.join(value or [])
for key, value in session.get('send_test_values', {}).items()
}
def make_and_upload_csv_file(service_id, template):
upload_id = s3upload(
service_id,
Spreadsheet.from_dict(
session['send_test_values'],
filename=current_app.config['TEST_MESSAGE_FILENAME']
).as_dict,
current_app.config['AWS_REGION'],
)
session['upload_data'] = {
"template_id": template.id,
"original_file_name": current_app.config['TEST_MESSAGE_FILENAME']
}
return redirect(url_for(
'.check_messages',
upload_id=upload_id,
service_id=service_id,
template_type=template.template_type,
from_test=True,
help=2 if get_help_argument() else 0
))

24
app/templates/views/send-test.html
View File

@@ -1,6 +1,7 @@
{% extends "withnav_template.html" %}
{% from "components/page-footer.html" import page_footer %}
{% from "components/file-upload.html" import file_upload %}
{% from "components/textbox.html" import textbox %}
{% from "components/table.html" import list_table, field, text_field, index_field, index_field_heading %}
{% block service_page_title %}
@@ -28,26 +29,11 @@
{{ template|string }}
<form method="post">
{% call(item, row_number) list_table(
example,
caption="Fill in the {}".format('field' if (recipient_columns + template.placeholders|list)|length == 2 else 'fields'),
field_headings=recipient_columns + template.placeholders|list
) %}
{% for column in item %}
{% call field() %}
{% if loop.index > 1 or template.template_type == 'letter' %}
<label class="visuallyhidden" for="placeholder-field-{{ loop.index }}">{{ column }}</label>
<input class="form-control form-control-1-1 " data-module="" name="{{ column }}" rows="8" type="text" value="" id="placeholder-field-{{ loop.index }}">
{% else %}
{{ column }}
{% endif %}
{% endcall %}
{% endfor %}
{% endcall %}
{{ page_footer("Preview", back_link=(
url_for('.send_messages', service_id=current_service.id, template_id=template.id)) if not help else None
{{ textbox(
form.placeholder_value,
hint='Optional' if optional_placeholder else None
) }}
{{ page_footer('Next', back_link=back_link) }}
</form>
{% endblock %}

9
app/utils.py
View File

@@ -223,6 +223,15 @@ class Spreadsheet():
output.writerow(row)
return cls(converted.getvalue(), filename)
@classmethod
def from_dict(cls, dictionary, filename=''):
return cls.from_rows(
zip(
*sorted(dictionary.items(), key=lambda pair: pair[0])
),
filename
)
@classmethod
def from_file(cls, file_content, filename=''):
extension = cls.get_extension(filename)

280
tests/app/main/views/test_send.py
View File

@@ -8,7 +8,7 @@ from functools import partial
import pytest
from bs4 import BeautifulSoup
from flask import url_for
from notifications_utils.template import LetterPreviewTemplate
from notifications_utils.template import LetterPreviewTemplate, LetterImageTemplate
from notifications_utils.recipients import RecipientCSV
from app.main.views.send import get_check_messages_back_url
@@ -284,7 +284,7 @@ def test_upload_valid_csv_shows_file_contents(
def test_send_test_doesnt_show_file_contents(
logged_in_client,
mocker,
mock_get_service_template_with_placeholders,
mock_get_service_template,
mock_s3_upload,
mock_get_users_by_service,
mock_get_detailed_service_for_today,
@@ -293,13 +293,12 @@ def test_send_test_doesnt_show_file_contents(
):
mocker.patch('app.main.views.send.s3download', return_value="""
phone number,name,thing,thing,thing
07700900986, Jo, foo, foo, foo
phone number
07700 900 986
""")
response = logged_in_client.post(
response = logged_in_client.get(
url_for('main.send_test', service_id=service_one['id'], template_id=fake_uuid),
data={},
follow_redirects=True,
)
@@ -336,7 +335,33 @@ def test_send_test_sms_message(
mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1')
def test_send_test_email_message(
def test_send_test_sms_message_redirects_with_help_argument(
logged_in_client,
mocker,
api_user_active,
mock_login,
mock_get_service,
mock_get_service_template,
mock_has_permissions,
mock_get_users_by_service,
mock_get_detailed_service_for_today,
fake_uuid
):
response = logged_in_client.get(
url_for('main.send_test', service_id=fake_uuid, template_id=fake_uuid, help=1)
)
assert response.status_code == 302
assert response.location == url_for(
'main.send_test_step',
service_id=fake_uuid,
template_id=fake_uuid,
step_index=0,
help=1,
_external=True,
)
def test_send_test_email_message_without_placeholders(
logged_in_client,
mocker,
api_user_active,
@@ -361,7 +386,139 @@ def test_send_test_email_message(
mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1')
def test_send_test_sms_message_with_placeholders(
def test_send_test_sms_message_with_placeholders_shows_first_field(
logged_in_client,
mocker,
service_one,
mock_login,
mock_get_service,
mock_get_service_template_with_placeholders,
fake_uuid,
):
with logged_in_client.session_transaction() as session:
assert 'send_test_values' not in session
response = logged_in_client.get(
url_for(
'main.send_test',
service_id=service_one['id'],
template_id=fake_uuid,
),
follow_redirects=True,
)
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.select('label')[0].text.strip() == 'name'
assert page.select('input')[0]['name'] == 'placeholder_value'
assert page.select('.page-footer-back-link')[0]['href'] == url_for(
'main.view_template',
service_id=service_one['id'],
template_id=fake_uuid,
)
with logged_in_client.session_transaction() as session:
assert session['send_test_values']['phone number'] == '07700 900762'
def test_send_test_populates_field_from_session(
logged_in_client,
mocker,
service_one,
mock_login,
mock_get_service,
mock_get_service_template_with_placeholders,
fake_uuid,
):
with logged_in_client.session_transaction() as session:
session['send_test_values'] = {}
session['send_test_values']['name'] = 'Jo'
response = logged_in_client.get(url_for(
'main.send_test_step',
service_id=service_one['id'],
template_id=fake_uuid,
step_index=0,
))
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.select('input')[0]['value'] == 'Jo'
def test_send_test_indicates_optional_address_columns(
logged_in_client,
mocker,
service_one,
mock_login,
mock_get_service,
mock_get_service_letter_template,
fake_uuid,
):
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=1)
with logged_in_client.session_transaction() as session:
session['send_test_values'] = {}
response = logged_in_client.get(url_for(
'main.send_test_step',
service_id=service_one['id'],
template_id=fake_uuid,
step_index=3,
))
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert normalize_spaces(page.select('label')[0].text) == (
'address line 4 '
'Optional'
)
assert page.select('.page-footer-back-link')[0]['href'] == url_for(
'main.send_test_step',
service_id=service_one['id'],
template_id=fake_uuid,
step_index=2,
)
def test_send_test_allows_empty_optional_address_columns(
logged_in_client,
mocker,
service_one,
mock_login,
mock_get_service,
mock_get_service_letter_template,
fake_uuid,
):
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=1)
with logged_in_client.session_transaction() as session:
session['send_test_values'] = {}
response = logged_in_client.post(
url_for(
'main.send_test_step',
service_id=service_one['id'],
template_id=fake_uuid,
step_index=3,
),
# no data here
)
assert response.status_code == 302
assert response.location == url_for(
'main.send_test_step',
service_id=service_one['id'],
template_id=fake_uuid,
step_index=4,
_external=True,
)
def test_send_test_sms_message_puts_submitted_data_in_session_and_file(
logged_in_client,
mocker,
api_user_active,
@@ -375,23 +532,107 @@ def test_send_test_sms_message_with_placeholders(
fake_uuid
):
expected_data = {
'data': 'phone number,name\r\n07700 900762,Jo\r\n',
'file_name': 'Test message'
}
with logged_in_client.session_transaction() as session:
session['send_test_values'] = {}
mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
response = logged_in_client.post(
url_for(
'main.send_test',
'main.send_test_step',
service_id=fake_uuid,
template_id=fake_uuid
template_id=fake_uuid,
step_index=0,
),
data={'name': 'Jo'},
follow_redirects=True
data={'placeholder_value': 'Jo'},
follow_redirects=True,
)
assert response.status_code == 200
mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1')
with logged_in_client.session_transaction() as session:
assert session['send_test_values']['phone number'] == '07700 900762'
assert session['send_test_values']['name'] == 'Jo'
mock_s3_upload.assert_called_with(
fake_uuid,
{
'data': 'name,phone number\r\nJo,07700 900762\r\n',
'file_name': 'Test message'
},
'eu-west-1'
)
@pytest.mark.parametrize('filetype', ['pdf', 'png'])
def test_send_test_works_as_letter_preview(
filetype,
logged_in_platform_admin_client,
mock_get_service_letter_template,
mock_get_users_by_service,
mock_get_detailed_service_for_today,
service_one,
fake_uuid,
mocker,
):
service_one['can_send_letters'] = True
mocker.patch('app.service_api_client.get_service', return_value={"data": service_one})
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=1)
mocked_preview = mocker.patch(
'app.main.views.send.TemplatePreview.from_utils_template',
return_value='foo'
)
service_id = service_one['id']
template_id = fake_uuid
with logged_in_platform_admin_client.session_transaction() as session:
session['send_test_values'] = {'address_line_1': 'Jo Lastname'}
response = logged_in_platform_admin_client.get(
url_for(
'main.send_test_preview',
service_id=service_id,
template_id=template_id,
filetype=filetype
)
)
mock_get_service_letter_template.assert_called_with(service_id, template_id)
assert response.status_code == 200
assert response.get_data(as_text=True) == 'foo'
assert mocked_preview.call_args[0][0].id == template_id
assert type(mocked_preview.call_args[0][0]) == LetterImageTemplate
assert mocked_preview.call_args[0][0].values == {'address_line_1': 'Jo Lastname'}
assert mocked_preview.call_args[0][1] == filetype
def test_send_test_clears_session(
logged_in_client,
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_detailed_service_for_today,
fake_uuid
):
with logged_in_client.session_transaction() as session:
session['send_test_values'] = {'foo': 'bar'}
response = logged_in_client.get(
url_for(
'main.send_test',
service_id=fake_uuid,
template_id=fake_uuid,
),
)
assert response.status_code == 302
with logged_in_client.session_transaction() as session:
assert session['send_test_values'] == {}
def test_download_example_csv(
@@ -527,6 +768,7 @@ def test_test_message_can_only_be_sent_now(
def test_letter_can_only_be_sent_now(
logged_in_client,
mocker,
mock_get_service,
mock_get_service_letter_template,
mock_s3_download,
@@ -536,6 +778,8 @@ def test_letter_can_only_be_sent_now(
fake_uuid,
):
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=1)
with logged_in_client.session_transaction() as session:
session['upload_data'] = {
'original_file_name': 'Test message',
@@ -543,6 +787,7 @@ def test_letter_can_only_be_sent_now(
'notification_count': 1,
'valid': True
}
response = logged_in_client.get(url_for(
'main.check_messages',
service_id=fake_uuid,
@@ -634,6 +879,7 @@ def test_should_show_preview_letter_message(
):
service_one['can_send_letters'] = True
mocker.patch('app.service_api_client.get_service', return_value={"data": service_one})
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=1)
mocker.patch(
'app.main.views.send.s3download',

16
tests/app/test_utils.py
View File

@@ -4,6 +4,8 @@ from csv import DictReader
import pytest
from collections import OrderedDict
from app.utils import (
email_safe,
generate_notifications_csv,
@@ -105,6 +107,20 @@ def test_can_create_spreadsheet_from_large_excel_file():
assert ret.as_csv_data
def test_can_create_spreadsheet_from_dict():
assert Spreadsheet.from_dict(OrderedDict(
foo='bar',
name='Jane',
)).as_csv_data == (
"foo,name\r\n"
"bar,Jane\r\n"
)
def test_can_create_spreadsheet_from_dict_with_filename():
assert Spreadsheet.from_dict({}, filename='empty.csv').as_dict['file_name'] == "empty.csv"
def test_generate_notifications_csv_returns_correct_csv_file(_get_notifications_csv_mock):
csv_content = generate_notifications_csv(service_id='1234')
csv_file = DictReader(StringIO('\n'.join(csv_content)))