From 9d38cd98b0ea77e1e8f7179082d27423fb1dacee Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 18 Feb 2016 14:54:59 +0000 Subject: [PATCH] Preview service name when adding a new service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds a new page, which appears after a user enters the name for their new service. It shows how the service name will appear in emails and text messages. This means that the new service is not created until after they have confirmed that the name is appropriate in context. This has also involved: - visual changes to the ‘email template’ pattern, which wasn’t very refined before - removing a bunch of words from the enter service name page, because most users don’t read them, and we reckon that showing a preview is a better way of getting them to understand what is meant by service name Still to do: - validating the the generated email address for a service is unique (on the API) side - having the API return the generated email address, rather than determining it in the admin app --- app/assets/stylesheets/_grids.scss | 14 +++-- .../stylesheets/components/email-message.scss | 36 +++++++------ .../stylesheets/components/sms-message.scss | 6 +++ .../stylesheets/components/textbox.scss | 4 ++ app/main/dao/services_dao.py | 11 ++++ app/main/views/add_service.py | 30 +++++++++-- app/templates/components/email-message.html | 29 ++++++++-- app/templates/components/sms-message.html | 7 ++- app/templates/components/textbox.html | 8 ++- app/templates/views/add-from-address.html | 42 +++++++++++++++ app/templates/views/add-service.html | 20 +++---- tests/app/main/views/test_add_service.py | 54 ++++++++++++++++--- 12 files changed, 211 insertions(+), 50 deletions(-) create mode 100644 app/templates/views/add-from-address.html diff --git a/app/assets/stylesheets/_grids.scss b/app/assets/stylesheets/_grids.scss index e349e22bb..7f8eb724f 100644 --- a/app/assets/stylesheets/_grids.scss +++ b/app/assets/stylesheets/_grids.scss @@ -1,11 +1,17 @@ -.column-one-quarter { - @include grid-column(1/4); -} - .column-three-quarters { @include grid-column(3/4); } + +.column-one-eighth { + @include grid-column(1/8); +} + +.column-seven-eighths { + @include grid-column(7/8); +} + .bottom-gutter { margin-bottom: $gutter; + clear: both; } diff --git a/app/assets/stylesheets/components/email-message.scss b/app/assets/stylesheets/components/email-message.scss index 2eec50050..b4771841a 100644 --- a/app/assets/stylesheets/components/email-message.scss +++ b/app/assets/stylesheets/components/email-message.scss @@ -1,24 +1,30 @@ .email-message { margin-bottom: $gutter; - border: 1px solid $border-colour; - - &-subject { - @include bold-19; - border-bottom: 1px solid $border-colour; - padding: 10px; - } - - &-body { - border-bottom: 1px solid $white; - padding: 10px; - overflow: hidden; - max-height: 103px; - } &-name { @include bold-19; - margin: 50px 0 10px 0; + margin: 20px 0 10px 0; + } + + &-subject, + &-from { + margin: 10px 0; + } + + &-from { + padding-top: 15px; + border-top: 1px solid $border-colour; + } + + &-body { + width: 100%; + box-sizing: border-box; + padding: $gutter-half 0 0 0; + margin: 0 0 $gutter 0; + clear: both; + border-top: 1px solid $border-colour; + border-bottom: 1px solid $border-colour; } } diff --git a/app/assets/stylesheets/components/sms-message.scss b/app/assets/stylesheets/components/sms-message.scss index 9fbe0b1af..898d04c24 100644 --- a/app/assets/stylesheets/components/sms-message.scss +++ b/app/assets/stylesheets/components/sms-message.scss @@ -8,6 +8,7 @@ border-radius: 5px; white-space: normal; margin: 0 0 $gutter 0; + clear: both; } .sms-message-wrapper-with-radio { @@ -53,3 +54,8 @@ } } + +.sms-message-from { + @include bold-19; + display: block; +} diff --git a/app/assets/stylesheets/components/textbox.scss b/app/assets/stylesheets/components/textbox.scss index d340dc494..efb692c27 100644 --- a/app/assets/stylesheets/components/textbox.scss +++ b/app/assets/stylesheets/components/textbox.scss @@ -85,3 +85,7 @@ .textbox-help-link { margin: 5px 0 0 0; } + +.textbox-right-aligned { + text-align: right; +} diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 52c217679..604b2e27d 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -1,5 +1,6 @@ from flask import url_for from app import notifications_api_client +from notifications_python_client.errors import HTTPError from app.utils import BrowsableItem @@ -26,6 +27,16 @@ def get_service_by_id(id_): return notifications_api_client.get_service(id_) +def get_service_by_id_or_404(id_): + try: + return get_service_by_id(id_) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + + def get_services(user_id=None): if user_id: return notifications_api_client.get_services({'user_id': str(user_id)}) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index dc7a3a7d4..ad5d8f715 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -1,4 +1,6 @@ -from flask import render_template, redirect, session, url_for +import re + +from flask import render_template, request, redirect, session, url_for from flask_login import login_required, current_user from app.main import main from app.main.dao import services_dao, users_dao @@ -15,13 +17,33 @@ def add_service(): else: heading = 'Add a new service' if form.validate_on_submit(): - user = users_dao.get_user_by_id(session['user_id']) - service_id = services_dao.insert_new_service(form.name.data, user.id) session['service_name'] = form.name.data - return redirect(url_for('main.service_dashboard', service_id=service_id)) + return redirect(url_for('main.add_from_address')) else: return render_template( 'views/add-service.html', form=form, heading=heading ) + + +@main.route("/confirm-add-service", methods=['GET', 'POST']) +@login_required +def add_from_address(): + if request.method == 'POST': + user = users_dao.get_user_by_id(session['user_id']) + service_id = services_dao.insert_new_service(session['service_name'], user.id) + return redirect(url_for('main.service_dashboard', service_id=service_id)) + else: + return render_template( + 'views/add-from-address.html', + service_name=session['service_name'], + from_address="{}@notifications.service.gov.uk".format(_email_safe(session['service_name'])) + ) + + +def _email_safe(string): + return "".join([ + character.lower() if character.isalnum() or character == "." else "" + for character in re.sub("\s+", ".", string.strip()) + ]) diff --git a/app/templates/components/email-message.html b/app/templates/components/email-message.html index 6d9920121..b619b16fa 100644 --- a/app/templates/components/email-message.html +++ b/app/templates/components/email-message.html @@ -1,4 +1,4 @@ -{% macro email_message(subject, body, name=None, edit_link=None) %} +{% macro email_message(subject, body, name=None, edit_link=None, from_name=None, from_address=None) %} {% if name %}

{% if edit_link %} @@ -9,9 +9,30 @@

{% endif %}
- + {% if from_name and from_address %} + + {% endif %} + {% if subject %} + + {% endif %} diff --git a/app/templates/components/sms-message.html b/app/templates/components/sms-message.html index 490254485..edd64887f 100644 --- a/app/templates/components/sms-message.html +++ b/app/templates/components/sms-message.html @@ -1,5 +1,5 @@ {% macro sms_message( - body, recipient=None, name=None, id=None, edit_link=None + body, recipient=None, name=None, id=None, edit_link=None, from=None ) %} {% if name %}

@@ -11,6 +11,11 @@

{% endif %}
+ {% if from %} + + {{ from }} + + {% endif %} {{ body }}
{% if recipient %} diff --git a/app/templates/components/textbox.html b/app/templates/components/textbox.html index cffcd48c9..b50fb9ee4 100644 --- a/app/templates/components/textbox.html +++ b/app/templates/components/textbox.html @@ -5,7 +5,8 @@ autofocus=False, help_link=None, help_link_text=None, - width='2-3' + width='2-3', + suffix=None ) %}
{{ field(**{ - 'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{}'.format(width), + 'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''), 'data-module': 'highlight-tags' if highlight_tags else '' }) }} + {% if suffix %} + {{ suffix }} + {% endif %} {% if help_link and help_link_text %}

{{ help_link_text }} diff --git a/app/templates/views/add-from-address.html b/app/templates/views/add-from-address.html new file mode 100644 index 000000000..a67f3c6d3 --- /dev/null +++ b/app/templates/views/add-from-address.html @@ -0,0 +1,42 @@ +{% extends "withoutnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/sms-message.html" import sms_message %} +{% from "components/email-message.html" import email_message %} + +{% block page_title %} + Preview your service name – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

+ Preview your service name +

+ +
+
+ {{ sms_message( + "{}: we received your payment, thank you".format(service_name), + name="Text message", + recipient='Sent from 40604' + ) }} +
+
+
+
+ {{ email_message( + subject="We received your payment, thank you", + body="Dear Alice Smith,\n\nThank you for…", + from_name=service_name, + from_address=from_address, + name="Email", + ) }} +
+
+ +
+ {{page_footer('Looks good', back_link=url_for(".add_service"))}} +
+ +{% endblock %} diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index 9502ae7d1..f0b4a5ff7 100644 --- a/app/templates/views/add-service.html +++ b/app/templates/views/add-service.html @@ -12,27 +12,21 @@

- {{ heading }} + When people receive notifications, who should they be from?

- Users will see your service name when they receive messages through GOV.UK - Notify: + Be specific. Remember that there might be other people in your + organisation using GOV.UK Notify.

-
    -
  • - at the start of every text message, eg ‘Vehicle tax: we received your - payment, thank you’ -
    • -
  • - as your email sender name -
    • -
-
+ {{ textbox(form.name, hint="You can change this later") }} + + {{ page_footer('Continue') }} +
diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index b326bebaa..12d97ae96 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -17,13 +17,14 @@ def test_get_should_render_add_service_template(app_, assert 'Add a new service' in response.get_data(as_text=True) -def test_should_add_service_and_redirect_to_next_page(app_, - mock_login, - mock_create_service, - mock_get_services, - api_user_active, - mock_get_user, - mock_get_user_by_email): +def test_should_add_service_and_redirect_to_next_page( + app_, + mock_login, + mock_get_services, + api_user_active, + mock_get_user, + mock_get_user_by_email +): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -31,6 +32,45 @@ def test_should_add_service_and_redirect_to_next_page(app_, url_for('main.add_service'), data={'name': 'testing the post'}) assert response.status_code == 302 + assert response.location == url_for('main.add_from_address', _external=True) + + +def test_should_confirm_add_service( + app_, + mock_login, + mock_get_services, + api_user_active, + mock_get_user, + mock_get_user_by_email +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + with client.session_transaction() as session: + session['service_name'] = 'Renew Your Pet Passport' + response = client.get(url_for('main.add_from_address')) + assert response.status_code == 200 + assert 'Preview your service name' in response.get_data(as_text=True) + assert 'Renew Your Pet Passport' in response.get_data(as_text=True) + assert 'renew.your.pet.passport@notifications.service.gov.uk' in response.get_data(as_text=True) + + +def test_should_add_service_after_confirmation( + app_, + mock_login, + mock_create_service, + mock_get_services, + api_user_active, + mock_get_user, + mock_get_user_by_email +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + with client.session_transaction() as session: + session['service_name'] = 'Renew Your Pet Passport' + response = client.post(url_for('main.add_from_address')) + assert response.status_code == 302 assert response.location == url_for('main.service_dashboard', service_id=101, _external=True) assert mock_create_service.called assert mock_get_services.called