From 18d11aa013968a6460ff0101cb65128774b97cdf Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 5 Dec 2016 11:51:19 +0000 Subject: [PATCH] =?UTF-8?q?Move=20code=20for=20rendering=20messages/templa?= =?UTF-8?q?tes=20=E2=86=92=20utils?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Utils is better structured to handle the logic of what thing to show for what template type, especially now that what we show for different template types in different contexts has diverged significantly. See https://github.com/alphagov/notifications-utils/commit/6b39c1a for an example of this code moving into utils Depends on and implements: https://github.com/alphagov/notifications-utils/pull/84 The main reason for doing this is to get Paul’s fix for the misaligned CSV columns: https://github.com/alphagov/notifications-utils/pull/87 --- .../stylesheets/components/sms-message.scss | 1 + app/main/views/send.py | 18 +++--- app/main/views/templates.py | 47 ++++++++------- app/templates/components/email-message.html | 60 ------------------- app/templates/components/letter.html | 7 --- app/templates/components/sms-message.html | 37 ------------ app/templates/views/check.html | 26 +------- app/templates/views/jobs/job.html | 17 +----- app/templates/views/send-from-api.html | 17 +----- app/templates/views/send-test.html | 24 +------- app/templates/views/send.html | 24 +------- app/templates/views/styleguide.html | 39 ------------ app/templates/views/templates/_template.html | 20 +------ .../views/templates/_template_history.html | 14 +---- .../views/templates/choose_history.html | 2 +- app/templates/views/templates/template.html | 2 - .../views/templates/template_history.html | 2 - app/utils.py | 19 ++++++ requirements.txt | 2 +- 19 files changed, 63 insertions(+), 315 deletions(-) delete mode 100644 app/templates/components/email-message.html delete mode 100644 app/templates/components/letter.html delete mode 100644 app/templates/components/sms-message.html diff --git a/app/assets/stylesheets/components/sms-message.scss b/app/assets/stylesheets/components/sms-message.scss index 9a60ecd5a..fda6d1116 100644 --- a/app/assets/stylesheets/components/sms-message.scss +++ b/app/assets/stylesheets/components/sms-message.scss @@ -2,6 +2,7 @@ .sms-message-wrapper { width: 100%; + max-width: 410px; box-sizing: border-box; padding: $gutter-half; background: $panel-colour; diff --git a/app/main/views/send.py b/app/main/views/send.py index 5e6f5b60c..281ce0d46 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -28,7 +28,7 @@ from app.main.uploader import ( s3download ) from app import job_api_client, service_api_client, current_service, user_api_client -from app.utils import user_has_permissions, get_errors_for_csv, Spreadsheet, get_help_argument +from app.utils import user_has_permissions, get_errors_for_csv, Spreadsheet, get_help_argument, get_renderer def get_page_headings(template_type): @@ -90,8 +90,7 @@ def choose_template(service_id, template_type): templates=[ Template( template, - prefix=current_service['name'], - sms_sender=current_service['sms_sender'] + renderer=get_renderer(template_type, current_service, show_recipient=False) ) for template in service_api_client.get_service_templates(service_id)['data'] if template['template_type'] == template_type ], @@ -105,10 +104,9 @@ def choose_template(service_id, template_type): @user_has_permissions('send_texts', 'send_emails', 'send_letters') def send_messages(service_id, template_id): template = Template( - service_api_client.get_service_template(service_id, template_id)['data'], - prefix=current_service['name'], - sms_sender=current_service['sms_sender'] + service_api_client.get_service_template(service_id, template_id)['data'] ) + template.renderer = get_renderer(template.template_type, current_service, show_recipient=True) form = CsvUploadForm() if form.validate_on_submit(): @@ -169,6 +167,8 @@ def send_test(service_id, template_id): sms_sender=current_service['sms_sender'] ) + template.renderer = get_renderer(template.template_type, current_service, show_recipient=True) + if len(template.placeholders) == 0 or request.method == 'POST': upload_id = s3upload( service_id, @@ -237,11 +237,11 @@ def check_messages(service_id, template_type, upload_id): service_api_client.get_service_template( service_id, session['upload_data'].get('template_id') - )['data'], - prefix=current_service['name'], - sms_sender=current_service['sms_sender'] + )['data'] ) + template.renderer = get_renderer(template_type, current_service, show_recipient=True) + recipients = RecipientCSV( contents, template_type=template.template_type, diff --git a/app/main/views/templates.py b/app/main/views/templates.py index a970a4b21..3146cbec0 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -10,7 +10,7 @@ from notifications_utils.recipients import first_column_headings from notifications_python_client.errors import HTTPError from app.main import main -from app.utils import user_has_permissions +from app.utils import user_has_permissions, get_renderer 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 @@ -39,13 +39,15 @@ page_headings = { admin_override=True, any_=True ) def view_template(service_id, template_id): + template = Template( + service_api_client.get_service_template(service_id, template_id)['data'] + ) + template.renderer = get_renderer( + template.template_type, current_service, show_recipient=False, expand_emails=True + ) return render_template( 'views/templates/template.html', - template=Template( - service_api_client.get_service_template(service_id, template_id)['data'], - prefix=current_service['name'], - sms_sender=current_service['sms_sender'] - ) + template=template ) @@ -61,13 +63,15 @@ def view_template(service_id, template_id): any_=True ) def view_template_version(service_id, template_id, version): + template = Template( + service_api_client.get_service_template(service_id, template_id, version)['data'] + ) + template.renderer = get_renderer( + template.template_type, current_service, show_recipient=False, expand_emails=True + ) return render_template( 'views/templates/template_history.html', - template=Template( - service_api_client.get_service_template(service_id, template_id, version)['data'], - prefix=current_service['name'], - sms_sender=current_service['sms_sender'] - ) + template=template ) @@ -227,19 +231,18 @@ def delete_service_template(service_id, template_id): any_=True ) def view_template_versions(service_id, template_id): + + versions = [] + for template in service_api_client.get_service_template_versions(service_id, template_id)['data']: + template = Template(template) + template.renderer = get_renderer( + template.template_type, current_service, show_recipient=False, expand_emails=True + ) + versions.append(template) + return render_template( 'views/templates/choose_history.html', - template=Template( - service_api_client.get_service_template(service_id, template_id)['data'], - prefix=current_service['name'] - ), - versions=[ - Template( - template, - prefix=current_service['name'], - sms_sender=current_service['sms_sender'] - ) for template in service_api_client.get_service_template_versions(service_id, template_id)['data'] - ] + versions=versions ) diff --git a/app/templates/components/email-message.html b/app/templates/components/email-message.html deleted file mode 100644 index 9766c17d6..000000000 --- a/app/templates/components/email-message.html +++ /dev/null @@ -1,60 +0,0 @@ -{% macro email_message( - subject, - body, - from_name=None, - from_address=None, - recipient=None, - id=None, - show_placeholder_for_recipient=False, - show_id=False, - expanded=False -) %} -
- {% if from_name or subject %} - - - {% if from_name and from_address %} - - - - - {% endif %} - {% if recipient is not none %} - - - - - {% endif %} - {% if subject %} - - - - - {% endif %} - - - {% endif %} - -
-{% endmacro %} diff --git a/app/templates/components/letter.html b/app/templates/components/letter.html deleted file mode 100644 index 8440ba31b..000000000 --- a/app/templates/components/letter.html +++ /dev/null @@ -1,7 +0,0 @@ -{% macro letter( - body -) %} -
- {{ body }} -
-{% endmacro %} diff --git a/app/templates/components/sms-message.html b/app/templates/components/sms-message.html deleted file mode 100644 index d062a479f..000000000 --- a/app/templates/components/sms-message.html +++ /dev/null @@ -1,37 +0,0 @@ -{% macro sms_message( - body, - recipient=None, - id=None, - from=None, - version=1, - updated_at=None, - versions_url=None, - show_placeholder_for_recipient=False, - show_id=False -) %} - {% if recipient is not none %} -

- To: - {% if show_placeholder_for_recipient %} - - phone number - - {% else %} - {{ recipient }} - {% endif %} -

- {% endif %} -
- {% if from %} - - {{ from }} - - {% endif %} - {{ body|nl2br }} -
- {% if show_id %} -

- Template ID: {{ id }} -

- {% endif %} -{% endmacro %} diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 5c3410a8f..79b9527ca 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -1,9 +1,6 @@ {% extends "withnav_template.html" %} {% from "components/banner.html" import banner_wrapper %} -{% 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 %} @@ -130,28 +127,7 @@ {% endif %} - {% if 'email' == template.template_type %} - {{ email_message( - template.formatted_subject_as_markup if errors else template.replaced_subject, - template.formatted_as_markup if errors else template.replaced|safe, - from_address='{}@notifications.service.gov.uk'.format(current_service.email_from), - from_name=current_service.name, - recipient=first_recipient, - show_placeholder_for_recipient=errors - )}} - {% elif 'sms' == template.template_type %} -
-
- {{ sms_message( - template.formatted_as_markup if errors else template.replaced|safe, - recipient=first_recipient, - show_placeholder_for_recipient=errors - )}} -
-
- {% elif 'letter' == template.template_type %} - {{ letter(template.formatted_as_markup if errors else template.replaced|safe) }} - {% endif %} + {{ template.rendered }} {% if errors %} {% if request.args.from_test %} diff --git a/app/templates/views/jobs/job.html b/app/templates/views/jobs/job.html index 061580509..70388542d 100644 --- a/app/templates/views/jobs/job.html +++ b/app/templates/views/jobs/job.html @@ -1,7 +1,5 @@ {% extends "withnav_template.html" %} {% from "components/banner.html" import banner %} -{% from "components/sms-message.html" import sms_message %} -{% from "components/email-message.html" import email_message %} {% from "components/ajax-block.html" import ajax_block %} {% block page_title %} @@ -14,20 +12,7 @@ {{ uploaded_file_name }} - {% 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 - )}} - {% endif %} + {{ template.rendered }} {{ ajax_block(partials, updates_url, 'status', finished=finished) }} {{ ajax_block(partials, updates_url, 'counts', finished=finished) }} diff --git a/app/templates/views/send-from-api.html b/app/templates/views/send-from-api.html index 266bdb088..34bc0cd13 100644 --- a/app/templates/views/send-from-api.html +++ b/app/templates/views/send-from-api.html @@ -1,6 +1,4 @@ {% extends "withnav_template.html" %} -{% from "components/email-message.html" import email_message %} -{% from "components/sms-message.html" import sms_message %} {% from "components/api-key.html" import api_key %} {% block page_title %} @@ -13,20 +11,7 @@ API info - {% if 'email' == template.template_type %} - {{ email_message( - template.formatted_subject_as_markup, - template.formatted_as_markup, - ) }} - {% elif 'sms' == template.template_type %} -
-
- {{ sms_message( - template.formatted_as_markup, - ) }} -
-
- {% endif %} + {{ template.rendered }}
{{ api_key(template.id, name="Template ID", thing='template ID') }} diff --git a/app/templates/views/send-test.html b/app/templates/views/send-test.html index 4f8fa4980..2c2820116 100644 --- a/app/templates/views/send-test.html +++ b/app/templates/views/send-test.html @@ -1,7 +1,4 @@ {% 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 %} @@ -22,26 +19,7 @@

Send yourself a test

{% endif %} - {% if 'sms' == template.template_type %} -
-
- {{ sms_message( - template.formatted_as_markup, - recipient='', - show_placeholder_for_recipient=True - ) }} -
-
- {% elif 'email' == template.template_type %} - {{ email_message( - template.formatted_subject_as_markup, - template.formatted_as_markup, - recipient='', - show_placeholder_for_recipient=True - ) }} - {% elif 'letter' == template.template_type %} - {{ letter(template.formatted_as_markup) }} - {% endif %} + {{ template.rendered }}
{% call(item, row_number) list_table( diff --git a/app/templates/views/send.html b/app/templates/views/send.html index 398ea7eb6..e0a2eb9ba 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -1,7 +1,4 @@ {% 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 %} @@ -14,26 +11,7 @@

Upload recipients

- {% if 'sms' == template.template_type %} -
-
- {{ sms_message( - template.formatted_as_markup, - recipient='', - show_placeholder_for_recipient=True - ) }} -
-
- {% elif 'email' == template.template_type %} - {{ email_message( - template.formatted_subject_as_markup, - template.formatted_as_markup, - recipient='', - show_placeholder_for_recipient=True - ) }} - {% elif 'letter' == template.template_type %} - {{ letter(template.formatted_as_markup) }} - {% endif %} + {{ template.rendered }}
{{file_upload( diff --git a/app/templates/views/styleguide.html b/app/templates/views/styleguide.html index 5a5ce64e4..650ec5254 100644 --- a/app/templates/views/styleguide.html +++ b/app/templates/views/styleguide.html @@ -4,8 +4,6 @@ {% from "components/big-number.html" import big_number %} {% from "components/browse-list.html" import browse_list %} {% from "components/page-footer.html" import page_footer %} -{% from "components/sms-message.html" import sms_message %} -{% from "components/email-message.html" import email_message %} {% from "components/table.html" import mapping_table, list_table, row, field, text_field, boolean_field, right_aligned_field_heading %} {% from "components/textbox.html" import textbox %} {% from "components/file-upload.html" import file_upload %} @@ -111,43 +109,6 @@ button_text='Sign in', secondary_link='http://example.com', secondary_link_text="I’ve forgotten my password" ) }} -

SMS message

- -

Used to show, preview or choose an SMS template.

- -
-
- {{ sms_message( - 'Your vehicle tax for LC12 BFL is due on 1 March 2016. Renew online at www.gov.uk/vehicle-tax', - ) }} - {{ sms_message( - template.formatted_as_markup - ) }} - {{ sms_message( - 'Your vehicle tax for LC12 BFL is due on 1 March 2016. Renew online at www.gov.uk/vehicle-tax', - '+44 7700 900 306' - ) }} - {{ sms_message( - template.formatted_as_markup - ) }} -
-
- -

Email message

- -

Used to show, preview or choose an email template.

- -
-
- {{ email_message( - subject="Vehicle tax reminder", - body="Dear Alice Smith,\n\nYour vehicle tax for LC12 BFL is due on 1 March 2016.\n\nRenew online at www.gov.uk/vehicle-tax", - from_name="Vehicle tax", - from_address="vehicle.tax@notifications.service.gov.uk" - ) }} -
-
-

Tables

diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index 0eeafe408..bd1355c7c 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -1,23 +1,5 @@ -{% 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 %} - {{ email_message( - template.formatted_subject_as_markup, - template.formatted_as_markup, - id=template.id, - expanded=expanded - ) }} - {% elif 'sms' == template.template_type %} - {{ sms_message( - template.formatted_as_markup, - id=template.id - ) }} - {% elif 'letter' == template.template_type %} - {{ letter(template.formatted_as_markup) }} - {% endif %} + {{ template.rendered }}
{% if template._template.archived %} diff --git a/app/templates/views/templates/_template_history.html b/app/templates/views/templates/_template_history.html index e0dba731c..ec90b46f7 100644 --- a/app/templates/views/templates/_template_history.html +++ b/app/templates/views/templates/_template_history.html @@ -1,21 +1,9 @@ -{% from "components/email-message.html" import email_message %} -{% from "components/sms-message.html" import sms_message %} -

{{ template.name }}

- {% if 'email' == template.template_type %} - {{ email_message( - template.formatted_subject_as_markup, - template.formatted_as_markup, - ) }} - {% elif 'sms' == template.template_type %} - {{ sms_message( - template.formatted_as_markup - ) }} - {% endif %} + {{ template.rendered }}
diff --git a/app/templates/views/templates/choose_history.html b/app/templates/views/templates/choose_history.html index a2dbe6e16..9d112a6cd 100644 --- a/app/templates/views/templates/choose_history.html +++ b/app/templates/views/templates/choose_history.html @@ -1,7 +1,7 @@ {% extends "withnav_template.html" %} {% block page_title %} - {{ template.name }} – GOV.UK Notify + Previous versions – GOV.UK Notify {% endblock %} {% block maincolumn_content %} diff --git a/app/templates/views/templates/template.html b/app/templates/views/templates/template.html index f75443f6e..7a6266f13 100644 --- a/app/templates/views/templates/template.html +++ b/app/templates/views/templates/template.html @@ -1,6 +1,4 @@ {% extends "withnav_template.html" %} -{% from "components/email-message.html" import email_message %} -{% from "components/sms-message.html" import sms_message %} {% from "components/page-footer.html" import page_footer %} {% from "components/textbox.html" import textbox %} diff --git a/app/templates/views/templates/template_history.html b/app/templates/views/templates/template_history.html index b462435a6..c726d9bf1 100644 --- a/app/templates/views/templates/template_history.html +++ b/app/templates/views/templates/template_history.html @@ -1,6 +1,4 @@ {% extends "withnav_template.html" %} -{% from "components/email-message.html" import email_message %} -{% from "components/sms-message.html" import sms_message %} {% from "components/page-footer.html" import page_footer %} {% from "components/textbox.html" import textbox %} diff --git a/app/utils.py b/app/utils.py index 3108fc40f..367d09cfc 100644 --- a/app/utils.py +++ b/app/utils.py @@ -8,6 +8,8 @@ import unicodedata from flask import (abort, current_app, session, request, redirect, url_for) from flask_login import current_user +from notifications_utils.renderers import SMSPreview, EmailPreview, LetterPreview + import pyexcel import pyexcel.ext.io import pyexcel.ext.xls @@ -216,3 +218,20 @@ def is_gov_user(email_address): valid_domains = current_app.config['EMAIL_DOMAIN_REGEXES'] email_regex = (r"[\.|@]({})$".format("|".join(valid_domains))) return bool(re.search(email_regex, email_address.lower())) + + +def get_renderer(template_type, service, show_recipient, expand_emails=False): + return { + 'email': EmailPreview( + from_name=service['name'], + from_address='{}@notifications.service.gov.uk'.format(service['email_from']), + expanded=expand_emails, + show_recipient=show_recipient + ), + 'sms': SMSPreview( + prefix=service['name'], + sender=service['sms_sender'], + show_recipient=show_recipient + ), + 'letter': LetterPreview(), + }[template_type] diff --git a/requirements.txt b/requirements.txt index 8a1833fb9..f430f8f6e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -18,4 +18,4 @@ pytz==2016.4 git+https://github.com/alphagov/notifications-python-client.git@3.0.1#egg=notifications-python-client==3.0.1 -git+https://github.com/alphagov/notifications-utils.git@10.3.1#egg=notifications-utils==10.3.1 +git+https://github.com/alphagov/notifications-utils.git@11.0.2#egg=notifications-utils==11.0.2