From e1c4e5199517e5369da89768c220193059ce9798 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 12 Jul 2019 10:35:49 +0100 Subject: [PATCH 1/2] =?UTF-8?q?Make=20=E2=80=98thanks=E2=80=99=20page=20co?= =?UTF-8?q?nsistent=20with=20support=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the support page we now promise that we’ll: - look at tickets within half an hour - reply within one working day The thanks page was always promising a reply within half an hour, during business hours or an emergency. This commit changes the ‘thanks’ page to be consistent with the support page. --- app/main/views/feedback.py | 36 ++++---- app/templates/views/support/thanks.html | 27 +++--- tests/app/main/views/test_feedback.py | 105 +++++++++++++++--------- 3 files changed, 100 insertions(+), 68 deletions(-) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index bfba526bb..88e30973e 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -71,21 +71,11 @@ def feedback(ticket_type): else: severe = None - p1 = False - urgent = False - - if in_business_hours(): - # if we're in the office, it's urgent (aka we'll get back in 30 mins) - urgent = True - elif ticket_type == PROBLEM_TICKET_TYPE and severe: - # out of hours, it's only a p1 and it's only urgent if it's a p1 - urgent = True - p1 = True - - anonymous = ( - (not form.email_address.data) and - (not current_user.is_authenticated) - ) + out_of_hours_emergency = all(( + ticket_type == PROBLEM_TICKET_TYPE, + not in_business_hours(), + severe, + )) if needs_triage(ticket_type, severe): session['feedback_message'] = form.feedback.data @@ -119,11 +109,17 @@ def feedback(ticket_type): subject='Notify feedback', message=feedback_msg, ticket_type=ticket_type, - p1=p1, + p1=out_of_hours_emergency, user_email=user_email, user_name=user_name ) - return redirect(url_for('.thanks', urgent=urgent, anonymous=anonymous)) + return redirect(url_for( + '.thanks', + out_of_hours_emergency=out_of_hours_emergency, + email_address_provided=( + current_user.is_authenticated or bool(form.email_address.data) + ), + )) if not form.feedback.data: form.feedback.data = get_prefilled_message() @@ -148,9 +144,9 @@ def bat_phone(): def thanks(): return render_template( 'views/support/thanks.html', - urgent=convert_to_boolean(request.args.get('urgent')), - anonymous=convert_to_boolean(request.args.get('anonymous')), - logged_in=current_user.is_authenticated, + out_of_hours_emergency=convert_to_boolean(request.args.get('out_of_hours_emergency')), + email_address_provided=convert_to_boolean(request.args.get('email_address_provided')), + out_of_hours=not in_business_hours(), ) diff --git a/app/templates/views/support/thanks.html b/app/templates/views/support/thanks.html index 45c1a5ea5..182b76128 100644 --- a/app/templates/views/support/thanks.html +++ b/app/templates/views/support/thanks.html @@ -9,19 +9,26 @@ {% block maincolumn_content %}

- Thanks for contacting GOV.UK Notify + Thanks for contacting us

- {% if anonymous %} - We’ll look into it + {% if out_of_hours_emergency %} + We’ll reply in the next 30 minutes. {% else %} - We’ll get back to you - {% endif %} - - {% if urgent %} - within 30 minutes. - {% else %} - by the next working day. + {% if email_address_provided %} + {% if out_of_hours %} + We’ll reply within one working day. + {% else %} + We’ll read it in the next 30 minutes and reply within one + working day. + {% endif %} + {% else %} + {% if out_of_hours %} + We’ll read it when we’re back in the office. + {% else %} + We’ll read it in the next 30 minutes. + {% endif %} + {% endif %} {% endif %}

diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index 9fa4afce6..6ff655ca6 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -142,7 +142,12 @@ def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_t ) assert resp.status_code == 302 - assert resp.location == url_for('main.thanks', urgent=True, anonymous=False, _external=True) + assert resp.location == url_for( + 'main.thanks', + out_of_hours_emergency=False, + email_address_provided=True, + _external=True, + ) mock_post.assert_called_with( subject='Notify feedback', message='blah\n', @@ -174,8 +179,8 @@ def test_passes_user_details_through_flow( _expected_status=302, _expected_redirect=url_for( 'main.thanks', - urgent=True, - anonymous=False, + email_address_provided=True, + out_of_hours_emergency=False, _external=True, ), ) @@ -207,7 +212,7 @@ def test_passes_user_details_through_flow( ]) @pytest.mark.parametrize('ticket_type, expected_response, things_expected_in_url, expected_error', [ (PROBLEM_TICKET_TYPE, 200, [], element.Tag), - (QUESTION_TICKET_TYPE, 302, ['thanks', 'anonymous=True', 'urgent=True'], type(None)), + (QUESTION_TICKET_TYPE, 302, ['thanks', 'email_address_provided=False', 'out_of_hours_emergency=False'], type(None)), ]) def test_email_address_required_for_problems( client, @@ -255,21 +260,21 @@ def test_email_address_must_be_valid_if_provided_to_support_form( ) -@pytest.mark.parametrize('ticket_type, severe, is_in_business_hours, is_urgent, is_p1', [ +@pytest.mark.parametrize('ticket_type, severe, is_in_business_hours, is_out_of_hours_emergency', [ - # business hours, always urgent, never p1 - (PROBLEM_TICKET_TYPE, 'yes', True, True, False), - (QUESTION_TICKET_TYPE, 'yes', True, True, False), - (PROBLEM_TICKET_TYPE, 'no', True, True, False), - (QUESTION_TICKET_TYPE, 'no', True, True, False), + # business hours, never an emergency + (PROBLEM_TICKET_TYPE, 'yes', True, False), + (QUESTION_TICKET_TYPE, 'yes', True, False), + (PROBLEM_TICKET_TYPE, 'no', True, False), + (QUESTION_TICKET_TYPE, 'no', True, False), - # out of hours, non emergency, never urgent, not p1 - (PROBLEM_TICKET_TYPE, 'no', False, False, False), - (QUESTION_TICKET_TYPE, 'no', False, False, False), + # out of hours, if the user says it’s not an emergency + (PROBLEM_TICKET_TYPE, 'no', False, False), + (QUESTION_TICKET_TYPE, 'no', False, False), - # out of hours, emergency problems are urgent and p1 - (PROBLEM_TICKET_TYPE, 'yes', False, True, True), - (QUESTION_TICKET_TYPE, 'yes', False, False, False), + # out of hours, only problems can be emergencies + (PROBLEM_TICKET_TYPE, 'yes', False, True), + (QUESTION_TICKET_TYPE, 'yes', False, False), ]) def test_urgency( @@ -278,8 +283,7 @@ def test_urgency( ticket_type, severe, is_in_business_hours, - is_urgent, - is_p1, + is_out_of_hours_emergency, ): mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours) mock_post = mocker.patch('app.main.views.feedback.zendesk_client.create_ticket') @@ -291,12 +295,12 @@ def test_urgency( _expected_status=302, _expected_redirect=url_for( 'main.thanks', - urgent=is_urgent, - anonymous=False, + out_of_hours_emergency=is_out_of_hours_emergency, + email_address_provided=True, _external=True, ), ) - assert mock_post.call_args[1]['p1'] == is_p1 + assert mock_post.call_args[1]['p1'] == is_out_of_hours_emergency ids, params = zip(*[ @@ -532,29 +536,54 @@ def test_bat_email_page( assert logged_in_response.location == url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, _external=True) -@pytest.mark.parametrize('logged_in', [True, False]) -@pytest.mark.parametrize('urgent, anonymous, message', [ +@pytest.mark.parametrize('out_of_hours_emergency, email_address_provided, out_of_hours, message', ( - (True, False, 'We’ll get back to you within 30 minutes.'), - (False, False, 'We’ll get back to you by the next working day.'), + # Out of hours emergencies trump everything else + ( + True, True, True, + 'We’ll reply in the next 30 minutes.', + ), + ( + True, False, False, # Not a real scenario + 'We’ll reply in the next 30 minutes.', + ), - (True, True, 'We’ll look into it within 30 minutes.'), - (False, True, 'We’ll look into it by the next working day.'), + # Anonymous tickets don’t promise a reply + ( + False, False, False, + 'We’ll read it in the next 30 minutes.', + ), + ( + False, False, True, + 'We’ll read it when we’re back in the office.', + ), -]) + # When we look at your ticket depends on whether we’re in normal + # business hours + ( + False, True, False, + 'We’ll read it in the next 30 minutes and reply within one working day.', + ), + ( + False, True, True, + 'We’ll reply within one working day.' + ), + +)) def test_thanks( - client, + client_request, mocker, api_user_active, mock_get_user, - urgent, - anonymous, + out_of_hours_emergency, + email_address_provided, + out_of_hours, message, - logged_in, ): - if logged_in: - client.login(api_user_active) - response = client.get(url_for('main.thanks', urgent=urgent, anonymous=anonymous)) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert ' '.join(page.find('main').find('p').text.split()) == message + mocker.patch('app.main.views.feedback.in_business_hours', return_value=(not out_of_hours)) + page = client_request.get( + 'main.thanks', + out_of_hours_emergency=out_of_hours_emergency, + email_address_provided=email_address_provided, + ) + assert normalize_spaces(page.find('main').find('p').text) == message From a68db38dd52f66eeb65310dda73b987bbc7818e9 Mon Sep 17 00:00:00 2001 From: karlchillmaid Date: Fri, 12 Jul 2019 11:23:54 +0100 Subject: [PATCH 2/2] Update 'your message' --- app/templates/views/support/thanks.html | 6 +++--- tests/app/main/views/test_feedback.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/templates/views/support/thanks.html b/app/templates/views/support/thanks.html index 182b76128..95797197c 100644 --- a/app/templates/views/support/thanks.html +++ b/app/templates/views/support/thanks.html @@ -19,14 +19,14 @@ {% if out_of_hours %} We’ll reply within one working day. {% else %} - We’ll read it in the next 30 minutes and reply within one + We’ll read your message in the next 30 minutes and reply within one working day. {% endif %} {% else %} {% if out_of_hours %} - We’ll read it when we’re back in the office. + We’ll read your message when we’re back in the office. {% else %} - We’ll read it in the next 30 minutes. + We’ll read your message in the next 30 minutes. {% endif %} {% endif %} {% endif %} diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index 6ff655ca6..bcf466689 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -551,18 +551,18 @@ def test_bat_email_page( # Anonymous tickets don’t promise a reply ( False, False, False, - 'We’ll read it in the next 30 minutes.', + 'We’ll read your message in the next 30 minutes.', ), ( False, False, True, - 'We’ll read it when we’re back in the office.', + 'We’ll read your message when we’re back in the office.', ), # When we look at your ticket depends on whether we’re in normal # business hours ( False, True, False, - 'We’ll read it in the next 30 minutes and reply within one working day.', + 'We’ll read your message in the next 30 minutes and reply within one working day.', ), ( False, True, True,