darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-06-21 13:43:14 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1293 from alphagov/one-off

Browse Source 
Make ‘send a one-off message’ a thing
This commit is contained in:
Chris Hill-Scott
2017-06-01 14:46:07 +01:00
committed by GitHub
parent d516f19a33 f12e0fde39
commit 20f1d73f76
9 changed files with 367 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
app/assets/stylesheets/_grids.scss
View File

@@ -26,6 +26,19 @@
@include grid-column(7/8);
}
%top-gutter,
.top-gutter {
@extend %contain-floats;
display: block;
margin-top: $gutter;
clear: both;
}
.top-gutter-4-3 {
@extend %top-gutter;
margin-top: $gutter * 4 / 3;
}
%bottom-gutter,
.bottom-gutter {
@extend %contain-floats;

2
app/config.py
View File

@@ -57,7 +57,7 @@ class Config(object):
CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload'
DESKPRO_PERSON_EMAIL = 'donotreply@notifications.service.gov.uk'
ACTIVITY_STATS_LIMIT_DAYS = 7
TEST_MESSAGE_FILENAME = 'Test message'
TEST_MESSAGE_FILENAME = 'Report'
STATSD_ENABLED = False
STATSD_HOST = "statsd.hostedgraphite.com"

42
app/main/forms.py
View File

@@ -7,6 +7,7 @@ from notifications_utils.recipients import (
validate_phone_number,
InvalidPhoneError
)
from notifications_utils.columns import Columns
from wtforms import (
validators,
StringField,
@@ -102,11 +103,26 @@ class UKMobileNumber(TelField):
raise ValidationError(str(e))
def mobile_number():
return UKMobileNumber('Mobile number',
class InternationalPhoneNumber(TelField):
def pre_validate(self, form):
try:
validate_phone_number(self.data, international=True)
except InvalidPhoneError as e:
raise ValidationError(str(e))
def mobile_number(label='Mobile number'):
return UKMobileNumber(label,
validators=[DataRequired(message='Can’t be empty')])
def international_phone_number(label='Mobile number'):
return InternationalPhoneNumber(
label,
validators=[DataRequired(message='Can’t be empty')]
)
def password(label='Password'):
return PasswordField(label,
validators=[DataRequired(message='Can’t be empty'),
@@ -633,15 +649,25 @@ class PlaceholderForm(Form):
def get_placeholder_form_instance(
placeholder_name,
dict_to_populate_from,
optional_placeholder=False
optional_placeholder=False,
allow_international_phone_numbers=False,
):
PlaceholderForm.placeholder_value = StringField(
placeholder_name,
validators=[
if Columns.make_key(placeholder_name) == 'emailaddress':
field = email_address(label=placeholder_name, gov_user=False)
elif Columns.make_key(placeholder_name) == 'phonenumber':
if allow_international_phone_numbers:
field = international_phone_number(label=placeholder_name)
else:
field = mobile_number(label=placeholder_name)
elif optional_placeholder:
field = StringField(placeholder_name)
else:
field = StringField(placeholder_name, validators=[
DataRequired(message='Can’t be empty')
] if not optional_placeholder else []
)
])
PlaceholderForm.placeholder_value = field
return PlaceholderForm(
placeholder_value=dict_to_populate_from.get(placeholder_name, '')

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

@@ -157,14 +157,18 @@ def get_example_csv(service_id, template_id):
}
@main.route("/services/<service_id>/send/<template_id>/test")
@main.route("/services/<service_id>/send/<template_id>/test", endpoint='send_test')
@main.route("/services/<service_id>/send/<template_id>/one-off", endpoint='send_one_off')
@login_required
@user_has_permissions('send_texts', 'send_emails', 'send_letters')
def send_test(service_id, template_id):
session['send_test_values'] = dict()
session['send_test_letter_page_count'] = None
return redirect(url_for(
'.send_test_step',
{
'main.send_test': '.send_test_step',
'main.send_one_off': '.send_one_off_step',
}[request.endpoint],
service_id=service_id,
template_id=template_id,
step_index=0,
@@ -172,14 +176,28 @@ def send_test(service_id, template_id):
))
@main.route("/services/<service_id>/send/<template_id>/test/step-<int:step_index>", methods=['GET', 'POST'])
@main.route(
"/services/<service_id>/send/<template_id>/test/step-<int:step_index>",
methods=['GET', 'POST'],
endpoint='send_test_step',
)
@main.route(
"/services/<service_id>/send/<template_id>/one-off/step-<int:step_index>",
methods=['GET', 'POST'],
endpoint='send_one_off_step',
)
@login_required
@user_has_permissions('send_texts', 'send_emails', 'send_letters')
def send_test_step(service_id, template_id, step_index):
if 'send_test_values' not in session:
return redirect(url_for(
'.send_test', service_id=service_id, template_id=template_id
{
'main.send_test_step': '.send_test',
'main.send_one_off_step': '.send_one_off',
}[request.endpoint],
service_id=service_id,
template_id=template_id,
))
template = service_api_client.get_service_template(service_id, template_id)['data']
@@ -201,7 +219,10 @@ def send_test_step(service_id, template_id, step_index):
page_count=session['send_test_letter_page_count']
)
placeholders = fields_to_fill_in(template)
placeholders = fields_to_fill_in(
template,
prefill_current_user=(request.endpoint == 'main.send_test_step'),
)
if len(placeholders) == 0:
return make_and_upload_csv_file(service_id, template)
@@ -209,28 +230,33 @@ def send_test_step(service_id, template_id, step_index):
try:
current_placeholder = placeholders[step_index]
except IndexError:
if all_placeholders_in_session(placeholders):
return make_and_upload_csv_file(service_id, template)
return redirect(url_for(
'.send_test', service_id=service_id, template_id=template_id
{
'main.send_test_step': '.send_test',
'main.send_one_off_step': '.send_one_off',
}[request.endpoint],
service_id=service_id,
template_id=template_id,
))
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,
allow_international_phone_numbers=current_service['can_send_international_sms'],
)
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
):
if all_placeholders_in_session(placeholders):
return make_and_upload_csv_file(service_id, template)
return redirect(url_for(
'.send_test_step',
request.endpoint,
service_id=service_id,
template_id=template_id,
step_index=step_index + 1,
@@ -247,7 +273,7 @@ def send_test_step(service_id, template_id, step_index):
)
else:
back_link = url_for(
'.send_test_step',
request.endpoint,
service_id=service_id,
template_id=template_id,
step_index=step_index - 1,
@@ -256,13 +282,27 @@ def send_test_step(service_id, template_id, step_index):
template.values = get_normalised_send_test_values_from_session()
template.values[current_placeholder] = None
if (
request.endpoint == 'main.send_one_off_step' and
step_index == 0 and
template.template_type != 'letter'
):
skip_link = (
'Use my {}'.format(first_column_headings[template.template_type][0]),
url_for('.send_test', service_id=service_id, template_id=template.id),
)
else:
skip_link = None
return render_template(
'views/send-test.html',
page_title=get_send_test_page_title(template.template_type, get_help_argument()),
template=template,
form=form,
skip_link=skip_link,
optional_placeholder=optional_placeholder,
help=get_help_argument(),
back_link=back_link,
help=get_help_argument(),
)
@@ -470,11 +510,11 @@ def get_check_messages_back_url(service_id, template_type):
return url_for('main.choose_template', service_id=service_id)
def fields_to_fill_in(template):
def fields_to_fill_in(template, prefill_current_user=False):
recipient_columns = first_column_headings[template.template_type]
if 'letter' == template.template_type:
if 'letter' == template.template_type or not prefill_current_user:
return recipient_columns + list(template.placeholders)
session['send_test_values'][recipient_columns[0]] = {
@@ -513,3 +553,18 @@ def make_and_upload_csv_file(service_id, template):
from_test=True,
help=2 if get_help_argument() else 0
))
def all_placeholders_in_session(placeholders):
return all(
get_normalised_send_test_values_from_session().get(placeholder, False) not in (False, None)
for placeholder in placeholders
)
def get_send_test_page_title(template_type, help_argument):
if help_argument:
return 'Example text message'
if template_type == 'letter':
return 'Print a test letter'
return 'Send to one recipient'

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

@@ -5,32 +5,30 @@
{% from "components/table.html" import list_table, field, text_field, index_field, index_field_heading %}
{% block service_page_title %}
{% if request.args['help'] %}
Example text message
{% else %}
Send yourself a test
{% endif %}
{{ page_title }}
{% endblock %}
{% block maincolumn_content %}
<h1 class="heading-large">
{% if request.args['help'] %}
Example text message
{% else %}
{% if template.template_type == 'letter' %}
Print a test letter
{% else %}
Send yourself a test
{% endif %}
{% endif %}
{{ page_title }}
</h1>
<form method="post" class="js-stick-at-top-when-scrolling" data-module="autofocus">
{{ textbox(
form.placeholder_value,
hint='Optional' if optional_placeholder else None
) }}
<div class="grid-row">
<div class="column-two-thirds">
{{ textbox(
form.placeholder_value,
hint='Optional' if optional_placeholder else None,
width='1-1',
) }}
</div>
{% if skip_link %}
<div class="column-one-third">
<a href="{{ skip_link[1] }}" class="top-gutter-4-3">{{ skip_link[0] }}</a>
</div>
{% endif %}
</div>
{{ page_footer('Next', back_link=back_link) }}
</form>

4
app/templates/views/templates/_template.html
View File

@@ -13,8 +13,8 @@
</a>
</div>
<div class="{{ 'column-half' if template.template_type == 'letter' else 'column-third' }}">
<a href="{{ url_for(".send_test", service_id=current_service.id, template_id=template.id) }}" class="pill-separate-item">
{{ 'Print a test letter' if template.template_type == 'letter' else 'Send yourself a test' }}
<a href="{{ url_for(".send_one_off", service_id=current_service.id, template_id=template.id) }}" class="pill-separate-item">
{{ 'Print a test letter' if template.template_type == 'letter' else 'Send to one recipient' }}
</a>
</div>
{% endif %}

44
tests/app/main/test_placeholder_form.py
View File

@@ -1,3 +1,4 @@
import pytest
from app.main.forms import get_placeholder_form_instance
from wtforms import Label
@@ -16,3 +17,46 @@ def test_form_class_not_mutated(app_):
assert str(form1.placeholder_value.label) == '<label for="placeholder_value">name</label>'
assert str(form2.placeholder_value.label) == '<label for="placeholder_value">city</label>'
@pytest.mark.parametrize('service_can_send_international_sms, placeholder_name, value, expected_error', [
(False, 'email address', '', 'Can’t be empty'),
(False, 'email address', '12345', 'Enter a valid email address'),
(False, 'email address', 'test@example.com', None),
(False, 'email address', 'test@example.gov.uk', None),
(False, 'phone number', '', 'Can’t be empty'),
(False, 'phone number', '+1-2345-678890', 'Not a UK mobile number'),
(False, 'phone number', '07900900123', None),
(False, 'phone number', '+44(0)7900 900-123', None),
(True, 'phone number', '+123', 'Not enough digits'),
(True, 'phone number', '+44(0)7900 900-123', None),
(True, 'phone number', '+1-2345-678890', None),
(False, 'anything else', '', 'Can’t be empty'),
])
def test_validates_recipients(
app_,
placeholder_name,
value,
service_can_send_international_sms,
expected_error,
):
with app_.test_request_context(
method='POST',
data={'placeholder_value': value}
):
form = get_placeholder_form_instance(
placeholder_name,
{},
allow_international_phone_numbers=service_can_send_international_sms,
)
if expected_error:
assert not form.validate_on_submit()
assert form.placeholder_value.errors[0] == expected_error
else:
assert form.validate_on_submit()

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

@@ -286,7 +286,6 @@ def test_send_test_sms_message(
mock_get_detailed_service_for_today,
):
expected_data = {'data': 'phone number\r\n07700 900762\r\n', 'file_name': 'Test message'}
mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
response = logged_in_client.get(
@@ -294,29 +293,174 @@ def test_send_test_sms_message(
follow_redirects=True
)
assert response.status_code == 200
mock_s3_upload.assert_called_with(service_one['id'], expected_data, 'eu-west-1')
mock_s3_upload.assert_called_with(
service_one['id'],
{'data': 'phone number\r\n07700 900762\r\n', 'file_name': 'Report'},
'eu-west-1'
)
@pytest.mark.parametrize('endpoint, template_mock, expected_session_contents', [
('main.send_test_step', mock_get_service_template_with_placeholders, {'phone number': '07700 900762'}),
('main.send_test_step', mock_get_service_email_template, {'email address': 'test@user.gov.uk'}),
('main.send_test_step', mock_get_service_letter_template, {}),
('main.send_one_off_step', mock_get_service_template, {}),
('main.send_one_off_step', mock_get_service_email_template, {}),
('main.send_one_off_step', mock_get_service_letter_template, {}),
])
def test_send_test_step_redirects_if_session_not_setup(
mocker,
logged_in_client,
service_one,
mock_get_detailed_service_for_today,
mock_get_users_by_service,
fake_uuid,
mock_get_service_email_template,
endpoint,
template_mock,
expected_session_contents,
):
template_mock(mocker)
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99)
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_step', service_id=service_one['id'], template_id=fake_uuid, step_index=0),
url_for(endpoint, service_id=SERVICE_ONE_ID, template_id=fake_uuid, step_index=0),
follow_redirects=True
)
assert response.status_code == 200
with logged_in_client.session_transaction() as session:
assert session['send_test_values'] == {'email address': 'test@user.gov.uk'}
assert session['send_test_values'] == expected_session_contents
@pytest.mark.parametrize('template_mock, partial_url, expected_h1, tour_shown', [
(
mock_get_service_template_with_placeholders,
partial(url_for, 'main.send_test'),
'Send to one recipient',
False,
),
(
mock_get_service_template_with_placeholders,
partial(url_for, 'main.send_one_off'),
'Send to one recipient',
False,
),
(
mock_get_service_template_with_placeholders,
partial(url_for, 'main.send_test', help=1),
'Example text message',
True,
),
(
mock_get_service_email_template,
partial(url_for, 'main.send_test', help=1),
'Example text message',
True,
),
(
mock_get_service_email_template,
partial(url_for, 'main.send_test'),
'Send to one recipient',
False,
),
(
mock_get_service_email_template,
partial(url_for, 'main.send_one_off'),
'Send to one recipient',
False,
),
(
mock_get_service_letter_template,
partial(url_for, 'main.send_test'),
'Print a test letter',
False,
),
(
mock_get_service_letter_template,
partial(url_for, 'main.send_one_off'),
'Print a test letter',
False,
),
])
def test_send_one_off_or_test_has_correct_page_titles(
logged_in_client,
service_one,
fake_uuid,
mocker,
template_mock,
partial_url,
expected_h1,
tour_shown,
):
template_mock(mocker)
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99)
response = logged_in_client.get(
partial_url(service_id=service_one['id'], template_id=fake_uuid, step_index=0),
follow_redirects=True,
)
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert response.status_code == 200
assert page.h1.text.strip() == expected_h1
assert (len(page.select('.banner-tour')) == 1) == tour_shown
@pytest.mark.parametrize('template_mock, expected_link_text, expected_link_url', [
(mock_get_service_template, 'Use my phone number', partial(url_for, 'main.send_test')),
(mock_get_service_email_template, 'Use my email address', partial(url_for, 'main.send_test')),
(mock_get_service_letter_template, None, None),
])
def test_send_one_off_has_skip_link(
logged_in_client,
service_one,
fake_uuid,
mock_get_service_email_template,
mocker,
template_mock,
expected_link_text,
expected_link_url,
):
template_mock(mocker)
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99)
response = logged_in_client.get(
url_for('main.send_one_off_step', service_id=service_one['id'], template_id=fake_uuid, step_index=0),
follow_redirects=True
)
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
skip_links = page.select('a.top-gutter-4-3')
assert response.status_code == 200
if expected_link_text and expected_link_url:
assert skip_links[0].text.strip() == expected_link_text
assert skip_links[0]['href'] == expected_link_url(
service_id=service_one['id'],
template_id=fake_uuid,
)
else:
assert not skip_links
@pytest.mark.parametrize('endpoint, expected_redirect, send_test_values', [
(
'main.send_test_step',
'main.send_test',
{'name': 'foo'},
),
(
'main.send_one_off_step',
'main.send_one_off',
{'name': 'foo', 'phone number': '07900900123'},
),
])
def test_send_test_redirects_to_end_if_step_out_of_bounds(
logged_in_client,
service_one,
@@ -325,13 +469,16 @@ def test_send_test_redirects_to_end_if_step_out_of_bounds(
mock_s3_upload,
mock_get_users_by_service,
mock_get_detailed_service_for_today,
endpoint,
send_test_values,
expected_redirect,
):
with logged_in_client.session_transaction() as session:
session['send_test_values'] = {'name': 'foo'}
session['send_test_values'] = send_test_values
response = logged_in_client.get(url_for(
'main.send_test_step',
endpoint,
service_id=service_one['id'],
template_id=fake_uuid,
step_index=999,
@@ -351,6 +498,10 @@ def test_send_test_redirects_to_end_if_step_out_of_bounds(
)
@pytest.mark.parametrize('endpoint, expected_redirect', [
('main.send_test_step', 'main.send_test'),
('main.send_one_off_step', 'main.send_one_off'),
])
def test_send_test_redirects_to_start_if_you_skip_steps(
logged_in_platform_admin_client,
service_one,
@@ -360,6 +511,8 @@ def test_send_test_redirects_to_start_if_you_skip_steps(
mock_get_users_by_service,
mock_get_detailed_service_for_today,
mocker,
endpoint,
expected_redirect,
):
with logged_in_platform_admin_client.session_transaction() as session:
@@ -367,20 +520,24 @@ def test_send_test_redirects_to_start_if_you_skip_steps(
session['send_test_values'] = {'address_line_1': 'foo'}
response = logged_in_platform_admin_client.get(url_for(
'main.send_test_step',
endpoint,
service_id=service_one['id'],
template_id=fake_uuid,
step_index=7, # letter template has 7 placeholders – we’re at the end
))
assert response.status_code == 302
assert response.location == url_for(
'main.send_test',
expected_redirect,
service_id=service_one['id'],
template_id=fake_uuid,
_external=True,
)
@pytest.mark.parametrize('endpoint, expected_redirect', [
('main.send_test_step', 'main.send_test'),
('main.send_one_off_step', 'main.send_one_off'),
])
def test_send_test_redirects_to_start_if_index_out_of_bounds_and_some_placeholders_empty(
logged_in_client,
service_one,
@@ -389,13 +546,15 @@ def test_send_test_redirects_to_start_if_index_out_of_bounds_and_some_placeholde
mock_s3_download,
mock_get_users_by_service,
mock_get_detailed_service_for_today,
endpoint,
expected_redirect,
):
with logged_in_client.session_transaction() as session:
session['send_test_values'] = {'name': 'foo'}
response = logged_in_client.get(url_for(
'main.send_test_step',
endpoint,
service_id=service_one['id'],
template_id=fake_uuid,
step_index=999,
@@ -403,24 +562,30 @@ def test_send_test_redirects_to_start_if_index_out_of_bounds_and_some_placeholde
assert response.status_code == 302
assert response.location == url_for(
'main.send_test',
expected_redirect,
service_id=service_one['id'],
template_id=fake_uuid,
_external=True,
)
@pytest.mark.parametrize('endpoint, expected_redirect', [
('main.send_test', 'main.send_test_step'),
('main.send_one_off', 'main.send_one_off_step'),
])
def test_send_test_sms_message_redirects_with_help_argument(
logged_in_client,
service_one,
fake_uuid,
endpoint,
expected_redirect,
):
response = logged_in_client.get(
url_for('main.send_test', service_id=service_one['id'], template_id=fake_uuid, help=1)
url_for(endpoint, service_id=service_one['id'], template_id=fake_uuid, help=1)
)
assert response.status_code == 302
assert response.location == url_for(
'main.send_test_step',
expected_redirect,
service_id=service_one['id'],
template_id=fake_uuid,
step_index=0,
@@ -440,7 +605,6 @@ def test_send_test_email_message_without_placeholders(
fake_uuid,
):
expected_data = {'data': 'email address\r\ntest@user.gov.uk\r\n', 'file_name': 'Test message'}
mocker.patch('app.main.views.send.s3download', return_value='email address\r\ntest@user.gov.uk')
response = logged_in_client.get(
@@ -448,7 +612,11 @@ def test_send_test_email_message_without_placeholders(
follow_redirects=True
)
assert response.status_code == 200
mock_s3_upload.assert_called_with(service_one['id'], expected_data, 'eu-west-1')
mock_s3_upload.assert_called_with(
service_one['id'],
{'data': 'email address\r\ntest@user.gov.uk\r\n', 'file_name': 'Report'},
'eu-west-1'
)
def test_send_test_sms_message_with_placeholders_shows_first_field(
@@ -667,7 +835,7 @@ def test_send_test_sms_message_puts_submitted_data_in_session_and_file(
service_one['id'],
{
'data': 'name,phone number\r\nJo,07700 900762\r\n',
'file_name': 'Test message'
'file_name': 'Report'
},
'eu-west-1'
)

4
tests/app/main/views/test_templates.py
View File

@@ -44,11 +44,11 @@ def test_should_show_page_for_one_template(
),
(
['send_texts', 'send_emails', 'send_letters'],
['.send_messages', '.send_test']
['.send_messages', '.send_one_off']
),
(
['send_texts', 'send_emails', 'send_letters', 'manage_templates'],
['.send_messages', '.send_test', '.edit_service_template']
['.send_messages', '.send_one_off', '.edit_service_template']
),
])
def test_should_be_able_to_view_a_template_with_links(