From c6de605311ded7a11d2d9e2d78fc6db1f3ab108a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 22 Feb 2016 14:45:13 +0000 Subject: [PATCH] Add basic flow for adding email _or_ sms templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Templates now have: - a type (email or sms) - a subject (if they are email templates) We don’t want two completely separate view files for email and SMS, because they would have an enormous amount of repetition. So this commit adds - different templates for SMS and email templates - different form objects for SMS and email templates …and wires them up. --- app/main/dao/templates_dao.py | 6 +- app/main/forms.py | 9 ++- app/main/views/email.py | 0 app/main/views/index.py | 12 --- app/main/views/sms.py | 21 +++--- app/main/views/templates.py | 74 ++++++++++--------- app/templates/main_nav.html | 4 +- .../views/choose-email-template.html | 38 ++++++++++ app/templates/views/choose-sms-template.html | 2 +- ...template.html => edit-email-template.html} | 7 +- app/templates/views/edit-sms-template.html | 43 +++++++++++ app/templates/views/send-email.html | 17 ----- tests/app/main/views/test_sms.py | 2 +- tests/app/main/views/test_templates.py | 32 ++------ 14 files changed, 160 insertions(+), 107 deletions(-) create mode 100644 app/main/views/email.py create mode 100644 app/templates/views/choose-email-template.html rename app/templates/views/{edit-template.html => edit-email-template.html} (80%) create mode 100644 app/templates/views/edit-sms-template.html delete mode 100644 app/templates/views/send-email.html diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py index d34f2b187..b7ce664f8 100644 --- a/app/main/dao/templates_dao.py +++ b/app/main/dao/templates_dao.py @@ -4,12 +4,12 @@ from app.utils import BrowsableItem from notifications_python_client.errors import HTTPError -def insert_service_template(name, content, service_id): +def insert_service_template(name, content, service_id, subject=None): return notifications_api_client.create_service_template( - name, 'sms', content, service_id) + name, 'sms' if subject is None else 'email', content, service_id, subject) -def update_service_template(id_, name, content, service_id): +def update_service_template(id_, name, content, service_id, subject=None): return notifications_api_client.update_service_template( id_, name, 'sms', content, service_id) diff --git a/app/main/forms.py b/app/main/forms.py index 20b007514..ea2e1cd69 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -188,7 +188,7 @@ class ConfirmPasswordForm(Form): raise ValidationError('Invalid password') -class TemplateForm(Form): +class SMSTemplateForm(Form): name = StringField( u'Template name', validators=[DataRequired(message="Template name cannot be empty")]) @@ -198,6 +198,13 @@ class TemplateForm(Form): validators=[DataRequired(message="Template content cannot be empty")]) +class EmailTemplateForm(SMSTemplateForm): + + subject = StringField( + u'Subject', + validators=[DataRequired(message="Subject cannot be empty")]) + + class ForgotPasswordForm(Form): email_address = email_address() diff --git a/app/main/views/email.py b/app/main/views/email.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/main/views/index.py b/app/main/views/index.py index 1200cf65b..8335284cd 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -22,15 +22,3 @@ def register_from_invite(): @login_required def verify_mobile(): return render_template('views/verify-mobile.html') - - -@main.route("/services//send-email") -@login_required -def send_email(service_id): - return render_template('views/send-email.html', service_id=service_id) - - -@main.route("/services//check-email") -@login_required -def check_email(service_id): - return render_template('views/check-email.html') diff --git a/app/main/views/sms.py b/app/main/views/sms.py index 42340cbaa..63b947218 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -35,8 +35,10 @@ from app.utils import ( ) -@main.route("/services//sms/send", methods=['GET']) -def choose_sms_template(service_id): +@main.route("/services//send/", methods=['GET']) +def choose_template(service_id, template_type): + if template_type not in ['email', 'sms']: + abort(404) try: jobs = job_api_client.get_job(service_id)['data'] except HTTPError as e: @@ -44,21 +46,18 @@ def choose_sms_template(service_id): abort(404) else: raise e - print("="*80) - print(jobs) - print(len(jobs)) - print(bool(len(jobs))) return render_template( - 'views/choose-sms-template.html', + 'views/choose-{}-template.html'.format(template_type), templates=[ Template(template) for template in templates_dao.get_service_templates(service_id)['data'] + if template['template_type'] == template_type ], has_jobs=len(jobs), service_id=service_id ) -@main.route("/services//sms/send/", methods=['GET', 'POST']) +@main.route("/services//send/", methods=['GET', 'POST']) @login_required def send_sms(service_id, template_id): @@ -91,7 +90,7 @@ def send_sms(service_id, template_id): ) -@main.route("/services//sms/send/.csv", methods=['GET']) +@main.route("/services//send/.csv", methods=['GET']) @login_required def get_example_csv(service_id, template_id): template = templates_dao.get_service_template_or_404(service_id, template_id)['data'] @@ -104,7 +103,7 @@ def get_example_csv(service_id, template_id): return(output.getvalue(), 200, {'Content-Type': 'text/csv; charset=utf-8'}) -@main.route("/services//sms/send//to-self", methods=['GET']) +@main.route("/services//send//to-self", methods=['GET']) @login_required def send_sms_to_self(service_id, template_id): template = templates_dao.get_service_template_or_404(service_id, template_id)['data'] @@ -126,7 +125,7 @@ def send_sms_to_self(service_id, template_id): upload_id=upload_id)) -@main.route("/services//sms/check/", +@main.route("/services//check/", methods=['GET', 'POST']) @login_required def check_sms(service_id, upload_id): diff --git a/app/main/views/templates.py b/app/main/views/templates.py index be1001113..b291e1b8c 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -5,45 +5,44 @@ from notifications_python_client.errors import HTTPError from utils.template import Template from app.main import main -from app.main.forms import TemplateForm +from app.main.forms import SMSTemplateForm, EmailTemplateForm from app import job_api_client -from app.main.dao.services_dao import get_service_by_id +from app.main.dao.services_dao import get_service_by_id_or_404 from app.main.dao import templates_dao as tdao from app.main.dao import services_dao as sdao -@main.route("/services//templates") +form_objects = { + 'email': EmailTemplateForm, + 'sms': SMSTemplateForm +} + + +@main.route("/services//templates/add-", methods=['GET', 'POST']) @login_required -def manage_service_templates(service_id): - return redirect(url_for( - '.choose_sms_template', - service_id=service_id - )) +def add_service_template(service_id, template_type): + service = sdao.get_service_by_id_or_404(service_id) -@main.route("/services//templates/add", methods=['GET', 'POST']) -@login_required -def add_service_template(service_id): - try: - service = sdao.get_service_by_id(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + if template_type not in ['sms', 'email']: + abort(404) - form = TemplateForm() + form = form_objects[template_type]() if form.validate_on_submit(): tdao.insert_service_template( - form.name.data, form.template_content.data, service_id) - return redirect(url_for( - '.choose_sms_template', service_id=service_id)) + form.name.data, form.template_content.data, service_id, form.subject.data or None + ) + return redirect( + url_for('.choose_template', service_id=service_id, template_type=template_type) + ) + return render_template( - 'views/edit-template.html', - h1='Add a text message template', + 'views/edit-{}-template.html'.format(template_type), form=form, - service_id=service_id) + template_type=template_type, + service_id=service_id + ) @main.route("/services//templates/", methods=['GET', 'POST']) @@ -51,20 +50,25 @@ def add_service_template(service_id): def edit_service_template(service_id, template_id): template = tdao.get_service_template_or_404(service_id, template_id)['data'] template['template_content'] = template['content'] - form = TemplateForm(**template) + form = form_objects[template['template_type']](**template) if form.validate_on_submit(): tdao.update_service_template( template_id, form.name.data, form.template_content.data, service_id) - return redirect(url_for('.choose_sms_template', service_id=service_id)) + return redirect(url_for( + '.choose_template', + service_id=service_id, + template_type=template['template_type'] + )) return render_template( - 'views/edit-template.html', - h1='Edit template', + 'views/edit-{}-template.html'.format(template['template_type']), form=form, service_id=service_id, - template_id=template_id) + template_id=template_id, + template_type=template['template_type'] + ) @main.route("/services//templates//delete", methods=['GET', 'POST']) @@ -74,13 +78,17 @@ def delete_service_template(service_id, template_id): if request.method == 'POST': tdao.delete_service_template(service_id, template_id) - return redirect(url_for('.manage_service_templates', service_id=service_id)) + return redirect(url_for( + '.choose_template', + service_id=service_id, + template_type=template['template_type'] + )) template['template_content'] = template['content'] - form = TemplateForm(**template) + form = form_objects[template['template_type']](**template) flash('Are you sure you want to delete ‘{}’?'.format(form.name.data), 'delete') return render_template( - 'views/edit-template.html', + 'views/edit-{}-template.html'.format(template['template_type']), h1='Edit template', form=form, service_id=service_id, diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 4a379f3d8..a08a24a4a 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -3,8 +3,8 @@ {{ session.get('service_name', 'Service') }}
  • Manage team
    • diff --git a/app/templates/views/choose-email-template.html b/app/templates/views/choose-email-template.html new file mode 100644 index 000000000..06ed1dec6 --- /dev/null +++ b/app/templates/views/choose-email-template.html @@ -0,0 +1,38 @@ +{% extends "withnav_template.html" %} +{% from "components/email-message.html" import email_message %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/textbox.html" import textbox %} + +{% block page_title %} + Send emails – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

    Send emails

    + +
    + + {% if templates %} +
    + {% for template in templates %} +
    + {{ email_message(template.subject, template.formatted_as_markup, name=template.name) }} +
    +
    +
    + Add recipients + Send yourself a test + Edit template +
    +
    + {% endfor %} +
    + {% endif %} + +

    + Add a new template +

    + +
    +{% endblock %} diff --git a/app/templates/views/choose-sms-template.html b/app/templates/views/choose-sms-template.html index 221fa781b..b83217a7c 100644 --- a/app/templates/views/choose-sms-template.html +++ b/app/templates/views/choose-sms-template.html @@ -40,7 +40,7 @@ {% endif %}

    - Add a new template + Add a new template

    diff --git a/app/templates/views/edit-template.html b/app/templates/views/edit-email-template.html similarity index 80% rename from app/templates/views/edit-template.html rename to app/templates/views/edit-email-template.html index 4720a2a61..b0c983f65 100644 --- a/app/templates/views/edit-template.html +++ b/app/templates/views/edit-email-template.html @@ -8,12 +8,15 @@ {% block maincolumn_content %} -

    {{ h1 }}

    +

    Edit email template

    {{ textbox(form.name, width='1-1') }} + {% if 'email' == template_type %} + {{ textbox(form.subject, width='1-1') }} + {% endif %}
    @@ -31,7 +34,7 @@ 'Save', delete_link=url_for('.delete_service_template', service_id=service_id, template_id=template_id) if template_id or None, delete_link_text='Delete this template', - back_link=url_for('.choose_sms_template', service_id=service_id), + back_link=url_for('.choose_template', template_type=template_type, service_id=service_id), back_link_text='Cancel' ) }} diff --git a/app/templates/views/edit-sms-template.html b/app/templates/views/edit-sms-template.html new file mode 100644 index 000000000..8c8d5e1c5 --- /dev/null +++ b/app/templates/views/edit-sms-template.html @@ -0,0 +1,43 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} + {{ h1 }} – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

    Edit text message template

    + +
    +
    +
    + {{ textbox(form.name, width='1-1') }} + {% if 'email' == template_type %} + {{ textbox(form.subject, width='1-1') }} + {% endif %} +
    +
    +
    +
    + {{ textbox(form.template_content, highlight_tags=True, width='1-1') }} +
    +
    + +
    +
    + {{ page_footer( + 'Save', + delete_link=url_for('.delete_service_template', service_id=service_id, template_id=template_id) if template_id or None, + delete_link_text='Delete this template', + back_link=url_for('.choose_template', service_id=service_id, template_type=template_type), + back_link_text='Cancel' + ) }} +
    + + +{% endblock %} diff --git a/app/templates/views/send-email.html b/app/templates/views/send-email.html deleted file mode 100644 index 3fe253f32..000000000 --- a/app/templates/views/send-email.html +++ /dev/null @@ -1,17 +0,0 @@ -{% extends "withnav_template.html" %} - -{% block page_title %} -Send email – GOV.UK Notify -{% endblock %} - -{% block maincolumn_content %} - -

    Send email

    - -

    This page will be where we construct email messages

    - -

    - Continue -

    - -{% endblock %} diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py index 6f9656e25..c48aeed26 100644 --- a/tests/app/main/views/test_sms.py +++ b/tests/app/main/views/test_sms.py @@ -14,7 +14,7 @@ def test_choose_sms_template(app_, with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) - response = client.get(url_for('main.choose_sms_template', service_id=12345)) + response = client.get(url_for('main.choose_template', template_type='sms', service_id=12345)) assert response.status_code == 200 content = response.get_data(as_text=True) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index cecc59353..1ec43d502 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -4,24 +4,6 @@ import uuid from flask import url_for -def test_should_return_list_of_all_templates(app_, - api_user_active, - mock_get_service_templates, - mock_get_user, - mock_get_user_by_email, - mock_login, - mock_get_jobs): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - service_id = str(uuid.uuid4()) - response = client.get(url_for( - '.manage_service_templates', service_id=service_id), follow_redirects=True) - - assert response.status_code == 200 - mock_get_service_templates.assert_called_with(service_id) - - def test_should_show_page_for_one_templates(app_, api_user_active, mock_get_service_template, @@ -62,8 +44,9 @@ def test_should_redirect_when_saving_a_template(app_, data = { 'id': template_id, 'name': name, - "template_content": content, - "service": service_id + 'template_content': content, + 'type': 'sms', + 'service': service_id } response = client.post(url_for( '.edit_service_template', @@ -72,7 +55,7 @@ def test_should_redirect_when_saving_a_template(app_, assert response.status_code == 302 assert response.location == url_for( - '.choose_sms_template', service_id=service_id, _external=True) + '.choose_template', service_id=service_id, template_type='sms', _external=True) mock_update_service_template.assert_called_with( template_id, name, 'sms', content, service_id) @@ -127,12 +110,13 @@ def test_should_redirect_when_deleting_a_template(app_, response = client.post(url_for( '.delete_service_template', service_id=service_id, - template_id=template_id), data=data) + template_id=template_id + ), data=data) assert response.status_code == 302 assert response.location == url_for( - '.manage_service_templates', - service_id=service_id, _external=True) + '.choose_template', + service_id=service_id, template_type=type_, _external=True) mock_get_service_template.assert_called_with( service_id, template_id) mock_delete_service_template.assert_called_with(