Merge pull request #3051 from alphagov/revise-thanks-wording

Make ‘thanks’ page consistent with support page

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(),
     )
 
 

app/templates/views/support/thanks.html

@@ -9,19 +9,26 @@
 {% block maincolumn_content %}
 
     <h1 class="heading-large">
-      Thanks for contacting GOV.UK Notify
+      Thanks for contacting us
     </h1>
     <p>
-      {% 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 your message in the next 30 minutes and reply within one
+            working day.
+          {% endif %}
+        {% else %}
+          {% if out_of_hours %}
+            We’ll read your message when we’re back in the office.
+          {% else %}
+            We’ll read your message in the next 30 minutes.
+          {% endif %}
+        {% endif %}
       {% endif %}
     </p>
     <p>

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 your message in the next 30 minutes.',
+    ),
+    (
+        False, False, True,
+        '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 your message 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