From ce221fc40bcaab7b2d30d24f1e604093a9d661e1 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 26 Feb 2016 10:21:51 +0000 Subject: [PATCH 1/2] =?UTF-8?q?Drop=20=E2=80=98preview=20service=20name?= =?UTF-8?q?=E2=80=99=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This page: - confused users in research - didn’t communicate what it was intended to (eg the generated email address, how your service name would appear in messages) This commit removes the page so that after typing in the service name the user is sent straight to the dashboard for their new service. --- app/main/views/add_service.py | 32 +++-------------- app/templates/views/add-from-address.html | 42 ----------------------- tests/app/main/views/test_add_service.py | 42 +---------------------- 3 files changed, 5 insertions(+), 111 deletions(-) delete mode 100644 app/templates/views/add-from-address.html diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index ad5d8f715..9cee69b8a 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -11,39 +11,15 @@ from app.main.forms import AddServiceForm @login_required def add_service(): form = AddServiceForm(services_dao.find_all_service_names) - services = services_dao.get_services(current_user.id) - if len(services['data']) == 0: - heading = 'Which service do you want to set up notifications for?' - else: - heading = 'Add a new service' + heading = 'Which service do you want to set up notifications for?' if form.validate_on_submit(): session['service_name'] = form.name.data - return redirect(url_for('main.add_from_address')) + 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-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/views/add-from-address.html b/app/templates/views/add-from-address.html deleted file mode 100644 index a67f3c6d3..000000000 --- a/app/templates/views/add-from-address.html +++ /dev/null @@ -1,42 +0,0 @@ -{% 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/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index d8b344ea5..4661aa641 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -14,7 +14,7 @@ def test_get_should_render_add_service_template(app_, client.login(api_user_active) response = client.get(url_for('main.add_service')) assert response.status_code == 200 - assert 'Add a new service' in response.get_data(as_text=True) + assert 'Which service do you want to set up notifications for?' in response.get_data(as_text=True) def test_should_add_service_and_redirect_to_next_page(app_, @@ -31,48 +31,8 @@ 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 def test_should_return_form_errors_when_service_name_is_empty(app_, From f129cc3db2eb14be04ae9482f783f6b3f843fd34 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 26 Feb 2016 10:46:49 +0000 Subject: [PATCH 2/2] Reword the add service page Without the preview service name we probably have to go back to communicating a bit more on the add service page. This commit brings back the two bullet points about where the service name will appear, and tries to tie it into the nice words that Matt Sheret wrote for us. --- app/main/forms.py | 4 ++-- app/templates/views/add-service.html | 21 +++++++++++++++++---- tests/app/main/test_add_service_form.py | 2 +- tests/app/main/views/test_add_service.py | 4 ++-- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index a2885fe8f..7bb000cc7 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -164,13 +164,13 @@ class AddServiceForm(Form): name = StringField( 'Service name', validators=[ - DataRequired(message='Service name can not be empty') + DataRequired(message='Service name can’t be empty') ] ) def validate_name(self, a): if a.data in self._names_func(): - raise ValidationError('Service name already exists') + raise ValidationError('This service name is already in use') class ServiceNameForm(Form): diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index f0b4a5ff7..ec03a91e9 100644 --- a/app/templates/views/add-service.html +++ b/app/templates/views/add-service.html @@ -16,16 +16,29 @@

- Be specific. Remember that there might be other people in your - organisation using GOV.UK Notify. + Be specific to your service. Remember that there might be + other people in your organisation using GOV.UK Notify.

+

+ Users will see this: +

+ + +
{{ textbox(form.name, hint="You can change this later") }} - - {{ page_footer('Continue') }} + {{ page_footer('Add service') }}
diff --git a/tests/app/main/test_add_service_form.py b/tests/app/main/test_add_service_form.py index a89170c1e..ca021b741 100644 --- a/tests/app/main/test_add_service_form.py +++ b/tests/app/main/test_add_service_form.py @@ -9,4 +9,4 @@ def test_form_should_have_errors_when_duplicate_service_is_added(app_): form = AddServiceForm(_get_form_names, formdata=MultiDict([('name', 'some service')])) form.validate() - assert {'name': ['Service name already exists']} == form.errors + assert {'name': ['This service name is already in use']} == form.errors diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 4661aa641..c5a4fc9e7 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -47,7 +47,7 @@ def test_should_return_form_errors_when_service_name_is_empty(app_, client.login(api_user_active) response = client.post(url_for('main.add_service'), data={}) assert response.status_code == 200 - assert 'Service name can not be empty' in response.get_data(as_text=True) + assert 'Service name can’t be empty' in response.get_data(as_text=True) def test_should_return_form_errors_with_duplicate_service_name(app_, @@ -62,5 +62,5 @@ def test_should_return_form_errors_with_duplicate_service_name(app_, response = client.post( url_for('main.add_service'), data={'name': 'service_one'}) assert response.status_code == 200 - assert 'Service name already exists' in response.get_data(as_text=True) + assert 'This service name is already in use' in response.get_data(as_text=True) assert mock_get_services.called