Split support into two pages

The kind of communications we’re getting at the moment can broadly be broken down into: - problems - questions and feedback We will need to triage problems differently, because they could potentially be urgent/severe/emergency/P1/whatever language we use. Questions or feedback will never be P1. Two reasons for making the user categorise their tickets themselves: - Outside of hours we can’t get someone out of bed in order to decide if a ticket is a problem or just feedback - We can tailor the subsequent pages to whether it’s a problem or feedback (eg showing a link to the status page if the user is having a problem) This commit let’s users make the choice with a pair of radio buttons. It also cleans up a bunch of the tests and parameterizes them so we’re testing the flow for both ticket types.
2026-07-19 14:03:52 -04:00 · 2016-12-12 11:25:43 +00:00
parent abc9343be4
commit 1df3c11ae9
8 changed files with 158 additions and 90 deletions
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -392,10 +392,21 @@ class CreateKeyForm(Form):
            raise ValidationError('A key with this name already exists')


+class SupportType(Form):
+    support_type = RadioField(
+        'How can we help you?',
+        choices=[
+            ('problem', 'Report a problem'),
+            ('question', 'Ask a question or give feedback'),
+        ],
+        validators=[DataRequired()]
+    )
+
+
 class Feedback(Form):
    name = StringField('Name')
    email_address = StringField('Email address')
-    feedback = TextAreaField(u'', validators=[DataRequired(message="Can’t be empty")])
+    feedback = TextAreaField('Your message', validators=[DataRequired(message="Can’t be empty")])


 class RequestToGoLiveForm(Form):
--- a/app/main/validators.py
+++ b/app/main/validators.py
@@ -35,7 +35,7 @@ class ValidGovEmail(object):
        message = (
            'Enter a central government email address.'
            ' If you think you should have access'
-            ' <a href="{}">contact us</a>').format(url_for('main.feedback'))
+            ' <a href="{}">contact us</a>').format(url_for('main.support'))
        if not is_gov_user(field.data.lower()):
            raise ValidationError(message)

--- a/app/main/views/feedback.py
+++ b/app/main/views/feedback.py
@@ -1,16 +1,24 @@
 import requests
 from flask import render_template, url_for, redirect, flash, current_app, abort
 from app.main import main
-from app.main.forms import Feedback
+from app.main.forms import SupportType, Feedback


@main.route('/support', methods=['GET', 'POST'])
 def support():
-    return render_template('views/support/index.html')
+    form = SupportType()
+    if form.validate_on_submit():
+        return redirect(url_for(
+            '.feedback',
+            ticket_type=form.support_type.data,
+        ))
+    return render_template('views/support/index.html', form=form)


-@main.route('/support/feedback', methods=['GET', 'POST'])
-def feedback():
+@main.route('/support/contact/<ticket_type>', methods=['GET', 'POST'])
+def feedback(ticket_type):
+    if ticket_type not in ['problem', 'question']:
+        abort(404)
    form = Feedback()
    if form.validate_on_submit():
        user_supplied_email = form.email_address.data != ''
@@ -25,7 +33,8 @@ def feedback():
            'department_id': current_app.config.get('DESKPRO_DEPT_ID'),
            'agent_team_id': current_app.config.get('DESKPRO_ASSIGNED_AGENT_TEAM_ID'),
            'subject': 'Notify feedback',
-            'message': feedback_msg
+            'message': feedback_msg,
+            'label': ticket_type,
        }
        headers = {
            "X-DeskPRO-API-Key": current_app.config.get('DESKPRO_API_KEY'),
@@ -43,6 +52,10 @@ def feedback():
                )
            abort(500, "Feedback submission failed")
        flash("Thanks, we’ve received your feedback", 'default_with_tick')
-        return redirect(url_for('.support'))
+        return redirect(url_for('.support', ticket_type=ticket_type))

-    return render_template('views/support/feedback.html', form=form)
+    return render_template(
+        'views/support/{}.html'.format(ticket_type),
+        form=form,
+        ticket_type=ticket_type
+    )
--- a/app/templates/views/support/index.html
+++ b/app/templates/views/support/index.html
@@ -1,9 +1,9 @@
 {% extends "withoutnav_template.html" %}
-{% from "components/textbox.html" import textbox %}
+{% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}

 {% block page_title %}
-  Feedback – GOV.UK Notify
+  Support – GOV.UK Notify
 {% endblock %}

 {% block maincolumn_content %}
@@ -15,12 +15,10 @@
    <div class="grid-row">
      <div class="column-two-thirds">

-    <p>
-      <a href="{{ url_for('.feedback') }}">I have a problem or question</a>
-    </p>
-
-    <div class="grid-row">
-      <div class="column-two-thirds">
+        <form method="post" class="bottom-gutter-2">
+          {{ radios(form.support_type) }}
+          {{ page_footer('Next') }}
+        </form>

        <h2 class="heading-medium">
          Support process
@@ -82,5 +80,4 @@
      </div>
    </div>

-
 {% endblock %}
--- a/app/templates/views/support/feedback.html
+++ b/app/templates/views/support/feedback.html
@@ -3,19 +3,19 @@
 {% from "components/page-footer.html" import page_footer %}

 {% block page_title %}
-  Feedback – GOV.UK Notify
+  Report a problem – GOV.UK Notify
 {% endblock %}

 {% block maincolumn_content %}

    <h1 class="heading-large">
-      Support and feedback
+      Report a problem
    </h1>
    <div class="grid-row">
      <div class="column-two-thirds">
        <div class="panel panel-border-wide">
          <p>
-            Check our <a href="https://status.notifications.service.gov.uk">system status</a> 
+            Check our <a href="https://status.notifications.service.gov.uk">system status</a>
            page to see if there are any known issues with GOV.UK Notify.
          </p>
        </div>
@@ -26,7 +26,11 @@
            <p>Leave your details below if you'd like a response.</p>
            {{ textbox(form.name, width='1-1') }}
            {{ textbox(form.email_address, width='1-1') }}
-            {{ page_footer('Send') }}  
+            {{ page_footer(
+              'Send',
+              secondary_link=url_for('.support'),
+              secondary_link_text='Back to support',
+            ) }}
        </form>
      </div>
    </div>
--- a/app/templates/views/support/question.html
+++ b/app/templates/views/support/question.html
@@ -0,0 +1,31 @@
+{% extends "withoutnav_template.html" %}
+{% from "components/textbox.html" import textbox %}
+{% from "components/page-footer.html" import page_footer %}
+
+{% block page_title %}
+  Ask a question or give feedback – GOV.UK Notify
+{% endblock %}
+
+{% block maincolumn_content %}
+
+    <h1 class="heading-large">
+      Ask a question or give feedback
+    </h1>
+    <div class="grid-row">
+      <div class="column-two-thirds">
+        <form method="post">
+            {{ textbox(form.feedback, width='1-1', hint='', rows=10) }}
+            <h3 class="heading-medium">Do you want a reply?</h3>
+            <p>Leave your details below if you'd like a response.</p>
+            {{ textbox(form.name, width='1-1') }}
+            {{ textbox(form.email_address, width='1-1') }}
+            {{ page_footer(
+              'Send',
+              secondary_link=url_for('.support'),
+              secondary_link_text='Back to support',
+            ) }}
+        </form>
+      </div>
+    </div>
+
+{% endblock %}
--- a/app/templates/views/terms-of-use.html
+++ b/app/templates/views/terms-of-use.html
@@ -18,7 +18,7 @@ Terms of use – GOV.UK Notify
        <h2 class="heading-medium">You must accept the GOV.UK Notify data sharing and financial agreement (Memorandum of Understanding) before we can process data for you.</h2>

        <p>
-          <a href="{{ url_for('main.feedback') }}">Contact the Notify team</a> to get a copy of the agreement or to find out if your organisation has already accepted it.
+          <a href="{{ url_for('main.support') }}">Contact the Notify team</a> to get a copy of the agreement or to find out if your organisation has already accepted it.
        </p>

    {% endcall %}
@@ -83,7 +83,7 @@ Terms of use – GOV.UK Notify

      <p>Cabinet Office act as data processor, as parent organisation of GOV.UK Notify. Your organisation remains the data controller.</p>

-      <p><a href="{{ url_for('main.feedback') }}">Contact us</a> if you want more information about our approach to data protection and information risk management.</p>
+      <p><a href="{{ url_for('main.support') }}">Contact us</a> if you want more information about our approach to data protection and information risk management.</p>

      <h3 class="heading-small" id="we-agree-to-give-you-one-months-notice">
        We agree to give you one months’ notice if we change these terms
@@ -167,7 +167,7 @@ Terms of use – GOV.UK Notify
        Leaving GOV.UK Notify
      </h2>

-      <p>You can remove your service from GOV.UK Notify at any time. <a href="{{ url_for('main.feedback') }}">Contact us</a> and we’ll delete your account.</p>
+      <p>You can remove your service from GOV.UK Notify at any time. <a href="{{ url_for('main.support') }}">Contact us</a> and we’ll delete your account.</p>

      <p>Any data that you have already processed through GOV.UK Notify will be deleted as part of the existing data deletion processes and data retention periods.</p>

--- a/tests/app/main/views/test_feedback.py
+++ b/tests/app/main/views/test_feedback.py
@@ -1,3 +1,5 @@
+from bs4 import BeautifulSoup
+from functools import partial
 import pytest
 from flask import url_for
 from werkzeug.exceptions import InternalServerError
@@ -24,76 +26,86 @@ def test_get_support_index_page(client):
    assert resp.status_code == 200


-def test_get_feedback_page(app_):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            resp = client.get(url_for('main.feedback'))
-            assert resp.status_code == 200
+@pytest.mark.parametrize('support_type, expected_h1', [
+    ('problem', 'Report a problem'),
+    ('question', 'Ask a question or give feedback'),
+])
+def test_choose_support_type(client, support_type, expected_h1):
+    response = client.post(
+        url_for('main.support'),
+        data={'support_type': support_type}, follow_redirects=True
+    )
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert page.h1.string.strip() == expected_h1


-def test_post_feedback_with_name_but_no_email(app_, mocker):
+@pytest.mark.parametrize('ticket_type, expected_status_code', [
+    ('problem', 200),
+    ('question', 200),
+    ('gripe', 404)
+])
+def test_get_feedback_page(client, ticket_type, expected_status_code):
+    response = client.get(url_for('main.feedback', ticket_type=ticket_type))
+    assert response.status_code == expected_status_code
+
+
+@pytest.mark.parametrize('data, expected_message, expected_person_name, expected_email', [
+    (
+        {'feedback': "blah", 'name': 'Fred'},
+        'Environment: http://localhost/\nFred (no email address supplied)\nblah',
+        'Fred',
+        'donotreply@notifications.service.gov.uk',
+    ),
+    (
+        {'feedback': "blah"},
+        'Environment: http://localhost/\n (no email address supplied)\nblah',
+        None,
+        'donotreply@notifications.service.gov.uk',
+    ),
+    (
+        {'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'},
+        'Environment: http://localhost/\n\nblah',
+        'Steve Irwin',
+        'rip@gmail.com',
+    ),
+])
+@pytest.mark.parametrize('ticket_type', ['problem', 'question'])
+def test_post_feedback_with_name_but_no_email(
+    client,
+    mocker,
+    ticket_type,
+    data,
+    expected_message,
+    expected_person_name,
+    expected_email,
+):
    mock_post = mocker.patch(
        'app.main.views.feedback.requests.post',
-        return_value=Mock(status_code=201))
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            resp = client.post(url_for('main.feedback'), data={'feedback': "blah", 'name': 'Fred'})
-            assert resp.status_code == 302
-            mock_post.assert_called_with(
-                ANY,
-                data={
-                    'department_id': ANY,
-                    'agent_team_id': ANY,
-                    'subject': 'Notify feedback',
-                    'message': 'Environment: http://localhost/\nFred (no email address supplied)\nblah',
-                    'person_email': app_.config['DESKPRO_PERSON_EMAIL'],
-                    'person_name': 'Fred'},
-                headers=ANY)
+        return_value=Mock(status_code=201)
+    )
+    resp = client.post(
+        url_for('main.feedback', ticket_type=ticket_type),
+        data=data,
+    )
+    assert resp.status_code == 302
+    mock_post.assert_called_with(
+        ANY,
+        data={
+            'department_id': ANY,
+            'agent_team_id': ANY,
+            'subject': 'Notify feedback',
+            'message': expected_message.format(ticket_type),
+            'person_email': expected_email,
+            'person_name': expected_person_name,
+            'label': ticket_type,
+        },
+        headers=ANY
+    )


-def test_post_feedback_with_no_name_or_email(app_, mocker):
-    mock_post = mocker.patch(
-        'app.main.views.feedback.requests.post',
-        return_value=Mock(status_code=201))
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            resp = client.post(url_for('main.feedback'), data={'feedback': "blah"})
-            assert resp.status_code == 302
-            mock_post.assert_called_with(
-                ANY,
-                data={
-                    'department_id': ANY,
-                    'agent_team_id': ANY,
-                    'subject': 'Notify feedback',
-                    'message': 'Environment: http://localhost/\n (no email address supplied)\nblah',
-                    'person_email': app_.config['DESKPRO_PERSON_EMAIL'],
-                    'person_name': None},
-                headers=ANY)
-
-
-def test_post_feedback_with_name_email(app_, mocker):
-    mock_post = mocker.patch(
-        'app.main.views.feedback.requests.post',
-        return_value=Mock(status_code=201))
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            resp = client.post(
-                url_for('main.feedback'),
-                data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'})
-            assert resp.status_code == 302
-            mock_post.assert_called_with(
-                ANY,
-                data={
-                    'subject': 'Notify feedback',
-                    'department_id': ANY,
-                    'agent_team_id': ANY,
-                    'message': 'Environment: http://localhost/\n\nblah',
-                    'person_name': 'Steve Irwin',
-                    'person_email': 'rip@gmail.com'},
-                headers=ANY)
-
-
-def test_log_error_on_post(app_, mocker):
+@pytest.mark.parametrize('ticket_type', ['problem', 'question'])
+def test_log_error_on_post(app_, mocker, ticket_type):
    mock_post = mocker.patch(
        'app.main.views.feedback.requests.post',
        return_value=Mock(
@@ -106,7 +118,7 @@ def test_log_error_on_post(app_, mocker):
        with app_.test_client() as client:
            with pytest.raises(InternalServerError):
                resp = client.post(
-                    url_for('main.feedback'),
+                    url_for('main.feedback', ticket_type=ticket_type),
                    data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'})
            assert mock_post.called
            mock_logger.assert_called_with(