From 6c47375d9f0d73b5752933199bedf34fc3f1bae8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 8 Mar 2018 12:12:18 +0000 Subject: [PATCH 1/3] Make terms page smarter about the agreement People are emailing us asking if their organisation has signed the agreement. In some cases they have, so this is a waste of their and our time. This commit adds a bit of logic to the terms of use page to tell users when their organisation has already signed the agreement. --- app/main/views/index.py | 4 ++- app/templates/views/terms-of-use.html | 15 +++++++-- tests/app/main/views/test_index.py | 46 +++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/app/main/views/index.py b/app/main/views/index.py index 56bdfe03f..ba77fb748 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -9,6 +9,7 @@ from app import convert_to_boolean from app.main import main from app.main.forms import SearchTemplatesForm from app.main.views.sub_navigation_dictionaries import features_nav +from app.utils import GovernmentDomain @main.route('/') @@ -152,7 +153,8 @@ def security(): def terms(): return render_template( 'views/terms-of-use.html', - navigation_links=features_nav() + navigation_links=features_nav(), + agreement_info=GovernmentDomain.from_current_user(), ) diff --git a/app/templates/views/terms-of-use.html b/app/templates/views/terms-of-use.html index 7a1fd95e3..a0cca480e 100644 --- a/app/templates/views/terms-of-use.html +++ b/app/templates/views/terms-of-use.html @@ -15,9 +15,18 @@ Terms of use

Terms of use

-

To go live on GOV.UK Notify, you must accept our data sharing and financial agreement.

-

Contact us to get a copy of the agreement or find out if your organisation has already accepted it.

-

To accept these terms of use, you must be the service manager for your service.

+ +

By using GOV.UK Notify, you agree to follow our terms of use.

+

The service manager for your service has to accept the terms of use when you request to go live on Notify.

+ +

Data sharing and financial agreement

+ + {% if agreement_info.agreement_signed %} +

Your organisation ({{ agreement_info.owner }}) has already accepted the GOV.UK Notify data sharing and financial agreement.

+ {% else %} +

For your service to go live on Notify, your organisation must accept our data sharing and financial agreement.

+

Contact us to get a copy of the agreement or find out if your organisation has already accepted it.

+ {% endif %}

Notify’s side of the agreement

We agree to:

diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 0c5b05ff6..44ae4f145 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -1,6 +1,7 @@ import pytest from bs4 import BeautifulSoup from flask import url_for +from tests.conftest import active_user_with_permissions, normalize_spaces def test_non_logged_in_user_can_see_homepage( @@ -86,3 +87,48 @@ def test_old_static_pages_redirect( 'main.{}'.format(expected_view), _external=True ) + + +def test_terms_is_generic_if_user_is_not_logged_in( + client +): + response = client.get(url_for('main.terms')) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert normalize_spaces(page.select('main p')[2].text) == ( + 'For your service to go live on Notify, your organisation must accept our data sharing and financial agreement.' + ) + assert normalize_spaces(page.select('main p')[3].text) == ( + 'Contact us to get a copy of the agreement or find out if your organisation has already accepted it.' + ) + + +@pytest.mark.parametrize('email_address, expected_first_paragraph', [ + ( + 'test@cabinet-office.gov.uk', + ( + 'Your organisation (Cabinet Office) has already accepted ' + 'the GOV.UK Notify data sharing and financial agreement.' + ), + ), + ( + 'larry@downing-street.gov.uk', + ( + 'For your service to go live on Notify, your organisation ' + 'must accept our data sharing and financial agreement.' + ), + ), +]) +def test_terms_tells_logged_in_users_what_we_know_about_their_agreement( + mocker, + fake_uuid, + client_request, + email_address, + expected_first_paragraph, +): + user = active_user_with_permissions(fake_uuid) + user.email_address = email_address + mocker.patch('app.user_api_client.get_user', return_value=user) + page = client_request.get('main.terms') + assert normalize_spaces(page.select('main p')[2].text) == expected_first_paragraph From 1ce4c874ada7dc860c1e4a83f0da3826ac8d1b9e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 14 Mar 2018 14:42:25 +0000 Subject: [PATCH 2/3] Allow message to be prefilled on feedback page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So that people don’t have to remember what they are supposed to be asking for and go to the effort of typing out the message. --- app/main/views/feedback.py | 3 +++ tests/app/main/views/test_feedback.py | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index 1c9b9106f..48e76c94a 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -134,6 +134,9 @@ def feedback(ticket_type): abort(500, "Feedback submission failed") return redirect(url_for('.thanks', urgent=urgent, anonymous=anonymous)) + if not form.feedback.data: + form.feedback.data = request.args.get('body', '') + return render_template( 'views/support/{}.html'.format(ticket_type), form=form, diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index 56e37fa33..5dc5da3c5 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -85,6 +85,31 @@ def test_get_feedback_page(client, ticket_type, expected_status_code): assert response.status_code == expected_status_code +@freeze_time('2016-12-12 12:00:00.000000') +def test_get_feedback_page_with_prefilled_body( + client_request, + mocker, +): + mock_post = mocker.patch('app.main.views.feedback.deskpro_client.create_ticket') + page = client_request.get( + 'main.feedback', + ticket_type=QUESTION_TICKET_TYPE, + body='Please send cat pictures ', + ) + assert page.select_one('textarea').text == ( + 'Please send cat pictures ' + ) + client_request.post( + 'main.feedback', + ticket_type=QUESTION_TICKET_TYPE, + body='Please send cat pictures ', + _data={'feedback': 'blah', 'name': 'Example', 'email_address': 'test@example.com'} + ) + message = mock_post.call_args[1]['message'] + assert message.endswith('blah') + assert 'cat pictures' not in message + + @freeze_time('2016-12-12 12:00:00.000000') @pytest.mark.parametrize('ticket_type', [PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE]) def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_type): From 59f9fe7df174fe15263c5c4d3cd957dc096bac79 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 12 Mar 2018 15:13:52 +0000 Subject: [PATCH 3/3] Finalise wording Main changes are to better differentiate between the data sharing and financial agreement and the terms of use. --- app/templates/views/terms-of-use.html | 47 +++++++++++++++------------ tests/app/main/views/test_index.py | 22 ++++++++----- 2 files changed, 41 insertions(+), 28 deletions(-) diff --git a/app/templates/views/terms-of-use.html b/app/templates/views/terms-of-use.html index a0cca480e..c6de90ced 100644 --- a/app/templates/views/terms-of-use.html +++ b/app/templates/views/terms-of-use.html @@ -16,31 +16,28 @@ Terms of use

Terms of use

-

By using GOV.UK Notify, you agree to follow our terms of use.

-

The service manager for your service has to accept the terms of use when you request to go live on Notify.

- -

Data sharing and financial agreement

+

+ These terms apply to your service’s use of GOV.UK Notify. You must be the service manager to accept them. +

{% if agreement_info.agreement_signed %}

Your organisation ({{ agreement_info.owner }}) has already accepted the GOV.UK Notify data sharing and financial agreement.

{% else %} -

For your service to go live on Notify, your organisation must accept our data sharing and financial agreement.

-

Contact us to get a copy of the agreement or find out if your organisation has already accepted it.

+

+ Your organisation + {% if agreement_info.owner %} + ({{ agreement_info.owner }}) + must also accept our data sharing and financial agreement. + Contact us to get a copy. + {% else %} + must also accept our data sharing and financial agreement. + Contact us to get a copy. + {% endif %} +

{% endif %} -

Notify’s side of the agreement

-

We agree to:

-
    -
  • send all the messages you pass to us, as long as they meet our guidelines
    • -
  • - show how Notify is performing (through our performance and status pages) -
    • -
  • keep your data secure
    • -
  • give you one month’s notice by email if we change our terms of use or delivery providers
    • -
- -

Your side of the agreement

-

You agree to:

+

When using Notify

+

You must:

  • complete your organisation’s information assurance process (you don’t need to include Notify or our delivery partners, we’ve already done that)
  • tell us immediately if you have any security breaches
    • @@ -52,7 +49,17 @@ Terms of use
  • not send messages containing any personally or commercially sensitive information
  • check that the data you add to Notify is accurate and complies with Data Protection Act principles
-

If you don’t keep to your side of the agreement, we might have to stop sending your messages.

+

If you don’t keep to these terms, we might have to stop sending your messages.

+ +

Notify will:

+
    +
  • send all the messages you pass to us, as long as they meet our guidelines
    • +
  • + show how Notify is performing (through our performance and status pages) +
    • +
  • keep your data secure
    • +
  • give you one month’s notice by email if we change our terms of use or delivery providers
    • +

Leaving Notify

You can leave Notify at any time. Just contact us and we’ll close your account.

diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 44ae4f145..0ade14647 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -96,11 +96,9 @@ def test_terms_is_generic_if_user_is_not_logged_in( assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert normalize_spaces(page.select('main p')[2].text) == ( - 'For your service to go live on Notify, your organisation must accept our data sharing and financial agreement.' - ) - assert normalize_spaces(page.select('main p')[3].text) == ( - 'Contact us to get a copy of the agreement or find out if your organisation has already accepted it.' + assert normalize_spaces(page.select('main p')[1].text) == ( + 'Your organisation must also accept our data sharing and ' + 'financial agreement. Contact us to get a copy.' ) @@ -112,11 +110,19 @@ def test_terms_is_generic_if_user_is_not_logged_in( 'the GOV.UK Notify data sharing and financial agreement.' ), ), + ( + 'test@aylesburytowncouncil.gov.uk', + ( + 'Your organisation (Aylesbury Town Council) must also ' + 'accept our data sharing and financial agreement. Contact ' + 'us to get a copy.' + ), + ), ( 'larry@downing-street.gov.uk', ( - 'For your service to go live on Notify, your organisation ' - 'must accept our data sharing and financial agreement.' + 'Your organisation must also accept our data sharing and ' + 'financial agreement. Contact us to get a copy.' ), ), ]) @@ -131,4 +137,4 @@ def test_terms_tells_logged_in_users_what_we_know_about_their_agreement( user.email_address = email_address mocker.patch('app.user_api_client.get_user', return_value=user) page = client_request.get('main.terms') - assert normalize_spaces(page.select('main p')[2].text) == expected_first_paragraph + assert normalize_spaces(page.select('main p')[1].text) == expected_first_paragraph