separate urgent and p1 concepts in feedback.

p1 == "should notify team be alerted of this (via pagerduty)"
urgent == "should the user be told we'll look at it"

* If it's in office hours, it's always urgent. It's never a P1 because
  we'll notice it anyway
* If it's outside of office hours, it's urgent and P1 if it's severe,
  otherwise it's neither
This commit is contained in:
Leo Hemsted
2018-04-27 14:20:15 +01:00
parent be038e345d
commit 023c4121b3
3 changed files with 35 additions and 50 deletions
+18 -16
View File
@@ -3,7 +3,6 @@ from datetime import datetime
import pytz import pytz
from flask import abort, redirect, render_template, request, session, url_for from flask import abort, redirect, render_template, request, session, url_for
from flask_login import current_user from flask_login import current_user
from notifications_utils.clients.zendesk.zendesk_client import ZendeskError
from app import ( from app import (
convert_to_boolean, convert_to_boolean,
@@ -77,10 +76,16 @@ def feedback(ticket_type):
else: else:
severe = None severe = None
urgent = ( p1 = False
in_business_hours() or urgent = False
(ticket_type == PROBLEM_TICKET_TYPE and severe)
) 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 = ( anonymous = (
(not form.email_address.data) and (not form.email_address.data) and
@@ -116,17 +121,14 @@ def feedback(ticket_type):
form.feedback.data form.feedback.data
) )
try: zendesk_client.create_ticket(
zendesk_client.create_ticket( subject='Notify feedback',
subject='Notify feedback {}'.format(user_name), message=feedback_msg,
message=feedback_msg, ticket_type=ticket_type,
ticket_type=ticket_type, p1=p1,
p1=urgent, user_email=user_email,
user_email=user_email, user_name=user_name
user_name=user_name )
)
except ZendeskError:
abort(500, "Feedback submission failed")
return redirect(url_for('.thanks', urgent=urgent, anonymous=anonymous)) return redirect(url_for('.thanks', urgent=urgent, anonymous=anonymous))
if not form.feedback.data: if not form.feedback.data:
+1 -1
View File
@@ -336,7 +336,7 @@
{% endif %} {% endif %}
<li class="bottom-gutter"> <li class="bottom-gutter">
<a href="{{ url_for('.service_switch_email_auth', service_id=current_service.id) }}" class="button"> <a href="{{ url_for('.service_switch_email_auth', service_id=current_service.id) }}" class="button">
{{ 'Stop editing user auth' if 'email_auth' in current_service.permissions else 'Allow editing user auth' }} {{ 'Stop user auth type editing' if 'email_auth' in current_service.permissions else 'Allow user auth type editing' }}
</a> </a>
</li> </li>
{% if current_service.active %} {% if current_service.active %}
+16 -33
View File
@@ -5,8 +5,6 @@ import pytest
from bs4 import BeautifulSoup, element from bs4 import BeautifulSoup, element
from flask import url_for from flask import url_for
from freezegun import freeze_time from freezegun import freeze_time
from notifications_utils.clients.zendesk.zendesk_client import ZendeskError
from werkzeug.exceptions import InternalServerError
from app.main.views.feedback import ( from app.main.views.feedback import (
PROBLEM_TICKET_TYPE, PROBLEM_TICKET_TYPE,
@@ -153,7 +151,7 @@ def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_t
assert resp.status_code == 302 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', urgent=True, anonymous=False, _external=True)
mock_post.assert_called_with( mock_post.assert_called_with(
subject='Notify feedback {}'.format(data['name']), subject='Notify feedback',
message='Environment: http://localhost/\n\nblah', message='Environment: http://localhost/\n\nblah',
user_email='rip@gmail.com', user_email='rip@gmail.com',
user_name='Steve Irwin', user_name='Steve Irwin',
@@ -184,7 +182,7 @@ def test_passes_user_details_through_flow(
assert resp.status_code == 302 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', urgent=True, anonymous=False, _external=True)
mock_post.assert_called_with( mock_post.assert_called_with(
subject='Notify feedback Test User', subject='Notify feedback',
message=ANY, message=ANY,
user_email='test@user.gov.uk', user_email='test@user.gov.uk',
user_name='Test User', user_name='Test User',
@@ -234,21 +232,21 @@ def test_email_address_required_for_problems(
assert isinstance(page.find('span', {'class': 'error-message'}), expected_error) assert isinstance(page.find('span', {'class': 'error-message'}), expected_error)
@pytest.mark.parametrize('ticket_type, severe, is_in_business_hours, is_p1', [ @pytest.mark.parametrize('ticket_type, severe, is_in_business_hours, is_urgent, is_p1', [
# business hours, always urgent # business hours, always urgent, never p1
(PROBLEM_TICKET_TYPE, 'yes', True, True), (PROBLEM_TICKET_TYPE, 'yes', True, True, False),
(QUESTION_TICKET_TYPE, 'yes', True, True), (QUESTION_TICKET_TYPE, 'yes', True, True, False),
(PROBLEM_TICKET_TYPE, 'no', True, True), (PROBLEM_TICKET_TYPE, 'no', True, True, False),
(QUESTION_TICKET_TYPE, 'no', True, True), (QUESTION_TICKET_TYPE, 'no', True, True, False),
# out of hours, non emergency, never urgent # out of hours, non emergency, never urgent, not p1
(PROBLEM_TICKET_TYPE, 'no', False, False), (PROBLEM_TICKET_TYPE, 'no', False, False, False),
(QUESTION_TICKET_TYPE, 'no', False, False), (QUESTION_TICKET_TYPE, 'no', False, False, False),
# out of hours, emergency problems are urgent # out of hours, emergency problems are urgent and p1
(PROBLEM_TICKET_TYPE, 'yes', False, True), (PROBLEM_TICKET_TYPE, 'yes', False, True, True),
(QUESTION_TICKET_TYPE, 'yes', False, False), (QUESTION_TICKET_TYPE, 'yes', False, False, False),
]) ])
def test_urgency( def test_urgency(
@@ -257,6 +255,7 @@ def test_urgency(
ticket_type, ticket_type,
severe, severe,
is_in_business_hours, is_in_business_hours,
is_urgent,
is_p1, is_p1,
): ):
mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours) mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours)
@@ -266,7 +265,7 @@ def test_urgency(
data={'feedback': 'blah', 'email_address': 'test@example.com'}, data={'feedback': 'blah', 'email_address': 'test@example.com'},
) )
assert response.status_code == 302 assert response.status_code == 302
assert response.location == url_for('main.thanks', urgent=is_p1, anonymous=False, _external=True) assert response.location == url_for('main.thanks', urgent=is_urgent, anonymous=False, _external=True)
assert mock_post.call_args[1]['p1'] == is_p1 assert mock_post.call_args[1]['p1'] == is_p1
@@ -501,22 +500,6 @@ def test_bat_email_page(
assert logged_in_response.location == url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, _external=True) assert logged_in_response.location == url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, _external=True)
@freeze_time('2016-12-12 12:00:00.000000')
@pytest.mark.parametrize('ticket_type', [PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE])
def test_log_error_on_post(app_, mocker, ticket_type):
mock_post = mocker.patch(
'app.main.views.feedback.zendesk_client.create_ticket',
side_effect=ZendeskError(None)
)
with app_.test_request_context():
with app_.test_client() as client:
with pytest.raises(InternalServerError):
client.post(
url_for('main.feedback', ticket_type=ticket_type),
data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'})
assert mock_post.called
@pytest.mark.parametrize('logged_in', [True, False]) @pytest.mark.parametrize('logged_in', [True, False])
@pytest.mark.parametrize('urgent, anonymous, message', [ @pytest.mark.parametrize('urgent, anonymous, message', [