From 438868257fc2b3aff1536ab0f1d807740c0ffd8d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 12 Dec 2016 11:44:11 +0000 Subject: [PATCH] Triage tickets based on time of day and services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TL;DR, as much as possible we should work out how to prioritise tickets and not put that burden on the user. However, there are some cases where we can’t. In business hours all tickets are high priority, ie we will at least acknowledge them within 30 mins. If we are not in business hours then we need to know if a ticket is serious enough to get someone out of bed. Only the user can tell us this, but we can give them some examples to help them decide. In addition, out-of-hours tickets are only a priority if the user has live services. Normally we can determine this and do the priority-setting in the background. If they can’t log in then we can’t determine what services they have. So in this case they will need to use the emergency email address, which only users with live services will have. The logic for this gets fairly complex. It might be to easier to understand what’s going on by walking through the test cases, which are a bit more declarative. N.B. Deskpro’s ‘urgency’ is descending, eg 10 is the most urgent and 1 is the least. --- app/main/forms.py | 11 ++ app/main/views/feedback.py | 136 ++++++++++++- app/templates/views/support/bat-phone.html | 46 +++++ app/templates/views/support/problem.html | 2 +- app/templates/views/support/triage.html | 61 ++++++ tests/app/main/views/test_feedback.py | 214 ++++++++++++++++++++- tests/conftest.py | 2 +- 7 files changed, 463 insertions(+), 9 deletions(-) create mode 100644 app/templates/views/support/bat-phone.html create mode 100644 app/templates/views/support/triage.html diff --git a/app/main/forms.py b/app/main/forms.py index 6a66016a1..daf6f53b1 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -409,6 +409,17 @@ class Support(Form): feedback = TextAreaField('Your message', validators=[DataRequired(message="Can’t be empty")]) +class Triage(Form): + severe = RadioField( + 'Is it an emergency?', + choices=[ + ('yes', 'Yes'), + ('no', 'No'), + ], + validators=[DataRequired()] + ) + + class RequestToGoLiveForm(Form): mou = RadioField( ( diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index 9d1d73304..f344f8484 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -1,8 +1,11 @@ import requests -from flask import render_template, url_for, redirect, flash, current_app, abort +import pytz +from flask import render_template, url_for, redirect, flash, current_app, abort, request from flask_login import current_user +from app import convert_to_boolean, current_service, service_api_client from app.main import main -from app.main.forms import SupportType, Support +from app.main.forms import SupportType, Support, Triage +from datetime import datetime @main.route('/support', methods=['GET', 'POST']) @@ -16,11 +19,46 @@ def support(): return render_template('views/support/index.html', form=form) -@main.route('/support/contact/', methods=['GET', 'POST']) +@main.route('/support/triage', methods=['GET', 'POST']) +def triage(): + form = Triage() + if form.validate_on_submit(): + return redirect(url_for( + '.feedback', + ticket_type='problem', + severe=(form.severe.data == 'yes') + )) + return render_template( + 'views/support/triage.html', + form=form + ) + + +@main.route('/support/submit/', methods=['GET', 'POST']) def feedback(ticket_type): - if ticket_type not in ['problem', 'question']: + + if ticket_type not in ['question', 'problem']: abort(404) + form = Support() + severe = request.args.get('severe') + + urgent = any(( + in_business_hours(), + (ticket_type == 'problem' and convert_to_boolean(severe)) + )) + + anonymous = all(( + (not form.email_address.data), + (not current_user.is_authenticated), + )) + + if needs_triage(ticket_type, severe): + return redirect(url_for('.triage')) + + if needs_escalation(ticket_type, severe): + return redirect(url_for('.bat_phone')) + if form.validate_on_submit(): if current_user.is_authenticated: user_email = current_user.email_address @@ -41,6 +79,7 @@ def feedback(ticket_type): 'subject': 'Notify feedback', 'message': feedback_msg, 'label': ticket_type, + 'urgency': 10 if urgent else 1, } headers = { "X-DeskPRO-API-Key": current_app.config.get('DESKPRO_API_KEY'), @@ -63,5 +102,92 @@ def feedback(ticket_type): return render_template( 'views/support/{}.html'.format(ticket_type), form=form, - ticket_type=ticket_type + ticket_type=ticket_type, ) + + +@main.route('/support/escalate', methods=['GET', 'POST']) +def bat_phone(): + + if current_user.is_authenticated: + return redirect(url_for('main.feedback', ticket_type='problem')) + + return render_template('views/support/bat-phone.html') + + +@main.route('/support/thanks', methods=['GET', 'POST']) +def thanks(): + return render_template( + 'views/support/thanks.html', + ticket_type=request.args.get('ticket_type'), + ) + + +def in_business_hours(): + + now = datetime.now().replace(tzinfo=pytz.timezone('Europe/London')) + + if is_weekend(now) or is_bank_holiday(now): + return False + + opening_time = now.replace(hour=9, minute=30, second=0, tzinfo=pytz.timezone('Europe/London')) + closing_time = now.replace(hour=17, minute=30, second=0, tzinfo=pytz.timezone('Europe/London')) + + return opening_time <= now < closing_time + + +def is_weekend(time): + return time.strftime('%A') in { + 'Saturday', + 'Sunday', + } + + +def is_bank_holiday(time): + return time.strftime('%d/%m/%Y') in { + # taken from + # https://github.com/alphagov/calendars/blob/7f6512b0a95d77aa22accef105860074c19f1ec0/lib/data/bank-holidays.json + "01/01/2016", + "25/03/2016", + "28/03/2016", + "02/05/2016", + "30/05/2016", + "29/08/2016", + "26/12/2016", + "27/12/2016", + "02/01/2017", + "14/04/2017", + "17/04/2017", + "01/05/2017", + "29/05/2017", + "28/08/2017", + "25/12/2017", + "26/12/2017", + } + + +def has_live_services(user_id): + return any( + service['restricted'] is False + for service in service_api_client.get_services({'user_id': user_id})['data'] + ) + + +def needs_triage(ticket_type, severe): + return all(( + ticket_type == 'problem', + severe is None, + ( + not current_user.is_authenticated or has_live_services(current_user.id) + ), + not in_business_hours(), + )) + + +def needs_escalation(ticket_type, severe): + return all(( + ticket_type == 'problem', + convert_to_boolean(severe), + not current_user.is_authenticated, + not in_business_hours(), + )) diff --git a/app/templates/views/support/bat-phone.html b/app/templates/views/support/bat-phone.html new file mode 100644 index 000000000..751c27cae --- /dev/null +++ b/app/templates/views/support/bat-phone.html @@ -0,0 +1,46 @@ +{% extends "withoutnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} + Out of hours emergencies – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

+ Out of hours emergencies +

+
+
+

+ First, check the + system status page. + You don’t need to contact us if + the problem you’re having is listed on that page. +

+

+ Otherwise, contact us using the emergency email address we + gave you or your service manager when we made your service live. +

+

+ We’ll reply within 30 minutes and give you hourly updates + until the problem’s fixed. +

+

+ We don’t offer out of hours support if your service is in + trial mode. +

+

Any other problems

+

+ Fill in this form + and we’ll get back to you by the next working day. +

+

+ Back to support +

+
+
+ + +{% endblock %} diff --git a/app/templates/views/support/problem.html b/app/templates/views/support/problem.html index dac9efe6c..e50c88ae7 100644 --- a/app/templates/views/support/problem.html +++ b/app/templates/views/support/problem.html @@ -1,4 +1,5 @@ {% extends "withoutnav_template.html" %} +{% from "components/checkbox.html" import checkbox %} {% from "components/textbox.html" import textbox %} {% from "components/page-footer.html" import page_footer %} @@ -19,7 +20,6 @@ page to see if there are any known issues with GOV.UK Notify.

-

What went wrong, if anything? What went well? How could we improve this service?

{{ textbox(form.feedback, width='1-1', hint='', rows=10) }} {% if not current_user.is_authenticated %} diff --git a/app/templates/views/support/triage.html b/app/templates/views/support/triage.html new file mode 100644 index 000000000..475a5fea9 --- /dev/null +++ b/app/templates/views/support/triage.html @@ -0,0 +1,61 @@ +{% extends "withoutnav_template.html" %} +{% from "components/radios.html" import radios %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} + Feedback – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +
+
+

+ Report a problem +

+ + {{ radios(form.severe) }} + {{ page_footer('Next') }} + +

+ It’s only an emergency if: +

+
    +
  • + no one in your team can log in +
    • +
  • + you get a ‘technical difficulties’ error message when you try + to upload a file +
    • +
  • + you get a 500 response code when you try to send messages + using the API +
    • +
+

+ It’s not an emergency if: +

+
    +
  • + all your messages stay in ‘sending’ for a few hours +
    • +
  • + you send the wrong message by accident +
    • +
  • + a team member uses GOV.UK Notify to send an + inappropriate message +
    • +
  • + your system is telling the GOV.UK Notify API to send the wrong + message +
    • +
+

+ Back to support +

+
+
+ +{% endblock %} diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index 29e83f215..c260f9a52 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -4,6 +4,17 @@ import pytest from flask import url_for from werkzeug.exceptions import InternalServerError from unittest.mock import Mock, ANY +from freezegun import freeze_time +from tests.conftest import ( + mock_get_services, + mock_get_services_with_no_services, + mock_get_services_with_one_service +) +from app.main.views.feedback import has_live_services, in_business_hours + + +def no_redirect(): + return lambda _external=True: None def test_logged_in_user_redirects_to_choose_service(app_, @@ -26,6 +37,7 @@ def test_get_support_index_page(client): assert resp.status_code == 200 +@freeze_time('2016-12-12 12:00:00.000000') @pytest.mark.parametrize('support_type, expected_h1', [ ('problem', 'Report a problem'), ('question', 'Ask a question or give feedback'), @@ -60,6 +72,7 @@ def test_choose_support_type( assert page.find('form').find('p').text.strip() == expected_contact_details +@freeze_time('2016-12-12 12:00:00.000000') @pytest.mark.parametrize('ticket_type, expected_status_code', [ ('problem', 200), ('question', 200), @@ -70,6 +83,7 @@ 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") @pytest.mark.parametrize('data, expected_message, expected_person_name, expected_email, logged_in', [ ( {'feedback': "blah", 'name': 'Fred'}, @@ -101,7 +115,7 @@ def test_get_feedback_page(client, ticket_type, expected_status_code): ), ]) @pytest.mark.parametrize('ticket_type', ['problem', 'question']) -def test_post_feedback( +def test_post_problem_or_question( client, api_user_active, mock_get_user, @@ -131,15 +145,211 @@ def test_post_feedback( 'department_id': ANY, 'agent_team_id': ANY, 'subject': 'Notify feedback', - 'message': expected_message.format(ticket_type), + 'message': expected_message, 'person_email': expected_email, 'person_name': expected_person_name, 'label': ticket_type, + 'urgency': ANY, }, headers=ANY ) +@pytest.mark.parametrize('ticket_type, severe, is_in_business_hours, expected_urgency', [ + + # business hours, always urgent + ('problem', True, True, 10), + ('question', True, True, 10), + ('problem', False, True, 10), + ('question', False, True, 10), + + # out of hours, non emergency, never urgent + ('problem', False, False, 1), + ('question', False, False, 1), + + # out of hours, emergency problems are urgent + ('problem', True, False, 10), + ('question', True, False, 1), + +]) +def test_urgency( + logged_in_client, + api_user_active, + mock_get_user, + mock_get_services, + mocker, + ticket_type, + severe, + is_in_business_hours, + expected_urgency, +): + mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours) + mock_post = mocker.patch('app.main.views.feedback.requests.post', return_value=Mock(status_code=201)) + response = logged_in_client.post( + url_for('main.feedback', ticket_type=ticket_type, severe=severe), + data={'feedback': "blah"}, + ) + assert response.status_code == 302 + assert mock_post.call_args[1]['data']['urgency'] == expected_urgency + + +ids, params = zip(*[ + ('non-logged in users always have to triage', ( + 'problem', False, False, True, + 302, partial(url_for, 'main.triage') + )), + ('trial services are never high priority', ( + 'problem', False, True, False, + 200, no_redirect() + )), + ('we can triage in hours', ( + 'problem', True, True, True, + 200, no_redirect() + )), + ('only problems are high priority', ( + 'question', False, True, True, + 200, no_redirect() + )), + ('should triage out of hours', ( + 'problem', False, True, True, + 302, partial(url_for, 'main.triage') + )) +]) + + +@pytest.mark.parametrize( + ( + 'ticket_type, is_in_business_hours, logged_in, has_live_services,' + 'expected_status, expected_redirect' + ), + params, ids=ids +) +def test_redirects_to_triage( + client, + api_user_active, + mocker, + mock_get_user, + ticket_type, + is_in_business_hours, + logged_in, + has_live_services, + expected_status, + expected_redirect, +): + mocker.patch('app.main.views.feedback.has_live_services', return_value=has_live_services) + mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours) + if logged_in: + client.login(api_user_active) + + response = client.get(url_for('main.feedback', ticket_type=ticket_type)) + assert response.status_code == expected_status + assert response.location == expected_redirect(_external=True) + + +@pytest.mark.parametrize('get_services_mock, expected_return_value', [ + (mock_get_services, True), + (mock_get_services_with_no_services, False), + (mock_get_services_with_one_service, False), +]) +def test_has_live_services( + mocker, + fake_uuid, + get_services_mock, + expected_return_value +): + get_services_mock(mocker, fake_uuid) + assert has_live_services(12345) == expected_return_value + + +@pytest.mark.parametrize('when, is_in_business_hours', [ + + ('2016-06-06 09:29:59', False), # opening time, summer and winter + ('2016-12-12 09:29:59', False), + ('2016-06-06 09:30:00', True), + ('2016-12-12 09:30:00', True), + + ('2016-12-12 12:00:00', True), # middle of the day + + ('2016-12-12 17:29:59', True), # closing time + ('2016-12-12 17:30:00', False), + + ('2016-12-10 12:00:00', False), # Saturday + ('2016-12-11 12:00:00', False), # Sunday + ('2016-01-01 12:00:00', False), # Bank holiday + +]) +def test_in_business_hours(when, is_in_business_hours): + with freeze_time(when): + assert in_business_hours() == is_in_business_hours + + +@pytest.mark.parametrize('choice, expected_redirect_param', [ + ('yes', True), + ('no', False), +]) +def test_triage_redirects_to_correct_url(client, mocker, choice, expected_redirect_param): + response = client.post(url_for('main.triage'), data={'severe': choice}) + assert response.status_code == 302 + assert response.location == url_for( + 'main.feedback', + ticket_type='problem', + severe=expected_redirect_param, + _external=True, + ) + + +@pytest.mark.parametrize('is_in_business_hours, severe, expected_status_code, expected_redirect', [ + (True, True, 200, no_redirect()), + (True, False, 200, no_redirect()), + (False, False, 200, no_redirect()), + (False, True, 302, partial(url_for, 'main.bat_phone')), +]) +def test_should_be_shown_the_bat_email( + client, + active_user_with_permissions, + mocker, + service_one, + mock_get_services, + is_in_business_hours, + severe, + expected_status_code, + expected_redirect, +): + + mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours) + + feedback_page = url_for('main.feedback', ticket_type='problem', severe=severe) + + response = client.get(feedback_page) + + assert response.status_code == expected_status_code + assert response.location == expected_redirect(_external=True) + + # logged in users should never be redirected to the bat email page + client.login(active_user_with_permissions, mocker, service_one) + logged_in_response = client.get(feedback_page) + assert logged_in_response.status_code == 200 + + +def test_bat_email_page( + client, + active_user_with_permissions, + mocker, + service_one, +): + + bat_phone_page = url_for('main.bat_phone') + + response = client.get(bat_phone_page) + assert response.status_code == 200 + + client.login(active_user_with_permissions, mocker, service_one) + logged_in_response = client.get(bat_phone_page) + assert logged_in_response.status_code == 302 + assert logged_in_response.location == url_for('main.feedback', ticket_type='problem', _external=True) + + +@freeze_time('2016-12-12 12:00:00.000000') @pytest.mark.parametrize('ticket_type', ['problem', 'question']) def test_log_error_on_post(app_, mocker, ticket_type): mock_post = mocker.patch( diff --git a/tests/conftest.py b/tests/conftest.py index c04ea82ac..3cceeff9f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -221,7 +221,7 @@ def mock_get_services_with_one_service(mocker, fake_uuid, user=None): def _get_services(params_dict=None): return {'data': [service_json( - SERVICE_ONE_ID, "service_one", [user.id], 1000, True, False + SERVICE_ONE_ID, "service_one", [user.id], 1000, True, True )]} return mocker.patch(