From 88631a680c88df49672ee61ddb3054e4ba40a465 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 8 Nov 2016 13:12:07 +0000 Subject: [PATCH] Sprinkle `letter` throughout the app Let users create/edit/delete letter templates. Let them upload a CSV file or send a test against a letter template. Big assumption at the moment is that addresses only have one line, and therefore one column in the CSV file. --- app/assets/stylesheets/main.scss | 1 + app/main/forms.py | 4 +++ app/main/views/send.py | 12 ++++--- app/main/views/templates.py | 9 +++-- .../components/message-count-label.html | 6 ++++ app/templates/views/check.html | 5 ++- app/templates/views/edit-letter-template.html | 36 +++++++++++++++++++ app/templates/views/send-test.html | 3 ++ app/templates/views/send.html | 3 ++ app/templates/views/templates/_template.html | 3 ++ app/templates/views/templates/template.html | 4 ++- tests/app/main/views/test_letters.py | 23 ++++++++++-- 12 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 app/templates/views/edit-letter-template.html diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 830a68391..f5a0e4d4f 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -58,6 +58,7 @@ $path: '/static/images/'; @import 'components/research-mode'; @import 'components/tick-cross'; @import 'components/list-entry'; +@import 'components/letter'; @import 'views/job'; @import 'views/edit-template'; diff --git a/app/main/forms.py b/app/main/forms.py index 95afc47c6..57599ee78 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -276,6 +276,10 @@ class EmailTemplateForm(SMSTemplateForm): validators=[DataRequired(message="Can’t be empty")]) +class LetterTemplateForm(EmailTemplateForm): + pass + + class ForgotPasswordForm(Form): email_address = email_address(gov_user=False) diff --git a/app/main/views/send.py b/app/main/views/send.py index 90f423d13..d79fbc194 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -33,7 +33,8 @@ from app.utils import user_has_permissions, get_errors_for_csv, Spreadsheet, get def get_page_headings(template_type): return { 'email': 'Email templates', - 'sms': 'Text message templates' + 'sms': 'Text message templates', + 'letter': 'Letter templates' }[template_type] @@ -52,7 +53,8 @@ def get_example_csv_rows(template, use_example_as_example=True, submitted_fields '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 - ) + ), + 'letter': current_user.name }[template.template_type] ] + get_example_csv_fields(template.placeholders, use_example_as_example, submitted_fields) @@ -66,8 +68,10 @@ def get_example_csv_rows(template, use_example_as_example=True, submitted_fields 'manage_api_keys', admin_override=True, any_=True) def choose_template(service_id, template_type): - if template_type not in ['email', 'sms']: + if template_type not in ['email', 'sms', 'letter']: abort(404) + if not current_service['can_send_letters'] and template_type == 'letter': + abort(403) return render_template( 'views/templates/choose.html', templates=[ @@ -233,7 +237,7 @@ def check_messages(service_id, template_type, upload_id): max_initial_rows_shown=50, max_errors_shown=50, whitelist=itertools.chain.from_iterable( - [user.mobile_number, user.email_address] for user in users + [user.name, user.mobile_number, user.email_address] for user in users ) if current_service['restricted'] else None, remaining_messages=remaining_messages ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index d835fd5af..65886896a 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -11,14 +11,15 @@ from notifications_python_client.errors import HTTPError from app.main import main from app.utils import user_has_permissions -from app.main.forms import SMSTemplateForm, EmailTemplateForm +from app.main.forms import SMSTemplateForm, EmailTemplateForm, LetterTemplateForm from app.main.views.send import get_example_csv_rows from app import service_api_client, current_service, template_statistics_client form_objects = { 'email': EmailTemplateForm, - 'sms': SMSTemplateForm + 'sms': SMSTemplateForm, + 'letter': LetterTemplateForm } page_headings = { @@ -74,8 +75,10 @@ def view_template_version(service_id, template_id, version): @login_required @user_has_permissions('manage_templates', admin_override=True) def add_service_template(service_id, template_type): - if template_type not in ['sms', 'email']: + if template_type not in ['sms', 'email', 'letter']: abort(404) + if not current_service['can_send_letters'] and template_type == 'letter': + abort(403) form = form_objects[template_type]() if form.validate_on_submit(): diff --git a/app/templates/components/message-count-label.html b/app/templates/components/message-count-label.html index a4d59afed..23a0a3ee6 100644 --- a/app/templates/components/message-count-label.html +++ b/app/templates/components/message-count-label.html @@ -11,5 +11,11 @@ {%- else -%} emails {%- endif -%} + {%- elif template_type == 'letter' -%} + {%- if count == 1 -%} + letter + {%- else -%} + letters + {%- endif -%} {%- endif %} {{ suffix }} {%- endmacro %} diff --git a/app/templates/views/check.html b/app/templates/views/check.html index a942579e9..3146d32e0 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -3,6 +3,7 @@ {% from "components/email-message.html" import email_message %} {% from "components/radios.html" import radio_select %} {% from "components/sms-message.html" import sms_message %} +{% from "components/letter.html" import letter %} {% from "components/table.html" import list_table, field, text_field, index_field, hidden_field_heading %} {% from "components/file-upload.html" import file_upload %} {% from "components/page-footer.html" import page_footer %} @@ -148,7 +149,9 @@ )}} - {% endif %} + {% elif 'letter' == template.template_type %} + {{ letter(template.formatted_as_markup if errors else template.replaced|safe) }} + {% endif %} {% if errors %} {% if request.args.from_test %} diff --git a/app/templates/views/edit-letter-template.html b/app/templates/views/edit-letter-template.html new file mode 100644 index 000000000..f53a75a94 --- /dev/null +++ b/app/templates/views/edit-letter-template.html @@ -0,0 +1,36 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} + {{ heading_action }} letter template – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

+ {{ heading_action }} letter template +

+ +
+
+
+ {{ textbox(form.name, width='1-1', hint='Your recipients won’t see this', rows=10) }} + {{ textbox(form.subject, width='1-1', highlight_tags=True, rows=2) }} +
+
+ {{ textbox(form.template_content, highlight_tags=True, width='1-1', rows=8) }} + {{ page_footer( + 'Save', + delete_link=url_for('.delete_service_template', service_id=current_service.id, template_id=template_id) if template_id or None, + delete_link_text='Delete this template' + ) }} +
+ +
+
+ +{% endblock %} diff --git a/app/templates/views/send-test.html b/app/templates/views/send-test.html index 498eaf8da..5a6c67643 100644 --- a/app/templates/views/send-test.html +++ b/app/templates/views/send-test.html @@ -1,6 +1,7 @@ {% extends "withnav_template.html" %} {% from "components/sms-message.html" import sms_message %} {% from "components/email-message.html" import email_message %} +{% from "components/letter.html" import letter %} {% 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 %} @@ -38,6 +39,8 @@ recipient='', show_placeholder_for_recipient=True ) }} + {% elif 'letter' == template.template_type %} + {{ letter(template.formatted_as_markup) }} {% endif %}
diff --git a/app/templates/views/send.html b/app/templates/views/send.html index 87309f156..398ea7eb6 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -1,6 +1,7 @@ {% extends "withnav_template.html" %} {% from "components/sms-message.html" import sms_message %} {% from "components/email-message.html" import email_message %} +{% from "components/letter.html" import letter %} {% from "components/page-footer.html" import page_footer %} {% from "components/file-upload.html" import file_upload %} {% from "components/table.html" import list_table, text_field, index_field, index_field_heading %} @@ -30,6 +31,8 @@ recipient='', show_placeholder_for_recipient=True ) }} + {% elif 'letter' == template.template_type %} + {{ letter(template.formatted_as_markup) }} {% endif %}
diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index 50c01ef71..0eeafe408 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -1,5 +1,6 @@ {% from "components/email-message.html" import email_message %} {% from "components/sms-message.html" import sms_message %} +{% from "components/letter.html" import letter %}
{% if 'email' == template.template_type %} @@ -14,6 +15,8 @@ template.formatted_as_markup, id=template.id ) }} + {% elif 'letter' == template.template_type %} + {{ letter(template.formatted_as_markup) }} {% endif %}
diff --git a/app/templates/views/templates/template.html b/app/templates/views/templates/template.html index 3f73a33ae..f75443f6e 100644 --- a/app/templates/views/templates/template.html +++ b/app/templates/views/templates/template.html @@ -21,7 +21,9 @@ {{ page_footer( secondary_link=url_for('.choose_template', service_id=current_service.id, template_type=template.template_type), - secondary_link_text='All {} templates'.format('email' if 'email' == template.template_type else 'text message') + secondary_link_text='All {} templates'.format( + 'email' if 'email' == template.template_type else 'text message' if 'sms' == template.template_type else 'letter' + ) ) }} {% endblock %} diff --git a/tests/app/main/views/test_letters.py b/tests/app/main/views/test_letters.py index 0051d3df3..cf253dd52 100644 --- a/tests/app/main/views/test_letters.py +++ b/tests/app/main/views/test_letters.py @@ -1,34 +1,51 @@ import pytest from flask import url_for +from functools import partial from tests import service_json +letters_urls = [ + partial(url_for, 'main.choose_template', template_type='letter'), + partial(url_for, 'main.add_service_template', template_type='letter'), +] + +@pytest.mark.parametrize('url', letters_urls) @pytest.mark.parametrize('can_send_letters, response_code', [ (True, 200), (False, 403) ]) -def test_letters_access_restricted(logged_in_client, mocker, can_send_letters, response_code): +def test_letters_access_restricted( + logged_in_client, + mocker, + can_send_letters, + response_code, + mock_get_service_templates, + url +): service = service_json(can_send_letters=can_send_letters) mocker.patch('app.service_api_client.get_service', return_value={"data": service}) - response = logged_in_client.get(url_for('main.letters', service_id=service['id'])) + response = logged_in_client.get(url(service_id=service['id'])) assert response.status_code == response_code +@pytest.mark.parametrize('url', letters_urls) def test_letters_lets_in_without_permission( client, mocker, mock_login, mock_has_permissions, api_user_active, + mock_get_service_templates, + url ): service = service_json(can_send_letters=True) mocker.patch('app.service_api_client.get_service', return_value={"data": service}) client.login(api_user_active) - response = client.get(url_for('main.letters', service_id=service['id'])) + response = client.get(url(service_id=service['id'])) assert api_user_active.permissions == {} assert response.status_code == 200